feat(cketh): burn-first accounting for sweeper fee funding - #11083
feat(cketh): burn-first accounting for sweeper fee funding#11083mbjorkqvist wants to merge 2 commits into
Conversation
Adds the accounting that makes the backing invariant checkable — "cumulative ckETH burned for sweeping >= cumulative ETH debited from the main address for sweeping" — plus proposal-configurable bounds for when to top the sweeper address up. `SweeperFundingAccounting` is a fold over events the minter already persists: the burn is recorded when the funding request is accepted (i.e. *before* any ETH moves, which is what makes the invariant hold at every instant rather than only in the steady state), and the spend when the transaction finalizes, next to the existing `eth_balance` update so both derive from the same event. No event type of its own, so replay reconstructs it exactly. Writing the accounting surfaced something worth stating plainly: a surplus arises on *every* funding, not just failed ones. The transferred value is the burn minus the transaction's `max` fee, while only the *effective* fee is spent, so the unused fee allowance stays at the main address as prepaid gas. Offsetting a later funding against an earlier burn is therefore the normal path, and `burn_required_for` returns zero while the outstanding credit covers the amount. A failed funding is just the extreme case of the same thing. The surplus is stored rather than derived from `eth_getBalance`, because it sits at the *main* address, not the sweeper's: the sweeper's on-chain balance answers "how much prepaid gas is in place", not "how much has been burned but not yet moved". `burned_not_yet_spent` panics if spend ever exceeds burn, rather than saturating to zero. Under-backed ckETH is not a state to tolerate, and a saturating subtraction would hide the breach; the check also runs eagerly at each finalized funding so a violation surfaces at the transition that caused it. The bounds are validated as a pair (target strictly above the low-water mark, otherwise funding would loop) and rejected wholesale rather than partially applied. Defaults are 0.02 ETH / 0.1 ETH — deliberately provisional, sized so a funding covers many sweeps and its own fee stays a small fraction of the amount moved; they are meant to be calibrated during the Sepolia rollout once real sweep gas costs are known. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds replayable burn-first sweeper funding accounting and configurable top-up bounds for ckETH.
Changes:
- Tracks cumulative sweeper burns, transfers, fees, and surplus.
- Adds validated low-water-mark and target upgrade settings.
- Adds accounting, configuration, replay, and withdrawal-flow tests.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
state/tests.rs |
Tests upgrades and funding accounting. |
state/sweeper_funding/tests.rs |
Tests accounting and bounds. |
state/sweeper_funding.rs |
Implements accounting and configuration. |
state/audit.rs |
Folds funding burns into state. |
state.rs |
Integrates accounting, validation, and finalization. |
lifecycle/upgrade.rs |
Adds upgrade parameters. |
lifecycle/init.rs |
Initializes funding state. |
cketh_minter.did |
Exposes configuration parameters. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| pub fn burn_required_for(&self, amount: Wei) -> Wei { | ||
| amount | ||
| .checked_sub(self.burned_not_yet_spent()) | ||
| .unwrap_or(Wei::ZERO) |
There was a problem hiding this comment.
Correct diagnosis, and it is exactly the change the next PR in the stack makes — documented in 49890c3 so the next reader does not have to re-derive it.
SweeperFundingRequest gains a cketh_burned field distinct from withdrawal_amount, for precisely the reason you give: with a single amount, reducing the burn shrinks the transfer by the same amount, so the credit is never consumed and grows forever instead. The accounting then records cketh_burned as the burn while withdrawal_amount stays the spend ceiling. This was found the hard way when the funding task was first written, and it is why the field exists.
Worth confirming the state of this PR though: burn_required_for has no production caller here, only unit tests — the funding task arrives with the field. So nothing computes a wrong burn at this commit; the helper is simply not reachable yet. That is the seam between this PR and the next, and it is the kind of thing splitting a feature into reviewable pieces produces.
I considered moving the function into the next PR so that this one contains nothing unusable, and decided against it: burn_required_for is what expresses the offsetting rule, which is the substance of this PR's accounting. Splitting it out would leave the accounting incomplete here and no clearer there. Instead its doc now states the requirement and that the field arrives with the task.
| /// Sweeper balance the minter tops up to. Must exceed the low-water mark. | ||
| #[cbor(n(11), with = "icrc_cbor::nat::option")] | ||
| pub sweeper_funding_target: Option<Nat>, |
There was a problem hiding this comment.
Fixed in 49890c3. You are right that a proposal following the documented rule could still be rejected — the doc gave the strict-inequality half of the check and omitted the headroom half.
Both UpgradeArg fields now state the complete constraint: the target must exceed the low-water mark by at least the minimum withdrawal amount, that difference being the smallest amount a funding ever moves. The low-water-mark field carries the full rule and the target refers to it, rather than repeating it in a way the two could drift apart.
| // Must stay strictly below sweeper_funding_target, otherwise funding would loop. | ||
| sweeper_funding_low_water_mark : opt nat; | ||
|
|
||
| // Sweeper balance (in wei) the minter tops up to. | ||
| sweeper_funding_target : opt nat; |
There was a problem hiding this comment.
Fixed in 49890c3, together with the same omission on UpgradeArg.
The Candid comment now states the full constraint and one further thing a proposal author needs: setting only one of the two bounds keeps the other's current value, and the pair is validated again as a whole. That matters because it means a proposal changing one bound can be rejected on account of a value it did not set.
…not yet usable The Candid file and `UpgradeArg` documented only that the target must exceed the low-water mark, while `validate` also requires that difference to cover the minimum withdrawal amount. A proposal author following the documented rule could still have the upgrade rejected, so both now state the complete constraint and that setting one bound keeps the other's value. `burn_required_for` also now says why nothing calls it yet: consuming the credit needs a request that records its burn separately from the ETH it moves, since reducing a single amount would shrink the transfer by as much as the burn and leave the credit untouched. That second field arrives with the funding task. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
✅ No security or compliance issues detected. Reviewed everything up to 49890c3. Security Overview
Detected Code Changes
|
Part of DEFI-2933 (sweeper fee funding), fourth of a seven-PR stack. Targets #11072.
Why
Sweep gas is prepaid: ckETH is burned from the minter's fee subaccount before the ETH moves, so that at every instant
Nothing yet keeps track of either side, so nothing can check it. This PR adds that bookkeeping, plus the bounds deciding when a top-up is due.
What
The accounting is a fold over events the minter already persists — the accepted funding request and the finalized transaction's receipt — so it is reconstructed exactly on replay and needs no event type of its own. It is deliberately not serializable, which keeps that property honest.
Writing it surfaced something worth stating plainly: a surplus arises on every funding, not only failed ones. The value transferred is the burn minus the transaction's max fee, while only the effective fee is ever spent, so the unused fee allowance stays at the main address as prepaid gas. Offsetting a later funding against an earlier burn is therefore the normal path, and a failed funding is just the extreme case of the same thing.
That surplus is tracked rather than read back from the chain because it sits at the main address, not the sweeper's. The sweeper's on-chain balance answers "how much prepaid gas is in place", which is a different question from "how much has been burned but not yet moved".
Spending more than was burned would mean ckETH is under-backed, so that traps rather than saturating to zero — and it is checked eagerly at each finalized funding, so a violation surfaces at the transition that caused it rather than whenever someone next reads the surplus.
The bounds
Proposal-configurable, validated as a pair rather than individually: a target at or below the low-water mark would make a funding immediately due again and loop, and headroom below the minimum burn would make every cycle burn more ckETH than the ETH it moves. Both are rejected wholesale rather than partially applied.
The same check runs when only the minimum withdrawal amount changes, since the invariant relates two independently configurable amounts and raising one alone would silently invalidate bounds that were valid when set.
Defaults are 0.02 / 0.1 ETH — deliberately provisional, sized so a funding covers many sweeps and its own fee stays a small fraction of the amount moved. They are meant to be calibrated during the Sepolia rollout once real sweep gas costs are known.
Stack
Merge in order; each PR targets the one above it.