feat(cketh): install a deposit address' delegation with its first sweep - #11250
feat(cketh): install a deposit address' delegation with its first sweep#11250gregorydemay wants to merge 11 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Adds EIP-7702 delegation installation to first-time ckETH deposit-address sweeps while generalizing transaction pipeline mechanics.
Changes:
- Introduces EIP-1559/EIP-7702 sweep transactions.
- Generalizes finalization and fee resubmission.
- Extends audit events and Candid types with signed authorizations.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
tests/dump_stable_memory.rs |
Maps new sweep events. |
src/tx/tests.rs |
Tests sweep encoding and signing. |
src/tx/sweep.rs |
Defines variant sweep transactions. |
src/tx/signed.rs |
Expands the signable transaction interface. |
src/tx/mod.rs |
Generalizes fee resubmission. |
src/tx/finalized.rs |
Adds generic finalized transactions. |
src/tx/eip_7702.rs |
Enables EIP-7702 fee bumps. |
src/tx/eip_1559.rs |
Adopts generic transaction machinery. |
src/state/transactions/tests.rs |
Tests delegating sweep pipelines. |
src/state/transactions/request.rs |
Creates the appropriate sweep variant. |
src/state/transactions/mod.rs |
Generalizes transaction pipelines. |
src/state/event.rs |
Stores sweep transaction variants. |
src/state/audit/tests.rs |
Updates event replay mapping. |
src/main.rs |
Exposes authorization data in events. |
src/endpoints.rs |
Adds Candid-facing sweep types. |
cketh_minter.did |
Updates the public Candid interface. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
02b7635 to
67bd529
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.
Suppressed comments (2)
rs/ethereum/cketh/minter/tests/dump_stable_memory.rs:193
- Delegating sweeps are signed type-0x04 transactions, but this path always calls an EIP-1559-only decoder and then constructs
SweepTransaction::Eip1559. Once a signed delegating sweep appears inget_events,decode_signed_transactionrejects it at itsTypedTransaction::Eip1559match, so stable-memory dumping cannot process the new event. Decode type 0x04 (including its authorization list) and construct the matching sweep variant.
fn map_signed_sweep_transaction(raw_transaction: &str) -> SignedSweepTransaction {
let (transaction, signature) = decode_signed_transaction(raw_transaction);
SignedSweepTransaction::from((SweepTransaction::Eip1559(transaction), signature))
rs/ethereum/cketh/minter/src/state/audit/tests.rs:250
- This replay mapper still accepts only
TypedTransaction::Eip1559and unconditionally rebuilds the signed sweep as that variant. A signed first sweep is type 0x04, so refreshing/replaying events after delegating sweeps begin will panic instead of reconstructing state. Decode EIP-7702 raw transactions and preserve their authorization list here.
fn map_signed_sweep_transaction(raw_transaction: &str) -> SignedSweepTransaction {
let (transaction, signature) = decode_signed_transaction(raw_transaction);
SignedSweepTransaction::from((SweepTransaction::Eip1559(transaction), signature))
86f084b to
62aa850
Compare
62aa850 to
afd8ef3
Compare
afd8ef3 to
e268990
Compare
Finalization and fee-bumping were written against EIP-1559 alone: `FinalizedEip1559Transaction` was a concrete struct, `try_finalize` an inherent method on `SignedEip1559TransactionRequest`, and `resubmit` an inherent method on `Resubmittable<SignedEip1559TransactionRequest>`. The sweeper pipeline will carry type-`0x04` transactions to install a deposit address' EIP-7702 delegation, and would have needed all three mirrored for it — the very TODO `SignedEip7702TransactionRequest` carried. All three reduce to one primitive. Give `SignableTransaction` its remaining field accessors and `with_price_and_amount`, and everything else follows generically: `Finalized<T>` replaces the concrete struct with `FinalizedEip1559Transaction` as its alias, `try_finalize` moves to `Signed<T>`, `resubmit` to `Resubmittable<Signed<T>>`, and `equal_ignoring_fee_and_amount` takes any two transactions of one type. EIP-7702 gets fee-bumping for free: an authorization is signed over `(chain_id, delegate, nonce)` only, so bumping the outer fee leaves the authorization list valid, and struct update syntax carries it along. `Finalized<T>` bounds `T: SignableTransaction` on the struct, as `TransactionPipeline<R: PipelineRequest>` does: a finalized transaction can only come from a signed one, and the derived `Decode` needs the bound to see through `Signed<T>`. No behaviour change. `equal_ignoring_fee_and_amount` still compares `gas_limit`, taking it from the right-hand transaction while overriding only the two fees and the amount. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The pipeline was generic over its request but not over what that request becomes: `created_tx`, `sent_tx` and `finalized_tx` were pinned to `Eip1559TransactionRequest`, as were `create_transaction`, `assert_created_transaction`, `TransactionStage` and every accessor between them. The sweeper pipeline needs to carry type-`0x04` transactions to install a deposit address' EIP-7702 delegation, and cannot while the pipeline names the transaction type itself. Add `PipelineRequest::Transaction` and let the pipeline speak it throughout: a request already knows the transaction it turns into, so it is the request that should say so. The associated type requires `SignableTransaction` and nothing more, since that is all a request needs to build one. `Clone + Eq + Debug` are required by the pipeline's impl block instead, which is what clones transactions into signing batches and compares them in its assertions. Both pipelines still set `Transaction = Eip1559TransactionRequest`, so this changes no behaviour: every stored type, event and call site resolves to exactly what it did before. The sweeper's own transaction type comes next. `CreatedTransaction<R>` and `SentTransaction<R>` name the two `Resubmittable` nestings the pipeline stores, taking over from the EIP-1559-pinned `TransactionRequest`/`SignedTransactionRequest` aliases, which stay in `tx` for the withdrawal-lane tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A deposit address holds ERC-20 tokens but no code, so it cannot sweep itself. The minter delegates it to the sweeper contract with an EIP-7702 authorization, and the cheapest place to carry that authorization is the sweep that needs it: the first sweep touching an address is a type-`0x04` transaction that installs the delegation on the way. The delegation then persists, so every later sweep of that address is a plain type-`0x02` transaction — which is what `deposit_from_cex_demo` measures, at 94'932 gas for the first sweep of one address against 63'252 for the next. `SweepTransaction` is what the sweeper pipeline now carries: an EIP-1559 transaction, or an EIP-7702 one whose authorization list installs the delegations. `SweepRequest::authorizations` holds them, signed, so `create_transaction` stays a pure function of the request and the accepted event carries them — a replay never re-signs, and neither does a fee bump, since an authorization covers `(chain_id, delegate, nonce)` only and nothing of the outer transaction. `SweepTransaction::new` is the single place that picks the variant, and it picks it from whether there is anything to install. That keeps the two representations in step: the `Eip7702` variant always carries a non-empty authorization list, which is what its RLP encoding already asserts, and the Candid mirror can carry the list next to the transaction without a tag of its own. The three sweeper transaction events change shape accordingly, with `UnsignedSweeperTransaction` as their Candid mirror rather than a widened `UnsignedTransaction`: the withdrawal events on mainnet keep the type they have today, untouched. `AcceptedSweepRequest` mirrors the delegations too. Which addresses still need delegating is not decided here — nothing enqueues a sweep yet. Because the request carries signed authorizations rather than a flag, that decision belongs entirely to the sweep-queue source, together with reading each address' delegation from the chain. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The `SignedSweeperTransaction` audit event records only the raw hex of the broadcast transaction, and the reverse mapping used to replay event dumps decoded it with `ethers_core`, which predates EIP-7702 and only understands type `0x02`. A genuine type-`0x04` sweep would therefore be replayed as an EIP-1559 transaction and compare unequal to the recorded one. Add the inverse of the type-`0x04` RLP encoder next to the encoder itself, and let both reverse mappings dispatch on the transaction type byte so a sweep is rebuilt as the variant it was actually sent as. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
f923cf7 to
441e5d1
Compare
…ady at hand `map_signed_sweep_transaction` reached for `hex::decode` and stripped the `0x` prefix by hand, while the very same file already turns a raw transaction into bytes with `ethers_core::types::Bytes::from_str`, prefix included. Reuse that and the `hex` dependency the sweeper binary just gained is no longer needed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The authorization list is independent of the sweep's call data: a batch sweeping N deposit addresses carries a tuple only for those not yet delegated to the sweeper contract, so the list can be shorter than the set of addresses swept. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Fold the two variant-selection tests into one table-driven test and add the mixed case, where a batch sweeping several deposit addresses carries an authorization only for those not yet delegated. Every case asserts that the sweep itself - destination, amount, call data and nonce - comes from the request regardless of the variant chosen. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…he library The decoder has two properties a reader is entitled to question: it reimplements what a library could do, and it sits in productive code although only tests call it. Neither is a preference, so note the constraint behind each and the ticket that lifts both. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.
Suppressed comments (1)
rs/ethereum/cketh/minter/src/tx/eip_7702.rs:287
decodecan panic on a malformed type-0x04 payload with an empty authorization list.Self::fromimmediately computes the hash, which callsEip7702TransactionRequest::rlp_innerand triggers its non-empty-list assertion. Since this decoder returnsResultand documents malformed payloads as errors, reject the empty list before constructingSigned.
authorization_list: decode_list(&rlp, 9)?
.iter()
.map(|item| {
Ok(SignedAuthorization {
chain_id: decode_val(item, 0)?,
delegate: decode_address(item, 1)?,
nonce: decode_amount(item, 2)?,
y_parity: decode_val(item, 3)?,
r: decode_u256(item, 4)?,
s: decode_u256(item, 5)?,
})
})
.collect::<Result<_, String>>()?,
|
✅ No security or compliance issues detected. Reviewed everything up to 2ca527d. Security Overview
Detected Code Changes
|
The decoder built the transaction before anything checked the list, and hashing it on the way into Signed hit rlp_inner's non-empty assertion, so the malformed input with the clearest error to report was the one that panicked instead of reporting it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…finity#11144) ## Why The minter's dedicated sweeper address needs to send Ethereum transactions, and it must not share the main address' nonce sequence: a sweep stuck behind a fee-starved transaction would head-of-line-block every user withdrawal. ## What A second instance of the transaction pipeline, for the sweeper address, on a nonce sequence of its own. A sweep burns no ckETH, so it is keyed by a plain counter rather than a ledger burn index, and is never reimbursed; it pays gas from the sweeper's prepaid balance, so it has no transaction fee it can fail to cover. Five audit events record the pipeline's transitions, with reconstruction, Candid mirrors and `.did`. The sweeper's start nonce can be set from both lifecycle arguments. It is optional on install where the main address' equivalent is required, because install arguments are replayed from the event log and a required field would be missing from every event already written. ## Scope The pipeline only. No timer drives it and nothing enqueues a sweep, so the new state stays empty: the sending task is [DEFI-2926](https://dfinity.atlassian.net/browse/DEFI-2926) in dfinity#11237, and EIP-7702 first-time delegation is dfinity#11250, stacked above. The sweep-queue source and prepaid-gas gating are still to come. Sweeper *funding* — burning ckETH from the minter's fee subaccount to prepay that gas — is a separate stack (DEFI-2933) that meets this one only in the withdrawal pipeline, where its request variant has already landed. Hence the audit events here are numbered from `n28`, above the funding event master now owns. ## Candid compatibility `CI_OVERRIDE_DIDC_CHECK` is set. The check flags the five cases this PR adds to the `Event` payload variant, since a variant returned to callers may not grow under Candid subtyping. It is the additive shape every new audit event has taken: existing cases keep their names and fields, and callers matching exhaustively on the old set see the new ones only for sweeper activity, which nothing enqueues yet. The new optional nonce field on the two argument types is compatible on its own. <sub>Stack created with <a href="https://github.com/github/gh-stack">GitHub Stacks CLI</a> • <a href="https://gh.io/stacks-feedback">Give Feedback 💬</a></sub> [DEFI-2917]: https://dfinity.atlassian.net/browse/DEFI-2917?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [DEFI-2926]: https://dfinity.atlassian.net/browse/DEFI-2926?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Part of DEFI-2917 (deposit-from-CEX), on
masternow that #11144 has landed.Why
deposit_from_cex_demomeasures 94'932 gas for the first sweep of an address against 63'252 for the next.What
Scope
Candid compatibility
CI_OVERRIDE_DIDC_CHECKlabel.transactionfield, and a third gains a required field.Stack created with GitHub Stacks CLI • Give Feedback 💬