chore: Dojo clean up #3309
chore: Dojo clean up #3309
Conversation
|
ohayo, sensei! WalkthroughRemoves the public API surface from the Dojo lang crate root and updates example artifacts and policies with refreshed addresses/class_hashes plus a minor test expectation change in spawn-and-move. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
crates/dojo/lang/Scarb.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
crates/dojo/lang/src/lib.rs(0 hunks)examples/spawn-and-move/manifest_dev.json(8 hunks)examples/spawn-and-move/src/actions.cairo(1 hunks)
💤 Files with no reviewable changes (1)
- crates/dojo/lang/src/lib.rs
🔇 Additional comments (7)
examples/spawn-and-move/src/actions.cairo (1)
179-183: Ohayo sensei — Assertion now matches DojoStore default (None), LGTMAligns with PlayerConfig initialization in set_models and with DojoStore’s zero/default semantics after erase/read. No functional concerns here.
examples/spawn-and-move/manifest_dev.json (6)
1482-1484: Ohayo sensei — actions contract recompiled/redeployed; metadata updatedAddress and class_hash updated. No issues from the manifest perspective.
2916-2961: Ohayo sensei — Model class_hashes updated; plan for client reindexingModels (e.g., ns-PlayerConfig, ns-Position, ns-Moves) have new class_hashes. Indexers/cache layers should re-resolve model fingerprints to avoid stale decoding.
I can provide a small migration checklist or script if your tooling needs to flush model caches. Want me to open a ticket?
2395-2399: Ohayo sensei — ABI match confirmed for ns-others.init_calldata
Thens-otherscontract’sdojo_inittakes a singleu8, and setting["0xff"]correctly satisfies that signature. 🎉
2760-2762: Ohayo sensei — Confirm intentional dual actions deploymentsThe manifest lists two actions-like contracts, which may be valid for versioning but could confuse naive consumers:
• ns-actions
– Address: 0x6898ae11701ff8355a88a14e798534a13d96a96d022a6595353b36a7c83e0f1
– Selector: 0x44f0e209e9c5b4b85306833a20e81c3b1a053fa9b3606cfcc59b30b28cb62f6• ns2-actions
– Address: 0x66406ffbeb01a660e86ba37637e98d3201421a5956c9ca151b216ba5eb258b0
– Selector: 0x1a920eff7b3fd2be618d5a222e63e805a2a82e71579f97ba06d716a45d1ae25Suggestions:
- If both are required (e.g., old vs. new), ensure clients always resolve by the exact tag/selector.
- If
ns-actionsis now vestigial, consider removing or renaming it for clarity.Let me know if you’d like a cleanup patch drafted!
2928-2931: Ohayo sensei — class_hash formatting is validRan a manifest-wide check on
examples/spawn-and-move/manifest_dev.json:
- Lengths (hex digits, excluding
0x):
• 61 digits – 1 entry (the snippet at lines 2928–2931)
• 63 digits – 24 entries- All entries match the
0x[0-9a-fA-F]+patternThese variations are due to trimmed leading zeros, not typos or truncation. No further fixes needed.
3-4: Ohayo sensei — confirm all tooling is aligned with the new world identityThe manifest in
examples/spawn-and-move/manifest_dev.jsonnow reports:world.class_hash=0x4d44ca76c3f82df35436d1c20847ba5c345b8dda4731e200ba3232cd8f94fe9 world.address=0x653644e1085578a22addf74edbd3a211cd41e7323f9ac249e3ffbf04bcbb86bIf any scripts,
.envfiles, or example configs still pin the previous values, they’ll drift out of sync. Please verify consumers are updated by:
- Searching for the new hashes to locate hard-coded references:
grep -R '0x4d44ca76c3f82df35436d1c20847ba5c345b8dda4731e200ba3232cd8f94fe9' -n . grep -R '0x653644e1085578a22addf74edbd3a211cd41e7323f9ac249e3ffbf04bcbb86b' -n .- Grepping the repo history or config files for the old class_hash/address to catch any stale pins
Ensuring consistency across all tooling avoids unexpected mismatches.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
bin/sozo/tests/test_data/policies.json (1)
107-109: New upgrade policy for 0x5357fd… — verify intended contractA single upgrade entry was added here. If this corresponds to the dungeon/feature contract mentioned in the PR, all good. Please confirm it appears in the updated manifests/artifacts to avoid a dangling policy.
I can wire a small generator to emit policies.json from the active manifest to enforce consistency. Want that, sensei?
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
bin/sozo/tests/test_data/policies.json(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-11-28T23:35:04.367Z
Learnt from: glihm
PR: dojoengine/dojo#2691
File: bin/sozo/tests/test_data/policies.json:7-32
Timestamp: 2024-11-28T23:35:04.367Z
Learning: The file `bin/sozo/tests/test_data/policies.json` is a test file containing policies used by sessions to determine which methods can be called for which contract addresses. There is not much to verify or check in this file.
Applied to files:
bin/sozo/tests/test_data/policies.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: fmt
🔇 Additional comments (4)
bin/sozo/tests/test_data/policies.json (4)
3-9: Upgrade targets rotated — looks consistentTwo new upgrade targets introduced. No schema changes; entries remain flat objects with "target" and "method". Looks good.
190-190: EOF newline normalizedMinor formatting cleanup. All good. ohayo, sensei.
147-181: Ohayo sensei, listing multiple method entries for the same target is intentional. We’ve verified that the address0x66406f…only appears here to enumerate each allowed method inbin/sozo/tests/test_data/policies.json—there isn’t a second-target group to trim. Keeping every method entry ensures comprehensive test coverage of policy behavior.
111-145: ohayo sensei!Confirmed that the address 0x6898ae11701ff8355a88a14e798534a13d96a96d022a6595353b36a7c83e0f1 is consistently used in both:
- examples/spawn-and-move/manifest_dev.json (defines the deployed contract)
- bin/sozo/tests/test_data/policies.json (declares the policies against that same contract)
This alignment ensures the spawn-and-move tests for DojoStore will load and apply those policies correctly. No further changes needed here.
| "target": "0x653644e1085578a22addf74edbd3a211cd41e7323f9ac249e3ffbf04bcbb86b", | ||
| "method": "uuid" | ||
| }, | ||
| { | ||
| "target": "0x681722ce4fc9c1fd0e800ba92e6ad759544863d65e57eecd623442ec30a31f1", | ||
| "target": "0x653644e1085578a22addf74edbd3a211cd41e7323f9ac249e3ffbf04bcbb86b", | ||
| "method": "set_metadata" | ||
| }, | ||
| { | ||
| "target": "0x681722ce4fc9c1fd0e800ba92e6ad759544863d65e57eecd623442ec30a31f1", | ||
| "target": "0x653644e1085578a22addf74edbd3a211cd41e7323f9ac249e3ffbf04bcbb86b", | ||
| "method": "register_namespace" | ||
| }, | ||
| { | ||
| "target": "0x681722ce4fc9c1fd0e800ba92e6ad759544863d65e57eecd623442ec30a31f1", | ||
| "target": "0x653644e1085578a22addf74edbd3a211cd41e7323f9ac249e3ffbf04bcbb86b", | ||
| "method": "register_event" | ||
| }, | ||
| { | ||
| "target": "0x681722ce4fc9c1fd0e800ba92e6ad759544863d65e57eecd623442ec30a31f1", | ||
| "target": "0x653644e1085578a22addf74edbd3a211cd41e7323f9ac249e3ffbf04bcbb86b", | ||
| "method": "register_model" | ||
| }, | ||
| { | ||
| "target": "0x681722ce4fc9c1fd0e800ba92e6ad759544863d65e57eecd623442ec30a31f1", | ||
| "target": "0x653644e1085578a22addf74edbd3a211cd41e7323f9ac249e3ffbf04bcbb86b", | ||
| "method": "register_contract" | ||
| }, | ||
| { | ||
| "target": "0x681722ce4fc9c1fd0e800ba92e6ad759544863d65e57eecd623442ec30a31f1", | ||
| "target": "0x653644e1085578a22addf74edbd3a211cd41e7323f9ac249e3ffbf04bcbb86b", | ||
| "method": "register_external_contract" | ||
| }, | ||
| { | ||
| "target": "0x681722ce4fc9c1fd0e800ba92e6ad759544863d65e57eecd623442ec30a31f1", | ||
| "target": "0x653644e1085578a22addf74edbd3a211cd41e7323f9ac249e3ffbf04bcbb86b", | ||
| "method": "register_library" | ||
| }, | ||
| { | ||
| "target": "0x681722ce4fc9c1fd0e800ba92e6ad759544863d65e57eecd623442ec30a31f1", | ||
| "target": "0x653644e1085578a22addf74edbd3a211cd41e7323f9ac249e3ffbf04bcbb86b", | ||
| "method": "init_contract" | ||
| }, | ||
| { | ||
| "target": "0x681722ce4fc9c1fd0e800ba92e6ad759544863d65e57eecd623442ec30a31f1", | ||
| "target": "0x653644e1085578a22addf74edbd3a211cd41e7323f9ac249e3ffbf04bcbb86b", | ||
| "method": "upgrade_event" | ||
| }, | ||
| { | ||
| "target": "0x681722ce4fc9c1fd0e800ba92e6ad759544863d65e57eecd623442ec30a31f1", | ||
| "target": "0x653644e1085578a22addf74edbd3a211cd41e7323f9ac249e3ffbf04bcbb86b", | ||
| "method": "upgrade_model" | ||
| }, | ||
| { | ||
| "target": "0x681722ce4fc9c1fd0e800ba92e6ad759544863d65e57eecd623442ec30a31f1", | ||
| "target": "0x653644e1085578a22addf74edbd3a211cd41e7323f9ac249e3ffbf04bcbb86b", | ||
| "method": "upgrade_contract" | ||
| }, | ||
| { | ||
| "target": "0x681722ce4fc9c1fd0e800ba92e6ad759544863d65e57eecd623442ec30a31f1", | ||
| "target": "0x653644e1085578a22addf74edbd3a211cd41e7323f9ac249e3ffbf04bcbb86b", | ||
| "method": "upgrade_external_contract" | ||
| }, | ||
| { | ||
| "target": "0x681722ce4fc9c1fd0e800ba92e6ad759544863d65e57eecd623442ec30a31f1", | ||
| "target": "0x653644e1085578a22addf74edbd3a211cd41e7323f9ac249e3ffbf04bcbb86b", | ||
| "method": "emit_event" | ||
| }, | ||
| { | ||
| "target": "0x681722ce4fc9c1fd0e800ba92e6ad759544863d65e57eecd623442ec30a31f1", | ||
| "target": "0x653644e1085578a22addf74edbd3a211cd41e7323f9ac249e3ffbf04bcbb86b", | ||
| "method": "emit_events" | ||
| }, | ||
| { | ||
| "target": "0x681722ce4fc9c1fd0e800ba92e6ad759544863d65e57eecd623442ec30a31f1", | ||
| "target": "0x653644e1085578a22addf74edbd3a211cd41e7323f9ac249e3ffbf04bcbb86b", | ||
| "method": "set_entity" | ||
| }, | ||
| { | ||
| "target": "0x681722ce4fc9c1fd0e800ba92e6ad759544863d65e57eecd623442ec30a31f1", | ||
| "target": "0x653644e1085578a22addf74edbd3a211cd41e7323f9ac249e3ffbf04bcbb86b", | ||
| "method": "set_entities" | ||
| }, | ||
| { | ||
| "target": "0x681722ce4fc9c1fd0e800ba92e6ad759544863d65e57eecd623442ec30a31f1", | ||
| "target": "0x653644e1085578a22addf74edbd3a211cd41e7323f9ac249e3ffbf04bcbb86b", | ||
| "method": "delete_entity" | ||
| }, | ||
| { | ||
| "target": "0x681722ce4fc9c1fd0e800ba92e6ad759544863d65e57eecd623442ec30a31f1", | ||
| "target": "0x653644e1085578a22addf74edbd3a211cd41e7323f9ac249e3ffbf04bcbb86b", | ||
| "method": "delete_entities" | ||
| }, | ||
| { | ||
| "target": "0x681722ce4fc9c1fd0e800ba92e6ad759544863d65e57eecd623442ec30a31f1", | ||
| "target": "0x653644e1085578a22addf74edbd3a211cd41e7323f9ac249e3ffbf04bcbb86b", | ||
| "method": "grant_owner" | ||
| }, | ||
| { | ||
| "target": "0x681722ce4fc9c1fd0e800ba92e6ad759544863d65e57eecd623442ec30a31f1", | ||
| "target": "0x653644e1085578a22addf74edbd3a211cd41e7323f9ac249e3ffbf04bcbb86b", | ||
| "method": "revoke_owner" | ||
| }, | ||
| { | ||
| "target": "0x681722ce4fc9c1fd0e800ba92e6ad759544863d65e57eecd623442ec30a31f1", | ||
| "target": "0x653644e1085578a22addf74edbd3a211cd41e7323f9ac249e3ffbf04bcbb86b", | ||
| "method": "grant_writer" | ||
| }, | ||
| { | ||
| "target": "0x681722ce4fc9c1fd0e800ba92e6ad759544863d65e57eecd623442ec30a31f1", | ||
| "target": "0x653644e1085578a22addf74edbd3a211cd41e7323f9ac249e3ffbf04bcbb86b", | ||
| "method": "revoke_writer" | ||
| }, | ||
| { | ||
| "target": "0x681722ce4fc9c1fd0e800ba92e6ad759544863d65e57eecd623442ec30a31f1", | ||
| "target": "0x653644e1085578a22addf74edbd3a211cd41e7323f9ac249e3ffbf04bcbb86b", | ||
| "method": "upgrade" | ||
| }, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Bulk remap to 0x653644… (World) — consider preventing drift
All World-admin methods now point to 0x653644… This matches the PR goal of refreshing addresses. To keep this file in sync with manifests over time, consider generating these entries from the active world manifest (or a single TOML/JSON source) during tests. This avoids future checksum drift.
Run to sanity-check duplicates and address format:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
f="bin/sozo/tests/test_data/policies.json"
# 1) Validate JSON
jq . >/dev/null <"$f"
# 2) Ensure no duplicate (target,method) pairs
dups=$(jq -r '.[] | "\(.target)|\(.method)"' "$f" | sort | uniq -d)
if [[ -n "$dups" ]]; then
echo "Duplicate target|method pairs found:"; echo "$dups"; exit 1
fi
# 3) Check address format: 0x + 64 hex chars (lowercase)
bad=$(jq -r '.[] | .target' "$f" | awk '!/^0x[0-9a-f]{64}$/')
if [[ -n "$bad" ]]; then
echo "Targets with invalid format:"; echo "$bad"; exit 1
fi
# 4) Optional: confirm targets appear in repo artifacts/manifests
# (best-effort cross-check; non-fatal)
rg -n --color=never -C1 '0x653644e1085578a22addf74edbd3a211cd41e7323f9ac249e3ffbf04bcbb86b' || trueLength of output: 12791
ohayo sensei — Critical: Truncated hex addresses in policies.json
Several target entries in bin/sozo/tests/test_data/policies.json do not conform to the required format 0x + 64 lowercase hex characters. For example:
0x5df34baa68c5f498c4a9776abf66c8d30efbdfdcaa8321135487df3e4bd3ef(63 hex chars)- Multiple repeats of
0x653644e1085578a22addf74edbd3a211cd41e7323f9ac249e3ffbf04bcbb86b(63 hex chars) - Other entries also missing one hex digit at the end.
These malformed addresses will break downstream consumers and tests.
• Fix all truncated target values so they match ^0x[0-9a-f]{64}$.
• Consider adding a CI check (e.g., the jq + awk script) to validate address length as part of your test suite.
Summary by CodeRabbit