feat: add declare and deploy to sozo #3382
Conversation
|
Ohayo, sensei! WalkthroughAdds two new CLI commands (Declare, Deploy) to sozo plus supporting features: Katana preset accounts, RPC header parsing, signer helpers, environment-based account loader, and two workspace dependencies in Cargo.toml. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Declare as Declare Command
participant Account as Account/Signer
participant RPC as Starknet RPC
participant Declarer as Declarer
User->>Declare: Run declare with Sierra artifacts
Declare->>Declare: Validate artifact paths
Declare->>Account: Load account from environment
Declare->>RPC: Determine Blake2s hash usage
loop For each artifact
Declare->>Declare: Load & parse Sierra class
Declare->>Declare: Compute Sierra class hash
Declare->>Declare: Flatten & compute CASM hash
end
Declare->>Declarer: Declare classes
Declarer->>RPC: Submit declare txs
RPC-->>Declarer: Tx result
Declarer-->>Declare: Result (Noop/Hash/HashReceipt)
Declare->>User: Display per-artifact status & summary
sequenceDiagram
participant User
participant Deploy as Deploy Command
participant Account as Account/Signer
participant RPC as Starknet RPC
participant Deployer as Deployer (UDC)
User->>Deploy: Run deploy with class_hash, salt, calldata
Deploy->>Deploy: Decode constructor calldata
Deploy->>Deploy: Compute expected contract address
Deploy->>Account: Resolve account from environment
Deploy->>RPC: Build transaction config
Deploy->>Deployer: Call deploy_via_udc
Deployer->>RPC: Submit UDC deployment tx
RPC-->>Deployer: Tx result
Deployer-->>Deploy: Result (Noop/Hash/HashReceipt)
Deploy->>Deploy: Validate on-chain vs expected address
Deploy->>User: Display deployment status, address, tx hash
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
bin/sozo/src/commands/options/account/mod.rs (1)
160-176: ohayo sensei, align the error message and behavior with Katana presets.Two small points here:
- The error message still says “Please specify it with --account-address or in the environment config.” but
--katana-accountis now also a valid way to supply an address. Consider updating the message, e.g.:- "Could not find account address. Please specify it with --account-address or in \ - the environment config." + "Could not find account address. Please specify it with --account-address, \ + --katana-account, or in the environment config."
katana_details()is now used both here and instd_account; good centralization. It might also be worth a couple of small tests asserting that:
--katana-account katana0with no other address sources resolves to the expected Katana address.- When an explicit
--account-addressis provided, it wins over the Katana preset.
🧹 Nitpick comments (4)
bin/sozo/src/commands/deploy.rs (1)
91-136: Consider verifying class declaration, sensei.The deployment flow handles all
TransactionResultvariants correctly with appropriate UI feedback and address mismatch warnings. However, the code doesn't verify that theclass_hashis actually declared before attempting deployment.While the UDC will fail if the class isn't declared, catching this earlier would provide better UX with a clearer error message.
You could add an early check:
// After line 73, before creating Deployer let provider = account.provider(); match provider.get_class_hash_at(Felt::ZERO, class_hash).await { Ok(_) => {}, Err(_) => return Err(anyhow!("Class {:#066x} is not declared", class_hash)), }bin/sozo/src/commands/declare.rs (1)
47-49: Redundant check, sensei!The empty contracts check is unnecessary because the
#[arg(num_args = 1..)]constraint on line 26 already ensures at least one path is provided. Clap will reject the command before reaching this code if no contracts are specified.Consider removing this check:
- if contracts.is_empty() { - return Err(anyhow!("At least one Sierra artifact path must be provided.")); - } - let account = get_account_from_env(account, &starknet).await?;bin/sozo/src/commands/options/account/mod.rs (2)
42-47: ohayo sensei, the new--katana-accountCLI flag is clear and ergonomic.The
value_enumusage, help text, andglobal = trueflag are consistent with the existing options surface; this looks good. It would be nice to add a small test thatCommand::parse_from(["sozo", "--katana-account", "katana0"])parses correctly to guard against future regressions.
289-364: ohayo sensei, embedded Katana dev keys are intentional but will keep tripping Gitleaks.These
private_keyconstants look like the standard Katana pre‑funded dev accounts, which is fine from a security standpoint if they are only ever used against local/dev chains. However, static analysis (Gitleaks) is flagging them as generic API keys.To reduce noise and avoid future confusion, consider one of:
-#[derive(Clone, Copy)] -struct KatanaAccountDetails { +#[derive(Clone, Copy)] +struct KatanaAccountDetails { @@ - KatanaAccount::Katana0 => KatanaAccountDetails { - address: "0x127f...", - private_key: "0x00c5...", - }, + KatanaAccount::Katana0 => KatanaAccountDetails { + // Dev-only Katana account; safe to keep in source. + address: "0x127f...", + private_key: "0x00c5...", + },and/or adding an explicit Gitleaks allow rule for these known dev keys, plus a brief comment indicating they are local‑only fixtures and must not be re‑used on shared/public networks. I can help sketch a
.gitleaks.tomlsnippet if you’d like, sensei.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
bin/sozo/Cargo.toml(1 hunks)bin/sozo/src/commands/declare.rs(1 hunks)bin/sozo/src/commands/deploy.rs(1 hunks)bin/sozo/src/commands/mod.rs(5 hunks)bin/sozo/src/commands/options/account/mod.rs(6 hunks)bin/sozo/src/commands/options/signer.rs(1 hunks)bin/sozo/src/commands/options/starknet.rs(4 hunks)bin/sozo/src/main.rs(1 hunks)bin/sozo/src/utils.rs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
bin/sozo/src/commands/options/signer.rs (1)
crates/dojo/world/src/config/environment.rs (1)
private_key(38-40)
bin/sozo/src/utils.rs (3)
bin/sozo/src/commands/options/account/mod.rs (1)
account(76-101)bin/sozo/src/commands/options/starknet.rs (1)
provider(48-70)bin/sozo/src/commands/options/account/type.rs (1)
provider(172-174)
bin/sozo/src/commands/deploy.rs (3)
crates/dojo/world/src/config/calldata_decoder.rs (1)
decode_calldata(204-212)crates/sozo/ui/src/lib.rs (1)
trace(173-175)bin/sozo/src/utils.rs (1)
get_account_from_env(190-197)
bin/sozo/src/commands/mod.rs (2)
crates/dojo/utils/src/tx/declarer.rs (1)
declare(86-147)crates/dojo/types/src/naming.rs (1)
f(15-15)
bin/sozo/src/commands/declare.rs (1)
crates/dojo/world/src/config/environment.rs (1)
rpc_url(30-32)
🪛 Gitleaks (8.29.0)
bin/sozo/src/commands/options/account/mod.rs
[high] 324-324: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 328-328: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 332-332: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 340-340: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 344-344: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 348-348: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 352-352: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 360-360: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ 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 (14)
bin/sozo/Cargo.toml (1)
45-46: Ohayo sensei! Dependencies look solid.The workspace dependencies for
cairo-lang-starknet-classesandstarknet_apiare appropriate additions for supporting the new declare/deploy workflows. Clean integration with workspace-level version management.bin/sozo/src/main.rs (1)
46-49: Ohayo sensei! Async command routing looks clean.The addition of Declare and Deploy command handling follows the established async pattern correctly. Both commands properly await their run methods, maintaining consistency with the existing async command structure.
bin/sozo/src/commands/mod.rs (1)
14-15: Ohayo sensei! Command wiring is properly integrated.The new Declare and Deploy commands are correctly wired into the CLI:
- Module declarations follow the existing pattern
- Box wrappers used consistently for args structs
- Display implementations match the convention
- Async dispatch properly awaits the run methods
Also applies to: 33-34, 66-69, 101-102, 130-131
bin/sozo/src/commands/options/signer.rs (1)
49-59: Ohayo sensei! Signer helper methods are well-designed.The new utility methods enhance signer configuration:
with_private_keyproperly clears conflicting keystore fields, maintaining the invariant that only one signer source is activehas_custom_signerprovides a clean predicate for detecting custom signer configurationbin/sozo/src/utils.rs (1)
189-197: Ohayo sensei! Account helper function looks good.The
get_account_from_envutility provides a clean entry point for environment-based account construction. The function correctly delegates to the existing account resolution logic with appropriate defaults (None for env, empty HashMap for contracts).bin/sozo/src/commands/options/starknet.rs (2)
116-144: Ohayo sensei! RpcHeader implementation is solid.The
RpcHeaderstruct with itsFromStrandDisplayimplementations is well-designed:
- Proper format validation (Name:Value)
- Non-empty checks for both name and value
- Clear error messages
32-36: Ohayo! RPC header integration looks clean.The
rpc_headersfield is properly integrated into theStarknetOptionsstruct and correctly applied in theprovidermethod. The CLI argument configuration allows multiple headers with clear help text.Also applies to: 65-67
bin/sozo/src/commands/deploy.rs (2)
66-69: Ohayo sensei! Address computation is correct.The constructor calldata decoding and expected address computation follow the standard UDC pattern. Proper error context is provided for calldata parsing failures.
95-136: Perfect! I now have all the information needed to verify the actual_address semantics. Let me summarize what I found:Key findings:
deploy_via_udcimplementation (fromdeployer.rs):
- Returns
Result<(Felt, TransactionResult), TransactionError>- When contract is already deployed: Returns
Ok((Felt::ZERO, TransactionResult::Noop))(line 67)- When deployment succeeds: Returns
Ok((contract_address, TransactionResult::Hash/HashReceipt))The
Felt::ZEROsentinel value:
Felt::ZEROis returned specifically whenis_deployed()returnstrue(contract already exists on-chain)- In this case, the UDC is not called, so no actual on-chain address is obtained
- The code correctly falls back to
expected_address(computed viaget_contract_address())The conditional pattern: The pattern
if actual_address == Felt::ZERO { expected_address } else { actual_address }is correct and necessary in all three branches because:
- It handles the
Noopcase whereactual_address = ZERO- For
HashandHashReceiptcases,actual_addressis the valid computed address from the UDC call
Rewritten Review Comment:
The conditional pattern is correct — verify the consistency across all three branches is intentional.
The pattern
if actual_address == Felt::ZERO { expected_address } else { actual_address }correctly handles the case wheredeploy_via_udcreturnsFelt::ZEROas a sentinel value when a contract is already deployed on-chain. This occurs in theNoopcase, where the UDC is not invoked. The code appropriately falls back to the computedexpected_addressin all three transaction result branches (Noop,Hash,HashReceipt). The implementation is consistent and follows the semantics of the underlyingDeployer::deploy_via_udcfunction.bin/sozo/src/commands/declare.rs (4)
53-61: Blake2s detection heuristic works but is fragile, sensei.The string matching approach for detecting Sepolia/testnet networks is pragmatic but fragile—custom RPC endpoints or alternative naming could bypass it. However, the explicit
use_blake2s_casm_class_hashflag provides an override, so this is acceptable as a convenience heuristic.
67-124: Ohayo sensei! Declare flow is well-structured.The batch declaration workflow is clean and user-friendly:
- Proper preparation of each class with error context
- Clear UI feedback for each step
- Efficient batch operation via
declare_all- Appropriate handling of Noop/Hash/Receipt results
- Good summary reporting
138-154: Ohayo! Class preparation logic is solid.The
prepare_classhelper correctly:
- Loads and parses the Sierra contract
- Computes the class hash
- Flattens the class for network compatibility
- Derives a meaningful label from the file stem
- Provides proper error context
156-164: Parameters verified — no issues found.The hardcoded parameters in
CasmContractClass::from_contract_class(sierra_class, false, usize::MAX)are:
false=add_pythonic_hints— pythonic hints are not included (correct for hash computation)usize::MAX=max_bytecode_size— no bytecode size limit (consistent with similar usage incrates/dojo/world/src/local/artifact_to_local.rs)These values are appropriate for the use case and follow the established pattern elsewhere in the codebase.
bin/sozo/src/commands/options/account/mod.rs (1)
6-6: ohayo sensei, ValueEnum import for KatanaAccount is wired correctly.Pulling in
ValueEnumis exactly what’s needed for the newKatanaAccountCLI enum; no issues here.
| let katana_details = self.katana_details(); | ||
| let signer_options = if let Some(details) = katana_details { | ||
| if !self.signer.has_custom_signer() { | ||
| self.signer.with_private_key(details.private_key) | ||
| } else { | ||
| self.signer.clone() | ||
| } | ||
| } else { | ||
| self.signer.clone() | ||
| }; | ||
|
|
||
| let signer = signer_options.signer(env_metadata, false)?; | ||
|
|
There was a problem hiding this comment.
ohayo sensei, guard against mixing Katana private keys with non‑Katana addresses.
Right now, if --katana-account is set but an account address comes from DOJO_ACCOUNT_ADDRESS_ENV_VAR or env_metadata, std_account can combine:
account_addressfrom env / config, andprivate_keyfrom the Katana preset,
which will produce an invalid signer for that address.
To keep key/address pairs coherent, consider only injecting the Katana private key when Katana is also the source of the address (i.e., no explicit/metadata address is set), e.g.:
- let katana_details = self.katana_details();
- let signer_options = if let Some(details) = katana_details {
- if !self.signer.has_custom_signer() {
- self.signer.with_private_key(details.private_key)
- } else {
- self.signer.clone()
- }
- } else {
- self.signer.clone()
- };
+ let katana_details = self.katana_details();
+ let signer_options = if let Some(details) = katana_details {
+ // Only use Katana’s private key when no explicit address is set anywhere.
+ if !self.signer.has_custom_signer()
+ && self.account_address.is_none()
+ && env_metadata.and_then(|env| env.account_address()).is_none()
+ {
+ self.signer.with_private_key(details.private_key)
+ } else {
+ self.signer.clone()
+ }
+ } else {
+ self.signer.clone()
+ };And ideally add a test that explicitly covers --katana-account + env‑provided address to lock this behavior down.
🤖 Prompt for AI Agents
In bin/sozo/src/commands/options/account/mod.rs around lines 128 to 140, guard
against mixing a Katana private key with a non‑Katana address: only inject the
Katana private key when the account address is also coming from the Katana
preset (i.e., no explicit env/config address is set). Change the logic so that
if katana_details is Some you check whether an address was provided by
env_metadata or config and only call
self.signer.with_private_key(details.private_key) when the address source is
Katana (otherwise clone existing signer unchanged); update control flow/comments
accordingly and add a unit/integration test that exercises --katana-account
together with an env‑provided account address to assert the signer remains
consistent (no private key injection).
| pub fn provider_from_url( | ||
| &self, | ||
| headers: Option<Vec<(String, String)>>, | ||
| ) -> Result<JsonRpcClient<HttpTransport>> { | ||
| let client = | ||
| ClientBuilder::default().timeout(Self::DEFAULT_REQUEST_TIMEOUT).build().unwrap(); | ||
|
|
||
| let mut transport = HttpTransport::new_with_client( | ||
| self.rpc_url.clone().unwrap_or(Url::parse("http://localhost:5050").unwrap()), | ||
| client, | ||
| ); | ||
|
|
||
| for header in &self.rpc_headers { | ||
| transport.add_header(header.name.clone(), header.value.clone()); | ||
| } | ||
|
|
||
| if let Some(headers) = headers { | ||
| for header in headers.into_iter() { | ||
| transport.add_header(header.0, header.1); | ||
| } | ||
| } | ||
|
|
||
| Ok(JsonRpcClient::new(transport)) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Script executed:
#!/bin/bash
# Search for HttpTransport and add_header implementation
rg -n "impl.*HttpTransport" --type rust -A 10
rg -n "fn add_header" --type rust -A 5 -B 2Repository: dojoengine/dojo
<shell_result>
Repository: dojoengine/dojo
Exit code: 1
</shell_result>
Script executed:
#!/bin/bash
# Check the imports in the file under review
head -80 bin/sozo/src/commands/options/starknet.rs | grep -E "^use|^extern"
# Also search for Cargo.toml to understand dependencies
fd "Cargo.toml" | head -5 | xargs grep -l "starknet"Repository: dojoengine/dojo
<shell_result>
Repository: dojoengine/dojo
Exit code: 0
stdout:
use std::fmt::{Display, Formatter};
use std::str::FromStr;
use std::time::Duration;
use anyhow::{anyhow, Result};
use clap::Args;
use dojo_utils::env::STARKNET_RPC_URL_ENV_VAR;
use dojo_world::config::Environment;
use reqwest::ClientBuilder;
use starknet::providers::jsonrpc::HttpTransport;
use starknet::providers::JsonRpcClient;
use tracing::trace;
use url::Url;
Cargo.toml
bin/sozo/Cargo.toml
crates/dojo/bindgen/Cargo.toml
</shell_result>
Script executed:
#!/bin/bash
# Search for all usages of add_header
rg -n "add_header" --type rust -B 3 -A 3
# Also check if there are any tests for provider_from_url
rg -n "provider_from_url" --type rust -B 2 -A 10Repository: dojoengine/dojo
<shell_result>
Repository: dojoengine/dojo
Exit code: 0
stdout:
bin/sozo/src/commands/options/starknet.rs-58-
bin/sozo/src/commands/options/starknet.rs-59- if let Some(headers) = env_metadata.and_then(|env| env.http_headers.as_ref()) {
bin/sozo/src/commands/options/starknet.rs-60- for header in headers.iter() {
bin/sozo/src/commands/options/starknet.rs:61: transport.add_header(header.name.clone(), header.value.clone());
bin/sozo/src/commands/options/starknet.rs-62- }
bin/sozo/src/commands/options/starknet.rs-63- }
bin/sozo/src/commands/options/starknet.rs-64-
bin/sozo/src/commands/options/starknet.rs-65- for header in &self.rpc_headers {
bin/sozo/src/commands/options/starknet.rs:66: transport.add_header(header.name.clone(), header.value.clone());
bin/sozo/src/commands/options/starknet.rs-67- }
bin/sozo/src/commands/options/starknet.rs-68-
bin/sozo/src/commands/options/starknet.rs-69- Ok((JsonRpcClient::new(transport), url.to_string()))
--
bin/sozo/src/commands/options/starknet.rs-83- );
bin/sozo/src/commands/options/starknet.rs-84-
bin/sozo/src/commands/options/starknet.rs-85- for header in &self.rpc_headers {
bin/sozo/src/commands/options/starknet.rs:86: transport.add_header(header.name.clone(), header.value.clone());
bin/sozo/src/commands/options/starknet.rs-87- }
bin/sozo/src/commands/options/starknet.rs-88-
bin/sozo/src/commands/options/starknet.rs-89- if let Some(headers) = headers {
bin/sozo/src/commands/options/starknet.rs-90- for header in headers.into_iter() {
bin/sozo/src/commands/options/starknet.rs:91: transport.add_header(header.0, header.1);
bin/sozo/src/commands/options/starknet.rs-92- }
bin/sozo/src/commands/options/starknet.rs-93- }
bin/sozo/src/commands/options/starknet.rs-94-
bin/sozo/src/utils.rs-192- starknet: &StarknetOptions,
bin/sozo/src/utils.rs-193-) -> Result<SozoAccount<JsonRpcClient<HttpTransport>>> {
bin/sozo/src/utils.rs:194: let provider = starknet.provider_from_url(None)?;
bin/sozo/src/utils.rs-195-
bin/sozo/src/utils.rs-196- account.account(provider, None, starknet, &HashMap::new()).await
bin/sozo/src/utils.rs-197-}
bin/sozo/src/utils.rs-198-
bin/sozo/src/utils.rs-199-fn show_profile_details(profile_config: &ProfileConfig, ui: &SozoUi) {
bin/sozo/src/utils.rs-200- ui.verbose("local profile");
bin/sozo/src/utils.rs-201- let local_ui = ui.subsection();
bin/sozo/src/utils.rs-202-
bin/sozo/src/utils.rs-203- local_ui.verbose(format!(
bin/sozo/src/utils.rs-204- "world: (seed: {}, name: {})",
--
bin/sozo/src/commands/options/starknet.rs-71-
bin/sozo/src/commands/options/starknet.rs-72- /// Returns a [`JsonRpcClient`] from the given rpc url and headers.
bin/sozo/src/commands/options/starknet.rs:73: pub fn provider_from_url(
bin/sozo/src/commands/options/starknet.rs-74- &self,
bin/sozo/src/commands/options/starknet.rs-75- headers: Option<Vec<(String, String)>>,
bin/sozo/src/commands/options/starknet.rs-76- ) -> Result<JsonRpcClient<HttpTransport>> {
bin/sozo/src/commands/options/starknet.rs-77- let client =
bin/sozo/src/commands/options/starknet.rs-78- ClientBuilder::default().timeout(Self::DEFAULT_REQUEST_TIMEOUT).build().unwrap();
bin/sozo/src/commands/options/starknet.rs-79-
bin/sozo/src/commands/options/starknet.rs-80- let mut transport = HttpTransport::new_with_client(
bin/sozo/src/commands/options/starknet.rs-81- self.rpc_url.clone().unwrap_or(Url::parse("http://localhost:5050").unwrap()),
bin/sozo/src/commands/options/starknet.rs-82- client,
bin/sozo/src/commands/options/starknet.rs-83- );
</shell_result>
Script executed:
#!/bin/bash
# Get the full context of both methods in starknet.rs
sed -n '45,100p' bin/sozo/src/commands/options/starknet.rsRepository: dojoengine/dojo
<shell_result>
Repository: dojoengine/dojo
Exit code: 0
stdout:
/// Returns a [`JsonRpcClient`] and the rpc url.
///
/// It would be convenient to have the rpc url retrievable from the Provider trait instead.
pub fn provider(
&self,
env_metadata: Option<&Environment>,
) -> Result<(JsonRpcClient<HttpTransport>, String)> {
let url = self.url(env_metadata)?;
let client =
ClientBuilder::default().timeout(Self::DEFAULT_REQUEST_TIMEOUT).build().unwrap();
let mut transport = HttpTransport::new_with_client(url.clone(), client);
if let Some(headers) = env_metadata.and_then(|env| env.http_headers.as_ref()) {
for header in headers.iter() {
transport.add_header(header.name.clone(), header.value.clone());
}
}
for header in &self.rpc_headers {
transport.add_header(header.name.clone(), header.value.clone());
}
Ok((JsonRpcClient::new(transport), url.to_string()))
}
/// Returns a [`JsonRpcClient`] from the given rpc url and headers.
pub fn provider_from_url(
&self,
headers: Option<Vec<(String, String)>>,
) -> Result<JsonRpcClient<HttpTransport>> {
let client =
ClientBuilder::default().timeout(Self::DEFAULT_REQUEST_TIMEOUT).build().unwrap();
let mut transport = HttpTransport::new_with_client(
self.rpc_url.clone().unwrap_or(Url::parse("http://localhost:5050").unwrap()),
client,
);
for header in &self.rpc_headers {
transport.add_header(header.name.clone(), header.value.clone());
}
if let Some(headers) = headers {
for header in headers.into_iter() {
transport.add_header(header.0, header.1);
}
}
Ok(JsonRpcClient::new(transport))
}
// We dont check the env var because that would be handled by `clap`.
// This function is made public because [`JsonRpcClient`] does not expose
// the raw rpc url.
</shell_result>
Script executed:
#!/bin/bash
# Search for any tests or documentation about header behavior
rg -n "HttpTransport" --type rust -B 5 -A 5 | head -50
# Check if there are any comments about header precedence or ordering
rg -n "header" --type rust bin/sozo/src/commands/options/starknet.rs -i -B 2 -A 2Repository: dojoengine/dojo
<shell_result>
Repository: dojoengine/dojo
Exit code: 0
stdout:
scripts/verify_db_balances/src/main.rs-5-use num_traits::ToPrimitive;
scripts/verify_db_balances/src/main.rs-6-use sqlx::sqlite::SqlitePool;
scripts/verify_db_balances/src/main.rs-7-use sqlx::Row;
scripts/verify_db_balances/src/main.rs-8-use starknet::core::types::{BlockId, Felt, FunctionCall, U256};
scripts/verify_db_balances/src/main.rs-9-use starknet::macros::selector;
scripts/verify_db_balances/src/main.rs:10:use starknet::providers::jsonrpc::HttpTransport;
scripts/verify_db_balances/src/main.rs-11-use starknet::providers::{JsonRpcClient, Provider, Url};
scripts/verify_db_balances/src/main.rs-12-use tracing::{error, info, Level};
scripts/verify_db_balances/src/main.rs-13-
scripts/verify_db_balances/src/main.rs-14-async fn get_balance_from_starknet(
scripts/verify_db_balances/src/main.rs-15- account_address: &str,
scripts/verify_db_balances/src/main.rs-16- contract_address: &str,
scripts/verify_db_balances/src/main.rs-17- contract_type: &str,
scripts/verify_db_balances/src/main.rs-18- token_id: &str,
scripts/verify_db_balances/src/main.rs:19: provider: Arc<JsonRpcClient<HttpTransport>>,
scripts/verify_db_balances/src/main.rs-20-) -> Result<String, Box<dyn std::error::Error + Send + Sync>> {
scripts/verify_db_balances/src/main.rs-21- let account_address = Felt::from_str(account_address).unwrap();
scripts/verify_db_balances/src/main.rs-22- let contract_address = Felt::from_str(contract_address).unwrap();
scripts/verify_db_balances/src/main.rs-23-
scripts/verify_db_balances/src/main.rs-24- let balance = match contract_type {
--
scripts/verify_db_balances/src/main.rs-104-
scripts/verify_db_balances/src/main.rs-105- // print number of balances
scripts/verify_db_balances/src/main.rs-106- info!("Checking {} balances", rows.len());
scripts/verify_db_balances/src/main.rs-107-
scripts/verify_db_balances/src/main.rs-108- let provider =
scripts/verify_db_balances/src/main.rs:109: Arc::new(JsonRpcClient::new(HttpTransport::new(Url::parse(&args.rpc_url).unwrap())));
scripts/verify_db_balances/src/main.rs-110-
scripts/verify_db_balances/src/main.rs-111- // IMPROVEMENT: batch multiple balanceOf calls in same rpc call
scripts/verify_db_balances/src/main.rs-112- for row in rows {
scripts/verify_db_balances/src/main.rs-113- let account_address: String = row.get("account_address");
scripts/verify_db_balances/src/main.rs-114- let contract_address: String = row.get("contract_address");
--
bin/sozo/src/utils.rs-15-use semver::{Version, VersionReq};
bin/sozo/src/utils.rs-16-use sozo_ui::SozoUi;
bin/sozo/src/utils.rs-17-use starknet::accounts::{Account, ConnectedAccount};
bin/sozo/src/utils.rs-18-use starknet::core::types::Felt;
bin/sozo/src/utils.rs-19-use starknet::core::utils as snutils;
bin/sozo/src/utils.rs:20:use starknet::providers::jsonrpc::HttpTransport;
bin/sozo/src/utils.rs-21-use starknet::providers::{JsonRpcClient, Provider};
bin/sozo/src/utils.rs-22-use tabled::settings::Style;
bin/sozo/src/utils.rs-23-use tabled::{Table, Tabled};
bin/sozo/src/utils.rs-24-use tracing::trace;
bin/sozo/src/utils.rs-25-
--
bin/sozo/src/utils.rs-93-pub async fn get_world_diff_and_provider(
bin/sozo/src/utils.rs-94- starknet: StarknetOptions,
bin/sozo/src/utils.rs-95- world: WorldOptions,
bin/sozo/src/utils.rs-96- scarb_metadata: &Metadata,
bin/sozo/src/utils.rs-97- ui: &SozoUi,
30- pub use_blake2s_casm_class_hash: bool,
31-
32: #[arg(long = "rpc-header")]
33: #[arg(value_name = "HEADER")]
34: #[arg(help = "Custom header(s) to add to Starknet RPC requests. Format: Name:Value")]
35- #[arg(global = true)]
36: pub rpc_headers: Vec<RpcHeader>,
37-}
38-
--
57- let mut transport = HttpTransport::new_with_client(url.clone(), client);
58-
59: if let Some(headers) = env_metadata.and_then(|env| env.http_headers.as_ref()) {
60: for header in headers.iter() {
61: transport.add_header(header.name.clone(), header.value.clone());
62- }
63- }
64-
65: for header in &self.rpc_headers {
66: transport.add_header(header.name.clone(), header.value.clone());
67- }
68-
--
70- }
71-
72: /// Returns a [`JsonRpcClient`] from the given rpc url and headers.
73- pub fn provider_from_url(
74- &self,
75: headers: Option<Vec<(String, String)>>,
76- ) -> Result<JsonRpcClient<HttpTransport>> {
77- let client =
--
83- );
84-
85: for header in &self.rpc_headers {
86: transport.add_header(header.name.clone(), header.value.clone());
87- }
88-
89: if let Some(headers) = headers {
90: for header in headers.into_iter() {
91: transport.add_header(header.0, header.1);
92- }
93- }
--
115-
116-#[derive(Clone, Debug)]
117:pub struct RpcHeader {
118- pub name: String,
119- pub value: String,
120-}
121-
122:impl FromStr for RpcHeader {
123- type Err = anyhow::Error;
124-
125- fn from_str(s: &str) -> Result<Self, Self::Err> {
126- let (name, value) =
127: s.split_once(':').ok_or_else(|| anyhow!("Invalid header format. Use `Name:Value`"))?;
128-
129- let name = name.trim();
--
131-
132- if name.is_empty() || value.is_empty() {
133: return Err(anyhow!("Header name and value must be non-empty."));
134- }
135-
136: Ok(RpcHeader { name: name.to_string(), value: value.to_string() })
137- }
138-}
139-
140:impl Display for RpcHeader {
141- fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
142- write!(f, "{}: {}", self.name, self.value)
</shell_result>
Script executed:
#!/bin/bash
# Search for any tests that show how headers are used
rg -n "test" --type rust bin/sozo/src/commands/options/starknet.rs -A 20 | head -40
# Also check if there's a Cargo.lock or explicit version of starknet being used
grep -i "starknet" bin/sozo/Cargo.toml | head -10Repository: dojoengine/dojo
<shell_result>
Repository: dojoengine/dojo
Exit code: 0
stdout:
27: this flag automatically if the RPC URL contains `sepolia` or `testnet`. \
28- Otherwise, use this flag to manually set it.")]
29- #[arg(global = true)]
30- pub use_blake2s_casm_class_hash: bool,
31-
32- #[arg(long = "rpc-header")]
33- #[arg(value_name = "HEADER")]
34- #[arg(help = "Custom header(s) to add to Starknet RPC requests. Format: Name:Value")]
35- #[arg(global = true)]
36- pub rpc_headers: Vec<RpcHeader>,
37-}
38-
39-impl StarknetOptions {
40- /// The default request timeout in milliseconds. This is not the transaction inclusion timeout.
41- /// Refer to [dojo_utils::TransactionWaiter::DEFAULT_TIMEOUT] for the transaction
42- /// inclusion timeout.
43- const DEFAULT_REQUEST_TIMEOUT: Duration = Duration::from_secs(30);
44-
45- /// Returns a [`JsonRpcClient`] and the rpc url.
46- ///
47- /// It would be convenient to have the rpc url retrievable from the Provider trait instead.
--
146:#[cfg(test)]
147:mod tests {
148- use clap::Parser;
149- use dojo_utils::env::STARKNET_RPC_URL_ENV_VAR;
150-
151- use super::StarknetOptions;
152-
153- const ENV_RPC: &str = "http://localhost:7474/";
154- const METADATA_RPC: &str = "http://localhost:6060/";
155- const DEFAULT_RPC: &str = "http://localhost:5050/";
156-
157- #[derive(clap::Parser)]
158- struct Command {
159- #[clap(flatten)]
160- options: StarknetOptions,
161- }
162-
163: #[test]
starknet.workspace = true
starknet-crypto.workspace = true
cairo-lang-starknet-classes.workspace = true
starknet_api.workspace = true
</shell_result>
Based on my verification, I've found that the review comment identifies a real issue with header precedence in provider_from_url. More importantly, I discovered that there's an inconsistency between two methods:
provider()method (lines 45-69): Adds environment headers first, then CLI headers (last)provider_from_url()method (lines 73-96): Adds CLI headers first, then runtime headers (last)
This means the precedence order is opposite between the two methods. The review comment is valid but incomplete—it correctly identifies the need for clarity but doesn't capture the broader inconsistency.
Clarify and align header precedence behavior across methods.
In provider_from_url, headers from self.rpc_headers (CLI) are added first (lines 85-87), followed by runtime headers (lines 89-93). However, provider() uses the opposite order—environment headers first, CLI headers second. This inconsistency creates confusion about which source takes priority.
In HTTP clients, the last-added header typically wins for duplicates. Consider aligning both methods to use consistent precedence (e.g., runtime/parameter headers should override CLI headers consistently), and document the intended behavior.
🤖 Prompt for AI Agents
In bin/sozo/src/commands/options/starknet.rs around lines 73 to 96,
provider_from_url currently adds CLI headers (self.rpc_headers) before the
runtime/parameter headers, which is the opposite order used in provider() and
leads to inconsistent header precedence; to fix, reorder the header additions so
both methods add the same sources in the same sequence (choose one consistent
rule, e.g., add environment/CLI headers first and then runtime/parameter headers
so runtime headers override CLI), update provider_from_url to match provider()’s
ordering, and add a short comment documenting that the last-added header wins
for duplicates.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
bin/sozo/src/commands/options/starknet.rs (1)
72-87: Document when to useprovider_from_url()vsprovider(), sensei.The method lacks a doc comment explaining its purpose. Since
provider()includes environment metadata and headers while this method doesn't, clarify thatprovider_from_url()is for scenarios where environment configuration isn't available or needed (e.g., standalone operations outside a Dojo project context).Consider adding a doc comment like:
+ /// Returns a [`JsonRpcClient`] configured with CLI options only. + /// + /// Unlike [`Self::provider`], this method does not consider environment metadata + /// and is suitable for standalone operations that don't require project-specific configuration. pub fn provider_from_url(&self) -> Result<JsonRpcClient<HttpTransport>> {bin/sozo/src/commands/deploy.rs (1)
95-136: Consider extracting the address resolution logic, sensei.The pattern for resolving the deployed address and checking mismatches is duplicated across all three match arms. While correct, this could be simplified for better maintainability.
Consider extracting the common logic:
let deployed = if actual_address == Felt::ZERO { expected_address } else { actual_address }; match tx_result { TransactionResult::Noop => { ui.result(format!("Contract already deployed.\n Address : {deployed:#066x}")); } TransactionResult::Hash(hash) => { ui.result(format!( "Deployment submitted.\n Tx hash : {hash:#066x}\n Address : {deployed:#066x}" )); } TransactionResult::HashReceipt(hash, receipt) => { ui.result(format!( "Contract deployed onchain.\n Tx hash : {hash:#066x}\n Address : {deployed:#066x}" )); ui.debug(format!("Receipt: {:?}", receipt)); } } if deployed != expected_address { ui.warn(format!( "Computed address {expected_address:#066x} differs from on-chain {deployed:#066x}." )); }bin/sozo/src/commands/declare.rs (2)
53-61: Consider case-insensitive network detection, sensei.The string matching for "sepolia" and "testnet" is case-sensitive. While RPC URLs typically use lowercase, making this case-insensitive would be more robust against variations.
Apply this diff for case-insensitive matching:
- let use_blake2s = if let Some(rpc_url) = starknet.rpc_url { - if rpc_url.to_string().contains("sepolia") || rpc_url.to_string().contains("testnet") { + let use_blake2s = if let Some(rpc_url) = &starknet.rpc_url { + let url_str = rpc_url.as_str().to_lowercase(); + if url_str.contains("sepolia") || url_str.contains("testnet") { true } else { starknet.use_blake2s_casm_class_hash
158-166: Consider documenting CASM compilation parameters, sensei.The
false(add_pythonic_hints) andusize::MAX(max_bytecode_size) parameters at line 160 are magic values that could benefit from brief inline comments or named constants for clarity.fn casm_class_hash_from_bytes(data: &[u8], use_blake2s: bool) -> Result<Felt> { let sierra_class: ContractClass = serde_json::from_slice(data)?; + // Compile to CASM without pythonic hints and no bytecode size limit let casm_class = CasmContractClass::from_contract_class(sierra_class, false, usize::MAX)?;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
bin/sozo/src/commands/declare.rs(1 hunks)bin/sozo/src/commands/deploy.rs(1 hunks)bin/sozo/src/commands/options/starknet.rs(4 hunks)bin/sozo/src/utils.rs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
bin/sozo/src/commands/deploy.rs (3)
crates/dojo/world/src/config/calldata_decoder.rs (1)
decode_calldata(204-212)crates/sozo/ui/src/lib.rs (1)
trace(173-175)bin/sozo/src/utils.rs (1)
get_account_from_env(190-197)
bin/sozo/src/utils.rs (3)
bin/sozo/src/commands/options/account/mod.rs (1)
account(76-101)bin/sozo/src/commands/options/starknet.rs (1)
provider(48-70)bin/sozo/src/commands/options/account/type.rs (1)
provider(172-174)
⏰ 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: clippy
🔇 Additional comments (9)
bin/sozo/src/commands/options/starknet.rs (3)
1-2: Ohayo! Clean CLI integration for custom headers, sensei!The RpcHeader field integration looks solid with proper clap attributes and helpful documentation for users.
Also applies to: 32-36
113-129: Ohayo! Solid parsing with good validation, sensei!The FromStr implementation correctly handles the Name:Value format with proper trimming and validation. Error messages are clear and actionable.
131-135: Clean Display implementation!The formatting matches standard HTTP header conventions.
bin/sozo/src/utils.rs (1)
189-197: Ohayo! Clean utility function for environment-based accounts, sensei!The implementation correctly uses
provider_from_url()and passes appropriate defaults for standalone operations outside world context.bin/sozo/src/commands/deploy.rs (2)
15-50: Well-structured command arguments, sensei!The DeployArgs struct has clear documentation and sensible defaults. The reference to CALLDATA_DOC maintains consistency with other commands.
53-93: Ohayo! Solid deployment flow with good error context, sensei!The setup handles constructor calldata decoding, address computation, and UI feedback nicely. Error contexts make debugging easier.
bin/sozo/src/commands/declare.rs (3)
20-39: Clean command structure, sensei!The DeclareArgs struct is well-organized with clear documentation for Sierra artifact paths.
65-92: Ohayo! Well-structured class preparation and declaration flow, sensei!The sequential preparation with error context and UI feedback is clear and user-friendly. The Declarer API integration looks clean.
94-126: Clean result processing with helpful user feedback!The differentiation between already-declared and newly-declared classes with appropriate UI messaging is well done.
Sozo can now declare and deploy standalone contracts on the network without relying on a dojo project.
Summary by CodeRabbit
declarecommand to declare Sierra contracts with CASM compilation and transaction reportingdeploycommand to deploy declared contracts via the Universal Deployer Contract✏️ Tip: You can customize this high-level summary in your review settings.