Conversation
Ohayo, senseiWalkthroughBumps toolchains and CI (Rust 1.88, scarb/dev pins), updates many dependency pins and manifests, refactors macros to use lifetimes/iterators and adjust derives, splits slot into slot-core/slot-session, adds sozo CLI flags to suppress Scarb warnings, enhances declare finality/receipt handling, simplifies world event fetching, and updates many tests/manifests/addresses. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Sozo as sozo CLI
participant Scarb as Scarb (process)
Note over User,Scarb: Build/Test with optional Scarb warning suppression
User->>Sozo: sozo build/test [--no-scarb-warnings]
Sozo->>Scarb: spawn "scarb ..." (+ "--verbosity no-warnings" if set)
Scarb-->>Sozo: exit status
Sozo-->>User: success or error
sequenceDiagram
autonumber
actor Dev
participant Declarer as Declarer
participant Provider as Network Provider
Note over Dev,Provider: DECLARE with finality-aware wait and optional receipt
Dev->>Declarer: declare(class, TxnConfig{wait=true,receipt=true})
Declarer->>Provider: submit DECLARE tx
alt wait=true
Declarer->>Provider: wait(AcceptedOnL2, timeout=60s)
Provider-->>Declarer: status + receipt
Declarer->>Declarer: sleep(5s) // propagation
alt receipt=true
Declarer-->>Dev: HashReceipt(hash, receipt)
else
Declarer-->>Dev: Hash(hash)
end
else
Declarer-->>Dev: Hash(hash)
end
sequenceDiagram
autonumber
participant World as World::from_events
participant Provider
Note over World,Provider: Simplified block-span fetch (single-span)
World->>World: set current_from/from_block
loop single span (effectively)
World->>Provider: get_events(from=current_from,to=to_block, paginated)
Provider-->>World: events (+continuation)
end
World-->>World: build snapshot
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes Possibly related PRs
Suggested labels
✨ 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: 25
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
.devcontainer/Dockerfile (1)
33-39: Fix ARG TARGETPLATFORM visibility — currently declared after first use.
TARGETPLATFORMis used in Line 33 before it’s declared (Line 57). In Docker, ARG scope starts at the declaration; earlier RUNs will see it unset, breaking arm64 detection.Apply:
FROM ubuntu:24.04 ARG RUST_VERSION +ARG TARGETPLATFORM @@ -# Platform specific tooling (hurl, llvm-tools) -ARG TARGETPLATFORM +# Platform specific tooling (hurl, llvm-tools)Also applies to: 57-61
.github/workflows/test.yml (1)
92-96: Ohayo, sensei — broken toolchain pin on Windows (env var case mismatch).
toolchain: ${{ env.rust_version }}should referenceRUST_VERSION. As written, the Windows job may not use 1.88.0.- toolchain: ${{ env.rust_version }} + toolchain: ${{ env.RUST_VERSION }}crates/dojo/core-tests/src/tests/world/acl.cairo (1)
147-149: Fix no-op test: add an assertion in test_writer_not_registered_resourceOhayo, sensei — this test currently does nothing; the negated call result is discarded and the test always passes.
Apply:
// 42 is not a registered resource ID - !world.is_writer(42, 69.try_into().unwrap()); + assert(!world.is_writer(42, 69.try_into().unwrap()), 'should not be writer');bin/sozo/src/commands/options/account/mod.rs (1)
21-28: Gate slot_session imports behindcontrollerfeatureohayo, sensei
- bin/sozo/src/commands/options/account/type.rs:5 – wrap
use slot_session::account_sdk::provider::CartridgeJsonRpcProvider;
in#[cfg(feature = "controller")].- bin/sozo/src/commands/options/account/mod.rs:11 – move or wrap the
unconditionaluse slot_session::account_sdk::provider::CartridgeJsonRpcProvider;
inside#[cfg(feature = "controller")].crates/sozo/ops/src/migrate/mod.rs (1)
582-589: Ohayo, sensei — handle HashReceipt in multicall or deploy block_number may be missed.When txn_config.receipt = true, txs_results[0] can be HashReceipt and the current code won’t set deploy_block_numbers, leading to a later panic during registration. Patch to support both variants:
- if !deploy_calls.is_empty() { - // TODO: @remybar, wondering if here we should - // also handle the case when it contains the receipt - // already, due to the tx configuration. - if let TransactionResult::Hash(tx_hash) = txs_results[0] { - let receipt = - TransactionWaiter::new(tx_hash, &self.world.account.provider()).await?; - let block_number = receipt.block.block_number(); - deploy_block_numbers = - deploy_calls.keys().map(|name| (name.clone(), block_number)).collect(); - } - } + if !deploy_calls.is_empty() { + let block_number = match &txs_results[0] { + TransactionResult::Hash(tx_hash) => { + let r = TransactionWaiter::new(*tx_hash, &self.world.account.provider()).await?; + r.block.block_number() + } + TransactionResult::HashReceipt(_, receipt) => receipt.block.block_number(), + _ => unreachable!(), + }; + deploy_block_numbers = + deploy_calls.keys().map(|name| (name.clone(), block_number)).collect(); + }Optional: consider issuing deploy_calls in a dedicated multicall (before other calls) to make the “first tx” block deterministic for all deployments. This reduces over-scanning when non-deploy calls precede deployments.
crates/dojo/macros/src/helpers/parser.rs (1)
131-139: Ohayo, sensei — Parse the last path segment for derive names (handles namespaced paths).Current code takes the first segment (
dojoindojo::Introspect). Prefer the last simple segment to robustly extract the trait name.- if let Some(ast::PathSegment::Simple(segment)) = - &path.segments(db).elements(db).next() - { - Some(segment.ident(db).text(db).to_string()) - } else { - None - } + // Use the last simple segment, e.g., `crate::foo::Bar` -> `Bar`. + { + let segs = path.segments(db).elements(db).collect::<Vec<_>>(); + if let Some(ast::PathSegment::Simple(segment)) = segs.last() { + Some(segment.ident(db).text(db).to_string()) + } else { + None + } + }
🧹 Nitpick comments (48)
crates/sozo/scarb_metadata_ext/src/metadata.rs (2)
284-289: Consider broader derives if reused.Deriving Eq and Clone can be handy for testing and reuse; optional.
-#[derive(Debug, PartialEq)] +#[derive(Debug, PartialEq, Eq, Clone)] enum WorldPackageResult {
291-308: Fix minor doc typos.s/virutal/virtual/, and use “set up” (verb) instead of “setup”.
-/// In a virutal workspace, it is handy to have all the dependencies setup, but no `src` at the root +/// In a virtual workspace, it is handy to have all the dependencies set up, but no `src` at the rootexamples/game-lib/Scarb.toml (2)
11-11: Pin policy: do you want exact Cairo 2.12.0 or any 2.12.x?If you intend to lock to 2.12.0 for reproducibility, pin explicitly; otherwise current spec is fine.
Apply if you want exact pin:
-cairo-version = "2.12" +cairo-version = "=2.12.0"
15-15: Ohayo sensei — Pin starknet to an exact patch version in examples/game-lib/Scarb.toml
examples/game-lib/Scarb.toml:15 uses a caret‐style spec ("2.12"), while other manifests mix caret and exact pins ("=2.12.0","2.12.1"). Align with your reproducibility policy by pinning an exact patch:-starknet = "2.12" +starknet = "=2.12.0"crates/sozo/mcp/tests/stdio_tests.rs (1)
236-236: Make the version check less brittle to future pre-release bumps.Consider asserting a prefix to reduce churn between alpha/beta patch bumps.
Apply this diff:
- assert_eq!(manifest_json["package"]["version"], "1.7.0-alpha.2"); + let v = manifest_json["package"]["version"].as_str().unwrap_or_default(); + assert!( + v.starts_with("1.7.0-"), + "unexpected manifest version: {v}" + );crates/dojo/core-tests/src/tests/world/metadata.cairo (1)
112-112: Avoid hardcoding the full malicious contract address in the panic expectation.Ohayo, sensei — asserting on a stable substring makes the test less flaky across toolchains/envs.
Apply this diff:
-#[should_panic( - expected: "Contract `0x252567a4ea339b58d479b6fd744d818af064abe29cc04fc746178e0d269c0b6` does NOT have OWNER role on model (or its namespace) `Foo`", -)] +#[should_panic( + expected: "does NOT have OWNER role on model (or its namespace) `Foo`", +)]crates/dojo/macros/src/helpers/diagnostic_ext.rs (1)
23-29: Minor future-proofing: tolerate unforeseen Severity variants.If upstream adds a new variant, default to “warning” (or use Display if available).
Apply this diff:
- let severity = match self.severity() { - Severity::Error => "error", - Severity::Warning => "warning", - }; + let severity = match self.severity() { + Severity::Error => "error", + Severity::Warning => "warning", + #[allow(unreachable_patterns)] + _ => "warning", + };.devcontainer/Dockerfile (1)
46-47: Deduplicate llvm-tools installation.
rustup component add llvm-tools-previewis run at Line 47 and again in platform branches (Lines 66/73). Keep one place to avoid redundant steps.RUN rustup component add clippy rustfmt -RUN rustup component add llvm-tools-preview +RUN rustup component add llvm-tools-preview @@ - rustup component add llvm-tools-preview; \ + # llvm-tools already added above @@ - rustup component add llvm-tools-preview && \ + # llvm-tools already added above && \Also applies to: 66-67, 73-74
.github/workflows/test.yml (1)
35-36: Optional: align dojo-dev container tag with Katana bump.Jobs use
dojo-dev:v1.7.0-alpha.1while Katana isv1.7.0-alpha.3. If the image bundles tools, consider bumping for consistency.- image: ghcr.io/dojoengine/dojo-dev:v1.7.0-alpha.1 + image: ghcr.io/dojoengine/dojo-dev:v1.7.0-alpha.3Also applies to: 60-61, 166-167, 180-181, 190-191
crates/dojo/utils/src/parse.rs (1)
103-184: Add an IPv6 regression test.Ensure bracketed IPv6 is handled end-to-end.
@@ fn test_parse_url_socket_address_ip_port_as_value() { let result = parse_url("127.0.0.1:8080").unwrap(); assert_eq!(result, Url::parse("http://127.0.0.1:8080").unwrap()); } + + #[test] + fn test_parse_url_socket_address_ipv6() { + let result = parse_url("[::1]:8080").unwrap(); + assert_eq!(result, Url::parse("http://[::1]:8080").unwrap()); + }crates/dojo/core-tests/src/tests/world/world.cairo (1)
249-251: Avoid hardcoding address in should_panic expectedOhayo, sensei — relax the match to a stable substring so future deploy-address changes don’t break the test.
- expected: "Contract `0x6e1592b8353d0fc098469cce233353af2b92a5b88ef81e8355e93c009a9617a` does NOT have OWNER role on contract (or its namespace) `test_contract`", + expected: "does NOT have OWNER role on contract (or its namespace) `test_contract`",crates/dojo/core-tests/src/tests/world/contract.cairo (2)
244-246: Relax brittle panic expectation (malicious deploy)Ohayo, sensei — use a stable substring instead of an embedded address.
- expected: "Contract `0x252567a4ea339b58d479b6fd744d818af064abe29cc04fc746178e0d269c0b6` does NOT have OWNER role on namespace `dojo`", + expected: "does NOT have OWNER role on namespace `dojo`",
352-354: Relax brittle panic expectation (malicious upgrade)Ohayo, sensei — same robustness tweak here.
- expected: "Contract `0x252567a4ea339b58d479b6fd744d818af064abe29cc04fc746178e0d269c0b6` does NOT have OWNER role on contract (or its namespace) `test_contract`", + expected: "does NOT have OWNER role on contract (or its namespace) `test_contract`",crates/sozo/scarb_interop/src/scarb.rs (1)
59-63: Include command context in failure messageOhayo, sensei — adding args to the error helps users reproduce/debug failed Scarb invocations.
- let status = child.wait()?; - if !status.success() { - bail!("Scarb command failed with exit code: {}", status); - } + let status = child.wait()?; + if !status.success() { + bail!("Scarb command failed: scarb {} (status: {})", args_with_manifest.join(" "), status); + }examples/simple/src/lib.cairo (1)
22-28: Keep event derives consistent (either rely on #[dojo::event] alone or mirror derives on both E and EH)E relies on #[dojo::event]’s implicit derives while EH specifies them explicitly. For consistency, consider removing explicit derives on EH or adding them to E as well.
crates/dojo/core-tests/src/tests/world/model.cairo (1)
356-359: Avoid hard-coding full malicious contract addresses in panic expectationsToolchain/node updates often change deployed addresses. Consider matching only a stable prefix (role text) or deriving the expected address string from the malicious_contract variable when asserting, instead of using #[should_panic(expected = "...")].
I can refactor this test to capture the panic/error and assert that it contains both the role text and the dynamic address.
crates/dojo/world/src/remote/events_to_remote.rs (2)
33-39: Keep max_block_range functional with a safe fallback (doc/code now diverge)Param is now unused and range chunking is removed; some providers still reject large ranges. Suggest adaptive use: honor max_block_range when > 0, else fetch full range. Minimal change below.
Please validate on Sepolia and at least one third-party provider.
@@ - pub async fn from_events<P: Provider>( + pub async fn from_events<P: Provider>( world_address: Felt, provider: &P, from_block: Option<u64>, - _max_block_range: u64, + max_block_range: u64, whitelisted_namespaces: Option<Vec<String>>, ) -> Result<Self> { @@ - while current_from <= to_block { - // Limitation of the node with too big block ranges seems to be fixed in `0.14`. - // Will keep for a moment this line commented to ensure we can quickly change our mind - // if needed. let current_to = std::cmp::min(current_from + max_block_range - // - 1, to_block); - let current_to = to_block; + while current_from <= to_block { + // If max_block_range == 0, fetch the whole span; otherwise chunk defensively. + let span_end = current_from + .saturating_add(max_block_range.saturating_sub(1)); + let current_to = if max_block_range == 0 { + to_block + } else { + std::cmp::min(span_end, to_block) + };Also applies to: 88-95
33-39: Optionally update the docstring to reflect the new behavior if you decide to keep single-span fetchIf keeping the “fetch-all” approach, the docs referencing provider limits and max_block_range should be updated to avoid confusion.
crates/dojo/core-tests/src/tests/world/event.cairo (1)
300-305: Avoid hard-coding the malicious contract address in expected panicDerive or assert on the role error text instead; full address strings are likely to change with toolchain/provider updates.
crates/dojo/macros/src/helpers/misc.rs (2)
18-23: ohayo, sensei — broaden API ergonomics with IntoIteratorAccept IntoIterator to make call sites simpler (Vec, arrays, iterators).
-pub fn compute_unique_hash<'a>( +pub fn compute_unique_hash<'a, I>( db: &SimpleParserDatabase, element_name: &str, is_packed: bool, - members: impl Iterator<Item = Member<'a>>, + members: I, -) -> Felt { +) -> Felt +where + I: IntoIterator<Item = Member<'a>>, +{ - hashes.extend( - members + hashes.extend( + members + .into_iter() .map(|m| {
28-39: ohayo, sensei — minor: avoid temporary Vec allocationYou can extend from the iterator directly; collecting is unnecessary.
- hashes.extend( - members - .map(|m| { - poseidon_hash_many(&[ - naming::compute_bytearray_hash(m.name(db).text(db)), - naming::compute_bytearray_hash( - m.type_clause(db).ty(db).as_syntax_node().get_text_without_trivia(db), - ), - ]) - }) - .collect::<Vec<_>>(), - ); + hashes.extend(members.map(|m| { + poseidon_hash_many(&[ + naming::compute_bytearray_hash(m.name(db).text(db).as_str()), + naming::compute_bytearray_hash( + m.type_clause(db).ty(db).as_syntax_node().get_text_without_trivia(db).as_str(), + ), + ]) + }));Cargo.toml (1)
180-180: serde_json 1.0.142 (+arbitrary_precision) — fine; watch for perf on hot paths.If used in tight loops, consider serde features or preallocations.
examples/game-lib/armory/Scarb.toml (1)
14-14: Ohayo sensei — Pincairo_testto 2.12.1 in examples/game-lib/armory/Scarb.toml to avoid resolver drift.-cairo_test = "2.12" +cairo_test = "2.12.1"crates/dojo/utils/src/tx/mod.rs (1)
183-186: Docstring still mentions 'pending' but code uses 'preconfirmed'.Update to reflect current semantics.
-/// * `block_str` - a string representing a block ID. It could be a block hash starting with 0x, a -/// block number, 'pending' or 'latest'. +/// * `block_str` - a string representing a block ID. It could be a block hash starting with 0x, a +/// block number, 'preconfirmed' or 'latest'.bin/sozo/src/commands/options/account/mod.rs (1)
97-99: Fix typo: “Catridge” → “Cartridge”.User-facing docs matter.
- /// Create a new Catridge Controller account based on session key. + /// Create a new Cartridge Controller account based on session key.examples/spawn-and-move/Scarb.toml (1)
6-6: Ohayo sensei, unify Cairo and Starknet versions across all Scarb.toml manifests:
- Cairo: replace all
cairo-version = "2.12"/"2.12.0"and=2.11.4with=2.12.1- Starknet: replace all
starknet = "2.12"/"2.12.0"and"2.11.4"with"2.12.1"This ensures consistent pinning and minimizes cache churn.
crates/dojo/core/Scarb.toml (1)
2-2: Unify Cairo/Starknet patch versions in core crates to match examplesohayo sensei, please:
- Pin
cairo-versionto=2.12.1andstarknetto2.12.1incrates/dojo/core/Scarb.toml(lines 2, 10).- Apply the same exact pinning in
crates/dojo/core-tests/Scarb.tomlcrates/dojo/dojo-cairo-test/Scarb.tomlcrates/dojo/dojo-snf-test/Scarb.toml
to avoid mixed resolver states.
Ensure everything compiles cleanly after updating.examples/simple/Scarb.toml (1)
2-2: ohayo sensei, unify version specifiers across all Scarb.toml files
We’ve got mixed pins for cairo-version, starknet, and cairo_test—some use exact (=2.12.1,=2.11.4), others loose minors ("2.12"), or variant patches ("2.12.0"). Pick one strategy (strict patch-pins or minor-range) and apply it consistently across every Scarb.toml.crates/dojo/dojo-cairo-test/Scarb.toml (1)
5-5: Pin tests to exact patch for deterministic CI (optional)If you go with strict pins, align these to 2.12.1 like other crates to avoid test flakiness from future 2.12.x releases.
-version = "1.7.0-alpha.2" -cairo-version = "2.12" -starknet = "2.12" -cairo_test = "2.12" +version = "1.7.0-alpha.2" +cairo-version = "2.12.1" +starknet = "2.12.1" +cairo_test = "2.12.1"Also applies to: 7-7, 10-11
examples/game-lib/bestiary/Scarb.toml (1)
3-3: LGTM on the bump; consider patch pin for cairo_test to match the restOptional alignment with 2.12.1 for reproducible tests.
-version = "1.7.0-alpha.2" -... -cairo_test = "2.12" +version = "1.7.0-alpha.2" +... +cairo_test = "2.12.1"Also applies to: 14-14
crates/dojo/macros/src/inlines/selector_from_tag.rs (1)
21-21: Avoid removing inner quotes accidentallyreplace(""", "") strips all quotes, not just wrapping ones. trim_matches('"') is safer.
- let tag = s.text(db).to_string().replace("\"", ""); + let tag = s.text(db).to_string().trim_matches('\"').to_string();crates/dojo/macros/src/lib.rs (1)
1-1: Crate-wide allow(elided_lifetimes_in_paths): verify necessity and narrow scope if possibleThis can mute useful lifetime diagnostics across the crate. If only helpers/inlines trigger the lint, prefer allowing it at module or item scope and plan to remove once refactors settle.
crates/dojo/macros/src/attributes/contract.rs (1)
335-338: Make the constructor param validation resilient to whitespace/trivia.Relying on
contains("ref self: ContractState")is brittle. Prefer an AST-level check (param isref, name isself, type path isContractState) or at leastget_text_without_trivia(db).contains("refself:ContractState"). I can draft the AST-based matcher if useful.bin/sozo/src/commands/options/account/controller.rs (2)
55-56: Consider handling session expiry before reusing/storing.You already log
expires_at. To avoid stale sessions, check expiry and forceslot_session::createwhen expired.Also applies to: 72-74, 81-83
22-24: Nit: fix typo in docstring (“Catridge” → “Cartridge”).Tiny polish for user-facing docs.
-/// Create a new Catridge Controller account based on session key. +/// Create a new Cartridge Controller account based on a session key.bin/sozo/Cargo.toml (1)
40-40: Tokio feature surface: ensure workspace enables what reqwest/CLI paths need.If not already in the workspace, consider enabling
rt-multi-thread,macros, andtimeto match async usage.- tokio.workspace = true + tokio = { workspace = true, features = ["rt-multi-thread","macros","time"] }crates/dojo/macros/src/derives/introspect/size.rs (1)
27-28: Preferis_empty()overlen() == 0.Idiomatic and avoids usize comparisons.
- if expr.expressions(db).elements(db).len() == 0 { + if expr.expressions(db).elements(db).is_empty() {crates/dojo/utils/src/tx/declarer.rs (2)
121-123: Nit: use the imported Duration and fix a small typo in the comment.
- Keep Duration usage consistent.
- “it event when” → “even when”.
- // Since `0.14`, it event when the transaction is accepted on L2, it might take a while + // Since `0.14`, even when the transaction is accepted on L2, it might take a while // to propagate the transaction to the nodes. - tokio::time::sleep(std::time::Duration::from_secs(5)).await; + tokio::time::sleep(Duration::from_secs(5)).await;
111-118: Make timeout configurable; 60s may be too short under load.Hard-coding
Duration::from_secs(60)can cause premature failures on congested networks. Consider reading fromtxn_config(if available) or exposing a knob with a sensible default.If
TxnConfigalready has a timeout, I can wire it here. Want me to draft that change?crates/dojo/macros/src/derives/introspect/structs.rs (1)
216-219: Simplify usize detection; one pass is enough.The initial
contains("usize")check is redundant. A single split-and-equality pass is sufficient.-fn type_contains_usize(type_str: &str) -> bool { - type_str.contains("usize") - && type_str.split(CAIRO_DELIMITERS).map(|s| s.trim()).collect::<Vec<_>>().contains(&"usize") -} +fn type_contains_usize(type_str: &str) -> bool { + type_str + .split(CAIRO_DELIMITERS) + .any(|s| s.trim() == "usize") +}crates/dojo/macros/src/helpers/checker.rs (1)
37-45: Ohayo, sensei — extract the 26 limit into a constant and tighten the check.Avoid magic numbers and make intent reusable.
- if (element == "model" || element == "event") && name.len() > 26 { + if matches!(element, "model" | "event") && name.len() > MAX_NAME_LEN { let name_len = name.len(); return Some(ProcMacroResult::fail(format!( - "The {element} name '{name}' must be shorter or equal to 26 characters (current \ + "The {element} name '{name}' must be shorter or equal to {MAX_NAME_LEN} characters (current \ length: {name_len})." ))); }Add near the top of this module:
const MAX_NAME_LEN: usize = 26;crates/dojo/macros/src/derives/introspect/enums.rs (2)
143-154: Prefer matching on first element; avoid len() + unwrap() on iterator.
Using len() with ExactSizeIterator then next().unwrap() is a bit brittle. Match on next() directly to avoid unwrap and reduce coupling to iterator semantics.Apply this diff:
- let mut elements = enum_ast.variants(db).elements(db); - let mut variant_layout = if elements.len() == 0 { - vec![] - } else { - match elements.next().unwrap().type_clause(db) { + let mut elements = enum_ast.variants(db).elements(db); + let mut variant_layout = match elements.next() { + None => vec![], + Some(first) => match first.type_clause(db) { OptionTypeClause::Empty(_) => vec![], OptionTypeClause::TypeClause(type_clause) => { super::layout::get_packed_field_layout_from_type_clause( db, &mut self.diagnostics, &type_clause, ) } - } - }; + }, + };
222-226: Tiny simplification: drop the emptiness branch; join("") handles empty.
Removes len() check and a needless temporary.Apply this diff:
- let variants = enum_ast.variants(db).elements(db); - - let variants_ty = if variants.len() == 0 { - "".to_string() - } else { - variants.map(|v| self.build_variant_ty(db, &v)).collect::<Vec<_>>().join(",\n") - }; + let variants_ty = enum_ast + .variants(db) + .elements(db) + .map(|v| self.build_variant_ty(db, &v)) + .collect::<Vec<_>>() + .join(",\n");crates/dojo/macros/src/attributes/event.rs (3)
94-99: Normalize derive list order for deterministic codegen.
Derive order drifts cause noisy diffs. Sort and dedup once after extraction.Apply this diff:
let mut derive_attr_names = DojoParser::extract_derive_attr_names( db, &mut event.diagnostics, struct_ast.attributes(db).query_attr(db, "derive"), ); + derive_attr_names.sort(); + derive_attr_names.dedup();
127-128: EventValue currently inherits all derives from the struct — is that intended?
Comment below says “Only derives strictly required traits.” If you want EventValue to keep just the minimal set, derive the expected ones explicitly instead of copying all.Apply this diff to keep EventValue minimal:
- event.event_value_derive_attr_names = derive_attr_names; + event.event_value_derive_attr_names = EXPECTED_DERIVE_ATTR_NAMES + .iter() + .map(|s| s.to_string()) + .collect::<Vec<_>>(); + if !event.event_value_derive_attr_names.contains(&DOJO_INTROSPECT_DERIVE.to_string()) { + event.event_value_derive_attr_names.push(DOJO_INTROSPECT_DERIVE.to_string()); + }
139-144: Nit: comment no longer matches behavior.
It says “missing derive attributes” but we now emit a full derive attr.Apply this diff:
- // original struct with missing derive attributes + // original struct with normalized derive attributescrates/dojo/macros/src/helpers/formatter.rs (1)
32-43: Align serialize_primitive_member_ty to take &str for consistency.
Minor API polish to mirror deserialize_* and avoid &String.Apply this diff:
- pub(crate) fn serialize_primitive_member_ty( - member_name: &String, + pub(crate) fn serialize_primitive_member_ty( + member_name: &str, with_self: bool, use_serde: bool, ) -> String {And its sole caller already has a String; passing &member_name as &str will keep things zero-cost.
crates/dojo/macros/src/helpers/parser.rs (1)
69-71: Ohayo, sensei — Remove stale commented param.The commented
// members: &[MemberAst],line is noise now that we accept an iterator.
📜 Review details
Configuration used: Path: .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 (7)
Cargo.lockis excluded by!**/*.lockcrates/dojo/core-tests/Scarb.lockis excluded by!**/*.lockcrates/dojo/core/Scarb.lockis excluded by!**/*.lockcrates/dojo/dojo-cairo-test/Scarb.lockis excluded by!**/*.lockexamples/simple/Scarb.lockis excluded by!**/*.lockexamples/spawn-and-move/Scarb.lockis excluded by!**/*.lockspawn-and-move-db.tar.gzis excluded by!**/*.gz
📒 Files selected for processing (70)
.devcontainer/Dockerfile(1 hunks).github/workflows/release.yml(1 hunks).github/workflows/test.yml(6 hunks).tool-versions(1 hunks)Cargo.toml(7 hunks)bin/cairo-bench/src/main.rs(1 hunks)bin/sozo/Cargo.toml(3 hunks)bin/sozo/src/commands/build.rs(3 hunks)bin/sozo/src/commands/options/account/controller.rs(4 hunks)bin/sozo/src/commands/options/account/mod.rs(1 hunks)bin/sozo/src/commands/options/account/type.rs(2 hunks)bin/sozo/src/commands/test.rs(4 hunks)bin/sozo/tests/test_data/policies.json(1 hunks)crates/dojo/core-tests/Scarb.toml(1 hunks)crates/dojo/core-tests/src/tests/helpers/event.cairo(7 hunks)crates/dojo/core-tests/src/tests/helpers/model.cairo(7 hunks)crates/dojo/core-tests/src/tests/world/acl.cairo(4 hunks)crates/dojo/core-tests/src/tests/world/contract.cairo(2 hunks)crates/dojo/core-tests/src/tests/world/event.cairo(5 hunks)crates/dojo/core-tests/src/tests/world/metadata.cairo(1 hunks)crates/dojo/core-tests/src/tests/world/model.cairo(5 hunks)crates/dojo/core-tests/src/tests/world/world.cairo(1 hunks)crates/dojo/core/Scarb.toml(1 hunks)crates/dojo/dojo-cairo-test/Scarb.toml(1 hunks)crates/dojo/dojo-snf-test/Scarb.toml(1 hunks)crates/dojo/macros/Scarb.toml(1 hunks)crates/dojo/macros/src/attributes/contract.rs(1 hunks)crates/dojo/macros/src/attributes/event.rs(2 hunks)crates/dojo/macros/src/attributes/library.rs(0 hunks)crates/dojo/macros/src/attributes/model.rs(5 hunks)crates/dojo/macros/src/derives/dojo_store.rs(3 hunks)crates/dojo/macros/src/derives/introspect/enums.rs(4 hunks)crates/dojo/macros/src/derives/introspect/layout.rs(1 hunks)crates/dojo/macros/src/derives/introspect/size.rs(2 hunks)crates/dojo/macros/src/derives/introspect/structs.rs(6 hunks)crates/dojo/macros/src/derives/introspect/ty.rs(2 hunks)crates/dojo/macros/src/helpers/checker.rs(2 hunks)crates/dojo/macros/src/helpers/diagnostic_ext.rs(1 hunks)crates/dojo/macros/src/helpers/formatter.rs(2 hunks)crates/dojo/macros/src/helpers/misc.rs(1 hunks)crates/dojo/macros/src/helpers/parser.rs(7 hunks)crates/dojo/macros/src/inlines/bytearray_hash.rs(2 hunks)crates/dojo/macros/src/inlines/selector_from_tag.rs(3 hunks)crates/dojo/macros/src/lib.rs(1 hunks)crates/dojo/utils/src/parse.rs(2 hunks)crates/dojo/utils/src/tx/declarer.rs(2 hunks)crates/dojo/utils/src/tx/mod.rs(1 hunks)crates/dojo/utils/src/tx/waiter.rs(2 hunks)crates/dojo/world/src/remote/events_to_remote.rs(2 hunks)crates/macros/merge-options/macro_test/Cargo.toml(1 hunks)crates/sozo/mcp/tests/stdio_tests.rs(1 hunks)crates/sozo/ops/src/migrate/mod.rs(5 hunks)crates/sozo/scarb_interop/src/scarb.rs(3 hunks)crates/sozo/scarb_metadata_ext/src/metadata.rs(4 hunks)examples/game-lib/Scarb.toml(1 hunks)examples/game-lib/armory/Scarb.toml(2 hunks)examples/game-lib/armory/src/lib.cairo(0 hunks)examples/game-lib/bestiary/Scarb.toml(2 hunks)examples/game-lib/bestiary/src/lib.cairo(0 hunks)examples/simple/Scarb.toml(2 hunks)examples/simple/dojo_sepolia.toml(1 hunks)examples/simple/manifest_dev.json(6 hunks)examples/simple/manifest_sepolia.json(6 hunks)examples/simple/src/lib.cairo(1 hunks)examples/spawn-and-move/Scarb.toml(1 hunks)examples/spawn-and-move/dojo_dev.toml(1 hunks)examples/spawn-and-move/manifest_dev.json(15 hunks)examples/spawn-and-move/src/models.cairo(2 hunks)rust-toolchain.toml(1 hunks)scripts/clippy.sh(1 hunks)
💤 Files with no reviewable changes (3)
- examples/game-lib/bestiary/src/lib.cairo
- examples/game-lib/armory/src/lib.cairo
- crates/dojo/macros/src/attributes/library.rs
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-11-05T04:29:12.288Z
Learnt from: glihm
PR: dojoengine/dojo#2633
File: crates/dojo/core/src/world/storage.cairo:484-0
Timestamp: 2024-11-05T04:29:12.288Z
Learning: In the Cairo codebase for the Dojo project, within `crates/dojo/core/src/world/storage.cairo`, length checks between `entity_ids` and `values` are not required in test API functions like `write_values_from_ids_test`.
Applied to files:
crates/dojo/core-tests/src/tests/world/metadata.cairocrates/dojo/core-tests/src/tests/world/world.cairocrates/dojo/core-tests/src/tests/world/model.cairo
📚 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
🧬 Code graph analysis (12)
bin/sozo/src/commands/options/account/mod.rs (3)
bin/sozo/src/commands/options/account/type.rs (1)
provider(172-174)crates/dojo/world/src/contracts/abigen/model.rs (2)
provider(27-29)provider(56-58)crates/dojo/world/src/contracts/abigen/world.rs (2)
provider(27-29)provider(56-58)
crates/dojo/utils/src/tx/declarer.rs (1)
crates/dojo/utils/src/tx/waiter.rs (1)
new(100-115)
crates/dojo/macros/src/helpers/misc.rs (3)
crates/dojo/macros/src/attributes/event.rs (1)
members(83-92)crates/dojo/macros/src/helpers/parser.rs (1)
members(75-104)crates/dojo/types/src/naming.rs (1)
compute_bytearray_hash(84-87)
crates/dojo/macros/src/derives/dojo_store.rs (2)
crates/dojo/macros/src/derives/introspect/structs.rs (2)
struct_ast(82-94)struct_ast(120-124)crates/dojo/macros/src/derives/introspect/enums.rs (1)
enum_ast(92-101)
crates/dojo/macros/src/attributes/contract.rs (1)
crates/dojo/macros/src/helpers/formatter.rs (1)
params(99-109)
bin/sozo/src/commands/options/account/controller.rs (2)
bin/sozo/src/commands/options/account/mod.rs (1)
account(70-95)bin/sozo/src/commands/options/account/type.rs (2)
provider(172-174)chain_id(108-114)
crates/dojo/macros/src/helpers/checker.rs (1)
crates/dojo/macros/src/helpers/parser.rs (1)
attrs(124-147)
crates/sozo/ops/src/migrate/mod.rs (5)
bin/sozo/src/commands/options/account/provider.rs (1)
block_number(268-273)crates/dojo/utils/src/tx/declarer.rs (2)
declare(86-147)new(52-54)crates/dojo/utils/src/tx/waiter.rs (1)
new(100-115)crates/dojo/utils/src/tx/deployer.rs (1)
new(34-36)crates/dojo/world/src/contracts/abigen/world.rs (2)
new(15-23)new(44-52)
crates/dojo/macros/src/helpers/parser.rs (2)
crates/dojo/macros/src/attributes/event.rs (1)
members(83-92)crates/dojo/macros/src/helpers/misc.rs (1)
members(29-38)
crates/dojo/macros/src/attributes/model.rs (3)
crates/dojo/macros/src/helpers/parser.rs (1)
extract_derive_attr_names(119-148)crates/dojo/macros/src/derives/introspect/structs.rs (2)
struct_ast(82-94)struct_ast(120-124)crates/dojo/macros/src/helpers/formatter.rs (2)
serialize_keys_and_values(72-88)deserialize_member_ty(45-58)
crates/dojo/macros/src/attributes/event.rs (4)
crates/dojo/macros/src/derives/introspect/structs.rs (2)
struct_ast(82-94)struct_ast(120-124)crates/dojo/macros/src/helpers/formatter.rs (1)
serialize_keys_and_values(72-88)crates/dojo/macros/src/helpers/parser.rs (1)
extract_derive_attr_names(119-148)crates/dojo/macros/src/helpers/misc.rs (1)
compute_unique_hash(18-41)
crates/dojo/macros/src/helpers/formatter.rs (2)
crates/dojo/macros/src/helpers/misc.rs (1)
members(29-38)crates/dojo/macros/src/helpers/parser.rs (1)
members(75-104)
⏰ 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: build
| env: | ||
| CARGO_TERM_COLOR: always | ||
| RUST_VERSION: 1.86.0 | ||
| RUST_VERSION: 1.88.0 |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ohayo, sensei — RUST_VERSION=1.88.0 matches toolchain; ship it.
Optional: upgrade rust-cache to v2 and pin cargo-get to a version for determinism.
Example (outside changed lines):
- uses: Swatinem/rust-cache@v1
+ uses: Swatinem/rust-cache@v2
...
- run: cargo install cargo-get
+ run: cargo install cargo-get --version ^1.0.0Confirm all workflows/tests use 1.88.0:
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -nP 'RUST_VERSION\s*:\s*.*' .github/workflows -n
rg -nP 'dtolnay/rust-toolchain@' .github/workflows -n -C2Length of output: 648
Bump RUST_VERSION to 1.88.0 in all workflows
- .github/workflows/docker_base.yml (line 21):
RUST_VERSION: 1.86.0→1.88.0 - .github/workflows/docker.yml (line 28):
RUST_VERSION: 1.86.0→1.88.0
Optional: upgrade Swatinem/rust-cache to v2, pin cargo-get (--version ^1.0.0), and tag dtolnay/rust-toolchain for deterministic CI setup.
🤖 Prompt for AI Agents
In .github/workflows/release.yml around line 23, the RUST_VERSION is set to
1.86.0 and needs to be updated to 1.88.0 to match other workflows; change the
RUST_VERSION value to 1.88.0 here and also update
.github/workflows/docker_base.yml (line ~21) and .github/workflows/docker.yml
(line ~28) to 1.88.0 for consistency, and optionally upgrade Swatinem/rust-cache
to v2, pin cargo-get by adding an explicit --version ^1.0.0 where it is invoked,
and reference a tagged dtolnay/rust-toolchain to ensure deterministic CI
toolchain resolution.
| scarb dev-2025-09-05 | ||
| starknet-foundry 0.48.1 | ||
| cairo-profiler 0.9.0 | ||
| katana 1.7.0-alpha.3 |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ohayo, sensei — Tool bumps look coherent; ensure CI/dev parity.
These pins should match CI setup-scarb and local dev expectations.
Sanity check parity with CI and docs:
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo ".tool-versions:"
cat .tool-versions
echo -e "\nWorkflows referencing Scarb/Katana/Foundry:"
rg -nP 'setup-scarb|scarb dev-|katana|starknet-foundry' .github/workflows -C2
echo -e "\nLocate any older tool pins lingering:"
rg -nP '(nightly-20|0\.43\.)' -n . || echo "No legacy pins found."Length of output: 3505
Ohayo, sensei — bench CI still uses an old scarb nightly and legacy pins remain.
- In
.github/workflows/bench.yml, bumpscarb-version: "nightly-2025-05-08"to"dev-2025-09-05"for CI/dev parity. - Align remaining legacy pins (
+nightly-2025-05-01,+nightly-2024-08-28, example metadata JSON,Scarb.lockversion 0.43.1) with.tool-versionsor document why they stay.
🤖 Prompt for AI Agents
In .tool-versions lines 1-4 and related CI/config files, update the scarb pin in
.github/workflows/bench.yml from "nightly-2025-05-08" to "dev-2025-09-05" to
match .tool-versions; then find and replace other legacy scarb pins (e.g.
nightly-2025-05-01, nightly-2024-08-28) and any example metadata JSON or
Scarb.lock entries to the new dev-2025-09-05 value, or add a brief comment in
those files explaining why a different pin/version is intentionally kept if you
must preserve it. Ensure all changes are consistent across CI, lockfiles, and
example metadata so CI/dev parity is maintained.
| /// Disable Scarb warnings. | ||
| #[arg(long)] | ||
| #[arg(help = "Scarb warnings are not shown in the output.")] | ||
| pub no_scarb_warnings: bool, | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Validate Scarb flag: ensure '--verbosity no-warnings' is supported in the targeted Scarb version.
Great UX addition. Please confirm this verbosity value exists for the Scarb version you ship against.
🌐 Web query:
Does Scarb support `--verbosity no-warnings`, and from which version?
💡 Result:
Short answer: No — Scarb does not support a literal "--verbosity no-warnings". Scarb's CLI accepts a numeric verbosity scale (0–6) via --verbosity, and warning behaviour is controlled via the Cairo manifest flag (cairo.allow-warnings) or environment/config variables. [1][2]
Sources:
- Scarb CLI: --verbosity described as numeric scale (0..6). [1]
- Scarb manifest: cairo.allow-warnings setting (controls whether warnings fail the build). [2]
References:
[1] Scarb docs — command-line / Scarab-based apps (mentions --verbosity scale).
[2] Scarb docs — Manifest format (cairo.allow-warnings).
ohayo sensei,
Use supported Scarb verbosity options
Scarb doesn’t support --verbosity no-warnings (it accepts a numeric scale 0–6). Update the --no-scarb-warnings flag to pass --verbosity 0 or manage warnings via cairo.allow-warnings in the manifest.
🤖 Prompt for AI Agents
In bin/sozo/src/commands/test.rs around lines 47 to 51, the --no-scarb-warnings
boolean flag currently implies passing an unsupported Scarb option; update the
flag handling so it does not attempt to use "--verbosity no-warnings". Instead,
keep the CLI flag (rename if desired) but map it to pass "--verbosity 0" to
Scarb when invoked, or alternatively instruct users to set cairo.allow-warnings
in the manifest; ensure the code that builds the Scarb command uses
"--verbosity" with the numeric value 0 when this flag is set.
| "target": "0x4ca02e0da71179ec7d0c250c717eb90d70f599282600e545dc1a07de9e4f6f9", | ||
| "method": "upgrade" | ||
| }, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
ohayo, sensei — likely malformed hex address (length 63 nibbles)
"0x4ca02e0da71179ec7d0c250c717eb90d70f599282600e545dc1a07de9e4f6f9" appears one nibble short (last chunk is 7 hex chars). This can break strict hex parsers and mismatches with manifests.
If this address was copied from a manifest, please replace it with the exact 64-hex-digit form.
🏁 Script executed:
#!/bin/bash
# Validate all targets are 0x + 64 hex digits
python - <<'PY'
import json,re,sys
p='bin/sozo/tests/test_data/policies.json'
data=json.load(open(p))
bad=[]
for i,e in enumerate(data,1):
t=e.get('target','')
if not re.fullmatch(r'0x[0-9a-f]{64}', t):
bad.append((i,t))
print("Invalid targets:" if bad else "All targets valid.")
for i,t in bad: print(f"line?<{i}> {t}")
PYLength of output: 3590
ohayo, sensei — fix all malformed target hex strings in bin/sozo/tests/test_data/policies.json
Each target value fails to match ^0x[0-9a-f]{64}$ (all entries are currently the wrong length); replace each with the exact 64-hex-digit form.
🤖 Prompt for AI Agents
In bin/sozo/tests/test_data/policies.json around lines 175-177, the "target" hex
strings are malformed (wrong length) and do not match the required pattern
^0x[0-9a-f]{64}$. Find every "target" entry in this file and replace each value
with the exact 0x-prefixed 64 lowercase hex-digit string (pad/replace so there
are exactly 64 hex chars after 0x); ensure no extra characters, use lowercase
a-f and keep quotes and commas intact.
| slot-core = { git = "https://github.com/cartridge-gg/slot", branch = "compartmentalization" } | ||
| slot-session = { git = "https://github.com/cartridge-gg/slot", branch = "compartmentalization" } | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Pin slot deps to a commit for reproducible builds.
Branch pins can drift and silently break the workspace.
Apply:
-slot-core = { git = "https://github.com/cartridge-gg/slot", branch = "compartmentalization" }
-slot-session = { git = "https://github.com/cartridge-gg/slot", branch = "compartmentalization" }
+slot-core = { git = "https://github.com/cartridge-gg/slot", rev = "<pin_exact_commit_sha>" }
+slot-session = { git = "https://github.com/cartridge-gg/slot", rev = "<pin_exact_commit_sha>" }🏁 Script executed:
#!/bin/bash
# Show current HEAD of the branch to help pick a rev to pin
git ls-remote https://github.com/cartridge-gg/slot refs/heads/compartmentalization | awk '{print $1 " compartmentalization"}'Length of output: 191
ohayo sensei: Pin slot deps to commit 29baeb6055d8ee6d31adcdb2e53f823cc3b57efb
In Cargo.toml (lines 228–230), replace the branch refs with this exact rev for reproducible builds:
-slot-core = { git = "https://github.com/cartridge-gg/slot", branch = "compartmentalization" }
-slot-session = { git = "https://github.com/cartridge-gg/slot", branch = "compartmentalization" }
+slot-core = { git = "https://github.com/cartridge-gg/slot", rev = "29baeb6055d8ee6d31adcdb2e53f823cc3b57efb" }
+slot-session = { git = "https://github.com/cartridge-gg/slot", rev = "29baeb6055d8ee6d31adcdb2e53f823cc3b57efb" }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| slot-core = { git = "https://github.com/cartridge-gg/slot", branch = "compartmentalization" } | |
| slot-session = { git = "https://github.com/cartridge-gg/slot", branch = "compartmentalization" } | |
| slot-core = { git = "https://github.com/cartridge-gg/slot", rev = "29baeb6055d8ee6d31adcdb2e53f823cc3b57efb" } | |
| slot-session = { git = "https://github.com/cartridge-gg/slot", rev = "29baeb6055d8ee6d31adcdb2e53f823cc3b57efb" } |
🤖 Prompt for AI Agents
In Cargo.toml around lines 228-230, the two dependencies reference a branch
which hinders reproducible builds; replace the branch = "compartmentalization"
entries for slot-core and slot-session with rev =
"29baeb6055d8ee6d31adcdb2e53f823cc3b57efb" (keep the same git URL), so both
entries pin to that exact commit; after updating the file, run cargo update -p
slot-core -p slot-session or cargo fetch to lock the resolved revision in
Cargo.lock.
| account_address = "0x2af9427c5a277474c079a1283c880ee8a6f0f8fbf73ce969c08d88befec1bba" | ||
| private_key = "0x1800000000300000180000000000030000000000003006001800006600" | ||
| world_address = "0x46e3f86feb22601de72852319d7e155b8b22d15a13d89c751515fcd408a3f29" | ||
| world_address = "0x72b2128fc2db8362beb744243d56c41022b93dfa8ca8d44c8be85764ecbdea1" |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify world_address consistency with manifests/docs
Confirm the new world_address matches examples/spawn-and-move/manifest_dev.json and any quickstart docs, otherwise deploys/scripts will drift.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ADDR='0x72b2128fc2db8362beb744243d56c41022b93dfa8ca8d44c8be85764ecbdea1'
rg -n "$ADDR" examples/spawn-and-move -S || { echo "Address not found in manifests"; exit 1; }Length of output: 446
ohayo sensei! Let’s verify the world_address in any quickstart docs—search all markdown files:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ADDR='0x72b2128fc2db8362beb744243d56c41022b93dfa8ca8d44c8be85764ecbdea1'
rg -n "$ADDR" -g '*.md' -SLength of output: 179
Add or update world_address in quickstart docs
The new world_address matches manifest_dev.json but isn’t referenced in any Markdown docs—please update your documentation or quickstart guides to include it.
| ipfs_config.username = "2EBrzr7ZASQZKH32sl2xWauXPSA" | ||
| ipfs_config.password = "12290b883db9138a8ae3363b6739d220" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove hard-coded IPFS credentials from repo and rotate immediately
Credentials (even for examples) shouldn’t live in VCS. Replace with placeholders or env-based config and rotate any exposed secrets.
-ipfs_config.username = "2EBrzr7ZASQZKH32sl2xWauXPSA"
-ipfs_config.password = "12290b883db9138a8ae3363b6739d220"
+# Set via environment or a non-committed .env file
+# ipfs_config.username = "${IPFS_USERNAME}"
+# ipfs_config.password = "${IPFS_PASSWORD}"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ipfs_config.username = "2EBrzr7ZASQZKH32sl2xWauXPSA" | |
| ipfs_config.password = "12290b883db9138a8ae3363b6739d220" | |
| # Set via environment or a non-committed .env file | |
| # ipfs_config.username = "${IPFS_USERNAME}" | |
| # ipfs_config.password = "${IPFS_PASSWORD}" |
🤖 Prompt for AI Agents
In examples/spawn-and-move/dojo_dev.toml around lines 74 to 75, there are
hard-coded IPFS credentials committed to the repo; remove these literal values
and replace them with placeholder values or load them from environment variables
(e.g., ipfs_config.username = "${IPFS_USERNAME}" and ipfs_config.password =
"${IPFS_PASSWORD}") or reference a secrets file not checked into VCS; after
updating the file, rotate the exposed credentials immediately and add the toml
file or pattern to .gitignore or update documentation to instruct using
env-based config so secrets are not stored in source control.
| "class_hash": "0x7bc7b264612d3ac261df0a6d72a6b73c84b9386ef0095dd340d42afc01bb53c", | ||
| "address": "0x72b2128fc2db8362beb744243d56c41022b93dfa8ca8d44c8be85764ecbdea1", | ||
| "seed": "dojo_examples", |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ohayo, sensei — Validate world address/class hash consistency across configs.
World identity changed. Ensure examples/spawn-and-move/dojo_dev.toml uses the same world_address, and purge cached artifacts referencing the old world.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
MANIFEST="examples/spawn-and-move/manifest_dev.json"
TOML="examples/spawn-and-move/dojo_dev.toml"
world_addr_json=$(jq -r '.world.address' "$MANIFEST")
echo "manifest world.address = $world_addr_json"
world_addr_toml=$(awk -F'=' '/^\s*world_address\s*=/ {gsub(/[ "]/,"",$2); print $2}' "$TOML")
echo "dojo_dev.toml world_address = $world_addr_toml"
test "$world_addr_json" = "$world_addr_toml" && echo "OK: addresses match" || { echo "MISMATCH"; exit 1; }Length of output: 1039
🏁 Script executed:
#!/bin/bash
set -euo pipefail
MANIFEST="examples/spawn-and-move/manifest_dev.json"
TOML="examples/spawn-and-move/dojo_dev.toml"
manifest_class_hash=$(jq -r '.world.class_hash' "$MANIFEST")
echo "manifest world.class_hash = $manifest_class_hash"
grep -Rn "class_hash" "$TOML" || echo "dojo_dev.toml has no class_hash"Length of output: 604
Ohayo, sensei — Add missing world.class_hash to dojo_dev.toml
The world.class_hash in examples/spawn-and-move/manifest_dev.json (0x7bc7b264612d3ac261df0a6d72a6b73c84b9386ef0095dd340d42afc01bb53c) isn’t defined in examples/spawn-and-move/dojo_dev.toml. Add under the [world] section:
class_hash = "0x7bc7b264612d3ac261df0a6d72a6b73c84b9386ef0095dd340d42afc01bb53c"Then purge any cached artifacts referencing the old world identity.
| @@ -1,2 +1,2 @@ | |||
| [toolchain] | |||
| channel = "1.86.0" | |||
| channel = "1.88.0" | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ohayo, sensei — Rust 1.88.0 pin LGTM; verify repo-wide alignment.
Looks consistent with the workflow bumps. Please confirm no stragglers (devcontainer, other workflows, docs).
Run to spot mismatches:
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "rust-toolchain.toml:"
rg -nP '^\s*channel\s*=\s*"([^"]+)"' rust-toolchain.toml
echo -e "\nWorkflows referencing RUST_VERSION:"
rg -nP 'RUST_VERSION\s*:\s*.*' .github/workflows
echo -e "\nDevcontainer ARGs:"
rg -nP 'ARG\s+RUST_VERSION\s*=\s*.*' -n . -g '.devcontainer/**/Dockerfile'
echo -e "\nAny explicit toolchain pins elsewhere:"
rg -nP 'toolchain:\s*([0-9]+\.[0-9]+\.[0-9]+|stable|beta|nightly)' .github/workflows -n -C2Length of output: 640
Ohayo, sensei — Update RUST_VERSION pins in workflows
- .github/workflows/docker_base.yml: line 21 still sets RUST_VERSION: 1.86.0
- .github/workflows/docker.yml: line 28 still sets RUST_VERSION: 1.86.0
Bump both to 1.88.0 to match rust-toolchain.toml.
🤖 Prompt for AI Agents
In rust-toolchain.toml around line 2, the channel is set to "1.88.0" but the
GitHub workflow files still pin RUST_VERSION to 1.86.0; update
.github/workflows/docker_base.yml (line ~21) and .github/workflows/docker.yml
(line ~28) to set RUST_VERSION: 1.88.0 so the workflows match
rust-toolchain.toml, commit the changes and run CI to verify.
|
|
||
| run_clippy() { | ||
| cargo +nightly-2025-02-20 clippy --all-targets "$@" -- -D warnings -D future-incompatible -D nonstandard-style -D rust-2018-idioms -D unused -D missing-debug-implementations | ||
| cargo +nightly-2025-05-01 clippy --all-targets "$@" -- -D warnings -D future-incompatible -D nonstandard-style -D rust-2018-idioms -D unused -D missing-debug-implementations -A clippy::uninlined_format_args |
There was a problem hiding this comment.
Ohayo, sensei — hard-pinning nightly may break CI unless installed.
Workflows don’t install nightly-2025-05-01. Either install it in CI or drop the override and rely on rust-toolchain.toml (now 1.88.0).
Preferred change (use repo toolchain):
- cargo +nightly-2025-05-01 clippy --all-targets "$@" -- -D warnings -D future-incompatible -D nonstandard-style -D rust-2018-idioms -D unused -D missing-debug-implementations -A clippy::uninlined_format_args
+ cargo clippy --all-targets "$@" -- -D warnings -D future-incompatible -D nonstandard-style -D rust-2018-idioms -D unused -D missing-debug-implementations -A clippy::uninlined_format_argsAlternative: add a CI step to rustup toolchain install nightly-2025-05-01 in jobs calling this script.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| cargo +nightly-2025-05-01 clippy --all-targets "$@" -- -D warnings -D future-incompatible -D nonstandard-style -D rust-2018-idioms -D unused -D missing-debug-implementations -A clippy::uninlined_format_args | |
| cargo clippy --all-targets "$@" -- -D warnings -D future-incompatible -D nonstandard-style -D rust-2018-idioms -D unused -D missing-debug-implementations -A clippy::uninlined_format_args |
🤖 Prompt for AI Agents
In scripts/clippy.sh around line 11, the cargo invocation hard-pins the nightly
toolchain to "nightly-2025-05-01" which may not be installed in CI; either
remove the "+nightly-2025-05-01" override so cargo uses the repository toolchain
(rust-toolchain.toml / stable 1.88.0), or if you must keep the pin, add a CI job
step that runs "rustup toolchain install nightly-2025-05-01" before this script
is invoked; update the script or CI workflows accordingly to ensure the
requested toolchain is available.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
.github/workflows/test.yml (4)
65-66: Ohayo, sensei — DRY Scarb version and make it easier to bump.Same literal appears 4x. Define once in env and reference everywhere.
env: CARGO_TERM_COLOR: always - RUST_VERSION: 1.88.0 + RUST_VERSION: 1.88.0 + SCARB_VERSION: dev-2025-09-05 @@ - - uses: software-mansion/setup-scarb@v1 - with: - scarb-version: "dev-2025-09-05" + - uses: software-mansion/setup-scarb@v1 + with: + scarb-version: "${{ env.SCARB_VERSION }}" @@ - - uses: software-mansion/setup-scarb@v1 - with: - scarb-version: "dev-2025-09-05" + - uses: software-mansion/setup-scarb@v1 + with: + scarb-version: "${{ env.SCARB_VERSION }}" @@ - - uses: software-mansion/setup-scarb@v1 - with: - scarb-version: "dev-2025-09-05" + - uses: software-mansion/setup-scarb@v1 + with: + scarb-version: "${{ env.SCARB_VERSION }}" @@ - - uses: software-mansion/setup-scarb@v1 - with: - scarb-version: "dev-2025-09-05" + - uses: software-mansion/setup-scarb@v1 + with: + scarb-version: "${{ env.SCARB_VERSION }}"Also applies to: 123-124, 133-134, 150-151
75-79: Ohayo, sensei — harden Katana download step.Add fail-fast flags, parameterize version, and verify the binary after extraction. Consider checksum verification if the release provides it.
- - name: Download Katana for integration tests - run: | - curl -L https://github.com/dojoengine/katana/releases/download/v1.7.0-alpha.3/katana_v1.7.0-alpha.3_linux_amd64.tar.gz -o katana.tar.gz; - tar -C /usr/local/bin -xzf katana.tar.gz + - name: Download Katana for integration tests + run: | + set -euo pipefail + KATANA_VERSION="v1.7.0-alpha.3" + URL="https://github.com/dojoengine/katana/releases/download/${KATANA_VERSION}/katana_${KATANA_VERSION}_linux_amd64.tar.gz" + curl -fL "$URL" -o katana.tar.gz + tar -C /usr/local/bin -xzf katana.tar.gz + /usr/local/bin/katana --version
130-130: Ohayo, sensei — nice add of snfoundry; consider pinning the action.For supply-chain hygiene, pin actions to a commit SHA (or use a version+SHA mapping) rather than a moving tag.
Also applies to: 147-147
92-96: Ohayo, sensei — actions-rs/toolchain is archived; consider switching.Migrate to dtolnay/rust-toolchain or actions-rust-lang/setup-rust for future-proofing on Windows.
Example:
- - uses: actions-rs/toolchain@v1 - with: - toolchain: ${{ env.RUST_VERSION }} - target: x86_64-pc-windows-msvc + - uses: dtolnay/rust-toolchain@stable + with: + toolchain: ${{ env.RUST_VERSION }} + targets: x86_64-pc-windows-msvc
📜 Review details
Configuration used: Path: .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)
.github/workflows/test.yml(5 hunks)
⏰ 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: build
🔇 Additional comments (2)
.github/workflows/test.yml (2)
35-36: Ohayo, sensei — verify container image/toolchain alignment.All Linux jobs use ghcr.io/dojoengine/dojo-dev:v1.7.0-alpha.1 while Rust is bumped to 1.88.0 and Katana to v1.7.0-alpha.3. Please confirm the image ships Rust 1.88.0 and compatible Scarb/Cairo bits, or bump the image to the matching alpha.3 to avoid subtle version drift.
Also applies to: 60-61, 168-169, 182-183, 192-193
102-105: Ohayo, sensei — verify token casing.arduino/setup-protoc typically uses secrets.GITHUB_TOKEN; here secrets.github_token may be undefined unless explicitly created.
| env: | ||
| CARGO_TERM_COLOR: always | ||
| RUST_VERSION: 1.86.0 | ||
| RUST_VERSION: 1.88.0 |
There was a problem hiding this comment.
Ohayo, sensei — Rust 1.88.0 bump looks good, but Windows job isn’t using it due to env name mismatch.
You set RUST_VERSION globally, but the Windows toolchain step references env.rust_version (lowercase). That resolves to empty and likely falls back to default/stable.
Apply:
- - uses: actions-rs/toolchain@v1
- with:
- toolchain: ${{ env.rust_version }}
+ - uses: actions-rs/toolchain@v1
+ with:
+ toolchain: ${{ env.RUST_VERSION }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| RUST_VERSION: 1.88.0 | |
| - uses: actions-rs/toolchain@v1 | |
| with: | |
| toolchain: ${{ env.RUST_VERSION }} |
🤖 Prompt for AI Agents
In .github/workflows/test.yml around line 28, the global env sets RUST_VERSION:
1.88.0 but the Windows job reads env.rust_version (lowercase) so it resolves
empty; fix by updating the Windows toolchain step to reference the correct
variable name ${{ env.RUST_VERSION }} (or rename the global env to rust_version
to match), ensuring the Windows job uses Rust 1.88.0.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/sozo/ops/src/tests/migration.rs (1)
128-129: Ohayo, sensei — Avoid blanket#[ignore]; isolate or serialize the migration test
Fix the grammar in the ignore message (change “an other” to “another”):
-#[ignore = "Flaky: this test passes when run alone and sometimes when all tests are run, an other \ - test may be cleaning the dev build."] +#[ignore = "Flaky: this test passes when run alone and sometimes when all tests are run; another \ + test may be cleaning the dev build."]Instead of ignoring, isolate the shared “dev build” state:
- Use
tempfile::TempDirandstd::env::set_var("SCARB_TARGET_DIR", tmp.path())inside the test.- Or modify
copy_spawn_and_move_db()to return a unique path per test.Alternatively, serialize this test with
serial_testto prevent concurrent cleaners.
📜 Review details
Configuration used: Path: .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)
crates/sozo/ops/src/tests/migration.rs(1 hunks)
⏰ 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
2 participants
Summary by CodeRabbit
New Features
Changes
Chores