diff --git a/rs/ethereum/cketh/minter/src/dashboard.rs b/rs/ethereum/cketh/minter/src/dashboard.rs index 351122a85ab9..74bda1e54667 100644 --- a/rs/ethereum/cketh/minter/src/dashboard.rs +++ b/rs/ethereum/cketh/minter/src/dashboard.rs @@ -338,7 +338,7 @@ impl DashboardTemplate { let mut withdrawal_requests: Vec<_> = state .withdrawal_transactions - .withdrawal_requests_iter() + .requests_iter() .cloned() .map(|request| match request { WithdrawalRequest::CkEth(req) | WithdrawalRequest::SweeperFunding(req) => { diff --git a/rs/ethereum/cketh/minter/src/dashboard/tests.rs b/rs/ethereum/cketh/minter/src/dashboard/tests.rs index b5a5e224faf5..e23fee59e3c2 100644 --- a/rs/ethereum/cketh/minter/src/dashboard/tests.rs +++ b/rs/ethereum/cketh/minter/src/dashboard/tests.rs @@ -14,8 +14,8 @@ use ic_cketh_minter::numeric::{ use ic_cketh_minter::state::audit::{EventType, apply_state_transition}; use ic_cketh_minter::state::eth_logs_scraping::LogScrapingId; use ic_cketh_minter::state::transactions::{ - Erc20WithdrawalRequest, EthWithdrawalRequest, ReimbursementIndex, WithdrawalRequest, - create_transaction, + Erc20WithdrawalRequest, EthWithdrawalRequest, PipelineRequest, ReimbursementIndex, + WithdrawalRequest, }; use ic_cketh_minter::state::{MintedEvent, State}; use ic_cketh_minter::tx::{ @@ -533,10 +533,7 @@ fn should_display_pending_transactions_sorted_by_decreasing_cketh_ledger_burn_in TransactionStatus::Success, ), ] { - apply_state_transition( - &mut state, - &req.clone().into_accepted_withdrawal_request_event(), - ); + apply_state_transition(&mut state, &accepted_withdrawal_request_event(req.clone())); apply_state_transition( &mut state, &EventType::CreatedTransaction { @@ -560,7 +557,7 @@ fn should_display_pending_transactions_sorted_by_decreasing_cketh_ledger_burn_in ), ] { let withdrawal_id = req.cketh_ledger_burn_index(); - apply_state_transition(&mut state, &req.into_accepted_withdrawal_request_event()); + apply_state_transition(&mut state, &accepted_withdrawal_request_event(req)); apply_state_transition( &mut state, &EventType::CreatedTransaction { @@ -674,7 +671,7 @@ fn should_display_finalized_transactions_sorted_by_decreasing_cketh_ledger_burn_ ), ] { let id = req.cketh_ledger_burn_index(); - apply_state_transition(&mut state, &req.into_accepted_withdrawal_request_event()); + apply_state_transition(&mut state, &accepted_withdrawal_request_event(req)); apply_state_transition( &mut state, &EventType::CreatedTransaction { @@ -840,10 +837,7 @@ fn should_display_reimbursed_requests() { ), ] { let id = req.cketh_ledger_burn_index(); - apply_state_transition( - &mut state, - &req.clone().into_accepted_withdrawal_request_event(), - ); + apply_state_transition(&mut state, &accepted_withdrawal_request_event(req.clone())); apply_state_transition( &mut state, &EventType::CreatedTransaction { @@ -1223,7 +1217,7 @@ fn add_finalized_transactions(state: &mut State, num_transactions: u64) { TransactionStatus::Success, ); let id = req.cketh_ledger_burn_index(); - apply_state_transition(state, &req.into_accepted_withdrawal_request_event()); + apply_state_transition(state, &accepted_withdrawal_request_event(req)); apply_state_transition( state, &EventType::CreatedTransaction { @@ -1274,7 +1268,7 @@ fn add_reimbursed_transactions(state: &mut State, num_transactions: u64) { TransactionStatus::Failure, ); let id = req.cketh_ledger_burn_index(); - apply_state_transition(state, &req.into_accepted_withdrawal_request_event()); + apply_state_transition(state, &accepted_withdrawal_request_event(req)); apply_state_transition( state, &EventType::CreatedTransaction { @@ -1369,6 +1363,16 @@ pub fn ckusdt() -> CkErc20Token { } } +fn accepted_withdrawal_request_event(request: WithdrawalRequest) -> EventType { + match request { + WithdrawalRequest::CkEth(request) => EventType::AcceptedEthWithdrawalRequest(request), + WithdrawalRequest::CkErc20(request) => EventType::AcceptedErc20WithdrawalRequest(request), + WithdrawalRequest::SweeperFunding(request) => { + EventType::AcceptedSweeperFundingRequest(request) + } + } +} + fn cketh_withdrawal_request_with_index(ledger_burn_index: LedgerBurnIndex) -> EthWithdrawalRequest { const DEFAULT_WITHDRAWAL_AMOUNT: u128 = 1_100_000_000_000_000; const DEFAULT_PRINCIPAL: &str = @@ -1483,14 +1487,15 @@ fn ckerc20_withdrawal_flow( base_fee_per_gas: WeiPerGas::from(250_000_000_u64), max_priority_fee_per_gas: WeiPerGas::from(1_500_000_000_u64), }; - let transaction = create_transaction( - &withdrawal_request.clone().into(), - nonce, - gas_fee, - GasAmount::from(65_000_u32), - EthereumNetwork::Sepolia, - ) - .unwrap(); + let pipeline_request: WithdrawalRequest = withdrawal_request.clone().into(); + let transaction = pipeline_request + .create_transaction( + nonce, + gas_fee, + GasAmount::from(65_000_u32), + EthereumNetwork::Sepolia, + ) + .unwrap(); let dummy_signature = TransactionSignature { signature_y_parity: false, r: Default::default(), diff --git a/rs/ethereum/cketh/minter/src/guard/mod.rs b/rs/ethereum/cketh/minter/src/guard/mod.rs index 24d9766335b0..537fcb8d5dd5 100644 --- a/rs/ethereum/cketh/minter/src/guard/mod.rs +++ b/rs/ethereum/cketh/minter/src/guard/mod.rs @@ -30,7 +30,7 @@ impl RequestsGuardedByPrincipal for PendingWithdrawalRequests { } fn pending_requests_count(state: &State) -> usize { - state.withdrawal_transactions.withdrawal_requests_len() + state.withdrawal_transactions.requests_len() } } diff --git a/rs/ethereum/cketh/minter/src/guard/tests.rs b/rs/ethereum/cketh/minter/src/guard/tests.rs index 8a9914dd1ade..ac0f3481926d 100644 --- a/rs/ethereum/cketh/minter/src/guard/tests.rs +++ b/rs/ethereum/cketh/minter/src/guard/tests.rs @@ -70,7 +70,7 @@ mod retrieve_eth_guard { fn record_withdrawal_request(ledger_burn_index: LedgerBurnIndex) { mutate_state(|s| { s.withdrawal_transactions - .record_withdrawal_request(EthWithdrawalRequest { + .record_request(EthWithdrawalRequest { withdrawal_amount: Wei::ONE, destination: Address::ZERO, ledger_burn_index, diff --git a/rs/ethereum/cketh/minter/src/main.rs b/rs/ethereum/cketh/minter/src/main.rs index 828f3b5ee317..ff37b4b9c8c2 100644 --- a/rs/ethereum/cketh/minter/src/main.rs +++ b/rs/ethereum/cketh/minter/src/main.rs @@ -1136,7 +1136,7 @@ fn http_request(req: HttpRequest) -> HttpResponse { let now_nanos = ic_cdk::api::time(); let age_nanos = now_nanos.saturating_sub( s.withdrawal_transactions - .oldest_incomplete_withdrawal_timestamp() + .oldest_incomplete_request_timestamp() .unwrap_or(now_nanos), ); w.encode_gauge( diff --git a/rs/ethereum/cketh/minter/src/state.rs b/rs/ethereum/cketh/minter/src/state.rs index 48ae8762b47f..41019cdadba4 100644 --- a/rs/ethereum/cketh/minter/src/state.rs +++ b/rs/ethereum/cketh/minter/src/state.rs @@ -377,8 +377,7 @@ impl State { "BUG: unsupported ERC-20 token {}", request.erc20_contract_address ); - self.withdrawal_transactions - .record_withdrawal_request(request); + self.withdrawal_transactions.record_request(request); } pub fn record_finalized_transaction( @@ -420,7 +419,7 @@ impl State { .expect("BUG: missing finalized transaction"); let withdrawal_request = self .withdrawal_transactions - .get_processed_withdrawal_request(withdrawal_id) + .get_processed_request(withdrawal_id) .expect("BUG: missing withdrawal request"); let charged_tx_fee = match withdrawal_request { WithdrawalRequest::CkEth(req) | WithdrawalRequest::SweeperFunding(req) => req diff --git a/rs/ethereum/cketh/minter/src/state/audit.rs b/rs/ethereum/cketh/minter/src/state/audit.rs index 93502baeff1d..4ccccb112a97 100644 --- a/rs/ethereum/cketh/minter/src/state/audit.rs +++ b/rs/ethereum/cketh/minter/src/state/audit.rs @@ -72,7 +72,7 @@ pub fn apply_state_transition(state: &mut State, payload: &EventType) { EventType::AcceptedEthWithdrawalRequest(request) => { state .withdrawal_transactions - .record_withdrawal_request(request.clone()); + .record_request(request.clone()); } EventType::AcceptedSweeperFundingRequest(request) => { state.sweeper_funding.record_burn(request.withdrawal_amount); @@ -80,7 +80,7 @@ pub fn apply_state_transition(state: &mut State, payload: &EventType) { // funding reimbursable. state .withdrawal_transactions - .record_withdrawal_request(WithdrawalRequest::SweeperFunding(request.clone())); + .record_request(WithdrawalRequest::SweeperFunding(request.clone())); } EventType::CreatedTransaction { withdrawal_id, diff --git a/rs/ethereum/cketh/minter/src/state/tests.rs b/rs/ethereum/cketh/minter/src/state/tests.rs index d57aaa0d94d6..260cf266eb70 100644 --- a/rs/ethereum/cketh/minter/src/state/tests.rs +++ b/rs/ethereum/cketh/minter/src/state/tests.rs @@ -978,13 +978,13 @@ fn state_equivalence() { ledger_burn_index: LedgerBurnIndex::new(20), ..withdrawal_request1.clone() }; - let pending_withdrawal_requests: VecDeque = vec![ + let pending_requests: VecDeque = vec![ withdrawal_request1.clone().into(), withdrawal_request2.clone().into(), ] .into_iter() .collect(); - let processed_withdrawal_requests = btreemap! { + let processed_requests = btreemap! { LedgerBurnIndex::new(4) => EthWithdrawalRequest { withdrawal_amount: Wei::new(1_000_000_000_000), ledger_burn_index: LedgerBurnIndex::new(4), @@ -1107,8 +1107,8 @@ fn state_equivalence() { }), }; let builder = WithdrawalTransactionsBuilder::default() - .with_pending_withdrawal_requests(pending_withdrawal_requests) - .with_processed_withdrawal_requests(processed_withdrawal_requests) + .with_pending_requests(pending_requests) + .with_processed_requests(processed_requests) .with_created_tx(created_tx) .with_sent_tx(sent_tx) .with_finalized_tx(finalized_tx) @@ -1333,7 +1333,7 @@ fn state_equivalence() { state.is_equivalent_to(&State { withdrawal_transactions: builder .clone() - .with_pending_withdrawal_requests( + .with_pending_requests( vec![ withdrawal_request2.clone().into(), withdrawal_request1.clone().into() @@ -1352,9 +1352,7 @@ fn state_equivalence() { state.is_equivalent_to(&State { withdrawal_transactions: builder .clone() - .with_pending_withdrawal_requests( - vec![withdrawal_request1.into()].into_iter().collect() - ) + .with_pending_requests(vec![withdrawal_request1.into()].into_iter().collect()) .build(), ..state.clone() }), @@ -1511,7 +1509,7 @@ mod sweeper_funding { let request = state .withdrawal_transactions - .withdrawal_requests_iter() + .requests_iter() .next() .expect("BUG: the funding request was not recorded"); assert_matches!(request, WithdrawalRequest::SweeperFunding(_)); @@ -1532,7 +1530,7 @@ mod eth_balance { use crate::state::audit::{EventType, apply_state_transition}; use crate::state::tests::checked_sub; use crate::state::tests::{initial_state, received_eth_event}; - use crate::state::transactions::{EthWithdrawalRequest, WithdrawalRequest, create_transaction}; + use crate::state::transactions::{EthWithdrawalRequest, PipelineRequest, WithdrawalRequest}; use crate::state::{EthBalance, State}; use crate::test_fixtures::sweeper_funding_request; use crate::tx::{SignedEip1559TransactionRequest, TransactionSignature}; @@ -2008,20 +2006,19 @@ mod eth_balance { } fn apply(self, state: &mut State) -> TransactionReceipt { - let accepted_withdrawal_request_event = self - .withdrawal_request - .clone() - .into_accepted_withdrawal_request_event(); + let accepted_withdrawal_request_event = + accepted_withdrawal_request_event(self.withdrawal_request.clone()); apply_state_transition(state, &accepted_withdrawal_request_event); - let transaction = create_transaction( - &self.withdrawal_request, - self.nonce, - self.tx_fee, - self.gas_limit, - EthereumNetwork::Sepolia, - ) - .expect("BUG: failed to create transaction"); + let transaction = self + .withdrawal_request + .create_transaction( + self.nonce, + self.tx_fee, + self.gas_limit, + EthereumNetwork::Sepolia, + ) + .expect("BUG: failed to create transaction"); apply_state_transition( state, &EventType::CreatedTransaction { @@ -2072,6 +2069,18 @@ mod eth_balance { state } + fn accepted_withdrawal_request_event(request: WithdrawalRequest) -> EventType { + match request { + WithdrawalRequest::CkEth(request) => EventType::AcceptedEthWithdrawalRequest(request), + WithdrawalRequest::CkErc20(request) => { + EventType::AcceptedErc20WithdrawalRequest(request) + } + WithdrawalRequest::SweeperFunding(request) => { + EventType::AcceptedSweeperFundingRequest(request) + } + } + } + fn add_erc20_token(state: &mut State) { use crate::state::CkErc20Token; apply_state_transition( diff --git a/rs/ethereum/cketh/minter/src/state/transactions/mod.rs b/rs/ethereum/cketh/minter/src/state/transactions/mod.rs index 4dd3a03d9bc5..02f65dff6757 100644 --- a/rs/ethereum/cketh/minter/src/state/transactions/mod.rs +++ b/rs/ethereum/cketh/minter/src/state/transactions/mod.rs @@ -1,21 +1,23 @@ +mod request; + #[cfg(test)] pub(in crate::state) mod tests; +pub use request::PipelineRequest; + use crate::endpoints::{EthTransaction, RetrieveEthStatus, TxFinalizedStatus, WithdrawalStatus}; use crate::eth_logs::LedgerSubaccount; use crate::eth_rpc::Hash; use crate::eth_rpc_client::responses::TransactionReceipt; use crate::eth_rpc_client::responses::TransactionStatus; -use crate::lifecycle::EthereumNetwork; use crate::logs::INFO; use crate::map::MultiKeyMap; use crate::numeric::{ - CkTokenAmount, Erc20Value, GasAmount, LedgerBurnIndex, LedgerMintIndex, TransactionCount, + CkTokenAmount, Erc20Value, LedgerBurnIndex, LedgerMintIndex, TransactionCount, TransactionNonce, Wei, }; -use crate::state::event::EventType; use crate::tx::{ - Eip1559TransactionRequest, FinalizedEip1559Transaction, GasFeeEstimate, ResubmissionStrategy, + Eip1559TransactionRequest, FinalizedEip1559Transaction, GasFeeEstimate, SignedEip1559TransactionRequest, SignedTransactionRequest, TransactionRequest, }; use candid::Principal; @@ -108,18 +110,6 @@ impl WithdrawalRequest { } } - pub fn into_accepted_withdrawal_request_event(self) -> EventType { - match self { - WithdrawalRequest::CkEth(request) => EventType::AcceptedEthWithdrawalRequest(request), - WithdrawalRequest::CkErc20(request) => { - EventType::AcceptedErc20WithdrawalRequest(request) - } - WithdrawalRequest::SweeperFunding(request) => { - EventType::AcceptedSweeperFundingRequest(request) - } - } - } - pub fn match_parameter(&self, parameter: &WithdrawalSearchParameter) -> bool { use WithdrawalSearchParameter::*; match parameter { @@ -377,31 +367,38 @@ impl fmt::Debug for Erc20WithdrawalRequest { } } -/// State machine holding Ethereum transactions issued by the minter. +/// State machine holding Ethereum transactions issued by the minter from a **single sender +/// address**, on that address' **own nonce sequence** — generic over the request type `R` so a +/// second sender address can drive the same machinery on a nonce sequence of its own without +/// interfering with user withdrawals (`R = WithdrawalRequest`, wrapped by [`WithdrawalTransactions`]). +/// /// Overall the transaction lifecycle is as follows: -/// 1. A withdrawal request is enqueued and processed in a FIFO order. -/// 2. A transaction is created by either consuming a withdrawal request -/// (the first time a transaction is created for that nonce and burn index) -/// or re-submitting an already sent transaction for that nonce and burn index. +/// 1. The request is enqueued and processed in a FIFO order. +/// 2. A transaction is created by either consuming a request +/// (the first time a transaction is created for that nonce and id) +/// or re-submitting an already sent transaction for that nonce and id. /// 3. The transaction is signed via threshold ECDSA and recorded by either consuming the /// previously created transaction or re-submitting an already sent transaction as is. /// 4. The transaction is sent to Ethereum. There may have been multiple -/// sent transactions for that nonce and burn index in case of resubmissions. -/// 5. For a given nonce (and burn index), at most one sent transaction is finalized; a failed -/// one is reported as such. The other sent transactions for that nonce were never mined and -/// can be discarded. Paying the requester back is not the pipeline's concern — see +/// sent transactions for that nonce and id in case of resubmissions. +/// 5. For a given nonce (and id), at most one sent transaction is finalized; a failed one is +/// reported as such. The other sent transactions for that nonce were never mined and can be +/// discarded. Paying the requester back is not the pipeline's concern — see /// [`WithdrawalTransactions`]. #[derive(Clone, Eq, PartialEq, Debug)] -pub(in crate::state) struct TransactionPipeline { - pending_withdrawal_requests: VecDeque, - // Processed withdrawal requests (transaction created, sent, or finalized). - processed_withdrawal_requests: BTreeMap, - created_tx: MultiKeyMap, - sent_tx: MultiKeyMap>, - finalized_tx: MultiKeyMap, +pub(in crate::state) struct TransactionPipeline { + pending_requests: VecDeque, + // Processed requests (transaction created, sent, or finalized). + processed_requests: BTreeMap, + created_tx: MultiKeyMap, + sent_tx: MultiKeyMap>, + finalized_tx: MultiKeyMap, next_nonce: TransactionNonce, } +/// The pipeline sending from the minter's main address, on which user withdrawals travel. +pub(in crate::state) type MinterTransactionPipeline = TransactionPipeline; + #[derive(Clone, Eq, PartialEq, Debug)] pub enum CreateTransactionError { InsufficientTransactionFee { @@ -412,9 +409,9 @@ pub enum CreateTransactionError { } #[derive(Clone, Eq, PartialEq, Debug)] -pub enum ResubmitTransactionError { +pub enum ResubmitTransactionError { InsufficientTransactionFee { - ledger_burn_index: LedgerBurnIndex, + id: Id, transaction_nonce: TransactionNonce, allowed_max_transaction_fee: Wei, max_transaction_fee: Wei, @@ -431,11 +428,15 @@ pub(in crate::state) enum TransactionStage<'a> { Finalized(&'a FinalizedEip1559Transaction), } -impl TransactionPipeline { +/// One outcome of [`TransactionPipeline::create_resubmit_transactions`]: the fee-bumped transaction to +/// re-sign (paired with its pipeline id), or why it could not be bumped. +type ResubmitResult = Result<(Id, Eip1559TransactionRequest), ResubmitTransactionError>; + +impl TransactionPipeline { pub fn new(next_nonce: TransactionNonce) -> Self { Self { - pending_withdrawal_requests: VecDeque::new(), - processed_withdrawal_requests: BTreeMap::new(), + pending_requests: VecDeque::new(), + processed_requests: BTreeMap::new(), created_tx: MultiKeyMap::default(), sent_tx: MultiKeyMap::default(), finalized_tx: MultiKeyMap::default(), @@ -451,107 +452,74 @@ impl TransactionPipeline { self.next_nonce = new_nonce; } - pub fn record_withdrawal_request>(&mut self, request: R) { + pub fn record_request>(&mut self, request: Req) { let request = request.into(); - let burn_index = request.cketh_ledger_burn_index(); - if self - .pending_withdrawal_requests - .iter() - .any(|r| r.cketh_ledger_burn_index() == burn_index) - || self.created_tx.contains_alt(&burn_index) - || self.sent_tx.contains_alt(&burn_index) - || self.finalized_tx.contains_alt(&burn_index) + let id = request.id(); + if self.pending_requests.iter().any(|r| r.id() == id) + || self.created_tx.contains_alt(&id) + || self.sent_tx.contains_alt(&id) + || self.finalized_tx.contains_alt(&id) { - panic!("BUG: duplicate ckETH ledger burn index {burn_index}"); + panic!("BUG: duplicate transaction id {id:?}"); } - self.pending_withdrawal_requests.push_back(request); + self.pending_requests.push_back(request); } - /// Move an existing withdrawal request to the back of the queue. - pub fn reschedule_withdrawal_request>(&mut self, request: R) { - let request = request.into(); + /// Move an existing request to the back of the queue. + pub fn reschedule_request(&mut self, id: R::Id) { assert_eq!( - self.pending_withdrawal_requests + self.pending_requests .iter() - .filter(|r| r.cketh_ledger_burn_index() == request.cketh_ledger_burn_index()) + .filter(|r| r.id() == id) .count(), 1, - "BUG: expected exactly one withdrawal request with ckETH ledger burn index {}", - request.cketh_ledger_burn_index() + "BUG: expected exactly one request with id {id:?}" ); - self.remove_withdrawal_request(&request); - self.record_withdrawal_request(request); + let position = self + .pending_requests + .iter() + .position(|r| r.id() == id) + .expect("BUG: exactly one request with this id was just counted"); + let request = self + .pending_requests + .remove(position) + .expect("BUG: position was just found in the queue"); + self.record_request(request); } pub fn record_created_transaction( &mut self, - withdrawal_id: LedgerBurnIndex, + id: R::Id, transaction: Eip1559TransactionRequest, ) { - let withdrawal_request = self - .pending_withdrawal_requests + let position = self + .pending_requests .iter() - .find(|req| req.cketh_ledger_burn_index() == withdrawal_id) - .cloned() - .unwrap_or_else(|| panic!("BUG: withdrawal request {withdrawal_id} not found")); - assert!( - self.pending_withdrawal_requests - .contains(&withdrawal_request), - "BUG: withdrawal request not found" - ); - assert_eq!( - withdrawal_request.destination(), - transaction.destination, - "BUG: withdrawal request and transaction destination mismatch" - ); - match &withdrawal_request { - WithdrawalRequest::CkEth(req) | WithdrawalRequest::SweeperFunding(req) => { - assert!( - req.withdrawal_amount > transaction.amount, - "BUG: transaction amount should be the withdrawal amount deducted from transaction fees" - ); - } - WithdrawalRequest::CkErc20(_req) => { - assert_eq!( - Wei::ZERO, - transaction.amount, - "BUG: ERC-20 transaction amount should be zero" - ); - } - } + .position(|req| req.id() == id) + .unwrap_or_else(|| panic!("BUG: request {id:?} not found")); + let request = &self.pending_requests[position]; + request.assert_created_transaction(&transaction); + let resubmission = request.resubmission_strategy(); let nonce = self.next_nonce; assert_eq!(transaction.nonce, nonce, "BUG: transaction nonce mismatch"); self.next_nonce = self .next_nonce .checked_increment() .expect("Transaction nonce overflow"); - self.remove_withdrawal_request(&withdrawal_request); + let request = self + .pending_requests + .remove(position) + .expect("BUG: position was just found in the queue"); let transaction_request = TransactionRequest { transaction, - resubmission: match &withdrawal_request { - WithdrawalRequest::CkEth(cketh) | WithdrawalRequest::SweeperFunding(cketh) => { - ResubmissionStrategy::ReduceEthAmount { - withdrawal_amount: cketh.withdrawal_amount, - } - } - WithdrawalRequest::CkErc20(ckerc20) => ResubmissionStrategy::GuaranteeEthAmount { - allowed_max_transaction_fee: ckerc20.max_transaction_fee, - }, - }, + resubmission, }; assert_eq!( - self.created_tx.try_insert( - nonce, - withdrawal_request.cketh_ledger_burn_index(), - transaction_request - ), + self.created_tx + .try_insert(nonce, request.id(), transaction_request), Ok(()) ); - assert_eq!( - self.processed_withdrawal_requests - .insert(withdrawal_id, withdrawal_request), - None - ); + assert_eq!(self.processed_requests.insert(id, request), None); } pub fn record_signed_transaction( @@ -568,18 +536,14 @@ impl TransactionPipeline { "BUG: mismatch between sent transaction and created transaction" ); let signed_tx = created_tx.clone_resubmission_strategy(signed_transaction); - let (nonce, ledger_burn_index, _created_tx) = self + let (nonce, id, _created_tx) = self .created_tx .remove_entry(&signed_tx.as_ref().nonce()) .expect("BUG: missing created transaction"); if let Some(sent_tx) = self.sent_tx.get_mut(&nonce) { sent_tx.push(signed_tx); } else { - assert_eq!( - self.sent_tx - .try_insert(nonce, ledger_burn_index, vec![signed_tx]), - Ok(()) - ); + assert_eq!(self.sent_tx.try_insert(nonce, id, vec![signed_tx]), Ok(())); } } @@ -596,21 +560,21 @@ impl TransactionPipeline { &self, latest_transaction_count: TransactionCount, current_gas_fee: GasFeeEstimate, - ) -> Vec> { + ) -> Vec> { // If transaction count at block height H is c > 0, then transactions with nonces // 0, 1, ..., c - 1 were mined. If transaction count is 0, then no transactions were mined. // The nonce of the first pending transaction is then exactly c. let first_pending_tx_nonce: TransactionNonce = latest_transaction_count.change_units(); let mut transactions_to_resubmit = Vec::new(); - for (nonce, burn_index, signed_tx) in self + for (nonce, id, signed_tx) in self .sent_tx .iter() - .filter(|(nonce, _burn_index, _signed_tx)| *nonce >= &first_pending_tx_nonce) + .filter(|(nonce, _id, _signed_tx)| *nonce >= &first_pending_tx_nonce) { let last_signed_tx = signed_tx.last().expect("BUG: empty sent transactions list"); match last_signed_tx.resubmit(current_gas_fee.clone()) { Ok(Some(new_tx)) => { - transactions_to_resubmit.push(Ok((*burn_index, new_tx))); + transactions_to_resubmit.push(Ok((*id, new_tx))); } Ok(None) => { // the transaction fee is still up-to-date but because the transaction did not get included, @@ -623,7 +587,7 @@ impl TransactionPipeline { }) => { transactions_to_resubmit.push(Err( ResubmitTransactionError::InsufficientTransactionFee { - ledger_burn_index: *burn_index, + id: *id, transaction_nonce: *nonce, allowed_max_transaction_fee, max_transaction_fee: actual_max_transaction_fee, @@ -638,39 +602,34 @@ impl TransactionPipeline { pub fn record_resubmit_transaction(&mut self, new_tx: Eip1559TransactionRequest) { let nonce = new_tx.nonce; - let (ledger_burn_index, last_sent_tx) = - Self::expect_last_sent_tx_entry(&self.sent_tx, &nonce); + let (id, last_sent_tx) = Self::expect_last_sent_tx_entry(&self.sent_tx, &nonce); assert!( equal_ignoring_fee_and_amount(last_sent_tx.as_ref().transaction(), &new_tx), "BUG: mismatch between last sent transaction {last_sent_tx:?} and the transaction to resubmit {new_tx:?}" ); Self::cleanup_failed_resubmitted_transactions(&mut self.created_tx, &nonce); let new_tx = last_sent_tx.clone_resubmission_strategy(new_tx); - assert_eq!( - self.created_tx - .try_insert(nonce, *ledger_burn_index, new_tx), - Ok(()) - ); + assert_eq!(self.created_tx.try_insert(nonce, *id, new_tx), Ok(())); } pub fn sent_transactions_to_finalize( &self, finalized_transaction_count: &TransactionCount, - ) -> BTreeMap { + ) -> BTreeMap { let first_non_finalized_tx_nonce: TransactionNonce = finalized_transaction_count.change_units(); let mut transactions = BTreeMap::new(); for (_nonce, index, sent_txs) in self .sent_tx .iter() - .filter(|(nonce, _burn_index, _signed_txs)| *nonce < &first_non_finalized_tx_nonce) + .filter(|(nonce, _id, _signed_txs)| *nonce < &first_non_finalized_tx_nonce) { for sent_tx in sent_txs { if let Some(prev_index) = transactions.insert(sent_tx.as_ref().hash(), *index) { assert_eq!( prev_index, *index, - "BUG: duplicate transaction hash {} for burn indices {prev_index} and {index}", + "BUG: duplicate transaction hash {} for ids {prev_index:?} and {index:?}", sent_tx.as_ref().hash() ); } @@ -683,12 +642,12 @@ impl TransactionPipeline { /// superseded resubmissions, returning the finalized transaction. pub fn record_finalized_transaction( &mut self, - ledger_burn_index: LedgerBurnIndex, + id: R::Id, receipt: &TransactionReceipt, ) -> FinalizedEip1559Transaction { let sent_tx = self .sent_tx - .get_alt(&ledger_burn_index) + .get_alt(&id) .expect("BUG: missing sent transactions") .iter() .find(|sent_tx| sent_tx.as_ref().hash() == receipt.transaction_hash) @@ -706,16 +665,16 @@ impl TransactionPipeline { } assert_eq!( self.finalized_tx - .try_insert(nonce, ledger_burn_index, finalized_tx.clone()), + .try_insert(nonce, id, finalized_tx.clone()), Ok(()) ); finalized_tx } - pub fn withdrawal_requests_batch(&self, requested_batch_size: usize) -> Vec { + pub fn requests_batch(&self, requested_batch_size: usize) -> Vec { // The number of pending transaction nonces is counted and not the number of pending transactions // because a nonce may be associated with several distinct transactions (due to re-submission and dynamic fees). - // However, once a nonce is chosen for a withdrawal request, it's in our interest that the corresponding transaction be finalized asap. + // However, once a nonce is chosen for a request, it's in our interest that the corresponding transaction be finalized asap. // Limiting the number of transactions would be counter-productive. const MAX_NUM_PENDING_TRANSACTION_NONCES: usize = 1000; let unique_pending_transaction_nonces: BTreeSet<_> = @@ -725,41 +684,35 @@ impl TransactionPipeline { .saturating_sub(unique_pending_transaction_nonces.len()), requested_batch_size, ); - self.withdrawal_requests_iter() + self.requests_iter() .take(actual_batch_size) .cloned() .collect() } - pub fn withdrawal_requests_iter(&self) -> impl Iterator { - self.pending_withdrawal_requests.iter() + pub fn requests_iter(&self) -> impl Iterator { + self.pending_requests.iter() } - pub fn withdrawal_requests_len(&self) -> usize { - self.pending_withdrawal_requests.len() + pub fn requests_len(&self) -> usize { + self.pending_requests.len() } pub fn transactions_to_sign_iter( &self, - ) -> impl Iterator< - Item = ( - &TransactionNonce, - &LedgerBurnIndex, - &Eip1559TransactionRequest, - ), - > { + ) -> impl Iterator { self.created_tx .iter() - .map(|(nonce, ledger_burn_index, tx)| (nonce, ledger_burn_index, tx.as_ref())) + .map(|(nonce, id, tx)| (nonce, id, tx.as_ref())) } pub fn transactions_to_sign_batch( &self, batch_size: usize, - ) -> Vec<(LedgerBurnIndex, Eip1559TransactionRequest)> { + ) -> Vec<(R::Id, Eip1559TransactionRequest)> { self.transactions_to_sign_iter() .take(batch_size) - .map(|(_nonce, withdrawal_id, tx)| (*withdrawal_id, tx.clone())) + .map(|(_nonce, id, tx)| (*id, tx.clone())) .collect() } @@ -771,10 +724,10 @@ impl TransactionPipeline { let first_pending_tx_nonce: TransactionNonce = latest_transaction_count.change_units(); self.sent_tx .iter() - .filter_map(move |(nonce, ledger_burn_index, txs)| { + .filter_map(move |(nonce, id, txs)| { txs.last() - .map(|tx| (nonce, ledger_burn_index, tx)) - .filter(|(nonce, _ledger_burn_index, _tx)| *nonce >= &first_pending_tx_nonce) + .map(|tx| (nonce, id, tx)) + .filter(|(nonce, _id, _tx)| *nonce >= &first_pending_tx_nonce) }) .take(batch_size) .map(|(_nonce, _index, tx)| tx.as_ref()) @@ -787,7 +740,7 @@ impl TransactionPipeline { ) -> impl Iterator< Item = ( &TransactionNonce, - &LedgerBurnIndex, + &R::Id, Vec<&SignedEip1559TransactionRequest>, ), > { @@ -796,47 +749,35 @@ impl TransactionPipeline { .map(|(nonce, index, txs)| (nonce, index, txs.iter().map(|tx| tx.as_ref()).collect())) } - pub fn get_finalized_transaction( - &self, - burn_index: &LedgerBurnIndex, - ) -> Option<&FinalizedEip1559Transaction> { - self.finalized_tx.get_alt(burn_index) + pub fn get_finalized_transaction(&self, id: &R::Id) -> Option<&FinalizedEip1559Transaction> { + self.finalized_tx.get_alt(id) } - pub fn processed_withdrawal_requests_iter(&self) -> impl Iterator { - self.processed_withdrawal_requests.values() + pub fn processed_requests_iter(&self) -> impl Iterator { + self.processed_requests.values() } - /// How far the transaction for `burn_index` has got, or `None` if the pipeline holds none. - pub fn transaction_stage(&self, burn_index: &LedgerBurnIndex) -> Option> { - if let Some(tx) = self.created_tx.get_alt(burn_index) { + /// How far the transaction for `id` has got, or `None` if the pipeline holds none. + pub fn transaction_stage(&self, id: &R::Id) -> Option> { + if let Some(tx) = self.created_tx.get_alt(id) { return Some(TransactionStage::Created(tx.as_ref())); } // The last one sent is the one with the highest fee, so it is the one that may be mined. - if let Some(tx) = self.sent_tx.get_alt(burn_index).and_then(|txs| txs.last()) { + if let Some(tx) = self.sent_tx.get_alt(id).and_then(|txs| txs.last()) { return Some(TransactionStage::Sent(tx)); } self.finalized_tx - .get_alt(burn_index) + .get_alt(id) .map(TransactionStage::Finalized) } - pub fn get_processed_withdrawal_request( - &self, - burn_index: &LedgerBurnIndex, - ) -> Option<&WithdrawalRequest> { - self.processed_withdrawal_requests.get(burn_index) + pub fn get_processed_request(&self, id: &R::Id) -> Option<&R> { + self.processed_requests.get(id) } pub fn finalized_transactions_iter( &self, - ) -> impl Iterator< - Item = ( - &TransactionNonce, - &LedgerBurnIndex, - &FinalizedEip1559Transaction, - ), - > { + ) -> impl Iterator { self.finalized_tx.iter() } @@ -845,28 +786,22 @@ impl TransactionPipeline { } pub fn has_pending_requests(&self) -> bool { - !self.pending_withdrawal_requests.is_empty() - || !self.created_tx.is_empty() - || !self.sent_tx.is_empty() - } - - fn remove_withdrawal_request(&mut self, request: &WithdrawalRequest) { - self.pending_withdrawal_requests.retain(|r| r != request); + !self.pending_requests.is_empty() || !self.created_tx.is_empty() || !self.sent_tx.is_empty() } fn expect_last_sent_tx_entry<'a>( - sent_tx: &'a MultiKeyMap>, + sent_tx: &'a MultiKeyMap>, nonce: &TransactionNonce, - ) -> (&'a LedgerBurnIndex, &'a SignedTransactionRequest) { - let (ledger_burn_index, sent_txs) = sent_tx + ) -> (&'a R::Id, &'a SignedTransactionRequest) { + let (id, sent_txs) = sent_tx .get_entry(nonce) .expect("BUG: sent transaction not found"); let last_sent_tx = sent_txs.last().expect("BUG: empty sent transactions list"); - (ledger_burn_index, last_sent_tx) + (id, last_sent_tx) } fn cleanup_failed_resubmitted_transactions( - created_tx: &mut MultiKeyMap, + created_tx: &mut MultiKeyMap, nonce: &TransactionNonce, ) { use crate::logs::INFO; @@ -884,17 +819,17 @@ impl TransactionPipeline { pub fn is_equivalent_to(&self, other: &Self) -> Result<(), String> { use ic_utils_ensure::ensure_eq; - fn sorted_requests(requests: &VecDeque) -> Vec { + fn sorted_requests(requests: &VecDeque) -> Vec { let mut buf: Vec<_> = requests.iter().cloned().collect(); - buf.sort_unstable_by_key(|req| req.cketh_ledger_burn_index()); + buf.sort_unstable_by_key(|req| req.id()); buf } - // We can reorder request in `reschedule_withdrawal_request`. The audit log won't + // We can reorder request in `reschedule_request`. The audit log won't // reflect this change, so we must sort the queues before comparing them. ensure_eq!( - sorted_requests(&self.pending_withdrawal_requests), - sorted_requests(&other.pending_withdrawal_requests) + sorted_requests(&self.pending_requests), + sorted_requests(&other.pending_requests) ); ensure_eq!(self.created_tx, other.created_tx); ensure_eq!(self.sent_tx, other.sent_tx); @@ -905,13 +840,13 @@ impl TransactionPipeline { } } -/// The minter's transaction pipeline, carrying user withdrawals, together with the reimbursement -/// bookkeeping only a withdrawal can need: a failed ckETH/ckERC20 transaction pays the user back, -/// so the send machinery and the ledger-side refund have to stay in step. +/// The minter's main-address pipeline, carrying user withdrawals, together with the reimbursement +/// bookkeeping that only a withdrawal can need: a failed ckETH/ckERC20 transaction pays the user +/// back, so the pipeline's send machinery and the ledger-side refund have to stay in step. #[derive(Clone, Eq, PartialEq, Debug)] pub struct WithdrawalTransactions { - pipeline: TransactionPipeline, - /// Requests whose transaction was created but has not finally settled, and which would + pipeline: MinterTransactionPipeline, + /// Requests whose transaction was created but has not yet finally settled, and which would /// therefore have to be paid back if it failed. maybe_reimburse: BTreeSet, reimbursement_requests: BTreeMap, @@ -928,16 +863,15 @@ impl WithdrawalTransactions { } } - /// Record a created transaction, and remember the request may still need paying back. + /// Record a created transaction, and remember that the request may still need paying back. pub fn record_created_transaction( &mut self, - withdrawal_id: LedgerBurnIndex, + id: LedgerBurnIndex, transaction: Eip1559TransactionRequest, ) { - self.pipeline - .record_created_transaction(withdrawal_id, transaction); - if self.is_reimbursable(&withdrawal_id) { - assert!(self.maybe_reimburse.insert(withdrawal_id)); + self.pipeline.record_created_transaction(id, transaction); + if self.is_reimbursable(&id) { + assert!(self.maybe_reimburse.insert(id)); } } @@ -945,7 +879,7 @@ impl WithdrawalTransactions { /// funding never is, so it is never armed for reimbursement in the first place. fn is_reimbursable(&self, withdrawal_id: &LedgerBurnIndex) -> bool { self.pipeline - .get_processed_withdrawal_request(withdrawal_id) + .get_processed_request(withdrawal_id) .expect("BUG: missing processed withdrawal request") .is_reimbursable() } @@ -969,7 +903,7 @@ impl WithdrawalTransactions { let request = self .pipeline - .get_processed_withdrawal_request(&ledger_burn_index) + .get_processed_request(&ledger_burn_index) .expect("BUG: missing processed withdrawal request"); if receipt.status != TransactionStatus::Failure { return; @@ -1043,12 +977,12 @@ impl WithdrawalTransactions { self.pipeline.update_next_transaction_nonce(new_nonce) } - pub fn record_withdrawal_request>(&mut self, request: R) { - self.pipeline.record_withdrawal_request(request) + pub fn record_request>(&mut self, request: Req) { + self.pipeline.record_request(request) } - pub fn reschedule_withdrawal_request>(&mut self, request: R) { - self.pipeline.reschedule_withdrawal_request(request) + pub fn reschedule_request(&mut self, id: LedgerBurnIndex) { + self.pipeline.reschedule_request(id) } pub fn record_signed_transaction( @@ -1062,7 +996,7 @@ impl WithdrawalTransactions { &self, latest_transaction_count: TransactionCount, current_gas_fee: GasFeeEstimate, - ) -> Vec> { + ) -> Vec> { self.pipeline .create_resubmit_transactions(latest_transaction_count, current_gas_fee) } @@ -1079,17 +1013,16 @@ impl WithdrawalTransactions { .sent_transactions_to_finalize(finalized_transaction_count) } - pub fn withdrawal_requests_batch(&self, requested_batch_size: usize) -> Vec { - self.pipeline - .withdrawal_requests_batch(requested_batch_size) + pub fn requests_batch(&self, requested_batch_size: usize) -> Vec { + self.pipeline.requests_batch(requested_batch_size) } - pub fn withdrawal_requests_iter(&self) -> impl Iterator { - self.pipeline.withdrawal_requests_iter() + pub fn requests_iter(&self) -> impl Iterator { + self.pipeline.requests_iter() } - pub fn withdrawal_requests_len(&self) -> usize { - self.pipeline.withdrawal_requests_len() + pub fn requests_len(&self) -> usize { + self.pipeline.requests_len() } pub fn transactions_to_sign_iter( @@ -1139,11 +1072,11 @@ impl WithdrawalTransactions { self.pipeline.get_finalized_transaction(burn_index) } - pub fn get_processed_withdrawal_request( + pub fn get_processed_request( &self, burn_index: &LedgerBurnIndex, ) -> Option<&WithdrawalRequest> { - self.pipeline.get_processed_withdrawal_request(burn_index) + self.pipeline.get_processed_request(burn_index) } pub fn finalized_transactions_iter( @@ -1231,6 +1164,11 @@ impl WithdrawalTransactions { ); } + /// Arm the reimbursement for a withdrawal whose transaction failed on chain. + /// + /// # Panics + /// If the withdrawal is still armed for reimbursement, or was already reimbursed — either + /// would let the same burn be minted back twice. pub fn record_reimbursement_request( &mut self, index: ReimbursementIndex, @@ -1253,14 +1191,15 @@ impl WithdrawalTransactions { ); } - pub fn maybe_reimburse_requests_iter(&self) -> impl Iterator { + fn maybe_reimburse_requests_iter(&self) -> impl Iterator { self.maybe_reimburse .iter() - .filter_map(|index| self.pipeline.get_processed_withdrawal_request(index)) + .filter_map(|index| self.pipeline.get_processed_request(index)) } - pub fn oldest_incomplete_withdrawal_timestamp(&self) -> Option { - self.withdrawal_requests_iter() + /// Whether any request is still in flight, either awaiting a transaction or a reimbursement. + pub fn oldest_incomplete_request_timestamp(&self) -> Option { + self.requests_iter() .chain(self.maybe_reimburse_requests_iter()) .flat_map(|req| req.created_at().into_iter()) .min() @@ -1275,7 +1214,7 @@ impl WithdrawalTransactions { Option<&Eip1559TransactionRequest>, )> { // Pending requests matching the given search parameter - let pending = self.pipeline.withdrawal_requests_iter().filter_map(|r| { + let pending = self.pipeline.requests_iter().filter_map(|r| { r.match_parameter(parameter) .then_some((r, WithdrawalStatus::Pending, None)) }); @@ -1283,7 +1222,7 @@ impl WithdrawalTransactions { // Processed withdrawal requests matching the given search parameter. let processed = self .pipeline - .processed_withdrawal_requests_iter() + .processed_requests_iter() .filter(|r| r.match_parameter(parameter)) .map(|request| { match self.processed_transaction_status(&request.cketh_ledger_burn_index()) { @@ -1308,7 +1247,7 @@ impl WithdrawalTransactions { pub fn transaction_status(&self, burn_index: &LedgerBurnIndex) -> RetrieveEthStatus { if self .pipeline - .withdrawal_requests_iter() + .requests_iter() .any(|r| &r.cketh_ledger_burn_index() == burn_index) { return RetrieveEthStatus::Pending; @@ -1372,101 +1311,6 @@ impl WithdrawalTransactions { } } -/// Creates an EIP-1559 transaction for the given withdrawal request. -/// The transaction fees are paid by the beneficiary, -/// meaning that the fees will be deducted from the withdrawal amount. -/// -/// # Errors -/// * `CreateTransactionError::InsufficientTransactionFee` if the ETH withdrawal amount does not cover the transaction fee. -pub fn create_transaction( - withdrawal_request: &WithdrawalRequest, - nonce: TransactionNonce, - gas_fee_estimate: GasFeeEstimate, - gas_limit: GasAmount, - ethereum_network: EthereumNetwork, -) -> Result { - assert!( - gas_limit > GasAmount::ZERO, - "BUG: gas limit should be non-zero" - ); - match withdrawal_request { - WithdrawalRequest::CkEth(EthWithdrawalRequest { - withdrawal_amount, - destination, - ledger_burn_index, - .. - }) - | WithdrawalRequest::SweeperFunding(EthWithdrawalRequest { - withdrawal_amount, - destination, - ledger_burn_index, - .. - }) => { - let transaction_price = gas_fee_estimate.to_price(gas_limit); - let max_transaction_fee = transaction_price.max_transaction_fee(); - let tx_amount = match withdrawal_amount.checked_sub(max_transaction_fee) { - Some(tx_amount) => tx_amount, - None => { - return Err(CreateTransactionError::InsufficientTransactionFee { - cketh_ledger_burn_index: *ledger_burn_index, - allowed_max_transaction_fee: *withdrawal_amount, - actual_max_transaction_fee: max_transaction_fee, - }); - } - }; - Ok(Eip1559TransactionRequest { - chain_id: ethereum_network.chain_id(), - nonce, - max_priority_fee_per_gas: transaction_price.max_priority_fee_per_gas, - max_fee_per_gas: transaction_price.max_fee_per_gas, - gas_limit: transaction_price.gas_limit, - destination: *destination, - amount: tx_amount, - data: Vec::new(), - access_list: Default::default(), - }) - } - WithdrawalRequest::CkErc20(request) => { - // The transaction fee is already paid and must be at most - // the `max_transaction_fee` in the withdrawal request, which, given a gas limit, gives us an upper bound on - // the `max_fee_per_gas`. We allocate the maximum from the beginning to minimize - // transaction resubmissions: even if the `base_fee_per_gas` increases considerably, - // the transaction could still make it as long as `transaction.max_fee_per_gas >= block.base_fee_per_gas`, - // since the `priority_fee_per_gas` received by the miner is capped to (see https://eips.ethereum.org/EIPS/eip-1559) - // min(transaction.max_priority_fee_per_gas, transaction.max_fee_per_gas - block.base_fee_per_gas). - let request_max_fee_per_gas = request - .max_transaction_fee - .into_wei_per_gas(gas_limit) - .expect("BUG: gas_limit should be non-zero"); - let actual_min_max_fee_per_gas = gas_fee_estimate.min_max_fee_per_gas(); - if actual_min_max_fee_per_gas > request_max_fee_per_gas { - return Err(CreateTransactionError::InsufficientTransactionFee { - cketh_ledger_burn_index: request.cketh_ledger_burn_index, - allowed_max_transaction_fee: request.max_transaction_fee, - actual_max_transaction_fee: actual_min_max_fee_per_gas - .transaction_cost(gas_limit) - .unwrap_or(Wei::MAX), - }); - } - Ok(Eip1559TransactionRequest { - chain_id: ethereum_network.chain_id(), - nonce, - max_priority_fee_per_gas: gas_fee_estimate.max_priority_fee_per_gas, - max_fee_per_gas: request_max_fee_per_gas, - gas_limit, - destination: request.erc20_contract_address, - amount: Wei::ZERO, - data: TransactionCallData::Erc20Transfer { - to: request.destination, - value: request.withdrawal_amount, - } - .encode(), - access_list: Default::default(), - }) - } - } -} - // First 4 bytes of keccak256(transfer(address,uint256)) const ERC_20_TRANSFER_FUNCTION_SELECTOR: [u8; 4] = hex_literal::hex!("a9059cbb"); diff --git a/rs/ethereum/cketh/minter/src/state/transactions/request.rs b/rs/ethereum/cketh/minter/src/state/transactions/request.rs new file mode 100644 index 000000000000..14d8780e6837 --- /dev/null +++ b/rs/ethereum/cketh/minter/src/state/transactions/request.rs @@ -0,0 +1,181 @@ +//! What it takes for a request to travel a [`TransactionPipeline`](super::TransactionPipeline), +//! and the minter's own implementation of it. + +use super::{CreateTransactionError, EthWithdrawalRequest, TransactionCallData, WithdrawalRequest}; +use crate::lifecycle::EthereumNetwork; +use crate::numeric::{GasAmount, LedgerBurnIndex, TransactionNonce, Wei}; +use crate::tx::{Eip1559TransactionRequest, GasFeeEstimate, ResubmissionStrategy}; +use std::fmt; + +/// A request that can flow through a `TransactionPipeline`: it carries an identity used as the +/// pipeline's alternate map key, and knows the EIP-1559 transaction it turns into. +/// +/// Implemented so far only by [`WithdrawalRequest`], the minter's main-address pipeline +/// (`Id = LedgerBurnIndex`); a second sender address will bring a second implementation. +pub trait PipelineRequest { + /// The pipeline's alternate map key — a ckETH `LedgerBurnIndex` for withdrawals. + type Id: Copy + Ord + fmt::Debug; + + /// Why [`Self::create_transaction`] could not build a transaction. A request that always funds + /// its own fee can set this to [`std::convert::Infallible`], making the failure unrepresentable + /// rather than merely unreachable. + type Error; + + /// The identity of this request, used as the pipeline's alternate map key. + fn id(&self) -> Self::Id; + + /// The fee-bump strategy for this request's resubmitted transactions. + fn resubmission_strategy(&self) -> ResubmissionStrategy; + + /// Assert that a freshly created transaction is consistent with the request: it goes to the + /// right address, and moves the right amount. + fn assert_created_transaction(&self, transaction: &Eip1559TransactionRequest); + + /// Creates the EIP-1559 transaction that fulfils this request. + /// + /// # Errors + /// * [`Self::Error`] if the request cannot cover the transaction fee. + fn create_transaction( + &self, + nonce: TransactionNonce, + gas_fee_estimate: GasFeeEstimate, + gas_limit: GasAmount, + ethereum_network: EthereumNetwork, + ) -> Result; +} + +impl PipelineRequest for WithdrawalRequest { + type Id = LedgerBurnIndex; + type Error = CreateTransactionError; + + fn id(&self) -> LedgerBurnIndex { + self.cketh_ledger_burn_index() + } + + fn resubmission_strategy(&self) -> ResubmissionStrategy { + match self { + WithdrawalRequest::CkEth(cketh) | WithdrawalRequest::SweeperFunding(cketh) => { + ResubmissionStrategy::ReduceEthAmount { + withdrawal_amount: cketh.withdrawal_amount, + } + } + WithdrawalRequest::CkErc20(ckerc20) => ResubmissionStrategy::GuaranteeEthAmount { + allowed_max_transaction_fee: ckerc20.max_transaction_fee, + }, + } + } + + fn assert_created_transaction(&self, transaction: &Eip1559TransactionRequest) { + assert_eq!( + self.destination(), + transaction.destination, + "BUG: request and transaction destination mismatch" + ); + match self { + WithdrawalRequest::CkEth(req) | WithdrawalRequest::SweeperFunding(req) => { + assert!( + req.withdrawal_amount > transaction.amount, + "BUG: transaction amount should be the withdrawal amount deducted from transaction fees" + ); + } + WithdrawalRequest::CkErc20(_req) => { + assert_eq!( + Wei::ZERO, + transaction.amount, + "BUG: ERC-20 transaction amount should be zero" + ); + } + } + } + + /// The transaction fees are paid by the beneficiary, meaning that the fees will be deducted + /// from the withdrawal amount. + fn create_transaction( + &self, + nonce: TransactionNonce, + gas_fee_estimate: GasFeeEstimate, + gas_limit: GasAmount, + ethereum_network: EthereumNetwork, + ) -> Result { + assert!( + gas_limit > GasAmount::ZERO, + "BUG: gas limit should be non-zero" + ); + match self { + WithdrawalRequest::CkEth(EthWithdrawalRequest { + withdrawal_amount, + destination, + ledger_burn_index, + .. + }) + | WithdrawalRequest::SweeperFunding(EthWithdrawalRequest { + withdrawal_amount, + destination, + ledger_burn_index, + .. + }) => { + let transaction_price = gas_fee_estimate.to_price(gas_limit); + let max_transaction_fee = transaction_price.max_transaction_fee(); + let tx_amount = match withdrawal_amount.checked_sub(max_transaction_fee) { + Some(tx_amount) => tx_amount, + None => { + return Err(CreateTransactionError::InsufficientTransactionFee { + cketh_ledger_burn_index: *ledger_burn_index, + allowed_max_transaction_fee: *withdrawal_amount, + actual_max_transaction_fee: max_transaction_fee, + }); + } + }; + Ok(Eip1559TransactionRequest { + chain_id: ethereum_network.chain_id(), + nonce, + max_priority_fee_per_gas: transaction_price.max_priority_fee_per_gas, + max_fee_per_gas: transaction_price.max_fee_per_gas, + gas_limit: transaction_price.gas_limit, + destination: *destination, + amount: tx_amount, + data: Vec::new(), + access_list: Default::default(), + }) + } + WithdrawalRequest::CkErc20(request) => { + // The transaction fee is already paid and must be at most + // the `max_transaction_fee` in the withdrawal request, which, given a gas limit, gives us an upper bound on + // the `max_fee_per_gas`. We allocate the maximum from the beginning to minimize + // transaction resubmissions: even if the `base_fee_per_gas` increases considerably, + // the transaction could still make it as long as `transaction.max_fee_per_gas >= block.base_fee_per_gas`, + // since the `priority_fee_per_gas` received by the miner is capped to (see https://eips.ethereum.org/EIPS/eip-1559) + // min(transaction.max_priority_fee_per_gas, transaction.max_fee_per_gas - block.base_fee_per_gas). + let request_max_fee_per_gas = request + .max_transaction_fee + .into_wei_per_gas(gas_limit) + .expect("BUG: gas_limit should be non-zero"); + let actual_min_max_fee_per_gas = gas_fee_estimate.min_max_fee_per_gas(); + if actual_min_max_fee_per_gas > request_max_fee_per_gas { + return Err(CreateTransactionError::InsufficientTransactionFee { + cketh_ledger_burn_index: request.cketh_ledger_burn_index, + allowed_max_transaction_fee: request.max_transaction_fee, + actual_max_transaction_fee: actual_min_max_fee_per_gas + .transaction_cost(gas_limit) + .unwrap_or(Wei::MAX), + }); + } + Ok(Eip1559TransactionRequest { + chain_id: ethereum_network.chain_id(), + nonce, + max_priority_fee_per_gas: gas_fee_estimate.max_priority_fee_per_gas, + max_fee_per_gas: request_max_fee_per_gas, + gas_limit, + destination: request.erc20_contract_address, + amount: Wei::ZERO, + data: TransactionCallData::Erc20Transfer { + to: request.destination, + value: request.withdrawal_amount, + } + .encode(), + access_list: Default::default(), + }) + } + } + } +} diff --git a/rs/ethereum/cketh/minter/src/state/transactions/tests.rs b/rs/ethereum/cketh/minter/src/state/transactions/tests.rs index d59e8d535881..19ba2fe7d67e 100644 --- a/rs/ethereum/cketh/minter/src/state/transactions/tests.rs +++ b/rs/ethereum/cketh/minter/src/state/transactions/tests.rs @@ -6,10 +6,9 @@ use crate::lifecycle::EthereumNetwork; use crate::numeric::{ BlockNumber, Erc20Value, GasAmount, LedgerBurnIndex, TransactionNonce, Wei, WeiPerGas, }; -use crate::state::transactions::TransactionPipeline; use crate::state::transactions::{ - Erc20WithdrawalRequest, EthWithdrawalRequest, WithdrawalRequest, WithdrawalTransactions, - create_transaction, + Erc20WithdrawalRequest, EthWithdrawalRequest, MinterTransactionPipeline, PipelineRequest, + WithdrawalRequest, WithdrawalTransactions, }; use crate::tx::{ AccessList, Eip1559TransactionRequest, GasFeeEstimate, SignedEip1559TransactionRequest, @@ -42,7 +41,7 @@ mod withdrawal_transactions { TransactionStatus, WithdrawalRequest, WithdrawalTransactions, }; - mod record_withdrawal_request { + mod record_request { use super::*; use crate::state::transactions::WithdrawalRequest; use crate::state::transactions::tests::{ @@ -55,10 +54,10 @@ mod withdrawal_transactions { fn should_record_withdrawal_request() { fn test + Clone>(withdrawal_request: R) { let mut transactions = WithdrawalTransactions::new(TransactionNonce::ZERO); - transactions.record_withdrawal_request(withdrawal_request.clone()); + transactions.record_request(withdrawal_request.clone()); assert_eq!( - transactions.withdrawal_requests_batch(5), + transactions.requests_batch(5), vec![withdrawal_request.into()] ); } @@ -79,11 +78,11 @@ mod withdrawal_transactions { duplicate_index: S, ) { let mut transactions = WithdrawalTransactions::new(TransactionNonce::ZERO); - transactions.record_withdrawal_request(withdrawal_request.clone()); + transactions.record_request(withdrawal_request.clone()); expect_panic_with_message( - || transactions.record_withdrawal_request(duplicate_index.clone()), - "duplicate ckETH ledger burn index", + || transactions.record_request(duplicate_index.clone()), + "duplicate transaction id", ); let created_tx = create_and_record_transaction( @@ -92,14 +91,14 @@ mod withdrawal_transactions { gas_fee_estimate(), ); expect_panic_with_message( - || transactions.record_withdrawal_request(duplicate_index.clone()), - "duplicate ckETH ledger burn index", + || transactions.record_request(duplicate_index.clone()), + "duplicate transaction id", ); let signed_tx = create_and_record_signed_transaction(&mut transactions, created_tx); expect_panic_with_message( - || transactions.record_withdrawal_request(duplicate_index.clone()), - "duplicate ckETH ledger burn index", + || transactions.record_request(duplicate_index.clone()), + "duplicate transaction id", ); transactions.record_finalized_transaction( @@ -107,8 +106,8 @@ mod withdrawal_transactions { transaction_receipt(&signed_tx, TransactionStatus::Success), ); expect_panic_with_message( - || transactions.record_withdrawal_request(duplicate_index.clone()), - "duplicate ckETH ledger burn index", + || transactions.record_request(duplicate_index.clone()), + "duplicate transaction id", ); } @@ -133,7 +132,7 @@ mod withdrawal_transactions { } } - mod withdrawal_requests_batch { + mod requests_batch { use super::*; use crate::state::transactions::WithdrawalRequest; use crate::state::transactions::tests::{ @@ -147,7 +146,7 @@ mod withdrawal_transactions { #[test] fn should_be_empty_when_no_withdrawal_requests() { let transactions = WithdrawalTransactions::new(TransactionNonce::ZERO); - assert_eq!(transactions.withdrawal_requests_batch(5), vec![]); + assert_eq!(transactions.requests_batch(5), vec![]); } #[test] @@ -157,13 +156,13 @@ mod withdrawal_transactions { let withdrawal_requests: [WithdrawalRequest; 5] = create_and_record_ck_withdrawal_requests(&mut transactions, &mut rng); - let requests = transactions.withdrawal_requests_batch(0); + let requests = transactions.requests_batch(0); assert_eq!(requests, vec![]); - let requests = transactions.withdrawal_requests_batch(1); + let requests = transactions.requests_batch(1); assert_eq!(requests.as_slice(), &withdrawal_requests[0..=0]); - let requests = transactions.withdrawal_requests_batch(2); + let requests = transactions.requests_batch(2); assert_eq!(&requests, &withdrawal_requests[0..=1]); } @@ -175,7 +174,7 @@ mod withdrawal_transactions { let withdrawal_requests: [WithdrawalRequest; 3] = create_and_record_ck_withdrawal_requests(&mut transactions, &mut rng); - let requests = transactions.withdrawal_requests_batch(batch_size); + let requests = transactions.requests_batch(batch_size); prop_assert_eq!(requests, withdrawal_requests); } @@ -199,7 +198,7 @@ mod withdrawal_transactions { }); assert_eq!( - transactions.withdrawal_requests_batch(3).as_slice(), + transactions.requests_batch(3).as_slice(), &withdrawal_requests[997..=999] ); @@ -209,7 +208,7 @@ mod withdrawal_transactions { rng.r#gen(), ); assert_eq!( - transactions.withdrawal_requests_batch(3).as_slice(), + transactions.requests_batch(3).as_slice(), &withdrawal_requests[998..=999] ); @@ -219,7 +218,7 @@ mod withdrawal_transactions { rng.r#gen(), ); assert_eq!( - transactions.withdrawal_requests_batch(3).as_slice(), + transactions.requests_batch(3).as_slice(), &withdrawal_requests[999..=999] ); @@ -228,7 +227,7 @@ mod withdrawal_transactions { withdrawal_requests[999].clone(), rng.r#gen(), ); - assert_eq!(transactions.withdrawal_requests_batch(3), vec![]); + assert_eq!(transactions.requests_batch(3), vec![]); } fn create_and_record_pending_transaction>( @@ -244,7 +243,7 @@ mod withdrawal_transactions { } } - mod reschedule_withdrawal_request { + mod reschedule_request { use crate::numeric::TransactionNonce; use crate::state::transactions::WithdrawalTransactions; use crate::state::transactions::tests::create_and_record_ck_withdrawal_requests; @@ -258,7 +257,7 @@ mod withdrawal_transactions { create_and_record_ck_withdrawal_requests(&mut transactions, &mut rng); // 3 -> 2 -> 1 assert_eq!( - transactions.withdrawal_requests_batch(5), + transactions.requests_batch(5), vec![ first_request.clone(), second_request.clone(), @@ -266,10 +265,10 @@ mod withdrawal_transactions { ] ); - transactions.reschedule_withdrawal_request(first_request.clone()); + transactions.reschedule_request(first_request.cketh_ledger_burn_index()); // 1 -> 3 -> 2 assert_eq!( - transactions.withdrawal_requests_batch(5), + transactions.requests_batch(5), vec![ second_request.clone(), third_request.clone(), @@ -277,10 +276,10 @@ mod withdrawal_transactions { ] ); - transactions.reschedule_withdrawal_request(second_request.clone()); + transactions.reschedule_request(second_request.cketh_ledger_burn_index()); // 2 -> 1 -> 3 assert_eq!( - transactions.withdrawal_requests_batch(5), + transactions.requests_batch(5), vec![ third_request.clone(), first_request.clone(), @@ -288,10 +287,10 @@ mod withdrawal_transactions { ] ); - transactions.reschedule_withdrawal_request(third_request.clone()); + transactions.reschedule_request(third_request.cketh_ledger_burn_index()); // 3 -> 2 -> 1 assert_eq!( - transactions.withdrawal_requests_batch(5), + transactions.requests_batch(5), vec![first_request, second_request, third_request] ); } @@ -306,7 +305,9 @@ mod withdrawal_transactions { cketh_withdrawal_request_with_index, create_and_record_ck_withdrawal_requests, create_and_record_transaction, create_ck_withdrawal_requests, gas_fee_estimate, }; - use crate::state::transactions::{WithdrawalTransactions, create_transaction}; + use crate::state::transactions::{ + PipelineRequest, WithdrawalRequest, WithdrawalTransactions, + }; use crate::test_fixtures::expect_panic_with_message; use crate::tx::Eip1559TransactionRequest; use crate::withdraw::{ @@ -323,19 +324,20 @@ mod withdrawal_transactions { let mut transactions = WithdrawalTransactions::new(TransactionNonce::ZERO); let mut rng = reproducible_rng(); let [withdrawal_request] = create_ck_withdrawal_requests(&mut rng); - let tx = create_transaction( - &withdrawal_request.clone(), - TransactionNonce::ZERO, - gas_fee_estimate(), - estimate_gas_limit(&withdrawal_request), - EthereumNetwork::Sepolia, - ) - .unwrap(); + let tx = withdrawal_request + .clone() + .create_transaction( + TransactionNonce::ZERO, + gas_fee_estimate(), + estimate_gas_limit(&withdrawal_request), + EthereumNetwork::Sepolia, + ) + .unwrap(); let burn_index = withdrawal_request.cketh_ledger_burn_index(); expect_panic_with_message( || transactions.record_created_transaction(burn_index, tx), - &format!("withdrawal request {burn_index} not found"), + &format!("request {burn_index} not found"), ); } @@ -343,15 +345,16 @@ mod withdrawal_transactions { fn should_fail_when_mismatch_with_cketh_withdrawal_request() { let mut transactions = WithdrawalTransactions::new(TransactionNonce::ZERO); let withdrawal_request = cketh_withdrawal_request_with_index(LedgerBurnIndex::new(15)); - transactions.record_withdrawal_request(withdrawal_request.clone()); - let correct_tx = create_transaction( - &withdrawal_request.clone().into(), - TransactionNonce::ZERO, - gas_fee_estimate(), - estimate_gas_limit(&withdrawal_request.clone().into()), - EthereumNetwork::Sepolia, - ) - .unwrap(); + transactions.record_request(withdrawal_request.clone()); + let pipeline_request: WithdrawalRequest = withdrawal_request.clone().into(); + let correct_tx = pipeline_request + .create_transaction( + TransactionNonce::ZERO, + gas_fee_estimate(), + estimate_gas_limit(&withdrawal_request.clone().into()), + EthereumNetwork::Sepolia, + ) + .unwrap(); let tx_with_wrong_destination = Eip1559TransactionRequest { destination: Address::ZERO, @@ -394,15 +397,16 @@ mod withdrawal_transactions { LedgerBurnIndex::new(3), LedgerBurnIndex::new(7), ); - transactions.record_withdrawal_request(withdrawal_request.clone()); - let correct_tx = create_transaction( - &withdrawal_request.clone().into(), - TransactionNonce::ZERO, - gas_fee_estimate(), - estimate_gas_limit(&withdrawal_request.clone().into()), - EthereumNetwork::Sepolia, - ) - .unwrap(); + transactions.record_request(withdrawal_request.clone()); + let pipeline_request: WithdrawalRequest = withdrawal_request.clone().into(); + let correct_tx = pipeline_request + .create_transaction( + TransactionNonce::ZERO, + gas_fee_estimate(), + estimate_gas_limit(&withdrawal_request.clone().into()), + EthereumNetwork::Sepolia, + ) + .unwrap(); let tx_mixing_payee_address_with_erc20_address = Eip1559TransactionRequest { destination: withdrawal_request.destination, ..correct_tx.clone() @@ -443,8 +447,7 @@ mod withdrawal_transactions { let mut transactions = WithdrawalTransactions::new(current_nonce); let mut rng = reproducible_rng(); let [withdrawal_request] = create_and_record_ck_withdrawal_requests(&mut transactions, &mut rng); - let tx_with_wrong_nonce = create_transaction( - &withdrawal_request.clone(), + let tx_with_wrong_nonce = withdrawal_request.clone().create_transaction( wrong_nonce, gas_fee_estimate(), CKETH_WITHDRAWAL_TRANSACTION_GAS_LIMIT, @@ -466,7 +469,7 @@ mod withdrawal_transactions { for i in 0..100_u64 { let ledger_burn_index = LedgerBurnIndex::new(15 + i); let withdrawal_request = cketh_withdrawal_request_with_index(ledger_burn_index); - transactions.record_withdrawal_request(withdrawal_request.clone()); + transactions.record_request(withdrawal_request.clone()); let expected_tx_amount = withdrawal_request .withdrawal_amount .checked_sub( @@ -515,7 +518,7 @@ mod withdrawal_transactions { cketh_ledger_burn_index, ckerc20_ledger_burn_index, ); - transactions.record_withdrawal_request(withdrawal_request.clone()); + transactions.record_request(withdrawal_request.clone()); let created_tx = create_and_record_transaction( &mut transactions, withdrawal_request.clone(), @@ -601,7 +604,7 @@ mod withdrawal_transactions { gas_fee_estimate(), ); - assert_eq!(transactions.withdrawal_requests_batch(1), vec![]); + assert_eq!(transactions.requests_batch(1), vec![]); } } @@ -867,7 +870,7 @@ mod withdrawal_transactions { let mut transactions = WithdrawalTransactions::new(TransactionNonce::ZERO); let withdrawal_request = withdrawal_request.into(); let cketh_ledger_burn_index = withdrawal_request.cketh_ledger_burn_index(); - transactions.record_withdrawal_request(withdrawal_request.clone()); + transactions.record_request(withdrawal_request.clone()); let initial_tx = create_and_record_transaction( &mut transactions, withdrawal_request.clone(), @@ -1027,7 +1030,7 @@ mod withdrawal_transactions { assert_eq!( resubmitted_txs, vec![Err(ResubmitTransactionError::InsufficientTransactionFee { - ledger_burn_index: 93_u64.into(), + id: 93_u64.into(), transaction_nonce: 30_u8.into(), allowed_max_transaction_fee: DEFAULT_MAX_TRANSACTION_FEE.into(), max_transaction_fee: 30_000_000_000_165_000_u128.into(), @@ -1091,7 +1094,7 @@ mod withdrawal_transactions { let mut transactions = WithdrawalTransactions::new(TransactionNonce::ZERO); let withdrawal_request = withdrawal_request.into(); let cketh_ledger_burn_index = withdrawal_request.cketh_ledger_burn_index(); - transactions.record_withdrawal_request(withdrawal_request.clone()); + transactions.record_request(withdrawal_request.clone()); let created_tx = create_and_record_transaction( &mut transactions, withdrawal_request, @@ -1206,7 +1209,7 @@ mod withdrawal_transactions { }; let withdrawal_request = withdrawal_request.into(); let cketh_ledger_burn_index = withdrawal_request.cketh_ledger_burn_index(); - transactions.record_withdrawal_request(withdrawal_request.clone()); + transactions.record_request(withdrawal_request.clone()); let created_tx = create_and_record_transaction( &mut transactions, withdrawal_request.clone(), @@ -1555,7 +1558,7 @@ mod withdrawal_transactions { let cketh_ledger_burn_index = LedgerBurnIndex::new(15); let withdrawal_request: WithdrawalRequest = cketh_withdrawal_request_with_index(cketh_ledger_burn_index).into(); - transactions.record_withdrawal_request(withdrawal_request.clone()); + transactions.record_request(withdrawal_request.clone()); let created_tx = create_and_record_transaction( &mut transactions, withdrawal_request.clone(), @@ -1585,7 +1588,7 @@ mod withdrawal_transactions { cketh_ledger_burn_index, ckerc20_ledger_burn_index, ); - transactions.record_withdrawal_request(withdrawal_request.clone()); + transactions.record_request(withdrawal_request.clone()); let created_tx = create_and_record_transaction( &mut transactions, withdrawal_request.clone(), @@ -1619,7 +1622,7 @@ mod withdrawal_transactions { ckerc20_ledger_burn_index, ) }; - transactions.record_withdrawal_request(withdrawal_request.clone()); + transactions.record_request(withdrawal_request.clone()); let created_tx = create_and_record_transaction( &mut transactions, withdrawal_request.clone(), @@ -1653,7 +1656,7 @@ mod withdrawal_transactions { cketh_ledger_burn_index, ckerc20_ledger_burn_index, ); - transactions.record_withdrawal_request(withdrawal_request.clone()); + transactions.record_request(withdrawal_request.clone()); let created_tx = create_and_record_transaction( &mut transactions, withdrawal_request.clone(), @@ -1696,7 +1699,7 @@ mod withdrawal_transactions { { let mut transactions = WithdrawalTransactions::new(TransactionNonce::ZERO); let withdrawal_request = cketh_withdrawal_request_with_index(LedgerBurnIndex::new(15)); - transactions.record_withdrawal_request(withdrawal_request.clone()); + transactions.record_request(withdrawal_request.clone()); let cketh_ledger_burn_index = withdrawal_request.ledger_burn_index; let created_tx = create_and_record_transaction( &mut transactions, @@ -2083,7 +2086,7 @@ mod withdrawal_transactions { RetrieveEthStatus::NotFound ); assert_withdrawal_status(transactions, &withdrawal_request.clone(), vec![]); - transactions.record_withdrawal_request(withdrawal_request.clone()); + transactions.record_request(withdrawal_request.clone()); assert_eq!( transactions.transaction_status(&cketh_ledger_burn_index), RetrieveEthStatus::Pending @@ -2142,8 +2145,8 @@ mod withdrawal_transactions { create_and_record_signed_transaction, }; use crate::state::transactions::{ - CreateTransactionError, EthWithdrawalRequest, NotReimbursable, ReimbursementIndex, - create_transaction, + CreateTransactionError, EthWithdrawalRequest, NotReimbursable, PipelineRequest, + ReimbursementIndex, }; use crate::tx::GasFeeEstimate; use crate::withdraw::CKETH_WITHDRAWAL_TRANSACTION_GAS_LIMIT; @@ -2184,7 +2187,7 @@ mod withdrawal_transactions { let mut transactions = WithdrawalTransactions::new(TransactionNonce::ZERO); let funding = sweeper_funding_request(); - transactions.record_withdrawal_request(funding.clone()); + transactions.record_request(funding.clone()); let created_tx = create_and_record_transaction( &mut transactions, funding.clone(), @@ -2243,14 +2246,14 @@ mod withdrawal_transactions { .to_price(CKETH_WITHDRAWAL_TRANSACTION_GAS_LIMIT) .max_transaction_fee(); - let tx = create_transaction( - &WithdrawalRequest::SweeperFunding(funding.clone()), - TransactionNonce::ZERO, - gas_fee, - CKETH_WITHDRAWAL_TRANSACTION_GAS_LIMIT, - EthereumNetwork::Mainnet, - ) - .expect("the funded amount must cover the fee"); + let tx = WithdrawalRequest::SweeperFunding(funding.clone()) + .create_transaction( + TransactionNonce::ZERO, + gas_fee, + CKETH_WITHDRAWAL_TRANSACTION_GAS_LIMIT, + EthereumNetwork::Mainnet, + ) + .expect("the funded amount must cover the fee"); assert_eq!(tx.destination, funding.destination); assert_eq!( @@ -2273,8 +2276,7 @@ mod withdrawal_transactions { let expected_index = funding.ledger_burn_index; assert_matches!( - create_transaction( - &WithdrawalRequest::SweeperFunding(funding), + WithdrawalRequest::SweeperFunding(funding).create_transaction( TransactionNonce::ZERO, gas_fee_estimate(), CKETH_WITHDRAWAL_TRANSACTION_GAS_LIMIT, @@ -2294,7 +2296,7 @@ mod withdrawal_transactions { let mut transactions = WithdrawalTransactions::new(TransactionNonce::ZERO); let funding = sweeper_funding_payload(); let request = WithdrawalRequest::SweeperFunding(funding.clone()); - transactions.record_withdrawal_request(request.clone()); + transactions.record_request(request.clone()); let created_tx = create_and_record_transaction(&mut transactions, request, gas_fee_estimate()); create_and_record_signed_transaction(&mut transactions, created_tx); @@ -2329,14 +2331,14 @@ mod withdrawal_transactions { } } -mod oldest_incomplete_withdrawal_timestamp { +mod oldest_incomplete_request_timestamp { use super::*; use ic_crypto_test_utils_reproducible_rng::reproducible_rng; #[test] fn should_return_none_when_no_requests() { let transactions = WithdrawalTransactions::new(TransactionNonce::ZERO); - assert_eq!(None, transactions.oldest_incomplete_withdrawal_timestamp()); + assert_eq!(None, transactions.oldest_incomplete_request_timestamp()); } #[test] @@ -2347,7 +2349,7 @@ mod oldest_incomplete_withdrawal_timestamp { create_and_record_ck_withdrawal_requests(&mut transactions, &mut rng); assert_eq!( - transactions.oldest_incomplete_withdrawal_timestamp(), + transactions.oldest_incomplete_request_timestamp(), withdrawal_request.created_at(), ); } @@ -2359,13 +2361,10 @@ mod oldest_incomplete_withdrawal_timestamp { let [mut first_request, mut second_request] = create_ck_withdrawal_requests(&mut rng); set_created_at(&mut first_request, 10); set_created_at(&mut second_request, 20); - transactions.record_withdrawal_request(first_request); - transactions.record_withdrawal_request(second_request); + transactions.record_request(first_request); + transactions.record_request(second_request); - assert_eq!( - transactions.oldest_incomplete_withdrawal_timestamp(), - Some(10), - ); + assert_eq!(transactions.oldest_incomplete_request_timestamp(), Some(10),); } #[test] @@ -2381,7 +2380,7 @@ mod oldest_incomplete_withdrawal_timestamp { ); assert_eq!( - transactions.oldest_incomplete_withdrawal_timestamp(), + transactions.oldest_incomplete_request_timestamp(), withdrawal_request.created_at(), ); } @@ -2394,14 +2393,11 @@ mod oldest_incomplete_withdrawal_timestamp { set_created_at(&mut first_request, 10); set_created_at(&mut second_request, 20); - transactions.record_withdrawal_request(first_request.clone()); - transactions.record_withdrawal_request(second_request.clone()); + transactions.record_request(first_request.clone()); + transactions.record_request(second_request.clone()); create_and_record_transaction(&mut transactions, first_request, gas_fee_estimate()); - assert_eq!( - transactions.oldest_incomplete_withdrawal_timestamp(), - Some(10), - ); + assert_eq!(transactions.oldest_incomplete_request_timestamp(), Some(10),); } #[test] @@ -2422,7 +2418,7 @@ mod oldest_incomplete_withdrawal_timestamp { transaction_receipt(&signed_tx, TransactionStatus::Success), ); - assert_eq!(transactions.oldest_incomplete_withdrawal_timestamp(), None); + assert_eq!(transactions.oldest_incomplete_request_timestamp(), None); } fn set_created_at(withdrawal_request: &mut WithdrawalRequest, created_at: u64) { @@ -2473,8 +2469,8 @@ mod create_transaction { gas_fee_estimate, }; use crate::state::transactions::{ - CreateTransactionError, Erc20WithdrawalRequest, EthWithdrawalRequest, TransactionCallData, - create_transaction, + CreateTransactionError, Erc20WithdrawalRequest, EthWithdrawalRequest, PipelineRequest, + TransactionCallData, WithdrawalRequest, }; use crate::tx::GasFeeEstimate; use crate::tx::{AccessList, Eip1559TransactionRequest}; @@ -2497,8 +2493,8 @@ mod create_transaction { withdrawal_amount: insufficient_amount, ..cketh_withdrawal_request_with_index(cketh_ledger_burn_index) }; - let result = create_transaction( - &cketh_withdrawal_request.clone().into(), + let pipeline_request: WithdrawalRequest = cketh_withdrawal_request.clone().into(); + let result = pipeline_request.create_transaction( TransactionNonce::TWO, gas_fee.clone(), gas_limit, @@ -2519,8 +2515,8 @@ mod create_transaction { max_transaction_fee: insufficient_amount, ..ckerc20_withdrawal_request_with_index(cketh_ledger_burn_index, LedgerBurnIndex::new(2)) }; - let result = create_transaction( - &ckerc20_withdrawal_request.clone().into(), + let pipeline_request: WithdrawalRequest = ckerc20_withdrawal_request.clone().into(); + let result = pipeline_request.create_transaction( TransactionNonce::TWO, gas_fee, gas_limit, @@ -2555,8 +2551,8 @@ mod create_transaction { Wei::from(31_500_001_050_000_u64) ); - let result = create_transaction( - &withdrawal_request.clone().into(), + let pipeline_request: WithdrawalRequest = withdrawal_request.clone().into(); + let result = pipeline_request.create_transaction( TransactionNonce::TWO, gas_fee, gas_limit, @@ -2609,8 +2605,8 @@ mod create_transaction { ) }; - let result = create_transaction( - &withdrawal_request.clone().into(), + let pipeline_request: WithdrawalRequest = withdrawal_request.clone().into(); + let result = pipeline_request.create_transaction( TransactionNonce::from(0x57_u32), gas_fee.clone(), gas_limit, @@ -2654,9 +2650,10 @@ mod create_transaction { mod withdrawal_flow { use super::arbitrary::{arb_checked_amount_of, arb_gas_fee_estimate, arb_withdrawal_request}; + use crate::lifecycle::EthereumNetwork; use crate::numeric::TransactionNonce; use crate::state::transactions::tests::sign_transaction; - use crate::state::transactions::{EthereumNetwork, WithdrawalTransactions, create_transaction}; + use crate::state::transactions::{PipelineRequest, WithdrawalTransactions}; use crate::withdraw::estimate_gas_limit; use proptest::proptest; use std::cell::RefCell; @@ -2668,7 +2665,7 @@ mod withdrawal_flow { let wrapped_txs = RefCell::new(transactions); proptest!(|(request in arb_withdrawal_request())| { - wrapped_txs.borrow_mut().record_withdrawal_request(request) + wrapped_txs.borrow_mut().record_request(request) }); proptest!(|(gas_fee_estimate in arb_gas_fee_estimate(), transaction_count in arb_checked_amount_of())| { @@ -2677,11 +2674,10 @@ mod withdrawal_flow { wrapped_txs.borrow_mut().record_resubmit_transaction(resubmit_tx); } - let withdrawal_requests = wrapped_txs.borrow().withdrawal_requests_batch(5); + let withdrawal_requests = wrapped_txs.borrow().requests_batch(5); for request in withdrawal_requests { let nonce = wrapped_txs.borrow().next_transaction_nonce(); - if let Ok(created_tx) = create_transaction( - &request, + if let Ok(created_tx) = request.create_transaction( nonce, gas_fee_estimate.clone(), estimate_gas_limit(&request), @@ -2977,7 +2973,7 @@ fn create_and_record_ck_withdrawal_requests( ) -> [WithdrawalRequest; N] { let requests = create_ck_withdrawal_requests(rng); for request in &requests { - transactions.record_withdrawal_request(request.clone()); + transactions.record_request(request.clone()); } requests } @@ -2988,7 +2984,7 @@ fn create_and_record_cketh_withdrawal_requests( ) -> [WithdrawalRequest; N] { let requests = create_cketh_withdrawal_requests(); for request in &requests { - transactions.record_withdrawal_request(request.clone()); + transactions.record_request(request.clone()); } requests } @@ -2999,7 +2995,7 @@ fn create_and_record_ckerc20_withdrawal_requests( ) -> [WithdrawalRequest; N] { let requests = create_ckerc20_withdrawal_requests(); for request in &requests { - transactions.record_withdrawal_request(request.clone()); + transactions.record_request(request.clone()); } requests } @@ -3059,14 +3055,14 @@ fn create_and_record_transaction>( gas_fee_estimate: GasFeeEstimate, ) -> Eip1559TransactionRequest { let withdrawal_request = withdrawal_request.into(); - let tx = create_transaction( - &withdrawal_request, - transactions.next_transaction_nonce(), - gas_fee_estimate, - estimate_gas_limit(&withdrawal_request), - EthereumNetwork::Sepolia, - ) - .expect("failed to create transaction"); + let tx = withdrawal_request + .create_transaction( + transactions.next_transaction_nonce(), + gas_fee_estimate, + estimate_gas_limit(&withdrawal_request), + EthereumNetwork::Sepolia, + ) + .expect("failed to create transaction"); let burn_index = withdrawal_request.cketh_ledger_burn_index(); transactions.record_created_transaction(burn_index, tx.clone()); tx @@ -3173,8 +3169,8 @@ use std::collections::{BTreeMap, BTreeSet, VecDeque}; /// that field alone changes. #[derive(Clone)] pub(in crate::state) struct WithdrawalTransactionsBuilder { - pending_withdrawal_requests: VecDeque, - processed_withdrawal_requests: BTreeMap, + pending_requests: VecDeque, + processed_requests: BTreeMap, created_tx: MultiKeyMap, sent_tx: MultiKeyMap>, finalized_tx: MultiKeyMap, @@ -3187,8 +3183,8 @@ pub(in crate::state) struct WithdrawalTransactionsBuilder { impl Default for WithdrawalTransactionsBuilder { fn default() -> Self { Self { - pending_withdrawal_requests: Default::default(), - processed_withdrawal_requests: Default::default(), + pending_requests: Default::default(), + processed_requests: Default::default(), created_tx: Default::default(), sent_tx: Default::default(), finalized_tx: Default::default(), @@ -3201,19 +3197,19 @@ impl Default for WithdrawalTransactionsBuilder { } impl WithdrawalTransactionsBuilder { - pub(in crate::state) fn with_pending_withdrawal_requests( + pub(in crate::state) fn with_pending_requests( mut self, - pending_withdrawal_requests: VecDeque, + pending_requests: VecDeque, ) -> Self { - self.pending_withdrawal_requests = pending_withdrawal_requests; + self.pending_requests = pending_requests; self } - pub(in crate::state) fn with_processed_withdrawal_requests( + pub(in crate::state) fn with_processed_requests( mut self, - processed_withdrawal_requests: BTreeMap, + processed_requests: BTreeMap, ) -> Self { - self.processed_withdrawal_requests = processed_withdrawal_requests; + self.processed_requests = processed_requests; self } @@ -3272,9 +3268,9 @@ impl WithdrawalTransactionsBuilder { pub(in crate::state) fn build(self) -> WithdrawalTransactions { WithdrawalTransactions { - pipeline: TransactionPipeline { - pending_withdrawal_requests: self.pending_withdrawal_requests, - processed_withdrawal_requests: self.processed_withdrawal_requests, + pipeline: MinterTransactionPipeline { + pending_requests: self.pending_requests, + processed_requests: self.processed_requests, created_tx: self.created_tx, sent_tx: self.sent_tx, finalized_tx: self.finalized_tx, diff --git a/rs/ethereum/cketh/minter/src/withdraw.rs b/rs/ethereum/cketh/minter/src/withdraw.rs index a0a73e3983c6..471692a369e5 100644 --- a/rs/ethereum/cketh/minter/src/withdraw.rs +++ b/rs/ethereum/cketh/minter/src/withdraw.rs @@ -13,8 +13,8 @@ use crate::{ audit::{EventType, process_event}, minter_address, mutate_state, read_state, transactions::{ - CreateTransactionError, Reimbursed, ReimbursementIndex, ReimbursementRequest, - WithdrawalRequest, create_transaction, + CreateTransactionError, PipelineRequest, Reimbursed, ReimbursementIndex, + ReimbursementRequest, WithdrawalRequest, }, }, tx::{GasFeeEstimate, lazy_refresh_gas_fee_estimate}, @@ -251,14 +251,13 @@ async fn resubmit_transactions_batch( fn create_transactions_batch(gas_fee_estimate: GasFeeEstimate) { for request in read_state(|s| { s.withdrawal_transactions - .withdrawal_requests_batch(WITHDRAWAL_REQUESTS_BATCH_SIZE) + .requests_batch(WITHDRAWAL_REQUESTS_BATCH_SIZE) }) { log!(DEBUG, "[create_transactions_batch]: processing {request:?}",); let ethereum_network = read_state(State::ethereum_network); let nonce = read_state(|s| s.withdrawal_transactions.next_transaction_nonce()); let gas_limit = estimate_gas_limit(&request); - match create_transaction( - &request, + match request.create_transaction( nonce, gas_fee_estimate.clone(), gas_limit, @@ -291,7 +290,7 @@ fn create_transactions_batch(gas_fee_estimate: GasFeeEstimate) { ); mutate_state(|s| { s.withdrawal_transactions - .reschedule_withdrawal_request(request) + .reschedule_request(ledger_burn_index) }); } };