collator-protocol revamp: Send RC PeerId with ApprovedPeer UMP signal (#12388)

Currently the collator sends its parachain PeerId within the
ApprovedPeer UMP signal, which is wrong because this peer id is not
known on the relay chain.
As a result on CandidateIncluded event the score is bumped for the wrong
PeerId and the collator remains with zero reputation no matter its
honest work.

---------

Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
(cherry picked from commit 44b3a38)

Author: tdimitrov

cumulus/polkadot-omni-node/lib/src/common/spec.rs

@@ -408,7 +408,7 @@ pub(crate) trait NodeSpec: BaseNodeSpec {
 					metrics,
 				})
 				.await?;
-			let peer_id = network.local_peer_id();
+			let peer_id = relay_chain_network.local_peer_id();
 
 			let statement_store = statement_handler_proto
 				.map(|(statement_handler_proto, config)| {

cumulus/test/service/src/lib.rs

@@ -72,7 +72,7 @@ use sc_network::{
 	config::{FullNetworkConfiguration, TransportConfig},
 	multiaddr,
 	service::traits::NetworkService,
-	NetworkBackend, NetworkBlock, NetworkStateInfo,
+	NetworkBackend, NetworkBlock, NetworkStateInfo, PeerId,
 };
 use sc_service::{
 	config::{
@@ -257,7 +257,7 @@ async fn build_relay_chain_interface(
 	collator_key: Option<CollatorPair>,
 	collator_options: CollatorOptions,
 	task_manager: &mut TaskManager,
-) -> RelayChainResult<Arc<dyn RelayChainInterface + 'static>> {
+) -> RelayChainResult<(Arc<dyn RelayChainInterface + 'static>, PeerId)> {
 	let relay_chain_node = match collator_options.relay_chain_mode {
 		cumulus_client_cli::RelayChainMode::Embedded => polkadot_test_service::new_full(
 			relay_chain_config,
@@ -279,20 +279,25 @@ async fn build_relay_chain_interface(
 				rpc_target_urls,
 			)
 			.await
-			.map(|r| r.0)
+			.map(|r| (r.0, r.2.local_peer_id()))
 		},
 	};
 
+	let relay_chain_peer_id = relay_chain_node.network.local_peer_id();
+
 	task_manager.add_child(relay_chain_node.task_manager);
 	tracing::info!("Using inprocess node.");
-	Ok(Arc::new(RelayChainInProcessInterface::new(
-		relay_chain_node.client.clone(),
-		relay_chain_node.backend.clone(),
-		relay_chain_node.sync_service.clone(),
-		relay_chain_node.overseer_handle.ok_or(RelayChainError::GenericError(
-			"Overseer should be running in full node.".to_string(),
-		))?,
-	)))
+	Ok((
+		Arc::new(RelayChainInProcessInterface::new(
+			relay_chain_node.client.clone(),
+			relay_chain_node.backend.clone(),
+			relay_chain_node.sync_service.clone(),
+			relay_chain_node.overseer_handle.ok_or(RelayChainError::GenericError(
+				"Overseer should be running in full node.".to_string(),
+			))?,
+		)),
+		relay_chain_peer_id,
+	))
 }
 
 /// Start a node with the given parachain `Configuration` and relay chain `Configuration`.
@@ -332,7 +337,7 @@ where
 
 	let block_import = params.other.0;
 	let slot_based_handle = params.other.1;
-	let relay_chain_interface = build_relay_chain_interface(
+	let (relay_chain_interface, relay_chain_peer_id) = build_relay_chain_interface(
 		relay_chain_config,
 		parachain_config.prometheus_registry(),
 		collator_key.clone(),
@@ -434,7 +439,7 @@ where
 		prometheus_registry: None,
 	})?;
 
-	let collator_peer_id = network.local_peer_id();
+	let collator_peer_id = relay_chain_peer_id;
 	if let Some(collator_key) = collator_key {
 		let proposer = sc_basic_authorship::ProposerFactory::new(
 			task_manager.spawn_handle(),

prdoc/pr_12388.prdoc

@@ -0,0 +1,9 @@
+title: 'collator-protocol revamp:  Send RC PeerId with ApprovedPeer UMP signal'
+doc:
+- audience: Node Dev
+  description: |-
+    Currently the collator sends its parachain PeerId within the ApprovedPeer UMP signal, which is wrong because this peer id is not known on the relay chain.
+    As a result on CandidateIncluded event the score is bumped for the wrong PeerId and the collator remains with zero reputation no matter its honest work.
+crates:
+- name: polkadot-omni-node-lib
+  bump: patch

templates/parachain/node/src/service.rs

@@ -303,7 +303,7 @@ pub async fn start_parachain_node(
 			),
 		})
 		.await?;
-	let collator_peer_id = network.local_peer_id();
+	let collator_peer_id = relay_chain_network.local_peer_id();
 
 	if parachain_config.offchain_worker.enabled {
 		use futures::FutureExt;