Skip to content

Delete bogus InvalidBestNode error#9364

Merged
mergify[bot] merged 4 commits into
sigp:unstablefrom
michaelsproul:delete-invalid-best-node
Jun 1, 2026
Merged

Delete bogus InvalidBestNode error#9364
mergify[bot] merged 4 commits into
sigp:unstablefrom
michaelsproul:delete-invalid-best-node

Conversation

@michaelsproul
Copy link
Copy Markdown
Member

Issue Addressed

On Glamsterdam devnets we started seeing Lighthouse nodes unable to start with errors like:

May 26 04:34:01.582 CRIT Failed to start beacon node reason: "Unable to load fork choice from disk: ForkChoiceError(ProtoArrayStringError("find_head failed: InvalidBestNode(InvalidBestNodeInfo { current_slot: Slot(23550), start_root: 0x2c70b1641c29ec46360c99f9a8512f077862cbbc603e16f4a423007d210b0c5f, justified_checkpoint: Checkpoint { epoch: Epoch(712), root: 0x2c70b1641c29ec46360c99f9a8512f077862cbbc603e16f4a423007d210b0c5f }, finalized_checkpoint: Checkpoint { epoch: Epoch(710), root: 0xede5e0b09b51bdb5445ade3398e685bd193b845e0b0ffb827f0c3fec8277ea51 }, head_root: 0x2c70b1641c29ec46360c99f9a8512f077862cbbc603e16f4a423007d210b0c5f, head_justified_checkpoint: Checkpoint { epoch: Epoch(710), root: 0xede5e0b09b51bdb5445ade3398e685bd193b845e0b0ffb827f0c3fec8277ea51 }, head_finalized_checkpoint: Checkpoint { epoch: Epoch(709), root: 0xbb243eff616ff362c52b83113e7c536d0a68cb9ca3d6a1cb1055e732219d9736 } })"))"

This error was the result of an overly-strict sanity check, based on assumptions that are not true under extreme network conditions.

Proposed Changes

Completely remove the InvalidBestNode failure path: it is not compliant with the spec, and is actively harmful when triggered (it prevents Lighthouse from starting at all). The error was reachable in any situation where all leaf nodes of fork choice were ineligible to be the head. The payload invalidation tests show some examples of cases where this would happen, and the newly-added regression test shows a contrived case where it can happen on a Gloas network without any slashings or invalid blocks. There are probably many more cases where it can happen.

We do not lose anything by removing it. The spec's implementation of get_head always returns something (unless it crashes), and in these cases it is correct to return the starting node of the traversal: the justified checkpoint block. This is what we now do, and what the new test verifies.

I've also added some facilities to the harness for injecting attestations with fixed payload_present fields. @hopinheimer found himself needing something similar when messing with reorg tests, so I think these are probably useful. It might be possible to do without them by juggling the payload reveal timing in just the right way, but I think this approach is just way simpler.

@michaelsproul michaelsproul added bug Something isn't working fork-choice v8.2.0 2026 Q2/Q3 new release labels May 28, 2026
@michaelsproul michaelsproul added the ready-for-review The code is ready for review label May 28, 2026
Copy link
Copy Markdown
Member

@eserilev eserilev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the changes and regression test make sense to me. LGTM!

@michaelsproul michaelsproul added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Jun 1, 2026
@mergify mergify Bot added the queued label Jun 1, 2026
@mergify
Copy link
Copy Markdown

mergify Bot commented Jun 1, 2026

Merge Queue Status

This pull request spent 29 minutes 55 seconds in the queue, including 27 minutes 52 seconds running CI.

Required conditions to merge

mergify Bot added a commit that referenced this pull request Jun 1, 2026
@mergify mergify Bot merged commit 74a5609 into sigp:unstable Jun 1, 2026
38 checks passed
@mergify mergify Bot removed the queued label Jun 1, 2026
@michaelsproul michaelsproul deleted the delete-invalid-best-node branch June 1, 2026 01:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working fork-choice ready-for-merge This PR is ready to merge. v8.2.0 2026 Q2/Q3 new release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants