Skip to content
9 changes: 8 additions & 1 deletion rs/ethereum/cketh/minter/cketh_minter.did
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down Expand Up @@ -570,6 +569,14 @@ type Event = record {
from_subaccount : opt blob;
created_at: opt nat64;
};
AcceptedSweeperFundingRequest : record {
withdrawal_amount : nat;
destination : text;
ledger_burn_index : nat;
from : principal;
from_subaccount : opt blob;
created_at: nat64;
};
CreatedTransaction : record {
withdrawal_id : nat;
transaction : UnsignedTransaction;
Expand Down
7 changes: 7 additions & 0 deletions rs/ethereum/cketh/minter/src/dashboard.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
6 changes: 5 additions & 1 deletion rs/ethereum/cketh/minter/src/dashboard/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
},
)
}
Expand Down
8 changes: 8 additions & 0 deletions rs/ethereum/cketh/minter/src/endpoints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,14 @@ pub mod events {
from_subaccount: Option<[u8; 32]>,
created_at: Option<u64>,
},
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,
Expand Down
7 changes: 7 additions & 0 deletions rs/ethereum/cketh/minter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
};
27 changes: 24 additions & 3 deletions rs/ethereum/cketh/minter/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -407,7 +407,9 @@ async fn withdrawal_status(parameter: WithdrawalSearchParameter) -> Vec<Withdraw
withdrawal_id: *request.cketh_ledger_burn_index().as_ref(),
recipient_address: request.payee().to_string(),
token_symbol: match request {
CkEth(_) => 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)
Expand All @@ -417,12 +419,16 @@ async fn withdrawal_status(parameter: WithdrawalSearchParameter) -> Vec<Withdraw
withdrawal_amount: match request {
CkEth(r) => 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(),
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 4 additions & 0 deletions rs/ethereum/cketh/minter/src/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,10 @@ impl State {
.checked_sub(tx.transaction().amount)
.expect("BUG: withdrawal amount MUST always be at least the transaction amount"),
WithdrawalRequest::CkErc20(req) => req.max_transaction_fee,
WithdrawalRequest::SweeperFunding(req) => req

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressing the suppressed comment from the latest Copilot round, which flagged that this arm has no coverage: correct, and fixed in f8b70da with should_update_after_successful_and_failed_sweeper_funding next to the ckETH and ckERC20 balance tests.

One detail of the report is worth correcting for anyone reading later: nothing in state/tests.rs calls State::record_finalized_transaction directly — the existing balance tests reach it by applying events, which is the idiom there. So the gap was not a missing call style but a missing case: the ckETH and ckERC20 tests cannot enter this arm, so an incorrect debit, an uncounted fee, or the expect above trapping on a funding whose ceiling is below its transaction amount would all have gone unnoticed.

The new test asserts the identity the accounting has to satisfy rather than fixed numbers — the funding ceiling covers the ETH delivered plus the fee, so whatever part of the fee goes unspent is exactly what stays with the minter — and then asserts the failing case debits only the fee actually paid, with both fee counters unchanged between the two outcomes. Numbers copied from the ckETH fixture would have hidden the first arithmetic mistake I made here, which is what pushed me to the identity form.

For what it is worth, richer funding-accounting tests do exist later in the stack (should_account_for_a_successful_sweeper_funding and should_keep_a_failed_sweeper_funding_as_prepaid_gas), but they cannot move here: they depend on the burn-first accounting added in #11083 and on the cketh_burned field added in #11086. This test is deliberately scoped to what this PR introduces.

.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",
Expand Down
5 changes: 5 additions & 0 deletions rs/ethereum/cketh/minter/src/state/audit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,11 @@ pub fn apply_state_transition(state: &mut State, payload: &EventType) {
.eth_transactions
.record_withdrawal_request(request.clone());
}
EventType::AcceptedSweeperFundingRequest(request) => {
state
.eth_transactions
.record_withdrawal_request(request.clone());
}
EventType::CreatedTransaction {
withdrawal_id,
transaction,
Expand Down
17 changes: 16 additions & 1 deletion rs/ethereum/cketh/minter/src/state/audit/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down
5 changes: 4 additions & 1 deletion rs/ethereum/cketh/minter/src/state/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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
Expand Down
139 changes: 130 additions & 9 deletions rs/ethereum/cketh/minter/src/state/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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::<u64>(),
from in arb_principal(),
from_subaccount in arb_ledger_subaccount(),
created_at in any::<u64>(),
) -> SweeperFundingRequest {
SweeperFundingRequest {
withdrawal_amount,
destination,
ledger_burn_index: ledger_burn_index.into(),
from,
from_subaccount,
created_at,
}
}
}

fn arb_event_type() -> impl Strategy<Value = EventType> {
prop_oneof![
arb_init_arg().prop_map(EventType::Init),
Expand All @@ -752,6 +774,7 @@ fn arb_event_type() -> impl Strategy<Value = EventType> {
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 }),
Expand Down Expand Up @@ -1501,6 +1524,108 @@ mod eth_balance {
);
}

/// Funding takes its own arm in the balance accounting, so the ckETH and ckERC20 cases above
/// cannot reach it. What is asserted is the same shape as a ckETH withdrawal — a success debits
/// the transferred ETH plus the fee actually paid, a failure debits only that fee — because
/// funding is an ordinary withdrawal to the accounting. What differs is that nothing is ever
/// reimbursed, which is why the failing case must still leave the fee counters moving.
#[test]
fn should_update_after_successful_and_failed_sweeper_funding() {
let mut state_before_funding = initial_state();
apply_state_transition(
&mut state_before_funding,
&EventType::AcceptedDeposit(received_eth_event()),
);
let eth_balance_before_funding = state_before_funding.eth_balance.clone();

let funding_amount = Wei::new(10_000_000_000_000_000);
let funding_request = SweeperFundingRequest {
withdrawal_amount: funding_amount,
destination: "0xb44B5e756A894775FC32EDdf3314Bb1B1944dC34"
.parse()
.unwrap(),
ledger_burn_index: LedgerBurnIndex::new(0),
from: "k2t6j-2nvnp-4zjm3-25dtz-6xhaa-c7boj-5gayf-oj3xs-i43lp-teztq-6ae"
.parse()
.unwrap(),
from_subaccount: None,
created_at: 1699527697000000000,
};
let funding_flow = WithdrawalFlow {
tx_fee: GasFeeEstimate {
base_fee_per_gas: WeiPerGas::from(0xbc9998d1_u64),
max_priority_fee_per_gas: WeiPerGas::from(1_500_000_000_u64),
},
gas_limit: GasAmount::from(21_000_u32),
effective_gas_price: WeiPerGas::from(0x1176e9eb9_u64),
tx_status: TransactionStatus::Success,
..WithdrawalFlow::for_request(funding_request)
};

let mut state_after_successful_funding = state_before_funding.clone();
let receipt_succeeded = funding_flow
.clone()
.apply(&mut state_after_successful_funding);
let after_success = state_after_successful_funding.eth_balance.clone();

// Asserted as the identity the accounting has to satisfy rather than as a fixed number: the
// funding ceiling covers both the ETH delivered and the fee, so whatever part of the fee
// went unspent is exactly what stays with the minter.
let unspent = after_success
.total_unspent_tx_fees
.checked_sub(eth_balance_before_funding.total_unspent_tx_fees)
.unwrap();
assert_eq!(
after_success.eth_balance,
eth_balance_before_funding
.eth_balance
.checked_sub(
funding_amount
.checked_sub(unspent)
.expect("the unspent fee is part of the funding")
)
.unwrap(),
"a successful funding debits what it moved plus the fee it paid"
);
assert_eq!(
after_success.total_effective_tx_fees,
eth_balance_before_funding
.total_effective_tx_fees
.checked_add(receipt_succeeded.effective_transaction_fee())
.unwrap(),
"the fee actually paid must be counted"
);
assert!(
unspent > Wei::ZERO,
"this fixture over-provisions the fee, so some of it must be recorded as unspent"
);

let mut state_after_failed_funding = state_before_funding.clone();
let receipt_failed = WithdrawalFlow {
tx_status: TransactionStatus::Failure,
..funding_flow
}
.apply(&mut state_after_failed_funding);
let after_failure = state_after_failed_funding.eth_balance.clone();

assert_eq!(
after_failure.eth_balance,
eth_balance_before_funding
.eth_balance
.checked_sub(receipt_failed.effective_transaction_fee())
.unwrap(),
"a failed funding moved no ETH, so only the fee is debited"
);
assert_eq!(
after_failure.total_effective_tx_fees, after_success.total_effective_tx_fees,
"the same fee was paid either way"
);
assert_eq!(
after_failure.total_unspent_tx_fees, after_success.total_unspent_tx_fees,
"and the same amount of it went unspent"
);
}

#[test]
fn should_update_after_successful_and_failed_erc20_withdrawal() {
let mut state_before_withdrawal = initial_erc20_state();
Expand Down Expand Up @@ -1620,14 +1745,10 @@ mod eth_balance {
}

fn apply(self, state: &mut State) -> TransactionReceipt {
let accepted_withdrawal_request_event = match &self.withdrawal_request {
WithdrawalRequest::CkEth(eth_request) => {
EventType::AcceptedEthWithdrawalRequest(eth_request.clone())
}
WithdrawalRequest::CkErc20(erc20_request) => {
EventType::AcceptedErc20WithdrawalRequest(erc20_request.clone())
}
};
let accepted_withdrawal_request_event = self
.withdrawal_request
.clone()
.into_accepted_withdrawal_request_event();
apply_state_transition(state, &accepted_withdrawal_request_event);

let transaction = create_transaction(
Expand Down
Loading
Loading