execution/stagedsync: drop stale eth/71 TODO in bal_create#21511
Open
yperbasis wants to merge 1 commit into
Open
execution/stagedsync: drop stale eth/71 TODO in bal_create#21511yperbasis wants to merge 1 commit into
yperbasis 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>
Contributor
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
approved these changes
May 29, 2026
Contributor
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
approved these changes
May 30, 2026
Contributor
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
approved these changes
May 30, 2026
Contributor
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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