Skip to content

Commit

Permalink
Validate max gas is greater than zero for strk fee settings (#2796)
Browse files Browse the repository at this point in the history
<!-- Reference any GitHub issues resolved by this PR -->

Closes #2706

## Introduced changes

<!-- A brief description of the changes -->

Validate if fee args are greater than 0
- Change type of `max_fee`, `max_gas` and `max_gas_unit_price` from
`Option<Felt>` to `Option<NonZeroFelt>`

## Checklist

<!-- Make sure all of these are complete -->

- [x] Linked relevant issue
- [x] Updated relevant documentation
- [x] Added relevant tests
- [x] Performed self-review of the code
- [x] Added changes to `CHANGELOG.md`
  • Loading branch information
franciszekjob authored Jan 14, 2025
1 parent 8248052 commit a530f45
Showing 22 changed files with 433 additions and 136 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -17,8 +17,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

#### 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
- interactive interface that allows setting created or imported account as the default

#### 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
3 changes: 3 additions & 0 deletions crates/conversions/src/lib.rs
Original file line number Diff line number Diff line change
@@ -6,6 +6,9 @@ pub mod contract_address;
pub mod entrypoint_selector;
pub mod eth_address;
pub mod felt;
pub mod non_zero_felt;
pub mod non_zero_u128;
pub mod non_zero_u64;
pub mod nonce;
pub mod padded_felt;
pub mod primitive;
23 changes: 23 additions & 0 deletions crates/conversions/src/non_zero_felt.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
use crate::FromConv;
use starknet_types_core::felt::{Felt, NonZeroFelt};
use std::num::{NonZeroU128, NonZeroU64};

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"
)
})
}
}
14 changes: 14 additions & 0 deletions crates/conversions/src/non_zero_u128.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
use crate::TryFromConv;
use starknet_types_core::felt::{Felt, NonZeroFelt};
use std::num::{NonZero, NonZeroU128};

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")))
}
}
14 changes: 14 additions & 0 deletions crates/conversions/src/non_zero_u64.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
use crate::TryFromConv;
use starknet_types_core::felt::{Felt, NonZeroFelt};
use std::num::{NonZero, NonZeroU64};

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")))
}
}
38 changes: 26 additions & 12 deletions crates/conversions/src/serde/deserialize/deserialize_impl.rs
Original file line number Diff line number Diff line change
@@ -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 {
@@ -80,15 +92,13 @@ macro_rules! impl_deserialize_for_felt_type {
}
};
}

macro_rules! impl_deserialize_for_num_type {
($type:ty) => {
impl CairoDeserialize for $type {
fn deserialize(reader: &mut BufferReader<'_>) -> BufferReadResult<Self> {
let felt = Felt::deserialize(reader)?;

felt.to_bigint()
.try_into()
.map_err(|_| BufferReadError::ParseFailed)
felt.try_into().map_err(|_| BufferReadError::ParseFailed)
}
}
};
@@ -99,6 +109,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);
3 changes: 3 additions & 0 deletions crates/conversions/tests/e2e/mod.rs
Original file line number Diff line number Diff line change
@@ -3,6 +3,9 @@ mod contract_address;
mod entrypoint_selector;
mod felt;
mod field_elements;
mod non_zero_felt;
mod non_zero_u128;
mod non_zero_u64;
mod nonce;
mod padded_felt;
mod string;
21 changes: 21 additions & 0 deletions crates/conversions/tests/e2e/non_zero_felt.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
#[cfg(test)]
mod tests_non_zero_felt {
use std::num::{NonZeroU128, NonZeroU64};

use conversions::FromConv;
use starknet_types_core::felt::{Felt, NonZeroFelt};

#[test]
fn test_happy_case() {
let non_zero_felt = NonZeroFelt::try_from(Felt::from(1_u8)).unwrap();

assert_eq!(
non_zero_felt,
NonZeroFelt::from_(NonZeroU64::new(1).unwrap())
);
assert_eq!(
non_zero_felt,
NonZeroFelt::from_(NonZeroU128::new(1).unwrap())
);
}
}
36 changes: 36 additions & 0 deletions crates/conversions/tests/e2e/non_zero_u128.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
#[cfg(test)]
mod tests_non_zero_u128 {
use conversions::TryFromConv;
use starknet_types_core::felt::{Felt, NonZeroFelt};
use std::num::NonZeroU128;

#[test]
fn test_happy_case() {
let non_zero_u128 = NonZeroU128::new(1).unwrap();

assert_eq!(
non_zero_u128,
NonZeroU128::try_from_(NonZeroFelt::try_from(Felt::from(1_u8)).unwrap()).unwrap()
);
}

#[test]
fn test_limit() {
let felt = Felt::from_dec_str(&u128::MAX.to_string()).unwrap();
let non_zero_felt = NonZeroFelt::try_from(felt).unwrap();

let result = NonZeroU128::try_from_(non_zero_felt);
assert!(result.is_ok());
assert_eq!(result.unwrap().get(), u128::MAX);
}

#[test]
fn test_felt_too_large() {
let large_felt = Felt::TWO.pow(128_u8);
let non_zero_felt = NonZeroFelt::try_from(large_felt).unwrap();

let result = NonZeroU128::try_from_(non_zero_felt);
assert!(result.is_err());
assert_eq!(result.unwrap_err(), "felt was too large to fit in u128");
}
}
36 changes: 36 additions & 0 deletions crates/conversions/tests/e2e/non_zero_u64.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
#[cfg(test)]
mod tests_non_zero_u64 {
use conversions::TryFromConv;
use starknet_types_core::felt::{Felt, NonZeroFelt};
use std::num::NonZeroU64;

#[test]
fn test_happy_case() {
let non_zero_u64 = NonZeroU64::new(1).unwrap();

assert_eq!(
non_zero_u64,
NonZeroU64::try_from_(NonZeroFelt::try_from(Felt::from(1_u8)).unwrap()).unwrap()
);
}

#[test]
fn test_limit() {
let felt = Felt::from_dec_str(&u64::MAX.to_string()).unwrap();
let non_zero_felt = NonZeroFelt::try_from(felt).unwrap();

let result = NonZeroU64::try_from_(non_zero_felt);
assert!(result.is_ok());
assert_eq!(result.unwrap().get(), u64::MAX);
}

#[test]
fn test_felt_too_large() {
let large_felt = Felt::TWO.pow(64_u8);
let non_zero_felt = NonZeroFelt::try_from(large_felt).unwrap();

let result = NonZeroU64::try_from_(non_zero_felt);
assert!(result.is_err());
assert_eq!(result.unwrap_err(), "felt was too large to fit in u64");
}
}
Loading

0 comments on commit a530f45

Please sign in to comment.