feat(cketh): burn-first accounting for sweeper fee funding - #11083
Merged
Conversation
This was referenced Aug 7, 2026
Contributor
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.
mbjorkqvist
marked this pull request as ready for review
August 10, 2026 12:01
|
✅ No security or compliance issues detected. Reviewed everything up to 74d0ea1. Security Overview
Detected Code Changes
|
Rachit2323
pushed a commit
to Rachit2323/ic
that referenced
this pull request
Aug 12, 2026
…ity#11060) Part of [DEFI-2933](https://dfinity.atlassian.net/browse/DEFI-2933) (sweeper fee funding), first of a seven-PR stack. ## Why Funding the sweeper address with gas requires knowing how much gas it already holds. The EVM RPC canister exposes no endpoint for a native ETH balance, and its Rust client offers no getter for one, so the minter currently has no way to ask. ## What Reads the balance through the EVM RPC canister's generic JSON-RPC passthrough, which forwards a payload to every provider and agrees on one answer under the configured consensus strategy. That strategy is a threshold of the providers — 3 of 4 on mainnet, 2 of 4 on Sepolia — and it is the only agreement accepted: there is no client-side reduction, so a result the canister reports as inconsistent stays an error rather than being resolved by picking a winner. Because the canister deserializes each response's `result` field, what the minter receives is the quantity itself rather than any surrounding JSON. It is therefore decoded exactly: quotes, padding, leading zeros and sign characters are the provider's own malformation and are rejected rather than repaired. A failed read is an error, never a zero. This is the decision the rest of the stack depends on: confusing "could not read the balance" with "no gas left" would burn ckETH to top up an address that is already funded, which is pure loss. The request builder and the result decoder are pure functions so both sides of that guarantee are pinned directly, including a test asserting that no error input can decode to a zero balance. The route was also proven end to end against a live EVM RPC canister and a local anvil node, reaching 3-of-4 consensus at both `latest` and `finalized`. ## Stack Merge in order; each PR targets the one above it. | # | PR | Status | |---|----|--------| | 1 | Read a native ETH balance via the EVM RPC canister | **this PR** | | 2 | dfinity#11065 — Burn ckETH from the minter's own fee subaccount | ready for review | | 3 | dfinity#11072 — Add the SweeperFunding withdrawal-request variant | ready for review | | 4 | dfinity#11083 — Burn-first accounting for sweeper fee funding | ready for review | | 5 | dfinity#11086 — Sweeper fee-funding task, with an end-to-end test | Copilot re-review pending, CI green incl. long tests | | 6 | dfinity#11094 — Sweeper funding observability and the prepaid-gas gate | open | | 7 | dfinity#11097 — Adversarial end-to-end coverage of sweeper fee funding | open | [DEFI-2933]: https://dfinity.atlassian.net/browse/DEFI-2933?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
mbjorkqvist
force-pushed
the
mathias/DEFI-2933-sweeper-funding-request
branch
from
August 14, 2026 08:48
3c5f94d to
b3bb391
Compare
mbjorkqvist
force-pushed
the
mathias/DEFI-2933-burn-first-accounting
branch
5 times, most recently
from
August 18, 2026 17:53
17f7542 to
9eb5ae1
Compare
Base automatically changed from
mathias/DEFI-2933-sweeper-funding-request
to
master
August 19, 2026 06:15
mbjorkqvist
force-pushed
the
mathias/DEFI-2933-burn-first-accounting
branch
from
August 19, 2026 06:56
9eb5ae1 to
f41cdcf
Compare
pull Bot
pushed a commit
to bit-cook/ic
that referenced
this pull request
Aug 19, 2026
…ty#11072) Part of [DEFI-2933](https://dfinity.atlassian.net/browse/DEFI-2933) (sweeper fee funding). Now targets `master`: dfinity#11060 has merged, and dfinity#11065 is being closed with its contents folded into the PRs that use them — the fee-subaccount constant into this one, the burn helper into dfinity#11086 alongside its caller. ## Why Sweeper fee funding is mechanically an ordinary ckETH withdrawal — same nonce sequence, same threshold-ECDSA signing, same fee-bumped resubmission — so it becomes a third `WithdrawalRequest` variant rather than a parallel pipeline. It differs in exactly one respect, and that difference is what the whole feature turns on: the ckETH burned for funding is **never** re-minted. A funding request must therefore never reach the reimbursement machinery. ## What 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 kept out of the set on insert, and the corresponding assertion on removal is made conditional. Both are production assertions, so a missed branch traps the canister. - The conversion to a reimbursement index becomes fallible, deliberately: a fallible conversion makes the compiler prove at every call site that funding cannot produce an index, rather than relying on a panicking arm that traps if a site is missed. The two callers construct their index inside the reimbursable arms instead. - Nothing is recorded to pay back if a funding transaction finalizes with a failure receipt — an outcome that requires code at the destination, so it cannot arise for a bare transfer to the sweeper, and is logged as unexpected rather than handled. The ckETH stays burned while no ETH reaches the sweeper — only the gas the failed transaction still paid leaves the main address — so the rest of the burn over-backs ckETH instead of anything being lost, and neither a reimbursement path nor a second event type is needed. Everything else follows ckETH: the 21'000 gas limit of a plain value transfer to a code-less address, a resubmission strategy ceilinged at the burned amount — so a climbing gas price shrinks the ETH delivered to the sweeper rather than spending more than was burned — and a fee carved out of that same amount, so balance accounting needs no change. Since reimbursement is the only difference, the variant carries an `EthWithdrawalRequest` — the same payload a user's ckETH withdrawal carries — rather than a near-copy of it, and the spec drops the one piece of funding accounting that had no counterpart in the withdrawal flow: burned-but-unspent amounts are no longer credited against the next funding's burn. They stay as backing, just like the unspent gas a user's withdrawal leaves behind. Funding appears in the withdrawal status endpoint, the dashboard and the event log rather than being hidden: it moves ckETH-denominated value and is a public, auditable action. A user query never matches one, since the sender is the minter itself. The new event takes tag 27; 26 went to `AutomaticDepositReceived` on master while this branch was open, and the tags are the durable CBOR encoding, so they cannot collide. ## Stack Merge in order; each PR targets the one above it. | # | PR | Status | |---|----|--------| | — | dfinity#11060 — Read a native ETH balance via the EVM RPC canister | merged | | — | dfinity#11065 — Burn ckETH from the minter's own fee subaccount | closed; folded into dfinity#11072 and dfinity#11086 | | 1 | Add the SweeperFunding withdrawal-request variant | **this PR** | | 2 | dfinity#11083 — Burn-first accounting for sweeper fee funding | ready for review | | 3 | dfinity#11086 — Sweeper fee-funding task, with an end-to-end test | draft | | 4 | dfinity#11094 — Sweeper funding observability and the prepaid-gas gate | draft | | 5 | dfinity#11097 — Adversarial end-to-end coverage of sweeper fee funding | draft | [DEFI-2933]: https://dfinity.atlassian.net/browse/DEFI-2933?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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>
…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>
…unting `burn_required_for` computed a discounted burn, which nothing will call now that each funding burns for its own transfer alone. The counters and the `burned_not_yet_spent` gauge stay: a funding in flight and the fees earlier fundings never paid still have to be visible, they are just no longer spendable.
…l amount The two bounds were proposal-configurable, and everything around them existed to police the one relation they had to keep: the gap between them is the smallest amount a funding moves, so it has to clear the minimum the burn is held to. Deriving both from that minimum -- target ten times it, refill at half the target -- makes the relation hold by construction, for any minimum rather than for the pairs a proposal happens to set. So the two upgrade arguments go, with the validation, its error type and the five tests that drove it. What remains is one check that the target still fits, which is what lets the bounds be computed on demand instead of stored: derived state kept in a field can drift from what it was derived from. It also unpicks a coupling nobody wanted: a minimum withdrawal amount above the headroom the fixed defaults left used to fail the install outright.
The module ran at 38% comments where its siblings sit between 6% and 30%, most of the excess being design rationale the spec it cites already carries, or field docs restating the field names. The spec pointer stays: `get_balance.rs` and the balance-scan batcher cite the same document.
Per review guidance on the earlier PRs: no doc comment ahead of a test, and no comment restating what the code says. The assertion messages stay, since those are what a failure prints.
mbjorkqvist
force-pushed
the
mathias/DEFI-2933-burn-first-accounting
branch
from
August 19, 2026 13:07
04ad8fc to
cfe0ced
Compare
gregorydemay
left a comment
gregorydemay
left a comment
Contributor
There was a problem hiding this comment.
Thanks @mbjorkqvist ! Couple of nits but generally LGTM!
Field docs now say when each counter moves and how they relate, which is what the struct doc alone did not convey. `amount_due` asserts the bound it relies on instead of noting it in a comment. Two tests restated the code they exercised: the derivation test went, and the top-up test became a property over every balance below the low-water mark. The install test went too — it existed to show that a large minimum withdrawal amount no longer collides with fixed bounds, which the property covers without the historical detour. The shared funding request moved to the fixtures, and the failed-funding test now pins the spend to the receipt's own fee.
…ount Deriving the funding bounds added a branch to `validate_config`: a minimum whose target would not fit is refused, which is what lets the accessor unwrap. Only the helper returning `None` was covered, so the rejection itself went untested — removing the branch left the test suite green.
gregorydemay
approved these changes
Aug 20, 2026
gregorydemay
left a comment
gregorydemay
left a comment
Contributor
There was a problem hiding this comment.
Thanks @mbjorkqvist !
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of DEFI-2933 (sweeper fee funding), first of the four remaining PRs. Targets
master, now that #11072 has merged.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. Nothing reuses it — each funding burns for its own transfer, and the surplus simply stays as ckETH backing, exactly like the unspent gas of a user withdrawal. A failed funding is 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 far has burn run ahead of spend" — the quantity the invariant is about, and the one an operator needs in order to check it.
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.