From 02e1a7f476d7d7c67153e975ab9a1bdc02ffea12 Mon Sep 17 00:00:00 2001 From: ordian <write@reusable.software> Date: Fri, 15 Mar 2024 20:43:28 +0100 Subject: [PATCH] collator protocol changes for elastic scaling (validator side) (#3302) Fixes #3128. This introduces a new variant for the collation response from the collator that includes the parent head data. For now, collators won't send this new variant. We'll need to change the collator side of the collator protocol to detect all the cores assigned to a para and send the parent head data in the case when it's more than 1 core. - [x] validate approach - [x] check head data hash --- polkadot/node/collation-generation/src/lib.rs | 8 +- .../node/collation-generation/src/tests.rs | 33 ++--- .../core/prospective-parachains/src/lib.rs | 17 ++- .../core/prospective-parachains/src/tests.rs | 4 +- .../src/collator_side/collation.rs | 4 +- .../src/collator_side/mod.rs | 65 +++++++--- .../src/collator_side/tests/mod.rs | 62 +++++---- .../tests/prospective_parachains.rs | 32 ++--- .../network/collator-protocol/src/error.rs | 10 +- .../src/validator_side/collation.rs | 13 +- .../src/validator_side/mod.rs | 86 ++++++++++--- .../tests/prospective_parachains.rs | 120 ++++++++++++++++++ .../protocol/src/request_response/mod.rs | 2 +- .../protocol/src/request_response/v1.rs | 15 ++- polkadot/node/subsystem-types/src/messages.rs | 53 +++++--- prdoc/pr_3302.prdoc | 15 +++ 16 files changed, 406 insertions(+), 133 deletions(-) create mode 100644 prdoc/pr_3302.prdoc diff --git a/polkadot/node/collation-generation/src/lib.rs b/polkadot/node/collation-generation/src/lib.rs index cfa75d7b441..a89351628a0 100644 --- a/polkadot/node/collation-generation/src/lib.rs +++ b/polkadot/node/collation-generation/src/lib.rs @@ -466,6 +466,7 @@ async fn construct_and_distribute_receipt( } = collation; let persisted_validation_data_hash = validation_data.hash(); + let parent_head_data = validation_data.parent_head.clone(); let parent_head_data_hash = validation_data.parent_head.hash(); // Apply compression to the block data. @@ -551,12 +552,13 @@ async fn construct_and_distribute_receipt( metrics.on_collation_generated(); sender - .send_message(CollatorProtocolMessage::DistributeCollation( - ccr, + .send_message(CollatorProtocolMessage::DistributeCollation { + candidate_receipt: ccr, parent_head_data_hash, pov, + parent_head_data, result_sender, - )) + }) .await; } diff --git a/polkadot/node/collation-generation/src/tests.rs b/polkadot/node/collation-generation/src/tests.rs index 9094f40cca8..eb0ede6ef6b 100644 --- a/polkadot/node/collation-generation/src/tests.rs +++ b/polkadot/node/collation-generation/src/tests.rs @@ -390,11 +390,11 @@ fn sends_distribute_collation_message() { assert_eq!(to_collator_protocol.len(), 1); match AllMessages::from(to_collator_protocol.pop().unwrap()) { - AllMessages::CollatorProtocol(CollatorProtocolMessage::DistributeCollation( - CandidateReceipt { descriptor, .. }, - _pov, - .., - )) => { + AllMessages::CollatorProtocol(CollatorProtocolMessage::DistributeCollation { + candidate_receipt, + .. + }) => { + let CandidateReceipt { descriptor, .. } = candidate_receipt; // signature generation is non-deterministic, so we can't just assert that the // expected descriptor is correct. What we can do is validate that the produced // descriptor has a valid signature, then just copy in the generated signature @@ -529,11 +529,11 @@ fn fallback_when_no_validation_code_hash_api() { assert_eq!(to_collator_protocol.len(), 1); match &to_collator_protocol[0] { - AllMessages::CollatorProtocol(CollatorProtocolMessage::DistributeCollation( - CandidateReceipt { descriptor, .. }, - _pov, - .., - )) => { + AllMessages::CollatorProtocol(CollatorProtocolMessage::DistributeCollation { + candidate_receipt, + .. + }) => { + let CandidateReceipt { descriptor, .. } = candidate_receipt; assert_eq!(expect_validation_code_hash, descriptor.validation_code_hash); }, _ => panic!("received wrong message type"), @@ -619,15 +619,16 @@ fn submit_collation_leads_to_distribution() { assert_matches!( overseer_recv(&mut virtual_overseer).await, - AllMessages::CollatorProtocol(CollatorProtocolMessage::DistributeCollation( - ccr, + AllMessages::CollatorProtocol(CollatorProtocolMessage::DistributeCollation { + candidate_receipt, parent_head_data_hash, .. - )) => { + }) => { + let CandidateReceipt { descriptor, .. } = candidate_receipt; assert_eq!(parent_head_data_hash, parent_head.hash()); - assert_eq!(ccr.descriptor().persisted_validation_data_hash, expected_pvd.hash()); - assert_eq!(ccr.descriptor().para_head, dummy_head_data().hash()); - assert_eq!(ccr.descriptor().validation_code_hash, validation_code_hash); + assert_eq!(descriptor.persisted_validation_data_hash, expected_pvd.hash()); + assert_eq!(descriptor.para_head, dummy_head_data().hash()); + assert_eq!(descriptor.validation_code_hash, validation_code_hash); } ); diff --git a/polkadot/node/core/prospective-parachains/src/lib.rs b/polkadot/node/core/prospective-parachains/src/lib.rs index 2b14e09b4fb..f5d50fb74fa 100644 --- a/polkadot/node/core/prospective-parachains/src/lib.rs +++ b/polkadot/node/core/prospective-parachains/src/lib.rs @@ -36,8 +36,9 @@ use futures::{channel::oneshot, prelude::*}; use polkadot_node_subsystem::{ messages::{ Ancestors, ChainApiMessage, FragmentTreeMembership, HypotheticalCandidate, - HypotheticalFrontierRequest, IntroduceCandidateRequest, ProspectiveParachainsMessage, - ProspectiveValidationDataRequest, RuntimeApiMessage, RuntimeApiRequest, + HypotheticalFrontierRequest, IntroduceCandidateRequest, ParentHeadData, + ProspectiveParachainsMessage, ProspectiveValidationDataRequest, RuntimeApiMessage, + RuntimeApiRequest, }, overseer, ActiveLeavesUpdate, FromOrchestra, OverseerSignal, SpawnedSubsystem, SubsystemError, }; @@ -764,8 +765,14 @@ fn answer_prospective_validation_data_request( Some(s) => s, }; - let mut head_data = - storage.head_data_by_hash(&request.parent_head_data_hash).map(|x| x.clone()); + let (mut head_data, parent_head_data_hash) = match request.parent_head_data { + ParentHeadData::OnlyHash(parent_head_data_hash) => ( + storage.head_data_by_hash(&parent_head_data_hash).map(|x| x.clone()), + parent_head_data_hash, + ), + ParentHeadData::WithData { head_data, hash } => (Some(head_data), hash), + }; + let mut relay_parent_info = None; let mut max_pov_size = None; @@ -783,7 +790,7 @@ fn answer_prospective_validation_data_request( } if head_data.is_none() { let required_parent = &fragment_tree.scope().base_constraints().required_parent; - if required_parent.hash() == request.parent_head_data_hash { + if required_parent.hash() == parent_head_data_hash { head_data = Some(required_parent.clone()); } } diff --git a/polkadot/node/core/prospective-parachains/src/tests.rs b/polkadot/node/core/prospective-parachains/src/tests.rs index 0beddbf1416..0e0079c02bb 100644 --- a/polkadot/node/core/prospective-parachains/src/tests.rs +++ b/polkadot/node/core/prospective-parachains/src/tests.rs @@ -19,7 +19,7 @@ use assert_matches::assert_matches; use polkadot_node_subsystem::{ errors::RuntimeApiError, messages::{ - AllMessages, HypotheticalFrontierRequest, ProspectiveParachainsMessage, + AllMessages, HypotheticalFrontierRequest, ParentHeadData, ProspectiveParachainsMessage, ProspectiveValidationDataRequest, }, }; @@ -468,7 +468,7 @@ async fn get_pvd( let request = ProspectiveValidationDataRequest { para_id, candidate_relay_parent, - parent_head_data_hash: parent_head_data.hash(), + parent_head_data: ParentHeadData::OnlyHash(parent_head_data.hash()), }; let (tx, rx) = oneshot::channel(); virtual_overseer diff --git a/polkadot/node/network/collator-protocol/src/collator_side/collation.rs b/polkadot/node/network/collator-protocol/src/collator_side/collation.rs index 53f947142d1..dc1ee725462 100644 --- a/polkadot/node/network/collator-protocol/src/collator_side/collation.rs +++ b/polkadot/node/network/collator-protocol/src/collator_side/collation.rs @@ -27,7 +27,7 @@ use polkadot_node_network_protocol::{ PeerId, }; use polkadot_node_primitives::PoV; -use polkadot_primitives::{CandidateHash, CandidateReceipt, Hash, Id as ParaId}; +use polkadot_primitives::{CandidateHash, CandidateReceipt, Hash, HeadData, Id as ParaId}; /// The status of a collation as seen from the collator. pub enum CollationStatus { @@ -63,6 +63,8 @@ pub struct Collation { pub parent_head_data_hash: Hash, /// Proof to verify the state transition of the parachain. pub pov: PoV, + /// Parent head-data needed for elastic scaling. + pub parent_head_data: HeadData, /// Collation status. pub status: CollationStatus, } diff --git a/polkadot/node/network/collator-protocol/src/collator_side/mod.rs b/polkadot/node/network/collator-protocol/src/collator_side/mod.rs index 8fb0bb21544..19cc1eb1a57 100644 --- a/polkadot/node/network/collator-protocol/src/collator_side/mod.rs +++ b/polkadot/node/network/collator-protocol/src/collator_side/mod.rs @@ -55,7 +55,7 @@ use polkadot_node_subsystem_util::{ }; use polkadot_primitives::{ AuthorityDiscoveryId, CandidateHash, CandidateReceipt, CollatorPair, CoreIndex, CoreState, - GroupIndex, Hash, Id as ParaId, SessionIndex, + GroupIndex, Hash, HeadData, Id as ParaId, SessionIndex, }; use super::LOG_TARGET; @@ -347,6 +347,7 @@ async fn distribute_collation<Context>( receipt: CandidateReceipt, parent_head_data_hash: Hash, pov: PoV, + parent_head_data: HeadData, result_sender: Option<oneshot::Sender<CollationSecondedSignal>>, ) -> Result<()> { let candidate_relay_parent = receipt.descriptor.relay_parent; @@ -465,7 +466,13 @@ async fn distribute_collation<Context>( per_relay_parent.collations.insert( candidate_hash, - Collation { receipt, parent_head_data_hash, pov, status: CollationStatus::Created }, + Collation { + receipt, + parent_head_data_hash, + pov, + parent_head_data, + status: CollationStatus::Created, + }, ); // If prospective parachains are disabled, a leaf should be known to peer. @@ -763,20 +770,26 @@ async fn process_msg<Context>( CollateOn(id) => { state.collating_on = Some(id); }, - DistributeCollation(receipt, parent_head_data_hash, pov, result_sender) => { + DistributeCollation { + candidate_receipt, + parent_head_data_hash, + pov, + parent_head_data, + result_sender, + } => { let _span1 = state .span_per_relay_parent - .get(&receipt.descriptor.relay_parent) + .get(&candidate_receipt.descriptor.relay_parent) .map(|s| s.child("distributing-collation")); let _span2 = jaeger::Span::new(&pov, "distributing-collation"); match state.collating_on { - Some(id) if receipt.descriptor.para_id != id => { + Some(id) if candidate_receipt.descriptor.para_id != id => { // If the ParaId of a collation requested to be distributed does not match // the one we expect, we ignore the message. gum::warn!( target: LOG_TARGET, - para_id = %receipt.descriptor.para_id, + para_id = %candidate_receipt.descriptor.para_id, collating_on = %id, "DistributeCollation for unexpected para_id", ); @@ -788,9 +801,10 @@ async fn process_msg<Context>( runtime, state, id, - receipt, + candidate_receipt, parent_head_data_hash, pov, + parent_head_data, result_sender, ) .await?; @@ -798,7 +812,7 @@ async fn process_msg<Context>( None => { gum::warn!( target: LOG_TARGET, - para_id = %receipt.descriptor.para_id, + para_id = %candidate_receipt.descriptor.para_id, "DistributeCollation message while not collating on any", ); }, @@ -835,6 +849,7 @@ async fn send_collation( request: VersionedCollationRequest, receipt: CandidateReceipt, pov: PoV, + _parent_head_data: HeadData, ) { let (tx, rx) = oneshot::channel(); @@ -842,13 +857,22 @@ async fn send_collation( let peer_id = request.peer_id(); let candidate_hash = receipt.hash(); - // The response payload is the same for both versions of protocol + // The response payload is the same for v1 and v2 versions of protocol // and doesn't have v2 alias for simplicity. - let response = OutgoingResponse { - result: Ok(request_v1::CollationFetchingResponse::Collation(receipt, pov)), - reputation_changes: Vec::new(), - sent_feedback: Some(tx), - }; + // For now, we don't send parent head data to the collation requester. + let result = + // if assigned_multiple_cores { + // Ok(request_v1::CollationFetchingResponse::CollationWithParentHeadData { + // receipt, + // pov, + // parent_head_data, + // }) + // } else { + Ok(request_v1::CollationFetchingResponse::Collation(receipt, pov)) + // } + ; + let response = + OutgoingResponse { result, reputation_changes: Vec::new(), sent_feedback: Some(tx) }; if let Err(_) = request.send_outgoing_response(response) { gum::warn!(target: LOG_TARGET, "Sending collation response failed"); @@ -1027,9 +1051,13 @@ async fn handle_incoming_request<Context>( return Ok(()) }, }; - let (receipt, pov) = if let Some(collation) = collation { + let (receipt, pov, parent_head_data) = if let Some(collation) = collation { collation.status.advance_to_requested(); - (collation.receipt.clone(), collation.pov.clone()) + ( + collation.receipt.clone(), + collation.pov.clone(), + collation.parent_head_data.clone(), + ) } else { gum::warn!( target: LOG_TARGET, @@ -1068,7 +1096,7 @@ async fn handle_incoming_request<Context>( waiting.collation_fetch_active = true; // Obtain a timer for sending collation let _ = state.metrics.time_collation_distribution("send"); - send_collation(state, req, receipt, pov).await; + send_collation(state, req, receipt, pov, parent_head_data).await; } }, Some(our_para_id) => { @@ -1453,8 +1481,9 @@ async fn run_inner<Context>( if let Some(collation) = next_collation { let receipt = collation.receipt.clone(); let pov = collation.pov.clone(); + let parent_head_data = collation.parent_head_data.clone(); - send_collation(&mut state, next, receipt, pov).await; + send_collation(&mut state, next, receipt, pov, parent_head_data).await; } }, (candidate_hash, peer_id) = state.advertisement_timeouts.select_next_some() => { diff --git a/polkadot/node/network/collator-protocol/src/collator_side/tests/mod.rs b/polkadot/node/network/collator-protocol/src/collator_side/tests/mod.rs index 1b1194c7270..beda941d37a 100644 --- a/polkadot/node/network/collator-protocol/src/collator_side/tests/mod.rs +++ b/polkadot/node/network/collator-protocol/src/collator_side/tests/mod.rs @@ -356,12 +356,13 @@ async fn distribute_collation_with_receipt( ) -> DistributeCollation { overseer_send( virtual_overseer, - CollatorProtocolMessage::DistributeCollation( - candidate.clone(), + CollatorProtocolMessage::DistributeCollation { + candidate_receipt: candidate.clone(), parent_head_data_hash, - pov.clone(), - None, - ), + pov: pov.clone(), + parent_head_data: HeadData(vec![1, 2, 3]), + result_sender: None, + }, ) .await; @@ -627,6 +628,18 @@ async fn send_peer_view_change( .await; } +fn decode_collation_response(bytes: &[u8]) -> (CandidateReceipt, PoV) { + let response: request_v1::CollationFetchingResponse = + request_v1::CollationFetchingResponse::decode(&mut &bytes[..]) + .expect("Decoding should work"); + match response { + request_v1::CollationFetchingResponse::Collation(receipt, pov) => (receipt, pov), + request_v1::CollationFetchingResponse::CollationWithParentHeadData { + receipt, pov, .. + } => (receipt, pov), + } +} + #[test] fn advertise_and_send_collation() { let mut test_state = TestState::default(); @@ -736,12 +749,10 @@ fn advertise_and_send_collation() { assert_matches!( rx.await, Ok(full_response) => { - let request_v1::CollationFetchingResponse::Collation(receipt, pov): request_v1::CollationFetchingResponse - = request_v1::CollationFetchingResponse::decode( - &mut full_response.result - .expect("We should have a proper answer").as_ref() - ) - .expect("Decoding should work"); + let (receipt, pov) = decode_collation_response( + full_response.result + .expect("We should have a proper answer").as_ref() + ); assert_eq!(receipt, candidate); assert_eq!(pov, pov_block); } @@ -1338,12 +1349,10 @@ where let feedback_tx = assert_matches!( rx.await, Ok(full_response) => { - let request_v1::CollationFetchingResponse::Collation(receipt, pov): request_v1::CollationFetchingResponse - = request_v1::CollationFetchingResponse::decode( - &mut full_response.result - .expect("We should have a proper answer").as_ref() - ) - .expect("Decoding should work"); + let (receipt, pov) = decode_collation_response( + full_response.result + .expect("We should have a proper answer").as_ref() + ); assert_eq!(receipt, candidate); assert_eq!(pov, pov_block); @@ -1375,12 +1384,10 @@ where assert_matches!( rx.await, Ok(full_response) => { - let request_v1::CollationFetchingResponse::Collation(receipt, pov): request_v1::CollationFetchingResponse - = request_v1::CollationFetchingResponse::decode( - &mut full_response.result - .expect("We should have a proper answer").as_ref() - ) - .expect("Decoding should work"); + let (receipt, pov) = decode_collation_response( + full_response.result + .expect("We should have a proper answer").as_ref() + ); assert_eq!(receipt, candidate); assert_eq!(pov, pov_block); @@ -1469,11 +1476,10 @@ fn connect_to_buffered_groups() { assert_matches!( rx.await, Ok(full_response) => { - let request_v1::CollationFetchingResponse::Collation(..) = - request_v1::CollationFetchingResponse::decode( - &mut full_response.result.expect("We should have a proper answer").as_ref(), - ) - .expect("Decoding should work"); + let _ = decode_collation_response( + full_response.result + .expect("We should have a proper answer").as_ref() + ); } ); diff --git a/polkadot/node/network/collator-protocol/src/collator_side/tests/prospective_parachains.rs b/polkadot/node/network/collator-protocol/src/collator_side/tests/prospective_parachains.rs index fd9d7a746eb..2bb7002a4f4 100644 --- a/polkadot/node/network/collator-protocol/src/collator_side/tests/prospective_parachains.rs +++ b/polkadot/node/network/collator-protocol/src/collator_side/tests/prospective_parachains.rs @@ -271,12 +271,13 @@ fn distribute_collation_from_implicit_view() { .build(); overseer_send( virtual_overseer, - CollatorProtocolMessage::DistributeCollation( - candidate.clone(), + CollatorProtocolMessage::DistributeCollation { + candidate_receipt: candidate.clone(), parent_head_data_hash, - pov.clone(), - None, - ), + pov: pov.clone(), + parent_head_data: HeadData(vec![1, 2, 3]), + result_sender: None, + }, ) .await; @@ -351,12 +352,13 @@ fn distribute_collation_up_to_limit() { .build(); overseer_send( virtual_overseer, - CollatorProtocolMessage::DistributeCollation( - candidate.clone(), + CollatorProtocolMessage::DistributeCollation { + candidate_receipt: candidate.clone(), parent_head_data_hash, - pov.clone(), - None, - ), + pov: pov.clone(), + parent_head_data: HeadData(vec![1, 2, 3]), + result_sender: None, + }, ) .await; @@ -469,12 +471,10 @@ fn advertise_and_send_collation_by_hash() { rx.await, Ok(full_response) => { // Response is the same for v2. - let request_v1::CollationFetchingResponse::Collation(receipt, pov): request_v1::CollationFetchingResponse - = request_v1::CollationFetchingResponse::decode( - &mut full_response.result - .expect("We should have a proper answer").as_ref() - ) - .expect("Decoding should work"); + let (receipt, pov) = decode_collation_response( + full_response.result + .expect("We should have a proper answer").as_ref() + ); assert_eq!(receipt, candidate); assert_eq!(pov, pov_block); } diff --git a/polkadot/node/network/collator-protocol/src/error.rs b/polkadot/node/network/collator-protocol/src/error.rs index 9348198e708..0f5e0699d85 100644 --- a/polkadot/node/network/collator-protocol/src/error.rs +++ b/polkadot/node/network/collator-protocol/src/error.rs @@ -89,13 +89,21 @@ pub enum SecondingError { #[error("Received duplicate collation from the peer")] Duplicate, + + #[error("The provided parent head data does not match the hash")] + ParentHeadDataMismatch, } impl SecondingError { /// Returns true if an error indicates that a peer is malicious. pub fn is_malicious(&self) -> bool { use SecondingError::*; - matches!(self, PersistedValidationDataMismatch | CandidateHashMismatch | Duplicate) + matches!( + self, + PersistedValidationDataMismatch | + CandidateHashMismatch | + Duplicate | ParentHeadDataMismatch + ) } } diff --git a/polkadot/node/network/collator-protocol/src/validator_side/collation.rs b/polkadot/node/network/collator-protocol/src/validator_side/collation.rs index d6f34fc81b8..8c3889a3554 100644 --- a/polkadot/node/network/collator-protocol/src/validator_side/collation.rs +++ b/polkadot/node/network/collator-protocol/src/validator_side/collation.rs @@ -41,7 +41,8 @@ use polkadot_node_subsystem_util::{ metrics::prometheus::prometheus::HistogramTimer, runtime::ProspectiveParachainsMode, }; use polkadot_primitives::{ - CandidateHash, CandidateReceipt, CollatorId, Hash, Id as ParaId, PersistedValidationData, + CandidateHash, CandidateReceipt, CollatorId, Hash, HeadData, Id as ParaId, + PersistedValidationData, }; use tokio_util::sync::CancellationToken; @@ -120,7 +121,7 @@ impl PendingCollation { } } -/// v2 advertisement that was rejected by the backing +/// v2 or v3 advertisement that was rejected by the backing /// subsystem. Validator may fetch it later if its fragment /// membership gets recognized before relay parent goes out of view. #[derive(Debug, Clone)] @@ -143,6 +144,7 @@ pub fn fetched_collation_sanity_check( advertised: &PendingCollation, fetched: &CandidateReceipt, persisted_validation_data: &PersistedValidationData, + maybe_parent_head_and_hash: Option<(HeadData, Hash)>, ) -> Result<(), SecondingError> { if persisted_validation_data.hash() != fetched.descriptor().persisted_validation_data_hash { Err(SecondingError::PersistedValidationDataMismatch) @@ -151,6 +153,8 @@ pub fn fetched_collation_sanity_check( .map_or(false, |pc| pc.candidate_hash() != fetched.hash()) { Err(SecondingError::CandidateHashMismatch) + } else if maybe_parent_head_and_hash.map_or(false, |(head, hash)| head.hash() != hash) { + Err(SecondingError::ParentHeadDataMismatch) } else { Ok(()) } @@ -176,6 +180,9 @@ pub struct PendingCollationFetch { pub candidate_receipt: CandidateReceipt, /// Proof of validity. pub pov: PoV, + /// Optional parachain parent head data. + /// Only needed for elastic scaling. + pub maybe_parent_head_data: Option<HeadData>, } /// The status of the collations in [`CollationsPerRelayParent`]. @@ -359,7 +366,7 @@ impl Future for CollationFetchRequest { }); match &res { - Poll::Ready((_, Ok(request_v1::CollationFetchingResponse::Collation(..)))) => { + Poll::Ready((_, Ok(_))) => { self.span.as_mut().map(|s| s.add_string_tag("success", "true")); }, Poll::Ready((_, Err(_))) => { diff --git a/polkadot/node/network/collator-protocol/src/validator_side/mod.rs b/polkadot/node/network/collator-protocol/src/validator_side/mod.rs index a1b93fff348..d23279e8754 100644 --- a/polkadot/node/network/collator-protocol/src/validator_side/mod.rs +++ b/polkadot/node/network/collator-protocol/src/validator_side/mod.rs @@ -44,7 +44,7 @@ use polkadot_node_subsystem::{ jaeger, messages::{ CanSecondRequest, CandidateBackingMessage, CollatorProtocolMessage, IfDisconnected, - NetworkBridgeEvent, NetworkBridgeTxMessage, ProspectiveParachainsMessage, + NetworkBridgeEvent, NetworkBridgeTxMessage, ParentHeadData, ProspectiveParachainsMessage, ProspectiveValidationDataRequest, }, overseer, CollatorProtocolSenderTrait, FromOrchestra, OverseerSignal, PerLeafSpan, @@ -55,7 +55,7 @@ use polkadot_node_subsystem_util::{ runtime::{prospective_parachains_mode, ProspectiveParachainsMode}, }; use polkadot_primitives::{ - CandidateHash, CollatorId, CoreState, Hash, Id as ParaId, OccupiedCoreAssumption, + CandidateHash, CollatorId, CoreState, Hash, HeadData, Id as ParaId, OccupiedCoreAssumption, PersistedValidationData, }; @@ -723,7 +723,7 @@ async fn request_collation( pending_collation, collator_id: collator_id.clone(), collator_protocol_version: peer_protocol_version, - from_collator: response_recv.boxed(), + from_collator: response_recv, cancellation_token: cancellation_token.clone(), span: state .span_per_relay_parent @@ -889,16 +889,16 @@ async fn process_incoming_peer_message<Context>( modify_reputation(&mut state.reputation, ctx.sender(), origin, rep).await; } }, - Versioned::V2(V2::AdvertiseCollation { + Versioned::V3(V2::AdvertiseCollation { relay_parent, candidate_hash, parent_head_data_hash, }) | - Versioned::V3(V2::AdvertiseCollation { + Versioned::V2(V2::AdvertiseCollation { relay_parent, candidate_hash, parent_head_data_hash, - }) => + }) => { if let Err(err) = handle_advertisement( ctx.sender(), state, @@ -920,7 +920,8 @@ async fn process_incoming_peer_message<Context>( if let Some(rep) = err.reputation_changes() { modify_reputation(&mut state.reputation, ctx.sender(), origin, rep).await; } - }, + } + }, Versioned::V1(V1::CollationSeconded(..)) | Versioned::V2(V2::CollationSeconded(..)) | Versioned::V3(V2::CollationSeconded(..)) => { @@ -1477,7 +1478,7 @@ async fn process_msg<Context>( "CollateOn message is not expected on the validator side of the protocol", ); }, - DistributeCollation(..) => { + DistributeCollation { .. } => { gum::warn!( target: LOG_TARGET, "DistributeCollation message is not expected on the validator side of the protocol", @@ -1776,14 +1777,21 @@ async fn request_prospective_validation_data<Sender>( candidate_relay_parent: Hash, parent_head_data_hash: Hash, para_id: ParaId, + maybe_parent_head_data: Option<HeadData>, ) -> std::result::Result<Option<PersistedValidationData>, SecondingError> where Sender: CollatorProtocolSenderTrait, { let (tx, rx) = oneshot::channel(); + let parent_head_data = if let Some(head_data) = maybe_parent_head_data { + ParentHeadData::WithData { head_data, hash: parent_head_data_hash } + } else { + ParentHeadData::OnlyHash(parent_head_data_hash) + }; + let request = - ProspectiveValidationDataRequest { para_id, candidate_relay_parent, parent_head_data_hash }; + ProspectiveValidationDataRequest { para_id, candidate_relay_parent, parent_head_data }; sender .send_message(ProspectiveParachainsMessage::GetProspectiveValidationData(request, tx)) @@ -1797,7 +1805,7 @@ where async fn kick_off_seconding<Context>( ctx: &mut Context, state: &mut State, - PendingCollationFetch { mut collation_event, candidate_receipt, pov }: PendingCollationFetch, + PendingCollationFetch { mut collation_event, candidate_receipt, pov, maybe_parent_head_data }: PendingCollationFetch, ) -> std::result::Result<(), SecondingError> { let pending_collation = collation_event.pending_collation; let relay_parent = pending_collation.relay_parent; @@ -1821,38 +1829,46 @@ async fn kick_off_seconding<Context>( collation_event.pending_collation.commitments_hash = Some(candidate_receipt.commitments_hash); - let pvd = match ( + let (maybe_pvd, maybe_parent_head_and_hash) = match ( collation_event.collator_protocol_version, collation_event.pending_collation.prospective_candidate, ) { (CollationVersion::V2, Some(ProspectiveCandidate { parent_head_data_hash, .. })) if per_relay_parent.prospective_parachains_mode.is_enabled() => - request_prospective_validation_data( + { + let pvd = request_prospective_validation_data( ctx.sender(), relay_parent, parent_head_data_hash, pending_collation.para_id, + maybe_parent_head_data.clone(), ) - .await?, + .await?; + + (pvd, maybe_parent_head_data.map(|head_data| (head_data, parent_head_data_hash))) + }, // Support V2 collators without async backing enabled. - (CollationVersion::V2, Some(_)) | (CollationVersion::V1, _) => - request_persisted_validation_data( + (CollationVersion::V2, Some(_)) | (CollationVersion::V1, _) => { + let pvd = request_persisted_validation_data( ctx.sender(), candidate_receipt.descriptor().relay_parent, candidate_receipt.descriptor().para_id, ) - .await?, + .await?; + (pvd, None) + }, _ => { // `handle_advertisement` checks for protocol mismatch. return Ok(()) }, - } - .ok_or(SecondingError::PersistedValidationDataNotFound)?; + }; + let pvd = maybe_pvd.ok_or(SecondingError::PersistedValidationDataNotFound)?; fetched_collation_sanity_check( &collation_event.pending_collation, &candidate_receipt, &pvd, + maybe_parent_head_and_hash, )?; ctx.send_message(CandidateBackingMessage::Second( @@ -1978,9 +1994,10 @@ async fn handle_collation_fetch_response( ); Err(None) }, - Ok(request_v1::CollationFetchingResponse::Collation(receipt, _)) - if receipt.descriptor().para_id != pending_collation.para_id => - { + Ok( + request_v1::CollationFetchingResponse::Collation(receipt, _) | + request_v1::CollationFetchingResponse::CollationWithParentHeadData { receipt, .. }, + ) if receipt.descriptor().para_id != pending_collation.para_id => { gum::debug!( target: LOG_TARGET, expected_para_id = ?pending_collation.para_id, @@ -2010,6 +2027,33 @@ async fn handle_collation_fetch_response( }, candidate_receipt, pov, + maybe_parent_head_data: None, + }) + }, + Ok(request_v2::CollationFetchingResponse::CollationWithParentHeadData { + receipt, + pov, + parent_head_data, + }) => { + gum::debug!( + target: LOG_TARGET, + para_id = %pending_collation.para_id, + hash = ?pending_collation.relay_parent, + candidate_hash = ?receipt.hash(), + "Received collation (v3)", + ); + let _span = jaeger::Span::new(&pov, "received-collation"); + + metrics_result = Ok(()); + Ok(PendingCollationFetch { + collation_event: CollationEvent { + collator_id, + pending_collation, + collator_protocol_version, + }, + candidate_receipt: receipt, + pov, + maybe_parent_head_data: Some(parent_head_data), }) }, }; diff --git a/polkadot/node/network/collator-protocol/src/validator_side/tests/prospective_parachains.rs b/polkadot/node/network/collator-protocol/src/validator_side/tests/prospective_parachains.rs index 23963e65554..eaa725f2642 100644 --- a/polkadot/node/network/collator-protocol/src/validator_side/tests/prospective_parachains.rs +++ b/polkadot/node/network/collator-protocol/src/validator_side/tests/prospective_parachains.rs @@ -754,6 +754,126 @@ fn fetched_collation_sanity_check() { }); } +#[test] +fn sanity_check_invalid_parent_head_data() { + let test_state = TestState::default(); + + test_harness(ReputationAggregator::new(|_| true), |test_harness| async move { + let TestHarness { mut virtual_overseer, .. } = test_harness; + + let pair = CollatorPair::generate().0; + + let head_c = Hash::from_low_u64_be(130); + let head_c_num = 3; + + update_view(&mut virtual_overseer, &test_state, vec![(head_c, head_c_num)], 1).await; + + let peer_a = PeerId::random(); + + connect_and_declare_collator( + &mut virtual_overseer, + peer_a, + pair.clone(), + test_state.chain_ids[0], + CollationVersion::V2, + ) + .await; + + let mut candidate = dummy_candidate_receipt_bad_sig(head_c, Some(Default::default())); + candidate.descriptor.para_id = test_state.chain_ids[0]; + + let commitments = CandidateCommitments { + head_data: HeadData(vec![1, 2, 3]), + horizontal_messages: Default::default(), + upward_messages: Default::default(), + new_validation_code: None, + processed_downward_messages: 0, + hrmp_watermark: 0, + }; + candidate.commitments_hash = commitments.hash(); + + let parent_head_data = HeadData(vec![4, 2, 0]); + let parent_head_data_hash = parent_head_data.hash(); + let wrong_parent_head_data = HeadData(vec![4, 2]); + + let mut pvd = dummy_pvd(); + pvd.parent_head = parent_head_data; + + candidate.descriptor.persisted_validation_data_hash = pvd.hash(); + + let candidate_hash = candidate.hash(); + + advertise_collation( + &mut virtual_overseer, + peer_a, + head_c, + Some((candidate_hash, parent_head_data_hash)), + ) + .await; + assert_matches!( + overseer_recv(&mut virtual_overseer).await, + AllMessages::CandidateBacking( + CandidateBackingMessage::CanSecond(request, tx), + ) => { + assert_eq!(request.candidate_hash, candidate_hash); + assert_eq!(request.candidate_para_id, test_state.chain_ids[0]); + assert_eq!(request.parent_head_data_hash, parent_head_data_hash); + tx.send(true).expect("receiving side should be alive"); + } + ); + + let response_channel = assert_fetch_collation_request( + &mut virtual_overseer, + head_c, + test_state.chain_ids[0], + Some(candidate_hash), + ) + .await; + + let pov = PoV { block_data: BlockData(vec![1]) }; + + response_channel + .send(Ok(( + request_v2::CollationFetchingResponse::CollationWithParentHeadData { + receipt: candidate.clone(), + pov: pov.clone(), + parent_head_data: wrong_parent_head_data, + } + .encode(), + ProtocolName::from(""), + ))) + .expect("Sending response should succeed"); + + // PVD request. + assert_matches!( + overseer_recv(&mut virtual_overseer).await, + AllMessages::ProspectiveParachains( + ProspectiveParachainsMessage::GetProspectiveValidationData(request, tx), + ) => { + assert_eq!(head_c, request.candidate_relay_parent); + assert_eq!(test_state.chain_ids[0], request.para_id); + tx.send(Some(pvd)).unwrap(); + } + ); + + // Reported malicious. + assert_matches!( + overseer_recv(&mut virtual_overseer).await, + AllMessages::NetworkBridgeTx( + NetworkBridgeTxMessage::ReportPeer(ReportPeerMessage::Single(peer_id, rep)), + ) => { + assert_eq!(peer_a, peer_id); + assert_eq!(rep.value, COST_REPORT_BAD.cost_or_benefit()); + } + ); + + test_helpers::Yield::new().await; + assert_matches!(virtual_overseer.recv().now_or_never(), None); + + virtual_overseer + }); +} + #[test] fn advertisement_spam_protection() { let test_state = TestState::default(); diff --git a/polkadot/node/network/protocol/src/request_response/mod.rs b/polkadot/node/network/protocol/src/request_response/mod.rs index a67d83aff0c..2fb62f56d10 100644 --- a/polkadot/node/network/protocol/src/request_response/mod.rs +++ b/polkadot/node/network/protocol/src/request_response/mod.rs @@ -31,7 +31,7 @@ //! data, like what is the corresponding response type. //! //! ## Versioning -//! +//! //! Versioning for request-response protocols can be done in multiple ways. //! //! If you're just changing the protocol name but the binary payloads are the same, just add a new diff --git a/polkadot/node/network/protocol/src/request_response/v1.rs b/polkadot/node/network/protocol/src/request_response/v1.rs index 0832593a6a3..ba29b32c4ce 100644 --- a/polkadot/node/network/protocol/src/request_response/v1.rs +++ b/polkadot/node/network/protocol/src/request_response/v1.rs @@ -22,7 +22,8 @@ use polkadot_node_primitives::{ AvailableData, DisputeMessage, ErasureChunk, PoV, Proof, UncheckedDisputeMessage, }; use polkadot_primitives::{ - CandidateHash, CandidateReceipt, CommittedCandidateReceipt, Hash, Id as ParaId, ValidatorIndex, + CandidateHash, CandidateReceipt, CommittedCandidateReceipt, Hash, HeadData, Id as ParaId, + ValidatorIndex, }; use super::{IsRequest, Protocol}; @@ -103,6 +104,18 @@ pub enum CollationFetchingResponse { /// Deliver requested collation. #[codec(index = 0)] Collation(CandidateReceipt, PoV), + + /// Deliver requested collation along with parent head data. + #[codec(index = 1)] + CollationWithParentHeadData { + /// The receipt of the candidate. + receipt: CandidateReceipt, + /// Candidate's proof of validity. + pov: PoV, + /// The head data of the candidate's parent. + /// This is needed for elastic scaling to work. + parent_head_data: HeadData, + }, } impl IsRequest for CollationFetchingRequest { diff --git a/polkadot/node/subsystem-types/src/messages.rs b/polkadot/node/subsystem-types/src/messages.rs index f11291fd0ea..eeb684149c8 100644 --- a/polkadot/node/subsystem-types/src/messages.rs +++ b/polkadot/node/subsystem-types/src/messages.rs @@ -46,11 +46,12 @@ use polkadot_primitives::{ vstaging::{ApprovalVotingParams, NodeFeatures}, AuthorityDiscoveryId, BackedCandidate, BlockNumber, CandidateEvent, CandidateHash, CandidateIndex, CandidateReceipt, CollatorId, CommittedCandidateReceipt, CoreState, - DisputeState, ExecutorParams, GroupIndex, GroupRotationInfo, Hash, Header as BlockHeader, - Id as ParaId, InboundDownwardMessage, InboundHrmpMessage, MultiDisputeStatementSet, - OccupiedCoreAssumption, PersistedValidationData, PvfCheckStatement, PvfExecKind, SessionIndex, - SessionInfo, SignedAvailabilityBitfield, SignedAvailabilityBitfields, ValidationCode, - ValidationCodeHash, ValidatorId, ValidatorIndex, ValidatorSignature, + DisputeState, ExecutorParams, GroupIndex, GroupRotationInfo, Hash, HeadData, + Header as BlockHeader, Id as ParaId, InboundDownwardMessage, InboundHrmpMessage, + MultiDisputeStatementSet, OccupiedCoreAssumption, PersistedValidationData, PvfCheckStatement, + PvfExecKind, SessionIndex, SessionInfo, SignedAvailabilityBitfield, + SignedAvailabilityBitfields, ValidationCode, ValidationCodeHash, ValidatorId, ValidatorIndex, + ValidatorSignature, }; use polkadot_statement_table::v2::Misbehavior; use std::{ @@ -207,16 +208,20 @@ pub enum CollatorProtocolMessage { /// This should be sent before any `DistributeCollation` message. CollateOn(ParaId), /// Provide a collation to distribute to validators with an optional result sender. - /// The second argument is the parent head-data hash. - /// - /// The result sender should be informed when at least one parachain validator seconded the - /// collation. It is also completely okay to just drop the sender. - DistributeCollation( - CandidateReceipt, - Hash, - PoV, - Option<oneshot::Sender<CollationSecondedSignal>>, - ), + DistributeCollation { + /// The receipt of the candidate. + candidate_receipt: CandidateReceipt, + /// The hash of the parent head-data. + /// Here to avoid computing the hash of the parent head data twice. + parent_head_data_hash: Hash, + /// Proof of validity. + pov: PoV, + /// This parent head-data is needed for elastic scaling. + parent_head_data: HeadData, + /// The result sender should be informed when at least one parachain validator seconded the + /// collation. It is also completely okay to just drop the sender. + result_sender: Option<oneshot::Sender<CollationSecondedSignal>>, + }, /// Report a collator as having provided an invalid collation. This should lead to disconnect /// and blacklist of the collator. ReportCollator(CollatorId), @@ -1104,8 +1109,22 @@ pub struct ProspectiveValidationDataRequest { pub para_id: ParaId, /// The relay-parent of the candidate. pub candidate_relay_parent: Hash, - /// The parent head-data hash. - pub parent_head_data_hash: Hash, + /// The parent head-data. + pub parent_head_data: ParentHeadData, +} + +/// The parent head-data hash with optional data itself. +#[derive(Debug)] +pub enum ParentHeadData { + /// Parent head-data hash. + OnlyHash(Hash), + /// Parent head-data along with its hash. + WithData { + /// This will be provided for collations with elastic scaling enabled. + head_data: HeadData, + /// Parent head-data hash. + hash: Hash, + }, } /// Indicates the relay-parents whose fragment tree a candidate diff --git a/prdoc/pr_3302.prdoc b/prdoc/pr_3302.prdoc new file mode 100644 index 00000000000..a2d93fc6073 --- /dev/null +++ b/prdoc/pr_3302.prdoc @@ -0,0 +1,15 @@ +title: Collator protocol changes for elastic scaling + +doc: + - audience: Node Dev + description: | + This PR introduces changes to the collator protocol to support elastic scaling. + Namely, a new variant added to the collation response to include parent head-data + along with the collation. Currently, the new variant is not being used. + - audience: Node Operator + description: | + Validators are required to upgrade to this version before collators in order to + support the elastic scaling of parachains. + +crates: + - name: polkadot-collator-protocol -- GitLab