diff --git a/polkadot/primitives/src/v1.rs b/polkadot/primitives/src/v1.rs index 1553100b74..a9666fe793 100644 --- a/polkadot/primitives/src/v1.rs +++ b/polkadot/primitives/src/v1.rs @@ -719,16 +719,37 @@ pub struct SessionInfo { sp_api::decl_runtime_apis! { /// The API for querying the state of parachains on-chain. pub trait ParachainHost { + // NOTE: Many runtime API are declared with `#[skip_initialize_block]`. This is because without + // this attribute before each runtime call, the `initialize_block` runtime API will be called. + // That in turns will lead to two things: + // + // (a) The frame_system module will be initialized to the next block. + // (b) Initialization sequences for each runtime module (pallet) will be run. + // + // (a) is undesirable because the runtime APIs are querying the state against a specific + // block state. However, due to that initialization the observed block number would be as if + // it was the next block. + // + // We dont want (b) mainly because block initialization can be very heavy. Upgrade enactment, + // storage migration, and whatever other logic exists in `on_initialize` will be executed + // if not explicitly opted out with the `#[skip_initialize_block]` attribute. + // + // Additionally, some runtime APIs may depend on state that is pruned on the `on_initialize`. + // At the moment of writing, this is `candidate_events`. + /// Get the current validators. + #[skip_initialize_block] fn validators() -> Vec; /// Returns the validator groups and rotation info localized based on the block whose state /// this is invoked on. Note that `now` in the `GroupRotationInfo` should be the successor of /// the number of the block. + #[skip_initialize_block] fn validator_groups() -> (Vec>, GroupRotationInfo); /// Yields information on all availability cores. Cores are either free or occupied. Free /// cores can have paras assigned to them. + #[skip_initialize_block] fn availability_cores() -> Vec>; /// Yields the full validation data for the given ParaId along with an assumption that @@ -736,6 +757,7 @@ sp_api::decl_runtime_apis! { /// /// Returns `None` if either the para is not registered or the assumption is `Freed` /// and the para already occupies a core. + #[skip_initialize_block] fn full_validation_data(para_id: Id, assumption: OccupiedCoreAssumption) -> Option>; @@ -744,24 +766,29 @@ sp_api::decl_runtime_apis! { /// /// Returns `None` if either the para is not registered or the assumption is `Freed` /// and the para already occupies a core. + #[skip_initialize_block] fn persisted_validation_data(para_id: Id, assumption: OccupiedCoreAssumption) -> Option>; /// Checks if the given validation outputs pass the acceptance criteria. + #[skip_initialize_block] fn check_validation_outputs(para_id: Id, outputs: CandidateCommitments) -> bool; /// Returns the session index expected at a child of the block. /// /// This can be used to instantiate a `SigningContext`. + #[skip_initialize_block] fn session_index_for_child() -> SessionIndex; /// Get the session info for the given session, if stored. + #[skip_initialize_block] fn session_info(index: SessionIndex) -> Option; /// Fetch the validation code used by a para, making the given `OccupiedCoreAssumption`. /// /// Returns `None` if either the para is not registered or the assumption is `Freed` /// and the para already occupies a core. + #[skip_initialize_block] fn validation_code(para_id: Id, assumption: OccupiedCoreAssumption) -> Option; @@ -770,26 +797,28 @@ sp_api::decl_runtime_apis! { /// /// `context_height` may be no greater than the height of the block in whose /// state the runtime API is executed. + #[skip_initialize_block] fn historical_validation_code(para_id: Id, context_height: N) -> Option; /// Get the receipt of a candidate pending availability. This returns `Some` for any paras /// assigned to occupied cores in `availability_cores` and `None` otherwise. + #[skip_initialize_block] fn candidate_pending_availability(para_id: Id) -> Option>; /// Get a vector of events concerning candidates that occurred within a block. - // NOTE: this needs to skip block initialization as events are wiped within block - // initialization. #[skip_initialize_block] fn candidate_events() -> Vec>; /// Get all the pending inbound messages in the downward message queue for a para. + #[skip_initialize_block] fn dmq_contents( recipient: Id, ) -> Vec>; /// Get the contents of all channels addressed to the given recipient. Channels that have no /// messages in them are also included. + #[skip_initialize_block] fn inbound_hrmp_channels_contents(recipient: Id) -> BTreeMap>>; } } diff --git a/polkadot/roadmap/implementers-guide/src/runtime/scheduler.md b/polkadot/roadmap/implementers-guide/src/runtime/scheduler.md index ead981b6d6..e66eceefc0 100644 --- a/polkadot/roadmap/implementers-guide/src/runtime/scheduler.md +++ b/polkadot/roadmap/implementers-guide/src/runtime/scheduler.md @@ -162,7 +162,7 @@ AvailabilityCores: Vec>; ParathreadClaimIndex: Vec; /// The block number where the session start occurred. Used to track how many group rotations have occurred. SessionStartBlock: BlockNumber; -/// Currently scheduled cores - free but up to be occupied. Ephemeral storage item that's wiped on finalization. +/// Currently scheduled cores - free but up to be occupied. Scheduled: Vec, // sorted ascending by CoreIndex. ``` @@ -190,13 +190,12 @@ Actions: ## Initialization +1. Free all scheduled cores and return parathread claims to queue, with retries incremented. 1. Schedule free cores using the `schedule(Vec::new())`. ## Finalization -Actions: - -1. Free all scheduled cores and return parathread claims to queue, with retries incremented. +No finalization routine runs for this module. ## Routines diff --git a/polkadot/runtime/common/src/paras_registrar.rs b/polkadot/runtime/common/src/paras_registrar.rs index 55facc5d3c..d34df19971 100644 --- a/polkadot/runtime/common/src/paras_registrar.rs +++ b/polkadot/runtime/common/src/paras_registrar.rs @@ -579,13 +579,9 @@ mod tests { t.into() } - fn init_block() { - println!("Initializing {}", System::block_number()); - System::on_initialize(System::block_number()); - Initializer::on_initialize(System::block_number()); - } - fn run_to_block(n: BlockNumber) { + // NOTE that this function only simulates modules of interest. Depending on new module may + // require adding it here. println!("Running until block {}", n); while System::block_number() < n { let b = System::block_number(); @@ -593,9 +589,11 @@ mod tests { if System::block_number() > 1 { println!("Finalizing {}", System::block_number()); System::on_finalize(System::block_number()); + Initializer::on_finalize(System::block_number()); } // Session change every 3 blocks. if (b + 1) % 3 == 0 { + println!("New session at {}", System::block_number()); Initializer::on_new_session( false, Vec::new().into_iter(), @@ -603,7 +601,9 @@ mod tests { ); } System::set_block_number(b + 1); - init_block(); + println!("Initializing {}", System::block_number()); + System::on_initialize(System::block_number()); + Initializer::on_initialize(System::block_number()); } } @@ -725,8 +725,9 @@ mod tests { WASM_MAGIC.to_vec().into(), ).is_err()); - run_to_block(6); - + // The session will be changed on the 6th block, as part of finalization. The change + // will be observed on the 7th. + run_to_block(7); assert_ok!(Registrar::register_parachain( 1u32.into(), vec![1; 3].into(), diff --git a/polkadot/runtime/parachains/src/inclusion.rs b/polkadot/runtime/parachains/src/inclusion.rs index 3e312a0d45..95481b57a0 100644 --- a/polkadot/runtime/parachains/src/inclusion.rs +++ b/polkadot/runtime/parachains/src/inclusion.rs @@ -399,7 +399,12 @@ impl Module { let validators = Validators::get(); let parent_hash = >::parent_hash(); - let check_cx = CandidateCheckContext::::new(); + + // At the moment we assume (and in fact enforce, below) that the relay-parent is always one + // before of the block where we include a candidate (i.e. this code path). + let now = >::block_number(); + let relay_parent_number = now - One::one(); + let check_cx = CandidateCheckContext::::new(now, relay_parent_number); // do all checks before writing storage. let core_indices_and_backers = { @@ -483,7 +488,10 @@ impl Module { { // this should never fail because the para is registered let persisted_validation_data = - match crate::util::make_persisted_validation_data::(para_id) { + match crate::util::make_persisted_validation_data::( + para_id, + relay_parent_number, + ) { Some(l) => l, None => { // We don't want to error out here because it will @@ -592,7 +600,7 @@ impl Module { hash: candidate_hash, descriptor, availability_votes, - relay_parent_number: check_cx.relay_parent_number, + relay_parent_number, backers, backed_in_number: check_cx.now, }); @@ -603,11 +611,17 @@ impl Module { } /// Run the acceptance criteria checks on the given candidate commitments. - pub(crate) fn check_validation_outputs( + pub(crate) fn check_validation_outputs_for_runtime_api( para_id: ParaId, validation_outputs: primitives::v1::CandidateCommitments, ) -> bool { - if let Err(err) = CandidateCheckContext::::new().check_validation_outputs( + // This function is meant to be called from the runtime APIs against the relay-parent, hence + // `relay_parent_number` is equal to `now`. + let now = >::block_number(); + let relay_parent_number = now; + let check_cx = CandidateCheckContext::::new(now, relay_parent_number); + + if let Err(err) = check_cx.check_validation_outputs( para_id, &validation_outputs.head_data, &validation_outputs.new_validation_code, @@ -812,12 +826,11 @@ struct CandidateCheckContext { } impl CandidateCheckContext { - fn new() -> Self { - let now = >::block_number(); + fn new(now: T::BlockNumber, relay_parent_number: T::BlockNumber) -> Self { Self { config: >::config(), now, - relay_parent_number: now - One::one(), + relay_parent_number, } } @@ -1111,8 +1124,12 @@ mod tests { } fn make_vdata_hash(para_id: ParaId) -> Option { + let relay_parent_number = >::block_number() - 1; let persisted_validation_data - = crate::util::make_persisted_validation_data::(para_id)?; + = crate::util::make_persisted_validation_data::( + para_id, + relay_parent_number, + )?; Some(persisted_validation_data.hash()) } diff --git a/polkadot/runtime/parachains/src/initializer.rs b/polkadot/runtime/parachains/src/initializer.rs index 409b52a260..dd19448925 100644 --- a/polkadot/runtime/parachains/src/initializer.rs +++ b/polkadot/runtime/parachains/src/initializer.rs @@ -25,7 +25,6 @@ use primitives::v1::ValidatorId; use frame_support::{ decl_storage, decl_module, decl_error, traits::Randomness, }; -use sp_runtime::traits::One; use parity_scale_codec::{Encode, Decode}; use crate::{ configuration::{self, HostConfiguration}, @@ -63,8 +62,7 @@ impl> Default for SessionChangeNotification { - apply_at: N, +struct BufferedSessionChange { validators: Vec, queued: Vec, session_index: sp_staking::SessionIndex, @@ -98,12 +96,12 @@ decl_storage! { HasInitialized: Option<()>; /// Buffered session changes along with the block number at which they should be applied. /// - /// Typically this will be empty or one element long, with the single element having a block - /// number of the next block. + /// Typically this will be empty or one element long. Apart from that this item never hits + /// the storage. /// /// However this is a `Vec` regardless to handle various edge cases that may occur at runtime /// upgrade boundaries or if governance intervenes. - BufferedSessionChanges: Vec>; + BufferedSessionChanges: Vec; } } @@ -117,21 +115,6 @@ decl_module! { type Error = Error; fn on_initialize(now: T::BlockNumber) -> Weight { - // Apply buffered session changes before initializing modules, so they - // can be initialized with respect to the current validator set. - >::mutate(|v| { - let drain_up_to = v.iter().take_while(|b| b.apply_at <= now).count(); - - // apply only the last session as all others lasted less than a block (weirdly). - if let Some(buffered) = v.drain(..drain_up_to).last() { - Self::apply_new_session( - buffered.session_index, - buffered.validators, - buffered.queued, - ); - } - }); - // The other modules are initialized in this order: // - Configuration // - Paras @@ -158,7 +141,6 @@ decl_module! { fn on_finalize() { // reverse initialization order. - hrmp::Module::::initializer_finalize(); ump::Module::::initializer_finalize(); dmp::Module::::initializer_finalize(); @@ -167,6 +149,20 @@ decl_module! { scheduler::Module::::initializer_finalize(); paras::Module::::initializer_finalize(); configuration::Module::::initializer_finalize(); + + // Apply buffered session changes as the last thing. This way the runtime APIs and the + // next block will observe the next session. + // + // Note that we only apply the last session as all others lasted less than a block (weirdly). + if let Some(BufferedSessionChange { + session_index, + validators, + queued, + }) = BufferedSessionChanges::take().pop() + { + Self::apply_new_session(session_index, validators, queued); + } + HasInitialized::take(); } } @@ -213,7 +209,7 @@ impl Module { } /// Should be called when a new session occurs. Buffers the session notification to be applied - /// at the next block. If `queued` is `None`, the `validators` are considered queued. + /// at the end of the block. If `queued` is `None`, the `validators` are considered queued. fn on_new_session<'a, I: 'a>( _changed: bool, session_index: sp_staking::SessionIndex, @@ -229,8 +225,7 @@ impl Module { validators.clone() }; - >::mutate(|v| v.push(BufferedSessionChange { - apply_at: >::block_number() + One::one(), + BufferedSessionChanges::mutate(|v| v.push(BufferedSessionChange { validators, queued, session_index, @@ -264,7 +259,7 @@ impl pallet_session::OneSessionHandler>::get(); + let v = ::get(); assert_eq!(v.len(), 1); - - let apply_at = now + 1; - assert_eq!(v[0].apply_at, apply_at); }); } #[test] - fn session_change_applied_on_initialize() { + fn session_change_applied_on_finalize() { new_test_ext(Default::default()).execute_with(|| { Initializer::on_initialize(1); - - let now = System::block_number(); Initializer::on_new_session( false, 1, @@ -302,9 +292,9 @@ mod tests { Some(Vec::new().into_iter()), ); - Initializer::on_initialize(now + 1); + Initializer::on_finalize(1); - assert!(>::get().is_empty()); + assert!(::get().is_empty()); }); } diff --git a/polkadot/runtime/parachains/src/runtime_api_impl/v1.rs b/polkadot/runtime/parachains/src/runtime_api_impl/v1.rs index a171f9eb44..d3ef7a1ee3 100644 --- a/polkadot/runtime/parachains/src/runtime_api_impl/v1.rs +++ b/polkadot/runtime/parachains/src/runtime_api_impl/v1.rs @@ -193,12 +193,13 @@ pub fn full_validation_data( ) -> Option> { + let relay_parent_number = >::block_number(); with_assumption::( para_id, assumption, || Some(ValidationData { - persisted: crate::util::make_persisted_validation_data::(para_id)?, - transient: crate::util::make_transient_validation_data::(para_id)?, + persisted: crate::util::make_persisted_validation_data::(para_id, relay_parent_number)?, + transient: crate::util::make_transient_validation_data::(para_id, relay_parent_number)?, }), ) } @@ -208,10 +209,11 @@ pub fn persisted_validation_data( para_id: ParaId, assumption: OccupiedCoreAssumption, ) -> Option> { + let relay_parent_number = >::block_number(); with_assumption::( para_id, assumption, - || crate::util::make_persisted_validation_data::(para_id), + || crate::util::make_persisted_validation_data::(para_id, relay_parent_number), ) } @@ -220,7 +222,7 @@ pub fn check_validation_outputs( para_id: ParaId, outputs: primitives::v1::CandidateCommitments, ) -> bool { - >::check_validation_outputs(para_id, outputs) + >::check_validation_outputs_for_runtime_api(para_id, outputs) } /// Implementation for the `session_index_for_child` function of the runtime API. diff --git a/polkadot/runtime/parachains/src/scheduler.rs b/polkadot/runtime/parachains/src/scheduler.rs index 7eaf8e6c13..27fa9efdfd 100644 --- a/polkadot/runtime/parachains/src/scheduler.rs +++ b/polkadot/runtime/parachains/src/scheduler.rs @@ -182,7 +182,7 @@ decl_storage! { ParathreadClaimIndex: Vec; /// The block number where the session start occurred. Used to track how many group rotations have occurred. SessionStartBlock get(fn session_start_block): T::BlockNumber; - /// Currently scheduled cores - free but up to be occupied. Ephemeral storage item that's wiped on finalization. + /// Currently scheduled cores - free but up to be occupied. /// /// Bounded by the number of cores: one for each parachain and parathread multiplexer. Scheduled get(fn scheduled): Vec; // sorted ascending by CoreIndex. @@ -203,13 +203,6 @@ decl_module! { impl Module { /// Called by the initializer to initialize the scheduler module. pub(crate) fn initializer_initialize(_now: T::BlockNumber) -> Weight { - Self::schedule(Vec::new()); - - 0 - } - - /// Called by the initializer to finalize the scheduler module. - pub(crate) fn initializer_finalize() { // Free all scheduled cores and return parathread claims to queue, with retries incremented. let config = >::config(); ParathreadQueue::mutate(|queue| { @@ -225,10 +218,16 @@ impl Module { } } } - }) + }); + Self::schedule(Vec::new()); + + 0 } + /// Called by the initializer to finalize the scheduler module. + pub(crate) fn initializer_finalize() {} + /// Called by the initializer to note that a new session has started. pub(crate) fn initializer_on_new_session(notification: &SessionChangeNotification) { let &SessionChangeNotification { diff --git a/polkadot/runtime/parachains/src/util.rs b/polkadot/runtime/parachains/src/util.rs index 151222cbec..d0f8913062 100644 --- a/polkadot/runtime/parachains/src/util.rs +++ b/polkadot/runtime/parachains/src/util.rs @@ -17,19 +17,19 @@ //! Utilities that don't belong to any particular module but may draw //! on all modules. -use sp_runtime::traits::{One, Saturating}; +use sp_runtime::traits::Saturating; use primitives::v1::{Id as ParaId, PersistedValidationData, TransientValidationData}; use crate::{configuration, paras, dmp, hrmp}; -/// Make the persisted validation data for a particular parachain. +/// Make the persisted validation data for a particular parachain and a specified relay-parent. /// /// This ties together the storage of several modules. pub fn make_persisted_validation_data( para_id: ParaId, + relay_parent_number: T::BlockNumber, ) -> Option> { let config = >::config(); - let relay_parent_number = >::block_number() - One::one(); Some(PersistedValidationData { parent_head: >::para_head(¶_id)?, @@ -40,14 +40,14 @@ pub fn make_persisted_validation_data( }) } -/// Make the transient validation data for a particular parachain. +/// Make the transient validation data for a particular parachain and a specified relay-parent. /// /// This ties together the storage of several modules. pub fn make_transient_validation_data( para_id: ParaId, + relay_parent_number: T::BlockNumber, ) -> Option> { let config = >::config(); - let relay_parent_number = >::block_number() - One::one(); let freq = config.validation_upgrade_frequency; let delay = config.validation_upgrade_delay;