diff --git a/substrate/Cargo.lock b/substrate/Cargo.lock index 1cb703fd5f..1b73e54785 100644 --- a/substrate/Cargo.lock +++ b/substrate/Cargo.lock @@ -1500,6 +1500,7 @@ dependencies = [ "sp-io", "sp-runtime", "sp-std", + "sp-version", ] [[package]] diff --git a/substrate/client/rpc/src/state/tests.rs b/substrate/client/rpc/src/state/tests.rs index 6d7f7a4ef5..75ce4ed9c3 100644 --- a/substrate/client/rpc/src/state/tests.rs +++ b/substrate/client/rpc/src/state/tests.rs @@ -402,7 +402,7 @@ fn should_return_runtime_version() { let api = new_full(client.clone(), Subscriptions::new(Arc::new(core.executor()))); let result = "{\"specName\":\"test\",\"implName\":\"parity-test\",\"authoringVersion\":1,\ - \"specVersion\":1,\"implVersion\":2,\"apis\":[[\"0xdf6acb689907609b\",2],\ + \"specVersion\":2,\"implVersion\":2,\"apis\":[[\"0xdf6acb689907609b\",2],\ [\"0x37e397fc7c91f5e4\",1],[\"0xd2bc9897eed08f15\",1],[\"0x40fe3ad401f8959a\",4],\ [\"0xc6e9a76309f39b09\",1],[\"0xdd718d5cc53262d4\",1],[\"0xcbca25e39f142387\",1],\ [\"0xf78b278be53f454c\",2],[\"0xab3c0572291feb8b\",1],[\"0xbc9d89904f5b923f\",1]]}"; diff --git a/substrate/frame/executive/Cargo.toml b/substrate/frame/executive/Cargo.toml index deafb0cadd..44751b0f3f 100644 --- a/substrate/frame/executive/Cargo.toml +++ b/substrate/frame/executive/Cargo.toml @@ -13,16 +13,17 @@ codec = { package = "parity-scale-codec", version = "1.2.0", default-features = frame-support = { version = "2.0.0-alpha.2", default-features = false, path = "../support" } frame-system = { version = "2.0.0-alpha.2", default-features = false, path = "../system" } serde = { version = "1.0.101", optional = true } -sp-io ={ path = "../../primitives/io", default-features = false , version = "2.0.0-alpha.2"} sp-runtime = { version = "2.0.0-alpha.2", default-features = false, path = "../../primitives/runtime" } sp-std = { version = "2.0.0-alpha.2", default-features = false, path = "../../primitives/std" } [dev-dependencies] hex-literal = "0.2.1" sp-core = { version = "2.0.0-alpha.2", path = "../../primitives/core" } +sp-io ={ path = "../../primitives/io", version = "2.0.0-alpha.2"} pallet-indices = { version = "2.0.0-alpha.2", path = "../indices" } pallet-balances = { version = "2.0.0-alpha.2", path = "../balances" } pallet-transaction-payment = { version = "2.0.0-alpha.2", path = "../transaction-payment" } +sp-version = { version = "2.0.0-alpha.2", path = "../../primitives/version" } [features] default = ["std"] @@ -31,7 +32,6 @@ std = [ "frame-support/std", "frame-system/std", "serde", - "sp-io/std", "sp-runtime/std", "sp-std/std", ] diff --git a/substrate/frame/executive/src/lib.rs b/substrate/frame/executive/src/lib.rs index 1d1fb95ce4..e81b883f00 100644 --- a/substrate/frame/executive/src/lib.rs +++ b/substrate/frame/executive/src/lib.rs @@ -75,10 +75,7 @@ #![cfg_attr(not(feature = "std"), no_std)] use sp_std::{prelude::*, marker::PhantomData}; -use frame_support::{ - storage::StorageValue, - weights::{GetDispatchInfo, WeighBlock, DispatchInfo} -}; +use frame_support::{storage::StorageValue, weights::{GetDispatchInfo, WeighBlock, DispatchInfo}}; use sp_runtime::{ generic::Digest, ApplyExtrinsicResult, traits::{ @@ -179,7 +176,9 @@ where extrinsics_root: &System::Hash, digest: &Digest, ) { - if frame_system::RuntimeUpgraded::take() { + if Self::runtime_upgraded() { + // System is not part of `AllModules`, so we need to call this manually. + as OnRuntimeUpgrade>::on_runtime_upgrade(); ::on_runtime_upgrade(); >::register_extra_weight_unchecked( >::on_runtime_upgrade() @@ -201,6 +200,21 @@ where ); } + /// Returns if the runtime was upgraded since the last time this function was called. + fn runtime_upgraded() -> bool { + let last = frame_system::LastRuntimeUpgrade::get(); + let current = >::get(); + + if last.map(|v| v.was_upgraded(¤t)).unwrap_or(true) { + frame_system::LastRuntimeUpgrade::put( + frame_system::LastRuntimeUpgradeInfo::from(current), + ); + true + } else { + false + } + } + fn initial_checks(block: &Block) { let header = block.header(); @@ -376,7 +390,7 @@ mod tests { weights::Weight, traits::{Currency, LockIdentifier, LockableCurrency, WithdrawReasons, WithdrawReason}, }; - use frame_system::{self as system, Call as SystemCall, ChainContext}; + use frame_system::{self as system, Call as SystemCall, ChainContext, LastRuntimeUpgradeInfo}; use pallet_balances::Call as BalancesCall; use hex_literal::hex; @@ -461,7 +475,7 @@ mod tests { type MaximumBlockWeight = MaximumBlockWeight; type AvailableBlockRatio = AvailableBlockRatio; type MaximumBlockLength = MaximumBlockLength; - type Version = (); + type Version = RuntimeVersion; type ModuleToIndex = (); type AccountData = pallet_balances::AccountData; type OnNewAccount = (); @@ -507,6 +521,18 @@ mod tests { } } + pub struct RuntimeVersion; + impl frame_support::traits::Get for RuntimeVersion { + fn get() -> sp_version::RuntimeVersion { + RUNTIME_VERSION.with(|v| v.borrow().clone()) + } + } + + thread_local! { + pub static RUNTIME_VERSION: std::cell::RefCell = + Default::default(); + } + type SignedExtra = ( frame_system::CheckEra, frame_system::CheckNonce, @@ -569,7 +595,7 @@ mod tests { header: Header { parent_hash: [69u8; 32].into(), number: 1, - state_root: hex!("8a22606e925c39bb0c8e8f6f5871c0aceab88a2fcff6b2d92660af8f6daff0b1").into(), + state_root: hex!("e97d724f480f6e3215bd5c24b9ba51250e2514ac1c99e563fd77bfb9d6100b1c").into(), extrinsics_root: hex!("03170a2e7597b7b7e3d84c05391d139a62b157e78786d8c082f29dcf4c111314").into(), digest: Digest { logs: vec![], }, }, @@ -759,4 +785,76 @@ mod tests { assert_eq!(>::all_extrinsics_weight(), 150 + 25); }) } + + #[test] + fn runtime_upgraded_should_work() { + new_test_ext(1).execute_with(|| { + RUNTIME_VERSION.with(|v| *v.borrow_mut() = Default::default()); + // It should be added at genesis + assert!(frame_system::LastRuntimeUpgrade::exists()); + assert!(!Executive::runtime_upgraded()); + + RUNTIME_VERSION.with(|v| *v.borrow_mut() = sp_version::RuntimeVersion { + spec_version: 1, + ..Default::default() + }); + assert!(Executive::runtime_upgraded()); + assert_eq!( + Some(LastRuntimeUpgradeInfo { spec_version: 1.into(), spec_name: "".into() }), + frame_system::LastRuntimeUpgrade::get(), + ); + + RUNTIME_VERSION.with(|v| *v.borrow_mut() = sp_version::RuntimeVersion { + spec_version: 1, + spec_name: "test".into(), + ..Default::default() + }); + assert!(Executive::runtime_upgraded()); + assert_eq!( + Some(LastRuntimeUpgradeInfo { spec_version: 1.into(), spec_name: "test".into() }), + frame_system::LastRuntimeUpgrade::get(), + ); + + RUNTIME_VERSION.with(|v| *v.borrow_mut() = sp_version::RuntimeVersion { + spec_version: 1, + spec_name: "test".into(), + impl_version: 2, + ..Default::default() + }); + assert!(!Executive::runtime_upgraded()); + + frame_system::LastRuntimeUpgrade::take(); + assert!(Executive::runtime_upgraded()); + assert_eq!( + Some(LastRuntimeUpgradeInfo { spec_version: 1.into(), spec_name: "test".into() }), + frame_system::LastRuntimeUpgrade::get(), + ); + }) + } + + #[test] + fn last_runtime_upgrade_was_upgraded_works() { + let test_data = vec![ + (0, "", 1, "", true), + (1, "", 1, "", false), + (1, "", 1, "test", true), + (1, "", 0, "", false), + (1, "", 0, "test", true), + ]; + + for (spec_version, spec_name, c_spec_version, c_spec_name, result) in test_data { + let current = sp_version::RuntimeVersion { + spec_version: c_spec_version, + spec_name: c_spec_name.into(), + ..Default::default() + }; + + let last = LastRuntimeUpgradeInfo { + spec_version: spec_version.into(), + spec_name: spec_name.into(), + }; + + assert_eq!(result, last.was_upgraded(¤t)); + } + } } diff --git a/substrate/frame/system/src/lib.rs b/substrate/frame/system/src/lib.rs index 5f9928d78d..b8cd11c07f 100644 --- a/substrate/frame/system/src/lib.rs +++ b/substrate/frame/system/src/lib.rs @@ -307,6 +307,33 @@ pub struct AccountInfo { pub data: AccountData, } +/// Stores the `spec_version` and `spec_name` of when the last runtime upgrade +/// happened. +#[derive(sp_runtime::RuntimeDebug, Encode, Decode)] +#[cfg_attr(feature = "std", derive(PartialEq))] +pub struct LastRuntimeUpgradeInfo { + pub spec_version: codec::Compact, + pub spec_name: sp_runtime::RuntimeString, +} + +impl LastRuntimeUpgradeInfo { + /// Returns if the runtime was upgraded in comparison of `self` and `current`. + /// + /// Checks if either the `spec_version` increased or the `spec_name` changed. + pub fn was_upgraded(&self, current: &sp_version::RuntimeVersion) -> bool { + current.spec_version > self.spec_version.0 || current.spec_name != self.spec_name + } +} + +impl From for LastRuntimeUpgradeInfo { + fn from(version: sp_version::RuntimeVersion) -> Self { + Self { + spec_version: version.spec_version.into(), + spec_name: version.spec_name, + } + } +} + decl_storage! { trait Store for Module as System { /// The full account information for a particular account ID. @@ -368,8 +395,8 @@ decl_storage! { /// no notification will be triggered thus the event might be lost. EventTopics get(fn event_topics): map hasher(blake2_256) T::Hash => Vec<(T::BlockNumber, EventIndex)>; - /// A bool to track if the runtime was upgraded last block. - pub RuntimeUpgraded: bool; + /// Stores the `spec_version` and `spec_name` of when the last runtime upgrade happened. + pub LastRuntimeUpgrade build(|_| Some(LastRuntimeUpgradeInfo::from(T::Version::get()))): Option; } add_extra_genesis { config(changes_trie_config): Option; @@ -416,13 +443,7 @@ decl_error! { InvalidSpecName, /// The specification version is not allowed to decrease between the current runtime /// and the new runtime. - SpecVersionNotAllowedToDecrease, - /// The implementation version is not allowed to decrease between the current runtime - /// and the new runtime. - ImplVersionNotAllowedToDecrease, - /// The specification or the implementation version need to increase between the - /// current runtime and the new runtime. - SpecOrImplVersionNeedToIncrease, + SpecVersionNeedsToIncrease, /// Failed to extract the runtime version from the new runtime. /// /// Either calling `Core_version` or decoding `RuntimeVersion` failed. @@ -478,18 +499,11 @@ decl_module! { Err(Error::::InvalidSpecName)? } - if new_version.spec_version < current_version.spec_version { - Err(Error::::SpecVersionNotAllowedToDecrease)? - } else if new_version.spec_version == current_version.spec_version { - if new_version.impl_version < current_version.impl_version { - Err(Error::::ImplVersionNotAllowedToDecrease)? - } else if new_version.impl_version == current_version.impl_version { - Err(Error::::SpecOrImplVersionNeedToIncrease)? - } + if new_version.spec_version <= current_version.spec_version { + Err(Error::::SpecVersionNeedsToIncrease)? } storage::unhashed::put_raw(well_known_keys::CODE, &code); - RuntimeUpgraded::put(true); Self::deposit_event(RawEvent::CodeUpdated); } @@ -498,7 +512,6 @@ decl_module! { pub fn set_code_without_checks(origin, code: Vec) { ensure_root(origin)?; storage::unhashed::put_raw(well_known_keys::CODE, &code); - RuntimeUpgraded::put(true); Self::deposit_event(RawEvent::CodeUpdated); } @@ -526,9 +539,6 @@ decl_module! { ensure_root(origin)?; for i in &items { storage::unhashed::put_raw(&i.0, &i.1); - if i.0 == well_known_keys::CODE { - RuntimeUpgraded::put(true); - } } } @@ -558,6 +568,13 @@ decl_module! { ensure!(account.data == T::AccountData::default(), Error::::NonDefaultComposite); Account::::remove(who); } + + fn on_runtime_upgrade() { + // Remove the old `RuntimeUpgraded` storage entry. + let mut runtime_upgraded_key = sp_io::hashing::twox_128(b"System").to_vec(); + runtime_upgraded_key.extend(&sp_io::hashing::twox_128(b"RuntimeUpgraded")); + sp_io::storage::clear(&runtime_upgraded_key); + } } } @@ -1935,12 +1952,12 @@ mod tests { } let test_data = vec![ - ("test", 1, 2, Ok(())), - ("test", 1, 1, Err(Error::::SpecOrImplVersionNeedToIncrease)), + ("test", 1, 2, Err(Error::::SpecVersionNeedsToIncrease)), + ("test", 1, 1, Err(Error::::SpecVersionNeedsToIncrease)), ("test2", 1, 1, Err(Error::::InvalidSpecName)), ("test", 2, 1, Ok(())), - ("test", 0, 1, Err(Error::::SpecVersionNotAllowedToDecrease)), - ("test", 1, 0, Err(Error::::ImplVersionNotAllowedToDecrease)), + ("test", 0, 1, Err(Error::::SpecVersionNeedsToIncrease)), + ("test", 1, 0, Err(Error::::SpecVersionNeedsToIncrease)), ]; for (spec_name, spec_version, impl_version, expected) in test_data.into_iter() { @@ -1980,8 +1997,6 @@ mod tests { System::events(), vec![EventRecord { phase: Phase::ApplyExtrinsic(0), event: 102u16, topics: vec![] }], ); - - assert_eq!(RuntimeUpgraded::get(), true); }); } @@ -1998,8 +2013,6 @@ mod tests { substrate_test_runtime_client::runtime::WASM_BINARY.to_vec() )], ).unwrap(); - - assert_eq!(RuntimeUpgraded::get(), true); }); } } diff --git a/substrate/primitives/storage/src/lib.rs b/substrate/primitives/storage/src/lib.rs index d32c54aae8..76fd4baac9 100644 --- a/substrate/primitives/storage/src/lib.rs +++ b/substrate/primitives/storage/src/lib.rs @@ -207,7 +207,7 @@ impl<'a> ChildInfo<'a> { } } - /// Create child info from a linear byte packed value and a given type. + /// Create child info from a linear byte packed value and a given type. pub fn resolve_child_info(child_type: u32, data: &'a[u8]) -> Option { match child_type { x if x == ChildType::CryptoUniqueId as u32 => Some(ChildInfo::new_default(data)), diff --git a/substrate/test-utils/runtime/src/lib.rs b/substrate/test-utils/runtime/src/lib.rs index 655cf96006..59955cce48 100644 --- a/substrate/test-utils/runtime/src/lib.rs +++ b/substrate/test-utils/runtime/src/lib.rs @@ -43,7 +43,7 @@ use sp_runtime::{ }, }; use sp_version::RuntimeVersion; -pub use sp_core::{hash::H256}; +pub use sp_core::hash::H256; #[cfg(any(feature = "std", test))] use sp_version::NativeVersion; use frame_support::{impl_outer_origin, parameter_types, weights::Weight}; @@ -64,10 +64,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: create_runtime_str!("test"), impl_name: create_runtime_str!("parity-test"), authoring_version: 1, - spec_version: 1, - #[cfg(feature = "std")] - impl_version: 1, - #[cfg(not(feature = "std"))] + spec_version: 2, impl_version: 2, apis: RUNTIME_API_VERSIONS, };