From 5400008f4519406221c24c4dc92093e9f1939164 Mon Sep 17 00:00:00 2001 From: Sebastian Kunert Date: Mon, 3 Apr 2023 18:12:43 +0200 Subject: [PATCH 1/4] Remove polkadot-service dependency from minimal-node --- Cargo.lock | 5 ++- client/relay-chain-minimal-node/Cargo.toml | 6 ++- .../src/collator_overseer.rs | 43 +++++++++++-------- client/relay-chain-minimal-node/src/lib.rs | 2 +- .../relay-chain-minimal-node/src/network.rs | 2 +- 5 files changed, 35 insertions(+), 23 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 85a361e944b..91fa8c5a72b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2332,13 +2332,16 @@ dependencies = [ "cumulus-relay-chain-rpc-interface", "futures", "lru 0.9.0", + "polkadot-availability-recovery", + "polkadot-collator-protocol", "polkadot-core-primitives", "polkadot-network-bridge", + "polkadot-node-collation-generation", + "polkadot-node-core-runtime-api", "polkadot-node-network-protocol", "polkadot-node-subsystem-util", "polkadot-overseer", "polkadot-primitives", - "polkadot-service", "sc-authority-discovery", "sc-client-api", "sc-network", diff --git a/client/relay-chain-minimal-node/Cargo.toml b/client/relay-chain-minimal-node/Cargo.toml index 737445c30c1..ff8d8bc11d3 100644 --- a/client/relay-chain-minimal-node/Cargo.toml +++ b/client/relay-chain-minimal-node/Cargo.toml @@ -9,10 +9,14 @@ edition = "2021" polkadot-primitives = { git = "https://github.com/paritytech/polkadot", branch = "master" } polkadot-core-primitives = { git = "https://github.com/paritytech/polkadot", branch = "master" } polkadot-overseer = { git = "https://github.com/paritytech/polkadot", branch = "master" } -polkadot-service = { git = "https://github.com/paritytech/polkadot", branch = "master" } polkadot-node-subsystem-util = { git = "https://github.com/paritytech/polkadot", branch = "master" } polkadot-node-network-protocol = { git = "https://github.com/paritytech/polkadot", branch = "master" } + +polkadot-availability-recovery = { git = "https://github.com/paritytech/polkadot", branch = "master" } +polkadot-collator-protocol = { git = "https://github.com/paritytech/polkadot", branch = "master" } polkadot-network-bridge = { git = "https://github.com/paritytech/polkadot", branch = "master" } +polkadot-node-collation-generation = { git = "https://github.com/paritytech/polkadot", branch = "master" } +polkadot-node-core-runtime-api = { git = "https://github.com/paritytech/polkadot", branch = "master" } # substrate deps sc-authority-discovery = { git = "https://github.com/paritytech/substrate", branch = "master" } diff --git a/client/relay-chain-minimal-node/src/collator_overseer.rs b/client/relay-chain-minimal-node/src/collator_overseer.rs index 9bcbc2a67a8..e1773af2e67 100644 --- a/client/relay-chain-minimal-node/src/collator_overseer.rs +++ b/client/relay-chain-minimal-node/src/collator_overseer.rs @@ -18,6 +18,15 @@ use futures::{select, StreamExt}; use lru::LruCache; use std::sync::Arc; +use polkadot_availability_recovery::AvailabilityRecoverySubsystem; +use polkadot_collator_protocol::CollatorProtocolSubsystem; +use polkadot_collator_protocol::ProtocolSide; +use polkadot_network_bridge::{ + Metrics as NetworkBridgeMetrics, NetworkBridgeRx as NetworkBridgeRxSubsystem, + NetworkBridgeTx as NetworkBridgeTxSubsystem, +}; +use polkadot_node_collation_generation::CollationGenerationSubsystem; +use polkadot_node_core_runtime_api::RuntimeApiSubsystem; use polkadot_node_network_protocol::{ peer_set::PeerSetProtocolNames, request_response::{ @@ -27,18 +36,10 @@ use polkadot_node_network_protocol::{ }; use polkadot_node_subsystem_util::metrics::{prometheus::Registry, Metrics}; use polkadot_overseer::{ - BlockInfo, DummySubsystem, Handle, MetricsTrait, Overseer, OverseerHandle, OverseerMetrics, - SpawnGlue, KNOWN_LEAVES_CACHE_SIZE, + BlockInfo, DummySubsystem, Handle, MetricsTrait, Overseer, OverseerConnector, OverseerHandle, + OverseerMetrics, SpawnGlue, KNOWN_LEAVES_CACHE_SIZE, }; use polkadot_primitives::CollatorPair; -use polkadot_service::{ - overseer::{ - AvailabilityRecoverySubsystem, CollationGenerationSubsystem, CollatorProtocolSubsystem, - NetworkBridgeMetrics, NetworkBridgeRxSubsystem, NetworkBridgeTxSubsystem, ProtocolSide, - RuntimeApiSubsystem, - }, - Error, OverseerConnector, -}; use sc_authority_discovery::Service as AuthorityDiscoveryService; use sc_network::NetworkStateInfo; @@ -93,16 +94,18 @@ fn build_overseer<'a>( }: CollatorOverseerGenArgs<'a>, ) -> Result< (Overseer, Arc>, OverseerHandle), - Error, + String, > { - let metrics = ::register(registry)?; + let metrics = + ::register(registry).map_err(|e| e.to_string())?; let spawner = SpawnGlue(spawner); - let network_bridge_metrics: NetworkBridgeMetrics = Metrics::register(registry)?; + let network_bridge_metrics: NetworkBridgeMetrics = + Metrics::register(registry).map_err(|e| e.to_string())?; let builder = Overseer::builder() .availability_distribution(DummySubsystem) .availability_recovery(AvailabilityRecoverySubsystem::with_chunks_only( available_data_req_receiver, - Metrics::register(registry)?, + Metrics::register(registry).map_err(|e| e.to_string())?, )) .availability_store(DummySubsystem) .bitfield_distribution(DummySubsystem) @@ -111,13 +114,15 @@ fn build_overseer<'a>( .candidate_validation(DummySubsystem) .pvf_checker(DummySubsystem) .chain_api(DummySubsystem) - .collation_generation(CollationGenerationSubsystem::new(Metrics::register(registry)?)) + .collation_generation(CollationGenerationSubsystem::new( + Metrics::register(registry).map_err(|e| e.to_string())?, + )) .collator_protocol({ let side = ProtocolSide::Collator( network_service.local_peer_id(), collator_pair, collation_req_receiver, - Metrics::register(registry)?, + Metrics::register(registry).map_err(|e| e.to_string())?, ); CollatorProtocolSubsystem::new(side) }) @@ -138,7 +143,7 @@ fn build_overseer<'a>( .provisioner(DummySubsystem) .runtime_api(RuntimeApiSubsystem::new( runtime_client.clone(), - Metrics::register(registry)?, + Metrics::register(registry).map_err(|e| e.to_string())?, spawner.clone(), )) .statement_distribution(DummySubsystem) @@ -156,14 +161,14 @@ fn build_overseer<'a>( .metrics(metrics) .spawner(spawner); - builder.build_with_connector(connector).map_err(|e| e.into()) + builder.build_with_connector(connector).map_err(|e| e.to_string()) } pub(crate) fn spawn_overseer( overseer_args: CollatorOverseerGenArgs, task_manager: &TaskManager, relay_chain_rpc_client: Arc, -) -> Result { +) -> Result { let (overseer, overseer_handle) = build_overseer(OverseerConnector::default(), overseer_args) .map_err(|e| { tracing::error!("Failed to initialize overseer: {}", e); diff --git a/client/relay-chain-minimal-node/src/lib.rs b/client/relay-chain-minimal-node/src/lib.rs index 90afd31b8b6..91b96bc50f8 100644 --- a/client/relay-chain-minimal-node/src/lib.rs +++ b/client/relay-chain-minimal-node/src/lib.rs @@ -182,7 +182,7 @@ async fn new_minimal_relay_chain( let overseer_handle = collator_overseer::spawn_overseer(overseer_args, &task_manager, relay_chain_rpc_client) - .map_err(|e| RelayChainError::Application(Box::new(e) as Box<_>))?; + .map_err(RelayChainError::GenericError)?; network_starter.start_network(); diff --git a/client/relay-chain-minimal-node/src/network.rs b/client/relay-chain-minimal-node/src/network.rs index 5225fa053cc..2ac48ddb77c 100644 --- a/client/relay-chain-minimal-node/src/network.rs +++ b/client/relay-chain-minimal-node/src/network.rs @@ -89,7 +89,7 @@ pub(crate) fn build_collator_network( ); // This `return` might seem unnecessary, but we don't want to make it look like // everything is working as normal even though the user is clearly misusing the API. - return + return; } network_worker.run().await; From 9c7b58b628156f25ff42f0c5d388897b67c516bc Mon Sep 17 00:00:00 2001 From: Sebastian Kunert Date: Mon, 3 Apr 2023 18:24:53 +0200 Subject: [PATCH 2/4] Clean up error handline --- client/relay-chain-interface/src/lib.rs | 6 +++++ .../src/collator_overseer.rs | 25 ++++++++----------- client/relay-chain-minimal-node/src/lib.rs | 3 +-- 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/client/relay-chain-interface/src/lib.rs b/client/relay-chain-interface/src/lib.rs index db4fd24c64f..d5987574fa7 100644 --- a/client/relay-chain-interface/src/lib.rs +++ b/client/relay-chain-interface/src/lib.rs @@ -86,6 +86,12 @@ impl From for sp_blockchain::Error { } } +impl From> for RelayChainError { + fn from(r: Box) -> Self { + RelayChainError::Application(r) + } +} + /// Trait that provides all necessary methods for interaction between collator and relay chain. #[async_trait] pub trait RelayChainInterface: Send + Sync { diff --git a/client/relay-chain-minimal-node/src/collator_overseer.rs b/client/relay-chain-minimal-node/src/collator_overseer.rs index e1773af2e67..972a732c164 100644 --- a/client/relay-chain-minimal-node/src/collator_overseer.rs +++ b/client/relay-chain-minimal-node/src/collator_overseer.rs @@ -94,18 +94,15 @@ fn build_overseer<'a>( }: CollatorOverseerGenArgs<'a>, ) -> Result< (Overseer, Arc>, OverseerHandle), - String, + RelayChainError, > { - let metrics = - ::register(registry).map_err(|e| e.to_string())?; let spawner = SpawnGlue(spawner); - let network_bridge_metrics: NetworkBridgeMetrics = - Metrics::register(registry).map_err(|e| e.to_string())?; + let network_bridge_metrics: NetworkBridgeMetrics = Metrics::register(registry)?; let builder = Overseer::builder() .availability_distribution(DummySubsystem) .availability_recovery(AvailabilityRecoverySubsystem::with_chunks_only( available_data_req_receiver, - Metrics::register(registry).map_err(|e| e.to_string())?, + Metrics::register(registry)?, )) .availability_store(DummySubsystem) .bitfield_distribution(DummySubsystem) @@ -114,15 +111,13 @@ fn build_overseer<'a>( .candidate_validation(DummySubsystem) .pvf_checker(DummySubsystem) .chain_api(DummySubsystem) - .collation_generation(CollationGenerationSubsystem::new( - Metrics::register(registry).map_err(|e| e.to_string())?, - )) + .collation_generation(CollationGenerationSubsystem::new(Metrics::register(registry)?)) .collator_protocol({ let side = ProtocolSide::Collator( network_service.local_peer_id(), collator_pair, collation_req_receiver, - Metrics::register(registry).map_err(|e| e.to_string())?, + Metrics::register(registry)?, ); CollatorProtocolSubsystem::new(side) }) @@ -143,7 +138,7 @@ fn build_overseer<'a>( .provisioner(DummySubsystem) .runtime_api(RuntimeApiSubsystem::new( runtime_client.clone(), - Metrics::register(registry).map_err(|e| e.to_string())?, + Metrics::register(registry)?, spawner.clone(), )) .statement_distribution(DummySubsystem) @@ -158,17 +153,19 @@ fn build_overseer<'a>( .active_leaves(Default::default()) .supports_parachains(runtime_client) .known_leaves(LruCache::new(KNOWN_LEAVES_CACHE_SIZE)) - .metrics(metrics) + .metrics(Metrics::register(registry)?) .spawner(spawner); - builder.build_with_connector(connector).map_err(|e| e.to_string()) + builder + .build_with_connector(connector) + .map_err(|e| RelayChainError::Application(e.into())) } pub(crate) fn spawn_overseer( overseer_args: CollatorOverseerGenArgs, task_manager: &TaskManager, relay_chain_rpc_client: Arc, -) -> Result { +) -> Result { let (overseer, overseer_handle) = build_overseer(OverseerConnector::default(), overseer_args) .map_err(|e| { tracing::error!("Failed to initialize overseer: {}", e); diff --git a/client/relay-chain-minimal-node/src/lib.rs b/client/relay-chain-minimal-node/src/lib.rs index 91b96bc50f8..102a8582745 100644 --- a/client/relay-chain-minimal-node/src/lib.rs +++ b/client/relay-chain-minimal-node/src/lib.rs @@ -181,8 +181,7 @@ async fn new_minimal_relay_chain( }; let overseer_handle = - collator_overseer::spawn_overseer(overseer_args, &task_manager, relay_chain_rpc_client) - .map_err(RelayChainError::GenericError)?; + collator_overseer::spawn_overseer(overseer_args, &task_manager, relay_chain_rpc_client)?; network_starter.start_network(); From aab1d33ed8deccfedcda2a1ea69eef05fa2a1d4b Mon Sep 17 00:00:00 2001 From: Sebastian Kunert Date: Tue, 4 Apr 2023 13:29:43 +0200 Subject: [PATCH 3/4] Remove unwanted changes --- client/relay-chain-interface/src/lib.rs | 4 +++- client/relay-chain-minimal-node/src/network.rs | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/client/relay-chain-interface/src/lib.rs b/client/relay-chain-interface/src/lib.rs index d5987574fa7..3629aea84cd 100644 --- a/client/relay-chain-interface/src/lib.rs +++ b/client/relay-chain-interface/src/lib.rs @@ -46,7 +46,9 @@ pub enum RelayChainError { WaitTimeout(PHash), #[error("Import listener closed while waiting for relay-chain block `{0}` to be imported.")] ImportListenerClosed(PHash), - #[error("Blockchain returned an error while waiting for relay-chain block `{0}` to be imported: {1}")] + #[error( + "Blockchain returned an error while waiting for relay-chain block `{0}` to be imported: {1}" + )] WaitBlockchainError(PHash, sp_blockchain::Error), #[error("Blockchain returned an error: {0}")] BlockchainError(#[from] sp_blockchain::Error), diff --git a/client/relay-chain-minimal-node/src/network.rs b/client/relay-chain-minimal-node/src/network.rs index 2ac48ddb77c..5225fa053cc 100644 --- a/client/relay-chain-minimal-node/src/network.rs +++ b/client/relay-chain-minimal-node/src/network.rs @@ -89,7 +89,7 @@ pub(crate) fn build_collator_network( ); // This `return` might seem unnecessary, but we don't want to make it look like // everything is working as normal even though the user is clearly misusing the API. - return; + return } network_worker.run().await; From 63bc0ff609ab1132169b7868d3f1af782b4031fa Mon Sep 17 00:00:00 2001 From: Sebastian Kunert Date: Tue, 4 Apr 2023 13:36:30 +0200 Subject: [PATCH 4/4] Unused deps --- client/relay-chain-minimal-node/src/collator_overseer.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/client/relay-chain-minimal-node/src/collator_overseer.rs b/client/relay-chain-minimal-node/src/collator_overseer.rs index 972a732c164..bd3dfa8e0f8 100644 --- a/client/relay-chain-minimal-node/src/collator_overseer.rs +++ b/client/relay-chain-minimal-node/src/collator_overseer.rs @@ -19,8 +19,7 @@ use lru::LruCache; use std::sync::Arc; use polkadot_availability_recovery::AvailabilityRecoverySubsystem; -use polkadot_collator_protocol::CollatorProtocolSubsystem; -use polkadot_collator_protocol::ProtocolSide; +use polkadot_collator_protocol::{CollatorProtocolSubsystem, ProtocolSide}; use polkadot_network_bridge::{ Metrics as NetworkBridgeMetrics, NetworkBridgeRx as NetworkBridgeRxSubsystem, NetworkBridgeTx as NetworkBridgeTxSubsystem, @@ -36,8 +35,8 @@ use polkadot_node_network_protocol::{ }; use polkadot_node_subsystem_util::metrics::{prometheus::Registry, Metrics}; use polkadot_overseer::{ - BlockInfo, DummySubsystem, Handle, MetricsTrait, Overseer, OverseerConnector, OverseerHandle, - OverseerMetrics, SpawnGlue, KNOWN_LEAVES_CACHE_SIZE, + BlockInfo, DummySubsystem, Handle, Overseer, OverseerConnector, OverseerHandle, SpawnGlue, + KNOWN_LEAVES_CACHE_SIZE, }; use polkadot_primitives::CollatorPair;