Skip to content

feat(cketh): enqueue the sweep a signed batch has become - #11362

Merged
gregorydemay merged 10 commits into
gdemay/DEFI-2926-sign-authorizationsfrom
gdemay/DEFI-2926-enqueue-sweeper-request
Aug 29, 2026
Merged

feat(cketh): enqueue the sweep a signed batch has become#11362
gregorydemay merged 10 commits into
gdemay/DEFI-2926-sign-authorizationsfrom
gdemay/DEFI-2926-enqueue-sweeper-request

Conversation

@gregorydemay

@gregorydemay gregorydemay commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

create_pending_sweeper_requests signed a batch's attestations and its authorizations and then discarded both — the signatures reached stable memory, but nothing was ever asked to move the funds. This closes that gap, and fixes an asymmetry between the two transaction pipelines on the way.

A sweep request now says what to sweep, not what its transaction looks like. WithdrawalTransactions starts from a WithdrawalRequest that carries no Ethereum detail — the nonce, the gas price and even the ERC-20 call data are the pipeline's to decide. SweepRequest arrived pre-baked with its destination, value and ABI-encoded delegate call, leaving the pipeline only a nonce to fill in. It now names the single token it moves and, per account, the deposit address, the account credited, the attestation binding the two, and the delegation to install; the call data and the zero value are derived in create_transaction. SweepTransaction is once again the only Ethereum-shaped type in play.

The delegation is optional, so an address already delegated needs no tuple and a sweep of none is sent as a plain EIP-1559 transaction rather than a type-0x04 one. max_transaction_fee stays on the request, as on Erc20WithdrawalRequest, because the resubmission strategy caps a fee bump against it.

A queued deposit is no longer offered to two sweeps. Batching only peeked at the sweep queue, so the deposits one tick took were still there for the next tick to take again — two sweeps moving a balance the minter has already accounted for once. A queued deposit now records the sweep holding it, and batching skips those. The entry stays queued until the sweep settles: while it is in flight this is the only record of which balance sits at which address, and a sweep that fails has to be able to say so. Releasing entries on success or failure comes with the code that sends them.

Then the batch is enqueued, one sweep per token, with its id minted as the event is recorded so two tokens batched in the same tick cannot claim the same one. A target missing either signature is left out rather than swept — without an attestation the sweep cannot prove the address credits the account, without an authorization it cannot delegate the address — and stays queued for the next tick.

A sweep is priced for the work its delegate does. Every sweep carried a flat 100'000 gas whatever it moved, which sits below the intrinsic cost of its own EIP-7702 authorizations as soon as it names more than a few addresses: a ten-deposit batch was accepted into the mempool and could never be included. The limit now grows with the addresses the sweep walks — a balance check, a transfer and an authorization each, over a fixed base — and max_transaction_fee is priced off the same figure, so a sweep is never created against a cap its own gas limit exceeds.

Why CI_OVERRIDE_DIDC_CHECK. The didc check requires the new Candid interface to be a subtype of the released one, and reshaping a record inside the Event variant get_events returns never is: AcceptedSweepRequest trades amount, data and authorizations for token and items, and SignedAuthorization and AttestedDepositAddress fold their loose y_parity/r/s fields into a TransactionSignature. Overriding is safe because every reshaped event belongs to the unreleased sweeper pipeline: no released minter has ever emitted one, so no stored log entry or client decodes the old shape.

@gregorydemay gregorydemay changed the title gdemay/DEFI 2926 enqueue sweeper request feat(cketh): enqueue the sweep a signed batch has become Aug 27, 2026
@github-actions github-actions Bot added the feat label Aug 27, 2026
@gregorydemay
gregorydemay requested a balanced review from Copilot August 27, 2026 22:08
@gregorydemay gregorydemay added the CI_OVERRIDE_DIDC_CHECK Skips the backwards compatibility didc check (explain in PR description why) label Aug 27, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Enqueues signed token sweeps into the ckETH minter transaction pipeline and prevents deposits from entering concurrent sweeps.

Changes:

  • Reshapes sweep requests around tokens and authorized deposit items.
  • Tracks sweep ownership and enqueues token-grouped batches.
  • Updates event APIs, stable-memory mappings, and tests.

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tests/dump_stable_memory.rs Maps the revised sweep events.
src/test_fixtures.rs Adds a second test account.
src/sweeper_contract/mod.rs Makes sweep items event-serializable.
src/sweep/tests.rs Tests batching and enqueue behavior.
src/sweep/mod.rs Builds and enqueues sweep requests.
src/state/transactions/tests.rs Tests derived sweep transactions.
src/state/transactions/request.rs Derives transaction data from requests.
src/state/transactions/mod.rs Redefines sweep request data.
src/state/automatic_deposits/tests.rs Tests queue ownership.
src/state/automatic_deposits/mod.rs Tracks deposits assigned to sweeps.
src/state/audit/tests.rs Updates event replay mappings.
src/state/audit.rs Replays sweep ownership.
src/management/mod.rs Adds a call-error constructor.
src/main.rs Exposes revised sweep events.
src/endpoints.rs Updates Candid event types.
cketh_minter.did Updates the public interface definition.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread rs/ethereum/cketh/minter/src/sweep/mod.rs Outdated
Comment thread rs/ethereum/cketh/minter/src/state/automatic_deposits/mod.rs
Comment thread rs/ethereum/cketh/minter/src/sweep/mod.rs
@gregorydemay
gregorydemay marked this pull request as ready for review August 28, 2026 07:42
@gregorydemay
gregorydemay requested a review from a team as a code owner August 28, 2026 07:42
@github-actions github-actions Bot added the @defi label Aug 28, 2026
@zeropath-ai

zeropath-ai Bot commented Aug 28, 2026

Copy link
Copy Markdown

No security or compliance issues detected. Reviewed everything up to 046664e.

Security Overview
Detected Code Changes

The diff is too large to display a summary of code changes.

@gregorydemay

Copy link
Copy Markdown
Contributor Author

🤖 Cross-checked the reviewer comments from the previous PR stack (#11330, #11331, #11258, #11337) against this stack. Most are addressed here by design: the fee cap is priced off request.gas_limit(), call_data() is derived from items so data cannot diverge from the deposits, one token per sweep kills the items × tokens gas blowup, attestations/authorizations are event-sourced and reused (no re-signing on unaffordable ticks, no replay trap), deposits are released on finalization, and a duplicate pair in a request traps instead of corrupting the queue.

Dispositions for what still differs:

@mbjorkqvist mbjorkqvist left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @gregorydemay!

Comment thread rs/ethereum/cketh/minter/src/state/automatic_deposits/mod.rs Outdated
Comment thread rs/ethereum/cketh/minter/src/state/transactions/mod.rs Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.

Comment thread rs/ethereum/cketh/minter/src/state/transactions/mod.rs

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.

Comment thread rs/ethereum/cketh/minter/src/sweep/mod.rs
@gregorydemay
gregorydemay force-pushed the gdemay/DEFI-2926-enqueue-sweeper-request branch from 742f1cf to 2685921 Compare August 28, 2026 15:23
@gregorydemay
gregorydemay added this pull request to the merge queue Aug 28, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to no response for status checks Aug 28, 2026
@gregorydemay
gregorydemay requested a review from a team as a code owner August 28, 2026 19:03

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pull request changes code owned by the Governance team. Therefore, make sure that
you have considered the following (for Governance-owned code):

  1. Update unreleased_changelog.md (if there are behavior changes, even if they are
    non-breaking).

  2. Are there BREAKING changes?

  3. Is a data migration needed?

  4. Security review?

How to Satisfy This Automatic Review

  1. Go to the bottom of the pull request page.

  2. Look for where it says this bot is requesting changes.

  3. Click the three dots to the right.

  4. Select "Dismiss review".

  5. In the text entry box, respond to each of the numbered items in the previous
    section, declare one of the following:

  • Done.

  • $REASON_WHY_NO_NEED. E.g. for unreleased_changelog.md, "No
    canister behavior changes.", or for item 2, "Existing APIs
    behave as before.".

Brief Guide to "Externally Visible" Changes

"Externally visible behavior change" is very often due to some NEW canister API.

Changes to EXISTING APIs are more likely to be "breaking".

If these changes are breaking, make sure that clients know how to migrate, how to
maintain their continuity of operations.

If your changes are behind a feature flag, then, do NOT add entrie(s) to
unreleased_changelog.md in this PR! But rather, add entrie(s) later, in the PR
that enables these changes in production.

Reference(s)

For a more comprehensive checklist, see here.

GOVERNANCE_CHECKLIST_REMINDER_DEDUP

@gregorydemay
gregorydemay deleted the branch gdemay/DEFI-2926-sign-authorizations August 28, 2026 19:03
@gregorydemay gregorydemay reopened this Aug 28, 2026
@zeropath-ai

zeropath-ai Bot commented Aug 28, 2026

Copy link
Copy Markdown

No security or compliance issues detected. Reviewed everything up to 046664e.

Security Overview
Detected Code Changes

The diff is too large to display a summary of code changes.

@zeropath-ai

zeropath-ai Bot commented Aug 28, 2026

Copy link
Copy Markdown

No security or compliance issues detected. Reviewed everything up to 046664e.

Security Overview
Detected Code Changes

The diff is too large to display a summary of code changes.

@zeropath-ai

zeropath-ai Bot commented Aug 28, 2026

Copy link
Copy Markdown

No security or compliance issues detected. Reviewed everything up to 046664e.

Security Overview
Detected Code Changes

The diff is too large to display a summary of code changes.

@gregorydemay gregorydemay reopened this Aug 29, 2026
@gregorydemay
gregorydemay dismissed github-actions[bot]’s stale review August 29, 2026 08:34

🤖 Dismissing: this review was filed while the PR's base branch was transiently missing, so its diff momentarily spanned unrelated master commits. The PR changes only rs/ethereum/cketh (16 files). 1. unreleased_changelog.md: no Governance-owned code changed. 2. No breaking changes to Governance. 3. No data migration for Governance. 4. No Governance security review needed.

@gregorydemay
gregorydemay removed request for a team August 29, 2026 08:38
@gregorydemay
gregorydemay added this pull request to the merge queue Aug 29, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to no response for status checks Aug 29, 2026
@gregorydemay
gregorydemay added this pull request to the merge queue Aug 29, 2026
Merged via the queue into master with commit 8eb36dc Aug 29, 2026
67 of 68 checks passed
@gregorydemay
gregorydemay deleted the gdemay/DEFI-2926-enqueue-sweeper-request branch August 29, 2026 10:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI_OVERRIDE_DIDC_CHECK Skips the backwards compatibility didc check (explain in PR description why) @defi feat

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants