Validate max gas is greater than zero for strk fee settings

CHANGELOG.md

@@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Cast
+
+### Changed
+
+- Values passed to the `--max-fee`, `--max-gas`, and `--max-gas-unit-price` flags must be greater than 0
+
 ## [0.35.1] - 2024-12-16
 
 ### Forge

crates/sncast/src/helpers/fee.rs

@@ -15,24 +15,24 @@ pub struct FeeArgs {
     pub fee_token: Option<FeeToken>,
 
     /// Max fee for the transaction. If not provided, will be automatically estimated.
-    #[clap(short, long)]
-    pub max_fee: Option<Felt>,
+    #[clap(value_parser = parse_non_zero_felt, short, long)]
+    pub max_fee: Option<NonZeroFelt>,
 
     /// Max gas amount. If not provided, will be automatically estimated. (Only for STRK fee payment)
-    #[clap(long)]
-    pub max_gas: Option<Felt>,
+    #[clap(value_parser = parse_non_zero_felt, long)]
+    pub max_gas: Option<NonZeroFelt>,
 
     /// Max gas price in Fri. If not provided, will be automatically estimated. (Only for STRK fee payment)
-    #[clap(long)]
-    pub max_gas_unit_price: Option<Felt>,
+    #[clap(value_parser = parse_non_zero_felt, long)]
+    pub max_gas_unit_price: Option<NonZeroFelt>,
 }
 
 impl From<ScriptFeeSettings> for FeeArgs {
     fn from(script_fee_settings: ScriptFeeSettings) -> Self {
         match script_fee_settings {
             ScriptFeeSettings::Eth { max_fee } => Self {
                 fee_token: Some(FeeToken::Eth),
-                max_fee,
+                max_fee: max_fee.and_then(|value| NonZeroFelt::try_from(value).ok()),
                 max_gas: None,
                 max_gas_unit_price: None,
             },
@@ -42,9 +42,13 @@ impl From<ScriptFeeSettings> for FeeArgs {
                 max_gas_unit_price,
             } => Self {
                 fee_token: Some(FeeToken::Strk),
-                max_fee,
-                max_gas: max_gas.map(Into::into),
-                max_gas_unit_price: max_gas_unit_price.map(Into::into),
+                max_fee: max_fee.and_then(|value| NonZeroFelt::try_from(value).ok()),
+                max_gas: max_gas
+                    .map(Into::into)
+                    .and_then(|val: Felt| NonZeroFelt::try_from(val).ok()),
+                max_gas_unit_price: max_gas_unit_price
+                    .map(Into::into)
+                    .and_then(|val: Felt| NonZeroFelt::try_from(val).ok()),
             },
         }
     }
@@ -75,7 +79,7 @@ impl FeeArgs {
                     "--max-gas-unit-price is not supported for ETH fee payment"
                 );
                 Ok(FeeSettings::Eth {
-                    max_fee: self.max_fee,
+                    max_fee: self.max_fee.map(Felt::from),
                 })
             }
             FeeToken::Strk => {
@@ -92,43 +96,44 @@ impl FeeArgs {
                         bail!("--max-fee should be greater than or equal to --max-gas-unit-price")
                     }
                     (None, _, _) => FeeSettings::Strk {
-                        max_gas: self.max_gas.map(TryIntoConv::try_into_).transpose()?,
+                        max_gas: self
+                            .max_gas
+                            .map(Felt::from)
+                            .map(TryIntoConv::try_into_)
+                            .transpose()?,
                         max_gas_unit_price: self
                             .max_gas_unit_price
+                            .map(Felt::from)
                             .map(TryIntoConv::try_into_)
                             .transpose()?,
                     },
                     (Some(max_fee), None, Some(max_gas_unit_price)) => FeeSettings::Strk {
                         max_gas: Some(
-                            max_fee
-                                .floor_div(&NonZeroFelt::from_felt_unchecked(max_gas_unit_price))
-                                .try_into_()?,
-                        ),
-                        max_gas_unit_price: Some(max_gas_unit_price.try_into_()?),
-                    },
-                    (Some(max_fee), Some(max_gas), None) => FeeSettings::Strk {
-                        max_gas: Some(max_gas.try_into_()?),
-                        max_gas_unit_price: Some(
-                            max_fee
-                                .floor_div(&NonZeroFelt::from_felt_unchecked(max_gas))
+                            Felt::from(max_fee)
+                                .floor_div(&max_gas_unit_price)
                                 .try_into_()?,
                         ),
+                        max_gas_unit_price: Some(Felt::from(max_gas_unit_price).try_into()?),
                     },
+                    (Some(max_fee), Some(max_gas), None) => {
+                        let max_gas_unit_price = NonZeroFelt::try_from(Felt::from(max_fee).floor_div(&max_gas)).expect("Calculated max gas unit price from provided --max-fee and --max-gas is zero. Please increase --max-fee or decrease --max-gas to ensure a positive gas unit price");
+
+                        FeeSettings::Strk {
+                            max_gas: Some(Felt::from(max_gas).try_into_()?),
+                            max_gas_unit_price: Some(Felt::from(max_gas_unit_price).try_into_()?),
+                        }
+                    }
                     (Some(max_fee), None, None) => {
                         let max_gas_unit_price = provider
                             .get_block_with_tx_hashes(block_id)
                             .await?
                             .l1_gas_price()
                             .price_in_fri;
+                        let max_gas = NonZeroFelt::try_from(Felt::from(max_fee)
+                            .floor_div(&NonZeroFelt::try_from(max_gas_unit_price)?)).expect("Calculated max-gas from provided --max-fee and the current network gas price is zero. Please increase --max-fee to obtain a positive gas amount");
 
                         FeeSettings::Strk {
-                            max_gas: Some(
-                                max_fee
-                                    .floor_div(&NonZeroFelt::from_felt_unchecked(
-                                        max_gas_unit_price,
-                                    ))
-                                    .try_into_()?,
-                            ),
+                            max_gas: Some(Felt::from(max_gas).try_into_()?),
                             max_gas_unit_price: Some(max_gas_unit_price.try_into_()?),
                         }
                     }
@@ -261,3 +266,9 @@ fn parse_fee_token(s: &str) -> Result<FeeToken, String> {
 
     Ok(parsed_token)
 }
+
+fn parse_non_zero_felt(s: &str) -> Result<NonZeroFelt, String> {
+    let felt: Felt = s.parse().map_err(|_| "Failed to parse value")?;
+    felt.try_into()
+        .map_err(|_| "Value should be greater than 0".to_string())
+}

crates/sncast/tests/e2e/invoke.rs

@@ -388,6 +388,76 @@ fn test_too_low_max_fee() {
     );
 }
 
+#[test]
+fn test_max_gas_equal_to_zero() {
+    let args = vec![
+        "--accounts-file",
+        ACCOUNT_FILE_PATH,
+        "--account",
+        "user11",
+        "--wait",
+        "invoke",
+        "--url",
+        URL,
+        "--contract-address",
+        MAP_CONTRACT_ADDRESS_SEPOLIA,
+        "--function",
+        "put",
+        "--calldata",
+        "0x1",
+        "0x2",
+        "--max-gas",
+        "0",
+        "--fee-token",
+        "strk",
+    ];
+
+    let snapbox = runner(&args);
+    let output = snapbox.assert().code(2);
+
+    assert_stderr_contains(
+        output,
+        indoc! {r"
+        error: invalid value '0' for '--max-gas <MAX_GAS>': Value should be greater than 0
+        "},
+    );
+}
+
+#[test]
+fn test_max_gas_calculated_from_max_fee_equal_to_zero() {
+    let args = vec![
+        "--accounts-file",
+        ACCOUNT_FILE_PATH,
+        "--account",
+        "user11",
+        "--wait",
+        "invoke",
+        "--url",
+        URL,
+        "--contract-address",
+        MAP_CONTRACT_ADDRESS_SEPOLIA,
+        "--function",
+        "put",
+        "--calldata",
+        "0x1",
+        "0x2",
+        "--max-fee",
+        "0",
+        "--fee-token",
+        "strk",
+    ];
+
+    let snapbox = runner(&args);
+    let output = snapbox.assert().code(2);
+
+    assert_stderr_contains(
+        output,
+        indoc! {r"
+        error: invalid value '0' for '--max-fee <MAX_FEE>': Value should be greater than 0
+        "},
+    );
+}
+
 #[tokio::test]
 async fn test_happy_case_cairo_expression_calldata() {
     let tempdir = create_and_deploy_oz_account().await;

crates/sncast/tests/integration/fee.rs

@@ -5,8 +5,8 @@ use starknet::accounts::{AccountFactory, OpenZeppelinAccountFactory};
 use starknet::providers::jsonrpc::HttpTransport;
 use starknet::providers::{JsonRpcClient, Provider};
 use starknet::signers::{LocalWallet, SigningKey};
+use starknet_types_core::felt::Felt;
 use url::Url;
-
 const MAX_FEE: u64 = 1_000_000_000_000;
 
 async fn get_factory() -> OpenZeppelinAccountFactory<LocalWallet, JsonRpcClient<HttpTransport>> {
@@ -26,7 +26,7 @@ async fn test_happy_case_eth() {
 
     let args = FeeArgs {
         fee_token: Some(FeeToken::Eth),
-        max_fee: Some(100_u32.into()),
+        max_fee: Some(Felt::from(100_u32).try_into().unwrap()),
         max_gas: None,
         max_gas_unit_price: None,
     };
@@ -50,8 +50,8 @@ async fn test_max_gas_eth() {
 
     let args = FeeArgs {
         fee_token: Some(FeeToken::Eth),
-        max_fee: Some(100_u32.into()),
-        max_gas: Some(100_u32.into()),
+        max_fee: Some(Felt::from(100_u32).try_into().unwrap()),
+        max_gas: Some(Felt::from(100_u32).try_into().unwrap()),
         max_gas_unit_price: None,
     };
 
@@ -71,9 +71,9 @@ async fn test_max_gas_unit_price_eth() {
 
     let args = FeeArgs {
         fee_token: Some(FeeToken::Eth),
-        max_fee: Some(100_u32.into()),
+        max_fee: Some(Felt::from(100).try_into().unwrap()),
         max_gas: None,
-        max_gas_unit_price: Some(100_u32.into()),
+        max_gas_unit_price: Some(Felt::from(100_u32).try_into().unwrap()),
     };
 
     let error = args
@@ -92,9 +92,9 @@ async fn test_all_args() {
 
     let args = FeeArgs {
         fee_token: Some(FeeToken::Strk),
-        max_fee: Some(100_u32.into()),
-        max_gas: Some(100_u32.into()),
-        max_gas_unit_price: Some(100_u32.into()),
+        max_fee: Some(Felt::from(100_u32).try_into().unwrap()),
+        max_gas: Some(Felt::from(100_u32).try_into().unwrap()),
+        max_gas_unit_price: Some(Felt::from(100_u32).try_into().unwrap()),
     };
 
     let error = args
@@ -113,8 +113,8 @@ async fn test_max_fee_less_than_max_gas() {
 
     let args = FeeArgs {
         fee_token: Some(FeeToken::Strk),
-        max_fee: Some(50_u32.into()),
-        max_gas: Some(100_u32.into()),
+        max_fee: Some(Felt::from(50_u32).try_into().unwrap()),
+        max_gas: Some(Felt::from(100_u32).try_into().unwrap()),
         max_gas_unit_price: None,
     };
 
@@ -134,9 +134,9 @@ async fn test_max_fee_less_than_max_gas_unit_price() {
 
     let args = FeeArgs {
         fee_token: Some(FeeToken::Strk),
-        max_fee: Some(50_u32.into()),
+        max_fee: Some(Felt::from(50_u32).try_into().unwrap()),
         max_gas: None,
-        max_gas_unit_price: Some(100_u32.into()),
+        max_gas_unit_price: Some(Felt::from(100).try_into().unwrap()),
     };
 
     let error = args
@@ -154,7 +154,7 @@ async fn test_strk_fee_get_max_fee() {
 
     let args = FeeArgs {
         fee_token: Some(FeeToken::Strk),
-        max_fee: Some(MAX_FEE.into()),
+        max_fee: Some(Felt::from(MAX_FEE).try_into().unwrap()),
         max_gas: None,
         max_gas_unit_price: None,
     };
@@ -184,8 +184,8 @@ async fn test_strk_fee_get_max_fee_with_max_gas() {
 
     let args = FeeArgs {
         fee_token: Some(FeeToken::Strk),
-        max_fee: Some(MAX_FEE.into()),
-        max_gas: Some(1_000_000_u32.into()),
+        max_fee: Some(Felt::from(MAX_FEE).try_into().unwrap()),
+        max_gas: Some(Felt::from(1_000_000_u32).try_into().unwrap()),
         max_gas_unit_price: None,
     };
 
@@ -223,8 +223,8 @@ async fn test_strk_fee_get_max_gas_and_max_gas_unit_price() {
     let args = FeeArgs {
         fee_token: Some(FeeToken::Strk),
         max_fee: None,
-        max_gas: Some(1_000_000_u32.into()),
-        max_gas_unit_price: Some(1_000_u32.into()),
+        max_gas: Some(Felt::from(1_000_000_u32).try_into().unwrap()),
+        max_gas_unit_price: Some(Felt::from(1_000_u32).try_into().unwrap()),
     };
 
     let settings = args
@@ -247,9 +247,9 @@ async fn test_strk_fee_get_max_fee_with_max_gas_unit_price() {
 
     let args = FeeArgs {
         fee_token: Some(FeeToken::Strk),
-        max_fee: Some(MAX_FEE.into()),
+        max_fee: Some(Felt::from(MAX_FEE).try_into().unwrap()),
         max_gas: None,
-        max_gas_unit_price: Some(1_000_u32.into()),
+        max_gas_unit_price: Some(Felt::from(1_000_u32).try_into().unwrap()),
     };
 
     let settings = args

docs/src/appendix/sncast/account/deploy.md

@@ -16,7 +16,7 @@ Overrides url from `snfoundry.toml`.
 ## `--max-fee, -m <MAX_FEE>`
 Optional.
 
-Maximum fee for the `deploy_account` transaction in Fri or Wei depending on fee token or transaction version. When not used, defaults to auto-estimation.
+Maximum fee for the `deploy_account` transaction in Fri or Wei depending on fee token or transaction version. When not used, defaults to auto-estimation. Must be greater than zero.
 
 ## `--fee-token <FEE_TOKEN>`
 Optional. When not used, defaults to STRK.
@@ -26,12 +26,12 @@ Token used for fee payment. Possible values: ETH, STRK.
 ## `--max-gas <MAX_GAS>`
 Optional.
 
-Maximum gas for the `deploy_account` transaction. When not used, defaults to auto-estimation. (Only for STRK fee payment)
+Maximum gas for the `deploy_account` transaction. When not used, defaults to auto-estimation. Must be greater than zero. (Only for STRK fee payment)
 
 ## ` --max-gas-unit-price <MAX_GAS_UNIT_PRICE>`
 Optional.
 
-Maximum gas unit price for the `deploy_account` transaction paid in Fri. When not used, defaults to auto-estimation. (Only for STRK fee payment)
+Maximum gas unit price for the `deploy_account` transaction paid in Fri. When not used, defaults to auto-estimation. Must be greater than zero. (Only for STRK fee payment)
 
 ## `--version, -v <VERSION>`
 Optional. When not used, defaults to v3.

docs/src/appendix/sncast/declare.md

@@ -20,7 +20,7 @@ Overrides url from `snfoundry.toml`.
 ## `--max-fee, -m <MAX_FEE>`
 Optional.
 
-Maximum fee for the `declare` transaction in Fri or Wei depending on fee token or transaction version. When not used, defaults to auto-estimation.
+Maximum fee for the `declare` transaction in Fri or Wei depending on fee token or transaction version. When not used, defaults to auto-estimation. Must be greater than zero.
 
 ## `--fee-token <FEE_TOKEN>`
 Optional. When not used, defaults to STRK.
@@ -30,12 +30,12 @@ Token used for fee payment. Possible values: ETH, STRK.
 ## `--max-gas <MAX_GAS>`
 Optional.
 
-Maximum gas for the `declare` transaction. When not used, defaults to auto-estimation. (Only for STRK fee payment)
+Maximum gas for the `declare` transaction. When not used, defaults to auto-estimation. Must be greater than zero. (Only for STRK fee payment)
 
 ## ` --max-gas-unit-price <MAX_GAS_UNIT_PRICE>`
 Optional.
 
-Maximum gas unit price for the `declare` transaction paid in Fri. When not used, defaults to auto-estimation. (Only for STRK fee payment)
+Maximum gas unit price for the `declare` transaction paid in Fri. When not used, defaults to auto-estimation. Must be greater than zero. (Only for STRK fee payment)
 
 ## `--version, -v <VERSION>`
 Optional. When not used, defaults to v3.