From 467aa71ede88c3c25ac941b5d4f6a0f45e35ee99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Wed, 18 Aug 2021 11:52:59 +0200 Subject: [PATCH 01/11] Emit digest item on Runtime Code changes. --- frame/system/src/lib.rs | 38 ++++++++++-------- frame/system/src/tests.rs | 5 +++ primitives/runtime/src/generic/digest.rs | 49 ++++++++++++++++-------- 3 files changed, 62 insertions(+), 30 deletions(-) diff --git a/frame/system/src/lib.rs b/frame/system/src/lib.rs index a8bf253c392c4..2833fda0bc741 100644 --- a/frame/system/src/lib.rs +++ b/frame/system/src/lib.rs @@ -350,11 +350,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::RuntimeUpdated); Ok(().into()) } @@ -365,6 +367,7 @@ pub mod pallet { /// - 1 storage write (codec `O(C)`). /// - 1 call to `can_set_code`: `O(S)` (calls `sp_io::misc::runtime_version` which is /// expensive). + /// - 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 +376,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); + Self::do_set_code(code)?; Ok(().into()) } @@ -384,6 +385,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. # @@ -393,8 +395,7 @@ pub mod pallet { code: Vec, ) -> DispatchResultWithPostInfo { ensure_root(origin)?; - T::OnSetCode::set_code(code)?; - Self::deposit_event(Event::CodeUpdated); + Self::do_set_code(code)?; Ok(().into()) } @@ -1068,6 +1069,13 @@ impl Pallet { Account::::contains_key(who) } + fn do_set_code(code: Vec) -> DispatchResult { + T::OnSetCode::set_code(code)?; + Self::deposit_log(generic::DigestItem::RuntimeUpdated); + 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) { @@ -1128,18 +1136,18 @@ impl Pallet { Pallet::::on_killed_account(who.clone()); Ok(DecRefStatus::Reaped) - }, + } (1, c, _) if c > 0 => { // Cannot remove last provider if there are consumers. Err(DispatchError::ConsumerRemaining) - }, + } (x, _, _) => { // Account will continue to exist as there is either > 1 provider or // > 0 sufficients. account.providers = x - 1; *maybe_account = Some(account); Ok(DecRefStatus::Exists) - }, + } } } else { log::error!( @@ -1183,12 +1191,12 @@ impl Pallet { (0, 0) | (1, 0) => { Pallet::::on_killed_account(who.clone()); DecRefStatus::Reaped - }, + } (x, _) => { account.sufficients = x - 1; *maybe_account = Some(account); DecRefStatus::Exists - }, + } } } else { log::error!( @@ -1280,7 +1288,7 @@ impl Pallet { let block_number = Self::block_number(); // Don't populate events on genesis. if block_number.is_zero() { - return + return; } let phase = ExecutionPhase::::get().unwrap_or_default(); @@ -1534,7 +1542,7 @@ impl Pallet { err, ); Event::ExtrinsicFailed(err.error, info) - }, + } }); let next_extrinsic_index = Self::extrinsic_index().unwrap_or_default() + 1u32; @@ -1668,10 +1676,10 @@ impl StoredMap for Pallet { DecRefStatus::Reaped => return Ok(result), DecRefStatus::Exists => { // Update value as normal... - }, + } } } else if !was_providing && !is_providing { - return Ok(result) + return Ok(result); } Account::::mutate(k, |a| a.data = some_data.unwrap_or_default()); Ok(result) @@ -1687,7 +1695,7 @@ pub fn split_inner( Some(inner) => { let (r, s) = splitter(inner); (Some(r), Some(s)) - }, + } None => (None, None), } } diff --git a/frame/system/src/tests.rs b/frame/system/src/tests.rs index f0a6a96ccc1e3..a321c089f54ad 100644 --- a/frame/system/src/tests.rs +++ b/frame/system/src/tests.rs @@ -478,3 +478,8 @@ fn extrinsics_root_is_calculated_correctly() { assert_eq!(ext_root, *header.extrinsics_root()); }); } + +#[test] +fn runtime_updated_digest_emitted() { + assert_eq!(true, false); +} diff --git a/primitives/runtime/src/generic/digest.rs b/primitives/runtime/src/generic/digest.rs index 390acb87f690d..fb767c78f1db6 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. + RuntimeUpdated, } /// 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. + RuntimeUpdated, } /// 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, + RuntimeUpdated = 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::RuntimeUpdated => DigestItemRef::RuntimeUpdated, } } @@ -310,18 +322,20 @@ impl Decode for DigestItem { DigestItemType::PreRuntime => { let vals: (ConsensusEngineId, Vec) = Decode::decode(input)?; Ok(Self::PreRuntime(vals.0, vals.1)) - }, + } DigestItemType::Consensus => { let vals: (ConsensusEngineId, Vec) = Decode::decode(input)?; Ok(Self::Consensus(vals.0, vals.1)) - }, + } DigestItemType::Seal => { let vals: (ConsensusEngineId, Vec) = Decode::decode(input)?; Ok(Self::Seal(vals.0, vals.1)) - }, - DigestItemType::ChangesTrieSignal => - Ok(Self::ChangesTrieSignal(Decode::decode(input)?)), + } + DigestItemType::ChangesTrieSignal => { + Ok(Self::ChangesTrieSignal(Decode::decode(input)?)) + } DigestItemType::Other => Ok(Self::Other(Decode::decode(input)?)), + DigestItemType::RuntimeUpdated => Ok(Self::RuntimeUpdated), } } } @@ -379,11 +393,13 @@ impl<'a, Hash> DigestItemRef<'a, Hash> { /// return the opaque data it contains. pub fn try_as_raw(&self, id: OpaqueDigestItemId) -> Option<&'a [u8]> { match (id, self) { - (OpaqueDigestItemId::Consensus(w), &Self::Consensus(v, s)) | - (OpaqueDigestItemId::Seal(w), &Self::Seal(v, s)) | - (OpaqueDigestItemId::PreRuntime(w), &Self::PreRuntime(v, s)) + (OpaqueDigestItemId::Consensus(w), &Self::Consensus(v, s)) + | (OpaqueDigestItemId::Seal(w), &Self::Seal(v, s)) + | (OpaqueDigestItemId::PreRuntime(w), &Self::PreRuntime(v, s)) if v == w => - Some(&s[..]), + { + Some(&s[..]) + } (OpaqueDigestItemId::Other, &Self::Other(s)) => Some(&s[..]), _ => None, } @@ -436,27 +452,30 @@ impl<'a, Hash: Encode> Encode for DigestItemRef<'a, Hash> { Self::ChangesTrieRoot(changes_trie_root) => { DigestItemType::ChangesTrieRoot.encode_to(&mut v); changes_trie_root.encode_to(&mut v); - }, + } Self::Consensus(val, data) => { DigestItemType::Consensus.encode_to(&mut v); (val, data).encode_to(&mut v); - }, + } Self::Seal(val, sig) => { DigestItemType::Seal.encode_to(&mut v); (val, sig).encode_to(&mut v); - }, + } Self::PreRuntime(val, data) => { DigestItemType::PreRuntime.encode_to(&mut v); (val, data).encode_to(&mut v); - }, + } Self::ChangesTrieSignal(changes_trie_signal) => { DigestItemType::ChangesTrieSignal.encode_to(&mut v); changes_trie_signal.encode_to(&mut v); - }, + } Self::Other(val) => { DigestItemType::Other.encode_to(&mut v); val.encode_to(&mut v); - }, + } + Self::RuntimeUpdated => { + DigestItemType::RuntimeUpdated.encode_to(&mut v); + } } v From 38abfa51065c00d1f17e3c53d7a06d464e6d73bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Wed, 18 Aug 2021 12:07:55 +0200 Subject: [PATCH 02/11] Add tests. --- frame/system/src/tests.rs | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/frame/system/src/tests.rs b/frame/system/src/tests.rs index 2ddfdad631168..340fb95a68913 100644 --- a/frame/system/src/tests.rs +++ b/frame/system/src/tests.rs @@ -390,21 +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); - - assert_eq!( - System::digest() - .logs - .into_iter() - .filter(|item| item == DigestItem::RuntimeUpdated) - .count(), - 1, - "Incorrect number of Runtime Updated digest items", - ); }); } } +fn assert_runtime_updated_digest(num: usize) { + assert_eq!( + System::digest() + .logs + .into_iter() + .filter(|item| *item == generic::DigestItem::RuntimeUpdated) + .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(); @@ -490,10 +493,10 @@ fn extrinsics_root_is_calculated_correctly() { } #[test] -fn runtime_updated_digest_emitted() { +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_code(vec![1, 2, 3, 4]); - System::set_code_checks_works + System::set_heap_pages(RawOrigin::Root.into(), 5).unwrap(); + assert_runtime_updated_digest(1); }); } From 7123eecd83d5a4f65089b67f80a40e2749daeec2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Wed, 18 Aug 2021 15:59:54 +0200 Subject: [PATCH 03/11] cargo +nightly fmt --all --- frame/system/src/lib.rs | 20 +++++++------- primitives/runtime/src/generic/digest.rs | 35 +++++++++++------------- 2 files changed, 26 insertions(+), 29 deletions(-) diff --git a/frame/system/src/lib.rs b/frame/system/src/lib.rs index 2833fda0bc741..1c8cba8cf16da 100644 --- a/frame/system/src/lib.rs +++ b/frame/system/src/lib.rs @@ -1136,18 +1136,18 @@ impl Pallet { Pallet::::on_killed_account(who.clone()); Ok(DecRefStatus::Reaped) - } + }, (1, c, _) if c > 0 => { // Cannot remove last provider if there are consumers. Err(DispatchError::ConsumerRemaining) - } + }, (x, _, _) => { // Account will continue to exist as there is either > 1 provider or // > 0 sufficients. account.providers = x - 1; *maybe_account = Some(account); Ok(DecRefStatus::Exists) - } + }, } } else { log::error!( @@ -1191,12 +1191,12 @@ impl Pallet { (0, 0) | (1, 0) => { Pallet::::on_killed_account(who.clone()); DecRefStatus::Reaped - } + }, (x, _) => { account.sufficients = x - 1; *maybe_account = Some(account); DecRefStatus::Exists - } + }, } } else { log::error!( @@ -1288,7 +1288,7 @@ impl Pallet { let block_number = Self::block_number(); // Don't populate events on genesis. if block_number.is_zero() { - return; + return } let phase = ExecutionPhase::::get().unwrap_or_default(); @@ -1542,7 +1542,7 @@ impl Pallet { err, ); Event::ExtrinsicFailed(err.error, info) - } + }, }); let next_extrinsic_index = Self::extrinsic_index().unwrap_or_default() + 1u32; @@ -1676,10 +1676,10 @@ impl StoredMap for Pallet { DecRefStatus::Reaped => return Ok(result), DecRefStatus::Exists => { // Update value as normal... - } + }, } } else if !was_providing && !is_providing { - return Ok(result); + return Ok(result) } Account::::mutate(k, |a| a.data = some_data.unwrap_or_default()); Ok(result) @@ -1695,7 +1695,7 @@ pub fn split_inner( Some(inner) => { let (r, s) = splitter(inner); (Some(r), Some(s)) - } + }, None => (None, None), } } diff --git a/primitives/runtime/src/generic/digest.rs b/primitives/runtime/src/generic/digest.rs index fb767c78f1db6..55f769964a828 100644 --- a/primitives/runtime/src/generic/digest.rs +++ b/primitives/runtime/src/generic/digest.rs @@ -322,18 +322,17 @@ impl Decode for DigestItem { DigestItemType::PreRuntime => { let vals: (ConsensusEngineId, Vec) = Decode::decode(input)?; Ok(Self::PreRuntime(vals.0, vals.1)) - } + }, DigestItemType::Consensus => { let vals: (ConsensusEngineId, Vec) = Decode::decode(input)?; Ok(Self::Consensus(vals.0, vals.1)) - } + }, DigestItemType::Seal => { let vals: (ConsensusEngineId, Vec) = Decode::decode(input)?; Ok(Self::Seal(vals.0, vals.1)) - } - DigestItemType::ChangesTrieSignal => { - Ok(Self::ChangesTrieSignal(Decode::decode(input)?)) - } + }, + DigestItemType::ChangesTrieSignal => + Ok(Self::ChangesTrieSignal(Decode::decode(input)?)), DigestItemType::Other => Ok(Self::Other(Decode::decode(input)?)), DigestItemType::RuntimeUpdated => Ok(Self::RuntimeUpdated), } @@ -393,13 +392,11 @@ impl<'a, Hash> DigestItemRef<'a, Hash> { /// return the opaque data it contains. pub fn try_as_raw(&self, id: OpaqueDigestItemId) -> Option<&'a [u8]> { match (id, self) { - (OpaqueDigestItemId::Consensus(w), &Self::Consensus(v, s)) - | (OpaqueDigestItemId::Seal(w), &Self::Seal(v, s)) - | (OpaqueDigestItemId::PreRuntime(w), &Self::PreRuntime(v, s)) + (OpaqueDigestItemId::Consensus(w), &Self::Consensus(v, s)) | + (OpaqueDigestItemId::Seal(w), &Self::Seal(v, s)) | + (OpaqueDigestItemId::PreRuntime(w), &Self::PreRuntime(v, s)) if v == w => - { - Some(&s[..]) - } + Some(&s[..]), (OpaqueDigestItemId::Other, &Self::Other(s)) => Some(&s[..]), _ => None, } @@ -452,30 +449,30 @@ impl<'a, Hash: Encode> Encode for DigestItemRef<'a, Hash> { Self::ChangesTrieRoot(changes_trie_root) => { DigestItemType::ChangesTrieRoot.encode_to(&mut v); changes_trie_root.encode_to(&mut v); - } + }, Self::Consensus(val, data) => { DigestItemType::Consensus.encode_to(&mut v); (val, data).encode_to(&mut v); - } + }, Self::Seal(val, sig) => { DigestItemType::Seal.encode_to(&mut v); (val, sig).encode_to(&mut v); - } + }, Self::PreRuntime(val, data) => { DigestItemType::PreRuntime.encode_to(&mut v); (val, data).encode_to(&mut v); - } + }, Self::ChangesTrieSignal(changes_trie_signal) => { DigestItemType::ChangesTrieSignal.encode_to(&mut v); changes_trie_signal.encode_to(&mut v); - } + }, Self::Other(val) => { DigestItemType::Other.encode_to(&mut v); val.encode_to(&mut v); - } + }, Self::RuntimeUpdated => { DigestItemType::RuntimeUpdated.encode_to(&mut v); - } + }, } v From 947ed2cefeb0821735c63ba892e9e586c22b2bec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Thu, 19 Aug 2021 14:53:23 +0200 Subject: [PATCH 04/11] Rename. --- frame/system/src/lib.rs | 4 ++-- frame/system/src/tests.rs | 2 +- primitives/runtime/src/generic/digest.rs | 15 ++++++++------- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/frame/system/src/lib.rs b/frame/system/src/lib.rs index 1c8cba8cf16da..75abe03234f25 100644 --- a/frame/system/src/lib.rs +++ b/frame/system/src/lib.rs @@ -356,7 +356,7 @@ pub mod pallet { 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::RuntimeUpdated); + Self::deposit_log(generic::DigestItem::RuntimeCodeOrHeapPagesUpdated); Ok(().into()) } @@ -1071,7 +1071,7 @@ impl Pallet { fn do_set_code(code: Vec) -> DispatchResult { T::OnSetCode::set_code(code)?; - Self::deposit_log(generic::DigestItem::RuntimeUpdated); + Self::deposit_log(generic::DigestItem::RuntimeCodeOrHeapPagesUpdated); Self::deposit_event(Event::CodeUpdated); Ok(()) } diff --git a/frame/system/src/tests.rs b/frame/system/src/tests.rs index 340fb95a68913..2499c2aaf884f 100644 --- a/frame/system/src/tests.rs +++ b/frame/system/src/tests.rs @@ -401,7 +401,7 @@ fn assert_runtime_updated_digest(num: usize) { System::digest() .logs .into_iter() - .filter(|item| *item == generic::DigestItem::RuntimeUpdated) + .filter(|item| *item == generic::DigestItem::RuntimeCodeOrHeapPagesUpdated) .count(), num, "Incorrect number of Runtime Updated digest items", diff --git a/primitives/runtime/src/generic/digest.rs b/primitives/runtime/src/generic/digest.rs index 55f769964a828..adb8ff0dee956 100644 --- a/primitives/runtime/src/generic/digest.rs +++ b/primitives/runtime/src/generic/digest.rs @@ -125,7 +125,7 @@ pub enum DigestItem { /// Currently this is triggered when: /// 1. Runtime code blob is changed or /// 2. `heap_pages` value is changed. - RuntimeUpdated, + RuntimeCodeOrHeapPagesUpdated, } /// Available changes trie signals. @@ -193,7 +193,7 @@ pub enum DigestItemRef<'a, Hash: 'a> { /// Any 'non-system' digest item, opaque to the native code. Other(&'a Vec), /// Runtime code or heap pages updated. - RuntimeUpdated, + RuntimeCodeOrHeapPagesUpdated, } /// Type of the digest item. Used to gain explicit control over `DigestItem` encoding @@ -209,7 +209,7 @@ pub enum DigestItemType { Seal = 5, PreRuntime = 6, ChangesTrieSignal = 7, - RuntimeUpdated = 8, + RuntimeCodeOrHeapPagesUpdated = 8, } /// Type of a digest item that contains raw data; this also names the consensus engine ID where @@ -236,7 +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::RuntimeUpdated => DigestItemRef::RuntimeUpdated, + Self::RuntimeCodeOrHeapPagesUpdated => DigestItemRef::RuntimeCodeOrHeapPagesUpdated, } } @@ -334,7 +334,8 @@ impl Decode for DigestItem { DigestItemType::ChangesTrieSignal => Ok(Self::ChangesTrieSignal(Decode::decode(input)?)), DigestItemType::Other => Ok(Self::Other(Decode::decode(input)?)), - DigestItemType::RuntimeUpdated => Ok(Self::RuntimeUpdated), + DigestItemType::RuntimeCodeOrHeapPagesUpdated => + Ok(Self::RuntimeCodeOrHeapPagesUpdated), } } } @@ -470,8 +471,8 @@ impl<'a, Hash: Encode> Encode for DigestItemRef<'a, Hash> { DigestItemType::Other.encode_to(&mut v); val.encode_to(&mut v); }, - Self::RuntimeUpdated => { - DigestItemType::RuntimeUpdated.encode_to(&mut v); + Self::RuntimeCodeOrHeapPagesUpdated => { + DigestItemType::RuntimeCodeOrHeapPagesUpdated.encode_to(&mut v); }, } From 33e440eea67b61e305793c56d6f0afba43563d02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Fri, 20 Aug 2021 13:06:54 +0200 Subject: [PATCH 05/11] Add comment. --- frame/system/src/lib.rs | 41 +++++++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/frame/system/src/lib.rs b/frame/system/src/lib.rs index 75abe03234f25..098f326fc5115 100644 --- a/frame/system/src/lib.rs +++ b/frame/system/src/lib.rs @@ -140,12 +140,12 @@ pub use pallet::*; /// Do something when we should be setting the code. pub trait SetCode { /// Set the code to the given blob. - fn set_code(code: Vec) -> DispatchResult; + fn set_code(code: Vec) -> DispatchResult; } impl SetCode for () { - fn set_code(code: Vec) -> DispatchResult { - storage::unhashed::put_raw(well_known_keys::CODE, &code); + fn set_code(code: Vec) -> DispatchResult { + >::update_code_in_storage(&code)?; Ok(()) } } @@ -364,9 +364,9 @@ 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 @@ -376,7 +376,7 @@ pub mod pallet { pub fn set_code(origin: OriginFor, code: Vec) -> DispatchResultWithPostInfo { ensure_root(origin)?; Self::can_set_code(&code)?; - Self::do_set_code(code)?; + T::OnSetCode::set_code::(code)?; Ok(().into()) } @@ -395,7 +395,7 @@ pub mod pallet { code: Vec, ) -> DispatchResultWithPostInfo { ensure_root(origin)?; - Self::do_set_code(code)?; + T::OnSetCode::set_code::(code)?; Ok(().into()) } @@ -1069,8 +1069,13 @@ impl Pallet { Account::::contains_key(who) } - fn do_set_code(code: Vec) -> DispatchResult { - T::OnSetCode::set_code(code)?; + /// 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::RuntimeCodeOrHeapPagesUpdated); Self::deposit_event(Event::CodeUpdated); Ok(()) @@ -1136,18 +1141,18 @@ impl Pallet { Pallet::::on_killed_account(who.clone()); Ok(DecRefStatus::Reaped) - }, + } (1, c, _) if c > 0 => { // Cannot remove last provider if there are consumers. Err(DispatchError::ConsumerRemaining) - }, + } (x, _, _) => { // Account will continue to exist as there is either > 1 provider or // > 0 sufficients. account.providers = x - 1; *maybe_account = Some(account); Ok(DecRefStatus::Exists) - }, + } } } else { log::error!( @@ -1191,12 +1196,12 @@ impl Pallet { (0, 0) | (1, 0) => { Pallet::::on_killed_account(who.clone()); DecRefStatus::Reaped - }, + } (x, _) => { account.sufficients = x - 1; *maybe_account = Some(account); DecRefStatus::Exists - }, + } } } else { log::error!( @@ -1288,7 +1293,7 @@ impl Pallet { let block_number = Self::block_number(); // Don't populate events on genesis. if block_number.is_zero() { - return + return; } let phase = ExecutionPhase::::get().unwrap_or_default(); @@ -1542,7 +1547,7 @@ impl Pallet { err, ); Event::ExtrinsicFailed(err.error, info) - }, + } }); let next_extrinsic_index = Self::extrinsic_index().unwrap_or_default() + 1u32; @@ -1676,10 +1681,10 @@ impl StoredMap for Pallet { DecRefStatus::Reaped => return Ok(result), DecRefStatus::Exists => { // Update value as normal... - }, + } } } else if !was_providing && !is_providing { - return Ok(result) + return Ok(result); } Account::::mutate(k, |a| a.data = some_data.unwrap_or_default()); Ok(result) @@ -1695,7 +1700,7 @@ pub fn split_inner( Some(inner) => { let (r, s) = splitter(inner); (Some(r), Some(s)) - }, + } None => (None, None), } } From 717a680da3e8f3446dfd480bb04e090fbbdbed15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Mon, 23 Aug 2021 12:48:11 +0200 Subject: [PATCH 06/11] Move generic parameter to the trait. --- frame/system/src/lib.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/frame/system/src/lib.rs b/frame/system/src/lib.rs index 098f326fc5115..ce3a283f7510e 100644 --- a/frame/system/src/lib.rs +++ b/frame/system/src/lib.rs @@ -138,13 +138,13 @@ 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; + fn set_code(code: Vec) -> DispatchResult; } -impl SetCode for () { - fn set_code(code: Vec) -> DispatchResult { +impl SetCode for () { + fn set_code(code: Vec) -> DispatchResult { >::update_code_in_storage(&code)?; Ok(()) } @@ -298,7 +298,7 @@ pub mod pallet { /// What to do if the user wants the code set to something. Just use `()` unless you are in /// cumulus. - type OnSetCode: SetCode; + type OnSetCode: SetCode; } #[pallet::pallet] @@ -376,7 +376,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)?; + T::OnSetCode::set_code(code)?; Ok(().into()) } @@ -395,7 +395,7 @@ pub mod pallet { code: Vec, ) -> DispatchResultWithPostInfo { ensure_root(origin)?; - T::OnSetCode::set_code::(code)?; + T::OnSetCode::set_code(code)?; Ok(().into()) } From ed0943dca1b7edb9cfa33adfcb1688b8332f81ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Tue, 24 Aug 2021 09:53:06 +0200 Subject: [PATCH 07/11] cargo +nightly fmt --all --- frame/system/src/lib.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/frame/system/src/lib.rs b/frame/system/src/lib.rs index ce3a283f7510e..8b201d583bf76 100644 --- a/frame/system/src/lib.rs +++ b/frame/system/src/lib.rs @@ -1141,18 +1141,18 @@ impl Pallet { Pallet::::on_killed_account(who.clone()); Ok(DecRefStatus::Reaped) - } + }, (1, c, _) if c > 0 => { // Cannot remove last provider if there are consumers. Err(DispatchError::ConsumerRemaining) - } + }, (x, _, _) => { // Account will continue to exist as there is either > 1 provider or // > 0 sufficients. account.providers = x - 1; *maybe_account = Some(account); Ok(DecRefStatus::Exists) - } + }, } } else { log::error!( @@ -1196,12 +1196,12 @@ impl Pallet { (0, 0) | (1, 0) => { Pallet::::on_killed_account(who.clone()); DecRefStatus::Reaped - } + }, (x, _) => { account.sufficients = x - 1; *maybe_account = Some(account); DecRefStatus::Exists - } + }, } } else { log::error!( @@ -1293,7 +1293,7 @@ impl Pallet { let block_number = Self::block_number(); // Don't populate events on genesis. if block_number.is_zero() { - return; + return } let phase = ExecutionPhase::::get().unwrap_or_default(); @@ -1547,7 +1547,7 @@ impl Pallet { err, ); Event::ExtrinsicFailed(err.error, info) - } + }, }); let next_extrinsic_index = Self::extrinsic_index().unwrap_or_default() + 1u32; @@ -1681,10 +1681,10 @@ impl StoredMap for Pallet { DecRefStatus::Reaped => return Ok(result), DecRefStatus::Exists => { // Update value as normal... - } + }, } } else if !was_providing && !is_providing { - return Ok(result); + return Ok(result) } Account::::mutate(k, |a| a.data = some_data.unwrap_or_default()); Ok(result) @@ -1700,7 +1700,7 @@ pub fn split_inner( Some(inner) => { let (r, s) = splitter(inner); (Some(r), Some(s)) - } + }, None => (None, None), } } From 6e99b76dea00aa63efe9242a36d7810685f177a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Mon, 30 Aug 2021 11:55:36 +0200 Subject: [PATCH 08/11] Elaborate in doc. --- frame/system/src/lib.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/frame/system/src/lib.rs b/frame/system/src/lib.rs index 58cd62c6e5dda..f15f85166c8f7 100644 --- a/frame/system/src/lib.rs +++ b/frame/system/src/lib.rs @@ -296,8 +296,12 @@ 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. + /// 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; } From 4803778d971f3e0b6274016761e82bf3f6e5cf52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Wed, 1 Sep 2021 17:20:30 +0200 Subject: [PATCH 09/11] Revert to RuntimeUpdated name --- frame/system/src/lib.rs | 4 ++-- frame/system/src/tests.rs | 2 +- primitives/runtime/src/generic/digest.rs | 16 ++++++++-------- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/frame/system/src/lib.rs b/frame/system/src/lib.rs index f15f85166c8f7..26082fa4cae15 100644 --- a/frame/system/src/lib.rs +++ b/frame/system/src/lib.rs @@ -360,7 +360,7 @@ pub mod pallet { 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::RuntimeCodeOrHeapPagesUpdated); + Self::deposit_log(generic::DigestItem::RuntimeUpdated); Ok(().into()) } @@ -1083,7 +1083,7 @@ impl Pallet { /// 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::RuntimeCodeOrHeapPagesUpdated); + Self::deposit_log(generic::DigestItem::RuntimeUpdated); Self::deposit_event(Event::CodeUpdated); Ok(()) } diff --git a/frame/system/src/tests.rs b/frame/system/src/tests.rs index 2499c2aaf884f..340fb95a68913 100644 --- a/frame/system/src/tests.rs +++ b/frame/system/src/tests.rs @@ -401,7 +401,7 @@ fn assert_runtime_updated_digest(num: usize) { System::digest() .logs .into_iter() - .filter(|item| *item == generic::DigestItem::RuntimeCodeOrHeapPagesUpdated) + .filter(|item| *item == generic::DigestItem::RuntimeUpdated) .count(), num, "Incorrect number of Runtime Updated digest items", diff --git a/primitives/runtime/src/generic/digest.rs b/primitives/runtime/src/generic/digest.rs index adb8ff0dee956..e88333c7d1b14 100644 --- a/primitives/runtime/src/generic/digest.rs +++ b/primitives/runtime/src/generic/digest.rs @@ -125,7 +125,7 @@ pub enum DigestItem { /// Currently this is triggered when: /// 1. Runtime code blob is changed or /// 2. `heap_pages` value is changed. - RuntimeCodeOrHeapPagesUpdated, + RuntimeUpdated, } /// Available changes trie signals. @@ -193,7 +193,7 @@ pub enum DigestItemRef<'a, Hash: 'a> { /// Any 'non-system' digest item, opaque to the native code. Other(&'a Vec), /// Runtime code or heap pages updated. - RuntimeCodeOrHeapPagesUpdated, + RuntimeUpdated, } /// Type of the digest item. Used to gain explicit control over `DigestItem` encoding @@ -209,7 +209,7 @@ pub enum DigestItemType { Seal = 5, PreRuntime = 6, ChangesTrieSignal = 7, - RuntimeCodeOrHeapPagesUpdated = 8, + RuntimeUpdated = 8, } /// Type of a digest item that contains raw data; this also names the consensus engine ID where @@ -236,7 +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::RuntimeCodeOrHeapPagesUpdated => DigestItemRef::RuntimeCodeOrHeapPagesUpdated, + Self::RuntimeUpdated => DigestItemRef::RuntimeUpdated, } } @@ -334,8 +334,8 @@ impl Decode for DigestItem { DigestItemType::ChangesTrieSignal => Ok(Self::ChangesTrieSignal(Decode::decode(input)?)), DigestItemType::Other => Ok(Self::Other(Decode::decode(input)?)), - DigestItemType::RuntimeCodeOrHeapPagesUpdated => - Ok(Self::RuntimeCodeOrHeapPagesUpdated), + DigestItemType::RuntimeUpdated => + Ok(Self::RuntimeUpdated), } } } @@ -471,8 +471,8 @@ impl<'a, Hash: Encode> Encode for DigestItemRef<'a, Hash> { DigestItemType::Other.encode_to(&mut v); val.encode_to(&mut v); }, - Self::RuntimeCodeOrHeapPagesUpdated => { - DigestItemType::RuntimeCodeOrHeapPagesUpdated.encode_to(&mut v); + Self::RuntimeUpdated => { + DigestItemType::RuntimeUpdated.encode_to(&mut v); }, } From b3df42afdf3454591032b39d284df4bb0f16b48c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Wed, 1 Sep 2021 21:18:51 +0200 Subject: [PATCH 10/11] cargo +nightly fmt --all --- primitives/runtime/src/generic/digest.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/primitives/runtime/src/generic/digest.rs b/primitives/runtime/src/generic/digest.rs index e88333c7d1b14..55f769964a828 100644 --- a/primitives/runtime/src/generic/digest.rs +++ b/primitives/runtime/src/generic/digest.rs @@ -334,8 +334,7 @@ impl Decode for DigestItem { DigestItemType::ChangesTrieSignal => Ok(Self::ChangesTrieSignal(Decode::decode(input)?)), DigestItemType::Other => Ok(Self::Other(Decode::decode(input)?)), - DigestItemType::RuntimeUpdated => - Ok(Self::RuntimeUpdated), + DigestItemType::RuntimeUpdated => Ok(Self::RuntimeUpdated), } } } From b403705d179197c29c55c686a883dea2c8d06a5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Tue, 14 Sep 2021 15:03:29 +0200 Subject: [PATCH 11/11] Rename to RuntimeEnvironmentUpdated --- frame/system/src/lib.rs | 4 ++-- frame/system/src/tests.rs | 2 +- primitives/runtime/src/generic/digest.rs | 14 +++++++------- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/frame/system/src/lib.rs b/frame/system/src/lib.rs index 26082fa4cae15..3d89e09a25a7e 100644 --- a/frame/system/src/lib.rs +++ b/frame/system/src/lib.rs @@ -360,7 +360,7 @@ pub mod pallet { 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::RuntimeUpdated); + Self::deposit_log(generic::DigestItem::RuntimeEnvironmentUpdated); Ok(().into()) } @@ -1083,7 +1083,7 @@ impl Pallet { /// 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::RuntimeUpdated); + Self::deposit_log(generic::DigestItem::RuntimeEnvironmentUpdated); Self::deposit_event(Event::CodeUpdated); Ok(()) } diff --git a/frame/system/src/tests.rs b/frame/system/src/tests.rs index 340fb95a68913..a4dd3403f2c3a 100644 --- a/frame/system/src/tests.rs +++ b/frame/system/src/tests.rs @@ -401,7 +401,7 @@ fn assert_runtime_updated_digest(num: usize) { System::digest() .logs .into_iter() - .filter(|item| *item == generic::DigestItem::RuntimeUpdated) + .filter(|item| *item == generic::DigestItem::RuntimeEnvironmentUpdated) .count(), num, "Incorrect number of Runtime Updated digest items", diff --git a/primitives/runtime/src/generic/digest.rs b/primitives/runtime/src/generic/digest.rs index 55f769964a828..99d27ad5826c4 100644 --- a/primitives/runtime/src/generic/digest.rs +++ b/primitives/runtime/src/generic/digest.rs @@ -125,7 +125,7 @@ pub enum DigestItem { /// Currently this is triggered when: /// 1. Runtime code blob is changed or /// 2. `heap_pages` value is changed. - RuntimeUpdated, + RuntimeEnvironmentUpdated, } /// Available changes trie signals. @@ -193,7 +193,7 @@ pub enum DigestItemRef<'a, Hash: 'a> { /// Any 'non-system' digest item, opaque to the native code. Other(&'a Vec), /// Runtime code or heap pages updated. - RuntimeUpdated, + RuntimeEnvironmentUpdated, } /// Type of the digest item. Used to gain explicit control over `DigestItem` encoding @@ -209,7 +209,7 @@ pub enum DigestItemType { Seal = 5, PreRuntime = 6, ChangesTrieSignal = 7, - RuntimeUpdated = 8, + RuntimeEnvironmentUpdated = 8, } /// Type of a digest item that contains raw data; this also names the consensus engine ID where @@ -236,7 +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::RuntimeUpdated => DigestItemRef::RuntimeUpdated, + Self::RuntimeEnvironmentUpdated => DigestItemRef::RuntimeEnvironmentUpdated, } } @@ -334,7 +334,7 @@ impl Decode for DigestItem { DigestItemType::ChangesTrieSignal => Ok(Self::ChangesTrieSignal(Decode::decode(input)?)), DigestItemType::Other => Ok(Self::Other(Decode::decode(input)?)), - DigestItemType::RuntimeUpdated => Ok(Self::RuntimeUpdated), + DigestItemType::RuntimeEnvironmentUpdated => Ok(Self::RuntimeEnvironmentUpdated), } } } @@ -470,8 +470,8 @@ impl<'a, Hash: Encode> Encode for DigestItemRef<'a, Hash> { DigestItemType::Other.encode_to(&mut v); val.encode_to(&mut v); }, - Self::RuntimeUpdated => { - DigestItemType::RuntimeUpdated.encode_to(&mut v); + Self::RuntimeEnvironmentUpdated => { + DigestItemType::RuntimeEnvironmentUpdated.encode_to(&mut v); }, }