From 6dd8400f120ac12c1adba9616b253685c7296829 Mon Sep 17 00:00:00 2001 From: joshieDo <93316087+joshieDo@users.noreply.github.com> Date: Tue, 4 Jun 2024 14:29:26 +0200 Subject: [PATCH] chore: remove `StaticFileProvider` field from `StaticFileProducer` (#8583) --- bin/reth/src/commands/debug_cmd/execution.rs | 9 +-- .../src/commands/debug_cmd/replay_engine.rs | 8 +- bin/reth/src/commands/import.rs | 7 +- bin/reth/src/commands/import_op.rs | 11 +-- bin/reth/src/commands/stage/unwind.rs | 6 +- crates/consensus/beacon/src/engine/sync.rs | 8 +- .../consensus/beacon/src/engine/test_utils.rs | 9 +-- crates/node/builder/src/launch/common.rs | 2 - crates/stages/api/src/pipeline/mod.rs | 36 ++------- crates/stages/stages/src/lib.rs | 1 - crates/stages/stages/src/sets.rs | 7 +- .../static-file/src/static_file_producer.rs | 80 +++++++------------ 12 files changed, 50 insertions(+), 134 deletions(-) diff --git a/bin/reth/src/commands/debug_cmd/execution.rs b/bin/reth/src/commands/debug_cmd/execution.rs index c23683642..d9eaef59c 100644 --- a/bin/reth/src/commands/debug_cmd/execution.rs +++ b/bin/reth/src/commands/debug_cmd/execution.rs @@ -31,7 +31,7 @@ use reth_primitives::{ }; use reth_provider::{ providers::StaticFileProvider, BlockExecutionWriter, HeaderSyncMode, ProviderFactory, - StageCheckpointReader, StaticFileProviderFactory, + StageCheckpointReader, }; use reth_stages::{ sets::DefaultStages, @@ -223,11 +223,8 @@ impl Command { ) .await?; - let static_file_producer = StaticFileProducer::new( - provider_factory.clone(), - provider_factory.static_file_provider(), - PruneModes::default(), - ); + let static_file_producer = + StaticFileProducer::new(provider_factory.clone(), PruneModes::default()); // Configure the pipeline let fetch_client = network.fetch_client().await?; diff --git a/bin/reth/src/commands/debug_cmd/replay_engine.rs b/bin/reth/src/commands/debug_cmd/replay_engine.rs index dbd407fdb..338e7235e 100644 --- a/bin/reth/src/commands/debug_cmd/replay_engine.rs +++ b/bin/reth/src/commands/debug_cmd/replay_engine.rs @@ -25,7 +25,7 @@ use reth_payload_builder::{PayloadBuilderHandle, PayloadBuilderService}; use reth_primitives::{ChainSpec, PruneModes}; use reth_provider::{ providers::{BlockchainProvider, StaticFileProvider}, - CanonStateSubscriptions, ProviderFactory, StaticFileProviderFactory, + CanonStateSubscriptions, ProviderFactory, }; use reth_stages::Pipeline; use reth_static_file::StaticFileProducer; @@ -181,11 +181,7 @@ impl Command { network_client, Pipeline::builder().build( provider_factory.clone(), - StaticFileProducer::new( - provider_factory.clone(), - provider_factory.static_file_provider(), - PruneModes::default(), - ), + StaticFileProducer::new(provider_factory.clone(), PruneModes::default()), ), blockchain_db.clone(), Box::new(ctx.task_executor.clone()), diff --git a/bin/reth/src/commands/import.rs b/bin/reth/src/commands/import.rs index 774339241..1ac8d8397 100644 --- a/bin/reth/src/commands/import.rs +++ b/bin/reth/src/commands/import.rs @@ -30,7 +30,6 @@ use reth_primitives::{stage::StageId, ChainSpec, PruneModes, B256}; use reth_provider::{ providers::StaticFileProvider, BlockNumReader, ChainSpecProvider, HeaderProvider, HeaderSyncMode, ProviderError, ProviderFactory, StageCheckpointReader, - StaticFileProviderFactory, }; use reth_stages::{prelude::*, Pipeline, StageSet}; use reth_static_file::StaticFileProducer; @@ -146,11 +145,7 @@ impl ImportCommand { provider_factory.clone(), &consensus, Arc::new(file_client), - StaticFileProducer::new( - provider_factory.clone(), - provider_factory.static_file_provider(), - PruneModes::default(), - ), + StaticFileProducer::new(provider_factory.clone(), PruneModes::default()), self.no_state, ) .await?; diff --git a/bin/reth/src/commands/import_op.rs b/bin/reth/src/commands/import_op.rs index 4fe11d311..f48688068 100644 --- a/bin/reth/src/commands/import_op.rs +++ b/bin/reth/src/commands/import_op.rs @@ -20,10 +20,7 @@ use reth_downloaders::file_client::{ use reth_node_core::args::DatadirArgs; use reth_optimism_primitives::bedrock_import::is_dup_tx; use reth_primitives::{stage::StageId, PruneModes}; -use reth_provider::{ - providers::StaticFileProvider, ProviderFactory, StageCheckpointReader, - StaticFileProviderFactory, -}; +use reth_provider::{providers::StaticFileProvider, ProviderFactory, StageCheckpointReader}; use reth_static_file::StaticFileProducer; use std::{path::PathBuf, sync::Arc}; use tracing::{debug, error, info}; @@ -134,11 +131,7 @@ impl ImportOpCommand { provider_factory.clone(), &consensus, Arc::new(file_client), - StaticFileProducer::new( - provider_factory.clone(), - provider_factory.static_file_provider(), - PruneModes::default(), - ), + StaticFileProducer::new(provider_factory.clone(), PruneModes::default()), true, ) .await?; diff --git a/bin/reth/src/commands/stage/unwind.rs b/bin/reth/src/commands/stage/unwind.rs index 1eadd88f5..b3bb56796 100644 --- a/bin/reth/src/commands/stage/unwind.rs +++ b/bin/reth/src/commands/stage/unwind.rs @@ -158,11 +158,7 @@ impl Command { ) .build( provider_factory.clone(), - StaticFileProducer::new( - provider_factory.clone(), - provider_factory.static_file_provider(), - PruneModes::default(), - ), + StaticFileProducer::new(provider_factory, PruneModes::default()), ); Ok(pipeline) } diff --git a/crates/consensus/beacon/src/engine/sync.rs b/crates/consensus/beacon/src/engine/sync.rs index 8add75eea..c61ef59ef 100644 --- a/crates/consensus/beacon/src/engine/sync.rs +++ b/crates/consensus/beacon/src/engine/sync.rs @@ -436,7 +436,6 @@ mod tests { }; use reth_provider::{ test_utils::create_test_provider_factory_with_chain_spec, BundleStateWithReceipts, - StaticFileProviderFactory, }; use reth_stages::{test_utils::TestStages, ExecOutput, StageError}; use reth_static_file::StaticFileProducer; @@ -499,11 +498,8 @@ mod tests { let provider_factory = create_test_provider_factory_with_chain_spec(chain_spec); - let static_file_producer = StaticFileProducer::new( - provider_factory.clone(), - provider_factory.static_file_provider(), - PruneModes::default(), - ); + let static_file_producer = + StaticFileProducer::new(provider_factory.clone(), PruneModes::default()); pipeline.build(provider_factory, static_file_producer) } diff --git a/crates/consensus/beacon/src/engine/test_utils.rs b/crates/consensus/beacon/src/engine/test_utils.rs index cc6181456..d97a626bf 100644 --- a/crates/consensus/beacon/src/engine/test_utils.rs +++ b/crates/consensus/beacon/src/engine/test_utils.rs @@ -24,7 +24,7 @@ use reth_payload_builder::test_utils::spawn_test_payload_service; use reth_primitives::{BlockNumber, ChainSpec, FinishedExExHeight, PruneModes, B256}; use reth_provider::{ providers::BlockchainProvider, test_utils::create_test_provider_factory_with_chain_spec, - BundleStateWithReceipts, HeaderSyncMode, StaticFileProviderFactory, + BundleStateWithReceipts, HeaderSyncMode, }; use reth_prune::Pruner; use reth_rpc_types::engine::{ @@ -349,11 +349,8 @@ where } }; - let static_file_producer = StaticFileProducer::new( - provider_factory.clone(), - provider_factory.static_file_provider(), - PruneModes::default(), - ); + let static_file_producer = + StaticFileProducer::new(provider_factory.clone(), PruneModes::default()); // Setup pipeline let (tip_tx, tip_rx) = watch::channel(B256::default()); diff --git a/crates/node/builder/src/launch/common.rs b/crates/node/builder/src/launch/common.rs index ae1c88ede..257af7bc6 100644 --- a/crates/node/builder/src/launch/common.rs +++ b/crates/node/builder/src/launch/common.rs @@ -373,7 +373,6 @@ where factory.clone(), StaticFileProducer::new( factory.clone(), - factory.static_file_provider(), self.prune_modes().unwrap_or_default(), ), ); @@ -432,7 +431,6 @@ where pub fn static_file_producer(&self) -> StaticFileProducer { StaticFileProducer::new( self.provider_factory().clone(), - self.static_file_provider(), self.prune_modes().unwrap_or_default(), ) } diff --git a/crates/stages/api/src/pipeline/mod.rs b/crates/stages/api/src/pipeline/mod.rs index a2a17e3c7..406b0e73b 100644 --- a/crates/stages/api/src/pipeline/mod.rs +++ b/crates/stages/api/src/pipeline/mod.rs @@ -647,11 +647,7 @@ mod tests { .with_max_block(10) .build( provider_factory.clone(), - StaticFileProducer::new( - provider_factory.clone(), - provider_factory.static_file_provider(), - PruneModes::default(), - ), + StaticFileProducer::new(provider_factory.clone(), PruneModes::default()), ); let events = pipeline.events(); @@ -726,11 +722,7 @@ mod tests { .with_max_block(10) .build( provider_factory.clone(), - StaticFileProducer::new( - provider_factory.clone(), - provider_factory.static_file_provider(), - PruneModes::default(), - ), + StaticFileProducer::new(provider_factory.clone(), PruneModes::default()), ); let events = pipeline.events(); @@ -858,11 +850,7 @@ mod tests { .with_max_block(10) .build( provider_factory.clone(), - StaticFileProducer::new( - provider_factory.clone(), - provider_factory.static_file_provider(), - PruneModes::default(), - ), + StaticFileProducer::new(provider_factory.clone(), PruneModes::default()), ); let events = pipeline.events(); @@ -972,11 +960,7 @@ mod tests { .with_max_block(10) .build( provider_factory.clone(), - StaticFileProducer::new( - provider_factory.clone(), - provider_factory.static_file_provider(), - PruneModes::default(), - ), + StaticFileProducer::new(provider_factory.clone(), PruneModes::default()), ); let events = pipeline.events(); @@ -1083,11 +1067,7 @@ mod tests { .with_max_block(10) .build( provider_factory.clone(), - StaticFileProducer::new( - provider_factory.clone(), - provider_factory.static_file_provider(), - PruneModes::default(), - ), + StaticFileProducer::new(provider_factory.clone(), PruneModes::default()), ); let result = pipeline.run().await; assert_matches!(result, Ok(())); @@ -1100,11 +1080,7 @@ mod tests { ))) .build( provider_factory.clone(), - StaticFileProducer::new( - provider_factory.clone(), - provider_factory.static_file_provider(), - PruneModes::default(), - ), + StaticFileProducer::new(provider_factory.clone(), PruneModes::default()), ); let result = pipeline.run().await; assert_matches!( diff --git a/crates/stages/stages/src/lib.rs b/crates/stages/stages/src/lib.rs index 37b8d0f33..1c3667d2c 100644 --- a/crates/stages/stages/src/lib.rs +++ b/crates/stages/stages/src/lib.rs @@ -48,7 +48,6 @@ //! # let executor_provider = EthExecutorProvider::mainnet(); //! # let static_file_producer = StaticFileProducer::new( //! # provider_factory.clone(), -//! # provider_factory.static_file_provider(), //! # PruneModes::default() //! # ); //! // Create a pipeline that can fully sync diff --git a/crates/stages/stages/src/sets.rs b/crates/stages/stages/src/sets.rs index 4ba738bbf..bd4e0bc7b 100644 --- a/crates/stages/stages/src/sets.rs +++ b/crates/stages/stages/src/sets.rs @@ -23,11 +23,8 @@ //! # fn create(exec: impl BlockExecutorProvider) { //! //! let provider_factory = create_test_provider_factory(); -//! let static_file_producer = StaticFileProducer::new( -//! provider_factory.clone(), -//! provider_factory.static_file_provider(), -//! PruneModes::default(), -//! ); +//! let static_file_producer = +//! StaticFileProducer::new(provider_factory.clone(), PruneModes::default()); //! // Build a pipeline with all offline stages. //! let pipeline = Pipeline::builder() //! .add_stages(OfflineStages::new(exec, StageConfig::default(), PruneModes::default())) diff --git a/crates/static-file/src/static_file_producer.rs b/crates/static-file/src/static_file_producer.rs index 982ae7af1..cf0645244 100644 --- a/crates/static-file/src/static_file_producer.rs +++ b/crates/static-file/src/static_file_producer.rs @@ -5,10 +5,7 @@ use parking_lot::Mutex; use rayon::prelude::*; use reth_db::database::Database; use reth_primitives::{static_file::HighestStaticFiles, BlockNumber, PruneModes}; -use reth_provider::{ - providers::{StaticFileProvider, StaticFileWriter}, - ProviderFactory, -}; +use reth_provider::{providers::StaticFileWriter, ProviderFactory, StaticFileProviderFactory}; use reth_storage_errors::provider::ProviderResult; use reth_tokio_util::{EventSender, EventStream}; use std::{ @@ -31,16 +28,8 @@ pub struct StaticFileProducer(Arc>>); impl StaticFileProducer { /// Creates a new [`StaticFileProducer`]. - pub fn new( - provider_factory: ProviderFactory, - static_file_provider: StaticFileProvider, - prune_modes: PruneModes, - ) -> Self { - Self(Arc::new(Mutex::new(StaticFileProducerInner::new( - provider_factory, - static_file_provider, - prune_modes, - )))) + pub fn new(provider_factory: ProviderFactory, prune_modes: PruneModes) -> Self { + Self(Arc::new(Mutex::new(StaticFileProducerInner::new(provider_factory, prune_modes)))) } } @@ -58,8 +47,6 @@ impl Deref for StaticFileProducer { pub struct StaticFileProducerInner { /// Provider factory provider_factory: ProviderFactory, - /// Static File provider - static_file_provider: StaticFileProvider, /// Pruning configuration for every part of the data that can be pruned. Set by user, and /// needed in [`StaticFileProducerInner`] to prevent attempting to move prunable data to static /// files. See [`StaticFileProducerInner::get_static_file_targets`]. @@ -102,17 +89,8 @@ impl StaticFileTargets { } impl StaticFileProducerInner { - fn new( - provider_factory: ProviderFactory, - static_file_provider: StaticFileProvider, - prune_modes: PruneModes, - ) -> Self { - Self { - provider_factory, - static_file_provider, - prune_modes, - event_sender: Default::default(), - } + fn new(provider_factory: ProviderFactory, prune_modes: PruneModes) -> Self { + Self { provider_factory, prune_modes, event_sender: Default::default() } } /// Listen for events on the `static_file_producer`. @@ -123,8 +101,9 @@ impl StaticFileProducerInner { /// Run the `static_file_producer`. /// /// For each [Some] target in [`StaticFileTargets`], initializes a corresponding [Segment] and - /// runs it with the provided block range using [`StaticFileProvider`] and a read-only - /// database transaction from [`ProviderFactory`]. All segments are run in parallel. + /// runs it with the provided block range using [`reth_provider::providers::StaticFileProvider`] + /// and a read-only database transaction from [`ProviderFactory`]. All segments are run in + /// parallel. /// /// NOTE: it doesn't delete the data from database, and the actual deleting (aka pruning) logic /// lives in the `prune` crate. @@ -135,7 +114,7 @@ impl StaticFileProducerInner { } debug_assert!(targets.is_contiguous_to_highest_static_files( - self.static_file_provider.get_highest_static_files() + self.provider_factory.static_file_provider().get_highest_static_files() )); self.event_sender.notify(StaticFileProducerEvent::Started { targets: targets.clone() }); @@ -162,7 +141,7 @@ impl StaticFileProducerInner { // Create a new database transaction on every segment to prevent long-lived read-only // transactions let provider = self.provider_factory.provider()?.disable_long_read_transaction_safety(); - segment.copy_to_static_files(provider, self.static_file_provider.clone(), block_range.clone())?; + segment.copy_to_static_files(provider, self.provider_factory.static_file_provider(), block_range.clone())?; let elapsed = start.elapsed(); // TODO(alexey): track in metrics debug!(target: "static_file", segment = %segment.segment(), ?block_range, ?elapsed, "Finished StaticFileProducer segment"); @@ -170,9 +149,11 @@ impl StaticFileProducerInner { Ok(()) })?; - self.static_file_provider.commit()?; + self.provider_factory.static_file_provider().commit()?; for (segment, block_range) in segments { - self.static_file_provider.update_index(segment.segment(), Some(*block_range.end()))?; + self.provider_factory + .static_file_provider() + .update_index(segment.segment(), Some(*block_range.end()))?; } let elapsed = start.elapsed(); // TODO(alexey): track in metrics @@ -186,12 +167,13 @@ impl StaticFileProducerInner { /// Returns a static file targets at the provided finalized block numbers per segment. /// The target is determined by the check against highest `static_files` using - /// [`StaticFileProvider::get_highest_static_files`]. + /// [`reth_provider::providers::StaticFileProvider::get_highest_static_files`]. pub fn get_static_file_targets( &self, finalized_block_numbers: HighestStaticFiles, ) -> ProviderResult { - let highest_static_files = self.static_file_provider.get_highest_static_files(); + let highest_static_files = + self.provider_factory.static_file_provider().get_highest_static_files(); let targets = StaticFileTargets { headers: finalized_block_numbers.headers.and_then(|finalized_block_number| { @@ -251,8 +233,7 @@ mod tests { static_file::HighestStaticFiles, PruneModes, StaticFileSegment, B256, U256, }; use reth_provider::{ - providers::{StaticFileProvider, StaticFileWriter}, - ProviderError, ProviderFactory, StaticFileProviderFactory, + providers::StaticFileWriter, ProviderError, ProviderFactory, StaticFileProviderFactory, }; use reth_stages::test_utils::{StorageKind, TestStageDB}; use reth_testing_utils::{ @@ -265,7 +246,7 @@ mod tests { }; use tempfile::TempDir; - fn setup() -> (ProviderFactory>>, StaticFileProvider, TempDir) { + fn setup() -> (ProviderFactory>>, TempDir) { let mut rng = generators::rng(); let db = TestStageDB::default(); @@ -297,19 +278,15 @@ mod tests { db.insert_receipts(receipts).expect("insert receipts"); let provider_factory = db.factory; - let static_file_provider = provider_factory.static_file_provider(); - (provider_factory, static_file_provider, db.temp_static_files_dir) + (provider_factory, db.temp_static_files_dir) } #[test] fn run() { - let (provider_factory, static_file_provider, _temp_static_files_dir) = setup(); + let (provider_factory, _temp_static_files_dir) = setup(); - let static_file_producer = StaticFileProducerInner::new( - provider_factory, - static_file_provider.clone(), - PruneModes::default(), - ); + let static_file_producer = + StaticFileProducerInner::new(provider_factory.clone(), PruneModes::default()); let targets = static_file_producer .get_static_file_targets(HighestStaticFiles { @@ -328,7 +305,7 @@ mod tests { ); assert_matches!(static_file_producer.run(targets), Ok(_)); assert_eq!( - static_file_provider.get_highest_static_files(), + provider_factory.static_file_provider().get_highest_static_files(), HighestStaticFiles { headers: Some(1), receipts: Some(1), transactions: Some(1) } ); @@ -349,7 +326,7 @@ mod tests { ); assert_matches!(static_file_producer.run(targets), Ok(_)); assert_eq!( - static_file_provider.get_highest_static_files(), + provider_factory.static_file_provider().get_highest_static_files(), HighestStaticFiles { headers: Some(3), receipts: Some(3), transactions: Some(3) } ); @@ -373,7 +350,7 @@ mod tests { Err(ProviderError::BlockBodyIndicesNotFound(4)) ); assert_eq!( - static_file_provider.get_highest_static_files(), + provider_factory.static_file_provider().get_highest_static_files(), HighestStaticFiles { headers: Some(3), receipts: Some(3), transactions: Some(3) } ); } @@ -381,10 +358,9 @@ mod tests { /// Tests that a cloneable [`StaticFileProducer`] type is not susceptible to any race condition. #[test] fn only_one() { - let (provider_factory, static_file_provider, _temp_static_files_dir) = setup(); + let (provider_factory, _temp_static_files_dir) = setup(); - let static_file_producer = - StaticFileProducer::new(provider_factory, static_file_provider, PruneModes::default()); + let static_file_producer = StaticFileProducer::new(provider_factory, PruneModes::default()); let (tx, rx) = channel();