From 8937a5875b5d9816cd773f4154d667c483c344fc Mon Sep 17 00:00:00 2001 From: Sergey Shulepov Date: Fri, 17 Dec 2021 14:12:18 +0000 Subject: [PATCH] configuration: refactor configuration initialization Refactor the configuration module's initializer_on_new_session in such a way that it returns the configuration. This would make it inline with other special initialization routines like `shared`'s or `paras`. This will be useful in a following PR that will check consistency of the configuration before setting it. --- runtime/parachains/src/configuration.rs | 61 ++++++++++++++++++++++--- runtime/parachains/src/initializer.rs | 10 ++-- 2 files changed, 57 insertions(+), 14 deletions(-) diff --git a/runtime/parachains/src/configuration.rs b/runtime/parachains/src/configuration.rs index 8b18c7d59559..8489c283e885 100644 --- a/runtime/parachains/src/configuration.rs +++ b/runtime/parachains/src/configuration.rs @@ -1054,6 +1054,15 @@ pub mod pallet { } } +/// A struct that holds the configuration that was active before the session change and optionally +/// a configuration that became active after the session change. +pub struct SessionChangeOutcome { + /// Previously active configuration. + pub prev_config: HostConfiguration, + /// If new configuration was applied during the session change, this is the new configuration. + pub new_config: Option>, +} + impl Pallet { /// Called by the initializer to initialize the configuration module. pub(crate) fn initializer_initialize(_now: T::BlockNumber) -> Weight { @@ -1064,13 +1073,22 @@ impl Pallet { pub(crate) fn initializer_finalize() {} /// Called by the initializer to note that a new session has started. - pub(crate) fn initializer_on_new_session(session_index: &SessionIndex) { + /// + /// Returns the configuration that was actual before the session change and the configuration + /// that became active after the session change. If there were no scheduled changes, both will + /// be the same. + pub(crate) fn initializer_on_new_session( + session_index: &SessionIndex, + ) -> SessionChangeOutcome { let pending_configs = >::get(); + let prev_config = ::ActiveConfig::get(); + + // No pending configuration changes, so we're done. if pending_configs.is_empty() { - return + return SessionChangeOutcome { prev_config, new_config: None } } - let (past_and_present, future) = pending_configs + let (mut past_and_present, future) = pending_configs .into_iter() .partition::, _>(|&(apply_at_session, _)| apply_at_session <= *session_index); @@ -1082,11 +1100,16 @@ impl Pallet { "Skipping applying configuration changes scheduled sessions in the past", ); } - if let Some((_, pending)) = past_and_present.last() { - ::ActiveConfig::put(pending); + + let new_config = past_and_present.pop().map(|(_, config)| config); + if let Some(ref new_config) = new_config { + // Apply the new configuration. + ::ActiveConfig::put(new_config); } >::put(future); + + SessionChangeOutcome { prev_config, new_config } } /// Return the session index that should be used for any future scheduled changes. @@ -1167,9 +1190,14 @@ mod tests { use frame_support::assert_ok; - fn on_new_session(session_index: SessionIndex) { + fn on_new_session( + session_index: SessionIndex, + ) -> (HostConfiguration, HostConfiguration) { ParasShared::set_session_index(session_index); - Configuration::initializer_on_new_session(&session_index); + let SessionChangeOutcome { prev_config, new_config } = + Configuration::initializer_on_new_session(&session_index); + let new_config = new_config.unwrap_or_else(|| prev_config.clone()); + (prev_config, new_config) } #[test] @@ -1182,6 +1210,25 @@ mod tests { }); } + #[test] + fn initializer_on_new_session() { + new_test_ext(Default::default()).execute_with(|| { + let (prev_config, new_config) = on_new_session(1); + assert_eq!(prev_config, new_config); + assert_ok!(Configuration::set_validation_upgrade_delay(Origin::root(), 100)); + + let (prev_config, new_config) = on_new_session(2); + assert_eq!(prev_config, new_config); + + let (prev_config, new_config) = on_new_session(3); + assert_eq!(prev_config, HostConfiguration::default()); + assert_eq!( + new_config, + HostConfiguration { validation_upgrade_delay: 100, ..prev_config } + ); + }); + } + #[test] fn config_changes_after_2_session_boundary() { new_test_ext(Default::default()).execute_with(|| { diff --git a/runtime/parachains/src/initializer.rs b/runtime/parachains/src/initializer.rs index a7cb23799e08..ebb2e4b9185f 100644 --- a/runtime/parachains/src/initializer.rs +++ b/runtime/parachains/src/initializer.rs @@ -221,8 +221,6 @@ impl Pallet { all_validators: Vec, queued: Vec, ) { - let prev_config = >::config(); - let random_seed = { let mut buf = [0u8; 32]; // TODO: audit usage of randomness API @@ -233,11 +231,9 @@ impl Pallet { buf }; - // We can't pass the new config into the thing that determines the new config, - // so we don't pass the `SessionChangeNotification` into this module. - configuration::Pallet::::initializer_on_new_session(&session_index); - - let new_config = >::config(); + let configuration::SessionChangeOutcome { prev_config, new_config } = + configuration::Pallet::::initializer_on_new_session(&session_index); + let new_config = new_config.unwrap_or_else(|| prev_config.clone()); let validators = shared::Pallet::::initializer_on_new_session( session_index,