feat(cketh): re-delegate deposit addresses to a newly configured sweeper contract - #11504
gregorydemay wants to merge 33 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The unbounded nonce-repair RPC fan-out is a critical scalability and availability issue.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds lazy EIP-7702 sweeper delegate rotation and nonce recovery after reverted sweeps.
Changes:
- Safely reuses or rotates authorization tuples.
- Re-anchors uncertain nonces using finalized chain state and audit events.
- Adds metrics, fixtures, and end-to-end coverage.
File summaries
| File | Summary |
|---|---|
rs/ethereum/cketh/test_utils/src/live.rs |
Adds delegate-rotation test fixtures. |
rs/ethereum/cketh/test_utils/src/anvil.rs |
Deploys the legacy delegate. |
rs/ethereum/cketh/minter/tests/dump_stable_memory.rs |
Maps the new audit event. |
rs/ethereum/cketh/minter/tests/deposit_from_cex.rs |
Tests rotation and nonce recovery. |
rs/ethereum/cketh/minter/tests/deposit_from_cex_demo/CkSweeperAttestedLegacy.sol |
Provides the legacy delegate contract. |
rs/ethereum/cketh/minter/src/tx/eip_7702.rs |
Exposes authorization chain IDs. |
rs/ethereum/cketh/minter/src/test_fixtures.rs |
Supports failed finalized sweeps. |
rs/ethereum/cketh/minter/src/sweep/tests.rs |
Tests authorization and recovery behavior. |
rs/ethereum/cketh/minter/src/sweep/mod.rs |
Critical (2 votes): Nonce repair launches an unbounded multi-provider RPC read for every queued address. Process a bounded, fairly selected batch per tick to avoid exhausting callback or cycle limits. |
rs/ethereum/cketh/minter/src/state/event.rs |
Defines the nonce-observation event. |
rs/ethereum/cketh/minter/src/state/automatic_deposits/tests.rs |
Tests nonce tracking and replay. |
rs/ethereum/cketh/minter/src/state/automatic_deposits/mod.rs |
Tracks verified and unresolved nonces. |
rs/ethereum/cketh/minter/src/state/audit/tests.rs |
Maps the new event in audit tests. |
rs/ethereum/cketh/minter/src/state/audit.rs |
Applies nonce-observation events. |
rs/ethereum/cketh/minter/src/state.rs |
Selects reusable or rotating authorizations. |
rs/ethereum/cketh/minter/src/main.rs |
Exposes events and nonce metrics. |
rs/ethereum/cketh/minter/src/endpoints.rs |
Adds the public event payload. |
rs/ethereum/cketh/minter/cketh_minter.did |
Updates the Candid interface. |
rs/ethereum/cketh/minter/BUILD.bazel |
Includes the legacy Solidity fixture. |
Review details
- Files reviewed: 19/19 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🟡 Changes recommended
A critical stale-read race can reset newer authorization state.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 19/19 changed files
- Comments generated: 1
- Review effort level: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
Delegation rotation and nonce recovery affect security-sensitive transaction and replay state across multiple components.
Review details
- Files reviewed: 19/19 changed files
- Comments generated: 0 new
- Review effort level: Balanced
The ETH counterparts of the ERC-20 deposit-from-CEX helpers: plan and register deposits via deposit_eth, credit them with plain ETH transfers from the dev account, await their detection through the endpoint's own status, and expect the resulting ckETH mints on the ledger. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Yv31YXb6YH1Gp71G54cL9
Queued ETH entries are now batched into sweeps of their own: a SweepRequest moves one asset, ETH or a single ERC-20 token, and encodes its calldata accordingly (sweepEthBatch takes the same SweepItem array as the ERC-20 entry point, without the token list). The asset field keeps the CBOR encoding of the previous bare token address for ERC-20 sweeps, so recorded AcceptedSweepRequest events replay unchanged. Attestations are reused as-is because the digest binds only the account. The end-to-end path is proven against a live anvil node: deposit_eth, a plain CEX-style transfer, balance-delta detection, sweepEthBatch through the delegated address, and the ckETH mint from the helper event, leaving the deposit address empty. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Yv31YXb6YH1Gp71G54cL9
The ETH mirror of the twenty-ERC-20-deposit test, subsuming the single-deposit one: twenty depositors with distinct accounts and amounts, credited by plain transfers, detected, swept in two full ten-deposit sweepEthBatch calls and minted. Mirrors the whole assertion chain: bare addresses before the credit, batch sizes from the events, addresses swept empty, the minter's ETH delta equal to the swept total (the main address also holds funding residue, so the total is asserted as a delta over a baseline), delegations installed and sweeper gas spent. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Yv31YXb6YH1Gp71G54cL9
The ETH mirror of the per-token-minimum threshold test, against a live anvil node: deposits above and at the 0.005 ETH minimum are detected and queued while a below-minimum deposit keeps scanning. Adds the ETH pendant of await_scan, which waits for a scan pinned at or past the block where the funding is visible. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Yv31YXb6YH1Gp71G54cL9
The ETH mirror of the second-sweep test: after the first sweep delegates the address (spending nonce 0), a second deposit to the same pair is re-registered at the same address, detected, and swept by a transaction that re-carries the nonce-0 authorization. The protocol skips the stale tuple without advancing the address' nonce and the sweep still succeeds through the installed delegation. Deposit mints are counted against a baseline because the fee-account funding mints ckETH as well. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Yv31YXb6YH1Gp71G54cL9
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Yv31YXb6YH1Gp71G54cL9
The demo gains the ETH pendant of the attested batch-gas scenarios, measured through sweepEthBatch for 1, 10 and 20 deposits with the same first-sweep (EIP-7702) and already-delegated (EIP-1559) columns: a ten-deposit first sweep costs 413_076 gas, about two thirds of its ERC-20 counterpart, since no token transfers are involved. The twenty-ETH-deposit live test asserts each of its ten-deposit sweeps within 10 percent of that figure, mirroring the ERC-20 gas assertion. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Yv31YXb6YH1Gp71G54cL9
CkEthSetup gains minter_count_events, counting the events a caller's filter matches, with a LiveSetup delegate. It replaces the ad hoc counting of ckETH mints, ckERC20 mints and automatic-deposit events in the CEX and integration tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Yv31YXb6YH1Gp71G54cL9
Three accounts with distinct principals, subaccounts and amounts: one deposits ckUSDC only, one deposits ckUSDT and ETH at its single shared address, one deposits ETH only. The scan detects all four pairs, the sweeper sends one sweep per asset (ckUSDC, ckUSDT, and a two-item ETH batch that includes the shared address already delegated by its ERC-20 sweep), each asset lands where it belongs and every account is credited on the matching ledger. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Yv31YXb6YH1Gp71G54cL9
assert_eth_sweeps_batched takes the deposits instead of bare batch sizes: the expected sizes derive from the now-public MAX_DEPOSITS_PER_SWEEP, and every deposit address must appear in exactly one ETH sweep, read from the AcceptedSweepRequest items. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Yv31YXb6YH1Gp71G54cL9
The batch sizes are already pinned by the gas-near-demo assertion, so assert_eth_sweeps_batched checks just that every deposit address appears in an ETH sweep; MAX_DEPOSITS_PER_SWEEP turns private again. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Yv31YXb6YH1Gp71G54cL9
…lied Signing an EIP-7702 tuple is not applying it: the sweep carrying it may never be sent, and once the delegate changes the minter holds tuples for two delegates at the same nonce. Pin down which tuple a finalized sweep actually installed, and the delegation that follows from it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X44H4nAU65nAexEUvG15MK
…ed on chain A stored EIP-7702 authorization now carries the sweep that applied it, if one has: finalizing a sweep marks every tuple it carried at the account's current nonce, whether the sweep succeeded or reverted, and leaves a tuple the protocol skipped alone. An account's delegation follows from those marks, so a rotated delegate is read off what the chain actually holds rather than off what the minter once signed. Also corrects what a skipped tuple costs: the existing-account refund comes after the nonce check, so a skipped tuple earns none of it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X44H4nAU65nAexEUvG15MK
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X44H4nAU65nAexEUvG15MK
Two sweeps of one account can be in flight at once, one per queued token, and both carry a tuple for nonce zero. Cover that race, and a tuple signed ahead of the nonce the address has reached. Move the rotation target below the incumbent in key order, so reading the delegation off the last key instead of the highest nonce fails. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X44H4nAU65nAexEUvG15MK
An authorization is keyed by its account first, so reading an account's delegation can range over its own entries instead of scanning every account ever swept. That scan ran once per tuple a finalized sweep carried, which on replay is quadratic in the accounts the log has ever swept and the map is never pruned. Also drops two ways the marking could pass over an inconsistency in silence: a tuple can only be marked once, since applying it spends the nonce that lets it be marked at all, and a sweep can only carry a tuple the minter signed, since its event precedes the sweep in the log. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X44H4nAU65nAexEUvG15MK
…delegation The sentence stacked two relative clauses on one noun and read as malformed. Say the same thing in one clause. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X44H4nAU65nAexEUvG15MK
The comment carried what the test name should. Drop it and let the name say it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X44H4nAU65nAexEUvG15MK
…d them Receipts came back keyed by sweep id and were processed in that order. A sweep the queue reschedules keeps its id but takes a later transaction nonce, so id order can invert chain order, and the tuple a sweep applied depends on the nonce its deposit addresses had reached when it ran. Order the finalized receipts by block and position within it, which is the order the chain executed them. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X44H4nAU65nAexEUvG15MK
…t a tuple Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X44H4nAU65nAexEUvG15MK
…tuple Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X44H4nAU65nAexEUvG15MK
…h address Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X44H4nAU65nAexEUvG15MK
…t address Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X44H4nAU65nAexEUvG15MK
…track Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X44H4nAU65nAexEUvG15MK
…nverified nonce Re-pointing the minter at another sweeper contract must re-delegate the addresses already delegated to the old one, at the nonce each has reached rather than at nonce zero, and must never put two tuples of one address' nonce in flight at once. A reverted sweep leaves the minter unable to say which tuples it applied, so its addresses stay out of sweeps until their nonce is read back off the chain and the record is re-anchored on it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X44H4nAU65nAexEUvG15MK
…gured delegate A tuple applies only at the authority's current nonce, so re-delegating an address means signing for the nonce it has reached rather than for zero. A tuple already signed for that nonce is re-carried instead, so the minter never puts two tuples of one address' nonce in flight: the rotation is retried at the next nonce once the record shows that tuple applied. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X44H4nAU65nAexEUvG15MK
…ir chain nonce A reverted sweep is the one sign that the record of which tuples the chain applied has drifted, so it leaves the nonce of every address it touched unverified and those addresses out of every sweep. Before one of them is swept again the minter reads its deposit address' transaction count back and records it: signing at most one tuple per (address, nonce) makes that nonce say which tuples applied, which re-anchors the record on the chain. Two test-side changes ride along: * The sweep unit tests now price with `test_fixtures::gas_fee_estimate()` (base fee 10 rather than 1) instead of an estimate of their own, so every sweep fixture agrees on one estimate and a test can drive an enqueued sweep through the shared pipeline helpers. * A test the one-tuple-per-nonce rule supersedes is dropped: re-pointing the minter mid-flight now re-carries the tuple already signed rather than signing a second one for the same nonce. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X44H4nAU65nAexEUvG15MK
…gate Runs the whole rotation against a real EVM: the minter is installed on the superseded delegate (the version before receive(), sweepEth and sweepEthBatch, kept as a test fixture), sweeps an address onto it, is re-pointed at the current one by upgrade, and the next sweep of that address carries the rotation tuple at the nonce the first sweep left it at. The sweep after that carries none. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X44H4nAU65nAexEUvG15MK
…d prove the repair end to end The repair path had no end-to-end coverage: deleting the chain read broke nothing. A live test now forces the drift it exists for, by pointing the minter at the deposit helper as its sweeper contract — the helper has neither sweepErc20Batch nor a fallback, so the batch call reverts while the tuples riding with it still apply. The unit tests pin what a read back must refuse: a nonce above every tuple the minter signed explains nothing, an address the read cannot place is not re-read every tick, neither takes a slot in a token's sweep batch, and a tuple recorded for another chain is not the one this chain applies. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X44H4nAU65nAexEUvG15MK
…ord can explain Reviewing the repair path turned up six ways it could mislead or cost more than it saves: * The nonce is read at `finalized`, not `latest`, and demands every provider agree. Every other mark on the record comes from a finalized receipt, so an anchor read one block deeper could be reorged out from under the marks it had just rewritten. * A nonce above every tuple the minter signed explains nothing: nothing the minter did could have taken the address there, so the record is missing tuples. Such an address is refused rather than re-anchored. * An address a read cannot place is now held apart from those still awaiting one, so it is never read again: a second read reports the same nonce and explains no more, and paying for one every tick, forever, per stuck address, is what that would cost. * An address whose nonce is not trusted takes no slot in its token's sweep batch either, so a handful of stuck ones cannot hold back the healthy addresses queued behind them. The read runs before the batch is built, so a queue holding nothing else still recovers. * Recording an observation re-checks that the address is still awaiting one: a sweep may have finalized while the read was in flight, and a count read before it would undo the marks that sweep left. * The lookup that re-carries an unapplied tuple keys on the chain id as well as the nonce, as the cache key it reads says it must. Both sets are exported as gauges next to the applied-authorizations one. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X44H4nAU65nAexEUvG15MK
…nts, not inline ones Five inline comments had crept in. Each is now where a reader looks for it: why the nonce read runs before the sweep batch is built, and why a count is only recorded while the address is still awaiting one, belong to `verify_deposit_address_nonces`, which is what does both; why a reverted sweep's deposits have to be queued again is a `rearm` helper and the live test's own doc. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X44H4nAU65nAexEUvG15MK
…onces per tick The repair pass fanned one chain read out per queued address waiting on a nonce, with no bound: a delegate whose batches all revert leaves every address those sweeps touched waiting on a read, so a single timer could turn into a multi-provider `eth_getTransactionCount` per re-armed address. Every other stage of the pipeline takes a batch a tick; this one now does too, at the same size as a sweep's deposits. What a tick leaves out the next one reads, since a read that places an address takes it out of the set. The batch counts addresses, not queued pairs: one read places an account whatever tokens it has funded, so a second funded token of the same account may not cost the batch a slot. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X44H4nAU65nAexEUvG15MK
…ecord stands still An address waiting on a nonce read may still have another of its tokens in a sweep that was sent before the revert: that sweep applies its tuple when it finalizes, moving the very marks a read re-anchors. The read window spans an await, so a count fetched before that sweep landed could be recorded after it, undoing what it left. The recording step's re-check does not catch this — a sweep that succeeds leaves the address waiting on its read, so the address is still flagged when the stale count arrives. Such an address is now left out until the sweep is done with it. Waiting costs nothing: the sweep that reverted dropped the funds it named, so a read a tick skips is taken by a later one, off a record no longer moving under it. Nothing else can move that record meanwhile, since an address waiting on a read is offered no new sweep and the minter alone holds its key. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X44H4nAU65nAexEUvG15MK
24d6830 to
bf350f0
Compare
There was a problem hiding this comment.
🟡 Changes recommended
An unapplied current-nonce tuple can be ignored when the recorded delegate matches the configuration, allowing delegation state to drift.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 19/19 changed files
- Comments generated: 1
- Review effort level: Balanced
| let nonce = match self.automatic_deposits.delegation(&account) { | ||
| Some(delegation) if delegation.delegate == delegate => { | ||
| return SweepAuthorization::AlreadyDelegated; | ||
| } | ||
| //TODO DEFI-2997: track and increment nonce of deposit address | ||
| Some(_delegated_elsewhere) => TransactionNonce::ZERO, | ||
| Some(delegated_elsewhere) => delegated_elsewhere.nonce, |
There was a problem hiding this comment.
🤖 I don't think this one holds, on either half.
The one-tuple-per-nonce invariant is not broken. AlreadyDelegated signs nothing and carries nothing: sign_authorizations_batch filters it out before the signing set is built, and enqueue_sweep maps it to authorization: None. So no second tuple for that (account, chain, nonce) ever comes into existence — which is what the invariant asks for. Re-carrying is the means by which SweepAuthorization::Required avoids signing a competitor; it is not an end in itself, and a branch that signs nothing has nothing to compete with.
The record cannot be left permanently out of sync. Only the minter ever signs a tuple for a deposit address, so a relayer can only replay a tuple the minter itself broadcast, and a tuple only reaches the chain inside a sweep transaction. A sent sweep is never abandoned: it is resubmitted with the same authorizations (record_resubmit_transaction asserts equal_ignoring_fee_and_amount against the last sent transaction) until a receipt finalizes it, and record_finalized_sweep_transaction settles every tuple it carried whichever way the call went — a tuple applies before the call runs and stays applied when that call reverts.
So take the scenario you describe: delegate reconfigured to B, tuple (B, n) signed and sent, configuration rolled back to A. Whether the relayer's transaction or the minter's own lands (B, n) first, the outcome of the minter's finalization is the same: the record's next authorization nonce is still n, so record_applied_authorization marks (B, n) applied and delegation becomes (B, n+1) — exactly what the chain holds. A tuple that was signed but never enqueued is a different case and an easier one: it was never broadcast, so nobody but the minter has the signature and nothing can apply it.
And the suggested change would hurt. When the recorded delegate already matches the configuration, carrying that stale tuple would deliberately move the address back onto the delegate the operator just configured away from, spend a further 25'000 gas doing it, and then run that delegate's code against calldata built for the configured one.
There is a narrow real window in the neighbourhood — if the delegate is pointed back at A while a (B, n) tuple is still in flight, a concurrent sweep of another token of the same address is built tuple-less against A while the pending tuple will switch the code to B. But re-carrying (B, n) in that second sweep does not fix it: both sweeps still carry calldata for A. What covers it is the path this PR adds — the reverted sweep flags the account, the nonce is read back at FINALIZED, and the marks are re-anchored.
Leaving this open for @gregorydemay as a design point rather than resolving it.
|
🤖 Closing: this stack is superseded by a leaner design in which the chain is the source of truth for each deposit address' delegate (a batched EXTCODECOPY read before every sweep) instead of a record derived from applied tuples. Replacement stack starts at #11549 on top of #11532. The branch is kept for reference. |
Why
R18and "Changing the sweeper delegate" in docs(cketh): specify ETH deposits on the shared address and sweeper delegate rotation #11491.What
Last PR of this stack, stacked on #11501. ETH entry-point gating is left to the Phase 2 ETH work.
🤖 Generated with Claude Code
https://claude.ai/code/session_01X44H4nAU65nAexEUvG15MK