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 a0f0bc83f022c..21bf2137e40b9 100644 --- a/polkadot/node/network/collator-protocol/src/collator_side/mod.rs +++ b/polkadot/node/network/collator-protocol/src/collator_side/mod.rs @@ -961,6 +961,7 @@ async fn advertise_collation( candidate_hash: *candidate_hash, parent_head_data_hash: collation.parent_head_data.hash(), candidate_descriptor_version, + relay_parent: collation.receipt.descriptor.relay_parent(), }, )) }, 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 82298ca95fab1..da2983cc1a909 100644 --- a/polkadot/node/network/collator-protocol/src/validator_side/collation.rs +++ b/polkadot/node/network/collator-protocol/src/validator_side/collation.rs @@ -103,6 +103,8 @@ impl FetchedCollation { pub struct PendingCollation { /// Candidate's scheduling parent pub scheduling_parent: Hash, + /// Candidate's relay parent + pub relay_parent: Hash, /// Parachain id. pub para_id: ParaId, /// Peer that advertised this collation. @@ -149,6 +151,7 @@ impl PendingCollation { /// For V3 protocol advertisements, pass `Some(version)` to track the advertised version. pub fn new( scheduling_parent: Hash, + relay_parent: Hash, para_id: ParaId, peer_id: &PeerId, prospective_candidate: Option, @@ -156,6 +159,7 @@ impl PendingCollation { ) -> Self { Self { scheduling_parent, + relay_parent, para_id, peer_id: *peer_id, prospective_candidate, @@ -199,6 +203,10 @@ pub fn fetched_collation_sanity_check( return Err(SecondingError::SchedulingParentMismatch); } + if advertised.relay_parent != fetched.descriptor.relay_parent() { + return Err(SecondingError::RelayParentMismatch); + } + if maybe_parent_head_and_hash.map_or(false, |(head, hash)| head.hash() != hash) { return Err(SecondingError::ParentHeadDataMismatch); } diff --git a/polkadot/node/network/collator-protocol/src/validator_side/error.rs b/polkadot/node/network/collator-protocol/src/validator_side/error.rs index 90fe762edd579..79cad44825d7a 100644 --- a/polkadot/node/network/collator-protocol/src/validator_side/error.rs +++ b/polkadot/node/network/collator-protocol/src/validator_side/error.rs @@ -76,6 +76,9 @@ pub enum SecondingError { #[error("Scheduling parent hash doesn't match the advertisement")] SchedulingParentMismatch, + #[error("Relay parent hash doesn't match the advertisement")] + RelayParentMismatch, + #[error("Received duplicate collation from the peer")] Duplicate, @@ -109,6 +112,7 @@ impl SecondingError { PersistedValidationDataMismatch | CandidateHashMismatch | SchedulingParentMismatch | + RelayParentMismatch | ParentHeadDataMismatch | InvalidCoreIndex(_, _) | InvalidSessionIndex(_, _) | 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 7ab8b92c3ab66..bf1b136220039 100644 --- a/polkadot/node/network/collator-protocol/src/validator_side/mod.rs +++ b/polkadot/node/network/collator-protocol/src/validator_side/mod.rs @@ -565,6 +565,8 @@ struct PerSchedulingParent { /// Information about a held off advertisement struct HeldOffAdvertisement { + /// The relay parent of the candidate. + relay_parent: Hash, /// The scheduling parent it's based on. scheduling_parent: Hash, /// The peer id of the collator that has sent the advertisement. @@ -574,6 +576,9 @@ struct HeldOffAdvertisement { /// The prospective candidate hash and its relay parent, if available. Will be none if collator /// protocol v1 is used. prospective_candidate: Option<(CandidateHash, Hash)>, + /// Advertised candidate descriptor version (for V3 protocol). + /// None for V1/V2 protocols. + advertised_descriptor_version: Option, } /// Scheduling info tracked per active leaf, used for V3 scheduling parent validation. @@ -1232,6 +1237,7 @@ async fn process_incoming_peer_message( candidate_hash, parent_head_data_hash, candidate_descriptor_version, + relay_parent, }) => { if let Err(err) = handle_advertisement_v3( ctx.sender(), @@ -1241,12 +1247,14 @@ async fn process_incoming_peer_message( candidate_hash, parent_head_data_hash, candidate_descriptor_version, + relay_parent, ) .await { gum::debug!( target: LOG_TARGET, peer_id = ?origin, + ?relay_parent, ?scheduling_parent, ?candidate_hash, ?candidate_descriptor_version, @@ -1282,6 +1290,8 @@ fn hold_off_asset_hub_collation_if_needed( collator_id: &CollatorId, scheduling_parent: Hash, prospective_candidate: Option<(CandidateHash, Hash)>, + relay_parent: Hash, + advertised_descriptor_version: Option, ) -> bool { // If we don't know the peer we should reject the advertisement but to avoid verbosity and // copy-pasted logic we'll just return `false` and let the caller handle it. @@ -1322,10 +1332,12 @@ fn hold_off_asset_hub_collation_if_needed( let hold_off_outcome = rp_state.ah_held_off_advertisements.hold_off_if_necessary(HeldOffAdvertisement { + relay_parent, scheduling_parent, peer_id, collator_id, prospective_candidate, + advertised_descriptor_version, }); match hold_off_outcome { @@ -1388,6 +1400,8 @@ enum AdvertisementError { Invalid(InsertAdvertisementError), /// Seconding not allowed by backing subsystem BlockedByBacking, + /// For non-V3 descriptors, the relay parent must equal the scheduling parent. + RelayParentMismatch, } impl AdvertisementError { @@ -1398,7 +1412,7 @@ impl AdvertisementError { SchedulingParentUnknown | UndeclaredCollator | Invalid(_) => { Some(COST_UNEXPECTED_MESSAGE) }, - SchedulingParentNotValid => Some(COST_INVALID_SCHEDULING_PARENT), + SchedulingParentNotValid | RelayParentMismatch => Some(COST_INVALID_SCHEDULING_PARENT), UnknownPeer | SecondedLimitReached | BlockedByBacking => None, } } @@ -1694,6 +1708,8 @@ where &collator_id, scheduling_parent, prospective_candidate, + scheduling_parent, // For V1/V2, the relay parent is the same as the scheduling parent + None, // V1/V2 don't have advertised descriptor version ) { return Ok(()); } @@ -1702,6 +1718,7 @@ where sender, state, scheduling_parent, + scheduling_parent, para_id, peer_id, collator_id, @@ -1719,12 +1736,20 @@ async fn handle_advertisement_v3( candidate_hash: CandidateHash, parent_head_data_hash: Hash, candidate_descriptor_version: CandidateDescriptorVersion, + relay_parent: Hash, ) -> std::result::Result<(), AdvertisementError> where Sender: CollatorProtocolSenderTrait, { let peer_data = state.peer_data.get_mut(&peer_id).ok_or(AdvertisementError::UnknownPeer)?; + // For non-V3 descriptors, the relay parent must equal the scheduling parent. + if candidate_descriptor_version != CandidateDescriptorVersion::V3 && + relay_parent != scheduling_parent + { + return Err(AdvertisementError::RelayParentMismatch); + } + // Fail fast if the scheduling parent is completely unknown. let per_scheduling_parent = state .per_scheduling_parent @@ -1779,6 +1804,8 @@ where &collator_id, scheduling_parent, Some((candidate_hash, parent_head_data_hash)), + relay_parent, + Some(candidate_descriptor_version), ) { return Ok(()); } @@ -1786,6 +1813,7 @@ where process_advertisement( sender, state, + relay_parent, scheduling_parent, para_id, peer_id, @@ -1799,6 +1827,7 @@ where async fn process_advertisement( sender: &mut Sender, state: &mut State, + relay_parent: Hash, scheduling_parent: Hash, para_id: ParaId, peer_id: PeerId, @@ -1830,6 +1859,7 @@ where let result = enqueue_collation( sender, state, + relay_parent, scheduling_parent, para_id, peer_id, @@ -1858,6 +1888,7 @@ where async fn enqueue_collation( sender: &mut Sender, state: &mut State, + relay_parent: Hash, scheduling_parent: Hash, para_id: ParaId, peer_id: PeerId, @@ -1899,6 +1930,7 @@ where let collations = &mut per_scheduling_parent.collations; let pending_collation = PendingCollation::new( scheduling_parent, + relay_parent, para_id, &peer_id, prospective_candidate, @@ -2544,7 +2576,7 @@ async fn run_inner( }; for held_off_advertisement in held_off_advertisements { - let HeldOffAdvertisement{scheduling_parent, peer_id, collator_id, prospective_candidate} = held_off_advertisement; + let HeldOffAdvertisement{relay_parent, scheduling_parent, peer_id, collator_id, prospective_candidate, advertised_descriptor_version} = held_off_advertisement; gum::debug!( target: LOG_TARGET, ?scheduling_parent, @@ -2556,12 +2588,13 @@ async fn run_inner( if let Err(err) = process_advertisement( ctx.sender(), &mut state, + relay_parent, scheduling_parent, ASSET_HUB_PARA_ID, peer_id, collator_id, prospective_candidate, - None, // V1/V2 advertisement, no descriptor version + advertised_descriptor_version ) .await { gum::debug!( 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 11df88cbb43d3..d41e9c53a7f8c 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 @@ -479,6 +479,7 @@ async fn advertise_collation_v3( candidate_hash: CandidateHash, parent_head_data_hash: Hash, candidate_descriptor_version: CandidateDescriptorVersion, + relay_parent: Hash, ) { let wire_message = CollationProtocols::V3(protocol_v3::CollatorProtocolMessage::AdvertiseCollation { @@ -486,6 +487,7 @@ async fn advertise_collation_v3( candidate_hash, parent_head_data_hash, candidate_descriptor_version, + relay_parent, }); overseer_send( virtual_overseer, 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 3cf9444e2f5e2..2a3c3f54c65dc 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 @@ -2109,6 +2109,7 @@ fn v3_scheduling_parent_rejected_on_stalled_relay_chain() { candidate_hash, parent_head_data_hash, CandidateDescriptorVersion::V3, + head_b, ) .await; @@ -2205,6 +2206,7 @@ fn v3_scheduling_parent_in_progress_slot_accepts_leaf_parent() { candidate_hash, parent_head_data_hash, CandidateDescriptorVersion::V3, + head_b_grandparent, ) .await; @@ -2336,6 +2338,7 @@ fn v3_scheduling_parent_finished_slot_accepts_leaf() { candidate_hash, parent_head_data_hash, CandidateDescriptorVersion::V3, + head_b_parent, ) .await; @@ -2457,6 +2460,7 @@ fn v3_scheduling_parent_in_progress_slot_rejects_leaf() { candidate_hash, parent_head_data_hash, CandidateDescriptorVersion::V3, + head_b_parent, ) .await; @@ -2553,6 +2557,7 @@ fn v3_scheduling_parent_finished_slot_rejects_parent() { candidate_hash, parent_head_data_hash, CandidateDescriptorVersion::V3, + head_b_grandparent, ) .await; @@ -2628,6 +2633,7 @@ fn v3_scheduling_parent_outside_allowed_ancestry_rejected() { candidate_hash, parent_head_data_hash, CandidateDescriptorVersion::V3, + head_b, ) .await; diff --git a/polkadot/node/network/protocol/src/lib.rs b/polkadot/node/network/protocol/src/lib.rs index ffe10188479df..f4973757bb07f 100644 --- a/polkadot/node/network/protocol/src/lib.rs +++ b/polkadot/node/network/protocol/src/lib.rs @@ -600,7 +600,7 @@ pub mod v3_collation { #[codec(index = 1)] AdvertiseCollation { /// Hash of the scheduling parent - used for validator assignment. - /// For V3 candidate descriptors, this must be an active leaf. + /// For non-v3 descriptors, this must be equal to the relay parent. scheduling_parent: Hash, /// Candidate hash. candidate_hash: CandidateHash, @@ -608,6 +608,8 @@ pub mod v3_collation { parent_head_data_hash: Hash, /// The version of the candidate descriptor. candidate_descriptor_version: CandidateDescriptorVersion, + /// The relay parent of the candidate. + relay_parent: Hash, }, /// A collation sent to a validator was seconded. #[codec(index = 4)] diff --git a/prdoc/pr_11393.prdoc b/prdoc/pr_11393.prdoc new file mode 100644 index 0000000000000..a84007508822b --- /dev/null +++ b/prdoc/pr_11393.prdoc @@ -0,0 +1,14 @@ +title: 'Add relay parent to V3 collation protocol advertisement' +doc: +- audience: Node Dev + description: | + Adds a `relay_parent` field to the V3 collation protocol `AdvertiseCollation` message. + + Also stores the relay parent and advertised descriptor version in pending collations + and held-off AssetHub advertisements, so that all sanity checks (including descriptor + version mismatch and relay parent mismatch) are correctly applied after fetching. +crates: +- name: polkadot-node-network-protocol + bump: major +- name: polkadot-collator-protocol + bump: major