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/subsystem-util/src/inclusion_emulator/mod.rs b/polkadot/node/subsystem-util/src/inclusion_emulator/mod.rs index b5aef325c8b4..2272f048089c 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}; @@ -521,18 +520,13 @@ 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 { /// 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. @@ -800,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() { @@ -1162,11 +1153,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(), @@ -1176,8 +1162,6 @@ mod tests { processed_downward_messages: 0, 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/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/polkadot/runtime/parachains/src/builder.rs b/polkadot/runtime/parachains/src/builder.rs index ec07cca2107e..b2a67ee8dd24 100644 --- a/polkadot/runtime/parachains/src/builder.rs +++ b/polkadot/runtime/parachains/src/builder.rs @@ -30,21 +30,30 @@ use bitvec::{order::Lsb0 as BitOrderLsb0, vec::BitVec}; use frame_support::pallet_prelude::*; use frame_system::pallet_prelude::*; use polkadot_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_core::{sr25519, ByteArray, H256}; use sp_runtime::{ generic::Digest, traits::{Header as HeaderT, One, TrailingZeroInput, Zero}, RuntimeAppPublic, }; +/// 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]) } @@ -584,7 +593,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(); @@ -614,14 +622,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(); @@ -634,11 +634,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 281dc5d0c5f4..a5bb6e6cc60f 100644 --- a/polkadot/runtime/parachains/src/inclusion/mod.rs +++ b/polkadot/runtime/parachains/src/inclusion/mod.rs @@ -338,8 +338,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. @@ -1222,7 +1220,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 /// @@ -1259,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)?; diff --git a/polkadot/runtime/parachains/src/inclusion/tests.rs b/polkadot/runtime/parachains/src/inclusion/tests.rs index 18def664f4b2..3ead456cde5a 100644 --- a/polkadot/runtime/parachains/src/inclusion/tests.rs +++ b/polkadot/runtime/parachains/src/inclusion/tests.rs @@ -1545,16 +1545,52 @@ fn candidate_checks() { 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 + 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() ); } diff --git a/prdoc/pr_4665.prdoc b/prdoc/pr_4665.prdoc new file mode 100644 index 000000000000..7a8ec7398e64 --- /dev/null +++ b/prdoc/pr_4665.prdoc @@ -0,0 +1,21 @@ +# 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. Add + `CandidateReceiptV2` node feature bit. + +crates: +- name: polkadot-primitives + bump: minor +- name: polkadot-node-subsystem-util + bump: minor +- name: polkadot-node-core-prospective-parachains + bump: patch +- name: polkadot-runtime-parachains + bump: patch