feat(stats): read execution status from receipts, not hashes by bdchatham · Pull Request #69 · sei-protocol/sei-load

bdchatham · 2026-08-26T23:04:34Z

Stacked on #68. Review everything after the first commit; that one is #68's.

The tracker decided whether a transaction worked by looking for its hash in a
block. A hash does not carry execution status, so one state covered a
transaction that committed and one that failed and burned its gas. A run could
report a million accepted, near-perfect inclusion and a healthy p99 while every
transaction failed, and nothing in the output said so. This matters most where
a failure is the expected result. A contract call with a bad argument fails. A
precompile call from an unassociated account fails. sei-load is growing exactly
those workloads.

The block fetch becomes a receipt fetch, one request per block whatever the
block carries. Each matched transaction resolves to committed or reverted and
gets reported once, and the two states the tracker already produced now go
through that same reporter, so the metric and the collector cannot disagree
about a transaction.

The larger half of this change is what happens when the run cannot read a
block, which four reviewers converged on independently. status_unavailable
existed and said in its own doc comment that counting an unread block as
expired reports a chain problem where the truth is a measurement problem.
Nothing produced it. Three cases now do.

A fetch that errors is one. An empty array is another, and it used to arrive
in silence: a node that holds a block but has lost its receipt bodies returns
an empty list with no error, so a whole block of transactions aged out with no
log line and no metric. The third is a receipt that carries a post-state root
instead of a status, which says the transaction executed and does not say how
it ended.

Attribution works off a watermark. Each registry entry stamps the count of
unreadable blocks at the moment it was registered, and the reap compares that
against the count now. A higher count means a block that could have carried
this transaction was never read. A transaction registered after the hole still
expires normally, which is its own test.

The run now proves the endpoint answers before it starts. Only nodes in
fullNode or archive mode serve EVM HTTP at all, and without the probe a run
against a validator completes, reports every transaction un-included and exits
zero, which reads as a chain that accepted nothing.

receiptEndpoint lets the tracker read from a node other than the one it
loads. It defaults to endpoints[0], so a single-node run is unchanged. This
matters more than the request count does: a receipts read costs the serving
node work that grows with the block's transaction count, and a node that both
takes the send load and answers the read degrades in a way the tracker would
report as a chain result.

Two things about locking. The tracker holds a collector, and the reference runs
one way: the tracker may take the collector's lock, the collector must never
take the tracker's. Nothing takes both, and stats/doc.go now names the third
holder of the registry lock so nothing starts. Reports also land after the
registry lock is released, because the sender blocks on that lock at every send
completion.

sender/doc.go states the conservation identity over the terminal states
instead of the older three-term one, and corrects the reorg boundary: first
observation now fixes an execution status, not only a time. stats/doc.go
gains the Lifecycle and Ownership sections it had deferred to this change, and
the partition claim it documented now has a test.

I broke each new guard on purpose and checked that a test noticed: every
receipt treated as committed, the operation label dropped, a per-transaction
fetch put back, reaped transactions no longer reported, the reap attribution,
the registration watermark, the empty-array branch, the status-presence branch,
the preflight, the nil guard, the cap-drop report, and a transaction reported
twice. All twelve failed, then passed again once restored.

Gate: gofmt clean, go vet clean, golangci-lint 0 issues, go test ./...
across 14 packages, and go test -race on stats and sender.

Two things the review raised needed a decision rather than a fix, and both are
now made.

The execution-failure state is called reverted. It was failed, which
collided with RunSummary.Failed, a send that returned an error, so an operator
reading one series beside the other saw one word for two layers of the same run.
The label values are declared frozen and nothing in the platform repo binds them
yet, so this was the last moment the rename was free. succeeded was not
available as the other half of the pair, because OperationStats already
carries Successes for the send path.

eth_getBlockReceipts costs the serving node work quadratic in the block's
transaction count. encodeReceipt calls filterTransactions once per receipt,
and filterTransactions decodes every transaction in the block and recovers
every sender with no memoization, because AsTransaction builds a fresh
transaction each pass so go-ethereum's own sender cache never applies. Verified
against sei-chain 63b0fdf77. The request count did not move and the work on
the other end did.

That is a property of the node, and every caller of the method pays it. It is
filed as PLT-1082, where BlockCacheEntry already exists per height and gives
the memoization a home. This PR keeps the cost off the box it is loading by
pointing the tracker at a node that takes no send load. The cost does not
disappear: it bounds what the tracking node can keep up with, so both
config.ReceiptEndpoint and receiptSource say to measure against the target
chain before turning receipt tracking on.

Covers TOT-001, TOT-002, TOT-004, TOT-009, TOT-013, TOT-015, TOT-016, TOT-017,
TOT-020 and TOT-021.

PLT-1076

The tracker derived inclusion from the transaction hashes in a block, and a hash carries no execution status. One state therefore covered a transaction that committed and one that failed and burned its gas. A run could report a million accepted, near-perfect inclusion and a healthy p99 while every transaction failed, and nothing in the output said so. blockSource becomes receiptSource, backed by ethclient.BlockReceipts. One call per block either way, so the request count does not move: an earlier design fetched a receipt per transaction and its cost grew with the load the run offered, which is the constraint that shaped this one. blockReceipt is this package's own type rather than a go-ethereum receipt. A receipt carries eleven more fields the tracker has no business reading, and a test supplies a hash and a status without constructing one. matchBlock resolves each matched transaction to Committed or Failed and reports it, and the two existing outcome sites now route through the same reporter, so the metric and the collector stay in step. The tracker holds a collector. The reference runs one way: the tracker may take the collector's lock, the collector must never take the tracker's state lock. Nothing takes both, and the field comment is where that is written down. Reports land outside the registry lock. The sender blocks on it at every send completion, so work held under it lands in the latency this package reports. Guards proven by breaking what they cover: every receipt treated as committed, the operation label dropped, a per-transaction fetch reintroduced, reaped transactions no longer reported. Requirements: TOT-001, TOT-002, TOT-009, TOT-015, TOT-016, TOT-017. Tasks T007, T009, T010, T011, T012. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

cursor · 2026-08-26T23:04:40Z

PR Summary

Medium Risk
Changes inclusion measurement semantics and RPC topology for receipt tracking, but the path is gated on --track-receipts and is heavily covered by new tests including race runs on stats/sender.

Overview
With --track-receipts, inclusion tracking no longer infers success from tx hashes in blocks. It uses one eth_getBlockReceipts call per block and classifies each matched send as committed or reverted, feeding a new Outcome model and Collector.RecordOutcome (per scenario/operation counts plus inclusion_outcome metrics). Shutdown logging and RunSummary now include status_unavailable separately from expired.

Operational behavior is stricter about what the run is allowed to claim. Unreadable blocks, missed heads, tracker backlog, txs accepted before the first read, and a dead head stream map to status_unavailable instead of blaming the chain. Startup preflights the receipt RPC (ErrEndpointUnusable on failure), heads are stamped on arrival (not when the block is processed), and not_found reads retry within a bounded budget.

Config adds optional receiptEndpoint: heads and receipts must come from the same node (defaults to endpoints[0] with a warning). README documents sizing eth_getBlockReceipts against block width before enabling tracking.

^{Reviewed by Cursor Bugbot for commit 193f5fc. Bugbot is set up for automated code reviews on this repo. Configure here.}

seidroid

Solid, well-tested change: the tracker now reads execution status from one eth_getBlockReceipts call per block and routes every terminal outcome (committed/failed/expired/dropped-at-cap) through a single reporter outside the registry lock, with tests pinning both the status split and the O(blocks) request cost. No correctness bugs found; the notes are about the never-produced StatusUnavailable state on the fetch-error path, the report surface, and documentation the base branch explicitly deferred to this change.

Findings: 0 blocking | 5 non-blocking | 2 posted inline

Blockers

None at the file/PR level.

Non-blocking

[suggestion] The outcome counts land in the Collector and in the inclusion_outcome metric, but not in the run report: OperationReport/FinalStats (stats/logger.go) still carry only Count/Successes/latency, so the printed and JSON output still cannot distinguish an all-reverted run from an all-committed one. The PR's motivating scenario is fixed for metric consumers only — worth surfacing Committed/Failed/Expired in OperationReport here or naming the follow-up in the stack.
[suggestion] Documentation the base branch deferred to exactly this change is now stale: stats/outcome.go still opens with "Nothing reports an Outcome yet ... wiring it to Collector.RecordOutcome, and reading execution status from a per-block receipts call, both come later"; stats/collector.go's OperationStats says "All of them stay zero until a tracker reports outcomes"; stats/doc.go's "Not documented yet" section says the tracker loop that reports outcomes "does not exist"; and sender/doc.go still describes a block-body fetch, "A failed block-body fetch", and the older three-term identity that outcome.go says should be rewritten by this change.
[suggestion] stats/doc.go states that the claim "Outcome's states partition every accepted transaction" is "documented and not yet guarded" and that its test "belongs with the change that wires the tracker." The new tests cover per-path reporting but none asserts the partition across committed + failed + expired + dropped_at_cap against accepted, so the identity is still unguarded.
2 suggestion(s)/nit(s) flagged inline on specific lines.

seidroid · 2026-08-26T23:15:16Z

 	// Explicit per-iteration cancel (not deferred-in-loop): bound the fetch.
 	fetchCtx, cancel := context.WithTimeout(ctx, 10*time.Second)
-	hashes, err := t.source.BlockTxHashes(fetchCtx, num)
+	receipts, err := t.source.BlockReceipts(fetchCtx, num)


[suggestion] A failed receipts fetch still drops the whole block silently, and its transactions reap as Expired — the state the package documents as "the chain did not take it" — even though the truth is that the run could not see. OutcomeStatusUnavailable exists in outcome.go for precisely this case and no code path ever produces it.

The fetch-error behaviour matches the pre-PR BlockByNumber path, so this is not a regression on its own, but the switch widens the exposure: eth_getBlockReceipts is a narrower dependency than eth_getBlockByNumber (endpoint support, and a receipt index that can lag the newHead that triggered the fetch), and there is no retry. A run against an endpoint that does not serve the method would report 100% expired with only a log line and inclusion_block_fetch_errors to explain it.

A fallback BlockByNumber on the error path would recover the hash list and let those matches report OutcomeStatusUnavailable; it costs one extra call per failed block, so the O(blocks) constraint the new test guards still holds. (Codex raised this as High; downgrading since the pre-existing path behaves identically and the failure is at least counted.)

seidroid · 2026-08-26T23:15:16Z

+	// this tracker's state lock. Nothing takes both today, and this is where the
+	// rule is written down so nothing starts.
+	//
+	// Nil when a run keeps no collector, which the tests do.


[suggestion] This comment is already false in this PR: newTestTrackerLoop now passes NewCollector(), and main.go always passes the run's collector, so nothing constructs a tracker with a nil collector. The nil guard in report is therefore unexercised by any test and unreachable in production. Either drop the nil-ability and document the collector as required, or fix the comment to say why nil must stay supported.

Four independent reviewers read the receipts change. Every one of them found the same hole: OutcomeStatusUnavailable was defined, documented as the state that keeps a measurement problem from being reported as a chain problem, and had no producer. A receipt read that failed left its block's transactions to age out as expired, which is a claim about the chain that the run had no grounds to make. The registry now records a watermark. Each entry stamps the count of unreadable blocks at registration; a reap compares it against the count now. A higher count means a block that could have carried this transaction was never read, so the transaction reaches status_unavailable instead of expired. A transaction registered after the hole still expires normally. An empty array is one of those unreadable answers and used to arrive silently. A node that holds a block but has lost its receipt bodies returns an empty list with no error, so a whole block of transactions aged out with no log line and no metric. It now takes the same path as an error, and block_fetch_errors carries a reason so an operator reads the cause off a dashboard rather than the pod log. A receipt carrying a post-state root instead of a status says the transaction executed and does not say how it ended. Reading that as a failure would invent a chain result, so blockReceipt carries whether the status was there. Run proves the endpoint answers before the run starts. A node in validator or seed mode serves no EVM HTTP at all, and without the probe such a run completes, reports every transaction un-included and exits zero, which reads as a chain that accepted nothing. The tracker can now read receipts from a node other than the one it loads. receiptEndpoint defaults to Endpoints[0], so a single-node run is unchanged, and a run that names a second node keeps the read work off the box under load. That matters more than the request count: a receipts read costs the serving node work that grows with the block's transaction count. An endpoint decides what it puts in a receipts array. A null element would have panicked the head loop, and nothing recovers there, so the run would have died and lost every result it had gathered. sender/doc.go states the conservation identity over the terminal states rather than the older three-term one, and corrects the reorg boundary: first observation now fixes an execution status, not only a time. stats/doc.go gains the Lifecycle and Ownership sections it deferred to this change, and the partition claim it documented now has a test. Also: context threads through the report path instead of being dropped; recordOutcome becomes meterOutcome, which is what it does; the two metric descriptions that contradicted their own series are rewritten; the collector is required rather than nil-checked, since no caller passed nil; and the comments that narrated this change rather than stating the present are gone. Guards proven by breaking what they cover: the reap attribution, the registration watermark, the empty-array branch, the status-presence branch, the preflight, the nil guard, the cap-drop report, and a double report. Requirements: TOT-004, TOT-013, TOT-020, TOT-021. Tasks T008, T013. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…a read costs Two decisions the review surfaced and could not make for itself. An operator reading seiload_inclusion_outcome_total{outcome="failed"} beside seiload_run_txs_failed_total sees one word for two layers of the same run. One is a transaction the chain took and ran to no effect, the other is a send that returned an error. reverted is what the EVM calls the first, and it does not collide. succeeded was not available as the other half of the pair, because OperationStats already carries Successes for the send path. The label values are declared frozen, and this is the last moment the rename is free: nothing in the platform repo binds them yet. The second decision stands rather than changes the code. eth_getBlockReceipts costs the node answering it work that grows with the square of the block's transaction count, because it resolves each receipt's index by walking the whole block and recovers every sender again while doing so. That is a property of the node, not of this change, and every caller of the method pays it. The run keeps that cost off the box it is loading by pointing the tracker at a node that takes no send load, which is what receiptEndpoint is for. The cost does not disappear: it bounds what the tracking node can keep up with. Both config.ReceiptEndpoint and receiptSource now say so, and say to measure against the target chain before turning receipt tracking on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Round 2 found that the previous commit's own fix was worse than the bug it removed. Three lenses reached it separately and one measured it: 80 of 100 consecutive Sei mainnet blocks return an empty receipts array. Treating that as a hole marked almost every block. Because the marker only ever grows and a transaction inherits it for its whole time in flight, one idle block in a 30-second window converted the entire registry, and expired stopped being reachable at all. The case the tool exists to detect makes it worse rather than better: a chain that stopped accepting work produces nothing but idle blocks, so the run would have reported "I could not see" for the one run where "the chain took nothing" is the answer. The premise behind that branch was wrong. sei-chain already separates the three answers on the wire. A block it cannot see returns null, which arrives as ethereum.NotFound. Pruned receipts return an error. An empty array means the block carried no EVM transaction, and it is the truthful answer. So the branch is gone rather than made conditional, and an array of nothing but nulls, which is a node answering nothing at all, becomes an error where it is read. The registry counters now split the way the outcomes do. They reached the operator through the closing log line while the outcome ledger reached the same operator through the metric, so one transaction was expired on the surface read first and status_unavailable on the surface read second. The preflight failed on the one case a Sei node never produces and passed on every case it does. A node serving no EVM HTTP refuses the connection, which classified as other and let the run start blind, and that is the case the doc comment named first. A gateway that filters methods answers with an HTTP status carrying the JSON-RPC code in a body the decoder never reads. Both refuse the run now. Classification leads with the typed checks that hold across servers, and the two substring tests that cannot be typed are ordered so pruning is tested before availability, because Sei's two messages differ by one word. The abort message named --receipt-endpoint. There is no such flag: the setting is receiptEndpoint in the profile. The one error allowed to end a run told the operator to use a control that does not exist, and a test now fails on the flag spelling. Register takes the context its caller already holds. The comment saying the caller had none was false; the signature declined it. A second registration of a hash already in flight overwrote the first, so two accepted transactions shared one terminal state. It is counted now. The partition guard covered three of seven states and passed with the whole reap report loop deleted. It exercises every reachable state, asserts each leg, and checks that the registry counters and the outcome ledger describe the same transactions. sender/doc.go is the file stats/doc.go nominates as owning the conservation identity, and it still stated it with the retired word and with registered on the left where dropped_at_cap sits on the right. The two shorter restatements elsewhere are replaced by a pointer to it. HasStatus detects a post-state-root receipt and no other shape. go-ethereum makes both fields optional, so a receipt carrying neither is indistinguishable from a failure after decoding, and the comment says so rather than implying a guarantee. Also: narrow becomes a plain function, matching every other helper in the package; the cost claim names seid rather than the method, because upstream go-ethereum is linear and the quadratic belongs to the implementation being measured; the nolint directive naming a linter this repo does not run is gone while its explanation stays; the fallback to the load endpoint warns; and the README documents receiptEndpoint. Guards proven by breaking what they cover: the idle block, the split counters, the duplicate registration, the unreachable endpoint, and the all-null array. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Shipping receiptEndpoint is what makes this reachable, so it belongs with it. Two reviewers named the same sequence: heads arrive from the load node, the receipt node has not committed that height yet, and it answers null. That arrives as ethereum.NotFound, which counted as a hole with no retry, so a receipt node trailing by one block produced a hole every block and expired became unreachable again. The defect this PR just removed, reintroduced through the topology the PR recommends. A height the node has not reached is now re-read on the next head. One re-read is the bound: a node still behind a block interval later is behind rather than busy, and that is a hole worth counting. This is TOT-020 pulled forward from phase 3c, for the same reason the unreadable block attribution came forward from 3b. Leaving it out means merging a change whose recommended configuration breaks it. Guards proven by breaking what they cover: a lagging node written off on first sight, and a node behind forever retried forever. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…shutdown Round 3 reviewed the re-read that round 2's fix needed, and found four defects in it. Both lenses reached the first two independently. The re-read stamped the transaction with the arrival of the head that triggered it, not the head the block actually arrived on. That arrival becomes the inclusion latency sample, and in the two-node topology this tracker recommends every height is deferred at least once, so the error landed on every sample rather than a few. One block interval against a histogram whose first bucket is half a second. The pending queue carries its own arrival now. A height still waiting when the heads stopped was dropped, and its transactions reaped as expired: a claim about the chain for a block nothing ever read. That is the defect round 2 existed to remove, through a new path. The queue drains whatever ends the head loop. A skipped head was never counted either. sender/doc.go called that an undercount rather than a miscount, which was true when the tracker only counted inclusions and stopped being true when expired became a claim about the chain. Every height in a gap is counted now. The retry bookkeeping was a map that only deleted on the failure path, so a node that caught up left an entry per block for the life of the run. One queue carrying a try count replaces it, bounded in both directions, which also raises the budget past one block: a node two behind used to produce no inclusion data at all. The hardened classifier refused a healthy run three ways. A connection reset is what a busy node does to a caller and read as unreachable. A rate-limited response whose request id happened to contain those six digits read as the method being absent, because the body was searched without regard to the status. A bare 404 from an ingress mid-reconcile read as nothing listening. Refusing a healthy run is worse than the blind run the refusal exists to prevent, so unreachable now means a refused dial or a name that does not resolve, the body is read only under a status that means refusal, and the preflight tries three times before it speaks for the whole run. An endpoint that answers nothing at all across all three is also a refusal, which is what a dropped route looks like. The typed checks the last commit added had no test. Every one could be deleted with the suite still green, because the tests drove error strings the substring fallback caught anyway. They are covered now, by construction rather than by text. Two more from the same review. The empty-array premise was wrong a second time, in the other direction. sei-chain swallows a per-hash receipt lookup that comes back not-found and compacts the slot out, so an empty array can also mean the block's transactions existed and their receipts were gone. Restoring the branch is not the answer: reading an empty array as a hole is what made expired unreachable on 80% of real blocks. The head's gas comes from the consensus result rather than from any receipt, so gas burned with no receipt returned is the one witness that the two cases differ, and it is counted rather than acted on. Gas covers Cosmos transactions too, so treating it as a hole would invent one on any chain carrying non-EVM traffic. Any null element in the array is an error now, not only an array of nothing but nulls. The run cannot see what that element was going to say either way. A re-read gets a shorter budget than a first read. Head processing is serial and the node drops a subscription whose head buffer fills, which ends the run, so two full-length reads in one head cost more than that affords. The head channel is buffered for the same reason. Guards proven by breaking what they cover: the arrival stamp, the queue drain on shutdown, the skipped head, the duplicate leg on both surfaces, the queue draining after a read, a reset read as unreachable, and a rate-limit status allowed to speak for the method. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Found by reading my own round-3 fix rather than by review. Treating any null element as a failed read threw away the receipts the endpoint did send, so a transaction that committed and whose receipt arrived got nothing and later reaped as unattributable. That trades a known outcome for an unknown one. narrowReceipts reports how many elements were null instead of refusing the array, and receiptSource says so in its signature, because a read that partly succeeded is a real answer and the interface should be able to express it. The block is a hole for the transactions the missing part would have named and not for the ones it named. Guards proven by breaking what they cover, in both directions: discarding what arrived, and hiding the loss. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The worst defect in this series, and it was mine from the last commit. The preflight's refusal wrapped the probe's own error with %w, and main treats a context.Canceled or a context.DeadlineExceeded as the run ending normally. So a run refused for an endpoint that never answered exited zero: a Complete run that carried no load, which the nightly harness then reports as a chain that accepted work and included none of it. The exact false accusation this whole tracker exists to remove, reintroduced through the path meant to prevent it. Refusal now carries its own sentinel, and a test asserts that no refusal reads as the run's context ending. The rest of what round 4 found, in the order it matters. The two read budgets were backwards. In the two-node topology this tracker recommends, the first read of a height returns not-found cheaply and the re-read is the one that carries the receipts, so the short budget landed on the only read that mattered. Measured against pacific-1, two of five idle-block reads already exceeded three seconds. One budget for both. The deferral bound counted heads, which made the failure a cliff: at four heads of lag every block read, at five every block became a hole and expired went unreachable for the whole run. It is a duration now, which is the quantity that actually matters, and the wait is recorded as a histogram so the drift is visible before it is crossed rather than after. The preflight kept only the last attempt's verdict, so two timeouts could erase an earlier answer that proved the endpoint was there, and refuse the run on evidence contradicting its own message. A refusal now needs every attempt to agree on one cause. Any DNS error refused the run. A resolver answering SERVFAIL is temporary, and only a name that does not exist is permanent. An endpoint answering prose rather than JSON stopped being refused when I narrowed the classifier last round. That is the common operator typo: the metrics port, the Cosmos RPC port, an ingress default backend. It is as durable a failure as a refused dial, and it refuses again. The summary's terms overlapped. A receipt whose status could not be read counted in both included and status_unavailable, so adding up the closing log line gave more than the run accepted. included now means the readable ones, and the terms are disjoint. A gap logged one line per height, so a fifty-height gap pushed the run's own summary out of the fifty-line log tail that is the only diagnostic a failed nightly carries. One record per gap, which changes nothing about attribution because the reap only asks whether the count rose. The shutdown sweep marked holes it could not justify. The head loop and the reap loop end on the same signal, so no reap follows it and those transactions are already counted as in flight at shutdown. Marking a hole put a failure on the series that answers "was this run blind?" at the end of every healthy run. Four causes shared one reason label. A node behind the head, a head never seen, a height out of budget, and a receipt the node could not produce now have their own, because the operator's next move differs for each. One reviewer finding I did not take. It measured that Sei's block gas is EVM-only, concluded block_empty_with_gas is clean signal, and asked me to act on it. That measurement was of eth_getBlockByNumber, which sums receipt.GasUsed and would be circular here. The newHeads header this code reads sums every transaction's consensus result, Cosmos included, so the counter stays observed rather than acted on. The comment now names which header, and names sei-chain's own TODO to change it, because the ambiguity misled a careful reader. Guards proven by breaking what they cover: the refusal's sentinel, the non-JSON refusal, agreement across attempts, a resolver blip, the disjoint summary terms, one record per gap, and a healthy shutdown marking no hole. Method note: two mutations in this round reported as surviving when they were really vet failures, and one survived because the fake fell through to success. The battery checks vet and drives every attempt now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Found by reading my own last commit. Making the deferral bound a duration raised how many heights can be waiting at once from four to about a dozen in steady state, and the sweep gave each one a full read budget. Head processing is serial, so one hanging node could spend minutes inside a single head while the chain moved on. The sweep shares one budget now. A height it does not reach stays queued for the next head, which costs a head of delay rather than a hole, and the requeue puts the oldest first so nothing is starved. Guard proven by lifting the budget: the sweep then ran to the full length of every queued read. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

An idiom review of the file as it now stands, rather than of the last delta. Five rounds of defect-driven fixes is exactly how a file accumulates incoherence, and it had. The retry count was dead. Round 5 replaced a count-based retirement with a duration and left the field written, passed along, and never compared, plus four comments still describing the mechanism it belonged to. An editor tuning re-read depth would have changed a constant that moves nothing. Removing it made a second fix obvious. matchBlockAttempt and deferHeight took four and five positional parameters carrying most of deferredRead's fields, two of them adjacent same-typed timestamps. Transposing arrival and deferredAt compiles, and it would have measured the inclusion latency and the retry budget from each other's instant. They take the struct now, so a first read passes no hand-written zero values at all. The take-and-clear critical section was duplicated verbatim at two sites that must change together. takePending owns it again. blindFetches counted heads that never arrived as well as reads that failed, so neither its name nor its doc was true. It is blindHeights. deferred_read_wait labelled its values outcome, which inclusion_outcome already uses for a disjoint set. One label name meaning two things across two instruments is the wire hazard Outcome's own type exists to prevent, and the file states that rule thirty lines above where it broke it. The label is disposition, and its three values have constants. requeue dropped a height past the cap silently. deferHeight already holds the queue at the cap so nothing reaches that branch, but a height vanishing from it would leave blindHeights unmoved and let a transaction from that block reap as a verdict about the chain. It records instead. block_gaps described the arithmetic of a different counter: it adds one per missed height, while the once-per-gap record lands on block_fetch_errors. Comment discipline, with the line drawn where the reviewer drew it. A present fact about the deployment shape stays, because it is the constraint that makes a shorter re-read budget wrong. The argument with the version that had it backwards goes, because that is the commit's job and not the code's. Same treatment for the conservation identity's history and for the empty-array incident. Two blocks that explain a non-obvious external API stay untouched. Also: flushDeferred promised a flush and performed an abandon, so it is recordUnreadAtShutdown, and the call site that still argued the behaviour it no longer has is gone. refusesTheRun reads as though the reason refuses; it is isPermanent. reasonNotJSON sat under a comment saying it was not a call failure. The DNS rationale had been appended to the paragraph about connection resets. Two nolint directives named a linter this repo does not enable, and their reasoning survives as plain comments. stats/doc.go gains the fourth sentinel, the queue's bounds under the lock it lives behind, the lifecycle branch where a preflight refuses a run, and the two tests that guard the queue. sender/doc.go gains the sweep bound and the shutdown boundary. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The commit titled "bound what one head spends re-reading" did not. The check ran before a read and never bounded the read in progress, so a read starting at 2.999s ran a further ten. Measured 12.8s against a stated 3s, and a worst case of 23s where the shape it replaced was 18s. The bound moved the wrong way in the commit named for fixing it. That is not only a cost. The head loop stamps arrival at dequeue, and that value becomes the inclusion latency sample, so a sweep that overruns writes the tracker's own backlog into the number the run exists to report. A re-read now gets whatever is left of the sweep. Which exposed the next thing: a read this process cut short is not evidence about the node, so that height goes back in the queue rather than being called a hole. Blaming the serving node for a deadline sei-load imposed on itself is the same error as blaming the chain for a block the run never read. requeue put the unreached tail in front of what the sweep had already re-deferred and its comment claimed the opposite. A sweep walks oldest first, so what it re-deferred is older; prepending served the newest first and let the oldest age out against a budget they were never given a turn under. The refusal reasons were two lists that had to agree: one deciding whether a reason ends the run, one turning it into a message. Adding a reason to one and not the other gave either a silent stall through the retry loop or a refusal nothing could reach, with no signal from the compiler or a test. One table now. block_fetch_errors counted a gap once while every other reason counted per height, so an operator summing it undercounted by the length of every gap. The watermark still rises once, because the reap only asks whether it rose. Six guards were missing and one was worse than missing: deleting requeue outright left the suite green while heights vanished, because the assertion only asked whether the queue was non-empty and the heights the sweep did read had refilled it. The whole duration-budget mechanism had no test at all, so it could be disabled, set to five hundred hours, or restarted on every read without a failure. Each now has a guard, proven by breaking what it covers. Also: the latency histogram's count is no longer the included count, since a matched receipt with an unreadable status still samples, and the comment said otherwise. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The strongest finding of the review, and the one every earlier round walked past. It was measured: 565 of 1000 transactions reported expired on a chain that included every one of them, with every hole counter reading zero. No read fails. No head is missed. No receipt is null. The tracker's head loop is serial, so when reads take longer than the block interval a backlog builds, and the block carrying a transaction is opened after the reap already evicted it. Five rounds hardened every path where a read fails; this is the path where every read succeeds and the tracker is late. sender/doc.go says expired is a claim about the chain and the run has no grounds for one about a block it did not read. At reap time the run had that block in hand and had not opened it. The registry now knows the highest head taken off the wire and the highest whose block has been read. A reap that finds them apart cannot say expired, because the transaction may be sitting in a height the run is holding. A caught-up run still says expired, which is the point of keeping the two states apart, and that direction has its own test. The same backlog corrupted the number this tool exists to report. Arrival was stamped after the head came off the channel, so every latency sample carried the queue. Measured at 8.2 seconds of error after 16 seconds of chain at 1.5x block time, and 43 seconds at 3.75x, against a histogram whose top bucket is 120. The stamp is taken where the head arrives now, by a step that exists to keep it there, and head_lag reports the gap. That is the number that separates a chain taking nothing from a run that could not keep up: both show un-included transactions, and only this one says which. A head stream that ends mid-run no longer fails the run. Any error from the tracker cancelled the whole scope and exited non-zero, so a dropped WebSocket on a read-only observer turned a good run red and killed the senders, the generator and the report with it. That is the mirror of the refused-run-exits-zero defect fixed earlier: this one is the false fail. Tracking stops, every later height is unread so nothing after it reaps as a chain verdict, and the run finishes. Also: the WebSocket client was never closed. Guards proven by breaking what they cover: the backlog rule in both directions, a dead head stream, a head never counted as received, a head counted only after its block was read, and a stamp taken late. The stamping needed the pump extracted to be testable at all. A guard that drives processHead directly cannot see where Run takes its timestamp, and that is the third time in this series a guard has named something it did not check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Three correctness findings, all mine from the last two commits, all measured by the reviewer and reproduced here. The sweep budget was three seconds and one read's budget was ten, so capping a re-read at whatever the sweep had left capped every re-read at three. A node answering in four seconds, well inside its own budget, never finished one: cut short, requeued, cut short again, then retired as though the node were behind. In the topology where every height is deferred that is every height in the run, and expired becomes unreachable. A sweep budget is now not smaller than a read's, and a test pins that relationship rather than the two numbers. The same class one layer up, found by a test I wrote for something else. The wait budget was shorter than a sweep, so a sweep could outlive it and retire its own tail: heights called receipt_node_behind for time this process spent on the heights ahead of them. The wait budget is now twice the sweep, and that is pinned too. Ordering the requeue by age starved the queue. A sweep walks front to back, so what it re-deferred got a turn and the tail did not, and putting the tail behind them meant the same front entries consumed every sweep. Measured: three queued heights, a node holding only the third, and the third retired without the run ever completing a read of it. Order here is service fairness, and the earlier change to age order was a mistake I took from a review without testing what it cost. A reap could still claim the chain while a height sat queued. The previous commit compared heads received against heads read, which a queued height passes: it was received and read once, the node did not have it, and the run is waiting to ask again. A transaction may be in it. Two guards were vacuous. One asserted a reap outcome on a tracker whose reap window was a minute, so nothing was ever old enough to reap; making it real showed the defect above. One asserted a sweep's bound with a source whose delay scaled with the constant under test, so shrinking that constant kept the test green. The shutdown sweep now raises the attribution watermark without emitting a failure. Those heights genuinely went unread, so nothing reaped afterwards may speak for the chain whatever order shutdown runs in; but a healthy run draining a queue it was always going to drain is not the run going blind, and the operator-facing counter should not say it was. Also: a single missed head went unrecorded, since the gap tests used two and forty-nine. And the guarantee that the first entry of a sweep gets a full read was dead code once the budgets were ordered correctly. Eleven guards proven by breaking what they cover. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

A traceability review, the first anyone has run on this work, and it found what six rounds of correctness review could not: the code is careful and it does not match its specification. TOT-022 is a MUST. The run takes its head signal from the same node it reads status from, because a head carries the raw committed height while a status read resolves through a watermark behind it. The two disagree even on one node, and taking them from separate nodes adds peer lag on top, which a node inside its readiness threshold carries for minutes without reporting unhealthy. The requirement's own verification row names the failure as "the head signal and the status read come from different nodes". That is what this code did, with a comment justifying it on cost grounds, answering a question the requirement did not ask. The cost of that violation is most of the machinery built since. The deferred read queue exists because heights arrived before the reading node held them, which is the condition TOT-022 forbids creating. It stays for now, because one node still disagrees with itself across the watermark, but it should be rare rather than constant. And it had made expired unreachable a second time. The reap refuses a chain verdict while a height sits queued, which is right, and it is only safe because the queue is normally empty. Split the nodes and the queue never empties, so a run against a healthy chain that took nothing reports "I could not see". That is the defect of two rounds ago, reintroduced through the topology this change recommended. A test now drives the single-node steady state and asserts expired is reachable. TOT-023 says the run must not read a block older than the deadline it gives a transaction to reach one. The read budget was a fixed twenty seconds while the deadline is operator-configurable, so a run reaping at five seconds re-read heights four times past its own bound. The budget answers to the deadline now. The traceability mechanism was absent rather than incomplete. One requirement ID appeared in the whole test suite, in a comment, from the previous PR, while the repo already does this properly for another feature. Forty-two guards now name what they cover. Eight requirements are cited by nothing, and that is the point of doing it: TOT-005, 007, 012 and 019 are the report, TOT-010, 014 and 018 are the hand-off channel, and all seven belong to later phases. TOT-013 is contradicted rather than deferred, and it is called out below. Also: sender/doc.go stated the identity with six terms where the data model has seven; the README advertised a report carrying committed and reverted, which it does not; and the duplicate-registration collision is filed under status_unavailable, which is the closest state the spec defines and not a clean fit, now said out loud where it happens. Open against the spec and not fixed here: the preflight refuses a run, and the spec's design section says nothing in this feature fails a run. Refusing is probably better than a silently blind run, but it is an unrecorded amendment, and it makes TOT-013 and SC-008 unreachable by construction. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Roughly 750 lines out, 110 in. The tracker drops from 1219 to 884 lines, its tests from 1224 to 784, and the suite from 20 seconds to 11. The deferred-read queue, its sweep budget, its requeue, its starvation rule, its three interacting constants, its shutdown drain and its wait budget derived from reapAfter all existed for one reason: heads arrived from a node that was not the one being read, so a height was routinely announced before the reader held it. Fixing that violation removed the cause and left the apparatus. The cause is not entirely gone, and that is why a retry stays. seid publishes a head from Commit while the receipt store's writer is still asynchronous, so a height answers null for the gap between the two. Our nodes run the pebbledb receipt store with an async write buffer of a hundred, set in the fleet's own defaults, so the window is real and bounded by that queue rather than by the chain. It is one write, not one block, so waiting in place resolves it in milliseconds where the queue waited for the next head. Deleting the retry as well was the tempting move and it is wrong. A hole raises a watermark that only grows, so one hole anywhere inside a transaction's reap window converts it. At a null rate of five percent, every transaction in a run reports status_unavailable and expired never fires: the tool loses the one verdict it exists to deliver, and a run that can never say the chain dropped anything is not a load test. The preflight loses its refusal table and most of its classifier. The probe asks for height 0, which sei-chain answers from a constant ahead of its watermark and its receipt store, so a healthy EVM RPC cannot fail it and the reason for a failure cannot change the verdict. That also closes a hole the larger version had: an endpoint answering not-found to genesis is not a Sei EVM RPC at all, and it used to be admitted. deferred_read_wait becomes block_read_wait and loses its disposition label. It now measures one thing, which is how far a node's watermark trails its own head, and that is the number nobody has and the one that says whether even the retry earns its place. Nothing in the platform repo reads either name. Fourteen tests lost their subject with the code they covered. Three guards replace them: the gap is waited out rather than written off, the retry gives up rather than holding the head loop, and expired stays reachable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…d it Run had no coverage. Every ordering fact seven rounds of fixes depend on — the pump stamping a head's arrival, the head loop reading its block, the reap deciding what to call a transaction, a dead subscription ending the tracking — was exercised only by helpers called by hand. That is why the two worst defects in this change survived seven rounds of review, and why four of the guards written along the way turned out to assert nothing. The harness is an in-process WebSocket server speaking eth_subscribe("newHeads"), so a test drives the real subscription, the real client and the real loop. It earned its keep immediately: the first version sent a header the go-ethereum decoder rejects, and the run reported the subscription dying rather than the header being wrong. Coverage of Run goes from nothing to 77%. The first defect it exposes fires on every run. The senders and the tracker start together, and the tracker dials and probes before it reads a block, so every run accepts transactions during a window in which it is observing nothing. Those transactions land in blocks the run never opened, and reaped as expired: a claim about the chain drawn from a period the run did not watch. The registry records when the run first read a block, and a reap will not speak for the chain about anything accepted before that. The second is what a dead subscription does. The reap loop ends with the head loop, so nothing reaps afterwards, and the registry kept filling from senders that were still working until everything reported dropped_at_cap. An operator reads that as a cap to raise rather than a subscription that died. Tracking stopping now settles everything in flight, and a later registration is answered rather than stored. That made the reap's own trackingStopped arm unreachable, which two reviews had already called dead for a different reason. It is gone; Register is the only reader now, and the field says so. The registry's conservation identity omitted status_unavailable, so it could not see a transaction migrating into that term. It has five terms now. Guards proven by breaking what they cover: the first-read watermark and its placement, the drain, the registration path after tracking stops, and the reap arms in both directions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Round eight found that both could be disabled with the suite green, and that is the over-reporting direction: every un-matched transaction in every run reports status_unavailable, and nothing says so. One transaction cannot catch it. The first-read watermark and marking a head resolved both move a single transaction's verdict the same way, so a test with one subject cannot tell a working watermark from a dead one. The guard drives Run with two: one accepted before the run read anything, which must not be blamed on the chain, and one accepted after and never included, which must be. A third covers a subscription that is up and has delivered no head, where every other watermark reads as healthy because the run is trivially caught up with the nothing it has seen. The drain reached the metric ledger and not the closing log line, and no test compared them. That is one transaction counted under two names, which the reap path has been guarded against for several rounds and the drain path had not. An ingress error page was retried three hundred and eighty-five times over ten seconds. rpc.HTTPError renders as "404 Not Found: 404 page not found", so the substring check read it as a height the node had not reached yet. Deleting the classifier's typed checks left that substring first to match. The typed checks are back above it, and a table pins which reasons the read retries: only the one that means the node will answer differently in a moment. The package doc still described the deleted queue, named four tests that no longer exist, and said two goroutines drive the run where there are three. It also claimed everything in flight at shutdown is inflight_at_shutdown, which stopped being true when a dead stream started settling them. Guards proven by breaking what they cover: the watermark unset, its arm removed, heads never resolved, the drain's counter, and the error page. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

cursor

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

There are 3 total unresolved issues (including 2 from previous reviews).

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit a44f34c. Configure here.}

…eipts-replace-hashes

seidroid Bot approved these changes Aug 26, 2026

View reviewed changes

cursor Bot reviewed Aug 26, 2026

View reviewed changes

Comment thread stats/inclusion_tracker.go Outdated

bdchatham and others added 3 commits August 26, 2026 16:42