refactor(cketh): make the transaction pipeline generic over its request

[gregorydemay]

Part of DEFI-2917 (deposit-from-CEX), stacked on #11177. Pure refactoring: no behaviour change, and no second pipeline yet.

Why

TransactionPipeline serves exactly one sender, the minter's main address: one nonce, every map keyed by a ckETH LedgerBurnIndex, the request type baked in. A second sender address cannot reuse any of it, and giving the sweeper its own address is the point of the stack.

What

Introduces PipelineRequest — an identity usable as the pipeline's alternate map key, a destination, a creation time, a fee-bump strategy, and the EIP-1559 transaction the request turns into — and makes TransactionPipeline<R> generic over it. WithdrawalRequest is the only implementation, so WithdrawalTransactions wraps TransactionPipeline<WithdrawalRequest> and every existing call site and behaviour is unchanged.

The pipeline's vocabulary follows the type: it speaks of requests and ids rather than withdrawals, since a future pipeline's requests are not withdrawals and its ids are not ledger burn indices. Reimbursement and withdrawal status keep their names — they live on WithdrawalTransactions and really are withdrawal concepts.

Two things fall out: create_transaction was a five-argument free function reaching into a request to build its transaction and is now PipelineRequest::to_transaction; and into_accepted_withdrawal_request_event had no production caller left, so it moves to the test file that was its only consumer, leaving EventType absent from state::transactions entirely.

_{Stack created with GitHub Stacks CLI • Give Feedback 💬}

[Copilot]

Pull request overview

This PR refactors the ckETH minter’s transaction state machine by making TransactionLane generic over a new LaneRequest trait, so the same send/resubmit/finalize machinery can later be reused for a second sender address without coupling it to withdrawal-specific concepts.

Changes:

• Introduces LaneRequest and makes TransactionLane<R> generic over it (with WithdrawalRequest as the current implementation).
• Moves transaction construction from a free function (create_transaction) into LaneRequest::to_transaction, and updates all call sites accordingly.
• Renames lane-facing APIs/vocabulary from “withdrawal” to generic “request” (e.g., record_request, requests_batch, oldest_incomplete_request_timestamp) while keeping withdrawal/reimbursement concepts on EthTransactions.

Reviewed changes

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

Show a summary per file

File	Description
rs/ethereum/cketh/minter/src/withdraw.rs	Updates batching flow to use generic request APIs and LaneRequest::to_transaction.
rs/ethereum/cketh/minter/src/state/transactions/tests.rs	Updates tests for renamed APIs and moves the “accepted withdrawal request event” helper into tests.
rs/ethereum/cketh/minter/src/state/transactions/mod.rs	Introduces LaneRequest, makes TransactionLane generic, migrates transaction creation into to_transaction, and updates request-centric terminology.
rs/ethereum/cketh/minter/src/state/tests.rs	Updates state tests to construct transactions via to_transaction.
rs/ethereum/cketh/minter/src/state/audit.rs	Updates audit transition handling to call record_request.
rs/ethereum/cketh/minter/src/state.rs	Updates state methods to use renamed request accessors (record_request, get_processed_request).
rs/ethereum/cketh/minter/src/main.rs	Updates metric to use oldest_incomplete_request_timestamp.
rs/ethereum/cketh/minter/src/guard/tests.rs	Updates guard tests to call record_request.
rs/ethereum/cketh/minter/src/guard/mod.rs	Updates pending-request count to use requests_len.
rs/ethereum/cketh/minter/src/dashboard/tests.rs	Replaces removed event-conversion method with a local helper and uses to_transaction.
rs/ethereum/cketh/minter/src/dashboard.rs	Updates dashboard to iterate pending requests via requests_iter.

---

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

[Copilot]

Pull request overview

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

Suppressed comments (2)

rs/ethereum/cketh/minter/src/state/transactions/mod.rs:1354

• remove_request removes by full Eq (r != request), but reschedule_request and the lane invariants are keyed by id(). If a caller passes a request value with the same id() but different non-id fields, reschedule_request will assert there is exactly one matching id, but remove_request won't remove it (and record_request will then panic on duplicate id). Removing by id() makes the API consistent and robust for non-withdrawal lane types.

    fn remove_request(&mut self, request: &R) {
        self.pending_requests.retain(|r| r != request);

rs/ethereum/cketh/minter/src/state/transactions/mod.rs:599

• This doc comment says the timestamp covers requests “awaiting a transaction or a reimbursement”, but the implementation only considers pending requests and maybe_reimburse (created-but-not-finalized). It does not include requests with a pending reimbursement_requests entry. Either adjust the wording to match the current behavior, or extend the calculation to include reimbursement backlog.

    /// Whether any request is still in flight, either awaiting a transaction or a reimbursement.

[gregorydemay]

🤖 The compile claim is not right, and CI on this exact commit (db25106) disproves it: Cargo Build Linux, Cargo Lint Linux and Bazel Test All are all green, and locally cargo check --all-targets --all-features -p ic-cketh-minter plus clippy with --deny warnings are clean.

E0446 is about private types — structs and enums — appearing in a public signature. A type alias is transparent: it is only a name, and the type it expands to here (Result, Eip1559TransactionRequest, ResubmitTransactionError) is entirely public, so nothing private leaks. The alias merely is not nameable from outside this module, and rustdoc renders the expanded type.

There is a smaller, real point buried in this though: the sibling alias ReimbursedResult right above is pub type, so making ResubmitResult public too would be more consistent and would let the rendered docs show the alias name. Leaving that to @gregorydemay as a judgement call rather than restacking the whole stack for a visibility keyword.