High-impact

1. Multi-sentry routing silently breaks SendMessageById

pickEth71Peer records the sentry index where each candidate peer lives (sentryI) but
https://github.com/erigontech/erigon/blob/feat/eip-8159/fetcher-downloader/p2p/sentry/sentry_multi_client/bal_downloader.go#L297-L301:
type candidate struct {
peerID [64]byte
sentryI int // collected but never returned
}
...
return c.peerID, true, nil

MultiClient.FetchBlockAccessLists (sentry_multi_client.go:642) then picks an independent sentry via randSentryIndex() — the same pattern the body/header downloaders use, but those
send via SendMessageByMinBlock (broadcast). For SendMessageById, this is broken in multi-sentry mode: GrpcServer.SendMessageById returns (reply, nil) with empty reply.Peers when the
peer isn't on that sentry (sentry_grpc_server.go:1276-1280, including the explicit //TODO: enable after support peer to sentry mapping). The fetcher only checks err, so it registers
the in-flight request and waits the full 30 s defaultFetchTimeout. With N sentries, ~(N-1)/N requests time out before any progress.

In production today it's almost always a single sentry, so this is latent — but the sentryI collection and discard implies the author was aware and the wiring is incomplete. Either:

• thread sentryI through MultiClient.FetchBlockAccessLists and use that exact sentry, or
• route through sentryProvider.Multiplexer (which fans out across all sentries — node/components/sentry/provider.go:326).

2. SendMessageById zero-peers reply not handled (bal_fetcher.go:167-169)

Even on the correct sentry, the peer can disconnect between pickEth71Peer and SendMessageById. The grpc server then returns (reply, nil) with reply.Peers == nil. Fetcher silently
waits 30 s. Worth adding:
sent, err := sentry.SendMessageById(ctx, &outreq, &grpc.EmptyCallOption{})
if err != nil { return nil, ... }
if sent == nil || len(sent.Peers) == 0 { return nil, ErrPeerGone /* or similar */ }

3. Head-reorg → innocent peers kicked (bal_downloader.go:177-209, bal_fetcher.go:202-204)

collectMissingBALs snapshots header.BlockAccessListHash from the canonical chain at scan time. With scanInterval=10s and defaultFetchTimeout=30s, a head reorg can complete between
scan and validation. The fetcher then sees the BAL bytes for the new canonical block, computes a non-matching hash against the old expected hash, and PenalizePeer(Kick)s an honest
peer. Restricting reorgs to the recent head suggests this only affects the last few entries, but it's still a real false-positive kick path.

Two reasonable mitigations: (a) skip expected != latest_canonical_hash slots silently rather than kick, or (b) re-fetch the canonical header.BlockAccessListHash inside fetchBatch
immediately before the kick.

Medium

4. Batch size cuts close to soft-limit (bal_downloader.go:121)

Constant batchSize = 32 × spec's "approximately 70 KiB" typical BAL ≈ 2.2 MiB, which exceeds the 2 MiB softResponseLimit enforced in AnswerGetBlockAccessListsQuery. Most batches will
hit the limit, peer truncates, fetcher pads with nil, downloader retries the trailing slots next pass. Functionally correct but wasteful. Drop to ~24 or compute against actual
observed BAL size on this chain.

5. DB read errors swallowed (bal_downloader.go:196)
   existing, _ := rawdb.ReadBlockAccessListBytes(tx, hash, n)
   Any real DB error gets re-treated as "missing", refetched, written, refetched, … forever. Either propagate or at least Debug-log.

6. Peers() errors silently dropped (bal_downloader.go:281-283)
   reply, err := sc.Peers(ctx, &emptypb.Empty{}, grpc.EmptyCallOption{})
   if err != nil { continue }
   Drops without logging — masks gRPC issues against external sentry.

7. Test coverage gaps

• bal_downloader.go (318 LOC) has zero unit tests. The PR description's go test -short ./p2p/... only covers the fetcher. collectMissingBALs walking, batching, parallel dispatch,
  persistence — all uncovered.
• bal_fetcher_test.go is missing tests for: timeout (30 s deadline), ctx.Done(), more-entries-than-requested kick path, short response (M < N entries) → trailing nils, length mismatch
  error, and empty input early return.

Minor

8. balRequest.deliver comment is wrong (bal_fetcher.go:69-70)

▎ "Closed on context cancel or timeout so a goroutine sending on it never leaks."

The channel is never closed. Leak-freedom comes from (a) buffered(1) + non-blocking send in Deliver, and (b) defer delete(f.inflight, reqID), which prevents future Delivers from
finding the request. That's fine but the comment misrepresents the mechanism.

9. Hardcoded 15 s initial delay (bal_downloader.go:91) — not configurable; trivial for tests.

10. Always-on across BAL-disabled chains (backend.go:711) — fine in practice because collectMissingBALs short-circuits on the first pre-Amsterdam header, but a
    chainConfig.AmsterdamTime != nil gate would be cheap to add and avoid even the first wake.

11. getBlockAccessLists71 deferred-Rollback pattern (sentry_multi_client.go:670-674)

The explicit tx.Rollback() after AnswerGetBlockAccessListsQuery plus the defer tx.Rollback() mirrors getBlockBodies66 exactly. Per CLAUDE.md lint rules ("Never remove an explicit
.Close() or .Rollback() — add defer as a safety net alongside it"), this is intentional and consistent with the existing pattern.

Spec compliance ✅

• Three-way decode (0x80 not-available, 0xc0 empty-BAL accepted only when expected == empty.BlockAccessListHash, else hash check) — implemented and tested.
• Validation via keccak256(payload) == header.BlockAccessListHash — correct.
• Positional alignment with shorter-than-requested allowed for soft-limit truncation — correct.
• softResponseLimit = 2 MiB honored on server side; MaxBlockAccessListsServe = 1024.
• Peer kick on hash mismatch — correct (per EIP §Security Considerations).
• Silent-withholding scoring deferred — acknowledged in PR description; reasonable.

Bottom line

The fetcher's validation/kick logic is solid and the BALFetcher primitive is well-shaped. The downloader's plumbing has the multi-sentry routing bug (#1) which is the only thing I'd
consider blocking, and the reorg-kick (#3) and soft-limit-overshoot (#4) are real but not urgent. The test coverage gap around bal_downloader.go (#7) is the biggest follow-up — easy
to land in a separate PR but worth at least a smoke test for collectMissingBALs.

Pull request overview

Implements the consumer side of EIP-8159 (eth/71 Block Access List exchange) by adding a BALFetcher primitive, a background BALDownloader that backfills missing BALs into rawdb, and enabling eth/71 advertisement by default. This integrates BAL fetching into node startup while keeping it negotiation-driven (no-op when no peers support eth/71).

Changes:

• Add BALFetcher (request/response correlation + hash/sentinel validation + peer penalization on bad payloads).
• Add BALDownloader background loop to scan recent canonical blocks for missing BALs and fetch/store them in batches.
• Enable eth/71 in defaults and start the downloader at node startup (fork-gated on Amsterdam).

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

🚨 New finding — Critical (blocking)

BLOCK_ACCESS_LISTS_71 (the response side) is not subscribed in any Recv*Loop. The fetcher's wire path is end-to-end broken in production.

sentry_multi_client.go:93 adds eth.ToProto[direct.ETH71][eth.GetBlockAccessListsMsg] (request) to RecvUploadMessageLoop but BlockAccessListsMsg (response) is missing from
RecvMessageLoop (line 121-129).

Trace for an inbound BlockAccessLists:

1. Peer's reply hits eth.BlockAccessListsMsg case at sentry_grpc_server.go:492.
2. hasSubscribers(MessageId_BLOCK_ACCESS_LISTS_71) returns false (no addMessagesStream call ever registered it).
3. → continue — message dropped silently. givePermit = true is also never reached, so the per-peer permit accounting may leak too.
4. Fetcher's Deliver is never called via real wire traffic; every FetchBlockAccessLists returns ErrFetchTimeout after 30s.

Confirmed by exhaustive search: BLOCK_ACCESS_LISTS_71 appears in ToProto/FromProto/ProtoIds, the gRPC fanout, and handleInboundMessage — but in zero MessagesRequest{Ids: [...]} calls
anywhere in the tree.

Why this wasn't caught:

• Unit tests use fakeSentry that calls Deliver directly — bypassing the gRPC subscription path entirely.
• The PR description acknowledges end-to-end devnet verification is blocked by the unrelated EIP-8037 bug at block 503; the earlier "two-node round trip" claim must have validated
  capability negotiation only, not actual BAL exchange.

Fix: add eth.ToProto[direct.ETH71][eth.BlockAccessListsMsg] to RecvMessageLoop's ids (mirrors how BlockBodiesMsg is wired). One line, plus an integration-style test that exercises the
gRPC stream path (or a smoke test with SentryClientDirect) so this regression class can't recur silently.

Partially addressed: downloader test coverage

The new bal_downloader_test.go (191 LOC) covers peerSupportsEth71 and pickEth71PeerFromSentries thoroughly — appropriate, since multi-sentry routing was the highest-risk fix in this
revision. But collectMissingBALs, fetchBatch, scanAndFetch, and Run still have no tests. These are the largest pieces of new code:

• collectMissingBALs walking semantics (scanDepth boundary, n==0 termination, pre-Amsterdam break, DB-error continue path)
• fetchBatch reorg revalidation logic (the high-impact fix from this revision)
• scanAndFetch parallel-batch dispatch and ctx cancellation under load

A smoke test against a MockBlockReader + in-memory kv.RwDB + fakeSentry covering at least the reorg-revalidation drop path and len(existing) > 0 skip would catch most regressions
cheaply. Reasonable to land separately, but the highest-impact behavior added in 763d39c (reorg revalidation) is currently asserted only by inspection.

Minor (non-blocking)

1. TestBALFetcher_TimeoutReturnsErrFetchTimeout doesn't test the timer branch. It uses context.WithTimeout(100ms), so the select fires ctx.Done() and returns ctx.Err(), not
   ErrFetchTimeout. The test only asserts err != nil and inflight cleanup, so it passes — but the 30s time.NewTimer path at bal_fetcher.go:198-199 is uncovered. A configurable timeout
   (or defaultFetchTimeout as a struct field) would let the test exercise that branch.
2. collectMissingBALs continue-then-underflow. When ReadBlockAccessListBytes errors at n == 0, we continue; the loop post-statement n-- underflows to MaxUint64; next iteration
   HeaderByNumber(MaxUint64) likely returns nil and breaks. One extraneous iteration on an exotic error path — wasteful, not buggy. Cheapest fix: move the if n == 0 { break } check to
   right before the n-- increment, or restructure as for n := headNum; ; n-- { ... if n == 0 { break } }.
3. All 4 concurrent batches dispatch to the same peer. pickEth71Peer is called once at the top of scanAndFetch; the maxConcurrent: 4 workers all use that single peer/sentryI pair.
   Other Erigon downloaders rotate peers per batch. For background priority work this is fine, but a slow peer serializes all four slots. Consider per-batch peer rotation as a follow-up.
4. keccak256 helper duplication at bal_fetcher.go:285-289 — minor, but crypto.Keccak256 likely exists at the standard location. Worth a follow-up grep but not worth a churn cycle
   here.

Bottom line

The fetcher's validation/kick logic, fetch-batch reorg-aware revalidation, and multi-sentry routing fix are all solid. The blocker is the missing BlockAccessListsMsg subscription —
without it, no real BAL response ever reaches the fetcher, so the whole stack is a no-op against actual peers. One-line wire-up fix + a test that exercises the gRPC stream end-to-end.
Test coverage for the downloader's loop bodies remains the biggest follow-up but is reasonable to land separately.

Non-blocking issues (worth addressing in this PR or follow-ups)

1. TestRecvMessageLoop_SubscribesToBlockAccessListsMsg doesn't actually test the subscription — sentry_multi_client_test.go:314-322 only asserts the
   eth.ToProto[ETH71][BlockAccessListsMsg] map. A regression that drops the entry from RecvMessageLoop's ids slice (sentry_multi_client.go:123-133) would NOT be caught — the very class
   of bug this guard is supposed to prevent. Cheap fix: extract the ids slice to a package-level var recvMessageIds = []sentryproto.MessageId{...} referenced by RecvMessageLoop and have
   the test assert membership. Avoids the "we extracted it just for testability" concern by having the loop genuinely read the var.
2. collectMissingBALs underflow path is fragile — bal_downloader.go:188-222. The if n == 0 { break } lives inside the missing-append branch (line 219), so the continues at lines 209
   and 212 skip it. At n == 0 the post-step n-- wraps to MaxUint64, then the next iteration only exits because HeaderByNumber(MaxUint64) returns nil and hits the if hdr == nil { break }.
   Currently safe-by-luck; restructuring as for n := headNum; ; n-- { ... if n == low { break } } removes the silent reliance on a downstream nil return.
3. bal_fetcher.go:236-238 recomputes keccak256(entry) twice on the kick path — once for bytes.Equal, once for the error formatter. Cache it locally:
   got := keccak256(entry)
   if !bytes.Equal(got, expected[:]) { ... ErrBadBALResponse, i, expected, got) }
4. Stale doc comment in parent-PR territory — p2p/protocols/eth/protocol.go:413-418 still says "An empty RLP list (0xc0) in slot i has two possible meanings". Post-#11553 the wire is
   unambiguous (0x80 = unavailable, 0xc0 = genuinely empty). The plan doc was updated (docs/plans/eip-8159-eth71-bal-exchange.md) but this docstring on BlockAccessListsPacket was missed.
   Easy one-line fixup here or in a follow-up.
5. bal_fetcher.go:285-289 reimplements keccak256 — common/crypto.Keccak256 already exists with a pooled state. Replace the local helper. (Yperbasis flagged in round 2; restating here
   as a follow-up.)
6. Downloader test coverage gap — bal_downloader_test.go only covers peerSupportsEth71 / pickEth71PeerFromSentries. The most behavior-sensitive change in this revision — fetchBatch's
   reorg revalidation drop path (bal_downloader.go:240-264) — is currently asserted only by inspection. A smoke test against MockBlockReader + in-memory kv.RwDB + fakeSentry covering at
   least the reorg-drop and len(existing) > 0 skip paths would catch most regressions cheaply. Reasonable as a follow-up; worth not letting it slide far.

Observations (informational, no action requested)

7. Use-after-rollback pattern in getBlockAccessLists71 — sentry_multi_client.go:683-687 calls tx.Rollback() then rlp.EncodeToBytes(response). response is []rlp.RawValue whose elements
   come from db.GetOne via rawdb.ReadBlockAccessListBytes, and kv_interface.go:360 documents "GetOne references a readonly section of memory that must not be accessed after txn has
   terminated". This exact pattern lives in getBlockBodies66 and is what CLAUDE.md guides toward (preserve explicit .Rollback() alongside defer), so it's a pre-existing repo convention
   rather than a new bug — likely safe in practice for short read txns because MDBX doesn't reclaim read pages until a writer commits a snapshot. Not a blocker for this PR; flagging the
   contract gap so the team can decide whether to revisit the convention separately.
8. All 4 concurrent batches share one peer — pickEth71Peer runs once per scanAndFetch (bal_downloader.go:116); a slow peer serializes all four slots. Fine for background priority
   work; per-batch peer rotation is a reasonable follow-up but no urgency.
9. No metrics — the plan doc mentions per-case fetcher metrics for production observability (docs/plans/eip-8159-eth71-bal-exchange.md:347); not in this PR. Reasonable to defer until
   the silent-withholding scoring layer arrives, since metrics shape will depend on it.
10. TestBALFetcher_TimeoutReturnsErrFetchTimeout documents that it doesn't actually exercise the time.NewTimer branch (uses ctx-cancel instead). Making defaultFetchTimeout a struct
    field would let a future test cover the timer path in <1s. Minor.