From 4c438e638622a64d08386570015dd4dedbcaad70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mathias=20Bj=C3=B6rkqvist?= Date: Fri, 14 Aug 2026 08:40:39 +0000 Subject: [PATCH 01/12] feat(cketh): name the minter's ckETH fee subaccount The ckETH ledger credits withdrawal fees to a subaccount of the minter, and the sweeper funding that follows spends from it. Naming it here rather than repeating the bytes keeps the withdrawal-request tests and the funding task talking about the same account. Co-Authored-By: Claude Opus 5 (1M context) --- rs/ethereum/cketh/minter/src/lib.rs | 7 +++++++ 1 file changed, 7 insertions(+) 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 +}; From fd2e9f393d6120bfc98d948a5f1144565ab038b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mathias=20Bj=C3=B6rkqvist?= Date: Tue, 4 Aug 2026 15:06:35 +0000 Subject: [PATCH 02/12] feat(cketh): add the SweeperFunding withdrawal-request variant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sweeper fee funding is mechanically an ordinary ckETH withdrawal — same nonce sequence, same tECDSA signing, same fee-bumped resubmission — so it becomes a third `WithdrawalRequest` variant rather than a parallel pipeline. It differs in exactly one respect, and everything here turns on it: the ckETH burned for funding is NEVER re-minted, so a funding request must never reach the reimbursement machinery. Three places enforce that, all of which would otherwise fail only at runtime: - `maybe_reimburse` is the double-minting guard, and `record_reimbursement_request` asserts membership has been cleared before minting. Funding is now kept out of the set on insert, and the corresponding `assert!` on removal is conditional. Both are production asserts, so a missed branch would trap the canister. - `From<&WithdrawalRequest> for ReimbursementIndex` becomes `TryFrom` with a `NotReimbursable` error, deliberately fallible so the compiler proves at every call site that funding cannot produce an index — rather than a panicking arm that traps if a site is missed. The two callers construct their index inside the reimbursable arms instead. - A failed funding transaction records nothing to pay back, only a log line. Everything a later funding needs in order to offset against the unspent burn is already reconstructible from the accepted-request event plus the finalized transaction's receipt, so no second event type is introduced. Everything else follows ckETH: the 21'000 gas limit (a plain value transfer to a code-less EOA, which cannot revert), `ResubmissionStrategy::ReduceEthAmount` with the burned amount as the ceiling — so a climbing gas price shrinks the ETH delivered to the sweeper instead of breaking the invariant — and a fee carved out of the burned amount so `eth_balance` accounting needs no change. Funding is reported in `withdrawal_status`, the dashboard and the event log rather than hidden: it is a public, auditable action. A dedicated dashboard section with the prepaid-gas balance arrives with the observability work. Tests pin that difference, including a deliberate contrast test asserting a failed *user* withdrawal is still reimbursed — without it the no-reimbursement tests would also pass if reimbursement were broken for everything. The fee ceiling is asserted through behaviour (a spike past the burn yields InsufficientTransactionFee) rather than by inspecting the stored strategy, and the new CBOR event tag round-trips. Co-Authored-By: Claude Opus 5 (1M context) --- rs/ethereum/cketh/minter/cketh_minter.did | 10 + rs/ethereum/cketh/minter/src/dashboard.rs | 7 + .../cketh/minter/src/dashboard/tests.rs | 6 +- rs/ethereum/cketh/minter/src/endpoints.rs | 10 + rs/ethereum/cketh/minter/src/main.rs | 27 +- rs/ethereum/cketh/minter/src/state.rs | 8 + rs/ethereum/cketh/minter/src/state/audit.rs | 7 + .../cketh/minter/src/state/audit/tests.rs | 17 +- rs/ethereum/cketh/minter/src/state/event.rs | 5 +- rs/ethereum/cketh/minter/src/state/tests.rs | 37 ++- .../minter/src/state/transactions/mod.rs | 168 +++++++++++-- .../minter/src/state/transactions/tests.rs | 233 +++++++++++++++++- rs/ethereum/cketh/minter/src/withdraw.rs | 6 +- .../cketh/minter/tests/dump_stable_memory.rs | 17 +- 14 files changed, 516 insertions(+), 42 deletions(-) diff --git a/rs/ethereum/cketh/minter/cketh_minter.did b/rs/ethereum/cketh/minter/cketh_minter.did index 95c12594059d..e83a47a3644f 100644 --- a/rs/ethereum/cketh/minter/cketh_minter.did +++ b/rs/ethereum/cketh/minter/cketh_minter.did @@ -570,6 +570,16 @@ type Event = record { from_subaccount : opt blob; created_at: opt nat64; }; + // The minter burned ckETH from its fee subaccount to prepay gas for sweeping, + // and is sending that ETH to its dedicated sweeper address. + 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..7237aa8dabb0 100644 --- a/rs/ethereum/cketh/minter/src/endpoints.rs +++ b/rs/ethereum/cketh/minter/src/endpoints.rs @@ -489,6 +489,16 @@ pub mod events { from_subaccount: Option<[u8; 32]>, created_at: Option, }, + /// The minter burned ckETH from its fee subaccount to prepay gas for sweeping, and is + /// sending that ETH to its dedicated sweeper address. + 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/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..da708df7ef09 100644 --- a/rs/ethereum/cketh/minter/src/state.rs +++ b/rs/ethereum/cketh/minter/src/state.rs @@ -371,6 +371,14 @@ 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, + // As for ckETH, the fee was carved out of the burned amount. The accounting below then + // debits `eth_balance` by only the effective fee when the transaction failed, which is + // exactly right for funding too: the ETH stayed at the main address while the ckETH was + // burned, leaving ckETH over-backed rather than under-backed. + 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..7bd3599af2aa 100644 --- a/rs/ethereum/cketh/minter/src/state/audit.rs +++ b/rs/ethereum/cketh/minter/src/state/audit.rs @@ -74,6 +74,13 @@ pub fn apply_state_transition(state: &mut State, payload: &EventType) { .eth_transactions .record_withdrawal_request(request.clone()); } + EventType::AcceptedSweeperFundingRequest(request) => { + // Recorded through the same pipeline as a user withdrawal, so replay reconstructs it + // with the existing machinery; it differs only in never being reimbursed (`R14`). + 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..4c67e50438a9 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 }), @@ -1620,14 +1643,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..1fbea41a99c3 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,19 @@ impl EthTransactions { ); } } + WithdrawalRequest::SweeperFunding(request) => { + if receipt.status == TransactionStatus::Failure { + log!( + INFO, + "[record_finalized_transaction]: sweeper funding {} of {} to {} FAILED \ + (tx {}); the burn is NOT reimbursed and stays available as prepaid gas", + ledger_burn_index, + request.withdrawal_amount, + request.destination, + receipt.transaction_hash, + ); + } + } } } @@ -1119,15 +1238,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 +1268,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..6938dd866d5d 100644 --- a/rs/ethereum/cketh/minter/src/withdraw.rs +++ b/rs/ethereum/cketh/minter/src/withdraw.rs @@ -297,7 +297,11 @@ 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, + // Sweeper funding is a plain value transfer to a code-less EOA, so the 21'000 base cost is + // exactly right — and, having no code to run, such a transfer cannot revert. + 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, From 7336271517625549e2f09788cd738a211fd5bae5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mathias=20Bj=C3=B6rkqvist?= Date: Fri, 7 Aug 2026 14:00:52 +0000 Subject: [PATCH 03/12] style(cketh): drop the added comments where siblings carry none The Candid file documents no other event variant, and neither do the corresponding variants in `endpoints.rs` nor the surrounding code in `state.rs`, `state/audit.rs` and `withdraw.rs`. What stays is what the surrounding code already does: `event.rs` gives every variant a one-line doc, and `state/transactions/mod.rs` documents struct fields one line each, so `SweeperFundingRequest` mirrors `EthWithdrawalRequest`. Co-Authored-By: Claude Opus 5 (1M context) --- rs/ethereum/cketh/minter/cketh_minter.did | 3 --- rs/ethereum/cketh/minter/src/endpoints.rs | 2 -- rs/ethereum/cketh/minter/src/state.rs | 4 ---- rs/ethereum/cketh/minter/src/state/audit.rs | 2 -- rs/ethereum/cketh/minter/src/withdraw.rs | 2 -- 5 files changed, 13 deletions(-) diff --git a/rs/ethereum/cketh/minter/cketh_minter.did b/rs/ethereum/cketh/minter/cketh_minter.did index e83a47a3644f..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,8 +569,6 @@ type Event = record { from_subaccount : opt blob; created_at: opt nat64; }; - // The minter burned ckETH from its fee subaccount to prepay gas for sweeping, - // and is sending that ETH to its dedicated sweeper address. AcceptedSweeperFundingRequest : record { withdrawal_amount : nat; destination : text; diff --git a/rs/ethereum/cketh/minter/src/endpoints.rs b/rs/ethereum/cketh/minter/src/endpoints.rs index 7237aa8dabb0..581d68be48e0 100644 --- a/rs/ethereum/cketh/minter/src/endpoints.rs +++ b/rs/ethereum/cketh/minter/src/endpoints.rs @@ -489,8 +489,6 @@ pub mod events { from_subaccount: Option<[u8; 32]>, created_at: Option, }, - /// The minter burned ckETH from its fee subaccount to prepay gas for sweeping, and is - /// sending that ETH to its dedicated sweeper address. AcceptedSweeperFundingRequest { withdrawal_amount: Nat, destination: String, diff --git a/rs/ethereum/cketh/minter/src/state.rs b/rs/ethereum/cketh/minter/src/state.rs index da708df7ef09..0dc60857d859 100644 --- a/rs/ethereum/cketh/minter/src/state.rs +++ b/rs/ethereum/cketh/minter/src/state.rs @@ -371,10 +371,6 @@ 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, - // As for ckETH, the fee was carved out of the burned amount. The accounting below then - // debits `eth_balance` by only the effective fee when the transaction failed, which is - // exactly right for funding too: the ETH stayed at the main address while the ckETH was - // burned, leaving ckETH over-backed rather than under-backed. WithdrawalRequest::SweeperFunding(req) => req .withdrawal_amount .checked_sub(tx.transaction().amount) diff --git a/rs/ethereum/cketh/minter/src/state/audit.rs b/rs/ethereum/cketh/minter/src/state/audit.rs index 7bd3599af2aa..09a38f5d6b46 100644 --- a/rs/ethereum/cketh/minter/src/state/audit.rs +++ b/rs/ethereum/cketh/minter/src/state/audit.rs @@ -75,8 +75,6 @@ pub fn apply_state_transition(state: &mut State, payload: &EventType) { .record_withdrawal_request(request.clone()); } EventType::AcceptedSweeperFundingRequest(request) => { - // Recorded through the same pipeline as a user withdrawal, so replay reconstructs it - // with the existing machinery; it differs only in never being reimbursed (`R14`). state .eth_transactions .record_withdrawal_request(request.clone()); diff --git a/rs/ethereum/cketh/minter/src/withdraw.rs b/rs/ethereum/cketh/minter/src/withdraw.rs index 6938dd866d5d..23d9a2e7dfc8 100644 --- a/rs/ethereum/cketh/minter/src/withdraw.rs +++ b/rs/ethereum/cketh/minter/src/withdraw.rs @@ -297,8 +297,6 @@ fn create_transactions_batch(gas_fee_estimate: GasFeeEstimate) { pub fn estimate_gas_limit(withdrawal_request: &WithdrawalRequest) -> GasAmount { match withdrawal_request { - // Sweeper funding is a plain value transfer to a code-less EOA, so the 21'000 base cost is - // exactly right — and, having no code to run, such a transfer cannot revert. WithdrawalRequest::CkEth(_) | WithdrawalRequest::SweeperFunding(_) => { CKETH_WITHDRAWAL_TRANSACTION_GAS_LIMIT } From b3bb391a7d51e5ea3af854c7080062cf632af95b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mathias=20Bj=C3=B6rkqvist?= Date: Mon, 10 Aug 2026 08:38:19 +0000 Subject: [PATCH 04/12] fix(cketh): report a failed funding as failed, not pending reimbursement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `processed_transaction_status` classified every failed finalized transaction as `PendingReimbursement`, which the Candid interface defines as "transaction failed and will be reimbursed". A sweeper funding is never reimbursed, so `retrieve_eth_status` and `withdrawal_status` promised a reimbursement that nothing will ever settle — and the status would stay wrong forever, since only recording a reimbursement moves it on. Add a `Failed` variant to `TxFinalizedStatus` for a failure that will not be reimbursed, and pick between the two by asking the request whether it is reimbursable. This needs the didc override, as any addition to a returned variant does. Tests cover all three paths: a failed funding reports `Failed`, a successful one still reports `Success`, and a failed *user* withdrawal still reports `PendingReimbursement` — without that last one the first would also pass if the branch were inverted. Co-Authored-By: Claude Opus 5 (1M context) --- rs/ethereum/cketh/minter/cketh_minter.did | 2 + rs/ethereum/cketh/minter/src/endpoints.rs | 4 ++ .../minter/src/state/transactions/mod.rs | 20 ++++--- .../minter/src/state/transactions/tests.rs | 53 +++++++++++++++++++ 4 files changed, 71 insertions(+), 8 deletions(-) diff --git a/rs/ethereum/cketh/minter/cketh_minter.did b/rs/ethereum/cketh/minter/cketh_minter.did index cc49aaba7c5c..3d7e9a86bdcd 100644 --- a/rs/ethereum/cketh/minter/cketh_minter.did +++ b/rs/ethereum/cketh/minter/cketh_minter.did @@ -273,6 +273,8 @@ type TxFinalizedStatus = variant { }; // Transaction failed and will be reimbursed, PendingReimbursement : EthTransaction; + // Transaction failed and will not be reimbursed. + Failed : EthTransaction; }; // Retrieve the status of a withdrawal request. diff --git a/rs/ethereum/cketh/minter/src/endpoints.rs b/rs/ethereum/cketh/minter/src/endpoints.rs index 581d68be48e0..19dfbe3924ba 100644 --- a/rs/ethereum/cketh/minter/src/endpoints.rs +++ b/rs/ethereum/cketh/minter/src/endpoints.rs @@ -171,6 +171,7 @@ pub enum TxFinalizedStatus { effective_transaction_fee: Option, }, PendingReimbursement(EthTransaction), + Failed(EthTransaction), Reimbursed { transaction_hash: String, reimbursed_amount: Nat, @@ -192,6 +193,9 @@ impl Display for RetrieveEthStatus { TxFinalizedStatus::PendingReimbursement(tx) => { write!(f, "PendingReimbursement({})", tx.transaction_hash) } + TxFinalizedStatus::Failed(tx) => { + write!(f, "Failed({})", tx.transaction_hash) + } TxFinalizedStatus::Reimbursed { reimbursed_in_block, transaction_hash, diff --git a/rs/ethereum/cketh/minter/src/state/transactions/mod.rs b/rs/ethereum/cketh/minter/src/state/transactions/mod.rs index 1fbea41a99c3..97e25ddb97ff 100644 --- a/rs/ethereum/cketh/minter/src/state/transactions/mod.rs +++ b/rs/ethereum/cketh/minter/src/state/transactions/mod.rs @@ -1000,14 +1000,18 @@ impl EthTransactions { ); } if tx.transaction_status() == &TransactionStatus::Failure { - return ( - RetrieveEthStatus::TxFinalized(TxFinalizedStatus::PendingReimbursement( - EthTransaction { - transaction_hash: tx.transaction_hash().to_string(), - }, - )), - Some(tx.as_ref()), - ); + let failure = EthTransaction { + transaction_hash: tx.transaction_hash().to_string(), + }; + // A request that cannot be reimbursed must not report a pending reimbursement: + // nothing will ever settle it, so the status would stay wrong forever. + let status = match self.processed_withdrawal_requests.get(burn_index) { + Some(request) if !request.is_reimbursable() => { + TxFinalizedStatus::Failed(failure) + } + _ => TxFinalizedStatus::PendingReimbursement(failure), + }; + return (RetrieveEthStatus::TxFinalized(status), Some(tx.as_ref())); } return ( diff --git a/rs/ethereum/cketh/minter/src/state/transactions/tests.rs b/rs/ethereum/cketh/minter/src/state/transactions/tests.rs index e7fce5e6bd79..013ee0090cdc 100644 --- a/rs/ethereum/cketh/minter/src/state/transactions/tests.rs +++ b/rs/ethereum/cketh/minter/src/state/transactions/tests.rs @@ -2126,6 +2126,7 @@ mod eth_transactions { mod sweeper_funding { use super::withdrawal_flow; use super::*; + use crate::endpoints::{RetrieveEthStatus, TxFinalizedStatus}; use crate::eth_logs::LedgerSubaccount; use crate::lifecycle::EthereumNetwork; use crate::numeric::TransactionCount; @@ -2208,6 +2209,58 @@ mod eth_transactions { } } + #[test] + fn should_report_a_failed_funding_as_failed_not_pending_reimbursement() { + let mut transactions = EthTransactions::new(TransactionNonce::ZERO); + let funding = sweeper_funding_request(); + let burn_index = funding.ledger_burn_index; + + let receipt = withdrawal_flow(&mut transactions, funding, TransactionStatus::Failure); + + assert_eq!( + transactions.transaction_status(&burn_index), + RetrieveEthStatus::TxFinalized(TxFinalizedStatus::Failed((&receipt).into())), + "nothing will ever reimburse a funding, so PendingReimbursement would stay wrong \ + forever" + ); + } + + #[test] + fn should_report_a_successful_funding_as_success() { + let mut transactions = EthTransactions::new(TransactionNonce::ZERO); + let funding = sweeper_funding_request(); + let burn_index = funding.ledger_burn_index; + + let receipt = withdrawal_flow(&mut transactions, funding, TransactionStatus::Success); + + assert_matches!( + transactions.transaction_status(&burn_index), + RetrieveEthStatus::TxFinalized(TxFinalizedStatus::Success { + transaction_hash, + .. + }) if transaction_hash == receipt.transaction_hash.to_string() + ); + } + + #[test] + fn should_still_report_a_failed_user_withdrawal_as_pending_reimbursement() { + let mut transactions = EthTransactions::new(TransactionNonce::ZERO); + let burn_index = LedgerBurnIndex::new(15); + + let receipt = withdrawal_flow( + &mut transactions, + cketh_withdrawal_request_with_index(burn_index), + TransactionStatus::Failure, + ); + + assert_eq!( + transactions.transaction_status(&burn_index), + RetrieveEthStatus::TxFinalized(TxFinalizedStatus::PendingReimbursement( + (&receipt).into() + )) + ); + } + #[test] fn should_still_reimburse_a_failed_user_withdrawal() { let mut transactions = EthTransactions::new(TransactionNonce::ZERO); From 51c3ccf10667e7ca45295db4d73007b1e74ff86f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mathias=20Bj=C3=B6rkqvist?= Date: Fri, 14 Aug 2026 09:00:01 +0000 Subject: [PATCH 05/12] Revert "fix(cketh): report a failed funding as failed, not pending reimbursement" This reverts commit b3bb391a7d51e5ea3af854c7080062cf632af95b. --- rs/ethereum/cketh/minter/cketh_minter.did | 2 - rs/ethereum/cketh/minter/src/endpoints.rs | 4 -- .../minter/src/state/transactions/mod.rs | 20 +++---- .../minter/src/state/transactions/tests.rs | 53 ------------------- 4 files changed, 8 insertions(+), 71 deletions(-) diff --git a/rs/ethereum/cketh/minter/cketh_minter.did b/rs/ethereum/cketh/minter/cketh_minter.did index 3d7e9a86bdcd..cc49aaba7c5c 100644 --- a/rs/ethereum/cketh/minter/cketh_minter.did +++ b/rs/ethereum/cketh/minter/cketh_minter.did @@ -273,8 +273,6 @@ type TxFinalizedStatus = variant { }; // Transaction failed and will be reimbursed, PendingReimbursement : EthTransaction; - // Transaction failed and will not be reimbursed. - Failed : EthTransaction; }; // Retrieve the status of a withdrawal request. diff --git a/rs/ethereum/cketh/minter/src/endpoints.rs b/rs/ethereum/cketh/minter/src/endpoints.rs index 19dfbe3924ba..581d68be48e0 100644 --- a/rs/ethereum/cketh/minter/src/endpoints.rs +++ b/rs/ethereum/cketh/minter/src/endpoints.rs @@ -171,7 +171,6 @@ pub enum TxFinalizedStatus { effective_transaction_fee: Option, }, PendingReimbursement(EthTransaction), - Failed(EthTransaction), Reimbursed { transaction_hash: String, reimbursed_amount: Nat, @@ -193,9 +192,6 @@ impl Display for RetrieveEthStatus { TxFinalizedStatus::PendingReimbursement(tx) => { write!(f, "PendingReimbursement({})", tx.transaction_hash) } - TxFinalizedStatus::Failed(tx) => { - write!(f, "Failed({})", tx.transaction_hash) - } TxFinalizedStatus::Reimbursed { reimbursed_in_block, transaction_hash, diff --git a/rs/ethereum/cketh/minter/src/state/transactions/mod.rs b/rs/ethereum/cketh/minter/src/state/transactions/mod.rs index 97e25ddb97ff..1fbea41a99c3 100644 --- a/rs/ethereum/cketh/minter/src/state/transactions/mod.rs +++ b/rs/ethereum/cketh/minter/src/state/transactions/mod.rs @@ -1000,18 +1000,14 @@ impl EthTransactions { ); } if tx.transaction_status() == &TransactionStatus::Failure { - let failure = EthTransaction { - transaction_hash: tx.transaction_hash().to_string(), - }; - // A request that cannot be reimbursed must not report a pending reimbursement: - // nothing will ever settle it, so the status would stay wrong forever. - let status = match self.processed_withdrawal_requests.get(burn_index) { - Some(request) if !request.is_reimbursable() => { - TxFinalizedStatus::Failed(failure) - } - _ => TxFinalizedStatus::PendingReimbursement(failure), - }; - return (RetrieveEthStatus::TxFinalized(status), Some(tx.as_ref())); + return ( + RetrieveEthStatus::TxFinalized(TxFinalizedStatus::PendingReimbursement( + EthTransaction { + transaction_hash: tx.transaction_hash().to_string(), + }, + )), + Some(tx.as_ref()), + ); } return ( diff --git a/rs/ethereum/cketh/minter/src/state/transactions/tests.rs b/rs/ethereum/cketh/minter/src/state/transactions/tests.rs index 013ee0090cdc..e7fce5e6bd79 100644 --- a/rs/ethereum/cketh/minter/src/state/transactions/tests.rs +++ b/rs/ethereum/cketh/minter/src/state/transactions/tests.rs @@ -2126,7 +2126,6 @@ mod eth_transactions { mod sweeper_funding { use super::withdrawal_flow; use super::*; - use crate::endpoints::{RetrieveEthStatus, TxFinalizedStatus}; use crate::eth_logs::LedgerSubaccount; use crate::lifecycle::EthereumNetwork; use crate::numeric::TransactionCount; @@ -2209,58 +2208,6 @@ mod eth_transactions { } } - #[test] - fn should_report_a_failed_funding_as_failed_not_pending_reimbursement() { - let mut transactions = EthTransactions::new(TransactionNonce::ZERO); - let funding = sweeper_funding_request(); - let burn_index = funding.ledger_burn_index; - - let receipt = withdrawal_flow(&mut transactions, funding, TransactionStatus::Failure); - - assert_eq!( - transactions.transaction_status(&burn_index), - RetrieveEthStatus::TxFinalized(TxFinalizedStatus::Failed((&receipt).into())), - "nothing will ever reimburse a funding, so PendingReimbursement would stay wrong \ - forever" - ); - } - - #[test] - fn should_report_a_successful_funding_as_success() { - let mut transactions = EthTransactions::new(TransactionNonce::ZERO); - let funding = sweeper_funding_request(); - let burn_index = funding.ledger_burn_index; - - let receipt = withdrawal_flow(&mut transactions, funding, TransactionStatus::Success); - - assert_matches!( - transactions.transaction_status(&burn_index), - RetrieveEthStatus::TxFinalized(TxFinalizedStatus::Success { - transaction_hash, - .. - }) if transaction_hash == receipt.transaction_hash.to_string() - ); - } - - #[test] - fn should_still_report_a_failed_user_withdrawal_as_pending_reimbursement() { - let mut transactions = EthTransactions::new(TransactionNonce::ZERO); - let burn_index = LedgerBurnIndex::new(15); - - let receipt = withdrawal_flow( - &mut transactions, - cketh_withdrawal_request_with_index(burn_index), - TransactionStatus::Failure, - ); - - assert_eq!( - transactions.transaction_status(&burn_index), - RetrieveEthStatus::TxFinalized(TxFinalizedStatus::PendingReimbursement( - (&receipt).into() - )) - ); - } - #[test] fn should_still_reimburse_a_failed_user_withdrawal() { let mut transactions = EthTransactions::new(TransactionNonce::ZERO); From a6d546aa388509fc301c986168271fcdc751e3c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mathias=20Bj=C3=B6rkqvist?= Date: Fri, 14 Aug 2026 09:02:00 +0000 Subject: [PATCH 06/12] docs(cketh): say why a failed funding needs no status of its own Records the reasoning behind the revert above it. A funding transaction 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 and the failure branch cannot be reached; reporting a pending reimbursement that will never come is imprecise but unreachable, which is cheaper than breaking every client of retrieve_eth_status. The log at finalization now says the same thing, so if the assumption ever breaks it says so rather than reading like a routine outcome. Co-Authored-By: Claude Opus 5 (1M context) --- .../cketh/minter/src/state/transactions/mod.rs | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/rs/ethereum/cketh/minter/src/state/transactions/mod.rs b/rs/ethereum/cketh/minter/src/state/transactions/mod.rs index 1fbea41a99c3..483bfdfbfa3c 100644 --- a/rs/ethereum/cketh/minter/src/state/transactions/mod.rs +++ b/rs/ethereum/cketh/minter/src/state/transactions/mod.rs @@ -852,10 +852,16 @@ 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]: sweeper funding {} of {} to {} FAILED \ - (tx {}); the burn is NOT reimbursed and stays available as prepaid gas", + "[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, @@ -1000,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 { From f8b70daa3ba3a20ab1e50ef6fcefd557aada0264 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mathias=20Bj=C3=B6rkqvist?= Date: Fri, 14 Aug 2026 14:15:24 +0000 Subject: [PATCH 07/12] test(cketh): cover the balance accounting for a sweeper funding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Funding takes its own arm in record_finalized_transaction, and the ckETH and ckERC20 balance tests cannot reach it, so nothing here caught an incorrect debit, an uncounted fee, or the expect that traps when a funding's ceiling is below what its transaction moved. Asserted as the identity the accounting must satisfy — the ceiling covers the ETH delivered plus the fee, so the part of the fee left unspent is what stays with the minter — rather than as fixed numbers, which is also what makes the failing case's "only the fee is debited" meaningful. Co-Authored-By: Claude Opus 5 (1M context) --- rs/ethereum/cketh/minter/src/state/tests.rs | 102 ++++++++++++++++++++ 1 file changed, 102 insertions(+) diff --git a/rs/ethereum/cketh/minter/src/state/tests.rs b/rs/ethereum/cketh/minter/src/state/tests.rs index 4c67e50438a9..319bd90504fc 100644 --- a/rs/ethereum/cketh/minter/src/state/tests.rs +++ b/rs/ethereum/cketh/minter/src/state/tests.rs @@ -1524,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(); From 2479790dd2752aa5648524f4a47eedd62baab41f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mathias=20Bj=C3=B6rkqvist?= Date: Tue, 18 Aug 2026 09:03:37 +0000 Subject: [PATCH 08/12] refactor(cketh): address the sweeper-funding review nits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reuses `CKETH_WITHDRAWAL_TRANSACTION_GAS_LIMIT` instead of a local copy of 21'000, so the test no longer asserts against its own duplicate of the value under test, and asserts the burn index from the request rather than restating the literal. Drops the hand-written event round-trip test: `event_encoding_roundtrip` already proptests it, since `arb_event_type` includes the accepted-funding event, and arbitrary values beat one fixed instance. Says "unreachable" where the comment on a failed funding's status said "imprecise" — the state cannot arise for a bare transfer to an address derived from the minter's own key, which is the reason the imprecision is tolerable rather than a separate excuse. The two balance tests now share the flow they had in common, so each states only what its own request type makes different. Co-Authored-By: Claude Opus 5 (1M context) --- rs/ethereum/cketh/minter/src/state/tests.rs | 108 ++++++++++-------- .../minter/src/state/transactions/mod.rs | 13 ++- .../minter/src/state/transactions/tests.rs | 33 ++---- 3 files changed, 72 insertions(+), 82 deletions(-) diff --git a/rs/ethereum/cketh/minter/src/state/tests.rs b/rs/ethereum/cketh/minter/src/state/tests.rs index 319bd90504fc..e268e1c0ae42 100644 --- a/rs/ethereum/cketh/minter/src/state/tests.rs +++ b/rs/ethereum/cketh/minter/src/state/tests.rs @@ -1429,6 +1429,53 @@ mod eth_balance { assert_eq!(balance_after_erc20_deposit, balance_before); } + /// Both outcomes of one withdrawal flow, applied to independent copies of the same starting + /// state so a test can compare them directly. + struct BothOutcomes { + after_success: State, + after_failure: State, + receipt_succeeded: TransactionReceipt, + receipt_failed: TransactionReceipt, + } + + /// Runs the shared 21'000-gas flow against `state_before` twice, once finalizing successfully and + /// once with a failure receipt, leaving each caller to assert only what its request type makes + /// different. The gas fixture comes from a real Sepolia transaction and over-provisions the fee, + /// so part of it is always recorded as unspent. + fn apply_eth_transfer_both_ways>( + state_before: &State, + request: T, + ) -> BothOutcomes { + //Values from https://sepolia.etherscan.io/tx/0xef628b8f45984bdf386f5b765b665a2e584295e1190d21c6acdfabe17c27e1bb + let 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(request) + }; + + let mut after_success = state_before.clone(); + let receipt_succeeded = flow.clone().apply(&mut after_success); + + let mut after_failure = state_before.clone(); + let receipt_failed = WithdrawalFlow { + tx_status: TransactionStatus::Failure, + ..flow + } + .apply(&mut after_failure); + + BothOutcomes { + after_success, + after_failure, + receipt_succeeded, + receipt_failed, + } + } + #[test] fn should_update_after_successful_and_failed_withdrawal() { let mut state_before_withdrawal = initial_state(); @@ -1437,10 +1484,8 @@ mod eth_balance { &EventType::AcceptedDeposit(received_eth_event()), ); - let mut state_after_successful_withdrawal = state_before_withdrawal.clone(); let eth_balance_before_withdrawal = state_before_withdrawal.eth_balance.clone(); let erc20_balance_before_withdrawal = state_before_withdrawal.erc20_balances.clone(); - //Values from https://sepolia.etherscan.io/tx/0xef628b8f45984bdf386f5b765b665a2e584295e1190d21c6acdfabe17c27e1bb let withdrawal_request = EthWithdrawalRequest { withdrawal_amount: Wei::new(10_000_000_000_000_000), destination: "0xb44B5e756A894775FC32EDdf3314Bb1B1944dC34" @@ -1453,23 +1498,10 @@ mod eth_balance { from_subaccount: None, created_at: Some(1699527697000000000), }; - let withdrawal_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(withdrawal_request) - }; - withdrawal_flow - .clone() - .apply(&mut state_after_successful_withdrawal); - let eth_balance_after_successful_withdrawal = - state_after_successful_withdrawal.eth_balance.clone(); + let outcomes = apply_eth_transfer_both_ways(&state_before_withdrawal, withdrawal_request); + let eth_balance_after_successful_withdrawal = outcomes.after_success.eth_balance.clone(); let erc20_balance_after_successful_withdrawal = - state_after_successful_withdrawal.erc20_balances.clone(); + outcomes.after_success.erc20_balances.clone(); assert_eq!( eth_balance_after_successful_withdrawal, @@ -1493,15 +1525,9 @@ mod eth_balance { erc20_balance_after_successful_withdrawal ); - let mut state_after_failed_withdrawal = state_before_withdrawal.clone(); - let receipt_failed = WithdrawalFlow { - tx_status: TransactionStatus::Failure, - ..withdrawal_flow - } - .apply(&mut state_after_failed_withdrawal); - let eth_balance_after_failed_withdrawal = state_after_failed_withdrawal.eth_balance.clone(); - let erc20_balance_after_failed_withdrawal = - state_after_failed_withdrawal.erc20_balances.clone(); + let receipt_failed = &outcomes.receipt_failed; + let eth_balance_after_failed_withdrawal = outcomes.after_failure.eth_balance.clone(); + let erc20_balance_after_failed_withdrawal = outcomes.after_failure.erc20_balances.clone(); assert_eq!( eth_balance_after_failed_withdrawal.eth_balance, @@ -1551,22 +1577,9 @@ mod eth_balance { 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(); + let outcomes = apply_eth_transfer_both_ways(&state_before_funding, funding_request); + let receipt_succeeded = &outcomes.receipt_succeeded; + let after_success = outcomes.after_success.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 @@ -1600,13 +1613,8 @@ mod eth_balance { "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(); + let receipt_failed = &outcomes.receipt_failed; + let after_failure = outcomes.after_failure.eth_balance.clone(); assert_eq!( after_failure.eth_balance, diff --git a/rs/ethereum/cketh/minter/src/state/transactions/mod.rs b/rs/ethereum/cketh/minter/src/state/transactions/mod.rs index 483bfdfbfa3c..1aed7208617c 100644 --- a/rs/ethereum/cketh/minter/src/state/transactions/mod.rs +++ b/rs/ethereum/cketh/minter/src/state/transactions/mod.rs @@ -1006,12 +1006,13 @@ 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. + // Unreachable for a funding: the destination is derived from the minter's own + // key, so a bare transfer there has no code to revert in. Were it reached, the + // status would be wrong, since nothing reimburses a funding — tolerable only + // because it cannot happen, and not worth a status of its own, which would mean + // adding a variant to `retrieve_eth_status`' return type and breaking existing + // clients. Revisit if funding ever goes through a contract, where a revert becomes + // possible. return ( RetrieveEthStatus::TxFinalized(TxFinalizedStatus::PendingReimbursement( EthTransaction { diff --git a/rs/ethereum/cketh/minter/src/state/transactions/tests.rs b/rs/ethereum/cketh/minter/src/state/transactions/tests.rs index e7fce5e6bd79..561c2d385ccd 100644 --- a/rs/ethereum/cketh/minter/src/state/transactions/tests.rs +++ b/rs/ethereum/cketh/minter/src/state/transactions/tests.rs @@ -2129,7 +2129,7 @@ mod eth_transactions { use crate::eth_logs::LedgerSubaccount; use crate::lifecycle::EthereumNetwork; use crate::numeric::TransactionCount; - use crate::numeric::{GasAmount, Wei, WeiPerGas}; + use crate::numeric::{Wei, WeiPerGas}; use crate::state::transactions::ResubmitTransactionError; use crate::state::transactions::tests::{ DEFAULT_CREATED_AT, DEFAULT_PRINCIPAL, DEFAULT_WITHDRAWAL_AMOUNT, @@ -2140,13 +2140,12 @@ mod eth_transactions { create_transaction, }; use crate::tx::GasFeeEstimate; + use crate::withdraw::CKETH_WITHDRAWAL_TRANSACTION_GAS_LIMIT; 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), @@ -2231,14 +2230,14 @@ mod eth_transactions { let gas_fee = gas_fee_estimate(); let expected_fee = gas_fee .clone() - .to_price(SWEEPER_FUNDING_GAS_LIMIT) + .to_price(CKETH_WITHDRAWAL_TRANSACTION_GAS_LIMIT) .max_transaction_fee(); let tx = create_transaction( &Into::::into(funding.clone()), TransactionNonce::ZERO, gas_fee, - SWEEPER_FUNDING_GAS_LIMIT, + CKETH_WITHDRAWAL_TRANSACTION_GAS_LIMIT, EthereumNetwork::Mainnet, ) .expect("the funded amount must cover the fee"); @@ -2261,20 +2260,21 @@ mod eth_transactions { withdrawal_amount: Wei::new(1), ..sweeper_funding_request() }; + let expected_index = funding.ledger_burn_index; assert_matches!( create_transaction( &Into::::into(funding), TransactionNonce::ZERO, gas_fee_estimate(), - SWEEPER_FUNDING_GAS_LIMIT, + CKETH_WITHDRAWAL_TRANSACTION_GAS_LIMIT, EthereumNetwork::Mainnet, ), Err(CreateTransactionError::InsufficientTransactionFee { cketh_ledger_burn_index, allowed_max_transaction_fee, .. - }) if cketh_ledger_burn_index == LedgerBurnIndex::new(15) + }) if cketh_ledger_burn_index == expected_index && allowed_max_transaction_fee == Wei::new(1) ); } @@ -2318,25 +2318,6 @@ mod eth_transactions { 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) - ); - } } } From e67b6c73658a6bcf155e09f43a2a49ed04464b7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mathias=20Bj=C3=B6rkqvist?= Date: Tue, 18 Aug 2026 12:20:45 +0000 Subject: [PATCH 09/12] refactor(cketh): carry a sweeper funding as an EthWithdrawalRequest A funding is a ckETH burn from the minter's own fee subaccount sent to an address the minter controls; the only thing that sets it apart from a user's ckETH withdrawal is that it is never reimbursed, which the variant itself already says. So drop the separate payload struct and reuse `EthWithdrawalRequest`, which lets every arm that was duplicating the ckETH behaviour merge with it. The event's `created_at` becomes optional, exactly like the one on `AcceptedEthWithdrawalRequest`. --- rs/ethereum/cketh/minter/cketh_minter.did | 2 +- rs/ethereum/cketh/minter/src/dashboard.rs | 23 ++-- rs/ethereum/cketh/minter/src/endpoints.rs | 2 +- rs/ethereum/cketh/minter/src/main.rs | 4 +- rs/ethereum/cketh/minter/src/state.rs | 6 +- .../cketh/minter/src/state/audit/tests.rs | 4 +- rs/ethereum/cketh/minter/src/state/event.rs | 4 +- rs/ethereum/cketh/minter/src/state/tests.rs | 14 +-- .../minter/src/state/transactions/mod.rs | 112 +++++------------- .../minter/src/state/transactions/tests.rs | 43 +++---- .../cketh/minter/tests/dump_stable_memory.rs | 4 +- 11 files changed, 79 insertions(+), 139 deletions(-) diff --git a/rs/ethereum/cketh/minter/cketh_minter.did b/rs/ethereum/cketh/minter/cketh_minter.did index cc49aaba7c5c..8b6dbc545fb3 100644 --- a/rs/ethereum/cketh/minter/cketh_minter.did +++ b/rs/ethereum/cketh/minter/cketh_minter.did @@ -575,7 +575,7 @@ type Event = record { ledger_burn_index : nat; from : principal; from_subaccount : opt blob; - created_at: nat64; + created_at: opt nat64; }; CreatedTransaction : record { withdrawal_id : nat; diff --git a/rs/ethereum/cketh/minter/src/dashboard.rs b/rs/ethereum/cketh/minter/src/dashboard.rs index a6af2f034a1e..5d6eff4c5f88 100644 --- a/rs/ethereum/cketh/minter/src/dashboard.rs +++ b/rs/ethereum/cketh/minter/src/dashboard.rs @@ -340,13 +340,15 @@ impl DashboardTemplate { .withdrawal_requests_iter() .cloned() .map(|request| match request { - WithdrawalRequest::CkEth(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: req.created_at, - }, + WithdrawalRequest::CkEth(req) | 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: req.created_at, + } + } WithdrawalRequest::CkErc20(req) => { let erc20_contract_address = &req.erc20_contract_address; DashboardWithdrawalRequest { @@ -361,13 +363,6 @@ 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/endpoints.rs b/rs/ethereum/cketh/minter/src/endpoints.rs index 581d68be48e0..435fb03b4841 100644 --- a/rs/ethereum/cketh/minter/src/endpoints.rs +++ b/rs/ethereum/cketh/minter/src/endpoints.rs @@ -495,7 +495,7 @@ pub mod events { ledger_burn_index: Nat, from: Principal, from_subaccount: Option<[u8; 32]>, - created_at: u64, + created_at: Option, }, CreatedTransaction { withdrawal_id: Nat, diff --git a/rs/ethereum/cketh/minter/src/main.rs b/rs/ethereum/cketh/minter/src/main.rs index 4c728b7c6fdb..e4ac0bd3072b 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, SweeperFundingRequest, + ReimbursementRequest, }; use ic_cketh_minter::state::{ STATE, State, lazy_call_ecdsa_public_key, mutate_state, read_state, transactions, @@ -838,7 +838,7 @@ fn get_events(arg: GetEventsArg) -> GetEventsResult { from_subaccount: from_subaccount.map(LedgerSubaccount::to_bytes), created_at, }, - EventType::AcceptedSweeperFundingRequest(SweeperFundingRequest { + EventType::AcceptedSweeperFundingRequest(EthWithdrawalRequest { withdrawal_amount, destination, ledger_burn_index, diff --git a/rs/ethereum/cketh/minter/src/state.rs b/rs/ethereum/cketh/minter/src/state.rs index 0dc60857d859..24f1dab6f6cb 100644 --- a/rs/ethereum/cketh/minter/src/state.rs +++ b/rs/ethereum/cketh/minter/src/state.rs @@ -366,15 +366,11 @@ impl State { .get_processed_withdrawal_request(withdrawal_id) .expect("BUG: missing withdrawal request"); let charged_tx_fee = match withdrawal_request { - WithdrawalRequest::CkEth(req) => req + WithdrawalRequest::CkEth(req) | WithdrawalRequest::SweeperFunding(req) => req .withdrawal_amount .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/tests.rs b/rs/ethereum/cketh/minter/src/state/audit/tests.rs index f7238597c983..e633eef78b14 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, SweeperFundingRequest}; + use crate::state::transactions::EthWithdrawalRequest; fn map_event_source( CandidEventSource { @@ -322,7 +322,7 @@ impl GetEventsFile { from, from_subaccount, created_at, - } => ET::AcceptedSweeperFundingRequest(SweeperFundingRequest { + } => ET::AcceptedSweeperFundingRequest(EthWithdrawalRequest { withdrawal_amount: withdrawal_amount.try_into().unwrap(), destination: destination.parse().unwrap(), ledger_burn_index: map_nat(ledger_burn_index), diff --git a/rs/ethereum/cketh/minter/src/state/event.rs b/rs/ethereum/cketh/minter/src/state/event.rs index dd7494cd795e..21efd7dce2c2 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, SweeperFundingRequest, + ReimbursementRequest, }; use crate::timed_sized_map::Timestamp; use crate::tx::{Eip1559TransactionRequest, SignedEip1559TransactionRequest}; @@ -184,7 +184,7 @@ pub enum EventType { 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), + AcceptedSweeperFundingRequest(#[n(0)] EthWithdrawalRequest), } /// 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 e268e1c0ae42..789affbd13a2 100644 --- a/rs/ethereum/cketh/minter/src/state/tests.rs +++ b/rs/ethereum/cketh/minter/src/state/tests.rs @@ -14,7 +14,7 @@ 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, SweeperFundingRequest, + Erc20WithdrawalRequest, EthWithdrawalRequest, ReimbursementIndex, }; use crate::state::{Erc20Balances, State}; use crate::test_fixtures::{ @@ -745,9 +745,9 @@ prop_compose! { ledger_burn_index in any::(), from in arb_principal(), from_subaccount in arb_ledger_subaccount(), - created_at in any::(), - ) -> SweeperFundingRequest { - SweeperFundingRequest { + created_at in proptest::option::of(any::()), + ) -> EthWithdrawalRequest { + EthWithdrawalRequest { withdrawal_amount, destination, ledger_burn_index: ledger_burn_index.into(), @@ -1565,7 +1565,7 @@ mod eth_balance { 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 { + let funding_request = WithdrawalRequest::SweeperFunding(EthWithdrawalRequest { withdrawal_amount: funding_amount, destination: "0xb44B5e756A894775FC32EDdf3314Bb1B1944dC34" .parse() @@ -1575,8 +1575,8 @@ mod eth_balance { .parse() .unwrap(), from_subaccount: None, - created_at: 1699527697000000000, - }; + created_at: Some(1699527697000000000), + }); let outcomes = apply_eth_transfer_both_ways(&state_before_funding, funding_request); let receipt_succeeded = &outcomes.receipt_succeeded; let after_success = outcomes.after_success.eth_balance.clone(); diff --git a/rs/ethereum/cketh/minter/src/state/transactions/mod.rs b/rs/ethereum/cketh/minter/src/state/transactions/mod.rs index 1aed7208617c..18b0752bc65c 100644 --- a/rs/ethereum/cketh/minter/src/state/transactions/mod.rs +++ b/rs/ethereum/cketh/minter/src/state/transactions/mod.rs @@ -38,7 +38,10 @@ pub enum WithdrawalSearchParameter { pub enum WithdrawalRequest { CkEth(EthWithdrawalRequest), CkErc20(Erc20WithdrawalRequest), - SweeperFunding(SweeperFundingRequest), + /// Carries the same payload as [`WithdrawalRequest::CkEth`] — a burn of the minter's own + /// ckETH, transferred to the sweeper address — but is never reimbursed, so it needs a + /// variant of its own rather than a flag on the payload. + SweeperFunding(EthWithdrawalRequest), } impl WithdrawalRequest { @@ -52,43 +55,48 @@ impl WithdrawalRequest { pub fn created_at(&self) -> Option { match self { - WithdrawalRequest::CkEth(request) => request.created_at, + WithdrawalRequest::CkEth(request) | WithdrawalRequest::SweeperFunding(request) => { + request.created_at + } WithdrawalRequest::CkErc20(request) => Some(request.created_at), - WithdrawalRequest::SweeperFunding(request) => Some(request.created_at), } } /// Address to which the funds are to be sent to. pub fn payee(&self) -> Address { match self { - WithdrawalRequest::CkEth(request) => request.destination, + WithdrawalRequest::CkEth(request) | WithdrawalRequest::SweeperFunding(request) => { + request.destination + } WithdrawalRequest::CkErc20(request) => request.destination, - WithdrawalRequest::SweeperFunding(request) => request.destination, } } /// Address to which the transaction is to be sent to. pub fn destination(&self) -> Address { match self { - WithdrawalRequest::CkEth(request) => request.destination, + WithdrawalRequest::CkEth(request) | WithdrawalRequest::SweeperFunding(request) => { + request.destination + } WithdrawalRequest::CkErc20(request) => request.erc20_contract_address, - WithdrawalRequest::SweeperFunding(request) => request.destination, } } pub fn from(&self) -> Principal { match self { - WithdrawalRequest::CkEth(request) => request.from, + WithdrawalRequest::CkEth(request) | WithdrawalRequest::SweeperFunding(request) => { + request.from + } WithdrawalRequest::CkErc20(request) => request.from, - WithdrawalRequest::SweeperFunding(request) => request.from, } } pub fn from_subaccount(&self) -> Option<&LedgerSubaccount> { match self { - WithdrawalRequest::CkEth(request) => request.from_subaccount.as_ref(), + WithdrawalRequest::CkEth(request) | WithdrawalRequest::SweeperFunding(request) => { + request.from_subaccount.as_ref() + } WithdrawalRequest::CkErc20(request) => request.from_subaccount.as_ref(), - WithdrawalRequest::SweeperFunding(request) => request.from_subaccount.as_ref(), } } @@ -138,12 +146,6 @@ 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 { @@ -167,29 +169,6 @@ 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 { @@ -366,30 +345,6 @@ 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 { @@ -583,7 +538,7 @@ impl EthTransactions { "BUG: withdrawal request and transaction destination mismatch" ); match &withdrawal_request { - WithdrawalRequest::CkEth(req) => { + WithdrawalRequest::CkEth(req) | WithdrawalRequest::SweeperFunding(req) => { assert!( req.withdrawal_amount > transaction.amount, "BUG: transaction amount should be the withdrawal amount deducted from transaction fees" @@ -596,12 +551,6 @@ 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"); @@ -613,17 +562,14 @@ impl EthTransactions { let transaction_request = TransactionRequest { transaction, resubmission: match &withdrawal_request { - WithdrawalRequest::CkEth(cketh) => ResubmissionStrategy::ReduceEthAmount { - withdrawal_amount: cketh.withdrawal_amount, - }, - WithdrawalRequest::CkErc20(ckerc20) => ResubmissionStrategy::GuaranteeEthAmount { - allowed_max_transaction_fee: ckerc20.max_transaction_fee, - }, - WithdrawalRequest::SweeperFunding(funding) => { + WithdrawalRequest::CkEth(cketh) | WithdrawalRequest::SweeperFunding(cketh) => { ResubmissionStrategy::ReduceEthAmount { - withdrawal_amount: funding.withdrawal_amount, + withdrawal_amount: cketh.withdrawal_amount, } } + WithdrawalRequest::CkErc20(ckerc20) => ResubmissionStrategy::GuaranteeEthAmount { + allowed_max_transaction_fee: ckerc20.max_transaction_fee, + }, }, }; assert_eq!( @@ -855,13 +801,13 @@ impl EthTransactions { // 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. + // either way — the burn stays burned and the ETH stays backing ckETH. 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", + address the minter controls; the burn is NOT reimbursed: the ETH \ + never left the main address and now over-backs ckETH", ledger_burn_index, request.withdrawal_amount, request.destination, @@ -1257,7 +1203,7 @@ pub fn create_transaction( ledger_burn_index, .. }) - | WithdrawalRequest::SweeperFunding(SweeperFundingRequest { + | WithdrawalRequest::SweeperFunding(EthWithdrawalRequest { withdrawal_amount, destination, ledger_burn_index, diff --git a/rs/ethereum/cketh/minter/src/state/transactions/tests.rs b/rs/ethereum/cketh/minter/src/state/transactions/tests.rs index 561c2d385ccd..7e7bf972a395 100644 --- a/rs/ethereum/cketh/minter/src/state/transactions/tests.rs +++ b/rs/ethereum/cketh/minter/src/state/transactions/tests.rs @@ -2136,7 +2136,7 @@ mod eth_transactions { create_and_record_signed_transaction, }; use crate::state::transactions::{ - CreateTransactionError, NotReimbursable, ReimbursementIndex, SweeperFundingRequest, + CreateTransactionError, EthWithdrawalRequest, NotReimbursable, ReimbursementIndex, create_transaction, }; use crate::tx::GasFeeEstimate; @@ -2146,20 +2146,24 @@ mod eth_transactions { use maplit::{btreemap, btreeset}; use std::str::FromStr; - fn sweeper_funding_request() -> SweeperFundingRequest { - SweeperFundingRequest { + fn sweeper_funding_payload() -> EthWithdrawalRequest { + EthWithdrawalRequest { 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, + created_at: Some(DEFAULT_CREATED_AT), } } + fn sweeper_funding_request() -> WithdrawalRequest { + WithdrawalRequest::SweeperFunding(sweeper_funding_payload()) + } + #[test] fn should_not_be_reimbursable() { - let request = Into::::into(sweeper_funding_request()); + let request = sweeper_funding_request(); assert!(!request.is_reimbursable()); assert_eq!( @@ -2226,7 +2230,7 @@ mod eth_transactions { #[test] fn should_deduct_the_transaction_fee_from_the_funded_amount() { - let funding = sweeper_funding_request(); + let funding = sweeper_funding_payload(); let gas_fee = gas_fee_estimate(); let expected_fee = gas_fee .clone() @@ -2234,7 +2238,7 @@ mod eth_transactions { .max_transaction_fee(); let tx = create_transaction( - &Into::::into(funding.clone()), + &WithdrawalRequest::SweeperFunding(funding.clone()), TransactionNonce::ZERO, gas_fee, CKETH_WITHDRAWAL_TRANSACTION_GAS_LIMIT, @@ -2256,15 +2260,15 @@ mod eth_transactions { #[test] fn should_fail_to_create_a_transaction_when_the_fee_exceeds_the_funded_amount() { - let funding = SweeperFundingRequest { + let funding = EthWithdrawalRequest { withdrawal_amount: Wei::new(1), - ..sweeper_funding_request() + ..sweeper_funding_payload() }; let expected_index = funding.ledger_burn_index; assert_matches!( create_transaction( - &Into::::into(funding), + &WithdrawalRequest::SweeperFunding(funding), TransactionNonce::ZERO, gas_fee_estimate(), CKETH_WITHDRAWAL_TRANSACTION_GAS_LIMIT, @@ -2282,13 +2286,11 @@ mod eth_transactions { #[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(), - ); + let funding = sweeper_funding_payload(); + let request = WithdrawalRequest::SweeperFunding(funding.clone()); + transactions.record_withdrawal_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); let spiked_fee = GasFeeEstimate { @@ -2311,7 +2313,7 @@ mod eth_transactions { #[test] fn should_use_the_plain_transfer_gas_limit() { - let request: WithdrawalRequest = sweeper_funding_request().into(); + let request = sweeper_funding_request(); assert_eq!( crate::withdraw::estimate_gas_limit(&request), @@ -2419,9 +2421,10 @@ mod oldest_incomplete_withdrawal_timestamp { fn set_created_at(withdrawal_request: &mut WithdrawalRequest, created_at: u64) { match withdrawal_request { - WithdrawalRequest::CkEth(request) => request.created_at = Some(created_at), + WithdrawalRequest::CkEth(request) | WithdrawalRequest::SweeperFunding(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/tests/dump_stable_memory.rs b/rs/ethereum/cketh/minter/tests/dump_stable_memory.rs index 5c368c30bf2f..aecf5588bb10 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, SweeperFundingRequest, + ReimbursementRequest, }; use ic_cketh_minter::timed_sized_map::Timestamp; use ic_cketh_minter::tx::{ @@ -270,7 +270,7 @@ fn map_event(CandidEvent { timestamp, payload }: CandidEvent) -> Event { from, from_subaccount, created_at, - } => ET::AcceptedSweeperFundingRequest(SweeperFundingRequest { + } => ET::AcceptedSweeperFundingRequest(EthWithdrawalRequest { withdrawal_amount: withdrawal_amount.try_into().unwrap(), destination: destination.parse().unwrap(), ledger_burn_index: map_nat(ledger_burn_index), From ed2177c4ca646bd5932e53a7dd6b2e305b1767ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mathias=20Bj=C3=B6rkqvist?= Date: Tue, 18 Aug 2026 12:20:45 +0000 Subject: [PATCH 10/12] docs(cketh): stop offsetting burned-but-unspent sweeper funds `R14` had the minter track what it burned but did not spend and discount it from the next funding burn. Dropping that keeps the burn-first invariant -- the surplus is simply left as backing, as with the unspent gas of a user withdrawal -- and removes the only piece of funding accounting that had no counterpart in the withdrawal flow. A failed funding is likewise not reimbursed, which the spec now says as well. --- rs/ethereum/cketh/docs/deposit_from_cex.md | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/rs/ethereum/cketh/docs/deposit_from_cex.md b/rs/ethereum/cketh/docs/deposit_from_cex.md index f8dda8f94bcb..88356cd8f03e 100644 --- a/rs/ethereum/cketh/docs/deposit_from_cex.md +++ b/rs/ethereum/cketh/docs/deposit_from_cex.md @@ -129,10 +129,11 @@ _Requirements are grouped by phase, not numbered sequentially: `R11` and `R12` a a sweep transaction, the minter burns from its fee account on the ckETH ledger at least the maximum fee of that transaction; at all times, cumulative ckETH burned for sweeping ≥ cumulative ETH spent on sweeping. Burned-but-unspent amounts are - tracked and offset against subsequent burns; they are never re-minted. If the fee - account cannot cover a sweep, no sweep is submitted. (The burn happens ahead of - time: funding the sweeper address is an ordinary ckETH withdrawal from the fee - account, covering many sweeps — see step 0.) + never re-minted, and are not credited against subsequent burns either: like the + unspent gas of a user withdrawal, the surplus simply stays with the minter as + additional backing. If the fee account cannot cover a sweep, no sweep is + submitted. (The burn happens ahead of time: funding the sweeper address is an + ordinary ckETH withdrawal from the fee account, covering many sweeps — see step 0.) * `R15`: A single user-visible step suffices: after one `deposit_erc20` call, a deposit arriving at that address within its *scanning window* is credited with no further canister call by the user or frontend. Re-calling @@ -350,7 +351,13 @@ with an `icrc1_balance_of` of `1_762_128_000_000_000_000` wei ≈ 1.76 ckETH as * The sweeper address' balance *is* the `prepaid_sweep_gas` counter, reconcilable on-chain with one `eth_getBalance`. Sweep gas draws it down; burned ckETH is **never re-minted**, so "cumulative burned ≥ cumulative spent" holds at every - instant. + instant. Each funding round burns for its own transfer alone: the fee a previous + funding provisioned but did not spend is left as backing rather than discounted + from the next burn, which keeps funding accounted for exactly like a user + withdrawal. In the same spirit, a funding whose transaction fails is not + reimbursed — the whole burn stays as backing. That is a plain-transfer send to an + address derived from the minter's own key, so there is no code there to revert in; + accepting the loss buys an accounting with no reimbursement path to audit. * Fundings and per-sweep effective fees are audit events; the sweeper balance and the fee/cost ratio are exposed on the dashboard (`R8`, `R9`) to recalibrate `deposit_fee` via proposal. @@ -1044,7 +1051,7 @@ Unit tests (in `tests.rs` files per module, helpers in `test_fixtures.rs`): * Balance-delta crediting monotonicity across sweep interleavings (`R11`). * `R14` funding accounting: the burn at sweeper funding covers the transferred amount plus the funding fee, the sweeper balance reconciles on-chain, and the - surplus is never re-minted. + surplus is neither re-minted nor offset against the next funding's burn. * Event replay: state reconstructed from audit events equals live state (`R8`). Integration tests (state-machine tests in `rs/ethereum/cketh/minter/tests` with the From cd5926a792e8927dee29134860f950f39e77d3d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mathias=20Bj=C3=B6rkqvist?= Date: Tue, 18 Aug 2026 15:42:33 +0000 Subject: [PATCH 11/12] fix(cketh): record an accepted funding as a funding, not a withdrawal Sharing `EthWithdrawalRequest` between the two variants made the conversion at this call site silent: `record_withdrawal_request` takes anything that converts into a `WithdrawalRequest`, and the payload's own conversion yields `CkEth`. So a funding replayed from the audit log came back as an ordinary user withdrawal -- reimbursable, and invisible to the funding accounting that keys off the variant. Names the variant instead, and pins it with a test that replays the event and checks both the variant and that it cannot be reimbursed. Nothing caught this before because balance accounting treats the two identically. --- rs/ethereum/cketh/minter/src/state/audit.rs | 6 ++- rs/ethereum/cketh/minter/src/state/tests.rs | 43 +++++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/rs/ethereum/cketh/minter/src/state/audit.rs b/rs/ethereum/cketh/minter/src/state/audit.rs index 09a38f5d6b46..903b9a550781 100644 --- a/rs/ethereum/cketh/minter/src/state/audit.rs +++ b/rs/ethereum/cketh/minter/src/state/audit.rs @@ -6,7 +6,7 @@ pub use super::event::{Event, EventType}; use crate::erc20::CkTokenSymbol; use crate::state::eth_logs_scraping::LogScrapingId; use crate::state::eth_logs_scraping::LogScrapingId::Erc20DepositWithoutSubaccount; -use crate::state::transactions::{Reimbursed, ReimbursementIndex}; +use crate::state::transactions::{Reimbursed, ReimbursementIndex, WithdrawalRequest}; use crate::storage::{record_event, with_event_iter}; /// Updates the state to reflect the given state transition. @@ -75,9 +75,11 @@ pub fn apply_state_transition(state: &mut State, payload: &EventType) { .record_withdrawal_request(request.clone()); } EventType::AcceptedSweeperFundingRequest(request) => { + // Named explicitly: the payload converts to `CkEth` on its own, which would make the + // funding reimbursable. state .eth_transactions - .record_withdrawal_request(request.clone()); + .record_withdrawal_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 789affbd13a2..4911c71d9134 100644 --- a/rs/ethereum/cketh/minter/src/state/tests.rs +++ b/rs/ethereum/cketh/minter/src/state/tests.rs @@ -1348,6 +1348,49 @@ fn state_equivalence() { ); } +mod sweeper_funding { + use super::*; + use crate::eth_logs::LedgerSubaccount; + use crate::numeric::{LedgerBurnIndex, Wei}; + use crate::state::audit::apply_state_transition; + use crate::state::transactions::{EthWithdrawalRequest, WithdrawalRequest}; + use crate::test_fixtures::initial_state; + use assert_matches::assert_matches; + + #[test] + fn should_record_an_accepted_funding_as_a_funding_request() { + let mut state = initial_state(); + let funding = EthWithdrawalRequest { + withdrawal_amount: Wei::new(10_000_000_000_000_000), + destination: "0x5353535353535353535353535353535353535353" + .parse() + .unwrap(), + ledger_burn_index: LedgerBurnIndex::new(0), + from: "k2t6j-2nvnp-4zjm3-25dtz-6xhaa-c7boj-5gayf-oj3xs-i43lp-teztq-6ae" + .parse() + .unwrap(), + from_subaccount: LedgerSubaccount::from_bytes(crate::CKETH_FEE_SUBACCOUNT), + created_at: Some(1699527697000000000), + }; + + apply_state_transition( + &mut state, + &EventType::AcceptedSweeperFundingRequest(funding), + ); + + let request = state + .eth_transactions + .withdrawal_requests_iter() + .next() + .expect("BUG: the funding request was not recorded"); + assert_matches!(request, WithdrawalRequest::SweeperFunding(_)); + assert!( + !request.is_reimbursable(), + "recorded as an ordinary withdrawal, a failed funding would be reimbursed" + ); + } +} + mod eth_balance { use super::*; use crate::eth_rpc_client::responses::{TransactionReceipt, TransactionStatus}; From 46e91ba5ea005c4df2e7aadcf83ad6cfdbd828a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mathias=20Bj=C3=B6rkqvist?= Date: Tue, 18 Aug 2026 17:53:04 +0000 Subject: [PATCH 12/12] docs(cketh): account for the gas a failed funding still pays A failed transaction is included in a block and its gas is charged, which `record_finalized_transaction` duly debits. Saying the whole burn stays as backing therefore overstated it by exactly that fee, in the design doc and in the operational log, which now names the amount. The behaviour and its tests are unchanged. --- rs/ethereum/cketh/docs/deposit_from_cex.md | 7 ++++--- rs/ethereum/cketh/minter/src/state/transactions/mod.rs | 9 ++++++--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/rs/ethereum/cketh/docs/deposit_from_cex.md b/rs/ethereum/cketh/docs/deposit_from_cex.md index 88356cd8f03e..db79b36d9c5e 100644 --- a/rs/ethereum/cketh/docs/deposit_from_cex.md +++ b/rs/ethereum/cketh/docs/deposit_from_cex.md @@ -355,9 +355,10 @@ with an `icrc1_balance_of` of `1_762_128_000_000_000_000` wei ≈ 1.76 ckETH as funding provisioned but did not spend is left as backing rather than discounted from the next burn, which keeps funding accounted for exactly like a user withdrawal. In the same spirit, a funding whose transaction fails is not - reimbursed — the whole burn stays as backing. That is a plain-transfer send to an - address derived from the minter's own key, so there is no code there to revert in; - accepting the loss buys an accounting with no reimbursement path to audit. + reimbursed: no ETH reaches the sweeper, the failed transaction still pays for its + gas, and the burn minus that gas stays as backing. That is a plain-transfer send to + an address derived from the minter's own key, so there is no code there to revert + in; accepting the loss buys an accounting with no reimbursement path to audit. * Fundings and per-sweep effective fees are audit events; the sweeper balance and the fee/cost ratio are exposed on the dashboard (`R8`, `R9`) to recalibrate `deposit_fee` via proposal. diff --git a/rs/ethereum/cketh/minter/src/state/transactions/mod.rs b/rs/ethereum/cketh/minter/src/state/transactions/mod.rs index 18b0752bc65c..4fb01418836e 100644 --- a/rs/ethereum/cketh/minter/src/state/transactions/mod.rs +++ b/rs/ethereum/cketh/minter/src/state/transactions/mod.rs @@ -801,17 +801,20 @@ impl EthTransactions { // 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 stays burned and the ETH stays backing ckETH. + // either way — the burn stays burned, and of the ETH it covered only the gas + // of the failed transaction actually left the main address. 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: the ETH \ - never left the main address and now over-backs ckETH", + address the minter controls; the burn is NOT reimbursed: no ETH reached \ + the sweeper, the failed transaction still paid {} of gas, and the rest \ + of the burn now over-backs ckETH", ledger_burn_index, request.withdrawal_amount, request.destination, receipt.transaction_hash, + receipt.effective_transaction_fee(), ); } }