From 72556a51b09159046993c51b39053bcb2a38c7af Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Mon, 25 Nov 2024 20:47:45 +0400 Subject: [PATCH 1/2] refactor: isolate BlockchainTree setup in DefaultEngineLauncher --- crates/node/builder/src/launch/common.rs | 97 ++++-------------------- crates/node/builder/src/launch/engine.rs | 12 +-- crates/node/builder/src/launch/mod.rs | 27 ++++++- 3 files changed, 40 insertions(+), 96 deletions(-) diff --git a/crates/node/builder/src/launch/common.rs b/crates/node/builder/src/launch/common.rs index 47ec68ff0d7d..1b9d36554b5c 100644 --- a/crates/node/builder/src/launch/common.rs +++ b/crates/node/builder/src/launch/common.rs @@ -11,10 +11,6 @@ use alloy_primitives::{BlockNumber, B256}; use eyre::{Context, OptionExt}; use rayon::ThreadPoolBuilder; use reth_beacon_consensus::EthBeaconConsensus; -use reth_blockchain_tree::{ - externals::TreeNodeTypes, BlockchainTree, BlockchainTreeConfig, ShareableBlockchainTree, - TreeExternals, -}; use reth_chainspec::{Chain, EthChainSpec, EthereumHardforks}; use reth_config::{config::EtlConfig, PruneConfig}; use reth_consensus::Consensus; @@ -46,10 +42,9 @@ use reth_node_metrics::{ }; use reth_primitives::Head; use reth_provider::{ - providers::{BlockchainProvider, BlockchainProvider2, ProviderNodeTypes, StaticFileProvider}, - BlockHashReader, BlockNumReader, CanonStateNotificationSender, ChainSpecProvider, - ProviderError, ProviderFactory, ProviderResult, StageCheckpointReader, StateProviderFactory, - StaticFileProviderFactory, TreeViewer, + providers::{ProviderNodeTypes, StaticFileProvider}, + BlockHashReader, BlockNumReader, ChainSpecProvider, ProviderError, ProviderFactory, + ProviderResult, StageCheckpointReader, StateProviderFactory, StaticFileProviderFactory, }; use reth_prune::{PruneModes, PrunerBuilder}; use reth_rpc_api::clients::EthApiClient; @@ -65,27 +60,6 @@ use tokio::sync::{ oneshot, watch, }; -/// Allows to set a tree viewer for a configured blockchain provider. -// TODO: remove this helper trait once the engine revamp is done, the new -// blockchain provider won't require a TreeViewer. -// https://github.com/paradigmxyz/reth/issues/8742 -pub trait WithTree { - /// Setter for tree viewer. - fn set_tree(self, tree: Arc) -> Self; -} - -impl WithTree for BlockchainProvider { - fn set_tree(self, tree: Arc) -> Self { - self.with_tree(tree) - } -} - -impl WithTree for BlockchainProvider2 { - fn set_tree(self, _tree: Arc) -> Self { - self - } -} - /// Reusable setup for launching a node. /// /// This provides commonly used boilerplate for launching a node. @@ -610,8 +584,6 @@ where pub fn with_blockchain_db( self, create_blockchain_provider: F, - tree_config: BlockchainTreeConfig, - canon_state_notification_sender: CanonStateNotificationSender, ) -> eyre::Result, WithMeteredProviders>>> where T: FullNodeTypes, @@ -625,8 +597,6 @@ where metrics_sender: self.sync_metrics_tx(), }, blockchain_db, - tree_config, - canon_state_notification_sender, }; let ctx = LaunchContextWith { @@ -643,7 +613,7 @@ impl Attached::ChainSpec>, WithMeteredProviders>, > where - T: FullNodeTypes, + T: FullNodeTypes, { /// Returns access to the underlying database. pub const fn database(&self) -> &::DB { @@ -674,16 +644,6 @@ where &self.right().blockchain_db } - /// Returns a reference to the `BlockchainTreeConfig`. - pub const fn tree_config(&self) -> &BlockchainTreeConfig { - &self.right().tree_config - } - - /// Returns the `CanonStateNotificationSender`. - pub fn canon_state_notification_sender(&self) -> CanonStateNotificationSender { - self.right().canon_state_notification_sender.clone() - } - /// Creates a `NodeAdapter` and attaches it to the launch context. pub async fn with_components( self, @@ -712,31 +672,13 @@ where debug!(target: "reth::cli", "creating components"); let components = components_builder.build_components(&builder_ctx).await?; - let consensus: Arc = Arc::new(components.consensus().clone()); - - let tree_externals = TreeExternals::new( - self.provider_factory().clone().with_prune_modes(self.prune_modes()), - consensus.clone(), - components.block_executor().clone(), - ); - let tree = BlockchainTree::new(tree_externals, *self.tree_config())? - .with_sync_metrics_tx(self.sync_metrics_tx()) - // Note: This is required because we need to ensure that both the components and the - // tree are using the same channel for canon state notifications. This will be removed - // once the Blockchain provider no longer depends on an instance of the tree - .with_canon_state_notification_sender(self.canon_state_notification_sender()); - - let blockchain_tree = Arc::new(ShareableBlockchainTree::new(tree)); - - // Replace the tree component with the actual tree - let blockchain_db = self.blockchain_db().clone().set_tree(blockchain_tree); - - debug!(target: "reth::cli", "configured blockchain tree"); + let blockchain_db = self.blockchain_db().clone(); + let consensus = Arc::new(components.consensus().clone()); let node_adapter = NodeAdapter { components, task_executor: self.task_executor().clone(), - provider: blockchain_db.clone(), + provider: blockchain_db, }; debug!(target: "reth::cli", "calling on_component_initialized hook"); @@ -747,8 +689,6 @@ where provider_factory: self.provider_factory().clone(), metrics_sender: self.sync_metrics_tx(), }, - blockchain_db, - tree_config: self.right().tree_config, node_adapter, head, consensus, @@ -768,7 +708,7 @@ impl Attached::ChainSpec>, WithComponents>, > where - T: FullNodeTypes, + T: FullNodeTypes, CB: NodeComponentsBuilder, { /// Returns the configured `ProviderFactory`. @@ -805,9 +745,14 @@ where &self.right().node_adapter } + /// Returns mutable reference to the configured `NodeAdapter`. + pub fn node_adapter_mut(&mut self) -> &mut NodeAdapter { + &mut self.right_mut().node_adapter + } + /// Returns a reference to the blockchain provider. pub const fn blockchain_db(&self) -> &T::Provider { - &self.right().blockchain_db + &self.node_adapter().provider } /// Returns the initial backfill to sync to at launch. @@ -912,11 +857,6 @@ where self.right().db_provider_container.metrics_sender.clone() } - /// Returns a reference to the `BlockchainTreeConfig`. - pub const fn tree_config(&self) -> &BlockchainTreeConfig { - &self.right().tree_config - } - /// Returns the node adapter components. pub const fn components(&self) -> &CB::Components { &self.node_adapter().components @@ -928,10 +868,7 @@ impl Attached::ChainSpec>, WithComponents>, > where - T: FullNodeTypes< - Provider: WithTree + StateProviderFactory + ChainSpecProvider, - Types: ProviderNodeTypes, - >, + T: FullNodeTypes, CB: NodeComponentsBuilder, { /// Returns the [`InvalidBlockHook`] to use for the node. @@ -1072,8 +1009,6 @@ where { db_provider_container: WithMeteredProvider, blockchain_db: T::Provider, - canon_state_notification_sender: CanonStateNotificationSender, - tree_config: BlockchainTreeConfig, } /// Helper container to bundle the metered providers container and [`NodeAdapter`]. @@ -1084,8 +1019,6 @@ where CB: NodeComponentsBuilder, { db_provider_container: WithMeteredProvider, - tree_config: BlockchainTreeConfig, - blockchain_db: T::Provider, node_adapter: NodeAdapter, head: Head, consensus: Arc, diff --git a/crates/node/builder/src/launch/engine.rs b/crates/node/builder/src/launch/engine.rs index 6afcace5b152..6a87ff3ce658 100644 --- a/crates/node/builder/src/launch/engine.rs +++ b/crates/node/builder/src/launch/engine.rs @@ -5,7 +5,6 @@ use reth_beacon_consensus::{ hooks::{EngineHooks, StaticFileHook}, BeaconConsensusEngineHandle, }; -use reth_blockchain_tree::BlockchainTreeConfig; use reth_chainspec::EthChainSpec; use reth_consensus_debug_client::{DebugConsensusClient, EtherscanBlockProvider}; use reth_engine_local::{LocalEngineService, LocalPayloadAttributesBuilder}; @@ -94,15 +93,6 @@ where } = target; let NodeHooks { on_component_initialized, on_node_started, .. } = hooks; - // TODO: move tree_config and canon_state_notification_sender - // initialization to with_blockchain_db once the engine revamp is done - // https://github.com/paradigmxyz/reth/issues/8742 - let tree_config = BlockchainTreeConfig::default(); - - // NOTE: This is a temporary workaround to provide the canon state notification sender to the components builder because there's a cyclic dependency between the blockchain provider and the tree component. This will be removed once the Blockchain provider no longer depends on an instance of the tree: - let (canon_state_notification_sender, _receiver) = - tokio::sync::broadcast::channel(tree_config.max_reorg_depth() as usize * 2); - // setup the launch context let ctx = ctx .with_configured_globals() @@ -132,7 +122,7 @@ where // later the components. .with_blockchain_db::(move |provider_factory| { Ok(BlockchainProvider2::new(provider_factory)?) - }, tree_config, canon_state_notification_sender)? + })? .with_components(components_builder, on_component_initialized).await?; // spawn exexs diff --git a/crates/node/builder/src/launch/mod.rs b/crates/node/builder/src/launch/mod.rs index 4335073b404c..627145d2df7a 100644 --- a/crates/node/builder/src/launch/mod.rs +++ b/crates/node/builder/src/launch/mod.rs @@ -17,7 +17,8 @@ use reth_beacon_consensus::{ BeaconConsensusEngine, }; use reth_blockchain_tree::{ - externals::TreeNodeTypes, noop::NoopBlockchainTree, BlockchainTreeConfig, + externals::TreeNodeTypes, noop::NoopBlockchainTree, BlockchainTree, BlockchainTreeConfig, + ShareableBlockchainTree, TreeExternals, }; use reth_chainspec::EthChainSpec; use reth_consensus_debug_client::{DebugConsensusClient, EtherscanBlockProvider, RpcBlockProvider}; @@ -134,7 +135,7 @@ where )); // setup the launch context - let ctx = ctx + let mut ctx = ctx .with_configured_globals() // load the toml config .with_loaded_toml_config(config)? @@ -162,9 +163,29 @@ where // later the components. .with_blockchain_db::(move |provider_factory| { Ok(BlockchainProvider::new(provider_factory, tree)?) - }, tree_config, canon_state_notification_sender)? + })? .with_components(components_builder, on_component_initialized).await?; + let consensus = Arc::new(ctx.components().consensus().clone()); + + let tree_externals = TreeExternals::new( + ctx.provider_factory().clone(), + consensus.clone(), + ctx.components().block_executor().clone(), + ); + let tree = BlockchainTree::new(tree_externals, tree_config)? + .with_sync_metrics_tx(ctx.sync_metrics_tx()) + // Note: This is required because we need to ensure that both the components and the + // tree are using the same channel for canon state notifications. This will be removed + // once the Blockchain provider no longer depends on an instance of the tree + .with_canon_state_notification_sender(canon_state_notification_sender); + + let blockchain_tree = Arc::new(ShareableBlockchainTree::new(tree)); + + ctx.node_adapter_mut().provider = ctx.blockchain_db().clone().with_tree(blockchain_tree); + + debug!(target: "reth::cli", "configured blockchain tree"); + // spawn exexs let exex_manager_handle = ExExLauncher::new( ctx.head(), From ea67702cf7fe339ec5fbb30cd3bed6ad742aed75 Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Mon, 25 Nov 2024 20:55:32 +0400 Subject: [PATCH 2/2] doc --- crates/node/builder/src/launch/common.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/node/builder/src/launch/common.rs b/crates/node/builder/src/launch/common.rs index 1b9d36554b5c..a1a2223a470d 100644 --- a/crates/node/builder/src/launch/common.rs +++ b/crates/node/builder/src/launch/common.rs @@ -1000,7 +1000,7 @@ pub struct WithMeteredProvider { metrics_sender: UnboundedSender, } -/// Helper container to bundle the [`ProviderFactory`], [`BlockchainProvider`] +/// Helper container to bundle the [`ProviderFactory`], [`FullNodeTypes::Provider`] /// and a metrics sender. #[allow(missing_debug_implementations)] pub struct WithMeteredProviders