fix(utils/invoker): add split for big multicalls - #3284
Conversation
WalkthroughOhayo sensei! This update introduces support for splitting multicall transactions into multiple chunks based on a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI_Command
participant Invoker
participant Blockchain
User->>CLI_Command: Run command with multicall
CLI_Command->>Invoker: multicall(max_calls)
Invoker->>Invoker: Split calls into chunks (if max_calls)
loop For each chunk
Invoker->>Blockchain: Send transaction
Blockchain-->>Invoker: TransactionResult
end
Invoker-->>CLI_Command: Vec<TransactionResult>
CLI_Command->>CLI_Command: Iterate and print each TransactionResult
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. 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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
examples/simple/dojo_sepolia.toml (1)
10-10: Avoid hard-coding a user-specific keystore pathCommitting
~/.snaccounts/dev-sepolia.keymay break other environments. Consider reading the path from an env-var, e.g.:-keystore_path = "~/.snaccounts/dev-sepolia.key" +keystore_path = "${SNACCOUNT_KEYSTORE:-~/.snaccounts/dev-sepolia.key}"Keeps defaults while remaining portable.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
bin/sozo/src/commands/auth.rs(3 hunks)bin/sozo/src/commands/execute.rs(1 hunks)bin/sozo/src/commands/options/transaction.rs(4 hunks)crates/dojo/utils/src/tx/invoker.rs(1 hunks)crates/dojo/utils/src/tx/mod.rs(1 hunks)crates/sozo/ops/src/migrate/mod.rs(1 hunks)examples/simple/dojo_sepolia.toml(1 hunks)examples/simple/manifest_sepolia.json(4 hunks)
🧰 Additional context used
🧠 Learnings (6)
bin/sozo/src/commands/auth.rs (2)
Learnt from: glihm
PR: dojoengine/dojo#2650
File: bin/sozo/src/commands/migrate.rs:64-65
Timestamp: 2024-11-07T13:57:57.616Z
Learning: In the codebase, the use of `into()` and `try_into()` for converting `TransactionOptions` is intentional. Conversions that may fail use `try_into()`, while those that cannot fail use `into()`. This approach is appropriate and should be preserved.
Examples:
- `try_into()` is used in `bin/sozo/src/commands/migrate.rs` and `bin/sozo/src/commands/execute.rs` where conversions can fail.
- `into()` is used in `bin/sozo/src/commands/register.rs` and `bin/sozo/src/commands/auth.rs` where conversions are infallible.
Learnt from: glihm
PR: dojoengine/dojo#2650
File: crates/dojo/utils/src/tx/mod.rs:201-272
Timestamp: 2024-11-07T14:43:23.530Z
Learning: In `crates/dojo/utils/src/tx/mod.rs`, fee token configurations are verified when the `fee_config` is built, making additional validations in the `send_with_cfg` methods redundant.
bin/sozo/src/commands/execute.rs (1)
Learnt from: glihm
PR: dojoengine/dojo#2650
File: bin/sozo/src/commands/migrate.rs:64-65
Timestamp: 2024-11-07T13:57:57.616Z
Learning: In the codebase, the use of `into()` and `try_into()` for converting `TransactionOptions` is intentional. Conversions that may fail use `try_into()`, while those that cannot fail use `into()`. This approach is appropriate and should be preserved.
Examples:
- `try_into()` is used in `bin/sozo/src/commands/migrate.rs` and `bin/sozo/src/commands/execute.rs` where conversions can fail.
- `into()` is used in `bin/sozo/src/commands/register.rs` and `bin/sozo/src/commands/auth.rs` where conversions are infallible.
crates/dojo/utils/src/tx/mod.rs (2)
Learnt from: glihm
PR: dojoengine/dojo#2650
File: crates/dojo/utils/src/tx/mod.rs:201-272
Timestamp: 2024-11-07T14:43:23.530Z
Learning: In `crates/dojo/utils/src/tx/mod.rs`, fee token configurations are verified when the `fee_config` is built, making additional validations in the `send_with_cfg` methods redundant.
Learnt from: glihm
PR: dojoengine/dojo#2650
File: crates/dojo/utils/src/tx/mod.rs:0-0
Timestamp: 2024-11-07T13:57:19.152Z
Learning: In the `send_with_cfg` method for `ExecutionV1` in `crates/dojo/utils/src/tx/mod.rs`, `FeeConfig::Strk` parameters are intentionally ignored for V1 execution, and this is expected behavior.
crates/sozo/ops/src/migrate/mod.rs (2)
Learnt from: glihm
PR: dojoengine/dojo#2650
File: bin/sozo/src/commands/migrate.rs:64-65
Timestamp: 2024-11-07T13:57:57.616Z
Learning: In the codebase, the use of `into()` and `try_into()` for converting `TransactionOptions` is intentional. Conversions that may fail use `try_into()`, while those that cannot fail use `into()`. This approach is appropriate and should be preserved.
Examples:
- `try_into()` is used in `bin/sozo/src/commands/migrate.rs` and `bin/sozo/src/commands/execute.rs` where conversions can fail.
- `into()` is used in `bin/sozo/src/commands/register.rs` and `bin/sozo/src/commands/auth.rs` where conversions are infallible.
Learnt from: glihm
PR: dojoengine/dojo#2650
File: crates/dojo/utils/src/tx/mod.rs:201-272
Timestamp: 2024-11-07T14:43:23.530Z
Learning: In `crates/dojo/utils/src/tx/mod.rs`, fee token configurations are verified when the `fee_config` is built, making additional validations in the `send_with_cfg` methods redundant.
crates/dojo/utils/src/tx/invoker.rs (1)
Learnt from: glihm
PR: dojoengine/dojo#2650
File: crates/dojo/utils/src/tx/mod.rs:201-272
Timestamp: 2024-11-07T14:43:23.530Z
Learning: In `crates/dojo/utils/src/tx/mod.rs`, fee token configurations are verified when the `fee_config` is built, making additional validations in the `send_with_cfg` methods redundant.
bin/sozo/src/commands/options/transaction.rs (3)
Learnt from: glihm
PR: dojoengine/dojo#2650
File: bin/sozo/src/commands/migrate.rs:64-65
Timestamp: 2024-11-07T13:57:57.616Z
Learning: In the codebase, the use of `into()` and `try_into()` for converting `TransactionOptions` is intentional. Conversions that may fail use `try_into()`, while those that cannot fail use `into()`. This approach is appropriate and should be preserved.
Examples:
- `try_into()` is used in `bin/sozo/src/commands/migrate.rs` and `bin/sozo/src/commands/execute.rs` where conversions can fail.
- `into()` is used in `bin/sozo/src/commands/register.rs` and `bin/sozo/src/commands/auth.rs` where conversions are infallible.
Learnt from: glihm
PR: dojoengine/dojo#2650
File: crates/dojo/utils/src/tx/mod.rs:201-272
Timestamp: 2024-11-07T14:43:23.530Z
Learning: In `crates/dojo/utils/src/tx/mod.rs`, fee token configurations are verified when the `fee_config` is built, making additional validations in the `send_with_cfg` methods redundant.
Learnt from: glihm
PR: dojoengine/dojo#2650
File: crates/dojo/utils/src/tx/mod.rs:0-0
Timestamp: 2024-11-07T13:57:19.152Z
Learning: In the `send_with_cfg` method for `ExecutionV1` in `crates/dojo/utils/src/tx/mod.rs`, `FeeConfig::Strk` parameters are intentionally ignored for V1 execution, and this is expected behavior.
🔇 Additional comments (20)
bin/sozo/src/commands/auth.rs (3)
377-380: LGTM! Consistent multicall result handling.Ohayo sensei! The update correctly handles the new
Vec<TransactionResult>return type from multicall by iterating over each result for printing. This approach aligns with the multicall splitting functionality.
597-600: LGTM! Consistent pattern maintained.The same clean pattern is used here for handling multiple transaction results from the owners permission updates.
641-644: LGTM! Pattern consistency confirmed.All three multicall usage points in this file follow the same correct pattern for handling multiple transaction results.
bin/sozo/src/commands/execute.rs (2)
169-172: LGTM! Proper multicall result handling.Ohayo sensei! The execute command correctly handles the new multicall behavior by iterating over multiple transaction results for display.
176-178: LGTM! Walnut debugger integration updated correctly.The debugger integration properly handles multiple transaction results by iterating through each one individually, maintaining the debugging capability for split transactions.
crates/dojo/utils/src/tx/mod.rs (2)
43-46: LGTM! Well-documented configuration addition.Ohayo sensei! The
max_callsfield is properly added with clear documentation explaining its purpose for preventing migration issues with too many resources. The optional type maintains backward compatibility.
51-57: LGTM! Consistent TxnAction update.The TxnAction::Send variant correctly includes the new max_calls parameter, maintaining consistency with the TxnConfig structure.
crates/sozo/ops/src/migrate/mod.rs (1)
572-591: LGTM! Proper handling of multicall splitting with thoughtful comments.Ohayo sensei! The implementation correctly adapts to handle multiple transaction results from the split multicall functionality. Taking the block number from the first transaction is a reasonable approach since the transactions should be in the same or consecutive blocks. The TODO comment shows good awareness of potential edge cases with transaction receipts.
crates/dojo/utils/src/tx/invoker.rs (3)
78-87: LGTM! Clear method documentation and proper empty case handling.Ohayo sensei! The updated method signature and documentation clearly explain the new multicall splitting behavior. The empty calls case correctly returns a vector with a single Noop result, maintaining consistency with the new return type.
91-131: LGTM! Robust chunking implementation with excellent tracing.The chunking logic is well-implemented with proper error handling, comprehensive tracing for debugging, and maintains the existing wait/receipt behavior for each chunk. The approach ensures transaction reliability while providing flexibility for large multicalls.
132-155: LGTM! Backward compatibility maintained with proper fallback.The fallback path for when
max_callsis not set preserves the original multicall behavior while adapting to the new return type. This ensures existing code continues to work seamlessly.bin/sozo/src/commands/options/transaction.rs (4)
39-43: Ohayo sensei! LGTM on the new max_calls field!The field declaration follows the established patterns perfectly - proper Option type, clear documentation, and consistent clap attributes. The help text clearly explains the default behavior when not specified.
63-63: Proper integration into TxnAction::Send, sensei!The max_calls field is correctly passed only to the Send variant, which makes logical sense since call limiting is only relevant when actually executing transactions, not during estimation or simulation.
78-78: Clean TryFrom implementation, sensei!The max_calls field is properly included in the TxnConfig conversion, following the same direct mapping pattern as other fields.
97-97: Solid test coverage for the new field, sensei!The test properly verifies that max_calls values are correctly preserved through the TransactionOptions to TxnConfig conversion, providing good confidence in the implementation.
Also applies to: 105-105
examples/simple/manifest_sepolia.json (4)
4-5: Ohayo sensei! Verify new world address & seed are propagatedThe updated
addressandseedlook fine, but please double-check that:
examples/simple/dojo_sepolia.tomland any CI scripts use the same values.- Down-stream tooling (deployment, tests) isn’t still pointing at the old world contract.
A quick
rgsearch in the repo can confirm no stale references remain.
1492-1493: Contract address change recorded – consistency check recommendedNice catch on the new
ns-c1address. Make sure any hard-coded allow-lists, docs, or front-end configs reference0x6a2a…32dnow.
1750-1751: Updated ns-c2 address – verify against class-hashThe address now ends with
…060. Confirm it actually hosts the class-hash0x1e48…02d; a mismatched deploy will brick migrations.
1926-1927: Duplicate class-hash / new address combo – just an FYI
ns2-c1keeps the same class-hash asns-c1, only the address moved. All good on my end.examples/simple/dojo_sepolia.toml (1)
4-4: Seed aligned with manifest – looks good
simple-amatches the manifest; no issues spotted.
1 participant
This PR adds a split to multicalls to ensure the user can control the maximum number of calls in a multi call.
Due to reduction of the network resources for a single transaction, when too much models are registered in the same transaction, this latter can fail.
By adding a
max_callsparameter, the user can adjust the number of calls in a multi call to avoid such issue.By default, there is no limit if the
max_callsis not supplied, and Sozo will attempt to send only one transaction per migration step.Summary by CodeRabbit
New Features
Bug Fixes
Documentation