diff --git a/Cargo.lock b/Cargo.lock index 9ad9e595f77d6..378398c5d61d6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5163,6 +5163,7 @@ dependencies = [ "scale-info", "sp-application-crypto 30.0.0", "sp-consensus-aura", + "sp-consensus-babe", "sp-core 28.0.0", "sp-io 30.0.0", "sp-keyring", diff --git a/cumulus/pallets/aura-ext/Cargo.toml b/cumulus/pallets/aura-ext/Cargo.toml index ad633865e0fb4..1721446d4db82 100644 --- a/cumulus/pallets/aura-ext/Cargo.toml +++ b/cumulus/pallets/aura-ext/Cargo.toml @@ -22,17 +22,18 @@ pallet-aura = { workspace = true } pallet-timestamp = { workspace = true } sp-application-crypto = { workspace = true } sp-consensus-aura = { workspace = true } +sp-consensus-babe = { workspace = true } sp-runtime = { workspace = true } # Cumulus cumulus-pallet-parachain-system = { workspace = true } +cumulus-primitives-core = { workspace = true } [dev-dependencies] rstest = { workspace = true } # Cumulus cumulus-pallet-parachain-system = { workspace = true, default-features = true } -cumulus-primitives-core = { workspace = true, default-features = true } cumulus-primitives-proof-size-hostfunction = { workspace = true, default-features = true } cumulus-test-relay-sproof-builder = { workspace = true, default-features = true } @@ -49,6 +50,7 @@ default = ["std"] std = [ "codec/std", "cumulus-pallet-parachain-system/std", + "cumulus-primitives-core/std", "frame-support/std", "frame-system/std", "pallet-aura/std", @@ -56,6 +58,7 @@ std = [ "scale-info/std", "sp-application-crypto/std", "sp-consensus-aura/std", + "sp-consensus-babe/std", "sp-runtime/std", ] try-runtime = [ diff --git a/cumulus/pallets/aura-ext/src/lib.rs b/cumulus/pallets/aura-ext/src/lib.rs index 5fcc09b8ef420..47ed329a29978 100644 --- a/cumulus/pallets/aura-ext/src/lib.rs +++ b/cumulus/pallets/aura-ext/src/lib.rs @@ -40,10 +40,12 @@ use sp_runtime::traits::{Block as BlockT, Header as HeaderT, LazyBlock}; pub mod consensus_hook; pub mod migration; +pub mod signature_verifier; #[cfg(test)] mod test; pub use consensus_hook::FixedVelocityConsensusHook; +pub use signature_verifier::AuraSchedulingVerifier; type Aura = pallet_aura::Pallet; diff --git a/cumulus/pallets/aura-ext/src/signature_verifier.rs b/cumulus/pallets/aura-ext/src/signature_verifier.rs new file mode 100644 index 0000000000000..25eafd17b3d39 --- /dev/null +++ b/cumulus/pallets/aura-ext/src/signature_verifier.rs @@ -0,0 +1,114 @@ +// Copyright (C) Parity Technologies (UK) Ltd. +// This file is part of Cumulus. +// SPDX-License-Identifier: Apache-2.0 + +//! V3 scheduling signature verifier backed by parachain Aura authorities. +//! +//! Implements [`VerifySchedulingSignature`] for parachains running Aura: derives the +//! parachain slot from the BABE pre-digest of the relay header at +//! `internal_scheduling_parent`, looks up the eligible Aura author from this pallet's +//! cached authority set, and verifies the 64-byte signature in [`SignedSchedulingInfo`] +//! over the encoded `SchedulingInfoPayload`. + +use crate::{Authorities, Config}; +use codec::{Decode, Encode}; +use cumulus_primitives_core::{ + relay_chain::{Header as RelayChainHeader, RELAY_CHAIN_SLOT_DURATION_MILLIS}, + SignedSchedulingInfo, VerifySchedulingSignature, +}; +use sp_application_crypto::RuntimeAppPublic; +use sp_consensus_aura::Slot; +use sp_consensus_babe::digests::CompatibleDigestItem as BabeDigestItem; + +/// Verifier for V3 [`SignedSchedulingInfo`] against parachain Aura authorities. +/// +/// Wired by the parachain runtime as +/// `type SchedulingSignatureVerifier = AuraSchedulingVerifier;` on +/// [`cumulus_pallet_parachain_system::Config`]. The relay slot duration is the +/// global `RELAY_CHAIN_SLOT_DURATION_MILLIS` (6000 ms), +/// which is fixed across Polkadot, Kusama, Westend, and Rococo. +/// +/// `T` is the runtime; the Aura crypto is derived from +/// [`pallet_aura::Config::AuthorityId`] (typically `sr25519` or `ed25519`). The +/// signature blob in [`SignedSchedulingInfo`] is decoded into +/// `::Signature` and verified with the +/// authority's own `verify` method, matching the existing Aura seal verification path. +pub struct AuraSchedulingVerifier(core::marker::PhantomData); + +impl VerifySchedulingSignature for AuraSchedulingVerifier +where + T: Config, + T: pallet_timestamp::Config, +{ + const V3_SCHEDULING_ENABLED: bool = true; + + /// Verify that `signed_info` was produced by the Aura author eligible at the parachain slot + /// derived from `internal_scheduling_parent_header`. + /// + /// Returns `true` only when every step succeeds; all error paths return `false` (fail-closed) + /// so the PVF rejects the candidate without panicking on adversarial input. + /// + /// Binds the signature to `internal_scheduling_parent_header` by asserting the payload's + /// `internal_scheduling_parent` field matches its hash. Derives the para slot from the + /// header's BABE pre-digest, then looks up the eligible Aura author in the cached authority + /// set and verifies the signature over the encoded `SchedulingInfoPayload`. + fn verify( + signed_info: &SignedSchedulingInfo, + internal_scheduling_parent_header: &RelayChainHeader, + ) -> bool { + if signed_info.payload.internal_scheduling_parent != + internal_scheduling_parent_header.hash() + { + return false; + } + + // 1. Relay slot at internal scheduling parent gives the para slot that determines the valid + // author. + let relay_slot: Slot = match internal_scheduling_parent_header + .digest + .logs() + .iter() + .find_map(|log| BabeDigestItem::as_babe_pre_digest(log)) + { + Some(pre_digest) => pre_digest.slot(), + None => return false, + }; + + // 2. Determine the para slot. + let para_slot_duration: u64 = + match TryInto::::try_into(pallet_aura::Pallet::::slot_duration()) { + Ok(d) if d > 0 => d, + _ => return false, + }; + + let para_slot: u64 = match u64::from(relay_slot) + .checked_mul(RELAY_CHAIN_SLOT_DURATION_MILLIS) + .map(|product| product / para_slot_duration) + { + Some(s) => s, + None => return false, + }; + + // 3. Look up the eligible Aura author. + let authorities = Authorities::::get(); + let author_idx = match pallet_aura::Pallet::::slot_author_index(Slot::from(para_slot)) { + Some(idx) => idx as usize, + None => return false, + }; + let author = match authorities.get(author_idx) { + Some(author) => author, + None => return false, + }; + + // 4. Decode the 64-byte signature blob as the authority's expected signature type and + // verify over the encoded SchedulingInfoPayload. + let signature = match ::Signature::decode( + &mut &signed_info.signature[..], + ) { + Ok(sig) => sig, + Err(_) => return false, + }; + + author.verify(&signed_info.payload.encode(), &signature) + } +} diff --git a/cumulus/pallets/aura-ext/src/test.rs b/cumulus/pallets/aura-ext/src/test.rs index 9ec6d91ced03f..973287654329c 100644 --- a/cumulus/pallets/aura-ext/src/test.rs +++ b/cumulus/pallets/aura-ext/src/test.rs @@ -522,3 +522,348 @@ fn block_executor_does_not_influence_proof_size_recordings() { BlockExecutor::::execute_verified_block(block); }); } + +// ============================================================================= +// AuraSchedulingVerifier tests +// ============================================================================= + +mod scheduling_verifier_tests { + use super::*; + use crate::signature_verifier::AuraSchedulingVerifier; + use codec::Encode; + use cumulus_primitives_core::{ + relay_chain::{ApprovedPeerId, Hash as RelayHash, Header as RelayChainHeader}, + SchedulingInfoPayload, SignedSchedulingInfo, VerifySchedulingSignature, + }; + use sp_consensus_babe::digests::{ + CompatibleDigestItem as BabeDigestItem, PreDigest, SecondaryPlainPreDigest, + }; + use sp_keyring::{Ed25519Keyring, Sr25519Keyring}; + use sp_runtime::generic::Digest; + + const PARA_SLOT_DURATION_MS: u64 = 6_000; + const RELAY_SLOT_DURATION_MS: u64 = 6_000; + + // Ed25519Test is a minimal mock runtime for the ed25519 smoke test only. + // All other tests use the top-level `Test` runtime (sr25519). + + type Ed25519Block = frame_system::mocking::MockBlock; + + frame_support::construct_runtime!( + pub enum Ed25519Test { + System: frame_system, + Timestamp: pallet_timestamp, + Aura: pallet_aura, + AuraExt: crate, + } + ); + + #[derive_impl(frame_system::config_preludes::TestDefaultConfig)] + impl frame_system::Config for Ed25519Test { + type Block = Ed25519Block; + type RuntimeEvent = (); + } + + impl crate::Config for Ed25519Test {} + + impl pallet_aura::Config for Ed25519Test { + type AuthorityId = sp_consensus_aura::ed25519::AuthorityId; + type MaxAuthorities = ConstU32<100_000>; + type DisabledValidators = (); + type AllowMultipleBlocksPerSlot = ConstBool; + type SlotDuration = TestSlotDuration; + } + + impl pallet_timestamp::Config for Ed25519Test { + type Moment = u64; + type OnTimestampSet = (); + type MinimumPeriod = (); + type WeightInfo = (); + } + + // ------------------------------------------------------------------------- + // Shared test helpers (crypto-agnostic). + // ------------------------------------------------------------------------- + + /// Build a relay chain header whose digest carries a BABE secondary-plain pre-digest + /// at the given slot. The verifier only reads the slot off the pre-digest, so the + /// exact pre-digest variant doesn't matter. + fn relay_header_at_slot(relay_slot: u64) -> RelayChainHeader { + let mut digest = Digest::default(); + digest.push(::babe_pre_digest( + PreDigest::SecondaryPlain(SecondaryPlainPreDigest { + authority_index: 0, + slot: relay_slot.into(), + }), + )); + RelayChainHeader { + parent_hash: Default::default(), + number: 0, + state_root: Default::default(), + extrinsics_root: Default::default(), + digest, + } + } + + /// Build a payload whose `internal_scheduling_parent` matches `isp`. Tests that want + /// a mismatch (replay-detection check) pass a different hash explicitly. + fn make_payload(isp: RelayHash) -> SchedulingInfoPayload { + SchedulingInfoPayload::new( + cumulus_primitives_core::CoreSelector(0), + 0, + ApprovedPeerId::default(), + isp, + ) + } + + /// Para slot derived as `relay_slot * 6000ms / para_slot_duration` (matches the + /// verifier's arithmetic). With equal slot durations this is the identity. + fn para_slot_from_relay(relay_slot: u64, para_slot_duration: u64) -> u64 { + relay_slot.saturating_mul(RELAY_SLOT_DURATION_MS) / para_slot_duration + } + + type Sr25519Id = sp_consensus_aura::sr25519::AuthorityId; + type Ed25519Id = sp_consensus_aura::ed25519::AuthorityId; + + fn set_authorities(authorities: Vec<::AuthorityId>) + where + T: crate::Config, + { + let bounded: BoundedVec<_, ::MaxAuthorities> = + authorities.try_into().expect("test fixture stays under MaxAuthorities; qed"); + pallet_aura::Authorities::::put(bounded.clone()); + Authorities::::put(bounded); + } + + // ------------------------------------------------------------------------- + // sr25519 tests (default crypto scheme). + // ------------------------------------------------------------------------- + + #[rstest] + #[case::eligible_author_signs(true)] + #[case::non_eligible_author_signs(false)] + fn single_authority_verifier(#[case] eligible_signer: bool) { + // Single-authority fixture (Alice). The eligible-author signature passes; any + // other signer is rejected. + TestSlotDuration::set_slot_duration(PARA_SLOT_DURATION_MS); + sp_io::TestExternalities::new_empty().execute_with(|| { + set_authorities::(vec![Sr25519Id::from(Sr25519Keyring::Alice.public())]); + let header = relay_header_at_slot(7); + let payload = make_payload(header.hash()); + let signer = if eligible_signer { Sr25519Keyring::Alice } else { Sr25519Keyring::Bob }; + let signed = + SignedSchedulingInfo { signature: signer.sign(&payload.encode()).0, payload }; + assert_eq!(AuraSchedulingVerifier::::verify(&signed, &header), eligible_signer); + }); + } + + #[test] + fn tampered_payload_is_rejected() { + // Sign one payload, verify against a different one — verification must fail. + TestSlotDuration::set_slot_duration(PARA_SLOT_DURATION_MS); + sp_io::TestExternalities::new_empty().execute_with(|| { + set_authorities::(vec![Sr25519Id::from(Sr25519Keyring::Alice.public())]); + let header = relay_header_at_slot(7); + let original = make_payload(header.hash()); + let signature = Sr25519Keyring::Alice.sign(&original.encode()).0; + let tampered = SchedulingInfoPayload::new( + cumulus_primitives_core::CoreSelector(99), + 0, + ApprovedPeerId::default(), + header.hash(), + ); + let signed = SignedSchedulingInfo { signature, payload: tampered }; + assert!(!AuraSchedulingVerifier::::verify(&signed, &header)); + }); + } + + #[test] + fn payload_isp_mismatching_header_is_rejected() { + // Replay-detection: an attacker takes a signature created at ISP X and tries to + // use it at ISP Y (different relay block). The verifier must reject because the + // payload's claimed `internal_scheduling_parent` no longer matches the header's + // hash, even though the signer is otherwise eligible. + TestSlotDuration::set_slot_duration(PARA_SLOT_DURATION_MS); + sp_io::TestExternalities::new_empty().execute_with(|| { + set_authorities::(vec![Sr25519Id::from(Sr25519Keyring::Alice.public())]); + let header = relay_header_at_slot(7); + let payload = make_payload(RelayHash::repeat_byte(0xAA)); // different ISP + let signed = SignedSchedulingInfo { + signature: Sr25519Keyring::Alice.sign(&payload.encode()).0, + payload, + }; + assert!(!AuraSchedulingVerifier::::verify(&signed, &header)); + }); + } + + #[rstest] + #[case::eligible_index_signs(3, true)] + #[case::non_eligible_index_signs(0, false)] + fn multi_authority_verifier_picks_index(#[case] signer_idx: usize, #[case] expected: bool) { + // Fixture: authorities = [Alice, Bob, Charlie, Dave], relay slot 7, 6s para slots. + // Para slot = 7, 7 mod 4 = 3 → only authorities[3] (Dave) is eligible. + TestSlotDuration::set_slot_duration(PARA_SLOT_DURATION_MS); + sp_io::TestExternalities::new_empty().execute_with(|| { + let keys = [ + Sr25519Keyring::Alice, + Sr25519Keyring::Bob, + Sr25519Keyring::Charlie, + Sr25519Keyring::Dave, + ]; + set_authorities::(keys.iter().map(|k| Sr25519Id::from(k.public())).collect()); + + let relay_slot = 7u64; + let para_slot = para_slot_from_relay(relay_slot, PARA_SLOT_DURATION_MS); + assert_eq!( + (para_slot % keys.len() as u64) as usize, + 3, + "test fixture: para_slot=7 mod 4 == 3" + ); + + let header = relay_header_at_slot(relay_slot); + let payload = make_payload(header.hash()); + let signed = SignedSchedulingInfo { + signature: keys[signer_idx].sign(&payload.encode()).0, + payload, + }; + assert_eq!(AuraSchedulingVerifier::::verify(&signed, &header), expected); + }); + } + + /// Exercises the relay→para slot conversion with a 4:1 ratio (24 s para slots, 6 s relay + /// slots). relay_slot 28 → para_slot = 28 * 6000 / 24000 = 7 → 7 mod 4 = 3 → Dave + /// (index 3) is the only eligible author. + #[rstest] + #[case::eligible_index_signs(3, true)] + #[case::non_eligible_index_signs(0, false)] + fn multi_authority_verifier_24s_para_slots(#[case] signer_idx: usize, #[case] expected: bool) { + // Para slot duration is 4× the relay slot duration, so the conversion ratio is + // non-identity. This catches any accidental identity short-circuit in the verifier. + const PARA_24S: u64 = 24_000; + TestSlotDuration::set_slot_duration(PARA_24S); + sp_io::TestExternalities::new_empty().execute_with(|| { + let keys = [ + Sr25519Keyring::Alice, + Sr25519Keyring::Bob, + Sr25519Keyring::Charlie, + Sr25519Keyring::Dave, + ]; + set_authorities::(keys.iter().map(|k| Sr25519Id::from(k.public())).collect()); + + let relay_slot = 28u64; + let para_slot = para_slot_from_relay(relay_slot, PARA_24S); + assert_eq!(para_slot, 7, "28 * 6000 / 24000 == 7"); + assert_eq!( + (para_slot % keys.len() as u64) as usize, + 3, + "test fixture: para_slot=7 mod 4 == 3 (Dave)" + ); + + let header = relay_header_at_slot(relay_slot); + let payload = make_payload(header.hash()); + let signed = SignedSchedulingInfo { + signature: keys[signer_idx].sign(&payload.encode()).0, + payload, + }; + assert_eq!(AuraSchedulingVerifier::::verify(&signed, &header), expected); + }); + } + + #[test] + fn empty_authority_set_is_rejected() { + // `Authorities::` empty means no eligible author exists; verification fails + // closed rather than panicking on `para_slot % 0`. + TestSlotDuration::set_slot_duration(PARA_SLOT_DURATION_MS); + sp_io::TestExternalities::new_empty().execute_with(|| { + set_authorities::(Vec::::new()); + let header = relay_header_at_slot(7); + let payload = make_payload(header.hash()); + let signed = SignedSchedulingInfo { + signature: Sr25519Keyring::Alice.sign(&payload.encode()).0, + payload, + }; + assert!(!AuraSchedulingVerifier::::verify(&signed, &header)); + }); + } + + #[test] + fn zero_para_slot_duration_is_rejected() { + // A misconfigured pallet-aura returning `slot_duration() == 0` would otherwise + // divide-by-zero; the verifier must reject up front. + TestSlotDuration::set_slot_duration(0); + sp_io::TestExternalities::new_empty().execute_with(|| { + set_authorities::(vec![Sr25519Id::from(Sr25519Keyring::Alice.public())]); + let header = relay_header_at_slot(7); + let payload = make_payload(header.hash()); + let signed = SignedSchedulingInfo { + signature: Sr25519Keyring::Alice.sign(&payload.encode()).0, + payload, + }; + assert!(!AuraSchedulingVerifier::::verify(&signed, &header)); + }); + } + + #[test] + fn missing_babe_pre_digest_is_rejected() { + // Without a BABE pre-digest the verifier can't derive the relay slot and must + // reject — there is no fallback that could pick an author. + TestSlotDuration::set_slot_duration(PARA_SLOT_DURATION_MS); + sp_io::TestExternalities::new_empty().execute_with(|| { + set_authorities::(vec![Sr25519Id::from(Sr25519Keyring::Alice.public())]); + let header_no_digest = RelayChainHeader { + parent_hash: Default::default(), + number: 0, + state_root: Default::default(), + extrinsics_root: Default::default(), + digest: Digest::default(), + }; + let payload = make_payload(header_no_digest.hash()); + let signed = SignedSchedulingInfo { + signature: Sr25519Keyring::Alice.sign(&payload.encode()).0, + payload, + }; + assert!(!AuraSchedulingVerifier::::verify(&signed, &header_no_digest)); + }); + } + + #[test] + fn relay_slot_overflow_is_rejected() { + // `relay_slot * RELAY_CHAIN_SLOT_DURATION_MILLIS` must overflow on adversarial + // input. `u64::MAX * 6000` overflows; `checked_mul` returns `None` and the + // verifier rejects rather than silently saturating to a wrong author index. + TestSlotDuration::set_slot_duration(PARA_SLOT_DURATION_MS); + sp_io::TestExternalities::new_empty().execute_with(|| { + set_authorities::(vec![Sr25519Id::from(Sr25519Keyring::Alice.public())]); + let header = relay_header_at_slot(u64::MAX); + let payload = make_payload(header.hash()); + let signed = SignedSchedulingInfo { + signature: Sr25519Keyring::Alice.sign(&payload.encode()).0, + payload, + }; + assert!(!AuraSchedulingVerifier::::verify(&signed, &header)); + }); + } + + // ------------------------------------------------------------------------- + // ed25519 smoke test — confirms the ed25519 code path through the verifier. + // ------------------------------------------------------------------------- + + #[test] + fn ed25519_smoke_eligible_author_verifies() { + // Exercises AuraSchedulingVerifier against Ed25519Test (ed25519 AuthorityId). + // Uses the same happy-path scenario as single_authority_verifier: Alice is the + // sole authority at relay slot 7 and signs the matching payload — must pass. + TestSlotDuration::set_slot_duration(PARA_SLOT_DURATION_MS); + sp_io::TestExternalities::new_empty().execute_with(|| { + set_authorities::(vec![Ed25519Id::from(Ed25519Keyring::Alice.public())]); + + let header = relay_header_at_slot(7); + let payload = make_payload(header.hash()); + let signed = SignedSchedulingInfo { + signature: Ed25519Keyring::Alice.sign(&payload.encode()).0, + payload, + }; + assert!(AuraSchedulingVerifier::::verify(&signed, &header)); + }); + } +} diff --git a/cumulus/pallets/parachain-system/src/validate_block/implementation.rs b/cumulus/pallets/parachain-system/src/validate_block/implementation.rs index 788e4cd26960d..b204950ac8d1d 100644 --- a/cumulus/pallets/parachain-system/src/validate_block/implementation.rs +++ b/cumulus/pallets/parachain-system/src/validate_block/implementation.rs @@ -18,12 +18,13 @@ use super::{scheduling, trie_cache, trie_recorder, MemoryOptimizedValidationParams}; use alloc::vec::Vec; -use codec::{Decode, Encode}; +use codec::Encode; use cumulus_primitives_core::{ relay_chain::{ - BlockNumber as RNumber, Hash as RHash, UMPSignal, MAX_HEAD_DATA_SIZE, UMP_SEPARATOR, + BlockNumber as RNumber, Hash as RHash, Header as RelayChainHeader, MAX_HEAD_DATA_SIZE, + UMP_SEPARATOR, }, - ClaimQueueOffset, CoreSelector, CumulusDigestItem, ParachainBlockData, PersistedValidationData, + CumulusDigestItem, ParachainBlockData, PersistedValidationData, SignedSchedulingInfo, VerifySchedulingSignature, }; use frame_support::{ @@ -136,18 +137,35 @@ where sp_io::transaction_index::host_renew.replace_implementation(host_transaction_index_renew), ); - // V3 scheduling validation. + // V3 scheduling validation (chain-shape only). Signature verification of + // `signed_scheduling_info` happens here at the call site so the verifier wiring + // stays out of the pure shape check. let validated_scheduling = scheduling::validate_v3_scheduling( PSC::SchedulingSignatureVerifier::V3_SCHEDULING_ENABLED, &extension.0, block_data.scheduling_proof(), PSC::RelayParentOffset::get(), + crate::Pallet::::max_claim_queue_offset(), ); - if let Some(result) = validated_scheduling { - if result.is_resubmission { - panic!("Resubmission not yet supported; reject candidate."); - } - } + + // Extract the override inputs (signed payload + the ISP header the signer + // committed to) from the proof. Override engages whenever `signed_scheduling_info` + // is present — on resubmissions it's required by `check_scheduling`, and on + // initial submissions a collator may still attach one to assert authoritative + // scheduling. The actual signature verification needs to read + // `Authorities::` from parachain state, so it runs inside the first block's + // seal-verification externalities scope below — externalities aren't installed + // at this point. + let scheduling_override_inputs: Option<(SignedSchedulingInfo, RelayChainHeader)> = + validated_scheduling.and_then(|_| { + let proof = block_data.scheduling_proof().expect( + "V3 scheduling validation succeeded → scheduling proof present; \ + enforced by `validate_v3_scheduling`; qed", + ); + proof.signed_scheduling_info.as_ref().map(|signed_info| { + (signed_info.clone(), proof.internal_scheduling_parent_header.clone()) + }) + }); // Initialize hashmaps randomness. sp_trie::add_extra_randomness(build_seed_from_head_data::( @@ -182,6 +200,34 @@ where let cache_provider = trie_cache::CacheProvider::new(); let seen_nodes = SeenNodes::>::default(); + let parent_backend: sp_state_machine::TrieBackend< + _, + HashingFor, + _, + SizeOnlyRecorderProvider>, + > = sp_state_machine::TrieBackendBuilder::new_with_cache( + &db, + *parent_header.state_root(), + &cache_provider, + ) + .build(); + run_with_externalities_and_recorder::( + &parent_backend, + &mut Default::default(), + &mut Default::default(), + || { + if let Some((signed_info, isp_header)) = scheduling_override_inputs.as_ref() { + if !PSC::SchedulingSignatureVerifier::verify(signed_info, isp_header) { + panic!( + "V3 scheduling validation failed: invalid \ + signed_scheduling_info (ISP: {:?})", + isp_header.hash(), + ); + } + } + }, + ); + for (block_index, mut block) in blocks.into_iter().enumerate() { // We use the storage root of the `parent_head` to ensure that it is the correct root. // This is already being done above while creating the in-memory db, but let's be paranoid!! @@ -319,46 +365,13 @@ where } } - if !upward_message_signals.is_empty() { - let mut selected_core: Option<(CoreSelector, ClaimQueueOffset)> = None; - let mut approved_peer = None; - - upward_message_signals.iter().for_each(|s| { - match UMPSignal::decode(&mut &s[..]).expect("Failed to decode `UMPSignal`") { - UMPSignal::SelectCore(selector, offset) => match &selected_core { - Some(selected_core) if *selected_core != (selector, offset) => { - panic!( - "All `SelectCore` signals need to select the same core: {selected_core:?} vs {:?}", - (selector, offset), - ) - }, - Some(_) => {}, - None => { - selected_core = Some((selector, offset)); - }, - }, - UMPSignal::ApprovedPeer(new_approved_peer) => match &approved_peer { - Some(approved_peer) if *approved_peer != new_approved_peer => { - panic!( - "All `ApprovedPeer` signals need to select the same peer_id: {new_approved_peer:?} vs {approved_peer:?}", - ) - }, - Some(_) => {}, - None => { - approved_peer = Some(new_approved_peer); - }, - }, - } - }); - - upward_messages - .try_push(UMP_SEPARATOR) - .expect("UMPSignals does not fit in UMPMessages"); - - upward_messages - .try_extend(upward_message_signals.into_iter()) - .expect("UMPSignals does not fit in UMPMessages"); - } + // A `signed_scheduling_info` overrides the block's emitted signals wholesale — they + // are ignored, not merged. + let scheduling_signals = match scheduling_override_inputs.as_ref() { + Some((signed_info, _)) => scheduling::SchedulingSignals::from_scheduling_info(signed_info), + None => scheduling::SchedulingSignals::from_block_signals(&upward_message_signals), + }; + scheduling_signals.emit(&mut upward_messages); horizontal_messages.sort_by(|a, b| a.recipient.cmp(&b.recipient)); diff --git a/cumulus/pallets/parachain-system/src/validate_block/scheduling.rs b/cumulus/pallets/parachain-system/src/validate_block/scheduling.rs index 16bfd15fbd75d..d56e7205715a9 100644 --- a/cumulus/pallets/parachain-system/src/validate_block/scheduling.rs +++ b/cumulus/pallets/parachain-system/src/validate_block/scheduling.rs @@ -7,7 +7,13 @@ //! Validates the header chain from scheduling_parent to internal_scheduling_parent, //! and verifies relay_parent is at or before internal_scheduling_parent. -use cumulus_primitives_core::SchedulingProof; +use alloc::vec::Vec; +use codec::{Decode, Encode}; +use cumulus_primitives_core::{ + relay_chain::{ApprovedPeerId, UMPSignal, UMP_SEPARATOR}, + ClaimQueueOffset, CoreSelector, SchedulingProof, SignedSchedulingInfo, +}; +use frame_support::{traits::Get, BoundedVec}; use polkadot_parachain_primitives::primitives::ValidationParamsExtension; use sp_runtime::traits::Header as HeaderT; @@ -30,27 +36,40 @@ pub enum SchedulingValidationError { /// When relay_parent != internal_scheduling_parent, the resubmitting collator must /// sign the core selection to prove slot eligibility. MissingSignedSchedulingInfo, -} - -/// Result of successful scheduling validation. -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct SchedulingValidationResult { - /// The internal scheduling parent (derived from header chain). - pub internal_scheduling_parent: RelayHash, - /// Whether this is a resubmission (relay_parent != internal_scheduling_parent). - pub is_resubmission: bool, + /// `internal_scheduling_parent_header` does not hash to the internal scheduling + /// parent derived from the header chain (or `scheduling_parent` when the chain + /// is empty). The PVF reads the BABE pre-digest from this header to derive the + /// parachain slot used for author lookup; without the linkage check a collator + /// could attach an unrelated header pointing the verifier at an arbitrary slot. + InternalSchedulingParentHeaderMismatch, + /// `signed_scheduling_info.payload.internal_scheduling_parent` does not match the + /// internal scheduling parent derived from the proof. The signer must have signed + /// over the same ISP the proof points to; rejecting the mismatch here prevents a + /// signature meant for a different scheduling context from being reused. + SignedSchedulingInfoIspMismatch, + /// Signed `claim_queue_offset` exceeds the runtime-enforced maximum. + ClaimQueueOffsetTooLarge { offset: u8, max: u8 }, } /// Validate V3 scheduling based on runtime config and candidate extension. /// -/// Returns `None` for V1/V2 candidates, `Some(result)` for valid V3. -/// Panics on config/extension mismatches or validation failures. +/// Returns `None` for V1/V2 candidates, `Some(internal_scheduling_parent)` for valid V3. +/// Panics on config/extension mismatches or chain-shape validation failures. +/// +/// This function only validates the *shape* of the scheduling proof (header chain +/// linkage, relay-parent position, presence of `signed_scheduling_info` when +/// required, and that `internal_scheduling_parent_header` hashes to the derived +/// internal scheduling parent). Signature verification on `signed_scheduling_info` +/// is the caller's responsibility — see `validate_block` for the call site that +/// invokes `PSC::SchedulingSignatureVerifier` using the returned +/// `internal_scheduling_parent`. pub fn validate_v3_scheduling( v3_enabled: bool, extension: &Option, scheduling_proof: Option<&SchedulingProof>, expected_header_chain_length: u32, -) -> Option { + max_claim_queue_offset: u8, +) -> Option { match (v3_enabled, extension) { (false, None) => { // V3 disabled and no extension: normal V1/V2 path @@ -81,23 +100,36 @@ pub fn validate_v3_scheduling( *relay_parent, *scheduling_parent, expected_header_chain_length, + max_claim_queue_offset, ) { - Ok(result) => Some(result), + Ok(isp) => Some(isp), Err(e) => panic!("V3 scheduling validation failed: {:?}", e), } }, } } -/// Check the scheduling proof against the relay parent, scheduling parent, -/// and expected header chain length. Returns the internal scheduling parent -/// and whether this is a resubmission. +/// Check the scheduling proof against the relay parent, scheduling parent, and +/// expected header chain length. +/// +/// Two submission shapes are valid: +/// - **Initial submission** (`relay_parent == internal_scheduling_parent`): +/// `signed_scheduling_info` is optional. When absent, core selection comes from the block's UMP +/// signals; when present it is legal but unused here. +/// - **Resubmission** (`relay_parent` is an ancestor of `internal_scheduling_parent`): +/// `signed_scheduling_info` is required and its `payload.internal_scheduling_parent` must match +/// the derived ISP. +/// +/// Returns the derived `internal_scheduling_parent`. Signature verification on +/// `signed_scheduling_info` is the caller's responsibility — see `validate_block` +/// for the call site that invokes `PSC::SchedulingSignatureVerifier`. pub fn check_scheduling( scheduling_proof: &SchedulingProof, relay_parent: RelayHash, scheduling_parent: RelayHash, expected_header_chain_length: u32, -) -> Result { + max_claim_queue_offset: u8, +) -> Result { let header_chain = &scheduling_proof.header_chain; // 1. Verify header chain length @@ -126,19 +158,28 @@ pub fn check_scheduling( } } - // 3. Derive internal_scheduling_parent - // It's the parent_hash of the last (oldest) header in the chain + // 3. Derive internal_scheduling_parent. It's the parent_hash of the last (oldest) + // header in the chain, or `scheduling_parent` itself when the chain is empty + // (`RelayParentOffset = 0`). let internal_scheduling_parent = if header_chain.is_empty() { - // If header chain is empty (length 0), internal_scheduling_parent == scheduling_parent scheduling_parent } else { - *header_chain.last().expect("checked non-empty").parent_hash() + *header_chain.last().expect("checked non-empty; qed").parent_hash() }; - // 4. Validate relay_parent position - // relay_parent must NOT be inside the header chain (it can equal internal_scheduling_parent - // or be an ancestor of it, but not somewhere between scheduling_parent and - // internal_scheduling_parent) + // 4. The internal_scheduling_parent_header carried in the proof must hash to the + // internal_scheduling_parent we just derived. The PVF reads the BABE pre-digest + // out of this header to derive the parachain slot used for author lookup; without + // the linkage check a collator could attach an unrelated header pointing the + // verifier at an arbitrary slot. + if scheduling_proof.internal_scheduling_parent_header.hash() != internal_scheduling_parent { + return Err(SchedulingValidationError::InternalSchedulingParentHeaderMismatch); + } + + // 5. Validate relay_parent position. relay_parent must NOT be inside the header + // chain — it either equals internal_scheduling_parent (initial submission) or is + // an ancestor of it (resubmission), but never between scheduling_parent and + // internal_scheduling_parent. for header in header_chain.iter() { let header_hash = header.hash(); if relay_parent == header_hash { @@ -146,27 +187,115 @@ pub fn check_scheduling( } } - // 5. Validate signed_scheduling_info based on relay_parent position - let is_initial_submission = relay_parent == internal_scheduling_parent; + // 6. Validate signed_scheduling_info based on relay_parent position. + // Resubmission (relay_parent != ISP) requires the resubmitter to sign the core + // selection; initial submission (relay_parent == ISP) may carry one optionally. + if relay_parent != internal_scheduling_parent && + scheduling_proof.signed_scheduling_info.is_none() + { + return Err(SchedulingValidationError::MissingSignedSchedulingInfo); + } - if !is_initial_submission { - // Resubmission: relay_parent is an ancestor of internal_scheduling_parent. - // The resubmitting collator must sign the core selection. - if scheduling_proof.signed_scheduling_info.is_none() { - return Err(SchedulingValidationError::MissingSignedSchedulingInfo); + // 7. When signed_scheduling_info is present, its payload must commit to the ISP the proof + // points to, and its claim_queue_offset must be within the runtime cap. + if let Some(signed_info) = &scheduling_proof.signed_scheduling_info { + if signed_info.payload.internal_scheduling_parent != internal_scheduling_parent { + return Err(SchedulingValidationError::SignedSchedulingInfoIspMismatch); } - // Signature verification is done separately after slot/authority lookup - } - // Note: For initial submission (relay_parent == internal_scheduling_parent), - // signed_scheduling_info is optional. If absent, core selection comes from the - // block's UMP signals. If present, signature verification is still performed. - // Collators should refuse to acknowledge blocks with invalid scheduling info, - // so providing signed_scheduling_info is not necessary but is legal. - - Ok(SchedulingValidationResult { - internal_scheduling_parent, - is_resubmission: !is_initial_submission, - }) + if signed_info.payload.claim_queue_offset > max_claim_queue_offset { + return Err(SchedulingValidationError::ClaimQueueOffsetTooLarge { + offset: signed_info.payload.claim_queue_offset, + max: max_claim_queue_offset, + }); + } + } + + Ok(internal_scheduling_parent) +} + +/// The UMP signal tail a candidate emits to the relay chain, parachain-side mirror of +/// [`polkadot_primitives::vstaging::CandidateUMPSignals`]. +/// +/// The relay decoder (`CandidateCommitments::ump_signals`) is the contract we build for: it +/// rejects a second occurrence of either variant (`DuplicateUMPSignal`) and any third signal +/// (`TooManyUMPSignals`), and parses only the run after the *first* `UMP_SEPARATOR`. We panic +/// rather than emit a tail it would reject — a violation here is our own runtime's bug, not +/// adversarial input. +#[derive(Debug, Default, PartialEq, Eq)] +pub struct SchedulingSignals { + select_core: Option<(CoreSelector, ClaimQueueOffset)>, + approved_peer: Option, +} + +impl SchedulingSignals { + /// Parse the encoded `UMPSignal`s a PoV's blocks emitted after the in-block `UMP_SEPARATOR`. + /// + /// Panics on a repeated variant *even when values match*: the relay decoder counts + /// occurrences, not distinct values, so a duplicate is a bug regardless. + pub fn from_block_signals(raw: &[Vec]) -> Self { + let mut signals = Self::default(); + for bytes in raw { + // NOTE: this match is intentionally exhaustive (no `_` arm). Adding a new + // `UMPSignal` variant must fail to compile here, forcing a deliberate decision: + // new non-scheduling signal classes (e.g. the speculative-messaging + // `Requires`/`Provides` commitments) must be handled explicitly — either passed + // through untouched or routed to their own override path — and must NOT be + // silently dropped. Such classes may also have different cardinality rules; the + // per-variant singleton check below applies only to the scheduling signals. + match UMPSignal::decode(&mut &bytes[..]).expect("Failed to decode `UMPSignal`") { + UMPSignal::SelectCore(selector, offset) => { + if signals.select_core.replace((selector, offset)).is_some() { + panic!("Parachain emitted more than one `SelectCore` UMP signal"); + } + }, + UMPSignal::ApprovedPeer(peer_id) => { + if signals.approved_peer.replace(peer_id).is_some() { + panic!("Parachain emitted more than one `ApprovedPeer` UMP signal"); + } + }, + } + } + signals + } + + /// Build the tail from a verified `SignedSchedulingInfo`, which wholesale replaces the + /// block's emitted signals (the signer signed all three fields). + pub fn from_scheduling_info(signed_info: &SignedSchedulingInfo) -> Self { + let payload = &signed_info.payload; + Self { + select_core: Some(( + payload.core_selector, + ClaimQueueOffset(payload.claim_queue_offset), + )), + approved_peer: Some(payload.peer_id.clone()), + } + } + + pub fn is_empty(&self) -> bool { + self.select_core.is_none() && self.approved_peer.is_none() + } + + /// Order is `SelectCore` then `ApprovedPeer`, matching + /// `pallet_parachain_system::send_ump_signals`. Emits nothing — not even a separator — when + /// empty, since the relay decoder keys off the first `UMP_SEPARATOR`. + pub fn emit>(self, upward_messages: &mut BoundedVec, S>) { + if self.is_empty() { + return; + } + upward_messages + .try_push(UMP_SEPARATOR) + .expect("UMPSignals does not fit in UMPMessages"); + if let Some((selector, offset)) = self.select_core { + upward_messages + .try_push(UMPSignal::SelectCore(selector, offset).encode()) + .expect("UMPSignals does not fit in UMPMessages"); + } + if let Some(peer_id) = self.approved_peer { + upward_messages + .try_push(UMPSignal::ApprovedPeer(peer_id).encode()) + .expect("UMPSignals does not fit in UMPMessages"); + } + } } #[cfg(test)] @@ -175,23 +304,44 @@ mod tests { use cumulus_primitives_core::{ CoreSelector, SchedulingInfoPayload, SchedulingProof, SignedSchedulingInfo, }; + use rstest::rstest; use sp_runtime::{generic::Header, traits::BlakeTwo256}; type RelayHeader = Header; + /// Claim-queue-offset cap used in tests. Matches the V3 value returned by + /// `pallet_parachain_system::max_claim_queue_offset()`. + const TEST_MAX_CQ_OFFSET: u8 = 2; + /// Creates a dummy signature blob for testing (not cryptographically valid). fn dummy_signature() -> [u8; 64] { [0u8; 64] } - /// Creates a chain of headers where each header's parent_hash points to the next, plus the - /// relay header at `internal_scheduling_parent` (its hash equals the chain's last header's - /// `parent_hash`, or `scheduling_parent` for an empty chain). + /// Builds a `SignedSchedulingInfo` with the given core selector, ISP, and a dummy + /// signature. `claim_queue_offset` and `peer_id` use default/zero values. /// - /// Returns: - /// - chain headers ordered newest-to-oldest (index 0 = newest = scheduling_parent), - /// - and the internal scheduling parent header. + /// `check_scheduling` cross-checks `payload.internal_scheduling_parent` against the + /// ISP derived from the proof, so callers must pass the ISP the proof points to (or + /// a deliberately-mismatched value to exercise the rejection path). + fn dummy_signed(core_selector: CoreSelector, isp: RelayHash) -> SignedSchedulingInfo { + SignedSchedulingInfo { + payload: SchedulingInfoPayload::new(core_selector, 0, Default::default(), isp), + signature: dummy_signature(), + } + } + + /// Creates a chain of headers where each header's parent_hash points to the next, + /// plus the relay header at `internal_scheduling_parent` (ISP). The ISP header's + /// hash equals the chain's last header's `parent_hash`, or coincides with + /// `scheduling_parent` when the chain is empty. + /// + /// Returns the chain headers ordered newest-to-oldest (index 0 = newest = + /// `scheduling_parent`) and the ISP header. Tests pick their own `relay_parent`: + /// `isp_header.hash()` for initial submission, an unrelated hash for resubmission. fn make_header_chain(len: usize) -> (Vec, RelayHeader) { + // Construct the ISP header first so we can derive its hash and build the chain + // on top of it. let isp_header = RelayHeader::new( 0u32, Default::default(), @@ -199,14 +349,13 @@ mod tests { Default::default(), Default::default(), ); - let relay_parent = isp_header.hash(); if len == 0 { return (vec![], isp_header); } let mut headers = Vec::with_capacity(len); - let mut parent_hash = relay_parent; + let mut parent_hash = isp_header.hash(); for i in 0..len { let header = RelayHeader::new( @@ -220,7 +369,7 @@ mod tests { headers.push(header); } - // Reverse so newest is first (matches expected ordering) + // Reverse so newest is first (matches expected ordering). headers.reverse(); (headers, isp_header) } @@ -229,104 +378,77 @@ mod tests { // Valid cases // ========================================================================= - #[test] - fn valid_header_chain_length_3() { - // Test: A valid 3-header chain should validate successfully. - let (headers, isp_header) = make_header_chain(3); - let relay_parent = isp_header.hash(); + #[rstest] + #[case::len_1(1)] + #[case::len_3(3)] + fn valid_non_empty_header_chain(#[case] len: usize) { + // Valid N-header chain on initial submission (`relay_parent == ISP`): validation + // passes and `internal_scheduling_parent == relay_parent`. Length 0 is structurally + // different (no chain headers) and lives in its own test. + let (headers, isp_header) = make_header_chain(len); let scheduling_parent = headers[0].hash(); + let relay_parent = isp_header.hash(); let proof = SchedulingProof { header_chain: headers, - internal_scheduling_parent_header: isp_header.clone(), + internal_scheduling_parent_header: isp_header, signed_scheduling_info: None, }; - let result = check_scheduling(&proof, relay_parent, scheduling_parent, 3); - - assert!(result.is_ok()); - // internal_scheduling_parent should equal relay_parent for valid chains - assert_eq!(result.unwrap().internal_scheduling_parent, relay_parent); + let result = check_scheduling( + &proof, + relay_parent, + scheduling_parent, + len as u32, + TEST_MAX_CQ_OFFSET, + ) + .expect("valid chain should pass"); + assert_eq!(result, relay_parent); } #[test] fn valid_empty_header_chain() { - // Test: Empty chain (offset=0) means scheduling_parent == relay_parent. + // Empty chain (offset=0) means scheduling_parent == relay_parent and the + // ISP header must hash to scheduling_parent. let (_, isp_header) = make_header_chain(0); let scheduling_parent = isp_header.hash(); - let relay_parent = scheduling_parent; + let relay_parent = scheduling_parent; // Must be equal for offset=0 let proof = SchedulingProof { header_chain: vec![], internal_scheduling_parent_header: isp_header, signed_scheduling_info: None, }; - let result = check_scheduling(&proof, relay_parent, scheduling_parent, 0); - - assert!(result.is_ok()); - assert_eq!(result.unwrap().internal_scheduling_parent, scheduling_parent); - } - - #[test] - fn valid_single_header_chain() { - // Test: Single header chain (offset=1). - let (headers, isp_header) = make_header_chain(1); - let relay_parent = isp_header.hash(); - let scheduling_parent = headers[0].hash(); - - let proof = SchedulingProof { - header_chain: headers, - internal_scheduling_parent_header: isp_header.clone(), - signed_scheduling_info: None, - }; - let result = check_scheduling(&proof, relay_parent, scheduling_parent, 1); - - assert!(result.is_ok()); - assert_eq!(result.unwrap().internal_scheduling_parent, relay_parent); + let result = + check_scheduling(&proof, relay_parent, scheduling_parent, 0, TEST_MAX_CQ_OFFSET) + .expect("valid empty chain should pass"); + assert_eq!(result, scheduling_parent); } // ========================================================================= // Invalid length cases // ========================================================================= - #[test] - fn reject_wrong_header_chain_length_too_short() { - // Test: Chain shorter than expected should be rejected. - let (headers, isp_header) = make_header_chain(2); - let relay_parent = isp_header.hash(); + #[rstest] + #[case::too_short(2)] + #[case::too_long(4)] + fn reject_wrong_header_chain_length(#[case] actual: usize) { + // Chain whose length doesn't match the expected (3) is rejected with + // `InvalidHeaderChainLength`, both when too short and when too long. + let (headers, isp_header) = make_header_chain(actual); let scheduling_parent = headers[0].hash(); - - let proof = SchedulingProof { - header_chain: headers, - internal_scheduling_parent_header: isp_header.clone(), - signed_scheduling_info: None, - }; - // Expect 3, but only 2 provided - let result = check_scheduling(&proof, relay_parent, scheduling_parent, 3); - - assert_eq!( - result, - Err(SchedulingValidationError::InvalidHeaderChainLength { expected: 3, actual: 2 }) - ); - } - - #[test] - fn reject_wrong_header_chain_length_too_long() { - // Test: Chain longer than expected should be rejected. - let (headers, isp_header) = make_header_chain(4); let relay_parent = isp_header.hash(); - let scheduling_parent = headers[0].hash(); let proof = SchedulingProof { header_chain: headers, - internal_scheduling_parent_header: isp_header.clone(), + internal_scheduling_parent_header: isp_header, signed_scheduling_info: None, }; - // Expect 3, but 4 provided - let result = check_scheduling(&proof, relay_parent, scheduling_parent, 3); + let result = + check_scheduling(&proof, relay_parent, scheduling_parent, 3, TEST_MAX_CQ_OFFSET); assert_eq!( result, - Err(SchedulingValidationError::InvalidHeaderChainLength { expected: 3, actual: 4 }) + Err(SchedulingValidationError::InvalidHeaderChainLength { expected: 3, actual }) ); } @@ -343,10 +465,11 @@ mod tests { let proof = SchedulingProof { header_chain: headers, - internal_scheduling_parent_header: isp_header.clone(), + internal_scheduling_parent_header: isp_header, signed_scheduling_info: None, }; - let result = check_scheduling(&proof, relay_parent, wrong_scheduling_parent, 3); + let result = + check_scheduling(&proof, relay_parent, wrong_scheduling_parent, 3, TEST_MAX_CQ_OFFSET); assert_eq!(result, Err(SchedulingValidationError::SchedulingParentMismatch)); } @@ -359,8 +482,8 @@ mod tests { fn reject_broken_header_chain() { // Test: Headers must form a valid chain via parent_hash linkage. let (mut headers, isp_header) = make_header_chain(3); - let relay_parent = isp_header.hash(); let scheduling_parent = headers[0].hash(); + let relay_parent = isp_header.hash(); // Corrupt the middle header's parent_hash to break the chain headers[1] = RelayHeader::new( @@ -373,10 +496,11 @@ mod tests { let proof = SchedulingProof { header_chain: headers, - internal_scheduling_parent_header: isp_header.clone(), + internal_scheduling_parent_header: isp_header, signed_scheduling_info: None, }; - let result = check_scheduling(&proof, relay_parent, scheduling_parent, 3); + let result = + check_scheduling(&proof, relay_parent, scheduling_parent, 3, TEST_MAX_CQ_OFFSET); // Chain breaks at index 0 (first header's parent doesn't match second header's hash) assert_eq!(result, Err(SchedulingValidationError::BrokenHeaderChain { index: 0 })); @@ -397,10 +521,16 @@ mod tests { let proof = SchedulingProof { header_chain: headers, - internal_scheduling_parent_header: isp_header.clone(), + internal_scheduling_parent_header: isp_header, signed_scheduling_info: None, }; - let result = check_scheduling(&proof, relay_parent_in_chain, scheduling_parent, 3); + let result = check_scheduling( + &proof, + relay_parent_in_chain, + scheduling_parent, + 3, + TEST_MAX_CQ_OFFSET, + ); assert_eq!(result, Err(SchedulingValidationError::RelayParentInHeaderChain)); } @@ -415,30 +545,21 @@ mod tests { // optionally include signed_scheduling_info. This is legal because collators // should refuse to acknowledge blocks with invalid scheduling info anyway. let (headers, isp_header) = make_header_chain(3); - let relay_parent = isp_header.hash(); let scheduling_parent = headers[0].hash(); + let relay_parent = isp_header.hash(); - let signed_info = SignedSchedulingInfo { - payload: SchedulingInfoPayload::new( - CoreSelector(0), - 0, - Default::default(), - relay_parent, - ), - signature: dummy_signature(), - }; + let signed_info = dummy_signed(CoreSelector(0), isp_header.hash()); let proof = SchedulingProof { header_chain: headers, - internal_scheduling_parent_header: isp_header.clone(), + internal_scheduling_parent_header: isp_header, signed_scheduling_info: Some(signed_info), }; - let result = check_scheduling(&proof, relay_parent, scheduling_parent, 3); + let result = + check_scheduling(&proof, relay_parent, scheduling_parent, 3, TEST_MAX_CQ_OFFSET); // Validation passes - signed_scheduling_info is optional for initial submission assert!(result.is_ok()); - let result = result.unwrap(); - assert!(!result.is_resubmission); } #[test] @@ -452,10 +573,11 @@ mod tests { let proof = SchedulingProof { header_chain: headers, - internal_scheduling_parent_header: isp_header.clone(), + internal_scheduling_parent_header: isp_header, signed_scheduling_info: None, }; - let result = check_scheduling(&proof, older_relay_parent, scheduling_parent, 3); + let result = + check_scheduling(&proof, older_relay_parent, scheduling_parent, 3, TEST_MAX_CQ_OFFSET); assert_eq!(result, Err(SchedulingValidationError::MissingSignedSchedulingInfo)); } @@ -465,54 +587,25 @@ mod tests { // Test: Resubmission with signed_scheduling_info passes validation // (signature verification happens separately). let (headers, isp_header) = make_header_chain(3); - let internal_scheduling_parent = isp_header.hash(); let scheduling_parent = headers[0].hash(); + let internal_scheduling_parent = isp_header.hash(); // Use an unrelated hash as relay_parent (simulates resubmission where // relay_parent is an ancestor of internal_scheduling_parent) let older_relay_parent = RelayHash::repeat_byte(0xBB); - let signed_info = SignedSchedulingInfo { - payload: SchedulingInfoPayload::new( - CoreSelector(0), - 0, - Default::default(), - internal_scheduling_parent, - ), - signature: dummy_signature(), - }; + let signed_info = dummy_signed(CoreSelector(0), internal_scheduling_parent); let proof = SchedulingProof { header_chain: headers, - internal_scheduling_parent_header: isp_header.clone(), + internal_scheduling_parent_header: isp_header, signed_scheduling_info: Some(signed_info), }; - let result = check_scheduling(&proof, older_relay_parent, scheduling_parent, 3); + let result = + check_scheduling(&proof, older_relay_parent, scheduling_parent, 3, TEST_MAX_CQ_OFFSET); // Validation passes - signature verification is done separately - assert!(result.is_ok()); - let result = result.unwrap(); - assert!(result.is_resubmission); - assert_eq!(result.internal_scheduling_parent, internal_scheduling_parent); - } - - #[test] - fn initial_submission_is_not_resubmission() { - // Test: Initial submission has is_resubmission = false - let (headers, isp_header) = make_header_chain(3); - let relay_parent = isp_header.hash(); - let scheduling_parent = headers[0].hash(); - - let proof = SchedulingProof { - header_chain: headers, - internal_scheduling_parent_header: isp_header.clone(), - signed_scheduling_info: None, - }; - let result = check_scheduling(&proof, relay_parent, scheduling_parent, 3); - - assert!(result.is_ok()); - let result = result.unwrap(); - assert!(!result.is_resubmission); - assert_eq!(result.internal_scheduling_parent, relay_parent); + let result = result.expect("valid resubmission proof"); + assert_eq!(result, internal_scheduling_parent); } // ========================================================================= @@ -520,31 +613,26 @@ mod tests { // ========================================================================= /// Helper: builds a valid V3 extension and scheduling proof for a given header chain length. - /// Returns (extension, proof, expected_result). + /// Returns (extension, proof, expected_internal_scheduling_parent). fn make_v3_initial_submission( chain_len: u32, - ) -> (ValidationParamsExtension, SchedulingProof, SchedulingValidationResult) { + ) -> (ValidationParamsExtension, SchedulingProof, RelayHash) { let (headers, isp_header) = make_header_chain(chain_len as usize); - let scheduling_parent = - if headers.is_empty() { isp_header.hash() } else { headers[0].hash() }; + let relay_parent = isp_header.hash(); + let scheduling_parent = if headers.is_empty() { relay_parent } else { headers[0].hash() }; - let extension = - ValidationParamsExtension::V3 { relay_parent: isp_header.hash(), scheduling_parent }; + let extension = ValidationParamsExtension::V3 { relay_parent, scheduling_parent }; let proof = SchedulingProof { header_chain: headers, - internal_scheduling_parent_header: isp_header.clone(), + internal_scheduling_parent_header: isp_header, signed_scheduling_info: None, }; - let expected = SchedulingValidationResult { - internal_scheduling_parent: isp_header.hash(), - is_resubmission: false, - }; - (extension, proof, expected) + (extension, proof, relay_parent) } #[test] fn v3_disabled_no_extension_returns_none() { - let result = validate_v3_scheduling(false, &None, None, 0); + let result = validate_v3_scheduling(false, &None, None, 0, TEST_MAX_CQ_OFFSET); assert!(result.is_none()); } @@ -555,26 +643,22 @@ mod tests { relay_parent: RelayHash::default(), scheduling_parent: RelayHash::default(), }; - validate_v3_scheduling(false, &Some(ext), None, 0); + validate_v3_scheduling(false, &Some(ext), None, 0, TEST_MAX_CQ_OFFSET); } #[test] #[should_panic(expected = "V3 scheduling is enabled but no V3 extension present")] fn v3_enabled_no_extension_panics() { - validate_v3_scheduling(true, &None, None, 0); - } - - #[test] - fn v3_enabled_valid_initial_submission() { - let (ext, proof, expected) = make_v3_initial_submission(3); - let result = validate_v3_scheduling(true, &Some(ext), Some(&proof), 3); - assert_eq!(result, Some(expected)); + validate_v3_scheduling(true, &None, None, 0, TEST_MAX_CQ_OFFSET); } - #[test] - fn v3_enabled_valid_empty_header_chain() { - let (ext, proof, expected) = make_v3_initial_submission(0); - let result = validate_v3_scheduling(true, &Some(ext), Some(&proof), 0); + #[rstest] + #[case::empty(0)] + #[case::len_3(3)] + fn v3_enabled_valid_initial_submission(#[case] chain_len: u32) { + let (ext, proof, expected) = make_v3_initial_submission(chain_len); + let result = + validate_v3_scheduling(true, &Some(ext), Some(&proof), chain_len, TEST_MAX_CQ_OFFSET); assert_eq!(result, Some(expected)); } @@ -583,7 +667,7 @@ mod tests { fn v3_enabled_missing_scheduling_proof_panics() { let (ext, _, _) = make_v3_initial_submission(3); // Pass None as scheduling_proof to simulate a V0/V1 POV - validate_v3_scheduling(true, &Some(ext), None, 3); + validate_v3_scheduling(true, &Some(ext), None, 3, TEST_MAX_CQ_OFFSET); } #[test] @@ -591,13 +675,14 @@ mod tests { fn v3_enabled_invalid_header_chain_length_panics() { let (ext, proof, _) = make_v3_initial_submission(3); // Expect 5 headers but proof only has 3 - validate_v3_scheduling(true, &Some(ext), Some(&proof), 5); + validate_v3_scheduling(true, &Some(ext), Some(&proof), 5, TEST_MAX_CQ_OFFSET); } #[test] fn v3_enabled_valid_resubmission() { let (headers, isp_header) = make_header_chain(3); let scheduling_parent = headers[0].hash(); + let internal_scheduling_parent = isp_header.hash(); // Use an unrelated hash as relay_parent to simulate a resubmission let older_relay_parent = RelayHash::repeat_byte(0xBB); @@ -605,22 +690,13 @@ mod tests { ValidationParamsExtension::V3 { relay_parent: older_relay_parent, scheduling_parent }; let proof = SchedulingProof { header_chain: headers, - internal_scheduling_parent_header: isp_header.clone(), - signed_scheduling_info: Some(SignedSchedulingInfo { - payload: SchedulingInfoPayload::new( - CoreSelector(0), - 0, - Default::default(), - isp_header.hash(), - ), - signature: dummy_signature(), - }), + internal_scheduling_parent_header: isp_header, + signed_scheduling_info: Some(dummy_signed(CoreSelector(0), internal_scheduling_parent)), }; - let result = validate_v3_scheduling(true, &Some(ext), Some(&proof), 3); + let result = validate_v3_scheduling(true, &Some(ext), Some(&proof), 3, TEST_MAX_CQ_OFFSET); let result = result.expect("should succeed"); - assert!(result.is_resubmission); - assert_eq!(result.internal_scheduling_parent, isp_header.hash()); + assert_eq!(result, internal_scheduling_parent); } #[test] @@ -634,11 +710,334 @@ mod tests { ValidationParamsExtension::V3 { relay_parent: older_relay_parent, scheduling_parent }; let proof = SchedulingProof { header_chain: headers, - internal_scheduling_parent_header: isp_header.clone(), + internal_scheduling_parent_header: isp_header, signed_scheduling_info: None, }; // Should panic because resubmission requires signed_scheduling_info - validate_v3_scheduling(true, &Some(ext), Some(&proof), 3); + validate_v3_scheduling(true, &Some(ext), Some(&proof), 3, TEST_MAX_CQ_OFFSET); + } + + #[test] + fn empty_chain_with_signed_info_passes_when_relay_parent_matches() { + // With an empty chain and `relay_parent == scheduling_parent`, the candidate + // is an initial submission. An accompanying `signed_scheduling_info` is legal + // (collators may refuse stale info, but `check_scheduling` doesn't forbid it). + let (_, isp_header) = make_header_chain(0); + let scheduling_parent = isp_header.hash(); + let relay_parent = scheduling_parent; + let proof = SchedulingProof { + header_chain: vec![], + internal_scheduling_parent_header: isp_header, + signed_scheduling_info: Some(dummy_signed(CoreSelector(0), scheduling_parent)), + }; + let result = + check_scheduling(&proof, relay_parent, scheduling_parent, 0, TEST_MAX_CQ_OFFSET); + assert!(result.is_ok()); + } + + #[test] + fn empty_chain_with_mismatched_relay_parent_is_resubmission() { + // With `RelayParentOffset = 0` the header chain is always empty, for both + // initial submissions and resubmissions. When `relay_parent != scheduling_parent` + // the candidate is a resubmission: `internal_scheduling_parent` falls back to + // `scheduling_parent`, and the linkage check (against the proof's ISP header) + // is what ultimately rejects an inconsistent proof. + let (_, isp_header) = make_header_chain(0); + let scheduling_parent = isp_header.hash(); + let relay_parent = RelayHash::repeat_byte(0xBB); + let proof = SchedulingProof { + header_chain: vec![], + internal_scheduling_parent_header: isp_header, + signed_scheduling_info: Some(dummy_signed(CoreSelector(0), scheduling_parent)), + }; + let result = + check_scheduling(&proof, relay_parent, scheduling_parent, 0, TEST_MAX_CQ_OFFSET) + .unwrap(); + assert_eq!(result, scheduling_parent); + } + + #[test] + fn empty_chain_resubmission_without_signed_info_is_rejected() { + // Empty chain + `relay_parent != scheduling_parent` is treated as a resubmission; + // without `signed_scheduling_info` we reject as we would for any other resubmission. + let (_, isp_header) = make_header_chain(0); + let scheduling_parent = isp_header.hash(); + let relay_parent = RelayHash::repeat_byte(0xBB); + let proof = SchedulingProof { + header_chain: vec![], + internal_scheduling_parent_header: isp_header, + signed_scheduling_info: None, + }; + let result = + check_scheduling(&proof, relay_parent, scheduling_parent, 0, TEST_MAX_CQ_OFFSET); + assert_eq!(result, Err(SchedulingValidationError::MissingSignedSchedulingInfo)); + } + + #[test] + fn reject_unlinked_internal_scheduling_parent_header() { + // ISP header that does not hash to the derived internal_scheduling_parent must + // be rejected: otherwise a collator could point the verifier at an arbitrary + // slot to satisfy the author lookup. + let (headers, real_isp_header) = make_header_chain(3); + let scheduling_parent = headers[0].hash(); + let relay_parent = real_isp_header.hash(); + // An unrelated header with a different block number → different hash. + let unrelated_isp_header = RelayHeader::new( + 42u32, + Default::default(), + Default::default(), + Default::default(), + Default::default(), + ); + + let proof = SchedulingProof { + header_chain: headers, + internal_scheduling_parent_header: unrelated_isp_header, + signed_scheduling_info: None, + }; + let result = + check_scheduling(&proof, relay_parent, scheduling_parent, 3, TEST_MAX_CQ_OFFSET); + assert_eq!(result, Err(SchedulingValidationError::InternalSchedulingParentHeaderMismatch)); + } + + #[test] + fn reject_signed_info_with_mismatched_isp() { + // A signed payload whose `internal_scheduling_parent` doesn't match the ISP + // derived from the proof must be rejected here, not just at signature-verifier + // time. Without this, an eligible author could sign a payload claiming a stale + // ISP and the verifier's signature check would still succeed over those bytes. + let (headers, isp_header) = make_header_chain(3); + let scheduling_parent = headers[0].hash(); + let older_relay_parent = RelayHash::repeat_byte(0xBB); + + // Payload commits to a different ISP than the proof carries. + let wrong_isp = RelayHash::repeat_byte(0xCC); + let signed_info = dummy_signed(CoreSelector(0), wrong_isp); + + let proof = SchedulingProof { + header_chain: headers, + internal_scheduling_parent_header: isp_header, + signed_scheduling_info: Some(signed_info), + }; + let result = + check_scheduling(&proof, older_relay_parent, scheduling_parent, 3, TEST_MAX_CQ_OFFSET); + assert_eq!(result, Err(SchedulingValidationError::SignedSchedulingInfoIspMismatch)); + } + + // ========================================================================= + // claim_queue_offset bound (step 7b) tests + // ========================================================================= + + /// Build a resubmission proof (empty chain, `relay_parent != scheduling_parent`) whose + /// signed payload carries the given `claim_queue_offset`. Used to drive the offset-bound + /// check in `check_scheduling`. + fn resubmission_proof_with_offset(offset: u8) -> (SchedulingProof, RelayHash, RelayHash) { + let (_, isp_header) = make_header_chain(0); + let scheduling_parent = isp_header.hash(); + let relay_parent = RelayHash::repeat_byte(0xBB); + let signed = SignedSchedulingInfo { + payload: SchedulingInfoPayload::new( + CoreSelector(0), + offset, + Default::default(), + scheduling_parent, + ), + signature: dummy_signature(), + }; + let proof = SchedulingProof { + header_chain: vec![], + internal_scheduling_parent_header: isp_header, + signed_scheduling_info: Some(signed), + }; + (proof, relay_parent, scheduling_parent) + } + + #[test] + fn reject_resubmission_offset_exceeding_cap() { + // A signed offset above the runtime cap is rejected: on resubmission the offset is + // taken from the signed payload and overrides the block's emitted value, so the + // bound must be re-applied here (the in-block check is bypassed). + let (proof, relay_parent, scheduling_parent) = + resubmission_proof_with_offset(TEST_MAX_CQ_OFFSET + 1); + let result = + check_scheduling(&proof, relay_parent, scheduling_parent, 0, TEST_MAX_CQ_OFFSET); + assert_eq!( + result, + Err(SchedulingValidationError::ClaimQueueOffsetTooLarge { + offset: TEST_MAX_CQ_OFFSET + 1, + max: TEST_MAX_CQ_OFFSET, + }) + ); + } + + #[test] + fn accept_resubmission_offset_at_cap() { + // An offset exactly at the cap is within bounds and passes. + let (proof, relay_parent, scheduling_parent) = + resubmission_proof_with_offset(TEST_MAX_CQ_OFFSET); + check_scheduling(&proof, relay_parent, scheduling_parent, 0, TEST_MAX_CQ_OFFSET) + .expect("offset at cap is valid"); + } + + // ========================================================================= + // SchedulingSignals tests + // ========================================================================= + + fn signed_with( + core_selector: CoreSelector, + claim_queue_offset: u8, + peer_id: ApprovedPeerId, + ) -> SignedSchedulingInfo { + SignedSchedulingInfo { + payload: SchedulingInfoPayload::new( + core_selector, + claim_queue_offset, + peer_id, + Default::default(), + ), + signature: [0u8; 64], + } + } + + fn peer(byte: u8) -> ApprovedPeerId { + ApprovedPeerId::try_from(vec![byte; 4]).expect("4 bytes fits the bound; qed") + } + + /// A `BoundedVec` with the same shape as `validate_block`'s `upward_messages`, for + /// exercising `SchedulingSignals::emit`. + type TestUpwardMessages = BoundedVec, frame_support::traits::ConstU32<1024>>; + + #[test] + fn from_block_signals_roundtrips_select_core_and_approved_peer() { + // Both signals present: parsed into the canonical tail, then emitted as + // [SEPARATOR, SelectCore, ApprovedPeer] in that exact order. + let raw = vec![ + UMPSignal::SelectCore(CoreSelector(7), ClaimQueueOffset(1)).encode(), + UMPSignal::ApprovedPeer(peer(0xAA)).encode(), + ]; + let signals = SchedulingSignals::from_block_signals(&raw); + + let mut out = TestUpwardMessages::default(); + signals.emit(&mut out); + assert_eq!( + out.into_inner(), + vec![ + UMP_SEPARATOR, + UMPSignal::SelectCore(CoreSelector(7), ClaimQueueOffset(1)).encode(), + UMPSignal::ApprovedPeer(peer(0xAA)).encode(), + ] + ); + } + + #[test] + fn from_block_signals_select_core_only() { + // Block emitted only a `SelectCore`: no `ApprovedPeer` field, one signal emitted. + let raw = vec![UMPSignal::SelectCore(CoreSelector(3), ClaimQueueOffset(0)).encode()]; + let signals = SchedulingSignals::from_block_signals(&raw); + + let mut out = TestUpwardMessages::default(); + signals.emit(&mut out); + assert_eq!( + out.into_inner(), + vec![ + UMP_SEPARATOR, + UMPSignal::SelectCore(CoreSelector(3), ClaimQueueOffset(0)).encode() + ] + ); + } + + #[test] + #[should_panic(expected = "more than one `SelectCore`")] + fn from_block_signals_panics_on_duplicate_select_core_same_value() { + // Two identical `SelectCore` signals: still an error. The relay decoder counts + // occurrences, not distinct values, so matching duplicates would be rejected too. + let raw = vec![ + UMPSignal::SelectCore(CoreSelector(1), ClaimQueueOffset(0)).encode(), + UMPSignal::SelectCore(CoreSelector(1), ClaimQueueOffset(0)).encode(), + ]; + let _ = SchedulingSignals::from_block_signals(&raw); + } + + #[test] + #[should_panic(expected = "more than one `SelectCore`")] + fn from_block_signals_panics_on_duplicate_select_core_different_value() { + let raw = vec![ + UMPSignal::SelectCore(CoreSelector(1), ClaimQueueOffset(0)).encode(), + UMPSignal::SelectCore(CoreSelector(2), ClaimQueueOffset(0)).encode(), + ]; + let _ = SchedulingSignals::from_block_signals(&raw); + } + + #[test] + #[should_panic(expected = "more than one `ApprovedPeer`")] + fn from_block_signals_panics_on_duplicate_approved_peer() { + let raw = vec![ + UMPSignal::ApprovedPeer(peer(0xAA)).encode(), + UMPSignal::ApprovedPeer(peer(0xBB)).encode(), + ]; + let _ = SchedulingSignals::from_block_signals(&raw); + } + + #[test] + fn from_block_signals_empty_emits_nothing() { + // No signals in, nothing out — not even a separator. + let signals = SchedulingSignals::from_block_signals(&[]); + assert!(signals.is_empty()); + + let mut out = TestUpwardMessages::default(); + signals.emit(&mut out); + assert!(out.is_empty()); + } + + #[test] + fn from_scheduling_info_sources_all_fields() { + // All three values — `core_selector`, `claim_queue_offset`, `peer_id` — are signed + // by the resubmitting collator, so the override sources every field from the signed + // payload. Distinct values ensure no field is sourced from the wrong place. + let signed = signed_with(CoreSelector(7), 3, peer(0xAA)); + let signals = SchedulingSignals::from_scheduling_info(&signed); + + let mut out = TestUpwardMessages::default(); + signals.emit(&mut out); + assert_eq!( + out.into_inner(), + vec![ + UMP_SEPARATOR, + UMPSignal::SelectCore(CoreSelector(7), ClaimQueueOffset(3)).encode(), + UMPSignal::ApprovedPeer(peer(0xAA)).encode(), + ] + ); + } + + #[test] + fn from_scheduling_info_emits_peer_verbatim_even_if_empty() { + // The payload `peer_id` is a plain (non-`Option`) type → always-override. An empty + // peer is emitted verbatim as `ApprovedPeer([])`, NOT omitted and NOT replaced by the + // block's peer. Empty/invalid peers are the resubmitter's own reputation loss and are + // handled gracefully downstream; the PVF forwards exactly what was signed. + let signed = signed_with(CoreSelector(5), 1, ApprovedPeerId::default()); + let signals = SchedulingSignals::from_scheduling_info(&signed); + + let mut out = TestUpwardMessages::default(); + signals.emit(&mut out); + assert_eq!( + out.into_inner(), + vec![ + UMP_SEPARATOR, + UMPSignal::SelectCore(CoreSelector(5), ClaimQueueOffset(1)).encode(), + UMPSignal::ApprovedPeer(ApprovedPeerId::default()).encode(), + ] + ); + } + + #[test] + fn from_scheduling_info_emits_even_when_block_emitted_nothing() { + // The override is authoritative and independent of what the block emitted: a + // resubmission always produces its tail. (At the call site this is what decouples + // the override from the old `!upward_message_signals.is_empty()` guard.) + let signed = signed_with(CoreSelector(0), 0, peer(0xCC)); + let signals = SchedulingSignals::from_scheduling_info(&signed); + assert!(!signals.is_empty()); } } diff --git a/prdoc/pr_12097.prdoc b/prdoc/pr_12097.prdoc new file mode 100644 index 0000000000000..6b35da84b04d7 --- /dev/null +++ b/prdoc/pr_12097.prdoc @@ -0,0 +1,48 @@ +title: 'cumulus: add SignedSchedulingInfo PVF verification' +doc: +- audience: Runtime Dev + description: |- + Verifies the `SignedSchedulingInfo` payload inside the PVF whenever it is present — + required on V3 resubmissions (`relay_parent != internal_scheduling_parent`) and + verified on initial submissions when a collator attaches one. The verifier + confirms that the signature was produced by the parachain author eligible at the slot + derived from the relay header at `internal_scheduling_parent`, proving the submitting + collator owns the para slot at that anchor. + + Changes: + - `cumulus-pallet-aura-ext` exposes a new `AuraSchedulingVerifier` implementing + `cumulus_primitives_core::VerifySchedulingSignature`. It derives the para slot from + the BABE pre-digest of the supplied relay header, looks up the eligible Aura author + from the cached authority set, and verifies the signature over the encoded + `SchedulingInfoPayload`. + - `cumulus-pallet-parachain-system` invokes the configured `SchedulingSignatureVerifier` + from `validate_block` whenever a `signed_scheduling_info` is present, and overrides + the block's emitted UMP signals (`SelectCore`, `ApprovedPeer`) with the values from + the verified payload. + - Three new variants are added to `SchedulingValidationError`: + `InternalSchedulingParentHeaderMismatch`, `SignedSchedulingInfoIspMismatch`, and + `ClaimQueueOffsetTooLarge`. The first ensures the proof's + `internal_scheduling_parent_header` actually hashes to the derived ISP (so a collator + cannot point the slot oracle at an arbitrary header); the second ensures the signed + payload commits to the same ISP the proof points to; the third bounds the signed + `claim_queue_offset` by the runtime-enforced maximum. + + Integration: + - Parachains that want to enable resubmissions must set + `type SchedulingSignatureVerifier = AuraSchedulingVerifier` on + `cumulus_pallet_parachain_system::Config`. Runtimes that leave the default `()` + keep V3 scheduling disabled and are unaffected. + - Resubmissions should only be enabled once the relay chain supports them: the + `CandidateReceiptV3` node feature must be set on-chain, and the V4 collator + protocol must be in use between collators and validators (which requires both + validators and collators to have upgraded their node binaries to versions that + support the V4 collator protocol). + + Closes paritytech/polkadot-sdk#12152. +crates: +- name: cumulus-pallet-aura-ext + bump: minor +- name: cumulus-pallet-parachain-system + bump: patch +- name: pallet-aura + bump: minor diff --git a/substrate/frame/aura/src/lib.rs b/substrate/frame/aura/src/lib.rs index d9e989e450cf9..27570882e67d6 100644 --- a/substrate/frame/aura/src/lib.rs +++ b/substrate/frame/aura/src/lib.rs @@ -267,6 +267,16 @@ impl Pallet { Authorities::::decode_len().unwrap_or(0) } + /// Map a slot to an author index in the current authority set, or `None` if the set is + /// empty. The set size is read from [`Authorities`]. + pub fn slot_author_index(slot: Slot) -> Option { + let authorities_count = Self::authorities_len(); + if authorities_count == 0 { + return None; + } + Some((u64::from(slot) % authorities_count as u64) as u32) + } + /// Get the current slot from the pre-runtime digests. fn current_slot_from_digests() -> Option { let digest = frame_system::Pallet::::digest(); @@ -329,9 +339,10 @@ impl Pallet { frame_support::ensure!(!authorities_len.is_zero(), "Authorities must be non-empty."); // Check that the current authority is not disabled. - let authority_index = *current_slot % authorities_len as u64; + let authority_index = + Self::slot_author_index(current_slot).ok_or("Authorities must be non-empty.")?; frame_support::ensure!( - !T::DisabledValidators::is_disabled(authority_index as u32), + !T::DisabledValidators::is_disabled(authority_index), "Current validator is disabled and should not be attempting to author blocks.", ); @@ -407,8 +418,7 @@ impl FindAuthor for Pallet { for (id, mut data) in digests.into_iter() { if id == AURA_ENGINE_ID { let slot = Slot::decode(&mut data).ok()?; - let author_index = *slot % Self::authorities_len() as u64; - return Some(author_index as u32); + return Self::slot_author_index(slot); } }