Fix assert caller permission - #3254
Conversation
|
""" WalkthroughOhayo sensei! This update revises expected panic messages across multiple test files to use explicit, descriptive error strings with hexadecimal addresses, enhancing clarity around access control failures. It also adjusts error formatting in the core world contract logic to match these changes, but does not alter any test logic or exported interfaces. Changes
Sequence Diagram(s)sequenceDiagram
participant TestCase
participant WorldContract
participant Syscall
participant DojoContract
TestCase->>WorldContract: Call restricted function as non-owner/writer
WorldContract->>Syscall: call_contract_syscall("dojo_name")
alt Syscall returns Ok
Syscall-->>WorldContract: Contract name (serialized)
WorldContract->>WorldContract: Deserialize and format error message
else Syscall returns Error
Syscall-->>WorldContract: Error
WorldContract->>WorldContract: Fallback to hex address in error message
end
WorldContract-->>TestCase: Panic with descriptive error string
Possibly related PRs
📜 Recent review detailsConfiguration used: .coderabbit.yaml ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches🧪 Generate Unit Tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Currently, Katana uses the new RPC types, without bumping the spec version. To ensure we can still use sozo with sepolia/mainnet and Katana, Sozo will not check the RPC version for now
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
bin/sozo/src/utils.rs(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: fmt
🔇 Additional comments (2)
bin/sozo/src/utils.rs (2)
35-35: Ohayo sensei! Good practice marking unused constant.The underscore prefix properly indicates this constant is temporarily unused, following Rust conventions.
235-235: Ohayo sensei! Proper dead code handling.Good practice using
#[allow(dead_code)]to suppress warnings for the temporarily unused function rather than removing it, since it will be needed when the version check is re-enabled.
| // TODO: @remybar @glihm currently Katana is using new types, but doesn't | ||
| // return the correct spec version. We comment this test for now to ensure | ||
| // one can deploy on sepolia/mainnet but also Katana. | ||
| // if !is_compatible_version(&spec_version, RPC_SPEC_VERSION)? { | ||
| // return Err(anyhow!( | ||
| // "Unsupported Starknet RPC version: {}, expected {}.", | ||
| // spec_version, | ||
| // RPC_SPEC_VERSION | ||
| // )); | ||
| // } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Temporary workaround creates deployment risk, sensei.
While commenting out the version compatibility check solves the immediate Katana deployment issue, this removes an important safety guard that prevents incompatible RPC versions from being used, potentially causing runtime failures on production networks.
Consider implementing a more targeted solution:
- // TODO: @remybar @glihm currently Katana is using new types, but doesn't
- // return the correct spec version. We comment this test for now to ensure
- // one can deploy on sepolia/mainnet but also Katana.
- // if !is_compatible_version(&spec_version, RPC_SPEC_VERSION)? {
- // return Err(anyhow!(
- // "Unsupported Starknet RPC version: {}, expected {}.",
- // spec_version,
- // RPC_SPEC_VERSION
- // ));
- // }
+ // Skip version check for Katana (known compatibility issue in 0.13.4)
+ let is_katana = chain_id.contains("KATANA") || spec_version.contains("katana");
+ if !is_katana && !is_compatible_version(&spec_version, _RPC_SPEC_VERSION)? {
+ return Err(anyhow!(
+ "Unsupported Starknet RPC version: {}, expected {}.",
+ spec_version,
+ _RPC_SPEC_VERSION
+ ));
+ }This maintains safety for production networks while allowing Katana deployments.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In bin/sozo/src/utils.rs around lines 157 to 166, the version compatibility
check is commented out as a temporary workaround, which removes an important
safety guard. To fix this, implement a conditional check that skips the
compatibility verification only when deploying on Katana, but enforces it on
sepolia/mainnet and other production networks. This approach maintains safety by
preventing incompatible RPC versions in production while allowing Katana
deployments.
* Fix broken getting started link in README (#3235) * chore(versions): add torii 1.5.6 * Update DEVELOPMENT.md (#3236) * Update DEVELOPMENT.md with new instructions * Update scripts/rebuild_test_artifacts.sh with Rabbit suggestion Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Respond to review comments * Respond to review comments II * Respond to review comments III --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * feat: update rpc spec to 0.8 (#3179) * release(prepare): v1.6.0-alpha.0 (#3241) Prepare release: v1.6.0-alpha.0 Co-authored-by: glihm <glihm@users.noreply.github.com> * chore(versions): bump katana to 1.6.0-alpha.0 * fix(dojoup): remove new line in the source cmd (#3244) fix: fixed new line in the source cmd * chore: edited the build badge and its link (#3221) Co-authored-by: Ammar Arif <evergreenkary@gmail.com> * chore(devcontainer): update image: v1.6.0-alpha.0 (#3242) Update devcontainer image: v1.6.0-alpha.0 Co-authored-by: glihm <glihm@users.noreply.github.com> * chore: add katana 1.5.4 (#3245) * chore: add the missing backticks in the comments (#3243) Signed-off-by: one230six <723682061@qq.com> * fix(bindgen): use world namespace for imported models in TypeScript S… (#3249) * fix(bindgen): filter out Value models (#3248) * fix(bindgen): fix ts bytearray type mapping (#3247) * chore: update katana versions (#3253) add katana versions * sozo(unrealengine): handle UE5.6 and Dojo 1.5 (#3252) * fix(sozo): assert caller permission with match (#3254) * handle call_contract_syscall() result for better panic trace * avoid using 0 as caller address * remove temporarly the RPC version check. Currently, Katana uses the new RPC types, without bumping the spec version. To ensure we can still use sozo with sepolia/mainnet and Katana, Sozo will not check the RPC version for now * update test dbs --------- Co-authored-by: glihm <dev@glihm.net> * feat(sozo): create standalone bindgen command (#3246) * feat: create standalone bindgen command * cleanup unused inputs * remove dbg * add meaningful error if project is not built --------- Co-authored-by: glihm <dev@glihm.net> * feat(sozo): add MCP sever (#3256) * feat(sozo): add mvp for mcp server * refacto: restructure MCP server * evaluate changes using official rust sdk * feat(mcp): add stdio support * refacto with rmcp crate * wip * bump rmcp * add testing support * wip * refacto * cleanup * add instructions * add instructions * wip test * refacto tests * add debugging * ignore test with katana at the moment * fix typos and sozo path * refacto uri parsing * remove dbg * disable test that should be run with katana * fix test, windows fails because of new reqwest version * refactor(types): schema json sql value (#3257) * refactor(types): schema json sql value * remove excess comma * fix clippy --------- Co-authored-by: glihm <dev@glihm.net> * chore: bump cairo packages to 1.6.0-alpha.0 * fix(mcp): ensure test is using latest version * split scarb metadata ext in a different crate for dependencies * wip: controller issue * wip * wip: resolve conflicts and update deps * cleanup * lint * wip * wip tests * fix all tests * rebuild test db * fix: run linter * fix slot dep with github commit * fix clippy * fix inspect by adding missing json format * fix mcp formatting * fix stdio_test * use sozo from path --------- Signed-off-by: one230six <723682061@qq.com> Co-authored-by: Ritik <ritikverma0050@gmail.com> Co-authored-by: Daniel Kronovet <kronovet@gmail.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Ammar Arif <evergreenkary@gmail.com> Co-authored-by: Tarrence van As <tarrencev@users.noreply.github.com> Co-authored-by: glihm <glihm@users.noreply.github.com> Co-authored-by: Benjamin <158306087+bengineer42@users.noreply.github.com> Co-authored-by: braveocheretovych <braveocheretovych@gmail.com> Co-authored-by: one230six <163239332+one230six@users.noreply.github.com> Co-authored-by: Brother MartianGreed <valentin@pupucecorp.com> Co-authored-by: Corentin Cailleaud <corentin.cailleaud@caillef.com> Co-authored-by: Rémy Baranx <remy.baranx@gmail.com> Co-authored-by: Larko <59736843+Larkooo@users.noreply.github.com>
…3266) * feat: add external contract registering in the world (#3195) * Use the world as external contract reference and manage upgrades with sozo * update spawn-and-move with new external contract features * update test artifacts + world_address in dojo_dev.toml * export entrypoints + fix tests + remove TODO RBA * restore test_metadata_updated_event test * fix tests * rabbit improvements * fix test * feat: add proc macros (#3212) * WIP * add macros + core-tests crates + update them from 1.6.0 * update scripts/cairo_fmt.sh + fix fmt * update CI + Scarb.toml files * fix fmt * change from 2.11.2 to 2.11.4 * fix scarb issue + fmt * one testing library for each test runner (cairo, snfoundry) + update examples * fix fmt * update comment * chore: apply patches to sync exact same commits than scarb * fix(ci): update rust-toolchain version * fix(ci): bump clippy to use nightly compatible with 1.86 * fix: clippy and fmt * update spawn-and-move to use dojo-snf-test * update world address --------- Co-authored-by: glihm <dev@glihm.net> * Scarb crate removing (#3223) * introduce scarb_interop crate * handle missing scarb * introduce metadata instead of workspace to manage dojo related paths and config * add stats back and ensure profile is propagated until build not only metadata * cleanup and add error message if SCARB is not set correctly * add features + packages to build command * update scarb build/test command * refactor packages/features * fix utils.rs + enable auth command * fix rust fmt * re-enable sozo commands * re-enable the last sozo commands + new version command * fix issue with conflicts_with_all * check if manifest_path does not exist * set run() functions as async instead of using tokio * update init command management * refactor metadata loading + dead code cleaning * add build_simple_dev() function to be used to easily build spawn-and-move for tests/benches * update tests + fmt + clippy * remove dojo/lang * remove useless sozo files * tiny change * fix CI * use scarb nightly in CI * fix manifest_path/manifest_dir issue * fix fmt * update snfoundry version + update tests accordingly + add cairo-profiler to .tool-versions --------- Co-authored-by: glihm <dev@glihm.net> * feat: add DojoStore trait to handle storage serialization (#3219) * add DojoStore management * fix rust fmt after abigen * fix ModelReader * fix fmt * set DojoStore functions as inline(always) * update test artifacts * fix fmt * remove dojo-core test from CI as all tests are now in core-tests * feat: add block number for external contract events (#3224) * add block_number to ExternalContractRegistered/ExternalContractUpgraded events * remove useless println * restore external_contracts tests + update with block_number * update test artifacts * fix clippy * rework: handle block_number * fix world.dns for external contract + update tests * update sozo events with missing events * add a self-managed external contract called from spawn-and-move * update manifest_dev.json * use dns_address * fix fmt + abigen * update test artifacts * fix fmt * update starkli path * update policies * rebuild test db * fix: restore resource order to avoid storage conflict (#3238) * Resource enum variant order should not change as it is used in the world storage * fix fmt * fix world address and test db * fix fmt --------- Co-authored-by: glihm <dev@glihm.net> * fix(lang): remove warning with unit type (#3255) fix warning with unit type for enumerations serialization generated code. * dev: merge main in dev 1.6.0 (#3263) * Fix broken getting started link in README (#3235) * chore(versions): add torii 1.5.6 * Update DEVELOPMENT.md (#3236) * Update DEVELOPMENT.md with new instructions * Update scripts/rebuild_test_artifacts.sh with Rabbit suggestion Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Respond to review comments * Respond to review comments II * Respond to review comments III --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * feat: update rpc spec to 0.8 (#3179) * release(prepare): v1.6.0-alpha.0 (#3241) Prepare release: v1.6.0-alpha.0 Co-authored-by: glihm <glihm@users.noreply.github.com> * chore(versions): bump katana to 1.6.0-alpha.0 * fix(dojoup): remove new line in the source cmd (#3244) fix: fixed new line in the source cmd * chore: edited the build badge and its link (#3221) Co-authored-by: Ammar Arif <evergreenkary@gmail.com> * chore(devcontainer): update image: v1.6.0-alpha.0 (#3242) Update devcontainer image: v1.6.0-alpha.0 Co-authored-by: glihm <glihm@users.noreply.github.com> * chore: add katana 1.5.4 (#3245) * chore: add the missing backticks in the comments (#3243) Signed-off-by: one230six <723682061@qq.com> * fix(bindgen): use world namespace for imported models in TypeScript S… (#3249) * fix(bindgen): filter out Value models (#3248) * fix(bindgen): fix ts bytearray type mapping (#3247) * chore: update katana versions (#3253) add katana versions * sozo(unrealengine): handle UE5.6 and Dojo 1.5 (#3252) * fix(sozo): assert caller permission with match (#3254) * handle call_contract_syscall() result for better panic trace * avoid using 0 as caller address * remove temporarly the RPC version check. Currently, Katana uses the new RPC types, without bumping the spec version. To ensure we can still use sozo with sepolia/mainnet and Katana, Sozo will not check the RPC version for now * update test dbs --------- Co-authored-by: glihm <dev@glihm.net> * feat(sozo): create standalone bindgen command (#3246) * feat: create standalone bindgen command * cleanup unused inputs * remove dbg * add meaningful error if project is not built --------- Co-authored-by: glihm <dev@glihm.net> * feat(sozo): add MCP sever (#3256) * feat(sozo): add mvp for mcp server * refacto: restructure MCP server * evaluate changes using official rust sdk * feat(mcp): add stdio support * refacto with rmcp crate * wip * bump rmcp * add testing support * wip * refacto * cleanup * add instructions * add instructions * wip test * refacto tests * add debugging * ignore test with katana at the moment * fix typos and sozo path * refacto uri parsing * remove dbg * disable test that should be run with katana * fix test, windows fails because of new reqwest version * refactor(types): schema json sql value (#3257) * refactor(types): schema json sql value * remove excess comma * fix clippy --------- Co-authored-by: glihm <dev@glihm.net> * chore: bump cairo packages to 1.6.0-alpha.0 * fix(mcp): ensure test is using latest version * split scarb metadata ext in a different crate for dependencies * wip: controller issue * wip * wip: resolve conflicts and update deps * cleanup * lint * wip * wip tests * fix all tests * rebuild test db * fix: run linter * fix slot dep with github commit * fix clippy * fix inspect by adding missing json format * fix mcp formatting * fix stdio_test * use sozo from path --------- Signed-off-by: one230six <723682061@qq.com> Co-authored-by: Ritik <ritikverma0050@gmail.com> Co-authored-by: Daniel Kronovet <kronovet@gmail.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Ammar Arif <evergreenkary@gmail.com> Co-authored-by: Tarrence van As <tarrencev@users.noreply.github.com> Co-authored-by: glihm <glihm@users.noreply.github.com> Co-authored-by: Benjamin <158306087+bengineer42@users.noreply.github.com> Co-authored-by: braveocheretovych <braveocheretovych@gmail.com> Co-authored-by: one230six <163239332+one230six@users.noreply.github.com> Co-authored-by: Brother MartianGreed <valentin@pupucecorp.com> Co-authored-by: Corentin Cailleaud <corentin.cailleaud@caillef.com> Co-authored-by: Rémy Baranx <remy.baranx@gmail.com> Co-authored-by: Larko <59736843+Larkooo@users.noreply.github.com> * feat(core): use metaprogramming for tuple and fixed size array Introspect and DojoStore (#3260) * reuse Starkware metaprogramming stuff to manage tuples * fix fmt + clippy + artifacts * use metaprogramming to handle tuple introspect * fix fmt + clippy + artifacts * add fixed size array support * update dojo-world * update abigen + artifacts after rebase * add a sum_sizes function * update artifacts * tooling: add cairo-bench tool + bench tests (#3240) * add cairo-bench tool * add bench tests * first optimisation batch * fix fmt * add license info * update artifacts after rebase * fix cairo-bench tests * update artifacts * update artifacts * update scarb lock --------- Co-authored-by: glihm <dev@glihm.net> * fix(cairo-bench): use write and threshold + refactor fixed size array (#3267) * remove update-ref-test-list argument * add threshold parameter to cairo-bench (set to 3% by default) * max_fee_raw and fee_estimate_multiplier don't exist anymore * update sozo model commands for fixed size arrays * fix fmt + clippy * rename --update-ref to --write * fix some tests * fix fmt * fix(ci): update katana to 1.6.2 for compatible db --------- Signed-off-by: one230six <723682061@qq.com> Co-authored-by: Rémy Baranx <remy.baranx@gmail.com> Co-authored-by: Ritik <ritikverma0050@gmail.com> Co-authored-by: Daniel Kronovet <kronovet@gmail.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Ammar Arif <evergreenkary@gmail.com> Co-authored-by: Tarrence van As <tarrencev@users.noreply.github.com> Co-authored-by: glihm <glihm@users.noreply.github.com> Co-authored-by: Benjamin <158306087+bengineer42@users.noreply.github.com> Co-authored-by: braveocheretovych <braveocheretovych@gmail.com> Co-authored-by: one230six <163239332+one230six@users.noreply.github.com> Co-authored-by: Brother MartianGreed <valentin@pupucecorp.com> Co-authored-by: Corentin Cailleaud <corentin.cailleaud@caillef.com> Co-authored-by: Larko <59736843+Larkooo@users.noreply.github.com>
Description
Since Starknet
0.13.4, it is now possible to managecall_contract_syscall()result using amatch.This PR updates the
panic_with_details()world function to have a better panic trace.This PR also updates
spawn-and-movetests to not use0as caller address as it is the default caller address.Tests
Added to documentation?
Checklist
scripts/rust_fmt.sh,scripts/cairo_fmt.sh)scripts/clippy.sh,scripts/docs.sh)Summary by CodeRabbit
Tests
Style
Chores