Add relay storage root to persisted validation data (#2161)

* Cont.: Implement the state root obtaining during inclusion

During inclusion now we obtain the storage root by passing it through
the inclusion_inherent.

* Fix tests

* Bump rococo spec version

* Reorder the parent header into the end

of the inclusion inherent.

When the parent header is in the beginning, it shifts the other two
fields, so that a previous version won't be able to decode that. If
we put the parent header in the end, the other two fields will stay
at their positions, thus make it possible to decode with the previous
version.

That allows us to perform upgrade of rococo runtime without needing of
simultanuous upgrade of nodes and runtime, or restart of the network.

* Squash a stray tab

node/core/av-store/src/tests.rs

@@ -74,6 +74,7 @@ impl Default for TestState {
 			hrmp_mqc_heads: Vec::new(),
 			dmq_mqc_head: Default::default(),
 			max_pov_size: 1024,
+			relay_storage_root: Default::default(),
 		};
 
 		let pruning_config = PruningConfig {

node/core/backing/src/lib.rs

@@ -1250,6 +1250,7 @@ mod tests {
 					hrmp_mqc_heads: Vec::new(),
 					dmq_mqc_head: Default::default(),
 					max_pov_size: 1024,
+					relay_storage_root: Default::default(),
 				},
 				transient: TransientValidationData {
 					max_code_size: 1000,

node/core/proposer/src/lib.rs

@@ -98,11 +98,13 @@ where
 		// data to be moved into the future
 		let overseer = self.overseer.clone();
 		let parent_header_hash = parent_header.hash();
+		let parent_header = parent_header.clone();
 
 		async move {
 			Ok(Proposer {
 				inner: proposer?,
 				overseer,
+				parent_header,
 				parent_header_hash,
 			})
 		}.boxed()
@@ -116,6 +118,7 @@ where
 pub struct Proposer<TxPool: TransactionPool<Block = Block>, Backend, Client> {
 	inner: sc_basic_authorship::Proposer<Backend, Block, Client, TxPool>,
 	overseer: OverseerHandler,
+	parent_header: Header,
 	parent_header_hash: Hash,
 }
 
@@ -209,9 +212,14 @@ where
 
 			drop(_span);
 
+			let inclusion_inherent_data = (
+				provisioner_data.0,
+				provisioner_data.1,
+				self.parent_header,
+			);
 			inherent_data.put_data(
 				polkadot_primitives::v1::INCLUSION_INHERENT_IDENTIFIER,
-				&provisioner_data,
+				&inclusion_inherent_data,
 			)?;
 
 			let _span = span.child("authorship-propose");

node/network/availability-distribution/src/tests.rs

@@ -192,6 +192,7 @@ impl Default for TestState {
 			hrmp_mqc_heads: Vec::new(),
 			dmq_mqc_head: Default::default(),
 			max_pov_size: 1024,
+			relay_storage_root: Default::default(),
 		};
 
 		let pov_block_a = PoV {

node/test/client/src/block_builder.rs

@@ -71,10 +71,19 @@ impl InitPolkadotBlockBuilder for Client {
 			.put_data(sp_timestamp::INHERENT_IDENTIFIER, &timestamp)
 			.expect("Put timestamp inherent data");
 
+		let parent_header = self.header(at)
+			.expect("Get the parent block header")
+			.expect("The target block header must exist");
+		let provisioner_data = polkadot_node_subsystem::messages::ProvisionerInherentData::default();
+		let inclusion_inherent_data = (
+			provisioner_data.0,
+			provisioner_data.1,
+			parent_header,
+		);
 		inherent_data
 			.put_data(
 				polkadot_primitives::v1::INCLUSION_INHERENT_IDENTIFIER,
-				&polkadot_node_subsystem::messages::ProvisionerInherentData::default(),
+				&inclusion_inherent_data,
 			)
 			.expect("Put inclusion inherent data");
 

primitives/src/v1.rs

@@ -284,6 +284,8 @@ pub struct PersistedValidationData<N = BlockNumber> {
 	pub parent_head: HeadData,
 	/// The relay-chain block number this is in the context of.
 	pub block_number: N,
+	/// The relay-chain block storage root this is in the context of.
+	pub relay_storage_root: Hash,
 	/// The list of MQC heads for the inbound channels paired with the sender para ids. This
 	/// vector is sorted ascending by the para id and doesn't contain multiple entries with the same
 	/// sender.

roadmap/implementers-guide/src/runtime/inclusion.md

@@ -59,7 +59,7 @@ All failed checks should lead to an unrecoverable error making the block invalid
   1. For each applied bit of each availability-bitfield, set the bit for the validator in the `CandidatePendingAvailability`'s `availability_votes` bitfield. Track all candidates that now have >2/3 of bits set in their `availability_votes`. These candidates are now available and can be enacted.
   1. For all now-available candidates, invoke the `enact_candidate` routine with the candidate and relay-parent number.
   1. Return a list of freed cores consisting of the cores where candidates have become available.
-* `process_candidates(BackedCandidates, scheduled: Vec<CoreAssignment>, group_validators: Fn(GroupIndex) -> Option<Vec<ValidatorIndex>>)`:
+* `process_candidates(parent_storage_root, BackedCandidates, scheduled: Vec<CoreAssignment>, group_validators: Fn(GroupIndex) -> Option<Vec<ValidatorIndex>>)`:
   1. check that each candidate corresponds to a scheduled core and that they are ordered in the same order the cores appear in assignments in `scheduled`.
   1. check that `scheduled` is sorted ascending by `CoreIndex`, without duplicates.
   1. check that there is no candidate pending availability for any scheduled `ParaId`.

roadmap/implementers-guide/src/runtime/inclusioninherent.md

@@ -16,11 +16,13 @@ Included: Option<()>,
 
 ## Entry Points
 
-* `inclusion`: This entry-point accepts two parameters: [`Bitfields`](../types/availability.md#signed-availability-bitfield) and [`BackedCandidates`](../types/backing.md#backed-candidate).
+* `inclusion`: This entry-point accepts three parameters: The relay-chain parent block header, [`Bitfields`](../types/availability.md#signed-availability-bitfield) and [`BackedCandidates`](../types/backing.md#backed-candidate).
+    1. Hash the parent header and make sure that it corresponds to the block hash of the parent (tracked by the `frame_system` FRAME module),
     1. The `Bitfields` are first forwarded to the `Inclusion::process_bitfields` routine, returning a set of freed cores. Provide a `Scheduler::core_para` as a core-lookup to the `process_bitfields` routine. Annotate each of these freed cores with `FreedReason::Concluded`.
     1. If `Scheduler::availability_timeout_predicate` is `Some`, invoke `Inclusion::collect_pending` using it, and add timed-out cores to the free cores, annotated with `FreedReason::TimedOut`.
     1. Invoke `Scheduler::schedule(freed)`
-	1. Invoke the `Inclusion::process_candidates` routine with the parameters `(backed_candidates, Scheduler::scheduled(), Scheduler::group_validators)`.
+    1. Extract `parent_storage_root` from the parent header,
+    1. Invoke the `Inclusion::process_candidates` routine with the parameters `(parent_storage_root, backed_candidates, Scheduler::scheduled(), Scheduler::group_validators)`.
     1. Call `Scheduler::occupied` using the return value of the `Inclusion::process_candidates` call above, first sorting the list of assigned core indices.
     1. Call the `Ump::process_pending_upward_messages` routine to execute all messages in upward dispatch queues.
     1. If all of the above succeeds, set `Included` to `Some(())`.

roadmap/implementers-guide/src/types/candidate.md

@@ -127,6 +127,8 @@ struct PersistedValidationData {
 	parent_head: HeadData,
 	/// The relay-chain block number this is in the context of. This informs the collator.
 	block_number: BlockNumber,
+	/// The relay-chain block storage root this is in the context of.
+	relay_storage_root: Hash,
 	/// The MQC head for the DMQ.
 	///
 	/// The DMQ MQC head will be used by the validation function to authorize the downward messages

runtime/parachains/src/inclusion.rs

@@ -25,7 +25,7 @@ use primitives::v1::{
 	ValidatorId, CandidateCommitments, CandidateDescriptor, ValidatorIndex, Id as ParaId,
 	AvailabilityBitfield as AvailabilityBitfield, SignedAvailabilityBitfields, SigningContext,
 	BackedCandidate, CoreIndex, GroupIndex, CommittedCandidateReceipt,
-	CandidateReceipt, HeadData, CandidateHash,
+	CandidateReceipt, HeadData, CandidateHash, Hash,
 };
 use frame_support::{
 	decl_storage, decl_module, decl_error, decl_event, ensure, debug,
@@ -382,11 +382,13 @@ impl<T: Config> Module<T> {
 		Ok(freed_cores)
 	}
 
-	/// Process candidates that have been backed. Provide a set of candidates and scheduled cores.
+	/// Process candidates that have been backed. Provide the relay storage root, a set of candidates
+	/// and scheduled cores.
 	///
 	/// Both should be sorted ascending by core index, and the candidates should be a subset of
 	/// scheduled cores. If these conditions are not met, the execution of the function fails.
 	pub(crate) fn process_candidates(
+		parent_storage_root: Hash,
 		candidates: Vec<BackedCandidate<T::Hash>>,
 		scheduled: Vec<CoreAssignment>,
 		group_validators: impl Fn(GroupIndex) -> Option<Vec<ValidatorIndex>>,
@@ -491,6 +493,7 @@ impl<T: Config> Module<T> {
 								match crate::util::make_persisted_validation_data::<T>(
 									para_id,
 									relay_parent_number,
+									parent_storage_root,
 								) {
 									Some(l) => l,
 									None => {
@@ -1129,6 +1132,7 @@ mod tests {
 			= crate::util::make_persisted_validation_data::<Test>(
 				para_id,
 				relay_parent_number,
+				Default::default(),
 			)?;
 		Some(persisted_validation_data.hash())
 	}
@@ -1634,6 +1638,7 @@ mod tests {
 
 				assert_eq!(
 					Inclusion::process_candidates(
+						Default::default(),
 						vec![backed],
 						vec![chain_b_assignment.clone()],
 						&group_validators,
@@ -1692,6 +1697,7 @@ mod tests {
 				// out-of-order manifests as unscheduled.
 				assert_eq!(
 					Inclusion::process_candidates(
+						Default::default(),
 						vec![backed_b, backed_a],
 						vec![chain_a_assignment.clone(), chain_b_assignment.clone()],
 						&group_validators,
@@ -1726,6 +1732,7 @@ mod tests {
 
 				assert_eq!(
 					Inclusion::process_candidates(
+						Default::default(),
 						vec![backed],
 						vec![chain_a_assignment.clone()],
 						&group_validators,
@@ -1762,6 +1769,7 @@ mod tests {
 
 				assert_eq!(
 					Inclusion::process_candidates(
+						Default::default(),
 						vec![backed],
 						vec![chain_a_assignment.clone()],
 						&group_validators,
@@ -1798,6 +1806,7 @@ mod tests {
 
 				assert_eq!(
 					Inclusion::process_candidates(
+						Default::default(),
 						vec![backed],
 						vec![
 							chain_a_assignment.clone(),
@@ -1841,6 +1850,7 @@ mod tests {
 
 				assert_eq!(
 					Inclusion::process_candidates(
+						Default::default(),
 						vec![backed],
 						vec![thread_a_assignment.clone()],
 						&group_validators,
@@ -1888,6 +1898,7 @@ mod tests {
 
 				assert_eq!(
 					Inclusion::process_candidates(
+						Default::default(),
 						vec![backed],
 						vec![chain_a_assignment.clone()],
 						&group_validators,
@@ -1929,6 +1940,7 @@ mod tests {
 
 				assert_eq!(
 					Inclusion::process_candidates(
+						Default::default(),
 						vec![backed],
 						vec![chain_a_assignment.clone()],
 						&group_validators,
@@ -1975,6 +1987,7 @@ mod tests {
 
 				assert_eq!(
 					Inclusion::process_candidates(
+						Default::default(),
 						vec![backed],
 						vec![chain_a_assignment.clone()],
 						&group_validators,
@@ -2010,6 +2023,7 @@ mod tests {
 
 				assert_eq!(
 					Inclusion::process_candidates(
+						Default::default(),
 						vec![backed],
 						vec![chain_a_assignment.clone()],
 						&group_validators,
@@ -2151,6 +2165,7 @@ mod tests {
 			));
 
 			let occupied_cores = Inclusion::process_candidates(
+				Default::default(),
 				vec![backed_a, backed_b, backed_c],
 				vec![
 					chain_a_assignment.clone(),
@@ -2283,6 +2298,7 @@ mod tests {
 			));
 
 			let occupied_cores = Inclusion::process_candidates(
+				Default::default(),
 				vec![backed_a],
 				vec![
 					chain_a_assignment.clone(),

runtime/parachains/src/inclusion_inherent.rs

@@ -23,8 +23,9 @@
 
 use sp_std::prelude::*;
 use primitives::v1::{
-	BackedCandidate, SignedAvailabilityBitfields, INCLUSION_INHERENT_IDENTIFIER,
+	BackedCandidate, SignedAvailabilityBitfields, INCLUSION_INHERENT_IDENTIFIER, Header,
 };
+use sp_runtime::traits::One;
 use frame_support::{
 	decl_error, decl_module, decl_storage, ensure,
 	dispatch::DispatchResult,
@@ -57,6 +58,9 @@ decl_error! {
 	pub enum Error for Module<T: Config> {
 		/// Inclusion inherent called more than once per block.
 		TooManyInclusionInherents,
+		/// The hash of the submitted parent header doesn't correspond to the saved block hash of
+		/// the parent.
+		InvalidParentHeader,
 	}
 }
 
@@ -81,10 +85,19 @@ decl_module! {
 			origin,
 			signed_bitfields: SignedAvailabilityBitfields,
 			backed_candidates: Vec<BackedCandidate<T::Hash>>,
+			parent_header: Header,
 		) -> DispatchResult {
 			ensure_none(origin)?;
 			ensure!(!<Included>::exists(), Error::<T>::TooManyInclusionInherents);
 
+			// Check that the submitted parent header indeed corresponds to the previous block hash.
+			let now = <frame_system::Module<T>>::block_number();
+			let parent_hash = <frame_system::Module<T>>::block_hash(now - One::one());
+			ensure!(
+				parent_header.hash().as_ref() == parent_hash.as_ref(),
+				Error::<T>::InvalidParentHeader,
+			);
+
 			// Process new availability bitfields, yielding any availability cores whose
 			// work has now concluded.
 			let freed_concluded = <inclusion::Module<T>>::process_bitfields(
@@ -107,7 +120,9 @@ decl_module! {
 			<scheduler::Module<T>>::schedule(freed);
 
 			// Process backed candidates according to scheduled cores.
+			let parent_storage_root = parent_header.state_root;
 			let occupied = <inclusion::Module<T>>::process_candidates(
+				parent_storage_root,
 				backed_candidates,
 				<scheduler::Module<T>>::scheduled(),
 				<scheduler::Module<T>>::group_validators,
@@ -135,14 +150,27 @@ impl<T: Config> ProvideInherent for Module<T> {
 	fn create_inherent(data: &InherentData) -> Option<Self::Call> {
 		data.get_data(&Self::INHERENT_IDENTIFIER)
 			.expect("inclusion inherent data failed to decode")
-			.map(|(signed_bitfields, backed_candidates): (SignedAvailabilityBitfields, Vec<BackedCandidate<T::Hash>>)| {
-				// Sanity check: session changes can invalidate an inherent, and we _really_ don't want that to happen.
-				// See github.com/paritytech/polkadot/issues/1327
-				if Self::inclusion(frame_system::RawOrigin::None.into(), signed_bitfields.clone(), backed_candidates.clone()).is_ok() {
-					Call::inclusion(signed_bitfields, backed_candidates)
-				} else {
-					Call::inclusion(Vec::new().into(), Vec::new())
+			.map(
+				|(signed_bitfields, backed_candidates, parent_header): (
+					SignedAvailabilityBitfields,
+					Vec<BackedCandidate<T::Hash>>,
+					Header,
+				)| {
+					// Sanity check: session changes can invalidate an inherent, and we _really_ don't want that to happen.
+					// See github.com/paritytech/polkadot/issues/1327
+					if Self::inclusion(
+						frame_system::RawOrigin::None.into(),
+						signed_bitfields.clone(),
+						backed_candidates.clone(),
+						parent_header.clone(),
+					)
+					.is_ok()
+					{
+						Call::inclusion(signed_bitfields, backed_candidates, parent_header)
+					} else {
+						Call::inclusion(Vec::new().into(), Vec::new(), parent_header)
+					}
 				}
-			})
+			)
 	}
 }

runtime/parachains/src/runtime_api_impl/v1.rs

@@ -24,7 +24,7 @@ use primitives::v1::{
 	Id as ParaId, OccupiedCoreAssumption, SessionIndex, ValidationCode,
 	CommittedCandidateReceipt, ScheduledCore, OccupiedCore, CoreOccupied, CoreIndex,
 	GroupIndex, CandidateEvent, PersistedValidationData, SessionInfo,
-	InboundDownwardMessage, InboundHrmpMessage,
+	InboundDownwardMessage, InboundHrmpMessage, Hash,
 };
 use frame_support::debug;
 use crate::{initializer, inclusion, scheduler, configuration, paras, session_info, dmp, hrmp};
@@ -190,18 +190,24 @@ fn with_assumption<Config, T, F>(
 pub fn full_validation_data<T: initializer::Config>(
 	para_id: ParaId,
 	assumption: OccupiedCoreAssumption,
-)
-	-> Option<ValidationData<T::BlockNumber>>
-{
+) -> Option<ValidationData<T::BlockNumber>> {
+	use parity_scale_codec::Decode as _;
 	let relay_parent_number = <frame_system::Module<T>>::block_number();
-	with_assumption::<T, _, _>(
-		para_id,
-		assumption,
-		|| Some(ValidationData {
-			persisted: crate::util::make_persisted_validation_data::<T>(para_id, relay_parent_number)?,
-			transient: crate::util::make_transient_validation_data::<T>(para_id, relay_parent_number)?,
-		}),
-	)
+	let relay_storage_root = Hash::decode(&mut &sp_io::storage::root()[..])
+		.expect("storage root must decode to the Hash type; qed");
+	with_assumption::<T, _, _>(para_id, assumption, || {
+		Some(ValidationData {
+			persisted: crate::util::make_persisted_validation_data::<T>(
+				para_id,
+				relay_parent_number,
+				relay_storage_root,
+			)?,
+			transient: crate::util::make_transient_validation_data::<T>(
+				para_id,
+				relay_parent_number,
+			)?,
+		})
+	})
 }
 
 /// Implementation for the `persisted_validation_data` function of the runtime API.
@@ -209,12 +215,17 @@ pub fn persisted_validation_data<T: initializer::Config>(
 	para_id: ParaId,
 	assumption: OccupiedCoreAssumption,
 ) -> Option<PersistedValidationData<T::BlockNumber>> {
+	use parity_scale_codec::Decode as _;
 	let relay_parent_number = <frame_system::Module<T>>::block_number();
-	with_assumption::<T, _, _>(
-		para_id,
-		assumption,
-		|| crate::util::make_persisted_validation_data::<T>(para_id, relay_parent_number),
-	)
+	let relay_storage_root = Hash::decode(&mut &sp_io::storage::root()[..])
+		.expect("storage root must decode to the Hash type; qed");
+	with_assumption::<T, _, _>(para_id, assumption, || {
+		crate::util::make_persisted_validation_data::<T>(
+			para_id,
+			relay_parent_number,
+			relay_storage_root,
+		)
+	})
 }
 
 /// Implementation for the `check_validation_outputs` function of the runtime API.