From bf3938b5b2d001ca6b7ed467e89ff6c20902940f Mon Sep 17 00:00:00 2001 From: Sergei Shulepov Date: Fri, 25 Jun 2021 11:38:48 +0000 Subject: [PATCH 1/5] Introduce upgrade goahead and upgrade restriction signals --- primitives/src/v1/mod.rs | 64 ++++- .../src/runtime/inclusion.md | 2 +- .../implementers-guide/src/runtime/paras.md | 38 ++- runtime/parachains/src/inclusion.rs | 21 +- runtime/parachains/src/paras.rs | 238 ++++++++++++++++-- 5 files changed, 332 insertions(+), 31 deletions(-) diff --git a/primitives/src/v1/mod.rs b/primitives/src/v1/mod.rs index 0fe6ed84702f..c5213a01d1a6 100644 --- a/primitives/src/v1/mod.rs +++ b/primitives/src/v1/mod.rs @@ -178,8 +178,41 @@ pub mod well_known_keys { .collect() }) } -} + /// The signal that indicates whether the parachain should go-ahead with the proposed validation + /// code upgrade. + /// + /// The storage entry stores a value of `UpgradeGoAhead` type. + pub fn upgrade_go_ahead_signal(para_id: Id) -> Vec { + let prefix = hex!["cd710b30bd2eab0352ddcc26417aa1949e94c040f5e73d9b7addd6cb603d15d3"]; + + para_id.using_encoded(|para_id: &[u8]| { + prefix.as_ref() + .iter() + .chain(twox_64(para_id).iter()) + .chain(para_id.iter()) + .cloned() + .collect() + }) + } + + /// The signal that indicates whether the parachain is disallowed to signal an upgrade at this + /// relay-parent. + /// + /// The storage entry stores a value of `UpgradeRestriction` type. + pub fn upgrade_restriction_signal(para_id: Id) -> Vec { + let prefix = hex!["cd710b30bd2eab0352ddcc26417aa194f27bbb460270642b5bcaf032ea04d56a"]; + + para_id.using_encoded(|para_id: &[u8]| { + prefix.as_ref() + .iter() + .chain(twox_64(para_id).iter()) + .chain(para_id.iter()) + .cloned() + .collect() + }) + } +} /// Unique identifier for the Parachains Inherent pub const PARACHAINS_INHERENT_IDENTIFIER: InherentIdentifier = *b"parachn0"; @@ -1030,6 +1063,35 @@ pub struct AbridgedHrmpChannel { pub mqc_head: Option, } +/// A possible upgrade restriction that prevents a parachain from performing an upgrade. +#[derive(Encode, Decode, PartialEq, RuntimeDebug)] +pub enum UpgradeRestriction { + /// There is an upgrade restriction and there are no details about its specifics nor how long + /// it could last. + Present, +} + +/// A struct that the relay-chain communicates to a parachain indicating what course of action the +/// parachain should take in the coordinated parachain validation code upgrade process. +/// +/// This data type appears in the last step of the upgrade process. After the parachain observes it +/// and reacts to it the upgrade process concludes. +#[derive(Encode, Decode, PartialEq, RuntimeDebug)] +pub enum UpgradeGoAhead { + /// Abort the upgrade process. There is something wrong with the validation code previously + /// submitted by the parachain. This variant can also be used to prevent upgrades by the governance + /// should an emergency emerge. + /// + /// The expected reaction on this variant is that the parachain will admit this message and + /// remove all the data about the pending upgrade. Depending on the nature of the problem (to + /// be examined offchain for now), it can try to send another validation code or just retry later. + Abort, + /// Apply the pending code change. The parablock that is built on a relay-parent that is descendant + /// of the relay-parent where the parachain observed this signal must use the upgraded validation + /// code. + GoAhead, +} + /// Consensus engine id for polkadot v1 consensus engine. pub const POLKADOT_ENGINE_ID: runtime_primitives::ConsensusEngineId = *b"POL1"; diff --git a/roadmap/implementers-guide/src/runtime/inclusion.md b/roadmap/implementers-guide/src/runtime/inclusion.md index 84c1cc9d8b61..44b13dd18945 100644 --- a/roadmap/implementers-guide/src/runtime/inclusion.md +++ b/roadmap/implementers-guide/src/runtime/inclusion.md @@ -70,7 +70,7 @@ All failed checks should lead to an unrecoverable error making the block invalid 1. create a corresponding entry in the `PendingAvailabilityCommitments` with the commitments. 1. Return a `Vec` of all scheduled cores of the list of passed assignments that a candidate was successfully backed for, sorted ascending by CoreIndex. * `enact_candidate(relay_parent_number: BlockNumber, CommittedCandidateReceipt)`: - 1. If the receipt contains a code upgrade, Call `Paras::schedule_code_upgrade(para_id, code, relay_parent_number + config.validationl_upgrade_delay)`. + 1. If the receipt contains a code upgrade, Call `Paras::schedule_code_upgrade(para_id, code, relay_parent_number, config)`. > TODO: Note that this is safe as long as we never enact candidates where the relay parent is across a session boundary. In that case, which we should be careful to avoid with contextual execution, the configuration might have changed and the para may de-sync from the host's understanding of it. 1. Reward all backing validators of each candidate, contained within the `backers` field. 1. call `Ump::receive_upward_messages` for each backed candidate, using the [`UpwardMessage`s](../types/messages.md#upward-message) from the [`CandidateCommitments`](../types/candidate.md#candidate-commitments). diff --git a/roadmap/implementers-guide/src/runtime/paras.md b/roadmap/implementers-guide/src/runtime/paras.md index a06580e312c8..349e47521652 100644 --- a/roadmap/implementers-guide/src/runtime/paras.md +++ b/roadmap/implementers-guide/src/runtime/paras.md @@ -137,6 +137,35 @@ PastCodePruning: Vec<(ParaId, BlockNumber)>; FutureCodeUpgrades: map ParaId => Option; /// The actual future code of a para. FutureCodeHash: map ParaId => Option; +/// This is used by the relay-chain to communicate to a parachain a go-ahead with in the upgrade procedure. +/// +/// This value is abscent when there are no upgrades scheduled or during the time the relay chain +/// performs the checks. It is set at the first relay-chain block when the corresponding parachain +/// can switch its upgrade function. As soon as the parachain's block is included, the value +/// gets reset to `None`. +/// +/// NOTE that this field is used by parachains via merkle storage proofs, therefore changing +/// the format will require migration of parachains. +UpgradeGoAheadSignal: map hasher(twox_64_concat) ParaId => Option; +/// This is used by the relay-chain to communicate that there are restrictions for performing +/// an upgrade for this parachain. +/// +/// This may be a because the parachain waits for the upgrade cooldown to expire. Another +/// potential use case is when we want to perform some maintanance (such as storage migration) +/// we could restrict upgrades to make the process simpler. +/// +/// NOTE that this field is used by parachains via merkle storage proofs, therefore changing +/// the format will require migration of parachains. +UpgradeRestrictionSignal: map hasher(twox_64_concat) ParaId => Option; +/// The list of parachains that are awaiting for their upgrade restriction to cooldown. +/// +/// Ordered ascending by block number. +UpgradeCooldowns: Vec<(ParaId, T::BlockNumber)>; +/// The list of upcoming code upgrades. Each item is a pair of which para performs a code +/// upgrade and at which relay-chain block it is expected at. +/// +/// Ordered ascending by block number. +UpcomingUpgrades: Vec<(ParaId, T::BlockNumber)>; /// The actions to perform during the start of a specific session index. ActionsQueue: map SessionIndex => Vec; /// Upcoming paras instantiation arguments. @@ -171,6 +200,9 @@ CodeByHash: map ValidationCodeHash => Option 1. Do pruning based on all entries in `PastCodePruning` with `BlockNumber <= now`. Update the corresponding `PastCodeMeta` and `PastCode` accordingly. +1. Toggle the upgrade related signals + 1. Collect all `(para_id, expected_at)` from `UpcomingUpgrades` where `expected_at <= now` and prune them. For each para pruned set `UpgradeGoAheadSignal` to `GoAhead`. + 1. Collect all `(para_id, next_possible_upgrade_at)` from `UpgradeCooldowns` where `next_possible_upgrade_at <= now` and prune them. For each para prunned set `UpgradeRestrictionSignal` to `Present`. ## Routines @@ -179,12 +211,12 @@ CodeByHash: map ValidationCodeHash => Option * `schedule_para_cleanup(ParaId)`: Schedule a para to be cleaned up after the next full session. * `schedule_parathread_upgrade(ParaId)`: Schedule a parathread to be upgraded to a parachain. * `schedule_parachain_downgrade(ParaId)`: Schedule a parachain to be downgraded to a parathread. -* `schedule_code_upgrade(ParaId, CurrentCode, expected_at: BlockNumber)`: Schedule a future code +* `schedule_code_upgrade(ParaId, CurrentCode, relay_parent: BlockNumber, HostConfiguration)`: Schedule a future code upgrade of the given parachain, to be applied after inclusion of a block of the same parachain - executed in the context of a relay-chain block with number >= `expected_at`. + executed in the context of a relay-chain block with number >= `relay_parent + config.validation_upgrade_delay`. If the upgrade is scheduled `UpgradeRestrictionSignal` is set and it will remain set until `relay_parent + config.validation_upgrade_frequency`. * `note_new_head(ParaId, HeadData, BlockNumber)`: note that a para has progressed to a new head, where the new head was executed in the context of a relay-chain block with given number. This will - apply pending code upgrades based on the block number provided. + apply pending code upgrades based on the block number provided. If an upgrade took place it will clear the `UpgradeGoAheadSignal`. * `validation_code_at(ParaId, at: BlockNumber, assume_intermediate: Option)`: Fetches the validation code to be used when validating a block in the context of the given relay-chain height. A second block number parameter may be used to tell the lookup to proceed as if an diff --git a/runtime/parachains/src/inclusion.rs b/runtime/parachains/src/inclusion.rs index 79b682a4d192..db2f770724bb 100644 --- a/runtime/parachains/src/inclusion.rs +++ b/runtime/parachains/src/inclusion.rs @@ -682,7 +682,8 @@ impl Module { weight += >::schedule_code_upgrade( receipt.descriptor.para_id, new_code, - relay_parent_number + config.validation_upgrade_delay, + relay_parent_number, + &config, ); } @@ -2038,13 +2039,19 @@ mod tests { BackingKind::Threshold, )); - Paras::schedule_code_upgrade( - chain_a, - vec![1, 2, 3, 4].into(), - 10, - ); + { + let cfg = Configuration::config(); + let expected_at = 10 + cfg.validation_upgrade_delay; + assert_eq!(expected_at, 10); + Paras::schedule_code_upgrade( + chain_a, + vec![1, 2, 3, 4].into(), + expected_at, + &cfg, + ); - assert_eq!(Paras::last_code_upgrade(chain_a, true), Some(10)); + assert_eq!(Paras::last_code_upgrade(chain_a, true), Some(expected_at)); + } assert_eq!( Inclusion::process_candidates( diff --git a/runtime/parachains/src/paras.rs b/runtime/parachains/src/paras.rs index 2d64721b833c..4b673294ae23 100644 --- a/runtime/parachains/src/paras.rs +++ b/runtime/parachains/src/paras.rs @@ -29,6 +29,7 @@ use sp_std::result; use sp_std::marker::PhantomData; use primitives::v1::{ Id as ParaId, ValidationCode, ValidationCodeHash, HeadData, SessionIndex, ConsensusLog, + UpgradeGoAhead, UpgradeRestriction, }; use sp_runtime::{traits::One, DispatchResult, SaturatedConversion}; use frame_system::ensure_root; @@ -316,6 +317,35 @@ decl_storage! { /// /// Corresponding code can be retrieved with [`CodeByHash`]. FutureCodeHash: map hasher(twox_64_concat) ParaId => Option; + /// This is used by the relay-chain to communicate to a parachain a go-ahead with in the upgrade procedure. + /// + /// This value is abscent when there are no upgrades scheduled or during the time the relay chain + /// performs the checks. It is set at the first relay-chain block when the corresponding parachain + /// can switch its upgrade function. As soon as the parachain's block is included, the value + /// gets reset to `None`. + /// + /// NOTE that this field is used by parachains via merkle storage proofs, therefore changing + /// the format will require migration of parachains. + UpgradeGoAheadSignal: map hasher(twox_64_concat) ParaId => Option; + /// This is used by the relay-chain to communicate that there are restrictions for performing + /// an upgrade for this parachain. + /// + /// This may be a because the parachain waits for the upgrade cooldown to expire. Another + /// potential use case is when we want to perform some maintanance (such as storage migration) + /// we could restrict upgrades to make the process simpler. + /// + /// NOTE that this field is used by parachains via merkle storage proofs, therefore changing + /// the format will require migration of parachains. + UpgradeRestrictionSignal: map hasher(twox_64_concat) ParaId => Option; + /// The list of parachains that are awaiting for their upgrade restriction to cooldown. + /// + /// Ordered ascending by block number. + UpgradeCooldowns: Vec<(ParaId, T::BlockNumber)>; + /// The list of upcoming code upgrades. Each item is a pair of which para performs a code + /// upgrade and at which relay-chain block it is expected at. + /// + /// Ordered ascending by block number. + UpcomingUpgrades: Vec<(ParaId, T::BlockNumber)>; /// The actions to perform during the start of a specific session index. ActionsQueue get(fn actions_queue): map hasher(twox_64_concat) SessionIndex => Vec; /// Upcoming paras instantiation arguments. @@ -421,11 +451,17 @@ decl_module! { Self::deposit_event(Event::CurrentHeadUpdated(para)); } - /// Schedule a code upgrade for block `expected_at`. + /// Schedule an upgrade as if it was scheduled in the given relay parent block. #[weight = 0] - fn force_schedule_code_upgrade(origin, para: ParaId, new_code: ValidationCode, expected_at: T::BlockNumber) { + fn force_schedule_code_upgrade( + origin, + para: ParaId, + new_code: ValidationCode, + relay_parent_number: T::BlockNumber, + ) { ensure_root(origin)?; - Self::schedule_code_upgrade(para, new_code, expected_at); + let config = configuration::Module::::config(); + Self::schedule_code_upgrade(para, new_code, relay_parent_number, &config); Self::deposit_event(Event::CodeUpgradeScheduled(para)); } @@ -458,7 +494,8 @@ decl_module! { impl Module { /// Called by the initializer to initialize the configuration module. pub(crate) fn initializer_initialize(now: T::BlockNumber) -> Weight { - Self::prune_old_code(now) + let weight = Self::prune_old_code(now); + weight + Self::process_scheduled_upgrade_changes(now) } /// Called by the initializer to finalize the configuration module. @@ -545,6 +582,8 @@ impl Module { ::Heads::remove(¶); ::FutureCodeUpgrades::remove(¶); + ::UpgradeGoAheadSignal::remove(¶); + ::UpgradeRestrictionSignal::remove(¶); ParaLifecycles::remove(¶); let removed_future_code_hash = ::FutureCodeHash::take(¶); if let Some(removed_future_code_hash) = removed_future_code_hash { @@ -561,6 +600,22 @@ impl Module { } } + if !outgoing.is_empty() { + // Filter offboarded parachains from the upcoming upgrades list. + // + // We do it after the offboarding to get away with only a single read/write. + ::UpcomingUpgrades::mutate(|upcoming_upgrades| { + *upcoming_upgrades = sp_std::mem::take(upcoming_upgrades) + .into_iter() + .filter(|&(ref para, _)| { + // NOTE this iterates over upcoming upgrades and outgoing. We do not expect + // either of these to be large. Thus should be fine. + !outgoing.contains(para) + }) + .collect(); + }); + } + // Place the new parachains set in storage. ::Parachains::set(parachains); @@ -654,6 +709,42 @@ impl Module { T::DbWeight::get().reads_writes(1 + pruning_tasks_done, 2 * pruning_tasks_done) } + /// Process the timers related to upgrades. Specifically, the upgrade go ahead signals toggle + /// and the upgrade cooldown restrictions. + /// + /// Takes the current block number and returns the weight consumed. + fn process_scheduled_upgrade_changes(now: T::BlockNumber) -> Weight { + let upgrades_signaled = ::UpcomingUpgrades::mutate( + |upcoming_upgrades: &mut Vec<(ParaId, T::BlockNumber)>| { + let num = upcoming_upgrades + .iter() + .take_while(|&(_, at)| at <= &now) + .count(); + for (para, _) in upcoming_upgrades.drain(..num) { + ::UpgradeGoAheadSignal::insert(¶, UpgradeGoAhead::GoAhead); + } + num + }, + ); + let cooldowns_expired = ::UpgradeCooldowns::mutate( + |upgrade_cooldowns: &mut Vec<(ParaId, T::BlockNumber)>| { + let num = upgrade_cooldowns + .iter() + .take_while(|&(_, at)| at <= &now) + .count(); + for (para, _) in upgrade_cooldowns.drain(..num) { + ::UpgradeRestrictionSignal::remove(¶); + } + num + }, + ); + + T::DbWeight::get().reads_writes( + 2, + upgrades_signaled as u64 + cooldowns_expired as u64 + ) + } + /// Verify that `schedule_para_initialize` can be called successfully. /// /// Returns false if para is already registered in the system. @@ -759,14 +850,39 @@ impl Module { pub(crate) fn schedule_code_upgrade( id: ParaId, new_code: ValidationCode, - expected_at: T::BlockNumber, + relay_parent_number: T::BlockNumber, + cfg: &configuration::HostConfiguration, ) -> Weight { ::FutureCodeUpgrades::mutate(&id, |up| { if up.is_some() { T::DbWeight::get().reads_writes(1, 0) } else { + let expected_at = relay_parent_number + cfg.validation_upgrade_delay; + let next_possible_upgrade_at = relay_parent_number + cfg.validation_upgrade_frequency; + *up = Some(expected_at); + ::UpcomingUpgrades::mutate(|upcoming_upgrades| { + let insert_idx = upcoming_upgrades + .binary_search_by_key(&expected_at, |&(_, b)| b) + .unwrap_or_else(|idx| idx); + upcoming_upgrades.insert(insert_idx, (id, expected_at)); + }); + + // From the moment of signalling of the upgrade until the cooldown expires, the + // parachain is disallowed to make further upgrades. Therefore set the upgrade + // permission signal to disallowed and activate the cooldown timer. + ::UpgradeRestrictionSignal::insert( + &id, + UpgradeRestriction::Present, + ); + ::UpgradeCooldowns::mutate(|upgrade_cooldowns| { + let insert_idx = upgrade_cooldowns + .binary_search_by_key(&next_possible_upgrade_at, |&(_, b)| b) + .unwrap_or_else(|idx| idx); + upgrade_cooldowns.insert(insert_idx, (id, next_possible_upgrade_at)); + }); + let new_code_hash = new_code.hash(); let expected_at_u32 = expected_at.saturated_into(); let log = ConsensusLog::ParaScheduleUpgradeCode(id, new_code_hash, expected_at_u32); @@ -774,14 +890,14 @@ impl Module { let (reads, writes) = Self::increase_code_ref(&new_code_hash, &new_code); FutureCodeHash::insert(&id, new_code_hash); - T::DbWeight::get().reads_writes(1 + reads, 2 + writes) + T::DbWeight::get().reads_writes(2 + reads, 3 + writes) } }) } /// Note that a para has progressed to a new head, where the new head was executed in the context /// of a relay-chain block with given number. This will apply pending code upgrades based - /// on the block number provided. + /// on the relay-parent block number provided. pub(crate) fn note_new_head( id: ParaId, new_head: HeadData, @@ -792,6 +908,7 @@ impl Module { if let Some(expected_at) = ::FutureCodeUpgrades::get(&id) { if expected_at <= execution_context { ::FutureCodeUpgrades::remove(&id); + ::UpgradeGoAheadSignal::remove(&id); // Both should always be `Some` in this case, since a code upgrade is scheduled. let new_code_hash = FutureCodeHash::take(&id).unwrap_or_default(); @@ -959,7 +1076,7 @@ mod tests { traits::{OnFinalize, OnInitialize} }; - use crate::mock::{new_test_ext, Paras, Shared, System, MockGenesisConfig}; + use crate::mock::{new_test_ext, Configuration, Paras, Shared, System, MockGenesisConfig}; use crate::configuration::HostConfiguration; fn run_to_block(to: BlockNumber, new_session: Option>) { @@ -1249,6 +1366,7 @@ mod tests { fn code_upgrade_applied_after_delay() { let code_retention_period = 10; let validation_upgrade_delay = 5; + let validation_upgrade_frequency = 10; let original_code = ValidationCode(vec![1, 2, 3]); let paras = vec![ @@ -1265,6 +1383,7 @@ mod tests { config: HostConfiguration { code_retention_period, validation_upgrade_delay, + validation_upgrade_frequency, ..Default::default() }, ..Default::default() @@ -1284,12 +1403,15 @@ mod tests { let expected_at = { // this parablock is in the context of block 1. let expected_at = 1 + validation_upgrade_delay; - Paras::schedule_code_upgrade(para_id, new_code.clone(), expected_at); + let next_possible_upgrade_at = 1 + validation_upgrade_frequency; + Paras::schedule_code_upgrade(para_id, new_code.clone(), 1, &Configuration::config()); Paras::note_new_head(para_id, Default::default(), 1); assert!(Paras::past_code_meta(¶_id).most_recent_change().is_none()); assert_eq!(::FutureCodeUpgrades::get(¶_id), Some(expected_at)); assert_eq!(::FutureCodeHash::get(¶_id), Some(new_code.hash())); + assert_eq!(::UpcomingUpgrades::get(), vec![(para_id, expected_at)]); + assert_eq!(::UpgradeCooldowns::get(), vec![(para_id, next_possible_upgrade_at)]); assert_eq!(Paras::current_code(¶_id), Some(original_code.clone())); check_code_is_stored(&original_code); check_code_is_stored(&new_code); @@ -1307,6 +1429,7 @@ mod tests { assert!(Paras::past_code_meta(¶_id).most_recent_change().is_none()); assert_eq!(::FutureCodeUpgrades::get(¶_id), Some(expected_at)); assert_eq!(::FutureCodeHash::get(¶_id), Some(new_code.hash())); + assert_eq!(::UpgradeGoAheadSignal::get(¶_id), Some(UpgradeGoAhead::GoAhead)); assert_eq!(Paras::current_code(¶_id), Some(original_code.clone())); check_code_is_stored(&original_code); check_code_is_stored(&new_code); @@ -1329,6 +1452,7 @@ mod tests { ); assert!(::FutureCodeUpgrades::get(¶_id).is_none()); assert!(::FutureCodeHash::get(¶_id).is_none()); + assert!(::UpgradeGoAheadSignal::get(¶_id).is_none()); assert_eq!(Paras::current_code(¶_id), Some(new_code.clone())); check_code_is_stored(&original_code); check_code_is_stored(&new_code); @@ -1340,6 +1464,7 @@ mod tests { fn code_upgrade_applied_after_delay_even_when_late() { let code_retention_period = 10; let validation_upgrade_delay = 5; + let validation_upgrade_frequency = 10; let original_code = ValidationCode(vec![1, 2, 3]); let paras = vec![ @@ -1356,6 +1481,7 @@ mod tests { config: HostConfiguration { code_retention_period, validation_upgrade_delay, + validation_upgrade_frequency, ..Default::default() }, ..Default::default() @@ -1373,12 +1499,16 @@ mod tests { let expected_at = { // this parablock is in the context of block 1. let expected_at = 1 + validation_upgrade_delay; - Paras::schedule_code_upgrade(para_id, new_code.clone(), expected_at); + let next_possible_upgrade_at = 1 + validation_upgrade_frequency; + Paras::schedule_code_upgrade(para_id, new_code.clone(), 1, &Configuration::config()); Paras::note_new_head(para_id, Default::default(), 1); assert!(Paras::past_code_meta(¶_id).most_recent_change().is_none()); assert_eq!(::FutureCodeUpgrades::get(¶_id), Some(expected_at)); assert_eq!(::FutureCodeHash::get(¶_id), Some(new_code.hash())); + assert_eq!(::UpcomingUpgrades::get(), vec![(para_id, expected_at)]); + assert_eq!(::UpgradeCooldowns::get(), vec![(para_id, next_possible_upgrade_at)]); + assert!(::UpgradeGoAheadSignal::get(¶_id).is_none()); assert_eq!(Paras::current_code(¶_id), Some(original_code.clone())); expected_at @@ -1389,6 +1519,12 @@ mod tests { // the candidate is in the context of the first descendent of `expected_at`, and triggers // the upgrade. { + // The signal should be set to go-ahead until the new head is actually processed. + assert_eq!( + ::UpgradeGoAheadSignal::get(¶_id), + Some(UpgradeGoAhead::GoAhead), + ); + Paras::note_new_head(para_id, Default::default(), expected_at + 4); assert_eq!( @@ -1423,6 +1559,7 @@ mod tests { ); assert!(::FutureCodeUpgrades::get(¶_id).is_none()); assert!(::FutureCodeHash::get(¶_id).is_none()); + assert!(::UpgradeGoAheadSignal::get(¶_id).is_none()); assert_eq!(Paras::current_code(¶_id), Some(new_code.clone())); } }); @@ -1431,6 +1568,7 @@ mod tests { #[test] fn submit_code_change_when_not_allowed_is_err() { let code_retention_period = 10; + let validation_upgrade_delay = 7; let paras = vec![ (0u32.into(), ParaGenesisArgs { @@ -1445,6 +1583,7 @@ mod tests { configuration: crate::configuration::GenesisConfig { config: HostConfiguration { code_retention_period, + validation_upgrade_delay, ..Default::default() }, ..Default::default() @@ -1458,14 +1597,29 @@ mod tests { let newer_code = ValidationCode(vec![4, 5, 6, 7]); run_to_block(1, None); - - Paras::schedule_code_upgrade(para_id, new_code.clone(), 8); - assert_eq!(::FutureCodeUpgrades::get(¶_id), Some(8)); + Paras::schedule_code_upgrade( + para_id, + new_code.clone(), + 1, + &Configuration::config(), + ); + assert_eq!(::FutureCodeUpgrades::get(¶_id), Some(1 + validation_upgrade_delay)); assert_eq!(::FutureCodeHash::get(¶_id), Some(new_code.hash())); check_code_is_stored(&new_code); - Paras::schedule_code_upgrade(para_id, newer_code.clone(), 10); - assert_eq!(::FutureCodeUpgrades::get(¶_id), Some(8)); + // We expect that if an upgrade is signalled while there is already one pending we just + // ignore it. Note that this is only true from perspective of this module. + run_to_block(2, None); + Paras::schedule_code_upgrade( + para_id, + newer_code.clone(), + 2, + &Configuration::config(), + ); + assert_eq!( + ::FutureCodeUpgrades::get(¶_id), + Some(1 + validation_upgrade_delay), // did not change since the same assertion from the last time. + ); assert_eq!(::FutureCodeHash::get(¶_id), Some(new_code.hash())); check_code_is_not_stored(&newer_code); }); @@ -1474,6 +1628,7 @@ mod tests { #[test] fn full_parachain_cleanup_storage() { let code_retention_period = 10; + let validation_upgrade_delay = 1 + 5; let original_code = ValidationCode(vec![1, 2, 3]); let paras = vec![ @@ -1489,6 +1644,7 @@ mod tests { configuration: crate::configuration::GenesisConfig { config: HostConfiguration { code_retention_period, + validation_upgrade_delay, ..Default::default() }, ..Default::default() @@ -1508,8 +1664,8 @@ mod tests { let expected_at = { // this parablock is in the context of block 1. - let expected_at = 1 + 5; - Paras::schedule_code_upgrade(para_id, new_code.clone(), expected_at); + let expected_at = 1 + validation_upgrade_delay; + Paras::schedule_code_upgrade(para_id, new_code.clone(), 1, &Configuration::config()); Paras::note_new_head(para_id, Default::default(), 1); assert!(Paras::past_code_meta(¶_id).most_recent_change().is_none()); @@ -1652,6 +1808,7 @@ mod tests { #[test] fn code_hash_at_with_intermediate() { let code_retention_period = 10; + let validation_upgrade_delay = 10; let paras = vec![ (0u32.into(), ParaGenesisArgs { @@ -1666,6 +1823,7 @@ mod tests { configuration: crate::configuration::GenesisConfig { config: HostConfiguration { code_retention_period, + validation_upgrade_delay, ..Default::default() }, ..Default::default() @@ -1677,7 +1835,11 @@ mod tests { let para_id = ParaId::from(0); let old_code: ValidationCode = vec![1, 2, 3].into(); let new_code: ValidationCode = vec![4, 5, 6].into(); - Paras::schedule_code_upgrade(para_id, new_code.clone(), 10); + + + // expected_at = 10 = 0 + validation_upgrade_delay = 0 + 10 + Paras::schedule_code_upgrade(para_id, new_code.clone(), 0, &Configuration::config()); + assert_eq!(::FutureCodeUpgrades::get(¶_id), Some(10)); // no intermediate, falls back on current/past. assert_eq!(fetch_validation_code_at(para_id, 1, None), Some(old_code.clone())); @@ -1704,6 +1866,7 @@ mod tests { #[test] fn code_hash_at_returns_up_to_end_of_code_retention_period() { let code_retention_period = 10; + let validation_upgrade_delay = 2; let paras = vec![ (0u32.into(), ParaGenesisArgs { @@ -1718,6 +1881,7 @@ mod tests { configuration: crate::configuration::GenesisConfig { config: HostConfiguration { code_retention_period, + validation_upgrade_delay, ..Default::default() }, ..Default::default() @@ -1729,7 +1893,7 @@ mod tests { let para_id = ParaId::from(0); let old_code: ValidationCode = vec![1, 2, 3].into(); let new_code: ValidationCode = vec![4, 5, 6].into(); - Paras::schedule_code_upgrade(para_id, new_code.clone(), 2); + Paras::schedule_code_upgrade(para_id, new_code.clone(), 0, &Configuration::config()); run_to_block(10, None); Paras::note_new_head(para_id, Default::default(), 7); @@ -1791,4 +1955,40 @@ mod tests { assert!(!CodeByHashRefs::contains_key(code.hash())); }); } + + #[test] + fn verify_upgrade_go_ahead_signal_is_externally_accessible() { + use primitives::v1::well_known_keys; + + let a = ParaId::from(2020); + + new_test_ext(Default::default()).execute_with(|| { + assert!( + sp_io::storage::get(&well_known_keys::upgrade_go_ahead_signal(a)).is_none() + ); + ::UpgradeGoAheadSignal::insert(&a, UpgradeGoAhead::GoAhead); + assert_eq!( + sp_io::storage::get(&well_known_keys::upgrade_go_ahead_signal(a)).unwrap(), + vec![1u8], + ); + }); + } + + #[test] + fn verify_upgrade_restriction_signal_is_externally_accessible() { + use primitives::v1::well_known_keys; + + let a = ParaId::from(2020); + + new_test_ext(Default::default()).execute_with(|| { + assert!( + sp_io::storage::get(&well_known_keys::upgrade_restriction_signal(a)).is_none() + ); + ::UpgradeRestrictionSignal::insert(&a, UpgradeRestriction::Present); + assert_eq!( + sp_io::storage::get(&well_known_keys::upgrade_restriction_signal(a)).unwrap(), + vec![0], + ); + }); + } } From b058f8e6ff600d0a113bf21256f40c7b53b84267 Mon Sep 17 00:00:00 2001 From: Sergei Shulepov Date: Fri, 9 Jul 2021 08:07:53 +0000 Subject: [PATCH 2/5] Explicit encoding indicies for exposed enums --- primitives/src/v1/mod.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/primitives/src/v1/mod.rs b/primitives/src/v1/mod.rs index 1fca4c28204f..3a2cc22db035 100644 --- a/primitives/src/v1/mod.rs +++ b/primitives/src/v1/mod.rs @@ -1050,6 +1050,7 @@ pub struct AbridgedHrmpChannel { pub enum UpgradeRestriction { /// There is an upgrade restriction and there are no details about its specifics nor how long /// it could last. + #[codec(index = 0)] Present, } @@ -1067,10 +1068,12 @@ pub enum UpgradeGoAhead { /// The expected reaction on this variant is that the parachain will admit this message and /// remove all the data about the pending upgrade. Depending on the nature of the problem (to /// be examined offchain for now), it can try to send another validation code or just retry later. + #[codec(index = 0)] Abort, /// Apply the pending code change. The parablock that is built on a relay-parent that is descendant /// of the relay-parent where the parachain observed this signal must use the upgraded validation /// code. + #[codec(index = 1)] GoAhead, } From d77aec15e9f35454970d595aa2ce34059c4bf4a7 Mon Sep 17 00:00:00 2001 From: Sergei Shulepov Date: Fri, 9 Jul 2021 08:09:16 +0000 Subject: [PATCH 3/5] typo: abscent -> absent --- roadmap/implementers-guide/src/runtime/paras.md | 2 +- runtime/parachains/src/paras.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/roadmap/implementers-guide/src/runtime/paras.md b/roadmap/implementers-guide/src/runtime/paras.md index 349e47521652..471a4effb4c3 100644 --- a/roadmap/implementers-guide/src/runtime/paras.md +++ b/roadmap/implementers-guide/src/runtime/paras.md @@ -139,7 +139,7 @@ FutureCodeUpgrades: map ParaId => Option; FutureCodeHash: map ParaId => Option; /// This is used by the relay-chain to communicate to a parachain a go-ahead with in the upgrade procedure. /// -/// This value is abscent when there are no upgrades scheduled or during the time the relay chain +/// This value is absent when there are no upgrades scheduled or during the time the relay chain /// performs the checks. It is set at the first relay-chain block when the corresponding parachain /// can switch its upgrade function. As soon as the parachain's block is included, the value /// gets reset to `None`. diff --git a/runtime/parachains/src/paras.rs b/runtime/parachains/src/paras.rs index 68764df8d215..86cefacfbfd9 100644 --- a/runtime/parachains/src/paras.rs +++ b/runtime/parachains/src/paras.rs @@ -383,7 +383,7 @@ pub mod pallet { /// This is used by the relay-chain to communicate to a parachain a go-ahead with in the upgrade procedure. /// - /// This value is abscent when there are no upgrades scheduled or during the time the relay chain + /// This value is absent when there are no upgrades scheduled or during the time the relay chain /// performs the checks. It is set at the first relay-chain block when the corresponding parachain /// can switch its upgrade function. As soon as the parachain's block is included, the value /// gets reset to `None`. From d0c134e794682405a6374e0e293d66671e38803a Mon Sep 17 00:00:00 2001 From: Sergei Shulepov Date: Fri, 9 Jul 2021 08:21:38 +0000 Subject: [PATCH 4/5] Prune cooldowns as well --- runtime/parachains/src/paras.rs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/runtime/parachains/src/paras.rs b/runtime/parachains/src/paras.rs index 86cefacfbfd9..5cfbe893c083 100644 --- a/runtime/parachains/src/paras.rs +++ b/runtime/parachains/src/paras.rs @@ -664,15 +664,24 @@ impl Pallet { } if !outgoing.is_empty() { - // Filter offboarded parachains from the upcoming upgrades list. + // Filter offboarded parachains from the upcoming upgrades and upgrade cooldowns list. // - // We do it after the offboarding to get away with only a single read/write. + // We do it after the offboarding to get away with only a single read/write per list. + // + // NOTE both of those iterates over the list and the outgoing. We do not expect either + // of these to be large. Thus should be fine. ::UpcomingUpgrades::mutate(|upcoming_upgrades| { *upcoming_upgrades = sp_std::mem::take(upcoming_upgrades) .into_iter() .filter(|&(ref para, _)| { - // NOTE this iterates over upcoming upgrades and outgoing. We do not expect - // either of these to be large. Thus should be fine. + !outgoing.contains(para) + }) + .collect(); + }); + ::UpgradeCooldowns::mutate(|upgrade_cooldowns| { + *upgrade_cooldowns = sp_std::mem::take(upgrade_cooldowns) + .into_iter() + .filter(|&(ref para, _)| { !outgoing.contains(para) }) .collect(); From 6c4af331c1f4ddfe89ce2a40a05e73987c9a3d32 Mon Sep 17 00:00:00 2001 From: Sergei Shulepov Date: Sat, 31 Jul 2021 15:00:36 +0000 Subject: [PATCH 5/5] Please hunspell --- roadmap/implementers-guide/src/runtime/inclusion.md | 2 +- roadmap/implementers-guide/src/runtime/paras.md | 2 +- runtime/parachains/src/paras.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/roadmap/implementers-guide/src/runtime/inclusion.md b/roadmap/implementers-guide/src/runtime/inclusion.md index 44b13dd18945..c800abd8f7a7 100644 --- a/roadmap/implementers-guide/src/runtime/inclusion.md +++ b/roadmap/implementers-guide/src/runtime/inclusion.md @@ -45,7 +45,7 @@ PendingAvailabilityCommitments: map ParaId => CandidateCommitments; All failed checks should lead to an unrecoverable error making the block invalid. * `process_bitfields(expected_bits, Bitfields, core_lookup: Fn(CoreIndex) -> Option)`: - 1. check that there is at most 1 bitfield per validator and that the number of bits in each bitfield is equal to expected_bits. + 1. check that there is at most 1 bitfield per validator and that the number of bits in each bitfield is equal to `expected_bits`. 1. check that there are no duplicates 1. check all validator signatures. 1. apply each bit of bitfield to the corresponding pending candidate. looking up parathread cores using the `core_lookup`. Disregard bitfields that have a `1` bit for any free cores. diff --git a/roadmap/implementers-guide/src/runtime/paras.md b/roadmap/implementers-guide/src/runtime/paras.md index 471a4effb4c3..1bd2cc8b3469 100644 --- a/roadmap/implementers-guide/src/runtime/paras.md +++ b/roadmap/implementers-guide/src/runtime/paras.md @@ -202,7 +202,7 @@ CodeByHash: map ValidationCodeHash => Option corresponding `PastCodeMeta` and `PastCode` accordingly. 1. Toggle the upgrade related signals 1. Collect all `(para_id, expected_at)` from `UpcomingUpgrades` where `expected_at <= now` and prune them. For each para pruned set `UpgradeGoAheadSignal` to `GoAhead`. - 1. Collect all `(para_id, next_possible_upgrade_at)` from `UpgradeCooldowns` where `next_possible_upgrade_at <= now` and prune them. For each para prunned set `UpgradeRestrictionSignal` to `Present`. + 1. Collect all `(para_id, next_possible_upgrade_at)` from `UpgradeCooldowns` where `next_possible_upgrade_at <= now` and prune them. For each para pruned set `UpgradeRestrictionSignal` to `Present`. ## Routines diff --git a/runtime/parachains/src/paras.rs b/runtime/parachains/src/paras.rs index 6e5e51d9db5e..c737d13a5ef0 100644 --- a/runtime/parachains/src/paras.rs +++ b/runtime/parachains/src/paras.rs @@ -397,7 +397,7 @@ pub mod pallet { /// an upgrade for this parachain. /// /// This may be a because the parachain waits for the upgrade cooldown to expire. Another - /// potential use case is when we want to perform some maintanance (such as storage migration) + /// potential use case is when we want to perform some maintenance (such as storage migration) /// we could restrict upgrades to make the process simpler. /// /// NOTE that this field is used by parachains via merkle storage proofs, therefore changing