Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Print max fee conversion info when using v3 txs #2797

Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
57 commits
Select commit Hold shift + click to select a range
e843e47
Validate max gas is greater than zero for in for strk fee settings
franciszekjob Dec 18, 2024
196ac53
Print max fee conversion info when using v3 txs
franciszekjob Dec 18, 2024
beb7e67
Fix typo
franciszekjob Dec 18, 2024
52fa843
Adjust test
franciszekjob Dec 18, 2024
eacff90
Update `test_happy_case_strk`
franciszekjob Dec 18, 2024
ef43fae
Fix tests
franciszekjob Dec 18, 2024
0a2f5bb
Fix tests
franciszekjob Dec 18, 2024
ea50757
Merge branch 'master' into franciszekjob/2706-validate-fee-args-great…
franciszekjob Dec 18, 2024
8dce237
Check if max gas unit price > 0 when it's passed with max fee
franciszekjob Dec 18, 2024
72f2fe8
Merge branch 'franciszekjob/2706-validate-fee-args-greater-than-zero'…
franciszekjob Dec 18, 2024
6dbe787
Merge branch 'franciszekjob/2706-validate-fee-args-greater-than-zero'…
franciszekjob Dec 18, 2024
cb3d529
Refactor checking if feee args are greater than zero
franciszekjob Dec 18, 2024
a411a4e
Update error messages
franciszekjob Dec 18, 2024
dbbcf3f
Merge branch 'franciszekjob/2706-validate-fee-args-greater-than-zero'…
franciszekjob Dec 18, 2024
397a8ba
Update docs and changelog
franciszekjob Dec 19, 2024
34770c3
Merge branch 'franciszekjob/2706-validate-fee-args-greater-than-zero'…
franciszekjob Dec 19, 2024
77f4d4d
Update changelog
franciszekjob Dec 19, 2024
e5c22d2
Refactor checking if calculated value is zero
franciszekjob Dec 19, 2024
b2963a7
Merge branch 'franciszekjob/2706-validate-fee-args-greater-than-zero'…
franciszekjob Dec 19, 2024
969e37b
Cover case when max fee and max gas unit price are passed
franciszekjob Dec 19, 2024
a6ed7db
Merge branch 'franciszekjob/2706-validate-fee-args-greater-than-zero'…
franciszekjob Dec 19, 2024
436e51f
Display converted values in `print_max_fee_conversion_info`
franciszekjob Dec 19, 2024
ee9b354
Add newline ad the end of fee conversion message
franciszekjob Dec 19, 2024
4788a9b
Refactor `ScriptFeeSettings` and `FeeSettings`
franciszekjob Dec 19, 2024
e23e54e
Merge branch 'master' into franciszekjob/2706-validate-fee-args-great…
franciszekjob Dec 19, 2024
2a765a4
Merge branch 'franciszekjob/2706-validate-fee-args-greater-than-zero'…
franciszekjob Dec 19, 2024
80916bf
Fix tests
franciszekjob Dec 20, 2024
31168e4
Format
franciszekjob Dec 20, 2024
69af0af
Merge branch 'franciszekjob/2706-validate-fee-args-greater-than-zero'…
franciszekjob Dec 20, 2024
7b7337e
Rename `zero` to `0`
franciszekjob Dec 20, 2024
2db18e6
Update changelog
franciszekjob Dec 20, 2024
41e4cd5
Add `impl_deserialize_for_nonzero_num_type` macro
franciszekjob Dec 20, 2024
73e085f
Apply other code review suggestions
franciszekjob Dec 20, 2024
84d08ba
Merge branch 'franciszekjob/2706-validate-fee-args-greater-than-zero'…
franciszekjob Dec 20, 2024
69ed7c8
Fix linting
franciszekjob Jan 6, 2025
7d4bbed
Update changelog
franciszekjob Jan 7, 2025
0779442
Add conversions tests
franciszekjob Jan 7, 2025
45cf019
Merge branch 'master' of https://github.com/foundry-rs/starknet-found…
franciszekjob Jan 7, 2025
0c80350
Update changelog - remove duplicated entry
franciszekjob Jan 7, 2025
0e185b6
Fix headings in changelog
franciszekjob Jan 7, 2025
a784e69
Refactor `FeeArgs.try_into_fee_settings()`
franciszekjob Jan 7, 2025
091e484
Update `unreachable!` messages in `FeeArgs.try_into_fee_settings()`
franciszekjob Jan 7, 2025
9bb9f1a
Merge branch 'franciszekjob/2706-validate-fee-args-greater-than-zero'…
franciszekjob Jan 7, 2025
b32e7a4
Update crates/sncast/src/helpers/fee.rs
franciszekjob Jan 7, 2025
dd12975
Merge branch 'franciszekjob/2707-show-info-about-converted-max-fee-wi…
franciszekjob Jan 7, 2025
f161eff
Fix linting
franciszekjob Jan 10, 2025
7f1fbbc
Merge branch 'master' of https://github.com/foundry-rs/starknet-found…
franciszekjob Jan 10, 2025
9682eab
Use `Felt::TWO.pow`
franciszekjob Jan 10, 2025
a7c0acf
Remove use of `to_bigint()` in `impl_deserialize_for_num_type`
franciszekjob Jan 10, 2025
2eb48ae
Merge branch 'master' of https://github.com/foundry-rs/starknet-found…
franciszekjob Jan 10, 2025
56e3122
Merge branch 'master' into franciszekjob/2706-validate-fee-args-great…
franciszekjob Jan 14, 2025
5831be3
Merge branch 'master' into franciszekjob/2706-validate-fee-args-great…
franciszekjob Jan 14, 2025
e200723
Add todo
franciszekjob Jan 14, 2025
d8b0c51
Merge branch 'franciszekjob/2706-validate-fee-args-greater-than-zero'…
franciszekjob Jan 14, 2025
4d7229f
Add todo
franciszekjob Jan 14, 2025
756b5c7
Merge branch 'franciszekjob/2706-validate-fee-args-greater-than-zero'…
franciszekjob Jan 14, 2025
df06e53
Merge branch 'master' of https://github.com/foundry-rs/starknet-found…
franciszekjob Jan 14, 2025
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
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Cast

### Changed

- Values passed with `--max-fee`, `--max-gas` and `--max-gas-unit-price` flags are required to be > 0

## [0.35.1] - 2024-12-16

### Forge
Expand Down
84 changes: 54 additions & 30 deletions crates/sncast/src/helpers/fee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand All @@ -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()),
},
}
}
Expand Down Expand Up @@ -75,10 +79,13 @@ 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 => {
if self.max_fee.is_some() {
ksew1 marked this conversation as resolved.
Show resolved Hide resolved
print_max_fee_conversion_info();
}
let settings = match (self.max_fee, self.max_gas, self.max_gas_unit_price) {
(Some(_), Some(_), Some(_)) => {
bail!("Passing all --max-fee, --max-gas and --max-gas-unit-price is conflicting. Please pass only two of them or less")
Expand All @@ -92,43 +99,50 @@ 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 = Felt::from(max_fee).floor_div(&max_gas);
if max_gas_unit_price == Felt::ZERO {
bail!("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(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 = Felt::from(max_fee)
.floor_div(&NonZeroFelt::try_from(max_gas_unit_price)?);
if max_gas == Felt::ZERO {
bail!("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(max_gas.try_into_()?),
max_gas_unit_price: Some(max_gas_unit_price.try_into_()?),
}
}
Expand Down Expand Up @@ -261,3 +275,13 @@ fn parse_fee_token(s: &str) -> Result<FeeToken, String> {

Ok(parsed_token)
}

fn print_max_fee_conversion_info() {
println!("Specifying '--max-fee' flag while using v3 transactions results in conversion to '--max-gas' and '--max-gas-unit-price' flags\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())
}
6 changes: 6 additions & 0 deletions crates/sncast/tests/e2e/account/deploy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,8 @@ pub async fn test_valid_class_hash() {
let snapbox = runner(&args).current_dir(tempdir.path());

snapbox.assert().success().stdout_matches(indoc! {r"
Specifying '--max-fee' flag while using v3 transactions results in conversion to '--max-gas' and '--max-gas-unit-price' flags

command: account deploy
transaction_hash: [..]

Expand Down Expand Up @@ -584,6 +586,8 @@ pub async fn test_happy_case_keystore(account_type: &str) {
let snapbox = runner(&args).current_dir(tempdir.path());

snapbox.assert().stdout_matches(indoc! {r"
Specifying '--max-fee' flag while using v3 transactions results in conversion to '--max-gas' and '--max-gas-unit-price' flags

command: account deploy
transaction_hash: 0x0[..]

Expand Down Expand Up @@ -857,6 +861,8 @@ pub async fn test_deploy_keystore_other_args() {

let snapbox = runner(&args).current_dir(tempdir.path());
snapbox.assert().stdout_matches(indoc! {r"
Specifying '--max-fee' flag while using v3 transactions results in conversion to '--max-gas' and '--max-gas-unit-price' flags

command: account deploy
transaction_hash: 0x0[..]

Expand Down
76 changes: 74 additions & 2 deletions crates/sncast/tests/e2e/invoke.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,11 +128,13 @@ async fn test_happy_case_strk(class_hash: Felt, account_type: AccountType) {
];

let snapbox = runner(&args).current_dir(tempdir.path());
let output = snapbox.assert().success().get_output().stdout.clone();
let output = snapbox.assert().success();
let stdout = output.get_output().stdout.clone();

let hash = get_transaction_hash(&output);
let hash = get_transaction_hash(&stdout);
let receipt = get_transaction_receipt(hash).await;

assert_stdout_contains(output, "Specifying '--max-fee' flag while using v3 transactions results in conversion to '--max-gas' and '--max-gas-unit-price' flags");
assert!(matches!(receipt, Invoke(_)));
}

Expand Down Expand Up @@ -388,6 +390,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;
Expand Down
Loading
Loading