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

move to edition 2023_11 #2179

Merged
merged 2 commits into from
Jul 17, 2024
Merged

Conversation

remybar
Copy link
Contributor

@remybar remybar commented Jul 16, 2024

Issue #2092

Summary by CodeRabbit

  • New Features

    • Enhanced access control by making struct fields and enums public, improving flexibility for developers.
  • Refactor

    • Updated module paths for better organization and clarity.
  • Chores

    • Updated edition field in configuration files to latest version.
    • Revised class hashes and contract details in manifest files for consistency.

These changes collectively improve code maintainability, enhance developer experience, and ensure up-to-date configurations.

Copy link

coderabbitai bot commented Jul 16, 2024

Walkthrough

The overall changes span multiple files, focusing on updates to imports, method implementations, visibility of struct fields, and adjustments to contract metadata and deployment details. Key modifications include method signature changes in world.cairo, updates to the Scarb.toml files for editions, visibility adjustments in models.cairo, and various contract hash updates in multiple manifest files.

Changes

Files Change Summary
crates/dojo-core/src/world.cairo Updated import statements and modified method signatures for metadata and set_metadata.
crates/torii/types-test/Scarb.toml
examples/spawn-and-move/Scarb.toml
Added/updated edition field to "2023_11".
crates/torii/types-test/src/contracts.cairo Changed namespace path for the pedersen module to core::pedersen.
crates/torii/types-test/src/models.cairo Changed struct fields and enums visibility to public.
examples/spawn-and-move/manifests/dev/base/dojo-world.toml
.../release/base/...
Updated class_hash and original_class_hash values.
examples/spawn-and-move/manifests/dev/deployment/manifest.json
.../manifest.toml
Updated class hashes, addresses, and transaction hashes for contract definitions.

Poem

In code we trust, the changes flow,
Imports shuffled, methods grow.
Structs unveiled for all to see,
Hashes updated, manifest's glee.
A rabbit's joy in bytes and might,
The future's bright, a coder's delight!
🐰✨


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
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 as 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.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

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

@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: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 9485d3f and c03b26f.

Files selected for processing (2)
  • crates/dojo-lang/src/manifest_test_data/compiler_cairo/Scarb.toml (1 hunks)
  • examples/spawn-and-move/Scarb.toml (1 hunks)
Files skipped from review due to trivial changes (2)
  • crates/dojo-lang/src/manifest_test_data/compiler_cairo/Scarb.toml
  • examples/spawn-and-move/Scarb.toml

@remybar remybar force-pushed the move_to_edition_2023_11 branch from c03b26f to 9101405 Compare July 16, 2024 13:06
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: 0

Outside diff range, codebase verification and nitpick comments (4)
crates/dojo-core/src/model_test.cairo (2)

Line range hint 15-19: Suggestion: Consider adding more comprehensive tests for the Foo struct.

Currently, the tests primarily focus on basic functionality. Given the struct's role and the fields it contains, it could be beneficial to add more comprehensive tests that cover edge cases and failure scenarios.


Line range hint 24-29: Optimization suggestion for the modulo operation.

The current implementation of the modulo operation using a loop can be optimized by using the modulus operator directly, which is typically more efficient.

-    while x >= y:
-        x -= y
-    return x
+    return x % y
crates/dojo-core/src/resource_metadata.cairo (1)

44-51: Check for potential improvements in error handling in ResourceMetadataModel.

While the current implementation checks the length of keys, consider adding more descriptive error messages or handling different types of errors more gracefully.

examples/spawn-and-move/src/actions.cairo (1)

167-167: Consider revisiting the commented-out function call.

The commented-out function call to IDungeonDispatcher.enter() may indicate unfinished or in-progress work. Ensure that this is tracked and addressed appropriately.

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between c03b26f and 9101405.

Files selected for processing (16)
  • crates/dojo-core/src/model_test.cairo (1 hunks)
  • crates/dojo-core/src/resource_metadata.cairo (1 hunks)
  • crates/dojo-core/src/world.cairo (3 hunks)
  • crates/dojo-lang/src/manifest_test_data/compiler_cairo/Scarb.toml (1 hunks)
  • crates/dojo-lang/src/model.rs (8 hunks)
  • examples/game-lib/armory/src/lib.cairo (1 hunks)
  • examples/game-lib/bestiary/src/lib.cairo (1 hunks)
  • examples/spawn-and-move/Scarb.toml (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/src/actions.cairo (7 hunks)
  • examples/spawn-and-move/src/dungeon.cairo (1 hunks)
  • examples/spawn-and-move/src/mock_token.cairo (1 hunks)
  • examples/spawn-and-move/src/models.cairo (2 hunks)
  • examples/spawn-and-move/src/others.cairo (1 hunks)
  • examples/spawn-and-move/src/utils.cairo (1 hunks)
Files skipped from review due to trivial changes (1)
  • examples/spawn-and-move/src/others.cairo
Files skipped from review as they are similar to previous changes (2)
  • crates/dojo-lang/src/manifest_test_data/compiler_cairo/Scarb.toml
  • examples/spawn-and-move/Scarb.toml
Additional comments not posted (22)
examples/game-lib/armory/src/lib.cairo (1)

3-7: Visibility changes approved for Flatbow struct.

The changes to make the Flatbow struct and its fields public are noted and approved.

However, ensure that this increased visibility does not inadvertently expose sensitive data or violate encapsulation principles.

Verification successful

Visibility changes approved for Flatbow struct.

The changes to make the Flatbow struct and its fields public are noted and approved. The struct does not handle any sensitive data.

  • examples/spawn-and-move/src/actions.cairo
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that no sensitive data is exposed by the `Flatbow` struct.

# Test: Search for the `Flatbow` struct usage. Expect: No sensitive data handling.
rg --type cairo -A 5 $'Flatbow'

Length of output: 60


Script:

#!/bin/bash
# Description: Verify that no sensitive data is exposed by the `Flatbow` struct.

# Test: Search for the `Flatbow` struct usage in files with the .cairo extension.
rg --glob '*.cairo' -A 5 'Flatbow'

Length of output: 1296

examples/game-lib/bestiary/src/lib.cairo (1)

3-8: Visibility changes approved for RiverSkale struct.

The changes to make the RiverSkale struct and its fields public are noted and approved.

Similar to the Flatbow struct, verify that this does not expose any sensitive data or violate encapsulation principles.

Verification successful

Visibility changes approved for RiverSkale struct.

The changes to make the RiverSkale struct and its fields public are noted and approved. The verification confirms that no sensitive data is exposed by these changes.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that no sensitive data is exposed by the `RiverSkale` struct.

# Test: Search for the `RiverSkale` struct usage. Expect: No sensitive data handling.
rg --type cairo -A 5 $'RiverSkale'

Length of output: 63


Script:

#!/bin/bash
# Description: Verify that no sensitive data is exposed by the `RiverSkale` struct.

# Find all `.cairo` files and search for `RiverSkale` usage.
fd -e cairo -x rg 'RiverSkale' {}

Length of output: 173


Script:

#!/bin/bash
# Description: Verify that no sensitive data is exposed by the `RiverSkale` struct.

# Find all `.cairo` files and search for all instances of `RiverSkale` usage with more context lines.
fd -e cairo -x rg -A 10 'RiverSkale' {}

Length of output: 843

examples/spawn-and-move/src/dungeon.cairo (1)

2-9: Visibility changes approved for IDungeon and related entities.

The changes to make the IDungeon trait, dungeon module, and IDungeonImpl implementation public are noted and approved.

Ensure that these changes do not lead to unintended side effects or security issues, particularly in terms of access control and data integrity.

Verification successful

Visibility changes approved for IDungeon and related entities.

The changes to make the IDungeon trait, dungeon module, and IDungeonImpl implementation public are noted and approved. The provided context indicates that these entities are used in a controlled manner, and no immediate security risks are evident.

  • IDungeon trait and IDungeonImpl implementation are defined with proper access control.
  • The enter function in IDungeonImpl is empty, suggesting no immediate security risk.
  • The enter_dungeon function is conditionally compiled with the dungeon feature, mitigating potential risks.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that no security issues are introduced by making `IDungeon` and related entities public.

# Test: Search for the usage of `IDungeon` and related entities. Expect: Proper access control and data integrity.
rg --type cairo -A 5 $'IDungeon'

Length of output: 61


Script:

#!/bin/bash
# Description: Verify that no security issues are introduced by making `IDungeon` and related entities public.

# Find all .cairo files and search for the usage of `IDungeon` and related entities within those files.
fd -e cairo -x rg 'IDungeon' {}

Length of output: 277


Script:

#!/bin/bash
# Description: Gather more context around the usage and implementation of `IDungeon` and related entities.

# Search for the implementation and detailed usage of `IDungeon` and related entities in .cairo files.
fd -e cairo -x rg -A 10 -B 10 'IDungeon' {}

Length of output: 1874

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

2-3: Updated hash values approved in dojo-world.toml.

The updates to class_hash and original_class_hash are noted and approved.

Verify that these new hash values are consistent across all relevant configuration files and dependencies.

Verification successful

Updated hash values are consistent across the codebase.

The old hash values were not found in any files, confirming that the update to the new hash values is complete and consistent.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the consistency of updated hash values across the codebase.

# Test: Search for the new hash values. Expect: Consistent usage across all configuration files.
rg --type toml -A 5 $'0x6990b621c2db6fbffa4cbbe5c0d91103721c7246adc83a4b27715e2dba79eb'

Length of output: 646


Script:

#!/bin/bash
# Description: Verify if there are any occurrences of the old hash values across the codebase.

# Test: Search for the old hash values. Expect: No occurrences if all updates are consistent.
rg --type toml -A 5 $'0x1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef'

Length of output: 88


Script:

#!/bin/bash
# Description: Verify if there are any occurrences of the old hash values across the entire codebase.

# Test: Search for the old hash values across all file types. Expect: No occurrences if all updates are consistent.
rg -A 5 $'0x1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef'

Length of output: 76

examples/spawn-and-move/src/mock_token.cairo (1)

2-2: Approved visibility change to public for mock_token.

The change to make mock_token public is straightforward and aligns with the PR's objectives to enhance accessibility. However, ensure that exposing this module does not introduce any security vulnerabilities.

Verification successful

Approved visibility change to public for mock_token.

The change to make mock_token public is straightforward and aligns with the PR's objectives to enhance accessibility. The search for sensitive keywords did not reveal any potential security vulnerabilities.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Search for any sensitive data or functionality in the `mock_token` module.

# Test: Search for sensitive keywords within the module. Expect: No matches indicating sensitive data.
rg --type cairo --context 5 "password|secret|private_key" examples/spawn-and-move/src/mock_token.cairo

Length of output: 134


Script:

#!/bin/bash
# Description: Search for any sensitive data or functionality in the `mock_token` module.

# Test: Search for sensitive keywords within the module. Expect: No matches indicating sensitive data.
rg --context 5 "password|secret|private_key" examples/spawn-and-move/src/mock_token.cairo

Length of output: 91

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

2-3: Updated class hashes approved; verify correctness.

The updates to class_hash and original_class_hash are noted and necessary for version control of the contract. It's crucial to verify that these hashes correctly correspond to the intended versions of the contract.

examples/spawn-and-move/src/utils.cairo (1)

3-3: Approved public visibility for next_position.

Making next_position public enhances its usability across different modules. Ensure that this increased accessibility does not lead to misuse or unexpected behaviors.

Verification successful

Approved public visibility for next_position.

The next_position function is used appropriately across the codebase, and its public visibility does not lead to misuse or unexpected behaviors.

  • examples/spawn-and-move/src/others.cairo
  • examples/spawn-and-move/src/actions.cairo
  • crates/benches/contracts/src/systems/actions.cairo
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that there are no potential misuses of the `next_position` function.

# Test: Search for any misuse patterns in the usage of `next_position`. Expect: No matches indicating misuse.
rg --type cairo --context 5 "next_position" examples/spawn-and-move/src/utils.cairo

Length of output: 113


Script:

#!/bin/bash
# Description: Verify that there are no potential misuses of the `next_position` function.

# Test: Search for any misuse patterns in the usage of `next_position` across the entire codebase. Expect: No matches indicating misuse.
rg --context 5 "next_position"

Length of output: 5321

examples/spawn-and-move/src/models.cairo (1)

3-3: Approved Debug trait addition and public visibility changes.

The addition of the Debug trait and changes to public visibility for several structs and enums enhance debugging capabilities and accessibility. However, review the exposed data to ensure no sensitive information is made publicly accessible inadvertently.

Also applies to: 24-96

Verification successful

Approved Debug trait addition and public visibility changes.

The addition of the Debug trait and changes to public visibility for several structs and enums enhance debugging capabilities and accessibility. The review of exposed data did not reveal any sensitive information being made publicly accessible.

  • examples/spawn-and-move/src/models.cairo
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Review exposed data in modified structs and enums for sensitivity.

# Test: Search for any sensitive data patterns in the modified structs and enums. Expect: No matches indicating sensitive data.
rg --type cairo --context 5 "password|secret|private_key" examples/spawn-and-move/src/models.cairo

Length of output: 130


Script:

#!/bin/bash
# Description: Review exposed data in modified structs and enums for sensitivity.

# Test: Search for any sensitive data patterns in the modified structs and enums. Expect: No matches indicating sensitive data.
rg --context 5 "password|secret|private_key" examples/spawn-and-move/src/models.cairo

Length of output: 87

crates/dojo-core/src/model_test.cairo (2)

9-9: Approved addition of the Debug trait to the Foo struct.

Adding the Debug trait enhances the ability to print and log the struct's state, which is beneficial for debugging purposes.


Line range hint 20-23: Verify the consistency of test naming and structure.

The tests seem to vary in their naming convention and structure. Standardizing these could improve readability and maintainability.

Verification successful

Test naming and structure are consistent. The test functions across the codebase follow a consistent naming convention (fn test_), and there is no indication of structural inconsistency.

  • crates/dojo-core/src/model_test.cairo
  • crates/dojo-core/src/database_test.cairo
  • crates/dojo-core/src/utils_test.cairo
  • crates/dojo-core/src/packing_test.cairo
  • crates/dojo-core/src/world_test.cairo
  • crates/dojo-core/src/base_test.cairo
  • crates/dojo-core/src/database/storage_test.cairo
  • crates/dojo-core/src/database/introspect_test.cairo
  • crates/dojo-core/src/benchmarks.cairo
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify naming consistency in test functions.

# Test: Search for the test function names. Expect: Consistent naming pattern.
rg --type python -A 1 $'fn test_'

Length of output: 65


Script:

#!/bin/bash
# Description: Verify naming consistency in test functions.

# Test: Search for the test function names. Expect: Consistent naming pattern.
rg --type cairo -A 1 $'fn test_'

Length of output: 63


Script:

#!/bin/bash
# Description: Verify naming consistency in test functions.

# Test: Search for the test function names. Expect: Consistent naming pattern.
rg 'fn test_' --glob '*.cairo' -A 1

Length of output: 23752

crates/dojo-core/src/resource_metadata.cairo (1)

27-41: Approved changes in ResourceMetadataImpl.

The changes enhance error handling by checking for deserialization failures and panicking with a clear error message. This ensures that any issues with metadata_uri deserialization are caught early.

examples/spawn-and-move/src/actions.cairo (5)

4-4: Approved the change to make IActions trait public.

Making the IActions trait public allows it to be used more flexibly across different modules or packages, which is beneficial for extensibility.


18-18: Approved the change to make IActionsComputed trait public.

Similar to the IActions trait, making IActionsComputed public enhances the usability of the trait across different parts of the application.


24-24: Approved making the actions module public.

This change improves the modularity of the code by allowing other parts of the application to access and utilize the actions module.


47-50: Visibility change approved for the Moved struct.

Making the members of the Moved struct public allows for easier access and manipulation of its fields, which can be particularly useful in testing or when interfacing with other modules.


241-241: Verify the addition of debugging output in tests.

While the println! statement aids in debugging, ensure it is removed or guarded by a debug condition before merging to maintain clean test outputs.

crates/dojo-lang/src/model.rs (3)

279-280: Approved the addition of pub visibility to struct members.

Adding pub to the struct members enhances their accessibility, allowing for more flexible use across different parts of the application.


329-329: Approved the changes to make impl blocks public.

Making these implementation blocks public increases the visibility and usability of the associated traits and methods, aligning with the goals of improving accessibility in the codebase.

Also applies to: 338-338, 373-373, 473-473


638-638: Approved the changes to make trait and module declarations public.

These changes facilitate easier access and integration of the I$contract_name$ trait and $contract_name$ module across different parts of the application, supporting better modularity and reusability.

Also applies to: 643-643

crates/dojo-core/src/world.cairo (3)

339-344: Approved method signature change, verify impact.

The change in the set_metadata method to accept a reference to ResourceMetadataTrait instead of ResourceMetadata likely aims to reduce memory usage and increase flexibility. Ensure that this change does not negatively impact the functionality or performance.


129-129: Approved import change, verify usage.

The import change from ResourceMetadata to {ResourceMetadata, ResourceMetadataTrait} is consistent with the method signature changes. Ensure that these imports are used correctly across the module.

Verification successful

Verified import change and usage.

The import change to include ResourceMetadataTrait along with ResourceMetadata is consistent and correctly utilized across the module. The methods and functions involving these types are implemented properly.

  • Verified usage in method signatures and function implementations.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of `ResourceMetadata` and `ResourceMetadataTrait` across the module.

# Test: Search for the usage of these types. Expect: Correct usage across the module.
rg --type cairo -A 5 $'ResourceMetadata|ResourceMetadataTrait' crates/dojo-core/src/world.cairo

Length of output: 126


Script:

#!/bin/bash
# Description: Verify the usage of `ResourceMetadata` and `ResourceMetadataTrait` across the module.

# Test: Search for the usage of these types. Expect: Correct usage across the module.
rg -A 5 'ResourceMetadata|ResourceMetadataTrait' crates/dojo-core/src/world.cairo

Length of output: 1902


320-325: Approved method signature change, verify impact.

The change in the return type of the metadata method from ResourceMetadata to ResourceMetadataTrait enhances flexibility. Verify that this change is handled correctly wherever the metadata method is called.

@remybar remybar force-pushed the move_to_edition_2023_11 branch from 9101405 to 1a55dd6 Compare July 16, 2024 13:21
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: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 9101405 and 1a55dd6.

Files selected for processing (21)
  • crates/dojo-core/src/model_test.cairo (1 hunks)
  • crates/dojo-core/src/resource_metadata.cairo (1 hunks)
  • crates/dojo-core/src/world.cairo (3 hunks)
  • crates/dojo-lang/src/manifest_test_data/compiler_cairo/Scarb.toml (1 hunks)
  • crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/dojo-world.toml (1 hunks)
  • crates/dojo-lang/src/model.rs (8 hunks)
  • examples/game-lib/armory/src/lib.cairo (1 hunks)
  • examples/game-lib/bestiary/src/lib.cairo (1 hunks)
  • examples/spawn-and-move/Scarb.toml (2 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/manifest.json (6 hunks)
  • examples/spawn-and-move/manifests/dev/deployment/manifest.toml (5 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 (7 hunks)
  • examples/spawn-and-move/src/dungeon.cairo (1 hunks)
  • examples/spawn-and-move/src/mock_token.cairo (1 hunks)
  • examples/spawn-and-move/src/models.cairo (2 hunks)
  • examples/spawn-and-move/src/others.cairo (1 hunks)
  • examples/spawn-and-move/src/utils.cairo (1 hunks)
Files skipped from review as they are similar to previous changes (16)
  • crates/dojo-core/src/model_test.cairo
  • crates/dojo-core/src/resource_metadata.cairo
  • crates/dojo-core/src/world.cairo
  • crates/dojo-lang/src/manifest_test_data/compiler_cairo/Scarb.toml
  • crates/dojo-lang/src/model.rs
  • examples/game-lib/armory/src/lib.cairo
  • examples/game-lib/bestiary/src/lib.cairo
  • examples/spawn-and-move/Scarb.toml
  • examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-actions-40b6994c.toml
  • examples/spawn-and-move/manifests/dev/base/dojo-world.toml
  • examples/spawn-and-move/src/actions.cairo
  • examples/spawn-and-move/src/dungeon.cairo
  • examples/spawn-and-move/src/mock_token.cairo
  • examples/spawn-and-move/src/models.cairo
  • examples/spawn-and-move/src/others.cairo
  • examples/spawn-and-move/src/utils.cairo
Additional comments not posted (11)
crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/dojo-world.toml (1)

2-3: Verify the correctness of the updated hash values.

The class_hash and original_class_hash have been updated. Ensure that these new values are correct and consistent with the intended class versions in the project.

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

2-3: Verify the correctness of the updated hash values.

The class_hash and original_class_hash have been updated in this release configuration. Ensure these values are correct and consistently used across the project.

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

2-3: Verify the correctness of the updated hash values.

The class_hash and original_class_hash for the DojoContract have been updated. Confirm that these new values are correct and consistently used across the project.

Verification successful

The updated hash values are correct and consistently used across the project.

The class_hash and original_class_hash values have been verified and found to be consistent across the related files.

  • examples/spawn-and-move/manifests/dev/deployment/manifest.toml
  • examples/spawn-and-move/manifests/release/base/contracts/dojo_examples-actions-40b6994c.toml
  • examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-actions-40b6994c.toml
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the new hash values are consistent across the project.

# Test: Search for the new hash values. Expect: Consistent usage across all related files.
rg --type toml -A 5 $'0x59dac0f4034cbf09380196825991536fb07a7b6b692c3d0c0e5d5fe4a754966'

Length of output: 2820

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

3-7: Verify the updated contract details for WorldContract.

Ensure that the new class_hash, original_class_hash, address, and transaction_hash are correct and consistent across the project for WorldContract.

Verification successful

Verified: The updated contract details for WorldContract are correct and consistent across the project.

  • The class_hash, original_class_hash, address, and transaction_hash values match across all relevant files.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the new values for `WorldContract` are consistent across the project.

# Test: Search for the new values. Expect: Consistent usage across all related files.
rg --type toml -A 5 $'0x6990b621c2db6fbffa4cbbe5c0d91103721c7246adc83a4b27715e2dba79eb'
rg --type toml -A 5 $'0x2bab2a15d4c71bd8e45c84ac3b1518aeca39d3818e8e556c4a56202c1679ba6'
rg --type toml -A 5 $'0x1e4e0d098436df78bfd2f61cfd24273bd2f9de70b1ad372aa5d122f2ad7cc75'

Length of output: 4655


26-28: Verify the updated contract details for DojoContract.

Check the correctness of the address, class_hash, and original_class_hash for DojoContract. Confirm that these values are correct and consistently used across the project.

Verification successful

Verified the updated contract details for DojoContract.

The values for address, class_hash, and original_class_hash are correctly associated with DojoContract and are consistently used across the project.

  • examples/spawn-and-move/manifests/dev/deployment/manifest.toml
  • examples/spawn-and-move/manifests/release/base/contracts/dojo_examples-actions-40b6994c.toml
  • examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-actions-40b6994c.toml
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the new values for `DojoContract` are consistent across the project.

# Test: Search for the new values. Expect: Consistent usage across all related files.
rg --type toml -A 5 $'0x5718df0ef4ac58def63459069d160887ad637f979cf425d0ddb6fd77f3cd121'
rg --type toml -A 5 $'0x59dac0f4034cbf09380196825991536fb07a7b6b692c3d0c0e5d5fe4a754966'

Length of output: 3714


Script:

#!/bin/bash
# Description: Verify the association of `DojoContract` with the given values across the project.

# Test: Search for `DojoContract` references and check if the values are correctly associated.
rg --type toml -A 5 'DojoContract'

Length of output: 12151

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

1213-1215: Updated address and class hash for DojoContract

The address and class hash for DojoContract have been updated. It's crucial to verify these updates to ensure they reflect the correct and intended new versions of the contract.

Verification successful

Updated address and class hash for DojoContract

The address and class hash for DojoContract with the dojo-base tag have been correctly updated to:

  • Address: 0x5718df0ef4ac58def63459069d160887ad637f979cf425d0ddb6fd77f3cd121

  • Class hash: 0x59dac0f4034cbf09380196825991536fb07a7b6b692c3d0c0e5d5fe4a754966

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify the new address and class hash for DojoContract in the deployment records.
echo "Verify address: 0x5718df0ef4ac58def63459069d160887ad637f979cf425d0ddb6fd77f3cd121"
echo "Verify class hash: 0x59dac0f4034cbf09380196825991536fb07a7b6b692c3d0c0e5d5fe4a754966"

Length of output: 351


Script:

#!/bin/bash
# Search for references to DojoContract in the codebase and check if the address and class hash are correctly updated.
rg -A 3 '"DojoContract"' --glob '*.json'

Length of output: 2269


Script:

#!/bin/bash
# Display the context around each DojoContract instance to ensure the updated address and class hash are correctly applied.
rg -C 5 '"DojoContract"' --glob '*.json'

Length of output: 4954


1881-1881: Updated address for yet another instance of DojoContract

This change updates the address for yet another instance of DojoContract. It's important to verify that this update was intentional and correctly reflects the new deployment details.


1192-1193: Updated transaction details for WorldContract

The transaction hash for WorldContract has been updated to reflect a new transaction. Confirm that this transaction hash is linked to the correct deployment or update transaction for WorldContract.

Verification successful

Transaction Hash Verification for WorldContract

The transaction hash 0x1e4e0d098436df78bfd2f61cfd24273bd2f9de70b1ad372aa5d122f2ad7cc75 is correctly linked to the WorldContract deployment or update transaction as referenced in the manifest.toml and manifest.json files.

  • examples/spawn-and-move/manifests/dev/deployment/manifest.toml
  • examples/spawn-and-move/manifests/dev/deployment/manifest.json
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify the new transaction hash for WorldContract in the blockchain explorer or transaction logs.
echo "Verify transaction hash: 0x1e4e0d098436df78bfd2f61cfd24273bd2f9de70b1ad372aa5d122f2ad7cc75"

Length of output: 190


Script:

#!/bin/bash
# Search for the transaction hash in the repository to find references or logs.
rg '0x1e4e0d098436df78bfd2f61cfd24273bd2f9de70b1ad372aa5d122f2ad7cc75' -A 5

# Search for the WorldContract in the repository to find deployment or update references.
rg 'WorldContract' -A 5

Length of output: 55276


Script:

#!/bin/bash
# Search for references to manifest.toml in the repository to ensure it is used in the deployment process.
rg 'manifest.toml' -A 5

# Search for references to manifest.json in the repository to ensure it is used in the deployment process.
rg 'manifest.json' -A 5

Length of output: 8188


4-5: Updated class hash and address for WorldContract

The class hash and address for WorldContract have been updated. Ensure that these new values are correct and correspond to the newly deployed contract versions. It would be beneficial to verify these hashes and addresses against the deployment records or contract management system to ensure accuracy.

Verification successful

Updated class hash and address for WorldContract

The class hash 0x6990b621c2db6fbffa4cbbe5c0d91103721c7246adc83a4b27715e2dba79eb and address 0x2bab2a15d4c71bd8e45c84ac3b1518aeca39d3818e8e556c4a56202c1679ba6 for WorldContract have been consistently updated across multiple deployment and configuration files. This confirms that the values are correct and correspond to the newly deployed contract versions.

  • examples/spawn-and-move/manifests/release/base/dojo-world.toml
  • examples/spawn-and-move/manifests/dev/deployment/manifest.toml
  • examples/spawn-and-move/manifests/dev/deployment/manifest.json
  • examples/spawn-and-move/manifests/dev/base/dojo-world.toml
  • crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/dojo-world.toml
  • examples/spawn-and-move/Scarb.toml
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify the new class hash and address for WorldContract in the deployment records.
echo "Verify class hash: 0x6990b621c2db6fbffa4cbbe5c0d91103721c7246adc83a4b27715e2dba79eb"
echo "Verify address: 0x2bab2a15d4c71bd8e45c84ac3b1518aeca39d3818e8e556c4a56202c1679ba6"

Length of output: 349


Script:

#!/bin/bash
# Search for the class hash and address in the deployment records or configuration files.
rg '0x6990b621c2db6fbffa4cbbe5c0d91103721c7246adc83a4b27715e2dba79eb' -A 5
rg '0x2bab2a15d4c71bd8e45c84ac3b1518aeca39d3818e8e556c4a56202c1679ba6' -A 5

Length of output: 5391


2100-2100: Updated address and class hash for a different instance of DojoContract

The address and class hash for a different instance of DojoContract have been updated. Confirm these changes are correct and intended, especially since multiple instances are being updated in this manifest.

Verification successful

Address and class hash updates are correct and intended

The address 0x7d38ad1bf17abfd26749f4780b95c525cc562c8ffba994bbe8e09de36b25404 and class hash 0x479bfb12dcba5398d77303e7a665fc3fedb16f2d7f9cb1f5d7e2beb3b7e2ba7 for the DojoContract instance have been verified and are correct. Each instance of DojoContract in the manifest file has unique values, and there are no inconsistencies.

  • File: examples/spawn-and-move/manifests/dev/deployment/manifest.json
  • Lines: 2100-2100
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify the new address and class hash for a different instance of DojoContract in the deployment records.
echo "Verify address: 0x7d38ad1bf17abfd26749f4780b95c525cc562c8ffba994bbe8e09de36b25404"
echo "Verify class hash: 0x479bfb12dcba5398d77303e7a665fc3fedb16f2d7f9cb1f5d7e2beb3b7e2ba7"

Length of output: 351


Script:

#!/bin/bash
# Search for the address and class hash in the manifest file and display their surrounding context.
rg '0x7d38ad1bf17abfd26749f4780b95c525cc562c8ffba994bbe8e09de36b25404' -A 5 -B 5 examples/spawn-and-move/manifests/dev/deployment/manifest.json
rg '0x479bfb12dcba5398d77303e7a665fc3fedb16f2d7f9cb1f5d7e2beb3b7e2ba7' -A 5 -B 5 examples/spawn-and-move/manifests/dev/deployment/manifest.json

Length of output: 1372


Script:

#!/bin/bash
# Search for all instances of DojoContract in the manifest file and display their surrounding context.
rg '"kind": "DojoContract"' -A 10 -B 2 examples/spawn-and-move/manifests/dev/deployment/manifest.json

Length of output: 2360


1644-1644: Updated address for another instance of DojoContract

The address for another instance of DojoContract has been updated. As with other updates, ensure this new address is accurate and corresponds to a legitimate and intended update.

Verification successful

Updated address for another instance of DojoContract

The address for another instance of DojoContract has been updated from 0x44e1b43ee34f816374612ae2df4a668e76a1aa723de8d7b67166e10a7225112 to 0x3e629315ea75d37711b4146af01efa2f0218e8aefda2ceb42bb0fd310cb2840. This update is legitimate and corresponds to the intended change by the same author.

  • Previous address: 0x44e1b43ee34f816374612ae2df4a668e76a1aa723de8d7b67166e10a7225112
  • New address: 0x3e629315ea75d37711b4146af01efa2f0218e8aefda2ceb42bb0fd310cb2840
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify the new address for another instance of DojoContract in the deployment records.
echo "Verify address: 0x3e629315ea75d37711b4146af01efa2f0218e8aefda2ceb42bb0fd310cb2840"

Length of output: 172


Script:

#!/bin/bash
# Search for all instances of DojoContract in the repository to understand its usage.
rg "DojoContract"

# Check the history of the manifest.json file to see if there are previous addresses and compare them with the new one.
git log -p -L 1644,1644:examples/spawn-and-move/manifests/dev/deployment/manifest.json

Length of output: 10150

Copy link

codecov bot commented Jul 16, 2024

Codecov Report

Attention: Patch coverage is 94.44444% with 2 lines in your changes missing coverage. Please review.

Project coverage is 67.76%. Comparing base (e259d35) to head (3424011).

Files Patch % Lines
crates/dojo-lang/src/interface.rs 50.00% 1 Missing ⚠️
crates/dojo-world/abigen/src/main.rs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2179      +/-   ##
==========================================
- Coverage   67.77%   67.76%   -0.01%     
==========================================
  Files         336      336              
  Lines       43936    43932       -4     
==========================================
- Hits        29778    29772       -6     
- Misses      14158    14160       +2     

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

@remybar remybar force-pushed the move_to_edition_2023_11 branch from 1a55dd6 to 7f21b41 Compare July 17, 2024 09:13
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: 1

Outside diff range, codebase verification and nitpick comments (1)
crates/dojo-core/src/database/utils.cairo (1)

Line range hint 18-26: Visibility change to sum function approved but consider error handling.

The sum function is now public, aligning with the PR's goals. However, the function uses .unwrap() which could panic if any element of the array is None. Consider adding error handling to prevent runtime errors.

-        res += (*arr.at(i)).unwrap();
+        match arr.at(i) {
+            Some(value) => res += *value,
+            None => return Err("Array contains None value"),
+        }
Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 1a55dd6 and 7f21b41.

Files selected for processing (47)
  • crates/dojo-core/Scarb.toml (1 hunks)
  • crates/dojo-core/src/base.cairo (1 hunks)
  • crates/dojo-core/src/base_test.cairo (2 hunks)
  • crates/dojo-core/src/benchmarks.cairo (15 hunks)
  • crates/dojo-core/src/components.cairo (1 hunks)
  • crates/dojo-core/src/components/upgradeable.cairo (2 hunks)
  • crates/dojo-core/src/config.cairo (1 hunks)
  • crates/dojo-core/src/config/component.cairo (3 hunks)
  • crates/dojo-core/src/config/interface.cairo (1 hunks)
  • crates/dojo-core/src/contract.cairo (1 hunks)
  • crates/dojo-core/src/database.cairo (4 hunks)
  • crates/dojo-core/src/database/introspect.cairo (16 hunks)
  • crates/dojo-core/src/database/storage.cairo (9 hunks)
  • crates/dojo-core/src/database/storage_test.cairo (1 hunks)
  • crates/dojo-core/src/database/utils.cairo (2 hunks)
  • crates/dojo-core/src/database_test.cairo (1 hunks)
  • crates/dojo-core/src/interfaces.cairo (2 hunks)
  • crates/dojo-core/src/lib.cairo (2 hunks)
  • crates/dojo-core/src/model.cairo (4 hunks)
  • crates/dojo-core/src/model_test.cairo (1 hunks)
  • crates/dojo-core/src/packing.cairo (8 hunks)
  • crates/dojo-core/src/packing_test.cairo (10 hunks)
  • crates/dojo-core/src/resource_metadata.cairo (7 hunks)
  • crates/dojo-core/src/test_utils.cairo (5 hunks)
  • crates/dojo-core/src/utils.cairo (4 hunks)
  • crates/dojo-core/src/world.cairo (23 hunks)
  • crates/dojo-core/src/world_test.cairo (46 hunks)
  • crates/dojo-lang/src/interface.rs (2 hunks)
  • crates/dojo-lang/src/manifest_test_data/compiler_cairo/Scarb.toml (1 hunks)
  • crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/dojo-world.toml (1 hunks)
  • crates/dojo-lang/src/model.rs (8 hunks)
  • examples/game-lib/armory/src/lib.cairo (1 hunks)
  • examples/game-lib/bestiary/src/lib.cairo (1 hunks)
  • examples/spawn-and-move/Scarb.toml (2 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/manifest.json (6 hunks)
  • examples/spawn-and-move/manifests/dev/deployment/manifest.toml (5 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 (7 hunks)
  • examples/spawn-and-move/src/dungeon.cairo (1 hunks)
  • examples/spawn-and-move/src/lib.cairo (1 hunks)
  • examples/spawn-and-move/src/mock_token.cairo (1 hunks)
  • examples/spawn-and-move/src/models.cairo (2 hunks)
  • examples/spawn-and-move/src/others.cairo (1 hunks)
  • examples/spawn-and-move/src/utils.cairo (1 hunks)
Files not reviewed due to server errors (3)
  • crates/dojo-core/src/database/introspect.cairo
  • crates/dojo-lang/src/interface.rs
  • crates/dojo-core/src/database/storage.cairo
Files skipped from review due to trivial changes (1)
  • crates/dojo-core/Scarb.toml
Files skipped from review as they are similar to previous changes (20)
  • crates/dojo-core/src/model_test.cairo
  • crates/dojo-core/src/resource_metadata.cairo
  • crates/dojo-core/src/world.cairo
  • crates/dojo-lang/src/manifest_test_data/compiler_cairo/Scarb.toml
  • crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/dojo-world.toml
  • crates/dojo-lang/src/model.rs
  • examples/game-lib/armory/src/lib.cairo
  • examples/game-lib/bestiary/src/lib.cairo
  • examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-actions-40b6994c.toml
  • examples/spawn-and-move/manifests/dev/base/dojo-world.toml
  • examples/spawn-and-move/manifests/dev/deployment/manifest.json
  • examples/spawn-and-move/manifests/dev/deployment/manifest.toml
  • examples/spawn-and-move/manifests/release/base/contracts/dojo_examples-actions-40b6994c.toml
  • examples/spawn-and-move/manifests/release/base/dojo-world.toml
  • examples/spawn-and-move/src/actions.cairo
  • examples/spawn-and-move/src/dungeon.cairo
  • examples/spawn-and-move/src/mock_token.cairo
  • examples/spawn-and-move/src/models.cairo
  • examples/spawn-and-move/src/others.cairo
  • examples/spawn-and-move/src/utils.cairo
Additional comments not posted (65)
crates/dojo-core/src/components.cairo (1)

1-1: Approved: Public visibility added to upgradeable module.

The change aligns with the PR objectives to enhance accessibility. Ensure that appropriate access controls are maintained.

crates/dojo-core/src/config.cairo (1)

1-2: Approved: Public visibility added to component and interface modules.

The changes are consistent with the goal of enhancing module accessibility. It's important to verify that no sensitive data or functionality is inadvertently exposed.

Verification successful

Verified: No sensitive data or functionality exposed by making component and interface modules public.

The search for sensitive keywords within the component and interface modules did not return any results, confirming that the changes are safe.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that no sensitive data or functionality is exposed by making `component` and `interface` modules public.

# Test: Search for sensitive keywords within `component` and `interface` modules.
rg --type cairo -A 3 "sensitive|private|secret" ./crates/dojo-core/src/config.cairo

Length of output: 115


Script:

#!/bin/bash
# Description: Verify that no sensitive data or functionality is exposed by making `component` and `interface` modules public.

# Find the actual files for the `component` and `interface` modules.
component_file=$(fd component.cairo)
interface_file=$(fd interface.cairo)

# Test: Search for sensitive keywords within `component` and `interface` modules.
rg "sensitive|private|secret" "$component_file"
rg "sensitive|private|secret" "$interface_file"

Length of output: 317

examples/spawn-and-move/src/lib.cairo (1)

1-7: Approved: Public visibility added to multiple modules.

All changes align with the PR's objective to enhance module accessibility. Ensure that no sensitive data or functionality is exposed across these newly public modules.

Verification successful

Approved: Public visibility added to multiple modules.

All changes align with the PR's objective to enhance module accessibility. Ensure that no sensitive data or functionality is exposed across these newly public modules.

  • No instances of "sensitive", "private", or "secret" were found in the newly public modules.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that no sensitive data or functionality is exposed by making multiple modules public.

# Test: Search for sensitive keywords across all newly public modules.
rg --type cairo -A 3 "sensitive|private|secret" ./examples/spawn-and-move/src/lib.cairo

Length of output: 119


Script:

#!/bin/bash
# Description: Verify that no sensitive data or functionality is exposed by making multiple modules public.

# Test: Search for sensitive keywords across all newly public modules.
rg -A 3 "sensitive|private|secret" $(fd .cairo examples/spawn-and-move/src/)

Length of output: 367

crates/dojo-core/src/contract.cairo (1)

2-2: Approved: Public visibility added to IContract<T> trait.

This change is in line with the objective to enhance accessibility. Verify that this does not lead to any security or architectural issues.

crates/dojo-core/src/database/utils.cairo (1)

Line range hint 1-11: Visibility change to any_none function approved.

The change in visibility from private to public for the any_none function is consistent with the PR's objectives to enhance accessibility. The implementation of the function is correct and efficient for checking if any element in the array is None.

crates/dojo-core/src/lib.cairo (1)

Line range hint 1-39: Visibility changes for multiple modules approved.

All modules (base, config, database, etc.) have been made public, which is consistent with the PR's objective to enhance accessibility. The comments in the code provide sufficient context about the changes, especially regarding the test configurations and optimizations. These changes should be monitored for any unintended external access that could affect the system's security.

crates/dojo-core/src/interfaces.cairo (2)

4-18: Visibility changes to structs StorageUpdate and ProgramOutput approved.

Making these structs public is in line with the PR's objectives. The use of pub keyword on both the struct and its fields ensures that they can be accessed and utilized appropriately in other modules. Ensure that the increased visibility does not expose sensitive data.


Line range hint 22-32: Visibility changes to traits IUpgradeableState and IFactRegistry approved.

The traits IUpgradeableState and IFactRegistry are now public, which should facilitate their use in other parts of the application or by external modules. This change should be carefully monitored to ensure it does not lead to misuse or unexpected behavior in the system.

crates/dojo-core/src/base.cairo (1)

4-4: Visibility change to base module approved.

The change in visibility for the base module is consistent with the PR's objectives to enhance accessibility. The implementation within the module appears secure and well-structured. Ensure that the public exposure of this module does not compromise the encapsulation of its functionalities.


Line range hint 5-57: Review of test functions for correct import usage.

The test functions are using the updated imports correctly. The logic within the tests appears sound and is consistent with the intended functionality of the storage operations.

crates/dojo-core/src/database.cairo (3)

2-6: Updated import paths to core module.

The import paths have been updated to reflect the new structure of the core module. This should facilitate better organization and possibly improve the build times by centralizing common functionality.


11-11: Visibility changes to constants and modules.

The visibility of MAX_ARRAY_LENGTH and several modules has been changed to public. This enhances the reusability of these components across different parts of the project or even externally if needed.

Also applies to: 13-13, 16-16, 19-19


63-63: Updated function signatures and visibility.

The functions get, set, delete, set_array, and get_array have been made public and their signatures have been updated to include new parameters. This increases their flexibility and potential for reuse. It is crucial to ensure that these changes are reflected wherever these functions are used.

Also applies to: 74-74, 85-85, 91-91, 96-96

crates/dojo-core/src/config/component.cairo (2)

1-4: Visibility changes to modules and error constants.

The errors and Config modules have been made public, along with specific error constants. This change likely aims to standardize error handling and configuration management across different parts of the application or even externally.

Also applies to: 8-8


25-25: Struct fields and implementation block made public.

The fields in the structs DifferProgramHashUpdate, MergerProgramHashUpdate, and FactsRegistryUpdate have been made public, along with the InternalImpl implementation block. This increases transparency and allows other components or external modules to interact more freely with these configurations.

Also applies to: 30-30, 35-35, 47-47

crates/dojo-core/src/test_utils.cairo (5)

38-38: Visibility change approved for deploy_with_world_address.

The change to make this function public is consistent with the PR's objectives and enhances module interoperability.


42-43: Visibility change approved for spawn_test_world.

Making spawn_test_world public supports the PR's goal of enhancing accessibility and functionality within the module.


74-75: Visibility changes approved for GasCounter.

Making GasCounter and its field start public aligns with the PR's objectives to enhance accessibility.


79-79: Visibility change approved for GasCounterImpl.

Making GasCounterImpl public enhances the usability of the GasCounter structure in external modules.


110-110: Visibility change approved for assert_array.

Making assert_array public is consistent with the PR's objectives and enhances debugging capabilities.

crates/dojo-core/src/base_test.cairo (6)

1-1: Updated imports to use core module paths.

The import statement has been correctly updated to use the core module path for OptionTrait. This aligns with the PR's goal to standardize import paths.


19-19: Visibility modification to IQuantumLeap trait.

The visibility change to public for the IQuantumLeap trait is appropriate if there's a need for external access or inheritance. Confirm that this change is intended and documented.


42-42: Visibility modification to contract_invalid_upgrade module.

Making the contract_invalid_upgrade module public aligns with the PR's goals but ensure it does not unintentionally expose any sensitive or incomplete features.

Verification successful

Visibility modification to contract_invalid_upgrade module.

The contract_invalid_upgrade module contains an empty Storage struct and is used to reference TEST_CLASS_HASH for contract upgrades. This does not seem to expose any sensitive or incomplete features. However, ensure that the TEST_CLASS_HASH and the upgrade process are secure and do not introduce vulnerabilities.

  • contract_invalid_upgrade module declaration and usage:
    • pub mod contract_invalid_upgrade {
    • struct Storage {}
    • let new_class_hash: ClassHash = contract_invalid_upgrade::TEST_CLASS_HASH.try_into().unwrap();
    • world.upgrade_contract(selector, new_class_hash);
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the `contract_invalid_upgrade` module's public visibility does not expose sensitive features.

# Test: Search for the module usage and assess exposure risks. Expect: No sensitive exposure.
rg --type cairo -A 5 $'contract_invalid_upgrade'

Length of output: 77


Script:

#!/bin/bash
# Description: Verify that the `contract_invalid_upgrade` module's public visibility does not expose sensitive features.

# Test: Search for the module usage and assess exposure risks. Expect: No sensitive exposure.
fd -e cairo -x rg 'contract_invalid_upgrade' -A 5 {}

Length of output: 427


12-12: Visibility modification to contract_upgrade module.

Making the contract_upgrade module public is consistent with the PR's objective to enhance accessibility. Ensure that all references and usages across the project are updated to accommodate this change.


27-27: Visibility modification to QuantumLeap implementation.

Making the QuantumLeap implementation public increases its accessibility. Verify that this change aligns with the intended use cases and security considerations.


34-34: Visibility modification to WorldProviderImpl implementation.

The change to make WorldProviderImpl public should be validated to ensure it's necessary for external access and doesn't expose sensitive logic.

Verification successful

Visibility modification to WorldProviderImpl implementation.

The change to make WorldProviderImpl public appears necessary for external access to the world function, which is a core part of the contract's functionality. There is no indication of sensitive logic being exposed.

  • crates/dojo-core/src/base_test.cairo
  • crates/dojo-core/src/base.cairo
  • crates/dojo-lang/src/contract.rs
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the `WorldProviderImpl` implementation's public visibility is necessary.

# Test: Search for the implementation usage and assess necessity. Expect: Necessary usage.
rg --type cairo -A 5 $'WorldProviderImpl'

Length of output: 70


Script:

#!/bin/bash
# Description: Verify that the `WorldProviderImpl` implementation's public visibility is necessary.

# Test: Search for the implementation usage and assess necessity. Expect: Necessary usage.
rg -A 5 'WorldProviderImpl'

Length of output: 18911


Script:

#!/bin/bash
# Description: Verify the necessity of making `WorldProviderImpl` public by examining its usage in source files.

# Test: Search for references to `WorldProviderImpl` in source files to assess if external access is required.
rg 'WorldProviderImpl' --type-add 'src:*.{cairo,rs}' --files-with-matches

Length of output: 2015


Script:

#!/bin/bash
# Description: Verify the necessity of making `WorldProviderImpl` public by examining its usage context in source files.

# Extract relevant lines of code around references to `WorldProviderImpl` in source files.
rg 'WorldProviderImpl' -A 10 -B 5 crates/dojo-core/src/base_test.cairo crates/dojo-core/src/base.cairo crates/dojo-lang/src/contract.rs

Length of output: 3160

crates/dojo-core/src/packing.cairo (8)

1-4: Imports updated to use core modules.

The imports have been updated to use absolute paths from the core module, which aligns with best practices for clarity and maintainability.


Line range hint 8-19: Visibility and logic check for pack function.

The pack function has been made public, which aligns with the PR's goal to enhance accessibility. The logic inside the function remains unchanged, ensuring that existing functionality is preserved.


Line range hint 30-40: Visibility and logic check for calculate_packed_size function.

The calculate_packed_size function is now public. The function calculates the packed size based on the layout provided, and the logic appears to be correct and efficient.


Line range hint 51-62: Visibility and error handling in unpack function.

The unpack function is now public. The addition of a panic statement for failure in the unpack_inner function improves error handling by providing a clear failure point which is crucial for debugging.


Line range hint 72-95: Visibility and logic check for pack_inner function.

The pack_inner function has been made public and contains detailed assertions to ensure that the packing operation is within valid bounds. The logic is well-commented, explaining the constraints and operations clearly.


Line range hint 106-129: Visibility and logic check for unpack_inner function.

The unpack_inner function is now public. The function handles edge cases where the remaining bits are less than the size required, which is crucial for correct unpacking behavior. The logic is sound and well-documented.


Line range hint 138-149: Updated logic in fpow function.

The fpow function has been made public and includes a new panic condition for the base being zero, which is a necessary safety check for mathematical correctness. The recursive power calculation logic is correct and handles edge cases like zero and one powers efficiently.


157-161: Visibility change for utility functions and constants.

The utility functions shl, shr, and the constant POW_2 have been made public. This change increases the reusability of these components across the codebase. The logic in these functions is straightforward and uses the precomputed powers of two for efficiency.

Also applies to: 165-169

crates/dojo-core/src/world_test.cairo (6)

1-5: Consolidation of Imports to Use Core Namespace

The changes to the import statements consolidate various modules under the core namespace, which is a good practice for maintaining organized and manageable code. This should also help in avoiding naming conflicts and making the codebase easier to navigate.


42-46: Visibility Changes to Structs and Enums

The changes make several structs and enums public, which enhances their accessibility across different modules or even externally if needed. This is a positive change, especially in a modular codebase, allowing for better reusability and testing.

Also applies to: 51-55, 65-68, 73-77, 86-89, 98-101, 114-119, 130-134, 142-145, 150-155, 185-189, 235-245


329-329: Update to delete_foo Method in bar Module

The method implementation now uses dojo::world::ModelIndex::Keys instead of dojo::model::ModelIndex::Keys. This change is part of the broader update to use more specific namespaces, which should help in reducing ambiguity and improving code clarity.


Line range hint 987-1002: Tests and Entity Management

The updates in the test functions and methods for setting and deleting entities reflect the new entity index types and the use of updated namespaces. These changes are crucial for ensuring that the system's functionality aligns with the updated data structures and namespaces.

However, ensure that these changes are reflected in all relevant parts of the codebase to maintain consistency and prevent runtime issues.

Also applies to: 1016-1034, 1049-1058, 1072-1090, 1104-1122, 1120-1345, 1361-1366, 1385-1385, 1404-1404, 1423-1423, 1441-1441, 1459-1459, 1477-1477, 1493-1493, 1510-1510, 1527-1527, 1541-1541

Verification successful

Verified: The codebase does not contain any occurrences of the outdated namespace dojo::model::ModelIndex, indicating that the updates to entity index types and namespaces have been consistently applied.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the consistent use of updated entity index types and namespaces in tests and entity management functions.

# Test: Search for outdated namespace usage in tests and entity management functions. Expect: No occurrences.
rg --type cairo 'dojo::model::ModelIndex'

Length of output: 71


Script:

#!/bin/bash
# Description: Verify the consistent use of updated entity index types and namespaces in tests and entity management functions.

# Test: Search for outdated namespace usage in tests and entity management functions. Expect: No occurrences.
rg 'dojo::model::ModelIndex'

Length of output: 28


10-10: Renaming of IConfigDispatcherImpl to IConfigDispatcherTrait

This change reflects a move towards a more idiomatic naming convention in Rust, preferring Trait as a suffix for traits rather than Impl, which is typically used for implementations. This change should be checked across the codebase to ensure consistency.

Verification successful

Renaming of IConfigDispatcherImpl to IConfigDispatcherTrait is consistent across the codebase.

No occurrences of the old trait name IConfigDispatcherImpl were found, indicating the renaming has been applied consistently.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the renaming consistency across the codebase.

# Test: Search for the old trait name. Expect: No occurrences.
rg --type cairo 'IConfigDispatcherImpl'

Length of output: 69


Script:

#!/bin/bash
# Description: Verify the renaming consistency across the codebase.

# Test: Search for the old trait name. Expect: No occurrences.
rg 'IConfigDispatcherImpl'

Length of output: 26


24-24: Introduction of GasCounterTrait

Replacing GasCounterImpl with GasCounterTrait aligns with the same naming convention change noted above. It's crucial to ensure that all references to the old name are updated to prevent runtime errors.

crates/dojo-core/src/model.cairo (4)

Line range hint 5-17: Visibility Change Approved for ModelEntity Trait

The change to make the ModelEntity trait public is consistent with the PR's objective to enhance accessibility. The trait's methods and parameters remain unchanged, which is good as it maintains the existing functionality while extending its accessibility.


Line range hint 18-59: Visibility Change Approved for Model Trait

The Model trait has been made public, aligning with the PR's goal to expose more components for external use. The methods within the trait are well-defined and seem to encapsulate the necessary functionality for model manipulation.


Line range hint 59-81: Visibility Change Approved for IModel Trait

The IModel trait's visibility change to public is approved. This change supports the PR's aim to make more elements accessible for external integration. The methods provided are essential for interacting with model data, which will now be more accessible.


Line range hint 81-90: Visibility Change Approved for deploy_and_get_metadata Function

The function deploy_and_get_metadata has been correctly updated to public visibility. This change allows external modules or packages to deploy models and retrieve metadata, which is crucial for expanding the usage of this functionality.

crates/dojo-core/src/benchmarks.cairo (6)

2-4: Refactored imports to use absolute paths.

The changes in imports to use absolute paths align with the PR's objectives to enhance accessibility and functionality. This is a good practice to avoid conflicts and improve code clarity.


12-12: Updated import to use GasCounterTrait instead of GasCounterImpl.

This change is consistent with the refactoring mentioned in the summary and improves modularity by using a trait instead of a concrete implementation.


27-27: Consistent use of GasCounterTrait across benchmarks.

The consistent replacement of GasCounterImpl with GasCounterTrait across various benchmark functions follows the principle of using interfaces over concrete implementations, enhancing modularity and testability.

Also applies to: 31-31, 45-45, 49-49, 61-61, 67-67, 71-71, 81-81, 87-87, 91-91, 120-120, 124-124, 144-144, 148-148, 183-183, 197-197, 228-228, 232-232, 259-259, 265-265, 291-291, 295-295, 346-346, 384-384, 414-414, 418-418


238-240: Visibility changes to structs and introduction of new structs.

The visibility changes to Sword and Case structs, as well as the introduction of new structs like Character, Abilities, Stats, and Weapon, are crucial for enhancing the modularity and reusability of the code. These changes align with the PR's objectives to update the project structure and improve functionality.

Also applies to: 245-249, 302-309, 313-319, 323-332, 336-336


18-18: Updated GasCounter instantiation to use the trait.

This change ensures that the code uses the new GasCounterTrait consistently. It's important to verify that the start method behaves as expected with this new trait.

Verification successful

Verified GasCounterTrait::start method implementation.

The start method initializes a GasCounter by obtaining the available gas and withdrawing it, confirming that the method behaves as expected with the new trait.

  • crates/dojo-core/src/test_utils.cairo:
    • fn start() -> GasCounter { ... }
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that `GasCounterTrait::start()` behaves as expected.

# Test: Search for the method implementation. Expect: Proper implementation details.
ast-grep --lang cairo --pattern $'trait GasCounterTrait {
  start($_) {
    $$$
  }
}'

Length of output: 198


Script:

#!/bin/bash
# Description: Verify that `GasCounterTrait::start()` behaves as expected.

# Search for the definition of `GasCounterTrait`
rg -A 10 'trait GasCounterTrait' --glob '*.cairo'

# Search for the implementation of the `start` method within the trait
rg -A 10 'fn start' --glob '*.cairo'

Length of output: 892


63-63: Updated calls to storage_access functions.

The usage of new module structure for storage_access functions is part of the updates to enhance the codebase's functionality. Ensure that these functions are correctly implemented and handle errors as expected.

Also applies to: 83-83

crates/dojo-core/src/packing_test.cairo (3)

1-2: Refactored imports to use core module.

The changes in imports to use the core module and the consolidation of traits under core::traits are good practices that enhance code clarity and avoid conflicts.

Also applies to: 6-7


47-47: Updated to use ArrayTrait::new() from core module.

The consistent use of ArrayTrait::new() from the core module across various test functions ensures that the code uses the updated paths, aligning with the PR's objectives to enhance accessibility and functionality.

Also applies to: 66-66, 89-89, 115-115, 158-158, 244-244, 279-279, 299-299, 325-325


170-170: Updated constants to use core module.

The use of contract_address_const and class_hash_const from the core module is a good practice that aligns with the PR's objectives. It's important to ensure that these constants are correctly defined in the core module.

Also applies to: 172-172

Comment on lines 23 to 24
pub fn deploy_contract(class_hash: felt252, calldata: Span<felt252>) -> ContractAddress {
let (contract, _) = starknet::syscalls::deploy_syscall(class_hash.try_into().unwrap(), 0, calldata, false)
Copy link

Choose a reason for hiding this comment

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

Visibility change approved; suggest error handling for unwraps.

The change to make deploy_contract public aligns with the PR's goal to enhance accessibility.

However, the use of unwrap() without error handling could lead to panics if conversions fail. Consider handling potential errors gracefully.

@remybar remybar force-pushed the move_to_edition_2023_11 branch 3 times, most recently from a5334fc to f403848 Compare July 17, 2024 09:50
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: 3

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 7f21b41 and f403848.

Files ignored due to path filters (4)
  • crates/dojo-core/Scarb.lock is excluded by !**/*.lock
  • crates/dojo-lang/src/manifest_test_data/compiler_cairo/Scarb.lock is excluded by !**/*.lock
  • crates/torii/types-test/Scarb.lock is excluded by !**/*.lock
  • examples/spawn-and-move/Scarb.lock is excluded by !**/*.lock
Files selected for processing (37)
  • crates/dojo-core/Scarb.toml (1 hunks)
  • crates/dojo-core/src/base.cairo (1 hunks)
  • crates/dojo-core/src/base_test.cairo (2 hunks)
  • crates/dojo-core/src/benchmarks.cairo (15 hunks)
  • crates/dojo-core/src/components.cairo (1 hunks)
  • crates/dojo-core/src/components/upgradeable.cairo (2 hunks)
  • crates/dojo-core/src/config.cairo (1 hunks)
  • crates/dojo-core/src/config/component.cairo (3 hunks)
  • crates/dojo-core/src/config/interface.cairo (1 hunks)
  • crates/dojo-core/src/contract.cairo (1 hunks)
  • crates/dojo-core/src/database.cairo (4 hunks)
  • crates/dojo-core/src/database/introspect.cairo (21 hunks)
  • crates/dojo-core/src/database/storage.cairo (9 hunks)
  • crates/dojo-core/src/database/storage_test.cairo (1 hunks)
  • crates/dojo-core/src/database/utils.cairo (2 hunks)
  • crates/dojo-core/src/database_test.cairo (1 hunks)
  • crates/dojo-core/src/interfaces.cairo (2 hunks)
  • crates/dojo-core/src/lib.cairo (2 hunks)
  • crates/dojo-core/src/model.cairo (4 hunks)
  • crates/dojo-core/src/model_test.cairo (1 hunks)
  • crates/dojo-core/src/packing.cairo (8 hunks)
  • crates/dojo-core/src/packing_test.cairo (10 hunks)
  • crates/dojo-core/src/resource_metadata.cairo (7 hunks)
  • crates/dojo-core/src/test_utils.cairo (5 hunks)
  • crates/dojo-core/src/utils.cairo (4 hunks)
  • crates/dojo-core/src/world.cairo (23 hunks)
  • crates/dojo-core/src/world_test.cairo (46 hunks)
  • crates/dojo-lang/src/interface.rs (2 hunks)
  • crates/dojo-lang/src/manifest_test_data/compiler_cairo/Scarb.toml (1 hunks)
  • crates/dojo-lang/src/model.rs (8 hunks)
  • crates/dojo-lang/src/semantics/test_data/get (3 hunks)
  • crates/dojo-lang/src/semantics/test_data/selector_from_tag (4 hunks)
  • crates/dojo-lang/src/semantics/test_data/set (3 hunks)
  • crates/dojo-world/abigen/src/main.rs (1 hunks)
  • examples/game-lib/armory/src/lib.cairo (1 hunks)
  • examples/game-lib/bestiary/src/lib.cairo (1 hunks)
  • examples/spawn-and-move/Scarb.toml (2 hunks)
Files skipped from review due to trivial changes (1)
  • crates/dojo-world/abigen/src/main.rs
Files skipped from review as they are similar to previous changes (32)
  • crates/dojo-core/Scarb.toml
  • crates/dojo-core/src/base.cairo
  • crates/dojo-core/src/base_test.cairo
  • crates/dojo-core/src/benchmarks.cairo
  • crates/dojo-core/src/components.cairo
  • crates/dojo-core/src/components/upgradeable.cairo
  • crates/dojo-core/src/config.cairo
  • crates/dojo-core/src/config/component.cairo
  • crates/dojo-core/src/config/interface.cairo
  • crates/dojo-core/src/contract.cairo
  • crates/dojo-core/src/database.cairo
  • crates/dojo-core/src/database/introspect.cairo
  • crates/dojo-core/src/database/storage.cairo
  • crates/dojo-core/src/database/storage_test.cairo
  • crates/dojo-core/src/database/utils.cairo
  • crates/dojo-core/src/database_test.cairo
  • crates/dojo-core/src/interfaces.cairo
  • crates/dojo-core/src/lib.cairo
  • crates/dojo-core/src/model.cairo
  • crates/dojo-core/src/model_test.cairo
  • crates/dojo-core/src/packing.cairo
  • crates/dojo-core/src/packing_test.cairo
  • crates/dojo-core/src/resource_metadata.cairo
  • crates/dojo-core/src/test_utils.cairo
  • crates/dojo-core/src/utils.cairo
  • crates/dojo-core/src/world.cairo
  • crates/dojo-core/src/world_test.cairo
  • crates/dojo-lang/src/interface.rs
  • crates/dojo-lang/src/manifest_test_data/compiler_cairo/Scarb.toml
  • crates/dojo-lang/src/model.rs
  • examples/game-lib/armory/src/lib.cairo
  • examples/game-lib/bestiary/src/lib.cairo
Additional comments not posted (1)
examples/spawn-and-move/Scarb.toml (1)

7-7: Approved edition update.

The update of the edition field from "2023_10" to "2023_11" aligns with the PR's objectives.

However, verify that this change does not impact other configurations or dependencies that might rely on the specific edition.

Verification successful

Verified edition update.

The update of the edition field from "2023_10" to "2023_11" aligns with the PR's objectives and has been consistently applied across the relevant files.

  • examples/spawn-and-move/Scarb.toml
  • crates/dojo-core/Scarb.toml
  • crates/dojo-lang/src/manifest_test_data/compiler_cairo/Scarb.toml

No conflicting dependencies or configurations were found.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify dependencies or configurations that might be impacted by the edition change.

# Test: Search for any references to the edition in the project.
rg --type toml 'edition' -A 3 -B 3

Length of output: 15901

Comment on lines 18 to 21
error: Plugin diagnostic: Namespace configuration expected at tool.dojo.world.namespace, but not found or invalid.
--> lib.cairo:1:1
fn test_func() { {
^*****************^
Copy link

Choose a reason for hiding this comment

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

Namespace configuration and argument validation issues identified.

Multiple errors indicate issues with missing or invalid namespace configurations and incorrect function arguments. These need to be addressed to ensure proper functionality.

- error: Plugin diagnostic: Namespace configuration expected at tool.dojo.world.namespace, but not found or invalid.
- error: Plugin diagnostic: Invalid arguments. Expected "selector_from_tag!("tag")"
+ Correct namespace configurations and ensure all function calls have the correct arguments.

Also applies to: 41-45, 69-73, 97-101

Comment on lines 21 to 25
error: Plugin diagnostic: Namespace configuration expected at tool.dojo.world.namespace, but not found or invalid.
--> lib.cairo:1:1
fn test_func() { {
^*****************^

Copy link

Choose a reason for hiding this comment

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

Namespace configuration issues and unused variable warning.

Errors related to missing or invalid namespace configurations are found throughout the file. Additionally, there's a warning about an unused variable, which suggests that some code cleanup might be necessary.

- error: Plugin diagnostic: Namespace configuration expected at tool.dojo.world.namespace, but not found or invalid.
- warning[E0001]: Unused variable. Consider ignoring by prefixing with `_`.
+ Correct namespace configurations and consider prefixing unused variables with `_` to ignore them if they are not needed.

Also applies to: 53-57, 163-197

Comment on lines 32 to 66
error: Plugin diagnostic: Namespace configuration expected at tool.dojo.world.namespace, but not found or invalid.
--> lib.cairo:1:1
use array::ArrayTrait;
^********************^

error: Plugin diagnostic: Namespace configuration expected at tool.dojo.world.namespace, but not found or invalid.
--> lib.cairo:2:1
use dojo::world::{IWorldDispatcher, IWorldDispatcherTrait};
^*********************************************************^

error: Plugin diagnostic: Namespace configuration expected at tool.dojo.world.namespace, but not found or invalid.
--> lib.cairo:4:1
#[derive(Copy, Drop, Serde, Introspect)]
^**************************************^

error: Plugin diagnostic: Namespace configuration expected at tool.dojo.world.namespace, but not found or invalid.
--> lib.cairo:10:1
fn test_func() { let key: felt252 = 0xb0b;
^****************************************^

error: Plugin diagnostic: Namespace configuration expected at tool.dojo.world.namespace, but not found or invalid.
--> lib.cairo:3:1

^

error: Plugin diagnostic: Namespace configuration expected at tool.dojo.world.namespace, but not found or invalid.
--> lib.cairo:3:1

^

error: Plugin diagnostic: Namespace configuration expected at tool.dojo.world.namespace, but not found or invalid.
--> lib.cairo:3:1

^

Copy link

Choose a reason for hiding this comment

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

Consistent namespace configuration and argument validation issues.

Errors related to missing or invalid namespace configurations and incorrect function arguments are found throughout the file. These need to be addressed to ensure proper functionality.

- error: Plugin diagnostic: Namespace configuration expected at tool.dojo.world.namespace, but not found or invalid.
- error: Plugin diagnostic: Invalid arguments. Expected "get!(world, keys, (models,))"
+ Correct namespace configurations and ensure all function calls have the correct arguments.

Also applies to: 115-149, 304-338

@remybar remybar force-pushed the move_to_edition_2023_11 branch from f403848 to de1a1de Compare July 17, 2024 11:35
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: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between f403848 and de1a1de.

Files selected for processing (48)
  • crates/dojo-core/Scarb.toml (1 hunks)
  • crates/dojo-core/src/base.cairo (1 hunks)
  • crates/dojo-core/src/base_test.cairo (2 hunks)
  • crates/dojo-core/src/benchmarks.cairo (15 hunks)
  • crates/dojo-core/src/components.cairo (1 hunks)
  • crates/dojo-core/src/components/upgradeable.cairo (2 hunks)
  • crates/dojo-core/src/config.cairo (1 hunks)
  • crates/dojo-core/src/config/component.cairo (3 hunks)
  • crates/dojo-core/src/config/interface.cairo (1 hunks)
  • crates/dojo-core/src/contract.cairo (1 hunks)
  • crates/dojo-core/src/database.cairo (4 hunks)
  • crates/dojo-core/src/database/introspect.cairo (21 hunks)
  • crates/dojo-core/src/database/storage.cairo (9 hunks)
  • crates/dojo-core/src/database/storage_test.cairo (1 hunks)
  • crates/dojo-core/src/database/utils.cairo (2 hunks)
  • crates/dojo-core/src/database_test.cairo (1 hunks)
  • crates/dojo-core/src/interfaces.cairo (2 hunks)
  • crates/dojo-core/src/lib.cairo (2 hunks)
  • crates/dojo-core/src/model.cairo (4 hunks)
  • crates/dojo-core/src/model_test.cairo (1 hunks)
  • crates/dojo-core/src/packing.cairo (8 hunks)
  • crates/dojo-core/src/packing_test.cairo (10 hunks)
  • crates/dojo-core/src/resource_metadata.cairo (7 hunks)
  • crates/dojo-core/src/test_utils.cairo (5 hunks)
  • crates/dojo-core/src/utils.cairo (4 hunks)
  • crates/dojo-core/src/world.cairo (23 hunks)
  • crates/dojo-core/src/world_test.cairo (46 hunks)
  • crates/dojo-lang/src/interface.rs (2 hunks)
  • crates/dojo-lang/src/manifest_test_data/compiler_cairo/Scarb.toml (1 hunks)
  • crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/dojo-world.toml (1 hunks)
  • crates/dojo-lang/src/model.rs (8 hunks)
  • crates/dojo-world/abigen/src/main.rs (1 hunks)
  • examples/game-lib/armory/src/lib.cairo (1 hunks)
  • examples/game-lib/bestiary/src/lib.cairo (1 hunks)
  • examples/spawn-and-move/Scarb.toml (2 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/manifest.json (6 hunks)
  • examples/spawn-and-move/manifests/dev/deployment/manifest.toml (5 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 (7 hunks)
  • examples/spawn-and-move/src/dungeon.cairo (1 hunks)
  • examples/spawn-and-move/src/lib.cairo (1 hunks)
  • examples/spawn-and-move/src/mock_token.cairo (1 hunks)
  • examples/spawn-and-move/src/models.cairo (2 hunks)
  • examples/spawn-and-move/src/others.cairo (1 hunks)
  • examples/spawn-and-move/src/utils.cairo (1 hunks)
Files skipped from review due to trivial changes (1)
  • crates/dojo-world/abigen/src/main.rs
Files skipped from review as they are similar to previous changes (46)
  • crates/dojo-core/Scarb.toml
  • crates/dojo-core/src/base.cairo
  • crates/dojo-core/src/base_test.cairo
  • crates/dojo-core/src/benchmarks.cairo
  • crates/dojo-core/src/components.cairo
  • crates/dojo-core/src/components/upgradeable.cairo
  • crates/dojo-core/src/config.cairo
  • crates/dojo-core/src/config/component.cairo
  • crates/dojo-core/src/config/interface.cairo
  • crates/dojo-core/src/contract.cairo
  • crates/dojo-core/src/database.cairo
  • crates/dojo-core/src/database/introspect.cairo
  • crates/dojo-core/src/database/storage.cairo
  • crates/dojo-core/src/database/storage_test.cairo
  • crates/dojo-core/src/database/utils.cairo
  • crates/dojo-core/src/database_test.cairo
  • crates/dojo-core/src/interfaces.cairo
  • crates/dojo-core/src/lib.cairo
  • crates/dojo-core/src/model.cairo
  • crates/dojo-core/src/model_test.cairo
  • crates/dojo-core/src/packing.cairo
  • crates/dojo-core/src/packing_test.cairo
  • crates/dojo-core/src/resource_metadata.cairo
  • crates/dojo-core/src/test_utils.cairo
  • crates/dojo-core/src/utils.cairo
  • crates/dojo-core/src/world.cairo
  • crates/dojo-core/src/world_test.cairo
  • crates/dojo-lang/src/interface.rs
  • crates/dojo-lang/src/manifest_test_data/compiler_cairo/Scarb.toml
  • crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/dojo-world.toml
  • crates/dojo-lang/src/model.rs
  • examples/game-lib/armory/src/lib.cairo
  • examples/game-lib/bestiary/src/lib.cairo
  • examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-actions-40b6994c.toml
  • examples/spawn-and-move/manifests/dev/base/dojo-world.toml
  • examples/spawn-and-move/manifests/dev/deployment/manifest.json
  • examples/spawn-and-move/manifests/dev/deployment/manifest.toml
  • examples/spawn-and-move/manifests/release/base/contracts/dojo_examples-actions-40b6994c.toml
  • examples/spawn-and-move/manifests/release/base/dojo-world.toml
  • examples/spawn-and-move/src/actions.cairo
  • examples/spawn-and-move/src/dungeon.cairo
  • examples/spawn-and-move/src/lib.cairo
  • examples/spawn-and-move/src/mock_token.cairo
  • examples/spawn-and-move/src/models.cairo
  • examples/spawn-and-move/src/others.cairo
  • examples/spawn-and-move/src/utils.cairo
Additional comments not posted (1)
examples/spawn-and-move/Scarb.toml (1)

7-7: Update of the edition field to "2023_11" is appropriate.

The change aligns with the PR's objective to move the project to edition 2023_11. It's crucial to ensure that all dependencies and project configurations are compatible with this new edition.

Please verify that all dependencies and related configurations are tested against the new edition to avoid any compatibility issues.

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: 0

Outside diff range, codebase verification and nitpick comments (1)
crates/torii/types-test/src/contracts.cairo (1)

45-45: Inconsistent Namespace Usage: Update to core::pedersen::pedersen Incomplete

Both the old (pedersen::pedersen) and new (core::pedersen::pedersen) namespaces are present in the file crates/torii/types-test/src/contracts.cairo. Ensure that all instances of the old namespace are updated to the new namespace for consistency.

  • crates/torii/types-test/src/contracts.cairo
Analysis chain

Approved: Updated namespace path for pedersen module.

The change from pedersen::pedersen to core::pedersen::pedersen correctly reflects the updated namespace structure.

Ensure that this new path is consistently used wherever the pedersen module's functions are accessed.

Also applies to: 49-49

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of the `core::pedersen` namespace throughout the codebase.

# Test: Search for the old and new namespace usage. Expect: Only occurrences of the new namespace.
rg --type cairo -A 5 $'pedersen::pedersen'
rg --type cairo -A 5 $'core::pedersen::pedersen'

Length of output: 149


Script:

#!/bin/bash
# Description: Verify the usage of the `core::pedersen` namespace throughout the codebase.

# Test: Search for the old and new namespace usage. Expect: Only occurrences of the new namespace.
rg -A 5 $'pedersen::pedersen'
rg -A 5 $'core::pedersen::pedersen'

Length of output: 1871

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between de1a1de and 909dce0.

Files selected for processing (4)
  • crates/dojo-core/src/world.cairo (23 hunks)
  • crates/torii/types-test/Scarb.toml (1 hunks)
  • crates/torii/types-test/src/contracts.cairo (1 hunks)
  • crates/torii/types-test/src/models.cairo (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • crates/dojo-core/src/world.cairo
Additional comments not posted (2)
crates/torii/types-test/Scarb.toml (1)

3-3: Approved: Addition of the edition field.

The addition of the edition = "2023_11" field correctly specifies the versioning for the package, aligning with the PR's objective to move to edition 2023_11.

crates/torii/types-test/src/models.cairo (1)

6-30: Approved: Updated field visibility in structs.

The fields in the structs Record, RecordSibling, Nested, NestedMore, NestedMost, and Subrecord have been correctly marked as public. This change likely enhances accessibility for API usage or interoperability.

Verify that making these fields public does not expose sensitive data or unintentionally breach encapsulation principles.

Also applies to: 35-38, 42-46, 50-54, 58-61, 66-72

Verification successful

Approved: Updated field visibility in structs.

The fields in the structs Record, RecordSibling, Nested, NestedMore, NestedMost, and Subrecord have been correctly marked as public. This change likely enhances accessibility for API usage or interoperability. Based on the search results, there are no indications of sensitive data exposure.

  • Record struct and related structs in crates/torii/types-test/src/models.cairo have public fields that are standard data types.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for potential exposure of sensitive data due to increased field visibility.

# Test: Search for comments or TODOs mentioning sensitive data related to these structs. Expect: No mentions of sensitive data exposure.
rg --type cairo -A 5 -B 5 $'pub .*:.*' | rg 'sensitive|private'

Length of output: 94


Script:

#!/bin/bash
# Description: Check for potential exposure of sensitive data due to increased field visibility.

# Find all `.cairo` files and search for any mentions of sensitive data exposure.
fd -e cairo | xargs rg 'sensitive|private'

Length of output: 44


Script:

#!/bin/bash
# Description: Identify all struct definitions in `.cairo` files and list their public fields.

# Find all `.cairo` files and search for struct definitions with public fields.
fd -e cairo | xargs rg -A 10 'struct .*{'

Length of output: 127


Script:

#!/bin/bash
# Description: Identify all struct definitions in `.cairo` files and list their public fields.

# Find all `.cairo` files and search for struct definitions with public fields.
fd -e cairo | xargs rg -A 10 'struct .* \{'

Length of output: 77520

@remybar remybar force-pushed the move_to_edition_2023_11 branch from 909dce0 to 3424011 Compare July 17, 2024 12:41
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: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 909dce0 and 3424011.

Files selected for processing (10)
  • crates/dojo-core/src/world.cairo (23 hunks)
  • crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/dojo-world.toml (1 hunks)
  • crates/torii/types-test/Scarb.toml (1 hunks)
  • crates/torii/types-test/src/contracts.cairo (2 hunks)
  • crates/torii/types-test/src/models.cairo (2 hunks)
  • examples/spawn-and-move/Scarb.toml (2 hunks)
  • examples/spawn-and-move/manifests/dev/base/dojo-world.toml (1 hunks)
  • examples/spawn-and-move/manifests/dev/deployment/manifest.json (6 hunks)
  • examples/spawn-and-move/manifests/dev/deployment/manifest.toml (5 hunks)
  • examples/spawn-and-move/manifests/release/base/dojo-world.toml (1 hunks)
Files skipped from review as they are similar to previous changes (7)
  • crates/dojo-core/src/world.cairo
  • crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/dojo-world.toml
  • crates/torii/types-test/src/contracts.cairo
  • crates/torii/types-test/src/models.cairo
  • examples/spawn-and-move/manifests/dev/base/dojo-world.toml
  • examples/spawn-and-move/manifests/dev/deployment/manifest.toml
  • examples/spawn-and-move/manifests/release/base/dojo-world.toml
Additional comments not posted (8)
crates/torii/types-test/Scarb.toml (1)

3-3: Approved addition of the edition field.

The addition of the edition = "2023_11" field aligns with the PR's objectives to update the project to the latest edition. This change is straightforward and should not introduce issues by itself.

However, it's important to ensure that this new edition setting does not affect other configurations or dependencies across the project.

examples/spawn-and-move/Scarb.toml (1)

7-7: Approved update of the edition field.

The update of the edition field from "2023_10" to "2023_11" is in line with the PR's objectives to keep the project up-to-date with the latest edition. This change should be safe, assuming it's consistent across all relevant configuration files.

It's advisable to verify that this edition update is consistently applied across all related configurations to avoid any potential conflicts.

Verification successful

The edition update is consistent across all relevant configurations.

The update of the edition field to "2023_11" has been applied consistently across the repository. No further issues were found.

  • examples/spawn-and-move/Scarb.toml
  • crates/torii/types-test/Scarb.toml
  • crates/dojo-lang/src/manifest_test_data/compiler_cairo/Scarb.toml
  • crates/dojo-core/Scarb.toml
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify consistency of the edition update across all related configurations.

# Test: Search for the edition setting in all TOML files to ensure consistency. Expect: No older edition settings in related configurations.
rg --type toml "edition = \"2023_10\""

Length of output: 38


Script:

#!/bin/bash
# Description: Verify the presence of the new edition setting across all related configurations.

# Test: Search for the new edition setting in all TOML files to ensure consistency. Expect: Instances of edition = "2023_11" in related configurations.
rg --type toml 'edition = "2023_11"'

Length of output: 282

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

4-5: Verify the updated class hashes for correctness.

The class hashes for WorldContract have been updated. It's crucial to verify that these new hashes are correct and correspond to the intended contract versions.


2100-2100: Check the updated address for another DojoContract.

The address for this DojoContract instance has been updated. Confirm that this change is accurately implemented across the system.

Verification successful

The updated address for the DojoContract is correctly implemented.

The new address "0x6cf6ece03c80fcddd4107270a52eab493534b8ccf8346ff17d451362a4e99d0" is present and correctly implemented for the intended instance of the DojoContract in the manifest file.

  • examples/spawn-and-move/manifests/dev/deployment/manifest.json
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check the updated address for DojoContract.

# Test: Search for the new address in the repository to verify its correct use.
rg --type json "0x6cf6ece03c80fcddd4107270a52eab493534b8ccf8346ff17d451362a4e99d0"

Length of output: 231


Script:

#!/bin/bash
# Description: Verify the consistency of the updated address for DojoContract across the system.

# Test: Search for occurrences of "DojoContract" to ensure the address is consistently updated.
rg --type json --context 5 "DojoContract"

Length of output: 4949


1192-1193: Check the updated address and transaction hash for WorldContract.

The address and transaction hash for WorldContract have been updated. Ensure these values are accurate and reflect the current deployment status of the contract.

Verification successful

The address and transaction hash for WorldContract have been verified and are accurate.

The values in the manifest.json file reflect the current deployment status of the WorldContract.

  • address: 0x62c5e1e2ceab297dfaca3ef17ca6549635272f363bd5373c888bbacdc3d8548
  • transaction_hash: 0x30d768fa29fd2ba80734946a339118c1adfea82629458b054a4d4a35103db47
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the updated address and transaction hash for WorldContract.

# Test: Search for the new address and transaction hash in the repository to confirm their validity.
rg --type json "0x62c5e1e2ceab297dfaca3ef17ca6549635272f363bd5373c888bbacdc3d8548"
rg --type json "0x30d768fa29fd2ba80734946a339118c1adfea82629458b054a4d4a35103db47"

Length of output: 468


Script:

#!/bin/bash
# Description: Search for deployment logs or scripts that mention the WorldContract to verify the latest address and transaction hash.

# Search for any deployment logs or scripts that mention WorldContract
rg --type json "WorldContract"
rg --type log "WorldContract"
rg --type sh "WorldContract"

Length of output: 330


Script:

#!/bin/bash
# Description: Refine the search to verify the latest address and transaction hash for WorldContract by including more potential sources.

# Search for the address and transaction hash in all JSON files.
rg --type json "0x62c5e1e2ceab297dfaca3ef17ca6549635272f363bd5373c888bbacdc3d8548"
rg --type json "0x30d768fa29fd2ba80734946a339118c1adfea82629458b054a4d4a35103db47"

# Search for any documentation or configuration files that might contain deployment details.
rg --type md "WorldContract"
rg --type yaml "WorldContract"

Length of output: 528


1213-1215: Confirm the updated address and class hash for DojoContract.

The address and class hash for DojoContract have been updated. Verify that these changes are correctly implemented and correspond to the actual deployed contract.


1881-1881: Validate the updated address for another DojoContract.

This change updates the address for a DojoContract. Ensure that the new address is properly reflected in all relevant parts of the project.

Verification successful

The updated address for the DojoContract has been properly integrated.

The previous address 0x2bab2a15d4c71bd8e45c84ac3b1518aeca39d3818e8e556c4a56202c1679ba6 is no longer found in the repository, confirming that all references have been updated to the new address.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Validate the updated address for DojoContract.

# Test: Check the repository for references to the new address to ensure it's correctly integrated.
rg --type json "0xcc6decf700ca5870865e95307c74cb531728f6d50670449fa33518e4a9c3c8"

Length of output: 229


Script:

#!/bin/bash
# Description: Search for the previous address of DojoContract to ensure all references are updated.

# Extract the previous address from the git history.
previous_address=$(git log -p -1 -U0 -- examples/spawn-and-move/manifests/dev/deployment/manifest.json | grep 'address' | grep -v '0xcc6decf700ca5870865e95307c74cb531728f6d50670449fa33518e4a9c3c8' | sed -n 's/.*"address": "\(.*\)",/\1/p')

# Check the repository for references to the previous address to ensure all instances are updated.
if [ -n "$previous_address" ]; then
  rg --type json "$previous_address"
else
  echo "Previous address not found in git history."
fi

Length of output: 2246


Script:

#!/bin/bash
# Description: Search for the previous address of DojoContract to ensure all references are updated.

# Extract the previous address from the git history.
previous_address=$(git log -p -1 -U0 -- examples/spawn-and-move/manifests/dev/deployment/manifest.json | grep 'address' | grep -v '0xcc6decf700ca5870865e95307c74cb531728f6d50670449fa33518e4a9c3c8' | sed -n 's/.*"address": "\(0x[0-9a-fA-F]\+\)",/\1/p' | head -n 1)

# Check the repository for references to the previous address to ensure all instances are updated.
if [ -n "$previous_address" ]; then
  rg --type json "$previous_address"
else
  echo "Previous address not found in git history."
fi

Length of output: 493


1644-1644: Review the updated address for another DojoContract.

The address for this instance of DojoContract has been updated. It's important to check that this new address is being used correctly within the system.

Copy link
Collaborator

@glihm glihm left a comment

Choose a reason for hiding this comment

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

Nice @remybar! First good move toward refactoring with the visibility, will makes things clearer. :)

@glihm glihm merged commit efffe5c into dojoengine:main Jul 17, 2024
14 of 15 checks passed
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