Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

opt(torii): avoid calculating poseidon hash where possible #2394

Merged
merged 4 commits into from
Sep 8, 2024

Conversation

lambda-0x
Copy link
Collaborator

@lambda-0x lambda-0x commented Sep 7, 2024

Summary by CodeRabbit

  • New Features

    • Added a new method for setting models, enhancing player-related functionalities.
    • Introduced a new entity_id field for better entity identification across various data structures.
    • Implemented a script to automate sending multiple transactions for testing purposes.
  • Bug Fixes

    • Improved handling of keys and model IDs in event processing for better clarity and functionality.
  • Documentation

    • Updated configuration files and manifest entries to reflect new features and structures.
  • Refactor

    • Enhanced data models and event processing logic to improve clarity and maintainability.

Copy link

coderabbitai bot commented Sep 7, 2024

Walkthrough

Ohayo, sensei! The changes involve a series of modifications across multiple Rust files in the torii project, primarily aimed at enhancing event processing, data handling, and configuration management. Key updates include the introduction of new fields such as entity_id and model_id in various structures, improvements to function signatures for better clarity and functionality, and the addition of utility functions for managing event queries and database interactions. The adjustments also encompass changes to JSON manifest files and configuration settings, reflecting an expansion of capabilities and a more structured approach to handling events and entities within the application.

Changes

Files Change Summary
bin/sozo/tests/test_data/policies.json Added a new method "set_models" to the JSON structure for enhanced functionality related to setting models.
crates/dojo-core/src/world/world_contract.cairo Introduced a new field entity_id in the StoreSetRecord struct, enhancing data storage capabilities.
crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/abis/dojo-world.json, examples/spawn-and-move/manifests/dev/base/abis/dojo-world.json Added new field "entity_id" of type core::felt252 to the JSON schema for better entity identification.
crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/dojo-world.toml, examples/spawn-and-move/manifests/dev/base/dojo-world.toml Updated class_hash and original_class_hash values to new hashes, indicating changes in class representation.
crates/dojo-world/src/contracts/abi/world.rs Added entity_id field to the existing structure in the abigen! macro, enhancing data model capabilities.
crates/sozo/ops/src/tests/model.rs Updated a hexadecimal string in the test_model_ops function to reflect changes in expected input/output for testing.
crates/torii/core/src/processors/mod.rs Modified NUM_KEYS_INDEX constant from 1 to 2, altering key indexing in the model.
crates/torii/core/src/processors/store_set_record.rs Renamed selector to model_id, added keys_str, and modified process_event function signature to include new parameters for improved database interaction.
examples/spawn-and-move/src/actions.cairo Introduced a new function set_models in the IActions trait and its implementation, allowing dynamic setting of player models based on specified parameters.
scripts/spam_txs.sh Added a new script to automate sending multiple transactions to the spawn-and-move example, enhancing testing capabilities.
xtask/generate-test-db/src/main.rs Added a new field dev to the KatanaRunnerConfig structure, indicating support for development mode.

Sequence Diagram(s)

sequenceDiagram
    participant EventProcessor
    participant Database
    participant SQL

    EventProcessor->>Database: process_event(event_id, event, entity_id, model_id, keys_str)
    Database->>SQL: set_entity(entity_id, model_id, keys_str)
    SQL-->>Database: Acknowledge entity set
    Database-->>EventProcessor: Acknowledge event processed
Loading

Possibly related PRs


Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 23df7be and ff6425a.

Files ignored due to path filters (2)
  • spawn-and-move-db.tar.gz is excluded by !**/*.gz
  • types-test-db.tar.gz is excluded by !**/*.gz
Files selected for processing (24)
  • bin/sozo/tests/test_data/policies.json (1 hunks)
  • crates/dojo-core/src/world/world_contract.cairo (2 hunks)
  • crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/abis/dojo-world.json (1 hunks)
  • crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/dojo-world.toml (1 hunks)
  • crates/dojo-world/src/contracts/abi/world.rs (1 hunks)
  • crates/sozo/ops/src/tests/model.rs (1 hunks)
  • crates/torii/core/src/processors/mod.rs (1 hunks)
  • crates/torii/core/src/processors/store_set_record.rs (4 hunks)
  • examples/spawn-and-move/dojo_dev.toml (1 hunks)
  • examples/spawn-and-move/manifests/dev/base/abis/contracts/dojo_examples-actions-40b6994c.json (1 hunks)
  • examples/spawn-and-move/manifests/dev/base/abis/dojo-world.json (1 hunks)
  • examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-actions-40b6994c.toml (1 hunks)
  • examples/spawn-and-move/manifests/dev/base/dojo-world.toml (1 hunks)
  • examples/spawn-and-move/manifests/dev/deployment/abis/contracts/dojo_examples-actions-40b6994c.json (1 hunks)
  • examples/spawn-and-move/manifests/dev/deployment/abis/dojo-world.json (1 hunks)
  • examples/spawn-and-move/manifests/dev/deployment/manifest.json (9 hunks)
  • examples/spawn-and-move/manifests/dev/deployment/manifest.toml (5 hunks)
  • examples/spawn-and-move/manifests/release/base/abis/contracts/dojo_examples-actions-40b6994c.json (1 hunks)
  • examples/spawn-and-move/manifests/release/base/abis/dojo-world.json (1 hunks)
  • examples/spawn-and-move/manifests/release/base/contracts/dojo_examples-actions-40b6994c.toml (1 hunks)
  • examples/spawn-and-move/manifests/release/base/dojo-world.toml (1 hunks)
  • examples/spawn-and-move/src/actions.cairo (2 hunks)
  • scripts/spam_txs.sh (1 hunks)
  • xtask/generate-test-db/src/main.rs (2 hunks)
Files skipped from review due to trivial changes (1)
  • examples/spawn-and-move/dojo_dev.toml
Files skipped from review as they are similar to previous changes (1)
  • crates/torii/core/src/processors/store_set_record.rs
Additional comments not posted (29)
crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/dojo-world.toml (1)

2-3: Ohayo, sensei! Hash values updated in dojo-world.toml.

The class_hash and original_class_hash have been updated to new values. Please ensure these hashes correctly correspond to the updated class definitions or metadata. If these hashes are generated or derived from a specific process, verify that the process has been executed correctly to avoid any discrepancies.

examples/spawn-and-move/manifests/dev/base/dojo-world.toml (1)

2-3: Ohayo, sensei! Consistency check for hash updates in dojo-world.toml.

The class_hash and original_class_hash have been updated here as well. It's essential to ensure that these updates are consistent across all configuration files and reflect the intended changes to the class definitions. If there's a central documentation or a source of truth for these hashes, please verify against it to maintain integrity throughout the system.

examples/spawn-and-move/manifests/release/base/dojo-world.toml (1)

2-3: Ohayo, sensei! Ensuring hash consistency in release environment dojo-world.toml.

Just like in the development environment, the class_hash and original_class_hash have been updated here. It's crucial to ensure that these hashes are consistent across both development and release environments to prevent any runtime discrepancies or errors. Please confirm that these hashes are accurately reflecting the changes intended for the release environment.

examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-actions-40b6994c.toml (2)

2-3: Ohayo, sensei! Verify the new hash values.

Please ensure that the updated class_hash and original_class_hash are correct and reflect the intended changes in the contract's implementation.


11-11: New functionality added: set_models.

The addition of "set_models" to the systems array introduces new capabilities. Please ensure it integrates well with existing systems and the intended functionality is achieved.

examples/spawn-and-move/manifests/release/base/contracts/dojo_examples-actions-40b6994c.toml (2)

2-3: Ohayo, sensei! Verify the new hash values for release.

Please ensure that the updated class_hash and original_class_hash in the release configuration are correct and consistent with the dev configuration.


11-11: Consistency maintained with new functionality: set_models.

The addition of "set_models" to the systems array in the release configuration should mirror the dev configuration. Please verify its functionality and integration.

crates/torii/core/src/processors/mod.rs (1)

23-23: Verify the impact of changing NUM_KEYS_INDEX.

The change from 1 to 2 in NUM_KEYS_INDEX might have significant implications on how keys are processed within the module. It's crucial to ensure that this adjustment aligns with the overall data structure and usage throughout the codebase.

xtask/generate-test-db/src/main.rs (1)

17-17: Approve the addition of the dev field but verify its impact.

The inclusion of the dev field in KatanaRunnerConfig is a thoughtful addition for development mode. It's essential to verify that this new configuration does not interfere with production settings or alter the expected behavior of migrations in different environments.

Also applies to: 49-49

Verification successful

Ohayo, sensei! After diving into the codebase, it's clear that the dev field is being used effectively to manage development-specific behavior and resources. The conditional checks and structured handling of "dev" directories and configurations confirm that this addition is well-implemented and does not interfere with production settings.

The dev field is verified and appropriately used. The codebase shows a clear separation of development and production logic, ensuring that the new configuration does not alter the expected behavior across different environments.

  • Conditional checks using if self.dev are present, ensuring environment-specific behavior.
  • Development-specific resources are organized under "dev" directories and configurations.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for conditional usage of the `dev` field to ensure it's appropriately handled across environments.

# Test: Search for conditional checks involving the `dev` field. Expect: Proper handling that differentiates development from production.
rg --type rust 'if.*dev'

Length of output: 3586

bin/sozo/tests/test_data/policies.json (1)

30-33: Approve the addition of the set_models method but verify its integration.

The introduction of the set_models method is a welcome enhancement to the system's capabilities. It's crucial to ensure that this new method is seamlessly integrated and does not conflict with existing functionality.

Verification successful

Ohayo, sensei! It seems like the set_models method is only mentioned in the policies.json file, and we need to ensure it's properly integrated into the rest of the application. Let's broaden our search to see if it's referenced or used elsewhere in the codebase.


Ohayo, sensei! The set_models method is well-integrated into the application, appearing in scripts, source files, and configuration manifests. Everything seems to be in harmony with this new addition.

The set_models method is properly integrated into the application. The method is referenced across various parts of the codebase, indicating its correct implementation and usage.

  • Integration Points:
    • scripts/spam_txs.sh: Used in execution commands.
    • examples/spawn-and-move/src/actions.cairo: Defined and implemented in source code.
    • Various manifest and configuration files: Recognized as a valid method.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the integration of the `set_models` method within the application.

# Test: Search for the usage of `set_models` in the application code. Expect: Proper handling and integration.
rg --type json '"method": "set_models"'

Length of output: 107


Script:

#!/bin/bash
# Description: Search for the usage of `set_models` across the entire codebase to verify its integration.

# Test: Look for references to `set_models` in source code files. Expect: Proper handling and integration.
rg 'set_models'

Length of output: 1319

crates/sozo/ops/src/tests/model.rs (1)

61-62: Updated Hexadecimal String in Test

The hexadecimal string for Felt::from_hex has been updated. This change is crucial as it directly affects the test's validation logic. Ensure that the new value "0x68e3a53988f20d84c6652f25d6add070633a5d05f8c4ac68285cacb228afa14" is the correct expected output for the test scenario being validated.

examples/spawn-and-move/manifests/dev/base/abis/contracts/dojo_examples-actions-40b6994c.json (1)

299-314: New Function Declaration: set_models

A new function set_models has been added to the contract's interface. This function allows setting models based on a seed and a number of models, which could be crucial for dynamic configurations or simulations within the contract. Ensure that the parameters seed of type core::felt252 and n_models of type core::integer::u32 are correctly used in the contract's logic to maintain state consistency and correctness.

examples/spawn-and-move/manifests/dev/deployment/abis/contracts/dojo_examples-actions-40b6994c.json (1)

299-314: New Function Declaration in Deployment Manifest: set_models

Identical to the base manifest, the deployment manifest also introduces the set_models function. This addition is crucial for ensuring that the contract can dynamically handle model settings based on operational parameters. Double-check that the deployment settings for seed and n_models align with the contract's operational requirements and security standards.

crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/abis/dojo-world.json (1)

942-946: Ohayo, sensei! Addition of entity_id looks good.

The new field entity_id with type core::felt252 has been added correctly to the JSON manifest. This should enhance the ability to uniquely identify entities, which is crucial for tracking and referencing within the dojo-world contract.

examples/spawn-and-move/manifests/dev/base/abis/dojo-world.json (1)

942-946: Ohayo, sensei! Consistent update with entity_id addition.

Just like in the previous file, the addition of entity_id with type core::felt252 is consistent and well-integrated into the JSON schema. This ensures uniformity across different versions or environments of the dojo-world contract.

examples/spawn-and-move/manifests/dev/deployment/abis/dojo-world.json (1)

942-946: Ohayo, sensei! Good consistency in deployment-specific configurations.

The addition of entity_id with type core::felt252 in this deployment-specific JSON manifest maintains consistency and integrity across different environments. This is crucial for the robustness and reliability of the dojo-world contract's deployment.

examples/spawn-and-move/manifests/dev/deployment/manifest.json (8)

4-5: Updated class_hash and original_class_hash fields.

The class_hash and original_class_hash have been updated to new values. This change suggests that the contract has been recompiled or redeployed. Ensure that all dependencies and references to these hashes are updated accordingly to avoid any integration issues.


947-951: Addition of entity_id in the ABI event StoreSetRecord.

The new field entity_id has been added to the event StoreSetRecord. This field is likely intended to uniquely identify entities within the contract's context. Ensure that the logic handling these events is updated to accommodate this new field.


1237-1237: Updated address field for the world contract.

The address field for the world contract has been updated. This change might affect how the contract is accessed or interacted with. Verify that all external references and interactions with this contract address are updated to reflect this change.


1258-1260: Updated address and class_hash fields for DojoContract.

The address and class_hash for the DojoContract have been updated. This indicates a redeployment or recompilation of the contract. It's crucial to ensure that all interactions with this contract are checked for compatibility with the new deployment.


1561-1575: Addition of set_models function in the ABI of DojoContract.

A new function set_models has been added to the ABI, accepting parameters seed and n_models. This function likely allows dynamic configuration or management of models within the contract. Review the implementation details of this function to ensure it aligns with the intended functionality and security practices.


1689-1689: Updated address for another instance of DojoContract.

Another DojoContract instance has its address updated. Similar to previous updates, confirm that all necessary references and interactions are adjusted to accommodate this new address.


1928-1928: Updated address for a DojoContract.

Yet another update to the address of a DojoContract. This frequent updating of addresses suggests significant changes or iterations in the deployment strategy. Ensure consistency and correctness in all related configurations and interactions.


2149-2149: Updated address for DojoContract.

The address for this DojoContract has also been updated. As with other updates, verify that this change is reflected wherever this contract is referenced or interacted with.

examples/spawn-and-move/manifests/release/base/abis/contracts/dojo_examples-actions-40b6994c.json (1)

299-314: Approved: Addition of set_models function.

The new function set_models has been correctly defined with appropriate parameters and state mutability. This addition aligns well with the PR's objectives of enhancing functionality and efficiency.

examples/spawn-and-move/manifests/dev/deployment/manifest.toml (4)

26-36: Approved: Updates to DojoContract configurations.

The address, class_hash, and original_class_hash for DojoContract have been updated along with the addition of the set_models system. These changes are crucial for the new functionalities introduced in the PR.


Line range hint 50-64: Approved: Additional DojoContract updates.

Further updates to DojoContract addresses and hashes have been made. These changes are essential for maintaining the integrity and functionality of the contract deployments.


78-78: Approved: Final DojoContract address update.

The last DojoContract address update has been correctly implemented, ensuring the contract's deployment aligns with the new system configurations.


3-7: Verify: Significant changes to WorldContract configurations.

The class_hash, original_class_hash, address, and transaction_hash for WorldContract have been updated. Please ensure these changes are consistent with the intended contract upgrades and redeployments.

Run the following script to verify the contract deployment details:

Verification successful

Ohayo, sensei! The digital scrolls have spoken, and the changes to the WorldContract configurations are consistent with the intended updates. The class_hash, original_class_hash, address, and transaction_hash are correctly reflected in the manifest.toml file. Everything seems to be in harmony.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the contract deployment details for `WorldContract`.

# Test: Search for the contract deployment details. Expect: Only occurrences of the new hashes and addresses.
rg --type toml -A 5 $'WorldContract'

Length of output: 826


Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

codecov bot commented Sep 7, 2024

Codecov Report

Attention: Patch coverage is 81.57895% with 7 lines in your changes missing coverage. Please review.

Project coverage is 68.43%. Comparing base (f8784c4) to head (ff6425a).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...s/torii/core/src/processors/store_update_record.rs 0.00% 5 Missing ⚠️
xtask/generate-test-db/src/main.rs 0.00% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2394   +/-   ##
=======================================
  Coverage   68.42%   68.43%           
=======================================
  Files         358      358           
  Lines       47328    47349   +21     
=======================================
+ Hits        32386    32404   +18     
- Misses      14942    14945    +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

scripts/spam_txs.sh Show resolved Hide resolved
examples/spawn-and-move/src/actions.cairo Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants