execution/stagedsync: drop stale eth/71 TODO in bal_create#21511
execution/stagedsync: drop stale eth/71 TODO in bal_create#21511yperbasis wants to merge 1 commit into
Conversation
The `if dbBALBytes != nil` guard in ProcessBAL was annotated "Remove after eth/71 has been implemented." eth/71 (EIP-8159) is now implemented — BAL sidecars are fetched by the always-on background BALDownloader — but that downloader is best-effort and explicitly never blocks stage progress, so a missing stored BAL remains a normal, expected state and the guard must stay. Only the stale instruction is removed; the comment is reworded to document why the nil-check is required. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR updates a stale comment in ProcessBAL to reflect that eth/71 BAL exchange is implemented, while stored BAL sidecars can still be absent because background backfill is best-effort.
Changes:
- Removes the obsolete TODO about deleting the nil guard after eth/71 implementation.
- Adds a comment explaining why the stored BAL cross-check remains conditional.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Giulio2002
left a comment
Giulio2002
left a comment
There was a problem hiding this comment.
LGTM — updates a stale comment to match the current best-effort BAL sidecar behavior; no logic change.
Giulio2002
left a comment
Giulio2002
left a comment
There was a problem hiding this comment.
LGTM — comment-only cleanup clarifying BAL sidecar handling; no behavior change.
Giulio2002
left a comment
Giulio2002
left a comment
There was a problem hiding this comment.
LGTM — removes a stale TODO and clarifies why the nil BAL sidecar check must remain; comment-only, no logic change.
Giulio2002
left a comment
Giulio2002
left a comment
There was a problem hiding this comment.
LGTM — stale TODO/comment cleanup only; no logic change.
Giulio2002
left a comment
Giulio2002
left a comment
There was a problem hiding this comment.
LGTM — comment-only clarification; no code behavior change.
Giulio2002
left a comment
Giulio2002
left a comment
There was a problem hiding this comment.
LGTM — comment-only clarification of BAL sidecar handling; no behavior change.
Giulio2002
left a comment
Giulio2002
left a comment
There was a problem hiding this comment.
LGTM — comment-only clarification matching current best-effort BAL sidecar behavior; no logic change.
Giulio2002
left a comment
Giulio2002
left a comment
There was a problem hiding this comment.
LGTM — comment-only clarification removing a stale eth/71 TODO; behavior is unchanged.
Giulio2002
left a comment
Giulio2002
left a comment
There was a problem hiding this comment.
LGTM — trivial comment cleanup clarifying best-effort BAL sidecar cross-check behavior; no code-path change.
Giulio2002
left a comment
Giulio2002
left a comment
There was a problem hiding this comment.
LGTM — removes a stale TODO/comment and replaces it with a more accurate explanation of current best-effort BAL sidecar handling.
Giulio2002
left a comment
Giulio2002
left a comment
There was a problem hiding this comment.
LGTM — comment-only clarification of best-effort BAL sidecar handling; no behavior change.
What
ProcessBAL'sif dbBALBytes != nilguard carried the comment "Remove after eth/71 has been implemented." This drops that stale instruction and rewords the comment to document why the nil-check must stay. Comment-only; no behaviour change.Why
eth/71 (EIP-8159, Block Access List Exchange) is now implemented — BAL sidecars are fetched over the wire by the always-on background
BALDownloader(p2p/sentry/sentry_multi_client/bal_downloader.go) and served viaAnswerGetBlockAccessListsQuery. So the premise behind the TODO — "once eth/71 lands, BALs will always be stored, so this guard becomes dead" — no longer holds.The downloader is best-effort by design and never blocks stage progress (per its own header doc);
ProcessBALregenerates and validates the BAL locally regardless, so a missing p2p-delivered BAL "is never a correctness issue — only a CPU-cost optimisation." A missing stored BAL (dbBALBytes == nil) therefore remains a normal, expected state, and the guard is still required — removing it would turn that normal state into a hard failure.Net: eth/71 being implemented invalidates the TODO rather than enabling it.
🤖 Generated with Claude Code