Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Emit log on Runtime Code change. #9580

Merged
15 commits merged into from
Sep 15, 2021
37 changes: 27 additions & 10 deletions frame/system/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,14 +138,14 @@ pub type ConsumedWeight = PerDispatchClass<Weight>;
pub use pallet::*;

/// Do something when we should be setting the code.
pub trait SetCode {
pub trait SetCode<T: Config> {
/// Set the code to the given blob.
fn set_code(code: Vec<u8>) -> DispatchResult;
}

impl SetCode for () {
impl<T: Config> SetCode<T> for () {
fn set_code(code: Vec<u8>) -> DispatchResult {
storage::unhashed::put_raw(well_known_keys::CODE, &code);
<Pallet<T>>::update_code_in_storage(&code)?;
Ok(())
}
}
Expand Down Expand Up @@ -296,9 +296,13 @@ pub mod pallet {
#[pallet::constant]
type SS58Prefix: Get<u16>;

/// 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<Self>;
}

#[pallet::pallet]
Expand Down Expand Up @@ -350,21 +354,24 @@ pub mod pallet {
/// - 1 storage write.
/// - Base Weight: 1.405 µs
/// - 1 write to HEAP_PAGES
/// - 1 digest item
/// # </weight>
#[pallet::weight((T::SystemWeightInfo::set_heap_pages(), DispatchClass::Operational))]
pub fn set_heap_pages(origin: OriginFor<T>, 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())
}

/// Set the new runtime code.
///
/// # <weight>
/// - `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.
Expand All @@ -373,9 +380,7 @@ pub mod pallet {
pub fn set_code(origin: OriginFor<T>, code: Vec<u8>) -> DispatchResultWithPostInfo {
ensure_root(origin)?;
Self::can_set_code(&code)?;

T::OnSetCode::set_code(code)?;
Self::deposit_event(Event::CodeUpdated);
Ok(().into())
}

Expand All @@ -384,6 +389,7 @@ pub mod pallet {
/// # <weight>
/// - `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. # </weight>
Expand All @@ -394,7 +400,6 @@ pub mod pallet {
) -> DispatchResultWithPostInfo {
ensure_root(origin)?;
T::OnSetCode::set_code(code)?;
Self::deposit_event(Event::CodeUpdated);
Ok(().into())
}

Expand Down Expand Up @@ -1071,6 +1076,18 @@ impl<T: Config> Pallet<T> {
Account::<T>::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::RuntimeUpdated);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will not work for Parachains.

Parachains have a special set_code implementation that will not update :code here directly. It will first queue the update and update it when it is allowed by the relay chain.

Copy link
Contributor Author

@tomusdrw tomusdrw Aug 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where can I find the queue implementation? Any chance the actual storage write can go through some function that would emit the log?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've exposed a helper function to update the storage and emit event + log. The idea is that this function will be used by parachain code implementation and is used by the default implementation of OnSetCode for unit type.

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) {
Expand Down
22 changes: 22 additions & 0 deletions frame/system/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::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();
Expand Down Expand Up @@ -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);
});
}
16 changes: 16 additions & 0 deletions primitives/runtime/src/generic/digest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,14 @@ pub enum DigestItem<Hash> {

/// Some other thing. Unsupported and experimental.
Other(Vec<u8>),

/// 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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally wouldn't mind the more accurate name RuntimeOrHeapPagesUpdated

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RuntimeEnvironmentUpdated

}

/// Available changes trie signals.
Expand Down Expand Up @@ -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<u8>),
/// Runtime code or heap pages updated.
RuntimeUpdated,
}

/// Type of the digest item. Used to gain explicit control over `DigestItem` encoding
Expand All @@ -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
Expand All @@ -225,6 +236,7 @@ impl<Hash> DigestItem<Hash> {
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,
}
}

Expand Down Expand Up @@ -322,6 +334,7 @@ impl<Hash: Decode> Decode for DigestItem<Hash> {
DigestItemType::ChangesTrieSignal =>
Ok(Self::ChangesTrieSignal(Decode::decode(input)?)),
DigestItemType::Other => Ok(Self::Other(Decode::decode(input)?)),
DigestItemType::RuntimeUpdated => Ok(Self::RuntimeUpdated),
}
}
}
Expand Down Expand Up @@ -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::RuntimeUpdated => {
DigestItemType::RuntimeUpdated.encode_to(&mut v);
},
}

v
Expand Down