From 43b6f1f97c87e5da79c685f0fa7194d50ec6384a Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Mon, 14 Feb 2022 10:11:23 +0100 Subject: [PATCH 1/2] Remove support for the changes trie --- src/executor/host.rs | 46 +++------------ src/executor/runtime_host.rs | 29 ++-------- src/header.rs | 107 +---------------------------------- 3 files changed, 17 insertions(+), 165 deletions(-) diff --git a/src/executor/host.rs b/src/executor/host.rs index bd773924a2..62d590c7fe 100644 --- a/src/executor/host.rs +++ b/src/executor/host.rs @@ -446,9 +446,6 @@ pub enum HostVm { /// Need to provide the trie root of the storage. #[from] ExternalStorageRoot(ExternalStorageRoot), - /// Need to provide the trie root of the changes trie. - #[from] - ExternalStorageChangesRoot(ExternalStorageChangesRoot), /// Need to provide the storage key that follows a specific one. #[from] ExternalStorageNextKey(ExternalStorageNextKey), @@ -500,7 +497,6 @@ impl HostVm { HostVm::ExternalStorageAppend(inner) => inner.inner.into_prototype(), HostVm::ExternalStorageClearPrefix(inner) => inner.inner.into_prototype(), HostVm::ExternalStorageRoot(inner) => inner.inner.into_prototype(), - HostVm::ExternalStorageChangesRoot(inner) => inner.inner.into_prototype(), HostVm::ExternalStorageNextKey(inner) => inner.inner.into_prototype(), HostVm::ExternalOffchainStorageSet(inner) => inner.inner.into_prototype(), HostVm::CallRuntimeVersion(inner) => inner.inner.into_prototype(), @@ -931,8 +927,15 @@ impl ReadyToRun { } } HostFunction::ext_storage_changes_root_version_1 => { - // TODO: there's a parameter - HostVm::ExternalStorageChangesRoot(ExternalStorageChangesRoot { inner: self.inner }) + // The changes trie is an obsolete attempt at having a second trie containing, for + // each storage item, the latest block height where this item has been modified. + // This mechanism has never been enabled in production and the only remaining + // trace of its existence is this host function. + // Writing a SCALE-encoded `None`. + self.inner.alloc_write_and_return_pointer_size( + HostFunction::ext_storage_changes_root_version_1.name(), + iter::once(&[0][..]), + ) } HostFunction::ext_storage_next_key_version_1 => { let (key_ptr, key_size) = expect_pointer_size_raw!(0); @@ -1996,37 +1999,6 @@ impl fmt::Debug for ExternalStorageRoot { } } -/// Must provide the trie root hash of the changes trie. -pub struct ExternalStorageChangesRoot { - inner: Inner, -} - -impl ExternalStorageChangesRoot { - /// Writes the trie root hash to the Wasm VM and prepares it for resume. - // TODO: document why it can be `None` - pub fn resume(self, hash: Option<&[u8; 32]>) -> HostVm { - if let Some(hash) = hash { - // Writing the `Some` of the SCALE-encoded `Option`. - self.inner.alloc_write_and_return_pointer_size( - HostFunction::ext_storage_changes_root_version_1.name(), - iter::once(&[1][..]).chain(iter::once(&hash[..])), - ) - } else { - // Writing a SCALE-encoded `None`. - self.inner.alloc_write_and_return_pointer_size( - HostFunction::ext_storage_changes_root_version_1.name(), - iter::once(&[0][..]), - ) - } - } -} - -impl fmt::Debug for ExternalStorageChangesRoot { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - f.debug_tuple("ExternalStorageChangesRoot").finish() - } -} - /// Must provide the storage key that follows, in lexicographic order, a specific one. pub struct ExternalStorageNextKey { inner: Inner, diff --git a/src/executor/runtime_host.rs b/src/executor/runtime_host.rs index 75ebdd43df..d42c4eca8f 100644 --- a/src/executor/runtime_host.rs +++ b/src/executor/runtime_host.rs @@ -196,12 +196,12 @@ impl StorageGet { /// Returns the key whose value must be passed to [`StorageGet::inject_value`]. pub fn key(&'_ self) -> impl Iterator + '_> + '_ { match &self.inner.vm { - host::HostVm::ExternalStorageGet(req) => either::Left(iter::once(either::Left( - either::Left(either::Left(req.key())), - ))), - host::HostVm::ExternalStorageAppend(req) => either::Left(iter::once(either::Left( - either::Left(either::Right(req.key())), - ))), + host::HostVm::ExternalStorageGet(req) => { + either::Left(iter::once(either::Left(either::Left(req.key())))) + } + host::HostVm::ExternalStorageAppend(req) => { + either::Left(iter::once(either::Left(either::Right(req.key())))) + } host::HostVm::ExternalStorageRoot(_) => { if let calculate_root::RootMerkleValueCalculation::StorageValue(value_request) = @@ -214,10 +214,6 @@ impl StorageGet { } } - host::HostVm::ExternalStorageChangesRoot(_) => either::Left(iter::once(either::Left( - either::Right(&b":changes_trie"[..]), - ))), - // We only create a `StorageGet` if the state is one of the above. _ => unreachable!(), } @@ -270,14 +266,6 @@ impl StorageGet { panic!() } } - host::HostVm::ExternalStorageChangesRoot(req) => { - if value.is_none() { - self.inner.vm = req.resume(None); - } else { - // TODO: this is probably one of the most complicated things to implement - todo!() - } - } // We only create a `StorageGet` if the state is one of the above. _ => unreachable!(), @@ -681,11 +669,6 @@ impl Inner { } } - host::HostVm::ExternalStorageChangesRoot(req) => { - self.vm = req.into(); - return RuntimeHostVm::StorageGet(StorageGet { inner: self }); - } - host::HostVm::ExternalStorageNextKey(req) => { self.vm = req.into(); return RuntimeHostVm::NextKey(NextKey { diff --git a/src/header.rs b/src/header.rs index aff7a8ec33..4298224d82 100644 --- a/src/header.rs +++ b/src/header.rs @@ -593,7 +593,6 @@ impl<'a> DigestRef<'a> { aura_predigest_index = Some(item_num); } DigestItem::AuraPreDigest(_) => return Err(Error::MultipleAuraPreRuntimeDigests), - DigestItem::ChangesTrieRoot(_) => {} DigestItem::AuraConsensus(_) => {} DigestItem::BabePreDigest(_) if babe_predigest_index.is_none() => { babe_predigest_index = Some(item_num); @@ -635,9 +634,7 @@ impl<'a> DigestRef<'a> { has_runtime_environment_updated = true; } DigestItem::BabeSeal(_) => return Err(Error::SealIsntLastItem), - DigestItem::ChangesTrieSignal(_) - | DigestItem::Beefy { .. } - | DigestItem::PolkadotParachain { .. } => {} + DigestItem::Beefy { .. } | DigestItem::PolkadotParachain { .. } => {} } } @@ -684,7 +681,6 @@ impl<'a> DigestRef<'a> { DigestItemRef::AuraPreDigest(_) => { return Err(Error::MultipleAuraPreRuntimeDigests) } - DigestItemRef::ChangesTrieRoot(_) => {} DigestItemRef::AuraConsensus(_) => {} DigestItemRef::BabePreDigest(_) if babe_predigest_index.is_none() => { babe_predigest_index = Some(item_num); @@ -728,9 +724,7 @@ impl<'a> DigestRef<'a> { has_runtime_environment_updated = true; } DigestItemRef::BabeSeal(_) => return Err(Error::SealIsntLastItem), - DigestItemRef::ChangesTrieSignal(_) - | DigestItemRef::Beefy { .. } - | DigestItemRef::PolkadotParachain { .. } => {} + DigestItemRef::Beefy { .. } | DigestItemRef::PolkadotParachain { .. } => {} } } @@ -961,9 +955,6 @@ pub enum DigestItemRef<'a> { GrandpaConsensus(GrandpaConsensusLogRef<'a>), - ChangesTrieRoot(&'a [u8; 32]), - ChangesTrieSignal(ChangesTrieSignal), - /// Item related to the BEEFY algorithm (Mountain Merkle Ranges). Allows proving that a block /// is a child of another. Beefy { @@ -1096,27 +1087,6 @@ impl<'a> DigestItemRef<'a> { ret.extend_from_slice(seal); iter::once(ret) } - DigestItemRef::ChangesTrieSignal(ref changes) => { - let mut ret = vec![7]; - match changes { - ChangesTrieSignal::NewConfiguration(Some(cfg)) => { - ret.extend_from_slice(&[0]); - ret.extend_from_slice(&[1]); - ret.extend_from_slice(&cfg.digest_interval.to_le_bytes()); - ret.extend_from_slice(&cfg.digest_levels.to_le_bytes()); - } - ChangesTrieSignal::NewConfiguration(None) => { - ret.extend_from_slice(&[0]); - ret.extend_from_slice(&[0]); - } - } - iter::once(ret) - } - DigestItemRef::ChangesTrieRoot(data) => { - let mut ret = vec![2]; - ret.extend_from_slice(data); - iter::once(ret) - } DigestItemRef::Beefy { opaque } => { let mut ret = vec![4]; ret.extend_from_slice(b"BEEF"); @@ -1146,8 +1116,6 @@ impl<'a> From<&'a DigestItem> for DigestItemRef<'a> { DigestItem::BabeConsensus(v) => DigestItemRef::BabeConsensus(v.into()), DigestItem::BabeSeal(v) => DigestItemRef::BabeSeal(v), DigestItem::GrandpaConsensus(v) => DigestItemRef::GrandpaConsensus(v.into()), - DigestItem::ChangesTrieRoot(v) => DigestItemRef::ChangesTrieRoot(v), - DigestItem::ChangesTrieSignal(v) => DigestItemRef::ChangesTrieSignal(v.clone()), DigestItem::Beefy { opaque } => DigestItemRef::Beefy { opaque: &*opaque }, DigestItem::PolkadotParachain { opaque } => { DigestItemRef::PolkadotParachain { opaque: &*opaque } @@ -1172,9 +1140,6 @@ pub enum DigestItem { GrandpaConsensus(GrandpaConsensusLog), - ChangesTrieRoot([u8; 32]), - ChangesTrieSignal(ChangesTrieSignal), - /// See [`DigestItemRef::Beefy`]. Beefy { /// Smoldot doesn't interpret the content of the log item at the moment. @@ -1210,8 +1175,6 @@ impl<'a> From> for DigestItem { DigestItem::BabeSeal(seal) } DigestItemRef::GrandpaConsensus(v) => DigestItem::GrandpaConsensus(v.into()), - DigestItemRef::ChangesTrieRoot(v) => DigestItem::ChangesTrieRoot(*v), - DigestItemRef::ChangesTrieSignal(v) => DigestItem::ChangesTrieSignal(v), DigestItemRef::Beefy { opaque } => DigestItem::Beefy { opaque: opaque.to_vec(), }, @@ -1223,42 +1186,6 @@ impl<'a> From> for DigestItem { } } -/// Available changes trie signals. -// TODO: review documentation -#[derive(Debug, PartialEq, Eq, Clone)] -pub enum ChangesTrieSignal { - /// New changes trie configuration is enacted, starting from **next block**. - /// - /// The block that emits this signal will contain changes trie (CT) that covers - /// blocks range [BEGIN; current block], where BEGIN is (order matters): - /// - LAST_TOP_LEVEL_DIGEST_BLOCK+1 if top level digest CT has ever been created - /// using current configuration AND the last top level digest CT has been created - /// at block LAST_TOP_LEVEL_DIGEST_BLOCK; - /// - LAST_CONFIGURATION_CHANGE_BLOCK+1 if there has been CT configuration change - /// before and the last configuration change happened at block - /// LAST_CONFIGURATION_CHANGE_BLOCK; - /// - 1 otherwise. - NewConfiguration(Option), -} - -/// Substrate changes trie configuration. -// TODO: review documentation -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct ChangesTrieConfiguration { - /// Interval (in blocks) at which level1-digests are created. Digests are not - /// created when this is less or equal to 1. - pub digest_interval: u32, - - /// Maximal number of digest levels in hierarchy. 0 means that digests are not - /// created at all (even level1 digests). 1 means only level1-digests are created. - /// 2 means that every digest_interval^2 there will be a level2-digest, and so on. - /// Please ensure that maximum digest interval (i.e. digest_interval^digest_levels) - /// is within `u32` limits. Otherwise you'll never see digests covering such intervals - /// && maximal digests interval will be truncated to the last interval that fits - /// `u32` limits. - pub digest_levels: u32, -} - /// Decodes a single digest log item. On success, returns the item and the data that remains /// after the item. fn decode_item(mut slice: &[u8]) -> Result<(DigestItemRef, &[u8]), Error> { @@ -1288,36 +1215,6 @@ fn decode_item(mut slice: &[u8]) -> Result<(DigestItemRef, &[u8]), Error> { let item = decode_item_from_parts(index, engine_id, content)?; Ok((item, slice)) } - 2 => { - if slice.len() < 32 { - return Err(Error::TooShort); - } - - let hash: &[u8; 32] = TryFrom::try_from(&slice[0..32]).unwrap(); - slice = &slice[32..]; - Ok((DigestItemRef::ChangesTrieRoot(hash), slice)) - } - 7 => { - let (slice, item) = nom::combinator::map( - nom::sequence::preceded( - nom::bytes::complete::tag(&[0u8]), - crate::util::nom_option_decode(nom::combinator::map( - nom::sequence::tuple(( - nom::number::complete::le_u32, - nom::number::complete::le_u32, - )), - |(digest_interval, digest_levels)| ChangesTrieConfiguration { - digest_interval, - digest_levels, - }, - )), - ), - ChangesTrieSignal::NewConfiguration, - )(slice) - .map_err(|_: nom::Err>| Error::DigestItemDecodeError)?; - - Ok((DigestItemRef::ChangesTrieSignal(item), slice)) - } 8 => Ok((DigestItemRef::RuntimeEnvironmentUpdated, slice)), ty => Err(Error::UnknownDigestLogType(ty)), } From 611e69e067002fbeccb360eff78fac9a6aa2d52a Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Mon, 14 Feb 2022 10:22:58 +0100 Subject: [PATCH 2/2] Better docs --- src/executor/host.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/executor/host.rs b/src/executor/host.rs index 62d590c7fe..86f3dcbea2 100644 --- a/src/executor/host.rs +++ b/src/executor/host.rs @@ -929,9 +929,13 @@ impl ReadyToRun { HostFunction::ext_storage_changes_root_version_1 => { // The changes trie is an obsolete attempt at having a second trie containing, for // each storage item, the latest block height where this item has been modified. - // This mechanism has never been enabled in production and the only remaining - // trace of its existence is this host function. - // Writing a SCALE-encoded `None`. + // When this function returns `None`, it indicates that the changes trie is + // disabled. While this function used to be called by the runtimes of + // Westend/Polkadot/Kusama (and maybe others), it has never returned anything else + // but `None`. The entire changes trie mechanism was ultimately removed in + // October 2021. + // This function is no longer called by recent runtimes, but must be preserved for + // backwards compatibility. self.inner.alloc_write_and_return_pointer_size( HostFunction::ext_storage_changes_root_version_1.name(), iter::once(&[0][..]),