From 504b91928db8eb3ae2c86da71f0f44ad6eb34feb Mon Sep 17 00:00:00 2001 From: ordian Date: Tue, 6 Feb 2024 18:09:31 +0100 Subject: [PATCH 1/9] add new collation advertisement --- cumulus/client/collator/src/lib.rs | 4 +- .../consensus/aura/src/collators/lookahead.rs | 4 + cumulus/polkadot-parachain/src/service.rs | 5 + polkadot/node/collation-generation/src/lib.rs | 18 +- .../node/collation-generation/src/tests.rs | 35 ++-- .../core/prospective-parachains/src/lib.rs | 5 +- .../core/prospective-parachains/src/tests.rs | 1 + polkadot/node/network/bridge/src/tx/mod.rs | 1 + .../src/collator_side/collation.rs | 31 +++- .../src/collator_side/mod.rs | 89 +++++++--- .../src/collator_side/tests/mod.rs | 13 +- .../tests/prospective_parachains.rs | 22 +-- .../src/validator_side/collation.rs | 33 +++- .../src/validator_side/mod.rs | 162 ++++++++++++++---- .../src/validator_side/tests/mod.rs | 2 +- polkadot/node/network/protocol/src/lib.rs | 12 +- .../protocol/src/request_response/mod.rs | 38 ++-- .../protocol/src/request_response/outgoing.rs | 7 +- .../protocol/src/request_response/v3.rs | 57 ++++++ polkadot/node/overseer/src/tests.rs | 1 + polkadot/node/primitives/src/lib.rs | 5 + polkadot/node/subsystem-types/src/messages.rs | 33 ++-- polkadot/node/test/service/src/lib.rs | 8 +- .../adder/collator/src/main.rs | 1 + .../undying/collator/src/main.rs | 1 + 25 files changed, 455 insertions(+), 133 deletions(-) create mode 100644 polkadot/node/network/protocol/src/request_response/v3.rs diff --git a/cumulus/client/collator/src/lib.rs b/cumulus/client/collator/src/lib.rs index 83249186f626..e11eb188c67e 100644 --- a/cumulus/client/collator/src/lib.rs +++ b/cumulus/client/collator/src/lib.rs @@ -220,6 +220,7 @@ pub mod relay_chain_driven { this_rx.await.ok().flatten() }) })), + with_elastic_scaling: false, }; overseer_handle @@ -243,8 +244,9 @@ pub async fn initialize_collator_subsystems( key: CollatorPair, para_id: ParaId, reinitialize: bool, + with_elastic_scaling: bool, ) { - let config = CollationGenerationConfig { key, para_id, collator: None }; + let config = CollationGenerationConfig { key, para_id, collator: None, with_elastic_scaling }; if reinitialize { overseer_handle diff --git a/cumulus/client/consensus/aura/src/collators/lookahead.rs b/cumulus/client/consensus/aura/src/collators/lookahead.rs index e24b7f6f1c93..6a01dc8f3934 100644 --- a/cumulus/client/consensus/aura/src/collators/lookahead.rs +++ b/cumulus/client/consensus/aura/src/collators/lookahead.rs @@ -107,6 +107,9 @@ pub struct Params { pub authoring_duration: Duration, /// Whether we should reinitialize the collator config (i.e. we are transitioning to aura). pub reinitialize: bool, + /// Whether elastic scaling is enabled for this collation. + /// If it is, the collator will send the parent-head data along with the collation. + pub with_elastic_scaling: bool, } /// Run async-backing-friendly Aura. @@ -152,6 +155,7 @@ where params.collator_key, params.para_id, params.reinitialize, + params.with_elastic_scaling, ) .await; diff --git a/cumulus/polkadot-parachain/src/service.rs b/cumulus/polkadot-parachain/src/service.rs index daeeadd5ecee..1bf0c0f25e80 100644 --- a/cumulus/polkadot-parachain/src/service.rs +++ b/cumulus/polkadot-parachain/src/service.rs @@ -977,6 +977,7 @@ pub async fn start_rococo_parachain_node( collator_service, authoring_duration: Duration::from_millis(1500), reinitialize: false, + with_elastic_scaling: false, }; let fut = aura::run::< @@ -1473,6 +1474,7 @@ where collator_service, authoring_duration: Duration::from_millis(1500), reinitialize: false, + with_elastic_scaling: false, }; let fut = @@ -1768,6 +1770,7 @@ where authoring_duration: Duration::from_millis(1500), reinitialize: true, /* we need to always re-initialize for asset-hub moving * to aura */ + with_elastic_scaling: false, }; aura::run::::Pair, _, _, _, _, _, _, _, _, _>(params) @@ -1870,6 +1873,7 @@ where collator_service, authoring_duration: Duration::from_millis(1500), reinitialize: false, + with_elastic_scaling: false, }; let fut = @@ -2180,6 +2184,7 @@ pub async fn start_contracts_rococo_node( // Very limited proposal time. authoring_duration: Duration::from_millis(1500), reinitialize: false, + with_elastic_scaling: false, }; let fut = aura::run::< diff --git a/polkadot/node/collation-generation/src/lib.rs b/polkadot/node/collation-generation/src/lib.rs index cfa75d7b4411..09f94cf32a6f 100644 --- a/polkadot/node/collation-generation/src/lib.rs +++ b/polkadot/node/collation-generation/src/lib.rs @@ -350,6 +350,8 @@ async fn handle_new_activations( }, }; + let with_elastic_scaling = task_config.with_elastic_scaling; + construct_and_distribute_receipt( PreparedCollation { collation, @@ -358,6 +360,7 @@ async fn handle_new_activations( validation_data, validation_code_hash, n_validators, + with_elastic_scaling, }, task_config.key.clone(), &mut task_sender, @@ -392,6 +395,7 @@ async fn handle_submit_collation( let validators = request_validators(relay_parent, ctx.sender()).await.await??; let n_validators = validators.len(); + let with_elastic_scaling = config.with_elastic_scaling; // We need to swap the parent-head data, but all other fields here will be correct. let mut validation_data = match request_persisted_validation_data( @@ -424,6 +428,7 @@ async fn handle_submit_collation( validation_data, validation_code_hash, n_validators, + with_elastic_scaling, }; construct_and_distribute_receipt( @@ -445,6 +450,7 @@ struct PreparedCollation { validation_data: PersistedValidationData, validation_code_hash: ValidationCodeHash, n_validators: usize, + with_elastic_scaling: bool, } /// Takes a prepared collation, along with its context, and produces a candidate receipt @@ -463,6 +469,7 @@ async fn construct_and_distribute_receipt( validation_data, validation_code_hash, n_validators, + with_elastic_scaling, } = collation; let persisted_validation_data_hash = validation_data.hash(); @@ -540,23 +547,28 @@ async fn construct_and_distribute_receipt( }, }; + let maybe_parent_head_data = + if with_elastic_scaling { Some(commitments.head_data.clone()) } else { None }; + gum::debug!( target: LOG_TARGET, candidate_hash = ?ccr.hash(), ?pov_hash, ?relay_parent, para_id = %para_id, + ?with_elastic_scaling, "candidate is generated", ); metrics.on_collation_generated(); sender - .send_message(CollatorProtocolMessage::DistributeCollation( - ccr, + .send_message(CollatorProtocolMessage::DistributeCollation { + candidate_receipt: ccr, parent_head_data_hash, pov, + maybe_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 9094f40cca84..6600c70a595e 100644 --- a/polkadot/node/collation-generation/src/tests.rs +++ b/polkadot/node/collation-generation/src/tests.rs @@ -117,6 +117,7 @@ fn test_config>(para_id: Id) -> CollationGenerationConfig { key: CollatorPair::generate().0, collator: Some(Box::new(|_: Hash, _vd: &PersistedValidationData| TestCollator.boxed())), para_id: para_id.into(), + with_elastic_scaling: false, } } @@ -125,6 +126,7 @@ fn test_config_no_collator>(para_id: Id) -> CollationGeneration key: CollatorPair::generate().0, collator: None, para_id: para_id.into(), + with_elastic_scaling: false, } } @@ -390,11 +392,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 +531,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 +621,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 6e6915b92728..f57730304724 100644 --- a/polkadot/node/core/prospective-parachains/src/lib.rs +++ b/polkadot/node/core/prospective-parachains/src/lib.rs @@ -771,8 +771,9 @@ 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 = request + .maybe_parent_head_data + .or_else(|| storage.head_data_by_hash(&request.parent_head_data_hash).map(|x| x.clone())); let mut relay_parent_info = None; let mut max_pov_size = None; diff --git a/polkadot/node/core/prospective-parachains/src/tests.rs b/polkadot/node/core/prospective-parachains/src/tests.rs index 732736b101de..b7ff9c2a9b3c 100644 --- a/polkadot/node/core/prospective-parachains/src/tests.rs +++ b/polkadot/node/core/prospective-parachains/src/tests.rs @@ -473,6 +473,7 @@ async fn get_pvd( para_id, candidate_relay_parent, parent_head_data_hash: parent_head_data.hash(), + maybe_parent_head_data: None, }; let (tx, rx) = oneshot::channel(); virtual_overseer diff --git a/polkadot/node/network/bridge/src/tx/mod.rs b/polkadot/node/network/bridge/src/tx/mod.rs index d5be6f01c337..86a9967f20c5 100644 --- a/polkadot/node/network/bridge/src/tx/mod.rs +++ b/polkadot/node/network/bridge/src/tx/mod.rs @@ -310,6 +310,7 @@ where Requests::DisputeSendingV1(_) => metrics.on_message("dispute_sending_v1"), Requests::StatementFetchingV1(_) => metrics.on_message("statement_fetching_v1"), Requests::AttestedCandidateV2(_) => metrics.on_message("attested_candidate_v2"), + Requests::CollationFetchingV3(_) => metrics.on_message("collation_fetching_v3"), } network_service 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 53f947142d10..198a747a23da 100644 --- a/polkadot/node/network/collator-protocol/src/collator_side/collation.rs +++ b/polkadot/node/network/collator-protocol/src/collator_side/collation.rs @@ -22,12 +22,13 @@ use futures::{future::BoxFuture, stream::FuturesUnordered}; use polkadot_node_network_protocol::{ request_response::{ - incoming::OutgoingResponse, v1 as protocol_v1, v2 as protocol_v2, IncomingRequest, + incoming::OutgoingResponse, v1 as protocol_v1, v2 as protocol_v2, v3 as protocol_v3, + IncomingRequest, }, 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 +64,8 @@ pub struct Collation { pub parent_head_data_hash: Hash, /// Proof to verify the state transition of the parachain. pub pov: PoV, + /// Optional parent head-data needed for elastic scaling. + pub maybe_parent_head_data: Option, /// Collation status. pub status: CollationStatus, } @@ -89,6 +92,7 @@ pub struct WaitingCollationFetches { pub enum VersionedCollationRequest { V1(IncomingRequest), V2(IncomingRequest), + V3(IncomingRequest), } impl From> for VersionedCollationRequest { @@ -103,12 +107,19 @@ impl From> for VersionedC } } +impl From> for VersionedCollationRequest { + fn from(req: IncomingRequest) -> Self { + Self::V3(req) + } +} + impl VersionedCollationRequest { /// Returns parachain id from the request payload. pub fn para_id(&self) -> ParaId { match self { VersionedCollationRequest::V1(req) => req.payload.para_id, VersionedCollationRequest::V2(req) => req.payload.para_id, + VersionedCollationRequest::V3(req) => req.payload.para_id, } } @@ -117,6 +128,7 @@ impl VersionedCollationRequest { match self { VersionedCollationRequest::V1(req) => req.payload.relay_parent, VersionedCollationRequest::V2(req) => req.payload.relay_parent, + VersionedCollationRequest::V3(req) => req.payload.relay_parent, } } @@ -125,6 +137,7 @@ impl VersionedCollationRequest { match self { VersionedCollationRequest::V1(req) => req.peer, VersionedCollationRequest::V2(req) => req.peer, + VersionedCollationRequest::V3(req) => req.peer, } } @@ -136,6 +149,20 @@ impl VersionedCollationRequest { match self { VersionedCollationRequest::V1(req) => req.send_outgoing_response(response), VersionedCollationRequest::V2(req) => req.send_outgoing_response(response), + VersionedCollationRequest::V3(_) => Err(()), + } + } + + /// Sends the response back to requester. + // TODO(ordian): this is ugly + pub fn send_outgoing_response_with_head_data( + self, + response: OutgoingResponse, + ) -> Result<(), ()> { + match self { + VersionedCollationRequest::V1(_) => Err(()), + VersionedCollationRequest::V2(_) => Err(()), + VersionedCollationRequest::V3(req) => req.send_outgoing_response(response), } } } 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 8fb0bb215444..2aa3c02acb47 100644 --- a/polkadot/node/network/collator-protocol/src/collator_side/mod.rs +++ b/polkadot/node/network/collator-protocol/src/collator_side/mod.rs @@ -31,7 +31,7 @@ use polkadot_node_network_protocol::{ peer_set::{CollationVersion, PeerSet}, request_response::{ incoming::{self, OutgoingResponse}, - v1 as request_v1, v2 as request_v2, IncomingRequestReceiver, + v1 as request_v1, v2 as request_v2, v3 as request_v3, IncomingRequestReceiver, }, v1 as protocol_v1, v2 as protocol_v2, OurView, PeerId, UnifiedReputationChange as Rep, Versioned, View, @@ -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( receipt: CandidateReceipt, parent_head_data_hash: Hash, pov: PoV, + maybe_parent_head_data: Option, result_sender: Option>, ) -> Result<()> { let candidate_relay_parent = receipt.descriptor.relay_parent; @@ -465,7 +466,13 @@ async fn distribute_collation( per_relay_parent.collations.insert( candidate_hash, - Collation { receipt, parent_head_data_hash, pov, status: CollationStatus::Created }, + Collation { + receipt, + parent_head_data_hash, + pov, + maybe_parent_head_data, + status: CollationStatus::Created, + }, ); // If prospective parachains are disabled, a leaf should be known to peer. @@ -715,10 +722,18 @@ async fn advertise_collation( let collation_message = match protocol_version { CollationVersion::V2 => { - let wire_message = protocol_v2::CollatorProtocolMessage::AdvertiseCollation { - relay_parent, - candidate_hash: *candidate_hash, - parent_head_data_hash: collation.parent_head_data_hash, + let wire_message = if collation.maybe_parent_head_data.is_none() { + protocol_v2::CollatorProtocolMessage::AdvertiseCollationV2 { + relay_parent, + candidate_hash: *candidate_hash, + parent_head_data_hash: collation.parent_head_data_hash, + } + } else { + protocol_v2::CollatorProtocolMessage::AdvertiseCollationV3 { + relay_parent, + candidate_hash: *candidate_hash, + parent_head_data_hash: collation.parent_head_data_hash, + } }; Versioned::V2(protocol_v2::CollationProtocol::CollatorProtocol(wire_message)) }, @@ -763,20 +778,26 @@ async fn process_msg( CollateOn(id) => { state.collating_on = Some(id); }, - DistributeCollation(receipt, parent_head_data_hash, pov, result_sender) => { + DistributeCollation { + candidate_receipt, + parent_head_data_hash, + pov, + maybe_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 +809,10 @@ async fn process_msg( runtime, state, id, - receipt, + candidate_receipt, parent_head_data_hash, pov, + maybe_parent_head_data, result_sender, ) .await?; @@ -798,7 +820,7 @@ async fn process_msg( 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 +857,7 @@ async fn send_collation( request: VersionedCollationRequest, receipt: CandidateReceipt, pov: PoV, + maybe_parent_head_data: Option, ) { let (tx, rx) = oneshot::channel(); @@ -842,15 +865,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), + let send_result = if let Some(parent_head_data) = maybe_parent_head_data { + let result = + Ok(request_v3::CollationFetchingResponse::Collation { receipt, pov, parent_head_data }); + let response = + OutgoingResponse { result, reputation_changes: Vec::new(), sent_feedback: Some(tx) }; + request.send_outgoing_response_with_head_data(response) + } else { + let result = Ok(request_v1::CollationFetchingResponse::Collation(receipt, pov)); + let response = + OutgoingResponse { result, reputation_changes: Vec::new(), sent_feedback: Some(tx) }; + request.send_outgoing_response(response) }; - if let Err(_) = request.send_outgoing_response(response) { + if let Err(_) = send_result { gum::warn!(target: LOG_TARGET, "Sending collation response failed"); } @@ -894,8 +924,10 @@ async fn handle_incoming_peer_message( .await; }, Versioned::V1(V1::AdvertiseCollation(_)) | - Versioned::V2(V2::AdvertiseCollation { .. }) | - Versioned::V3(V2::AdvertiseCollation { .. }) => { + Versioned::V2(V2::AdvertiseCollationV2 { .. }) | + Versioned::V2(V2::AdvertiseCollationV3 { .. }) | + Versioned::V3(V2::AdvertiseCollationV2 { .. }) | + Versioned::V3(V2::AdvertiseCollationV3 { .. }) => { gum::trace!( target: LOG_TARGET, ?origin, @@ -1015,6 +1047,8 @@ async fn handle_incoming_request( per_relay_parent.collations.values_mut().next(), VersionedCollationRequest::V2(req) => per_relay_parent.collations.get_mut(&req.payload.candidate_hash), + VersionedCollationRequest::V3(req) => + per_relay_parent.collations.get_mut(&req.payload.candidate_hash), _ => { gum::warn!( target: LOG_TARGET, @@ -1027,9 +1061,13 @@ async fn handle_incoming_request( return Ok(()) }, }; - let (receipt, pov) = if let Some(collation) = collation { + let (receipt, pov, maybe_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.maybe_parent_head_data.clone(), + ) } else { gum::warn!( target: LOG_TARGET, @@ -1068,7 +1106,7 @@ async fn handle_incoming_request( 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, maybe_parent_head_data).await; } }, Some(our_para_id) => { @@ -1453,8 +1491,9 @@ async fn run_inner( if let Some(collation) = next_collation { let receipt = collation.receipt.clone(); let pov = collation.pov.clone(); + let maybe_parent_head_data = collation.maybe_parent_head_data.clone(); - send_collation(&mut state, next, receipt, pov).await; + send_collation(&mut state, next, receipt, pov, maybe_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 1b1194c72706..32835df50064 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(), + maybe_parent_head_data: None, + result_sender: None, + }, ) .await; @@ -591,7 +592,7 @@ async fn expect_advertise_collation_msg( ) => { assert_matches!( wire_message, - protocol_v2::CollatorProtocolMessage::AdvertiseCollation { + protocol_v2::CollatorProtocolMessage::AdvertiseCollationV2 { relay_parent, candidate_hash, .. 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 fd9d7a746ebe..c4ae6eea1abb 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(), + maybe_parent_head_data: None, + 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(), + maybe_parent_head_data: None, + result_sender: None, + }, ) .await; 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 d6f34fc81b82..4d9510f47f2a 100644 --- a/polkadot/node/network/collator-protocol/src/validator_side/collation.rs +++ b/polkadot/node/network/collator-protocol/src/validator_side/collation.rs @@ -32,7 +32,9 @@ use std::{collections::VecDeque, future::Future, pin::Pin, task::Poll}; use futures::{future::BoxFuture, FutureExt}; use polkadot_node_network_protocol::{ peer_set::CollationVersion, - request_response::{outgoing::RequestError, v1 as request_v1, OutgoingResult}, + request_response::{ + outgoing::RequestError, v1 as request_v1, v3 as request_v3, OutgoingResult, + }, PeerId, }; use polkadot_node_primitives::PoV; @@ -41,7 +43,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; @@ -101,6 +104,9 @@ pub struct PendingCollation { pub prospective_candidate: Option, /// Hash of the candidate's commitments. pub commitments_hash: Option, + /// Whether the collation was advertised with elastic scaling enabled. + /// If it was, the validator will request the parent-head data along with the collation. + pub with_elastic_scaling: bool, } impl PendingCollation { @@ -109,6 +115,7 @@ impl PendingCollation { para_id: ParaId, peer_id: &PeerId, prospective_candidate: Option, + with_elastic_scaling: bool, ) -> Self { Self { relay_parent, @@ -116,11 +123,12 @@ impl PendingCollation { peer_id: *peer_id, prospective_candidate, commitments_hash: None, + with_elastic_scaling, } } } -/// 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)] @@ -133,6 +141,8 @@ pub struct BlockedAdvertisement { pub candidate_relay_parent: Hash, /// Hash of the candidate. pub candidate_hash: CandidateHash, + /// Whether the collation was advertised with elastic scaling enabled. + pub with_elastic_scaling: bool, } /// Performs a sanity check between advertised and fetched collations. @@ -176,6 +186,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, } /// The status of the collations in [`CollationsPerRelayParent`]. @@ -313,7 +326,7 @@ pub(super) struct CollationFetchRequest { /// The network protocol version the collator is using. pub collator_protocol_version: CollationVersion, /// Responses from collator. - pub from_collator: BoxFuture<'static, OutgoingResult>, + pub from_collator: BoxFuture<'static, OutgoingResult>, /// Handle used for checking if this request was cancelled. pub cancellation_token: CancellationToken, /// A jaeger span corresponding to the lifetime of the request. @@ -322,11 +335,13 @@ pub(super) struct CollationFetchRequest { pub _lifetime_timer: Option, } +pub enum VersionedResponse { + V1(request_v1::CollationFetchingResponse), + V3(request_v3::CollationFetchingResponse), +} + impl Future for CollationFetchRequest { - type Output = ( - CollationEvent, - std::result::Result, - ); + type Output = (CollationEvent, std::result::Result); fn poll(mut self: Pin<&mut Self>, cx: &mut std::task::Context<'_>) -> Poll { // First check if this fetch request was cancelled. @@ -359,7 +374,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 48ad3c711a6d..e87ec561e807 100644 --- a/polkadot/node/network/collator-protocol/src/validator_side/mod.rs +++ b/polkadot/node/network/collator-protocol/src/validator_side/mod.rs @@ -34,7 +34,7 @@ use polkadot_node_network_protocol::{ peer_set::{CollationVersion, PeerSet}, request_response::{ outgoing::{Recipient, RequestError}, - v1 as request_v1, v2 as request_v2, OutgoingRequest, Requests, + v1 as request_v1, v2 as request_v2, v3 as request_v3, OutgoingRequest, Requests, }, v1 as protocol_v1, v2 as protocol_v2, OurView, PeerId, UnifiedReputationChange as Rep, Versioned, View, @@ -55,11 +55,14 @@ 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, }; -use crate::error::{Error, FetchError, Result, SecondingError}; +use crate::{ + error::{Error, FetchError, Result, SecondingError}, + validator_side::collation::VersionedResponse, +}; use super::{modify_reputation, tick_stream, LOG_TARGET}; @@ -690,8 +693,14 @@ async fn request_collation( return Err(FetchError::AlreadyRequested) } - let PendingCollation { relay_parent, para_id, peer_id, prospective_candidate, .. } = - pending_collation; + let PendingCollation { + relay_parent, + para_id, + peer_id, + prospective_candidate, + with_elastic_scaling, + .. + } = pending_collation; let per_relay_parent = state .per_relay_parent .get_mut(&relay_parent) @@ -705,16 +714,24 @@ async fn request_collation( request_v1::CollationFetchingRequest { relay_parent, para_id }, ); let requests = Requests::CollationFetchingV1(req); - (requests, response_recv.boxed()) - }, - (CollationVersion::V2, Some(ProspectiveCandidate { candidate_hash, .. })) => { - let (req, response_recv) = OutgoingRequest::new( - Recipient::Peer(peer_id), - request_v2::CollationFetchingRequest { relay_parent, para_id, candidate_hash }, - ); - let requests = Requests::CollationFetchingV2(req); - (requests, response_recv.boxed()) + (requests, response_recv.map(|r| r.map(VersionedResponse::V1)).boxed()) }, + (CollationVersion::V2, Some(ProspectiveCandidate { candidate_hash, .. })) => + if with_elastic_scaling { + let (req, response_recv) = OutgoingRequest::new( + Recipient::Peer(peer_id), + request_v3::CollationFetchingRequest { relay_parent, para_id, candidate_hash }, + ); + let requests = Requests::CollationFetchingV3(req); + (requests, response_recv.map(|r| r.map(VersionedResponse::V3)).boxed()) + } else { + let (req, response_recv) = OutgoingRequest::new( + Recipient::Peer(peer_id), + request_v2::CollationFetchingRequest { relay_parent, para_id, candidate_hash }, + ); + let requests = Requests::CollationFetchingV2(req); + (requests, response_recv.map(|r| r.map(VersionedResponse::V1)).boxed()) + }, _ => return Err(FetchError::ProtocolMismatch), }; @@ -723,7 +740,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 @@ -875,7 +892,7 @@ async fn process_incoming_peer_message( }, Versioned::V1(V1::AdvertiseCollation(relay_parent)) => if let Err(err) = - handle_advertisement(ctx.sender(), state, relay_parent, origin, None).await + handle_advertisement(ctx.sender(), state, relay_parent, origin, None, false).await { gum::debug!( target: LOG_TARGET, @@ -889,22 +906,57 @@ async fn process_incoming_peer_message( modify_reputation(&mut state.reputation, ctx.sender(), origin, rep).await; } }, - Versioned::V2(V2::AdvertiseCollation { + Versioned::V2(V2::AdvertiseCollationV3 { relay_parent, candidate_hash, parent_head_data_hash, }) | - Versioned::V3(V2::AdvertiseCollation { + Versioned::V3(V2::AdvertiseCollationV3 { relay_parent, candidate_hash, parent_head_data_hash, - }) => + }) => { if let Err(err) = handle_advertisement( ctx.sender(), state, relay_parent, origin, Some((candidate_hash, parent_head_data_hash)), + true, + ) + .await + { + gum::debug!( + target: LOG_TARGET, + peer_id = ?origin, + ?relay_parent, + ?candidate_hash, + error = ?err, + "Rejected v3 advertisement", + ); + + if let Some(rep) = err.reputation_changes() { + modify_reputation(&mut state.reputation, ctx.sender(), origin, rep).await; + } + } + }, + Versioned::V3(V2::AdvertiseCollationV2 { + relay_parent, + candidate_hash, + parent_head_data_hash, + }) | + Versioned::V2(V2::AdvertiseCollationV2 { + relay_parent, + candidate_hash, + parent_head_data_hash, + }) => { + if let Err(err) = handle_advertisement( + ctx.sender(), + state, + relay_parent, + origin, + Some((candidate_hash, parent_head_data_hash)), + false, ) .await { @@ -920,7 +972,8 @@ async fn process_incoming_peer_message( 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(..)) => { @@ -1031,6 +1084,7 @@ where blocked.peer_id, blocked.collator_id, Some((blocked.candidate_hash, para_head)), + blocked.with_elastic_scaling, ) .await; if let Err(fetch_error) = result { @@ -1061,6 +1115,7 @@ async fn handle_advertisement( relay_parent: Hash, peer_id: PeerId, prospective_candidate: Option<(CandidateHash, Hash)>, + with_elastic_scaling: bool, ) -> std::result::Result<(), AdvertisementError> where Sender: CollatorProtocolSenderTrait, @@ -1140,6 +1195,7 @@ where collator_id: collator_id.clone(), candidate_relay_parent: relay_parent, candidate_hash, + with_elastic_scaling, }); return Ok(()) @@ -1154,6 +1210,7 @@ where peer_id, collator_id, prospective_candidate, + with_elastic_scaling, ) .await; @@ -1181,6 +1238,7 @@ async fn enqueue_collation( peer_id: PeerId, collator_id: CollatorId, prospective_candidate: Option<(CandidateHash, Hash)>, + with_elastic_scaling: bool, ) -> std::result::Result<(), FetchError> where Sender: CollatorProtocolSenderTrait, @@ -1226,8 +1284,13 @@ where return Ok(()) } - let pending_collation = - PendingCollation::new(relay_parent, para_id, &peer_id, prospective_candidate); + let pending_collation = PendingCollation::new( + relay_parent, + para_id, + &peer_id, + prospective_candidate, + with_elastic_scaling, + ); match collations.status { CollationStatus::Fetching | CollationStatus::WaitingOnValidation => { @@ -1477,7 +1540,7 @@ async fn process_msg( "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 +1839,19 @@ async fn request_prospective_validation_data( candidate_relay_parent: Hash, parent_head_data_hash: Hash, para_id: ParaId, + maybe_parent_head_data: Option, ) -> std::result::Result, SecondingError> where Sender: CollatorProtocolSenderTrait, { let (tx, rx) = oneshot::channel(); - let request = - ProspectiveValidationDataRequest { para_id, candidate_relay_parent, parent_head_data_hash }; + let request = ProspectiveValidationDataRequest { + para_id, + candidate_relay_parent, + parent_head_data_hash, + maybe_parent_head_data, + }; sender .send_message(ProspectiveParachainsMessage::GetProspectiveValidationData(request, tx)) @@ -1797,7 +1865,7 @@ where async fn kick_off_seconding( 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; @@ -1832,6 +1900,7 @@ async fn kick_off_seconding( relay_parent, parent_head_data_hash, pending_collation.para_id, + maybe_parent_head_data, ) .await?, // Support V2 collators without async backing enabled. @@ -1978,9 +2047,12 @@ async fn handle_collation_fetch_response( ); Err(None) }, - Ok(request_v1::CollationFetchingResponse::Collation(receipt, _)) - if receipt.descriptor().para_id != pending_collation.para_id => - { + Ok( + VersionedResponse::V1(request_v1::CollationFetchingResponse::Collation(receipt, _)) | + VersionedResponse::V3(request_v3::CollationFetchingResponse::Collation { + receipt, .. + }), + ) if receipt.descriptor().para_id != pending_collation.para_id => { gum::debug!( target: LOG_TARGET, expected_para_id = ?pending_collation.para_id, @@ -1991,7 +2063,10 @@ async fn handle_collation_fetch_response( Err(Some((pending_collation.peer_id, COST_WRONG_PARA))) }, - Ok(request_v1::CollationFetchingResponse::Collation(candidate_receipt, pov)) => { + Ok(VersionedResponse::V1(request_v1::CollationFetchingResponse::Collation( + candidate_receipt, + pov, + ))) => { gum::debug!( target: LOG_TARGET, para_id = %pending_collation.para_id, @@ -2010,6 +2085,33 @@ async fn handle_collation_fetch_response( }, candidate_receipt, pov, + maybe_parent_head_data: None, + }) + }, + Ok(VersionedResponse::V3(request_v3::CollationFetchingResponse::Collation { + 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/mod.rs b/polkadot/node/network/collator-protocol/src/validator_side/tests/mod.rs index 1ba6389212cc..4df4eb1c9f45 100644 --- a/polkadot/node/network/collator-protocol/src/validator_side/tests/mod.rs +++ b/polkadot/node/network/collator-protocol/src/validator_side/tests/mod.rs @@ -421,7 +421,7 @@ async fn advertise_collation( ) { let wire_message = match candidate { Some((candidate_hash, parent_head_data_hash)) => - Versioned::V2(protocol_v2::CollatorProtocolMessage::AdvertiseCollation { + Versioned::V2(protocol_v2::CollatorProtocolMessage::AdvertiseCollationV2 { relay_parent, candidate_hash, parent_head_data_hash, diff --git a/polkadot/node/network/protocol/src/lib.rs b/polkadot/node/network/protocol/src/lib.rs index 7a0ff9f4fa9a..8fb208ac1bfe 100644 --- a/polkadot/node/network/protocol/src/lib.rs +++ b/polkadot/node/network/protocol/src/lib.rs @@ -820,7 +820,7 @@ pub mod v2 { /// Advertise a collation to a validator. Can only be sent once the peer has /// declared that they are a collator with given ID. #[codec(index = 1)] - AdvertiseCollation { + AdvertiseCollationV2 { /// Hash of the relay parent advertised collation is based on. relay_parent: Hash, /// Candidate hash. @@ -831,6 +831,16 @@ pub mod v2 { /// A collation sent to a validator was seconded. #[codec(index = 4)] CollationSeconded(Hash, UncheckedSignedFullStatement), + /// Same as `AdvertiseCollationV2`, but triggers a different req/response version. + #[codec(index = 5)] + AdvertiseCollationV3 { + /// Hash of the relay parent advertised collation is based on. + relay_parent: Hash, + /// Candidate hash. + candidate_hash: CandidateHash, + /// Parachain head data hash before candidate execution. + parent_head_data_hash: Hash, + }, } /// All network messages on the validation peer-set. diff --git a/polkadot/node/network/protocol/src/request_response/mod.rs b/polkadot/node/network/protocol/src/request_response/mod.rs index a67d83aff0c9..2aee3b31f7e0 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 @@ -74,6 +74,9 @@ pub mod v1; /// Actual versioned requests and responses that are sent over the wire. pub mod v2; +/// Actual versioned requests and responses that are sent over the wire. +pub mod v3; + /// A protocol per subsystem seems to make the most sense, this way we don't need any dispatching /// within protocols. #[derive(Copy, Clone, Debug, Hash, PartialEq, Eq, EnumIter)] @@ -96,6 +99,9 @@ pub enum Protocol { /// Protocol for requesting candidates with attestations in statement distribution /// when async backing is enabled. AttestedCandidateV2, + + /// Protocol for fetching collations from collators for elastic scaling. + CollationFetchingV3, } /// Minimum bandwidth we expect for validators - 500Mbit/s is the recommendation, so approximately @@ -216,16 +222,17 @@ impl Protocol { request_timeout: CHUNK_REQUEST_TIMEOUT, inbound_queue: tx, }, - Protocol::CollationFetchingV1 | Protocol::CollationFetchingV2 => - RequestResponseConfig { - name, - fallback_names: legacy_names, - max_request_size: 1_000, - max_response_size: POV_RESPONSE_SIZE, - // Taken from initial implementation in collator protocol: - request_timeout: POV_REQUEST_TIMEOUT_CONNECTED, - inbound_queue: tx, - }, + Protocol::CollationFetchingV1 | + Protocol::CollationFetchingV2 | + Protocol::CollationFetchingV3 => RequestResponseConfig { + name, + fallback_names: legacy_names, + max_request_size: 1_000, + max_response_size: POV_RESPONSE_SIZE, + // Taken from initial implementation in collator protocol: + request_timeout: POV_REQUEST_TIMEOUT_CONNECTED, + inbound_queue: tx, + }, Protocol::PoVFetchingV1 => RequestResponseConfig { name, fallback_names: legacy_names, @@ -292,7 +299,9 @@ impl Protocol { // as well. Protocol::ChunkFetchingV1 => 100, // 10 seems reasonable, considering group sizes of max 10 validators. - Protocol::CollationFetchingV1 | Protocol::CollationFetchingV2 => 10, + Protocol::CollationFetchingV1 | + Protocol::CollationFetchingV2 | + Protocol::CollationFetchingV3 => 10, // 10 seems reasonable, considering group sizes of max 10 validators. Protocol::PoVFetchingV1 => 10, // Validators are constantly self-selecting to request available data which may lead @@ -356,10 +365,11 @@ impl Protocol { Protocol::AvailableDataFetchingV1 => Some("/polkadot/req_available_data/1"), Protocol::StatementFetchingV1 => Some("/polkadot/req_statement/1"), Protocol::DisputeSendingV1 => Some("/polkadot/send_dispute/1"), + Protocol::CollationFetchingV2 => Some("/polkadot/req_collation/2"), // Introduced after legacy names became legacy. Protocol::AttestedCandidateV2 => None, - Protocol::CollationFetchingV2 => None, + Protocol::CollationFetchingV3 => None, } } } @@ -419,6 +429,8 @@ impl ReqProtocolNames { Protocol::CollationFetchingV2 => "/req_collation/2", Protocol::AttestedCandidateV2 => "/req_attested_candidate/2", + + Protocol::CollationFetchingV3 => "/req_collation/3", }; format!("{}{}", prefix, short_name).into() diff --git a/polkadot/node/network/protocol/src/request_response/outgoing.rs b/polkadot/node/network/protocol/src/request_response/outgoing.rs index 88439ad40367..701e3611b35d 100644 --- a/polkadot/node/network/protocol/src/request_response/outgoing.rs +++ b/polkadot/node/network/protocol/src/request_response/outgoing.rs @@ -24,7 +24,7 @@ use sc_network::PeerId; use polkadot_primitives::AuthorityDiscoveryId; -use super::{v1, v2, IsRequest, Protocol}; +use super::{v1, v2, v3, IsRequest, Protocol}; /// All requests that can be sent to the network bridge via `NetworkBridgeTxMessage::SendRequest`. #[derive(Debug)] @@ -47,6 +47,10 @@ pub enum Requests { /// Fetch a collation from a collator which previously announced it. /// Compared to V1 it requires specifying which candidate is requested by its hash. CollationFetchingV2(OutgoingRequest), + + /// Fetch a collation from a collator which previously announced it. + /// Compared to V2, the response includes the parent head data. + CollationFetchingV3(OutgoingRequest), } impl Requests { @@ -67,6 +71,7 @@ impl Requests { Self::StatementFetchingV1(r) => r.encode_request(), Self::DisputeSendingV1(r) => r.encode_request(), Self::AttestedCandidateV2(r) => r.encode_request(), + Self::CollationFetchingV3(r) => r.encode_request(), } } } diff --git a/polkadot/node/network/protocol/src/request_response/v3.rs b/polkadot/node/network/protocol/src/request_response/v3.rs new file mode 100644 index 000000000000..3854a3758bfa --- /dev/null +++ b/polkadot/node/network/protocol/src/request_response/v3.rs @@ -0,0 +1,57 @@ +// Copyright (C) Parity Technologies (UK) Ltd. +// This file is part of Polkadot. + +// Polkadot is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Polkadot is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Polkadot. If not, see . + +//! Requests and responses as sent over the wire for the individual protocols. + +use parity_scale_codec::{Decode, Encode}; + +use polkadot_node_primitives::PoV; +use polkadot_primitives::{CandidateHash, CandidateReceipt, Hash, HeadData, Id as ParaId}; + +use super::{IsRequest, Protocol}; + +/// Request the advertised collation at that relay-parent. +// Same as v2. +#[derive(Debug, Clone, Encode, Decode)] +pub struct CollationFetchingRequest { + /// Relay parent collation is built on top of. + pub relay_parent: Hash, + /// The `ParaId` of the collation. + pub para_id: ParaId, + /// Candidate hash. + pub candidate_hash: CandidateHash, +} + +/// Responses as sent by collators. +#[derive(Debug, Clone, Encode, Decode)] +pub enum CollationFetchingResponse { + /// Deliver requested collation along with parent head data. + #[codec(index = 1)] + Collation { + /// 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 { + type Response = CollationFetchingResponse; + const PROTOCOL: Protocol = Protocol::CollationFetchingV3; +} diff --git a/polkadot/node/overseer/src/tests.rs b/polkadot/node/overseer/src/tests.rs index 0494274367d9..f9c29c2debe7 100644 --- a/polkadot/node/overseer/src/tests.rs +++ b/polkadot/node/overseer/src/tests.rs @@ -824,6 +824,7 @@ fn test_collator_generation_msg() -> CollationGenerationMessage { key: CollatorPair::generate().0, collator: Some(Box::new(|_, _| TestCollator.boxed())), para_id: Default::default(), + with_elastic_scaling: false, }) } struct TestCollator; diff --git a/polkadot/node/primitives/src/lib.rs b/polkadot/node/primitives/src/lib.rs index 6e3eefbcbe8c..24d88a03da24 100644 --- a/polkadot/node/primitives/src/lib.rs +++ b/polkadot/node/primitives/src/lib.rs @@ -498,6 +498,11 @@ pub struct CollationGenerationConfig { pub collator: Option, /// The parachain that this collator collates for pub para_id: ParaId, + /// Whether elastic scaling is enabled for this collation. + /// If it is, the collator will send the parent-head data along with the collation. + /// If your parachain is not assigned to multiple cores at the same time, + /// you should set this to `false`. + pub with_elastic_scaling: bool, } #[cfg(not(target_os = "unknown"))] diff --git a/polkadot/node/subsystem-types/src/messages.rs b/polkadot/node/subsystem-types/src/messages.rs index 549e43a671d6..29cb1b64d487 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::{ @@ -211,12 +212,19 @@ pub enum CollatorProtocolMessage { /// /// 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>, - ), + DistributeCollation { + /// TODO(ordian) + candidate_receipt: CandidateReceipt, + /// TODO(ordian) + parent_head_data_hash: Hash, + /// TODO(ordian) + pov: PoV, + /// This field is only used for elastic scaling. + // TODO(ordian): maybe using an enum + maybe_parent_head_data: Option, + /// TODO(ordian) + result_sender: Option>, + }, /// Report a collator as having provided an invalid collation. This should lead to disconnect /// and blacklist of the collator. ReportCollator(CollatorId), @@ -1106,6 +1114,9 @@ pub struct ProspectiveValidationDataRequest { pub candidate_relay_parent: Hash, /// The parent head-data hash. pub parent_head_data_hash: Hash, + /// Optionally, the head-data of the parent. + /// This will be provided for collations with elastic scaling enabled. + pub maybe_parent_head_data: Option, } /// Indicates the relay-parents whose fragment tree a candidate diff --git a/polkadot/node/test/service/src/lib.rs b/polkadot/node/test/service/src/lib.rs index e4eec32baf2a..d862decf2e7e 100644 --- a/polkadot/node/test/service/src/lib.rs +++ b/polkadot/node/test/service/src/lib.rs @@ -353,8 +353,12 @@ impl PolkadotTestNode { para_id: ParaId, collator: CollatorFn, ) { - let config = - CollationGenerationConfig { key: collator_key, collator: Some(collator), para_id }; + let config = CollationGenerationConfig { + key: collator_key, + collator: Some(collator), + para_id, + with_elastic_scaling: false, + }; self.overseer_handle .send_msg(CollationGenerationMessage::Initialize(config), "Collator") diff --git a/polkadot/parachain/test-parachains/adder/collator/src/main.rs b/polkadot/parachain/test-parachains/adder/collator/src/main.rs index fec90fc41cdb..70171afae2f9 100644 --- a/polkadot/parachain/test-parachains/adder/collator/src/main.rs +++ b/polkadot/parachain/test-parachains/adder/collator/src/main.rs @@ -119,6 +119,7 @@ fn main() -> Result<()> { collator.create_collation_function(full_node.task_manager.spawn_handle()), ), para_id, + with_elastic_scaling: false, }; overseer_handle .send_msg(CollationGenerationMessage::Initialize(config), "Collator") diff --git a/polkadot/parachain/test-parachains/undying/collator/src/main.rs b/polkadot/parachain/test-parachains/undying/collator/src/main.rs index 45f21e7b8596..84e1f9b19fd7 100644 --- a/polkadot/parachain/test-parachains/undying/collator/src/main.rs +++ b/polkadot/parachain/test-parachains/undying/collator/src/main.rs @@ -121,6 +121,7 @@ fn main() -> Result<()> { collator.create_collation_function(full_node.task_manager.spawn_handle()), ), para_id, + with_elastic_scaling: false, }; overseer_handle .send_msg(CollationGenerationMessage::Initialize(config), "Collator") From efceb5ea2819c5c2d9e38141f7f166abe798398e Mon Sep 17 00:00:00 2001 From: ordian Date: Wed, 14 Feb 2024 05:45:48 +0100 Subject: [PATCH 2/9] remove v3 in favor of enum variant --- polkadot/node/network/bridge/src/tx/mod.rs | 1 - .../src/collator_side/collation.rs | 27 +---- .../src/collator_side/mod.rs | 47 +++----- .../src/collator_side/tests/mod.rs | 53 ++++---- .../tests/prospective_parachains.rs | 10 +- .../src/validator_side/collation.rs | 23 +--- .../src/validator_side/mod.rs | 113 ++++-------------- .../src/validator_side/tests/mod.rs | 2 +- polkadot/node/network/protocol/src/lib.rs | 12 +- .../protocol/src/request_response/mod.rs | 36 ++---- .../protocol/src/request_response/outgoing.rs | 7 +- .../protocol/src/request_response/v1.rs | 15 ++- .../protocol/src/request_response/v3.rs | 57 --------- 13 files changed, 109 insertions(+), 294 deletions(-) delete mode 100644 polkadot/node/network/protocol/src/request_response/v3.rs diff --git a/polkadot/node/network/bridge/src/tx/mod.rs b/polkadot/node/network/bridge/src/tx/mod.rs index 86a9967f20c5..d5be6f01c337 100644 --- a/polkadot/node/network/bridge/src/tx/mod.rs +++ b/polkadot/node/network/bridge/src/tx/mod.rs @@ -310,7 +310,6 @@ where Requests::DisputeSendingV1(_) => metrics.on_message("dispute_sending_v1"), Requests::StatementFetchingV1(_) => metrics.on_message("statement_fetching_v1"), Requests::AttestedCandidateV2(_) => metrics.on_message("attested_candidate_v2"), - Requests::CollationFetchingV3(_) => metrics.on_message("collation_fetching_v3"), } network_service 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 198a747a23da..88f463951f79 100644 --- a/polkadot/node/network/collator-protocol/src/collator_side/collation.rs +++ b/polkadot/node/network/collator-protocol/src/collator_side/collation.rs @@ -22,8 +22,7 @@ use futures::{future::BoxFuture, stream::FuturesUnordered}; use polkadot_node_network_protocol::{ request_response::{ - incoming::OutgoingResponse, v1 as protocol_v1, v2 as protocol_v2, v3 as protocol_v3, - IncomingRequest, + incoming::OutgoingResponse, v1 as protocol_v1, v2 as protocol_v2, IncomingRequest, }, PeerId, }; @@ -92,7 +91,6 @@ pub struct WaitingCollationFetches { pub enum VersionedCollationRequest { V1(IncomingRequest), V2(IncomingRequest), - V3(IncomingRequest), } impl From> for VersionedCollationRequest { @@ -107,19 +105,12 @@ impl From> for VersionedC } } -impl From> for VersionedCollationRequest { - fn from(req: IncomingRequest) -> Self { - Self::V3(req) - } -} - impl VersionedCollationRequest { /// Returns parachain id from the request payload. pub fn para_id(&self) -> ParaId { match self { VersionedCollationRequest::V1(req) => req.payload.para_id, VersionedCollationRequest::V2(req) => req.payload.para_id, - VersionedCollationRequest::V3(req) => req.payload.para_id, } } @@ -128,7 +119,6 @@ impl VersionedCollationRequest { match self { VersionedCollationRequest::V1(req) => req.payload.relay_parent, VersionedCollationRequest::V2(req) => req.payload.relay_parent, - VersionedCollationRequest::V3(req) => req.payload.relay_parent, } } @@ -137,7 +127,6 @@ impl VersionedCollationRequest { match self { VersionedCollationRequest::V1(req) => req.peer, VersionedCollationRequest::V2(req) => req.peer, - VersionedCollationRequest::V3(req) => req.peer, } } @@ -149,20 +138,6 @@ impl VersionedCollationRequest { match self { VersionedCollationRequest::V1(req) => req.send_outgoing_response(response), VersionedCollationRequest::V2(req) => req.send_outgoing_response(response), - VersionedCollationRequest::V3(_) => Err(()), - } - } - - /// Sends the response back to requester. - // TODO(ordian): this is ugly - pub fn send_outgoing_response_with_head_data( - self, - response: OutgoingResponse, - ) -> Result<(), ()> { - match self { - VersionedCollationRequest::V1(_) => Err(()), - VersionedCollationRequest::V2(_) => Err(()), - VersionedCollationRequest::V3(req) => req.send_outgoing_response(response), } } } 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 2aa3c02acb47..e63987b2080b 100644 --- a/polkadot/node/network/collator-protocol/src/collator_side/mod.rs +++ b/polkadot/node/network/collator-protocol/src/collator_side/mod.rs @@ -31,7 +31,7 @@ use polkadot_node_network_protocol::{ peer_set::{CollationVersion, PeerSet}, request_response::{ incoming::{self, OutgoingResponse}, - v1 as request_v1, v2 as request_v2, v3 as request_v3, IncomingRequestReceiver, + v1 as request_v1, v2 as request_v2, IncomingRequestReceiver, }, v1 as protocol_v1, v2 as protocol_v2, OurView, PeerId, UnifiedReputationChange as Rep, Versioned, View, @@ -722,18 +722,10 @@ async fn advertise_collation( let collation_message = match protocol_version { CollationVersion::V2 => { - let wire_message = if collation.maybe_parent_head_data.is_none() { - protocol_v2::CollatorProtocolMessage::AdvertiseCollationV2 { - relay_parent, - candidate_hash: *candidate_hash, - parent_head_data_hash: collation.parent_head_data_hash, - } - } else { - protocol_v2::CollatorProtocolMessage::AdvertiseCollationV3 { - relay_parent, - candidate_hash: *candidate_hash, - parent_head_data_hash: collation.parent_head_data_hash, - } + let wire_message = protocol_v2::CollatorProtocolMessage::AdvertiseCollation { + relay_parent, + candidate_hash: *candidate_hash, + parent_head_data_hash: collation.parent_head_data_hash, }; Versioned::V2(protocol_v2::CollationProtocol::CollatorProtocol(wire_message)) }, @@ -867,20 +859,19 @@ async fn send_collation( // The response payload is the same for v1 and v2 versions of protocol // and doesn't have v2 alias for simplicity. - let send_result = if let Some(parent_head_data) = maybe_parent_head_data { - let result = - Ok(request_v3::CollationFetchingResponse::Collation { receipt, pov, parent_head_data }); - let response = - OutgoingResponse { result, reputation_changes: Vec::new(), sent_feedback: Some(tx) }; - request.send_outgoing_response_with_head_data(response) + let result = if let Some(parent_head_data) = maybe_parent_head_data { + Ok(request_v1::CollationFetchingResponse::CollationWithParentHeadData { + receipt, + pov, + parent_head_data, + }) } else { - let result = Ok(request_v1::CollationFetchingResponse::Collation(receipt, pov)); - let response = - OutgoingResponse { result, reputation_changes: Vec::new(), sent_feedback: Some(tx) }; - request.send_outgoing_response(response) + Ok(request_v1::CollationFetchingResponse::Collation(receipt, pov)) }; + let response = + OutgoingResponse { result, reputation_changes: Vec::new(), sent_feedback: Some(tx) }; - if let Err(_) = send_result { + if let Err(_) = request.send_outgoing_response(response) { gum::warn!(target: LOG_TARGET, "Sending collation response failed"); } @@ -924,10 +915,8 @@ async fn handle_incoming_peer_message( .await; }, Versioned::V1(V1::AdvertiseCollation(_)) | - Versioned::V2(V2::AdvertiseCollationV2 { .. }) | - Versioned::V2(V2::AdvertiseCollationV3 { .. }) | - Versioned::V3(V2::AdvertiseCollationV2 { .. }) | - Versioned::V3(V2::AdvertiseCollationV3 { .. }) => { + Versioned::V2(V2::AdvertiseCollation { .. }) | + Versioned::V3(V2::AdvertiseCollation { .. }) => { gum::trace!( target: LOG_TARGET, ?origin, @@ -1047,8 +1036,6 @@ async fn handle_incoming_request( per_relay_parent.collations.values_mut().next(), VersionedCollationRequest::V2(req) => per_relay_parent.collations.get_mut(&req.payload.candidate_hash), - VersionedCollationRequest::V3(req) => - per_relay_parent.collations.get_mut(&req.payload.candidate_hash), _ => { gum::warn!( target: LOG_TARGET, 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 32835df50064..b7c99d053c2e 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 @@ -592,7 +592,7 @@ async fn expect_advertise_collation_msg( ) => { assert_matches!( wire_message, - protocol_v2::CollatorProtocolMessage::AdvertiseCollationV2 { + protocol_v2::CollatorProtocolMessage::AdvertiseCollation { relay_parent, candidate_hash, .. @@ -628,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(); @@ -737,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); } @@ -1339,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); @@ -1376,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); @@ -1470,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 c4ae6eea1abb..3cef43060823 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 @@ -471,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/validator_side/collation.rs b/polkadot/node/network/collator-protocol/src/validator_side/collation.rs index 4d9510f47f2a..653511ba53a9 100644 --- a/polkadot/node/network/collator-protocol/src/validator_side/collation.rs +++ b/polkadot/node/network/collator-protocol/src/validator_side/collation.rs @@ -32,9 +32,7 @@ use std::{collections::VecDeque, future::Future, pin::Pin, task::Poll}; use futures::{future::BoxFuture, FutureExt}; use polkadot_node_network_protocol::{ peer_set::CollationVersion, - request_response::{ - outgoing::RequestError, v1 as request_v1, v3 as request_v3, OutgoingResult, - }, + request_response::{outgoing::RequestError, v1 as request_v1, OutgoingResult}, PeerId, }; use polkadot_node_primitives::PoV; @@ -104,9 +102,6 @@ pub struct PendingCollation { pub prospective_candidate: Option, /// Hash of the candidate's commitments. pub commitments_hash: Option, - /// Whether the collation was advertised with elastic scaling enabled. - /// If it was, the validator will request the parent-head data along with the collation. - pub with_elastic_scaling: bool, } impl PendingCollation { @@ -115,7 +110,6 @@ impl PendingCollation { para_id: ParaId, peer_id: &PeerId, prospective_candidate: Option, - with_elastic_scaling: bool, ) -> Self { Self { relay_parent, @@ -123,7 +117,6 @@ impl PendingCollation { peer_id: *peer_id, prospective_candidate, commitments_hash: None, - with_elastic_scaling, } } } @@ -141,8 +134,6 @@ pub struct BlockedAdvertisement { pub candidate_relay_parent: Hash, /// Hash of the candidate. pub candidate_hash: CandidateHash, - /// Whether the collation was advertised with elastic scaling enabled. - pub with_elastic_scaling: bool, } /// Performs a sanity check between advertised and fetched collations. @@ -326,7 +317,7 @@ pub(super) struct CollationFetchRequest { /// The network protocol version the collator is using. pub collator_protocol_version: CollationVersion, /// Responses from collator. - pub from_collator: BoxFuture<'static, OutgoingResult>, + pub from_collator: BoxFuture<'static, OutgoingResult>, /// Handle used for checking if this request was cancelled. pub cancellation_token: CancellationToken, /// A jaeger span corresponding to the lifetime of the request. @@ -335,13 +326,11 @@ pub(super) struct CollationFetchRequest { pub _lifetime_timer: Option, } -pub enum VersionedResponse { - V1(request_v1::CollationFetchingResponse), - V3(request_v3::CollationFetchingResponse), -} - impl Future for CollationFetchRequest { - type Output = (CollationEvent, std::result::Result); + type Output = ( + CollationEvent, + std::result::Result, + ); fn poll(mut self: Pin<&mut Self>, cx: &mut std::task::Context<'_>) -> Poll { // First check if this fetch request was cancelled. 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 e87ec561e807..248a88123a7b 100644 --- a/polkadot/node/network/collator-protocol/src/validator_side/mod.rs +++ b/polkadot/node/network/collator-protocol/src/validator_side/mod.rs @@ -34,7 +34,7 @@ use polkadot_node_network_protocol::{ peer_set::{CollationVersion, PeerSet}, request_response::{ outgoing::{Recipient, RequestError}, - v1 as request_v1, v2 as request_v2, v3 as request_v3, OutgoingRequest, Requests, + v1 as request_v1, v2 as request_v2, OutgoingRequest, Requests, }, v1 as protocol_v1, v2 as protocol_v2, OurView, PeerId, UnifiedReputationChange as Rep, Versioned, View, @@ -59,10 +59,7 @@ use polkadot_primitives::{ PersistedValidationData, }; -use crate::{ - error::{Error, FetchError, Result, SecondingError}, - validator_side::collation::VersionedResponse, -}; +use crate::error::{Error, FetchError, Result, SecondingError}; use super::{modify_reputation, tick_stream, LOG_TARGET}; @@ -693,14 +690,8 @@ async fn request_collation( return Err(FetchError::AlreadyRequested) } - let PendingCollation { - relay_parent, - para_id, - peer_id, - prospective_candidate, - with_elastic_scaling, - .. - } = pending_collation; + let PendingCollation { relay_parent, para_id, peer_id, prospective_candidate, .. } = + pending_collation; let per_relay_parent = state .per_relay_parent .get_mut(&relay_parent) @@ -714,24 +705,16 @@ async fn request_collation( request_v1::CollationFetchingRequest { relay_parent, para_id }, ); let requests = Requests::CollationFetchingV1(req); - (requests, response_recv.map(|r| r.map(VersionedResponse::V1)).boxed()) + (requests, response_recv.boxed()) + }, + (CollationVersion::V2, Some(ProspectiveCandidate { candidate_hash, .. })) => { + let (req, response_recv) = OutgoingRequest::new( + Recipient::Peer(peer_id), + request_v2::CollationFetchingRequest { relay_parent, para_id, candidate_hash }, + ); + let requests = Requests::CollationFetchingV2(req); + (requests, response_recv.boxed()) }, - (CollationVersion::V2, Some(ProspectiveCandidate { candidate_hash, .. })) => - if with_elastic_scaling { - let (req, response_recv) = OutgoingRequest::new( - Recipient::Peer(peer_id), - request_v3::CollationFetchingRequest { relay_parent, para_id, candidate_hash }, - ); - let requests = Requests::CollationFetchingV3(req); - (requests, response_recv.map(|r| r.map(VersionedResponse::V3)).boxed()) - } else { - let (req, response_recv) = OutgoingRequest::new( - Recipient::Peer(peer_id), - request_v2::CollationFetchingRequest { relay_parent, para_id, candidate_hash }, - ); - let requests = Requests::CollationFetchingV2(req); - (requests, response_recv.map(|r| r.map(VersionedResponse::V1)).boxed()) - }, _ => return Err(FetchError::ProtocolMismatch), }; @@ -892,7 +875,7 @@ async fn process_incoming_peer_message( }, Versioned::V1(V1::AdvertiseCollation(relay_parent)) => if let Err(err) = - handle_advertisement(ctx.sender(), state, relay_parent, origin, None, false).await + handle_advertisement(ctx.sender(), state, relay_parent, origin, None).await { gum::debug!( target: LOG_TARGET, @@ -906,46 +889,12 @@ async fn process_incoming_peer_message( modify_reputation(&mut state.reputation, ctx.sender(), origin, rep).await; } }, - Versioned::V2(V2::AdvertiseCollationV3 { - relay_parent, - candidate_hash, - parent_head_data_hash, - }) | - Versioned::V3(V2::AdvertiseCollationV3 { - relay_parent, - candidate_hash, - parent_head_data_hash, - }) => { - if let Err(err) = handle_advertisement( - ctx.sender(), - state, - relay_parent, - origin, - Some((candidate_hash, parent_head_data_hash)), - true, - ) - .await - { - gum::debug!( - target: LOG_TARGET, - peer_id = ?origin, - ?relay_parent, - ?candidate_hash, - error = ?err, - "Rejected v3 advertisement", - ); - - if let Some(rep) = err.reputation_changes() { - modify_reputation(&mut state.reputation, ctx.sender(), origin, rep).await; - } - } - }, - Versioned::V3(V2::AdvertiseCollationV2 { + Versioned::V3(V2::AdvertiseCollation { relay_parent, candidate_hash, parent_head_data_hash, }) | - Versioned::V2(V2::AdvertiseCollationV2 { + Versioned::V2(V2::AdvertiseCollation { relay_parent, candidate_hash, parent_head_data_hash, @@ -956,7 +905,6 @@ async fn process_incoming_peer_message( relay_parent, origin, Some((candidate_hash, parent_head_data_hash)), - false, ) .await { @@ -1084,7 +1032,6 @@ where blocked.peer_id, blocked.collator_id, Some((blocked.candidate_hash, para_head)), - blocked.with_elastic_scaling, ) .await; if let Err(fetch_error) = result { @@ -1115,7 +1062,6 @@ async fn handle_advertisement( relay_parent: Hash, peer_id: PeerId, prospective_candidate: Option<(CandidateHash, Hash)>, - with_elastic_scaling: bool, ) -> std::result::Result<(), AdvertisementError> where Sender: CollatorProtocolSenderTrait, @@ -1195,7 +1141,6 @@ where collator_id: collator_id.clone(), candidate_relay_parent: relay_parent, candidate_hash, - with_elastic_scaling, }); return Ok(()) @@ -1210,7 +1155,6 @@ where peer_id, collator_id, prospective_candidate, - with_elastic_scaling, ) .await; @@ -1238,7 +1182,6 @@ async fn enqueue_collation( peer_id: PeerId, collator_id: CollatorId, prospective_candidate: Option<(CandidateHash, Hash)>, - with_elastic_scaling: bool, ) -> std::result::Result<(), FetchError> where Sender: CollatorProtocolSenderTrait, @@ -1284,13 +1227,8 @@ where return Ok(()) } - let pending_collation = PendingCollation::new( - relay_parent, - para_id, - &peer_id, - prospective_candidate, - with_elastic_scaling, - ); + let pending_collation = + PendingCollation::new(relay_parent, para_id, &peer_id, prospective_candidate); match collations.status { CollationStatus::Fetching | CollationStatus::WaitingOnValidation => { @@ -2048,10 +1986,8 @@ async fn handle_collation_fetch_response( Err(None) }, Ok( - VersionedResponse::V1(request_v1::CollationFetchingResponse::Collation(receipt, _)) | - VersionedResponse::V3(request_v3::CollationFetchingResponse::Collation { - receipt, .. - }), + request_v1::CollationFetchingResponse::Collation(receipt, _) | + request_v1::CollationFetchingResponse::CollationWithParentHeadData { receipt, .. }, ) if receipt.descriptor().para_id != pending_collation.para_id => { gum::debug!( target: LOG_TARGET, @@ -2063,10 +1999,7 @@ async fn handle_collation_fetch_response( Err(Some((pending_collation.peer_id, COST_WRONG_PARA))) }, - Ok(VersionedResponse::V1(request_v1::CollationFetchingResponse::Collation( - candidate_receipt, - pov, - ))) => { + Ok(request_v1::CollationFetchingResponse::Collation(candidate_receipt, pov)) => { gum::debug!( target: LOG_TARGET, para_id = %pending_collation.para_id, @@ -2088,11 +2021,11 @@ async fn handle_collation_fetch_response( maybe_parent_head_data: None, }) }, - Ok(VersionedResponse::V3(request_v3::CollationFetchingResponse::Collation { + Ok(request_v2::CollationFetchingResponse::CollationWithParentHeadData { receipt, pov, parent_head_data, - })) => { + }) => { gum::debug!( target: LOG_TARGET, para_id = %pending_collation.para_id, diff --git a/polkadot/node/network/collator-protocol/src/validator_side/tests/mod.rs b/polkadot/node/network/collator-protocol/src/validator_side/tests/mod.rs index 4df4eb1c9f45..1ba6389212cc 100644 --- a/polkadot/node/network/collator-protocol/src/validator_side/tests/mod.rs +++ b/polkadot/node/network/collator-protocol/src/validator_side/tests/mod.rs @@ -421,7 +421,7 @@ async fn advertise_collation( ) { let wire_message = match candidate { Some((candidate_hash, parent_head_data_hash)) => - Versioned::V2(protocol_v2::CollatorProtocolMessage::AdvertiseCollationV2 { + Versioned::V2(protocol_v2::CollatorProtocolMessage::AdvertiseCollation { relay_parent, candidate_hash, parent_head_data_hash, diff --git a/polkadot/node/network/protocol/src/lib.rs b/polkadot/node/network/protocol/src/lib.rs index 8fb208ac1bfe..7a0ff9f4fa9a 100644 --- a/polkadot/node/network/protocol/src/lib.rs +++ b/polkadot/node/network/protocol/src/lib.rs @@ -820,7 +820,7 @@ pub mod v2 { /// Advertise a collation to a validator. Can only be sent once the peer has /// declared that they are a collator with given ID. #[codec(index = 1)] - AdvertiseCollationV2 { + AdvertiseCollation { /// Hash of the relay parent advertised collation is based on. relay_parent: Hash, /// Candidate hash. @@ -831,16 +831,6 @@ pub mod v2 { /// A collation sent to a validator was seconded. #[codec(index = 4)] CollationSeconded(Hash, UncheckedSignedFullStatement), - /// Same as `AdvertiseCollationV2`, but triggers a different req/response version. - #[codec(index = 5)] - AdvertiseCollationV3 { - /// Hash of the relay parent advertised collation is based on. - relay_parent: Hash, - /// Candidate hash. - candidate_hash: CandidateHash, - /// Parachain head data hash before candidate execution. - parent_head_data_hash: Hash, - }, } /// All network messages on the validation peer-set. diff --git a/polkadot/node/network/protocol/src/request_response/mod.rs b/polkadot/node/network/protocol/src/request_response/mod.rs index 2aee3b31f7e0..2fb62f56d104 100644 --- a/polkadot/node/network/protocol/src/request_response/mod.rs +++ b/polkadot/node/network/protocol/src/request_response/mod.rs @@ -74,9 +74,6 @@ pub mod v1; /// Actual versioned requests and responses that are sent over the wire. pub mod v2; -/// Actual versioned requests and responses that are sent over the wire. -pub mod v3; - /// A protocol per subsystem seems to make the most sense, this way we don't need any dispatching /// within protocols. #[derive(Copy, Clone, Debug, Hash, PartialEq, Eq, EnumIter)] @@ -99,9 +96,6 @@ pub enum Protocol { /// Protocol for requesting candidates with attestations in statement distribution /// when async backing is enabled. AttestedCandidateV2, - - /// Protocol for fetching collations from collators for elastic scaling. - CollationFetchingV3, } /// Minimum bandwidth we expect for validators - 500Mbit/s is the recommendation, so approximately @@ -222,17 +216,16 @@ impl Protocol { request_timeout: CHUNK_REQUEST_TIMEOUT, inbound_queue: tx, }, - Protocol::CollationFetchingV1 | - Protocol::CollationFetchingV2 | - Protocol::CollationFetchingV3 => RequestResponseConfig { - name, - fallback_names: legacy_names, - max_request_size: 1_000, - max_response_size: POV_RESPONSE_SIZE, - // Taken from initial implementation in collator protocol: - request_timeout: POV_REQUEST_TIMEOUT_CONNECTED, - inbound_queue: tx, - }, + Protocol::CollationFetchingV1 | Protocol::CollationFetchingV2 => + RequestResponseConfig { + name, + fallback_names: legacy_names, + max_request_size: 1_000, + max_response_size: POV_RESPONSE_SIZE, + // Taken from initial implementation in collator protocol: + request_timeout: POV_REQUEST_TIMEOUT_CONNECTED, + inbound_queue: tx, + }, Protocol::PoVFetchingV1 => RequestResponseConfig { name, fallback_names: legacy_names, @@ -299,9 +292,7 @@ impl Protocol { // as well. Protocol::ChunkFetchingV1 => 100, // 10 seems reasonable, considering group sizes of max 10 validators. - Protocol::CollationFetchingV1 | - Protocol::CollationFetchingV2 | - Protocol::CollationFetchingV3 => 10, + Protocol::CollationFetchingV1 | Protocol::CollationFetchingV2 => 10, // 10 seems reasonable, considering group sizes of max 10 validators. Protocol::PoVFetchingV1 => 10, // Validators are constantly self-selecting to request available data which may lead @@ -365,11 +356,10 @@ impl Protocol { Protocol::AvailableDataFetchingV1 => Some("/polkadot/req_available_data/1"), Protocol::StatementFetchingV1 => Some("/polkadot/req_statement/1"), Protocol::DisputeSendingV1 => Some("/polkadot/send_dispute/1"), - Protocol::CollationFetchingV2 => Some("/polkadot/req_collation/2"), // Introduced after legacy names became legacy. Protocol::AttestedCandidateV2 => None, - Protocol::CollationFetchingV3 => None, + Protocol::CollationFetchingV2 => None, } } } @@ -429,8 +419,6 @@ impl ReqProtocolNames { Protocol::CollationFetchingV2 => "/req_collation/2", Protocol::AttestedCandidateV2 => "/req_attested_candidate/2", - - Protocol::CollationFetchingV3 => "/req_collation/3", }; format!("{}{}", prefix, short_name).into() diff --git a/polkadot/node/network/protocol/src/request_response/outgoing.rs b/polkadot/node/network/protocol/src/request_response/outgoing.rs index 701e3611b35d..88439ad40367 100644 --- a/polkadot/node/network/protocol/src/request_response/outgoing.rs +++ b/polkadot/node/network/protocol/src/request_response/outgoing.rs @@ -24,7 +24,7 @@ use sc_network::PeerId; use polkadot_primitives::AuthorityDiscoveryId; -use super::{v1, v2, v3, IsRequest, Protocol}; +use super::{v1, v2, IsRequest, Protocol}; /// All requests that can be sent to the network bridge via `NetworkBridgeTxMessage::SendRequest`. #[derive(Debug)] @@ -47,10 +47,6 @@ pub enum Requests { /// Fetch a collation from a collator which previously announced it. /// Compared to V1 it requires specifying which candidate is requested by its hash. CollationFetchingV2(OutgoingRequest), - - /// Fetch a collation from a collator which previously announced it. - /// Compared to V2, the response includes the parent head data. - CollationFetchingV3(OutgoingRequest), } impl Requests { @@ -71,7 +67,6 @@ impl Requests { Self::StatementFetchingV1(r) => r.encode_request(), Self::DisputeSendingV1(r) => r.encode_request(), Self::AttestedCandidateV2(r) => r.encode_request(), - Self::CollationFetchingV3(r) => r.encode_request(), } } } diff --git a/polkadot/node/network/protocol/src/request_response/v1.rs b/polkadot/node/network/protocol/src/request_response/v1.rs index 0832593a6a3d..ba29b32c4ce0 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/network/protocol/src/request_response/v3.rs b/polkadot/node/network/protocol/src/request_response/v3.rs deleted file mode 100644 index 3854a3758bfa..000000000000 --- a/polkadot/node/network/protocol/src/request_response/v3.rs +++ /dev/null @@ -1,57 +0,0 @@ -// Copyright (C) Parity Technologies (UK) Ltd. -// This file is part of Polkadot. - -// Polkadot is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. - -// Polkadot is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. - -// You should have received a copy of the GNU General Public License -// along with Polkadot. If not, see . - -//! Requests and responses as sent over the wire for the individual protocols. - -use parity_scale_codec::{Decode, Encode}; - -use polkadot_node_primitives::PoV; -use polkadot_primitives::{CandidateHash, CandidateReceipt, Hash, HeadData, Id as ParaId}; - -use super::{IsRequest, Protocol}; - -/// Request the advertised collation at that relay-parent. -// Same as v2. -#[derive(Debug, Clone, Encode, Decode)] -pub struct CollationFetchingRequest { - /// Relay parent collation is built on top of. - pub relay_parent: Hash, - /// The `ParaId` of the collation. - pub para_id: ParaId, - /// Candidate hash. - pub candidate_hash: CandidateHash, -} - -/// Responses as sent by collators. -#[derive(Debug, Clone, Encode, Decode)] -pub enum CollationFetchingResponse { - /// Deliver requested collation along with parent head data. - #[codec(index = 1)] - Collation { - /// 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 { - type Response = CollationFetchingResponse; - const PROTOCOL: Protocol = Protocol::CollationFetchingV3; -} From 94ba49bfa00a4f0ce68c2dc77f95364772e84e11 Mon Sep 17 00:00:00 2001 From: ordian Date: Fri, 16 Feb 2024 04:45:50 +0100 Subject: [PATCH 3/9] get rid of with_elastic_scaling --- cumulus/client/collator/src/lib.rs | 4 +--- .../client/consensus/aura/src/collators/lookahead.rs | 4 ---- cumulus/polkadot-parachain/src/service.rs | 5 ----- polkadot/node/collation-generation/src/lib.rs | 11 +---------- polkadot/node/collation-generation/src/tests.rs | 2 -- polkadot/node/overseer/src/tests.rs | 1 - polkadot/node/primitives/src/lib.rs | 5 ----- polkadot/node/test/service/src/lib.rs | 8 ++------ .../test-parachains/adder/collator/src/main.rs | 1 - .../test-parachains/undying/collator/src/main.rs | 1 - 10 files changed, 4 insertions(+), 38 deletions(-) diff --git a/cumulus/client/collator/src/lib.rs b/cumulus/client/collator/src/lib.rs index e11eb188c67e..83249186f626 100644 --- a/cumulus/client/collator/src/lib.rs +++ b/cumulus/client/collator/src/lib.rs @@ -220,7 +220,6 @@ pub mod relay_chain_driven { this_rx.await.ok().flatten() }) })), - with_elastic_scaling: false, }; overseer_handle @@ -244,9 +243,8 @@ pub async fn initialize_collator_subsystems( key: CollatorPair, para_id: ParaId, reinitialize: bool, - with_elastic_scaling: bool, ) { - let config = CollationGenerationConfig { key, para_id, collator: None, with_elastic_scaling }; + let config = CollationGenerationConfig { key, para_id, collator: None }; if reinitialize { overseer_handle diff --git a/cumulus/client/consensus/aura/src/collators/lookahead.rs b/cumulus/client/consensus/aura/src/collators/lookahead.rs index 6a01dc8f3934..e24b7f6f1c93 100644 --- a/cumulus/client/consensus/aura/src/collators/lookahead.rs +++ b/cumulus/client/consensus/aura/src/collators/lookahead.rs @@ -107,9 +107,6 @@ pub struct Params { pub authoring_duration: Duration, /// Whether we should reinitialize the collator config (i.e. we are transitioning to aura). pub reinitialize: bool, - /// Whether elastic scaling is enabled for this collation. - /// If it is, the collator will send the parent-head data along with the collation. - pub with_elastic_scaling: bool, } /// Run async-backing-friendly Aura. @@ -155,7 +152,6 @@ where params.collator_key, params.para_id, params.reinitialize, - params.with_elastic_scaling, ) .await; diff --git a/cumulus/polkadot-parachain/src/service.rs b/cumulus/polkadot-parachain/src/service.rs index 1bf0c0f25e80..daeeadd5ecee 100644 --- a/cumulus/polkadot-parachain/src/service.rs +++ b/cumulus/polkadot-parachain/src/service.rs @@ -977,7 +977,6 @@ pub async fn start_rococo_parachain_node( collator_service, authoring_duration: Duration::from_millis(1500), reinitialize: false, - with_elastic_scaling: false, }; let fut = aura::run::< @@ -1474,7 +1473,6 @@ where collator_service, authoring_duration: Duration::from_millis(1500), reinitialize: false, - with_elastic_scaling: false, }; let fut = @@ -1770,7 +1768,6 @@ where authoring_duration: Duration::from_millis(1500), reinitialize: true, /* we need to always re-initialize for asset-hub moving * to aura */ - with_elastic_scaling: false, }; aura::run::::Pair, _, _, _, _, _, _, _, _, _>(params) @@ -1873,7 +1870,6 @@ where collator_service, authoring_duration: Duration::from_millis(1500), reinitialize: false, - with_elastic_scaling: false, }; let fut = @@ -2184,7 +2180,6 @@ pub async fn start_contracts_rococo_node( // Very limited proposal time. authoring_duration: Duration::from_millis(1500), reinitialize: false, - with_elastic_scaling: false, }; let fut = aura::run::< diff --git a/polkadot/node/collation-generation/src/lib.rs b/polkadot/node/collation-generation/src/lib.rs index 09f94cf32a6f..877933c2ef23 100644 --- a/polkadot/node/collation-generation/src/lib.rs +++ b/polkadot/node/collation-generation/src/lib.rs @@ -350,8 +350,6 @@ async fn handle_new_activations( }, }; - let with_elastic_scaling = task_config.with_elastic_scaling; - construct_and_distribute_receipt( PreparedCollation { collation, @@ -360,7 +358,6 @@ async fn handle_new_activations( validation_data, validation_code_hash, n_validators, - with_elastic_scaling, }, task_config.key.clone(), &mut task_sender, @@ -395,7 +392,6 @@ async fn handle_submit_collation( let validators = request_validators(relay_parent, ctx.sender()).await.await??; let n_validators = validators.len(); - let with_elastic_scaling = config.with_elastic_scaling; // We need to swap the parent-head data, but all other fields here will be correct. let mut validation_data = match request_persisted_validation_data( @@ -428,7 +424,6 @@ async fn handle_submit_collation( validation_data, validation_code_hash, n_validators, - with_elastic_scaling, }; construct_and_distribute_receipt( @@ -450,7 +445,6 @@ struct PreparedCollation { validation_data: PersistedValidationData, validation_code_hash: ValidationCodeHash, n_validators: usize, - with_elastic_scaling: bool, } /// Takes a prepared collation, along with its context, and produces a candidate receipt @@ -469,7 +463,6 @@ async fn construct_and_distribute_receipt( validation_data, validation_code_hash, n_validators, - with_elastic_scaling, } = collation; let persisted_validation_data_hash = validation_data.hash(); @@ -547,8 +540,7 @@ async fn construct_and_distribute_receipt( }, }; - let maybe_parent_head_data = - if with_elastic_scaling { Some(commitments.head_data.clone()) } else { None }; + let maybe_parent_head_data = None; gum::debug!( target: LOG_TARGET, @@ -556,7 +548,6 @@ async fn construct_and_distribute_receipt( ?pov_hash, ?relay_parent, para_id = %para_id, - ?with_elastic_scaling, "candidate is generated", ); metrics.on_collation_generated(); diff --git a/polkadot/node/collation-generation/src/tests.rs b/polkadot/node/collation-generation/src/tests.rs index 6600c70a595e..eb0ede6ef6b1 100644 --- a/polkadot/node/collation-generation/src/tests.rs +++ b/polkadot/node/collation-generation/src/tests.rs @@ -117,7 +117,6 @@ fn test_config>(para_id: Id) -> CollationGenerationConfig { key: CollatorPair::generate().0, collator: Some(Box::new(|_: Hash, _vd: &PersistedValidationData| TestCollator.boxed())), para_id: para_id.into(), - with_elastic_scaling: false, } } @@ -126,7 +125,6 @@ fn test_config_no_collator>(para_id: Id) -> CollationGeneration key: CollatorPair::generate().0, collator: None, para_id: para_id.into(), - with_elastic_scaling: false, } } diff --git a/polkadot/node/overseer/src/tests.rs b/polkadot/node/overseer/src/tests.rs index f9c29c2debe7..0494274367d9 100644 --- a/polkadot/node/overseer/src/tests.rs +++ b/polkadot/node/overseer/src/tests.rs @@ -824,7 +824,6 @@ fn test_collator_generation_msg() -> CollationGenerationMessage { key: CollatorPair::generate().0, collator: Some(Box::new(|_, _| TestCollator.boxed())), para_id: Default::default(), - with_elastic_scaling: false, }) } struct TestCollator; diff --git a/polkadot/node/primitives/src/lib.rs b/polkadot/node/primitives/src/lib.rs index 24d88a03da24..6e3eefbcbe8c 100644 --- a/polkadot/node/primitives/src/lib.rs +++ b/polkadot/node/primitives/src/lib.rs @@ -498,11 +498,6 @@ pub struct CollationGenerationConfig { pub collator: Option, /// The parachain that this collator collates for pub para_id: ParaId, - /// Whether elastic scaling is enabled for this collation. - /// If it is, the collator will send the parent-head data along with the collation. - /// If your parachain is not assigned to multiple cores at the same time, - /// you should set this to `false`. - pub with_elastic_scaling: bool, } #[cfg(not(target_os = "unknown"))] diff --git a/polkadot/node/test/service/src/lib.rs b/polkadot/node/test/service/src/lib.rs index d862decf2e7e..e4eec32baf2a 100644 --- a/polkadot/node/test/service/src/lib.rs +++ b/polkadot/node/test/service/src/lib.rs @@ -353,12 +353,8 @@ impl PolkadotTestNode { para_id: ParaId, collator: CollatorFn, ) { - let config = CollationGenerationConfig { - key: collator_key, - collator: Some(collator), - para_id, - with_elastic_scaling: false, - }; + let config = + CollationGenerationConfig { key: collator_key, collator: Some(collator), para_id }; self.overseer_handle .send_msg(CollationGenerationMessage::Initialize(config), "Collator") diff --git a/polkadot/parachain/test-parachains/adder/collator/src/main.rs b/polkadot/parachain/test-parachains/adder/collator/src/main.rs index 70171afae2f9..fec90fc41cdb 100644 --- a/polkadot/parachain/test-parachains/adder/collator/src/main.rs +++ b/polkadot/parachain/test-parachains/adder/collator/src/main.rs @@ -119,7 +119,6 @@ fn main() -> Result<()> { collator.create_collation_function(full_node.task_manager.spawn_handle()), ), para_id, - with_elastic_scaling: false, }; overseer_handle .send_msg(CollationGenerationMessage::Initialize(config), "Collator") diff --git a/polkadot/parachain/test-parachains/undying/collator/src/main.rs b/polkadot/parachain/test-parachains/undying/collator/src/main.rs index 84e1f9b19fd7..45f21e7b8596 100644 --- a/polkadot/parachain/test-parachains/undying/collator/src/main.rs +++ b/polkadot/parachain/test-parachains/undying/collator/src/main.rs @@ -121,7 +121,6 @@ fn main() -> Result<()> { collator.create_collation_function(full_node.task_manager.spawn_handle()), ), para_id, - with_elastic_scaling: false, }; overseer_handle .send_msg(CollationGenerationMessage::Initialize(config), "Collator") From 211edb7f615727e8b552548f48eaa4e50dfe166a Mon Sep 17 00:00:00 2001 From: ordian Date: Mon, 19 Feb 2024 15:43:06 +0100 Subject: [PATCH 4/9] no maybes --- polkadot/node/collation-generation/src/lib.rs | 5 +-- .../src/collator_side/collation.rs | 4 +- .../src/collator_side/mod.rs | 39 ++++++++++--------- .../src/collator_side/tests/mod.rs | 2 +- .../tests/prospective_parachains.rs | 4 +- polkadot/node/subsystem-types/src/messages.rs | 18 ++++----- 6 files changed, 35 insertions(+), 37 deletions(-) diff --git a/polkadot/node/collation-generation/src/lib.rs b/polkadot/node/collation-generation/src/lib.rs index 877933c2ef23..a89351628a08 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. @@ -540,8 +541,6 @@ async fn construct_and_distribute_receipt( }, }; - let maybe_parent_head_data = None; - gum::debug!( target: LOG_TARGET, candidate_hash = ?ccr.hash(), @@ -557,7 +556,7 @@ async fn construct_and_distribute_receipt( candidate_receipt: ccr, parent_head_data_hash, pov, - maybe_parent_head_data, + parent_head_data, result_sender, }) .await; 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 88f463951f79..dc1ee725462f 100644 --- a/polkadot/node/network/collator-protocol/src/collator_side/collation.rs +++ b/polkadot/node/network/collator-protocol/src/collator_side/collation.rs @@ -63,8 +63,8 @@ pub struct Collation { pub parent_head_data_hash: Hash, /// Proof to verify the state transition of the parachain. pub pov: PoV, - /// Optional parent head-data needed for elastic scaling. - pub maybe_parent_head_data: Option, + /// 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 e63987b2080b..19cc1eb1a57c 100644 --- a/polkadot/node/network/collator-protocol/src/collator_side/mod.rs +++ b/polkadot/node/network/collator-protocol/src/collator_side/mod.rs @@ -347,7 +347,7 @@ async fn distribute_collation( receipt: CandidateReceipt, parent_head_data_hash: Hash, pov: PoV, - maybe_parent_head_data: Option, + parent_head_data: HeadData, result_sender: Option>, ) -> Result<()> { let candidate_relay_parent = receipt.descriptor.relay_parent; @@ -470,7 +470,7 @@ async fn distribute_collation( receipt, parent_head_data_hash, pov, - maybe_parent_head_data, + parent_head_data, status: CollationStatus::Created, }, ); @@ -774,7 +774,7 @@ async fn process_msg( candidate_receipt, parent_head_data_hash, pov, - maybe_parent_head_data, + parent_head_data, result_sender, } => { let _span1 = state @@ -804,7 +804,7 @@ async fn process_msg( candidate_receipt, parent_head_data_hash, pov, - maybe_parent_head_data, + parent_head_data, result_sender, ) .await?; @@ -849,7 +849,7 @@ async fn send_collation( request: VersionedCollationRequest, receipt: CandidateReceipt, pov: PoV, - maybe_parent_head_data: Option, + _parent_head_data: HeadData, ) { let (tx, rx) = oneshot::channel(); @@ -859,15 +859,18 @@ async fn send_collation( // The response payload is the same for v1 and v2 versions of protocol // and doesn't have v2 alias for simplicity. - let result = if let Some(parent_head_data) = maybe_parent_head_data { - Ok(request_v1::CollationFetchingResponse::CollationWithParentHeadData { - receipt, - pov, - parent_head_data, - }) - } else { + // 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) }; @@ -1048,12 +1051,12 @@ async fn handle_incoming_request( return Ok(()) }, }; - let (receipt, pov, maybe_parent_head_data) = 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.maybe_parent_head_data.clone(), + collation.parent_head_data.clone(), ) } else { gum::warn!( @@ -1093,7 +1096,7 @@ async fn handle_incoming_request( waiting.collation_fetch_active = true; // Obtain a timer for sending collation let _ = state.metrics.time_collation_distribution("send"); - send_collation(state, req, receipt, pov, maybe_parent_head_data).await; + send_collation(state, req, receipt, pov, parent_head_data).await; } }, Some(our_para_id) => { @@ -1478,9 +1481,9 @@ async fn run_inner( if let Some(collation) = next_collation { let receipt = collation.receipt.clone(); let pov = collation.pov.clone(); - let maybe_parent_head_data = collation.maybe_parent_head_data.clone(); + let parent_head_data = collation.parent_head_data.clone(); - send_collation(&mut state, next, receipt, pov, maybe_parent_head_data).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 b7c99d053c2e..beda941d37ab 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 @@ -360,7 +360,7 @@ async fn distribute_collation_with_receipt( candidate_receipt: candidate.clone(), parent_head_data_hash, pov: pov.clone(), - maybe_parent_head_data: None, + parent_head_data: HeadData(vec![1, 2, 3]), result_sender: None, }, ) 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 3cef43060823..2bb7002a4f47 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 @@ -275,7 +275,7 @@ fn distribute_collation_from_implicit_view() { candidate_receipt: candidate.clone(), parent_head_data_hash, pov: pov.clone(), - maybe_parent_head_data: None, + parent_head_data: HeadData(vec![1, 2, 3]), result_sender: None, }, ) @@ -356,7 +356,7 @@ fn distribute_collation_up_to_limit() { candidate_receipt: candidate.clone(), parent_head_data_hash, pov: pov.clone(), - maybe_parent_head_data: None, + parent_head_data: HeadData(vec![1, 2, 3]), result_sender: None, }, ) diff --git a/polkadot/node/subsystem-types/src/messages.rs b/polkadot/node/subsystem-types/src/messages.rs index 29cb1b64d487..c15f38237ddb 100644 --- a/polkadot/node/subsystem-types/src/messages.rs +++ b/polkadot/node/subsystem-types/src/messages.rs @@ -208,21 +208,17 @@ 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 { - /// TODO(ordian) + /// The receipt of the candidate. candidate_receipt: CandidateReceipt, - /// TODO(ordian) + /// The hash of the parent head-data. parent_head_data_hash: Hash, - /// TODO(ordian) + /// Proof of validity. pov: PoV, - /// This field is only used for elastic scaling. - // TODO(ordian): maybe using an enum - maybe_parent_head_data: Option, - /// TODO(ordian) + /// 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>, }, /// Report a collator as having provided an invalid collation. This should lead to disconnect From 332044750cf049eae4e211acff2e6331e2f66e44 Mon Sep 17 00:00:00 2001 From: ordian Date: Mon, 19 Feb 2024 16:22:40 +0100 Subject: [PATCH 5/9] prdoc --- prdoc/pr_3302.prdoc | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 prdoc/pr_3302.prdoc diff --git a/prdoc/pr_3302.prdoc b/prdoc/pr_3302.prdoc new file mode 100644 index 000000000000..a2d93fc60735 --- /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 From 10a3e9c91fdfa712a27965a2916dc3465953ad97 Mon Sep 17 00:00:00 2001 From: ordian Date: Tue, 20 Feb 2024 07:51:41 +0100 Subject: [PATCH 6/9] sanity check the provided parent head data hash --- .../network/collator-protocol/src/error.rs | 3 +++ .../src/validator_side/collation.rs | 3 +++ .../src/validator_side/mod.rs | 25 ++++++++++++------- 3 files changed, 22 insertions(+), 9 deletions(-) diff --git a/polkadot/node/network/collator-protocol/src/error.rs b/polkadot/node/network/collator-protocol/src/error.rs index 9348198e7085..8338ddfa9c54 100644 --- a/polkadot/node/network/collator-protocol/src/error.rs +++ b/polkadot/node/network/collator-protocol/src/error.rs @@ -89,6 +89,9 @@ 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 { 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 653511ba53a9..8c3889a35548 100644 --- a/polkadot/node/network/collator-protocol/src/validator_side/collation.rs +++ b/polkadot/node/network/collator-protocol/src/validator_side/collation.rs @@ -144,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) @@ -152,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(()) } 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 248a88123a7b..45ce3e4a8891 100644 --- a/polkadot/node/network/collator-protocol/src/validator_side/mod.rs +++ b/polkadot/node/network/collator-protocol/src/validator_side/mod.rs @@ -1827,39 +1827,46 @@ async fn kick_off_seconding( 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, + 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( From b7466c2de33d52d49c56d88772b3a6dcd118413d Mon Sep 17 00:00:00 2001 From: ordian Date: Tue, 27 Feb 2024 11:22:42 +0100 Subject: [PATCH 7/9] address review: use enum for parent head-data --- .../core/prospective-parachains/src/lib.rs | 18 ++++++++++------ .../core/prospective-parachains/src/tests.rs | 5 ++--- .../src/validator_side/mod.rs | 14 +++++++------ polkadot/node/subsystem-types/src/messages.rs | 21 ++++++++++++++----- 4 files changed, 38 insertions(+), 20 deletions(-) diff --git a/polkadot/node/core/prospective-parachains/src/lib.rs b/polkadot/node/core/prospective-parachains/src/lib.rs index f57730304724..cd7dd8f20d22 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::{ ChainApiMessage, FragmentTreeMembership, HypotheticalCandidate, - HypotheticalFrontierRequest, IntroduceCandidateRequest, ProspectiveParachainsMessage, - ProspectiveValidationDataRequest, RuntimeApiMessage, RuntimeApiRequest, + HypotheticalFrontierRequest, IntroduceCandidateRequest, ParentHeadData, + ProspectiveParachainsMessage, ProspectiveValidationDataRequest, RuntimeApiMessage, + RuntimeApiRequest, }, overseer, ActiveLeavesUpdate, FromOrchestra, OverseerSignal, SpawnedSubsystem, SubsystemError, }; @@ -771,9 +772,14 @@ fn answer_prospective_validation_data_request( Some(s) => s, }; - let mut head_data = request - .maybe_parent_head_data - .or_else(|| 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; @@ -791,7 +797,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 b7ff9c2a9b3c..eb2a9a80f228 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, }, }; @@ -472,8 +472,7 @@ async fn get_pvd( let request = ProspectiveValidationDataRequest { para_id, candidate_relay_parent, - parent_head_data_hash: parent_head_data.hash(), - maybe_parent_head_data: None, + 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/validator_side/mod.rs b/polkadot/node/network/collator-protocol/src/validator_side/mod.rs index 45ce3e4a8891..74095b20082b 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, @@ -1784,13 +1784,15 @@ where { let (tx, rx) = oneshot::channel(); - let request = ProspectiveValidationDataRequest { - para_id, - candidate_relay_parent, - parent_head_data_hash, - maybe_parent_head_data, + 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 }; + sender .send_message(ProspectiveParachainsMessage::GetProspectiveValidationData(request, tx)) .await; diff --git a/polkadot/node/subsystem-types/src/messages.rs b/polkadot/node/subsystem-types/src/messages.rs index c15f38237ddb..ac5d0c47c791 100644 --- a/polkadot/node/subsystem-types/src/messages.rs +++ b/polkadot/node/subsystem-types/src/messages.rs @@ -1108,11 +1108,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, - /// Optionally, the head-data of the parent. - /// This will be provided for collations with elastic scaling enabled. - pub maybe_parent_head_data: Option, + /// 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 From 1ff7a54d7f7fe7080a0cc6cd4ebc4526ae7bf8c9 Mon Sep 17 00:00:00 2001 From: ordian Date: Wed, 28 Feb 2024 10:23:04 +0100 Subject: [PATCH 8/9] test validator sanity check parent_head_data hash --- .../network/collator-protocol/src/error.rs | 7 +- .../tests/prospective_parachains.rs | 120 ++++++++++++++++++ 2 files changed, 126 insertions(+), 1 deletion(-) diff --git a/polkadot/node/network/collator-protocol/src/error.rs b/polkadot/node/network/collator-protocol/src/error.rs index 8338ddfa9c54..0f5e0699d85c 100644 --- a/polkadot/node/network/collator-protocol/src/error.rs +++ b/polkadot/node/network/collator-protocol/src/error.rs @@ -98,7 +98,12 @@ 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/tests/prospective_parachains.rs b/polkadot/node/network/collator-protocol/src/validator_side/tests/prospective_parachains.rs index 23963e65554e..eaa725f2642e 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(); From 7322058efada8210bff4989f4b96b8904e976b40 Mon Sep 17 00:00:00 2001 From: ordian Date: Mon, 4 Mar 2024 09:00:48 +0100 Subject: [PATCH 9/9] add a comment (review) --- polkadot/node/subsystem-types/src/messages.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/polkadot/node/subsystem-types/src/messages.rs b/polkadot/node/subsystem-types/src/messages.rs index 13e61bc976c4..eeb684149c80 100644 --- a/polkadot/node/subsystem-types/src/messages.rs +++ b/polkadot/node/subsystem-types/src/messages.rs @@ -212,6 +212,7 @@ pub enum CollatorProtocolMessage { /// 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,