Skip to content

feat(cketh): sign the attestations a batched sweep needs - #11359

Merged
gregorydemay merged 9 commits into
masterfrom
gdemay/DEFI-2926-sign-attestations
Aug 28, 2026
Merged

feat(cketh): sign the attestations a batched sweep needs#11359
gregorydemay merged 9 commits into
masterfrom
gdemay/DEFI-2926-sign-attestations

Conversation

@gregorydemay

@gregorydemay gregorydemay commented Aug 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Why

  • The sweep queue folds deposits into a batch per token, but nothing produced the attestation each deposit needs before it can be swept: the delegate recovers that signature against the deposit address it moves funds from, so an unattested deposit cannot be swept at all.

What

  • Each batch resolves the attestation request its deposits need and signs the ones the minter has not signed before.
  • An attestation binds an account to one chain and one deposit helper — never to a token — so an account depositing several tokens is attested once and the signature is reused across its batches.
  • Signatures are recorded through the event log, so replaying after an upgrade keeps them rather than paying for another threshold-ECDSA signature.
  • A deposit whose attestation cannot be signed is reported and left for the next tick.

@github-actions github-actions Bot added the feat label Aug 27, 2026
@gregorydemay
gregorydemay requested a balanced review from Copilot August 27, 2026 17:27
Copilot AI reviewed Aug 27, 2026
View reviewed changes

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

Adds threshold-ECDSA attestation signing for queued ckERC20 deposit sweeps.

Changes:

  • Groups queued deposits into per-token batches.
  • Deduplicates, signs, and event-logs attestations.
  • Adds shared fixtures and sweep signing tests.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
attestation/mod.rs Exposes and reuses attestation derivation paths.
state.rs Builds batched attestation requests.
state/automatic_deposits/mod.rs Groups sweep targets by token.
state/automatic_deposits/tests.rs Uses shared fixtures.
sweep/mod.rs Signs and records missing attestations.
sweep/tests.rs Tests signing and configuration guards.
test_fixtures.rs Adds shared deposit and token fixtures.

💡 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/sweep/tests.rs Outdated
@gregorydemay
gregorydemay marked this pull request as ready for review August 27, 2026 17:32
@gregorydemay
gregorydemay requested a review from a team as a code owner August 27, 2026 17:32
@github-actions github-actions Bot added the @defi label Aug 27, 2026
@zeropath-ai

zeropath-ai Bot commented Aug 27, 2026 •
edited
Loading

Copy link
Copy Markdown

✅ No security or compliance issues detected. Reviewed everything up to f33a0c6.

Security Overview
Detected Code Changes
Change Type Relevant files
Enhancement ► rs/ethereum/cketh/minter/src/attestation/mod.rs
    Add derivation_path() and refactor sign_attestation usage to use it
Enhancement ► rs/ethereum/cketh/minter/src/state.rs
    Add attestation_requests() to generate AttestationRequest list for accounts
Enhancement ► rs/ethereum/cketh/minter/src/state/automatic_deposits/mod.rs
    Add requests_batch() to batch deposit sweep targets by token
Enhancement ► rs/ethereum/cketh/minter/src/state/automatic_deposits/tests.rs
    Update tests to reflect new deposit/address helpers and test data changes
Enhancement ► rs/ethereum/cketh/minter/src/sweep/mod.rs
    Use AttestationRequest and sign_attestation; accommodate batching and imports adjustments
Enhancement ► rs/ethereum/cketh/minter/src/sweep/tests.rs
    Add tests for pending sweeper requests behavior, signing flow, and related fixtures
Enhancement ► rs/ethereum/cketh/minter/src/test_fixtures.rs
    Add deposit_address, usdc, usdt fixtures for tests

Base automatically changed from gdemay/DEFI-2926-enqueue-skeleton to master August 27, 2026 19:03
gregorydemay and others added 7 commits August 27, 2026 21:03
@gregorydemay @claude
feat(cketh): skip sweeper requests when no sweeper contract is config…
d5ca6bd 
…ured

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@gregorydemay @claude
feat(cketh): batch the sweep queue by token
5306b36 
Groups the queued deposits into one batch per token, capped at ten deposits
each, and resolves the attestation request every deposit in a batch will need.
Nothing is signed or enqueued yet.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@gregorydemay @claude
feat(cketh): sign the attestations a batch needs
375c8fc 
Signs one attestation per deposit in a batch, skipping those already stored
so a repeat sweep of the same address costs no threshold-ECDSA signature.
A deposit whose attestation fails to sign is only logged for now.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
IDX GitHub Automation @gregorydemay
Automatically fixing code for linting and formatting issues
7e77f47
@gregorydemay
test: sign attestations
1d7ab6b
@gregorydemay @claude
test(cketh): cover every token of an account with one attestation
5ab33e5 
Folds the three attestation tests into one: an account with a deposit in
two tokens must yield a single signature, since an attestation binds the
account and not the token. Asserts the whole event log, so bypassing it
fails the test.

Shares the usdc/usdt and deposit_address helpers through test_fixtures
rather than keeping them private to the automatic_deposits tests.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@gregorydemay
fix(cketh): price the sweep queue only once it holds something
019bda3 
The gas fee estimate was refreshed before the queue was known to be non-empty,
and it expires on this task's own 60s interval, so an idle minter paid for an
eth_feeHistory outcall on every tick. Checking the queue first leaves the
estimate where sweep-request construction will consume it.

The deposit-helper test asserted a guard it never reached: the fixture it built
on configures no sweeper contract, so the run returned at the earlier guard and
the test was a duplicate of its sibling. It now starts from a state that gets
as far as the helper.
gregorydemay
gregorydemay commented Aug 27, 2026
View reviewed changes
Comment thread rs/ethereum/cketh/minter/src/sweep/mod.rs Outdated
@gregorydemay
gregorydemay force-pushed the gdemay/DEFI-2926-sign-attestations branch from 44d30c4 to 019bda3 Compare August 27, 2026 19:03
mbjorkqvist
mbjorkqvist approved these changes Aug 28, 2026
View reviewed changes

@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/sweep/mod.rs
Comment thread rs/ethereum/cketh/minter/src/sweep/mod.rs
Comment thread rs/ethereum/cketh/minter/src/sweep/mod.rs Outdated
@gregorydemay @claude
fix(cketh): prefix the attestation error log like the rest of the swe…
f33a0c6 
…ep enqueue

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YK3ggw1xtE3HZTGGBu66oD
This was referenced Aug 28, 2026
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
@gregorydemay
gregorydemay requested a balanced review from Copilot August 28, 2026 08:45
Copilot AI reviewed Aug 28, 2026
View reviewed changes

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 7 out of 7 changed files in this pull request and generated no new comments.

@gregorydemay
gregorydemay added this pull request to the merge queue Aug 28, 2026
Merged via the queue into master with commit e2a4283 Aug 28, 2026
72 of 73 checks passed
@gregorydemay
gregorydemay deleted the gdemay/DEFI-2926-sign-attestations branch August 28, 2026 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@mbjorkqvist mbjorkqvist mbjorkqvist approved these changes

Assignees

No one assigned

Labels

@defi feat

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@gregorydemay @mbjorkqvist