Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 22 additions & 22 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ license-file = "LICENSE"
repository = "https://github.com/dojoengine/dojo/"
documentation = "https://book.dojoengine.org"
homepage = "https://dojoengine.org"
version = "1.7.1"
version = "1.7.2"

[profile.performance]
codegen-units = 1
Expand All @@ -64,7 +64,7 @@ dojo-metrics = { path = "crates/metrics" }
dojo-bindgen = { path = "crates/dojo/bindgen" }
dojo-core = { path = "crates/dojo/core" }
dojo-test-utils = { path = "crates/dojo/test-utils" }
dojo-types = { path = "crates/dojo/types", version = "1.7.1" }
dojo-types = { path = "crates/dojo/types", version = "1.7.2" }
dojo-world = { path = "crates/dojo/world" }

# sozo
Expand Down
61 changes: 61 additions & 0 deletions bin/sozo/src/commands/options/transaction.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use anyhow::{bail, Result};
use clap::Args;
use dojo_utils::{FeeConfig, TxnAction, TxnConfig};
use starknet::core::types::TransactionFinalityStatus;

#[derive(Debug, Clone, Args, Default)]
#[command(next_help_heading = "Transaction options")]
Expand Down Expand Up @@ -62,6 +63,15 @@ pub struct TransactionOptions {
#[arg(global = true)]
#[arg(default_value = "10")]
pub max_calls: Option<usize>,

#[arg(long)]
#[arg(help = "The finality status to wait for. Since 0.14, the nodes syncing is sometime \
not fast enough to propagate the transaction to the nodes in the \
PRE-CONFIRMED state. The default is ACCEPTED_ON_L2. Available options are: \
PRE-CONFIRMED, ACCEPTED_ON_L2, ACCEPTED_ON_L1.")]
#[arg(global = true)]
#[arg(default_value = "ACCEPTED_ON_L2")]
pub finality_status: Option<String>,
Comment on lines +67 to +74
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Fix inconsistency between help text and parser!

The help text documents the finality status values with hyphens (PRE-CONFIRMED, ACCEPTED-ON-L2, ACCEPTED-ON-L1), but the parser at line 142 expects underscores (PRE_CONFIRMED, ACCEPTED_ON_L2, ACCEPTED_ON_L1).

This will confuse users who copy the values from the help text and encounter parsing errors.

Apply this diff to fix the help text:

-    #[arg(help = "The finality status to wait for. Since 0.14, the nodes syncing is sometime \
-                  not fast enough to propagate the transaction to the nodes in the \
-                  PRE-CONFIRMED state. The default is ACCEPTED_ON_L2. Available options are: \
-                  PRE-CONFIRMED, ACCEPTED_ON_L2, ACCEPTED_ON_L1.")]
+    #[arg(help = "The finality status to wait for. Since 0.14, the nodes syncing is sometime \
+                  not fast enough to propagate the transaction to the nodes in the \
+                  PRE_CONFIRMED state. The default is ACCEPTED_ON_L2. Available options are: \
+                  PRE_CONFIRMED, ACCEPTED_ON_L2, ACCEPTED_ON_L1.")]
🤖 Prompt for AI Agents
In bin/sozo/src/commands/options/transaction.rs around lines 67 to 74, the help
text lists finality status values with hyphens but the parser expects
underscores; update the help text examples to use underscores (PRE_CONFIRMED,
ACCEPTED_ON_L2, ACCEPTED_ON_L1) so they match the parser and the default value,
and ensure the descriptive sentence still reads correctly with the underscore
variants.

}

impl TransactionOptions {
Expand Down Expand Up @@ -89,6 +99,7 @@ impl TransactionOptions {
},
walnut: self.walnut,
max_calls: self.max_calls,
finality_status: parse_finality_status(self.finality_status.clone())?,
}),
}
}
Expand All @@ -111,10 +122,33 @@ impl TryFrom<TransactionOptions> for TxnConfig {
l2_gas_price: value.l2_gas_price,
},
max_calls: value.max_calls,
finality_status: parse_finality_status(value.finality_status.clone())?,
})
}
}

/// Parses the finality status from a string.
/// If no status is provided, the default is ACCEPTED_ON_L2.
/// # Arguments
///
/// * `status` - The finality status to parse.
///
/// # Returns
///
/// The parsed finality status.
fn parse_finality_status(status: Option<String>) -> Result<TransactionFinalityStatus> {
if let Some(status) = status {
match status.to_uppercase().as_str() {
"PRE_CONFIRMED" => Ok(TransactionFinalityStatus::PreConfirmed),
"ACCEPTED_ON_L2" => Ok(TransactionFinalityStatus::AcceptedOnL2),
"ACCEPTED_ON_L1" => Ok(TransactionFinalityStatus::AcceptedOnL1),
_ => bail!("Invalid finality status: {}", status),
}
} else {
Ok(TransactionFinalityStatus::AcceptedOnL2)
}
}

#[cfg(test)]
mod tests {
use anyhow::Result;
Expand All @@ -134,6 +168,7 @@ mod tests {
l2_gas_price: Some(1_000),
walnut: false,
max_calls: Some(10),
finality_status: Some("PRE_CONFIRMED".to_string()),
};

let config: TxnConfig = opts.try_into()?;
Expand All @@ -150,6 +185,32 @@ mod tests {
assert_eq!(config.fee_config.l2_gas, Some(10_000));
assert_eq!(config.fee_config.l2_gas_price, Some(1_000));

assert_eq!(config.finality_status, TransactionFinalityStatus::PreConfirmed);

Ok(())
}

#[test]
fn test_parse_finality_status() -> Result<()> {
matches!(
parse_finality_status(Some("PRE_CONFIRMED".to_string())),
Ok(TransactionFinalityStatus::PreConfirmed)
);

matches!(
parse_finality_status(Some("ACCEPTED_ON_L2".to_string())),
Ok(TransactionFinalityStatus::AcceptedOnL2)
);

matches!(
parse_finality_status(Some("ACCEPTED_ON_L1".to_string())),
Ok(TransactionFinalityStatus::AcceptedOnL1)
);

matches!(parse_finality_status(None), Ok(TransactionFinalityStatus::AcceptedOnL2));

assert!(parse_finality_status(Some("INVALID".to_string())).is_err());

Ok(())
}
Comment on lines +193 to 215
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: Test assertions are not working!

Lines 195-210 use the matches! macro but don't assert the result, meaning these test cases always pass regardless of whether the parsing succeeds or fails. The matches! expression evaluates but the boolean result is discarded.

Apply this diff to fix the test assertions:

     #[test]
     fn test_parse_finality_status() -> Result<()> {
-        matches!(
+        assert!(matches!(
             parse_finality_status(Some("PRE_CONFIRMED".to_string())),
             Ok(TransactionFinalityStatus::PreConfirmed)
-        );
+        ));

-        matches!(
+        assert!(matches!(
             parse_finality_status(Some("ACCEPTED_ON_L2".to_string())),
             Ok(TransactionFinalityStatus::AcceptedOnL2)
-        );
+        ));

-        matches!(
+        assert!(matches!(
             parse_finality_status(Some("ACCEPTED_ON_L1".to_string())),
             Ok(TransactionFinalityStatus::AcceptedOnL1)
-        );
+        ));

-        matches!(parse_finality_status(None), Ok(TransactionFinalityStatus::AcceptedOnL2));
+        assert!(matches!(parse_finality_status(None), Ok(TransactionFinalityStatus::AcceptedOnL2)));

         assert!(parse_finality_status(Some("INVALID".to_string())).is_err());
📝 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.

Suggested change
#[test]
fn test_parse_finality_status() -> Result<()> {
matches!(
parse_finality_status(Some("PRE_CONFIRMED".to_string())),
Ok(TransactionFinalityStatus::PreConfirmed)
);
matches!(
parse_finality_status(Some("ACCEPTED_ON_L2".to_string())),
Ok(TransactionFinalityStatus::AcceptedOnL2)
);
matches!(
parse_finality_status(Some("ACCEPTED_ON_L1".to_string())),
Ok(TransactionFinalityStatus::AcceptedOnL1)
);
matches!(parse_finality_status(None), Ok(TransactionFinalityStatus::AcceptedOnL2));
assert!(parse_finality_status(Some("INVALID".to_string())).is_err());
Ok(())
}
#[test]
fn test_parse_finality_status() -> Result<()> {
assert!(matches!(
parse_finality_status(Some("PRE_CONFIRMED".to_string())),
Ok(TransactionFinalityStatus::PreConfirmed)
));
assert!(matches!(
parse_finality_status(Some("ACCEPTED_ON_L2".to_string())),
Ok(TransactionFinalityStatus::AcceptedOnL2)
));
assert!(matches!(
parse_finality_status(Some("ACCEPTED_ON_L1".to_string())),
Ok(TransactionFinalityStatus::AcceptedOnL1)
));
assert!(matches!(parse_finality_status(None), Ok(TransactionFinalityStatus::AcceptedOnL2)));
assert!(parse_finality_status(Some("INVALID".to_string())).is_err());
Ok(())
}
🤖 Prompt for AI Agents
In bin/sozo/src/commands/options/transaction.rs around lines 193 to 215, the
test uses matches! without asserting its boolean result so failures are ignored;
update each standalone matches!(...) to assert!(matches!(...)) (or replace with
assert_eq!(parse_finality_status(...), Ok(...)) if preferred) for the three
explicit string cases and the None case so the test actually fails on mismatch,
leaving the existing
assert!(parse_finality_status(Some("INVALID".to_string())).is_err()) and Ok(())
unchanged.

}
6 changes: 3 additions & 3 deletions crates/dojo/core-tests/Scarb.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ version = 1

[[package]]
name = "dojo"
version = "1.7.1"
version = "1.7.2"
dependencies = [
"dojo_cairo_macros",
]
Expand All @@ -14,7 +14,7 @@ version = "1.7.1"

[[package]]
name = "dojo_core_test"
version = "1.7.1"
version = "1.7.2"
dependencies = [
"dojo",
"dojo_cairo_macros",
Expand All @@ -24,7 +24,7 @@ dependencies = [

[[package]]
name = "dojo_snf_test"
version = "1.7.1"
version = "1.7.2"
dependencies = [
"dojo",
"snforge_std",
Expand Down
2 changes: 1 addition & 1 deletion crates/dojo/core-tests/Scarb.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
name = "dojo_core_test"
description = "Testing library for Dojo using Starknet foundry."

version = "1.7.1"
version = "1.7.2"
edition = "2024_07"
cairo-version = "2.12"

Expand Down
2 changes: 1 addition & 1 deletion crates/dojo/core/Scarb.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ version = 1

[[package]]
name = "dojo"
version = "1.7.1"
version = "1.7.2"
dependencies = [
"dojo_cairo_macros",
]
Expand Down
2 changes: 1 addition & 1 deletion crates/dojo/core/Scarb.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ cairo-version = "2.12"
edition = "2024_07"
description = "The Dojo Core library for autonomous worlds."
name = "dojo"
version = "1.7.1"
version = "1.7.2"
license = "MIT"
repository = "https://github.com/dojoengine/dojo"
documentation = "https://book.dojoengine.org"
Expand Down
2 changes: 1 addition & 1 deletion crates/dojo/core/src/world/world_contract.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ pub mod world {

pub const WORLD: felt252 = 0;
pub const DOJO_INIT_SELECTOR: felt252 = selector!("dojo_init");
pub const WORLD_VERSION: felt252 = '1.7.1';
pub const WORLD_VERSION: felt252 = '1.7.2';

#[event]
#[derive(Drop, starknet::Event)]
Expand Down
4 changes: 2 additions & 2 deletions crates/dojo/dojo-cairo-test/Scarb.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ version = 1

[[package]]
name = "dojo"
version = "1.7.1"
version = "1.7.2"
dependencies = [
"dojo_cairo_macros",
]
Expand All @@ -14,7 +14,7 @@ version = "1.7.1"

[[package]]
name = "dojo_cairo_test"
version = "1.7.1"
version = "1.7.2"
dependencies = [
"dojo",
]
Loading
Loading