From 7ef26ac2a371e9f024dca62bdd69e93a873b9b88 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Fri, 8 Sep 2023 17:28:40 +0200 Subject: [PATCH 1/3] Adds fee-related errors to sdk --- shared/src/types/error.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/shared/src/types/error.rs b/shared/src/types/error.rs index 20d278ddea..5d0f3cbd22 100644 --- a/shared/src/types/error.rs +++ b/shared/src/types/error.rs @@ -189,6 +189,12 @@ pub enum TxError { to be transferred. Amount to transfer is {2} and the balance is {3}." )] BalanceTooLow(Address, Address, String, String), + /// Balance is too low for fee payment + #[error( + "The balance of the source {0} of token {1} is lower than the amount \ + required for fees. Amount of the fees is {2} and the balance is {3}." + )] + BalanceTooLowForFees(Address, Address, String, String), /// Token Address does not exist on chain #[error("The token address {0} doesn't exist on chain.")] TokenDoesNotExist(Address), @@ -213,6 +219,9 @@ pub enum TxError { /// No Balance found for token #[error("{0}")] MaspError(String), + /// Error in the fee unshielding transaction + #[error("Error in fee unshielding: {0}")] + FeeUnshieldingError(String), /// Wasm validation failed #[error("Validity predicate code validation failed with {0}")] WasmValidationFailure(WasmValidationError), From 80c6c60ef27db4db090aac840a8c62158363bcf5 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Fri, 8 Sep 2023 17:42:04 +0200 Subject: [PATCH 2/3] Removes fee panics from sdk --- shared/src/ledger/signing.rs | 119 ++++++++++++++++++++--------------- shared/src/ledger/tx.rs | 4 +- 2 files changed, 71 insertions(+), 52 deletions(-) diff --git a/shared/src/ledger/signing.rs b/shared/src/ledger/signing.rs index 1753c645d3..19257aa30b 100644 --- a/shared/src/ledger/signing.rs +++ b/shared/src/ledger/signing.rs @@ -277,10 +277,11 @@ pub async fn aux_signing_data< }; if fee_payer == masp_tx_key().to_public() { - panic!( + other_err( "The gas payer cannot be the MASP, please provide a different gas \ payer." - ); + .to_string(), + )?; } Ok(SigningTxData { @@ -301,18 +302,18 @@ pub async fn solve_pow_challenge( total_fee: Amount, balance: Amount, source: Address, -) -> Option { +) -> Result, Error> { let is_bal_sufficient = total_fee <= balance; if !is_bal_sufficient { let token_addr = args.fee_token.clone(); - let err_msg = format!( - "The wrapper transaction source doesn't have enough balance to \ - pay fee {}, got {}.", - format_denominated_amount(client, &token_addr, total_fee).await, - format_denominated_amount(client, &token_addr, balance).await, - ); if !args.force && cfg!(feature = "mainnet") { - panic!("{}", err_msg); + let fee_amount = + format_denominated_amount(client, &token_addr, total_fee).await; + let balance = + format_denominated_amount(client, &token_addr, balance).await; + return Err(Error::from(TxError::BalanceTooLow( + source, token_addr, fee_amount, balance, + ))); } } // A PoW solution can be used to allow zero-fee testnet transactions @@ -325,9 +326,9 @@ pub async fn solve_pow_challenge( // Solve the solution, this blocks until a solution is found let solution = challenge.solve(); - Some(solution) + Ok(Some(solution)) } else { - None + Ok(None) } } @@ -339,7 +340,7 @@ pub async fn update_pow_challenge( tx: &mut Tx, requires_pow: bool, source: Address, -) { +) -> Result<(), Error> { let gas_cost_key = parameter_storage::get_gas_cost_key(); let minimum_fee = match rpc::query_storage_value::< C, @@ -349,16 +350,17 @@ pub async fn update_pow_challenge( .and_then(|map| { map.get(&args.fee_token) .map(ToOwned::to_owned) - .ok_or_else(|| Error::Other("no fee found".to_string())) + .ok_or_else(|| { + Error::Other(format!( + "Could not retrieve from storage the gas cost for token {}", + args.fee_token + )) + }) }) { Ok(amount) => amount, - Err(_e) => { - eprintln!( - "Could not retrieve the gas cost for token {}", - args.fee_token - ); + Err(e) => { if !args.force { - panic!(); + return Err(e); } else { token::Amount::default() } @@ -408,13 +410,15 @@ pub async fn update_pow_challenge( balance, source, ) - .await; + .await?; wrapper.fee = Fee { amount_per_gas_unit: fee_amount, token: args.fee_token.clone(), }; wrapper.pow_solution = pow_solution; } + + Ok(()) } /// Informations about the post-tx balance of the tx's source. Used to correctly @@ -444,7 +448,7 @@ pub async fn wrap_tx< epoch: Epoch, fee_payer: common::PublicKey, #[cfg(not(feature = "mainnet"))] requires_pow: bool, -) -> Option { +) -> Result, Error> { let fee_payer_address = Address::from(&fee_payer); // Validate fee amount and token let gas_cost_key = parameter_storage::get_gas_cost_key(); @@ -456,16 +460,17 @@ pub async fn wrap_tx< .and_then(|map| { map.get(&args.fee_token) .map(ToOwned::to_owned) - .ok_or_else(|| Error::Other("no fee found".to_string())) + .ok_or_else(|| { + Error::Other(format!( + "Could not retrieve from storage the gas cost for token {}", + args.fee_token + )) + }) }) { Ok(amount) => amount, - Err(_e) => { - eprintln!( - "Could not retrieve the gas cost for token {}", - args.fee_token - ); + Err(e) => { if !args.force { - panic!(); + return Err(e); } else { token::Amount::default() } @@ -587,26 +592,35 @@ pub async fn wrap_tx< && !args.force && cfg!(feature = "mainnet") { - panic!( - "Fee unshielding descriptions exceed the limit" - ); + return Err(Error::from( + TxError::FeeUnshieldingError(format!( + "Descriptions exceed the limit: found \ + {descriptions}, limit \ + {descriptions_limit}" + )), + )); } updated_balance += total_fee; (Some(transaction), Some(unshielding_epoch)) } Ok(None) => { - eprintln!("Missing unshielding transaction"); if !args.force && cfg!(feature = "mainnet") { - panic!(); + return Err(Error::from( + TxError::FeeUnshieldingError( + "Missing unshielding transaction" + .to_string(), + ), + )); } (None, None) } Err(e) => { - eprintln!("Error in fee unshielding generation: {}", e); if !args.force && cfg!(feature = "mainnet") { - panic!(); + return Err(Error::from( + TxError::FeeUnshieldingError(e.to_string()), + )); } (None, None) @@ -614,21 +628,26 @@ pub async fn wrap_tx< } } else { let token_addr = args.fee_token.clone(); - let err_msg = format!( - "The wrapper transaction source doesn't have enough \ - balance to pay fee {}, balance: {}.", - format_denominated_amount(client, &token_addr, total_fee) - .await, - format_denominated_amount( + if !args.force && cfg!(feature = "mainnet") { + let fee_amount = format_denominated_amount( client, &token_addr, - updated_balance + total_fee, ) - .await, - ); - eprintln!("{}", err_msg); - if !args.force && cfg!(feature = "mainnet") { - panic!("{}", err_msg); + .await; + + let balance = format_denominated_amount( + client, + &token_addr, + updated_balance, + ) + .await; + return Err(Error::from(TxError::BalanceTooLowForFees( + fee_payer_address, + token_addr, + fee_amount, + balance, + ))); } (None, None) @@ -662,7 +681,7 @@ pub async fn wrap_tx< updated_balance, fee_payer_address, ) - .await; + .await?; tx.add_wrapper( Fee { @@ -678,7 +697,7 @@ pub async fn wrap_tx< unshield_section_hash, ); - unshielding_epoch + Ok(unshielding_epoch) } #[allow(clippy::result_large_err)] diff --git a/shared/src/ledger/tx.rs b/shared/src/ledger/tx.rs index 5ebc7f52bb..1177e8df17 100644 --- a/shared/src/ledger/tx.rs +++ b/shared/src/ledger/tx.rs @@ -163,7 +163,7 @@ pub async fn prepare_tx< if !args.dry_run { let epoch = rpc::query_epoch(client).await?; - Ok(signing::wrap_tx( + signing::wrap_tx( client, shielded, tx, @@ -174,7 +174,7 @@ pub async fn prepare_tx< #[cfg(not(feature = "mainnet"))] requires_pow, ) - .await) + .await } else { Ok(None) } From 63467f7757180df7fcc50295184753a2bcf872c2 Mon Sep 17 00:00:00 2001 From: Marco Granelli Date: Fri, 8 Sep 2023 17:44:30 +0200 Subject: [PATCH 3/3] changelog: add #1878 --- .changelog/unreleased/SDK/1878-remove-sdk-fee-panics.md | 3 +++ .changelog/unreleased/bug-fixes/1878-remove-sdk-fee-panics.md | 2 ++ 2 files changed, 5 insertions(+) create mode 100644 .changelog/unreleased/SDK/1878-remove-sdk-fee-panics.md create mode 100644 .changelog/unreleased/bug-fixes/1878-remove-sdk-fee-panics.md diff --git a/.changelog/unreleased/SDK/1878-remove-sdk-fee-panics.md b/.changelog/unreleased/SDK/1878-remove-sdk-fee-panics.md new file mode 100644 index 0000000000..297572536c --- /dev/null +++ b/.changelog/unreleased/SDK/1878-remove-sdk-fee-panics.md @@ -0,0 +1,3 @@ +- Added two new variants to the `TxError` enum, `BalanceToLowForFees` and + `FeeUnshieldingError`, representing possible failures in transactions' fees. + ([\#1878](https://github.com/anoma/namada/pull/1878)) \ No newline at end of file diff --git a/.changelog/unreleased/bug-fixes/1878-remove-sdk-fee-panics.md b/.changelog/unreleased/bug-fixes/1878-remove-sdk-fee-panics.md new file mode 100644 index 0000000000..47eacd8dba --- /dev/null +++ b/.changelog/unreleased/bug-fixes/1878-remove-sdk-fee-panics.md @@ -0,0 +1,2 @@ +- Removed gas and fees related panics from the sdk. + ([\#1878](https://github.com/anoma/namada/pull/1878)) \ No newline at end of file