From 813666ae0876556a3d69a5a918fa7b8d486cdb44 Mon Sep 17 00:00:00 2001 From: Daan van der Plas Date: Mon, 11 Sep 2023 17:01:22 +0200 Subject: [PATCH] reapply changes --- .../runtime/common/src/paras_registrar/mod.rs | 4 +- .../runtime/parachains/src/inclusion/mod.rs | 13 +- .../runtime/parachains/src/inclusion/tests.rs | 14 + polkadot/runtime/parachains/src/lib.rs | 5 +- polkadot/runtime/parachains/src/paras/mod.rs | 47 ++++ .../runtime/parachains/src/paras/tests.rs | 263 ++++++++++++++++++ 6 files changed, 341 insertions(+), 5 deletions(-) diff --git a/polkadot/runtime/common/src/paras_registrar/mod.rs b/polkadot/runtime/common/src/paras_registrar/mod.rs index f2751803a4133..8b8c6d89d0185 100644 --- a/polkadot/runtime/common/src/paras_registrar/mod.rs +++ b/polkadot/runtime/common/src/paras_registrar/mod.rs @@ -29,7 +29,7 @@ use frame_system::{self, ensure_root, ensure_signed}; use primitives::{HeadData, Id as ParaId, ValidationCode, LOWEST_PUBLIC_ID}; use runtime_parachains::{ configuration, ensure_parachain, - paras::{self, ParaGenesisArgs}, + paras::{self, ParaGenesisArgs, SetGoAhead}, Origin, ParaLifecycle, }; use sp_std::{prelude::*, result}; @@ -412,7 +412,7 @@ pub mod pallet { new_code: ValidationCode, ) -> DispatchResult { Self::ensure_root_para_or_owner(origin, para)?; - runtime_parachains::schedule_code_upgrade::(para, new_code)?; + runtime_parachains::schedule_code_upgrade::(para, new_code, SetGoAhead::No)?; Ok(()) } diff --git a/polkadot/runtime/parachains/src/inclusion/mod.rs b/polkadot/runtime/parachains/src/inclusion/mod.rs index bb16c804150dc..25ce516b8ecc9 100644 --- a/polkadot/runtime/parachains/src/inclusion/mod.rs +++ b/polkadot/runtime/parachains/src/inclusion/mod.rs @@ -21,7 +21,8 @@ use crate::{ configuration::{self, HostConfiguration}, - disputes, dmp, hrmp, paras, + disputes, dmp, hrmp, + paras::{self, SetGoAhead}, scheduler::{self, AvailabilityTimeoutStatus}, shared::{self, AllowedRelayParentsTracker}, }; @@ -448,8 +449,14 @@ impl fmt::Debug for UmpAcceptanceCheckErr { "the ump queue would have grown past the max size permitted by config ({} > {})", total_size, limit, ), +<<<<<<< Updated upstream UmpAcceptanceCheckErr::IsOffboarding => write!(fmt, "upward message rejected because the para is off-boarding",), +======= + UmpAcceptanceCheckErr::IsOffboarding => { + write!(fmt, "upward message rejected because the para is off-boarding") + }, +>>>>>>> Stashed changes } } } @@ -885,6 +892,10 @@ impl Pallet { new_code, now, &config, +<<<<<<< Updated upstream +======= + SetGoAhead::Yes, +>>>>>>> Stashed changes )); } diff --git a/polkadot/runtime/parachains/src/inclusion/tests.rs b/polkadot/runtime/parachains/src/inclusion/tests.rs index 7677108d73dec..28dd110bdf643 100644 --- a/polkadot/runtime/parachains/src/inclusion/tests.rs +++ b/polkadot/runtime/parachains/src/inclusion/tests.rs @@ -1255,7 +1255,17 @@ fn candidate_checks() { let cfg = Configuration::config(); let expected_at = 10 + cfg.validation_upgrade_delay; assert_eq!(expected_at, 12); +<<<<<<< Updated upstream Paras::schedule_code_upgrade(chain_a, vec![1, 2, 3, 4].into(), expected_at, &cfg); +======= + Paras::schedule_code_upgrade( + chain_a, + vec![1, 2, 3, 4].into(), + expected_at, + &cfg, + SetGoAhead::Yes, + ); +>>>>>>> Stashed changes } assert_noop!( @@ -2235,7 +2245,11 @@ fn para_upgrade_delay_scheduled_from_inclusion() { let cause = &active_vote_state.causes()[0]; // Upgrade block is the block of inclusion, not candidate's parent. assert_matches!(cause, +<<<<<<< Updated upstream paras::PvfCheckCause::Upgrade { id, included_at } +======= + paras::PvfCheckCause::Upgrade { id, included_at, set_go_ahead: SetGoAhead::Yes } +>>>>>>> Stashed changes if id == &chain_a && included_at == &7 ); }); diff --git a/polkadot/runtime/parachains/src/lib.rs b/polkadot/runtime/parachains/src/lib.rs index 64365f17b7e8b..e0ace86d3794a 100644 --- a/polkadot/runtime/parachains/src/lib.rs +++ b/polkadot/runtime/parachains/src/lib.rs @@ -53,7 +53,7 @@ mod mock; mod ump_tests; pub use origin::{ensure_parachain, Origin}; -pub use paras::ParaLifecycle; +pub use paras::{ParaLifecycle, SetGoAhead}; use primitives::{HeadData, Id as ParaId, ValidationCode}; use sp_runtime::{DispatchResult, FixedU128}; @@ -89,8 +89,9 @@ pub fn schedule_parachain_downgrade(id: ParaId) -> Result<(), pub fn schedule_code_upgrade( id: ParaId, new_code: ValidationCode, + set_go_ahead: SetGoAhead, ) -> DispatchResult { - paras::Pallet::::schedule_code_upgrade_external(id, new_code) + paras::Pallet::::schedule_code_upgrade_external(id, new_code, set_go_ahead) } /// Sets the current parachain head with the given id. diff --git a/polkadot/runtime/parachains/src/paras/mod.rs b/polkadot/runtime/parachains/src/paras/mod.rs index 2f370b5bfe472..2e247cd1cf313 100644 --- a/polkadot/runtime/parachains/src/paras/mod.rs +++ b/polkadot/runtime/parachains/src/paras/mod.rs @@ -386,9 +386,21 @@ pub(crate) enum PvfCheckCause { /// /// See https://github.com/paritytech/polkadot/issues/4601 for detailed explanation. included_at: BlockNumber, +<<<<<<< Updated upstream +======= + /// Whether or not the given para should be sent the `GoAhead` signal. + set_go_ahead: SetGoAhead, +>>>>>>> Stashed changes }, } +/// Should the `GoAhead` signal be set after a successful check of the new wasm binary? +#[derive(Debug, Copy, Clone, PartialEq, TypeInfo, Decode, Encode)] +pub enum SetGoAhead { + Yes, + No, +} + impl PvfCheckCause { /// Returns the ID of the para that initiated or subscribed to the pre-checking vote. fn para_id(&self) -> ParaId { @@ -888,7 +900,17 @@ pub mod pallet { ) -> DispatchResult { ensure_root(origin)?; let config = configuration::Pallet::::config(); +<<<<<<< Updated upstream Self::schedule_code_upgrade(para, new_code, relay_parent_number, &config); +======= + Self::schedule_code_upgrade( + para, + new_code, + relay_parent_number, + &config, + SetGoAhead::No, + ); +>>>>>>> Stashed changes Self::deposit_event(Event::CodeUpgradeScheduled(para)); Ok(()) } @@ -1186,6 +1208,7 @@ impl Pallet { pub(crate) fn schedule_code_upgrade_external( id: ParaId, new_code: ValidationCode, + set_go_ahead: SetGoAhead, ) -> DispatchResult { // Check that we can schedule an upgrade at all. ensure!(Self::can_upgrade_validation_code(id), Error::::CannotUpgradeCode); @@ -1193,7 +1216,11 @@ impl Pallet { let current_block = frame_system::Pallet::::block_number(); // Schedule the upgrade with a delay just like if a parachain triggered the upgrade. let upgrade_block = current_block.saturating_add(config.validation_upgrade_delay); +<<<<<<< Updated upstream Self::schedule_code_upgrade(id, new_code, upgrade_block, &config); +======= + Self::schedule_code_upgrade(id, new_code, upgrade_block, &config, set_go_ahead); +>>>>>>> Stashed changes Self::deposit_event(Event::CodeUpgradeScheduled(id)); Ok(()) } @@ -1568,6 +1595,10 @@ impl Pallet { now: BlockNumberFor, relay_parent_number: BlockNumberFor, cfg: &configuration::HostConfiguration>, +<<<<<<< Updated upstream +======= + set_go_ahead: SetGoAhead, +>>>>>>> Stashed changes ) -> Weight { let mut weight = Weight::zero(); @@ -1591,12 +1622,24 @@ impl Pallet { weight += T::DbWeight::get().reads_writes(1, 4); FutureCodeUpgrades::::insert(&id, expected_at); +<<<<<<< Updated upstream 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)); }); +======= + // Only set an upcoming upgrade if `GoAhead` signal should be set for the respective para. + if set_go_ahead == SetGoAhead::Yes { + 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)); + }); + } +>>>>>>> Stashed changes let expected_at = expected_at.saturated_into(); let log = ConsensusLog::ParaScheduleUpgradeCode(id, *code_hash, expected_at); @@ -1835,6 +1878,10 @@ impl Pallet { new_code: ValidationCode, inclusion_block_number: BlockNumberFor, cfg: &configuration::HostConfiguration>, +<<<<<<< Updated upstream +======= + set_go_ahead: SetGoAhead, +>>>>>>> Stashed changes ) -> Weight { let mut weight = T::DbWeight::get().reads(1); diff --git a/polkadot/runtime/parachains/src/paras/tests.rs b/polkadot/runtime/parachains/src/paras/tests.rs index a024525c817a8..4b88b88758d9c 100644 --- a/polkadot/runtime/parachains/src/paras/tests.rs +++ b/polkadot/runtime/parachains/src/paras/tests.rs @@ -440,7 +440,17 @@ fn code_upgrade_applied_after_delay() { // this parablock is in the context of block 1. let expected_at = 1 + validation_upgrade_delay; let next_possible_upgrade_at = 1 + validation_upgrade_cooldown; +<<<<<<< Updated upstream Paras::schedule_code_upgrade(para_id, new_code.clone(), 1, &Configuration::config()); +======= + Paras::schedule_code_upgrade( + para_id, + new_code.clone(), + 1, + &Configuration::config(), + SetGoAhead::Yes, + ); +>>>>>>> Stashed changes // Include votes for super-majority. submit_super_majority_pvf_votes(&new_code, EXPECTED_SESSION, true); @@ -509,6 +519,131 @@ fn code_upgrade_applied_after_delay() { } #[test] +<<<<<<< Updated upstream +======= +fn code_upgrade_applied_without_setting_go_ahead_signal() { + let code_retention_period = 10; + let validation_upgrade_delay = 5; + let validation_upgrade_cooldown = 10; + + let original_code = ValidationCode(vec![1, 2, 3]); + let paras = vec![( + 0u32.into(), + ParaGenesisArgs { + para_kind: ParaKind::Parachain, + genesis_head: dummy_head_data(), + validation_code: original_code.clone(), + }, + )]; + + let genesis_config = MockGenesisConfig { + paras: GenesisConfig { paras, ..Default::default() }, + configuration: crate::configuration::GenesisConfig { + config: HostConfiguration { + code_retention_period, + validation_upgrade_delay, + validation_upgrade_cooldown, + ..Default::default() + }, + ..Default::default() + }, + ..Default::default() + }; + + new_test_ext(genesis_config).execute_with(|| { + check_code_is_stored(&original_code); + + let para_id = ParaId::from(0); + let new_code = ValidationCode(vec![4, 5, 6]); + + // Wait for at least one session change to set active validators. + const EXPECTED_SESSION: SessionIndex = 1; + run_to_block(2, Some(vec![1])); + assert_eq!(Paras::current_code(¶_id), Some(original_code.clone())); + + let (expected_at, next_possible_upgrade_at) = { + // this parablock is in the context of block 1. + let expected_at = 1 + validation_upgrade_delay; + let next_possible_upgrade_at = 1 + validation_upgrade_cooldown; + // `set_go_ahead` parameter set to `false` which prevents signaling the parachain + // with the `GoAhead` signal. + Paras::schedule_code_upgrade( + para_id, + new_code.clone(), + 1, + &Configuration::config(), + SetGoAhead::No, + ); + // Include votes for super-majority. + submit_super_majority_pvf_votes(&new_code, EXPECTED_SESSION, true); + + 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![]); + 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); + + (expected_at, next_possible_upgrade_at) + }; + + run_to_block(expected_at, None); + + // the candidate is in the context of the parent of `expected_at`, + // thus does not trigger the code upgrade. However, now the `UpgradeGoAheadSignal` + // should not be set. + { + Paras::note_new_head(para_id, Default::default(), expected_at - 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!(UpgradeGoAheadSignal::::get(¶_id).is_none()); + assert_eq!(Paras::current_code(¶_id), Some(original_code.clone())); + check_code_is_stored(&original_code); + check_code_is_stored(&new_code); + } + + run_to_block(expected_at + 1, None); + + // the candidate is in the context of `expected_at`, and triggers + // the upgrade. + { + Paras::note_new_head(para_id, Default::default(), expected_at); + + assert_eq!(Paras::past_code_meta(¶_id).most_recent_change(), Some(expected_at)); + assert_eq!( + PastCodeHash::::get(&(para_id, expected_at)), + Some(original_code.hash()), + ); + 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())); + assert_eq!( + UpgradeRestrictionSignal::::get(¶_id), + Some(UpgradeRestriction::Present), + ); + assert_eq!(UpgradeCooldowns::::get(), vec![(para_id, next_possible_upgrade_at)]); + check_code_is_stored(&original_code); + check_code_is_stored(&new_code); + } + + run_to_block(next_possible_upgrade_at + 1, None); + + { + assert!(UpgradeRestrictionSignal::::get(¶_id).is_none()); + assert!(UpgradeCooldowns::::get().is_empty()); + } + }); +} + +#[test] +>>>>>>> Stashed changes fn code_upgrade_applied_after_delay_even_when_late() { let code_retention_period = 10; let validation_upgrade_delay = 5; @@ -550,7 +685,17 @@ fn code_upgrade_applied_after_delay_even_when_late() { // this parablock is in the context of block 1. let expected_at = 1 + validation_upgrade_delay; let next_possible_upgrade_at = 1 + validation_upgrade_cooldown; +<<<<<<< Updated upstream Paras::schedule_code_upgrade(para_id, new_code.clone(), 1, &Configuration::config()); +======= + Paras::schedule_code_upgrade( + para_id, + new_code.clone(), + 1, + &Configuration::config(), + SetGoAhead::Yes, + ); +>>>>>>> Stashed changes // Include votes for super-majority. submit_super_majority_pvf_votes(&new_code, EXPECTED_SESSION, true); @@ -628,7 +773,17 @@ fn submit_code_change_when_not_allowed_is_err() { const EXPECTED_SESSION: SessionIndex = 1; run_to_block(1, Some(vec![1])); +<<<<<<< Updated upstream Paras::schedule_code_upgrade(para_id, new_code.clone(), 1, &Configuration::config()); +======= + Paras::schedule_code_upgrade( + para_id, + new_code.clone(), + 1, + &Configuration::config(), + SetGoAhead::Yes, + ); +>>>>>>> Stashed changes // Include votes for super-majority. submit_super_majority_pvf_votes(&new_code, EXPECTED_SESSION, true); @@ -640,7 +795,17 @@ fn submit_code_change_when_not_allowed_is_err() { // ignore it. Note that this is only true from perspective of this module. run_to_block(2, None); assert!(!Paras::can_upgrade_validation_code(para_id)); +<<<<<<< Updated upstream Paras::schedule_code_upgrade(para_id, newer_code.clone(), 2, &Configuration::config()); +======= + Paras::schedule_code_upgrade( + para_id, + newer_code.clone(), + 2, + &Configuration::config(), + SetGoAhead::Yes, + ); +>>>>>>> Stashed changes assert_eq!( FutureCodeUpgrades::::get(¶_id), Some(1 + validation_upgrade_delay), /* did not change since the same assertion from @@ -698,7 +863,17 @@ fn upgrade_restriction_elapsed_doesnt_mean_can_upgrade() { const EXPECTED_SESSION: SessionIndex = 1; run_to_block(1, Some(vec![1])); +<<<<<<< Updated upstream Paras::schedule_code_upgrade(para_id, new_code.clone(), 0, &Configuration::config()); +======= + Paras::schedule_code_upgrade( + para_id, + new_code.clone(), + 0, + &Configuration::config(), + SetGoAhead::Yes, + ); +>>>>>>> Stashed changes // Include votes for super-majority. submit_super_majority_pvf_votes(&new_code, EXPECTED_SESSION, true); @@ -717,7 +892,17 @@ fn upgrade_restriction_elapsed_doesnt_mean_can_upgrade() { assert!(!Paras::can_upgrade_validation_code(para_id)); // And scheduling another upgrade does not do anything. `expected_at` is still the same. +<<<<<<< Updated upstream Paras::schedule_code_upgrade(para_id, newer_code.clone(), 30, &Configuration::config()); +======= + Paras::schedule_code_upgrade( + para_id, + newer_code.clone(), + 30, + &Configuration::config(), + SetGoAhead::Yes, + ); +>>>>>>> Stashed changes assert_eq!(FutureCodeUpgrades::::get(¶_id), Some(0 + validation_upgrade_delay)); }); } @@ -769,7 +954,17 @@ fn full_parachain_cleanup_storage() { let expected_at = { // this parablock is in the context of block 1. let expected_at = 1 + validation_upgrade_delay; +<<<<<<< Updated upstream Paras::schedule_code_upgrade(para_id, new_code.clone(), 1, &Configuration::config()); +======= + Paras::schedule_code_upgrade( + para_id, + new_code.clone(), + 1, + &Configuration::config(), + SetGoAhead::Yes, + ); +>>>>>>> Stashed changes // Include votes for super-majority. submit_super_majority_pvf_votes(&new_code, EXPECTED_SESSION, true); @@ -864,6 +1059,10 @@ fn cannot_offboard_ongoing_pvf_check() { new_code.clone(), RELAY_PARENT, &Configuration::config(), +<<<<<<< Updated upstream +======= + SetGoAhead::Yes, +>>>>>>> Stashed changes ); assert!(!Paras::pvfs_require_precheck().is_empty()); @@ -1016,7 +1215,17 @@ fn code_hash_at_returns_up_to_end_of_code_retention_period() { let para_id = ParaId::from(0); let old_code: ValidationCode = vec![1, 2, 3].into(); let new_code: ValidationCode = vec![4, 5, 6].into(); +<<<<<<< Updated upstream Paras::schedule_code_upgrade(para_id, new_code.clone(), 0, &Configuration::config()); +======= + Paras::schedule_code_upgrade( + para_id, + new_code.clone(), + 0, + &Configuration::config(), + SetGoAhead::Yes, + ); +>>>>>>> Stashed changes // Include votes for super-majority. submit_super_majority_pvf_votes(&new_code, EXPECTED_SESSION, true); @@ -1124,6 +1333,10 @@ fn pvf_check_coalescing_onboarding_and_upgrade() { validation_code.clone(), RELAY_PARENT, &Configuration::config(), +<<<<<<< Updated upstream +======= + SetGoAhead::Yes, +>>>>>>> Stashed changes ); assert!(!Paras::pvfs_require_precheck().is_empty()); @@ -1228,7 +1441,17 @@ fn pvf_check_upgrade_reject() { // Expected current session index. const EXPECTED_SESSION: SessionIndex = 1; +<<<<<<< Updated upstream Paras::schedule_code_upgrade(a, new_code.clone(), RELAY_PARENT, &Configuration::config()); +======= + Paras::schedule_code_upgrade( + a, + new_code.clone(), + RELAY_PARENT, + &Configuration::config(), + SetGoAhead::Yes, + ); +>>>>>>> Stashed changes check_code_is_stored(&new_code); // 1/3 of validators vote against `new_code`. PVF should not be rejected yet. @@ -1408,7 +1631,17 @@ fn include_pvf_check_statement_refunds_weight() { // Expected current session index. const EXPECTED_SESSION: SessionIndex = 1; +<<<<<<< Updated upstream Paras::schedule_code_upgrade(a, new_code.clone(), RELAY_PARENT, &Configuration::config()); +======= + Paras::schedule_code_upgrade( + a, + new_code.clone(), + RELAY_PARENT, + &Configuration::config(), + SetGoAhead::Yes, + ); +>>>>>>> Stashed changes let mut stmts = IntoIterator::into_iter([0, 1, 2, 3]) .map(|i| { @@ -1503,7 +1736,17 @@ fn poke_unused_validation_code_doesnt_remove_code_with_users() { // Then we add a user to the code, say by upgrading. run_to_block(2, None); +<<<<<<< Updated upstream Paras::schedule_code_upgrade(para_id, validation_code.clone(), 1, &Configuration::config()); +======= + Paras::schedule_code_upgrade( + para_id, + validation_code.clone(), + 1, + &Configuration::config(), + SetGoAhead::Yes, + ); +>>>>>>> Stashed changes Paras::note_new_head(para_id, HeadData::default(), 1); // Finally we poke the code, which should not remove it from the storage. @@ -1568,7 +1811,17 @@ fn add_trusted_validation_code_insta_approval() { // Then some parachain upgrades it's code with the relay-parent 1. run_to_block(2, None); +<<<<<<< Updated upstream Paras::schedule_code_upgrade(para_id, validation_code.clone(), 1, &Configuration::config()); +======= + Paras::schedule_code_upgrade( + para_id, + validation_code.clone(), + 1, + &Configuration::config(), + SetGoAhead::Yes, + ); +>>>>>>> Stashed changes Paras::note_new_head(para_id, HeadData::default(), 1); // Verify that the code upgrade has `expected_at` set to `26`. @@ -1604,7 +1857,17 @@ fn add_trusted_validation_code_enacts_existing_pvf_vote() { new_test_ext(genesis_config).execute_with(|| { // First, some parachain upgrades it's code with the relay-parent 1. run_to_block(2, None); +<<<<<<< Updated upstream Paras::schedule_code_upgrade(para_id, validation_code.clone(), 1, &Configuration::config()); +======= + Paras::schedule_code_upgrade( + para_id, + validation_code.clone(), + 1, + &Configuration::config(), + SetGoAhead::Yes, + ); +>>>>>>> Stashed changes Paras::note_new_head(para_id, HeadData::default(), 1); // No upgrade should be scheduled at this point. PVF pre-checking vote should run for