diff --git a/Cargo.lock b/Cargo.lock index 3c55a14256c5..bedf57174906 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4847,6 +4847,8 @@ name = "cumulus-pallet-aura-ext" version = "0.7.0" dependencies = [ "cumulus-pallet-parachain-system 0.7.0", + "cumulus-primitives-core 0.7.0", + "cumulus-test-relay-sproof-builder 0.7.0", "frame-support 28.0.0", "frame-system 28.0.0", "pallet-aura 27.0.0", @@ -4855,7 +4857,10 @@ dependencies = [ "scale-info", "sp-application-crypto 30.0.0", "sp-consensus-aura 0.32.0", + "sp-core 28.0.0", + "sp-io 30.0.0", "sp-runtime 31.0.1", + "sp-version 29.0.0", ] [[package]] diff --git a/cumulus/client/consensus/aura/src/collators/lookahead.rs b/cumulus/client/consensus/aura/src/collators/lookahead.rs index 2dbcf5eb58e9..7723de5a576a 100644 --- a/cumulus/client/consensus/aura/src/collators/lookahead.rs +++ b/cumulus/client/consensus/aura/src/collators/lookahead.rs @@ -336,6 +336,7 @@ where ); Some(super::can_build_upon::<_, _, P>( slot_now, + relay_slot, timestamp, block_hash, included_block, diff --git a/cumulus/client/consensus/aura/src/collators/mod.rs b/cumulus/client/consensus/aura/src/collators/mod.rs index 89070607fbab..54f059fe2b61 100644 --- a/cumulus/client/consensus/aura/src/collators/mod.rs +++ b/cumulus/client/consensus/aura/src/collators/mod.rs @@ -34,7 +34,7 @@ use polkadot_primitives::{ ValidationCodeHash, }; use sc_consensus_aura::{standalone as aura_internal, AuraApi}; -use sp_api::ProvideRuntimeApi; +use sp_api::{ApiExt, ProvideRuntimeApi}; use sp_core::Pair; use sp_keystore::KeystorePtr; use sp_timestamp::Timestamp; @@ -160,7 +160,8 @@ async fn cores_scheduled_for_para( // Checks if we own the slot at the given block and whether there // is space in the unincluded segment. async fn can_build_upon( - slot: Slot, + para_slot: Slot, + relay_slot: Slot, timestamp: Timestamp, parent_hash: Block::Hash, included_block: Block::Hash, @@ -169,25 +170,29 @@ async fn can_build_upon( ) -> Option> where Client: ProvideRuntimeApi, - Client::Api: AuraApi + AuraUnincludedSegmentApi, + Client::Api: AuraApi + AuraUnincludedSegmentApi + ApiExt, P: Pair, P::Public: Codec, P::Signature: Codec, { let runtime_api = client.runtime_api(); let authorities = runtime_api.authorities(parent_hash).ok()?; - let author_pub = aura_internal::claim_slot::

(slot, &authorities, keystore).await?; + let author_pub = aura_internal::claim_slot::

(para_slot, &authorities, keystore).await?; - // Here we lean on the property that building on an empty unincluded segment must always - // be legal. Skipping the runtime API query here allows us to seamlessly run this - // collator against chains which have not yet upgraded their runtime. - if parent_hash != included_block && - !runtime_api.can_build_upon(parent_hash, included_block, slot).ok()? - { - return None - } + let Ok(Some(api_version)) = + runtime_api.api_version::>(parent_hash) + else { + return (parent_hash == included_block) + .then(|| SlotClaim::unchecked::

(author_pub, para_slot, timestamp)); + }; - Some(SlotClaim::unchecked::

(author_pub, slot, timestamp)) + let slot = if api_version > 1 { relay_slot } else { para_slot }; + + if runtime_api.can_build_upon(parent_hash, included_block, slot).ok()? { + Some(SlotClaim::unchecked::

(author_pub, para_slot, timestamp)) + } else { + None + } } /// Use [`cumulus_client_consensus_common::find_potential_parents`] to find parachain blocks that diff --git a/cumulus/client/consensus/aura/src/collators/slot_based/block_builder_task.rs b/cumulus/client/consensus/aura/src/collators/slot_based/block_builder_task.rs index 41751f1db530..48287555dea6 100644 --- a/cumulus/client/consensus/aura/src/collators/slot_based/block_builder_task.rs +++ b/cumulus/client/consensus/aura/src/collators/slot_based/block_builder_task.rs @@ -23,7 +23,7 @@ use cumulus_primitives_aura::AuraUnincludedSegmentApi; use cumulus_primitives_core::{GetCoreSelectorApi, PersistedValidationData}; use cumulus_relay_chain_interface::RelayChainInterface; -use polkadot_primitives::Id as ParaId; +use polkadot_primitives::{Block as RelayBlock, Id as ParaId}; use futures::prelude::*; use sc_client_api::{backend::AuxStore, BlockBackend, BlockOf, UsageProvider}; @@ -302,8 +302,17 @@ where // on-chain data. collator.collator_service().check_block_status(parent_hash, &parent_header); + let Ok(relay_slot) = + sc_consensus_babe::find_pre_digest::(relay_parent_header) + .map(|babe_pre_digest| babe_pre_digest.slot()) + else { + tracing::error!(target: crate::LOG_TARGET, "Relay chain does not contain babe slot. This should never happen."); + continue; + }; + let slot_claim = match crate::collators::can_build_upon::<_, _, P>( para_slot.slot, + relay_slot, para_slot.timestamp, parent_hash, included_block, diff --git a/cumulus/client/parachain-inherent/src/mock.rs b/cumulus/client/parachain-inherent/src/mock.rs index e08aca932564..8dbc6ace0f06 100644 --- a/cumulus/client/parachain-inherent/src/mock.rs +++ b/cumulus/client/parachain-inherent/src/mock.rs @@ -17,8 +17,9 @@ use crate::{ParachainInherentData, INHERENT_IDENTIFIER}; use codec::Decode; use cumulus_primitives_core::{ - relay_chain, relay_chain::UpgradeGoAhead, InboundDownwardMessage, InboundHrmpMessage, ParaId, - PersistedValidationData, + relay_chain, + relay_chain::{Slot, UpgradeGoAhead}, + InboundDownwardMessage, InboundHrmpMessage, ParaId, PersistedValidationData, }; use cumulus_primitives_parachain_inherent::MessageQueueChain; use cumulus_test_relay_sproof_builder::RelayStateSproofBuilder; @@ -28,9 +29,6 @@ use sp_inherents::{InherentData, InherentDataProvider}; use sp_runtime::traits::Block; use std::collections::BTreeMap; -/// Relay chain slot duration, in milliseconds. -pub const RELAY_CHAIN_SLOT_DURATION_MILLIS: u32 = 6000; - /// Inherent data provider that supplies mocked validation data. /// /// This is useful when running a node that is not actually backed by any relay chain. @@ -175,8 +173,7 @@ impl> InherentDataProvider // Calculate the mocked relay block based on the current para block let relay_parent_number = self.relay_offset + self.relay_blocks_per_para_block * self.current_para_block; - sproof_builder.current_slot = - ((relay_parent_number / RELAY_CHAIN_SLOT_DURATION_MILLIS) as u64).into(); + sproof_builder.current_slot = Slot::from(relay_parent_number as u64); sproof_builder.upgrade_go_ahead = self.upgrade_go_ahead; // Process the downward messages and set up the correct head diff --git a/cumulus/pallets/aura-ext/Cargo.toml b/cumulus/pallets/aura-ext/Cargo.toml index fcda79f1d5c1..7484428e0da2 100644 --- a/cumulus/pallets/aura-ext/Cargo.toml +++ b/cumulus/pallets/aura-ext/Cargo.toml @@ -28,9 +28,15 @@ sp-runtime = { workspace = true } cumulus-pallet-parachain-system = { workspace = true } [dev-dependencies] - # Cumulus cumulus-pallet-parachain-system = { workspace = true, default-features = true } +cumulus-test-relay-sproof-builder = { workspace = true, default-features = true } +cumulus-primitives-core = { workspace = true, default-features = true } + +# Substrate +sp-io = { workspace = true, default-features = true } +sp-core = { workspace = true, default-features = true } +sp-version = { workspace = true, default-features = true } [features] default = ["std"] diff --git a/cumulus/pallets/aura-ext/src/consensus_hook.rs b/cumulus/pallets/aura-ext/src/consensus_hook.rs index c1a8568bdd83..56966aa0c8f8 100644 --- a/cumulus/pallets/aura-ext/src/consensus_hook.rs +++ b/cumulus/pallets/aura-ext/src/consensus_hook.rs @@ -18,7 +18,6 @@ //! block velocity. //! //! The velocity `V` refers to the rate of block processing by the relay chain. - use super::{pallet, Aura}; use core::{marker::PhantomData, num::NonZeroU32}; use cumulus_pallet_parachain_system::{ @@ -54,8 +53,23 @@ where let velocity = V.max(1); let relay_chain_slot = state_proof.read_slot().expect("failed to read relay chain slot"); - let (slot, authored) = - pallet::SlotInfo::::get().expect("slot info is inserted on block initialization"); + let (relay_chain_slot, authored_in_relay) = match pallet::RelaySlotInfo::::get() { + Some((slot, authored)) if slot == relay_chain_slot => (slot, authored), + Some((slot, _)) if slot < relay_chain_slot => (relay_chain_slot, 0), + Some((slot, _)) => { + panic!("Slot moved backwards: stored_slot={slot:?}, relay_chain_slot={relay_chain_slot:?}") + }, + None => (relay_chain_slot, 0), + }; + + // We need to allow one additional block to be built to fill the unincluded segment. + if authored_in_relay > velocity { + panic!("authored blocks limit is reached for the slot: relay_chain_slot={relay_chain_slot:?}, authored={authored_in_relay:?}, velocity={velocity:?}"); + } + + pallet::RelaySlotInfo::::put((relay_chain_slot, authored_in_relay + 1)); + + let para_slot = pallet_aura::CurrentSlot::::get(); // Convert relay chain timestamp. let relay_chain_timestamp = @@ -67,19 +81,16 @@ where // Check that we are not too far in the future. Since we expect `V` parachain blocks // during the relay chain slot, we can allow for `V` parachain slots into the future. - if *slot > *para_slot_from_relay + u64::from(velocity) { + if *para_slot > *para_slot_from_relay + u64::from(velocity) { panic!( - "Parachain slot is too far in the future: parachain_slot: {:?}, derived_from_relay_slot: {:?} velocity: {:?}", - slot, + "Parachain slot is too far in the future: parachain_slot={:?}, derived_from_relay_slot={:?} velocity={:?}, relay_chain_slot={:?}", + para_slot, para_slot_from_relay, - velocity + velocity, + relay_chain_slot ); } - // We need to allow authoring multiple blocks in the same slot. - if slot != para_slot_from_relay && authored > velocity { - panic!("authored blocks limit is reached for the slot") - } let weight = T::DbWeight::get().reads(1); ( @@ -110,7 +121,7 @@ impl< /// is more recent than the included block itself. pub fn can_build_upon(included_hash: T::Hash, new_slot: Slot) -> bool { let velocity = V.max(1); - let (last_slot, authored_so_far) = match pallet::SlotInfo::::get() { + let (last_slot, authored_so_far) = match pallet::RelaySlotInfo::::get() { None => return true, Some(x) => x, }; @@ -123,11 +134,8 @@ impl< return false } - // TODO: This logic needs to be adjusted. - // It checks that we have not authored more than `V + 1` blocks in the slot. - // As a slot however, we take the parachain slot here. Velocity should - // be measured in relation to the relay chain slot. - // https://github.com/paritytech/polkadot-sdk/issues/3967 + // Check that we have not authored more than `V + 1` parachain blocks in the current relay + // chain slot. if last_slot == new_slot { authored_so_far < velocity + 1 } else { diff --git a/cumulus/pallets/aura-ext/src/lib.rs b/cumulus/pallets/aura-ext/src/lib.rs index dc854eb82018..19c2634ca708 100644 --- a/cumulus/pallets/aura-ext/src/lib.rs +++ b/cumulus/pallets/aura-ext/src/lib.rs @@ -40,6 +40,9 @@ use sp_consensus_aura::{digests::CompatibleDigestItem, Slot}; use sp_runtime::traits::{Block as BlockT, Header as HeaderT}; pub mod consensus_hook; +pub mod migration; +mod test; + pub use consensus_hook::FixedVelocityConsensusHook; type Aura = pallet_aura::Pallet; @@ -57,6 +60,7 @@ pub mod pallet { pub trait Config: pallet_aura::Config + frame_system::Config {} #[pallet::pallet] + #[pallet::storage_version(migration::STORAGE_VERSION)] pub struct Pallet(_); #[pallet::hooks] @@ -70,20 +74,7 @@ pub mod pallet { // Fetch the authorities once to get them into the storage proof of the PoV. Authorities::::get(); - let new_slot = pallet_aura::CurrentSlot::::get(); - - let (new_slot, authored) = match SlotInfo::::get() { - Some((slot, authored)) if slot == new_slot => (slot, authored + 1), - Some((slot, _)) if slot < new_slot => (new_slot, 1), - Some(..) => { - panic!("slot moved backwards") - }, - None => (new_slot, 1), - }; - - SlotInfo::::put((new_slot, authored)); - - T::DbWeight::get().reads_writes(4, 2) + T::DbWeight::get().reads_writes(1, 0) } } @@ -99,11 +90,12 @@ pub mod pallet { ValueQuery, >; - /// Current slot paired with a number of authored blocks. + /// Current relay chain slot paired with a number of authored blocks. /// - /// Updated on each block initialization. + /// This is updated in [`FixedVelocityConsensusHook::on_state_proof`] with the current relay + /// chain slot as provided by the relay chain state proof. #[pallet::storage] - pub(crate) type SlotInfo = StorageValue<_, (Slot, u32), OptionQuery>; + pub(crate) type RelaySlotInfo = StorageValue<_, (Slot, u32), OptionQuery>; #[pallet::genesis_config] #[derive(frame_support::DefaultNoBound)] diff --git a/cumulus/pallets/aura-ext/src/migration.rs b/cumulus/pallets/aura-ext/src/migration.rs new file mode 100644 index 000000000000..b580c19fc733 --- /dev/null +++ b/cumulus/pallets/aura-ext/src/migration.rs @@ -0,0 +1,74 @@ +// Copyright (C) Parity Technologies (UK) Ltd. +// This file is part of Cumulus. + +// Cumulus is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Cumulus is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Cumulus. If not, see . +extern crate alloc; + +use crate::{Config, Pallet}; +#[cfg(feature = "try-runtime")] +use alloc::vec::Vec; +use frame_support::{migrations::VersionedMigration, pallet_prelude::StorageVersion}; + +/// The in-code storage version. +pub const STORAGE_VERSION: StorageVersion = StorageVersion::new(1); + +mod v0 { + use super::*; + use frame_support::{pallet_prelude::OptionQuery, storage_alias}; + use sp_consensus_aura::Slot; + + /// Current slot paired with a number of authored blocks. + /// + /// Updated on each block initialization. + #[storage_alias] + pub(super) type SlotInfo = StorageValue, (Slot, u32), OptionQuery>; +} +mod v1 { + use super::*; + use frame_support::{pallet_prelude::*, traits::UncheckedOnRuntimeUpgrade}; + + pub struct UncheckedMigrationToV1(PhantomData); + + impl UncheckedOnRuntimeUpgrade for UncheckedMigrationToV1 { + fn on_runtime_upgrade() -> Weight { + let mut weight: Weight = Weight::zero(); + weight += migrate::(); + weight + } + + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, sp_runtime::TryRuntimeError> { + Ok(Vec::new()) + } + #[cfg(feature = "try-runtime")] + fn post_upgrade(_state: Vec) -> Result<(), sp_runtime::TryRuntimeError> { + ensure!(!v0::SlotInfo::::exists(), "SlotInfo should not exist"); + Ok(()) + } + } + + pub fn migrate() -> Weight { + v0::SlotInfo::::kill(); + T::DbWeight::get().writes(1) + } +} + +/// Migrate `V0` to `V1`. +pub type MigrateV0ToV1 = VersionedMigration< + 0, + 1, + v1::UncheckedMigrationToV1, + Pallet, + ::DbWeight, +>; diff --git a/cumulus/pallets/aura-ext/src/test.rs b/cumulus/pallets/aura-ext/src/test.rs new file mode 100644 index 000000000000..b0099381e682 --- /dev/null +++ b/cumulus/pallets/aura-ext/src/test.rs @@ -0,0 +1,338 @@ +// Copyright Parity Technologies (UK) Ltd. +// This file is part of Cumulus. + +// Cumulus is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Cumulus is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Cumulus. If not, see . + +#![cfg(test)] +extern crate alloc; + +use super::*; + +use core::num::NonZeroU32; +use cumulus_pallet_parachain_system::{ + consensus_hook::ExpectParentIncluded, AnyRelayNumber, DefaultCoreSelector, ParachainSetCode, +}; +use cumulus_primitives_core::ParaId; +use frame_support::{ + derive_impl, + pallet_prelude::ConstU32, + parameter_types, + traits::{ConstBool, ConstU64, EnqueueWithOrigin}, +}; +use sp_io::TestExternalities; +use sp_version::RuntimeVersion; + +type Block = frame_system::mocking::MockBlock; + +frame_support::construct_runtime!( + pub enum Test { + System: frame_system, + ParachainSystem: cumulus_pallet_parachain_system, + Aura: pallet_aura, + AuraExt: crate, + } +); + +parameter_types! { + pub Version: RuntimeVersion = RuntimeVersion { + spec_name: "test".into(), + impl_name: "system-test".into(), + authoring_version: 1, + spec_version: 1, + impl_version: 1, + apis: sp_version::create_apis_vec!([]), + transaction_version: 1, + system_version: 1, + }; +} + +#[derive_impl(frame_system::config_preludes::TestDefaultConfig)] +impl frame_system::Config for Test { + type Block = Block; + type Version = Version; + type OnSetCode = ParachainSetCode; + type RuntimeEvent = (); +} + +impl crate::Config for Test {} + +impl pallet_aura::Config for Test { + type AuthorityId = sp_consensus_aura::sr25519::AuthorityId; + type MaxAuthorities = ConstU32<100_000>; + type DisabledValidators = (); + type AllowMultipleBlocksPerSlot = ConstBool; + type SlotDuration = ConstU64<6000>; +} + +impl pallet_timestamp::Config for Test { + type Moment = u64; + type OnTimestampSet = (); + type MinimumPeriod = (); + type WeightInfo = (); +} + +impl cumulus_pallet_parachain_system::Config for Test { + type WeightInfo = (); + type RuntimeEvent = (); + type OnSystemEvent = (); + type SelfParaId = (); + type OutboundXcmpMessageSource = (); + // Ignore all DMP messages by enqueueing them into `()`: + type DmpQueue = EnqueueWithOrigin<(), sp_core::ConstU8<0>>; + type ReservedDmpWeight = (); + type XcmpMessageHandler = (); + type ReservedXcmpWeight = (); + type CheckAssociatedRelayNumber = AnyRelayNumber; + type ConsensusHook = ExpectParentIncluded; + type SelectCore = DefaultCoreSelector; +} + +#[cfg(test)] +mod test { + use crate::test::*; + use cumulus_pallet_parachain_system::{ + Ancestor, ConsensusHook, RelayChainStateProof, UsedBandwidth, + }; + use sp_core::H256; + + fn set_ancestors() { + let mut ancestors = Vec::new(); + for i in 0..3 { + let mut ancestor = Ancestor::new_unchecked(UsedBandwidth::default(), None); + ancestor.replace_para_head_hash(H256::repeat_byte(i + 1)); + ancestors.push(ancestor); + } + cumulus_pallet_parachain_system::UnincludedSegment::::put(ancestors); + } + + pub fn new_test_ext(para_slot: u64) -> sp_io::TestExternalities { + let mut ext = TestExternalities::new_empty(); + ext.execute_with(|| { + set_ancestors(); + // Set initial parachain slot + pallet_aura::CurrentSlot::::put(Slot::from(para_slot)); + }); + ext + } + + fn set_relay_slot(slot: u64, authored: u32) { + RelaySlotInfo::::put((Slot::from(slot), authored)) + } + + fn relay_chain_state_proof(relay_slot: u64) -> RelayChainStateProof { + let mut builder = cumulus_test_relay_sproof_builder::RelayStateSproofBuilder::default(); + builder.current_slot = relay_slot.into(); + + let (hash, state_proof) = builder.into_state_root_and_proof(); + + RelayChainStateProof::new(ParaId::from(200), hash, state_proof) + .expect("Should be able to construct state proof.") + } + + fn assert_slot_info(expected_slot: u64, expected_authored: u32) { + let (slot, authored) = pallet::RelaySlotInfo::::get().unwrap(); + assert_eq!(slot, Slot::from(expected_slot), "Slot stored in RelaySlotInfo is incorrect."); + assert_eq!( + authored, expected_authored, + "Number of authored blocks stored in RelaySlotInfo is incorrect." + ); + } + + #[test] + fn test_velocity() { + type Hook = FixedVelocityConsensusHook; + + new_test_ext(1).execute_with(|| { + let state_proof = relay_chain_state_proof(10); + let (_, capacity) = Hook::on_state_proof(&state_proof); + assert_eq!(capacity, NonZeroU32::new(1).unwrap().into()); + assert_slot_info(10, 1); + + let (_, capacity) = Hook::on_state_proof(&state_proof); + assert_eq!(capacity, NonZeroU32::new(1).unwrap().into()); + assert_slot_info(10, 2); + }); + } + + #[test] + #[should_panic(expected = "authored blocks limit is reached for the slot")] + fn test_exceeding_velocity_limit() { + const VELOCITY: u32 = 2; + type Hook = FixedVelocityConsensusHook; + + new_test_ext(1).execute_with(|| { + let state_proof = relay_chain_state_proof(10); + for authored in 0..=VELOCITY + 1 { + Hook::on_state_proof(&state_proof); + assert_slot_info(10, authored + 1); + } + }); + } + + #[test] + fn test_para_slot_calculated_from_slot_duration() { + const VELOCITY: u32 = 2; + type Hook = FixedVelocityConsensusHook; + + new_test_ext(6).execute_with(|| { + let state_proof = relay_chain_state_proof(10); + Hook::on_state_proof(&state_proof); + + let para_slot = Slot::from(7); + pallet_aura::CurrentSlot::::put(para_slot); + Hook::on_state_proof(&state_proof); + }); + } + + #[test] + fn test_velocity_at_least_one() { + // Even though this is 0, one block should always be allowed. + const VELOCITY: u32 = 0; + type Hook = FixedVelocityConsensusHook; + + new_test_ext(6).execute_with(|| { + let state_proof = relay_chain_state_proof(10); + Hook::on_state_proof(&state_proof); + }); + } + + #[test] + #[should_panic( + expected = "Parachain slot is too far in the future: parachain_slot=Slot(8), derived_from_relay_slot=Slot(5) velocity=2" + )] + fn test_para_slot_calculated_from_slot_duration_2() { + const VELOCITY: u32 = 2; + type Hook = FixedVelocityConsensusHook; + + new_test_ext(8).execute_with(|| { + let state_proof = relay_chain_state_proof(10); + let (_, _) = Hook::on_state_proof(&state_proof); + }); + } + + #[test] + fn test_velocity_resets_on_new_relay_slot() { + const VELOCITY: u32 = 2; + type Hook = FixedVelocityConsensusHook; + + new_test_ext(1).execute_with(|| { + let state_proof = relay_chain_state_proof(10); + for authored in 0..=VELOCITY { + Hook::on_state_proof(&state_proof); + assert_slot_info(10, authored + 1); + } + + let state_proof = relay_chain_state_proof(11); + for authored in 0..=VELOCITY { + Hook::on_state_proof(&state_proof); + assert_slot_info(11, authored + 1); + } + }); + } + + #[test] + #[should_panic( + expected = "Slot moved backwards: stored_slot=Slot(10), relay_chain_slot=Slot(9)" + )] + fn test_backward_relay_slot_not_tolerated() { + type Hook = FixedVelocityConsensusHook; + + new_test_ext(1).execute_with(|| { + let state_proof = relay_chain_state_proof(10); + Hook::on_state_proof(&state_proof); + assert_slot_info(10, 1); + + let state_proof = relay_chain_state_proof(9); + Hook::on_state_proof(&state_proof); + }); + } + + #[test] + #[should_panic( + expected = "Parachain slot is too far in the future: parachain_slot=Slot(13), derived_from_relay_slot=Slot(10) velocity=2" + )] + fn test_future_parachain_slot_errors() { + type Hook = FixedVelocityConsensusHook; + + new_test_ext(13).execute_with(|| { + let state_proof = relay_chain_state_proof(10); + Hook::on_state_proof(&state_proof); + }); + } + + #[test] + fn test_can_build_upon_true_when_empty() { + const VELOCITY: u32 = 2; + type Hook = FixedVelocityConsensusHook; + + new_test_ext(1).execute_with(|| { + let hash = H256::repeat_byte(0x1); + assert!(Hook::can_build_upon(hash, Slot::from(1))); + }); + } + + #[test] + fn test_can_build_upon_respects_velocity() { + const VELOCITY: u32 = 2; + type Hook = FixedVelocityConsensusHook; + + new_test_ext(1).execute_with(|| { + let hash = H256::repeat_byte(0x1); + let relay_slot = Slot::from(10); + + set_relay_slot(10, VELOCITY - 1); + assert!(Hook::can_build_upon(hash, relay_slot)); + + set_relay_slot(10, VELOCITY); + assert!(Hook::can_build_upon(hash, relay_slot)); + + set_relay_slot(10, VELOCITY + 1); + // Velocity too high + assert!(!Hook::can_build_upon(hash, relay_slot)); + }); + } + + #[test] + fn test_can_build_upon_slot_can_not_decrease() { + const VELOCITY: u32 = 2; + type Hook = FixedVelocityConsensusHook; + + new_test_ext(1).execute_with(|| { + let hash = H256::repeat_byte(0x1); + + set_relay_slot(10, VELOCITY); + // Slot moves backwards + assert!(!Hook::can_build_upon(hash, Slot::from(9))); + }); + } + + #[test] + fn test_can_build_upon_unincluded_segment_size() { + const VELOCITY: u32 = 2; + type Hook = FixedVelocityConsensusHook; + + new_test_ext(1).execute_with(|| { + let relay_slot = Slot::from(10); + + set_relay_slot(10, VELOCITY); + // Size after included is two, we can not build + let hash = H256::repeat_byte(0x1); + assert!(!Hook::can_build_upon(hash, relay_slot)); + + // Size after included is one, we can build + let hash = H256::repeat_byte(0x2); + assert!(Hook::can_build_upon(hash, relay_slot)); + }); + } +} diff --git a/cumulus/pallets/parachain-system/src/consensus_hook.rs b/cumulus/pallets/parachain-system/src/consensus_hook.rs index 3062396a4e78..6d65bdc77186 100644 --- a/cumulus/pallets/parachain-system/src/consensus_hook.rs +++ b/cumulus/pallets/parachain-system/src/consensus_hook.rs @@ -22,7 +22,7 @@ use core::num::NonZeroU32; use frame_support::weights::Weight; /// The possible capacity of the unincluded segment. -#[derive(Clone)] +#[derive(Clone, Debug, PartialEq)] pub struct UnincludedSegmentCapacity(UnincludedSegmentCapacityInner); impl UnincludedSegmentCapacity { @@ -41,7 +41,7 @@ impl UnincludedSegmentCapacity { } } -#[derive(Clone)] +#[derive(Clone, Debug, PartialEq)] pub(crate) enum UnincludedSegmentCapacityInner { ExpectParentIncluded, Value(NonZeroU32), diff --git a/cumulus/pallets/parachain-system/src/lib.rs b/cumulus/pallets/parachain-system/src/lib.rs index 0fa759357f65..6857b08e66b7 100644 --- a/cumulus/pallets/parachain-system/src/lib.rs +++ b/cumulus/pallets/parachain-system/src/lib.rs @@ -80,8 +80,7 @@ pub mod relay_state_snapshot; pub mod validate_block; use unincluded_segment::{ - Ancestor, HrmpChannelUpdate, HrmpWatermarkUpdate, OutboundBandwidthLimits, SegmentTracker, - UsedBandwidth, + HrmpChannelUpdate, HrmpWatermarkUpdate, OutboundBandwidthLimits, SegmentTracker, }; pub use consensus_hook::{ConsensusHook, ExpectParentIncluded}; @@ -109,6 +108,7 @@ pub use consensus_hook::{ConsensusHook, ExpectParentIncluded}; /// ``` pub use cumulus_pallet_parachain_system_proc_macro::register_validate_block; pub use relay_state_snapshot::{MessagingStateSnapshot, RelayChainStateProof}; +pub use unincluded_segment::{Ancestor, UsedBandwidth}; pub use pallet::*; diff --git a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs index dd1535826152..3006bb0123c6 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs @@ -1043,6 +1043,7 @@ pub type Migrations = ( >, // permanent pallet_xcm::migration::MigrateToLatestXcmVersion, + cumulus_pallet_aura_ext::migration::MigrateV0ToV1, ); parameter_types! { diff --git a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs index 707d1c52f743..982754cc979e 100644 --- a/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs @@ -337,7 +337,6 @@ pub type LocalAndForeignAssets = fungibles::UnionOf< xcm::v5::Location, AccountId, >; - /// Union fungibles implementation for [`LocalAndForeignAssets`] and `Balances`. pub type NativeAndAssets = fungible::UnionOf< Balances, @@ -1121,6 +1120,7 @@ pub type Migrations = ( >, // permanent pallet_xcm::migration::MigrateToLatestXcmVersion, + cumulus_pallet_aura_ext::migration::MigrateV0ToV1, ); /// Asset Hub Westend has some undecodable storage, delete it. diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/lib.rs b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/lib.rs index 492b731610ce..0e9d80f48115 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/lib.rs @@ -182,6 +182,7 @@ pub type Migrations = ( pallet_bridge_relayers::migration::v1::MigrationToV1, // permanent pallet_xcm::migration::MigrateToLatestXcmVersion, + cumulus_pallet_aura_ext::migration::MigrateV0ToV1, ); parameter_types! { diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/lib.rs b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/lib.rs index edf79ea0c315..22be8fa545bb 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-westend/src/lib.rs @@ -169,6 +169,7 @@ pub type Migrations = ( bridge_to_ethereum_config::migrations::MigrationForXcmV5, // permanent pallet_xcm::migration::MigrateToLatestXcmVersion, + cumulus_pallet_aura_ext::migration::MigrateV0ToV1, ); parameter_types! { diff --git a/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs b/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs index 5c2ba2e24c22..8e721193e33c 100644 --- a/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs @@ -762,6 +762,7 @@ type Migrations = ( pallet_core_fellowship::migration::MigrateV0ToV1, // unreleased pallet_core_fellowship::migration::MigrateV0ToV1, + cumulus_pallet_aura_ext::migration::MigrateV0ToV1, ); /// Executive: handles dispatch to the various modules. diff --git a/cumulus/parachains/runtimes/contracts/contracts-rococo/src/lib.rs b/cumulus/parachains/runtimes/contracts/contracts-rococo/src/lib.rs index 594c9b26f57e..472dc75f573a 100644 --- a/cumulus/parachains/runtimes/contracts/contracts-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/contracts/contracts-rococo/src/lib.rs @@ -116,6 +116,7 @@ pub type Migrations = ( cumulus_pallet_xcmp_queue::migration::v5::MigrateV4ToV5, // permanent pallet_xcm::migration::MigrateToLatestXcmVersion, + cumulus_pallet_aura_ext::migration::MigrateV0ToV1, ); type EventRecord = frame_system::EventRecord< diff --git a/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs b/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs index e8f6e6659e13..6d3844087429 100644 --- a/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs @@ -127,6 +127,7 @@ pub type Migrations = ( pallet_broker::migration::MigrateV3ToV4, // permanent pallet_xcm::migration::MigrateToLatestXcmVersion, + cumulus_pallet_aura_ext::migration::MigrateV0ToV1, ); /// Executive: handles dispatch to the various modules. diff --git a/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs b/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs index ce965f0ad1ba..eef653133fc6 100644 --- a/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs @@ -127,6 +127,7 @@ pub type Migrations = ( pallet_broker::migration::MigrateV3ToV4, // permanent pallet_xcm::migration::MigrateToLatestXcmVersion, + cumulus_pallet_aura_ext::migration::MigrateV0ToV1, ); /// Executive: handles dispatch to the various modules. diff --git a/cumulus/parachains/runtimes/people/people-rococo/src/lib.rs b/cumulus/parachains/runtimes/people/people-rococo/src/lib.rs index b8db687da625..fac069cde544 100644 --- a/cumulus/parachains/runtimes/people/people-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/people/people-rococo/src/lib.rs @@ -114,6 +114,7 @@ pub type Migrations = ( cumulus_pallet_xcmp_queue::migration::v5::MigrateV4ToV5, // permanent pallet_xcm::migration::MigrateToLatestXcmVersion, + cumulus_pallet_aura_ext::migration::MigrateV0ToV1, ); /// Executive: handles dispatch to the various modules. diff --git a/cumulus/parachains/runtimes/people/people-westend/src/lib.rs b/cumulus/parachains/runtimes/people/people-westend/src/lib.rs index 620ec41c071c..664a597867fe 100644 --- a/cumulus/parachains/runtimes/people/people-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/people/people-westend/src/lib.rs @@ -113,6 +113,7 @@ pub type Migrations = ( pallet_collator_selection::migration::v2::MigrationToV2, // permanent pallet_xcm::migration::MigrateToLatestXcmVersion, + cumulus_pallet_aura_ext::migration::MigrateV0ToV1, ); /// Executive: handles dispatch to the various modules. diff --git a/cumulus/primitives/aura/src/lib.rs b/cumulus/primitives/aura/src/lib.rs index aeeee5f8bafa..4e7d7dc3e79d 100644 --- a/cumulus/primitives/aura/src/lib.rs +++ b/cumulus/primitives/aura/src/lib.rs @@ -34,10 +34,14 @@ sp_api::decl_runtime_apis! { /// When the unincluded segment is short, Aura chains will allow authors to create multiple /// blocks per slot in order to build a backlog. When it is saturated, this API will limit /// the amount of blocks that can be created. + /// + /// Changes: + /// - Version 2: Update to `can_build_upon` to take a relay chain `Slot` instead of a parachain `Slot`. + #[api_version(2)] pub trait AuraUnincludedSegmentApi { /// Whether it is legal to extend the chain, assuming the given block is the most /// recently included one as-of the relay parent that will be built against, and - /// the given slot. + /// the given relay chain slot. /// /// This should be consistent with the logic the runtime uses when validating blocks to /// avoid issues. diff --git a/cumulus/xcm/xcm-emulator/src/lib.rs b/cumulus/xcm/xcm-emulator/src/lib.rs index ff14b747973c..d9b1e7fd9d04 100644 --- a/cumulus/xcm/xcm-emulator/src/lib.rs +++ b/cumulus/xcm/xcm-emulator/src/lib.rs @@ -1118,6 +1118,7 @@ macro_rules! decl_test_networks { ) -> $crate::ParachainInherentData { let mut sproof = $crate::RelayStateSproofBuilder::default(); sproof.para_id = para_id.into(); + sproof.current_slot = $crate::polkadot_primitives::Slot::from(relay_parent_number as u64); // egress channel let e_index = sproof.hrmp_egress_channel_index.get_or_insert_with(Vec::new); diff --git a/prdoc/pr_6825.prdoc b/prdoc/pr_6825.prdoc new file mode 100644 index 000000000000..7605d0557a51 --- /dev/null +++ b/prdoc/pr_6825.prdoc @@ -0,0 +1,15 @@ +# 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: Use relay chain slot for velocity measurement on parachains + +doc: + - audience: Runtime Dev + description: | + The AuraExt pallets `ConsensusHook` is performing checks based on a parachains velocity. It was previously + checking how many blocks where produced in a given parachain slot. This only works well when the parachain + and relay chain slot length is the same. After this PR, we are checking against the relay chain slot. + + A migration is included that cleans up a renamed storage item. + +crates: [ ]