fix(dojo-core): adjust ACL to restore governance without relaxing permissions

[glihm]

Description

In the previous rework on ACLs, we added a constraint to have some functions only callable from an account.

This PR ensures that we are keeping important restriction on permissions granting functions, without relying on the caller being an account.

This restores governance for Dojo worlds.

Summary by CodeRabbit

• New Features

  • Enhanced clarity in panic messages for various contract tests, aiding in debugging and understanding failure contexts.
  • New functions added to improve error handling related to contract ownership and initialization.

• Improvements

  • Streamlined logic for caller validation and contract initialization, enforcing stricter ownership checks.
  • Modifications to test function names for better readability, emphasizing their purpose and constraints.

• Configuration Changes

  • Updated class hashes and addresses across multiple manifest files, indicating potential redeployments of related contracts.

• Tests

  • Adjusted expected values in several test cases to align with updated contract behaviors.

[coderabbitai]

Walkthrough

Ohayo, sensei! The changes primarily involve updates to test functions, enhancing contract initialization and ownership checks. New tests were added to enforce ownership constraints, and existing functions were renamed for clarity. Additionally, the logic in the contract implementation was refined to focus on resource ownership rather than account checks. Manifest files were modified to reflect updated class identifiers, ensuring consistency in contract references.

Changes

File	Change Summary
crates/dojo-core/src/tests/world/world.cairo	Enhanced test functions focusing on contract initialization and ownership checks. Renamed and added tests to ensure only the world and owner can initialize contracts.
crates/dojo-core/src/world/world_contract.cairo	Removed account checks in favor of resource owner checks and added ownership validation during contract operations. Simplified error handling and improved initialization logic.
crates/dojo-lang/src/contract.rs	Made the IDojoInit trait and its implementation public. Updated method signatures for better flexibility and error handling improvements.
crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/dojo-world.toml, examples/spawn-and-move/manifests/dev/base/dojo-world.toml, examples/spawn-and-move/manifests/release/base/dojo-world.toml	Changed class_hash and original_class_hash values to new identifiers, reflecting updates in class definitions.
examples/spawn-and-move/dojo_dev.toml	Updated world_address configuration to a new hexadecimal address.
examples/spawn-and-move/manifests/dev/deployment/manifest.json, examples/spawn-and-move/manifests/dev/deployment/manifest.toml	Modified various address and transaction_hash fields for world and DojoContract entries, indicating reconfiguration or migration of contracts.

Sequence Diagram(s)

sequenceDiagram
    participant Caller
    participant Contract

    Caller->>Contract: Initiate contract operation
    alt Caller is resource owner
        Contract-->>Caller: Operation successful
    else Caller is not resource owner
        Contract-->>Caller: Panic: Caller is not the owner
    end

Loading

---

Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between b782b15 and a1446e5.

Files ignored due to path filters (2)

• spawn-and-move-db.tar.gz is excluded by !**/*.gz
• types-test-db.tar.gz is excluded by !**/*.gz

Files selected for processing (19)

• crates/dojo-core/src/tests/helpers.cairo (1 hunks)
• crates/dojo-core/src/tests/world/world.cairo (2 hunks)
• crates/dojo-core/src/world/world_contract.cairo (23 hunks)
• crates/dojo-lang/src/contract.rs (3 hunks)
• crates/dojo-lang/src/manifest_test_data/compiler_cairo/manifests/dev/base/dojo-world.toml (1 hunks)
• crates/sozo/ops/src/tests/model.rs (1 hunks)
• examples/spawn-and-move/dojo_dev.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/contracts/dojo_examples-dungeon-6620e0e6.toml (1 hunks)
• examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-mock_token-31599eb2.toml (1 hunks)
• examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-others-61de2c18.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/contracts/dojo_examples-dungeon-6620e0e6.toml (1 hunks)
• examples/spawn-and-move/manifests/release/base/contracts/dojo_examples-mock_token-31599eb2.toml (1 hunks)
• examples/spawn-and-move/manifests/release/base/contracts/dojo_examples-others-61de2c18.toml (1 hunks)
• examples/spawn-and-move/manifests/release/base/dojo-world.toml (1 hunks)

Files skipped from review as they are similar to previous changes (6)

• crates/sozo/ops/src/tests/model.rs
• examples/spawn-and-move/dojo_dev.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/dojo-world.toml

Additional comments not posted (25)

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

2-3: Ohayo, sensei! Verify the integrity of updated class hashes.

The class_hash and original_class_hash have been updated. Ensure these changes align with the intended class updates and verify their consistency across related files.

Run the following script to check for consistency across the repository:

Verification successful

---

Ohayo, sensei! The class hashes are consistent across the repository.

The class_hash and original_class_hash have been updated consistently across several manifest files, indicating alignment with the intended updates.

• Files with consistent hashes:
  • examples/spawn-and-move/manifests/dev/deployment/manifest.toml
  • examples/spawn-and-move/manifests/release/base/dojo-world.toml
  • 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

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the consistency of updated class hashes across the repository.

# Test: Search for occurrences of the new class hashes. Expect: Consistent usage across related files.
rg --type toml '0xa349b743d361ce4567361475a89b84a386bb383448c6926954e5fe0b525597'

Length of output: 1307

examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-others-61de2c18.toml (1)

2-3: Ohayo, sensei! Ensure updated contract hashes are consistent.

The class_hash and original_class_hash have been updated. Verify that these changes reflect the intended contract updates and maintain consistency across related files.

Run the following script to verify the consistency of the new contract hashes:

Verification successful

---

Ohayo, sensei! Contract hashes are consistent across files.

The updated contract hashes are consistently used across the related .toml files in the repository. No inconsistencies were found.

• examples/spawn-and-move/manifests/dev/deployment/manifest.toml
• examples/spawn-and-move/manifests/release/base/contracts/dojo_examples-others-61de2c18.toml
• examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-others-61de2c18.toml

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the consistency of updated contract hashes across the repository.

# Test: Search for occurrences of the new contract hashes. Expect: Consistent usage across related files.
rg --type toml '0x40e824b8814bafef18cce2cf68f5765e9c9a1c86f55a8491b0c2a4faebdcc87'

Length of output: 1081

examples/spawn-and-move/manifests/release/base/contracts/dojo_examples-others-61de2c18.toml (1)

2-3: Ohayo, sensei! Verify the integrity of updated release contract hashes.

The class_hash and original_class_hash have been updated. Ensure these changes align with the intended contract updates and verify their consistency across related files.

Run the following script to check for consistency of the new release contract hashes:

Verification successful

---

Ohayo, sensei! The updated release contract hashes are consistent. The class_hash and original_class_hash are identical across all related TOML files, confirming their alignment with the intended contract updates.

• examples/spawn-and-move/manifests/dev/deployment/manifest.toml
• examples/spawn-and-move/manifests/release/base/contracts/dojo_examples-others-61de2c18.toml
• examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-others-61de2c18.toml

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the consistency of updated release contract hashes across the repository.

# Test: Search for occurrences of the new release contract hashes. Expect: Consistent usage across related files.
rg --type toml '0x40e824b8814bafef18cce2cf68f5765e9c9a1c86f55a8491b0c2a4faebdcc87'

Length of output: 1081

examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-dungeon-6620e0e6.toml (1)

2-3: Ohayo, sensei! Verify consistency of class hash updates.

The class_hash and original_class_hash have been updated. Ensure these changes are consistently applied across all relevant configuration files and documentation.

Run the following script to check for consistency:

Verification successful

---

Ohayo, sensei! Class hash updates are consistent.

The new class hash 0x4590a27e4ec7366358ba5f60323777f301435ebbdd113ab02c54b947717530d is consistently applied across all relevant configuration files, and there are no occurrences of the old hash. Everything looks good!

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify consistency of class hash updates across related files.

# Test: Search for the old and new class hashes. Expect: Only occurrences of the new hash.
rg --type toml '0x5d2892f0389e921a051daaad07efb49af7a13213ba309a901fc386acef15c3c|0x4590a27e4ec7366358ba5f60323777f301435ebbdd113ab02c54b947717530d'

Length of output: 1153

examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-mock_token-31599eb2.toml (1)

2-3: Ohayo, sensei! Verify consistency of class hash updates.

The class_hash and original_class_hash have been updated. Ensure these changes are consistently applied across all relevant configuration files and documentation.

Run the following script to check for consistency:

Verification successful

---

Ohayo, sensei! Class hash updates are consistent.

The class_hash and original_class_hash are consistently applied across all relevant configuration files. No discrepancies were found.

• examples/spawn-and-move/manifests/release/base/contracts/dojo_examples-mock_token-31599eb2.toml
• examples/spawn-and-move/manifests/dev/deployment/manifest.toml
• examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-mock_token-31599eb2.toml

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify consistency of class hash updates across related files.

# Test: Search for the old and new class hashes. Expect: Only occurrences of the new hash.
rg --type toml '0x67edb33671cd2f5b766d073e3dec53b03400761a20f349ea9628cf4c883b393'

Length of output: 1097

examples/spawn-and-move/manifests/release/base/contracts/dojo_examples-dungeon-6620e0e6.toml (1)

2-3: Ohayo, sensei! Verify consistency of class hash updates.

The class_hash and original_class_hash have been updated. Ensure these changes are consistently applied across all relevant configuration files and documentation.

Run the following script to check for consistency:

Verification successful

---

Ohayo, sensei! Class hash updates are consistent.

The new class hash 0x4590a27e4ec7366358ba5f60323777f301435ebbdd113ab02c54b947717530d is consistently applied across all relevant configuration files, and the old hash is not present.

• examples/spawn-and-move/manifests/release/base/contracts/dojo_examples-dungeon-6620e0e6.toml
• examples/spawn-and-move/manifests/dev/deployment/manifest.toml
• examples/spawn-and-move/manifests/dev/base/contracts/dojo_examples-dungeon-6620e0e6.toml

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify consistency of class hash updates across related files.

# Test: Search for the old and new class hashes. Expect: Only occurrences of the new hash.
rg --type toml '0x5d2892f0389e921a051daaad07efb49af7a13213ba309a901fc386acef15c3c|0x4590a27e4ec7366358ba5f60323777f301435ebbdd113ab02c54b947717530d'

Length of output: 1153

examples/spawn-and-move/manifests/release/base/contracts/dojo_examples-mock_token-31599eb2.toml (1)

2-3: Ohayo, sensei! Verify the impact of updated class hashes.

The class_hash and original_class_hash have been updated. Ensure that these changes are reflected in all relevant parts of the system to avoid inconsistencies.

Run the following script to verify the usage of the new class hashes:

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

2-3: Ohayo, sensei! Verify the impact of updated class hashes.

The class_hash and original_class_hash have been updated. Ensure that these changes are reflected in all relevant parts of the system to avoid inconsistencies.

Run the following script to verify the usage of the new class hashes:

Verification successful

---

Ohayo, sensei! The class hashes have been updated consistently across the codebase. The new hashes appear in both TOML and JSON files for different environments, ensuring consistency and correctness.

• 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
• examples/spawn-and-move/manifests/dev/deployment/manifest.toml
• examples/spawn-and-move/manifests/dev/deployment/manifest.json

No further occurrences were found, confirming the update is complete.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of the updated class hashes in the codebase.

# Test: Search for the new class hash. Expect: All occurrences of the new hash are intentional and correct.
rg --type toml '0x31df0c74e76e64d5701a806c62961a7ca62eda8978666e14c34497b48a7a501'

Length of output: 1085

---

Script:

#!/bin/bash
# Description: Search for the updated class hashes in the entire codebase to ensure consistency.

# Test: Search for the new class hash in all file types.
rg '0x31df0c74e76e64d5701a806c62961a7ca62eda8978666e14c34497b48a7a501'

Length of output: 1386

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

2-3: Ohayo, sensei! Verify the impact of updated class hashes.

The class_hash and original_class_hash have been updated. Ensure that these changes are reflected in all relevant parts of the system to avoid inconsistencies.

Run the following script to verify the usage of the new class hashes:

Verification successful

---

Ohayo, sensei! The updated class hashes are consistently reflected across the codebase. The changes are present in both TOML and JSON manifest files for dev and release environments, indicating intentional updates. No inconsistencies were found.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of the updated class hashes in the codebase.

# Test: Search for the new class hash. Expect: All occurrences of the new hash are intentional and correct.
rg --type toml '0x31df0c74e76e64d5701a806c62961a7ca62eda8978666e14c34497b48a7a501'

Length of output: 1085

---

Script:

#!/bin/bash
# Description: Search for the updated class hash in all file types to verify its usage across the codebase.

# Test: Search for the new class hash in all files. Expect: All occurrences of the new hash are intentional and correct.
rg '0x31df0c74e76e64d5701a806c62961a7ca62eda8978666e14c34497b48a7a501'

Length of output: 1386

crates/dojo-core/src/tests/world/world.cairo (4)

344-351: Verify panic message accuracy.

The test test_can_call_init_only_world checks for a specific panic message. Ensure the message matches the actual implementation to avoid false positives.

---

361-370: Verify ownership logic.

The test test_can_call_init_only_owner ensures only the owner can initialize the contract. Double-check the implementation logic to ensure the test accurately reflects the intended behavior.

---

404-413: Verify argument handling in dojo_init.

The test test_can_call_init_only_world_args checks the dojo_init function with arguments. Verify that the function correctly handles arguments and the panic message is accurate.

---

422-443: Ensure state upgrade restrictions are correct.

The test test_upgrade_state_not_owner checks that only the world owner can upgrade the state. Verify that the logic aligns with the intended security measures.

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

133-147: Visibility change to pub is appropriate.

Making the IDojoInit trait and its implementation public enhances modularity and usability, allowing other modules to interact with them. Ensure this change aligns with the overall architecture.

---

301-308: Use of DOJO_INIT_FN improves maintainability.

Replacing the hardcoded function name with a variable enhances flexibility and maintainability. This is a good practice for managing function names.

---

331-333: Improved error handling with panic.

Switching from a simple assertion to a panic mechanism provides clearer error reporting, which is beneficial for debugging and user feedback.

crates/dojo-core/src/world/world_contract.cairo (9)

364-364: Ohayo, sensei! LGTM on the permission update.

The shift to assert_caller_is_resource_owner aligns well with the new permission model focusing on resource ownership. Nice work!

---

408-408: Ohayo, sensei! Permission check looks solid.

Switching to assert_caller_is_resource_owner ensures that only resource owners can grant ownership, maintaining security integrity.

---

429-429: Ohayo, sensei! Revoke logic is on point.

Using assert_caller_is_resource_owner ensures that only resource owners can revoke ownership, which is crucial for maintaining control.

---

464-464: Ohayo, sensei! Writer permission logic is well-implemented.

The use of assert_caller_is_resource_owner ensures that only resource owners can grant writer permissions, enhancing security.

---

485-485: Ohayo, sensei! Revoke writer logic is secure.

Switching to assert_caller_is_resource_owner ensures that only resource owners can revoke writer permissions, maintaining proper access control.

---

517-517: Ohayo, sensei! Namespace write access check is spot on.

Using assert_caller_namespace_write_access ensures that the caller has the necessary permissions to register a model, aligning with the new security model.

---

545-545: Ohayo, sensei! Upgrade model logic is consistent.

The use of assert_caller_namespace_write_access ensures that the caller has the necessary permissions to upgrade a model, maintaining security.

---

642-642: Ohayo, sensei! Contract deployment logic is secure.

Using assert_caller_namespace_write_access ensures that the caller has the necessary permissions to deploy a contract, aligning with the new security model.

---

669-669: Ohayo, sensei! Contract upgrade logic is well-guarded.

Switching to assert_caller_is_resource_owner ensures that only resource owners can upgrade contracts, maintaining proper access control.

---

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.

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.

[coderabbitai]

Actionable comments posted: 0

Outside diff range, codebase verification and nitpick comments (1)

crates/dojo-core/src/world/errors.cairo (1)

67-69: Clarify the error message in no_world_owner.

The placeholder {} in the error message could be confusing. Consider specifying the action or resource involved.

Apply this diff to enhance clarity:

 format!("Caller `{:?}` can't perform action on `{}` (not world owner)", caller, target)

[coderabbitai]

Actionable comments posted: 0

Outside diff range, codebase verification and nitpick comments (1)

crates/dojo-core/src/tests/helpers.cairo (1)

47-52: Ensure proper use of the UUID.

The dojo_init function retrieves the UUID from the world instance but doesn't use it. Ensure this is intentional and consider adding comments for clarity.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.25%. Comparing base (a3ddf39) to head (a1446e5).
Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2341      +/-   ##
==========================================
- Coverage   67.26%   67.25%   -0.02%     
==========================================
  Files         357      357              
  Lines       46634    46662      +28     
==========================================
+ Hits        31367    31381      +14     
- Misses      15267    15281      +14     

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