Print max fee conversion info when using v3 txs

CHANGELOG.md

@@ -7,6 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Cast
+
+### Added
+
+- When using `--max-fee` with transactions v3, calculated max gas and max gas unit price are automatically validated to ensure they are greater than 0 after conversion
+
+### 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/conversions/src/felt.rs

@@ -2,12 +2,15 @@ use crate::{
     byte_array::ByteArray,
     serde::serialize::SerializeToFeltVec,
     string::{TryFromDecStr, TryFromHexStr},
-    FromConv, IntoConv,
+    FromConv, IntoConv, TryFromConv,
 };
 use conversions::padded_felt::PaddedFelt;
 use starknet_api::core::{ClassHash, ContractAddress, EntryPointSelector, Nonce};
-use starknet_types_core::felt::{Felt, FromStrError};
-use std::vec;
+use starknet_types_core::felt::{Felt, FromStrError, NonZeroFelt};
+use std::{
+    num::{NonZero, NonZeroU128, NonZeroU64},
+    vec,
+};
 
 impl FromConv<ClassHash> for Felt {
     fn from_(value: ClassHash) -> Felt {
@@ -39,6 +42,48 @@ impl FromConv<PaddedFelt> for Felt {
     }
 }
 
+impl TryFromConv<NonZeroFelt> for NonZeroU64 {
+    type Error = String;
+    fn try_from_(value: NonZeroFelt) -> Result<Self, Self::Error> {
+        let value: u64 = Felt::from(value)
+            .try_into()
+            .map_err(|_| "felt was too large to fit in u64")?;
+        Ok(NonZero::new(value)
+            .unwrap_or_else(|| unreachable!("non zero felt is always greater than 0")))
+    }
+}
+
+impl TryFromConv<NonZeroFelt> for NonZeroU128 {
+    type Error = String;
+    fn try_from_(value: NonZeroFelt) -> Result<Self, Self::Error> {
+        let value: u128 = Felt::from(value)
+            .try_into()
+            .map_err(|_| "felt was too large to fit in u128")?;
+        Ok(NonZero::new(value)
+            .unwrap_or_else(|| unreachable!("non zero felt is always greater than 0")))
+    }
+}
+
+impl FromConv<NonZeroU64> for NonZeroFelt {
+    fn from_(value: NonZeroU64) -> Self {
+        NonZeroFelt::try_from(Felt::from(value.get())).unwrap_or_else(|_| {
+            unreachable!(
+                "NonZeroU64 is always greater than 0, so it should be convertible to NonZeroFelt"
+            )
+        })
+    }
+}
+
+impl FromConv<NonZeroU128> for NonZeroFelt {
+    fn from_(value: NonZeroU128) -> Self {
+        NonZeroFelt::try_from(Felt::from(value.get())).unwrap_or_else(|_| {
+            unreachable!(
+                "NonZeroU128 is always greater than 0, so it should be convertible to NonZeroFelt"
+            )
+        })
+    }
+}
+
 impl<T> TryFromDecStr for T
 where
     T: FromConv<Felt>,

crates/conversions/src/serde/deserialize/deserialize_impl.rs

@@ -3,8 +3,8 @@ use crate::{byte_array::ByteArray, IntoConv};
 use num_traits::cast::ToPrimitive;
 use starknet::providers::Url;
 use starknet_api::core::{ClassHash, ContractAddress, EntryPointSelector, Nonce};
-use starknet_types_core::felt::Felt;
-use std::num::NonZeroU32;
+use starknet_types_core::felt::{Felt, NonZeroFelt};
+use std::num::NonZero;
 
 impl CairoDeserialize for Url {
     fn deserialize(reader: &mut BufferReader<'_>) -> BufferReadResult<Self> {
@@ -13,12 +13,6 @@ impl CairoDeserialize for Url {
     }
 }
 
-impl CairoDeserialize for NonZeroU32 {
-    fn deserialize(reader: &mut BufferReader<'_>) -> BufferReadResult<Self> {
-        NonZeroU32::new(reader.read()?).ok_or(BufferReadError::ParseFailed)
-    }
-}
-
 impl CairoDeserialize for Felt {
     fn deserialize(reader: &mut BufferReader<'_>) -> BufferReadResult<Self> {
         reader.read_felt()
@@ -71,6 +65,24 @@ impl CairoDeserialize for bool {
     }
 }
 
+impl CairoDeserialize for NonZeroFelt {
+    fn deserialize(reader: &mut BufferReader<'_>) -> BufferReadResult<Self> {
+        let felt = reader.read::<Felt>()?;
+        NonZeroFelt::try_from(felt).map_err(|_| BufferReadError::ParseFailed)
+    }
+}
+
+macro_rules! impl_deserialize_for_nonzero_num_type {
+    ($type:ty) => {
+        impl CairoDeserialize for NonZero<$type> {
+            fn deserialize(reader: &mut BufferReader<'_>) -> BufferReadResult<Self> {
+                let val = <$type>::deserialize(reader)?;
+                NonZero::new(val).ok_or(BufferReadError::ParseFailed)
+            }
+        }
+    };
+}
+
 macro_rules! impl_deserialize_for_felt_type {
     ($type:ty) => {
         impl CairoDeserialize for $type {
@@ -99,6 +111,10 @@ impl_deserialize_for_felt_type!(ContractAddress);
 impl_deserialize_for_felt_type!(Nonce);
 impl_deserialize_for_felt_type!(EntryPointSelector);
 
+impl_deserialize_for_nonzero_num_type!(u32);
+impl_deserialize_for_nonzero_num_type!(u64);
+impl_deserialize_for_nonzero_num_type!(u128);
+
 impl_deserialize_for_num_type!(u8);
 impl_deserialize_for_num_type!(u16);
 impl_deserialize_for_num_type!(u32);

crates/sncast/src/helpers/fee.rs

@@ -1,12 +1,14 @@
-use anyhow::{bail, ensure, Error, Result};
+use anyhow::{bail, ensure, Context, Error, Result};
 use clap::{Args, ValueEnum};
-use conversions::serde::deserialize::CairoDeserialize;
-use conversions::TryIntoConv;
+use conversions::{serde::deserialize::CairoDeserialize, FromConv, TryFromConv};
 use shared::print::print_as_warning;
 use starknet::core::types::BlockId;
 use starknet::providers::Provider;
 use starknet_types_core::felt::{Felt, NonZeroFelt};
-use std::str::FromStr;
+use std::{
+    num::{NonZeroU128, NonZeroU64},
+    str::FromStr,
+};
 
 #[derive(Args, Debug, Clone)]
 pub struct FeeArgs {
@@ -15,16 +17,16 @@ 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 {
@@ -43,8 +45,8 @@ impl From<ScriptFeeSettings> for FeeArgs {
             } => 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_gas: max_gas.map(NonZeroFelt::from_),
+                max_gas_unit_price: max_gas_unit_price.map(NonZeroFelt::from_),
             },
         }
     }
@@ -59,6 +61,7 @@ impl FeeArgs {
         }
     }
 
+    #[allow(clippy::too_many_lines)]
     pub async fn try_into_fee_settings<P: Provider>(
         &self,
         provider: P,
@@ -92,44 +95,74 @@ 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(NonZeroU64::try_from_)
+                            .transpose()
+                            .map_err(anyhow::Error::msg)?,
                         max_gas_unit_price: self
                             .max_gas_unit_price
-                            .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))
-                                .try_into_()?,
-                        ),
+                            .map(NonZeroU128::try_from_)
+                            .transpose()
+                            .map_err(anyhow::Error::msg)?,
                     },
+                    (Some(max_fee), None, Some(max_gas_unit_price)) => {
+                        let max_gas = NonZeroFelt::try_from(Felt::from(max_fee).floor_div(&max_gas_unit_price)).context("Calculated max gas from provided --max-fee and --max-gas-unit-price is 0. Please increase --max-fee to obtain a positive gas amount")?;
+                        print_max_fee_conversion_info(
+                            max_fee.into(),
+                            max_gas.into(),
+                            max_gas_unit_price.into(),
+                        );
+                        FeeSettings::Strk {
+                            max_gas: Some(
+                                NonZeroU64::try_from_(max_gas).map_err(anyhow::Error::msg)?,
+                            ),
+                            max_gas_unit_price: Some(
+                                NonZeroU128::try_from_(max_gas_unit_price)
+                                    .map_err(anyhow::Error::msg)?,
+                            ),
+                        }
+                    }
+                    (Some(max_fee), Some(max_gas), None) => {
+                        let max_gas_unit_price = NonZeroFelt::try_from(Felt::from(max_fee).floor_div(&max_gas)).context("Calculated max gas unit price from provided --max-fee and --max-gas is 0. Please increase --max-fee or decrease --max-gas to ensure a positive gas unit price")?;
+                        print_max_fee_conversion_info(
+                            max_fee.into(),
+                            max_gas.into(),
+                            max_gas_unit_price.into(),
+                        );
+                        FeeSettings::Strk {
+                            max_gas: Some(
+                                NonZeroU64::try_from_(max_gas).map_err(anyhow::Error::msg)?,
+                            ),
+                            max_gas_unit_price: Some(
+                                NonZeroU128::try_from_(max_gas_unit_price)
+                                    .map_err(anyhow::Error::msg)?,
+                            ),
+                        }
+                    }
                     (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_unit_price = NonZeroFelt::try_from(
+                            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(&max_gas_unit_price)).context("Calculated max-gas from provided --max-fee and the current network gas price is 0. Please increase --max-fee to obtain a positive gas amount")?;
+                        print_max_fee_conversion_info(
+                            max_fee.into(),
+                            max_gas.into(),
+                            max_gas_unit_price.into(),
+                        );
                         FeeSettings::Strk {
                             max_gas: Some(
-                                max_fee
-                                    .floor_div(&NonZeroFelt::from_felt_unchecked(
-                                        max_gas_unit_price,
-                                    ))
-                                    .try_into_()?,
+                                NonZeroU64::try_from_(max_gas).map_err(anyhow::Error::msg)?,
+                            ),
+                            max_gas_unit_price: Some(
+                                NonZeroU128::try_from_(max_gas_unit_price)
+                                    .map_err(anyhow::Error::msg)?,
                             ),
-                            max_gas_unit_price: Some(max_gas_unit_price.try_into_()?),
                         }
                     }
                 };
@@ -152,23 +185,23 @@ pub enum FeeToken {
 #[derive(Debug, PartialEq, CairoDeserialize)]
 pub enum ScriptFeeSettings {
     Eth {
-        max_fee: Option<Felt>,
+        max_fee: Option<NonZeroFelt>,
     },
     Strk {
-        max_fee: Option<Felt>,
-        max_gas: Option<u64>,
-        max_gas_unit_price: Option<u128>,
+        max_fee: Option<NonZeroFelt>,
+        max_gas: Option<NonZeroU64>,
+        max_gas_unit_price: Option<NonZeroU128>,
     },
 }
 
 #[derive(Debug, PartialEq)]
 pub enum FeeSettings {
     Eth {
-        max_fee: Option<Felt>,
+        max_fee: Option<NonZeroFelt>,
     },
     Strk {
-        max_gas: Option<u64>,
-        max_gas_unit_price: Option<u128>,
+        max_gas: Option<NonZeroU64>,
+        max_gas_unit_price: Option<NonZeroU128>,
     },
 }
 
@@ -261,3 +294,15 @@ fn parse_fee_token(s: &str) -> Result<FeeToken, String> {
 
     Ok(parsed_token)
 }
+
+fn print_max_fee_conversion_info(max_fee: Felt, max_gas: Felt, max_gas_unit_price: Felt) {
+    println!(
+        "Specifying '--max-fee' flag while using v3 transactions results in conversion to '--max-gas' and '--max-gas-unit-price' flags\nConverted {max_fee} max fee to {max_gas} max gas and {max_gas_unit_price} max gas unit price\n",
+    );
+}
+
+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/src/starknet_commands/account/deploy.rs

@@ -263,18 +263,26 @@ where
     let result = match fee_settings {
         FeeSettings::Eth { max_fee } => {
             let deployment = account_factory.deploy_v1(salt);
-            let deployment = apply_optional(deployment, max_fee, AccountDeploymentV1::max_fee);
+            let deployment = apply_optional(
+                deployment,
+                max_fee.map(Felt::from),
+                AccountDeploymentV1::max_fee,
+            );
             deployment.send().await
         }
         FeeSettings::Strk {
             max_gas,
             max_gas_unit_price,
         } => {
             let deployment = account_factory.deploy_v3(salt);
-            let deployment = apply_optional(deployment, max_gas, AccountDeploymentV3::gas);
             let deployment = apply_optional(
                 deployment,
-                max_gas_unit_price,
+                max_gas.map(std::num::NonZero::get),
+                AccountDeploymentV3::gas,
+            );
+            let deployment = apply_optional(
+                deployment,
+                max_gas_unit_price.map(std::num::NonZero::get),
                 AccountDeploymentV3::gas_price,
             );
             deployment.send().await