Always send full parent header, not only hash, part of collation response

polkadot/node/network/collator-protocol/src/collator_side/collation.rs

@@ -25,9 +25,8 @@ use polkadot_node_network_protocol::{
 	PeerId,
 };
 use polkadot_node_primitives::PoV;
-use polkadot_node_subsystem::messages::ParentHeadData;
 use polkadot_primitives::{
-	vstaging::CandidateReceiptV2 as CandidateReceipt, CandidateHash, Hash, Id as ParaId,
+	vstaging::CandidateReceiptV2 as CandidateReceipt, CandidateHash, Hash, HeadData, Id as ParaId,
 };
 
 /// The status of a collation as seen from the collator.
@@ -72,8 +71,8 @@ pub struct Collation {
 	pub receipt: CandidateReceipt,
 	/// Proof to verify the state transition of the parachain.
 	pub pov: PoV,
-	/// Parent head-data (or just hash).
-	pub parent_head_data: ParentHeadData,
+	/// Parent head-data
+	pub parent_head_data: HeadData,
 	/// Collation status.
 	pub status: CollationStatus,
 }

polkadot/node/network/collator-protocol/src/collator_side/mod.rs

@@ -39,9 +39,7 @@ use polkadot_node_network_protocol::{
 };
 use polkadot_node_primitives::{CollationSecondedSignal, PoV, Statement};
 use polkadot_node_subsystem::{
-	messages::{
-		CollatorProtocolMessage, NetworkBridgeEvent, NetworkBridgeTxMessage, ParentHeadData,
-	},
+	messages::{CollatorProtocolMessage, NetworkBridgeEvent, NetworkBridgeTxMessage},
 	overseer, FromOrchestra, OverseerSignal,
 };
 use polkadot_node_subsystem_util::{
@@ -162,9 +160,9 @@ impl ValidatorGroup {
 				.get(candidate_hash)
 				.map_or(true, |advertised| !advertised[validator_index])
 			{
-				return ShouldAdvertiseTo::Yes
+				return ShouldAdvertiseTo::Yes;
 			} else {
-				return ShouldAdvertiseTo::AlreadyAdvertised
+				return ShouldAdvertiseTo::AlreadyAdvertised;
 			}
 		}
 
@@ -401,7 +399,7 @@ async fn distribute_collation<Context>(
 				candidate_hash = ?candidate_hash,
 				"Candidate relay parent is out of our view",
 			);
-			return Ok(())
+			return Ok(());
 		},
 	};
 
@@ -415,7 +413,7 @@ async fn distribute_collation<Context>(
 			"Attempting to distribute collation for a core we are not assigned to ",
 		);
 
-		return Ok(())
+		return Ok(());
 	};
 
 	let current_collations_count = per_relay_parent
@@ -431,7 +429,7 @@ async fn distribute_collation<Context>(
 			collations_limit,
 			core_index.0,
 		);
-		return Ok(())
+		return Ok(());
 	}
 
 	// We have already seen collation for this relay parent.
@@ -442,7 +440,7 @@ async fn distribute_collation<Context>(
 			?candidate_hash,
 			"Already seen this candidate",
 		);
-		return Ok(())
+		return Ok(());
 	}
 
 	let elastic_scaling = per_relay_parent.assignments.len() > 1;
@@ -468,7 +466,7 @@ async fn distribute_collation<Context>(
 			"there are no validators assigned to core",
 		);
 
-		return Ok(())
+		return Ok(());
 	}
 
 	// It's important to insert new collation interests **before**
@@ -508,12 +506,6 @@ async fn distribute_collation<Context>(
 		state.collation_result_senders.insert(candidate_hash, result_sender);
 	}
 
-	let parent_head_data = if elastic_scaling {
-		ParentHeadData::WithData { hash: parent_head_data_hash, head_data: parent_head_data }
-	} else {
-		ParentHeadData::OnlyHash(parent_head_data_hash)
-	};
-
 	let para_head = receipt.descriptor.para_head();
 	per_relay_parent.collations.insert(
 		candidate_hash,
@@ -716,7 +708,7 @@ async fn advertise_collation<Context>(
 				?core_index,
 				"Skipping advertising to validator, validator group for core not found",
 			);
-			return
+			return;
 		};
 
 		let should_advertise = validator_group.should_advertise_to(candidate_hash, peer_ids, &peer);
@@ -731,7 +723,7 @@ async fn advertise_collation<Context>(
 					reason = ?should_advertise,
 					"Not advertising collation"
 				);
-				continue
+				continue;
 			},
 		}
 
@@ -859,24 +851,19 @@ async fn send_collation(
 	request: VersionedCollationRequest,
 	receipt: CandidateReceipt,
 	pov: PoV,
-	parent_head_data: ParentHeadData,
+	parent_head_data: HeadData,
 ) {
 	let (tx, rx) = oneshot::channel();
 
 	let relay_parent = request.relay_parent();
 	let peer_id = request.peer_id();
 	let candidate_hash = receipt.hash();
 
-	let result = match parent_head_data {
-		ParentHeadData::WithData { head_data, .. } =>
-			Ok(request_v2::CollationFetchingResponse::CollationWithParentHeadData {
-				receipt,
-				pov,
-				parent_head_data: head_data,
-			}),
-		ParentHeadData::OnlyHash(_) =>
-			Ok(request_v2::CollationFetchingResponse::Collation(receipt, pov)),
-	};
+	let result = Ok(request_v2::CollationFetchingResponse::CollationWithParentHeadData {
+		receipt,
+		pov,
+		parent_head_data,
+	});
 
 	let response =
 		OutgoingResponse { result, reputation_changes: Vec::new(), sent_feedback: Some(tx) };
@@ -928,8 +915,8 @@ async fn handle_incoming_peer_message<Context>(
 			))
 			.await;
 		},
-		CollationProtocols::V1(V1::AdvertiseCollation(_)) |
-		CollationProtocols::V2(V2::AdvertiseCollation { .. }) => {
+		CollationProtocols::V1(V1::AdvertiseCollation(_))
+		| CollationProtocols::V2(V2::AdvertiseCollation { .. }) => {
 			gum::trace!(
 				target: LOG_TARGET,
 				?origin,
@@ -992,7 +979,7 @@ async fn handle_incoming_peer_message<Context>(
 								candidate_hash = ?&statement.payload().candidate_hash(),
 								"Seconded statement relay parent is out of our view",
 							);
-							return Ok(())
+							return Ok(());
 						},
 					};
 					match relay_parent.collations.get(&statement.payload().candidate_hash()) {
@@ -1045,13 +1032,14 @@ async fn handle_incoming_request<Context>(
 						"received a `RequestCollation` for a relay parent out of our view",
 					);
 
-					return Ok(())
+					return Ok(());
 				},
 			};
 
 			let collation_with_core = match &req {
-				VersionedCollationRequest::V2(req) =>
-					per_relay_parent.collations.get_mut(&req.payload.candidate_hash),
+				VersionedCollationRequest::V2(req) => {
+					per_relay_parent.collations.get_mut(&req.payload.candidate_hash)
+				},
 			};
 			let (receipt, pov, parent_head_data) =
 				if let Some(collation_with_core) = collation_with_core {
@@ -1069,7 +1057,7 @@ async fn handle_incoming_request<Context>(
 						"received a `RequestCollation` for a relay parent we don't have collation stored.",
 					);
 
-					return Ok(())
+					return Ok(());
 				};
 
 			state.metrics.on_collation_sent_requested();
@@ -1089,7 +1077,7 @@ async fn handle_incoming_request<Context>(
 					COST_APPARENT_FLOOD.into(),
 				)
 				.await;
-				return Ok(())
+				return Ok(());
 			}
 
 			if waiting.collation_fetch_active {
@@ -1131,7 +1119,7 @@ async fn handle_peer_view_change<Context>(
 	view: View,
 ) {
 	let Some(PeerData { view: current, unknown_heads }) = state.peer_data.get_mut(&peer_id) else {
-		return
+		return;
 	};
 
 	let added: Vec<Hash> = view.difference(&*current).cloned().collect();
@@ -1157,13 +1145,13 @@ async fn handle_peer_view_change<Context>(
 
 				unknown_heads.insert(added, ());
 
-				continue
+				continue;
 			},
 		};
 
 		for block_hash in block_hashes {
 			let Some(per_relay_parent) = state.per_relay_parent.get_mut(block_hash) else {
-				continue
+				continue;
 			};
 
 			advertise_collation(
@@ -1207,7 +1195,7 @@ async fn handle_network_msg<Context>(
 						?err,
 						"Unsupported protocol version"
 					);
-					return Ok(())
+					return Ok(());
 				},
 			};
 			if version == CollationVersion::V1 {
@@ -1224,7 +1212,7 @@ async fn handle_network_msg<Context>(
 					PeerSet::Collation,
 				))
 				.await;
-				return Ok(())
+				return Ok(());
 			}
 
 			state.peer_data.entry(peer_id).or_insert_with(|| PeerData {
@@ -1298,27 +1286,27 @@ async fn process_block_events<Context>(
 				?para_id,
 				"Failed to get relay chain block number",
 			);
-			return
+			return;
 		};
 
 		for ev in events {
 			match ev {
 				CandidateEvent::CandidateIncluded(receipt, _, _, _) => {
 					if receipt.descriptor.para_id() != para_id {
-						continue
+						continue;
 					}
 					collation_tracker.collation_included(block_number, leaf, receipt, metrics);
 				},
 				CandidateEvent::CandidateBacked(receipt, _, _, _) => {
 					if receipt.descriptor.para_id() != para_id {
-						continue
+						continue;
 					}
 
 					let Some(block_number) = maybe_block_number else { continue };
 					let Some(stats) =
 						collation_tracker.collation_backed(block_number, leaf, receipt, metrics)
 					else {
-						continue
+						continue;
 					};
 
 					// Continue measuring inclusion latency.

polkadot/node/network/collator-protocol/src/collator_side/tests/mod.rs

@@ -521,7 +521,8 @@ fn decode_collation_response(bytes: &[u8]) -> (CandidateReceipt, PoV) {
 	let response: CollationFetchingResponse =
 		CollationFetchingResponse::decode(&mut &bytes[..]).expect("Decoding should work");
 	match response {
-		CollationFetchingResponse::Collation(receipt, pov) => (receipt, pov),
+		CollationFetchingResponse::Collation(_, _) =>
+			panic!("Expected to always receive CollationWithParentHeadData"),
 		CollationFetchingResponse::CollationWithParentHeadData { receipt, pov, .. } =>
 			(receipt, pov),
 	}

prdoc/pr_8939.prdoc

@@ -0,0 +1,11 @@
+title: Always send full parent header, not only hash, part of collation response
+doc:
+- audience: Node Dev
+  description: |-
+    Implementation of https://github.com/paritytech/polkadot-sdk/issues/7733
+
+    # Description
+    Instead of **conditionally** sending the full parent header in the collation response we now **always** send it (never the hash of it).
+crates:
+- name: polkadot-collator-protocol
+  bump: patch