diff --git a/rs/ethereum/cketh/docs/deposit_from_cex.md b/rs/ethereum/cketh/docs/deposit_from_cex.md index 67bde742c08b..35a25480b3f9 100644 --- a/rs/ethereum/cketh/docs/deposit_from_cex.md +++ b/rs/ethereum/cketh/docs/deposit_from_cex.md @@ -351,12 +351,16 @@ 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, and the minter tracks a lower bound on it from its own events — what finalized fundings - delivered, less the gas submitted sweeps provisioned — and may reconcile that - bound against the chain whenever it chooses. The bound errs low: ETH anyone else - sends to the address only pushes the true balance above it. That is the safe - direction for both readers, in opposite ways — a funding may be triggered earlier - than strictly needed, never skipped; a sweep may be held back, never authorised - against gas that is not there. + delivered, less what accepted sweeps have provisioned, plus what finalized + sweeps handed back — and may reconcile that bound against the chain whenever it + chooses. A sweep provisions the most it can cost the moment it is accepted (the + ETH it moves plus its fee ceiling, which caps every resubmission), and gets back + what it did not need when it finalizes, so a sweep whose finalization is never + observed leaves the bound too *low* rather than too high. ETH anyone else sends + to the address only pushes the true balance further above it. The bound therefore + errs low in the safe direction for both readers: a funding may be triggered + earlier than strictly needed, never skipped; a sweep may be held back, never + authorised against gas that is not there. Sweep gas draws it down; burned ckETH is **never re-minted**, so "cumulative burned ≥ cumulative spent" holds at every instant. Each funding round burns for its own transfer alone: the fee a previous diff --git a/rs/ethereum/cketh/minter/src/state.rs b/rs/ethereum/cketh/minter/src/state.rs index 4efee27124d1..4d027135dce9 100644 --- a/rs/ethereum/cketh/minter/src/state.rs +++ b/rs/ethereum/cketh/minter/src/state.rs @@ -15,7 +15,9 @@ use crate::numeric::{ use crate::state::automatic_deposits::{AutomaticDeposits, ScanProgress}; use crate::state::eth_logs_scraping::{LogScrapingId, LogScrapings}; use crate::state::sweeper_funding::{SweeperFundingAccounting, SweeperFundingConfig}; -use crate::state::transactions::{Erc20WithdrawalRequest, TransactionCallData, WithdrawalRequest}; +use crate::state::transactions::{ + Erc20WithdrawalRequest, SweepRequest, TransactionCallData, WithdrawalRequest, +}; use crate::timed_sized_map::{Entry, Timestamp}; use crate::tx::GasFeeEstimate; use crate::tx::TransactionSignature; @@ -437,6 +439,20 @@ impl State { self.update_balance_upon_withdrawal(withdrawal_id, receipt); } + pub fn record_finalized_sweeper_transaction( + &mut self, + sweep_id: &SweepId, + receipt: &TransactionReceipt, + ) { + // The sweeper pipeline is never reimbursed and holds no ckETH balance, so unlike the main + // pipeline there is no reimbursement tail and no ckETH accounting here — only the sweeper + // address' own balance bound to settle. + let _ = self + .sweeper_transactions + .record_finalized_transaction(*sweep_id, receipt); + self.update_sweeper_balance_upon_sweep(sweep_id, receipt); + } + pub fn next_request_id(&mut self) -> u64 { let current_request_id = self.http_request_counter; // overflow is not an issue here because we only use `next_request_id` to correlate @@ -454,6 +470,45 @@ impl State { }; } + /// Takes the most an accepted sweep can cost the sweeper address out of the balance bound: the + /// ETH it moves plus its fee ceiling, which caps every resubmission the pipeline makes for it. + pub fn update_sweeper_balance_upon_accepted_sweep(&mut self, request: &SweepRequest) { + self.sweeper_funding.record_accepted_sweep( + request + .amount + .checked_add(request.max_transaction_fee) + .expect("BUG: sweep provision always fits into U256"), + ); + } + + /// Hands back the part of a finalized sweep's provision it did not need: the fee it did not pay, + /// plus the value it did not move if it failed. + fn update_sweeper_balance_upon_sweep( + &mut self, + sweep_id: &SweepId, + receipt: &TransactionReceipt, + ) { + let request = self + .sweeper_transactions + .get_processed_request(sweep_id) + .expect("BUG: missing sweep request"); + // Cannot underflow: a sweep transaction is created, and resubmitted, only while its fee + // stays within this ceiling, and the fee it actually pays is at most the fee it was signed + // for. + let unspent_fee = request + .max_transaction_fee + .checked_sub(receipt.effective_transaction_fee()) + .expect("BUG: a sweep may not pay more than its fee ceiling"); + let refunded = match receipt.status { + TransactionStatus::Success => unspent_fee, + TransactionStatus::Failure => request + .amount + .checked_add(unspent_fee) + .expect("BUG: sweep refund always fits into U256"), + }; + self.sweeper_funding.record_finalized_sweep(refunded); + } + fn update_balance_upon_withdrawal( &mut self, withdrawal_id: &LedgerBurnIndex, diff --git a/rs/ethereum/cketh/minter/src/state/audit.rs b/rs/ethereum/cketh/minter/src/state/audit.rs index 7165b6b3993c..de4fbb016a69 100644 --- a/rs/ethereum/cketh/minter/src/state/audit.rs +++ b/rs/ethereum/cketh/minter/src/state/audit.rs @@ -117,6 +117,7 @@ pub fn apply_state_transition(state: &mut State, payload: &EventType) { } EventType::AcceptedSweepRequest(request) => { state.next_sweep_id = request.id.next(); + state.update_sweeper_balance_upon_accepted_sweep(request); state.sweeper_transactions.record_request(request.clone()); } EventType::CreatedSweeperTransaction { @@ -147,11 +148,7 @@ pub fn apply_state_transition(state: &mut State, payload: &EventType) { sweep_id, transaction_receipt, } => { - // The sweeper pipeline is never reimbursed and holds no ckETH balance, so unlike the main - // pipeline there is no reimbursement tail or balance update — just the finalize mechanics. - let _ = state - .sweeper_transactions - .record_finalized_transaction(*sweep_id, transaction_receipt); + state.record_finalized_sweeper_transaction(sweep_id, transaction_receipt); } EventType::ReimbursedEthWithdrawal(Reimbursed { burn_in_block: withdrawal_id, diff --git a/rs/ethereum/cketh/minter/src/state/sweeper_funding.rs b/rs/ethereum/cketh/minter/src/state/sweeper_funding.rs index 6e740f7f3f23..365cf7974106 100644 --- a/rs/ethereum/cketh/minter/src/state/sweeper_funding.rs +++ b/rs/ethereum/cketh/minter/src/state/sweeper_funding.rs @@ -26,6 +26,15 @@ pub struct SweeperFundingAccounting { /// Grows at the same point by the fee the transaction paid, which it does either way. Together /// with the amount transferred it is the spend, which never overtakes the burn. cumulative_transaction_fees: Wei, + /// Grows when a sweep is accepted, by the most that sweep can cost the sweeper address: the ETH + /// it moves plus the ceiling on its transaction fee, which caps every resubmission too. Not part + /// of the invariant above — this ETH was already counted as spent when the funding that + /// delivered it finalized — but subtracted from the balance bound, so that gas a committed sweep + /// will consume stops counting as available the moment it is committed. + cumulative_sweep_provisioned: Wei, + /// Grows when a sweep finalizes, by the part of that provision it turned out not to need: the + /// fee it did not pay, plus the value it did not move if it failed. + cumulative_sweep_refunded: Wei, } impl Default for SweeperFundingAccounting { @@ -34,6 +43,8 @@ impl Default for SweeperFundingAccounting { cumulative_burned: Wei::ZERO, cumulative_transferred: Wei::ZERO, cumulative_transaction_fees: Wei::ZERO, + cumulative_sweep_provisioned: Wei::ZERO, + cumulative_sweep_refunded: Wei::ZERO, } } } @@ -62,6 +73,30 @@ impl SweeperFundingAccounting { let _ = self.burned_not_yet_spent(); } + /// Records the most an accepted sweep can cost the sweeper address, taking it out of the balance + /// bound up front. Recorded when the sweep is accepted rather than when its transaction is + /// signed, so that a sweep provisions once however many times the pipeline resubmits it — every + /// attempt is capped by the same fee ceiling. + /// + /// Deliberately not added to [`Self::cumulative_spent`]: that counter is the minter's own ETH, + /// and this ETH was counted there once already, when the funding that delivered it to the + /// sweeper finalized. Counting it twice would make spend overtake burn and trip the invariant. + pub fn record_accepted_sweep(&mut self, provisioned: Wei) { + self.cumulative_sweep_provisioned = self + .cumulative_sweep_provisioned + .checked_add(provisioned) + .expect("BUG: overflow in cumulative sweep provisioned"); + } + + /// Records the part of a finalized sweep's provision it did not need, putting it back into the + /// balance bound. + pub fn record_finalized_sweep(&mut self, refunded: Wei) { + self.cumulative_sweep_refunded = self + .cumulative_sweep_refunded + .checked_add(refunded) + .expect("BUG: overflow in cumulative sweep refunded"); + } + /// Total ETH debited from the main address on account of sweeping. pub fn cumulative_spent(&self) -> Wei { self.cumulative_transferred @@ -73,12 +108,25 @@ impl SweeperFundingAccounting { self.cumulative_burned } - /// A lower bound on the sweeper address' ETH balance: what finalized fundings delivered. Nothing - /// debits it yet; when sweeping lands it will subtract the gas submitted sweeps provisioned, so - /// the bound stays conservative. ETH sent to the address by anyone else only pushes the true - /// balance above it. + /// A lower bound on the sweeper address' ETH balance: what finalized fundings delivered, less + /// what accepted sweeps have provisioned, plus what finalized sweeps handed back. + /// + /// Provisioning at acceptance rather than at spend is what keeps this a bound while sweeps are in + /// flight: gas a committed sweep will pay stops counting as available immediately, and a sweep + /// whose finalization is never observed leaves the bound too *low* — which delays a funding — + /// rather than too high, which would let the minter believe in gas that is gone. ETH sent to the + /// address by anyone else only pushes the true balance further above the bound. + /// + /// Saturating rather than checked: these counters are the minter's own record, and after an + /// upgrade that starts them from zero — or a sweeper address funded before the minter tracked it + /// — provisioning can legitimately exceed the deliveries. Trapping here would trap the replay of + /// every event after it, so it floors at zero, which reads as "assume nothing is prepaid". pub fn sweeper_balance_lower_bound(&self) -> Wei { self.cumulative_transferred + .checked_add(self.cumulative_sweep_refunded) + .expect("BUG: overflow in the sweeper balance bound") + .checked_sub(self.cumulative_sweep_provisioned) + .unwrap_or(Wei::ZERO) } /// ckETH burned for sweeping that has not been spent yet: the burn of a funding in flight, plus diff --git a/rs/ethereum/cketh/minter/src/state/sweeper_funding/tests.rs b/rs/ethereum/cketh/minter/src/state/sweeper_funding/tests.rs index 3f6ff2507fd6..2741b3deabb9 100644 --- a/rs/ethereum/cketh/minter/src/state/sweeper_funding/tests.rs +++ b/rs/ethereum/cketh/minter/src/state/sweeper_funding/tests.rs @@ -207,3 +207,174 @@ mod config { } } } + +/// The sweeper's own spending, driven through the state transitions the sweep pipeline records, so +/// that the wiring is covered and not only the arithmetic. The funding that delivers the ETH is a +/// precondition here rather than the subject, so it is arranged directly. +mod sweep_events { + use crate::eth_rpc_client::responses::{TransactionReceipt, TransactionStatus}; + use crate::lifecycle::EthereumNetwork; + use crate::numeric::{BlockNumber, GasAmount, Wei, WeiPerGas}; + use crate::state::State; + use crate::state::audit::{EventType, apply_state_transition}; + use crate::state::transactions::{PipelineRequest, SweepRequest}; + use crate::sweep::SWEEP_TRANSACTION_GAS_LIMIT; + use crate::test_fixtures::{initial_state, sweep_request}; + use crate::tx::{GasFeeEstimate, SignedSweepTransaction, TransactionSignature}; + + /// The gas the receipt below charges: the sweep's whole gas limit at one wei per gas. + const SWEEP_GAS: u64 = 100_000; + /// Comfortably above [`SWEEP_GAS`], so a sweep leaves an unspent part to hand back. + const FEE_CEILING: u64 = 10 * SWEEP_GAS; + /// What a funding is taken to have delivered to the sweeper, arranged directly. + const DELIVERED: u128 = 1_000_000 * SWEEP_GAS as u128; + + /// A state whose sweeper address holds `DELIVERED`, as a finalized funding would have left it. + fn state_with_a_funded_sweeper() -> State { + let mut state = initial_state(); + state.sweeper_funding.record_burn(Wei::new(DELIVERED)); + state + .sweeper_funding + .record_finalized_funding(Wei::new(DELIVERED), Wei::ZERO); + state + } + + fn bound(state: &State) -> Wei { + state.sweeper_funding.sweeper_balance_lower_bound() + } + + fn accept(state: &mut State, amount: Wei) -> SweepRequest { + let request = SweepRequest { + amount, + max_transaction_fee: Wei::from(FEE_CEILING), + ..sweep_request(1) + }; + apply_state_transition(state, &EventType::AcceptedSweepRequest(request.clone())); + request + } + + fn finalize(state: &mut State, request: &SweepRequest, status: TransactionStatus) { + let sweep_id = request.id; + let transaction = request + .clone() + .create_transaction( + state.sweeper_transactions.next_transaction_nonce(), + GasFeeEstimate { + base_fee_per_gas: WeiPerGas::ONE, + max_priority_fee_per_gas: WeiPerGas::ONE, + }, + SWEEP_TRANSACTION_GAS_LIMIT, + EthereumNetwork::Mainnet, + ) + .expect("test setup: the fee ceiling covers the fixture's fee"); + apply_state_transition( + state, + &EventType::CreatedSweeperTransaction { + sweep_id, + transaction: transaction.clone(), + }, + ); + let signed = SignedSweepTransaction::from(( + transaction, + TransactionSignature { + signature_y_parity: false, + r: Default::default(), + s: Default::default(), + }, + )); + apply_state_transition( + state, + &EventType::SignedSweeperTransaction { + sweep_id, + transaction: signed.clone(), + }, + ); + apply_state_transition( + state, + &EventType::FinalizedSweeperTransaction { + sweep_id, + transaction_receipt: TransactionReceipt { + block_hash: + "0xce67a85c9fb8bc50213815c32814c159fd75160acf7cb8631e8e7b7cf7f1d472" + .parse() + .unwrap(), + block_number: BlockNumber::new(4190269), + effective_gas_price: WeiPerGas::ONE, + gas_used: GasAmount::from(SWEEP_GAS), + status, + transaction_hash: signed.hash(), + }, + }, + ); + } + + #[test] + fn should_provision_an_accepted_sweep_before_it_has_spent_anything() { + let mut state = state_with_a_funded_sweeper(); + let value = Wei::from(7 * SWEEP_GAS); + + accept(&mut state, value); + + assert_eq!( + bound(&state), + Wei::new(DELIVERED) + .checked_sub(value.checked_add(Wei::from(FEE_CEILING)).unwrap()) + .unwrap(), + "the whole of what the sweep may cost stops counting as available gas" + ); + } + + /// What each outcome leaves subtracted, which is the refund rule in full: gas is paid either + /// way, the value only leaves if the transaction succeeded, and the unused fee always returns. + #[test] + fn should_settle_the_bound_on_what_a_finalized_sweep_actually_cost() { + let value = Wei::from(7 * SWEEP_GAS); + for (status, amount, cost) in [ + (TransactionStatus::Success, Wei::ZERO, Wei::from(SWEEP_GAS)), + ( + TransactionStatus::Success, + value, + value.checked_add(Wei::from(SWEEP_GAS)).unwrap(), + ), + (TransactionStatus::Failure, value, Wei::from(SWEEP_GAS)), + ] { + let mut state = state_with_a_funded_sweeper(); + let request = accept(&mut state, amount); + + finalize(&mut state, &request, status); + + assert_eq!( + bound(&state), + Wei::new(DELIVERED).checked_sub(cost).unwrap(), + "a {status:?} sweep moving {amount} must cost the sweeper {cost}" + ); + } + } + + /// Sweep spending is the sweeper's ETH, already counted as spent when the funding delivered it. + /// Counting it again here would make spend overtake burn and trip the invariant. + #[test] + fn should_leave_the_burn_first_accounting_alone() { + let mut state = state_with_a_funded_sweeper(); + let burned = state.sweeper_funding.cumulative_burned(); + let spent = state.sweeper_funding.cumulative_spent(); + let request = accept(&mut state, Wei::ZERO); + + finalize(&mut state, &request, TransactionStatus::Success); + + assert_eq!(state.sweeper_funding.cumulative_burned(), burned); + assert_eq!(state.sweeper_funding.cumulative_spent(), spent); + } + + /// Provisioning beyond what the minter has recorded as delivered — an upgrade that starts the + /// counters from zero, or a sweeper funded before it was tracked — floors the bound rather than + /// trapping, which would take the replay of every later event with it. + #[test] + fn should_floor_the_bound_at_zero_rather_than_trap() { + let mut state = initial_state(); + + accept(&mut state, Wei::from(SWEEP_GAS)); + + assert_eq!(bound(&state), Wei::ZERO); + } +} diff --git a/rs/ethereum/cketh/minter/src/state/transactions/tests.rs b/rs/ethereum/cketh/minter/src/state/transactions/tests.rs index cd4465c8d8ba..8e4b09946278 100644 --- a/rs/ethereum/cketh/minter/src/state/transactions/tests.rs +++ b/rs/ethereum/cketh/minter/src/state/transactions/tests.rs @@ -2977,6 +2977,7 @@ mod sweep_lane { SweepRequest, TransactionPipeline, }; use crate::sweep::SWEEP_TRANSACTION_GAS_LIMIT; + use crate::test_fixtures::sweep_request; use crate::tx::{ DelegatingSweep, Eip1559TransactionRequest, Eip7702TransactionRequest, GasFeeEstimate, SignableTransaction, SignedAuthorization, SweepTransaction, @@ -2988,18 +2989,6 @@ mod sweep_lane { const EIP1559_TX_ID: u8 = 2; const SET_CODE_TX_ID: u8 = 4; - fn sweep_request(id: u64) -> SweepRequest { - SweepRequest { - id: SweepId(id), - destination: Address::new([id as u8; 20]), - amount: Wei::ZERO, - data: vec![0xaa, 0xbb, 0xcc], - max_transaction_fee: Wei::from(1_000_000_000_000_000_u64), - created_at: 1_620_328_630_000_000_000, - authorizations: vec![], - } - } - /// A sweep of two deposit addresses that are not yet delegated to the sweeper contract. fn delegating_sweep_request(id: u64) -> SweepRequest { SweepRequest { diff --git a/rs/ethereum/cketh/minter/src/test_fixtures.rs b/rs/ethereum/cketh/minter/src/test_fixtures.rs index fb35c57de06c..e3f6d9d925a3 100644 --- a/rs/ethereum/cketh/minter/src/test_fixtures.rs +++ b/rs/ethereum/cketh/minter/src/test_fixtures.rs @@ -4,7 +4,7 @@ use crate::lifecycle::init::InitArg; use crate::numeric::{LedgerBurnIndex, Wei}; use crate::state::State; use crate::state::eth_logs_scraping::LogScrapingId; -use crate::state::transactions::EthWithdrawalRequest; +use crate::state::transactions::{EthWithdrawalRequest, SweepId, SweepRequest}; use crate::tx::TransactionSignature; use candid::{Nat, Principal}; use ethnum::u256; @@ -92,6 +92,18 @@ pub fn sweeper_funding_request(withdrawal_amount: Wei) -> EthWithdrawalRequest { } } +pub fn sweep_request(id: u64) -> SweepRequest { + SweepRequest { + id: SweepId(id), + destination: Address::new([id as u8; 20]), + amount: Wei::ZERO, + data: vec![0xaa, 0xbb, 0xcc], + max_transaction_fee: Wei::from(1_000_000_000_000_000_u64), + created_at: 1_620_328_630_000_000_000, + authorizations: vec![], + } +} + /// Install `state` into the global thread-local `STATE`, so `read_state`/`mutate_state` see it in a /// unit test. Each test runs on its own thread, so the `thread_local!` `STATE` is per-test. pub fn init_state(state: State) {