sequencer, rawdb, ethapi: audit the store for the window a node did not watch

[pratikspatil024]

Summary

Picks up the component deferred from #2373 (review) — "back track the sequence store if the node is just started" — so that a node which was down for an hour can identify the invalid preconfirmations from while it was down.

bor_getInvalidPreconfBlocks reported that window as clean regardless of what the store held during it. Invalidation records come from one place, the in-memory PendingStore reconciled against canonical blocks, and a node that was down holds no entries for the heights it missed — so nothing was compared and nothing recorded. Silence was indistinguishable from "every preconfirmation in that window held". Nothing recorded where the consumer last watched, either: resumeRequest cold-starts at the local canonical head, and the store's caught-up-to-tip Live frame was received and discarded rather than ending a catch-up phase.

An audit pass (eth/sequencer/audit.go) walks from the watermark to the head and compares the store's final sealed header at each height against the canonical hash:

• A mismatch is recorded as unobserved_mismatch — the store sealed something that never became canonical, and this node served nothing from it. The existing reasons (canonical_mismatch, reorged, skipped, session_lost) all mean a preconfirmation reached callers and was then invalidated, which is a stronger claim, so this gets its own reason rather than sharing theirs. The write is conditional on absence: a height the live path already judged keeps that record, since a backfill must not overwrite an observation.
• A height the store never held, or left open, records nothing — nothing was promised there — but still clears the watermark, or the same range is re-walked forever.
• The comparison needs no execution and no historic state, which is what makes it viable on a pruned node. It runs on its own connection off the session loop, so closing a gap never delays a reconnect and the node keeps serving preconfirmations at the tip while the walk runs.

Triggers: consumer start, every session loss, and reaching the store tip. Session loss earns a pass because a stream that drops while the node stays up leaves the same class of hole as a restart — which also makes this cover gaps from store outages, not just node restarts.

Position (core/rawdb): PreconfAuditedThrough carries the watermark across restarts, and is the only mark. It may only step one height at a time from the live path — a jump would carry it over the catch-up backlog the session dropped without comparing, so a gap requests a pass instead. A node with no watermark seeds at the current head and audits nothing; auditing backwards from an arbitrary point on first enable would produce records with no operational meaning.

bor_getInvalidPreconfBlocks carries the audit's coverage of the range alongside the heights, so no second call is needed to interpret an empty result:

// bor_getInvalidPreconfBlocks("0x5", "0x60")
{ "invalid": ["0x9", "0x6"], "pendingFrom": "0x4e" }

pendingFrom is max(from, auditedThrough + 1): heights from there to the range end are pending rather than clean, null once the whole range is audited, and equal to from on a node that has not audited at all. Every other height in range was compared and matched. The range is capped at 1024 heights and a wider one is an error, which is what makes truncation unnecessary — one record per height means bounding the request bounds the response, so a caller can no longer receive a silently short answer it cannot tell from a complete one. An unreadable watermark is an RPC error, never a null pendingFrom, because null is the claim the range was compared.

Bounded by the store, not by a block count. The walk runs from auditedThrough + 1 to the head, and the store's retention is the only ceiling. A Range request with after unset resolves to the oldest entry the store still serves; the first BlockOpen at or after it is the oldest height a pass can compare, and the walk starts there rather than paying one NOT_FOUND per aged-out height. The skipped range is logged and counted (sequencer/audit/retentionskipped), as is any NOT_FOUND mid-walk (sequencer/audit/unheld).

Retention can age out the middle of a block, so the oldest retained entry is not always an open. A seal reached before any open closes a block whose earlier entries are gone — a partial record list is not comparable against a canonical block — so the floor is the height above it; records carry no height at all and the scan pages on. Failing to resolve a floor is not a correctness boundary: the walk then discovers the same heights unheld one at a time, which is the pre-existing behaviour.

Revised after review

Jerry's review asked for three things; all three are in. The PR is smaller as a result — a knob, a database key, an RPC method and their tests are gone.

1. Coverage folded into bor_getInvalidPreconfBlocks (above). bor_getPreconfAuditStatus, the PreconfAuditStatus type, the silent 1024-record truncation and rawdb.InvalidPreconfQueryLimit are removed.
2. The depth bound is gone. sequencer.audit-window and its config plumbing, defaultAuditWindow, auditDepth, skippedTo, recordSkippedWindow, SetAuditWindow, the PreconfUnauditedThrough key with its accessors, leadingUnheld, and the docs entries all go with it. The retention floor resolved from Range replaces them.
3. The gateway's served depth is a sequence-store matter and nothing here waits on it.

Two notes on the consequences, neither of which changes what was asked for:

• A clean answer is no longer proof of comparison. With unauditedThrough gone, the watermark advances over the skipped range, so an empty invalid with pendingFrom: null covering it reads the same as a compared-and-clean range. The two counters above are the only record, so the alert belongs on them rather than on the RPC. Deliberate per the review; flagging it because it is the one guarantee this revision narrows. Widening the gateway's served depth shrinks the window where it can happen but does not close it.
• rawdb.ReadInvalidPreconfs(db, limit) — the non-range accessor, called only by tests — now honours the caller's limit with no ceiling of its own, since the constant that clamped it is gone. Say the word if you would rather it kept one.

Validated on a kurtosis devnet

Run on kurtosis-pos @ vbhattaccmu/sequence-store-benchmark with the store
live (redpanda / ingress / gateway / auditor), two publishing validators, and
two RPC consumers on the same chain — one on this branch, one on
cffls/sequence-publisher as a control. Full write-up in the workspace report;
the parts that matter here:

A consumer restarted after downtime audits exactly the window it missed.
Stopped at watermark 272 under sustained load (60 tx per batch, continuous),
restarted after ~160 blocks:

from=278 through=383  walked=106 compared=106 mismatched=0 uncomparable=0
from=384 through=445  walked=62  compared=62
from=446 through=451  walked=6   compared=6

Three passes — startup, session-end, Live — with compared == walked
throughout, so the store held a generation at every audited height and all of
them were compared against canonical. The watermark was not re-seeded; the only
watermark seeded line is from first boot. In an earlier run the audit started,
the stream went live 9 ms later, and the audit finished after — it runs off the
session loop and does not delay the reconnect.

A store outage freezes the watermark rather than advancing it. With the
gateway stopped, the head advanced from 1295 to 1361 while the watermark held
at 1299, then the gap closed on reconnect: from=1300 through=1395 walked=96 compared=96. Without the watching gate the mark would have tracked
the head and declared 62 uncompared heights clean — this is the invariant the
change exists for.

Preconfirmations are unaffected: 50 concurrent eth_sendRawTransactionSync
→ 50/50 succeeded, 50/50 preconfirmed, 50 tx in one block, p50 117 ms, in line
with the figures Vikram posted on 2026-09-05.

Mismatch detection was not exercised. No store-vs-canonical divergence
arose: the store auditor logged zero supersede events for the whole run, so the
condition the verdict detects never occurred — the absence of the trigger, not
a failure of detection. It is unit-covered, including
TestAuditPassOverGRPCRecordsMismatches, which detects one over a real
ConsumerService server. Forcing it needs deliberate producer contention and
belongs in the e2e rather than a hand-driven devnet.

These runs predate the reshape above. The behaviour they pin — watermark
advance, the watching gate, restart coverage, preconf latency — is untouched
by it; the depth-bound and unauditedThrough observations from those runs are
not reproduced here because neither exists any more. The retention-floor path
that replaces them is unit-covered only, and the honest position is that it has
not yet run against a real store with an aged-out floor: a devnet's store never
reaches its retention limit inside a test run. What a devnet did show is the
condition the floor now resolves — heights 1..127 hold nothing because the
publisher is gated off before Rio at 128 — and on two independent chains the
old mark landed on exactly 127, so the floor has a real case to resolve the
moment it runs there.

Three defects the devnet found, now fixed

1. The audit ran every two seconds. Pre-Rio deterministic() fails, so
   run retries every consumerRetryDelay, and requestAudit() sat at the top
   of that loop — a gRPC client built, the store walked, the client torn down,
   every 2s indefinitely for any node sitting pre-Rio or with a persistently
   failing stream. The trigger moved into runSession, which asks for a pass
   only after a session that actually ran. Measured: 66 pre-Rio retries now
   produce 0 passes. Covered by TestIneligibleSessionDoesNotQueueAnAudit,
   verified to fail against the old placement.
2. walked hid compared. The first post-Rio pass read walked=129 compared=2 — publishing starts at Rio activation, so 127 heights were
   NotFound. The old single counter made that look like a clean audit of 129
   heights, and I misread it that way myself. Both are now reported.
3. An invisible log at Debug while bor runs at INFO (moot now, see below).

Dropped from this PR: the backlog-open change

An earlier revision also dropped store opens more than 64 blocks below the
canonical head, to avoid the parent state unavailable lookup on a pruned
node. It is removed. It never fired — it needs p2p import to run 64+ blocks
ahead of the store stream, and across a 330-block downtime the stream kept
pace. The warning storm that motivated it did not reproduce on either build.

An earlier A/B suggested a 20x reduction in skipped opens, but that control was
built from feat/pbc-rpc-endpoints before #2373's later parent-resolution
work — the exact code being counted — and against the PR's real base the
difference disappears. That left a behaviour change in the preconf path, while
coverage is being measured, with nothing exercising it. consumer_session.go
is byte-identical to base as a result, so this PR no longer touches the hot
path at all.

Worth reproducing separately if anyone hits the storm again.

Two further defects found reviewing the feature end to end, now fixed

Both were found by re-reading the paths against the questions the tests did not
ask, after the devnet run.

The audit could overwrite a live invalidation record. invalidPreconfKey is
one key per height and the write was a bare Put, so a pass that judged a
height the live path had already recorded replaced it. The reasons correlate —
a height whose served preconfirmation missed canonical is exactly where the
store's final seal probably missed it too — so this was reachable whenever a
session dropped between the live record and the watermark advance past it. The
damage ran in the worst direction: unobserved_mismatch asserts nothing was
served from this height, so the ledger would have told an operator there was
no user-visible impact at a height where a preconfirmation was served and
then invalidated. The audit now writes only where a height carries no record
(rawdb.WriteInvalidPreconfIfAbsent) and counts what it left alone.

A range the store held nothing for read as clean. NOT_FOUND cannot
distinguish the producer never published here from retention aged this
height out, and the pass advanced the watermark across either. So a node down
longer than retention — or restored from an older snapshot — walked its range,
compared nothing, recorded nothing, and reported auditedThrough across the
whole of it: silence again indistinguishable from "everything held", which is
the exact bug this PR exists to remove, at the retention boundary instead of
the downtime one.

The first fix for this raised a second unauditedThrough mark. The review
replaced it with the retention floor read from Range, which is a better
answer to the same problem: the floor separates aged out from never
published exactly, where a leading run of NOT_FOUND could only ever be
conservative and call both unknown. What it does not do is persist the fact —
hence the alerting note above.

One case of the same shape remains unrecorded. A height the store held
but that could not be decided — no canonical hash, or a seal that does not
decode or sits at the wrong height — advances the watermark without a record.
It is counted in the pass summary (uncomparable) and in
sequencer/audit/unknown, and logged. All three causes are a store or data
fault rather than a normal state, so it should not fire in practice.

Executed tests

• All pre-existing tests green in eth/sequencer, core/rawdb, internal/ethapi, internal/cli/server, eth/ethconfig. -race clean on eth/sequencer and core/rawdb. go vet, gofmt and golangci-lint (v2.11.4, repo config) clean; gofumpt clean on every file this PR touches.
• 52 new test functions plus 28 table cases. The audit's mismatch / clean / NotFound / unsealed / cancel / transport-failure / write-failure / no-rewind / checkpoint paths, an auditHeight verdict table and a recordVerdict table, the summary-counter assertions, the watermark accessor including a truncated stored value, the contiguity and watching gates, and the Live frame.
• The retention floor has its own set: floorFromEntries over open-first / records-then-open / seal-first / records-only / empty / undecodable-seal pages, storeFloor's paging and its four fall-back cases (no reader, failing read, empty page, no boundary inside the page budget), skipToStoreFloor across a floor below / at / inside / above the range, and a pass asserting it starts at the floor and never probes a height below it. TestAuditPassStartsAtTheStoreFloorOverGRPC then pins the request the store really receives — after unset, which is what resolves to the earliest retained entry — because the unit tests inject the reader and so cannot catch a malformed RangeRequest.
• The new RPC shape is covered by a pendingFrom table over six watermark positions (absent, below, at the start, inside, at the end, above), the wire-format assertion against the shape in the review, the range cap at exactly 1024 and 1025 heights, the widest expressible range (where a count-based cap would overflow to zero and wave it through), and an unreadable watermark returning an error with a nil result.
• The pass is also exercised end to end over gRPC: TestAuditPassOverGRPCRecordsMismatches stands up a real ConsumerService server, points the consumer at it, and asserts the dial, the per-height GetBlock reads, the recorded mismatch and the watermark write. Unreachable-endpoint and nothing-to-audit cases are covered too, the latter asserting the pass opens no connection at all — a trigger fires on every session retry and must not each cost a dial.
• Regression tests were confirmed to fail without their fix, so none passes vacuously: TestWatermarkAdvancesOnlyWhileWatchingTheTip, TestIneligibleSessionDoesNotQueueAnAudit, TestAuditDoesNotSeedTheWatermarkOnAReadFailure, TestAuditLeavesALiveRecordInPlace and the rawdb three-state tests each fail against the code they guard against.
• Diffguard passes with CI's flags: tier-1 logic 86.7% (gate 80), tier-2 semantic 81.8% (gate 60), complexity, sizes, dependency structure and dead code clean. Neither the headline score nor the tiers are stable run to run: --mutation-sample-rate 20 samples a different fifth of the pool each time, and the three runs behind this revision sampled 19, 25 and 30 tier-1 mutants respectively, so their scores are not comparable with each other. The same-tree experiment on the previous revision is the one that measures the variance — three runs of identical code scored 78.7% / 68.1% / 78.7% overall with tier-1 at 88.9% / 93.8% / 95.2%. The gate is the tiers, and the survivor list is drawn fresh every run, so it has to be read per run rather than trusted as a fixed set.
• All eight survivors in the final run are accounted for, and two rounds of them were resolved rather than accepted:
  • Four are log statements or metric increments (report, auditUnknownCount, the stopped-early warning, the unheld warning's guard) — tier-3, ungated.
  • Three are equivalent mutants: persist's current >= number relaxed to > still falls through to a write of the same value; fetchOldestVia's error return zeroed leaves storeFloor reading an empty page, which returns the same "no floor"; and WriteInvalidPreconfIfAbsent's error-path bool is ignored by its only caller, which tests err first.
  • One is a false positive: accessors_preconf.go:59 (if present { return false, nil } → true) is reported as surviving, but applying that mutation by hand fails TestWriteInvalidPreconfIfAbsent deterministically (second write replaced an existing record), and so does deleting the branch. Worth knowing the survivor list overstates in both directions — an earlier revision of this PR had the same thing happen at audit.go:305.
  • Two survivors from the previous run were removed by rewriting rather than by adding a test: floorFromEntries now type-switches on the entry kind the way entryHeight already does, so there is no nil-seal guard whose removal falls through to the same decode error, and storeFloor's empty-page exit is pinned by a call count — without it, removing the branch changed nothing observable but cost three wasted round trips.
• docs/cli/{server.md,default_config.toml} regenerated with make docs; the flag's entries are gone.

The automated scenario now exists. The restart-and-audit path above was
driven by hand, and as a standing regression it belongs with Jerry's e2e work
rather than a parallel harness —
pos-workflows#47 and
#52 are both merged to
main, so the sequencer leg gates PRs there. #52 carries the consumer-side
half: preconf receipt shape, canonicalisation, the invalid-preconf range
contract, and chaos episodes against the store.

That leg already knows about this PR's response shape. Its range-contract
test detects which shape it got and checks each against its own contract, so
the {invalid, pendingFrom} branch — including the 1024-height cap and the
rule that a missing pendingFrom fails where a null one passes — starts
asserting by itself once this merges and the workflow's bor ref carries it. No
change needed there.

The mismatch case is still not covered anywhere: forcing a store-vs-canonical
divergence needs deliberate producer contention, which is why neither the
devnet runs above nor the e2e leg has triggered one.

⚠️ Incidental, unrelated to this change: make docs also added rpc.txsync.maxconcurrent to docs/cli/server.md. #2385 regenerated those docs, but they were dropped when #2373 merged into cffls/sequence-publisher, so the flag exists in code with no doc entry. Two lines of generated output; the docs should match the flag set, so I've left the repair in.

Rollout notes

Not consensus-affecting, no coordinated upgrade. Confined to the sequence-store consumer, which is off by default and requires explicit configuration.

No new operator knobs. One new rawdb key, PreconfAuditedThrough, a single uint64 — no migration, no resync. Four new metrics under sequencer/audit/.

bor_getInvalidPreconfBlocks changes shape, so any existing caller needs updating: the result is an object rather than an array, the per-record reason is no longer on the wire (it stays in the database for logs), and a range wider than 1024 heights is now an error instead of a truncated list. The method has not shipped outside this branch as far as I can tell — worth confirming against anything you have pointed at it.

Retention. The audit can only see what the store retained; below the floor GetBlock answers NOT_FOUND. The floor is now resolved up front and the skipped heights are counted, so exceeding retention is visible in metrics — but it is not visible in the RPC, which reports those heights as audited. Alert on sequencer/audit/retentionskipped and sequencer/audit/unheld. With the planned 7-day retention any realistic downtime sits inside the window anyway, and the gateway's served depth is the nearer limit until the sequence-store follow-up lands.

A read error stalls rather than skips. Any store read that is not NOT_FOUND ends the pass, which persists progress and resumes from the same height on the next trigger. A deterministic per-height error therefore freezes auditedThrough at that height and re-warns on every session event. That is the safe direction — the watermark never advances over heights nobody compared, so nothing reads as clean — and it is visible rather than silent, so I have left it alone instead of adding a skip policy that could hide heights. The realistic trigger, a generation over the call's receive limit, needs 33 MB at one height against a ~2 MB gas-bound ceiling, so it is remote.

No repair path. The watermark is monotonic, so heights the audit passed over are not revisited: there is no operator hook to re-audit a range, even where the store still holds the data. That is a deliberate consequence of never rewinding the mark. Worth an admin method later if operators actually hit it — I have not added API surface on speculation.

Disk growth. #2373 established that InvalidPreconf-* is retained without pruning by design, with the gradual growth called out in its rollout notes. This PR does not change that, but it does add a second writer to that keyspace, so the growth rate is no longer bounded by "preconfirmations this node actually served" — a node that restarts repeatedly, or one auditing a range where the store superseded heavily, writes more records than before. Worth a look at whether the existing decision still holds at that rate; I have not assumed it needs changing.

Open questions

The review settled most of the original list: the response shape (2), the depth bound (3), a seal-only read (5, deferred behind the gateway's served depth), per-height "not compared" records (6, answered as log plus metric), and the retention floor (7, answered as Range with after unset). Nothing was raised against the record semantics (1) or seeding at the head on first enable (4), so both stand as built.

Still open:

1. Reorg policy. A verdict is only as good as the canonical chain when it was reached, and the watermark never rewinds, so a reorg below it replaces heights nothing revisits. Re-auditing would mean rewinding on every reorg and re-walking. I have left it documented rather than changed, on the grounds that Bor reorgs are shallow and milestones make deep ones rare — tell me if that is the wrong call for your use of the ledger.
2. Where the alert lives. Per the note above, the uncompared-range signal is now metric-only. If you would rather it stayed queryable, the cheapest version is a single auditedFrom alongside pendingFrom — the oldest height the watermark can vouch for — which is one more uint64 and no new method.

One semantic worth stating explicitly: GetBlock resolves to the latest generation at a height, so a preconfirmation that came from an earlier, superseded generation is invisible to this pass. For downtime auditing that's the right question — nothing was served, so what matters is whether the store's final answer matched the chain — but the ledger shouldn't be read as stronger than that.

[codecov]

Codecov Report

❌ Patch coverage is 93.56322% with 28 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.11%. Comparing base (7e6f183) to head (928e994).

Files with missing lines	Patch %	Lines
eth/sequencer/audit.go	94.92%	10 Missing and 4 partials ⚠️
eth/sequencer/consumer.go	90.69%	6 Missing and 2 partials ⚠️
core/rawdb/accessors_preconf.go	82.85%	4 Missing and 2 partials ⚠️

Additional details and impacted files

@@                     Coverage Diff                      @@
##           cffls/sequence-publisher    #2388      +/-   ##
============================================================
+ Coverage                     57.03%   57.11%   +0.07%     
============================================================
  Files                           952      953       +1     
  Lines                        174451   174862     +411     
============================================================
+ Hits                          99497    99871     +374     
- Misses                        69283    69315      +32     
- Partials                       5671     5676       +5     

Files with missing lines	Coverage Δ
core/rawdb/schema.go	37.28% <ø> (ø)
eth/backend.go	54.92% <100.00%> (+0.31%)	⬆️
eth/sequencer/consumer_prepare.go	79.03% <100.00%> (ø)
internal/ethapi/bor_api.go	65.47% <100.00%> (+0.97%)	⬆️
core/rawdb/accessors_preconf.go	85.39% <82.85%> (-2.75%)	⬇️
eth/sequencer/consumer.go	86.47% <90.69%> (+2.81%)	⬆️
eth/sequencer/audit.go	94.92% <94.92%> (ø)

... and 31 files with indirect coverage changes

Files with missing lines	Coverage Δ
core/rawdb/schema.go	37.28% <ø> (ø)
eth/backend.go	54.92% <100.00%> (+0.31%)	⬆️
eth/sequencer/consumer_prepare.go	79.03% <100.00%> (ø)
internal/ethapi/bor_api.go	65.47% <100.00%> (+0.97%)	⬆️
core/rawdb/accessors_preconf.go	85.39% <82.85%> (-2.75%)	⬇️
eth/sequencer/consumer.go	86.47% <90.69%> (+2.81%)	⬆️
eth/sequencer/audit.go	94.92% <94.92%> (ø)

... and 31 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[claude]

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.

_{Tip: disable this comment in your organization's Code Review settings.}

[pratikspatil024]

codegenie review

[github-actions]

🧞 Codegenie Review

Two verified findings, both low severity, in the new startup audit-pass code. One is a real correctness gap in the rawdb watermark accessor: readPreconfHeight collapses DB read errors and malformed values into the same (0, false) "never audited" sentinel, which makes windowToAudit seed the watermark at the current canonical head and report an uncompared window as clean — the exact false-clean answer this PR exists to remove. The other is a test-quality gap: TestAuditPassSkipsTheDialWithNothingToAudit asserts only that the watermark is untouched, which stays true whether or not the pre-dial early return exists, so the no-dial-per-session-retry contract it is named for is not actually covered.

Coverage: 28 of 33 hunks reviewed (14 deep, 9 normal, 5 light); 5 hunks skipped by configured rules (docs/cli/default_config.toml, docs/cli/server.md). No failed hunks, no degraded planning, verification complete.

Open questions not resolved during review (worth confirming before merge): whether a canonical head at watermark+1 can advance the mark past a height whose store entries have not yet streamed in; whether SetAuditWindow(0) from eth/backend.go keeps the package default depth or zeroes the startup audit; whether auditor.run persists partial progress at the last verified height on fetch error/cancellation given auditCheckpointInterval is 256; whether SetAuditWindow is always called before Start so the unsynchronized c.auditWindow write cannot race auditLoop; and whether a backlog open dropped while s.env != nil leaves an unresolved PendingStore entry since dropBacklogOpen never calls invalidatePendingFrom.

Coverage

Reviewed 28/33 hunks.
Incomplete work: skipped 5.
Coverage levels: deep 14, normal 9, light 5, skip 5.

• docs/cli/default_config.toml: configured skip rule
• docs/cli/server.md: configured skip rule

⚠️ Findings

⚪ Low: "Skips the dial" test cannot fail if the pre-dial window check is removed

File: eth/sequencer/audit_pass_grpc_test.go:124 ↗
Confidence: high

TestAuditPassSkipsTheDialWithNothingToAudit cannot fail on the behavior it is named for. Its only assertion is that the watermark is untouched:

// Nothing to audit means no connection is opened at all: a trigger fires on
// every session retry, and those must not each cost a dial.
func TestAuditPassSkipsTheDialWithNothingToAudit(t *testing.T) {
	...
	consumer.endpoint = "127.0.0.1:1"
	head := h.chain.CurrentBlock().Number.Uint64()
	if err := rawdb.WritePreconfAuditedThrough(h.chain.DB(), head); err != nil { ... }
	consumer.runAuditPass(t.Context())
	if got, _ := rawdb.ReadPreconfAuditedThrough(h.chain.DB()); got != head {
 t.Fatalf("watermark = %d, want it untouched at %d", got, head)
	}
}

The windowToAudit guard exists in two places. runAuditPass checks it before dialing:

func (c *Consumer) runAuditPass(ctx context.Context) {
	audit := &auditor{...}
	// Resolve the window before dialing: the common case is nothing to audit,
	// and a trigger fired on every session retry must not cost a connection.
	from, through, _, ok := audit.windowToAudit()
	if !ok { return }
	conn, err := grpc.NewClient(...)

and auditor.run repeats it:

func (a *auditor) run(ctx context.Context) (auditSummary, error) {
	from, through, skippedTo, ok := a.windowToAudit()
	if !ok {
 return auditSummary{}, nil
	}

Impact: with the watermark seeded at head, deleting the pre-dial early return — the exact optimization the comment describes — would make the pass dial 127.0.0.1:1, perform zero fetches, never call persist, and still leave the watermark at head. The test passes either way, so a per-session-retry dial regression would go undetected by the test named for it while giving reviewers false confidence that the no-dial contract is covered.

Suggested fix: observe the dial rather than a proxy. Start a listener (or reuse startAuditStore with a wrapping net.Listener) whose Accept increments a counter, point consumer.endpoint at it, and assert the counter stays 0 after runAuditPass; keep the watermark assertion as a secondary check.

Suggested test: wrap the test listener so each Accept sends on a buffered channel; assert len(accepts) == 0 after runAuditPass with watermark == head, and assert it is > 0 in the mismatch test to prove the counter observes real dials.

⚪ Low: readPreconfHeight maps DB read errors and malformed values onto the "never audited" sentinel, seeding the watermark at head

File: core/rawdb/accessors_preconf.go:137 ↗
Confidence: medium

readPreconfHeight returns the same (0, false) result for a genuinely missing key, a non-not-found Get error, and a stored value whose length is not 8:

func readPreconfHeight(db ethdb.KeyValueReader, key []byte) (uint64, bool) {
	value, err := db.Get(key)
	if err != nil || len(value) != 8 {
 return 0, false
	}
	return binary.BigEndian.Uint64(value), true
}

The caller treats that sentinel strictly as "this node has never audited" and seeds the watermark at the current canonical head (eth/sequencer/audit.go:81-108):

watermark, stored := rawdb.ReadPreconfAuditedThrough(a.db)
if !stored {
	a.persist(through)
	log.Info("Sequence store audit watermark seeded", "height", through)
	return 0, 0, 0, false
}

Because this returns ok=false, run() exits before recordSkippedWindow and no unauditedThrough mark is written. Both write paths are monotonic and never lower the mark:

func (a *auditor) persist(number uint64) {
	if a.advance != nil { a.advance(number); return }
	if current, ok := rawdb.ReadPreconfAuditedThrough(a.db); ok && current >= number { return }
	if err := rawdb.WritePreconfAuditedThrough(a.db, number); err != nil { log.Warn(...) }
}

Impact: on a transient read failure or a corrupted value, the heights between the real watermark and head are marked audited without ever being compared, and the mark cannot be lowered afterwards. bor_getPreconfAuditStatus then reports auditedThrough=head, unauditedThrough=null for a window that was never audited (internal/ethapi/bor_api.go:174-184) — the false-clean answer this PR exists to prevent. Reachability is narrow: writePreconfHeight always writes 8 bytes, and a backend failing Get will usually also fail the subsequent Put, leaving the watermark unchanged with only a logged warning, so the sticky outcome requires a read-only failure.

The doc comment on ReadPreconfAuditedThrough states that "a node that has never audited has no watermark, which is not the same as having audited through block zero"; no intent signal says read errors or malformed values should map onto that same absence state. Please confirm the intended contract for the error case.

Suggested fix: give readPreconfHeight a third state — return (0, false, nil) only for ethdb not-found, and propagate other Get errors and unexpected value lengths as an error. windowToAudit should then abort the pass (or record unauditedThrough=head) rather than calling persist(through).

Suggested test: a rawdb test with a KeyValueReader stub whose Get returns a non-not-found error, plus one storing a 4-byte value, asserting both are distinguished from a missing key; and a sequencer test asserting windowToAudit does not seed the watermark at head when ReadPreconfAuditedThrough fails. Current tests in eth/sequencer/audit_pass_test.go only exercise valid 8-byte watermarks or an empty DB.

🙋 Needs Human Attention

• behindHead computes number+backlogOpenDepth, which wraps for absurd numbers near MaxUint64 from a faulty/malicious store entry, making the open silently dropped instead of skipped/invalidated. Are open block numbers validated upstream against the local head before applyOpen?

  • Files: eth/sequencer/backlog.go, eth/sequencer/consumer_session.go
  • Symbols: applyOpen, behindHead, validateOpenExecutionContext
  • Reason: Packet reviewer could not resolve this question from the reviewed context.

• Is any iteration over a key prefix in the DB affected by the new plain keys "PreconfAuditedThrough"/"PreconfUnauditedThrough" (e.g., a prefix scan that would now pick them up)?

  • Files: core/rawdb/accessors_preconf.go, core/rawdb/schema.go
  • Symbols: ReadInvalidPreconfs, ReadInvalidPreconfsInRange, invalidPreconfPrefix
  • Reason: Packet reviewer could not resolve this question from the reviewed context.

Stats

• 🤖 Model: anthropic claude-opus-5 high
• 🧞 Codegenie: v0.5.6 (a662388fde)
• Elapsed time: 5m 33s
• Git: 0xPolygon/bor from cffls/sequence-publisher to ppatil/preconf-store-audit (cc65e04371)
• Posting: 2 inline
• Review completeness: complete.
• Usage: model calls 99, tokens 2411597, cost $8.9487.
• Effective caps: tokens 8000000.
• Local context pressure: 4 tool-budget rejections, 23 degraded tool results, 5 degraded hunks.

— View Workflow Job

[Copilot]

🟡 Changes recommended

There are a few concrete correctness/API semantics issues to address (including a uint64 overflow risk in the new backlog predicate and JSON presence semantics for the new audit-status RPC).

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR adds an “audit pass” to the Sequence Store consumer so a node can retrospectively detect and record invalid preconfirmations for chain heights it did not observe while offline (or while the stream was replaying history), and exposes audit watermarks via a new Bor RPC method.

Changes:

• Introduces a bounded, non-executing audit walker (eth/sequencer/audit.go) that compares the store’s final sealed header per height against canonical hashes and records unobserved_mismatch invalidations plus persisted audit/unaudited watermarks.
• Extends the sequencer consumer to (a) track when it is truly “watching the tip” and (b) drop deeply-backlogged opens without hitting pruned-state paths.
• Adds rawdb accessors + schema keys for PreconfAuditedThrough / PreconfUnauditedThrough, wires a new CLI flag (sequencer.audit-window), and adds bor_getPreconfAuditStatus.

File summaries

File	Description
internal/ethapi/bor_api.go	Adds PreconfAuditStatus and GetPreconfAuditStatus RPC surface.
internal/ethapi/bor_api_test.go	Tests new audit-status RPC serialization and watermark reads.
internal/cli/server/sequencer_flags_test.go	Tests config→ethconfig wiring for sequencer.audit-window.
internal/cli/server/flags.go	Adds sequencer.audit-window CLI flag.
internal/cli/server/config.go	Adds SequencerConfig.AuditWindow and gating helper sequencerAuditWindow().
eth/sequencer/consumer.go	Adds “watching tip” gating + audit triggers/loop and watermark advancement logic.
eth/sequencer/consumer_session.go	Drops backlog opens when sufficiently behind canonical head.
eth/sequencer/consumer_prepare.go	Propagates “live” marker frame so the consumer can start “watching”.
eth/sequencer/backlog.go	New depth predicate + backlog-open drop behavior.
eth/sequencer/audit.go	New audit implementation: windowing, gRPC reads, verdicts, watermark persistence.
eth/sequencer/audit_watermark_test.go	Unit tests for watermark stepping rules, “live” frame, backlog predicate, audit loop.
eth/sequencer/audit_pass_test.go	Unit tests for audit window selection, mismatch/unknown/no-seal handling, checkpointing, write failures.
eth/sequencer/audit_pass_grpc_test.go	End-to-end gRPC audit pass tests (dial-skipping, unreachable store, mismatch recording).
eth/ethconfig/config.go	Adds SequencerAuditWindow to runtime config.
eth/backend.go	Wires SequencerAuditWindow into consumer via SetAuditWindow.
docs/cli/server.md	Regenerated CLI docs (includes sequencer.audit-window and rpc.txsync.maxconcurrent).
docs/cli/default_config.toml	Regenerated default config (adds sequencer.audit-window and txsync.maxconcurrent).
core/rawdb/schema.go	Adds rawdb keys for audit/unaudited watermarks.
core/rawdb/accessors_preconf.go	Adds read/write helpers for PreconfAuditedThrough and PreconfUnauditedThrough.
core/rawdb/accessors_preconf_test.go	Tests watermark semantics and truncated-value handling.

Review details

• Files reviewed: 20/20 changed files
• Comments generated: 3
• Review effort level: Lite

---

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

[github-actions]

🧞 Codegenie Review

Two verified findings, both low severity, in the new startup audit-pass code. One is a real correctness gap in the rawdb watermark accessor: readPreconfHeight collapses DB read errors and malformed values into the same (0, false) "never audited" sentinel, which makes windowToAudit seed the watermark at the current canonical head and report an uncompared window as clean — the exact false-clean answer this PR exists to remove. The other is a test-quality gap: TestAuditPassSkipsTheDialWithNothingToAudit asserts only that the watermark is untouched, which stays true whether or not the pre-dial early return exists, so the no-dial-per-session-retry contract it is named for is not actually covered.

Coverage: 28 of 33 hunks reviewed (14 deep, 9 normal, 5 light); 5 hunks skipped by configured rules (docs/cli/default_config.toml, docs/cli/server.md). No failed hunks, no degraded planning, verification complete.

Open questions not resolved during review (worth confirming before merge): whether a canonical head at watermark+1 can advance the mark past a height whose store entries have not yet streamed in; whether SetAuditWindow(0) from eth/backend.go keeps the package default depth or zeroes the startup audit; whether auditor.run persists partial progress at the last verified height on fetch error/cancellation given auditCheckpointInterval is 256; whether SetAuditWindow is always called before Start so the unsynchronized c.auditWindow write cannot race auditLoop; and whether a backlog open dropped while s.env != nil leaves an unresolved PendingStore entry since dropBacklogOpen never calls invalidatePendingFrom.

Reviewed 28/33 hunks.
Incomplete work: skipped 5.

Coverage disclosure:

• docs/cli/default_config.toml: configured skip rule
• docs/cli/server.md: configured skip rule

🙋 Needs human attention:

• behindHead computes number+backlogOpenDepth, which wraps for absurd numbers near MaxUint64 from a faulty/malicious store entry, making the open silently dropped instead of skipped/invalidated. Are open block numbers validated upstream against the local head before applyOpen?
• Is any iteration over a key prefix in the DB affected by the new plain keys "PreconfAuditedThrough"/"PreconfUnauditedThrough" (e.g., a prefix scan that would now pick them up)?

— codegenie v0.5.6 (a662388fde) · View Workflow Job

[cffls]

Thanks, the watermark approach is the right shape. A few changes before this lands, plus one follow-up that belongs in sequence-store rather than here.

1. Return the audit state from bor_getInvalidPreconfBlocks itself

Callers should not need a second call to interpret an empty result. Since auditedThrough is a prefix mark, the not-yet-audited part of any query range is always its tail, so one extra field is enough:

// bor_getInvalidPreconfBlocks("0x5", "0x60")
{
  "invalid": ["0x9", "0x6"],
  "pendingFrom": "0x4e"
}

• invalid: heights in range with an invalidation record, newest first, as hexutil.Uint64. The reason field is not needed on the wire; callers only need the heights. Keep it in the DB for logs.
• pendingFrom: the first height in range the audit has not reached, i.e. max(from, auditedThrough + 1); heights from there to to are pending and callers query again later for them. null when the whole range is audited. Equal to from when the node has not audited yet.
• Every other height in range is clean.
• Cap the range at 1024 heights (to - from + 1 <= 1024) and return an error above that, like from > to does today. With one record per height this makes the silent 1024-record truncation in ReadInvalidPreconfsInRange and InvalidPreconfQueryLimit unnecessary.
• A failed watermark read returns an RPC error, not a result with pendingFrom null.

With this in place bor_getPreconfAuditStatus and PreconfAuditStatus can go; the range call covers what they report.

2. Audit depth: walk from the watermark to head, bounded by the store, not by a block count

The audit should start at auditedThrough + 1 and continue to head. The store's retention is the ceiling; a separate block window on our side is a second bound operators would have to keep aligned with it, so defaultAuditWindow, auditDepth, skippedTo, recordSkippedWindow, SetAuditWindow, the --sequencer.audit-window flag and its config plumbing, TestSequencerAuditWindow, and the docs/cli changes can be dropped. The unauditedThrough mark, leadingUnheld, and their accessors go with them; auditedThrough is the only mark the API needs.

Two details for the walk:

• Do not probe heights the store no longer holds one GetBlock at a time. A Range request with after unset resolves to the oldest entry the store still serves, and that entry's BlockOpen.block_number is where the walk starts. If it is above auditedThrough + 1, log a warning with the skipped range and count it in a metric so operators can alert on it, then continue from there.
• A NOT_FOUND mid-walk means the store aged the height out while the walk was running or nothing was published there; treat it the same way, log and count, and advance.

Progress checkpointing every 256 heights stays as is.

3. Follow-up in sequence-store

The gateway serves GetBlock and Range from an in-memory window bounded by --window-bytes (256 MiB by default), which is shorter than the log's retention. The gateway should serve as far back as the log retains so the audit can cover a full outage. That is a sequence-store change and will be tracked there; nothing in this PR waits on it. Once the served depth grows, a seal-only read in the proto would keep the walk cheap, since GetBlock currently returns full transaction payloads.

[pratikspatil024]

All three are in, and the PR is smaller for it — the knob, the PreconfUnauditedThrough key, bor_getPreconfAuditStatus and their tests are all gone.

bor_getInvalidPreconfBlocks answers exactly the shape you wrote ({"invalid":["0x9","0x6"],"pendingFrom":"0x4e"}, pinned as a wire-format assertion), capped at 1024 heights, reason off the wire and still in the DB for logs, unreadable watermark an RPC error rather than a null. The walk runs watermark→head with the floor resolved from Range with after unset; the skipped range is logged and counted in sequencer/audit/retentionskipped, a mid-walk NOT_FOUND in sequencer/audit/unheld. Checkpointing unchanged.

One thing I'd like your explicit yes on. With unauditedThrough gone the watermark advances over the skipped range, so an empty invalid with pendingFrom: null covering those heights now reads identically to compared-and-clean — the counters are the only record that nothing was checked. I'm fine with that and the rollout note says "alert on the metrics", but it is the one guarantee this revision narrows, so I'd rather you confirmed it than found it later. If you'd sooner keep it queryable, the cheap version is an auditedFrom beside pendingFrom — the oldest height the watermark can vouch for — one more uint64, no new method.

Two smaller notes:

• Entry is a oneof, so the earliest retained entry isn't always a BlockOpen — retention can age out mid-block. The floor takes the first open in the page; a seal reached before any open closes a block that is only partly retained, so the floor is the height above it; records carry no height, so the scan pages on (4 pages of 1024 entries, then it gives up and lets the walk discover the heights unheld one at a time, which is the old behaviour).
• rawdb.ReadInvalidPreconfs(db, limit) — the non-range accessor, called only by tests now — lost its ceiling along with InvalidPreconfQueryLimit. Say the word if you'd rather it kept one.

Still open from the original list: reorg policy (8). Everything else your review settled.

[cffls]

Checked the head; it matches what you describe. Answers in order:

1. Metric-only, explicit yes. No auditedFrom. A gap wider than what the store serves is an operational condition, not an API state: the alerts on sequencer/audit/retentionskipped and sequencer/audit/unheld are the record, and the sequence-store follow-up (serve as far back as the log retains) is what closes the gap.

2. Floor resolution. The gateway only ever starts its window at a BlockOpen: on cold start it skips entries until the first open, and eviction cuts at generation boundaries. So Range with after unset returns an open as its first entry. The seal case and the paging guard against something the gateway does not produce; keeping them is fine since they fall back to the walk, but a single page with "first entry must be an open, otherwise fall back" would be enough. I'll get that invariant into the proto comment so it is a contract rather than an implementation detail.

3. ReadInvalidPreconfs. The caller supplies limit, so it does not need a ceiling of its own. Leave it.

4. Reorg policy. Rather than rewinding the mark, bound the walk at the finalized head (milestone) instead of the current head. Verdicts at or below finality cannot be reorged, so nothing below the watermark ever needs revisiting, and heights between finality and head show up as pendingFrom, which is the right answer for them anyway. The finality lag is a handful of blocks, so coverage is unaffected. For the canonical-head path that also advances the mark, I take it that only covers heights the live path reconciled, where a reorg already produces a reorged record; confirm that and I think the open question is closed.

[pratikspatil024]

All four in, pushed as 6c6c41bd2. One of them does not confirm the way you expected, so taking 4 first.

4. Reorg policy — the walk now stops at finality, and so does the canonical-head path. The bound is exactly as you describe and it is a better answer than rewinding: nothing below the mark can be replaced, so monotonicity costs nothing.

But I cannot confirm the second half. reconcileCanonicalLocked removes the pending entry once the height is reconciled — whether it matched the chain or was invalidated (pending_reconcile.go, the s.removeLocked(key) at the end of the loop body, unconditional). So reorged only covers a reorg that arrives while the entry is still pending. A reorg of a height the live path has already reconciled finds no entry, writes no record, and markCanonicalHeadAudited has already stepped the mark past it — which is precisely the reorg-below-the-mark case the bound is meant to close.

So the ceiling has to apply to both paths or the invariant does not hold, and it now does. The live path steps to watermark+1 only while that height is at or below the milestone, keeping the existing contiguity and watching gates; the mark then trails finality by a few blocks and closes the distance one height per canonical head, which is the cadence blocks arrive at anyway. If you would rather leave the live path alone and accept that narrow exposure, say so and I will drop that half — it is the one piece here that goes beyond what you asked for.

1. Metric-only — taken, no auditedFrom. The rollout notes point at the two counters.

2. Floor — simplified to what you describe: one page, first entry must be an open, anything else resolves nothing and falls back to the walk. The seal case and the paging loop are gone. It reads the invariant off your gateway rather than rediscovering it, so the proto comment is what makes it a contract — no rush, nothing here breaks without it, it just becomes documented rather than assumed.

3. ReadInvalidPreconfs — left with the caller's limit and no ceiling.

One thing worth flagging from wiring this up: WhitelistedMilestone needed a nil guard. The consumer holds it and calls it from the audit loop, which Start launches during construction, so it cannot assume the handler is up — attachSequencer runs after newHandler in production, but the wiring tests build a bare Ethereum and nil-dereferenced it. That surfaced as one flaky package failure before it surfaced as a test; there is a deterministic one on it now.