Skip to content

refactor(cketh): move the sweeper pipeline into AutomaticDeposits - #11351

Queued
gregorydemay wants to merge 2 commits into
masterfrom
gdemay/DEFI-2926-move-sweeper-pipeline
Queued

refactor(cketh): move the sweeper pipeline into AutomaticDeposits#11351
gregorydemay wants to merge 2 commits into
masterfrom
gdemay/DEFI-2926-move-sweeper-pipeline

Conversation

@gregorydemay

@gregorydemay gregorydemay commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Why

  • The sweeper pipeline sat on State, away from the sweep queue that feeds it, and its field was public where every sibling aggregate keeps its own private.

What

  • The pipeline moves into AutomaticDeposits, encapsulated the way WithdrawalTransactions encapsulates its own: the field is private and callers reach it through delegating methods, qualified with sweep so they cannot be confused with the deposit requests the same struct already holds.
  • AutomaticDeposits gains an is_equivalent_to. Replay compared the pipeline with its own, which sorts a queue that reschedule_request reorders without recording an event; nesting it under the strict equality on automatic_deposits would have made replay fail after any reschedule.

@gregorydemay
gregorydemay requested a balanced review from Copilot August 27, 2026 12:01
@gregorydemay
gregorydemay marked this pull request as ready for review August 27, 2026 12:03
@gregorydemay
gregorydemay requested a review from a team as a code owner August 27, 2026 12:03
@github-actions github-actions Bot added the @defi label Aug 27, 2026
@zeropath-ai

zeropath-ai Bot commented Aug 27, 2026

Copy link
Copy Markdown

No security or compliance issues detected. Reviewed everything up to 4dbc233.

Security Overview
Detected Code Changes
Change Type Relevant files
Refactor ► rs/ethereum/cketh/minter/src/lifecycle/init.rs
    Update state initialization to remove sweeper_transactions from InitArg and adjust AutomaticDeposits initialization
► rs/ethereum/cketh/minter/src/state.rs
    Remove SweeperTransactionPipeline usage from State and adapt references to AutomaticDeposits for sweeper nonce handling
► rs/ethereum/cketh/minter/src/state/audit.rs
    Redirect sweeper transaction records to AutomaticDeposits
► rs/ethereum/cketh/minter/src/state/automatic_deposits/mod.rs
    Replace sweeper_transactions with AutomaticDeposits integration and add new methods for sweeper handling
► rs/ethereum/cketh/minter/src/state/transactions/mod.rs
    Adjust references in tests and transaction module to align with new sweeper handling via AutomaticDeposits

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

Moves the sweeper transaction pipeline into AutomaticDeposits, preserving replay, initialization, upgrade, and processing behavior.

Changes:

  • Encapsulates sweeper transactions behind sweep-specific delegation methods.
  • Preserves replay-aware state equivalence.
  • Updates initialization, upgrades, processing, audit replay, and tests.

Reviewed changes

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

Show a summary per file
File Description
sweep.rs Uses the nested sweeper pipeline API.
state/tests.rs Updates state construction.
state/automatic_deposits/mod.rs Owns and delegates the sweeper pipeline.
state/audit.rs Routes sweeper events through AutomaticDeposits.
state.rs Removes the standalone pipeline and updates equivalence/upgrades.
lifecycle/tests.rs Updates nonce assertions.
lifecycle/init.rs Initializes the nested pipeline nonce.

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

@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/automatic_deposits/mod.rs
Comment thread rs/ethereum/cketh/minter/src/state/automatic_deposits/mod.rs
Comment thread rs/ethereum/cketh/minter/src/state.rs
@gregorydemay
gregorydemay force-pushed the gdemay/DEFI-2926-move-sweeper-pipeline branch from 46e161d to b0db3f1 Compare August 27, 2026 14:10
Base automatically changed from gdemay/DEFI-2926-mocks to master August 27, 2026 15:08
gregorydemay and others added 2 commits August 27, 2026 17:08
The pipeline sits next to the sweep queue that feeds it, encapsulated the
way WithdrawalTransactions encapsulates its own: the field stays private
and callers reach it through delegating methods.

State's replay equivalence check compared the pipeline with its own
`is_equivalent_to`, which sorts a queue that `reschedule_request` reorders
without recording an event. Nesting it under the strict `ensure_eq!` on
`automatic_deposits` would have made replay fail after any reschedule, so
AutomaticDeposits gained an `is_equivalent_to` of its own.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ence

The pipeline's equivalence check compared every field except
`processed_requests`, so a replay that rebuilt it differently still passed. The
field is written only when a transaction is created and `reschedule_request`
never touches it, so unlike the pending queue it needs no reordering
allowance.

Destructuring `Self` in the three equivalence checks makes the next forgotten
field a compile error rather than a silent gap.
@gregorydemay
gregorydemay force-pushed the gdemay/DEFI-2926-move-sweeper-pipeline branch from 46fcae8 to 4dbc233 Compare August 27, 2026 15:08
@gregorydemay
gregorydemay added this pull request to the merge queue Aug 27, 2026
Any commits made after this event will not be merged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants