Feature/optimistic sync - #1576

alok-108 · 2026-09-12T16:00:43Z

What was wrong?

Currently, the beacon node halts or rejects block imports if the execution layer is not yet synced or returns SYNCING / ACCEPTED (only VALID was permitted). Furthermore, historic block range syncing eagerly called the execution engine without optimistic synchronization support, slowing down sync and lacking a rollback mechanism for invalid execution payloads.

How was it fixed?

Optimistic Sync Engine Handling: Updated verify_and_notify_new_payload to return PayloadStatusV1 and allow block imports on VALID, ACCEPTED, and SYNCING.
Optimistic Block Range Syncer: Added --optimistic-sync support to BlockRangeSyncer with is_optimistic_candidate_block safe distance logic (128 slots) and process_block_optimistic for deferred EL verification.
Root Tracking: Added OptimisticRootsTable to BeaconDB to track roots of optimistically imported blocks.
Invalid Payload Recovery & Rollback: Implemented handle_invalid_payload to prune invalid blocks, dependent states, and column sidecars, back-tracking to latest_valid_hash without deadlock.
Testing: Added unit and integration tests covering optimistic candidate filtering, invalid payload rollback, and table persistence.

Closes #395

To-Do

I have read CONTRIBUTING.md.
This PR title follows conventional commits.

- Add skip_execution_validation parameter to on_block - Introduce OptimisticRootsTable for tracking optimistic blocks - Implement process_block_optimistic and handle_invalid_payload - Add is_optimistic_candidate_block helper (SAFE_SLOTS_TO_IMPORT_OPTIMISTICALLY = 128) - Extend BlockRangeSyncer with optimistic_mode and new_optimistic constructor - Wire --optimistic-sync CLI flag (default true) into ManagerConfig and service - Hook PayloadStatus::Invalid to handle_invalid_payload for chain backtracking

… remove

Bugs fixed: - handle_invalid_payload: after removing blocks from DB, store.get_head() called filter_block_tree() which still traversed parent_root_index_multimap and hit bail!("failed to get block") for the just-removed block. Fix: - Track last_valid_root during the head-to-invalid traversal loop so the new head is known without re-querying the fork choice tree post-removal. - Also clean slot_index_provider, parent_root_index_multimap (new remove_child()), unrealized_justifications_provider for each removed root so the in-memory fork choice tree stays fully consistent. - Add remove_child(parent_root, child_root) to ParentRootIndexMultimapTable. - Fix unused-mut warning in BeaconBlockTable::remove(). Tests added: - test_optimistic_root_storage: B256->bool REDB insert/remove verified - test_invalid_payload_multi_descendant_rollback: A->B->C->D chain, C invalid, C+D removed, B becomes head, optimistic roots cleaned - Unit tests for is_optimistic_candidate_block safe-distance logic: test_within_safe_distance_is_candidate, test_beyond_safe_distance_not_candidate_by_distance, test_safe_slots_constant (SAFE_SLOTS_TO_IMPORT_OPTIMISTICALLY == 128), test_make_block_fields All 3 beacon chain tests pass. Related to ReamLabs#1395

… sync code

KolbyML · 2026-09-13T22:22:14Z

@perfogic is this PR fine for me to review, I seen the other 2 are blocked on your concern.

…nge target - Align is_optimistic_candidate_block with Ethereum spec: block must be >= 128 slots behind head to be a candidate based on distance. - Add safety checks to handle_invalid_payload so unimported invalid blocks do not purge the canonical chain. - Decouple BlockRangeSyncer from head-sync by targeting finalized_slot, avoiding conflict with EPF Cohort 7 head-sync work. - Fix clippy warnings across execution-engine, beacon-state, and syncer. - Add test_handle_invalid_payload_unimported_block_preserves_head.

perfogic · 2026-09-17T06:02:34Z

@KolbyML Sorry for the late reply, I thought you were asking @alok-108.

Yes, you can. Our head sync currently only imports blocks when the EL returns VALID.
With proper optimistic sync support, we could also import blocks with SYNCING or ACCEPTED responses.

alok-108 · 2026-09-17T13:01:52Z

Thanks @perfogic and @KolbyML!

Exactly — with the changes in this PR:

verify_and_notify_new_payload now returns PayloadStatusV1, allowing blocks to be imported when the EL returns SYNCING or ACCEPTED rather than bailing on anything that isn't VALID.
Historic range sync supports --optimistic-sync via process_block_optimistic and tracks optimistic roots in OptimisticRootsTable.
An invalid payload cleanup/rollback mechanism (handle_invalid_payload) is in place if the EL later marks an imported block invalid.

Looking forward to your review @KolbyML, and happy to address any comments or adjustments!

…nd resolve validated ancestors - Propagate PayloadStatus from state_transition to on_block in fork choice - Mark blocks as optimistic in OptimisticRootsTable when EL status is Syncing/Accepted or execution validation is skipped - Resolve optimistic roots recursively on Valid execution payload - Query db.optimistic_roots_provider dynamically in calculate_sync_status for Beacon Node RPC - Fix pattern match bug on PayloadStatusV1 in gossipsub beacon_block validation - Add comprehensive ancestor resolution unit test in ream-chain-beacon

alok-108 · 2026-09-17T13:36:56Z

Update: Implementation & Test Verification

Hey, quick update on this PR. I've pushed the remaining optimistic sync spec changes

Here's what's new:

Fork choice now tracks optimistic roots on the fly. We're passing the execution payload status (Option<PayloadStatus>) from state_transition into on_block. So when a block comes in with SYNCING / ACCEPTED (or when skip_execution_validation = true), it gets registered in OptimisticRootsTable right away.
Optimistic ancestors get resolved recursively. Once the EL says a payload is VALID, we walk up the parent chain and unmark all its optimistic ancestors in OptimisticRootsTable.
The node syncing RPC is actually dynamic now. GET /eth/v1/node/syncing checks db.optimistic_roots_provider().get(head) instead of just returning false all the time.
Fixed the gossipsub validation. The payload status pattern matching in beacon_block.rs was off, so I cleaned that up.

Tests

Ran everything locally, and it's all passing:

ream-chain-beacon
cargo test --features devnet5 -p ream-chain-beacon
5/5 passed:
- test_optimistic_root_storage
- test_optimistic_ancestor_resolution (checks that ancestors get resolved on a valid payload)
- test_handle_invalid_payload_cleanup
- test_invalid_payload_multi_descendant_rollback
- test_handle_invalid_payload_unimported_block_preserves_head
ream-syncer
cargo test --features devnet5 -p ream-syncer
6/6 passed:
- Distance check with SAFE_SLOTS_TO_IMPORT_OPTIMISTICALLY
- Range and block cache fetch tests
Checks:
- cargo check --features devnet5 -p ream-fork-choice-beacon — clean
- cargo check --features devnet5 -p ream-rpc-beacon — clean
- cargo check --features devnet5 -p ream-network-manager — clean

Branch is rebased, tests are green, and this PR is ready for review and merge 🚀

alok-108 · 2026-09-18T12:10:20Z

Hey @KolbyML 👋
Hope you're doing well! Just a gentle check-in on this PR — totally understand you're busy. Any chance you might be able to take a look sometime? No rush at all, just wanted to get a rough idea. Everything's passing locally and it's ready from my side.
Thanks a lot!

KolbyML

I left 4 comments, the same issues reoccur a lot in the pR, please fix them all then rerequest my review again

KolbyML · 2026-09-19T19:34:26Z

+        if let Err(e) = result {
+            let err_str = e.to_string();
+            if err_str.starts_with("INVALID_PAYLOAD") {
+                let block_root = signed_block.message.tree_hash_root();
+                let latest_valid = if let Some((_, hash_str)) = err_str.split_once(':') {
+                    hash_str
+                        .parse::<alloy_primitives::B256>()
+                        .unwrap_or(B256::ZERO)
+                } else {
+                    B256::ZERO
+                };
+                self.handle_invalid_payload(store, block_root, latest_valid)
+                    .await?;
+                bail!("Block payload is invalid");
+            }
+            return Err(e);
+        }


Err(err) and we shouldn't be parsing errors like this, we should be doing type checks

KolbyML · 2026-09-19T19:34:53Z

+            true, // skip execution validation
+        )
+        .await?;
+
+        // Insert the root as optimistic in the database


these commands don't add anything, they should be removed

KolbyML · 2026-09-19T19:35:46Z

+            let _ = store.db.slot_index_provider().remove(slot);
+            let _ = store
+                .db
+                .parent_root_index_multimap_provider()
+                .remove_child(parent_root, root);
+            let _ = store.db.unrealized_justifications_provider().remove(root);
+            let _ = store.db.optimistic_roots_provider().remove(root);


why aren't we handling results, please fix this across the whole pr

KolbyML · 2026-09-19T19:35:58Z

+                .db
+                .block_provider()
+                .get(justified_checkpoint.root)?
+                .map(|b| b.message.body.execution_payload.block_hash)


- Replace string error parsing with typed EngineError enum (InvalidPayload, InvalidBlockHash) - Remove redundant optimistic root insertion and dead comment in process_block_optimistic - Properly propagate database Results with '?' across fork choice, chain, and store instead of discarding with 'let _ =' or '.ok()' - Handle file deletion idempotently for missing blob and column sidecars - Clarify variable naming (rename 'b' to 'beacon_block', fix 'execution_enigne' typo) - Add comprehensive tests for EngineError downcast and latest_valid_hash: None rollback behavior

alok-108 · 2026-09-20T20:32:00Z

Thanks @KolbyML for the detailed review. I've addressed all four points in 9fd9a41:

Typed errors: Added an EngineError enum with InvalidPayload { latest_valid_hash: Option<B256> } and InvalidBlockHash. process_execution_payload now returns these instead of string bail, and process_block uses err.downcast_ref::<EngineError>() for type-safe matching.
Dead code: Removed the redundant optimistic roots insertion and the stale comment from process_block_optimistic. on_block already handles it when skip_execution_validation = true.
DB results: Replaced all let _ = and .ok() discards with ? propagation across beacon_chain.rs, handlers.rs, store.rs, and syncing.rs. File deletions handle NotFound idempotently while bubbling real I/O errors.
Naming: Renamed b to beacon_block, e to err, and fixed the execution_enigne typo to execution_engine.

Also added tests for the EngineError downcast and the latest_valid_hash: None rollback. All tests green locally:

ream-chain-beacon: 5/5 pass
ream-syncer: 6/6 pass
cargo clippy and cargo fmt --check: clean

Thanks again for the review.

alok-108 added 5 commits September 12, 2026 13:01

test: add invalid payload cleanup test and fix redb deadlock in block…

fef26c7

… remove

refactor: improve invalid payload logging and handling

9bf0804

fix(syncer): add missing fork choice dependency and format optimistic…

7485994

… sync code

alok-108 requested review from Kayden-ML, KolbyML and shariqnaiyer as code owners September 12, 2026 16:00

This was referenced Sep 12, 2026

Add optimistic sync so the node can sync faster #1395

Closed

docs(projects): add project proposal for Ream Beacon Sync and PeerDAS Sampling eth-protocol-fellows/cohort-seven#410

Open

KolbyML reviewed Sep 19, 2026

View reviewed changes

alok-108 requested a review from KolbyML September 20, 2026 20:42

Navigation Menu

Feature/optimistic sync - #1576