erigontech / erigontech/erigon

cl/phase1/network: blob backfill still derives block roots from payload-stripped blocks

Open
#23,896 3 comments 0 reactions 1 assignee Claimed by @lystopad View on GitHub
Caplin
Dominant language
Go
Stars
3.6k
Forks
1.5k
Avg merge
1d 16h
Merged PRs (30d)
455

Description

#23895 fixes one site where a block root was derived by hashing a payload-stripped block. Four
more remain in the same subsystem. They share a cause: `ReadBeaconBlockBodyBySlot` returns a
block **without its execution payload**, so `block.Block.HashSSZ()` yields a root that never
existed on chain.

## Part 1 — store lookups keyed on the wrong root

`cl/phase1/network/blob_downloader.go`, all feeding `storedBlobSidecarsComplete`, which reads
`KzgCommitmentsCount` and `ReadBlobSidecars`:

- `retryBlock` — block from `readRetryBlock` → `ReadBeaconBlockBodyBySlot`
- `recoverDenebBlobs` — blocks from `collectIncompleteBlocks`
- `recoverFuluColumns` — same

Consequence: a slot whose sidecars were stored successfully never reports complete, so
`addRetrySlot` re-queues it and `resolveRetrySlot` never clears it. Work already done is redone
indefinitely.

Why it is not in #23895: the fix needs the canonical root carried alongside the block rather than
re-derived per call. A helper opening a read-only transaction per block is the wrong shape inside
those loops, and it panics in unit tests that construct the downloader without a DB. Changing
those signatures churns a number of tests, so it wants its own change.

## Part 2 — the wrong root goes on the wire

`cl/phase1/network/blobs.go:50`, in `BlobsIdentifiersFromBlocks`, also hashes the block. Those
identifiers are what get requested from peers and what `denebRecoveryBatch.groups` is keyed by,
so peers are asked for `(root, index)` pairs whose root never existed. Historical blob backfill
therefore fails at the network level, not only in bookkeeping, and this is likely degrading blob
backfill on **every** chain today rather than only the short-retention ones.

This cannot be changed in place: the only other caller is `cmd/capcli`, which passes *full*
blocks where the hash is correct. It needs a roots-aware variant, with the backfill supplying
canonical roots.

## Notes for whoever picks this up

- `PruneBlocks` only touches `kv.BeaconBlocks`, so `kv.CanonicalBlockRoots` survives pruning and
is a safe source for frozen slots. Worth re-confirming for `prune.mode=minimal`.
- A regression test here passes under mutation if the fixture seeds the canonical root *as*
`block.HashSSZ()` — the two roots become identical and the bug cannot manifest. Keep them
distinct.
- Severity scales with whichever retention window applies, and both are epoch-based. For
post-Fulu slots blobs are reconstructed from data columns, so
`MIN_EPOCHS_FOR_DATA_COLUMN_SIDECARS_REQUESTS` (4096, no per-chain override) binds: 18.2 days
on mainnet/sepolia/hoodi at 32 slots and 12s, versus **3.79 days** on gnosis/chiado at 16 slots
and 5s. Past it the data is gone network-wide, so a gap becomes permanent about 4.8x faster on
gnosis/chiado. Gnosis separately sets `MIN_EPOCHS_FOR_BLOB_SIDECARS_REQUESTS = 16384` (15.2
days), which is the window #23752 concerns — not the one that binds for Fulu-era slots.

## Related

- #23224 — the wedge this class of bug produces: `blob storage count mismatch at slot 29403906`
retried every 12s forever. That slot was repaired out-of-band on 2026-09-09 and the frontier
then advanced, which answers its open question; see the comment there.
- #23752 — a different reason blob backfill cannot converge: the pruning floor rejects writes
inside the advertised backfill window.
- #23252 / #23408 — backoff so a failing retirement step stops hammering; merged to `main` and
`release/3.6`, **not** on `release/3.5`.
- #23223 — the compressor-worker leak that made the wedge fatal rather than merely wasteful.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.