From a864eaa093b75f7f93421e57d21085ee382007da Mon Sep 17 00:00:00 2001 From: Sergei Shulepov Date: Mon, 4 Jan 2021 17:58:20 +0100 Subject: [PATCH] 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 --- polkadot/node/core/av-store/src/tests.rs | 1 + polkadot/node/core/backing/src/lib.rs | 1 + polkadot/node/core/proposer/src/lib.rs | 10 +++- .../availability-distribution/src/tests.rs | 1 + .../node/test/client/src/block_builder.rs | 11 ++++- polkadot/primitives/src/v1.rs | 2 + .../src/runtime/inclusion.md | 2 +- .../src/runtime/inclusioninherent.md | 6 ++- .../implementers-guide/src/types/candidate.md | 2 + polkadot/runtime/parachains/src/inclusion.rs | 20 +++++++- .../parachains/src/inclusion_inherent.rs | 46 +++++++++++++++---- .../parachains/src/runtime_api_impl/v1.rs | 45 +++++++++++------- polkadot/runtime/parachains/src/util.rs | 7 ++- polkadot/runtime/rococo/src/lib.rs | 2 +- 14 files changed, 120 insertions(+), 36 deletions(-) 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,