From 701af23fa5810cf96ddd3a0fb4faa1c985d6d732 Mon Sep 17 00:00:00 2001 From: Harrish Bansal <145403921+Haxry@users.noreply.github.com> Date: Tue, 22 Apr 2025 15:10:56 +0530 Subject: [PATCH] refactor: Simplify HeaderSyncGapProvider trait (#15819) Co-authored-by: Matthias Seitz --- crates/stages/stages/src/stages/headers.rs | 9 +++++-- crates/storage/provider/Cargo.toml | 6 ++--- .../provider/src/providers/database/mod.rs | 22 ++++++++-------- .../src/providers/database/provider.rs | 25 ++++++++----------- .../provider/src/traits/header_sync_gap.rs | 8 +++--- 5 files changed, 35 insertions(+), 35 deletions(-) diff --git a/crates/stages/stages/src/stages/headers.rs b/crates/stages/stages/src/stages/headers.rs index 3282d3cead..10c3162614 100644 --- a/crates/stages/stages/src/stages/headers.rs +++ b/crates/stages/stages/src/stages/headers.rs @@ -12,7 +12,10 @@ use reth_db_api::{ DbTxUnwindExt, RawKey, RawTable, RawValue, }; use reth_etl::Collector; -use reth_network_p2p::headers::{downloader::HeaderDownloader, error::HeadersDownloaderError}; +use reth_network_p2p::headers::{ + downloader::{HeaderDownloader, SyncTarget}, + error::HeadersDownloaderError, +}; use reth_primitives_traits::{serde_bincode_compat, FullBlockHeader, NodePrimitives, SealedHeader}; use reth_provider::{ providers::StaticFileWriter, BlockHashReader, DBProvider, HeaderProvider, HeaderSyncGap, @@ -226,7 +229,9 @@ where } // Lookup the head and tip of the sync range - let gap = self.provider.sync_gap(self.tip.clone(), current_checkpoint.block_number)?; + let local_head = self.provider.local_tip_header(current_checkpoint.block_number)?; + let target = SyncTarget::Tip(*self.tip.borrow()); + let gap = HeaderSyncGap { local_head, target }; let tip = gap.target.tip(); self.sync_gap = Some(gap.clone()); diff --git a/crates/storage/provider/Cargo.toml b/crates/storage/provider/Cargo.toml index af63cac22b..a4b3df145a 100644 --- a/crates/storage/provider/Cargo.toml +++ b/crates/storage/provider/Cargo.toml @@ -42,9 +42,6 @@ alloy-consensus.workspace = true revm-database.workspace = true revm-state = { workspace = true, optional = true } -# async -tokio = { workspace = true, features = ["sync", "macros", "rt-multi-thread"] } - # tracing tracing.workspace = true @@ -63,6 +60,7 @@ eyre.workspace = true # test-utils reth-ethereum-engine-primitives = { workspace = true, optional = true } +tokio = { workspace = true, features = ["sync"], optional = true } # parallel utils rayon.workspace = true @@ -84,6 +82,7 @@ assert_matches.workspace = true rand.workspace = true eyre.workspace = true +tokio = { workspace = true, features = ["sync", "macros", "rt-multi-thread"] } alloy-consensus.workspace = true [features] @@ -104,4 +103,5 @@ test-utils = [ "reth-prune-types/test-utils", "reth-stages-types/test-utils", "revm-state", + "tokio", ] diff --git a/crates/storage/provider/src/providers/database/mod.rs b/crates/storage/provider/src/providers/database/mod.rs index e48d4fbe40..fab58f2ce6 100644 --- a/crates/storage/provider/src/providers/database/mod.rs +++ b/crates/storage/provider/src/providers/database/mod.rs @@ -3,7 +3,7 @@ use crate::{ to_range, traits::{BlockSource, ReceiptProvider}, BlockHashReader, BlockNumReader, BlockReader, ChainSpecProvider, DatabaseProviderFactory, - HashedPostStateProvider, HeaderProvider, HeaderSyncGap, HeaderSyncGapProvider, ProviderError, + HashedPostStateProvider, HeaderProvider, HeaderSyncGapProvider, ProviderError, PruneCheckpointReader, StageCheckpointReader, StateProviderBox, StaticFileProviderFactory, TransactionVariant, TransactionsProvider, WithdrawalsProvider, }; @@ -35,7 +35,7 @@ use std::{ path::Path, sync::Arc, }; -use tokio::sync::watch; + use tracing::trace; mod provider; @@ -228,12 +228,11 @@ impl StaticFileProviderFactory for ProviderFactory { impl HeaderSyncGapProvider for ProviderFactory { type Header = HeaderTy; - fn sync_gap( + fn local_tip_header( &self, - tip: watch::Receiver, highest_uninterrupted_block: BlockNumber, - ) -> ProviderResult> { - self.provider()?.sync_gap(tip, highest_uninterrupted_block) + ) -> ProviderResult> { + self.provider()?.local_tip_header(highest_uninterrupted_block) } } @@ -650,8 +649,8 @@ mod tests { use crate::{ providers::{StaticFileProvider, StaticFileWriter}, test_utils::{blocks::TEST_BLOCK, create_test_provider_factory, MockNodeTypesWithDB}, - BlockHashReader, BlockNumReader, BlockWriter, DBProvider, HeaderSyncGapProvider, - StorageLocation, TransactionsProvider, + BlockHashReader, BlockNumReader, BlockWriter, DBProvider, HeaderSyncGap, + HeaderSyncGapProvider, StorageLocation, TransactionsProvider, }; use alloy_primitives::{TxNumber, B256, U256}; use assert_matches::assert_matches; @@ -662,6 +661,7 @@ mod tests { test_utils::{create_test_static_files_dir, ERROR_TEMPDIR}, }; use reth_db_api::tables; + use reth_network_p2p::headers::downloader::SyncTarget; use reth_primitives_traits::SignedTransaction; use reth_prune_types::{PruneMode, PruneModes}; use reth_storage_errors::provider::ProviderError; @@ -806,7 +806,7 @@ mod tests { // Empty database assert_matches!( - provider.sync_gap(tip_rx.clone(), checkpoint), + provider.local_tip_header(checkpoint), Err(ProviderError::HeaderNotFound(block_number)) if block_number.as_number().unwrap() == checkpoint ); @@ -819,7 +819,9 @@ mod tests { static_file_writer.commit().unwrap(); drop(static_file_writer); - let gap = provider.sync_gap(tip_rx, checkpoint).unwrap(); + let local_head = provider.local_tip_header(checkpoint).unwrap(); + let gap = HeaderSyncGap { local_head, target: SyncTarget::Tip(*tip_rx.borrow()) }; + assert_eq!(gap.local_head, head); assert_eq!(gap.target.tip(), consensus_tip.into()); } diff --git a/crates/storage/provider/src/providers/database/provider.rs b/crates/storage/provider/src/providers/database/provider.rs index 3626a6b2ff..dff8ececc9 100644 --- a/crates/storage/provider/src/providers/database/provider.rs +++ b/crates/storage/provider/src/providers/database/provider.rs @@ -11,13 +11,13 @@ use crate::{ }, AccountReader, BlockBodyWriter, BlockExecutionWriter, BlockHashReader, BlockNumReader, BlockReader, BlockWriter, BundleStateInit, ChainStateBlockReader, ChainStateBlockWriter, - DBProvider, HashingWriter, HeaderProvider, HeaderSyncGap, HeaderSyncGapProvider, - HistoricalStateProvider, HistoricalStateProviderRef, HistoryWriter, LatestStateProvider, - LatestStateProviderRef, OriginalValuesKnown, ProviderError, PruneCheckpointReader, - PruneCheckpointWriter, RevertsInit, StageCheckpointReader, StateCommitmentProvider, - StateProviderBox, StateWriter, StaticFileProviderFactory, StatsReader, StorageLocation, - StorageReader, StorageTrieWriter, TransactionVariant, TransactionsProvider, - TransactionsProviderExt, TrieWriter, WithdrawalsProvider, + DBProvider, HashingWriter, HeaderProvider, HeaderSyncGapProvider, HistoricalStateProvider, + HistoricalStateProviderRef, HistoryWriter, LatestStateProvider, LatestStateProviderRef, + OriginalValuesKnown, ProviderError, PruneCheckpointReader, PruneCheckpointWriter, RevertsInit, + StageCheckpointReader, StateCommitmentProvider, StateProviderBox, StateWriter, + StaticFileProviderFactory, StatsReader, StorageLocation, StorageReader, StorageTrieWriter, + TransactionVariant, TransactionsProvider, TransactionsProviderExt, TrieWriter, + WithdrawalsProvider, }; use alloy_consensus::{transaction::TransactionMeta, BlockHeader, Header, TxReceipt}; use alloy_eips::{eip2718::Encodable2718, eip4895::Withdrawals, BlockHashOrNumber}; @@ -42,7 +42,6 @@ use reth_db_api::{ BlockNumberList, DatabaseError, PlainAccountState, PlainStorageState, }; use reth_execution_types::{Chain, ExecutionOutcome}; -use reth_network_p2p::headers::downloader::SyncTarget; use reth_node_types::{BlockTy, BodyTy, HeaderTy, NodeTypes, ReceiptTy, TxTy}; use reth_primitives_traits::{ Account, Block as _, BlockBody as _, Bytecode, GotExpected, NodePrimitives, RecoveredBlock, @@ -74,7 +73,6 @@ use std::{ ops::{Deref, DerefMut, Range, RangeBounds, RangeInclusive}, sync::{mpsc, Arc}, }; -use tokio::sync::watch; use tracing::{debug, trace}; /// A [`DatabaseProvider`] that holds a read-only database transaction. @@ -939,11 +937,10 @@ impl HeaderSyncGapProvider { type Header = HeaderTy; - fn sync_gap( + fn local_tip_header( &self, - tip: watch::Receiver, highest_uninterrupted_block: BlockNumber, - ) -> ProviderResult> { + ) -> ProviderResult> { let static_file_provider = self.static_file_provider(); // Make sure Headers static file is at the same height. If it's further, this @@ -976,9 +973,7 @@ impl HeaderSyncGapProvider .sealed_header(highest_uninterrupted_block)? .ok_or_else(|| ProviderError::HeaderNotFound(highest_uninterrupted_block.into()))?; - let target = SyncTarget::Tip(*tip.borrow()); - - Ok(HeaderSyncGap { local_head, target }) + Ok(local_head) } } diff --git a/crates/storage/provider/src/traits/header_sync_gap.rs b/crates/storage/provider/src/traits/header_sync_gap.rs index 8c3d29325c..d08ab4c88d 100644 --- a/crates/storage/provider/src/traits/header_sync_gap.rs +++ b/crates/storage/provider/src/traits/header_sync_gap.rs @@ -1,10 +1,9 @@ use alloy_consensus::{BlockHeader, Header}; use alloy_eips::BlockHashOrNumber; -use alloy_primitives::{BlockNumber, Sealable, B256}; +use alloy_primitives::{BlockNumber, Sealable}; use reth_network_p2p::headers::downloader::SyncTarget; use reth_primitives_traits::SealedHeader; use reth_storage_errors::provider::ProviderResult; -use tokio::sync::watch; /// Represents a gap to sync: from `local_head` to `target` #[derive(Clone, Debug)] @@ -37,9 +36,8 @@ pub trait HeaderSyncGapProvider: Send + Sync { /// uninterrupted block number. Last uninterrupted block represents the block number before /// which there are no gaps. It's up to the caller to ensure that last uninterrupted block is /// determined correctly. - fn sync_gap( + fn local_tip_header( &self, - tip: watch::Receiver, highest_uninterrupted_block: BlockNumber, - ) -> ProviderResult>; + ) -> ProviderResult>; }