From 598d74bc067ab4532ee008e339f48278d3bf568b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Wed, 15 Sep 2021 10:07:06 +0200 Subject: [PATCH] Emit log on Runtime Code change. (#9580) * Emit digest item on Runtime Code changes. * Add tests. * cargo +nightly fmt --all * Rename. * Add comment. * Move generic parameter to the trait. * cargo +nightly fmt --all * Elaborate in doc. * Revert to RuntimeUpdated name * cargo +nightly fmt --all * Rename to RuntimeEnvironmentUpdated --- frame/system/src/lib.rs | 37 +++++++++++++++++------- frame/system/src/tests.rs | 22 ++++++++++++++ primitives/runtime/src/generic/digest.rs | 16 ++++++++++ 3 files changed, 65 insertions(+), 10 deletions(-) diff --git a/frame/system/src/lib.rs b/frame/system/src/lib.rs index 7b6ec9856d9f4..3d89e09a25a7e 100644 --- a/frame/system/src/lib.rs +++ b/frame/system/src/lib.rs @@ -138,14 +138,14 @@ pub type ConsumedWeight = PerDispatchClass; pub use pallet::*; /// Do something when we should be setting the code. -pub trait SetCode { +pub trait SetCode { /// Set the code to the given blob. fn set_code(code: Vec) -> DispatchResult; } -impl SetCode for () { +impl SetCode for () { fn set_code(code: Vec) -> DispatchResult { - storage::unhashed::put_raw(well_known_keys::CODE, &code); + >::update_code_in_storage(&code)?; Ok(()) } } @@ -296,9 +296,13 @@ pub mod pallet { #[pallet::constant] type SS58Prefix: Get; - /// What to do if the user wants the code set to something. Just use `()` unless you are in - /// cumulus. - type OnSetCode: SetCode; + /// What to do if the runtime wants to change the code to something new. + /// + /// The default (`()`) implementation is responsible for setting the correct storage + /// entry and emitting corresponding event and log item. (see [`update_code_in_storage`]). + /// It's unlikely that this needs to be customized, unless you are writing a parachain using + /// `Cumulus`, where the actual code change is deferred. + type OnSetCode: SetCode; } #[pallet::pallet] @@ -350,11 +354,13 @@ pub mod pallet { /// - 1 storage write. /// - Base Weight: 1.405 µs /// - 1 write to HEAP_PAGES + /// - 1 digest item /// # #[pallet::weight((T::SystemWeightInfo::set_heap_pages(), DispatchClass::Operational))] pub fn set_heap_pages(origin: OriginFor, pages: u64) -> DispatchResultWithPostInfo { ensure_root(origin)?; storage::unhashed::put_raw(well_known_keys::HEAP_PAGES, &pages.encode()); + Self::deposit_log(generic::DigestItem::RuntimeEnvironmentUpdated); Ok(().into()) } @@ -362,9 +368,10 @@ pub mod pallet { /// /// # /// - `O(C + S)` where `C` length of `code` and `S` complexity of `can_set_code` - /// - 1 storage write (codec `O(C)`). /// - 1 call to `can_set_code`: `O(S)` (calls `sp_io::misc::runtime_version` which is /// expensive). + /// - 1 storage write (codec `O(C)`). + /// - 1 digest item. /// - 1 event. /// The weight of this function is dependent on the runtime, but generally this is very /// expensive. We will treat this as a full block. @@ -373,9 +380,7 @@ pub mod pallet { pub fn set_code(origin: OriginFor, code: Vec) -> DispatchResultWithPostInfo { ensure_root(origin)?; Self::can_set_code(&code)?; - T::OnSetCode::set_code(code)?; - Self::deposit_event(Event::CodeUpdated); Ok(().into()) } @@ -384,6 +389,7 @@ pub mod pallet { /// # /// - `O(C)` where `C` length of `code` /// - 1 storage write (codec `O(C)`). + /// - 1 digest item. /// - 1 event. /// The weight of this function is dependent on the runtime. We will treat this as a full /// block. # @@ -394,7 +400,6 @@ pub mod pallet { ) -> DispatchResultWithPostInfo { ensure_root(origin)?; T::OnSetCode::set_code(code)?; - Self::deposit_event(Event::CodeUpdated); Ok(().into()) } @@ -1071,6 +1076,18 @@ impl Pallet { Account::::contains_key(who) } + /// Write code to the storage and emit related events and digest items. + /// + /// Note this function almost never should be used directly. It is exposed + /// for `OnSetCode` implementations that defer actual code being written to + /// the storage (for instance in case of parachains). + pub fn update_code_in_storage(code: &[u8]) -> DispatchResult { + storage::unhashed::put_raw(well_known_keys::CODE, code); + Self::deposit_log(generic::DigestItem::RuntimeEnvironmentUpdated); + Self::deposit_event(Event::CodeUpdated); + Ok(()) + } + /// Increment the reference counter on an account. #[deprecated = "Use `inc_consumers` instead"] pub fn inc_ref(who: &T::AccountId) { diff --git a/frame/system/src/tests.rs b/frame/system/src/tests.rs index f0a6a96ccc1e3..a4dd3403f2c3a 100644 --- a/frame/system/src/tests.rs +++ b/frame/system/src/tests.rs @@ -390,11 +390,24 @@ fn set_code_checks_works() { ext.execute_with(|| { let res = System::set_code(RawOrigin::Root.into(), vec![1, 2, 3, 4]); + assert_runtime_updated_digest(if res.is_ok() { 1 } else { 0 }); assert_eq!(expected.map_err(DispatchErrorWithPostInfo::from), res); }); } } +fn assert_runtime_updated_digest(num: usize) { + assert_eq!( + System::digest() + .logs + .into_iter() + .filter(|item| *item == generic::DigestItem::RuntimeEnvironmentUpdated) + .count(), + num, + "Incorrect number of Runtime Updated digest items", + ); +} + #[test] fn set_code_with_real_wasm_blob() { let executor = substrate_test_runtime_client::new_native_executor(); @@ -478,3 +491,12 @@ fn extrinsics_root_is_calculated_correctly() { assert_eq!(ext_root, *header.extrinsics_root()); }); } + +#[test] +fn runtime_updated_digest_emitted_when_heap_pages_changed() { + new_test_ext().execute_with(|| { + System::initialize(&1, &[0u8; 32].into(), &Default::default(), InitKind::Full); + System::set_heap_pages(RawOrigin::Root.into(), 5).unwrap(); + assert_runtime_updated_digest(1); + }); +} diff --git a/primitives/runtime/src/generic/digest.rs b/primitives/runtime/src/generic/digest.rs index 390acb87f690d..99d27ad5826c4 100644 --- a/primitives/runtime/src/generic/digest.rs +++ b/primitives/runtime/src/generic/digest.rs @@ -118,6 +118,14 @@ pub enum DigestItem { /// Some other thing. Unsupported and experimental. Other(Vec), + + /// An indication for the light clients that the runtime execution + /// environment is updated. + /// + /// Currently this is triggered when: + /// 1. Runtime code blob is changed or + /// 2. `heap_pages` value is changed. + RuntimeEnvironmentUpdated, } /// Available changes trie signals. @@ -184,6 +192,8 @@ pub enum DigestItemRef<'a, Hash: 'a> { ChangesTrieSignal(&'a ChangesTrieSignal), /// Any 'non-system' digest item, opaque to the native code. Other(&'a Vec), + /// Runtime code or heap pages updated. + RuntimeEnvironmentUpdated, } /// Type of the digest item. Used to gain explicit control over `DigestItem` encoding @@ -199,6 +209,7 @@ pub enum DigestItemType { Seal = 5, PreRuntime = 6, ChangesTrieSignal = 7, + RuntimeEnvironmentUpdated = 8, } /// Type of a digest item that contains raw data; this also names the consensus engine ID where @@ -225,6 +236,7 @@ impl DigestItem { Self::Seal(ref v, ref s) => DigestItemRef::Seal(v, s), Self::ChangesTrieSignal(ref s) => DigestItemRef::ChangesTrieSignal(s), Self::Other(ref v) => DigestItemRef::Other(v), + Self::RuntimeEnvironmentUpdated => DigestItemRef::RuntimeEnvironmentUpdated, } } @@ -322,6 +334,7 @@ impl Decode for DigestItem { DigestItemType::ChangesTrieSignal => Ok(Self::ChangesTrieSignal(Decode::decode(input)?)), DigestItemType::Other => Ok(Self::Other(Decode::decode(input)?)), + DigestItemType::RuntimeEnvironmentUpdated => Ok(Self::RuntimeEnvironmentUpdated), } } } @@ -457,6 +470,9 @@ impl<'a, Hash: Encode> Encode for DigestItemRef<'a, Hash> { DigestItemType::Other.encode_to(&mut v); val.encode_to(&mut v); }, + Self::RuntimeEnvironmentUpdated => { + DigestItemType::RuntimeEnvironmentUpdated.encode_to(&mut v); + }, } v