Parachains: Use relay chain slot for velocity measurement (#6825)

closes #3967 

## Changes
We now use relay chain slots to measure velocity on chain. Previously we
were storing the current parachain slot. Then in `on_state_proof` of the
`ConsensusHook` we were checking how many blocks were athored in the
current parachain slot. This works well when the parachain slot time and
relay chain slot time is the same. With elastic scaling, we can have
parachain slot times lower than that of the relay chain. In these cases
we want to measure velocity in relation to the relay chain. This PR
adjusts that.


##  Migration
This PR includes a migration. Storage item `SlotInfo` of pallet
`aura-ext` is renamed to `RelaySlotInfo` to better reflect its new
content. A migration has been added that just kills the old storage
item. `RelaySlotInfo` will be `None` initially but its value will be
adjusted after one new relay chain slot arrives.

---------

Co-authored-by: command-bot <>
Co-authored-by: Bastian Köcher <[email protected]>

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,

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<Block: BlockT, Client, P>(
-	slot: Slot,
+	para_slot: Slot,
+	relay_slot: Slot,
 	timestamp: Timestamp,
 	parent_hash: Block::Hash,
 	included_block: Block::Hash,
@@ -169,25 +170,28 @@ async fn can_build_upon<Block: BlockT, Client, P>(
 ) -> Option<SlotClaim<P::Public>>
 where
 	Client: ProvideRuntimeApi<Block>,
-	Client::Api: AuraApi<Block, P::Public> + AuraUnincludedSegmentApi<Block>,
+	Client::Api: AuraApi<Block, P::Public> + AuraUnincludedSegmentApi<Block> + ApiExt<Block>,
 	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::<P>(slot, &authorities, keystore).await?;
+	let author_pub = aura_internal::claim_slot::<P>(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::<dyn AuraUnincludedSegmentApi<Block>>(parent_hash)
+	else {
+		return (parent_hash == included_block)
+			.then(|| SlotClaim::unchecked::<P>(author_pub, para_slot, timestamp));
+	};
+
+	let slot = if api_version > 1 { relay_slot } else { para_slot };
 
-	Some(SlotClaim::unchecked::<P>(author_pub, slot, timestamp))
+	runtime_api
+		.can_build_upon(parent_hash, included_block, slot)
+		.ok()?
+		.then(|| SlotClaim::unchecked::<P>(author_pub, para_slot, timestamp))
 }
 
 /// Use [`cumulus_client_consensus_common::find_potential_parents`] to find parachain blocks that

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::<RelayBlock>(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,

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<R: Send + Sync + GenerateRandomness<u64>> 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

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-primitives-core = { workspace = true, default-features = true }
+cumulus-test-relay-sproof-builder = { workspace = true, default-features = true }
+
+# Substrate
+sp-core = { workspace = true, default-features = true }
+sp-io = { workspace = true, default-features = true }
+sp-version = { workspace = true, default-features = true }
 
 [features]
 default = ["std"]

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::<T>::get().expect("slot info is inserted on block initialization");
+		let (relay_chain_slot, authored_in_relay) = match pallet::RelaySlotInfo::<T>::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::<T>::put((relay_chain_slot, authored_in_relay + 1));
+
+		let para_slot = pallet_aura::CurrentSlot::<T>::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::<T>::get() {
+		let (last_slot, authored_so_far) = match pallet::RelaySlotInfo::<T>::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 {

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<T> = pallet_aura::Pallet<T>;
@@ -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<T>(_);
 
 	#[pallet::hooks]
@@ -70,20 +74,7 @@ pub mod pallet {
 			// Fetch the authorities once to get them into the storage proof of the PoV.
 			Authorities::<T>::get();
 
-			let new_slot = pallet_aura::CurrentSlot::<T>::get();
-
-			let (new_slot, authored) = match SlotInfo::<T>::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::<T>::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<T: Config> = StorageValue<_, (Slot, u32), OptionQuery>;
+	pub(crate) type RelaySlotInfo<T: Config> = StorageValue<_, (Slot, u32), OptionQuery>;
 
 	#[pallet::genesis_config]
 	#[derive(frame_support::DefaultNoBound)]

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 <http://www.gnu.org/licenses/>.
+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<T: Config> = StorageValue<Pallet<T>, (Slot, u32), OptionQuery>;
+}
+mod v1 {
+	use super::*;
+	use frame_support::{pallet_prelude::*, traits::UncheckedOnRuntimeUpgrade};
+
+	pub struct UncheckedMigrationToV1<T: Config>(PhantomData<T>);
+
+	impl<T: Config> UncheckedOnRuntimeUpgrade for UncheckedMigrationToV1<T> {
+		fn on_runtime_upgrade() -> Weight {
+			let mut weight: Weight = Weight::zero();
+			weight += migrate::<T>();
+			weight
+		}
+
+		#[cfg(feature = "try-runtime")]
+		fn pre_upgrade() -> Result<Vec<u8>, sp_runtime::TryRuntimeError> {
+			Ok(Vec::new())
+		}
+		#[cfg(feature = "try-runtime")]
+		fn post_upgrade(_state: Vec<u8>) -> Result<(), sp_runtime::TryRuntimeError> {
+			ensure!(!v0::SlotInfo::<T>::exists(), "SlotInfo should not exist");
+			Ok(())
+		}
+	}
+
+	pub fn migrate<T: Config>() -> Weight {
+		v0::SlotInfo::<T>::kill();
+		T::DbWeight::get().writes(1)
+	}
+}
+
+/// Migrate `V0` to `V1`.
+pub type MigrateV0ToV1<T> = VersionedMigration<
+	0,
+	1,
+	v1::UncheckedMigrationToV1<T>,
+	Pallet<T>,
+	<T as frame_system::Config>::DbWeight,
+>;