diff --git a/polkadot/node/core/av-store/src/tests.rs b/polkadot/node/core/av-store/src/tests.rs index 4e83e3d7f7..330e0c529a 100644 --- a/polkadot/node/core/av-store/src/tests.rs +++ b/polkadot/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 { diff --git a/polkadot/node/core/backing/src/lib.rs b/polkadot/node/core/backing/src/lib.rs index 841ea69670..8ac7df309f 100644 --- a/polkadot/node/core/backing/src/lib.rs +++ b/polkadot/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, diff --git a/polkadot/node/core/proposer/src/lib.rs b/polkadot/node/core/proposer/src/lib.rs index 96eb6bb95f..3e1e2919f7 100644 --- a/polkadot/node/core/proposer/src/lib.rs +++ b/polkadot/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, Backend, Client> { inner: sc_basic_authorship::Proposer, 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"); diff --git a/polkadot/node/network/availability-distribution/src/tests.rs b/polkadot/node/network/availability-distribution/src/tests.rs index a34caec7ab..d06a98402d 100644 --- a/polkadot/node/network/availability-distribution/src/tests.rs +++ b/polkadot/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 { diff --git a/polkadot/node/test/client/src/block_builder.rs b/polkadot/node/test/client/src/block_builder.rs index de49ae4886..f9848a3064 100644 --- a/polkadot/node/test/client/src/block_builder.rs +++ b/polkadot/node/test/client/src/block_builder.rs @@ -71,10 +71,19 @@ impl InitPolkadotBlockBuilder for Client { .put_data(sp_timestamp::INHERENT_IDENTIFIER, ×tamp) .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"); diff --git a/polkadot/primitives/src/v1.rs b/polkadot/primitives/src/v1.rs index a9666fe793..6104d6aae2 100644 --- a/polkadot/primitives/src/v1.rs +++ b/polkadot/primitives/src/v1.rs @@ -284,6 +284,8 @@ pub struct PersistedValidationData { 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. diff --git a/polkadot/roadmap/implementers-guide/src/runtime/inclusion.md b/polkadot/roadmap/implementers-guide/src/runtime/inclusion.md index 9a6228a8c1..5cabb109dd 100644 --- a/polkadot/roadmap/implementers-guide/src/runtime/inclusion.md +++ b/polkadot/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, group_validators: Fn(GroupIndex) -> Option>)`: +* `process_candidates(parent_storage_root, BackedCandidates, scheduled: Vec, group_validators: Fn(GroupIndex) -> Option>)`: 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`. diff --git a/polkadot/roadmap/implementers-guide/src/runtime/inclusioninherent.md b/polkadot/roadmap/implementers-guide/src/runtime/inclusioninherent.md index 54ebf3af7b..3f23989642 100644 --- a/polkadot/roadmap/implementers-guide/src/runtime/inclusioninherent.md +++ b/polkadot/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(())`. diff --git a/polkadot/roadmap/implementers-guide/src/types/candidate.md b/polkadot/roadmap/implementers-guide/src/types/candidate.md index 86c80153f3..c9da4b683f 100644 --- a/polkadot/roadmap/implementers-guide/src/types/candidate.md +++ b/polkadot/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 diff --git a/polkadot/runtime/parachains/src/inclusion.rs b/polkadot/runtime/parachains/src/inclusion.rs index 95481b57a0..4f0466f788 100644 --- a/polkadot/runtime/parachains/src/inclusion.rs +++ b/polkadot/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 Module { 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>, scheduled: Vec, group_validators: impl Fn(GroupIndex) -> Option>, @@ -491,6 +493,7 @@ impl Module { match crate::util::make_persisted_validation_data::( para_id, relay_parent_number, + parent_storage_root, ) { Some(l) => l, None => { @@ -1129,6 +1132,7 @@ mod tests { = crate::util::make_persisted_validation_data::( 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(), diff --git a/polkadot/runtime/parachains/src/inclusion_inherent.rs b/polkadot/runtime/parachains/src/inclusion_inherent.rs index bb25f5c80f..27a319dced 100644 --- a/polkadot/runtime/parachains/src/inclusion_inherent.rs +++ b/polkadot/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 { /// 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>, + parent_header: Header, ) -> DispatchResult { ensure_none(origin)?; ensure!(!::exists(), Error::::TooManyInclusionInherents); + // Check that the submitted parent header indeed corresponds to the previous block hash. + let now = >::block_number(); + let parent_hash = >::block_hash(now - One::one()); + ensure!( + parent_header.hash().as_ref() == parent_hash.as_ref(), + Error::::InvalidParentHeader, + ); + // Process new availability bitfields, yielding any availability cores whose // work has now concluded. let freed_concluded = >::process_bitfields( @@ -107,7 +120,9 @@ decl_module! { >::schedule(freed); // Process backed candidates according to scheduled cores. + let parent_storage_root = parent_header.state_root; let occupied = >::process_candidates( + parent_storage_root, backed_candidates, >::scheduled(), >::group_validators, @@ -135,14 +150,27 @@ impl ProvideInherent for Module { fn create_inherent(data: &InherentData) -> Option { data.get_data(&Self::INHERENT_IDENTIFIER) .expect("inclusion inherent data failed to decode") - .map(|(signed_bitfields, backed_candidates): (SignedAvailabilityBitfields, Vec>)| { - // 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>, + 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) + } } - }) + ) } } diff --git a/polkadot/runtime/parachains/src/runtime_api_impl/v1.rs b/polkadot/runtime/parachains/src/runtime_api_impl/v1.rs index d3ef7a1ee3..46503d2977 100644 --- a/polkadot/runtime/parachains/src/runtime_api_impl/v1.rs +++ b/polkadot/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( pub fn full_validation_data( para_id: ParaId, assumption: OccupiedCoreAssumption, -) - -> Option> -{ +) -> Option> { + use parity_scale_codec::Decode as _; let relay_parent_number = >::block_number(); - with_assumption::( - para_id, - assumption, - || Some(ValidationData { - persisted: crate::util::make_persisted_validation_data::(para_id, relay_parent_number)?, - transient: crate::util::make_transient_validation_data::(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::(para_id, assumption, || { + Some(ValidationData { + persisted: crate::util::make_persisted_validation_data::( + para_id, + relay_parent_number, + relay_storage_root, + )?, + transient: crate::util::make_transient_validation_data::( + 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( para_id: ParaId, assumption: OccupiedCoreAssumption, ) -> Option> { + use parity_scale_codec::Decode as _; let relay_parent_number = >::block_number(); - with_assumption::( - para_id, - assumption, - || crate::util::make_persisted_validation_data::(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::(para_id, assumption, || { + crate::util::make_persisted_validation_data::( + para_id, + relay_parent_number, + relay_storage_root, + ) + }) } /// Implementation for the `check_validation_outputs` function of the runtime API. diff --git a/polkadot/runtime/parachains/src/util.rs b/polkadot/runtime/parachains/src/util.rs index d0f8913062..ce041981eb 100644 --- a/polkadot/runtime/parachains/src/util.rs +++ b/polkadot/runtime/parachains/src/util.rs @@ -18,22 +18,25 @@ //! on all modules. use sp_runtime::traits::Saturating; -use primitives::v1::{Id as ParaId, PersistedValidationData, TransientValidationData}; +use primitives::v1::{Id as ParaId, PersistedValidationData, TransientValidationData, Hash}; use crate::{configuration, paras, dmp, hrmp}; -/// Make the persisted validation data for a particular parachain and a specified relay-parent. +/// Make the persisted validation data for a particular parachain, a specified relay-parent and it's +/// storage root. /// /// This ties together the storage of several modules. pub fn make_persisted_validation_data( para_id: ParaId, relay_parent_number: T::BlockNumber, + relay_storage_root: Hash, ) -> Option> { let config = >::config(); Some(PersistedValidationData { parent_head: >::para_head(¶_id)?, block_number: relay_parent_number, + relay_storage_root, hrmp_mqc_heads: >::hrmp_mqc_heads(para_id), dmq_mqc_head: >::dmq_mqc_head(para_id), max_pov_size: config.max_pov_size, diff --git a/polkadot/runtime/rococo/src/lib.rs b/polkadot/runtime/rococo/src/lib.rs index 05b5b92b8d..d5dc73a7ce 100644 --- a/polkadot/runtime/rococo/src/lib.rs +++ b/polkadot/runtime/rococo/src/lib.rs @@ -105,7 +105,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: create_runtime_str!("rococo"), impl_name: create_runtime_str!("parity-rococo-v1"), authoring_version: 0, - spec_version: 12, + spec_version: 13, impl_version: 0, #[cfg(not(feature = "disable-runtime-api"))] apis: RUNTIME_API_VERSIONS,