Conversation
d390a56 to
eccb418
Compare
kariy
changed the title
* move out * fmt
Optimized workflow by increasing runner cores and using the latest Ubuntu version for release and docker build jobs.
* fix(Dockerfile): move dependencies in base image Moved installation of curl, ca-certificates, and tini to the base image stage. Cleaned up apt cache to reduce image size. * fix(Dockerfile): adjust tini installation and entrypoint path Ensure tini is copied to a new path and update the entrypoint accordingly to prevent runtime issues.
Prepare release: v Co-authored-by: steebchen <steebchen@users.noreply.github.com>
Update devcontainer image: v1.4.2 Co-authored-by: steebchen <steebchen@users.noreply.github.com>
#3164) * fix(torii-indexer): stack overflow when dealing with a high number of events pages * get rid of recursive func * chore
* feat(katana): add fact registry arg for `init` This PR adds a new optional argument `--settlement-facts-registry-contract` to `katana init` which allows passing a custom fact registry contract. Closes #3034 * fix: remove fact registry setter * feat: error in custom init w/ no fact registry * chore: refactor according to ai reviews * use setter function * fix test * chore: remove unneccessary changes --------- Co-authored-by: Ammar Arif <evergreenkary@gmail.com>
* feat: handle long values (int64) * fix: array escape error * feat: add ControllerConnect
* remove katana and only keep katana-runner dependency * chore: bump scarb * chore: bump scarb + remove unuse deps * chore: use katana runner from git repo * update katana runner dep * remove test sequencer * remove rpc test utils * fix controller account * handle not controller feature * update * rebuild test db * refactor(torii): remove torii in favor of dedicated repo * fix(ci): download Katana for integration test * fix(ci): rename ci to test * fix(ci): fix yml file dependencies * fix(ci): run clippy before build * fix: remove types-test from built projects * fix(ci): bump scarb version * fix(docs): ensure docs can run without torii/katana * fix(ci): update dojo container * fix(ci): use ubuntu24 devcontainer --------- Co-authored-by: Ammar Arif <evergreenkary@gmail.com>
WalkthroughOhayo sensei! This update refactors fee handling across the codebase, unifying transaction fee configuration and removing multi-token support. It upgrades various dependencies, updates provider abstractions, and migrates contract execution flows to use ExecutionV3. Additional refactoring simplifies account and provider management, error handling, and server startup logic. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant Account
participant Provider
participant Contract
User->>CLI: Initiate transaction (deploy/invoke/declare)
CLI->>Account: Build transaction with unified FeeConfig
Account->>Provider: Sign and send transaction (always ExecutionV3)
Provider->>Contract: Execute transaction
Contract-->>Provider: Return result or error
Provider-->>Account: Pass result/error
Account-->>CLI: Return outcome
CLI-->>User: Display result (with improved error formatting)
sequenceDiagram
participant MetricsServer
participant TcpListener
participant Connection
participant Exporter
MetricsServer->>TcpListener: Bind and listen on address
loop For each connection
TcpListener->>MetricsServer: Accept connection
MetricsServer->>Connection: Spawn task for connection
Connection->>Exporter: Export metrics (on request)
Exporter-->>Connection: Return metrics data
Connection-->>MetricsServer: Serve response
end
📜 Recent review detailsConfiguration used: .coderabbit.yaml ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
✨ Finishing Touches🧪 Generate Unit Tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![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
🔭 Outside diff range comments (2)
bin/sozo/src/commands/call.rs (1)
145-169: 🛠️ Refactor suggestionOhayo sensei — recursive error formatter risks stack explosion on deeply-nested traces
format_execution_errorrecurses without depth-limit. A malicious contract could craft an intentionally deep nesting to overflow the stack.-fn format_execution_error(error: &starknet::core::types::ContractExecutionError) -> String { +const MAX_NESTING: usize = 32; + +fn format_execution_error( + error: &starknet::core::types::ContractExecutionError, +) -> String { + format_execution_error_inner(error, 0) +} + +fn format_execution_error_inner( + error: &starknet::core::types::ContractExecutionError, + depth: usize, +) -> String { + if depth >= MAX_NESTING { + return "<truncated too deep>".into(); + } + match error { - starknet::core::types::ContractExecutionError::Message(msg) => msg.clone(), + starknet::core::types::ContractExecutionError::Message(msg) => msg.clone(), starknet::core::types::ContractExecutionError::Nested(inner) => { let address = format!("0x{:x}", inner.contract_address); let selector = format!("0x{:x}", inner.selector); - let inner_error = format_execution_error(&inner.error); + let inner_error = format_execution_error_inner(&inner.error, depth + 1); format!("Error in contract at {address} when calling {selector}:\n {inner_error}",) } } }bin/sozo/src/commands/options/transaction.rs (1)
16-16:⚠️ Potential issueFix conflicts_with_all usage
The
conflicts_with_allattribute references non-existent options (max_fee_raw,fee_estimate_multiplier) that were removed. This will cause validation issues.- #[arg(conflicts_with_all = ["max_fee_raw", "fee_estimate_multiplier"])]
🧹 Nitpick comments (5)
crates/metrics/src/server.rs (1)
78-114: Ohayo sensei — lack of graceful shutdown & back-pressureThe endless
loop { listener.accept()... }spawns an unbounded number of tasks. A DoS with many TCP SYNs will exhaust resources.Recommend:
- Use
tokio_util::sync::CancellationTokenortokio::select!with a shutdown signal.- Gate task spawning behind a
Semaphoreto cap concurrent connections.bin/sozo/src/commands/options/account/provider.rs (1)
318-351: Ohayo sensei — duplicated boilerplate suggests a macroBoth new methods replicate the left/right match pattern. Consider a small macro (
forward!) orenum_dispatchto cut 400+ LOC, reducing maintenance burden when Starknet adds more RPCs.bin/sozo/src/commands/options/transaction.rs (1)
8-18: Breaking change: Legacy fee options removed, sensei!The removal of
max_fee_rawandfee_estimate_multiplieroptions represents a breaking change for users who relied on these ETH fee configurations. This aligns with the migration to V3 transactions, but ensure users are properly notified.Consider adding a migration guide or deprecation notice in the documentation to help users transition from the old fee options to the new gas/gas_price model.
crates/dojo/utils/src/tx/mod.rs (1)
102-169: Consider adding validation for fee configurationWhile the current implementation is clean, consider adding validation to prevent potential issues like setting gas_price without gas.
+impl FeeConfig { + pub fn validate(&self) -> Result<(), &'static str> { + if self.gas_price.is_some() && self.gas.is_none() { + return Err("gas_price cannot be set without gas"); + } + Ok(()) + } +}bin/sozo/src/commands/options/account/type.rs (1)
60-70: Consider extracting the provider wrapping logicThe conditional compilation logic for wrapping the provider could be extracted into a helper method to improve readability and reduce duplication with potential future constructors.
+ #[inline] + fn wrap_standard_provider(provider: Arc<P>) -> RpcProvider<P> { + #[cfg(feature = "controller")] + return EitherProvider::Left(provider); + #[cfg(not(feature = "controller"))] + provider + } + pub fn new_standard( provider: Arc<P>, account: SingleOwnerAccount<Arc<P>, LocalWallet>, ) -> Self { let account = SozoAccountKind::Standard(account); - #[cfg(feature = "controller")] - let provider = EitherProvider::Left(provider); - #[cfg(not(feature = "controller"))] - let provider = provider; + let provider = Self::wrap_standard_provider(provider); Self { account, provider } }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockspawn-and-move-db.tar.gzis excluded by!**/*.gz
📒 Files selected for processing (27)
Cargo.toml(6 hunks)bin/sozo/Cargo.toml(1 hunks)bin/sozo/src/commands/call.rs(2 hunks)bin/sozo/src/commands/hash.rs(1 hunks)bin/sozo/src/commands/options/account/controller.rs(8 hunks)bin/sozo/src/commands/options/account/mod.rs(7 hunks)bin/sozo/src/commands/options/account/provider.rs(3 hunks)bin/sozo/src/commands/options/account/type.rs(6 hunks)bin/sozo/src/commands/options/transaction.rs(4 hunks)crates/dojo/bindgen/src/plugins/typescript/generator/mod.rs(3 hunks)crates/dojo/utils/Cargo.toml(1 hunks)crates/dojo/utils/src/tx/declarer.rs(2 hunks)crates/dojo/utils/src/tx/deployer.rs(2 hunks)crates/dojo/utils/src/tx/error.rs(3 hunks)crates/dojo/utils/src/tx/invoker.rs(2 hunks)crates/dojo/utils/src/tx/mod.rs(6 hunks)crates/dojo/utils/src/tx/waiter.rs(3 hunks)crates/dojo/world/abigen/src/main.rs(2 hunks)crates/dojo/world/src/contracts/abigen/world.rs(43 hunks)crates/dojo/world/src/contracts/model.rs(1 hunks)crates/metrics/Cargo.toml(1 hunks)crates/metrics/src/server.rs(2 hunks)crates/sozo/ops/Cargo.toml(2 hunks)crates/sozo/ops/src/migrate/mod.rs(1 hunks)crates/sozo/walnut/src/verification.rs(1 hunks)scripts/clippy.sh(1 hunks)xtask/generate-test-db/Cargo.toml(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
crates/dojo/utils/src/tx/declarer.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.
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 (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.
crates/dojo/utils/src/tx/mod.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.
🔇 Additional comments (45)
scripts/clippy.sh (1)
11-11:Details
✅ Verification successful
Ohayo sensei! Verify the nightly toolchain version availability.
The toolchain update from
2024-08-28to2025-02-20looks good and aligns with the broader dependency modernization efforts in this PR.Please verify that this nightly version is available and stable:
🌐 Web query:
Is Rust nightly-2025-02-20 available and what are its known issues?💡 Result:
Based on the search results, I can provide information about the Rust nightly build from February 20, 2025.
Availability of Rust nightly-2025-02-20
Rust nightly-2025-02-20 is available and significant because it corresponds to the stable release of Rust 1.85.0, which was officially released on February 20, 2025[1]. This release is particularly notable as it introduced the Rust 2024 Edition[1][4].
If you want to install this specific nightly version, you can do so using rustup with the following command:
rustup toolchain install nightly-2025-02-20Features and Changes
The nightly-2025-02-20 build includes all features that were stabilized in Rust 1.85.0, including:
- The 2024 Edition, which is now stable[1][6]
- Async closures (as per RFC 3668)[6]
- Stabilization of
#[diagnostic::do_not_recommend][6]- Addition of the
unpredictable_function_pointer_comparisonslint[6]- A new lint for combining
#[no_mangle]and#[export_name]attributes[6]The 2024 Edition is particularly significant as it's described as "the largest edition since Rust 2015"[5]. It includes several improvements that make the language more consistent and ergonomic, with a focus on safety. Some notable features enabled by this edition include:
genblocksletchains- The never (
!) type[5]Known Issues
While the search results don't explicitly mention known issues specific to the nightly-2025-02-20 build, there are some general points to be aware of regarding nightly builds:
Nightly builds can sometimes be unavailable or have installation failures, as mentioned in issue #3390 in the rustup repository[7]. This can lead to errors like "update not yet available, sorry try again later" or "toolchain is not installable."
The nightly version might contain experimental features that are still under development. Since this specific nightly corresponds to a stable release (1.85.0), it should be more stable than typical nightly builds.
The memory usage of rustc has been improved throughout the release cycle leading to this version[2].
Some behavior changes related to path handling and documentation were implemented, such as changes to how
rustchandles paths whenrust-srcis present[2].If you're using this nightly version for production work, it would be more reliable to use the stable 1.85.0 release instead, which contains the same features but with more thorough testing.
Citations:
- 1: https://blog.rust-lang.org/2025/02/20/Rust-1.85.0.html
- 2: https://doc.rust-lang.org/beta/releases.html
- 3: https://internals.rust-lang.org/t/nightly-version-for-a-specific-rust-compiler-version/22434
- 4: rust-lang/rust-project-goals#117
- 5: https://blog.rust-lang.org/2024/11/27/Rust-2024-public-testing.html
- 6: https://www.suse.com/support/update/announcement/2025/suse-ru-20250773-1
- 7: rust-lang/rustup#3390
- 8: https://rust-lang.github.io/rust-project-goals/2024h2/Rust-2024-Edition.html
- 9: https://doc.rust-lang.org/nightly/cargo/CHANGELOG.html
- 10: https://rust-lang.github.io/rustup/concepts/channels.html
Nightly-2025-02-20 Availability Confirmed
Thenightly-2025-02-20toolchain is available (it aligns with Rust 1.85.0 and the new 2024 Edition) and installs cleanly viarustup toolchain install nightly-2025-02-20. No major issues have been reported for this build, so the bump in scripts/clippy.sh (line 11) is safe to merge.bin/sozo/src/commands/hash.rs (1)
64-64: Ohayo sensei! Nice idiomatic improvement.The replacement of
map_or(false, |c| c.is_alphabetic())withis_some_and(|c| c.is_alphabetic())is more expressive and idiomatic Rust. This makes the intent clearer while maintaining the exact same functionality.crates/sozo/walnut/src/verification.rs (1)
96-96: Ohayo sensei! Consistent idiomatic improvement.Great consistency with the similar change in
hash.rs! Usingis_some_and(|name| name.starts_with("dojo_"))instead ofmap_or(false, |name| name.starts_with("dojo_"))makes the code more readable and idiomatic.crates/sozo/ops/src/migrate/mod.rs (1)
224-224: Ohayo sensei! Clean refactor with preserved logic.The refactor to use
is_none_or(|m| !m.disable_multicall.unwrap_or(false))is more concise and idiomatic while preserving the original behavior of enabling multicall by default unless explicitly disabled.crates/dojo/world/src/contracts/model.rs (1)
165-165: Ohayo sensei! Excellent lifetime simplification.The change from
impl<'a, P> ModelReader<ModelError> for ModelRPCReader<'a, P>toimpl<P> ModelReader<ModelError> for ModelRPCReader<'_, P>is a clean simplification using anonymous lifetimes. This makes the code more readable while maintaining identical functionality.crates/dojo/bindgen/src/plugins/typescript/generator/mod.rs (2)
139-139: Ohayo sensei! Nice cleanup removing the redundant return statement.The code is more idiomatic now by relying on implicit returns.
220-220: LGTM! Another good cleanup removing explicit return.Consistent with Rust idioms and improves readability.
crates/dojo/utils/src/tx/waiter.rs (2)
186-186: Ohayo sensei! Good simplification using anonymous lifetime.Using
'_instead of explicit lifetime parameter'ais more idiomatic when the lifetime isn't referenced in the impl block.
312-323: Nice cleanup of test dependencies and constants.The simplified
EXECUTION_RESOURCESconstant and cleaned up imports reduce test verbosity while maintaining functionality.xtask/generate-test-db/Cargo.toml (1)
13-13: Ohayo sensei! Good move switching to workspace dependency.Using
workspace = trueforkatana-runnerensures consistent dependency management across the workspace.bin/sozo/Cargo.toml (1)
51-51: Ohayo sensei! Consistent workspace dependency management.Good to see
katana-runnermoved to workspace dependency, maintaining consistency across the codebase.crates/dojo/world/abigen/src/main.rs (1)
18-18: Ohayo sensei! Good addition of explicit execution version configuration.Adding
ExecutionVersion::V3to theAbigensetup aligns with the broader codebase migration to ExecutionV3, ensuring consistency across contract execution methods.Also applies to: 69-69
crates/dojo/utils/src/tx/declarer.rs (2)
19-19: Ohayo sensei! LGTM on the import cleanup.The removal of
FeeConfigimport aligns perfectly with the fee handling unification across the codebase.
104-107: Excellent simplification, sensei!The migration to unconditional
declare_v3usage removes the complexity of fee configuration variant handling while maintaining all necessary functionality. This aligns with the broader refactoring effort to unify transaction execution paths.crates/sozo/ops/Cargo.toml (2)
25-25: Ohayo sensei! Nice dependency ordering cleanup.The reordering of
starknet-cryptoafterstarknetimproves readability.
35-35: Great workspace unification, sensei!Switching
katana-runnerfrom a Git reference to workspace dependency ensures consistent versioning across the project and simplifies dependency management.crates/metrics/Cargo.toml (1)
9-14: Ohayo sensei! Solid dependency updates for the server refactoring.The addition of
bytes,http-body-util, andhyper-utildependencies, along with the hyper feature changes (removingtcp) and addingnetto tokio, properly supports the migration from hyper's built-in server to manual TCP listener implementation. This architectural change improves control over connection handling.crates/dojo/utils/src/tx/deployer.rs (2)
12-12: Ohayo sensei! Clean import removal.The
FeeConfigimport removal is consistent with the fee handling unification across the transaction utilities.
81-82: Perfect execution path simplification, sensei!The migration to unconditional
execute_v3usage eliminates the fee configuration variant branching logic while preserving all deployment functionality. This change aligns beautifully with the broader fee handling refactoring effort.crates/dojo/utils/Cargo.toml (3)
10-10: Ohayo sensei! Good workspace dependency additions.Adding
colored_jsonandserde_jsonas workspace dependencies improves version consistency across the project.Also applies to: 14-14
17-17: Nice feature ordering cleanup, sensei!The tokio feature reordering maintains the same functionality while improving readability.
23-23: Excellent workspace unification, sensei!Converting
katana-runnerto a workspace dependency ensures consistent versioning and aligns with similar changes across other workspace crates.crates/dojo/utils/src/tx/invoker.rs (1)
81-83: Ohayo sensei — multicall path likewise simplifiedSame remark as above; LGTM once the search shows all callers migrated.
Cargo.toml (1)
197-198:Details
✅ Verification successful
Ohayo sensei — remember to update feature flags when bumping
hyperto 1.x
hyper = "1.5"removes the old default features (tcp, etc.). Ensure all crates that depended onhyper/tcpnow depend onhyper-util+tokio(as you already did inmetrics). Scan workspace forfeatures = ["tcp"].
🏁 Script executed:
#!/bin/bash rg -n $'"hyper".*tcp' --glob '*.toml'Length of output: 38
Ohayo sensei — let’s scan all Cargo.toml files for any
features = ["tcp"]to be sure we didn’t miss a flag:
🏁 Script executed:
#!/bin/bash rg -n 'features *= *\[.*"tcp".*\]' --glob '*.toml'Length of output: 52
Ohayo sensei — no TCP feature flags found in the workspace
Ran a workspace-wide search forfeatures = ["tcp"]in all Cargo.toml files and found zero matches. It looks like no other crates rely on the oldtcpfeature, so bumping tohyper = "1.5"is safe and no further changes are needed.bin/sozo/src/commands/options/account/provider.rs (1)
106-115: Ohayo sensei — 👍 newget_messages_statusshimBrings the enum up-to-date with the provider trait. Looks good.
crates/dojo/utils/src/tx/error.rs (2)
18-19: Ohayo! Good refactor to use specific error type instead of StringThe change from generic
StringtoTransactionExecutionErrorDataimproves type safety and provides more structured error information, sensei.
49-58:Details
✅ Verification successful
Verify ownership semantics change for error conversions
The pattern matching was changed from borrowing (
&value) to moving (value). While this is functionally correct sinceProviderErrorimplementsClone, ensure this ownership change doesn't impact any code that might need to reuse the original error value.
🏁 Script executed:
#!/bin/bash # Search for other conversions or usages of ProviderError that might be affected rg -A 5 "From<ProviderError>" --type rustLength of output: 399
Ohayo sensei,
No action needed: safe to move
ProviderErrorin conversion
I verified that the onlyFrom<ProviderError>implementation lives incrates/dojo/utils/src/tx/error.rs, so consuming the error here won’t impact any other code paths. Moving the value is idiomatic and doesn’t break downstream error handling.bin/sozo/src/commands/options/transaction.rs (1)
77-104: Ohayo! Clean test update reflecting the unified fee modelThe test correctly validates the new simplified fee configuration structure.
bin/sozo/src/commands/options/account/controller.rs (3)
17-18: Nice simplification of the controller account type, sensei!Removing the generic parameters and Arc wrapper makes the type much cleaner and easier to work with.
95-101:Details
❓ Verification inconclusive
Ohayo! Verify the policy construction logic
The explicit
CallPolicyconstruction withauthorized: Some(true)is cleaner, but ensure this matches the expected policy format for the session system.
🏁 Script executed:
#!/bin/bash # Search for other CallPolicy usages to ensure consistency rg "CallPolicy" --type rust -A 3Length of output: 900
Ohayo Sensei! Confirm CallPolicy Authorization
I didn’t find any otherCallPolicyusages to compare against—please double-check that settingauthorized: Some(true)here matches your session system’s expected policy format.• File:
bin/sozo/src/commands/options/account/controller.rs(lines 95–101)
• Snippet:.map(|p| { Policy::Call(CallPolicy { authorized: Some(true), contract_address: p.target, selector: get_selector_from_name(&p.method).expect("valid selector"), }) })
55-55:Details
❓ Verification inconclusive
Verify the nested session field access pattern
The change from direct field access to
session.session.inner.expires_atandsession.session.inner.allowed_policies_rootsuggests a structural change in the session data. This nested access pattern seems unusual.Also applies to: 117-117
🏁 Script executed:
#!/bin/bash # Check the FullSessionInfo structure to understand the nested fields ast-grep --pattern 'struct FullSessionInfo { $$$ }'Length of output: 55
Ohayo sensei! The AST grep didn’t catch the struct—let’s locate the definitions with ripgrep to confirm the nested
innerfield:
🏁 Script executed:
#!/bin/bash # Search for FullSessionInfo struct definition rg --color=never -n "FullSessionInfo" . # Search for Session struct definition to see its fields rg --color=never -n "struct Session" .Length of output: 342
Ohayo sensei!
Verify nested session field access pattern
I wasn’t able to find
FullSessionInfoorSessiondefinitions locally—they’re pulled in from the externalslotcrate. Please confirm that:
slot::session::FullSessionInfostill exposes aninnerfield withexpires_atandallowed_policies_root.- No recent updates flattened or renamed those fields in the dependency.
If this nesting is intentional and stable, consider adding a helper method or documenting the access path to keep the controller code clear.
crates/dojo/utils/src/tx/mod.rs (2)
24-30: Elegant unification of fee configuration, sensei!The simplified
FeeConfigstruct with optional gas and gas_price fields is much cleaner than the previous multi-token approach. This aligns perfectly with the V3 transaction model.
113-119: Consistent fee configuration across all transaction typesThe implementation correctly uses
l1_gasandl1_gas_pricemethods for V3 transactions across ExecutionV3, DeclarationV3, and AccountDeploymentV3. The optional chaining pattern is clean and consistent.Also applies to: 136-142, 159-165
bin/sozo/src/commands/options/account/mod.rs (3)
3-3: Ohayo! Good use of Arc for thread-safe provider sharingWrapping providers in
Arcis the right approach for concurrent access, sensei. This ensures safe sharing across async operations.Also applies to: 92-94, 113-115
240-241: Test updates correctly reflect Arc usageThe test modifications properly use
Arc::newto wrap the provider, maintaining consistency with the new API.Also applies to: 258-258
84-86:Details
✅ Verification successful
Verify the double clone pattern for CartridgeJsonRpcProvider
Creating the provider and immediately cloning it seems unusual. Consider if both the original and clone are necessary.
🏁 Script executed:
#!/bin/bash # Check if CartridgeJsonRpcProvider requires this pattern ast-grep --pattern 'impl $_ for CartridgeJsonRpcProvider { $$$ }'Length of output: 69
ohayo sensei! Let’s confirm how
CartridgeJsonRpcProvideris defined and whether it derives or implementsClone. Please run:
🏁 Script executed:
#!/bin/bash # Locate the struct definition and Clone derivation/impl rg -n "struct CartridgeJsonRpcProvider" rg -n "derive.*Clone" rg -n "impl Clone for CartridgeJsonRpcProvider" rg -n "CartridgeJsonRpcProvider::new"Length of output: 11658
ohayo sensei! To be sure the double‐clone is intentional, let’s inspect the
CartridgeJsonRpcProviderdefinition and itsClonederivation inprovider.rs:
🏁 Script executed:
#!/bin/bash # Show the struct and derives for CartridgeJsonRpcProvider sed -n '1,200p' bin/sozo/src/commands/options/account/provider.rsLength of output: 6302
ohayo sensei! Let’s find where
CartridgeJsonRpcProvideris defined:
🏁 Script executed:
#!/bin/bash rg -n "CartridgeJsonRpcProvider"Length of output: 898
No changes needed: the clone is intentional
The single
.clone()call oncartridge_provideris required because:
CartridgeJsonRpcProvider::new(url.clone())clones the URL for the provider, preserving the original for the next call..clone()on the provider moves a fresh copy intoself.controller, while the remaining instance is stored inSozoAccount.Everything looks correct—no further edits are necessary.
bin/sozo/src/commands/options/account/type.rs (4)
39-54: Ohayo! Clean architecture with the enum wrapper pattern, sensei! 👍The refactoring from a single type to an enum-based approach with a unified struct wrapper is well-designed. The separation between
SozoAccountKind(for the account variants) andSozoAccount(for the unified interface) provides good flexibility.
20-22: Smart conditional type aliasing for provider abstraction!The use of type aliases to conditionally define
RpcProvider<P>based on the "controller" feature flag is elegant. This abstracts away the complexity from the rest of the code.
93-98: Consistent delegation pattern across all trait methods!The match expressions consistently delegate to the underlying account implementations. This pattern ensures that all account types maintain their specific behaviors while presenting a unified interface.
Also applies to: 101-106, 109-114, 157-162, 177-182
121-131: Clean error propagation in signing methods, sensei!The error handling correctly maps both
single_owner::SignErrorandslot::account_sdk::signers::SignErrorto the unifiedSozoAccountSignErrortype through the?operator. Nice use of the error conversion traits!Also applies to: 138-148
crates/dojo/world/src/contracts/abigen/world.rs (5)
3577-3588: Ohayo sensei! LGTM - Clean ExecutionV3 migration!The systematic update from
ExecutionV1<A>toExecutionV3<A>return types and correspondingexecute_v3method calls looks spot on. This pattern is consistently applied throughout all the execution methods.
3616-3627: Consistent migration pattern continues!Another clean update following the same V1→V3 migration pattern. The return type and method call changes are perfectly aligned.
3655-3666: Systematic execution upgrade looking good sensei!The migration continues to follow the established pattern correctly - both return type and execution method are updated in sync.
3694-3705: V3 migration maintains consistency!Another properly updated execution method following the same systematic approach.
3730-3740: Excellent systematic migration across all execution methods!Ohayo sensei! All remaining execution methods have been consistently updated from ExecutionV1 to ExecutionV3. The auto-generated bindings maintain perfect consistency with:
- Return type changes:
ExecutionV1<A>→ExecutionV3<A>- Method calls:
execute_v1→execute_v3- Read-only methods appropriately left unchanged
This clean migration aligns perfectly with the broader RPC spec upgrade to 0.8 and demonstrates proper handling of the execution interface changes.
Also applies to: 3765-3775, 3800-3810, 3838-3849, 3874-3884, 3915-3927, 3952-3962, 3984-3993, 4018-4028, 4053-4063, 4094-4106, 4137-4149, 4165-4174, 4196-4205, 4230-4240, 4265-4275, 4300-4310, 4325-4333
| let tx = self.account.execute_v3(vec![call]).send_with_cfg(&self.txn_config).await?; | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ohayo sensei — single-call path now hard-wired to execute_v3
Switching unconditionally to execute_v3 is consistent with the fee-config refactor and keeps the codebase lean—nice.
Just be sure every downstream crate is already on starknet >= 0.14 where execute_v3 exists; otherwise a version-mismatch will break builds.
Run:
Expect no hits.
🏁 Script executed:
#!/bin/bash
# Check for any remaining `execute_v1`/`execute_v2` usages
rg -n $'execute_v[12]\('Length of output: 153
Ohayo sensei — remaining execute_v1 calls found
Switching to execute_v3 is spot on, but we still have two lingering execute_v1 usages that will break on starknet < 0.14:
• crates/sozo/ops/src/execute.rs:28
• crates/sozo/ops/src/register.rs:73
Please migrate these to execute_v3 (or pin those crates appropriately) to keep builds green.
🤖 Prompt for AI Agents
In crates/dojo/utils/src/tx/invoker.rs around lines 57 to 58, the code correctly
uses execute_v3 for the single-call path, but there are still two other files
(crates/sozo/ops/src/execute.rs at line 28 and crates/sozo/ops/src/register.rs
at line 73) using execute_v1 which will cause build failures on starknet
versions below 0.14. To fix this, update those two files to replace execute_v1
calls with execute_v3 calls or ensure those crates are pinned to starknet
version 0.14 or higher to maintain compatibility and keep builds green.
| // Clone the hooks and exporter for each connection | ||
| let hooks = Arc::clone(&hooks); | ||
| let exporter = self.exporter.clone(); | ||
|
|
||
| // Spawn a new task to handle each connection | ||
| tokio::task::spawn(async move { | ||
| // Handle the connection using HTTP1 |
There was a problem hiding this comment.
Ohayo sensei — clone on unconstrained generic → compile-time failure
self.exporter.clone() requires MetricsExporter: Clone, but the type param isn’t bound. Compilation will fail unless every concrete exporter happens to implement Clone and the compiler can infer that.
Quick fix (no semantic change):
- pub async fn start(self, addr: SocketAddr) -> Result<(), Error> {
+ pub async fn start(self, addr: SocketAddr) -> Result<(), Error>
+ where
+ MetricsExporter: Clone,
+ {Long-term: consider wrapping the exporter in Arc, then clone the pointer instead of the whole exporter—safer if the exporter holds heavyweight state.
📝 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.
| // Clone the hooks and exporter for each connection | |
| let hooks = Arc::clone(&hooks); | |
| let exporter = self.exporter.clone(); | |
| // Spawn a new task to handle each connection | |
| tokio::task::spawn(async move { | |
| // Handle the connection using HTTP1 | |
| pub async fn start(self, addr: SocketAddr) -> Result<(), Error> | |
| where | |
| MetricsExporter: Clone, | |
| { | |
| // Clone the hooks and exporter for each connection | |
| let hooks = Arc::clone(&hooks); | |
| let exporter = self.exporter.clone(); | |
| // Spawn a new task to handle each connection | |
| tokio::task::spawn(async move { | |
| // Handle the connection using HTTP1 |
🤖 Prompt for AI Agents
In crates/metrics/src/server.rs around lines 91 to 97, the call to
self.exporter.clone() requires the generic type MetricsExporter to implement
Clone, but this trait bound is missing, causing compilation errors. To fix this,
add a Clone trait bound to the MetricsExporter generic parameter in the struct
or function definition where self.exporter is used. For a more robust solution,
consider wrapping exporter in an Arc to clone the pointer instead of the whole
exporter, which avoids requiring Clone on the exporter itself and is more
efficient for heavyweight state.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/test.yml (1)
71-75: Ensure Katana container readiness before testing.
Pulling & running the Katana Docker image is the right move, but integration tests may start before Katana is ready. Consider adding a health check or simple wait loop (e.g.,curl --retry-all --retry-delay 2 http://localhost:5050/health) to verify port 5050 is accepting connections before moving on. sensei
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/test.yml(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: fmt
🔇 Additional comments (2)
.github/workflows/test.yml (2)
8-14: Approve quoting style change.
Switching to double-quoted paths is purely stylistic and doesn’t affect functionality. Nice cleanup! ohayo sensei
18-24: Approve quoting style change in PR trigger.
Consistent with the push trigger paths; no functional impact. sensei
* Fix broken getting started link in README (#3235) * chore(versions): add torii 1.5.6 * Update DEVELOPMENT.md (#3236) * Update DEVELOPMENT.md with new instructions * Update scripts/rebuild_test_artifacts.sh with Rabbit suggestion Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Respond to review comments * Respond to review comments II * Respond to review comments III --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * feat: update rpc spec to 0.8 (#3179) * release(prepare): v1.6.0-alpha.0 (#3241) Prepare release: v1.6.0-alpha.0 Co-authored-by: glihm <glihm@users.noreply.github.com> * chore(versions): bump katana to 1.6.0-alpha.0 * fix(dojoup): remove new line in the source cmd (#3244) fix: fixed new line in the source cmd * chore: edited the build badge and its link (#3221) Co-authored-by: Ammar Arif <evergreenkary@gmail.com> * chore(devcontainer): update image: v1.6.0-alpha.0 (#3242) Update devcontainer image: v1.6.0-alpha.0 Co-authored-by: glihm <glihm@users.noreply.github.com> * chore: add katana 1.5.4 (#3245) * chore: add the missing backticks in the comments (#3243) Signed-off-by: one230six <723682061@qq.com> * fix(bindgen): use world namespace for imported models in TypeScript S… (#3249) * fix(bindgen): filter out Value models (#3248) * fix(bindgen): fix ts bytearray type mapping (#3247) * chore: update katana versions (#3253) add katana versions * sozo(unrealengine): handle UE5.6 and Dojo 1.5 (#3252) * fix(sozo): assert caller permission with match (#3254) * handle call_contract_syscall() result for better panic trace * avoid using 0 as caller address * remove temporarly the RPC version check. Currently, Katana uses the new RPC types, without bumping the spec version. To ensure we can still use sozo with sepolia/mainnet and Katana, Sozo will not check the RPC version for now * update test dbs --------- Co-authored-by: glihm <dev@glihm.net> * feat(sozo): create standalone bindgen command (#3246) * feat: create standalone bindgen command * cleanup unused inputs * remove dbg * add meaningful error if project is not built --------- Co-authored-by: glihm <dev@glihm.net> * feat(sozo): add MCP sever (#3256) * feat(sozo): add mvp for mcp server * refacto: restructure MCP server * evaluate changes using official rust sdk * feat(mcp): add stdio support * refacto with rmcp crate * wip * bump rmcp * add testing support * wip * refacto * cleanup * add instructions * add instructions * wip test * refacto tests * add debugging * ignore test with katana at the moment * fix typos and sozo path * refacto uri parsing * remove dbg * disable test that should be run with katana * fix test, windows fails because of new reqwest version * refactor(types): schema json sql value (#3257) * refactor(types): schema json sql value * remove excess comma * fix clippy --------- Co-authored-by: glihm <dev@glihm.net> * chore: bump cairo packages to 1.6.0-alpha.0 * fix(mcp): ensure test is using latest version * split scarb metadata ext in a different crate for dependencies * wip: controller issue * wip * wip: resolve conflicts and update deps * cleanup * lint * wip * wip tests * fix all tests * rebuild test db * fix: run linter * fix slot dep with github commit * fix clippy * fix inspect by adding missing json format * fix mcp formatting * fix stdio_test * use sozo from path --------- Signed-off-by: one230six <723682061@qq.com> Co-authored-by: Ritik <ritikverma0050@gmail.com> Co-authored-by: Daniel Kronovet <kronovet@gmail.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Ammar Arif <evergreenkary@gmail.com> Co-authored-by: Tarrence van As <tarrencev@users.noreply.github.com> Co-authored-by: glihm <glihm@users.noreply.github.com> Co-authored-by: Benjamin <158306087+bengineer42@users.noreply.github.com> Co-authored-by: braveocheretovych <braveocheretovych@gmail.com> Co-authored-by: one230six <163239332+one230six@users.noreply.github.com> Co-authored-by: Brother MartianGreed <valentin@pupucecorp.com> Co-authored-by: Corentin Cailleaud <corentin.cailleaud@caillef.com> Co-authored-by: Rémy Baranx <remy.baranx@gmail.com> Co-authored-by: Larko <59736843+Larkooo@users.noreply.github.com>
…3266) * feat: add external contract registering in the world (#3195) * Use the world as external contract reference and manage upgrades with sozo * update spawn-and-move with new external contract features * update test artifacts + world_address in dojo_dev.toml * export entrypoints + fix tests + remove TODO RBA * restore test_metadata_updated_event test * fix tests * rabbit improvements * fix test * feat: add proc macros (#3212) * WIP * add macros + core-tests crates + update them from 1.6.0 * update scripts/cairo_fmt.sh + fix fmt * update CI + Scarb.toml files * fix fmt * change from 2.11.2 to 2.11.4 * fix scarb issue + fmt * one testing library for each test runner (cairo, snfoundry) + update examples * fix fmt * update comment * chore: apply patches to sync exact same commits than scarb * fix(ci): update rust-toolchain version * fix(ci): bump clippy to use nightly compatible with 1.86 * fix: clippy and fmt * update spawn-and-move to use dojo-snf-test * update world address --------- Co-authored-by: glihm <dev@glihm.net> * Scarb crate removing (#3223) * introduce scarb_interop crate * handle missing scarb * introduce metadata instead of workspace to manage dojo related paths and config * add stats back and ensure profile is propagated until build not only metadata * cleanup and add error message if SCARB is not set correctly * add features + packages to build command * update scarb build/test command * refactor packages/features * fix utils.rs + enable auth command * fix rust fmt * re-enable sozo commands * re-enable the last sozo commands + new version command * fix issue with conflicts_with_all * check if manifest_path does not exist * set run() functions as async instead of using tokio * update init command management * refactor metadata loading + dead code cleaning * add build_simple_dev() function to be used to easily build spawn-and-move for tests/benches * update tests + fmt + clippy * remove dojo/lang * remove useless sozo files * tiny change * fix CI * use scarb nightly in CI * fix manifest_path/manifest_dir issue * fix fmt * update snfoundry version + update tests accordingly + add cairo-profiler to .tool-versions --------- Co-authored-by: glihm <dev@glihm.net> * feat: add DojoStore trait to handle storage serialization (#3219) * add DojoStore management * fix rust fmt after abigen * fix ModelReader * fix fmt * set DojoStore functions as inline(always) * update test artifacts * fix fmt * remove dojo-core test from CI as all tests are now in core-tests * feat: add block number for external contract events (#3224) * add block_number to ExternalContractRegistered/ExternalContractUpgraded events * remove useless println * restore external_contracts tests + update with block_number * update test artifacts * fix clippy * rework: handle block_number * fix world.dns for external contract + update tests * update sozo events with missing events * add a self-managed external contract called from spawn-and-move * update manifest_dev.json * use dns_address * fix fmt + abigen * update test artifacts * fix fmt * update starkli path * update policies * rebuild test db * fix: restore resource order to avoid storage conflict (#3238) * Resource enum variant order should not change as it is used in the world storage * fix fmt * fix world address and test db * fix fmt --------- Co-authored-by: glihm <dev@glihm.net> * fix(lang): remove warning with unit type (#3255) fix warning with unit type for enumerations serialization generated code. * dev: merge main in dev 1.6.0 (#3263) * Fix broken getting started link in README (#3235) * chore(versions): add torii 1.5.6 * Update DEVELOPMENT.md (#3236) * Update DEVELOPMENT.md with new instructions * Update scripts/rebuild_test_artifacts.sh with Rabbit suggestion Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Respond to review comments * Respond to review comments II * Respond to review comments III --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * feat: update rpc spec to 0.8 (#3179) * release(prepare): v1.6.0-alpha.0 (#3241) Prepare release: v1.6.0-alpha.0 Co-authored-by: glihm <glihm@users.noreply.github.com> * chore(versions): bump katana to 1.6.0-alpha.0 * fix(dojoup): remove new line in the source cmd (#3244) fix: fixed new line in the source cmd * chore: edited the build badge and its link (#3221) Co-authored-by: Ammar Arif <evergreenkary@gmail.com> * chore(devcontainer): update image: v1.6.0-alpha.0 (#3242) Update devcontainer image: v1.6.0-alpha.0 Co-authored-by: glihm <glihm@users.noreply.github.com> * chore: add katana 1.5.4 (#3245) * chore: add the missing backticks in the comments (#3243) Signed-off-by: one230six <723682061@qq.com> * fix(bindgen): use world namespace for imported models in TypeScript S… (#3249) * fix(bindgen): filter out Value models (#3248) * fix(bindgen): fix ts bytearray type mapping (#3247) * chore: update katana versions (#3253) add katana versions * sozo(unrealengine): handle UE5.6 and Dojo 1.5 (#3252) * fix(sozo): assert caller permission with match (#3254) * handle call_contract_syscall() result for better panic trace * avoid using 0 as caller address * remove temporarly the RPC version check. Currently, Katana uses the new RPC types, without bumping the spec version. To ensure we can still use sozo with sepolia/mainnet and Katana, Sozo will not check the RPC version for now * update test dbs --------- Co-authored-by: glihm <dev@glihm.net> * feat(sozo): create standalone bindgen command (#3246) * feat: create standalone bindgen command * cleanup unused inputs * remove dbg * add meaningful error if project is not built --------- Co-authored-by: glihm <dev@glihm.net> * feat(sozo): add MCP sever (#3256) * feat(sozo): add mvp for mcp server * refacto: restructure MCP server * evaluate changes using official rust sdk * feat(mcp): add stdio support * refacto with rmcp crate * wip * bump rmcp * add testing support * wip * refacto * cleanup * add instructions * add instructions * wip test * refacto tests * add debugging * ignore test with katana at the moment * fix typos and sozo path * refacto uri parsing * remove dbg * disable test that should be run with katana * fix test, windows fails because of new reqwest version * refactor(types): schema json sql value (#3257) * refactor(types): schema json sql value * remove excess comma * fix clippy --------- Co-authored-by: glihm <dev@glihm.net> * chore: bump cairo packages to 1.6.0-alpha.0 * fix(mcp): ensure test is using latest version * split scarb metadata ext in a different crate for dependencies * wip: controller issue * wip * wip: resolve conflicts and update deps * cleanup * lint * wip * wip tests * fix all tests * rebuild test db * fix: run linter * fix slot dep with github commit * fix clippy * fix inspect by adding missing json format * fix mcp formatting * fix stdio_test * use sozo from path --------- Signed-off-by: one230six <723682061@qq.com> Co-authored-by: Ritik <ritikverma0050@gmail.com> Co-authored-by: Daniel Kronovet <kronovet@gmail.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Ammar Arif <evergreenkary@gmail.com> Co-authored-by: Tarrence van As <tarrencev@users.noreply.github.com> Co-authored-by: glihm <glihm@users.noreply.github.com> Co-authored-by: Benjamin <158306087+bengineer42@users.noreply.github.com> Co-authored-by: braveocheretovych <braveocheretovych@gmail.com> Co-authored-by: one230six <163239332+one230six@users.noreply.github.com> Co-authored-by: Brother MartianGreed <valentin@pupucecorp.com> Co-authored-by: Corentin Cailleaud <corentin.cailleaud@caillef.com> Co-authored-by: Rémy Baranx <remy.baranx@gmail.com> Co-authored-by: Larko <59736843+Larkooo@users.noreply.github.com> * feat(core): use metaprogramming for tuple and fixed size array Introspect and DojoStore (#3260) * reuse Starkware metaprogramming stuff to manage tuples * fix fmt + clippy + artifacts * use metaprogramming to handle tuple introspect * fix fmt + clippy + artifacts * add fixed size array support * update dojo-world * update abigen + artifacts after rebase * add a sum_sizes function * update artifacts * tooling: add cairo-bench tool + bench tests (#3240) * add cairo-bench tool * add bench tests * first optimisation batch * fix fmt * add license info * update artifacts after rebase * fix cairo-bench tests * update artifacts * update artifacts * update scarb lock --------- Co-authored-by: glihm <dev@glihm.net> * fix(cairo-bench): use write and threshold + refactor fixed size array (#3267) * remove update-ref-test-list argument * add threshold parameter to cairo-bench (set to 3% by default) * max_fee_raw and fee_estimate_multiplier don't exist anymore * update sozo model commands for fixed size arrays * fix fmt + clippy * rename --update-ref to --write * fix some tests * fix fmt * fix(ci): update katana to 1.6.2 for compatible db --------- Signed-off-by: one230six <723682061@qq.com> Co-authored-by: Rémy Baranx <remy.baranx@gmail.com> Co-authored-by: Ritik <ritikverma0050@gmail.com> Co-authored-by: Daniel Kronovet <kronovet@gmail.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Ammar Arif <evergreenkary@gmail.com> Co-authored-by: Tarrence van As <tarrencev@users.noreply.github.com> Co-authored-by: glihm <glihm@users.noreply.github.com> Co-authored-by: Benjamin <158306087+bengineer42@users.noreply.github.com> Co-authored-by: braveocheretovych <braveocheretovych@gmail.com> Co-authored-by: one230six <163239332+one230six@users.noreply.github.com> Co-authored-by: Brother MartianGreed <valentin@pupucecorp.com> Co-authored-by: Corentin Cailleaud <corentin.cailleaud@caillef.com> Co-authored-by: Larko <59736843+Larkooo@users.noreply.github.com>
7 participants
depends on #3169Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Chores
Style