diff --git a/.github/workflows/pr-main_l1.yaml b/.github/workflows/pr-main_l1.yaml index 394f1a0026..b902f7a42d 100644 --- a/.github/workflows/pr-main_l1.yaml +++ b/.github/workflows/pr-main_l1.yaml @@ -174,11 +174,10 @@ jobs: simulation: ethereum/sync test_pattern: "" ethrex_flags: "--syncmode full" - # Flaky test, reenable when fixed - # - name: "Sync snap" - # simulation: ethereum/sync - # test_pattern: "" - # ethrex_flags: "--syncmode snap" + - name: "Sync snap" + simulation: ethereum/sync + test_pattern: "" + ethrex_flags: "--syncmode snap" steps: - name: Checkout sources uses: actions/checkout@v4 diff --git a/crates/networking/p2p/sync_manager.rs b/crates/networking/p2p/sync_manager.rs index 77a7885775..58713de27f 100644 --- a/crates/networking/p2p/sync_manager.rs +++ b/crates/networking/p2p/sync_manager.rs @@ -5,7 +5,7 @@ use std::sync::{ use ethrex_blockchain::Blockchain; use ethrex_common::H256; -use ethrex_storage::{error::StoreError, Store}; +use ethrex_storage::Store; use tokio::{ sync::Mutex, time::{sleep, Duration}, @@ -18,12 +18,6 @@ use crate::{ sync::{SyncMode, Syncer}, }; -#[derive(Debug)] -pub enum SyncStatus { - Active(SyncMode), - Inactive, -} - /// Abstraction to interact with the active sync process without disturbing it #[derive(Debug)] pub struct SyncManager { @@ -80,8 +74,25 @@ impl SyncManager { } } + /// Sets the latest fcu head and starts the next sync cycle if the syncer is currently inactive + pub fn sync_to_head(&self, fcu_head: H256) { + self.set_head(fcu_head); + if !self.is_active() { + self.start_sync(); + } + } + + /// Returns the syncer's current syncmode (either snap or full) + pub fn sync_mode(&self) -> SyncMode { + if self.snap_enabled.load(Ordering::Relaxed) { + SyncMode::Snap + } else { + SyncMode::Full + } + } + /// Updates the last fcu head. This may be used on the next sync cycle if needed - pub fn set_head(&self, fcu_head: H256) { + fn set_head(&self, fcu_head: H256) { if let Ok(mut latest_fcu_head) = self.last_fcu_head.try_lock() { *latest_fcu_head = fcu_head; } else { @@ -89,26 +100,22 @@ impl SyncManager { } } - /// Returns the current sync status, either active or inactive and what the current syncmode is in the case of active - pub fn status(&self) -> Result { - Ok(if self.syncer.try_lock().is_err() { - SyncStatus::Active(self.sync_mode()) - } else { - SyncStatus::Inactive - }) + /// Returns true is the syncer is active + fn is_active(&self) -> bool { + self.syncer.try_lock().is_err() } /// Attempts to sync to the last received fcu head /// Will do nothing if the syncer is already involved in a sync process /// If the sync process would require multiple sync cycles (such as snap sync), starts all required sync cycles until the sync is complete - pub fn start_sync(&self) { + fn start_sync(&self) { let syncer = self.syncer.clone(); let store = self.store.clone(); let sync_head = self.last_fcu_head.clone(); tokio::spawn(async move { let Ok(Some(current_head)) = store.get_latest_canonical_block_hash().await else { - tracing::error!("Failed to fecth latest canonical block, unable to sync"); + tracing::error!("Failed to fetch latest canonical block, unable to sync"); return; }; @@ -148,13 +155,4 @@ impl SyncManager { } }); } - - /// Returns the syncer's current syncmode (either snap or full) - pub fn sync_mode(&self) -> SyncMode { - if self.snap_enabled.load(Ordering::Relaxed) { - SyncMode::Snap - } else { - SyncMode::Full - } - } } diff --git a/crates/networking/rpc/engine/fork_choice.rs b/crates/networking/rpc/engine/fork_choice.rs index 570a53fd06..235f57a5d6 100644 --- a/crates/networking/rpc/engine/fork_choice.rs +++ b/crates/networking/rpc/engine/fork_choice.rs @@ -250,7 +250,9 @@ async fn handle_forkchoice( } if context.syncer.sync_mode() == SyncMode::Snap { - context.syncer.set_head(fork_choice_state.head_block_hash); + context + .syncer + .sync_to_head(fork_choice_state.head_block_hash); return Ok((None, PayloadStatus::syncing().into())); } @@ -312,8 +314,9 @@ async fn handle_forkchoice( .update_sync_status(false) .await .map_err(|e| RpcErr::Internal(e.to_string()))?; - context.syncer.set_head(fork_choice_state.head_block_hash); - context.syncer.start_sync(); + context + .syncer + .sync_to_head(fork_choice_state.head_block_hash); ForkChoiceResponse::from(PayloadStatus::syncing()) } InvalidForkChoice::Disconnected(_, _) | InvalidForkChoice::ElementNotFound(_) => { diff --git a/crates/networking/rpc/engine/payload.rs b/crates/networking/rpc/engine/payload.rs index 6852e882e9..13a28f4960 100644 --- a/crates/networking/rpc/engine/payload.rs +++ b/crates/networking/rpc/engine/payload.rs @@ -698,9 +698,7 @@ async fn try_execute_payload( .update_sync_status(false) .await .map_err(|e| RpcErr::Internal(e.to_string()))?; - context.syncer.set_head(block_hash); - context.syncer.start_sync(); - + context.syncer.sync_to_head(block_hash); Ok(PayloadStatus::syncing()) } // Under the current implementation this is not possible: we always calculate the state