From 860af97c5deb3cb9c2b12b64485b467726101cf0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9gory=20Demay?= Date: Wed, 10 Apr 2024 20:41:08 +0000 Subject: [PATCH] feat(ckerc20): ckERC20 withdrawals without reimbursement of transaction fees [override-didc-check] --- rs/ethereum/cketh/minter/cketh_minter.did | 6 +- .../cketh/minter/src/endpoints/ckerc20.rs | 5 +- rs/ethereum/cketh/minter/src/main.rs | 19 +- .../minter/src/state/transactions/mod.rs | 54 ++-- .../minter/src/state/transactions/tests.rs | 255 +++++++++--------- rs/ethereum/cketh/minter/src/tx.rs | 2 +- rs/ethereum/cketh/minter/tests/ckerc20.rs | 197 ++++++++++---- rs/ethereum/cketh/test_utils/src/ckerc20.rs | 56 +++- rs/ethereum/cketh/test_utils/src/flow.rs | 11 + rs/ethereum/cketh/test_utils/src/lib.rs | 2 + rs/ethereum/cketh/test_utils/src/mock.rs | 85 +++--- rs/ethereum/cketh/test_utils/src/response.rs | 12 +- rs/ethereum/cketh/test_utils/src/tests.rs | 25 +- rs/state_machine_tests/src/lib.rs | 4 +- 14 files changed, 463 insertions(+), 270 deletions(-) diff --git a/rs/ethereum/cketh/minter/cketh_minter.did b/rs/ethereum/cketh/minter/cketh_minter.did index 96d666c6ec6..b16bc4e454d 100644 --- a/rs/ethereum/cketh/minter/cketh_minter.did +++ b/rs/ethereum/cketh/minter/cketh_minter.did @@ -229,7 +229,7 @@ type WithdrawalError = variant { // Recipient's address is blocked. // No withdrawal can be made to that address. RecipientAddressBlocked : record { address : text }; - // The minter is overloaded, retry the request. + // The minter or the ckETH ledger is temporarily unavailable, retry the request. // The payload contains a human-readable message explaining what caused the unavailability. TemporarilyUnavailable : text; }; @@ -270,6 +270,10 @@ type WithdrawErc20Error = variant { // The minter could not burn the requested amount of ckERC20 tokens. // The `cketh_block_index` identifies the burn that occurred on the ckETH ledger and that will be reimbursed. CkErc20LedgerError : record { cketh_block_index : nat; error : LedgerError }; + + // The minter is temporarily unavailable, retry the request. + // The payload contains a human-readable message explaining what caused the unavailability. + TemporarilyUnavailable : text; }; type LedgerError = variant { diff --git a/rs/ethereum/cketh/minter/src/endpoints/ckerc20.rs b/rs/ethereum/cketh/minter/src/endpoints/ckerc20.rs index 8b8c9cd5cff..18bbd3c4c67 100644 --- a/rs/ethereum/cketh/minter/src/endpoints/ckerc20.rs +++ b/rs/ethereum/cketh/minter/src/endpoints/ckerc20.rs @@ -24,7 +24,7 @@ impl From for RetrieveErc20Request { } } -#[derive(CandidType, Deserialize, Debug, PartialEq)] +#[derive(CandidType, Deserialize, Clone, Debug, PartialEq)] pub enum WithdrawErc20Error { TokenNotSupported { supported_tokens: Vec, @@ -39,9 +39,10 @@ pub enum WithdrawErc20Error { cketh_block_index: Nat, error: LedgerError, }, + TemporarilyUnavailable(String), } -#[derive(CandidType, Deserialize, Debug, PartialEq)] +#[derive(CandidType, Deserialize, Clone, Debug, PartialEq)] pub enum LedgerError { InsufficientFunds { balance: Nat, diff --git a/rs/ethereum/cketh/minter/src/main.rs b/rs/ethereum/cketh/minter/src/main.rs index a105a1a032d..edb26a85265 100644 --- a/rs/ethereum/cketh/minter/src/main.rs +++ b/rs/ethereum/cketh/minter/src/main.rs @@ -26,8 +26,10 @@ use ic_cketh_minter::state::transactions::{ Erc20WithdrawalRequest, EthWithdrawalRequest, Reimbursed, ReimbursementRequest, }; use ic_cketh_minter::state::{lazy_call_ecdsa_public_key, mutate_state, read_state, State, STATE}; +use ic_cketh_minter::tx::lazy_refresh_gas_fee_estimate; use ic_cketh_minter::withdraw::{ - process_reimbursement, process_retrieve_eth_requests, CKETH_WITHDRAWAL_TRANSACTION_GAS_LIMIT, + process_reimbursement, process_retrieve_eth_requests, CKERC20_WITHDRAWAL_TRANSACTION_GAS_LIMIT, + CKETH_WITHDRAWAL_TRANSACTION_GAS_LIMIT, }; use ic_cketh_minter::{endpoints, erc20}; use ic_cketh_minter::{ @@ -316,7 +318,9 @@ async fn withdraw_erc20( } })?; let cketh_ledger = read_state(LedgerClient::cketh_ledger_from_state); - let erc20_tx_fee = estimate_erc20_transaction_fee(); + let erc20_tx_fee = estimate_erc20_transaction_fee().await.ok_or_else(|| { + WithdrawErc20Error::TemporarilyUnavailable("Failed to retrieve current gas fee".to_string()) + })?; let now = ic_cdk::api::time(); log!(INFO, "[withdraw_erc20]: burning {:?} ckETH", erc20_tx_fee); match cketh_ledger @@ -411,9 +415,14 @@ async fn withdraw_erc20( } } -fn estimate_erc20_transaction_fee() -> Wei { - //TODO XC-58: better fee estimation - read_state(|s| s.minimum_withdrawal_amount) +async fn estimate_erc20_transaction_fee() -> Option { + lazy_refresh_gas_fee_estimate() + .await + .map(|gas_fee_estimate| { + gas_fee_estimate + .to_price(CKERC20_WITHDRAWAL_TRANSACTION_GAS_LIMIT) + .max_transaction_fee() + }) } #[query] diff --git a/rs/ethereum/cketh/minter/src/state/transactions/mod.rs b/rs/ethereum/cketh/minter/src/state/transactions/mod.rs index 48f3b2c1d78..59031252348 100644 --- a/rs/ethereum/cketh/minter/src/state/transactions/mod.rs +++ b/rs/ethereum/cketh/minter/src/state/transactions/mod.rs @@ -333,10 +333,10 @@ pub enum CreateTransactionError { #[derive(Clone, Debug, Eq, PartialEq)] pub enum ResubmitTransactionError { - InsufficientTransactionAmount { + InsufficientTransactionFee { ledger_burn_index: LedgerBurnIndex, transaction_nonce: TransactionNonce, - transaction_amount: Wei, + allowed_max_transaction_fee: Wei, max_transaction_fee: Wei, }, } @@ -373,6 +373,26 @@ impl EthTransactions { self.reimbursed.values().cloned().collect() } + fn find_reimbursed_transaction_by_cketh_ledger_burn_index( + &self, + searched_burn_index: &LedgerBurnIndex, + ) -> Option<&Reimbursed> { + self.reimbursed + .iter() + .find_map(|(index, value)| match index { + ReimbursementIndex::CkEth { ledger_burn_index } + if ledger_burn_index == searched_burn_index => + { + Some(value) + } + ReimbursementIndex::CkErc20 { + cketh_ledger_burn_index, + .. + } if cketh_ledger_burn_index == searched_burn_index => Some(value), + _ => None, + }) + } + pub fn record_withdrawal_request>(&mut self, request: R) { let request = request.into(); let burn_index = request.cketh_ledger_burn_index(); @@ -537,10 +557,10 @@ impl EthTransactions { actual_max_transaction_fee, }) => { transactions_to_resubmit.push(Err( - ResubmitTransactionError::InsufficientTransactionAmount { + ResubmitTransactionError::InsufficientTransactionFee { ledger_burn_index: *burn_index, transaction_nonce: *nonce, - transaction_amount: allowed_max_transaction_fee, + allowed_max_transaction_fee, max_transaction_fee: actual_max_transaction_fee, }, )); @@ -636,23 +656,6 @@ impl EthTransactions { } } WithdrawalRequest::CkErc20(request) => { - if let Some(unused_tx_fee) = request - .max_transaction_fee - .checked_sub(finalized_tx.transaction_price().max_transaction_fee()) - { - if unused_tx_fee > Wei::ZERO { - self.record_reimbursement_request( - ReimbursementIndex::CkEth { ledger_burn_index }, - ReimbursementRequest { - ledger_burn_index, - to: request.from, - to_subaccount: request.from_subaccount.clone(), - reimbursed_amount: unused_tx_fee.change_units(), - transaction_hash: Some(receipt.transaction_hash), - }, - ); - } - } if receipt.status == TransactionStatus::Failure { self.record_reimbursement_request( ReimbursementIndex::CkErc20 { @@ -693,7 +696,7 @@ impl EthTransactions { let reimbursement_request = self .reimbursement_requests .remove(&index) - .expect("BUG: missing reimbursement request"); + .unwrap_or_else(|| panic!("BUG: missing reimbursement request with index {index:?}")); let burn_in_block = index.burn_in_block(); assert_eq!( self.reimbursed.insert( @@ -727,16 +730,15 @@ impl EthTransactions { } if let Some(tx) = self.finalized_tx.get_alt(burn_index) { - if let Some(reimbursed) = self.reimbursed.get(&ReimbursementIndex::CkEth { - ledger_burn_index: *burn_index, - }) { + if let Some(reimbursed) = + self.find_reimbursed_transaction_by_cketh_ledger_burn_index(burn_index) + { return RetrieveEthStatus::TxFinalized(TxFinalizedStatus::Reimbursed { reimbursed_in_block: reimbursed.reimbursed_in_block.get().into(), transaction_hash: tx.transaction_hash().to_string(), reimbursed_amount: reimbursed.reimbursed_amount.into(), }); } - //TODO XC-78: status for pending transactioon fee reimbursement for ckERC20? if tx.transaction_status() == &TransactionStatus::Failure { return RetrieveEthStatus::TxFinalized(TxFinalizedStatus::PendingReimbursement( EthTransaction { diff --git a/rs/ethereum/cketh/minter/src/state/transactions/tests.rs b/rs/ethereum/cketh/minter/src/state/transactions/tests.rs index d35cddee1df..777ff2228a7 100644 --- a/rs/ethereum/cketh/minter/src/state/transactions/tests.rs +++ b/rs/ethereum/cketh/minter/src/state/transactions/tests.rs @@ -3,8 +3,7 @@ use crate::eth_rpc::Hash; use crate::eth_rpc_client::responses::{TransactionReceipt, TransactionStatus}; use crate::lifecycle::EthereumNetwork; use crate::numeric::{ - BlockNumber, CkTokenAmount, Erc20Value, GasAmount, LedgerBurnIndex, TransactionNonce, Wei, - WeiPerGas, + BlockNumber, Erc20Value, GasAmount, LedgerBurnIndex, TransactionNonce, Wei, WeiPerGas, }; use crate::state::transactions::{ create_transaction, Erc20WithdrawalRequest, EthTransactions, EthWithdrawalRequest, Subaccount, @@ -1410,8 +1409,7 @@ mod eth_transactions { use crate::state::transactions::tests::{ ckerc20_withdrawal_request_with_index, cketh_withdrawal_request_with_index, create_and_record_ck_withdrawal_requests, create_and_record_signed_transaction, - create_and_record_transaction, dummy_signature, expected_cketh_reimbursed_amount, - gas_fee_estimate, transaction_receipt, + create_and_record_transaction, dummy_signature, gas_fee_estimate, transaction_receipt, }; use crate::state::transactions::{ Erc20WithdrawalRequest, EthTransactions, ReimbursementIndex, ReimbursementRequest, @@ -1496,7 +1494,7 @@ mod eth_transactions { } #[test] - fn should_reimburse_unused_transaction_fee_when_ckerc20_withdrawal_successful() { + fn should_not_reimburse_unused_transaction_fee_when_ckerc20_withdrawal_successful() { let mut transactions = EthTransactions::new(TransactionNonce::ZERO); let cketh_ledger_burn_index = LedgerBurnIndex::new(7); let ckerc20_ledger_burn_index = LedgerBurnIndex::new(7); @@ -1521,29 +1519,9 @@ mod eth_transactions { Wei::from(4_000_000_u32) ); transactions.record_finalized_transaction(cketh_ledger_burn_index, receipt.clone()); - let expected_cketh_reimbursed_amount = withdrawal_request - .max_transaction_fee - .checked_sub( - signed_tx - .transaction() - .transaction_price() - .max_transaction_fee(), - ) - .unwrap(); assert_eq!(transactions.maybe_reimburse, btreemap! {}); - assert_eq!( - transactions.reimbursement_requests, - btreemap! { - ReimbursementIndex::CkEth { ledger_burn_index: cketh_ledger_burn_index } => ReimbursementRequest { - ledger_burn_index: cketh_ledger_burn_index, - reimbursed_amount: expected_cketh_reimbursed_amount.change_units(), - to: withdrawal_request.from, - to_subaccount: withdrawal_request.from_subaccount, - transaction_hash: Some(receipt.transaction_hash), - } - } - ); + assert_eq!(transactions.reimbursement_requests, btreemap! {}); } #[test] @@ -1584,7 +1562,7 @@ mod eth_transactions { } #[test] - fn should_reimburse_unused_transaction_fee_and_tokens_when_ckerc20_withdrawal_fails() { + fn should_reimburse_tokens_when_ckerc20_withdrawal_fails() { let mut transactions = EthTransactions::new(TransactionNonce::ZERO); let cketh_ledger_burn_index = LedgerBurnIndex::new(7); let ckerc20_ledger_burn_index = LedgerBurnIndex::new(7); @@ -1609,30 +1587,12 @@ mod eth_transactions { Wei::from(4_000_000_u32) ); transactions.record_finalized_transaction(cketh_ledger_burn_index, receipt.clone()); - let expected_cketh_reimbursed_amount = withdrawal_request - //TODO XC-59: rename max_transaction_fee to burn_cketh_amount - .max_transaction_fee - .checked_sub( - signed_tx - .transaction() - .transaction_price() - .max_transaction_fee(), - ) - .unwrap(); let expected_ckerc20_reimbursed_amount = withdrawal_request.withdrawal_amount; assert_eq!(transactions.maybe_reimburse, btreemap! {}); assert_eq!( transactions.reimbursement_requests, btreemap! { - ReimbursementIndex::CkEth { ledger_burn_index: cketh_ledger_burn_index } => - ReimbursementRequest { - ledger_burn_index: cketh_ledger_burn_index, - reimbursed_amount: expected_cketh_reimbursed_amount.change_units(), - to: withdrawal_request.from, - to_subaccount: withdrawal_request.from_subaccount.clone(), - transaction_hash: Some(receipt.transaction_hash), - }, ReimbursementIndex::CkErc20 { cketh_ledger_burn_index, ledger_id: withdrawal_request.ckerc20_ledger_id, @@ -1649,12 +1609,12 @@ mod eth_transactions { } #[test] - fn should_record_finalized_transaction_and_reimburse() { + fn should_record_finalized_transaction_and_reimburse_unused_tx_fee_when_cketh_withdrawal_fails( + ) { let mut transactions = EthTransactions::new(TransactionNonce::ZERO); - let mut rng = reproducible_rng(); - let [withdrawal_request] = - create_and_record_ck_withdrawal_requests(&mut transactions, &mut rng); - let cketh_ledger_burn_index = withdrawal_request.cketh_ledger_burn_index(); + let withdrawal_request = cketh_withdrawal_request_with_index(LedgerBurnIndex::new(15)); + transactions.record_withdrawal_request(withdrawal_request.clone()); + let cketh_ledger_burn_index = withdrawal_request.ledger_burn_index; let created_tx = create_and_record_transaction( &mut transactions, withdrawal_request.clone(), @@ -1665,7 +1625,7 @@ mod eth_transactions { .maybe_reimburse .get(&cketh_ledger_burn_index) .expect("maybe reimburse request not found"); - assert_eq!(maybe_reimburse_request, &withdrawal_request); + assert_eq!(maybe_reimburse_request, &withdrawal_request.clone().into()); let receipt = transaction_receipt(&signed_tx, TransactionStatus::Failure); transactions.record_finalized_transaction(cketh_ledger_burn_index, receipt.clone()); @@ -1689,13 +1649,13 @@ mod eth_transactions { &ReimbursementRequest { transaction_hash: Some(receipt.transaction_hash), ledger_burn_index: cketh_ledger_burn_index, - to: withdrawal_request.from(), - to_subaccount: withdrawal_request.from_subaccount().clone(), - reimbursed_amount: expected_cketh_reimbursed_amount( - &withdrawal_request, - effective_fee_paid - ) - .unwrap(), + to: withdrawal_request.from, + to_subaccount: withdrawal_request.from_subaccount, + reimbursed_amount: withdrawal_request + .withdrawal_amount + .checked_sub(effective_fee_paid) + .unwrap() + .change_units() } ); } @@ -1768,57 +1728,32 @@ mod eth_transactions { mod transaction_status { use crate::endpoints::{EthTransaction, RetrieveEthStatus, TxFinalizedStatus}; - use crate::numeric::{LedgerMintIndex, TransactionNonce}; - use crate::state::transactions::tests::create_ck_withdrawal_requests; + use crate::numeric::{LedgerBurnIndex, LedgerMintIndex, TransactionNonce}; use crate::state::transactions::tests::{ - create_and_record_ck_withdrawal_requests, create_and_record_transaction, - expected_cketh_reimbursed_amount, gas_fee_estimate, sign_transaction, - transaction_receipt, + ckerc20_withdrawal_request_with_index, cketh_withdrawal_request_with_index, + create_ck_withdrawal_requests, + }; + use crate::state::transactions::tests::{ + create_and_record_transaction, gas_fee_estimate, sign_transaction, transaction_receipt, + }; + use crate::state::transactions::{ + EthTransactions, ReimbursementIndex, TransactionStatus, WithdrawalRequest, }; - use crate::state::transactions::{EthTransactions, ReimbursementIndex, TransactionStatus}; use ic_crypto_test_utils_reproducible_rng::reproducible_rng; #[test] - fn should_withdrawal_flow_succeed_with_correct_status() { + fn should_have_finalized_success_status() { let mut transactions = EthTransactions::new(TransactionNonce::ZERO); let mut rng = reproducible_rng(); let [withdrawal_request] = create_ck_withdrawal_requests(&mut rng); let cketh_ledger_burn_index = withdrawal_request.cketh_ledger_burn_index(); - assert_eq!( - transactions.transaction_status(&cketh_ledger_burn_index), - RetrieveEthStatus::NotFound - ); - transactions.record_withdrawal_request(withdrawal_request.clone()); - assert_eq!( - transactions.transaction_status(&cketh_ledger_burn_index), - RetrieveEthStatus::Pending - ); - - let created_tx = create_and_record_transaction( + let eth_transaction = withdrawal_flow( &mut transactions, withdrawal_request, - gas_fee_estimate(), - ); - assert_eq!( - transactions.transaction_status(&cketh_ledger_burn_index), - RetrieveEthStatus::TxCreated + TransactionStatus::Success, ); - let signed_tx = sign_transaction(created_tx); - let eth_transaction = EthTransaction { - transaction_hash: signed_tx.hash().to_string(), - }; - transactions.record_signed_transaction(signed_tx.clone()); - assert_eq!( - transactions.transaction_status(&cketh_ledger_burn_index), - RetrieveEthStatus::TxSent(eth_transaction.clone()) - ); - - transactions.record_finalized_transaction( - cketh_ledger_burn_index, - transaction_receipt(&signed_tx, TransactionStatus::Success), - ); assert_eq!( transactions.transaction_status(&cketh_ledger_burn_index), RetrieveEthStatus::TxFinalized(TxFinalizedStatus::Success(eth_transaction)) @@ -1826,29 +1761,16 @@ mod eth_transactions { } #[test] - fn should_withdrawal_flow_succeed_with_reimbursed_status() { + fn should_have_finalized_reimbursed_status_for_cketh_withdrawal() { let mut transactions = EthTransactions::new(TransactionNonce::ZERO); - let mut rng = reproducible_rng(); - let [withdrawal_request] = - create_and_record_ck_withdrawal_requests(&mut transactions, &mut rng); - let cketh_ledger_burn_index = withdrawal_request.cketh_ledger_burn_index(); - - let created_tx = create_and_record_transaction( + let withdrawal_request = cketh_withdrawal_request_with_index(LedgerBurnIndex::new(15)); + let cketh_ledger_burn_index = withdrawal_request.ledger_burn_index; + let eth_transaction = withdrawal_flow( &mut transactions, withdrawal_request.clone(), - gas_fee_estimate(), + TransactionStatus::Failure, ); - let signed_tx = sign_transaction(created_tx); - let eth_transaction = EthTransaction { - transaction_hash: signed_tx.hash().to_string(), - }; - transactions.record_signed_transaction(signed_tx.clone()); - - transactions.record_finalized_transaction( - cketh_ledger_burn_index, - transaction_receipt(&signed_tx, TransactionStatus::Failure), - ); assert_eq!( transactions.transaction_status(&cketh_ledger_burn_index), RetrieveEthStatus::TxFinalized(TxFinalizedStatus::PendingReimbursement( @@ -1868,23 +1790,103 @@ mod eth_transactions { .finalized_tx .get_alt(&cketh_ledger_burn_index) .expect("finalized tx not found"); - let effective_fee_paid = finalized_transaction.effective_transaction_fee(); assert_eq!( transactions.transaction_status(&cketh_ledger_burn_index), RetrieveEthStatus::TxFinalized(TxFinalizedStatus::Reimbursed { reimbursed_in_block: candid::Nat::from(16_u8), - transaction_hash: signed_tx.hash().to_string(), - reimbursed_amount: expected_cketh_reimbursed_amount( - &withdrawal_request, - effective_fee_paid - ) - .unwrap() - .into(), + transaction_hash: eth_transaction.transaction_hash, + reimbursed_amount: withdrawal_request + .withdrawal_amount + .checked_sub(effective_fee_paid) + .unwrap() + .into(), }) ); } + + #[test] + fn should_have_finalized_reimbursed_status_for_ckerc20_withdrawal() { + let mut transactions = EthTransactions::new(TransactionNonce::ZERO); + let withdrawal_request = ckerc20_withdrawal_request_with_index( + LedgerBurnIndex::new(15), + LedgerBurnIndex::new(7), + ); + let cketh_ledger_burn_index = withdrawal_request.cketh_ledger_burn_index; + let eth_transaction = withdrawal_flow( + &mut transactions, + withdrawal_request.clone(), + TransactionStatus::Failure, + ); + + assert_eq!( + transactions.transaction_status(&cketh_ledger_burn_index), + RetrieveEthStatus::TxFinalized(TxFinalizedStatus::PendingReimbursement( + eth_transaction.clone() + )) + ); + + let ckerc20_reimbursement_index = ReimbursementIndex::CkErc20 { + cketh_ledger_burn_index: withdrawal_request.cketh_ledger_burn_index, + ledger_id: withdrawal_request.ckerc20_ledger_id, + ckerc20_ledger_burn_index: withdrawal_request.ckerc20_ledger_burn_index, + }; + transactions.record_finalized_reimbursement( + ckerc20_reimbursement_index, + LedgerMintIndex::new(16), + ); + + assert_eq!( + transactions.transaction_status(&cketh_ledger_burn_index), + RetrieveEthStatus::TxFinalized(TxFinalizedStatus::Reimbursed { + reimbursed_in_block: candid::Nat::from(16_u8), + transaction_hash: eth_transaction.transaction_hash, + reimbursed_amount: withdrawal_request.withdrawal_amount.into(), + }) + ); + } + + fn withdrawal_flow>( + transactions: &mut EthTransactions, + withdrawal_request: T, + status: TransactionStatus, + ) -> EthTransaction { + let withdrawal_request = withdrawal_request.into(); + let cketh_ledger_burn_index = withdrawal_request.cketh_ledger_burn_index(); + + assert_eq!( + transactions.transaction_status(&cketh_ledger_burn_index), + RetrieveEthStatus::NotFound + ); + transactions.record_withdrawal_request(withdrawal_request.clone()); + assert_eq!( + transactions.transaction_status(&cketh_ledger_burn_index), + RetrieveEthStatus::Pending + ); + + let created_tx = + create_and_record_transaction(transactions, withdrawal_request, gas_fee_estimate()); + assert_eq!( + transactions.transaction_status(&cketh_ledger_burn_index), + RetrieveEthStatus::TxCreated + ); + + let signed_tx = sign_transaction(created_tx); + let eth_transaction = EthTransaction { + transaction_hash: signed_tx.hash().to_string(), + }; + transactions.record_signed_transaction(signed_tx.clone()); + assert_eq!( + transactions.transaction_status(&cketh_ledger_burn_index), + RetrieveEthStatus::TxSent(eth_transaction.clone()) + ); + transactions.record_finalized_transaction( + cketh_ledger_burn_index, + transaction_receipt(&signed_tx, status), + ); + eth_transaction + } } } @@ -2619,17 +2621,6 @@ fn transaction_receipt( } } -fn expected_cketh_reimbursed_amount( - withdrawal_request: &WithdrawalRequest, - effective_fee_paid: Wei, -) -> Option { - let wei_amount = match withdrawal_request { - WithdrawalRequest::CkEth(req) => req.withdrawal_amount.checked_sub(effective_fee_paid), - WithdrawalRequest::CkErc20(req) => req.max_transaction_fee.checked_sub(effective_fee_paid), - }; - wei_amount.map(CheckedAmountOf::change_units) -} - fn sign_transaction(transaction: Eip1559TransactionRequest) -> SignedEip1559TransactionRequest { SignedEip1559TransactionRequest::from((transaction, dummy_signature())) } diff --git a/rs/ethereum/cketh/minter/src/tx.rs b/rs/ethereum/cketh/minter/src/tx.rs index 177b18adfa8..9f2025878d1 100644 --- a/rs/ethereum/cketh/minter/src/tx.rs +++ b/rs/ethereum/cketh/minter/src/tx.rs @@ -701,7 +701,7 @@ pub enum TransactionFeeEstimationError { /// /// From the fee history, the current base fee per gas and the max priority fee per gas are determined. /// Then, the max fee per gas is computed as `2 * base_fee_per_gas + max_priority_fee_per_gas` to ensure that -/// the estimate remains valid for the next few blocks, see https://www.blocknative.com/blog/eip-1559-fees. +/// the estimate remains valid for the next few blocks, see ``. pub fn estimate_transaction_fee( fee_history: &FeeHistory, ) -> Result { diff --git a/rs/ethereum/cketh/minter/tests/ckerc20.rs b/rs/ethereum/cketh/minter/tests/ckerc20.rs index d24527f2f32..e51c6aaf858 100644 --- a/rs/ethereum/cketh/minter/tests/ckerc20.rs +++ b/rs/ethereum/cketh/minter/tests/ckerc20.rs @@ -145,6 +145,7 @@ fn should_mint_with_ckerc20_setup() { mod withdraw_erc20 { use super::*; + use ic_base_types::PrincipalId; use ic_cketh_minter::endpoints::ckerc20::{ LedgerError, RetrieveErc20Request, WithdrawErc20Error, }; @@ -155,10 +156,11 @@ mod withdraw_erc20 { use ic_cketh_minter::memo::BurnMemo; use ic_cketh_minter::PROCESS_REIMBURSEMENT; use ic_cketh_test_utils::ckerc20::{ - erc20_transfer_data, DEFAULT_ERC20_WITHDRAWAL_DESTINATION_ADDRESS, ONE_USDC, TWO_USDC, + erc20_transfer_data, Erc20WithdrawalFlow, RefreshGasFeeEstimate, + DEFAULT_ERC20_WITHDRAWAL_DESTINATION_ADDRESS, ONE_USDC, TWO_USDC, }; use ic_cketh_test_utils::flow::{ - double_and_increment_base_fee_per_gas, DepositParams, ProcessWithdrawalParams, + increment_max_priority_fee_per_gas, DepositParams, ProcessWithdrawalParams, }; use ic_cketh_test_utils::mock::JsonRpcProvider; use ic_cketh_test_utils::response::{ @@ -166,14 +168,15 @@ mod withdraw_erc20 { }; use ic_cketh_test_utils::{ CKETH_TRANSFER_FEE, DEFAULT_BLOCK_HASH, DEFAULT_BLOCK_NUMBER, - DEFAULT_CKERC20_WITHDRAWAL_TRANSACTION, DEFAULT_CKERC20_WITHDRAWAL_TRANSACTION_HASH, - EXPECTED_BALANCE, + DEFAULT_CKERC20_WITHDRAWAL_TRANSACTION, DEFAULT_CKERC20_WITHDRAWAL_TRANSACTION_FEE, + DEFAULT_CKERC20_WITHDRAWAL_TRANSACTION_HASH, DEFAULT_PRINCIPAL_ID, EXPECTED_BALANCE, }; use ic_ledger_suite_orchestrator_test_utils::{new_state_machine, CKERC20_TRANSFER_FEE}; use icrc_ledger_types::icrc3::transactions::Burn; use num_bigint::BigUint; use num_traits::ToPrimitive; use serde_bytes::ByteBuf; + use std::convert::identity; use std::sync::Arc; const NOT_SUPPORTED_CKERC20_LEDGER_ID: Principal = Principal::management_canister(); @@ -187,6 +190,7 @@ mod withdraw_erc20 { NOT_SUPPORTED_CKERC20_LEDGER_ID, DEFAULT_ERC20_WITHDRAWAL_DESTINATION_ADDRESS, ) + .expect_no_refresh_gas_fee_estimate() .expect_trap("disabled"); } @@ -199,6 +203,7 @@ mod withdraw_erc20 { NOT_SUPPORTED_CKERC20_LEDGER_ID, DEFAULT_ERC20_WITHDRAWAL_DESTINATION_ADDRESS, ) + .expect_no_refresh_gas_fee_estimate() .expect_trap("anonymous"); } @@ -213,6 +218,7 @@ mod withdraw_erc20 { NOT_SUPPORTED_CKERC20_LEDGER_ID, "0xinvalid-address", ) + .expect_no_refresh_gas_fee_estimate() .expect_trap("address"); } @@ -228,6 +234,7 @@ mod withdraw_erc20 { NOT_SUPPORTED_CKERC20_LEDGER_ID, blocked_address, ) + .expect_no_refresh_gas_fee_estimate() .expect_error(WithdrawErc20Error::RecipientAddressBlocked { address: blocked_address.to_string(), }); @@ -250,6 +257,7 @@ mod withdraw_erc20 { NOT_SUPPORTED_CKERC20_LEDGER_ID, DEFAULT_ERC20_WITHDRAWAL_DESTINATION_ADDRESS, ) + .expect_no_refresh_gas_fee_estimate() .expect_trap("u256"); } @@ -270,6 +278,7 @@ mod withdraw_erc20 { NOT_SUPPORTED_CKERC20_LEDGER_ID, DEFAULT_ERC20_WITHDRAWAL_DESTINATION_ADDRESS, ) + .expect_no_refresh_gas_fee_estimate() .expect_error(WithdrawErc20Error::TokenNotSupported { supported_tokens }); } @@ -287,10 +296,11 @@ mod withdraw_erc20 { ckusdc.ledger_canister_id, DEFAULT_ERC20_WITHDRAWAL_DESTINATION_ADDRESS, ) + .expect_refresh_gas_fee_estimate(identity) .expect_error(WithdrawErc20Error::CkEthLedgerError { error: LedgerError::InsufficientAllowance { allowance: Nat::from(0_u8), - failed_burn_amount: Nat::from(CKETH_MINIMUM_WITHDRAWAL_AMOUNT), + failed_burn_amount: DEFAULT_CKERC20_WITHDRAWAL_TRANSACTION_FEE.into(), token_symbol: "ckETH".to_string(), ledger_id: cketh_ledger, }, @@ -306,20 +316,25 @@ mod withdraw_erc20 { ckerc20 .deposit_cketh(DepositParams { - amount: CKETH_MINIMUM_WITHDRAWAL_AMOUNT, + amount: DEFAULT_CKERC20_WITHDRAWAL_TRANSACTION_FEE + CKETH_TRANSFER_FEE - 1, ..DepositParams::default() }) - .call_cketh_ledger_approve_minter(caller, CKETH_MINIMUM_WITHDRAWAL_AMOUNT, None) + .call_cketh_ledger_approve_minter( + caller, + DEFAULT_CKERC20_WITHDRAWAL_TRANSACTION_FEE, + None, + ) .call_minter_withdraw_erc20( caller, 0_u8, ckusdc.ledger_canister_id, DEFAULT_ERC20_WITHDRAWAL_DESTINATION_ADDRESS, ) + .expect_refresh_gas_fee_estimate(identity) .expect_error(WithdrawErc20Error::CkEthLedgerError { error: LedgerError::InsufficientFunds { - balance: Nat::from(CKETH_MINIMUM_WITHDRAWAL_AMOUNT - CKETH_TRANSFER_FEE), - failed_burn_amount: Nat::from(CKETH_MINIMUM_WITHDRAWAL_AMOUNT), + balance: Nat::from(DEFAULT_CKERC20_WITHDRAWAL_TRANSACTION_FEE - 1), + failed_burn_amount: Nat::from(DEFAULT_CKERC20_WITHDRAWAL_TRANSACTION_FEE), token_symbol: "ckETH".to_string(), ledger_id: cketh_ledger, }, @@ -328,7 +343,7 @@ mod withdraw_erc20 { #[test] fn should_error_when_minter_fails_to_burn_ckerc20_and_reimburse_cketh() { - let transaction_fee = CKETH_MINIMUM_WITHDRAWAL_AMOUNT; + let transaction_fee = DEFAULT_CKERC20_WITHDRAWAL_TRANSACTION_FEE; let cketh_burn_index = 2_u8; let mut tests = vec![]; @@ -398,6 +413,7 @@ mod withdraw_erc20 { ckusdc.ledger_canister_id, DEFAULT_ERC20_WITHDRAWAL_DESTINATION_ADDRESS, ) + .expect_refresh_gas_fee_estimate(identity) .expect_error(test.expected_withdrawal_error) .call_cketh_ledger_get_transaction(cketh_burn_index) .expect_burn(Burn { @@ -473,13 +489,117 @@ mod withdraw_erc20 { } #[test] - fn should_withdraw_ckusdc_and_reimburse_unused_transaction_fees() { + fn should_refresh_gas_fee_estimate_only_once_within_a_minute() { + let ckerc20 = CkErc20Setup::default().add_supported_erc20_tokens(); + let ckusdc = ckerc20.find_ckerc20_token("ckUSDC"); + let cketh_ledger = ckerc20.cketh_ledger_id(); + let user_1 = ckerc20.caller(); + let user_2: Principal = PrincipalId::new_user_test_id(DEFAULT_PRINCIPAL_ID + 1).into(); + assert_ne!(user_1, user_2); + let insufficient_allowance_error = WithdrawErc20Error::CkEthLedgerError { + error: LedgerError::InsufficientAllowance { + allowance: Nat::from(0_u8), + failed_burn_amount: DEFAULT_CKERC20_WITHDRAWAL_TRANSACTION_FEE.into(), + token_symbol: "ckETH".to_string(), + ledger_id: cketh_ledger, + }, + }; + + let ckerc20 = ckerc20 + .call_minter_withdraw_erc20( + user_1, + 0_u8, + ckusdc.ledger_canister_id, + DEFAULT_ERC20_WITHDRAWAL_DESTINATION_ADDRESS, + ) + .expect_refresh_gas_fee_estimate(identity) + .expect_error(insufficient_allowance_error.clone()); + + ckerc20.env.advance_time(Duration::from_secs(59)); + + let ckerc20 = ckerc20 + .call_minter_withdraw_erc20( + user_2, + 0_u8, + ckusdc.ledger_canister_id, + DEFAULT_ERC20_WITHDRAWAL_DESTINATION_ADDRESS, + ) + .expect_no_refresh_gas_fee_estimate() + .expect_error(insufficient_allowance_error.clone()); + + ckerc20.env.advance_time(Duration::from_millis(1_001)); + + ckerc20 + .call_minter_withdraw_erc20( + user_2, + 0_u8, + ckusdc.ledger_canister_id, + DEFAULT_ERC20_WITHDRAWAL_DESTINATION_ADDRESS, + ) + .expect_refresh_gas_fee_estimate(identity) + .expect_error(insufficient_allowance_error); + } + + #[test] + fn should_prevent_parallel_refresh_of_gas_fee_estimate() { + let ckerc20 = CkErc20Setup::default().add_supported_erc20_tokens(); + let ckusdc = ckerc20.find_ckerc20_token("ckUSDC"); + let cketh_ledger = ckerc20.cketh_ledger_id(); + let user_1 = ckerc20.caller(); + let user_2: Principal = PrincipalId::new_user_test_id(DEFAULT_PRINCIPAL_ID + 1).into(); + assert_ne!(user_1, user_2); + let insufficient_allowance_error = WithdrawErc20Error::CkEthLedgerError { + error: LedgerError::InsufficientAllowance { + allowance: Nat::from(0_u8), + failed_burn_amount: DEFAULT_CKERC20_WITHDRAWAL_TRANSACTION_FEE.into(), + token_symbol: "ckETH".to_string(), + ledger_id: cketh_ledger, + }, + }; + + let RefreshGasFeeEstimate { + setup: ckerc20, + message_id: first_withdrawal_msg_id, + } = ckerc20.call_minter_withdraw_erc20( + user_1, + 0_u8, + ckusdc.ledger_canister_id, + DEFAULT_ERC20_WITHDRAWAL_DESTINATION_ADDRESS, + ); + + let RefreshGasFeeEstimate { + setup: ckerc20, + message_id: second_withdrawal_msg_id, + } = ckerc20.call_minter_withdraw_erc20( + user_2, + 0_u8, + ckusdc.ledger_canister_id, + DEFAULT_ERC20_WITHDRAWAL_DESTINATION_ADDRESS, + ); + let ckerc20 = Erc20WithdrawalFlow { + setup: ckerc20, + message_id: second_withdrawal_msg_id, + } + .expect_error(WithdrawErc20Error::TemporarilyUnavailable( + "Failed to retrieve current gas fee".to_string(), + )); + + RefreshGasFeeEstimate { + setup: ckerc20, + message_id: first_withdrawal_msg_id, + } + .expect_refresh_gas_fee_estimate(identity) + .expect_error(insufficient_allowance_error); + } + + #[test] + fn should_withdraw_ckusdc() { fn test(transaction_status: TransactionStatus) { let ckerc20 = CkErc20Setup::default().add_supported_erc20_tokens(); let minter = ckerc20.cketh.minter_id; let ckusdc = ckerc20.find_ckerc20_token("ckUSDC"); let caller = ckerc20.caller(); - let ckerc20_tx_fee = CKETH_MINIMUM_WITHDRAWAL_AMOUNT; + let ckerc20_tx_fee = DEFAULT_CKERC20_WITHDRAWAL_TRANSACTION_FEE; let ckerc20 = ckerc20 .deposit_cketh_and_ckerc20( @@ -502,6 +622,7 @@ mod withdraw_erc20 { ckusdc.ledger_canister_id, DEFAULT_ERC20_WITHDRAWAL_DESTINATION_ADDRESS, ) + .expect_refresh_gas_fee_estimate(identity) .expect_withdrawal_request_accepted(); assert_eq!( @@ -578,7 +699,7 @@ mod withdraw_erc20 { }); let expected_cketh_balance_after_withdrawal = - Nat::from(EXPECTED_BALANCE - CKETH_TRANSFER_FEE - CKETH_MINIMUM_WITHDRAWAL_AMOUNT); + Nat::from(EXPECTED_BALANCE - CKETH_TRANSFER_FEE - ckerc20_tx_fee); assert_eq!( ckerc20.cketh.balance_of(caller), expected_cketh_balance_after_withdrawal @@ -650,9 +771,6 @@ mod withdraw_erc20 { }, ]); - let estimated_tx_fee = estimated_max_fee_per_gas * estimated_gas_limit; - assert!(estimated_tx_fee < ckerc20_tx_fee); - let reimbursed_cketh_amount = ckerc20_tx_fee - estimated_tx_fee; ckerc20.env.advance_time(PROCESS_REIMBURSEMENT); let cketh_balance_after_reimbursement = ckerc20.wait_for_updated_ledger_balance( ckerc20.cketh_ledger_id(), @@ -660,32 +778,10 @@ mod withdraw_erc20 { &expected_cketh_balance_after_withdrawal, ); assert_eq!( - cketh_balance_after_reimbursement - expected_cketh_balance_after_withdrawal, - reimbursed_cketh_amount.clone(), + cketh_balance_after_reimbursement, + expected_cketh_balance_after_withdrawal ); - let ckerc20 = ckerc20 - .check_events() - .assert_has_unique_events_in_order(&vec![EventPayload::ReimbursedEthWithdrawal { - withdrawal_id: cketh_block_index.clone(), - reimbursed_in_block: Nat::from(3_u8), - reimbursed_amount: reimbursed_cketh_amount.clone(), - transaction_hash: Some(DEFAULT_CKERC20_WITHDRAWAL_TRANSACTION_HASH.to_string()), - }]) - .call_cketh_ledger_get_transaction(3_u8) - .expect_mint(Mint { - amount: reimbursed_cketh_amount, - to: Account { - owner: caller, - subaccount: None, - }, - memo: Some(Memo::from(MintMemo::ReimburseTransaction { - withdrawal_id: cketh_block_index.0.to_u64().unwrap(), - tx_hash: DEFAULT_CKERC20_WITHDRAWAL_TRANSACTION_HASH.parse().unwrap(), - })), - created_at_time: None, - }); - if transaction_status == TransactionStatus::Failure { let ckerc20_balance_after_reimbursement = ckerc20.wait_for_updated_ledger_balance( ckusdc.ledger_canister_id, @@ -751,6 +847,7 @@ mod withdraw_erc20 { ckusdc.ledger_canister_id, DEFAULT_ERC20_WITHDRAWAL_DESTINATION_ADDRESS, ) + .expect_refresh_gas_fee_estimate(identity) .expect_withdrawal_request_accepted() .process_withdrawal_with_resubmission_and_same_price(expected_tx, expected_sig) .check_events() @@ -760,22 +857,19 @@ mod withdraw_erc20 { } #[test] - fn should_resubmit_new_transaction_when_price_increased() { + fn should_resubmit_new_transaction_with_same_max_fee_per_gas_when_price_increased() { let ckerc20 = CkErc20Setup::default().add_supported_erc20_tokens(); let ckusdc = ckerc20.find_ckerc20_token("ckUSDC"); let caller = ckerc20.caller(); - let ckerc20_tx_fee = CKETH_MINIMUM_WITHDRAWAL_AMOUNT; + let ckerc20_tx_fee = DEFAULT_CKERC20_WITHDRAWAL_TRANSACTION_FEE; let (first_tx, first_tx_sig) = default_erc20_signed_eip_1559_transaction(); let first_tx_hash = hash_transaction(first_tx.clone(), first_tx_sig); - let resubmitted_sent_tx = "0x02f8b001808462590080850f0de1e14682fde894a0b86991c6218b36c1d19d4a2e9eb0ce3606eb4880b844a9059cbb000000000000000000000000221e931fbfcb9bd54ddd26ce6f5e29e98add01c000000000000000000000000000000000000000000000000000000000001e8480c080a069eab386ad525029a368cd105f4bf125a426301433a5c53770fb9fca6e1bd857a0580fd7bd1ea3359ae4d6fa5d17f20d0e7ae0aeec5c48ac2be680dd6e11608706"; + let resubmitted_sent_tx = "0x02f8b0018084625900808507af2c9f6282fde894a0b86991c6218b36c1d19d4a2e9eb0ce3606eb4880b844a9059cbb000000000000000000000000221e931fbfcb9bd54ddd26ce6f5e29e98add01c000000000000000000000000000000000000000000000000000000000001e8480c001a03acbc792d2f821acaab8da81517f1905e30cd3acd2f85d7995c68c0ad1fd8817a0793a076f2163658c833ccddd37ee0a762a18adb423f689db5ffcf528ae667bf0"; let (resubmitted_tx, resubmitted_tx_sig) = decode_transaction(resubmitted_sent_tx); let resubmitted_tx_hash = hash_transaction(resubmitted_tx.clone(), resubmitted_tx_sig); assert_eq!( resubmitted_tx, - first_tx - .clone() - .max_priority_fee_per_gas(1_650_000_000_u64) - .max_fee_per_gas(64_657_416_518_u64) + first_tx.clone().max_priority_fee_per_gas(1_650_000_000_u64) ); assert_ne!(first_tx_hash, resubmitted_tx_hash); @@ -795,6 +889,7 @@ mod withdraw_erc20 { ckusdc.ledger_canister_id, DEFAULT_ERC20_WITHDRAWAL_DESTINATION_ADDRESS, ) + .expect_refresh_gas_fee_estimate(identity) .expect_withdrawal_request_accepted(); let RetrieveErc20Request { @@ -804,9 +899,9 @@ mod withdraw_erc20 { ckerc20 .process_withdrawal_with_resubmission_and_increased_price( - first_tx, + first_tx.clone(), first_tx_sig, - &mut double_and_increment_base_fee_per_gas, + &mut increment_max_priority_fee_per_gas, resubmitted_tx, resubmitted_tx_sig, ) @@ -818,7 +913,7 @@ mod withdraw_erc20 { chain_id: Nat::from(1_u8), nonce: Nat::from(0_u8), max_priority_fee_per_gas: Nat::from(1_650_000_000_u64), - max_fee_per_gas: Nat::from(64_657_416_518_u64), + max_fee_per_gas: Nat::from(first_tx.max_fee_per_gas.unwrap().as_u64()), gas_limit: Nat::from(65_000_u64), destination: ckusdc.erc20_contract_address, value: 0_u8.into(), @@ -871,6 +966,7 @@ mod withdraw_erc20 { ckusdc.ledger_canister_id, DEFAULT_ERC20_WITHDRAWAL_DESTINATION_ADDRESS, ) + .expect_refresh_gas_fee_estimate(identity) .expect_withdrawal_request_accepted() .wait_and_validate_withdrawal( ProcessWithdrawalParams::default().with_inconsistent_transaction_receipt(), @@ -903,6 +999,7 @@ mod withdraw_erc20 { ckusdc.ledger_canister_id, DEFAULT_ERC20_WITHDRAWAL_DESTINATION_ADDRESS, ) + .expect_refresh_gas_fee_estimate(identity) .expect_withdrawal_request_accepted() .start_processing_withdrawals() .retrieve_fee_history(move |mock| { diff --git a/rs/ethereum/cketh/test_utils/src/ckerc20.rs b/rs/ethereum/cketh/test_utils/src/ckerc20.rs index dc7a33423c6..a0f9d27893f 100644 --- a/rs/ethereum/cketh/test_utils/src/ckerc20.rs +++ b/rs/ethereum/cketh/test_utils/src/ckerc20.rs @@ -1,7 +1,9 @@ use crate::events::MinterEventAssert; use crate::flow::{DepositParams, LedgerTransactionAssert, ProcessWithdrawal}; -use crate::mock::{JsonRpcMethod, MockJsonRpcProviders}; -use crate::response::{block_response, empty_logs, Erc20LogEntry}; +use crate::mock::{ + JsonRpcMethod, JsonRpcRequestMatcher, MockJsonRpcProviders, MockJsonRpcProvidersBuilder, +}; +use crate::response::{block_response, empty_logs, fee_history, Erc20LogEntry}; use crate::{ assert_reply, format_ethereum_address_to_eip_55, new_state_machine, CkEthSetup, DEFAULT_DEPOSIT_BLOCK_NUMBER, DEFAULT_DEPOSIT_FROM_ADDRESS, DEFAULT_DEPOSIT_LOG_INDEX, @@ -29,7 +31,7 @@ use ic_state_machine_tests::{ErrorCode, MessageId, StateMachine}; use icrc_ledger_types::icrc1::account::Account; use num_traits::ToPrimitive; use serde_json::json; -use std::collections::BTreeSet; +use std::collections::{BTreeMap, BTreeSet}; use std::convert::identity; use std::iter::zip; use std::str::FromStr; @@ -290,7 +292,7 @@ impl CkErc20Setup { amount: A, ckerc20_ledger_id: Principal, recipient: R, - ) -> Erc20WithdrawalFlow { + ) -> RefreshGasFeeEstimate { let arg = WithdrawErc20Arg { amount: amount.into(), ckerc20_ledger_id, @@ -302,7 +304,7 @@ impl CkErc20Setup { "withdraw_erc20", Encode!(&arg).expect("failed to encode withdraw args"), ); - Erc20WithdrawalFlow { + RefreshGasFeeEstimate { setup: self, message_id, } @@ -573,9 +575,49 @@ impl CkErc20DepositFlow { } } +pub struct RefreshGasFeeEstimate { + pub setup: CkErc20Setup, + pub message_id: MessageId, +} + +impl RefreshGasFeeEstimate { + pub fn expect_refresh_gas_fee_estimate< + F: FnMut(MockJsonRpcProvidersBuilder) -> MockJsonRpcProvidersBuilder, + >( + self, + mut override_mock: F, + ) -> Erc20WithdrawalFlow { + let default_eth_fee_history = MockJsonRpcProviders::when(JsonRpcMethod::EthFeeHistory) + .respond_for_all_with(fee_history()); + (override_mock)(default_eth_fee_history) + .build() + .expect_rpc_calls(&self.setup); + Erc20WithdrawalFlow { + setup: self.setup, + message_id: self.message_id, + } + } + + pub fn expect_no_refresh_gas_fee_estimate(self) -> Erc20WithdrawalFlow { + assert_eq!( + JsonRpcRequestMatcher::new_for_all_providers(JsonRpcMethod::EthFeeHistory) + .iter() + .filter(|(_provider, matcher)| matcher.find_rpc_call(&self.setup.env).is_some()) + .collect::>(), + BTreeMap::new(), + "BUG: unexpected EthFeeHistory RPC call" + ); + + Erc20WithdrawalFlow { + setup: self.setup, + message_id: self.message_id, + } + } +} + pub struct Erc20WithdrawalFlow { - setup: CkErc20Setup, - message_id: MessageId, + pub setup: CkErc20Setup, + pub message_id: MessageId, } impl Erc20WithdrawalFlow { diff --git a/rs/ethereum/cketh/test_utils/src/flow.rs b/rs/ethereum/cketh/test_utils/src/flow.rs index 59bdcd0fd12..326633c52e8 100644 --- a/rs/ethereum/cketh/test_utils/src/flow.rs +++ b/rs/ethereum/cketh/test_utils/src/flow.rs @@ -826,6 +826,17 @@ pub fn encode_principal(principal: Principal) -> String { format!("0x{}", hex::encode(fixed_bytes)) } +pub fn increment_max_priority_fee_per_gas(fee_history: &mut ethers_core::types::FeeHistory) { + for rewards in fee_history.reward.iter_mut() { + for reward in rewards.iter_mut() { + *reward = reward + .checked_add(1_u64.into()) + .unwrap() + .max((1_500_000_000_u64 + 1_u64).into()); + } + } +} + pub fn double_and_increment_base_fee_per_gas(fee_history: &mut ethers_core::types::FeeHistory) { for base_fee_per_gas in fee_history.base_fee_per_gas.iter_mut() { *base_fee_per_gas = base_fee_per_gas diff --git a/rs/ethereum/cketh/test_utils/src/lib.rs b/rs/ethereum/cketh/test_utils/src/lib.rs index bcf419a8e45..e5729101edb 100644 --- a/rs/ethereum/cketh/test_utils/src/lib.rs +++ b/rs/ethereum/cketh/test_utils/src/lib.rs @@ -65,6 +65,8 @@ pub const DEFAULT_WITHDRAWAL_TRANSACTION: &str = "0x02f87301808459682f008507af2c pub const DEFAULT_CKERC20_WITHDRAWAL_TRANSACTION: &str = "0x02f8b001808459682f008507af2c9f6282fde894a0b86991c6218b36c1d19d4a2e9eb0ce3606eb4880b844a9059cbb000000000000000000000000221e931fbfcb9bd54ddd26ce6f5e29e98add01c000000000000000000000000000000000000000000000000000000000001e8480c080a0bb694aec6175b489523a55d5fce39452368e97096d4afa2cdcc35cf2d805152fa00112b26a028af84dd397d23549844efdaf761d90cdcfdbe6c3608239648a85a3"; pub const DEFAULT_CKERC20_WITHDRAWAL_TRANSACTION_HASH: &str = "0x2c0c328876b8d60580e00d8e5a82599e22099e78d9d9c25cc5e6164bc8f4db62"; + +pub const DEFAULT_CKERC20_WITHDRAWAL_TRANSACTION_FEE: u64 = 2_145_241_036_770_000_u64; pub const USDC_ERC20_CONTRACT_ADDRESS: &str = "0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48"; pub const MINTER_ADDRESS: &str = "0xfd644a761079369962386f8e4259217c2a10b8d0"; pub const DEFAULT_WITHDRAWAL_DESTINATION_ADDRESS: &str = diff --git a/rs/ethereum/cketh/test_utils/src/mock.rs b/rs/ethereum/cketh/test_utils/src/mock.rs index 7e636022039..5101efb69b8 100644 --- a/rs/ethereum/cketh/test_utils/src/mock.rs +++ b/rs/ethereum/cketh/test_utils/src/mock.rs @@ -5,8 +5,8 @@ use ic_cdk::api::management_canister::http_request::{ HttpResponse as OutCallHttpResponse, TransformArgs, }; use ic_state_machine_tests::{ - CanisterHttpMethod, CanisterHttpRequestContext, CanisterHttpResponsePayload, PayloadBuilder, - RejectCode, StateMachine, + CallbackId, CanisterHttpMethod, CanisterHttpRequestContext, CanisterHttpResponsePayload, + PayloadBuilder, RejectCode, StateMachine, }; use serde::de::DeserializeOwned; use serde::Serialize; @@ -114,6 +114,12 @@ impl JsonRpcRequestMatcher { } } + pub fn new_for_all_providers(method: JsonRpcMethod) -> BTreeMap { + JsonRpcProvider::iter() + .map(|provider| (provider, Self::new(provider, method.clone()))) + .collect() + } + pub fn with_request_params(mut self, params: Option) -> Self { self.match_request_params = params; self @@ -123,6 +129,39 @@ impl JsonRpcRequestMatcher { self.max_response_bytes = max_response_bytes; self } + + fn tick_until_next_http_request(&self, env: &StateMachine) { + let method = self.json_rpc_method.to_string(); + for _ in 0..MAX_TICKS { + let matching_method = env + .canister_http_request_contexts() + .values() + .any(|context| { + JsonRpcRequest::from_str( + std::str::from_utf8(&context.body.clone().unwrap()).unwrap(), + ) + .expect("BUG: invalid JSON RPC method") + .method + .to_string() + == method + }); + if matching_method { + break; + } + env.tick(); + env.advance_time(Duration::from_nanos(1)); + } + } + + pub fn find_rpc_call( + &self, + env: &StateMachine, + ) -> Option<(CallbackId, CanisterHttpRequestContext)> { + self.tick_until_next_http_request(env); + env.canister_http_request_contexts() + .into_iter() + .find(|(_id, context)| self.matches(context)) + } } impl Matcher for JsonRpcRequestMatcher { @@ -166,18 +205,13 @@ struct StubOnce { impl StubOnce { fn expect_rpc_call(self, env: &StateMachine, canister_id_cleanup_response: CanisterId) { - self.tick_until_next_http_request(env); - let (id, context) = env - .canister_http_request_contexts() - .into_iter() - .find(|(_id, context)| self.matcher.matches(context)) - .unwrap_or_else(|| { - panic!( - "no request found matching the stub {:?}. Current requests {}", - self, - debug_http_outcalls(env) - ) - }); + let (id, context) = self.matcher.find_rpc_call(env).unwrap_or_else(|| { + panic!( + "no request found matching the stub {:?}. Current requests {}", + self, + debug_http_outcalls(env) + ) + }); let request_id = { let request_body = context .body @@ -262,29 +296,6 @@ impl StubOnce { payload = payload.http_response(id, &http_response); env.execute_payload(payload); } - - fn tick_until_next_http_request(&self, env: &StateMachine) { - let method = self.matcher.json_rpc_method.to_string(); - for _ in 0..MAX_TICKS { - let matching_method = env - .canister_http_request_contexts() - .values() - .any(|context| { - JsonRpcRequest::from_str( - std::str::from_utf8(&context.body.clone().unwrap()).unwrap(), - ) - .expect("BUG: invalid JSON RPC method") - .method - .to_string() - == method - }); - if matching_method { - break; - } - env.tick(); - env.advance_time(Duration::from_nanos(1)); - } - } } fn debug_http_outcalls(env: &StateMachine) -> String { diff --git a/rs/ethereum/cketh/test_utils/src/response.rs b/rs/ethereum/cketh/test_utils/src/response.rs index 481c8fe4710..300b9bfb115 100644 --- a/rs/ethereum/cketh/test_utils/src/response.rs +++ b/rs/ethereum/cketh/test_utils/src/response.rs @@ -7,7 +7,7 @@ use crate::{ use ethers_core::abi::AbiDecode; use ethers_core::utils::rlp; use ic_ethereum_types::Address; -use serde_json::json; +use serde_json::{json, Value}; use std::str::FromStr; #[derive(Clone)] @@ -158,7 +158,12 @@ pub fn transaction_count_response(count: u32) -> String { } pub fn fee_history() -> ethers_core::types::FeeHistory { - let json_value = json!({ + let json_value = fee_history_json_value(); + serde_json::from_value(json_value).expect("BUG: invalid fee history") +} + +pub fn fee_history_json_value() -> Value { + json!({ "oldestBlock": "0x1134b57", "reward": [ ["0x25ed41c"], @@ -182,8 +187,7 @@ pub fn fee_history() -> ethers_core::types::FeeHistory { 0.5756615333333334, 0.3254294 ] - }); - serde_json::from_value(json_value).expect("BUG: invalid fee history") + }) } pub fn default_signed_eip_1559_transaction() -> ( diff --git a/rs/ethereum/cketh/test_utils/src/tests.rs b/rs/ethereum/cketh/test_utils/src/tests.rs index 7d14c47d815..bde0f58784d 100644 --- a/rs/ethereum/cketh/test_utils/src/tests.rs +++ b/rs/ethereum/cketh/test_utils/src/tests.rs @@ -1,11 +1,15 @@ use crate::response::{ default_erc20_signed_eip_1559_transaction, default_signed_eip_1559_transaction, - encode_transaction, hash_transaction, + encode_transaction, fee_history_json_value, hash_transaction, }; use crate::{ - DEFAULT_CKERC20_WITHDRAWAL_TRANSACTION, DEFAULT_CKERC20_WITHDRAWAL_TRANSACTION_HASH, - DEFAULT_WITHDRAWAL_TRANSACTION, DEFAULT_WITHDRAWAL_TRANSACTION_HASH, + DEFAULT_CKERC20_WITHDRAWAL_TRANSACTION, DEFAULT_CKERC20_WITHDRAWAL_TRANSACTION_FEE, + DEFAULT_CKERC20_WITHDRAWAL_TRANSACTION_HASH, DEFAULT_WITHDRAWAL_TRANSACTION, + DEFAULT_WITHDRAWAL_TRANSACTION_HASH, }; +use ic_cketh_minter::eth_rpc::FeeHistory; +use ic_cketh_minter::numeric::{GasAmount, Wei}; +use ic_cketh_minter::tx::estimate_transaction_fee; #[test] fn should_use_meaningful_constants() { @@ -29,3 +33,18 @@ fn should_use_meaningful_constants() { DEFAULT_CKERC20_WITHDRAWAL_TRANSACTION_HASH ); } + +#[test] +fn should_have_meaningful_ckerc20_withdrawal_transaction_fee() { + let fee_history: FeeHistory = serde_json::from_value(fee_history_json_value()).unwrap(); + let ckerc20_tx_price = estimate_transaction_fee(&fee_history).map(|gas_fee| { + gas_fee + .to_price(GasAmount::new(65_000)) + .max_transaction_fee() + }); + + assert_eq!( + ckerc20_tx_price, + Ok(Wei::from(DEFAULT_CKERC20_WITHDRAWAL_TRANSACTION_FEE)) + ); +} diff --git a/rs/state_machine_tests/src/lib.rs b/rs/state_machine_tests/src/lib.rs index 23f3e9d43bf..5a1debb2d8e 100644 --- a/rs/state_machine_tests/src/lib.rs +++ b/rs/state_machine_tests/src/lib.rs @@ -109,7 +109,7 @@ use ic_types::{ batch::{Batch, BatchMessages, XNetPayload}, consensus::certification::Certification, messages::{ - Blob, CallbackId, Certificate, CertificateDelegation, HttpCallContent, HttpCanisterUpdate, + Blob, Certificate, CertificateDelegation, HttpCallContent, HttpCanisterUpdate, HttpRequestEnvelope, Payload as MsgPayload, RejectContext, SignedIngress, SignedIngressContent, UserQuery, EXPECTED_MESSAGE_ID_LENGTH, }, @@ -118,7 +118,7 @@ use ic_types::{ }; pub use ic_types::{ ingress::{IngressState, IngressStatus, WasmResult}, - messages::{HttpRequestError, MessageId}, + messages::{CallbackId, HttpRequestError, MessageId}, time::Time, CanisterId, CryptoHashOfState, Cycles, PrincipalId, SubnetId, UserId, };