expose resources through resource() function in world API #2266
Conversation
WalkthroughOhayo, sensei! The recent updates greatly enhance resource management in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WorldContract
participant Resource
User->>WorldContract: requestResource(selector)
WorldContract->>Resource: retrieveResource(selector)
Resource-->>WorldContract: return Resource
WorldContract-->>User: return Resource
This sequence diagram illustrates how the user interacts with the Recent review detailsConfiguration used: .coderabbit.yaml Files selected for processing (1)
Files skipped from review as they are similar to previous changes (1)
TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Additionally, you can add CodeRabbit Configuration File (
|
remybar
force-pushed
the
expose_resources_in_world_api
branch
from
7a9c1aa
to
11bffa7
Compare
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
crates/dojo-core/src/world/world_contract.cairo (1)
45-45: Ohayo sensei!The addition of the
resourcemethod looks good, but it seems the removal of themodelandcontractmethods has not been fully propagated throughout the codebase. There are still multiple occurrences of these methods:
modelmethod references:
crates/torii/core/src/processors/store_update_record.rscrates/torii/core/src/sql.rscrates/torii/core/src/processors/store_set_record.rscrates/torii/core/src/processors/store_update_member.rscrates/torii/libp2p/src/tests.rscrates/torii/core/src/processors/store_del_record.rscrates/torii/libp2p/src/server/mod.rscrates/torii/core/src/processors/event_message.rscrates/torii/core/src/processors/register_model.rscrates/torii/graphql/src/tests/mod.rscrates/torii/graphql/src/tests/subscription_test.rscrates/torii/client/src/client/storage.rscrates/dojo-world/src/contracts/model_test.rscrates/torii/client/src/client/mod.rscrates/torii/client/src/client/subscription.rscrates/dojo-types/src/lib.rscrates/dojo-core/src/tests/utils.cairocrates/dojo-core/src/tests/model/model.cairocrates/dojo-core/src/tests/world.cairocrates/dojo-core/src/tests/base.cairocrates/dojo-lang/src/plugin_test_data/modelcrates/dojo-lang/src/plugin.rscrates/dojo-lang/src/semantics/utils.rscrates/dojo-core/src/world/world_contract.cairocrates/dojo-bindgen/src/plugins/typescript/tests.rscrates/dojo-bindgen/src/plugins/unity/mod.rscrates/dojo-bindgen/src/plugins/typescript/mod.rs
contractmethod references:
crates/sozo/ops/src/migration/migrate.rscrates/katana/contracts/test_contract.cairocrates/katana/rpc/rpc/tests/starknet.rscrates/katana/rpc/rpc/src/starknet/mod.rscrates/katana/storage/provider/tests/contract.rscrates/katana/storage/provider/src/lib.rscrates/katana/storage/provider/src/traits/state.rscrates/katana/storage/provider/src/providers/fork/mod.rscrates/katana/storage/provider/src/providers/fork/state.rscrates/katana/storage/provider/src/providers/fork/backend.rscrates/katana/storage/provider/src/providers/db/state.rscrates/katana/storage/provider/src/providers/in_memory/mod.rscrates/katana/storage/provider/src/providers/in_memory/state.rscrates/katana/executor/tests/executor.rscrates/katana/core/src/backend/storage.rscrates/katana/executor/src/abstraction/mod.rscrates/katana/executor/src/implementation/noop.rscrates/katana/executor/src/implementation/blockifier/state.rscrates/dojo-world/src/contracts/world_test.rscrates/dojo-bindgen/src/plugins/unity/mod.rscrates/dojo-core/src/contract/upgradeable.cairocrates/dojo-core/src/utils/test.cairocrates/dojo-core/src/tests/world.cairocrates/dojo-lang/src/plugin_test_data/systemcrates/benches/src/deployer.rscrates/benches/contracts/src/tests/test_world.cairoexamples/spawn-and-move/src/actions.cairoPlease ensure all references to these methods are updated to reflect the new
resourcemethod.Analysis chain
LGTM! But verify the removal of old methods.
The addition of the
resourcemethod improves the interface by consolidating resource retrieval into a single method.However, ensure that all references to the removed
modelandcontractmethods are updated in the codebase.
Ohayo sensei!
The addition of the
resourcemethod looks good, but we need to verify the complete removal of the oldmodelandcontractmethods. The previous attempt to search for these methods failed due to a regex error. Let's correct this and search across all relevant files.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the removal of the `model` and `contract` methods. # Test: Search for the usage of the removed methods. Expect: No occurrences. rg --type python -A 5 $'model(' rg --type python -A 5 $'contract('Length of output: 200
Script:
#!/bin/bash # Description: Verify the removal of the `model` and `contract` methods. # Test: Search for the usage of the removed methods. Expect: No occurrences. rg 'model\(' -A 5 rg 'contract\(' -A 5Length of output: 125172
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2266 +/- ##
==========================================
- Coverage 69.94% 66.88% -3.06%
==========================================
Files 342 342
Lines 44990 45149 +159
==========================================
- Hits 31468 30199 -1269
- Misses 13522 14950 +1428 ☔ View full report in Codecov by Sentry. |
remybar
force-pushed
the
expose_resources_in_world_api
branch
2 times, most recently
from
d791753
to
a84949e
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (3)
examples/spawn-and-move/manifests/dev/base/abis/dojo-world.json (1)
387-402: Ohayo, sensei!It looks like there are still numerous references to the old
modelandcontractfunctions scattered throughout the codebase. These need to be updated to the newresourcefunction to ensure consistency and correctness.Please update the following references to use the new
resourcefunction:
modelreferences:
crates/torii/libp2p/src/tests.rscrates/torii/libp2p/src/server/mod.rscrates/torii/graphql/src/tests/mod.rscrates/torii/graphql/src/tests/subscription_test.rscrates/torii/core/src/sql.rscrates/torii/core/src/processors/store_update_member.rscrates/torii/core/src/processors/store_update_record.rscrates/torii/core/src/processors/store_del_record.rscrates/torii/core/src/processors/store_set_record.rscrates/torii/core/src/processors/event_message.rscrates/torii/core/src/processors/register_model.rscrates/dojo-world/src/contracts/model_test.rscrates/dojo-lang/src/semantics/utils.rscrates/dojo-lang/src/plugin_test_data/modelcrates/dojo-lang/src/plugin.rscrates/dojo-types/src/lib.rscrates/dojo-core/src/world/world_contract.cairocrates/dojo-core/src/utils/test.cairocrates/dojo-core/src/tests/utils.cairocrates/dojo-core/src/tests/world.cairocrates/dojo-core/src/tests/base.cairocrates/dojo-core/src/tests/model/model.cairocrates/dojo-bindgen/src/plugins/unity/mod.rscrates/dojo-bindgen/src/plugins/typescript/tests.rscrates/dojo-bindgen/src/plugins/typescript/mod.rs
contractreferences:
examples/spawn-and-move/src/actions.cairocrates/sozo/ops/src/migration/migrate.rscrates/dojo-world/src/contracts/world_test.rscrates/katana/storage/provider/src/traits/state.rscrates/katana/storage/provider/src/providers/in_memory/mod.rscrates/katana/storage/provider/src/providers/db/mod.rscrates/katana/storage/provider/src/providers/db/state.rscrates/katana/storage/provider/src/lib.rscrates/katana/storage/provider/src/providers/fork/state.rscrates/katana/storage/provider/src/providers/fork/mod.rscrates/katana/storage/provider/src/providers/fork/backend.rscrates/katana/storage/db/benches/codec.rscrates/katana/storage/provider/tests/contract.rscrates/katana/storage/provider/src/providers/in_memory/state.rscrates/katana/rpc/rpc/tests/starknet.rscrates/katana/runner/src/lib.rscrates/katana/rpc/rpc/src/starknet/mod.rscrates/katana/executor/tests/executor.rscrates/katana/executor/src/implementation/noop.rscrates/katana/executor/src/implementation/blockifier/state.rscrates/katana/core/src/backend/storage.rscrates/katana/executor/src/abstraction/mod.rscrates/katana/contracts/test_contract.cairocrates/dojo-lang/src/plugin_test_data/systemcrates/dojo-core/src/contract/upgradeable.cairocrates/dojo-core/src/world/world_contract.cairocrates/dojo-core/src/utils/test.cairocrates/dojo-bindgen/src/plugins/unity/mod.rscrates/dojo-core/src/tests/world.cairocrates/dojo-core/src/tests/base.cairocrates/benches/src/deployer.rscrates/benches/contracts/src/tests/test_world.cairoAnalysis chain
LGTM! But verify the function usage in the codebase.
The introduction of the
resourcefunction is appropriate and aligns with the newResourceenum.However, ensure that all references to the old
modelandcontractfunctions are updated accordingly.
Ohayo, sensei!
The initial check didn't show direct references to the
modelandcontractfunctions, but there are somecall_contractreferences that might need further investigation. Let's dig deeper to ensure all references to the old functions are properly updated.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all references to the removed `model` and `contract` functions are updated. # Test: Search for the function usage. Expect: No occurrences of the removed functions. rg --type json -e 'model\(' -e 'contract\('Length of output: 368
Script:
#!/bin/bash # Description: Verify all references to the removed `model` and `contract` functions are updated. # Search for the function definitions and usages of `model` and `contract` in the codebase. rg 'model\(' rg 'contract\('Length of output: 26913
crates/dojo-core/src/world/world_contract.cairo (1)
45-45: Ohayo sensei!It looks like there are still several references to the old
modelandcontractfunctions in the codebase. These need to be updated to use the newresourcefunction. Here are the locations that need attention:
examples/spawn-and-move/src/actions.cairocrates/katana/contracts/test_contract.cairocrates/dojo-core/src/utils/test.cairocrates/dojo-core/src/world/world_contract.cairocrates/dojo-core/src/tests/world.cairocrates/dojo-core/src/tests/utils.cairocrates/dojo-core/src/tests/model/model.cairocrates/dojo-core/src/tests/base.cairocrates/dojo-core/src/contract/upgradeable.cairocrates/benches/contracts/src/tests/test_world.cairoPlease update these references to ensure consistency with the new
resourcefunction.Analysis chain
LGTM! But verify the function usage in the codebase.
The introduction of the
resourcefunction in theIWorldtrait is appropriate and aligns with the newResourceenum.However, ensure that all references to the old
modelandcontractfunctions are updated accordingly.
Ohayo sensei!
The initial verification script encountered an issue due to an unrecognized file type for Cairo. Let's refine our approach to ensure we can accurately verify the removal of
modelandcontractfunction references.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all references to the removed `model` and `contract` functions are updated. # Test: Search for the function usage. Expect: No occurrences of the removed functions. rg --type cairo -e 'model\(' -e 'contract\('Length of output: 76
Script:
#!/bin/bash # Description: Verify all references to the removed `model` and `contract` functions are updated. # Find all Cairo files and search for the function usage. fd -e cairo | xargs rg -e 'model\(' -e 'contract\('Length of output: 11928
examples/spawn-and-move/manifests/dev/deployment/manifest.json (1)
392-407: Ohayo, sensei! There are inconsistencies in theresourcefunction definitions.The new definition of the
resourcefunction should have inputsselector: core::felt252and outputsdojo::world::world_contract::Resource. However, some instances have different inputs and outputs. Please review and update the following occurrences to match the new definition:
examples/spawn-and-move/manifests/dev/deployment/manifest.json
- Inputs:
selector: core::felt252,address: core::starknet::contract_address::ContractAddress- Outputs:
core::bool- State Mutability:
externalexamples/spawn-and-move/manifests/dev/deployment/abis/dojo-world.json
- Inputs:
selector: core::felt252,address: core::starknet::contract_address::ContractAddress- Outputs:
core::bool- State Mutability:
externalexamples/game-lib/manifests/dev/base/abis/dojo-world.json
- Inputs:
selector: core::felt252- Outputs:
core::bool- State Mutability:
viewbin/sozo/tests/test_data/manifest.json
- Inputs:
selector: core::felt252- Outputs:
core::array::Span::<core::felt252>- State Mutability:
viewPlease ensure all instances of the
resourcefunction match the new definition.Analysis chain
Ohayo, sensei! The new
resourcefunction looks good.The addition of the
resourcefunction with inputselector: core::felt252and outputdojo::world::world_contract::Resourceis well-defined.However, ensure that all function calls to
resourcematch the new definition.
Ohayo, sensei! The
resourcefunction has been found in multiple files. To ensure all instances match the new definition, let's verify the inputs and outputs of theresourcefunction across the codebase.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `resource` match the new definition. # Test: Search for the function usage. Expect: Only occurrences of the new definition. rg --type json -A 5 $'"name": "resource"'Length of output: 46317
Script:
#!/bin/bash # Description: Verify the consistency of the `resource` function definition across the codebase. # Test: Search for the function definition. Expect: Consistent inputs and outputs. rg --type json -A 10 $'"name": "resource"' | grep -E -A 10 '"name": "resource"|inputs|outputs'Length of output: 81627
remybar
force-pushed
the
expose_resources_in_world_api
branch
from
a84949e
to
f1e3dfe
Compare
remybar
force-pushed
the
expose_resources_in_world_api
branch
from
f1e3dfe
to
9856856
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
xtask/generate-test-db/src/main.rs (1)
60-61: Ohayo, sensei! Directory cleanup looks solid.Using
fs::remove_dir_allensures a clean state, which is crucial for preventing conflicts with old data.Consider adding a comment to clarify why errors are ignored, enhancing code readability.
// Ignore errors as the directories may not exist. let _ = fs::remove_dir_all(&spawn_and_move_compressed_path); let _ = fs::remove_dir_all(&types_test_compressed_path);
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (1)
scripts/build_spawn_and_move.sh (1)
1-1: Ensure the script has execute permissions.The shebang line is correct for a bash script. Make sure this script has execute permissions set in the repository.
chmod +x scripts/build_spawn_and_move.sh
scripts/build_spawn_and_move.sh
Outdated
| $sozo_path build --manifest-path examples/spawn-and-move/Scarb.toml | ||
| $sozo_path build --manifest-path examples/spawn-and-move/Scarb.toml -P release |
There was a problem hiding this comment.
Consider handling build errors.
The build commands do not currently handle errors. Consider adding error handling to improve robustness.
$sozo_path build --manifest-path examples/spawn-and-move/Scarb.toml || { echo "Build failed"; exit 1; }
$sozo_path build --manifest-path examples/spawn-and-move/Scarb.toml -P release || { echo "Build failed"; exit 1; }
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range, codebase verification and nitpick comments (1)
crates/torii/types-test/src/contracts.cairo (1)
124-124: Remove or complete the implementation comment.The comment
// Implemment fn delete, input param: record_idseems incomplete or unnecessary. Consider removing it or expanding it to provide meaningful context.- // Implemment fn delete, input param: record_id
| #!/bin/bash | ||
|
|
||
| # usage: bash scripts/build_cairo_projects.sh <sozo_path> | ||
| sozo_path=$1 |
There was a problem hiding this comment.
Validate the input path.
Ohayo, sensei! Ensure the script checks if the sozo_path is provided and valid before proceeding.
if [ -z "$sozo_path" ]; then
echo "Error: sozo_path is not provided."
exit 1
fi
if [ ! -x "$sozo_path" ]; then
echo "Error: sozo_path is not executable."
exit 1
fi| $sozo_path build --manifest-path examples/spawn-and-move/Scarb.toml | ||
| $sozo_path build --manifest-path examples/spawn-and-move/Scarb.toml -P release | ||
| $sozo_path build --manifest-path crates/torii/types-test/Scarb.toml |
There was a problem hiding this comment.
Add error handling for build commands.
Consider adding error handling to ensure that each build command executes successfully.
$sozo_path build --manifest-path examples/spawn-and-move/Scarb.toml || { echo "Build failed for spawn-and-move."; exit 1; }
$sozo_path build --manifest-path examples/spawn-and-move/Scarb.toml -P release || { echo "Build failed for spawn-and-move release."; exit 1; }
$sozo_path build --manifest-path crates/torii/types-test/Scarb.toml || { echo "Build failed for types-test."; exit 1; }
See #2244.
Summary by CodeRabbit
New Features
Resourceenum to enhance resource management and accessibility in the world module.resourcefunction to replace the previousmodelandcontractfunctions, simplifying resource retrieval.build_cairo_projects.shscript.Bug Fixes
Documentation
Chores