From 3ba4d96b89d39982dacce6955a164d84f3b1dac7 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Fri, 31 May 2024 21:25:25 +0300 Subject: [PATCH 01/13] remove the use of collatorId and collator signature Signed-off-by: Andrei Sandu --- cumulus/client/collator/src/lib.rs | 4 +- .../consensus/aura/src/collators/lookahead.rs | 5 +-- cumulus/polkadot-parachain/src/service.rs | 2 - polkadot/node/collation-generation/src/lib.rs | 31 ++++---------- .../node/collation-generation/src/tests.rs | 42 ++++--------------- .../node/core/candidate-validation/src/lib.rs | 4 -- .../src/fragment_chain/mod.rs | 2 - polkadot/node/primitives/src/lib.rs | 4 +- .../src/inclusion_emulator/mod.rs | 10 +---- polkadot/primitives/test-helpers/src/lib.rs | 7 ++-- polkadot/runtime/parachains/src/builder.rs | 36 ++++++++-------- .../runtime/parachains/src/inclusion/mod.rs | 7 ---- 12 files changed, 42 insertions(+), 112 deletions(-) diff --git a/cumulus/client/collator/src/lib.rs b/cumulus/client/collator/src/lib.rs index 47da0f6d96f2..b3804800e35f 100644 --- a/cumulus/client/collator/src/lib.rs +++ b/cumulus/client/collator/src/lib.rs @@ -199,7 +199,6 @@ pub mod relay_chain_driven { let (stream_tx, stream_rx) = mpsc::channel(0); let config = CollationGenerationConfig { - key, para_id, collator: Some(Box::new(move |relay_parent, validation_data| { // Cloning the channel on each usage effectively makes the channel @@ -240,11 +239,10 @@ pub mod relay_chain_driven { /// For callback-driven collators, use the [`relay_chain_driven`] module. pub async fn initialize_collator_subsystems( overseer_handle: &mut OverseerHandle, - key: CollatorPair, para_id: ParaId, reinitialize: bool, ) { - let config = CollationGenerationConfig { key, para_id, collator: None }; + let config = CollationGenerationConfig { 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 09416233ea9b..27a1ae03d47e 100644 --- a/cumulus/client/consensus/aura/src/collators/lookahead.rs +++ b/cumulus/client/consensus/aura/src/collators/lookahead.rs @@ -50,7 +50,7 @@ use polkadot_node_subsystem::messages::{ }; use polkadot_overseer::Handle as OverseerHandle; use polkadot_primitives::{ - AsyncBackingParams, CollatorPair, CoreIndex, CoreState, Id as ParaId, OccupiedCoreAssumption, + AsyncBackingParams, CoreIndex, CoreState, Id as ParaId, OccupiedCoreAssumption, }; use futures::{channel::oneshot, prelude::*}; @@ -91,8 +91,6 @@ pub struct Params { pub sync_oracle: SO, /// The underlying keystore, which should contain Aura consensus keys. pub keystore: KeystorePtr, - /// The collator key used to sign collations before submitting to validators. - pub collator_key: CollatorPair, /// The para's ID. pub para_id: ParaId, /// A handle to the relay-chain client's "Overseer" or task orchestrator. @@ -149,7 +147,6 @@ where async move { cumulus_client_collator::initialize_collator_subsystems( &mut params.overseer_handle, - params.collator_key, params.para_id, params.reinitialize, ) diff --git a/cumulus/polkadot-parachain/src/service.rs b/cumulus/polkadot-parachain/src/service.rs index 12eda3e8a9cb..432a0a355aad 100644 --- a/cumulus/polkadot-parachain/src/service.rs +++ b/cumulus/polkadot-parachain/src/service.rs @@ -832,7 +832,6 @@ where }, sync_oracle, keystore, - collator_key, para_id, overseer_handle, relay_chain_slot_duration, @@ -973,7 +972,6 @@ fn start_lookahead_aura_consensus( }, sync_oracle, keystore, - collator_key, para_id, overseer_handle, relay_chain_slot_duration, diff --git a/polkadot/node/collation-generation/src/lib.rs b/polkadot/node/collation-generation/src/lib.rs index 374f090a2671..f47f0f488068 100644 --- a/polkadot/node/collation-generation/src/lib.rs +++ b/polkadot/node/collation-generation/src/lib.rs @@ -48,11 +48,11 @@ use polkadot_node_subsystem_util::{ request_validators, vstaging::fetch_claim_queue, }; use polkadot_primitives::{ - collator_signature_payload, CandidateCommitments, CandidateDescriptor, CandidateReceipt, - CollatorPair, CoreIndex, CoreState, Hash, Id as ParaId, OccupiedCoreAssumption, - PersistedValidationData, ScheduledCore, ValidationCodeHash, + CandidateCommitments, CandidateDescriptor, CandidateReceipt, CollatorId, CollatorSignature, + CoreIndex, CoreState, Hash, Id as ParaId, OccupiedCoreAssumption, PersistedValidationData, + ScheduledCore, ValidationCodeHash, }; -use sp_core::crypto::Pair; +use sp_core::ByteArray; use std::sync::Arc; mod error; @@ -424,7 +424,6 @@ async fn handle_new_activations( n_validators, core_index, }, - task_config.key.clone(), &mut task_sender, result_sender, &metrics, @@ -497,14 +496,7 @@ async fn handle_submit_collation( core_index, }; - construct_and_distribute_receipt( - collation, - config.key.clone(), - ctx.sender(), - result_sender, - metrics, - ) - .await; + construct_and_distribute_receipt(collation, ctx.sender(), result_sender, metrics).await; Ok(()) } @@ -523,7 +515,6 @@ struct PreparedCollation { /// which is distributed to validators. async fn construct_and_distribute_receipt( collation: PreparedCollation, - key: CollatorPair, sender: &mut impl overseer::CollationGenerationSenderTrait, result_sender: Option>, metrics: &Metrics, @@ -569,14 +560,6 @@ async fn construct_and_distribute_receipt( let pov_hash = pov.hash(); - let signature_payload = collator_signature_payload( - &relay_parent, - ¶_id, - &persisted_validation_data_hash, - &pov_hash, - &validation_code_hash, - ); - let erasure_root = match erasure_root(n_validators, validation_data, pov.clone()) { Ok(erasure_root) => erasure_root, Err(err) => { @@ -602,10 +585,10 @@ async fn construct_and_distribute_receipt( let ccr = CandidateReceipt { commitments_hash: commitments.hash(), descriptor: CandidateDescriptor { - signature: key.sign(&signature_payload), + signature: CollatorSignature::from_slice(&vec![0u8; 64]).expect("64 bytes; qed"), para_id, relay_parent, - collator: key.public(), + collator: CollatorId::from_slice(&vec![0u8; 32]).expect("32 bytes; qed"), persisted_validation_data_hash, pov_hash, erasure_root, diff --git a/polkadot/node/collation-generation/src/tests.rs b/polkadot/node/collation-generation/src/tests.rs index 10c391cba25d..6119136de02a 100644 --- a/polkadot/node/collation-generation/src/tests.rs +++ b/polkadot/node/collation-generation/src/tests.rs @@ -31,8 +31,8 @@ use polkadot_node_subsystem_test_helpers::{subsystem_test_harness, TestSubsystem use polkadot_node_subsystem_util::TimeoutExt; use polkadot_primitives::{ async_backing::{BackingState, CandidatePendingAvailability}, - AsyncBackingParams, BlockNumber, CollatorPair, HeadData, PersistedValidationData, - ScheduledCore, ValidationCode, + AsyncBackingParams, BlockNumber, CollatorId, CollatorSignature, HeadData, + PersistedValidationData, ScheduledCore, ValidationCode, }; use rstest::rstest; use sp_keyring::sr25519::Keyring as Sr25519Keyring; @@ -40,6 +40,8 @@ use std::{ collections::{BTreeMap, VecDeque}, pin::Pin, }; + +use sp_core::ByteArray; use test_helpers::{ dummy_candidate_descriptor, dummy_hash, dummy_head_data, dummy_validator, make_candidate, }; @@ -123,18 +125,13 @@ async fn overseer_recv(overseer: &mut VirtualOverseer) -> AllMessages { fn test_config>(para_id: Id) -> CollationGenerationConfig { CollationGenerationConfig { - key: CollatorPair::generate().0, collator: Some(Box::new(|_: Hash, _vd: &PersistedValidationData| TestCollator.boxed())), para_id: para_id.into(), } } fn test_config_no_collator>(para_id: Id) -> CollationGenerationConfig { - CollationGenerationConfig { - key: CollatorPair::generate().0, - collator: None, - para_id: para_id.into(), - } + CollationGenerationConfig { collator: None, para_id: para_id.into() } } fn scheduled_core_for>(para_id: Id) -> ScheduledCore { @@ -473,18 +470,12 @@ fn sends_distribute_collation_message(#[case] runtime_version: u32) { let expect_validation_data_hash = test_validation_data().hash(); let expect_relay_parent = Hash::repeat_byte(4); let expect_validation_code_hash = ValidationCode(vec![1, 2, 3]).hash(); - let expect_payload = collator_signature_payload( - &expect_relay_parent, - &config.para_id, - &expect_validation_data_hash, - &expect_pov_hash, - &expect_validation_code_hash, - ); + let expect_descriptor = CandidateDescriptor { - signature: config.key.sign(&expect_payload), + signature: CollatorSignature::from_slice(&vec![0u8; 64]).expect("64 bytes; qed"), para_id: config.para_id, relay_parent: expect_relay_parent, - collator: config.key.public(), + collator: CollatorId::from_slice(&vec![0u8; 32]).expect("32 bytes; qed"), persisted_validation_data_hash: expect_validation_data_hash, pov_hash: expect_pov_hash, erasure_root: dummy_hash(), // this isn't something we're checking right now @@ -499,22 +490,7 @@ fn sends_distribute_collation_message(#[case] runtime_version: u32) { .. }) => { 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 - // and check the rest of the fields for equality. - assert!(CollatorPair::verify( - &descriptor.signature, - &collator_signature_payload( - &descriptor.relay_parent, - &descriptor.para_id, - &descriptor.persisted_validation_data_hash, - &descriptor.pov_hash, - &descriptor.validation_code_hash, - ) - .as_ref(), - &descriptor.collator, - )); + let expect_descriptor = { let mut expect_descriptor = expect_descriptor; expect_descriptor.signature = descriptor.signature.clone(); diff --git a/polkadot/node/core/candidate-validation/src/lib.rs b/polkadot/node/core/candidate-validation/src/lib.rs index 08881dad1961..bc4fc9e7328d 100644 --- a/polkadot/node/core/candidate-validation/src/lib.rs +++ b/polkadot/node/core/candidate-validation/src/lib.rs @@ -958,10 +958,6 @@ fn perform_basic_checks( return Err(InvalidCandidate::CodeHashMismatch) } - if let Err(()) = candidate.check_collator_signature() { - return Err(InvalidCandidate::BadSignature) - } - Ok(()) } diff --git a/polkadot/node/core/prospective-parachains/src/fragment_chain/mod.rs b/polkadot/node/core/prospective-parachains/src/fragment_chain/mod.rs index f87d4820ff9a..b5fe70e76923 100644 --- a/polkadot/node/core/prospective-parachains/src/fragment_chain/mod.rs +++ b/polkadot/node/core/prospective-parachains/src/fragment_chain/mod.rs @@ -160,8 +160,6 @@ impl CandidateStorage { state, candidate: Arc::new(ProspectiveCandidate { commitments: candidate.commitments, - collator: candidate.descriptor.collator, - collator_signature: candidate.descriptor.signature, persisted_validation_data, pov_hash: candidate.descriptor.pov_hash, validation_code_hash: candidate.descriptor.validation_code_hash, diff --git a/polkadot/node/primitives/src/lib.rs b/polkadot/node/primitives/src/lib.rs index 5f007bc8d67d..6814ba827ffc 100644 --- a/polkadot/node/primitives/src/lib.rs +++ b/polkadot/node/primitives/src/lib.rs @@ -30,7 +30,7 @@ use parity_scale_codec::{Decode, Encode, Error as CodecError, Input}; use serde::{de, Deserialize, Deserializer, Serialize, Serializer}; use polkadot_primitives::{ - BlakeTwo256, BlockNumber, CandidateCommitments, CandidateHash, ChunkIndex, CollatorPair, + BlakeTwo256, BlockNumber, CandidateCommitments, CandidateHash, ChunkIndex, CommittedCandidateReceipt, CompactStatement, CoreIndex, EncodeAs, Hash, HashT, HeadData, Id as ParaId, PersistedValidationData, SessionIndex, Signed, UncheckedSigned, ValidationCode, ValidationCodeHash, MAX_CODE_SIZE, MAX_POV_SIZE, @@ -490,8 +490,6 @@ pub type CollatorFn = Box< /// Configuration for the collation generator #[cfg(not(target_os = "unknown"))] pub struct CollationGenerationConfig { - /// Collator's authentication key, so it can sign things. - pub key: CollatorPair, /// Collation function. See [`CollatorFn`] for more details. /// /// If this is `None`, it implies that collations are intended to be submitted diff --git a/polkadot/node/subsystem-util/src/inclusion_emulator/mod.rs b/polkadot/node/subsystem-util/src/inclusion_emulator/mod.rs index b5aef325c8b4..6fe8dc94ddaf 100644 --- a/polkadot/node/subsystem-util/src/inclusion_emulator/mod.rs +++ b/polkadot/node/subsystem-util/src/inclusion_emulator/mod.rs @@ -117,9 +117,8 @@ /// That means a few blocks of execution time lost, which is not a big deal for code upgrades /// in practice at most once every few weeks. use polkadot_primitives::{ - async_backing::Constraints as PrimitiveConstraints, BlockNumber, CandidateCommitments, - CollatorId, CollatorSignature, Hash, HeadData, Id as ParaId, PersistedValidationData, - UpgradeRestriction, ValidationCodeHash, + async_backing::Constraints as PrimitiveConstraints, BlockNumber, CandidateCommitments, Hash, + HeadData, Id as ParaId, PersistedValidationData, UpgradeRestriction, ValidationCodeHash, }; use std::{collections::HashMap, sync::Arc}; @@ -529,10 +528,6 @@ impl ConstraintModifications { pub struct ProspectiveCandidate { /// The commitments to the output of the execution. pub commitments: CandidateCommitments, - /// The collator that created the candidate. - pub collator: CollatorId, - /// The signature of the collator on the payload. - pub collator_signature: CollatorSignature, /// The persisted validation data used to create the candidate. pub persisted_validation_data: PersistedValidationData, /// The hash of the PoV. @@ -1177,7 +1172,6 @@ mod tests { hrmp_watermark: relay_parent.number, }, collator, - collator_signature: sig, persisted_validation_data: PersistedValidationData { parent_head: constraints.required_parent.clone(), relay_parent_number: relay_parent.number, diff --git a/polkadot/primitives/test-helpers/src/lib.rs b/polkadot/primitives/test-helpers/src/lib.rs index d43cf3317e57..a7afffba1a30 100644 --- a/polkadot/primitives/test-helpers/src/lib.rs +++ b/polkadot/primitives/test-helpers/src/lib.rs @@ -28,7 +28,7 @@ use polkadot_primitives::{ ValidationCode, ValidationCodeHash, ValidatorId, }; pub use rand; -use sp_application_crypto::sr25519; +use sp_application_crypto::{sr25519, ByteArray}; use sp_keyring::Sr25519Keyring; use sp_runtime::generic::Digest; @@ -136,7 +136,7 @@ pub fn dummy_head_data() -> HeadData { /// Create a meaningless collator id. pub fn dummy_collator() -> CollatorId { - CollatorId::from(sr25519::Public::default()) + CollatorId::from_slice(&vec![0u8; 32]).expect("32 bytes; qed") } /// Create a meaningless validator id. @@ -146,7 +146,7 @@ pub fn dummy_validator() -> ValidatorId { /// Create a meaningless collator signature. pub fn dummy_collator_signature() -> CollatorSignature { - CollatorSignature::from(sr25519::Signature::default()) + CollatorSignature::from_slice(&vec![0u8; 64]).expect("64 bytes; qed") } /// Create a meaningless persisted validation data. @@ -228,7 +228,6 @@ pub fn make_valid_candidate_descriptor>( validation_code_hash, }; - assert!(descriptor.check_collator_signature().is_ok()); descriptor } diff --git a/polkadot/runtime/parachains/src/builder.rs b/polkadot/runtime/parachains/src/builder.rs index d1e2bc392feb..2fbed927e676 100644 --- a/polkadot/runtime/parachains/src/builder.rs +++ b/polkadot/runtime/parachains/src/builder.rs @@ -25,13 +25,12 @@ use bitvec::{order::Lsb0 as BitOrderLsb0, vec::BitVec}; use frame_support::pallet_prelude::*; use frame_system::pallet_prelude::*; use primitives::{ - collator_signature_payload, node_features::FeatureIndex, AvailabilityBitfield, BackedCandidate, - CandidateCommitments, CandidateDescriptor, CandidateHash, CollatorId, CollatorSignature, - CommittedCandidateReceipt, CompactStatement, CoreIndex, DisputeStatement, DisputeStatementSet, - GroupIndex, HeadData, Id as ParaId, IndexedVec, InherentData as ParachainsInherentData, - InvalidDisputeStatementKind, PersistedValidationData, SessionIndex, SigningContext, - UncheckedSigned, ValidDisputeStatementKind, ValidationCode, ValidatorId, ValidatorIndex, - ValidityAttestation, + node_features::FeatureIndex, AvailabilityBitfield, BackedCandidate, CandidateCommitments, + CandidateDescriptor, CandidateHash, CollatorId, CollatorSignature, CommittedCandidateReceipt, + CompactStatement, CoreIndex, DisputeStatement, DisputeStatementSet, GroupIndex, HeadData, + Id as ParaId, IndexedVec, InherentData as ParachainsInherentData, InvalidDisputeStatementKind, + PersistedValidationData, SessionIndex, SigningContext, UncheckedSigned, + ValidDisputeStatementKind, ValidationCode, ValidatorId, ValidatorIndex, ValidityAttestation, }; use sp_core::{sr25519, H256}; use sp_runtime::{ @@ -45,6 +44,16 @@ use sp_std::{ vec, }; +/// Create a null collator id. +pub fn dummy_collator() -> CollatorId { + CollatorId::from_slice(&vec![0u8; 32]).expect("32 bytes; qed") +} + +/// Create a null collator signature. +pub fn dummy_collator_signature() -> CollatorSignature { + CollatorSignature::from_slice(&vec![0u8; 64]).expect("64 bytes; qed") +} + fn mock_validation_code() -> ValidationCode { ValidationCode(vec![1, 2, 3]) } @@ -571,7 +580,6 @@ impl BenchBuilder { // This generates a pair and adds it to the keystore, returning just the // public. - let collator_public = CollatorId::generate_pair(None); let header = Self::header(self.block_number); let relay_parent = header.hash(); @@ -601,14 +609,6 @@ impl BenchBuilder { let pov_hash = Default::default(); let validation_code_hash = mock_validation_code().hash(); - let payload = collator_signature_payload( - &relay_parent, - ¶_id, - &persisted_validation_data_hash, - &pov_hash, - &validation_code_hash, - ); - let signature = collator_public.sign(&payload).unwrap(); let mut past_code_meta = paras::ParaPastCodeMeta::>::default(); @@ -621,11 +621,11 @@ impl BenchBuilder { descriptor: CandidateDescriptor:: { para_id, relay_parent, - collator: collator_public, + collator: dummy_collator(), persisted_validation_data_hash, pov_hash, erasure_root: Default::default(), - signature, + signature: dummy_collator_signature(), para_head: head_data.hash(), validation_code_hash, }, diff --git a/polkadot/runtime/parachains/src/inclusion/mod.rs b/polkadot/runtime/parachains/src/inclusion/mod.rs index 0c7274984085..95224ecc02bd 100644 --- a/polkadot/runtime/parachains/src/inclusion/mod.rs +++ b/polkadot/runtime/parachains/src/inclusion/mod.rs @@ -337,8 +337,6 @@ pub mod pallet { InsufficientBacking, /// Invalid (bad signature, unknown validator, etc.) backing. InvalidBacking, - /// Collator did not sign PoV. - NotCollatorSigned, /// The validation data hash does not match expected. ValidationDataHashMismatch, /// The downward message queue is not processed correctly. @@ -1258,11 +1256,6 @@ impl CandidateCheckContext { ); } - ensure!( - backed_candidate_receipt.descriptor().check_collator_signature().is_ok(), - Error::::NotCollatorSigned, - ); - let validation_code_hash = paras::CurrentCodeHash::::get(para_id) // A candidate for a parachain without current validation code is not scheduled. .ok_or_else(|| Error::::UnscheduledCandidate)?; From 3f529d24c361e19d383335eb40bd9b0a97538be4 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Fri, 31 May 2024 21:34:55 +0300 Subject: [PATCH 02/13] use dummy Signed-off-by: Andrei Sandu --- polkadot/node/collation-generation/src/tests.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/polkadot/node/collation-generation/src/tests.rs b/polkadot/node/collation-generation/src/tests.rs index 6119136de02a..c161fd46e084 100644 --- a/polkadot/node/collation-generation/src/tests.rs +++ b/polkadot/node/collation-generation/src/tests.rs @@ -472,10 +472,10 @@ fn sends_distribute_collation_message(#[case] runtime_version: u32) { let expect_validation_code_hash = ValidationCode(vec![1, 2, 3]).hash(); let expect_descriptor = CandidateDescriptor { - signature: CollatorSignature::from_slice(&vec![0u8; 64]).expect("64 bytes; qed"), + signature: dummy_collator_signature(), para_id: config.para_id, relay_parent: expect_relay_parent, - collator: CollatorId::from_slice(&vec![0u8; 32]).expect("32 bytes; qed"), + collator: dummy_collator(), persisted_validation_data_hash: expect_validation_data_hash, pov_hash: expect_pov_hash, erasure_root: dummy_hash(), // this isn't something we're checking right now From 5a71f42e4e8f71a1c76c2588da62bc9f104e5133 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Thu, 18 Jul 2024 02:22:21 +0300 Subject: [PATCH 03/13] undo Signed-off-by: Andrei Sandu --- cumulus/client/collator/src/lib.rs | 4 +- .../consensus/aura/src/collators/lookahead.rs | 5 ++- cumulus/polkadot-parachain/src/service.rs | 2 + polkadot/node/collation-generation/src/lib.rs | 31 ++++++++++---- .../node/collation-generation/src/tests.rs | 42 +++++++++++++++---- polkadot/node/primitives/src/lib.rs | 4 +- 6 files changed, 69 insertions(+), 19 deletions(-) diff --git a/cumulus/client/collator/src/lib.rs b/cumulus/client/collator/src/lib.rs index b3804800e35f..47da0f6d96f2 100644 --- a/cumulus/client/collator/src/lib.rs +++ b/cumulus/client/collator/src/lib.rs @@ -199,6 +199,7 @@ pub mod relay_chain_driven { let (stream_tx, stream_rx) = mpsc::channel(0); let config = CollationGenerationConfig { + key, para_id, collator: Some(Box::new(move |relay_parent, validation_data| { // Cloning the channel on each usage effectively makes the channel @@ -239,10 +240,11 @@ pub mod relay_chain_driven { /// For callback-driven collators, use the [`relay_chain_driven`] module. pub async fn initialize_collator_subsystems( overseer_handle: &mut OverseerHandle, + key: CollatorPair, para_id: ParaId, reinitialize: bool, ) { - let config = CollationGenerationConfig { para_id, collator: None }; + 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 27a1ae03d47e..09416233ea9b 100644 --- a/cumulus/client/consensus/aura/src/collators/lookahead.rs +++ b/cumulus/client/consensus/aura/src/collators/lookahead.rs @@ -50,7 +50,7 @@ use polkadot_node_subsystem::messages::{ }; use polkadot_overseer::Handle as OverseerHandle; use polkadot_primitives::{ - AsyncBackingParams, CoreIndex, CoreState, Id as ParaId, OccupiedCoreAssumption, + AsyncBackingParams, CollatorPair, CoreIndex, CoreState, Id as ParaId, OccupiedCoreAssumption, }; use futures::{channel::oneshot, prelude::*}; @@ -91,6 +91,8 @@ pub struct Params { pub sync_oracle: SO, /// The underlying keystore, which should contain Aura consensus keys. pub keystore: KeystorePtr, + /// The collator key used to sign collations before submitting to validators. + pub collator_key: CollatorPair, /// The para's ID. pub para_id: ParaId, /// A handle to the relay-chain client's "Overseer" or task orchestrator. @@ -147,6 +149,7 @@ where async move { cumulus_client_collator::initialize_collator_subsystems( &mut params.overseer_handle, + params.collator_key, params.para_id, params.reinitialize, ) diff --git a/cumulus/polkadot-parachain/src/service.rs b/cumulus/polkadot-parachain/src/service.rs index 432a0a355aad..12eda3e8a9cb 100644 --- a/cumulus/polkadot-parachain/src/service.rs +++ b/cumulus/polkadot-parachain/src/service.rs @@ -832,6 +832,7 @@ where }, sync_oracle, keystore, + collator_key, para_id, overseer_handle, relay_chain_slot_duration, @@ -972,6 +973,7 @@ fn start_lookahead_aura_consensus( }, sync_oracle, keystore, + collator_key, para_id, overseer_handle, relay_chain_slot_duration, diff --git a/polkadot/node/collation-generation/src/lib.rs b/polkadot/node/collation-generation/src/lib.rs index f47f0f488068..374f090a2671 100644 --- a/polkadot/node/collation-generation/src/lib.rs +++ b/polkadot/node/collation-generation/src/lib.rs @@ -48,11 +48,11 @@ use polkadot_node_subsystem_util::{ request_validators, vstaging::fetch_claim_queue, }; use polkadot_primitives::{ - CandidateCommitments, CandidateDescriptor, CandidateReceipt, CollatorId, CollatorSignature, - CoreIndex, CoreState, Hash, Id as ParaId, OccupiedCoreAssumption, PersistedValidationData, - ScheduledCore, ValidationCodeHash, + collator_signature_payload, CandidateCommitments, CandidateDescriptor, CandidateReceipt, + CollatorPair, CoreIndex, CoreState, Hash, Id as ParaId, OccupiedCoreAssumption, + PersistedValidationData, ScheduledCore, ValidationCodeHash, }; -use sp_core::ByteArray; +use sp_core::crypto::Pair; use std::sync::Arc; mod error; @@ -424,6 +424,7 @@ async fn handle_new_activations( n_validators, core_index, }, + task_config.key.clone(), &mut task_sender, result_sender, &metrics, @@ -496,7 +497,14 @@ async fn handle_submit_collation( core_index, }; - construct_and_distribute_receipt(collation, ctx.sender(), result_sender, metrics).await; + construct_and_distribute_receipt( + collation, + config.key.clone(), + ctx.sender(), + result_sender, + metrics, + ) + .await; Ok(()) } @@ -515,6 +523,7 @@ struct PreparedCollation { /// which is distributed to validators. async fn construct_and_distribute_receipt( collation: PreparedCollation, + key: CollatorPair, sender: &mut impl overseer::CollationGenerationSenderTrait, result_sender: Option>, metrics: &Metrics, @@ -560,6 +569,14 @@ async fn construct_and_distribute_receipt( let pov_hash = pov.hash(); + let signature_payload = collator_signature_payload( + &relay_parent, + ¶_id, + &persisted_validation_data_hash, + &pov_hash, + &validation_code_hash, + ); + let erasure_root = match erasure_root(n_validators, validation_data, pov.clone()) { Ok(erasure_root) => erasure_root, Err(err) => { @@ -585,10 +602,10 @@ async fn construct_and_distribute_receipt( let ccr = CandidateReceipt { commitments_hash: commitments.hash(), descriptor: CandidateDescriptor { - signature: CollatorSignature::from_slice(&vec![0u8; 64]).expect("64 bytes; qed"), + signature: key.sign(&signature_payload), para_id, relay_parent, - collator: CollatorId::from_slice(&vec![0u8; 32]).expect("32 bytes; qed"), + collator: key.public(), persisted_validation_data_hash, pov_hash, erasure_root, diff --git a/polkadot/node/collation-generation/src/tests.rs b/polkadot/node/collation-generation/src/tests.rs index c161fd46e084..10c391cba25d 100644 --- a/polkadot/node/collation-generation/src/tests.rs +++ b/polkadot/node/collation-generation/src/tests.rs @@ -31,8 +31,8 @@ use polkadot_node_subsystem_test_helpers::{subsystem_test_harness, TestSubsystem use polkadot_node_subsystem_util::TimeoutExt; use polkadot_primitives::{ async_backing::{BackingState, CandidatePendingAvailability}, - AsyncBackingParams, BlockNumber, CollatorId, CollatorSignature, HeadData, - PersistedValidationData, ScheduledCore, ValidationCode, + AsyncBackingParams, BlockNumber, CollatorPair, HeadData, PersistedValidationData, + ScheduledCore, ValidationCode, }; use rstest::rstest; use sp_keyring::sr25519::Keyring as Sr25519Keyring; @@ -40,8 +40,6 @@ use std::{ collections::{BTreeMap, VecDeque}, pin::Pin, }; - -use sp_core::ByteArray; use test_helpers::{ dummy_candidate_descriptor, dummy_hash, dummy_head_data, dummy_validator, make_candidate, }; @@ -125,13 +123,18 @@ async fn overseer_recv(overseer: &mut VirtualOverseer) -> AllMessages { fn test_config>(para_id: Id) -> CollationGenerationConfig { CollationGenerationConfig { + key: CollatorPair::generate().0, collator: Some(Box::new(|_: Hash, _vd: &PersistedValidationData| TestCollator.boxed())), para_id: para_id.into(), } } fn test_config_no_collator>(para_id: Id) -> CollationGenerationConfig { - CollationGenerationConfig { collator: None, para_id: para_id.into() } + CollationGenerationConfig { + key: CollatorPair::generate().0, + collator: None, + para_id: para_id.into(), + } } fn scheduled_core_for>(para_id: Id) -> ScheduledCore { @@ -470,12 +473,18 @@ fn sends_distribute_collation_message(#[case] runtime_version: u32) { let expect_validation_data_hash = test_validation_data().hash(); let expect_relay_parent = Hash::repeat_byte(4); let expect_validation_code_hash = ValidationCode(vec![1, 2, 3]).hash(); - + let expect_payload = collator_signature_payload( + &expect_relay_parent, + &config.para_id, + &expect_validation_data_hash, + &expect_pov_hash, + &expect_validation_code_hash, + ); let expect_descriptor = CandidateDescriptor { - signature: dummy_collator_signature(), + signature: config.key.sign(&expect_payload), para_id: config.para_id, relay_parent: expect_relay_parent, - collator: dummy_collator(), + collator: config.key.public(), persisted_validation_data_hash: expect_validation_data_hash, pov_hash: expect_pov_hash, erasure_root: dummy_hash(), // this isn't something we're checking right now @@ -490,7 +499,22 @@ fn sends_distribute_collation_message(#[case] runtime_version: u32) { .. }) => { 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 + // and check the rest of the fields for equality. + assert!(CollatorPair::verify( + &descriptor.signature, + &collator_signature_payload( + &descriptor.relay_parent, + &descriptor.para_id, + &descriptor.persisted_validation_data_hash, + &descriptor.pov_hash, + &descriptor.validation_code_hash, + ) + .as_ref(), + &descriptor.collator, + )); let expect_descriptor = { let mut expect_descriptor = expect_descriptor; expect_descriptor.signature = descriptor.signature.clone(); diff --git a/polkadot/node/primitives/src/lib.rs b/polkadot/node/primitives/src/lib.rs index 6814ba827ffc..5f007bc8d67d 100644 --- a/polkadot/node/primitives/src/lib.rs +++ b/polkadot/node/primitives/src/lib.rs @@ -30,7 +30,7 @@ use parity_scale_codec::{Decode, Encode, Error as CodecError, Input}; use serde::{de, Deserialize, Deserializer, Serialize, Serializer}; use polkadot_primitives::{ - BlakeTwo256, BlockNumber, CandidateCommitments, CandidateHash, ChunkIndex, + BlakeTwo256, BlockNumber, CandidateCommitments, CandidateHash, ChunkIndex, CollatorPair, CommittedCandidateReceipt, CompactStatement, CoreIndex, EncodeAs, Hash, HashT, HeadData, Id as ParaId, PersistedValidationData, SessionIndex, Signed, UncheckedSigned, ValidationCode, ValidationCodeHash, MAX_CODE_SIZE, MAX_POV_SIZE, @@ -490,6 +490,8 @@ pub type CollatorFn = Box< /// Configuration for the collation generator #[cfg(not(target_os = "unknown"))] pub struct CollationGenerationConfig { + /// Collator's authentication key, so it can sign things. + pub key: CollatorPair, /// Collation function. See [`CollatorFn`] for more details. /// /// If this is `None`, it implies that collations are intended to be submitted From 3d3d850bbfa0518ce15ba1ebdc18cacb07c8516a Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Thu, 18 Jul 2024 12:20:11 +0300 Subject: [PATCH 04/13] fix comment Signed-off-by: Andrei Sandu --- polkadot/runtime/parachains/src/inclusion/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/polkadot/runtime/parachains/src/inclusion/mod.rs b/polkadot/runtime/parachains/src/inclusion/mod.rs index 95224ecc02bd..7e8ed6d73138 100644 --- a/polkadot/runtime/parachains/src/inclusion/mod.rs +++ b/polkadot/runtime/parachains/src/inclusion/mod.rs @@ -1219,7 +1219,6 @@ impl CandidateCheckContext { /// /// Assures: /// * relay-parent in-bounds - /// * collator signature check passes /// * code hash of commitments matches current code hash /// * para head in the descriptor and commitments match /// From d507439e285da09d3994876b9436e5d3953a6c20 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Tue, 23 Jul 2024 14:40:08 +0300 Subject: [PATCH 05/13] add back node side sig check until runtime is upgraded Signed-off-by: Andrei Sandu --- polkadot/node/core/candidate-validation/src/lib.rs | 4 ++++ polkadot/primitives/test-helpers/src/lib.rs | 7 ++++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/polkadot/node/core/candidate-validation/src/lib.rs b/polkadot/node/core/candidate-validation/src/lib.rs index acf986a65e89..76619bd391f2 100644 --- a/polkadot/node/core/candidate-validation/src/lib.rs +++ b/polkadot/node/core/candidate-validation/src/lib.rs @@ -958,6 +958,10 @@ fn perform_basic_checks( return Err(InvalidCandidate::CodeHashMismatch) } + if let Err(()) = candidate.check_collator_signature() { + return Err(InvalidCandidate::BadSignature) + } + Ok(()) } diff --git a/polkadot/primitives/test-helpers/src/lib.rs b/polkadot/primitives/test-helpers/src/lib.rs index a7afffba1a30..d43cf3317e57 100644 --- a/polkadot/primitives/test-helpers/src/lib.rs +++ b/polkadot/primitives/test-helpers/src/lib.rs @@ -28,7 +28,7 @@ use polkadot_primitives::{ ValidationCode, ValidationCodeHash, ValidatorId, }; pub use rand; -use sp_application_crypto::{sr25519, ByteArray}; +use sp_application_crypto::sr25519; use sp_keyring::Sr25519Keyring; use sp_runtime::generic::Digest; @@ -136,7 +136,7 @@ pub fn dummy_head_data() -> HeadData { /// Create a meaningless collator id. pub fn dummy_collator() -> CollatorId { - CollatorId::from_slice(&vec![0u8; 32]).expect("32 bytes; qed") + CollatorId::from(sr25519::Public::default()) } /// Create a meaningless validator id. @@ -146,7 +146,7 @@ pub fn dummy_validator() -> ValidatorId { /// Create a meaningless collator signature. pub fn dummy_collator_signature() -> CollatorSignature { - CollatorSignature::from_slice(&vec![0u8; 64]).expect("64 bytes; qed") + CollatorSignature::from(sr25519::Signature::default()) } /// Create a meaningless persisted validation data. @@ -228,6 +228,7 @@ pub fn make_valid_candidate_descriptor>( validation_code_hash, }; + assert!(descriptor.check_collator_signature().is_ok()); descriptor } From 056bb8f5de18c2699d8b6d730e313f0ad4a93bf5 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Tue, 23 Jul 2024 14:52:06 +0300 Subject: [PATCH 06/13] add PRDoc and fix comment Signed-off-by: Andrei Sandu --- .../subsystem-util/src/inclusion_emulator/mod.rs | 5 ++--- prdoc/pr_4665.prdoc | 16 ++++++++++++++++ 2 files changed, 18 insertions(+), 3 deletions(-) create mode 100644 prdoc/pr_4665.prdoc diff --git a/polkadot/node/subsystem-util/src/inclusion_emulator/mod.rs b/polkadot/node/subsystem-util/src/inclusion_emulator/mod.rs index 6fe8dc94ddaf..571196fb87ee 100644 --- a/polkadot/node/subsystem-util/src/inclusion_emulator/mod.rs +++ b/polkadot/node/subsystem-util/src/inclusion_emulator/mod.rs @@ -520,9 +520,8 @@ impl ConstraintModifications { /// The prospective candidate. /// /// This comprises the key information that represent a candidate -/// without pinning it to a particular session. For example, everything -/// to do with the collator's signature and commitments are represented -/// here. But the erasure-root is not. This means that prospective candidates +/// without pinning it to a particular session. For example commitments are +/// represented here. But the erasure-root is not. This means that prospective candidates /// are not correlated to any session in particular. #[derive(Debug, Clone, PartialEq)] pub struct ProspectiveCandidate { diff --git a/prdoc/pr_4665.prdoc b/prdoc/pr_4665.prdoc new file mode 100644 index 000000000000..db1e8458bec9 --- /dev/null +++ b/prdoc/pr_4665.prdoc @@ -0,0 +1,16 @@ +# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0 +# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json + +title: "Remove runtime collator signature checks" + +doc: + - audience: [Runtime Dev, Node Dev] + description: | + Removes runtime collator signature checks, but these are still being done on the node. Remove collator + and signature from the `ProspectiveCandidate` definition in the inclusion emulator. + +crates: +- name: polkadot-node-subsystem-util + bump: minor +- name: polkadot-runtime-parachains + bump: patch From bb5f7d06fbe35081abaeae70ef42d10f3639c1f4 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Tue, 23 Jul 2024 14:55:31 +0300 Subject: [PATCH 07/13] fix clippy Signed-off-by: Andrei Sandu --- polkadot/runtime/parachains/src/builder.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/polkadot/runtime/parachains/src/builder.rs b/polkadot/runtime/parachains/src/builder.rs index c481be3e35a4..21ba265e1d4e 100644 --- a/polkadot/runtime/parachains/src/builder.rs +++ b/polkadot/runtime/parachains/src/builder.rs @@ -29,7 +29,7 @@ use alloc::{ use bitvec::{order::Lsb0 as BitOrderLsb0, vec::BitVec}; use frame_support::pallet_prelude::*; use frame_system::pallet_prelude::*; -use primitives::{ +use polkadot_primitives::{ node_features::FeatureIndex, AvailabilityBitfield, BackedCandidate, CandidateCommitments, CandidateDescriptor, CandidateHash, CollatorId, CollatorSignature, CommittedCandidateReceipt, CompactStatement, CoreIndex, DisputeStatement, DisputeStatementSet, GroupIndex, HeadData, From e911c4482c0def3d5b72b140260f2e46ec1fc48b Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Tue, 23 Jul 2024 15:39:05 +0300 Subject: [PATCH 08/13] fix tests Signed-off-by: Andrei Sandu --- polkadot/runtime/parachains/src/builder.rs | 2 +- .../runtime/parachains/src/inclusion/tests.rs | 49 ++----------------- 2 files changed, 4 insertions(+), 47 deletions(-) diff --git a/polkadot/runtime/parachains/src/builder.rs b/polkadot/runtime/parachains/src/builder.rs index 21ba265e1d4e..b2a67ee8dd24 100644 --- a/polkadot/runtime/parachains/src/builder.rs +++ b/polkadot/runtime/parachains/src/builder.rs @@ -37,7 +37,7 @@ use polkadot_primitives::{ PersistedValidationData, SessionIndex, SigningContext, UncheckedSigned, ValidDisputeStatementKind, ValidationCode, ValidatorId, ValidatorIndex, ValidityAttestation, }; -use sp_core::{sr25519, H256}; +use sp_core::{sr25519, ByteArray, H256}; use sp_runtime::{ generic::Digest, traits::{Header as HeaderT, One, TrailingZeroInput, Zero}, diff --git a/polkadot/runtime/parachains/src/inclusion/tests.rs b/polkadot/runtime/parachains/src/inclusion/tests.rs index 18def664f4b2..1b83c956b141 100644 --- a/polkadot/runtime/parachains/src/inclusion/tests.rs +++ b/polkadot/runtime/parachains/src/inclusion/tests.rs @@ -34,9 +34,9 @@ use assert_matches::assert_matches; use codec::DecodeAll; use frame_support::assert_noop; use polkadot_primitives::{ - BlockNumber, CandidateCommitments, CandidateDescriptor, CollatorId, - CompactStatement as Statement, Hash, SignedAvailabilityBitfield, SignedStatement, - ValidationCode, ValidatorId, ValidityAttestation, PARACHAIN_KEY_TYPE_ID, + BlockNumber, CandidateCommitments, CandidateDescriptor, CompactStatement as Statement, Hash, + SignedAvailabilityBitfield, SignedStatement, ValidationCode, ValidatorId, ValidityAttestation, + PARACHAIN_KEY_TYPE_ID, }; use polkadot_primitives_test_helpers::{ dummy_collator, dummy_collator_signature, dummy_validation_code, @@ -1223,11 +1223,9 @@ fn candidate_checks() { ]; Scheduler::set_validator_groups(validator_groups); - let thread_collator: CollatorId = Sr25519Keyring::Two.public().into(); let chain_a_assignment = (chain_a, CoreIndex::from(0)); let chain_b_assignment = (chain_b, CoreIndex::from(1)); - let thread_a_assignment = (thread_a, CoreIndex::from(2)); let allowed_relay_parents = default_allowed_relay_parent_tracker(); // no candidates. @@ -1517,47 +1515,6 @@ fn candidate_checks() { ); } - // candidate not well-signed by collator. - { - let mut candidate = TestCandidateBuilder { - para_id: thread_a, - relay_parent: System::parent_hash(), - pov_hash: Hash::repeat_byte(1), - persisted_validation_data_hash: make_vdata_hash(thread_a).unwrap(), - hrmp_watermark: RELAY_PARENT_NUM, - ..Default::default() - } - .build(); - - assert_eq!(CollatorId::from(Sr25519Keyring::Two.public()), thread_collator); - collator_sign_candidate(Sr25519Keyring::Two, &mut candidate); - - // change the candidate after signing. - candidate.descriptor.pov_hash = Hash::repeat_byte(2); - - let backed = back_candidate( - candidate, - &validators, - group_validators(GroupIndex::from(2)).unwrap().as_ref(), - &keystore, - &signing_context, - BackingKind::Threshold, - None, - ); - - assert_noop!( - ParaInclusion::process_candidates( - &allowed_relay_parents, - &vec![(thread_a_assignment.0, vec![(backed, thread_a_assignment.1)])] - .into_iter() - .collect(), - &group_validators, - false - ), - Error::::NotCollatorSigned - ); - } - // interfering code upgrade - reject { let mut candidate = TestCandidateBuilder { From 644b235987dd3e7892df43ef3160e58ad657de87 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Tue, 23 Jul 2024 15:42:04 +0300 Subject: [PATCH 09/13] update prdoc Signed-off-by: Andrei Sandu --- prdoc/pr_4665.prdoc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/prdoc/pr_4665.prdoc b/prdoc/pr_4665.prdoc index db1e8458bec9..43ca0762fd1e 100644 --- a/prdoc/pr_4665.prdoc +++ b/prdoc/pr_4665.prdoc @@ -12,5 +12,7 @@ doc: crates: - name: polkadot-node-subsystem-util bump: minor +- name: polkadot-node-core-prospective-parachains + bump: patch - name: polkadot-runtime-parachains bump: patch From eab5ce96dc602ab404e8e7cbc641972764e96d0a Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Wed, 24 Jul 2024 16:29:35 +0300 Subject: [PATCH 10/13] add test Signed-off-by: Andrei Sandu --- .../runtime/parachains/src/inclusion/tests.rs | 85 ++++++++++++++++++- 1 file changed, 82 insertions(+), 3 deletions(-) diff --git a/polkadot/runtime/parachains/src/inclusion/tests.rs b/polkadot/runtime/parachains/src/inclusion/tests.rs index 1b83c956b141..3ead456cde5a 100644 --- a/polkadot/runtime/parachains/src/inclusion/tests.rs +++ b/polkadot/runtime/parachains/src/inclusion/tests.rs @@ -34,9 +34,9 @@ use assert_matches::assert_matches; use codec::DecodeAll; use frame_support::assert_noop; use polkadot_primitives::{ - BlockNumber, CandidateCommitments, CandidateDescriptor, CompactStatement as Statement, Hash, - SignedAvailabilityBitfield, SignedStatement, ValidationCode, ValidatorId, ValidityAttestation, - PARACHAIN_KEY_TYPE_ID, + BlockNumber, CandidateCommitments, CandidateDescriptor, CollatorId, + CompactStatement as Statement, Hash, SignedAvailabilityBitfield, SignedStatement, + ValidationCode, ValidatorId, ValidityAttestation, PARACHAIN_KEY_TYPE_ID, }; use polkadot_primitives_test_helpers::{ dummy_collator, dummy_collator_signature, dummy_validation_code, @@ -1223,9 +1223,11 @@ fn candidate_checks() { ]; Scheduler::set_validator_groups(validator_groups); + let thread_collator: CollatorId = Sr25519Keyring::Two.public().into(); let chain_a_assignment = (chain_a, CoreIndex::from(0)); let chain_b_assignment = (chain_b, CoreIndex::from(1)); + let thread_a_assignment = (thread_a, CoreIndex::from(2)); let allowed_relay_parents = default_allowed_relay_parent_tracker(); // no candidates. @@ -1515,6 +1517,83 @@ fn candidate_checks() { ); } + // candidate not well-signed by collator. + { + let mut candidate = TestCandidateBuilder { + para_id: thread_a, + relay_parent: System::parent_hash(), + pov_hash: Hash::repeat_byte(1), + persisted_validation_data_hash: make_vdata_hash(thread_a).unwrap(), + hrmp_watermark: RELAY_PARENT_NUM, + ..Default::default() + } + .build(); + + assert_eq!(CollatorId::from(Sr25519Keyring::Two.public()), thread_collator); + collator_sign_candidate(Sr25519Keyring::Two, &mut candidate); + + // change the candidate after signing. + candidate.descriptor.pov_hash = Hash::repeat_byte(2); + + let backed = back_candidate( + candidate, + &validators, + group_validators(GroupIndex::from(2)).unwrap().as_ref(), + &keystore, + &signing_context, + BackingKind::Threshold, + None, + ); + + let ProcessedCandidates { + core_indices: occupied_cores, + candidate_receipt_with_backing_validator_indices, + } = ParaInclusion::process_candidates( + &allowed_relay_parents, + &vec![(thread_a_assignment.0, vec![(backed.clone(), thread_a_assignment.1)])] + .into_iter() + .collect(), + &group_validators, + false, + ) + .expect("candidate is accepted with bad collator signature"); + + assert_eq!(occupied_cores, vec![(CoreIndex::from(2), thread_a)]); + + let mut expected = std::collections::HashMap::< + CandidateHash, + (CandidateReceipt, Vec<(ValidatorIndex, ValidityAttestation)>), + >::new(); + let backed_candidate = backed; + let candidate_receipt_with_backers = expected + .entry(backed_candidate.hash()) + .or_insert_with(|| (backed_candidate.receipt(), Vec::new())); + let (validator_indices, _maybe_core_index) = + backed_candidate.validator_indices_and_core_index(true); + assert_eq!(backed_candidate.validity_votes().len(), validator_indices.count_ones()); + candidate_receipt_with_backers.1.extend( + validator_indices + .iter() + .enumerate() + .filter(|(_, signed)| **signed) + .zip(backed_candidate.validity_votes().iter().cloned()) + .filter_map(|((validator_index_within_group, _), attestation)| { + let grp_idx = GroupIndex(2); + group_validators(grp_idx).map(|validator_indices| { + (validator_indices[validator_index_within_group], attestation) + }) + }), + ); + + assert_eq!( + expected, + candidate_receipt_with_backing_validator_indices + .into_iter() + .map(|c| (c.0.hash(), c)) + .collect() + ); + } + // interfering code upgrade - reject { let mut candidate = TestCandidateBuilder { From f1dde9918b75ff7f113001d0c9d55dc93931b219 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Wed, 24 Jul 2024 19:11:37 +0300 Subject: [PATCH 11/13] add feature bit & update prdoc Signed-off-by: Andrei Sandu --- polkadot/primitives/src/v7/mod.rs | 6 +++++- prdoc/pr_4665.prdoc | 5 ++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/polkadot/primitives/src/v7/mod.rs b/polkadot/primitives/src/v7/mod.rs index 06b704652083..a729d8cee4c7 100644 --- a/polkadot/primitives/src/v7/mod.rs +++ b/polkadot/primitives/src/v7/mod.rs @@ -2040,10 +2040,14 @@ pub mod node_features { /// Must not be enabled unless all validators and collators have stopped using `req_chunk` /// protocol version 1. If it is enabled, validators can start systematic chunk recovery. AvailabilityChunkMapping = 2, + /// Enables node side support of `CoreIndex` committed candidate receipts. + /// See [RFC-103](https://github.com/polkadot-fellows/RFCs/pull/103) for details. + /// Only enable if at least 2/3 of nodes support the feature. + CandidateReceiptV2 = 3, /// First unassigned feature bit. /// Every time a new feature flag is assigned it should take this value. /// and this should be incremented. - FirstUnassigned = 3, + FirstUnassigned = 4, } } diff --git a/prdoc/pr_4665.prdoc b/prdoc/pr_4665.prdoc index 43ca0762fd1e..7a8ec7398e64 100644 --- a/prdoc/pr_4665.prdoc +++ b/prdoc/pr_4665.prdoc @@ -7,9 +7,12 @@ doc: - audience: [Runtime Dev, Node Dev] description: | Removes runtime collator signature checks, but these are still being done on the node. Remove collator - and signature from the `ProspectiveCandidate` definition in the inclusion emulator. + and signature from the `ProspectiveCandidate` definition in the inclusion emulator. Add + `CandidateReceiptV2` node feature bit. crates: +- name: polkadot-primitives + bump: minor - name: polkadot-node-subsystem-util bump: minor - name: polkadot-node-core-prospective-parachains From 78fee3bc69baaae4d113e2641afae04ae9b93c07 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Wed, 24 Jul 2024 19:15:18 +0300 Subject: [PATCH 12/13] fix clippy Signed-off-by: Andrei Sandu --- polkadot/node/subsystem-util/src/inclusion_emulator/mod.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/polkadot/node/subsystem-util/src/inclusion_emulator/mod.rs b/polkadot/node/subsystem-util/src/inclusion_emulator/mod.rs index 571196fb87ee..31d3ed026d64 100644 --- a/polkadot/node/subsystem-util/src/inclusion_emulator/mod.rs +++ b/polkadot/node/subsystem-util/src/inclusion_emulator/mod.rs @@ -1156,11 +1156,6 @@ mod tests { constraints: &Constraints, relay_parent: &RelayChainBlockInfo, ) -> ProspectiveCandidate { - let collator_pair = CollatorPair::generate().0; - let collator = collator_pair.public(); - - let sig = collator_pair.sign(b"blabla".as_slice()); - ProspectiveCandidate { commitments: CandidateCommitments { upward_messages: Default::default(), @@ -1170,7 +1165,6 @@ mod tests { processed_downward_messages: 0, hrmp_watermark: relay_parent.number, }, - collator, persisted_validation_data: PersistedValidationData { parent_head: constraints.required_parent.clone(), relay_parent_number: relay_parent.number, From 927ad3380af23e730e51e912870a33050164ccf1 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Thu, 25 Jul 2024 10:35:00 +0300 Subject: [PATCH 13/13] unused Signed-off-by: Andrei Sandu --- polkadot/node/subsystem-util/src/inclusion_emulator/mod.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/polkadot/node/subsystem-util/src/inclusion_emulator/mod.rs b/polkadot/node/subsystem-util/src/inclusion_emulator/mod.rs index 31d3ed026d64..2272f048089c 100644 --- a/polkadot/node/subsystem-util/src/inclusion_emulator/mod.rs +++ b/polkadot/node/subsystem-util/src/inclusion_emulator/mod.rs @@ -794,10 +794,7 @@ fn validate_against_constraints( #[cfg(test)] mod tests { use super::*; - use polkadot_primitives::{ - CollatorPair, HorizontalMessages, OutboundHrmpMessage, ValidationCode, - }; - use sp_application_crypto::Pair; + use polkadot_primitives::{HorizontalMessages, OutboundHrmpMessage, ValidationCode}; #[test] fn stack_modifications() {