Skip to content

Conversation

@fmoletta
Copy link
Contributor

@fmoletta fmoletta commented Oct 21, 2025

Motivation
In hive tests we often receive an fcu with either the genesis or last imported block as head to notify the node that it is already synced. As this is not usual in a real-case scenario, we just trigger a snap sync to the advertised head. This PR solves this by first checking if the fcu head is already part of our canonic chain before triggering a sync

Description

  • When handling a forkchoice in snap sync mode, check that the head is not already canonical before triggering a sync, if it is already canonical, apply the forkchoice

Closes #4846

@github-actions github-actions bot added the L1 Ethereum client label Oct 21, 2025
@github-actions
Copy link

github-actions bot commented Oct 21, 2025

Lines of code report

Total lines added: 10
Total lines removed: 0
Total lines changed: 10

Detailed view
+----------------------------------------------------+-------+------+
| File                                               | Lines | Diff |
+----------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/sync_manager.rs       | 132   | +3   |
+----------------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/engine/fork_choice.rs | 367   | +7   |
+----------------------------------------------------+-------+------+

@fmoletta fmoletta marked this pull request as ready for review October 21, 2025 19:51
@fmoletta fmoletta requested a review from a team as a code owner October 21, 2025 19:51
@ethrex-project-sync ethrex-project-sync bot moved this to In Review in ethrex_l1 Oct 21, 2025
}

if context.syncer.sync_mode() == SyncMode::Snap {
// Don't trigger a sync if the block is already canonical
Copy link
Collaborator

Choose a reason for hiding this comment

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

In that case, will the sync mode be set as Full somewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The next FCU to an unknown head will trigger a sync, the syncer will detect that the sync_head is only a few blocks away and switch to full sync mode

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Update: changed to also disable snap sync so we can process incoming new payloads right away

Copy link
Collaborator

@mpaulucci mpaulucci left a comment

Choose a reason for hiding this comment

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

Please verify that hive --sim ethereum/engine --sim.limit "engine-auth/" passes all tests when on snap sync mode, see https://github.com/ethereum/hive/blob/7709e5892146c793307da072e1593f48039a7e4b/clients/ethrex/ethrex.sh#L79

@github-project-automation github-project-automation bot moved this from In Review to In Progress in ethrex_l1 Oct 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

L1 Ethereum client

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

[Snap Sync] Ethrex returns syncing when calling FCU on the genesis block

3 participants