From 69494ea70ba9082b2f8494695361b15db25ec29d Mon Sep 17 00:00:00 2001
From: Sebastian Kunert <skunert49@gmail.com>
Date: Wed, 8 Nov 2023 18:33:45 +0100
Subject: [PATCH] Add prospective-parachain subsystem to minimal-relay-node +
 QoL improvements (#2223)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This PR contains some fixes and cleanups for parachain nodes:

1. When using async backing, node no longer complains about being unable
to reach the prospective-parachain subsystem.
2. Parachain warp sync now informs users that the finalized para block
has been retrieved.
```
2023-11-08 13:24:42 [Parachain] 🎉 Received finalized parachain header #5747719 (0xa0aa…674b) from the relay chain.
```
3. When a user supplied an invalid `--relay-chain-rpc-url`, we were
crashing with a very verbose message. Removed the `expect` and improved
the error message.
```
2023-11-08 13:57:56 [Parachain] No valid RPC url found. Stopping RPC worker.
2023-11-08 13:57:56 [Parachain] Essential task `relay-chain-rpc-worker` failed. Shutting down service.
Error: Service(Application(WorkerCommunicationError("RPC worker channel closed. This can hint and connectivity issues with the supplied RPC endpoints. Message: oneshot canceled")))
```
---
 Cargo.lock                                     |  1 +
 .../client/relay-chain-minimal-node/Cargo.toml |  1 +
 .../src/collator_overseer.rs                   |  3 ++-
 .../client/relay-chain-minimal-node/src/lib.rs |  7 +------
 .../src/rpc_client.rs                          |  2 +-
 cumulus/client/service/src/lib.rs              | 18 +++++++++++-------
 6 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 18eb00a13c1..54fa7c6485b 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -4198,6 +4198,7 @@ dependencies = [
  "polkadot-core-primitives",
  "polkadot-network-bridge",
  "polkadot-node-collation-generation",
+ "polkadot-node-core-prospective-parachains",
  "polkadot-node-core-runtime-api",
  "polkadot-node-network-protocol",
  "polkadot-node-subsystem-util",
diff --git a/cumulus/client/relay-chain-minimal-node/Cargo.toml b/cumulus/client/relay-chain-minimal-node/Cargo.toml
index 6518e09cbb5..8c196f66b72 100644
--- a/cumulus/client/relay-chain-minimal-node/Cargo.toml
+++ b/cumulus/client/relay-chain-minimal-node/Cargo.toml
@@ -18,6 +18,7 @@ polkadot-collator-protocol = { path = "../../../polkadot/node/network/collator-p
 polkadot-network-bridge = { path = "../../../polkadot/node/network/bridge" }
 polkadot-node-collation-generation = { path = "../../../polkadot/node/collation-generation" }
 polkadot-node-core-runtime-api = { path = "../../../polkadot/node/core/runtime-api" }
+polkadot-node-core-prospective-parachains = { path = "../../../polkadot/node/core/prospective-parachains" }
 
 # substrate deps
 sc-authority-discovery = { path = "../../../substrate/client/authority-discovery" }
diff --git a/cumulus/client/relay-chain-minimal-node/src/collator_overseer.rs b/cumulus/client/relay-chain-minimal-node/src/collator_overseer.rs
index 216ca0d3965..379217e4a63 100644
--- a/cumulus/client/relay-chain-minimal-node/src/collator_overseer.rs
+++ b/cumulus/client/relay-chain-minimal-node/src/collator_overseer.rs
@@ -24,6 +24,7 @@ use polkadot_network_bridge::{
 	NetworkBridgeTx as NetworkBridgeTxSubsystem,
 };
 use polkadot_node_collation_generation::CollationGenerationSubsystem;
+use polkadot_node_core_prospective_parachains::ProspectiveParachainsSubsystem;
 use polkadot_node_core_runtime_api::RuntimeApiSubsystem;
 use polkadot_node_network_protocol::{
 	peer_set::PeerSetProtocolNames,
@@ -144,7 +145,7 @@ fn build_overseer(
 			spawner.clone(),
 		))
 		.statement_distribution(DummySubsystem)
-		.prospective_parachains(DummySubsystem)
+		.prospective_parachains(ProspectiveParachainsSubsystem::new(Metrics::register(registry)?))
 		.approval_distribution(DummySubsystem)
 		.approval_voting(DummySubsystem)
 		.gossip_support(DummySubsystem)
diff --git a/cumulus/client/relay-chain-minimal-node/src/lib.rs b/cumulus/client/relay-chain-minimal-node/src/lib.rs
index c8fba923dde..8801f93640c 100644
--- a/cumulus/client/relay-chain-minimal-node/src/lib.rs
+++ b/cumulus/client/relay-chain-minimal-node/src/lib.rs
@@ -171,12 +171,7 @@ async fn new_minimal_relay_chain(
 		);
 	}
 
-	let genesis_hash = relay_chain_rpc_client
-		.block_get_hash(Some(0))
-		.await
-		.expect("Genesis block hash is always available; qed")
-		.unwrap_or_default();
-
+	let genesis_hash = relay_chain_rpc_client.block_get_hash(Some(0)).await?.unwrap_or_default();
 	let peer_set_protocol_names =
 		PeerSetProtocolNames::new(genesis_hash, config.chain_spec.fork_id());
 	let is_authority = if role.is_authority() { IsAuthority::Yes } else { IsAuthority::No };
diff --git a/cumulus/client/relay-chain-rpc-interface/src/rpc_client.rs b/cumulus/client/relay-chain-rpc-interface/src/rpc_client.rs
index 5924716adcb..90af334e133 100644
--- a/cumulus/client/relay-chain-rpc-interface/src/rpc_client.rs
+++ b/cumulus/client/relay-chain-rpc-interface/src/rpc_client.rs
@@ -201,7 +201,7 @@ impl RelayChainRpcClient {
 
 		let value = rx.await.map_err(|err| {
 			RelayChainError::WorkerCommunicationError(format!(
-				"Unexpected channel close on RPC worker side: {}",
+				"RPC worker channel closed. This can hint and connectivity issues with the supplied RPC endpoints. Message: {}",
 				err
 			))
 		})??;
diff --git a/cumulus/client/service/src/lib.rs b/cumulus/client/service/src/lib.rs
index 82890666eba..f8ebca11c8c 100644
--- a/cumulus/client/service/src/lib.rs
+++ b/cumulus/client/service/src/lib.rs
@@ -49,7 +49,7 @@ use sc_utils::mpsc::TracingUnboundedSender;
 use sp_api::ProvideRuntimeApi;
 use sp_blockchain::{HeaderBackend, HeaderMetadata};
 use sp_core::{traits::SpawnNamed, Decode};
-use sp_runtime::traits::{Block as BlockT, BlockIdTo};
+use sp_runtime::traits::{Block as BlockT, BlockIdTo, Header};
 use std::{sync::Arc, time::Duration};
 
 // Given the sporadic nature of the explicit recovery operation and the
@@ -505,11 +505,11 @@ where
 		None,
 		async move {
 			log::debug!(
-				target: "cumulus-network",
+				target: LOG_TARGET_SYNC,
 				"waiting for announce block in a background task...",
 			);
 
-			let _ = wait_for_target_block::<B, _>(sender, para_id, relay_chain_interface)
+			let _ = wait_for_finalized_para_head::<B, _>(sender, para_id, relay_chain_interface)
 				.await
 				.map_err(|e| {
 					log::error!(
@@ -527,7 +527,7 @@ where
 
 /// Waits for the relay chain to have finished syncing and then gets the parachain header that
 /// corresponds to the last finalized relay chain block.
-async fn wait_for_target_block<B, RCInterface>(
+async fn wait_for_finalized_para_head<B, RCInterface>(
 	sender: oneshot::Sender<<B as BlockT>::Header>,
 	para_id: ParaId,
 	relay_chain_interface: RCInterface,
@@ -560,11 +560,15 @@ where
 				.map_err(|e| format!("{e:?}"))?
 				.ok_or("Could not find parachain head in relay chain")?;
 
-			let target_block = B::Header::decode(&mut &validation_data.parent_head.0[..])
+			let finalized_header = B::Header::decode(&mut &validation_data.parent_head.0[..])
 				.map_err(|e| format!("Failed to decode parachain head: {e}"))?;
 
-			log::debug!(target: LOG_TARGET_SYNC, "Target block reached {:?}", target_block);
-			let _ = sender.send(target_block);
+			log::info!(
+				"🎉 Received target parachain header #{} ({}) from the relay chain.",
+				finalized_header.number(),
+				finalized_header.hash()
+			);
+			let _ = sender.send(finalized_header);
 			return Ok(())
 		}
 	}
-- 
GitLab