diff --git a/rs/ethereum/cketh/minter/cketh_minter.did b/rs/ethereum/cketh/minter/cketh_minter.did index 95c12594059d..cc49aaba7c5c 100644 --- a/rs/ethereum/cketh/minter/cketh_minter.did +++ b/rs/ethereum/cketh/minter/cketh_minter.did @@ -242,7 +242,6 @@ type MinterInfo = record { evm_rpc_id : opt principal; }; - type GasFeeEstimate = record { // Maximum amount of Wei per gas unit that the transaction is willing to pay in total. // This covers the base fee determined by the network and the `max_priority_fee_per_gas`. @@ -570,6 +569,14 @@ type Event = record { from_subaccount : opt blob; created_at: opt nat64; }; + AcceptedSweeperFundingRequest : record { + withdrawal_amount : nat; + destination : text; + ledger_burn_index : nat; + from : principal; + from_subaccount : opt blob; + created_at: nat64; + }; CreatedTransaction : record { withdrawal_id : nat; transaction : UnsignedTransaction; diff --git a/rs/ethereum/cketh/minter/src/dashboard.rs b/rs/ethereum/cketh/minter/src/dashboard.rs index 9e4908e037ca..a6af2f034a1e 100644 --- a/rs/ethereum/cketh/minter/src/dashboard.rs +++ b/rs/ethereum/cketh/minter/src/dashboard.rs @@ -361,6 +361,13 @@ impl DashboardTemplate { created_at: Some(req.created_at), } } + WithdrawalRequest::SweeperFunding(req) => DashboardWithdrawalRequest { + cketh_ledger_burn_index: req.ledger_burn_index, + destination: req.destination, + value: req.withdrawal_amount.into(), + token_symbol: CkTokenSymbol::cketh_symbol_from_state(state), + created_at: Some(req.created_at), + }, }) .collect(); withdrawal_requests.sort_unstable_by_key(|req| Reverse(req.cketh_ledger_burn_index)); diff --git a/rs/ethereum/cketh/minter/src/dashboard/tests.rs b/rs/ethereum/cketh/minter/src/dashboard/tests.rs index c2dd88646e51..5611db971c4e 100644 --- a/rs/ethereum/cketh/minter/src/dashboard/tests.rs +++ b/rs/ethereum/cketh/minter/src/dashboard/tests.rs @@ -884,12 +884,16 @@ fn should_display_reimbursed_requests() { }, ); } + WithdrawalRequest::SweeperFunding(_) => { + unreachable!("sweeper funding is never reimbursed") + } } } else { apply_state_transition( &mut state, &EventType::QuarantinedReimbursement { - index: ReimbursementIndex::from(&req), + index: ReimbursementIndex::try_from(&req) + .expect("BUG: this test's fixtures are all user withdrawals"), }, ) } diff --git a/rs/ethereum/cketh/minter/src/endpoints.rs b/rs/ethereum/cketh/minter/src/endpoints.rs index 98cf64b54aa3..581d68be48e0 100644 --- a/rs/ethereum/cketh/minter/src/endpoints.rs +++ b/rs/ethereum/cketh/minter/src/endpoints.rs @@ -489,6 +489,14 @@ pub mod events { from_subaccount: Option<[u8; 32]>, created_at: Option, }, + AcceptedSweeperFundingRequest { + withdrawal_amount: Nat, + destination: String, + ledger_burn_index: Nat, + from: Principal, + from_subaccount: Option<[u8; 32]>, + created_at: u64, + }, CreatedTransaction { withdrawal_id: Nat, transaction: UnsignedTransaction, diff --git a/rs/ethereum/cketh/minter/src/lib.rs b/rs/ethereum/cketh/minter/src/lib.rs index 3dc0d7f8d542..ccced932f0a9 100644 --- a/rs/ethereum/cketh/minter/src/lib.rs +++ b/rs/ethereum/cketh/minter/src/lib.rs @@ -45,3 +45,10 @@ pub const EVM_RPC_ID_PRODUCTION: Principal = Principal::from_slice(&[0, 0, 0, 0, 2, 48, 0, 204, 1, 1]); pub const EVM_RPC_ID_STAGING: Principal = Principal::from_slice(&[0, 0, 0, 0, 2, 48, 0, 161, 1, 1]); pub const CKETH_LEDGER_MEMO_SIZE: u16 = 80; + +pub const CKETH_FEE_SUBACCOUNT: [u8; 32] = { + let mut subaccount = [0_u8; 32]; + subaccount[30] = 0x0f; + subaccount[31] = 0xee; + subaccount +}; diff --git a/rs/ethereum/cketh/minter/src/main.rs b/rs/ethereum/cketh/minter/src/main.rs index 90d848c130f3..4c728b7c6fdb 100644 --- a/rs/ethereum/cketh/minter/src/main.rs +++ b/rs/ethereum/cketh/minter/src/main.rs @@ -32,7 +32,7 @@ use ic_cketh_minter::state::audit::{Event, EventType, process_event}; use ic_cketh_minter::state::eth_logs_scraping::{LogScrapingId, LogScrapingInfo}; use ic_cketh_minter::state::transactions::{ Erc20WithdrawalRequest, EthWithdrawalRequest, Reimbursed, ReimbursementIndex, - ReimbursementRequest, + ReimbursementRequest, SweeperFundingRequest, }; use ic_cketh_minter::state::{ STATE, State, lazy_call_ecdsa_public_key, mutate_state, read_state, transactions, @@ -407,7 +407,9 @@ async fn withdrawal_status(parameter: WithdrawalSearchParameter) -> Vec CkTokenSymbol::cketh_symbol_from_state(s).to_string(), + CkEth(_) | SweeperFunding(_) => { + CkTokenSymbol::cketh_symbol_from_state(s).to_string() + } CkErc20(r) => s .ckerc20_tokens .get_alt(&r.erc20_contract_address) @@ -417,12 +419,16 @@ async fn withdrawal_status(parameter: WithdrawalSearchParameter) -> Vec r.withdrawal_amount.into(), CkErc20(r) => r.withdrawal_amount.into(), + SweeperFunding(r) => r.withdrawal_amount.into(), }, max_transaction_fee: match (request, tx) { - (CkEth(_), None) => None, + (CkEth(_) | SweeperFunding(_), None) => None, (CkEth(r), Some(tx)) => { r.withdrawal_amount.checked_sub(tx.amount).map(|x| x.into()) } + (SweeperFunding(r), Some(tx)) => { + r.withdrawal_amount.checked_sub(tx.amount).map(|x| x.into()) + } (CkErc20(r), _) => Some(r.max_transaction_fee.into()), }, from: request.from(), @@ -832,6 +838,21 @@ fn get_events(arg: GetEventsArg) -> GetEventsResult { from_subaccount: from_subaccount.map(LedgerSubaccount::to_bytes), created_at, }, + EventType::AcceptedSweeperFundingRequest(SweeperFundingRequest { + withdrawal_amount, + destination, + ledger_burn_index, + from, + from_subaccount, + created_at, + }) => EP::AcceptedSweeperFundingRequest { + withdrawal_amount: withdrawal_amount.into(), + destination: destination.to_string(), + ledger_burn_index: ledger_burn_index.get().into(), + from, + from_subaccount: from_subaccount.map(LedgerSubaccount::to_bytes), + created_at, + }, EventType::CreatedTransaction { withdrawal_id, transaction, diff --git a/rs/ethereum/cketh/minter/src/state.rs b/rs/ethereum/cketh/minter/src/state.rs index e43c17681a94..0dc60857d859 100644 --- a/rs/ethereum/cketh/minter/src/state.rs +++ b/rs/ethereum/cketh/minter/src/state.rs @@ -371,6 +371,10 @@ impl State { .checked_sub(tx.transaction().amount) .expect("BUG: withdrawal amount MUST always be at least the transaction amount"), WithdrawalRequest::CkErc20(req) => req.max_transaction_fee, + WithdrawalRequest::SweeperFunding(req) => req + .withdrawal_amount + .checked_sub(tx.transaction().amount) + .expect("BUG: funded amount MUST always be at least the transaction amount"), }; let unspent_tx_fee = charged_tx_fee.checked_sub(tx_fee).expect( "BUG: charged transaction fee MUST always be at least the effective transaction fee", diff --git a/rs/ethereum/cketh/minter/src/state/audit.rs b/rs/ethereum/cketh/minter/src/state/audit.rs index 4368ba823981..09a38f5d6b46 100644 --- a/rs/ethereum/cketh/minter/src/state/audit.rs +++ b/rs/ethereum/cketh/minter/src/state/audit.rs @@ -74,6 +74,11 @@ pub fn apply_state_transition(state: &mut State, payload: &EventType) { .eth_transactions .record_withdrawal_request(request.clone()); } + EventType::AcceptedSweeperFundingRequest(request) => { + state + .eth_transactions + .record_withdrawal_request(request.clone()); + } EventType::CreatedTransaction { withdrawal_id, transaction, diff --git a/rs/ethereum/cketh/minter/src/state/audit/tests.rs b/rs/ethereum/cketh/minter/src/state/audit/tests.rs index 9a37e4b436f1..f7238597c983 100644 --- a/rs/ethereum/cketh/minter/src/state/audit/tests.rs +++ b/rs/ethereum/cketh/minter/src/state/audit/tests.rs @@ -97,7 +97,7 @@ impl GetEventsFile { use crate::eth_logs::EventSource; use crate::state::TransactionStatus; use crate::state::audit::EventType as ET; - use crate::state::transactions::EthWithdrawalRequest; + use crate::state::transactions::{EthWithdrawalRequest, SweeperFundingRequest}; fn map_event_source( CandidEventSource { @@ -315,6 +315,21 @@ impl GetEventsFile { from_subaccount: from_subaccount.and_then(LedgerSubaccount::from_bytes), created_at, }), + EventPayload::AcceptedSweeperFundingRequest { + withdrawal_amount, + destination, + ledger_burn_index, + from, + from_subaccount, + created_at, + } => ET::AcceptedSweeperFundingRequest(SweeperFundingRequest { + withdrawal_amount: withdrawal_amount.try_into().unwrap(), + destination: destination.parse().unwrap(), + ledger_burn_index: map_nat(ledger_burn_index), + from, + from_subaccount: from_subaccount.and_then(LedgerSubaccount::from_bytes), + created_at, + }), EventPayload::CreatedTransaction { withdrawal_id, transaction, diff --git a/rs/ethereum/cketh/minter/src/state/event.rs b/rs/ethereum/cketh/minter/src/state/event.rs index f5aeac8dcf2f..dd7494cd795e 100644 --- a/rs/ethereum/cketh/minter/src/state/event.rs +++ b/rs/ethereum/cketh/minter/src/state/event.rs @@ -5,7 +5,7 @@ use crate::lifecycle::{init::InitArg, upgrade::UpgradeArg}; use crate::numeric::{BlockNumber, Erc20Value, LedgerBurnIndex, LedgerMintIndex}; use crate::state::transactions::{ Erc20WithdrawalRequest, EthWithdrawalRequest, Reimbursed, ReimbursementIndex, - ReimbursementRequest, + ReimbursementRequest, SweeperFundingRequest, }; use crate::timed_sized_map::Timestamp; use crate::tx::{Eip1559TransactionRequest, SignedEip1559TransactionRequest}; @@ -182,6 +182,9 @@ pub enum EventType { /// durable even across an ungraceful trap (unlike the pre-upgrade snapshot). #[n(26)] AutomaticDepositReceived(#[n(0)] AutomaticDeposit), + /// The minter burned ckETH from its fee subaccount to top up the sweeper address with gas. + #[n(27)] + AcceptedSweeperFundingRequest(#[n(0)] SweeperFundingRequest), } /// Full snapshot of the ckERC20 deposit address registry. Carries the limits in diff --git a/rs/ethereum/cketh/minter/src/state/tests.rs b/rs/ethereum/cketh/minter/src/state/tests.rs index b5b7d2de7a2d..319bd90504fc 100644 --- a/rs/ethereum/cketh/minter/src/state/tests.rs +++ b/rs/ethereum/cketh/minter/src/state/tests.rs @@ -13,7 +13,9 @@ use crate::state::audit::apply_state_transition; use crate::state::automatic_deposits::AutomaticDeposits; use crate::state::eth_logs_scraping::{LogScrapingId, LogScrapings}; use crate::state::event::{Event, EventType}; -use crate::state::transactions::{Erc20WithdrawalRequest, ReimbursementIndex}; +use crate::state::transactions::{ + Erc20WithdrawalRequest, ReimbursementIndex, SweeperFundingRequest, +}; use crate::state::{Erc20Balances, State}; use crate::test_fixtures::{ arb::{arb_address, arb_checked_amount_of, arb_hash, arb_ledger_subaccount}, @@ -736,6 +738,26 @@ prop_compose! { } } +prop_compose! { + fn arb_sweeper_funding_request()( + withdrawal_amount in arb_checked_amount_of(), + destination in arb_address(), + ledger_burn_index in any::(), + from in arb_principal(), + from_subaccount in arb_ledger_subaccount(), + created_at in any::(), + ) -> SweeperFundingRequest { + SweeperFundingRequest { + withdrawal_amount, + destination, + ledger_burn_index: ledger_burn_index.into(), + from, + from_subaccount, + created_at, + } + } +} + fn arb_event_type() -> impl Strategy { prop_oneof![ arb_init_arg().prop_map(EventType::Init), @@ -752,6 +774,7 @@ fn arb_event_type() -> impl Strategy { mint_block_index: index.into(), } }), + arb_sweeper_funding_request().prop_map(EventType::AcceptedSweeperFundingRequest), arb_checked_amount_of().prop_map(|block_number| EventType::SyncedToBlock { block_number }), arb_checked_amount_of() .prop_map(|block_number| EventType::SyncedErc20ToBlock { block_number }), @@ -1501,6 +1524,108 @@ mod eth_balance { ); } + /// Funding takes its own arm in the balance accounting, so the ckETH and ckERC20 cases above + /// cannot reach it. What is asserted is the same shape as a ckETH withdrawal — a success debits + /// the transferred ETH plus the fee actually paid, a failure debits only that fee — because + /// funding is an ordinary withdrawal to the accounting. What differs is that nothing is ever + /// reimbursed, which is why the failing case must still leave the fee counters moving. + #[test] + fn should_update_after_successful_and_failed_sweeper_funding() { + let mut state_before_funding = initial_state(); + apply_state_transition( + &mut state_before_funding, + &EventType::AcceptedDeposit(received_eth_event()), + ); + let eth_balance_before_funding = state_before_funding.eth_balance.clone(); + + let funding_amount = Wei::new(10_000_000_000_000_000); + let funding_request = SweeperFundingRequest { + withdrawal_amount: funding_amount, + destination: "0xb44B5e756A894775FC32EDdf3314Bb1B1944dC34" + .parse() + .unwrap(), + ledger_burn_index: LedgerBurnIndex::new(0), + from: "k2t6j-2nvnp-4zjm3-25dtz-6xhaa-c7boj-5gayf-oj3xs-i43lp-teztq-6ae" + .parse() + .unwrap(), + from_subaccount: None, + created_at: 1699527697000000000, + }; + let funding_flow = WithdrawalFlow { + tx_fee: GasFeeEstimate { + base_fee_per_gas: WeiPerGas::from(0xbc9998d1_u64), + max_priority_fee_per_gas: WeiPerGas::from(1_500_000_000_u64), + }, + gas_limit: GasAmount::from(21_000_u32), + effective_gas_price: WeiPerGas::from(0x1176e9eb9_u64), + tx_status: TransactionStatus::Success, + ..WithdrawalFlow::for_request(funding_request) + }; + + let mut state_after_successful_funding = state_before_funding.clone(); + let receipt_succeeded = funding_flow + .clone() + .apply(&mut state_after_successful_funding); + let after_success = state_after_successful_funding.eth_balance.clone(); + + // Asserted as the identity the accounting has to satisfy rather than as a fixed number: the + // funding ceiling covers both the ETH delivered and the fee, so whatever part of the fee + // went unspent is exactly what stays with the minter. + let unspent = after_success + .total_unspent_tx_fees + .checked_sub(eth_balance_before_funding.total_unspent_tx_fees) + .unwrap(); + assert_eq!( + after_success.eth_balance, + eth_balance_before_funding + .eth_balance + .checked_sub( + funding_amount + .checked_sub(unspent) + .expect("the unspent fee is part of the funding") + ) + .unwrap(), + "a successful funding debits what it moved plus the fee it paid" + ); + assert_eq!( + after_success.total_effective_tx_fees, + eth_balance_before_funding + .total_effective_tx_fees + .checked_add(receipt_succeeded.effective_transaction_fee()) + .unwrap(), + "the fee actually paid must be counted" + ); + assert!( + unspent > Wei::ZERO, + "this fixture over-provisions the fee, so some of it must be recorded as unspent" + ); + + let mut state_after_failed_funding = state_before_funding.clone(); + let receipt_failed = WithdrawalFlow { + tx_status: TransactionStatus::Failure, + ..funding_flow + } + .apply(&mut state_after_failed_funding); + let after_failure = state_after_failed_funding.eth_balance.clone(); + + assert_eq!( + after_failure.eth_balance, + eth_balance_before_funding + .eth_balance + .checked_sub(receipt_failed.effective_transaction_fee()) + .unwrap(), + "a failed funding moved no ETH, so only the fee is debited" + ); + assert_eq!( + after_failure.total_effective_tx_fees, after_success.total_effective_tx_fees, + "the same fee was paid either way" + ); + assert_eq!( + after_failure.total_unspent_tx_fees, after_success.total_unspent_tx_fees, + "and the same amount of it went unspent" + ); + } + #[test] fn should_update_after_successful_and_failed_erc20_withdrawal() { let mut state_before_withdrawal = initial_erc20_state(); @@ -1620,14 +1745,10 @@ mod eth_balance { } fn apply(self, state: &mut State) -> TransactionReceipt { - let accepted_withdrawal_request_event = match &self.withdrawal_request { - WithdrawalRequest::CkEth(eth_request) => { - EventType::AcceptedEthWithdrawalRequest(eth_request.clone()) - } - WithdrawalRequest::CkErc20(erc20_request) => { - EventType::AcceptedErc20WithdrawalRequest(erc20_request.clone()) - } - }; + let accepted_withdrawal_request_event = self + .withdrawal_request + .clone() + .into_accepted_withdrawal_request_event(); apply_state_transition(state, &accepted_withdrawal_request_event); let transaction = create_transaction( diff --git a/rs/ethereum/cketh/minter/src/state/transactions/mod.rs b/rs/ethereum/cketh/minter/src/state/transactions/mod.rs index d317e0405521..483bfdfbfa3c 100644 --- a/rs/ethereum/cketh/minter/src/state/transactions/mod.rs +++ b/rs/ethereum/cketh/minter/src/state/transactions/mod.rs @@ -7,6 +7,7 @@ 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, @@ -18,6 +19,7 @@ use crate::tx::{ SignedEip1559TransactionRequest, SignedTransactionRequest, TransactionRequest, }; use candid::Principal; +use ic_canister_log::log; use ic_ethereum_types::Address; use icrc_ledger_types::icrc1::account::Account; use minicbor::{Decode, Encode}; @@ -36,6 +38,7 @@ pub enum WithdrawalSearchParameter { pub enum WithdrawalRequest { CkEth(EthWithdrawalRequest), CkErc20(Erc20WithdrawalRequest), + SweeperFunding(SweeperFundingRequest), } impl WithdrawalRequest { @@ -43,6 +46,7 @@ impl WithdrawalRequest { match self { WithdrawalRequest::CkEth(request) => request.ledger_burn_index, WithdrawalRequest::CkErc20(request) => request.cketh_ledger_burn_index, + WithdrawalRequest::SweeperFunding(request) => request.ledger_burn_index, } } @@ -50,6 +54,7 @@ impl WithdrawalRequest { match self { WithdrawalRequest::CkEth(request) => request.created_at, WithdrawalRequest::CkErc20(request) => Some(request.created_at), + WithdrawalRequest::SweeperFunding(request) => Some(request.created_at), } } @@ -58,6 +63,7 @@ impl WithdrawalRequest { match self { WithdrawalRequest::CkEth(request) => request.destination, WithdrawalRequest::CkErc20(request) => request.destination, + WithdrawalRequest::SweeperFunding(request) => request.destination, } } @@ -66,6 +72,7 @@ impl WithdrawalRequest { match self { WithdrawalRequest::CkEth(request) => request.destination, WithdrawalRequest::CkErc20(request) => request.erc20_contract_address, + WithdrawalRequest::SweeperFunding(request) => request.destination, } } @@ -73,6 +80,7 @@ impl WithdrawalRequest { match self { WithdrawalRequest::CkEth(request) => request.from, WithdrawalRequest::CkErc20(request) => request.from, + WithdrawalRequest::SweeperFunding(request) => request.from, } } @@ -80,6 +88,15 @@ impl WithdrawalRequest { match self { WithdrawalRequest::CkEth(request) => request.from_subaccount.as_ref(), WithdrawalRequest::CkErc20(request) => request.from_subaccount.as_ref(), + WithdrawalRequest::SweeperFunding(request) => request.from_subaccount.as_ref(), + } + } + + /// Whether this request can be paid back if its transaction fails. + pub fn is_reimbursable(&self) -> bool { + match self { + WithdrawalRequest::CkEth(_) | WithdrawalRequest::CkErc20(_) => true, + WithdrawalRequest::SweeperFunding(_) => false, } } @@ -89,6 +106,9 @@ impl WithdrawalRequest { WithdrawalRequest::CkErc20(request) => { EventType::AcceptedErc20WithdrawalRequest(request) } + WithdrawalRequest::SweeperFunding(request) => { + EventType::AcceptedSweeperFundingRequest(request) + } } } @@ -118,6 +138,12 @@ impl From for WithdrawalRequest { } } +impl From for WithdrawalRequest { + fn from(value: SweeperFundingRequest) -> Self { + WithdrawalRequest::SweeperFunding(value) + } +} + /// Ethereum withdrawal request issued by the user. #[derive(Clone, Eq, PartialEq, Decode, Encode)] pub struct EthWithdrawalRequest { @@ -141,6 +167,29 @@ pub struct EthWithdrawalRequest { pub created_at: Option, } +/// Sweeper gas funding request issued by the minter itself. Never reimbursed. +#[derive(Clone, Eq, PartialEq, Decode, Encode)] +pub struct SweeperFundingRequest { + /// The ckETH burned for this funding, and the ceiling on what it may spend in total. + #[n(0)] + pub withdrawal_amount: Wei, + /// The minter's dedicated sweeper address (tECDSA derivation path `[3]`). + #[n(1)] + pub destination: Address, + /// The transaction ID of the ckETH burn on the ckETH ledger. + #[cbor(n(2), with = "crate::cbor::id")] + pub ledger_burn_index: LedgerBurnIndex, + /// The owner of the account from which the minter burned ckETH: the minter itself. + #[cbor(n(3), with = "icrc_cbor::principal")] + pub from: Principal, + /// The subaccount from which the minter burned ckETH. + #[n(4)] + pub from_subaccount: Option, + /// The IC time at which the funding was decided. + #[n(5)] + pub created_at: u64, +} + /// ERC-20 withdrawal request issued by the user. #[derive(Clone, Eq, PartialEq, Decode, Encode)] pub struct Erc20WithdrawalRequest { @@ -197,17 +246,23 @@ pub enum ReimbursementIndex { }, } -impl From<&WithdrawalRequest> for ReimbursementIndex { - fn from(value: &WithdrawalRequest) -> Self { +#[derive(Clone, Copy, Eq, PartialEq, Debug)] +pub struct NotReimbursable; + +impl TryFrom<&WithdrawalRequest> for ReimbursementIndex { + type Error = NotReimbursable; + + fn try_from(value: &WithdrawalRequest) -> Result { match value { - WithdrawalRequest::CkEth(request) => ReimbursementIndex::CkEth { + WithdrawalRequest::CkEth(request) => Ok(ReimbursementIndex::CkEth { ledger_burn_index: request.ledger_burn_index, - }, - WithdrawalRequest::CkErc20(request) => ReimbursementIndex::CkErc20 { + }), + WithdrawalRequest::CkErc20(request) => Ok(ReimbursementIndex::CkErc20 { cketh_ledger_burn_index: request.cketh_ledger_burn_index, ledger_id: request.ckerc20_ledger_id, ckerc20_ledger_burn_index: request.ckerc20_ledger_burn_index, - }, + }), + WithdrawalRequest::SweeperFunding(_) => Err(NotReimbursable), } } } @@ -311,6 +366,30 @@ impl fmt::Debug for EthWithdrawalRequest { } } +impl fmt::Debug for SweeperFundingRequest { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> { + let SweeperFundingRequest { + withdrawal_amount, + destination, + ledger_burn_index, + from, + from_subaccount, + created_at, + } = self; + f.debug_struct("SweeperFundingRequest") + .field("withdrawal_amount", withdrawal_amount) + .field("destination", destination) + .field("ledger_burn_index", ledger_burn_index) + .field("from", &format_args!("{from}")) + .field( + "from_subaccount", + &format_args!("{}", DisplayOption(from_subaccount)), + ) + .field("created_at", created_at) + .finish() + } +} + impl fmt::Debug for Erc20WithdrawalRequest { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> { let Erc20WithdrawalRequest { @@ -517,6 +596,12 @@ impl EthTransactions { "BUG: ERC-20 transaction amount should be zero" ); } + WithdrawalRequest::SweeperFunding(req) => { + assert!( + req.withdrawal_amount > transaction.amount, + "BUG: transaction amount should be the funded amount deducted from transaction fees" + ); + } } let nonce = self.next_nonce; assert_eq!(transaction.nonce, nonce, "BUG: transaction nonce mismatch"); @@ -534,6 +619,11 @@ impl EthTransactions { WithdrawalRequest::CkErc20(ckerc20) => ResubmissionStrategy::GuaranteeEthAmount { allowed_max_transaction_fee: ckerc20.max_transaction_fee, }, + WithdrawalRequest::SweeperFunding(funding) => { + ResubmissionStrategy::ReduceEthAmount { + withdrawal_amount: funding.withdrawal_amount, + } + } }, }; assert_eq!( @@ -544,12 +634,15 @@ impl EthTransactions { ), Ok(()) ); + let is_reimbursable = withdrawal_request.is_reimbursable(); assert_eq!( self.processed_withdrawal_requests .insert(withdrawal_id, withdrawal_request), None ); - assert!(self.maybe_reimburse.insert(withdrawal_id)); + if is_reimbursable { + assert!(self.maybe_reimburse.insert(withdrawal_id)); + } } pub fn record_signed_transaction( @@ -706,20 +799,29 @@ impl EthTransactions { Ok(()) ); - assert!( - self.maybe_reimburse.remove(&ledger_burn_index), - "failed to remove entry from maybe_reimburse with block index: {ledger_burn_index}", - ); + // Funding was never inserted, so asserting on its removal would trap the canister. + if self + .processed_withdrawal_requests + .get(&ledger_burn_index) + .expect("BUG: missing processed withdrawal request") + .is_reimbursable() + { + assert!( + self.maybe_reimburse.remove(&ledger_burn_index), + "failed to remove entry from maybe_reimburse with block index: {ledger_burn_index}", + ); + } let request = self.processed_withdrawal_requests .get(&ledger_burn_index) .expect("failed to find entry from processed_withdrawal_requests with block index: {ledger_burn_index}"); - let index = ReimbursementIndex::from(request); match &request { WithdrawalRequest::CkEth(request) => { if receipt.status == TransactionStatus::Failure { self.record_reimbursement_request( - index, + ReimbursementIndex::CkEth { + ledger_burn_index: request.ledger_burn_index, + }, ReimbursementRequest { ledger_burn_index, to: request.from, @@ -733,7 +835,11 @@ impl EthTransactions { WithdrawalRequest::CkErc20(request) => { if receipt.status == TransactionStatus::Failure { self.record_reimbursement_request( - index, + ReimbursementIndex::CkErc20 { + cketh_ledger_burn_index: request.cketh_ledger_burn_index, + ledger_id: request.ckerc20_ledger_id, + ckerc20_ledger_burn_index: request.ckerc20_ledger_burn_index, + }, ReimbursementRequest { ledger_burn_index: request.ckerc20_ledger_burn_index, reimbursed_amount: request.withdrawal_amount.change_units(), @@ -744,6 +850,25 @@ impl EthTransactions { ); } } + WithdrawalRequest::SweeperFunding(request) => { + if receipt.status == TransactionStatus::Failure { + // Funding is a plain value transfer to an address derived from the minter's + // own key, so there is no code for it to revert in: reaching this means an + // assumption broke. Logged rather than trapped, since the accounting holds + // either way — the burn simply stays unspent. + log!( + INFO, + "[record_finalized_transaction]: UNEXPECTED: sweeper funding {} of {} to \ + {} FAILED (tx {}), which should be impossible for a transfer to an \ + address the minter controls; the burn is NOT reimbursed and stays \ + available as prepaid gas", + ledger_burn_index, + request.withdrawal_amount, + request.destination, + receipt.transaction_hash, + ); + } + } } } @@ -881,6 +1006,12 @@ impl EthTransactions { ); } if tx.transaction_status() == &TransactionStatus::Failure { + // A sweeper funding request is never reimbursed, so this status is imprecise for + // one. Tolerated rather than given a status of its own, which would mean adding a + // variant to `retrieve_eth_status`' return type and breaking existing clients: + // funding cannot fail in the first place, being a plain value transfer to an + // address derived from the minter's own key. Revisit if funding ever goes through + // a contract, where a revert becomes possible. return ( RetrieveEthStatus::TxFinalized(TxFinalizedStatus::PendingReimbursement( EthTransaction { @@ -1119,15 +1250,26 @@ pub fn create_transaction( "BUG: gas limit should be non-zero" ); match withdrawal_request { - WithdrawalRequest::CkEth(request) => { + WithdrawalRequest::CkEth(EthWithdrawalRequest { + withdrawal_amount, + destination, + ledger_burn_index, + .. + }) + | WithdrawalRequest::SweeperFunding(SweeperFundingRequest { + 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 request.withdrawal_amount.checked_sub(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: request.ledger_burn_index, - allowed_max_transaction_fee: request.withdrawal_amount, + cketh_ledger_burn_index: *ledger_burn_index, + allowed_max_transaction_fee: *withdrawal_amount, actual_max_transaction_fee: max_transaction_fee, }); } @@ -1138,7 +1280,7 @@ pub fn create_transaction( 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: request.destination, + destination: *destination, amount: tx_amount, data: Vec::new(), 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 d6904c6542d6..e7fce5e6bd79 100644 --- a/rs/ethereum/cketh/minter/src/state/transactions/tests.rs +++ b/rs/ethereum/cketh/minter/src/state/transactions/tests.rs @@ -874,10 +874,12 @@ mod eth_transactions { test.price_at_tx_resubmission.clone(), ); let expected_resubmitted_tx_amount = match withdrawal_request { - WithdrawalRequest::CkEth(_) => initial_tx - .amount - .checked_sub(test.resubmitted_cketh_tx_amount_deduction) - .unwrap(), + WithdrawalRequest::CkEth(_) | WithdrawalRequest::SweeperFunding(_) => { + initial_tx + .amount + .checked_sub(test.resubmitted_cketh_tx_amount_deduction) + .unwrap() + } WithdrawalRequest::CkErc20(_) => initial_tx.amount, }; let expected_resubmitted_tx = Eip1559TransactionRequest { @@ -1819,7 +1821,8 @@ mod eth_transactions { let mut transactions = EthTransactions::new(TransactionNonce::ZERO); let mut rng = reproducible_rng(); let [withdrawal_request] = create_ck_withdrawal_requests(&mut rng); - let reimbursement_index = ReimbursementIndex::from(&withdrawal_request); + let reimbursement_index = ReimbursementIndex::try_from(&withdrawal_request) + .expect("BUG: create_ck_withdrawal_requests only builds user withdrawals"); let _eth_transaction = withdrawal_flow( &mut transactions, withdrawal_request, @@ -2040,7 +2043,8 @@ mod eth_transactions { let mut transactions = EthTransactions::new(TransactionNonce::ZERO); let mut rng = reproducible_rng(); let [withdrawal_request] = create_ck_withdrawal_requests(&mut rng); - let reimbursement_index = ReimbursementIndex::from(&withdrawal_request); + let reimbursement_index = ReimbursementIndex::try_from(&withdrawal_request) + .expect("BUG: create_ck_withdrawal_requests only builds user withdrawals"); let receipt = withdrawal_flow( &mut transactions, withdrawal_request, @@ -2118,6 +2122,222 @@ mod eth_transactions { transactions.record_finalized_transaction(cketh_ledger_burn_index, receipt.clone()); receipt } + + mod sweeper_funding { + use super::withdrawal_flow; + use super::*; + use crate::eth_logs::LedgerSubaccount; + use crate::lifecycle::EthereumNetwork; + use crate::numeric::TransactionCount; + use crate::numeric::{GasAmount, Wei, WeiPerGas}; + use crate::state::transactions::ResubmitTransactionError; + use crate::state::transactions::tests::{ + DEFAULT_CREATED_AT, DEFAULT_PRINCIPAL, DEFAULT_WITHDRAWAL_AMOUNT, + create_and_record_signed_transaction, + }; + use crate::state::transactions::{ + CreateTransactionError, NotReimbursable, ReimbursementIndex, SweeperFundingRequest, + create_transaction, + }; + use crate::tx::GasFeeEstimate; + use assert_matches::assert_matches; + use ic_ethereum_types::Address; + use maplit::{btreemap, btreeset}; + use std::str::FromStr; + + const SWEEPER_FUNDING_GAS_LIMIT: GasAmount = GasAmount::new(21_000); + + fn sweeper_funding_request() -> SweeperFundingRequest { + SweeperFundingRequest { + withdrawal_amount: Wei::new(DEFAULT_WITHDRAWAL_AMOUNT), + destination: Address::new([0x53; 20]), + ledger_burn_index: LedgerBurnIndex::new(15), + from: candid::Principal::from_str(DEFAULT_PRINCIPAL).unwrap(), + from_subaccount: LedgerSubaccount::from_bytes(crate::CKETH_FEE_SUBACCOUNT), + created_at: DEFAULT_CREATED_AT, + } + } + + #[test] + fn should_not_be_reimbursable() { + let request = Into::::into(sweeper_funding_request()); + + assert!(!request.is_reimbursable()); + assert_eq!( + ReimbursementIndex::try_from(&request), + Err(NotReimbursable), + "a funding request must not yield a reimbursement index" + ); + } + + #[test] + fn should_never_enter_maybe_reimburse() { + let mut transactions = EthTransactions::new(TransactionNonce::ZERO); + let funding = sweeper_funding_request(); + + transactions.record_withdrawal_request(funding.clone()); + let created_tx = create_and_record_transaction( + &mut transactions, + funding.clone(), + gas_fee_estimate(), + ); + create_and_record_signed_transaction(&mut transactions, created_tx); + + assert_eq!( + transactions.maybe_reimburse, + btreeset! {}, + "funding must not be tracked for reimbursement" + ); + } + + #[test] + fn should_not_reimburse_a_failed_funding() { + for status in [TransactionStatus::Success, TransactionStatus::Failure] { + let mut transactions = EthTransactions::new(TransactionNonce::ZERO); + + let _receipt = + withdrawal_flow(&mut transactions, sweeper_funding_request(), status); + + assert_eq!(transactions.maybe_reimburse, btreeset! {}); + assert_eq!( + transactions.reimbursement_requests, + btreemap! {}, + "a {status:?} funding must not create a reimbursement request" + ); + assert_eq!(transactions.reimbursed, btreemap! {}); + } + } + + #[test] + fn should_still_reimburse_a_failed_user_withdrawal() { + let mut transactions = EthTransactions::new(TransactionNonce::ZERO); + + let _receipt = withdrawal_flow( + &mut transactions, + cketh_withdrawal_request_with_index(LedgerBurnIndex::new(15)), + TransactionStatus::Failure, + ); + + assert_eq!( + transactions.reimbursement_requests.len(), + 1, + "a failed user withdrawal must still be reimbursed" + ); + } + + #[test] + fn should_deduct_the_transaction_fee_from_the_funded_amount() { + let funding = sweeper_funding_request(); + let gas_fee = gas_fee_estimate(); + let expected_fee = gas_fee + .clone() + .to_price(SWEEPER_FUNDING_GAS_LIMIT) + .max_transaction_fee(); + + let tx = create_transaction( + &Into::::into(funding.clone()), + TransactionNonce::ZERO, + gas_fee, + SWEEPER_FUNDING_GAS_LIMIT, + EthereumNetwork::Mainnet, + ) + .expect("the funded amount must cover the fee"); + + assert_eq!(tx.destination, funding.destination); + assert_eq!( + tx.amount, + funding + .withdrawal_amount + .checked_sub(expected_fee) + .expect("test setup: the fee must fit inside the funded amount"), + "the ETH delivered is the burn minus the fee, so total spend never exceeds the burn" + ); + assert!(tx.data.is_empty(), "funding is a plain value transfer"); + } + + #[test] + fn should_fail_to_create_a_transaction_when_the_fee_exceeds_the_funded_amount() { + let funding = SweeperFundingRequest { + withdrawal_amount: Wei::new(1), + ..sweeper_funding_request() + }; + + assert_matches!( + create_transaction( + &Into::::into(funding), + TransactionNonce::ZERO, + gas_fee_estimate(), + SWEEPER_FUNDING_GAS_LIMIT, + EthereumNetwork::Mainnet, + ), + Err(CreateTransactionError::InsufficientTransactionFee { + cketh_ledger_burn_index, + allowed_max_transaction_fee, + .. + }) if cketh_ledger_burn_index == LedgerBurnIndex::new(15) + && allowed_max_transaction_fee == Wei::new(1) + ); + } + + #[test] + fn should_cap_resubmission_at_the_funded_amount() { + let mut transactions = EthTransactions::new(TransactionNonce::ZERO); + let funding = sweeper_funding_request(); + transactions.record_withdrawal_request(funding.clone()); + let created_tx = create_and_record_transaction( + &mut transactions, + funding.clone(), + gas_fee_estimate(), + ); + create_and_record_signed_transaction(&mut transactions, created_tx); + + let spiked_fee = GasFeeEstimate { + base_fee_per_gas: WeiPerGas::from(10_000_000_000_000_u64), + ..gas_fee_estimate() + }; + let resubmitted = + transactions.create_resubmit_transactions(TransactionCount::ZERO, spiked_fee); + + assert_matches!( + resubmitted.first().expect("BUG: nothing to resubmit"), + Err(ResubmitTransactionError::InsufficientTransactionFee { + allowed_max_transaction_fee, + max_transaction_fee, + .. + }) if *allowed_max_transaction_fee == funding.withdrawal_amount + && *max_transaction_fee > funding.withdrawal_amount + ); + } + + #[test] + fn should_use_the_plain_transfer_gas_limit() { + let request: WithdrawalRequest = sweeper_funding_request().into(); + + assert_eq!( + crate::withdraw::estimate_gas_limit(&request), + crate::withdraw::CKETH_WITHDRAWAL_TRANSACTION_GAS_LIMIT, + ); + } + + #[test] + fn should_survive_an_event_encoding_roundtrip() { + use crate::state::event::{Event, EventType}; + use ic_stable_structures::storable::Storable; + + let event = Event { + timestamp: DEFAULT_CREATED_AT, + payload: EventType::AcceptedSweeperFundingRequest(sweeper_funding_request()), + }; + let bytes = event.to_bytes(); + + assert_eq!( + event, + Event::from_bytes(bytes.clone()), + "failed to decode {}", + hex::encode(bytes) + ); + } + } } mod oldest_incomplete_withdrawal_timestamp { @@ -2220,6 +2440,7 @@ mod oldest_incomplete_withdrawal_timestamp { match withdrawal_request { WithdrawalRequest::CkEth(request) => request.created_at = Some(created_at), WithdrawalRequest::CkErc20(request) => request.created_at = created_at, + WithdrawalRequest::SweeperFunding(request) => request.created_at = created_at, } } } diff --git a/rs/ethereum/cketh/minter/src/withdraw.rs b/rs/ethereum/cketh/minter/src/withdraw.rs index 2151505ea912..23d9a2e7dfc8 100644 --- a/rs/ethereum/cketh/minter/src/withdraw.rs +++ b/rs/ethereum/cketh/minter/src/withdraw.rs @@ -297,7 +297,9 @@ fn create_transactions_batch(gas_fee_estimate: GasFeeEstimate) { pub fn estimate_gas_limit(withdrawal_request: &WithdrawalRequest) -> GasAmount { match withdrawal_request { - WithdrawalRequest::CkEth(_) => CKETH_WITHDRAWAL_TRANSACTION_GAS_LIMIT, + WithdrawalRequest::CkEth(_) | WithdrawalRequest::SweeperFunding(_) => { + CKETH_WITHDRAWAL_TRANSACTION_GAS_LIMIT + } WithdrawalRequest::CkErc20(_) => CKERC20_WITHDRAWAL_TRANSACTION_GAS_LIMIT, } } diff --git a/rs/ethereum/cketh/minter/tests/dump_stable_memory.rs b/rs/ethereum/cketh/minter/tests/dump_stable_memory.rs index 682a243c67a9..5c368c30bf2f 100644 --- a/rs/ethereum/cketh/minter/tests/dump_stable_memory.rs +++ b/rs/ethereum/cketh/minter/tests/dump_stable_memory.rs @@ -18,7 +18,7 @@ use ic_cketh_minter::state::audit::EventType as ET; use ic_cketh_minter::state::event::Event; use ic_cketh_minter::state::transactions::{ Erc20WithdrawalRequest, EthWithdrawalRequest, Reimbursed, ReimbursementIndex, - ReimbursementRequest, + ReimbursementRequest, SweeperFundingRequest, }; use ic_cketh_minter::timed_sized_map::Timestamp; use ic_cketh_minter::tx::{ @@ -263,6 +263,21 @@ fn map_event(CandidEvent { timestamp, payload }: CandidEvent) -> Event { from_subaccount: from_subaccount.and_then(LedgerSubaccount::from_bytes), created_at, }), + EventPayload::AcceptedSweeperFundingRequest { + withdrawal_amount, + destination, + ledger_burn_index, + from, + from_subaccount, + created_at, + } => ET::AcceptedSweeperFundingRequest(SweeperFundingRequest { + withdrawal_amount: withdrawal_amount.try_into().unwrap(), + destination: destination.parse().unwrap(), + ledger_burn_index: map_nat(ledger_burn_index), + from, + from_subaccount: from_subaccount.and_then(LedgerSubaccount::from_bytes), + created_at, + }), EventPayload::CreatedTransaction { withdrawal_id, transaction,