Skip to content

feat(cketh): DEFI-3013: Bound and adapt the per-round transaction receipt fan-out - #11636

Open
mbjorkqvist wants to merge 21 commits into
masterfrom
mathias/DEFI-3013-adaptive-receipt-fetch-window
Open

mbjorkqvist wants to merge 21 commits into
masterfrom
mathias/DEFI-3013-adaptive-receipt-fetch-window

Conversation

@mbjorkqvist

@mbjorkqvist mbjorkqvist commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

When two of the ckETH minter's four Ethereum RPC providers started failing HTTPS-outcall consensus on eth_getTransactionReceipt, no withdrawal could be finalized — and the minter responded by re-polling its entire pending set every couple of minutes, indefinitely. The backlog grew to roughly a hundred transactions and receipt outcalls went from a few hundred a day to a peak of 11,583 an hour, which exhausted one provider's request quota and quadrupled the minter's cycle burn. The withdrawals themselves were fine throughout; only the bookkeeping was stuck.

This bounds that behaviour. The number of withdrawals whose receipts are fetched in a round is now capped and adapts to what the providers are actually doing: a clean round doubles it, a partly failed round halves it, and a totally failed round drops it straight to one. Rounds also stop discarding the receipts they did get because one lookup failed, so a backlog drains as providers recover instead of being refetched whole every round.

Three supporting changes fall out of that:

  • Receipts are grouped by withdrawal before a round is sliced, so a withdrawal that has been resubmitted at a higher gas price never has its variants split across rounds.
  • A cursor rotates the window over the pending set, so a withdrawal whose receipt can never be retrieved cannot pin the loop onto itself and starve everything behind it.
  • The batch-wide assertion that every withdrawal in a round produced exactly one receipt is gone. It was a live trap risk: a withdrawal whose transactions all return "not found" — after a reorg, or a nonce filled by something else — would have brought the canister down rather than simply stalling. Such a withdrawal now stays pending and is counted.

New metrics expose the window size, lookup outcomes per pipeline, stalled withdrawals, and rounds abandoned on conflicting receipts. That last one should never be non-zero; it indicates a broken invariant rather than an unhealthy provider.

Scope

The window bounds eth_getTransactionReceipt. The other calls a finalization round makes — eth_feeHistory for the gas estimate, eth_getTransactionCount(Latest) for the nonce, eth_sendRawTransaction — are untouched, as are deposit log scraping, balance scans and the sweeper's delegation reads. That is deliberate: during the incident the same providers served every other method fine, and only receipts failed consensus.

One exception, and it is not receipt fan-out: a round begins by reading eth_getTransactionCount(Finalized) to decide which nonces are eligible, and when that read itself keeps failing the round is skipped outright — three rounds in four, once it has failed three times running. This exists because the adaptive window is blind to that failure: no read means no lookups, so there are no failures to shrink the window with, and the minter would otherwise spin at full cadence indefinitely making no progress. A single successful read clears it, even one that finds nothing to fetch. So this does reduce polling of eth_getTransactionCount(Finalized) during a sustained outage of that call, by a bounded factor of four.

This means a shrunken window does not slow the rest of the minter. The throttle sits in the last step of the round, so new withdrawals are still created, signed and sent on the normal cadence, resubmission still happens, and deposits run on their own timer; only the bookkeeping of already-sent withdrawals waits. Even pinned at the floor the minter finalizes roughly 480 withdrawals a day against an observed volume of 15–20, and a single clean round doubles the window back up.

The flip side is that this is narrowly targeted: if a future incident hits a different method, eth_getLogs say, nothing here helps.

Deliberately not included: batching the lookups into a single JSON-RPC call per provider, and general exponential backoff on the round cadence. Batching would cut fan-out but makes one non-deterministic response poison every lookup sharing its outcall, which is the opposite of what this incident calls for. General backoff was rejected because nothing here was rate limiting — no provider returned a 429 across two weeks and roughly a million responses — and because it recovers worst exactly when the backlog is largest. The skip described under Scope is not that: it is a fixed factor rather than a growing one, it applies only to the nonce read rather than to the round as a whole, and it resets on the first success.

Both the withdrawal and sweeper pipelines are covered, each with its own window.

🤖 Generated with Claude Code

mbjorkqvist and others added 10 commits September 21, 2026 13:59
A receipt lookup fans out to every provider on every replica, so a finalization
round that fetches the whole pending set at once fans out hardest exactly when
that set is large - which is when receipts are failing in the first place.

Add a per-pipeline window over the ids a round fetches receipts for: it doubles
while no lookup fails, halves when some do, and drops straight to its floor when
they all do. Only provider-level failures shrink it, so the "not mined" answer a
superseded resubmission gives does not collapse it on a healthy minter. The
window takes whole ids rather than slicing the hash-keyed pending map, and
carries a cursor so consecutive rounds walk the whole pending set instead of
retrying the same arbitrary subset. It also counts the rounds that could not read
the chain at all, so a pipeline stuck before its lookups stops re-running at full
cadence.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A single failed lookup used to discard every receipt the round had already
collected, so one unhealthy provider kept the whole pending set unfinalized and
the next round re-fetched all of it. Handle each id on its own instead: a failed
lookup leaves its id pending, the ids that answered finalize.

An id whose every transaction answered "not mined" - a nonce filled by another
transaction, or a reorg - used to trip the assert on the ids expected to
finalize, trapping the canister every round rather than letting that one
withdrawal stall. It now stays pending and is counted. Two different receipts for
the same id still abandons the round: no chain can produce that.

Both pipelines now run their receipt fetch through one shared round, each on its
own window, so a sweeper problem cannot throttle user withdrawals.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both pipelines report the window their next round will use, how many consecutive
rounds could not read the chain, what their lookups returned, and the ids a round
left pending because none of their transactions came back with a receipt - the
withdrawal that stalls instead of trapping is now visible rather than silent.

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

Addresses review comment M1. Recording the receipts a round did get makes a gap
in `sent_tx` reachable: a withdrawal whose lookup failed stays there while a
later nonce finalizes, which the assert that was removed ruled out. The
consequences are benign, but nothing pinned them - every existing
`record_finalized_transaction` test finalizes a single id.

Finalize nonce 1 while nonce 0 is still sent, and assert it neither panics nor
resubmits the straggler the chain has already passed, that a straggler the chain
has not passed is still considered for resubmission, and that finalizing the two
out of order leaves the same pipeline and the same balances as finalizing them in
order.

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

Addresses review comment M2. Two different receipts for the same id is the one
outcome that no chain can produce - one id maps to one nonce, so at most one of
its variants can be mined - yet it was surfaced only by a log while every benign
outcome had a counter, leaving operators unable to tell it apart from ordinary
provider failure: both merely show the window dropping to its floor.

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

Addresses review comments N3 and N7. `skip_round` both decided and incremented,
which reads as a query but is not one, and the call site bound its result to a
local named `window`, shadowing the accessor parameter of the same name.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Addresses review comments N1 and N4. The window bounds ids, not lookups: a
resubmitted id is looked up once per variant, so the real fan-out is a multiple
of it. And `rounds_without_reads` counts skipped rounds as well as the rounds
whose transaction-count read failed, which its help text attributed solely to
the failed read.

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

Addresses review comment N5. Nothing compares two outcomes; the window and its
counters, which are compared, keep theirs.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Addresses review comment N8. A window as wide as the pending set, taken from a
cursor in the middle of it, is the boundary where the chained iterator could
take an id twice: pin that it takes each exactly once.

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

Addresses review comment N9. The lookup counter was asserted for the withdrawal
pipeline only, unlike the other receipt-fetch metrics around it.

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

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.

Copilot review overview

🟡 Changes recommended

Abandoned rounds stop reducing completed lookup results, causing the new outcome counters to underreport.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

Bounds and adapts ckETH receipt-fetch fan-out while allowing successful withdrawals and sweeps to finalize independently.

Changes:

  • Adds adaptive per-pipeline receipt windows with fair cursor rotation.
  • Preserves successful receipts during partial failures and tracks stalled or conflicting results.
  • Adds unit, state-transition, and metrics coverage.
File Description
tests/​cketh.rs Verifies exported receipt-fetch metrics.
src/​withdraw/​tests.rs Tests partial receipt collection and skipping.
src/​withdraw.rs Implements shared bounded receipt fetching.
src/​sweep/​tests.rs Tests independent sweeper throttling.
src/​sweep/​mod.rs Adopts bounded fetching for sweeps.
src/​state/​tests.rs Tests out-of-order finalization.
src/​state/​receipt_fetch/​tests.rs Covers window adaptation and rotation.
src/​state/​receipt_fetch.rs Defines receipt-window state and counters.
src/​state.rs Adds per-pipeline soft state.
src/​main.rs Exports receipt-fetch metrics.
src/​lifecycle/​init.rs Initializes receipt-window state.

💡 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/withdraw.rs Outdated
Addresses the Copilot review comment on the conflict arm of
`collect_finalized_receipts`. Returning as soon as two receipts named the same
id left the results after it uncounted, even though every lookup of the round
had already come back: `cketh_minter_receipt_lookups_total` then understated
what the round actually asked the providers.

Walk the whole result vector, and return the empty receipt map afterwards. An
abandoned round still reports no stalled ids - those ids were answered and
thrown away, not left unanswered - and still counts as exactly one abandoned
round however many of its ids conflicted, since abandoning is a flag.

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

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.

Copilot review overview

🔵 Needs a closer look

It changes financial transaction finalization and outage-recovery behavior, warranting final human validation despite strong test coverage.

Review effort: Balanced
Findings: None

Resolved since last review (1)

The metrics help text ran to four and five lines each, far longer than anything
else in the file, and the change carried far more comment than the surrounding
code does. Cut each help string to one line under 80 characters, drop every
comment from the test modules - renaming the tests and helpers whose intent the
comment was carrying - and keep only the handful of production comments that
record a why: why an id whose transactions all answer "not mined" no longer
traps, why an abandoned round reports no stalled ids, why "not mined" must not
shrink the window, why rounds take whole ids, and why abandoning is a flag.

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

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.

Copilot review overview

🟡 Changes recommended

The new finalized-count backoff contradicts the stated receipt-only scope, and one metric description is inaccurate.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 Low severity

Open (1)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Remove or document finalized-count polling backoff

rs/​ethereum/​cketh/​minter/​src/​state/​receipt_fetch.rs:150

This introduces a fixed backoff after finalized-transaction-count failures (three of every four finalization rounds are skipped), which conflicts with the PR description's stated receipt-only scope and its claim that round backoff was dropped. It also changes polling of eth_getTransactionCount(Finalized), not just receipt fan-out. Either remove this skipping path or explicitly document and justify this additional operational behavior in the PR scope.

Comment thread rs/ethereum/cketh/minter/src/main.rs Outdated
mbjorkqvist and others added 2 commits September 22, 2026 06:54
The help text said the gauge counts rounds that made no receipt lookup,
but a round whose finalized-count read succeeds with nothing eligible to
fetch makes no lookup and resets it. It counts rounds that failed or
skipped that read.

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

rounds_without_reads read as "rounds that made no receipt lookup", which
describes the case that resets it rather than the cases that increment
it. It counts rounds since the last successful chain read, so say that.

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

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.

Copilot review overview

🟢 Approval recommended

The bounded polling behavior is consistently integrated and comprehensively tested without unresolved correctness issues.

Review effort: Balanced
Findings: None

Resolved since last review (1)

mbjorkqvist and others added 3 commits September 22, 2026 10:57
The multiplier only reordered ids from 7 upwards, and every selection
test uses ids below that, so hash order matched id order and a selection
that walked the hashes would have passed. Order the hashes backwards
instead, so the two can never agree.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
mbjorkqvist and others added 2 commits September 22, 2026 11:39
The branch that starts the skipping had no test: deleting its counter
bump disabled the mechanism silently. Reaching it needs the RPC client
to come from the runtime rather than from read_state, which is what
CanisterRuntime::evm_rpc_client is for, so take that route in the round.

The two skip tests now pass a runtime expecting no calls at all, so they
also pin that a skipped round reaches no provider.

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

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.

Copilot review overview

🔵 Needs a closer look

It changes financially sensitive asynchronous finalization across both transaction pipelines and warrants final human validation despite strong test coverage.

Review effort: Balanced
Findings: None

The skip tests only held because an unset mockall expectation panics, so
handing them a client would have quietly made them pass either way. Set
the expectation to never instead.

Also record why the skipping threshold exists, and why the two windows
are left out of is_equivalent_to as sweep_observations already says.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@mbjorkqvist
mbjorkqvist marked this pull request as ready for review September 22, 2026 13:11
@mbjorkqvist
mbjorkqvist requested a review from a team as a code owner September 22, 2026 13:11
@github-actions github-actions Bot added the @defi label Sep 22, 2026
@zeropath-ai

zeropath-ai Bot commented Sep 22, 2026

Copy link
Copy Markdown

No security or compliance issues detected. Reviewed everything up to cd3ba2a.

Security Overview
Detected Code Changes
Change Type Relevant files
Enhancement ► rs/ethereum/cketh/minter/src/lifecycle/init.rs
withdrawal_receipt_fetch: Default::default(),
sweeper_receipt_fetch: Default::default()
Enhancement ► rs/ethereum/cketh/minter/src/main.rs
Added Prometheus-like receipt fetch metrics and counters (cketh_minter_receipt_fetch_window, cketh_minter_receipt_fetch_rounds_since_chain_read, cketh_minter_receipt_lookups_total, cketh_minter_receipt_fetch_abandoned_rounds_total, cketh_minter_receipt_fetch_stalled_ids_total, and integration into HTTP request handling)
Enhancement ► rs/ethereum/cketh/minter/src/state.rs
Add withdrawal_receipt_fetch: ReceiptFetchWindow,
Add sweeper_receipt_fetch: ReceiptFetchWindow,
Add ReceiptFetchWindow import and modules for receipt_fetch
Enhancement ► rs/ethereum/cketh/minter/src/state/receipt_fetch.rs (new file)
Implement ReceiptFetchWindow and RoundOutcome logic, including window sizing, selection of next round, counters, and grouping by id
Enhancement ► rs/ethereum/cketh/minter/src/state/receipt_fetch/tests.rs (new file)
Tests for ReceiptFetchWindow and RoundOutcome behavior, window sizing, selection, and skipping logic
Enhancement ► rs/ethereum/cketh/minter/src/state/receipt_fetch/tests.rs (new tests content)
Enhancement ► rs/ethereum/cketh/minter/src/state/tests.rs
Initialize new fields withdrawal_receipt_fetch and sweeper_receipt_fetch in state setup
Enhancement ► rs/ethereum/cketh/minter/src/sweep/mod.rs
Update documentation to reflect fetch_receipts_for_round usage and remove fetch_finalized_receipts reference in docs
Enhancement ► rs/ethereum/cketh/minter/src/sweep/mod.rs (code adjustment)
Replace fetch_finalized_receipts usage with fetch_receipts_for_round in sweep finalize flow, adjust imports accordingly
Enhancement ► rs/ethereum/cketh/minter/src/sweep/tests.rs
Import fetch_receipts_for_round and receipt_fetch constants, prepare for new fetch flow tests
Enhancement ► rs/ethereum/cketh/minter/src/withdraw.rs
Add tests module, import ReceiptFetchWindow and RoundOutcome, integrate new fetch_receipts_for_round flow into finalize_transactions_batch and withdrawal receipt handling
Replace finalize flow to use fetch_receipts_for_round with withdrawal_receipt_fetch
Enhancement ► rs/ethereum/cketh/minter/src/withdraw.rs (new or moved test imports and integration for receipt_fetch)
Enhancement ► rs/ethereum/cketh/minter/src/withdraw/tests.rs (new file)
Tests for withdrawal receipt collection and finalization paths using new receipt fetch logic

…ered

An id whose lookups failed had no receipt either, so it was counted as
stalled. During a receipt outage that is every id of every round, which
made the counter a copy of the failure counter and buried the case it
exists for: a withdrawal the chain will never finalize.

Track the ids a provider failed to answer and leave them out. An id is
excluded on its own failures, so a stalled id still counts when some
unrelated id in the same round errored.

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

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.

Copilot review overview

🔵 Needs a closer look

The adaptive polling and finalization changes affect high-impact transaction-processing behavior and warrant final human review.

Review effort: Balanced
Findings: None

This branch has not been deployed

No deployments
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.

2 participants