Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 23 additions & 15 deletions frame/system/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -350,11 +350,13 @@ 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())
}

Expand All @@ -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.
Expand All @@ -373,9 +376,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);
Self::do_set_code(code)?;
Ok(().into())
}

Expand All @@ -384,6 +385,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 @@ -393,8 +395,7 @@ pub mod pallet {
code: Vec<u8>,
) -> DispatchResultWithPostInfo {
ensure_root(origin)?;
T::OnSetCode::set_code(code)?;
Self::deposit_event(Event::CodeUpdated);
Self::do_set_code(code)?;
Ok(().into())
}

Expand Down Expand Up @@ -1068,6 +1069,13 @@ impl<T: Config> Pallet<T> {
Account::<T>::contains_key(who)
}

fn do_set_code(code: Vec<u8>) -> DispatchResult {
T::OnSetCode::set_code(code)?;
Self::deposit_log(generic::DigestItem::RuntimeUpdated);

Copy link
Copy Markdown
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.

@tomusdrw tomusdrw Aug 19, 2021

Copy link
Copy Markdown
Contributor Author

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
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
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 Expand Up @@ -1128,18 +1136,18 @@ impl<T: Config> Pallet<T> {

Pallet::<T>::on_killed_account(who.clone());
Ok(DecRefStatus::Reaped)
},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

needs to run cargo fmt.

}
(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!(
Expand Down Expand Up @@ -1183,12 +1191,12 @@ impl<T: Config> Pallet<T> {
(0, 0) | (1, 0) => {
Pallet::<T>::on_killed_account(who.clone());
DecRefStatus::Reaped
},
}
(x, _) => {
account.sufficients = x - 1;
*maybe_account = Some(account);
DecRefStatus::Exists
},
}
}
} else {
log::error!(
Expand Down Expand Up @@ -1280,7 +1288,7 @@ impl<T: Config> Pallet<T> {
let block_number = Self::block_number();
// Don't populate events on genesis.
if block_number.is_zero() {
return
return;
}

let phase = ExecutionPhase::<T>::get().unwrap_or_default();
Expand Down Expand Up @@ -1534,7 +1542,7 @@ impl<T: Config> Pallet<T> {
err,
);
Event::ExtrinsicFailed(err.error, info)
},
}
});

let next_extrinsic_index = Self::extrinsic_index().unwrap_or_default() + 1u32;
Expand Down Expand Up @@ -1668,10 +1676,10 @@ impl<T: Config> StoredMap<T::AccountId, T::AccountData> for Pallet<T> {
DecRefStatus::Reaped => return Ok(result),
DecRefStatus::Exists => {
// Update value as normal...
},
}
}
} else if !was_providing && !is_providing {
return Ok(result)
return Ok(result);
}
Account::<T>::mutate(k, |a| a.data = some_data.unwrap_or_default());
Ok(result)
Expand All @@ -1687,7 +1695,7 @@ pub fn split_inner<T, R, S>(
Some(inner) => {
let (r, s) = splitter(inner);
(Some(r), Some(s))
},
}
None => (None, None),
}
}
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);
});
}
49 changes: 34 additions & 15 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
Copy Markdown
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
Copy Markdown
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 @@ -310,18 +322,20 @@ impl<Hash: Decode> Decode for DigestItem<Hash> {
DigestItemType::PreRuntime => {
let vals: (ConsensusEngineId, Vec<u8>) = Decode::decode(input)?;
Ok(Self::PreRuntime(vals.0, vals.1))
},
}
DigestItemType::Consensus => {
let vals: (ConsensusEngineId, Vec<u8>) = Decode::decode(input)?;
Ok(Self::Consensus(vals.0, vals.1))
},
}
DigestItemType::Seal => {
let vals: (ConsensusEngineId, Vec<u8>) = 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),
}
}
}
Expand Down Expand Up @@ -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,
}
Expand Down Expand Up @@ -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
Expand Down