[domiwei]

Fixes #25205

Stacked on #25212: review only the last four commits. The last commit updates one test of this PR for the ReadRequiredQueueSSZ signature from #25212.

This PR must ship in the same release as #25231 (builders per-index diff) and #25234 (builder pending payments base plus deltas), which are stacked on it. Without them, a Gloas node writes builders rows that rewrite the whole list on every in-place change.

Problem

The state antiquary stores per-slot diffs of the Electra queues (pending_deposits, pending_consolidations, pending_partial_withdrawals) and the Gloas lists (builders, builder_pending_withdrawals, payload_expected_withdrawals) with SSZQueueEncoder. The encoder remembered only the previous size and the previous last element. It searched the new list backwards for that element and wrote "truncate N from the front" plus the elements after it. Two cases broke this:

• builders changes in place (withdrawals, deposits, exits). When a builder that is not the last one changed, the diff was "truncate 0, append nothing", so the change was lost until the next dump.
• When an appended entry equalled the previous last entry, the search picked the appended copy, the truncate count went negative and wrapped in uint32, and replay emptied the queue.

Fix

The diff byte format and the replay format are unchanged.

• Initialize keeps a copy of the previous list's element pointers in a slice the encoder owns.
• WriteDiff first tries a fast path based on pointer identity: queue elements are copy-on-write, so equal pointers hold equal values. If the previous list's tail and the new list's head hold the same pointers, it writes the diff directly. This covers unchanged queues, append-only slots and front pops plus appends.
• Otherwise it finds the longest suffix of the previous list that equals a prefix of the new list with KMP, which is linear in the list sizes, and writes truncate = prevLen - overlap plus new[overlap:]. An in-place builder change becomes a full rewrite of the list for that slot.
• ApplySSZQueueDiff returns the new ErrCorruptQueueDiff when the truncate count is larger than the queue length, before it changes the queue. The readers wrap it, and the antiquary resume backs off on it like it does for ErrMissingGloasData.

Upgrade note

There is no migration: the stored bytes of a wrong diff cannot be repaired, and each wrong window ends at the next dump (at most 1535 slots). Re-antiquation does not repair rows already frozen in Caplin state snapshot files, and nothing regenerates a frozen range automatically. On Sepolia the Gloas range written by older releases stays wrong. Electra ranges are affected only in the rare case where two equal queue entries were appended in a row.

Tests

• Collector tests: an in-place change to builder 0 is kept, and an appended duplicate builder pending withdrawal no longer empties the queue.
• Encoder tests: in-place Set on the same list, duplicate last append, front cut plus postponed re-append, the fast path and its fallback when equal values have different pointers, truncate == len accepted and truncate > len rejected without changing the queue, an exhaustive round trip over all small lists with duplicates, and a check that the overlap search makes a linear number of equality calls.
• Reader and antiquary tests: the corrupt-diff error is wrapped by both readers, and the antiquary backoff includes it.
• Benchmark in ssz_queue_bench_test.go: at 100k pending_deposits entries, a diff takes about 0.24 ms.

TDD: the tests were written first and failed on the old code for the intended reason, except the truncate == len check and the linear-call check, which also pass on the old encoder and guard the new one. The fast-path test failed before the fast path was added.

Validation

• go test ./cl/persistence/... ./cl/antiquary/... -count=1
• make lint, two runs, 0 issues
• make erigon integration

Follow-ups (not in this PR)

• builders gets its own per-index row format in cl/persistence, cl/antiquary: store builders history as a per-index diff #25231, so an in-place builder change no longer rewrites the whole list.
• No end-to-end test drives the antiquary backoff through a stored corrupt row.
• Copy-on-write of queue elements is a convention, not enforced by the type.

[Copilot]

🔵 Needs a closer look

Persistent-state encoding changes, an unresolved overlap optimization issue, and required coordination with stacked Gloas fixes warrant human validation.

1 open issue

• Pointer shortcut misses maximal overlap for duplicate equal values · New

What changed in this PR

This PR fixes historical-state queue diffs in Caplin while preserving the stored byte format. It addresses lost in-place changes and duplicate entries that could produce invalid truncate counts.

Changes:

• Retains previous queue pointers and adds pointer-based and value-based overlap searches.
• Rejects oversized truncation, adds reader error context, and enables antiquary backoff on corrupt diffs.
• Adds regression tests, exhaustive round-trip checks, and queue benchmarks.

File	Description
cl/​persistence/​state/​historical_states_reader/​historical_states_reader.go	Adds table and slot context to diff errors.
cl/​persistence/​state/​historical_states_reader/​gloas_roundtrip_test.go	Tests corrupt-diff error wrapping in both readers.
cl/​persistence/​base_encoding/​ssz_queue.go	Replaces tail matching with overlap searches and validates truncation.
cl/​persistence/​base_encoding/​ssz_queue_test.go	Tests replacements, duplicates, overlap selection, and corruption handling.
cl/​persistence/​base_encoding/​ssz_queue_bench_test.go	Benchmarks large pending-deposit queue diffs.
cl/​antiquary/​state_antiquary.go	Includes corrupt queue diffs in resume backoff.
cl/​antiquary/​state_antiquary_test.go	Tests backoff error classification.
cl/​antiquary/​beacon_states_collector_test.go	Adds builder-change and duplicate-withdrawal regressions.

_{Review effort: Balanced}

[lystopad]

LGTM. Reviewed head 861039b29d (the four commits on top of #25212).

Checked the copy-on-write premise of the pointer fast path against every builder mutation site (deposit, withdrawal, exit, top-up): all copy and Set a new pointer. KMP bounds and the pre-mutation truncate > len rejection look right; the antiquary back-off on ErrCorruptQueueDiff is bounded. cl/persistence/... and cl/antiquary/... pass with -race.

On the overlap-maximality point: agreed it's non-blocking, since replay is correct and the extra size is bounded by the slot's appended entries.

Reminder for release planning: this needs #25231 and #25234 in the same release.

[Copilot]

🔵 Needs a closer look

Historical-state persistence and recovery changes require final human validation and coordinated release with #25231 and #25234.

No open issues 🎉

1 resolved since last review

• Pointer shortcut misses maximal overlap for duplicate equal values

_{Review effort: Balanced}