fix(tn-reth): floor snapshot-scaffold heights out of the canonical execution check - #1325
Open
MavenRain wants to merge 5 commits into
Open
fix(tn-reth): floor snapshot-scaffold heights out of the canonical execution check#1325MavenRain wants to merge 5 commits into
MavenRain wants to merge 5 commits into
Conversation
…odes A snapshot-restored validator scaffolds the header region below its restored-state floor B: zero-hash dummies below the shipped window and real-hashed but stateless headers within it. The execution-result check that gates voting (wait_for_execution) read those heights and accepted them on both paths, so a Byzantine latest_execution_block pointing at a scaffold height cleared the check on restored nodes. With a restored voting quorum the poison header certified, and at commit time every full-history validator failed the same check on the sub-dag leader and self-halted. Closes #1323. Close both accept paths at the source: - RethEnv::canonical_execution_hash returns None below the restored-state floor, mirroring read_only_state_db, so the canonical-DB fallback never attests a scaffold block. - RecentBlocks::contains_execution_hash rejects the zero hash outright, so a dummy seeded into the ring can never match; sound independent of the restore scan-floor work in #1321. Tests: a new ring unit test rejecting a zero-hash dummy, and the restored floor scaffold test extended to assert canonical_execution_hash refuses below B and answers truthfully at B. Both guards mutation-confirmed. Signed-off-by: Onyeka Obi <softwareengineerasaservant@isurvivable.cv>
…fix The scaffold dummies a snapshot restore fabricates are written only to the reth static files, never seeded into the RecentBlocks ring: the ring is primed from last_executed_output_blocks, which carry real sealed hashes. The ring is therefore not a reachable accept path for a scaffold height, so the zero-hash reject added in beba215 was defence-in-depth against a path that cannot occur. The blanket reject also broke the legitimate {0, B256::ZERO} genesis / no-exec sentinel that a healthy node queries through wait_for_execution before its first execution: the query missed, wait_for_execution returned Err, and the leader's sub-dag was declared bogus, flipping the node to CvvInactive and self-halting. That hung two tn-node integration tests (test_entry_reads_static_fee_at_boundary, test_sync_then_catchup_recovers_two_worker_accumulator) for 30s each. Keep the load-bearing half: the restored_state_floor guard in canonical_execution_hash (the DB fallback), which is the only reachable accept path for a scaffold height and closes #1323 on its own. Refs #1323 Signed-off-by: Onyeka Obi <softwareengineerasaservant@isurvivable.cv>
…onicity-floor Signed-off-by: Onyeka Obi <softwareengineerasaservant@isurvivable.cv>
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
1 participant
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 (Cantina #28)
Fixes #1323. A snapshot restore scaffolds every pre-window execution header
with a
B256::ZEROplaceholder hash so the reth static files stay contiguousbelow the restore window. The vote-time canonical-execution check read those
placeholders directly from the DB, so a snapshot-restored validator would
attest a
(scaffold_height, B256::ZERO)pair as canonical execution and let apeer clear the execution-result gate against a block that was never executed.
Fix
RethEnvnow carries the restore window startBasrestored_state_floor.canonical_execution_hashrefuses any lookup below the floor, so a scaffoldheight resolves to
Noneinstead of the zero placeholder and can never beattested as canonical. On a fresh or normally-synced node
restored_state_flooris
None, so the guard is a strict no-op.Scope note
An earlier revision of this change also rejected the zero hash inside the
RecentBlocksring as defence-in-depth. That reject was dropped for tworeasons:
the reth static files, never seeded into the
RecentBlocksring. The ring isprimed from
last_executed_output_blocks, which carry real sealed hashes, soa scaffold height is never a ring member. The canonical-DB floor guard is the
only reachable accept path for a scaffold height, and it is fully closed here.
{0, B256::ZERO}is the default"genesis / no execution yet" execution pointer that healthy nodes query
through
wait_for_executionbefore their first execution. A blanket zero-hashreject in the ring makes that query miss and hangs the gate; two
tn-nodeintegration tests reproduced the hang.
The canonical-DB floor guard is the load-bearing fix and closes #1323 on its own.
Testing
tn-rethunit test assertingcanonical_execution_hashreturnsNonefor aheight below the restore floor and the real hash at/above it.
(mutant killed).
tn-nodeintegration tests that caught the earlier ring-guardregression (
test_entry_reads_static_fee_at_boundary,test_sync_then_catchup_recovers_two_worker_accumulator) pass single-threaded.fmt --check,check --all-targets,clippy --all-targets --no-deps -D warningson
tn-reth.