feat(sozo): add invoke command for standalone contract #3384
Conversation
|
Ohayo, sensei! 👋 Here's the breakdown of these changes: WalkthroughThis PR introduces a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Areas requiring extra attention:
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
crates/dojo/world/src/config/calldata_decoder.rs (1)
36-52: Ohayo sensei! The implementation looks solid, but consider adding a unit test.The
SelectorCalldataDecoderfollows the established pattern in this file. However, I notice there's no corresponding unit test for the newselector:prefix, unlike other decoders which have test coverage.Consider adding a test case:
#[test] fn test_selector_decoder() { let input = vec_of_strings!["selector:transfer"]; let expected = vec![get_selector_from_name("transfer").unwrap()]; let result = decode_calldata(&input).unwrap(); assert_eq!(result, expected); }bin/sozo/src/commands/mod.rs (1)
134-134: This match arm appears to be unreachable dead code, sensei.Looking at
main.rs(lines 50-51),Commands::Invokeis dispatched directly before reaching thecommands::run()function. This means theCommands::Invokecase here will never execute.Consider either:
- Removing this match arm and adding
Commands::Invoke(_)to an unreachable pattern likeCommands::Init, or- Adding an explicit comment explaining why it exists (e.g., for future use or consistency).
Commands::Invoke(args) => args.run(ui).await, + // Note: This case is handled in main.rs before reaching this function. + // Kept here for pattern exhaustiveness.Or align with
Init:- Commands::Invoke(args) => args.run(ui).await, ... Commands::Init(_) => { // `sozo init` is directly managed in main.rs as scarb metadata // cannot be loaded in this case (the project does not exist yet). Ok(()) } + Commands::Invoke(_) => { + // `sozo invoke` is directly managed in main.rs as it doesn't + // require the world context (Scarb.toml). + Ok(()) + }bin/sozo/src/commands/invoke.rs (1)
68-69: Consider validating entrypoint before selector computation, sensei.If the entrypoint string contains non-ASCII characters,
get_selector_from_namewill fail with a potentially cryptic error. TheSelectorCalldataDecoderyou added provides a nicer error message for this case.You could add context to the error:
- let selector = get_selector_from_name(&entrypoint)?; + let selector = get_selector_from_name(&entrypoint) + .with_context(|| format!("Invalid entrypoint name `{entrypoint}`"))?;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
bin/sozo/src/commands/execute.rs(1 hunks)bin/sozo/src/commands/invoke.rs(1 hunks)bin/sozo/src/commands/mod.rs(5 hunks)bin/sozo/src/main.rs(1 hunks)crates/dojo/world/src/config/calldata_decoder.rs(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
bin/sozo/src/commands/mod.rs (2)
crates/dojo/utils/src/tx/invoker.rs (1)
invoke(57-78)crates/dojo/types/src/naming.rs (1)
f(15-15)
bin/sozo/src/commands/invoke.rs (3)
bin/sozo/src/commands/options/account/mod.rs (1)
account(76-101)bin/sozo/src/utils.rs (1)
get_account_from_env(190-197)crates/dojo/world/src/config/calldata_decoder.rs (1)
decode_single_calldata(240-261)
⏰ 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 (9)
bin/sozo/src/commands/execute.rs (1)
22-22: Ohayo, sensei! LGTM on the description clarification.The updated description clearly distinguishes this command from the new
invokecommand by emphasizing "world context". This helps users understand when to useexecutevsinvoke.crates/dojo/world/src/config/calldata_decoder.rs (1)
245-245: LGTM on the selector prefix integration.The new
selectorprefix is properly wired into the decode dispatch, consistent with other prefix handlers.bin/sozo/src/main.rs (1)
50-51: Ohayo sensei! Perfect placement for the standalone command.The
Invokecommand is correctly placed alongsideInit,Declare, andDeployin the early dispatch block, ensuring it can run independently ofScarb.tomland world context as intended by the PR objectives.bin/sozo/src/commands/mod.rs (1)
21-21: Ohayo sensei! Clean integration of the new invoke module.The module declaration, import, enum variant, and Display implementation all follow the established patterns in this file.
Also applies to: 41-41, 64-65, 105-105
bin/sozo/src/commands/invoke.rs (5)
1-14: Ohayo sensei! Clean imports for the new command.The imports are well-organized and include everything needed for the invoke functionality.
56-66: LGTM on separator handling and entrypoint validation.The separator tokens (
/,-,\) are correctly skipped, and missing entrypoints produce a clear error message guiding the user on the expected format.
71-83: Ohayo! Good calldata parsing with contextual errors.The use of
.with_context()on line 77-78 provides helpful error messages when calldata parsing fails. The separator break logic is consistent with the target parsing above.
96-98: Good defensive check for empty calls.Prevents confusing behavior if the user somehow provides only separators or the parsing logic consumes everything.
122-130: Clean address parsing with appropriate fallback.The hex-first, decimal-fallback approach is intuitive and the error message clearly guides users on valid formats.
| #[arg(long, default_value = "0x0", help = "Selector for the entrypoint in felt form.")] | ||
| pub selector: Option<Felt>, |
There was a problem hiding this comment.
The selector field is defined but never used, sensei.
This argument is declared with a default value of 0x0 and help text suggesting it's for passing an entrypoint selector in felt form. However, in the run() method (line 69), the selector is always computed from the entrypoint name using get_selector_from_name(&entrypoint).
Either:
- Remove the unused field if it's not needed, or
- Implement the intended behavior where users can optionally pass a raw selector instead of an entrypoint name.
If removal is intended:
- #[arg(long, default_value = "0x0", help = "Selector for the entrypoint in felt form.")]
- pub selector: Option<Felt>,
-If the feature should work, the logic would need to check if a custom selector is provided and use it instead of computing from the entrypoint name.
📝 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.
| #[arg(long, default_value = "0x0", help = "Selector for the entrypoint in felt form.")] | |
| pub selector: Option<Felt>, |
🤖 Prompt for AI Agents
In bin/sozo/src/commands/invoke.rs around lines 37-38, the selector field is
declared but never used; update the run() logic to honor this optional CLI
selector: if self.selector is Some(use that Felt) then use it as the entrypoint
selector, otherwise compute the selector with
get_selector_from_name(&entrypoint). Also remove or adjust the default_value
"0x0" (so the arg truly behaves as optional) and update help text if needed;
alternatively, if the field is not required, delete the selector arg and any
associated docs instead of wiring it into run().
| for (idx, result) in results.iter().enumerate() { | ||
| let display_idx = idx + 1; | ||
| match result { | ||
| dojo_utils::TransactionResult::Noop => { | ||
| ui.result(format!("Call #{display_idx} noop (no transaction sent).")); | ||
| } | ||
| dojo_utils::TransactionResult::Hash(hash) => { | ||
| ui.result(format!("Call #{display_idx} sent.\n Tx hash : {hash:#066x}")); | ||
| } | ||
| dojo_utils::TransactionResult::HashReceipt(hash, receipt) => { | ||
| ui.result(format!("Call #{display_idx} included.\n Tx hash : {hash:#066x}")); | ||
| ui.debug(format!("Receipt: {:?}", receipt)); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for the multicall method in invoker.rs
rg -n -A 20 "pub async fn multicall" crates/dojo/utils/src/tx/invoker.rsRepository: dojoengine/dojo
Length of output: 864
🏁 Script executed:
#!/bin/bash
# Get more of the multicall implementation
rg -n -A 50 "pub async fn multicall" crates/dojo/utils/src/tx/invoker.rs | head -80Repository: dojoengine/dojo
Length of output: 2202
🏁 Script executed:
#!/bin/bash
# Get the complete multicall implementation including the else branch
rg -n -A 70 "pub async fn multicall" crates/dojo/utils/src/tx/invoker.rs | tail -50Repository: dojoengine/dojo
Length of output: 2312
🏁 Script executed:
#!/bin/bash
# Check the invoke.rs file around lines 102-116
cat -n bin/sozo/src/commands/invoke.rs | sed -n '85,125p'Repository: dojoengine/dojo
Length of output: 1830
🏁 Script executed:
#!/bin/bash
# Search for max_calls configuration
rg -n "max_calls" crates/dojo/utils/src/tx/ -A 2 -B 2Repository: dojoengine/dojo
Length of output: 2972
The UI output message is misleading about transaction batching—display_idx represents transaction/batch number, not individual call number.
The verification confirms the reviewer's concern:
- Default behavior (
max_calls = None):multicall()returns a singleTransactionResultfor all calls bundled into one transaction. - With chunking (
max_calls = Some(n)): Returns oneTransactionResultper chunk, not per individual call. - The issue: The display says
"Call #{display_idx} sent", which implies one result per user-provided call. However,display_idxactually represents the transaction or chunk number.
During input, users see "Call #1", "Call #2", "Call #3" (per individual call), but in the output they see "Call #1 sent" which appears to correspond to the same calls—when actually it represents the transaction/batch result.
Consider updating the output message to clarify this is a batched transaction result, such as:
"Transaction #{display_idx} sent..."or"Batch #{display_idx} sent..."or- Add context like
"All {n} calls in batch #{display_idx} sent..."
🤖 Prompt for AI Agents
In bin/sozo/src/commands/invoke.rs around lines 102 to 116, the UI text wrongly
calls each result "Call #{display_idx}" even though each result represents a
transaction/batch (one per multicall chunk), so update the messages to refer to
transactions/batches (e.g. "Transaction #{display_idx} ..." or "Batch
#{display_idx} ...") and, where possible, include the number of user calls in
that batch (e.g. "All {n} calls in batch #{display_idx} sent...") for the Noop,
Hash and HashReceipt branches so output clearly reflects transaction/chunk
semantics.
The invoke command doesn't depend on the dojo context. This can be called from anywhere without a
Scarb.tomlfile required.Summary by CodeRabbit
Release Notes
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.