From 292b74706ca64e314a67ef0c313c5836b90f0451 Mon Sep 17 00:00:00 2001 From: Piet Wolff Date: Thu, 5 Oct 2023 09:47:31 +0200 Subject: [PATCH 01/24] patch outcome --- .../bin/node-template/runtime/src/lib.rs | 9 + substrate/bin/node/runtime/src/lib.rs | 2 + substrate/decode-all.patch | 862 ++++++++++++++++++ substrate/frame/glutton/src/lib.rs | 5 +- .../procedural/src/pallet/expand/event.rs | 3 +- .../procedural/src/pallet/expand/storage.rs | 33 + .../procedural/src/pallet/parse/storage.rs | 2 +- .../support/src/storage/generator/mod.rs | 6 +- .../frame/support/src/storage/migration.rs | 33 + .../support/src/storage/types/counted_map.rs | 14 +- .../support/src/storage/types/counted_nmap.rs | 6 +- .../frame/support/src/storage/types/mod.rs | 2 +- .../frame/support/src/storage/types/value.rs | 4 +- substrate/frame/support/src/traits.rs | 4 +- .../frame/support/src/traits/try_runtime.rs | 437 +++++++++ .../call_argument_invalid_bound.stderr | 2 +- .../call_argument_invalid_bound_2.stderr | 2 +- .../call_argument_invalid_bound_3.stderr | 2 +- .../pallet_ui/event_field_not_member.stderr | 2 +- .../storage_info_unsatisfied_nmap.stderr | 4 +- 20 files changed, 1409 insertions(+), 25 deletions(-) create mode 100644 substrate/decode-all.patch diff --git a/substrate/bin/node-template/runtime/src/lib.rs b/substrate/bin/node-template/runtime/src/lib.rs index 4653b49bf2c3..52bfc9c95f19 100644 --- a/substrate/bin/node-template/runtime/src/lib.rs +++ b/substrate/bin/node-template/runtime/src/lib.rs @@ -312,6 +312,14 @@ pub type SignedExtra = ( pallet_transaction_payment::ChargeTransactionPayment, ); +/// All migrations of the runtime, aside from the ones declared in the pallets. +/// +/// This can be a tuple of types, each implementing `OnRuntimeUpgrade`. Add other migration types +/// before `EnsureStateDecodes` as needed -- this is only for testing, and +// should come last. +#[allow(unused_parens)] +type Migrations = (frame_support::migration::EnsureStateDecodes); + /// Unchecked extrinsic type as expected by this runtime. pub type UncheckedExtrinsic = generic::UncheckedExtrinsic; @@ -324,6 +332,7 @@ pub type Executive = frame_executive::Executive< frame_system::ChainContext, Runtime, AllPalletsWithSystem, + Migrations, >; #[cfg(feature = "runtime-benchmarks")] diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index f018639b732e..1a6f9d0c48a5 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -2147,6 +2147,8 @@ type Migrations = ( pallet_nomination_pools::migration::v2::MigrateToV2, pallet_alliance::migration::Migration, pallet_contracts::Migration, + // This should always be the last migration item. + frame_support::storage::migration::EnsureStateDecodes, ); type EventRecord = frame_system::EventRecord< diff --git a/substrate/decode-all.patch b/substrate/decode-all.patch new file mode 100644 index 000000000000..8c9d4ba2e0eb --- /dev/null +++ b/substrate/decode-all.patch @@ -0,0 +1,862 @@ +diff --git a/bin/node-template/runtime/src/lib.rs b/bin/node-template/runtime/src/lib.rs +index 133bae7ae7..216be9588b 100644 +--- a/bin/node-template/runtime/src/lib.rs ++++ b/bin/node-template/runtime/src/lib.rs +@@ -311,14 +311,6 @@ pub type SignedExtra = ( + pallet_transaction_payment::ChargeTransactionPayment, + ); + +-/// All migrations of the runtime, aside from the ones declared in the pallets. +-/// +-/// This can be a tuple of types, each implementing `OnRuntimeUpgrade`. Add other migration types +-/// before `EnsureStateDecodes` as needed -- this is only for testing, and +-// should come last. +-#[allow(unused_parens)] +-type Migrations = (frame_support::migration::EnsureStateDecodes); +- + /// Unchecked extrinsic type as expected by this runtime. + pub type UncheckedExtrinsic = + generic::UncheckedExtrinsic; +@@ -331,7 +323,6 @@ pub type Executive = frame_executive::Executive< + frame_system::ChainContext, + Runtime, + AllPalletsWithSystem, +- Migrations, + >; + + #[cfg(feature = "runtime-benchmarks")] +diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs +index dd21c53397..4f34e4ecd8 100644 +--- a/bin/node/runtime/src/lib.rs ++++ b/bin/node/runtime/src/lib.rs +@@ -2137,8 +2137,6 @@ type Migrations = ( + pallet_nomination_pools::migration::v2::MigrateToV2, + pallet_alliance::migration::Migration, + pallet_contracts::Migration, +- // This should always be the last migration item. +- frame_support::storage::migration::EnsureStateDecodes, + ); + + type EventRecord = frame_system::EventRecord< +diff --git a/frame/glutton/src/lib.rs b/frame/glutton/src/lib.rs +index 176cd566d9..c76cc30017 100644 +--- a/frame/glutton/src/lib.rs ++++ b/frame/glutton/src/lib.rs +@@ -21,8 +21,9 @@ + //! + //! # Glutton Pallet + //! +-//! Pallet that consumes `ref_time` and `proof_size` of a block. Based on the `Compute` and +-//! `Storage` parameters the pallet consumes the adequate amount of weight. ++//! Pallet that consumes `ref_time` and `proof_size` of a block. Based on the ++//! `Compute` and `Storage` parameters the pallet consumes the adequate amount ++//! of weight. + + #![deny(missing_docs)] + #![cfg_attr(not(feature = "std"), no_std)] +diff --git a/frame/support/procedural/src/pallet/expand/event.rs b/frame/support/procedural/src/pallet/expand/event.rs +index 2713f45fc3..fbb699b4d4 100644 +--- a/frame/support/procedural/src/pallet/expand/event.rs ++++ b/frame/support/procedural/src/pallet/expand/event.rs +@@ -127,12 +127,11 @@ pub fn expand_event(def: &mut Def) -> proc_macro2::TokenStream { + let trait_use_gen = &def.trait_use_generics(event.attr_span); + let type_impl_gen = &def.type_impl_generics(event.attr_span); + let type_use_gen = &def.type_use_generics(event.attr_span); +- let pallet_ident = &def.pallet_struct.pallet; + + let PalletEventDepositAttr { fn_vis, fn_span, .. } = deposit_event; + + quote::quote_spanned!(*fn_span => +- impl<#type_impl_gen> #pallet_ident<#type_use_gen> #completed_where_clause { ++ impl<#type_impl_gen> Pallet<#type_use_gen> #completed_where_clause { + #fn_vis fn deposit_event(event: Event<#event_use_gen>) { + let event = < + ::RuntimeEvent as +diff --git a/frame/support/procedural/src/pallet/expand/storage.rs b/frame/support/procedural/src/pallet/expand/storage.rs +index 2e9c4ba6d3..1a941f6cb3 100644 +--- a/frame/support/procedural/src/pallet/expand/storage.rs ++++ b/frame/support/procedural/src/pallet/expand/storage.rs +@@ -780,43 +780,12 @@ pub fn expand_storages(def: &mut Def) -> proc_macro2::TokenStream { + ) + }); + +- // aggregated where clause of all storage types and the whole pallet. + let mut where_clauses = vec![&def.config.where_clause]; + where_clauses.extend(def.storages.iter().map(|storage| &storage.where_clause)); + let completed_where_clause = super::merge_where_clauses(&where_clauses); + let type_impl_gen = &def.type_impl_generics(proc_macro2::Span::call_site()); + let type_use_gen = &def.type_use_generics(proc_macro2::Span::call_site()); + +- let try_decode_entire_state = { +- let storage_names = def +- .storages +- .iter() +- .filter_map(|storage| { +- if storage.cfg_attrs.is_empty() { +- let ident = &storage.ident; +- let gen = &def.type_use_generics(storage.attr_span); +- Some(quote::quote_spanned!(storage.attr_span => #ident<#gen> )) +- } else { +- None +- } +- }) +- .collect::>(); +- +- quote::quote!( +- #[cfg(feature = "try-runtime")] +- impl<#type_impl_gen> #frame_support::traits::TryDecodeEntireStorage +- for #pallet_ident<#type_use_gen> #completed_where_clause +- { +- fn try_decode_entire_state() -> Result { +- // simply delegate impl to a tuple of all storage items we have. +- // +- // NOTE: for now, we have to exclude storage items that are feature gated. +- <( #( #storage_names ),*) as frame_support::traits::TryDecodeEntireStorage>::try_decode_entire_state() +- } +- } +- ) +- }; +- + quote::quote!( + impl<#type_impl_gen> #pallet_ident<#type_use_gen> + #completed_where_clause +@@ -842,7 +811,5 @@ pub fn expand_storages(def: &mut Def) -> proc_macro2::TokenStream { + #( #getters )* + #( #prefix_structs )* + #( #on_empty_structs )* +- +- #try_decode_entire_state + ) + } +diff --git a/frame/support/procedural/src/pallet/parse/storage.rs b/frame/support/procedural/src/pallet/parse/storage.rs +index d1c7ba2e5e..3a0ec47471 100644 +--- a/frame/support/procedural/src/pallet/parse/storage.rs ++++ b/frame/support/procedural/src/pallet/parse/storage.rs +@@ -151,7 +151,7 @@ pub enum QueryKind { + /// `type MyStorage = StorageValue` + /// The keys and values types are parsed in order to get metadata + pub struct StorageDef { +- /// The index of storage item in pallet module. ++ /// The index of error item in pallet module. + pub index: usize, + /// Visibility of the storage type. + pub vis: syn::Visibility, +diff --git a/frame/support/src/storage/generator/mod.rs b/frame/support/src/storage/generator/mod.rs +index 2b2abdc2e8..bac9f642e3 100644 +--- a/frame/support/src/storage/generator/mod.rs ++++ b/frame/support/src/storage/generator/mod.rs +@@ -24,10 +24,10 @@ + //! + //! This is internal api and is subject to change. + +-pub(crate) mod double_map; ++mod double_map; + pub(crate) mod map; +-pub(crate) mod nmap; +-pub(crate) mod value; ++mod nmap; ++mod value; + + pub use double_map::StorageDoubleMap; + pub use map::StorageMap; +diff --git a/frame/support/src/storage/migration.rs b/frame/support/src/storage/migration.rs +index e581c8a00a..568c475bdc 100644 +--- a/frame/support/src/storage/migration.rs ++++ b/frame/support/src/storage/migration.rs +@@ -385,39 +385,6 @@ pub fn move_prefix(from_prefix: &[u8], to_prefix: &[u8]) { + } + } + +-/// A phony migration that does nothing, except executing `TryDecodeEntireStorage` on +-/// `post_upgrade`, which implies it is only available if `try-state` feature is used. +-/// +-/// This can be used typically in the top level runtime, where `AllPallets` typically comes from +-/// `construct_runtime!`. +-pub struct EnsureStateDecodes(sp_std::marker::PhantomData); +- +-#[cfg(not(feature = "try-runtime"))] +-impl crate::traits::OnRuntimeUpgrade for EnsureStateDecodes { +- fn on_runtime_upgrade() -> crate::weights::Weight { +- Default::default() +- } +-} +- +-#[cfg(feature = "try-runtime")] +-impl crate::traits::OnRuntimeUpgrade +- for EnsureStateDecodes +-{ +- fn on_runtime_upgrade() -> sp_weights::Weight { +- Default::default() +- } +- +- fn post_upgrade(_: Vec) -> Result<(), sp_runtime::TryRuntimeError> { +- let decoded = AllPallets::try_decode_entire_state()?; +- crate::log::info!( +- target: crate::LOG_TARGET, +- "decoded the entire state, total size = {} bytes", +- decoded +- ); +- Ok(()) +- } +-} +- + #[cfg(test)] + mod tests { + use super::{ +diff --git a/frame/support/src/storage/types/counted_map.rs b/frame/support/src/storage/types/counted_map.rs +index 4a022b4403..5b750a7409 100644 +--- a/frame/support/src/storage/types/counted_map.rs ++++ b/frame/support/src/storage/types/counted_map.rs +@@ -74,11 +74,7 @@ impl MapWrapper + type Map = StorageMap; + } + +-/// The numeric counter type. +-pub type Counter = u32; +- +-type CounterFor

= +- StorageValue<

::CounterPrefix, Counter, ValueQuery>; ++type CounterFor

= StorageValue<

::CounterPrefix, u32, ValueQuery>; + + /// On removal logic for updating counter while draining upon some prefix with + /// [`crate::storage::PrefixIterator`]. +@@ -382,14 +378,14 @@ where + /// can be very heavy, so use with caution. + /// + /// Returns the number of items in the map which is used to set the counter. +- pub fn initialize_counter() -> Counter { +- let count = Self::iter_values().count() as Counter; ++ pub fn initialize_counter() -> u32 { ++ let count = Self::iter_values().count() as u32; + CounterFor::::set(count); + count + } + + /// Return the count. +- pub fn count() -> Counter { ++ pub fn count() -> u32 { + CounterFor::::get() + } + } +@@ -1166,7 +1162,7 @@ mod test { + StorageEntryMetadataIR { + name: "counter_for_foo", + modifier: StorageEntryModifierIR::Default, +- ty: StorageEntryTypeIR::Plain(scale_info::meta_type::()), ++ ty: StorageEntryTypeIR::Plain(scale_info::meta_type::()), + default: vec![0, 0, 0, 0], + docs: if cfg!(feature = "no-metadata-docs") { + vec![] +diff --git a/frame/support/src/storage/types/counted_nmap.rs b/frame/support/src/storage/types/counted_nmap.rs +index 1dc8af22ae..7dbcb74f00 100644 +--- a/frame/support/src/storage/types/counted_nmap.rs ++++ b/frame/support/src/storage/types/counted_nmap.rs +@@ -71,10 +71,8 @@ impl MapWrapper + type Map = StorageNMap; + } + +-type Counter = super::counted_map::Counter; +- + type CounterFor

= +- StorageValue<

::CounterPrefix, Counter, ValueQuery>; ++ StorageValue<

::CounterPrefix, u32, ValueQuery>; + + /// On removal logic for updating counter while draining upon some prefix with + /// [`crate::storage::PrefixIterator`]. +@@ -431,7 +429,7 @@ where + } + + /// Return the count. +- pub fn count() -> Counter { ++ pub fn count() -> u32 { + CounterFor::::get() + } + } +diff --git a/frame/support/src/storage/types/mod.rs b/frame/support/src/storage/types/mod.rs +index 3d8d53bb16..c7f2557099 100644 +--- a/frame/support/src/storage/types/mod.rs ++++ b/frame/support/src/storage/types/mod.rs +@@ -30,7 +30,7 @@ mod map; + mod nmap; + mod value; + +-pub use counted_map::{CountedStorageMap, CountedStorageMapInstance, Counter}; ++pub use counted_map::{CountedStorageMap, CountedStorageMapInstance}; + pub use counted_nmap::{CountedStorageNMap, CountedStorageNMapInstance}; + pub use double_map::StorageDoubleMap; + pub use key::{ +diff --git a/frame/support/src/storage/types/value.rs b/frame/support/src/storage/types/value.rs +index eaf0a841a4..3c7f24715a 100644 +--- a/frame/support/src/storage/types/value.rs ++++ b/frame/support/src/storage/types/value.rs +@@ -23,7 +23,7 @@ use crate::{ + types::{OptionQuery, QueryKindTrait, StorageEntryMetadataBuilder}, + StorageAppend, StorageDecodeLength, StorageTryAppend, + }, +- traits::{Get, GetDefault, StorageInfo, StorageInstance}, ++ traits::{GetDefault, StorageInfo, StorageInstance}, + }; + use codec::{Decode, Encode, EncodeLike, FullCodec, MaxEncodedLen}; + use sp_arithmetic::traits::SaturatedConversion; +@@ -46,7 +46,7 @@ where + Prefix: StorageInstance, + Value: FullCodec, + QueryKind: QueryKindTrait, +- OnEmpty: Get + 'static, ++ OnEmpty: crate::traits::Get + 'static, + { + type Query = QueryKind::Query; + fn module_prefix() -> &'static [u8] { +diff --git a/frame/support/src/traits.rs b/frame/support/src/traits.rs +index eb6527d38a..f669046f85 100644 +--- a/frame/support/src/traits.rs ++++ b/frame/support/src/traits.rs +@@ -125,6 +125,4 @@ pub use tx_pause::{TransactionPause, TransactionPauseError}; + #[cfg(feature = "try-runtime")] + mod try_runtime; + #[cfg(feature = "try-runtime")] +-pub use try_runtime::{ +- Select as TryStateSelect, TryDecodeEntireStorage, TryState, UpgradeCheckSelect, +-}; ++pub use try_runtime::{Select as TryStateSelect, TryState, UpgradeCheckSelect}; +diff --git a/frame/support/src/traits/try_runtime.rs b/frame/support/src/traits/try_runtime.rs +index 130e3f1672..31aebeeb4d 100644 +--- a/frame/support/src/traits/try_runtime.rs ++++ b/frame/support/src/traits/try_runtime.rs +@@ -17,19 +17,8 @@ + + //! Try-runtime specific traits and types. + +-use super::StorageInstance; +-use crate::{ +- storage::types::{ +- CountedStorageMapInstance, CountedStorageNMapInstance, Counter, KeyGenerator, +- QueryKindTrait, +- }, +- traits::{PartialStorageInfoTrait, StorageInfo}, +- StorageHasher, +-}; +-use codec::{Decode, DecodeAll, FullCodec}; + use impl_trait_for_tuples::impl_for_tuples; + use sp_arithmetic::traits::AtLeast32BitUnsigned; +-use sp_core::Get; + use sp_runtime::TryRuntimeError; + use sp_std::prelude::*; + +@@ -54,203 +43,6 @@ impl Default for Select { + } + } + +-/// Decode the entire data under the given storage type. +-/// +-/// For values, this is trivial. For all kinds of maps, it should decode all the values associated +-/// with all keys existing in the map. +-/// +-/// Tuple implementations are provided and simply decode each type in the tuple, summing up the +-/// decoded bytes if `Ok(_)`. +-pub trait TryDecodeEntireStorage { +- /// Decode the entire data under the given storage, returning `Ok(bytes_decoded)` if success. +- fn try_decode_entire_state() -> Result; +-} +- +-#[cfg_attr(all(not(feature = "tuples-96"), not(feature = "tuples-128")), impl_for_tuples(64))] +-#[cfg_attr(all(feature = "tuples-96", not(feature = "tuples-128")), impl_for_tuples(96))] +-#[cfg_attr(feature = "tuples-128", impl_for_tuples(128))] +-impl TryDecodeEntireStorage for Tuple { +- fn try_decode_entire_state() -> Result { +- let mut len = 0usize; +- for_tuples!( #( len = len.saturating_add(Tuple::try_decode_entire_state()?); )* ); +- Ok(len) +- } +-} +- +-/// Decode all the values based on the prefix of `info` to `V`. +-/// +-/// Basically, it decodes and sums up all the values who's key start with `info.prefix`. For values, +-/// this would be the value itself. For all sorts of maps, this should be all map items in the +-/// absence of key collision. +-fn decode_storage_info(info: StorageInfo) -> Result { +- let mut next_key = info.prefix.clone(); +- let mut decoded = 0; +- +- let decode_key = |key: &[u8]| match sp_io::storage::get(key) { +- None => Ok(0), +- Some(bytes) => { +- let len = bytes.len(); +- let _ = ::decode_all(&mut bytes.as_ref()).map_err(|_| { +- log::error!(target: crate::LOG_TARGET, "failed to decoded {:?}", info,); +- "failed to decode value under existing key" +- })?; +- Ok::(len) +- }, +- }; +- +- decoded += decode_key(&next_key)?; +- loop { +- match sp_io::storage::next_key(&next_key) { +- Some(key) if key.starts_with(&info.prefix) => { +- decoded += decode_key(&key)?; +- next_key = key; +- }, +- _ => break, +- } +- } +- +- Ok(decoded) +-} +- +-impl TryDecodeEntireStorage +- for crate::storage::types::StorageValue +-where +- Prefix: StorageInstance, +- Value: FullCodec, +- QueryKind: QueryKindTrait, +- OnEmpty: Get + 'static, +-{ +- fn try_decode_entire_state() -> Result { +- let info = Self::partial_storage_info() +- .first() +- .cloned() +- .expect("Value has only one storage info; qed"); +- decode_storage_info::(info) +- } +-} +- +-impl TryDecodeEntireStorage +- for crate::storage::types::StorageMap +-where +- Prefix: StorageInstance, +- Hasher: StorageHasher, +- Key: FullCodec, +- Value: FullCodec, +- QueryKind: QueryKindTrait, +- OnEmpty: Get + 'static, +- MaxValues: Get>, +-{ +- fn try_decode_entire_state() -> Result { +- let info = Self::partial_storage_info() +- .first() +- .cloned() +- .expect("Map has only one storage info; qed"); +- decode_storage_info::(info) +- } +-} +- +-impl TryDecodeEntireStorage +- for crate::storage::types::CountedStorageMap< +- Prefix, +- Hasher, +- Key, +- Value, +- QueryKind, +- OnEmpty, +- MaxValues, +- > where +- Prefix: CountedStorageMapInstance, +- Hasher: StorageHasher, +- Key: FullCodec, +- Value: FullCodec, +- QueryKind: QueryKindTrait, +- OnEmpty: Get + 'static, +- MaxValues: Get>, +-{ +- fn try_decode_entire_state() -> Result { +- let (map_info, counter_info) = match &Self::partial_storage_info()[..] { +- [a, b] => (a.clone(), b.clone()), +- _ => panic!("Counted map has two storage info items; qed"), +- }; +- let mut decoded = decode_storage_info::(counter_info)?; +- decoded += decode_storage_info::(map_info)?; +- Ok(decoded) +- } +-} +- +-impl +- TryDecodeEntireStorage +- for crate::storage::types::StorageDoubleMap< +- Prefix, +- Hasher1, +- Key1, +- Hasher2, +- Key2, +- Value, +- QueryKind, +- OnEmpty, +- MaxValues, +- > where +- Prefix: StorageInstance, +- Hasher1: StorageHasher, +- Key1: FullCodec, +- Hasher2: StorageHasher, +- Key2: FullCodec, +- Value: FullCodec, +- QueryKind: QueryKindTrait, +- OnEmpty: Get + 'static, +- MaxValues: Get>, +-{ +- fn try_decode_entire_state() -> Result { +- let info = Self::partial_storage_info() +- .first() +- .cloned() +- .expect("Double-map has only one storage info; qed"); +- decode_storage_info::(info) +- } +-} +- +-impl TryDecodeEntireStorage +- for crate::storage::types::StorageNMap +-where +- Prefix: StorageInstance, +- Key: KeyGenerator, +- Value: FullCodec, +- QueryKind: QueryKindTrait, +- OnEmpty: Get + 'static, +- MaxValues: Get>, +-{ +- fn try_decode_entire_state() -> Result { +- let info = Self::partial_storage_info() +- .first() +- .cloned() +- .expect("N-map has only one storage info; qed"); +- decode_storage_info::(info) +- } +-} +- +-impl TryDecodeEntireStorage +- for crate::storage::types::CountedStorageNMap +-where +- Prefix: CountedStorageNMapInstance, +- Key: KeyGenerator, +- Value: FullCodec, +- QueryKind: QueryKindTrait, +- OnEmpty: Get + 'static, +- MaxValues: Get>, +-{ +- fn try_decode_entire_state() -> Result { +- let (map_info, counter_info) = match &Self::partial_storage_info()[..] { +- [a, b] => (a.clone(), b.clone()), +- _ => panic!("Counted NMap has two storage info items; qed"), +- }; +- +- let mut decoded = decode_storage_info::(counter_info)?; +- decoded += decode_storage_info::(map_info)?; +- Ok(decoded) +- } +-} +- + impl sp_std::fmt::Debug for Select { + fn fmt(&self, f: &mut sp_std::fmt::Formatter<'_>) -> sp_std::fmt::Result { + match self { +@@ -395,232 +187,3 @@ impl TryState { +- struct $name; +- impl StorageInstance for $name { +- fn pallet_prefix() -> &'static str { +- "test_pallet" +- } +- const STORAGE_PREFIX: &'static str = stringify!($name); +- } +- }; +- } +- +- build_prefix!(ValuePrefix); +- type Value = types::StorageValue; +- +- build_prefix!(MapPrefix); +- type Map = types::StorageMap; +- +- build_prefix!(CMapCounterPrefix); +- build_prefix!(CMapPrefix); +- impl CountedStorageMapInstance for CMapPrefix { +- type CounterPrefix = CMapCounterPrefix; +- } +- type CMap = types::CountedStorageMap; +- +- build_prefix!(DMapPrefix); +- type DMap = types::StorageDoubleMap; +- +- build_prefix!(NMapPrefix); +- type NMap = types::StorageNMap, Key), u128>; +- +- build_prefix!(CountedNMapCounterPrefix); +- build_prefix!(CountedNMapPrefix); +- impl CountedStorageNMapInstance for CountedNMapPrefix { +- type CounterPrefix = CountedNMapCounterPrefix; +- } +- type CNMap = types::CountedStorageNMap, Key), u128>; +- +- #[test] +- fn try_decode_entire_state_value_works() { +- sp_io::TestExternalities::new_empty().execute_with(|| { +- assert_eq!(Value::try_decode_entire_state(), Ok(0)); +- +- Value::put(42); +- assert_eq!(Value::try_decode_entire_state(), Ok(4)); +- +- Value::kill(); +- assert_eq!(Value::try_decode_entire_state(), Ok(0)); +- +- // two bytes, cannot be decoded into u32. +- sp_io::storage::set(&Value::hashed_key(), &[0u8, 1]); +- assert!(Value::try_decode_entire_state().is_err()); +- }) +- } +- +- #[test] +- fn try_decode_entire_state_map_works() { +- sp_io::TestExternalities::new_empty().execute_with(|| { +- assert_eq!(Map::try_decode_entire_state(), Ok(0)); +- +- Map::insert(0, 42); +- assert_eq!(Map::try_decode_entire_state(), Ok(4)); +- +- Map::insert(0, 42); +- assert_eq!(Map::try_decode_entire_state(), Ok(4)); +- +- Map::insert(1, 42); +- assert_eq!(Map::try_decode_entire_state(), Ok(8)); +- +- Map::remove(0); +- assert_eq!(Map::try_decode_entire_state(), Ok(4)); +- +- // two bytes, cannot be decoded into u32. +- sp_io::storage::set(&Map::hashed_key_for(2), &[0u8, 1]); +- assert!(Map::try_decode_entire_state().is_err()); +- }) +- } +- +- #[test] +- fn try_decode_entire_state_counted_map_works() { +- sp_io::TestExternalities::new_empty().execute_with(|| { +- // counter is not even initialized; +- assert_eq!(CMap::try_decode_entire_state(), Ok(0 + 0)); +- +- let counter = 4; +- let value_size = std::mem::size_of::(); +- +- CMap::insert(0, 42); +- assert_eq!(CMap::try_decode_entire_state(), Ok(value_size + counter)); +- +- CMap::insert(0, 42); +- assert_eq!(CMap::try_decode_entire_state(), Ok(value_size + counter)); +- +- CMap::insert(1, 42); +- assert_eq!(CMap::try_decode_entire_state(), Ok(value_size * 2 + counter)); +- +- CMap::remove(0); +- assert_eq!(CMap::try_decode_entire_state(), Ok(value_size + counter)); +- +- // counter is cleared again. +- let _ = CMap::clear(u32::MAX, None); +- assert_eq!(CMap::try_decode_entire_state(), Ok(0 + 0)); +- +- // 1 bytes, cannot be decoded into u16. +- sp_io::storage::set(&CMap::hashed_key_for(2), &[0u8]); +- assert!(CMap::try_decode_entire_state().is_err()); +- }) +- } +- +- #[test] +- fn try_decode_entire_state_double_works() { +- sp_io::TestExternalities::new_empty().execute_with(|| { +- assert_eq!(DMap::try_decode_entire_state(), Ok(0)); +- +- DMap::insert(0, 0, 42); +- assert_eq!(DMap::try_decode_entire_state(), Ok(4)); +- +- DMap::insert(0, 0, 42); +- assert_eq!(DMap::try_decode_entire_state(), Ok(4)); +- +- DMap::insert(0, 1, 42); +- assert_eq!(DMap::try_decode_entire_state(), Ok(8)); +- +- DMap::insert(1, 0, 42); +- assert_eq!(DMap::try_decode_entire_state(), Ok(12)); +- +- DMap::remove(0, 0); +- assert_eq!(DMap::try_decode_entire_state(), Ok(8)); +- +- // two bytes, cannot be decoded into u32. +- sp_io::storage::set(&DMap::hashed_key_for(1, 1), &[0u8, 1]); +- assert!(DMap::try_decode_entire_state().is_err()); +- }) +- } +- +- #[test] +- fn try_decode_entire_state_n_map_works() { +- sp_io::TestExternalities::new_empty().execute_with(|| { +- assert_eq!(NMap::try_decode_entire_state(), Ok(0)); +- +- let value_size = std::mem::size_of::(); +- +- NMap::insert((0u8, 0), 42); +- assert_eq!(NMap::try_decode_entire_state(), Ok(value_size)); +- +- NMap::insert((0, 0), 42); +- assert_eq!(NMap::try_decode_entire_state(), Ok(value_size)); +- +- NMap::insert((0, 1), 42); +- assert_eq!(NMap::try_decode_entire_state(), Ok(value_size * 2)); +- +- NMap::insert((1, 0), 42); +- assert_eq!(NMap::try_decode_entire_state(), Ok(value_size * 3)); +- +- NMap::remove((0, 0)); +- assert_eq!(NMap::try_decode_entire_state(), Ok(value_size * 2)); +- +- // two bytes, cannot be decoded into u128. +- sp_io::storage::set(&NMap::hashed_key_for((1, 1)), &[0u8, 1]); +- assert!(NMap::try_decode_entire_state().is_err()); +- }) +- } +- +- #[test] +- fn try_decode_entire_state_counted_n_map_works() { +- sp_io::TestExternalities::new_empty().execute_with(|| { +- sp_io::TestExternalities::new_empty().execute_with(|| { +- assert_eq!(NMap::try_decode_entire_state(), Ok(0)); +- +- let value_size = std::mem::size_of::(); +- let counter = 4; +- +- CNMap::insert((0u8, 0), 42); +- assert_eq!(CNMap::try_decode_entire_state(), Ok(value_size + counter)); +- +- CNMap::insert((0, 0), 42); +- assert_eq!(CNMap::try_decode_entire_state(), Ok(value_size + counter)); +- +- CNMap::insert((0, 1), 42); +- assert_eq!(CNMap::try_decode_entire_state(), Ok(value_size * 2 + counter)); +- +- CNMap::insert((1, 0), 42); +- assert_eq!(CNMap::try_decode_entire_state(), Ok(value_size * 3 + counter)); +- +- CNMap::remove((0, 0)); +- assert_eq!(CNMap::try_decode_entire_state(), Ok(value_size * 2 + counter)); +- +- // two bytes, cannot be decoded into u128. +- sp_io::storage::set(&CNMap::hashed_key_for((1, 1)), &[0u8, 1]); +- assert!(CNMap::try_decode_entire_state().is_err()); +- }) +- }) +- } +- +- #[test] +- fn extra_bytes_are_rejected() { +- sp_io::TestExternalities::new_empty().execute_with(|| { +- assert_eq!(Map::try_decode_entire_state(), Ok(0)); +- +- // 6bytes, too many to fit in u32, should be rejected. +- sp_io::storage::set(&Map::hashed_key_for(2), &[0u8, 1, 3, 4, 5, 6]); +- assert!(Map::try_decode_entire_state().is_err()); +- }) +- } +- +- #[test] +- fn try_decode_entire_state_tuple_of_storage_works() { +- sp_io::TestExternalities::new_empty().execute_with(|| { +- assert_eq!(<(Value, Map) as TryDecodeEntireStorage>::try_decode_entire_state(), Ok(0)); +- +- Value::put(42); +- assert_eq!(<(Value, Map) as TryDecodeEntireStorage>::try_decode_entire_state(), Ok(4)); +- +- Map::insert(0, 42); +- assert_eq!(<(Value, Map) as TryDecodeEntireStorage>::try_decode_entire_state(), Ok(8)); +- }); +- } +-} +diff --git a/frame/support/test/tests/pallet_ui/call_argument_invalid_bound.stderr b/frame/support/test/tests/pallet_ui/call_argument_invalid_bound.stderr +index cc8c4fda76..d10bf13590 100644 +--- a/frame/support/test/tests/pallet_ui/call_argument_invalid_bound.stderr ++++ b/frame/support/test/tests/pallet_ui/call_argument_invalid_bound.stderr +@@ -19,7 +19,7 @@ error[E0277]: `::Bar` doesn't implement `std::fmt::Debug` + | + = help: the trait `std::fmt::Debug` is not implemented for `::Bar` + = note: required for `&::Bar` to implement `std::fmt::Debug` +- = note: required for the cast from `&&::Bar` to `&dyn std::fmt::Debug` ++ = note: required for the cast from `&::Bar` to the object type `dyn std::fmt::Debug` + + error[E0277]: the trait bound `::Bar: Clone` is not satisfied + --> tests/pallet_ui/call_argument_invalid_bound.rs:21:36 +diff --git a/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_2.stderr b/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_2.stderr +index b8d0bbf347..7173cdcd47 100644 +--- a/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_2.stderr ++++ b/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_2.stderr +@@ -19,7 +19,7 @@ error[E0277]: `::Bar` doesn't implement `std::fmt::Debug` + | + = help: the trait `std::fmt::Debug` is not implemented for `::Bar` + = note: required for `&::Bar` to implement `std::fmt::Debug` +- = note: required for the cast from `&&::Bar` to `&dyn std::fmt::Debug` ++ = note: required for the cast from `&::Bar` to the object type `dyn std::fmt::Debug` + + error[E0277]: the trait bound `::Bar: Clone` is not satisfied + --> tests/pallet_ui/call_argument_invalid_bound_2.rs:21:36 +diff --git a/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_3.stderr b/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_3.stderr +index 158d0c7638..4cbed37096 100644 +--- a/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_3.stderr ++++ b/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_3.stderr +@@ -20,7 +20,7 @@ error[E0277]: `Bar` doesn't implement `std::fmt::Debug` + = help: the trait `std::fmt::Debug` is not implemented for `Bar` + = note: add `#[derive(Debug)]` to `Bar` or manually `impl std::fmt::Debug for Bar` + = note: required for `&Bar` to implement `std::fmt::Debug` +- = note: required for the cast from `&&Bar` to `&dyn std::fmt::Debug` ++ = note: required for the cast from `&Bar` to the object type `dyn std::fmt::Debug` + help: consider annotating `Bar` with `#[derive(Debug)]` + | + 17 + #[derive(Debug)] +diff --git a/frame/support/test/tests/pallet_ui/event_field_not_member.stderr b/frame/support/test/tests/pallet_ui/event_field_not_member.stderr +index d853406c95..1161f4a190 100644 +--- a/frame/support/test/tests/pallet_ui/event_field_not_member.stderr ++++ b/frame/support/test/tests/pallet_ui/event_field_not_member.stderr +@@ -18,4 +18,4 @@ error[E0277]: `::Bar` doesn't implement `std::fmt::Debug` + | + = help: the trait `std::fmt::Debug` is not implemented for `::Bar` + = note: required for `&::Bar` to implement `std::fmt::Debug` +- = note: required for the cast from `&&::Bar` to `&dyn std::fmt::Debug` ++ = note: required for the cast from `&::Bar` to the object type `dyn std::fmt::Debug` +diff --git a/frame/support/test/tests/pallet_ui/inherent_check_inner_span.stderr b/frame/support/test/tests/pallet_ui/inherent_check_inner_span.stderr +index 4acf57fb3e..bc34c55241 100644 +--- a/frame/support/test/tests/pallet_ui/inherent_check_inner_span.stderr ++++ b/frame/support/test/tests/pallet_ui/inherent_check_inner_span.stderr +@@ -1,11 +1,11 @@ + error[E0046]: not all trait items implemented, missing: `Call`, `Error`, `INHERENT_IDENTIFIER`, `create_inherent`, `is_inherent` +- --> tests/pallet_ui/inherent_check_inner_span.rs:19:2 ++ --> $DIR/inherent_check_inner_span.rs:19:2 + | + 19 | impl ProvideInherent for Pallet {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing `Call`, `Error`, `INHERENT_IDENTIFIER`, `create_inherent`, `is_inherent` in implementation + | +- = help: implement the missing item: `type Call = /* Type */;` +- = help: implement the missing item: `type Error = /* Type */;` ++ = help: implement the missing item: `type Call = Type;` ++ = help: implement the missing item: `type Error = Type;` + = help: implement the missing item: `const INHERENT_IDENTIFIER: [u8; 8] = value;` + = help: implement the missing item: `fn create_inherent(_: &InherentData) -> std::option::Option<::Call> { todo!() }` + = help: implement the missing item: `fn is_inherent(_: &::Call) -> bool { todo!() }` +diff --git a/frame/support/test/tests/pallet_ui/storage_info_unsatisfied_nmap.stderr b/frame/support/test/tests/pallet_ui/storage_info_unsatisfied_nmap.stderr +index bbbb3ed1b2..c34c796fe5 100644 +--- a/frame/support/test/tests/pallet_ui/storage_info_unsatisfied_nmap.stderr ++++ b/frame/support/test/tests/pallet_ui/storage_info_unsatisfied_nmap.stderr +@@ -14,5 +14,5 @@ error[E0277]: the trait bound `Bar: MaxEncodedLen` is not satisfied + (TupleElement0, TupleElement1, TupleElement2, TupleElement3, TupleElement4, TupleElement5, TupleElement6) + (TupleElement0, TupleElement1, TupleElement2, TupleElement3, TupleElement4, TupleElement5, TupleElement6, TupleElement7) + and $N others +- = note: required for `NMapKey` to implement `KeyGeneratorMaxEncodedLen` +- = note: required for `frame_support::pallet_prelude::StorageNMap<_GeneratedPrefixForStorageFoo, NMapKey, u32>` to implement `StorageInfoTrait` ++ = note: required for `Key` to implement `KeyGeneratorMaxEncodedLen` ++ = note: required for `frame_support::pallet_prelude::StorageNMap<_GeneratedPrefixForStorageFoo, Key, u32>` to implement `StorageInfoTrait` diff --git a/substrate/frame/glutton/src/lib.rs b/substrate/frame/glutton/src/lib.rs index f5e90a17c735..344a70becaeb 100644 --- a/substrate/frame/glutton/src/lib.rs +++ b/substrate/frame/glutton/src/lib.rs @@ -21,9 +21,8 @@ //! //! # Glutton Pallet //! -//! Pallet that consumes `ref_time` and `proof_size` of a block. Based on the -//! `Compute` and `Storage` parameters the pallet consumes the adequate amount -//! of weight. +//! Pallet that consumes `ref_time` and `proof_size` of a block. Based on the `Compute` and +//! `Storage` parameters the pallet consumes the adequate amount of weight. #![deny(missing_docs)] #![cfg_attr(not(feature = "std"), no_std)] diff --git a/substrate/frame/support/procedural/src/pallet/expand/event.rs b/substrate/frame/support/procedural/src/pallet/expand/event.rs index fbb699b4d41c..2713f45fc3d5 100644 --- a/substrate/frame/support/procedural/src/pallet/expand/event.rs +++ b/substrate/frame/support/procedural/src/pallet/expand/event.rs @@ -127,11 +127,12 @@ pub fn expand_event(def: &mut Def) -> proc_macro2::TokenStream { let trait_use_gen = &def.trait_use_generics(event.attr_span); let type_impl_gen = &def.type_impl_generics(event.attr_span); let type_use_gen = &def.type_use_generics(event.attr_span); + let pallet_ident = &def.pallet_struct.pallet; let PalletEventDepositAttr { fn_vis, fn_span, .. } = deposit_event; quote::quote_spanned!(*fn_span => - impl<#type_impl_gen> Pallet<#type_use_gen> #completed_where_clause { + impl<#type_impl_gen> #pallet_ident<#type_use_gen> #completed_where_clause { #fn_vis fn deposit_event(event: Event<#event_use_gen>) { let event = < ::RuntimeEvent as diff --git a/substrate/frame/support/procedural/src/pallet/expand/storage.rs b/substrate/frame/support/procedural/src/pallet/expand/storage.rs index e7f7cf548f0e..191ec2e949bf 100644 --- a/substrate/frame/support/procedural/src/pallet/expand/storage.rs +++ b/substrate/frame/support/procedural/src/pallet/expand/storage.rs @@ -822,12 +822,43 @@ pub fn expand_storages(def: &mut Def) -> proc_macro2::TokenStream { ) }); + // aggregated where clause of all storage types and the whole pallet. let mut where_clauses = vec![&def.config.where_clause]; where_clauses.extend(def.storages.iter().map(|storage| &storage.where_clause)); let completed_where_clause = super::merge_where_clauses(&where_clauses); let type_impl_gen = &def.type_impl_generics(proc_macro2::Span::call_site()); let type_use_gen = &def.type_use_generics(proc_macro2::Span::call_site()); + let try_decode_entire_state = { + let storage_names = def + .storages + .iter() + .filter_map(|storage| { + if storage.cfg_attrs.is_empty() { + let ident = &storage.ident; + let gen = &def.type_use_generics(storage.attr_span); + Some(quote::quote_spanned!(storage.attr_span => #ident<#gen> )) + } else { + None + } + }) + .collect::>(); + + quote::quote!( + #[cfg(feature = "try-runtime")] + impl<#type_impl_gen> #frame_support::traits::TryDecodeEntireStorage + for #pallet_ident<#type_use_gen> #completed_where_clause + { + fn try_decode_entire_state() -> Result { + // simply delegate impl to a tuple of all storage items we have. + // + // NOTE: for now, we have to exclude storage items that are feature gated. + <( #( #storage_names ),*) as frame_support::traits::TryDecodeEntireStorage>::try_decode_entire_state() + } + } + ) + }; + quote::quote!( impl<#type_impl_gen> #pallet_ident<#type_use_gen> #completed_where_clause @@ -853,5 +884,7 @@ pub fn expand_storages(def: &mut Def) -> proc_macro2::TokenStream { #( #getters )* #( #prefix_structs )* #( #on_empty_structs )* + + #try_decode_entire_state ) } diff --git a/substrate/frame/support/procedural/src/pallet/parse/storage.rs b/substrate/frame/support/procedural/src/pallet/parse/storage.rs index 3a0ec4747153..d1c7ba2e5e3c 100644 --- a/substrate/frame/support/procedural/src/pallet/parse/storage.rs +++ b/substrate/frame/support/procedural/src/pallet/parse/storage.rs @@ -151,7 +151,7 @@ pub enum QueryKind { /// `type MyStorage = StorageValue` /// The keys and values types are parsed in order to get metadata pub struct StorageDef { - /// The index of error item in pallet module. + /// The index of storage item in pallet module. pub index: usize, /// Visibility of the storage type. pub vis: syn::Visibility, diff --git a/substrate/frame/support/src/storage/generator/mod.rs b/substrate/frame/support/src/storage/generator/mod.rs index bac9f642e37d..2b2abdc2e830 100644 --- a/substrate/frame/support/src/storage/generator/mod.rs +++ b/substrate/frame/support/src/storage/generator/mod.rs @@ -24,10 +24,10 @@ //! //! This is internal api and is subject to change. -mod double_map; +pub(crate) mod double_map; pub(crate) mod map; -mod nmap; -mod value; +pub(crate) mod nmap; +pub(crate) mod value; pub use double_map::StorageDoubleMap; pub use map::StorageMap; diff --git a/substrate/frame/support/src/storage/migration.rs b/substrate/frame/support/src/storage/migration.rs index 568c475bdc69..e581c8a00a21 100644 --- a/substrate/frame/support/src/storage/migration.rs +++ b/substrate/frame/support/src/storage/migration.rs @@ -385,6 +385,39 @@ pub fn move_prefix(from_prefix: &[u8], to_prefix: &[u8]) { } } +/// A phony migration that does nothing, except executing `TryDecodeEntireStorage` on +/// `post_upgrade`, which implies it is only available if `try-state` feature is used. +/// +/// This can be used typically in the top level runtime, where `AllPallets` typically comes from +/// `construct_runtime!`. +pub struct EnsureStateDecodes(sp_std::marker::PhantomData); + +#[cfg(not(feature = "try-runtime"))] +impl crate::traits::OnRuntimeUpgrade for EnsureStateDecodes { + fn on_runtime_upgrade() -> crate::weights::Weight { + Default::default() + } +} + +#[cfg(feature = "try-runtime")] +impl crate::traits::OnRuntimeUpgrade + for EnsureStateDecodes +{ + fn on_runtime_upgrade() -> sp_weights::Weight { + Default::default() + } + + fn post_upgrade(_: Vec) -> Result<(), sp_runtime::TryRuntimeError> { + let decoded = AllPallets::try_decode_entire_state()?; + crate::log::info!( + target: crate::LOG_TARGET, + "decoded the entire state, total size = {} bytes", + decoded + ); + Ok(()) + } +} + #[cfg(test)] mod tests { use super::{ diff --git a/substrate/frame/support/src/storage/types/counted_map.rs b/substrate/frame/support/src/storage/types/counted_map.rs index 50e2c678248c..e7040a2e1d80 100644 --- a/substrate/frame/support/src/storage/types/counted_map.rs +++ b/substrate/frame/support/src/storage/types/counted_map.rs @@ -74,7 +74,11 @@ impl MapWrapper type Map = StorageMap; } -type CounterFor

= StorageValue<

::CounterPrefix, u32, ValueQuery>; +/// The numeric counter type. +pub type Counter = u32; + +type CounterFor

= + StorageValue<

::CounterPrefix, Counter, ValueQuery>; /// On removal logic for updating counter while draining upon some prefix with /// [`crate::storage::PrefixIterator`]. @@ -378,14 +382,14 @@ where /// can be very heavy, so use with caution. /// /// Returns the number of items in the map which is used to set the counter. - pub fn initialize_counter() -> u32 { - let count = Self::iter_values().count() as u32; + pub fn initialize_counter() -> Counter { + let count = Self::iter_values().count() as Counter; CounterFor::::set(count); count } /// Return the count. - pub fn count() -> u32 { + pub fn count() -> Counter { CounterFor::::get() } } @@ -1162,7 +1166,7 @@ mod test { StorageEntryMetadataIR { name: "counter_for_foo", modifier: StorageEntryModifierIR::Default, - ty: StorageEntryTypeIR::Plain(scale_info::meta_type::()), + ty: StorageEntryTypeIR::Plain(scale_info::meta_type::()), default: vec![0, 0, 0, 0], docs: if cfg!(feature = "no-metadata-docs") { vec![] diff --git a/substrate/frame/support/src/storage/types/counted_nmap.rs b/substrate/frame/support/src/storage/types/counted_nmap.rs index 5da31c059225..a49e4b7cf332 100644 --- a/substrate/frame/support/src/storage/types/counted_nmap.rs +++ b/substrate/frame/support/src/storage/types/counted_nmap.rs @@ -71,8 +71,10 @@ impl MapWrapper type Map = StorageNMap; } +type Counter = super::counted_map::Counter; + type CounterFor

= - StorageValue<

::CounterPrefix, u32, ValueQuery>; + StorageValue<

::CounterPrefix, Counter, ValueQuery>; /// On removal logic for updating counter while draining upon some prefix with /// [`crate::storage::PrefixIterator`]. @@ -429,7 +431,7 @@ where } /// Return the count. - pub fn count() -> u32 { + pub fn count() -> Counter { CounterFor::::get() } } diff --git a/substrate/frame/support/src/storage/types/mod.rs b/substrate/frame/support/src/storage/types/mod.rs index c7f2557099b3..3d8d53bb1696 100644 --- a/substrate/frame/support/src/storage/types/mod.rs +++ b/substrate/frame/support/src/storage/types/mod.rs @@ -30,7 +30,7 @@ mod map; mod nmap; mod value; -pub use counted_map::{CountedStorageMap, CountedStorageMapInstance}; +pub use counted_map::{CountedStorageMap, CountedStorageMapInstance, Counter}; pub use counted_nmap::{CountedStorageNMap, CountedStorageNMapInstance}; pub use double_map::StorageDoubleMap; pub use key::{ diff --git a/substrate/frame/support/src/storage/types/value.rs b/substrate/frame/support/src/storage/types/value.rs index 3e1f2fe9551d..29649227d64f 100644 --- a/substrate/frame/support/src/storage/types/value.rs +++ b/substrate/frame/support/src/storage/types/value.rs @@ -23,7 +23,7 @@ use crate::{ types::{OptionQuery, QueryKindTrait, StorageEntryMetadataBuilder}, StorageAppend, StorageDecodeLength, StorageTryAppend, }, - traits::{GetDefault, StorageInfo, StorageInstance}, + traits::{Get, GetDefault, StorageInfo, StorageInstance}, }; use codec::{Decode, Encode, EncodeLike, FullCodec, MaxEncodedLen}; use sp_arithmetic::traits::SaturatedConversion; @@ -46,7 +46,7 @@ where Prefix: StorageInstance, Value: FullCodec, QueryKind: QueryKindTrait, - OnEmpty: crate::traits::Get + 'static, + OnEmpty: Get + 'static, { type Query = QueryKind::Query; fn pallet_prefix() -> &'static [u8] { diff --git a/substrate/frame/support/src/traits.rs b/substrate/frame/support/src/traits.rs index 2179ee38f848..d6d231841c8d 100644 --- a/substrate/frame/support/src/traits.rs +++ b/substrate/frame/support/src/traits.rs @@ -124,4 +124,6 @@ pub use tx_pause::{TransactionPause, TransactionPauseError}; #[cfg(feature = "try-runtime")] mod try_runtime; #[cfg(feature = "try-runtime")] -pub use try_runtime::{Select as TryStateSelect, TryState, UpgradeCheckSelect}; +pub use try_runtime::{ + Select as TryStateSelect, TryDecodeEntireStorage, TryState, UpgradeCheckSelect, +}; diff --git a/substrate/frame/support/src/traits/try_runtime.rs b/substrate/frame/support/src/traits/try_runtime.rs index 31aebeeb4d99..130e3f16722d 100644 --- a/substrate/frame/support/src/traits/try_runtime.rs +++ b/substrate/frame/support/src/traits/try_runtime.rs @@ -17,8 +17,19 @@ //! Try-runtime specific traits and types. +use super::StorageInstance; +use crate::{ + storage::types::{ + CountedStorageMapInstance, CountedStorageNMapInstance, Counter, KeyGenerator, + QueryKindTrait, + }, + traits::{PartialStorageInfoTrait, StorageInfo}, + StorageHasher, +}; +use codec::{Decode, DecodeAll, FullCodec}; use impl_trait_for_tuples::impl_for_tuples; use sp_arithmetic::traits::AtLeast32BitUnsigned; +use sp_core::Get; use sp_runtime::TryRuntimeError; use sp_std::prelude::*; @@ -43,6 +54,203 @@ impl Default for Select { } } +/// Decode the entire data under the given storage type. +/// +/// For values, this is trivial. For all kinds of maps, it should decode all the values associated +/// with all keys existing in the map. +/// +/// Tuple implementations are provided and simply decode each type in the tuple, summing up the +/// decoded bytes if `Ok(_)`. +pub trait TryDecodeEntireStorage { + /// Decode the entire data under the given storage, returning `Ok(bytes_decoded)` if success. + fn try_decode_entire_state() -> Result; +} + +#[cfg_attr(all(not(feature = "tuples-96"), not(feature = "tuples-128")), impl_for_tuples(64))] +#[cfg_attr(all(feature = "tuples-96", not(feature = "tuples-128")), impl_for_tuples(96))] +#[cfg_attr(feature = "tuples-128", impl_for_tuples(128))] +impl TryDecodeEntireStorage for Tuple { + fn try_decode_entire_state() -> Result { + let mut len = 0usize; + for_tuples!( #( len = len.saturating_add(Tuple::try_decode_entire_state()?); )* ); + Ok(len) + } +} + +/// Decode all the values based on the prefix of `info` to `V`. +/// +/// Basically, it decodes and sums up all the values who's key start with `info.prefix`. For values, +/// this would be the value itself. For all sorts of maps, this should be all map items in the +/// absence of key collision. +fn decode_storage_info(info: StorageInfo) -> Result { + let mut next_key = info.prefix.clone(); + let mut decoded = 0; + + let decode_key = |key: &[u8]| match sp_io::storage::get(key) { + None => Ok(0), + Some(bytes) => { + let len = bytes.len(); + let _ = ::decode_all(&mut bytes.as_ref()).map_err(|_| { + log::error!(target: crate::LOG_TARGET, "failed to decoded {:?}", info,); + "failed to decode value under existing key" + })?; + Ok::(len) + }, + }; + + decoded += decode_key(&next_key)?; + loop { + match sp_io::storage::next_key(&next_key) { + Some(key) if key.starts_with(&info.prefix) => { + decoded += decode_key(&key)?; + next_key = key; + }, + _ => break, + } + } + + Ok(decoded) +} + +impl TryDecodeEntireStorage + for crate::storage::types::StorageValue +where + Prefix: StorageInstance, + Value: FullCodec, + QueryKind: QueryKindTrait, + OnEmpty: Get + 'static, +{ + fn try_decode_entire_state() -> Result { + let info = Self::partial_storage_info() + .first() + .cloned() + .expect("Value has only one storage info; qed"); + decode_storage_info::(info) + } +} + +impl TryDecodeEntireStorage + for crate::storage::types::StorageMap +where + Prefix: StorageInstance, + Hasher: StorageHasher, + Key: FullCodec, + Value: FullCodec, + QueryKind: QueryKindTrait, + OnEmpty: Get + 'static, + MaxValues: Get>, +{ + fn try_decode_entire_state() -> Result { + let info = Self::partial_storage_info() + .first() + .cloned() + .expect("Map has only one storage info; qed"); + decode_storage_info::(info) + } +} + +impl TryDecodeEntireStorage + for crate::storage::types::CountedStorageMap< + Prefix, + Hasher, + Key, + Value, + QueryKind, + OnEmpty, + MaxValues, + > where + Prefix: CountedStorageMapInstance, + Hasher: StorageHasher, + Key: FullCodec, + Value: FullCodec, + QueryKind: QueryKindTrait, + OnEmpty: Get + 'static, + MaxValues: Get>, +{ + fn try_decode_entire_state() -> Result { + let (map_info, counter_info) = match &Self::partial_storage_info()[..] { + [a, b] => (a.clone(), b.clone()), + _ => panic!("Counted map has two storage info items; qed"), + }; + let mut decoded = decode_storage_info::(counter_info)?; + decoded += decode_storage_info::(map_info)?; + Ok(decoded) + } +} + +impl + TryDecodeEntireStorage + for crate::storage::types::StorageDoubleMap< + Prefix, + Hasher1, + Key1, + Hasher2, + Key2, + Value, + QueryKind, + OnEmpty, + MaxValues, + > where + Prefix: StorageInstance, + Hasher1: StorageHasher, + Key1: FullCodec, + Hasher2: StorageHasher, + Key2: FullCodec, + Value: FullCodec, + QueryKind: QueryKindTrait, + OnEmpty: Get + 'static, + MaxValues: Get>, +{ + fn try_decode_entire_state() -> Result { + let info = Self::partial_storage_info() + .first() + .cloned() + .expect("Double-map has only one storage info; qed"); + decode_storage_info::(info) + } +} + +impl TryDecodeEntireStorage + for crate::storage::types::StorageNMap +where + Prefix: StorageInstance, + Key: KeyGenerator, + Value: FullCodec, + QueryKind: QueryKindTrait, + OnEmpty: Get + 'static, + MaxValues: Get>, +{ + fn try_decode_entire_state() -> Result { + let info = Self::partial_storage_info() + .first() + .cloned() + .expect("N-map has only one storage info; qed"); + decode_storage_info::(info) + } +} + +impl TryDecodeEntireStorage + for crate::storage::types::CountedStorageNMap +where + Prefix: CountedStorageNMapInstance, + Key: KeyGenerator, + Value: FullCodec, + QueryKind: QueryKindTrait, + OnEmpty: Get + 'static, + MaxValues: Get>, +{ + fn try_decode_entire_state() -> Result { + let (map_info, counter_info) = match &Self::partial_storage_info()[..] { + [a, b] => (a.clone(), b.clone()), + _ => panic!("Counted NMap has two storage info items; qed"), + }; + + let mut decoded = decode_storage_info::(counter_info)?; + decoded += decode_storage_info::(map_info)?; + Ok(decoded) + } +} + impl sp_std::fmt::Debug for Select { fn fmt(&self, f: &mut sp_std::fmt::Formatter<'_>) -> sp_std::fmt::Result { match self { @@ -187,3 +395,232 @@ impl TryState { + struct $name; + impl StorageInstance for $name { + fn pallet_prefix() -> &'static str { + "test_pallet" + } + const STORAGE_PREFIX: &'static str = stringify!($name); + } + }; + } + + build_prefix!(ValuePrefix); + type Value = types::StorageValue; + + build_prefix!(MapPrefix); + type Map = types::StorageMap; + + build_prefix!(CMapCounterPrefix); + build_prefix!(CMapPrefix); + impl CountedStorageMapInstance for CMapPrefix { + type CounterPrefix = CMapCounterPrefix; + } + type CMap = types::CountedStorageMap; + + build_prefix!(DMapPrefix); + type DMap = types::StorageDoubleMap; + + build_prefix!(NMapPrefix); + type NMap = types::StorageNMap, Key), u128>; + + build_prefix!(CountedNMapCounterPrefix); + build_prefix!(CountedNMapPrefix); + impl CountedStorageNMapInstance for CountedNMapPrefix { + type CounterPrefix = CountedNMapCounterPrefix; + } + type CNMap = types::CountedStorageNMap, Key), u128>; + + #[test] + fn try_decode_entire_state_value_works() { + sp_io::TestExternalities::new_empty().execute_with(|| { + assert_eq!(Value::try_decode_entire_state(), Ok(0)); + + Value::put(42); + assert_eq!(Value::try_decode_entire_state(), Ok(4)); + + Value::kill(); + assert_eq!(Value::try_decode_entire_state(), Ok(0)); + + // two bytes, cannot be decoded into u32. + sp_io::storage::set(&Value::hashed_key(), &[0u8, 1]); + assert!(Value::try_decode_entire_state().is_err()); + }) + } + + #[test] + fn try_decode_entire_state_map_works() { + sp_io::TestExternalities::new_empty().execute_with(|| { + assert_eq!(Map::try_decode_entire_state(), Ok(0)); + + Map::insert(0, 42); + assert_eq!(Map::try_decode_entire_state(), Ok(4)); + + Map::insert(0, 42); + assert_eq!(Map::try_decode_entire_state(), Ok(4)); + + Map::insert(1, 42); + assert_eq!(Map::try_decode_entire_state(), Ok(8)); + + Map::remove(0); + assert_eq!(Map::try_decode_entire_state(), Ok(4)); + + // two bytes, cannot be decoded into u32. + sp_io::storage::set(&Map::hashed_key_for(2), &[0u8, 1]); + assert!(Map::try_decode_entire_state().is_err()); + }) + } + + #[test] + fn try_decode_entire_state_counted_map_works() { + sp_io::TestExternalities::new_empty().execute_with(|| { + // counter is not even initialized; + assert_eq!(CMap::try_decode_entire_state(), Ok(0 + 0)); + + let counter = 4; + let value_size = std::mem::size_of::(); + + CMap::insert(0, 42); + assert_eq!(CMap::try_decode_entire_state(), Ok(value_size + counter)); + + CMap::insert(0, 42); + assert_eq!(CMap::try_decode_entire_state(), Ok(value_size + counter)); + + CMap::insert(1, 42); + assert_eq!(CMap::try_decode_entire_state(), Ok(value_size * 2 + counter)); + + CMap::remove(0); + assert_eq!(CMap::try_decode_entire_state(), Ok(value_size + counter)); + + // counter is cleared again. + let _ = CMap::clear(u32::MAX, None); + assert_eq!(CMap::try_decode_entire_state(), Ok(0 + 0)); + + // 1 bytes, cannot be decoded into u16. + sp_io::storage::set(&CMap::hashed_key_for(2), &[0u8]); + assert!(CMap::try_decode_entire_state().is_err()); + }) + } + + #[test] + fn try_decode_entire_state_double_works() { + sp_io::TestExternalities::new_empty().execute_with(|| { + assert_eq!(DMap::try_decode_entire_state(), Ok(0)); + + DMap::insert(0, 0, 42); + assert_eq!(DMap::try_decode_entire_state(), Ok(4)); + + DMap::insert(0, 0, 42); + assert_eq!(DMap::try_decode_entire_state(), Ok(4)); + + DMap::insert(0, 1, 42); + assert_eq!(DMap::try_decode_entire_state(), Ok(8)); + + DMap::insert(1, 0, 42); + assert_eq!(DMap::try_decode_entire_state(), Ok(12)); + + DMap::remove(0, 0); + assert_eq!(DMap::try_decode_entire_state(), Ok(8)); + + // two bytes, cannot be decoded into u32. + sp_io::storage::set(&DMap::hashed_key_for(1, 1), &[0u8, 1]); + assert!(DMap::try_decode_entire_state().is_err()); + }) + } + + #[test] + fn try_decode_entire_state_n_map_works() { + sp_io::TestExternalities::new_empty().execute_with(|| { + assert_eq!(NMap::try_decode_entire_state(), Ok(0)); + + let value_size = std::mem::size_of::(); + + NMap::insert((0u8, 0), 42); + assert_eq!(NMap::try_decode_entire_state(), Ok(value_size)); + + NMap::insert((0, 0), 42); + assert_eq!(NMap::try_decode_entire_state(), Ok(value_size)); + + NMap::insert((0, 1), 42); + assert_eq!(NMap::try_decode_entire_state(), Ok(value_size * 2)); + + NMap::insert((1, 0), 42); + assert_eq!(NMap::try_decode_entire_state(), Ok(value_size * 3)); + + NMap::remove((0, 0)); + assert_eq!(NMap::try_decode_entire_state(), Ok(value_size * 2)); + + // two bytes, cannot be decoded into u128. + sp_io::storage::set(&NMap::hashed_key_for((1, 1)), &[0u8, 1]); + assert!(NMap::try_decode_entire_state().is_err()); + }) + } + + #[test] + fn try_decode_entire_state_counted_n_map_works() { + sp_io::TestExternalities::new_empty().execute_with(|| { + sp_io::TestExternalities::new_empty().execute_with(|| { + assert_eq!(NMap::try_decode_entire_state(), Ok(0)); + + let value_size = std::mem::size_of::(); + let counter = 4; + + CNMap::insert((0u8, 0), 42); + assert_eq!(CNMap::try_decode_entire_state(), Ok(value_size + counter)); + + CNMap::insert((0, 0), 42); + assert_eq!(CNMap::try_decode_entire_state(), Ok(value_size + counter)); + + CNMap::insert((0, 1), 42); + assert_eq!(CNMap::try_decode_entire_state(), Ok(value_size * 2 + counter)); + + CNMap::insert((1, 0), 42); + assert_eq!(CNMap::try_decode_entire_state(), Ok(value_size * 3 + counter)); + + CNMap::remove((0, 0)); + assert_eq!(CNMap::try_decode_entire_state(), Ok(value_size * 2 + counter)); + + // two bytes, cannot be decoded into u128. + sp_io::storage::set(&CNMap::hashed_key_for((1, 1)), &[0u8, 1]); + assert!(CNMap::try_decode_entire_state().is_err()); + }) + }) + } + + #[test] + fn extra_bytes_are_rejected() { + sp_io::TestExternalities::new_empty().execute_with(|| { + assert_eq!(Map::try_decode_entire_state(), Ok(0)); + + // 6bytes, too many to fit in u32, should be rejected. + sp_io::storage::set(&Map::hashed_key_for(2), &[0u8, 1, 3, 4, 5, 6]); + assert!(Map::try_decode_entire_state().is_err()); + }) + } + + #[test] + fn try_decode_entire_state_tuple_of_storage_works() { + sp_io::TestExternalities::new_empty().execute_with(|| { + assert_eq!(<(Value, Map) as TryDecodeEntireStorage>::try_decode_entire_state(), Ok(0)); + + Value::put(42); + assert_eq!(<(Value, Map) as TryDecodeEntireStorage>::try_decode_entire_state(), Ok(4)); + + Map::insert(0, 42); + assert_eq!(<(Value, Map) as TryDecodeEntireStorage>::try_decode_entire_state(), Ok(8)); + }); + } +} diff --git a/substrate/frame/support/test/tests/pallet_ui/call_argument_invalid_bound.stderr b/substrate/frame/support/test/tests/pallet_ui/call_argument_invalid_bound.stderr index c86930f8a64e..08ea7c0bec3a 100644 --- a/substrate/frame/support/test/tests/pallet_ui/call_argument_invalid_bound.stderr +++ b/substrate/frame/support/test/tests/pallet_ui/call_argument_invalid_bound.stderr @@ -19,7 +19,7 @@ error[E0277]: `::Bar` doesn't implement `std::fmt::Debug` | = help: the trait `std::fmt::Debug` is not implemented for `::Bar` = note: required for `&::Bar` to implement `std::fmt::Debug` - = note: required for the cast from `&::Bar` to the object type `dyn std::fmt::Debug` + = note: required for the cast from `&&::Bar` to `&dyn std::fmt::Debug` error[E0277]: the trait bound `::Bar: Clone` is not satisfied --> tests/pallet_ui/call_argument_invalid_bound.rs:38:36 diff --git a/substrate/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_2.stderr b/substrate/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_2.stderr index 1b04f44c78fe..80316fcd2489 100644 --- a/substrate/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_2.stderr +++ b/substrate/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_2.stderr @@ -19,7 +19,7 @@ error[E0277]: `::Bar` doesn't implement `std::fmt::Debug` | = help: the trait `std::fmt::Debug` is not implemented for `::Bar` = note: required for `&::Bar` to implement `std::fmt::Debug` - = note: required for the cast from `&::Bar` to the object type `dyn std::fmt::Debug` + = note: required for the cast from `&&::Bar` to `&dyn std::fmt::Debug` error[E0277]: the trait bound `::Bar: Clone` is not satisfied --> tests/pallet_ui/call_argument_invalid_bound_2.rs:38:36 diff --git a/substrate/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_3.stderr b/substrate/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_3.stderr index 7429bce050c2..d45b74bad842 100644 --- a/substrate/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_3.stderr +++ b/substrate/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_3.stderr @@ -20,7 +20,7 @@ error[E0277]: `Bar` doesn't implement `std::fmt::Debug` = help: the trait `std::fmt::Debug` is not implemented for `Bar` = note: add `#[derive(Debug)]` to `Bar` or manually `impl std::fmt::Debug for Bar` = note: required for `&Bar` to implement `std::fmt::Debug` - = note: required for the cast from `&Bar` to the object type `dyn std::fmt::Debug` + = note: required for the cast from `&&Bar` to `&dyn std::fmt::Debug` help: consider annotating `Bar` with `#[derive(Debug)]` | 34 + #[derive(Debug)] diff --git a/substrate/frame/support/test/tests/pallet_ui/event_field_not_member.stderr b/substrate/frame/support/test/tests/pallet_ui/event_field_not_member.stderr index 4df6deafa0df..fc4a33b72150 100644 --- a/substrate/frame/support/test/tests/pallet_ui/event_field_not_member.stderr +++ b/substrate/frame/support/test/tests/pallet_ui/event_field_not_member.stderr @@ -18,4 +18,4 @@ error[E0277]: `::Bar` doesn't implement `std::fmt::Debug` | = help: the trait `std::fmt::Debug` is not implemented for `::Bar` = note: required for `&::Bar` to implement `std::fmt::Debug` - = note: required for the cast from `&::Bar` to the object type `dyn std::fmt::Debug` + = note: required for the cast from `&&::Bar` to `&dyn std::fmt::Debug` diff --git a/substrate/frame/support/test/tests/pallet_ui/storage_info_unsatisfied_nmap.stderr b/substrate/frame/support/test/tests/pallet_ui/storage_info_unsatisfied_nmap.stderr index cdcd1b401f80..6b4530f5398e 100644 --- a/substrate/frame/support/test/tests/pallet_ui/storage_info_unsatisfied_nmap.stderr +++ b/substrate/frame/support/test/tests/pallet_ui/storage_info_unsatisfied_nmap.stderr @@ -14,5 +14,5 @@ error[E0277]: the trait bound `Bar: MaxEncodedLen` is not satisfied (TupleElement0, TupleElement1, TupleElement2, TupleElement3, TupleElement4, TupleElement5, TupleElement6) (TupleElement0, TupleElement1, TupleElement2, TupleElement3, TupleElement4, TupleElement5, TupleElement6, TupleElement7) and $N others - = note: required for `Key` to implement `KeyGeneratorMaxEncodedLen` - = note: required for `frame_support::pallet_prelude::StorageNMap<_GeneratedPrefixForStorageFoo, Key, u32>` to implement `StorageInfoTrait` + = note: required for `NMapKey` to implement `KeyGeneratorMaxEncodedLen` + = note: required for `frame_support::pallet_prelude::StorageNMap<_GeneratedPrefixForStorageFoo, NMapKey, u32>` to implement `StorageInfoTrait` From 5ebb6db4244bb96fa20efdaaceaaf5d73001214d Mon Sep 17 00:00:00 2001 From: Piet Wolff Date: Fri, 6 Oct 2023 09:46:30 +0200 Subject: [PATCH 02/24] remove patchfile --- substrate/decode-all.patch | 862 ------------------------------------- 1 file changed, 862 deletions(-) delete mode 100644 substrate/decode-all.patch diff --git a/substrate/decode-all.patch b/substrate/decode-all.patch deleted file mode 100644 index 8c9d4ba2e0eb..000000000000 --- a/substrate/decode-all.patch +++ /dev/null @@ -1,862 +0,0 @@ -diff --git a/bin/node-template/runtime/src/lib.rs b/bin/node-template/runtime/src/lib.rs -index 133bae7ae7..216be9588b 100644 ---- a/bin/node-template/runtime/src/lib.rs -+++ b/bin/node-template/runtime/src/lib.rs -@@ -311,14 +311,6 @@ pub type SignedExtra = ( - pallet_transaction_payment::ChargeTransactionPayment, - ); - --/// All migrations of the runtime, aside from the ones declared in the pallets. --/// --/// This can be a tuple of types, each implementing `OnRuntimeUpgrade`. Add other migration types --/// before `EnsureStateDecodes` as needed -- this is only for testing, and --// should come last. --#[allow(unused_parens)] --type Migrations = (frame_support::migration::EnsureStateDecodes); -- - /// Unchecked extrinsic type as expected by this runtime. - pub type UncheckedExtrinsic = - generic::UncheckedExtrinsic; -@@ -331,7 +323,6 @@ pub type Executive = frame_executive::Executive< - frame_system::ChainContext, - Runtime, - AllPalletsWithSystem, -- Migrations, - >; - - #[cfg(feature = "runtime-benchmarks")] -diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs -index dd21c53397..4f34e4ecd8 100644 ---- a/bin/node/runtime/src/lib.rs -+++ b/bin/node/runtime/src/lib.rs -@@ -2137,8 +2137,6 @@ type Migrations = ( - pallet_nomination_pools::migration::v2::MigrateToV2, - pallet_alliance::migration::Migration, - pallet_contracts::Migration, -- // This should always be the last migration item. -- frame_support::storage::migration::EnsureStateDecodes, - ); - - type EventRecord = frame_system::EventRecord< -diff --git a/frame/glutton/src/lib.rs b/frame/glutton/src/lib.rs -index 176cd566d9..c76cc30017 100644 ---- a/frame/glutton/src/lib.rs -+++ b/frame/glutton/src/lib.rs -@@ -21,8 +21,9 @@ - //! - //! # Glutton Pallet - //! --//! Pallet that consumes `ref_time` and `proof_size` of a block. Based on the `Compute` and --//! `Storage` parameters the pallet consumes the adequate amount of weight. -+//! Pallet that consumes `ref_time` and `proof_size` of a block. Based on the -+//! `Compute` and `Storage` parameters the pallet consumes the adequate amount -+//! of weight. - - #![deny(missing_docs)] - #![cfg_attr(not(feature = "std"), no_std)] -diff --git a/frame/support/procedural/src/pallet/expand/event.rs b/frame/support/procedural/src/pallet/expand/event.rs -index 2713f45fc3..fbb699b4d4 100644 ---- a/frame/support/procedural/src/pallet/expand/event.rs -+++ b/frame/support/procedural/src/pallet/expand/event.rs -@@ -127,12 +127,11 @@ pub fn expand_event(def: &mut Def) -> proc_macro2::TokenStream { - let trait_use_gen = &def.trait_use_generics(event.attr_span); - let type_impl_gen = &def.type_impl_generics(event.attr_span); - let type_use_gen = &def.type_use_generics(event.attr_span); -- let pallet_ident = &def.pallet_struct.pallet; - - let PalletEventDepositAttr { fn_vis, fn_span, .. } = deposit_event; - - quote::quote_spanned!(*fn_span => -- impl<#type_impl_gen> #pallet_ident<#type_use_gen> #completed_where_clause { -+ impl<#type_impl_gen> Pallet<#type_use_gen> #completed_where_clause { - #fn_vis fn deposit_event(event: Event<#event_use_gen>) { - let event = < - ::RuntimeEvent as -diff --git a/frame/support/procedural/src/pallet/expand/storage.rs b/frame/support/procedural/src/pallet/expand/storage.rs -index 2e9c4ba6d3..1a941f6cb3 100644 ---- a/frame/support/procedural/src/pallet/expand/storage.rs -+++ b/frame/support/procedural/src/pallet/expand/storage.rs -@@ -780,43 +780,12 @@ pub fn expand_storages(def: &mut Def) -> proc_macro2::TokenStream { - ) - }); - -- // aggregated where clause of all storage types and the whole pallet. - let mut where_clauses = vec![&def.config.where_clause]; - where_clauses.extend(def.storages.iter().map(|storage| &storage.where_clause)); - let completed_where_clause = super::merge_where_clauses(&where_clauses); - let type_impl_gen = &def.type_impl_generics(proc_macro2::Span::call_site()); - let type_use_gen = &def.type_use_generics(proc_macro2::Span::call_site()); - -- let try_decode_entire_state = { -- let storage_names = def -- .storages -- .iter() -- .filter_map(|storage| { -- if storage.cfg_attrs.is_empty() { -- let ident = &storage.ident; -- let gen = &def.type_use_generics(storage.attr_span); -- Some(quote::quote_spanned!(storage.attr_span => #ident<#gen> )) -- } else { -- None -- } -- }) -- .collect::>(); -- -- quote::quote!( -- #[cfg(feature = "try-runtime")] -- impl<#type_impl_gen> #frame_support::traits::TryDecodeEntireStorage -- for #pallet_ident<#type_use_gen> #completed_where_clause -- { -- fn try_decode_entire_state() -> Result { -- // simply delegate impl to a tuple of all storage items we have. -- // -- // NOTE: for now, we have to exclude storage items that are feature gated. -- <( #( #storage_names ),*) as frame_support::traits::TryDecodeEntireStorage>::try_decode_entire_state() -- } -- } -- ) -- }; -- - quote::quote!( - impl<#type_impl_gen> #pallet_ident<#type_use_gen> - #completed_where_clause -@@ -842,7 +811,5 @@ pub fn expand_storages(def: &mut Def) -> proc_macro2::TokenStream { - #( #getters )* - #( #prefix_structs )* - #( #on_empty_structs )* -- -- #try_decode_entire_state - ) - } -diff --git a/frame/support/procedural/src/pallet/parse/storage.rs b/frame/support/procedural/src/pallet/parse/storage.rs -index d1c7ba2e5e..3a0ec47471 100644 ---- a/frame/support/procedural/src/pallet/parse/storage.rs -+++ b/frame/support/procedural/src/pallet/parse/storage.rs -@@ -151,7 +151,7 @@ pub enum QueryKind { - /// `type MyStorage = StorageValue` - /// The keys and values types are parsed in order to get metadata - pub struct StorageDef { -- /// The index of storage item in pallet module. -+ /// The index of error item in pallet module. - pub index: usize, - /// Visibility of the storage type. - pub vis: syn::Visibility, -diff --git a/frame/support/src/storage/generator/mod.rs b/frame/support/src/storage/generator/mod.rs -index 2b2abdc2e8..bac9f642e3 100644 ---- a/frame/support/src/storage/generator/mod.rs -+++ b/frame/support/src/storage/generator/mod.rs -@@ -24,10 +24,10 @@ - //! - //! This is internal api and is subject to change. - --pub(crate) mod double_map; -+mod double_map; - pub(crate) mod map; --pub(crate) mod nmap; --pub(crate) mod value; -+mod nmap; -+mod value; - - pub use double_map::StorageDoubleMap; - pub use map::StorageMap; -diff --git a/frame/support/src/storage/migration.rs b/frame/support/src/storage/migration.rs -index e581c8a00a..568c475bdc 100644 ---- a/frame/support/src/storage/migration.rs -+++ b/frame/support/src/storage/migration.rs -@@ -385,39 +385,6 @@ pub fn move_prefix(from_prefix: &[u8], to_prefix: &[u8]) { - } - } - --/// A phony migration that does nothing, except executing `TryDecodeEntireStorage` on --/// `post_upgrade`, which implies it is only available if `try-state` feature is used. --/// --/// This can be used typically in the top level runtime, where `AllPallets` typically comes from --/// `construct_runtime!`. --pub struct EnsureStateDecodes(sp_std::marker::PhantomData); -- --#[cfg(not(feature = "try-runtime"))] --impl crate::traits::OnRuntimeUpgrade for EnsureStateDecodes { -- fn on_runtime_upgrade() -> crate::weights::Weight { -- Default::default() -- } --} -- --#[cfg(feature = "try-runtime")] --impl crate::traits::OnRuntimeUpgrade -- for EnsureStateDecodes --{ -- fn on_runtime_upgrade() -> sp_weights::Weight { -- Default::default() -- } -- -- fn post_upgrade(_: Vec) -> Result<(), sp_runtime::TryRuntimeError> { -- let decoded = AllPallets::try_decode_entire_state()?; -- crate::log::info!( -- target: crate::LOG_TARGET, -- "decoded the entire state, total size = {} bytes", -- decoded -- ); -- Ok(()) -- } --} -- - #[cfg(test)] - mod tests { - use super::{ -diff --git a/frame/support/src/storage/types/counted_map.rs b/frame/support/src/storage/types/counted_map.rs -index 4a022b4403..5b750a7409 100644 ---- a/frame/support/src/storage/types/counted_map.rs -+++ b/frame/support/src/storage/types/counted_map.rs -@@ -74,11 +74,7 @@ impl MapWrapper - type Map = StorageMap; - } - --/// The numeric counter type. --pub type Counter = u32; -- --type CounterFor

= -- StorageValue<

::CounterPrefix, Counter, ValueQuery>; -+type CounterFor

= StorageValue<

::CounterPrefix, u32, ValueQuery>; - - /// On removal logic for updating counter while draining upon some prefix with - /// [`crate::storage::PrefixIterator`]. -@@ -382,14 +378,14 @@ where - /// can be very heavy, so use with caution. - /// - /// Returns the number of items in the map which is used to set the counter. -- pub fn initialize_counter() -> Counter { -- let count = Self::iter_values().count() as Counter; -+ pub fn initialize_counter() -> u32 { -+ let count = Self::iter_values().count() as u32; - CounterFor::::set(count); - count - } - - /// Return the count. -- pub fn count() -> Counter { -+ pub fn count() -> u32 { - CounterFor::::get() - } - } -@@ -1166,7 +1162,7 @@ mod test { - StorageEntryMetadataIR { - name: "counter_for_foo", - modifier: StorageEntryModifierIR::Default, -- ty: StorageEntryTypeIR::Plain(scale_info::meta_type::()), -+ ty: StorageEntryTypeIR::Plain(scale_info::meta_type::()), - default: vec![0, 0, 0, 0], - docs: if cfg!(feature = "no-metadata-docs") { - vec![] -diff --git a/frame/support/src/storage/types/counted_nmap.rs b/frame/support/src/storage/types/counted_nmap.rs -index 1dc8af22ae..7dbcb74f00 100644 ---- a/frame/support/src/storage/types/counted_nmap.rs -+++ b/frame/support/src/storage/types/counted_nmap.rs -@@ -71,10 +71,8 @@ impl MapWrapper - type Map = StorageNMap; - } - --type Counter = super::counted_map::Counter; -- - type CounterFor

= -- StorageValue<

::CounterPrefix, Counter, ValueQuery>; -+ StorageValue<

::CounterPrefix, u32, ValueQuery>; - - /// On removal logic for updating counter while draining upon some prefix with - /// [`crate::storage::PrefixIterator`]. -@@ -431,7 +429,7 @@ where - } - - /// Return the count. -- pub fn count() -> Counter { -+ pub fn count() -> u32 { - CounterFor::::get() - } - } -diff --git a/frame/support/src/storage/types/mod.rs b/frame/support/src/storage/types/mod.rs -index 3d8d53bb16..c7f2557099 100644 ---- a/frame/support/src/storage/types/mod.rs -+++ b/frame/support/src/storage/types/mod.rs -@@ -30,7 +30,7 @@ mod map; - mod nmap; - mod value; - --pub use counted_map::{CountedStorageMap, CountedStorageMapInstance, Counter}; -+pub use counted_map::{CountedStorageMap, CountedStorageMapInstance}; - pub use counted_nmap::{CountedStorageNMap, CountedStorageNMapInstance}; - pub use double_map::StorageDoubleMap; - pub use key::{ -diff --git a/frame/support/src/storage/types/value.rs b/frame/support/src/storage/types/value.rs -index eaf0a841a4..3c7f24715a 100644 ---- a/frame/support/src/storage/types/value.rs -+++ b/frame/support/src/storage/types/value.rs -@@ -23,7 +23,7 @@ use crate::{ - types::{OptionQuery, QueryKindTrait, StorageEntryMetadataBuilder}, - StorageAppend, StorageDecodeLength, StorageTryAppend, - }, -- traits::{Get, GetDefault, StorageInfo, StorageInstance}, -+ traits::{GetDefault, StorageInfo, StorageInstance}, - }; - use codec::{Decode, Encode, EncodeLike, FullCodec, MaxEncodedLen}; - use sp_arithmetic::traits::SaturatedConversion; -@@ -46,7 +46,7 @@ where - Prefix: StorageInstance, - Value: FullCodec, - QueryKind: QueryKindTrait, -- OnEmpty: Get + 'static, -+ OnEmpty: crate::traits::Get + 'static, - { - type Query = QueryKind::Query; - fn module_prefix() -> &'static [u8] { -diff --git a/frame/support/src/traits.rs b/frame/support/src/traits.rs -index eb6527d38a..f669046f85 100644 ---- a/frame/support/src/traits.rs -+++ b/frame/support/src/traits.rs -@@ -125,6 +125,4 @@ pub use tx_pause::{TransactionPause, TransactionPauseError}; - #[cfg(feature = "try-runtime")] - mod try_runtime; - #[cfg(feature = "try-runtime")] --pub use try_runtime::{ -- Select as TryStateSelect, TryDecodeEntireStorage, TryState, UpgradeCheckSelect, --}; -+pub use try_runtime::{Select as TryStateSelect, TryState, UpgradeCheckSelect}; -diff --git a/frame/support/src/traits/try_runtime.rs b/frame/support/src/traits/try_runtime.rs -index 130e3f1672..31aebeeb4d 100644 ---- a/frame/support/src/traits/try_runtime.rs -+++ b/frame/support/src/traits/try_runtime.rs -@@ -17,19 +17,8 @@ - - //! Try-runtime specific traits and types. - --use super::StorageInstance; --use crate::{ -- storage::types::{ -- CountedStorageMapInstance, CountedStorageNMapInstance, Counter, KeyGenerator, -- QueryKindTrait, -- }, -- traits::{PartialStorageInfoTrait, StorageInfo}, -- StorageHasher, --}; --use codec::{Decode, DecodeAll, FullCodec}; - use impl_trait_for_tuples::impl_for_tuples; - use sp_arithmetic::traits::AtLeast32BitUnsigned; --use sp_core::Get; - use sp_runtime::TryRuntimeError; - use sp_std::prelude::*; - -@@ -54,203 +43,6 @@ impl Default for Select { - } - } - --/// Decode the entire data under the given storage type. --/// --/// For values, this is trivial. For all kinds of maps, it should decode all the values associated --/// with all keys existing in the map. --/// --/// Tuple implementations are provided and simply decode each type in the tuple, summing up the --/// decoded bytes if `Ok(_)`. --pub trait TryDecodeEntireStorage { -- /// Decode the entire data under the given storage, returning `Ok(bytes_decoded)` if success. -- fn try_decode_entire_state() -> Result; --} -- --#[cfg_attr(all(not(feature = "tuples-96"), not(feature = "tuples-128")), impl_for_tuples(64))] --#[cfg_attr(all(feature = "tuples-96", not(feature = "tuples-128")), impl_for_tuples(96))] --#[cfg_attr(feature = "tuples-128", impl_for_tuples(128))] --impl TryDecodeEntireStorage for Tuple { -- fn try_decode_entire_state() -> Result { -- let mut len = 0usize; -- for_tuples!( #( len = len.saturating_add(Tuple::try_decode_entire_state()?); )* ); -- Ok(len) -- } --} -- --/// Decode all the values based on the prefix of `info` to `V`. --/// --/// Basically, it decodes and sums up all the values who's key start with `info.prefix`. For values, --/// this would be the value itself. For all sorts of maps, this should be all map items in the --/// absence of key collision. --fn decode_storage_info(info: StorageInfo) -> Result { -- let mut next_key = info.prefix.clone(); -- let mut decoded = 0; -- -- let decode_key = |key: &[u8]| match sp_io::storage::get(key) { -- None => Ok(0), -- Some(bytes) => { -- let len = bytes.len(); -- let _ = ::decode_all(&mut bytes.as_ref()).map_err(|_| { -- log::error!(target: crate::LOG_TARGET, "failed to decoded {:?}", info,); -- "failed to decode value under existing key" -- })?; -- Ok::(len) -- }, -- }; -- -- decoded += decode_key(&next_key)?; -- loop { -- match sp_io::storage::next_key(&next_key) { -- Some(key) if key.starts_with(&info.prefix) => { -- decoded += decode_key(&key)?; -- next_key = key; -- }, -- _ => break, -- } -- } -- -- Ok(decoded) --} -- --impl TryDecodeEntireStorage -- for crate::storage::types::StorageValue --where -- Prefix: StorageInstance, -- Value: FullCodec, -- QueryKind: QueryKindTrait, -- OnEmpty: Get + 'static, --{ -- fn try_decode_entire_state() -> Result { -- let info = Self::partial_storage_info() -- .first() -- .cloned() -- .expect("Value has only one storage info; qed"); -- decode_storage_info::(info) -- } --} -- --impl TryDecodeEntireStorage -- for crate::storage::types::StorageMap --where -- Prefix: StorageInstance, -- Hasher: StorageHasher, -- Key: FullCodec, -- Value: FullCodec, -- QueryKind: QueryKindTrait, -- OnEmpty: Get + 'static, -- MaxValues: Get>, --{ -- fn try_decode_entire_state() -> Result { -- let info = Self::partial_storage_info() -- .first() -- .cloned() -- .expect("Map has only one storage info; qed"); -- decode_storage_info::(info) -- } --} -- --impl TryDecodeEntireStorage -- for crate::storage::types::CountedStorageMap< -- Prefix, -- Hasher, -- Key, -- Value, -- QueryKind, -- OnEmpty, -- MaxValues, -- > where -- Prefix: CountedStorageMapInstance, -- Hasher: StorageHasher, -- Key: FullCodec, -- Value: FullCodec, -- QueryKind: QueryKindTrait, -- OnEmpty: Get + 'static, -- MaxValues: Get>, --{ -- fn try_decode_entire_state() -> Result { -- let (map_info, counter_info) = match &Self::partial_storage_info()[..] { -- [a, b] => (a.clone(), b.clone()), -- _ => panic!("Counted map has two storage info items; qed"), -- }; -- let mut decoded = decode_storage_info::(counter_info)?; -- decoded += decode_storage_info::(map_info)?; -- Ok(decoded) -- } --} -- --impl -- TryDecodeEntireStorage -- for crate::storage::types::StorageDoubleMap< -- Prefix, -- Hasher1, -- Key1, -- Hasher2, -- Key2, -- Value, -- QueryKind, -- OnEmpty, -- MaxValues, -- > where -- Prefix: StorageInstance, -- Hasher1: StorageHasher, -- Key1: FullCodec, -- Hasher2: StorageHasher, -- Key2: FullCodec, -- Value: FullCodec, -- QueryKind: QueryKindTrait, -- OnEmpty: Get + 'static, -- MaxValues: Get>, --{ -- fn try_decode_entire_state() -> Result { -- let info = Self::partial_storage_info() -- .first() -- .cloned() -- .expect("Double-map has only one storage info; qed"); -- decode_storage_info::(info) -- } --} -- --impl TryDecodeEntireStorage -- for crate::storage::types::StorageNMap --where -- Prefix: StorageInstance, -- Key: KeyGenerator, -- Value: FullCodec, -- QueryKind: QueryKindTrait, -- OnEmpty: Get + 'static, -- MaxValues: Get>, --{ -- fn try_decode_entire_state() -> Result { -- let info = Self::partial_storage_info() -- .first() -- .cloned() -- .expect("N-map has only one storage info; qed"); -- decode_storage_info::(info) -- } --} -- --impl TryDecodeEntireStorage -- for crate::storage::types::CountedStorageNMap --where -- Prefix: CountedStorageNMapInstance, -- Key: KeyGenerator, -- Value: FullCodec, -- QueryKind: QueryKindTrait, -- OnEmpty: Get + 'static, -- MaxValues: Get>, --{ -- fn try_decode_entire_state() -> Result { -- let (map_info, counter_info) = match &Self::partial_storage_info()[..] { -- [a, b] => (a.clone(), b.clone()), -- _ => panic!("Counted NMap has two storage info items; qed"), -- }; -- -- let mut decoded = decode_storage_info::(counter_info)?; -- decoded += decode_storage_info::(map_info)?; -- Ok(decoded) -- } --} -- - impl sp_std::fmt::Debug for Select { - fn fmt(&self, f: &mut sp_std::fmt::Formatter<'_>) -> sp_std::fmt::Result { - match self { -@@ -395,232 +187,3 @@ impl TryState { -- struct $name; -- impl StorageInstance for $name { -- fn pallet_prefix() -> &'static str { -- "test_pallet" -- } -- const STORAGE_PREFIX: &'static str = stringify!($name); -- } -- }; -- } -- -- build_prefix!(ValuePrefix); -- type Value = types::StorageValue; -- -- build_prefix!(MapPrefix); -- type Map = types::StorageMap; -- -- build_prefix!(CMapCounterPrefix); -- build_prefix!(CMapPrefix); -- impl CountedStorageMapInstance for CMapPrefix { -- type CounterPrefix = CMapCounterPrefix; -- } -- type CMap = types::CountedStorageMap; -- -- build_prefix!(DMapPrefix); -- type DMap = types::StorageDoubleMap; -- -- build_prefix!(NMapPrefix); -- type NMap = types::StorageNMap, Key), u128>; -- -- build_prefix!(CountedNMapCounterPrefix); -- build_prefix!(CountedNMapPrefix); -- impl CountedStorageNMapInstance for CountedNMapPrefix { -- type CounterPrefix = CountedNMapCounterPrefix; -- } -- type CNMap = types::CountedStorageNMap, Key), u128>; -- -- #[test] -- fn try_decode_entire_state_value_works() { -- sp_io::TestExternalities::new_empty().execute_with(|| { -- assert_eq!(Value::try_decode_entire_state(), Ok(0)); -- -- Value::put(42); -- assert_eq!(Value::try_decode_entire_state(), Ok(4)); -- -- Value::kill(); -- assert_eq!(Value::try_decode_entire_state(), Ok(0)); -- -- // two bytes, cannot be decoded into u32. -- sp_io::storage::set(&Value::hashed_key(), &[0u8, 1]); -- assert!(Value::try_decode_entire_state().is_err()); -- }) -- } -- -- #[test] -- fn try_decode_entire_state_map_works() { -- sp_io::TestExternalities::new_empty().execute_with(|| { -- assert_eq!(Map::try_decode_entire_state(), Ok(0)); -- -- Map::insert(0, 42); -- assert_eq!(Map::try_decode_entire_state(), Ok(4)); -- -- Map::insert(0, 42); -- assert_eq!(Map::try_decode_entire_state(), Ok(4)); -- -- Map::insert(1, 42); -- assert_eq!(Map::try_decode_entire_state(), Ok(8)); -- -- Map::remove(0); -- assert_eq!(Map::try_decode_entire_state(), Ok(4)); -- -- // two bytes, cannot be decoded into u32. -- sp_io::storage::set(&Map::hashed_key_for(2), &[0u8, 1]); -- assert!(Map::try_decode_entire_state().is_err()); -- }) -- } -- -- #[test] -- fn try_decode_entire_state_counted_map_works() { -- sp_io::TestExternalities::new_empty().execute_with(|| { -- // counter is not even initialized; -- assert_eq!(CMap::try_decode_entire_state(), Ok(0 + 0)); -- -- let counter = 4; -- let value_size = std::mem::size_of::(); -- -- CMap::insert(0, 42); -- assert_eq!(CMap::try_decode_entire_state(), Ok(value_size + counter)); -- -- CMap::insert(0, 42); -- assert_eq!(CMap::try_decode_entire_state(), Ok(value_size + counter)); -- -- CMap::insert(1, 42); -- assert_eq!(CMap::try_decode_entire_state(), Ok(value_size * 2 + counter)); -- -- CMap::remove(0); -- assert_eq!(CMap::try_decode_entire_state(), Ok(value_size + counter)); -- -- // counter is cleared again. -- let _ = CMap::clear(u32::MAX, None); -- assert_eq!(CMap::try_decode_entire_state(), Ok(0 + 0)); -- -- // 1 bytes, cannot be decoded into u16. -- sp_io::storage::set(&CMap::hashed_key_for(2), &[0u8]); -- assert!(CMap::try_decode_entire_state().is_err()); -- }) -- } -- -- #[test] -- fn try_decode_entire_state_double_works() { -- sp_io::TestExternalities::new_empty().execute_with(|| { -- assert_eq!(DMap::try_decode_entire_state(), Ok(0)); -- -- DMap::insert(0, 0, 42); -- assert_eq!(DMap::try_decode_entire_state(), Ok(4)); -- -- DMap::insert(0, 0, 42); -- assert_eq!(DMap::try_decode_entire_state(), Ok(4)); -- -- DMap::insert(0, 1, 42); -- assert_eq!(DMap::try_decode_entire_state(), Ok(8)); -- -- DMap::insert(1, 0, 42); -- assert_eq!(DMap::try_decode_entire_state(), Ok(12)); -- -- DMap::remove(0, 0); -- assert_eq!(DMap::try_decode_entire_state(), Ok(8)); -- -- // two bytes, cannot be decoded into u32. -- sp_io::storage::set(&DMap::hashed_key_for(1, 1), &[0u8, 1]); -- assert!(DMap::try_decode_entire_state().is_err()); -- }) -- } -- -- #[test] -- fn try_decode_entire_state_n_map_works() { -- sp_io::TestExternalities::new_empty().execute_with(|| { -- assert_eq!(NMap::try_decode_entire_state(), Ok(0)); -- -- let value_size = std::mem::size_of::(); -- -- NMap::insert((0u8, 0), 42); -- assert_eq!(NMap::try_decode_entire_state(), Ok(value_size)); -- -- NMap::insert((0, 0), 42); -- assert_eq!(NMap::try_decode_entire_state(), Ok(value_size)); -- -- NMap::insert((0, 1), 42); -- assert_eq!(NMap::try_decode_entire_state(), Ok(value_size * 2)); -- -- NMap::insert((1, 0), 42); -- assert_eq!(NMap::try_decode_entire_state(), Ok(value_size * 3)); -- -- NMap::remove((0, 0)); -- assert_eq!(NMap::try_decode_entire_state(), Ok(value_size * 2)); -- -- // two bytes, cannot be decoded into u128. -- sp_io::storage::set(&NMap::hashed_key_for((1, 1)), &[0u8, 1]); -- assert!(NMap::try_decode_entire_state().is_err()); -- }) -- } -- -- #[test] -- fn try_decode_entire_state_counted_n_map_works() { -- sp_io::TestExternalities::new_empty().execute_with(|| { -- sp_io::TestExternalities::new_empty().execute_with(|| { -- assert_eq!(NMap::try_decode_entire_state(), Ok(0)); -- -- let value_size = std::mem::size_of::(); -- let counter = 4; -- -- CNMap::insert((0u8, 0), 42); -- assert_eq!(CNMap::try_decode_entire_state(), Ok(value_size + counter)); -- -- CNMap::insert((0, 0), 42); -- assert_eq!(CNMap::try_decode_entire_state(), Ok(value_size + counter)); -- -- CNMap::insert((0, 1), 42); -- assert_eq!(CNMap::try_decode_entire_state(), Ok(value_size * 2 + counter)); -- -- CNMap::insert((1, 0), 42); -- assert_eq!(CNMap::try_decode_entire_state(), Ok(value_size * 3 + counter)); -- -- CNMap::remove((0, 0)); -- assert_eq!(CNMap::try_decode_entire_state(), Ok(value_size * 2 + counter)); -- -- // two bytes, cannot be decoded into u128. -- sp_io::storage::set(&CNMap::hashed_key_for((1, 1)), &[0u8, 1]); -- assert!(CNMap::try_decode_entire_state().is_err()); -- }) -- }) -- } -- -- #[test] -- fn extra_bytes_are_rejected() { -- sp_io::TestExternalities::new_empty().execute_with(|| { -- assert_eq!(Map::try_decode_entire_state(), Ok(0)); -- -- // 6bytes, too many to fit in u32, should be rejected. -- sp_io::storage::set(&Map::hashed_key_for(2), &[0u8, 1, 3, 4, 5, 6]); -- assert!(Map::try_decode_entire_state().is_err()); -- }) -- } -- -- #[test] -- fn try_decode_entire_state_tuple_of_storage_works() { -- sp_io::TestExternalities::new_empty().execute_with(|| { -- assert_eq!(<(Value, Map) as TryDecodeEntireStorage>::try_decode_entire_state(), Ok(0)); -- -- Value::put(42); -- assert_eq!(<(Value, Map) as TryDecodeEntireStorage>::try_decode_entire_state(), Ok(4)); -- -- Map::insert(0, 42); -- assert_eq!(<(Value, Map) as TryDecodeEntireStorage>::try_decode_entire_state(), Ok(8)); -- }); -- } --} -diff --git a/frame/support/test/tests/pallet_ui/call_argument_invalid_bound.stderr b/frame/support/test/tests/pallet_ui/call_argument_invalid_bound.stderr -index cc8c4fda76..d10bf13590 100644 ---- a/frame/support/test/tests/pallet_ui/call_argument_invalid_bound.stderr -+++ b/frame/support/test/tests/pallet_ui/call_argument_invalid_bound.stderr -@@ -19,7 +19,7 @@ error[E0277]: `::Bar` doesn't implement `std::fmt::Debug` - | - = help: the trait `std::fmt::Debug` is not implemented for `::Bar` - = note: required for `&::Bar` to implement `std::fmt::Debug` -- = note: required for the cast from `&&::Bar` to `&dyn std::fmt::Debug` -+ = note: required for the cast from `&::Bar` to the object type `dyn std::fmt::Debug` - - error[E0277]: the trait bound `::Bar: Clone` is not satisfied - --> tests/pallet_ui/call_argument_invalid_bound.rs:21:36 -diff --git a/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_2.stderr b/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_2.stderr -index b8d0bbf347..7173cdcd47 100644 ---- a/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_2.stderr -+++ b/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_2.stderr -@@ -19,7 +19,7 @@ error[E0277]: `::Bar` doesn't implement `std::fmt::Debug` - | - = help: the trait `std::fmt::Debug` is not implemented for `::Bar` - = note: required for `&::Bar` to implement `std::fmt::Debug` -- = note: required for the cast from `&&::Bar` to `&dyn std::fmt::Debug` -+ = note: required for the cast from `&::Bar` to the object type `dyn std::fmt::Debug` - - error[E0277]: the trait bound `::Bar: Clone` is not satisfied - --> tests/pallet_ui/call_argument_invalid_bound_2.rs:21:36 -diff --git a/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_3.stderr b/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_3.stderr -index 158d0c7638..4cbed37096 100644 ---- a/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_3.stderr -+++ b/frame/support/test/tests/pallet_ui/call_argument_invalid_bound_3.stderr -@@ -20,7 +20,7 @@ error[E0277]: `Bar` doesn't implement `std::fmt::Debug` - = help: the trait `std::fmt::Debug` is not implemented for `Bar` - = note: add `#[derive(Debug)]` to `Bar` or manually `impl std::fmt::Debug for Bar` - = note: required for `&Bar` to implement `std::fmt::Debug` -- = note: required for the cast from `&&Bar` to `&dyn std::fmt::Debug` -+ = note: required for the cast from `&Bar` to the object type `dyn std::fmt::Debug` - help: consider annotating `Bar` with `#[derive(Debug)]` - | - 17 + #[derive(Debug)] -diff --git a/frame/support/test/tests/pallet_ui/event_field_not_member.stderr b/frame/support/test/tests/pallet_ui/event_field_not_member.stderr -index d853406c95..1161f4a190 100644 ---- a/frame/support/test/tests/pallet_ui/event_field_not_member.stderr -+++ b/frame/support/test/tests/pallet_ui/event_field_not_member.stderr -@@ -18,4 +18,4 @@ error[E0277]: `::Bar` doesn't implement `std::fmt::Debug` - | - = help: the trait `std::fmt::Debug` is not implemented for `::Bar` - = note: required for `&::Bar` to implement `std::fmt::Debug` -- = note: required for the cast from `&&::Bar` to `&dyn std::fmt::Debug` -+ = note: required for the cast from `&::Bar` to the object type `dyn std::fmt::Debug` -diff --git a/frame/support/test/tests/pallet_ui/inherent_check_inner_span.stderr b/frame/support/test/tests/pallet_ui/inherent_check_inner_span.stderr -index 4acf57fb3e..bc34c55241 100644 ---- a/frame/support/test/tests/pallet_ui/inherent_check_inner_span.stderr -+++ b/frame/support/test/tests/pallet_ui/inherent_check_inner_span.stderr -@@ -1,11 +1,11 @@ - error[E0046]: not all trait items implemented, missing: `Call`, `Error`, `INHERENT_IDENTIFIER`, `create_inherent`, `is_inherent` -- --> tests/pallet_ui/inherent_check_inner_span.rs:19:2 -+ --> $DIR/inherent_check_inner_span.rs:19:2 - | - 19 | impl ProvideInherent for Pallet {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing `Call`, `Error`, `INHERENT_IDENTIFIER`, `create_inherent`, `is_inherent` in implementation - | -- = help: implement the missing item: `type Call = /* Type */;` -- = help: implement the missing item: `type Error = /* Type */;` -+ = help: implement the missing item: `type Call = Type;` -+ = help: implement the missing item: `type Error = Type;` - = help: implement the missing item: `const INHERENT_IDENTIFIER: [u8; 8] = value;` - = help: implement the missing item: `fn create_inherent(_: &InherentData) -> std::option::Option<::Call> { todo!() }` - = help: implement the missing item: `fn is_inherent(_: &::Call) -> bool { todo!() }` -diff --git a/frame/support/test/tests/pallet_ui/storage_info_unsatisfied_nmap.stderr b/frame/support/test/tests/pallet_ui/storage_info_unsatisfied_nmap.stderr -index bbbb3ed1b2..c34c796fe5 100644 ---- a/frame/support/test/tests/pallet_ui/storage_info_unsatisfied_nmap.stderr -+++ b/frame/support/test/tests/pallet_ui/storage_info_unsatisfied_nmap.stderr -@@ -14,5 +14,5 @@ error[E0277]: the trait bound `Bar: MaxEncodedLen` is not satisfied - (TupleElement0, TupleElement1, TupleElement2, TupleElement3, TupleElement4, TupleElement5, TupleElement6) - (TupleElement0, TupleElement1, TupleElement2, TupleElement3, TupleElement4, TupleElement5, TupleElement6, TupleElement7) - and $N others -- = note: required for `NMapKey` to implement `KeyGeneratorMaxEncodedLen` -- = note: required for `frame_support::pallet_prelude::StorageNMap<_GeneratedPrefixForStorageFoo, NMapKey, u32>` to implement `StorageInfoTrait` -+ = note: required for `Key` to implement `KeyGeneratorMaxEncodedLen` -+ = note: required for `frame_support::pallet_prelude::StorageNMap<_GeneratedPrefixForStorageFoo, Key, u32>` to implement `StorageInfoTrait` From ee5b1dfc4b3f240821ddfa7ac0fdddfeab3f6f84 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Sat, 21 Oct 2023 13:39:21 +0200 Subject: [PATCH 03/24] Make stuff compile Signed-off-by: Oliver Tale-Yazdi --- .../frame/support/procedural/src/pallet/expand/pallet_struct.rs | 2 ++ substrate/frame/support/src/storage/migration.rs | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/substrate/frame/support/procedural/src/pallet/expand/pallet_struct.rs b/substrate/frame/support/procedural/src/pallet/expand/pallet_struct.rs index c2102f0284db..e6fe1a2b556e 100644 --- a/substrate/frame/support/procedural/src/pallet/expand/pallet_struct.rs +++ b/substrate/frame/support/procedural/src/pallet/expand/pallet_struct.rs @@ -225,6 +225,8 @@ pub fn expand_pallet_struct(def: &mut Def) -> proc_macro2::TokenStream { } } + + // Implement `PalletInfoAccess` for `Pallet` impl<#type_impl_gen> #frame_support::traits::PalletInfoAccess for #pallet_ident<#type_use_gen> diff --git a/substrate/frame/support/src/storage/migration.rs b/substrate/frame/support/src/storage/migration.rs index e581c8a00a21..f5b1765fdb91 100644 --- a/substrate/frame/support/src/storage/migration.rs +++ b/substrate/frame/support/src/storage/migration.rs @@ -409,7 +409,7 @@ impl crate::traits::OnRuntime fn post_upgrade(_: Vec) -> Result<(), sp_runtime::TryRuntimeError> { let decoded = AllPallets::try_decode_entire_state()?; - crate::log::info!( + log::info!( target: crate::LOG_TARGET, "decoded the entire state, total size = {} bytes", decoded From 720ebb2db4b3f2842c19cb1bd6abef0937bbb401 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Sat, 21 Oct 2023 14:00:10 +0200 Subject: [PATCH 04/24] Introduce custom error type Signed-off-by: Oliver Tale-Yazdi --- .../procedural/src/pallet/expand/storage.rs | 4 +- .../frame/support/src/storage/migration.rs | 14 +++++- substrate/frame/support/src/traits.rs | 2 +- .../frame/support/src/traits/try_runtime.rs | 43 ++++++++++++++----- 4 files changed, 48 insertions(+), 15 deletions(-) diff --git a/substrate/frame/support/procedural/src/pallet/expand/storage.rs b/substrate/frame/support/procedural/src/pallet/expand/storage.rs index 191ec2e949bf..e8149ecc19cb 100644 --- a/substrate/frame/support/procedural/src/pallet/expand/storage.rs +++ b/substrate/frame/support/procedural/src/pallet/expand/storage.rs @@ -849,11 +849,11 @@ pub fn expand_storages(def: &mut Def) -> proc_macro2::TokenStream { impl<#type_impl_gen> #frame_support::traits::TryDecodeEntireStorage for #pallet_ident<#type_use_gen> #completed_where_clause { - fn try_decode_entire_state() -> Result { + fn try_decode_entire_state() -> Result { // simply delegate impl to a tuple of all storage items we have. // // NOTE: for now, we have to exclude storage items that are feature gated. - <( #( #storage_names ),*) as frame_support::traits::TryDecodeEntireStorage>::try_decode_entire_state() + <( #( #storage_names ),*) as #frame_support::traits::TryDecodeEntireStorage>::try_decode_entire_state() } } ) diff --git a/substrate/frame/support/src/storage/migration.rs b/substrate/frame/support/src/storage/migration.rs index f5b1765fdb91..3e8eceffed6e 100644 --- a/substrate/frame/support/src/storage/migration.rs +++ b/substrate/frame/support/src/storage/migration.rs @@ -408,12 +408,24 @@ impl crate::traits::OnRuntime } fn post_upgrade(_: Vec) -> Result<(), sp_runtime::TryRuntimeError> { - let decoded = AllPallets::try_decode_entire_state()?; + let decoded = match AllPallets::try_decode_entire_state() { + Ok(bytes) => bytes, + Err(err) => { + log::info!( + target: crate::LOG_TARGET, + "failed to decode the entire state: {}", err + ); + // NOTE: This only supports static strings. + return Err("failed to decode a value from the storage".into()) + } + }; + log::info!( target: crate::LOG_TARGET, "decoded the entire state, total size = {} bytes", decoded ); + Ok(()) } } diff --git a/substrate/frame/support/src/traits.rs b/substrate/frame/support/src/traits.rs index 1a5c79b82e59..b9979983f93a 100644 --- a/substrate/frame/support/src/traits.rs +++ b/substrate/frame/support/src/traits.rs @@ -126,5 +126,5 @@ pub use tx_pause::{TransactionPause, TransactionPauseError}; mod try_runtime; #[cfg(feature = "try-runtime")] pub use try_runtime::{ - Select as TryStateSelect, TryDecodeEntireStorage, TryState, UpgradeCheckSelect, + Select as TryStateSelect, TryDecodeEntireStorageError, TryDecodeEntireStorage, TryState, UpgradeCheckSelect, }; diff --git a/substrate/frame/support/src/traits/try_runtime.rs b/substrate/frame/support/src/traits/try_runtime.rs index 130e3f16722d..f7e7d215bdf2 100644 --- a/substrate/frame/support/src/traits/try_runtime.rs +++ b/substrate/frame/support/src/traits/try_runtime.rs @@ -63,26 +63,47 @@ impl Default for Select { /// decoded bytes if `Ok(_)`. pub trait TryDecodeEntireStorage { /// Decode the entire data under the given storage, returning `Ok(bytes_decoded)` if success. - fn try_decode_entire_state() -> Result; + fn try_decode_entire_state() -> Result; } #[cfg_attr(all(not(feature = "tuples-96"), not(feature = "tuples-128")), impl_for_tuples(64))] #[cfg_attr(all(feature = "tuples-96", not(feature = "tuples-128")), impl_for_tuples(96))] #[cfg_attr(feature = "tuples-128", impl_for_tuples(128))] impl TryDecodeEntireStorage for Tuple { - fn try_decode_entire_state() -> Result { + fn try_decode_entire_state() -> Result { let mut len = 0usize; for_tuples!( #( len = len.saturating_add(Tuple::try_decode_entire_state()?); )* ); Ok(len) } } +/// A value could not be decoded. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct TryDecodeEntireStorageError { + /// The key of the undecodable value. + key: Vec, + /// The raw value. + raw: Option>, + /// The storage info of the key. + info: StorageInfo, +} + +impl core::fmt::Display for TryDecodeEntireStorageError { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + write!( + f, + "Failed to decode value at key {:?} with storage info {:?}. Raw value: {:?}", + self.key, self.info, self.raw, + ) + } +} + /// Decode all the values based on the prefix of `info` to `V`. /// /// Basically, it decodes and sums up all the values who's key start with `info.prefix`. For values, /// this would be the value itself. For all sorts of maps, this should be all map items in the /// absence of key collision. -fn decode_storage_info(info: StorageInfo) -> Result { +fn decode_storage_info(info: StorageInfo) -> Result { let mut next_key = info.prefix.clone(); let mut decoded = 0; @@ -92,9 +113,9 @@ fn decode_storage_info(info: StorageInfo) -> Result::decode_all(&mut bytes.as_ref()).map_err(|_| { log::error!(target: crate::LOG_TARGET, "failed to decoded {:?}", info,); - "failed to decode value under existing key" + TryDecodeEntireStorageError { key: key.to_vec(), raw: Some(bytes.to_vec()), info: info.clone() } })?; - Ok::(len) + Ok::(len) }, }; @@ -120,7 +141,7 @@ where QueryKind: QueryKindTrait, OnEmpty: Get + 'static, { - fn try_decode_entire_state() -> Result { + fn try_decode_entire_state() -> Result { let info = Self::partial_storage_info() .first() .cloned() @@ -140,7 +161,7 @@ where OnEmpty: Get + 'static, MaxValues: Get>, { - fn try_decode_entire_state() -> Result { + fn try_decode_entire_state() -> Result { let info = Self::partial_storage_info() .first() .cloned() @@ -167,7 +188,7 @@ impl TryDecodeEntireS OnEmpty: Get + 'static, MaxValues: Get>, { - fn try_decode_entire_state() -> Result { + fn try_decode_entire_state() -> Result { let (map_info, counter_info) = match &Self::partial_storage_info()[..] { [a, b] => (a.clone(), b.clone()), _ => panic!("Counted map has two storage info items; qed"), @@ -201,7 +222,7 @@ impl OnEmpty: Get + 'static, MaxValues: Get>, { - fn try_decode_entire_state() -> Result { + fn try_decode_entire_state() -> Result { let info = Self::partial_storage_info() .first() .cloned() @@ -220,7 +241,7 @@ where OnEmpty: Get + 'static, MaxValues: Get>, { - fn try_decode_entire_state() -> Result { + fn try_decode_entire_state() -> Result { let info = Self::partial_storage_info() .first() .cloned() @@ -239,7 +260,7 @@ where OnEmpty: Get + 'static, MaxValues: Get>, { - fn try_decode_entire_state() -> Result { + fn try_decode_entire_state() -> Result { let (map_info, counter_info) = match &Self::partial_storage_info()[..] { [a, b] => (a.clone(), b.clone()), _ => panic!("Counted NMap has two storage info items; qed"), From 993c2c7d9764698e6eef89564e2248b74a49441f Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Sat, 21 Oct 2023 14:07:28 +0200 Subject: [PATCH 05/24] Move stuff into own files Signed-off-by: Oliver Tale-Yazdi --- .../src/pallet/expand/pallet_struct.rs | 2 +- .../frame/support/src/storage/migration.rs | 2 +- substrate/frame/support/src/traits.rs | 3 +- .../decode_entire_state.rs} | 178 +--------------- .../support/src/traits/try_runtime/mod.rs | 194 ++++++++++++++++++ 5 files changed, 205 insertions(+), 174 deletions(-) rename substrate/frame/support/src/traits/{try_runtime.rs => try_runtime/decode_entire_state.rs} (73%) create mode 100644 substrate/frame/support/src/traits/try_runtime/mod.rs diff --git a/substrate/frame/support/procedural/src/pallet/expand/pallet_struct.rs b/substrate/frame/support/procedural/src/pallet/expand/pallet_struct.rs index e6fe1a2b556e..32a5aa080aed 100644 --- a/substrate/frame/support/procedural/src/pallet/expand/pallet_struct.rs +++ b/substrate/frame/support/procedural/src/pallet/expand/pallet_struct.rs @@ -225,7 +225,7 @@ pub fn expand_pallet_struct(def: &mut Def) -> proc_macro2::TokenStream { } } - + // Implement `PalletInfoAccess` for `Pallet` impl<#type_impl_gen> #frame_support::traits::PalletInfoAccess diff --git a/substrate/frame/support/src/storage/migration.rs b/substrate/frame/support/src/storage/migration.rs index 3e8eceffed6e..cba9ca556eb0 100644 --- a/substrate/frame/support/src/storage/migration.rs +++ b/substrate/frame/support/src/storage/migration.rs @@ -417,7 +417,7 @@ impl crate::traits::OnRuntime ); // NOTE: This only supports static strings. return Err("failed to decode a value from the storage".into()) - } + }, }; log::info!( diff --git a/substrate/frame/support/src/traits.rs b/substrate/frame/support/src/traits.rs index b9979983f93a..cd503fbfee02 100644 --- a/substrate/frame/support/src/traits.rs +++ b/substrate/frame/support/src/traits.rs @@ -126,5 +126,6 @@ pub use tx_pause::{TransactionPause, TransactionPauseError}; mod try_runtime; #[cfg(feature = "try-runtime")] pub use try_runtime::{ - Select as TryStateSelect, TryDecodeEntireStorageError, TryDecodeEntireStorage, TryState, UpgradeCheckSelect, + Select as TryStateSelect, TryDecodeEntireStorage, TryDecodeEntireStorageError, TryState, + UpgradeCheckSelect, }; diff --git a/substrate/frame/support/src/traits/try_runtime.rs b/substrate/frame/support/src/traits/try_runtime/decode_entire_state.rs similarity index 73% rename from substrate/frame/support/src/traits/try_runtime.rs rename to substrate/frame/support/src/traits/try_runtime/decode_entire_state.rs index f7e7d215bdf2..73de0be60ed4 100644 --- a/substrate/frame/support/src/traits/try_runtime.rs +++ b/substrate/frame/support/src/traits/try_runtime/decode_entire_state.rs @@ -15,7 +15,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! Try-runtime specific traits and types. +//! Types to check that the entire storage can be decoded. use super::StorageInstance; use crate::{ @@ -28,32 +28,9 @@ use crate::{ }; use codec::{Decode, DecodeAll, FullCodec}; use impl_trait_for_tuples::impl_for_tuples; -use sp_arithmetic::traits::AtLeast32BitUnsigned; use sp_core::Get; -use sp_runtime::TryRuntimeError; use sp_std::prelude::*; -/// Which state tests to execute. -#[derive(codec::Encode, codec::Decode, Clone, scale_info::TypeInfo)] -pub enum Select { - /// None of them. - None, - /// All of them. - All, - /// Run a fixed number of them in a round robin manner. - RoundRobin(u32), - /// Run only pallets who's name matches the given list. - /// - /// Pallet names are obtained from [`super::PalletInfoAccess`]. - Only(Vec>), -} - -impl Default for Select { - fn default() -> Self { - Select::None - } -} - /// Decode the entire data under the given storage type. /// /// For values, this is trivial. For all kinds of maps, it should decode all the values associated @@ -113,7 +90,11 @@ fn decode_storage_info(info: StorageInfo) -> Result::decode_all(&mut bytes.as_ref()).map_err(|_| { log::error!(target: crate::LOG_TARGET, "failed to decoded {:?}", info,); - TryDecodeEntireStorageError { key: key.to_vec(), raw: Some(bytes.to_vec()), info: info.clone() } + TryDecodeEntireStorageError { + key: key.to_vec(), + raw: Some(bytes.to_vec()), + info: info.clone(), + } })?; Ok::(len) }, @@ -272,156 +253,11 @@ where } } -impl sp_std::fmt::Debug for Select { - fn fmt(&self, f: &mut sp_std::fmt::Formatter<'_>) -> sp_std::fmt::Result { - match self { - Select::RoundRobin(x) => write!(f, "RoundRobin({})", x), - Select::Only(x) => write!( - f, - "Only({:?})", - x.iter() - .map(|x| sp_std::str::from_utf8(x).unwrap_or("")) - .collect::>(), - ), - Select::All => write!(f, "All"), - Select::None => write!(f, "None"), - } - } -} - -#[cfg(feature = "std")] -impl sp_std::str::FromStr for Select { - type Err = &'static str; - fn from_str(s: &str) -> Result { - match s { - "all" | "All" => Ok(Select::All), - "none" | "None" => Ok(Select::None), - _ => - if s.starts_with("rr-") { - let count = s - .split_once('-') - .and_then(|(_, count)| count.parse::().ok()) - .ok_or("failed to parse count")?; - Ok(Select::RoundRobin(count)) - } else { - let pallets = s.split(',').map(|x| x.as_bytes().to_vec()).collect::>(); - Ok(Select::Only(pallets)) - }, - } - } -} - -/// Select which checks should be run when trying a runtime upgrade upgrade. -#[derive(codec::Encode, codec::Decode, Clone, Debug, Copy, scale_info::TypeInfo)] -pub enum UpgradeCheckSelect { - /// Run no checks. - None, - /// Run the `try_state`, `pre_upgrade` and `post_upgrade` checks. - All, - /// Run the `pre_upgrade` and `post_upgrade` checks. - PreAndPost, - /// Run the `try_state` checks. - TryState, -} - -impl UpgradeCheckSelect { - /// Whether the pre- and post-upgrade checks are selected. - pub fn pre_and_post(&self) -> bool { - matches!(self, Self::All | Self::PreAndPost) - } - - /// Whether the try-state checks are selected. - pub fn try_state(&self) -> bool { - matches!(self, Self::All | Self::TryState) - } -} - -#[cfg(feature = "std")] -impl core::str::FromStr for UpgradeCheckSelect { - type Err = &'static str; - - fn from_str(s: &str) -> Result { - match s.to_lowercase().as_str() { - "none" => Ok(Self::None), - "all" => Ok(Self::All), - "pre-and-post" => Ok(Self::PreAndPost), - "try-state" => Ok(Self::TryState), - _ => Err("Invalid CheckSelector"), - } - } -} - -/// Execute some checks to ensure the internal state of a pallet is consistent. -/// -/// Usually, these checks should check all of the invariants that are expected to be held on all of -/// the storage items of your pallet. -/// -/// This hook should not alter any storage. -pub trait TryState { - /// Execute the state checks. - fn try_state(_: BlockNumber, _: Select) -> Result<(), TryRuntimeError>; -} - -#[cfg_attr(all(not(feature = "tuples-96"), not(feature = "tuples-128")), impl_for_tuples(64))] -#[cfg_attr(all(feature = "tuples-96", not(feature = "tuples-128")), impl_for_tuples(96))] -#[cfg_attr(all(feature = "tuples-128"), impl_for_tuples(128))] -impl TryState - for Tuple -{ - for_tuples!( where #( Tuple: crate::traits::PalletInfoAccess )* ); - fn try_state(n: BlockNumber, targets: Select) -> Result<(), TryRuntimeError> { - match targets { - Select::None => Ok(()), - Select::All => { - let mut result = Ok(()); - for_tuples!( #( result = result.and(Tuple::try_state(n.clone(), targets.clone())); )* ); - result - }, - Select::RoundRobin(len) => { - let functions: &[fn(BlockNumber, Select) -> Result<(), TryRuntimeError>] = - &[for_tuples!(#( Tuple::try_state ),*)]; - let skip = n.clone() % (functions.len() as u32).into(); - let skip: u32 = - skip.try_into().unwrap_or_else(|_| sp_runtime::traits::Bounded::max_value()); - let mut result = Ok(()); - for try_state_fn in functions.iter().cycle().skip(skip as usize).take(len as usize) - { - result = result.and(try_state_fn(n.clone(), targets.clone())); - } - result - }, - Select::Only(ref pallet_names) => { - let try_state_fns: &[( - &'static str, - fn(BlockNumber, Select) -> Result<(), TryRuntimeError>, - )] = &[for_tuples!( - #( (::name(), Tuple::try_state) ),* - )]; - let mut result = Ok(()); - pallet_names.iter().for_each(|pallet_name| { - if let Some((name, try_state_fn)) = - try_state_fns.iter().find(|(name, _)| name.as_bytes() == pallet_name) - { - result = result.and(try_state_fn(n.clone(), targets.clone())); - } else { - log::warn!( - "Pallet {:?} not found", - sp_std::str::from_utf8(pallet_name).unwrap_or_default() - ); - } - }); - - result - }, - } - } -} - #[cfg(test)] mod tests { use super::*; use crate::{ - storage::types::{self, Key}, + storage::types::{self, CountedStorageMapInstance, CountedStorageNMapInstance, Key}, Blake2_128Concat, }; diff --git a/substrate/frame/support/src/traits/try_runtime/mod.rs b/substrate/frame/support/src/traits/try_runtime/mod.rs new file mode 100644 index 000000000000..4379b9df959b --- /dev/null +++ b/substrate/frame/support/src/traits/try_runtime/mod.rs @@ -0,0 +1,194 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Try-runtime specific traits and types. + +pub mod decode_entire_state; +pub use decode_entire_state::{TryDecodeEntireStorage, TryDecodeEntireStorageError}; + +use super::StorageInstance; + +use impl_trait_for_tuples::impl_for_tuples; +use sp_arithmetic::traits::AtLeast32BitUnsigned; +use sp_runtime::TryRuntimeError; +use sp_std::prelude::*; + +/// Which state tests to execute. +#[derive(codec::Encode, codec::Decode, Clone, scale_info::TypeInfo)] +pub enum Select { + /// None of them. + None, + /// All of them. + All, + /// Run a fixed number of them in a round robin manner. + RoundRobin(u32), + /// Run only pallets who's name matches the given list. + /// + /// Pallet names are obtained from [`super::PalletInfoAccess`]. + Only(Vec>), +} + +impl Default for Select { + fn default() -> Self { + Select::None + } +} + +impl sp_std::fmt::Debug for Select { + fn fmt(&self, f: &mut sp_std::fmt::Formatter<'_>) -> sp_std::fmt::Result { + match self { + Select::RoundRobin(x) => write!(f, "RoundRobin({})", x), + Select::Only(x) => write!( + f, + "Only({:?})", + x.iter() + .map(|x| sp_std::str::from_utf8(x).unwrap_or("")) + .collect::>(), + ), + Select::All => write!(f, "All"), + Select::None => write!(f, "None"), + } + } +} + +#[cfg(feature = "std")] +impl sp_std::str::FromStr for Select { + type Err = &'static str; + fn from_str(s: &str) -> Result { + match s { + "all" | "All" => Ok(Select::All), + "none" | "None" => Ok(Select::None), + _ => + if s.starts_with("rr-") { + let count = s + .split_once('-') + .and_then(|(_, count)| count.parse::().ok()) + .ok_or("failed to parse count")?; + Ok(Select::RoundRobin(count)) + } else { + let pallets = s.split(',').map(|x| x.as_bytes().to_vec()).collect::>(); + Ok(Select::Only(pallets)) + }, + } + } +} + +/// Select which checks should be run when trying a runtime upgrade upgrade. +#[derive(codec::Encode, codec::Decode, Clone, Debug, Copy, scale_info::TypeInfo)] +pub enum UpgradeCheckSelect { + /// Run no checks. + None, + /// Run the `try_state`, `pre_upgrade` and `post_upgrade` checks. + All, + /// Run the `pre_upgrade` and `post_upgrade` checks. + PreAndPost, + /// Run the `try_state` checks. + TryState, +} + +impl UpgradeCheckSelect { + /// Whether the pre- and post-upgrade checks are selected. + pub fn pre_and_post(&self) -> bool { + matches!(self, Self::All | Self::PreAndPost) + } + + /// Whether the try-state checks are selected. + pub fn try_state(&self) -> bool { + matches!(self, Self::All | Self::TryState) + } +} + +#[cfg(feature = "std")] +impl core::str::FromStr for UpgradeCheckSelect { + type Err = &'static str; + + fn from_str(s: &str) -> Result { + match s.to_lowercase().as_str() { + "none" => Ok(Self::None), + "all" => Ok(Self::All), + "pre-and-post" => Ok(Self::PreAndPost), + "try-state" => Ok(Self::TryState), + _ => Err("Invalid CheckSelector"), + } + } +} + +/// Execute some checks to ensure the internal state of a pallet is consistent. +/// +/// Usually, these checks should check all of the invariants that are expected to be held on all of +/// the storage items of your pallet. +/// +/// This hook should not alter any storage. +pub trait TryState { + /// Execute the state checks. + fn try_state(_: BlockNumber, _: Select) -> Result<(), TryRuntimeError>; +} + +#[cfg_attr(all(not(feature = "tuples-96"), not(feature = "tuples-128")), impl_for_tuples(64))] +#[cfg_attr(all(feature = "tuples-96", not(feature = "tuples-128")), impl_for_tuples(96))] +#[cfg_attr(all(feature = "tuples-128"), impl_for_tuples(128))] +impl TryState + for Tuple +{ + for_tuples!( where #( Tuple: crate::traits::PalletInfoAccess )* ); + fn try_state(n: BlockNumber, targets: Select) -> Result<(), TryRuntimeError> { + match targets { + Select::None => Ok(()), + Select::All => { + let mut result = Ok(()); + for_tuples!( #( result = result.and(Tuple::try_state(n.clone(), targets.clone())); )* ); + result + }, + Select::RoundRobin(len) => { + let functions: &[fn(BlockNumber, Select) -> Result<(), TryRuntimeError>] = + &[for_tuples!(#( Tuple::try_state ),*)]; + let skip = n.clone() % (functions.len() as u32).into(); + let skip: u32 = + skip.try_into().unwrap_or_else(|_| sp_runtime::traits::Bounded::max_value()); + let mut result = Ok(()); + for try_state_fn in functions.iter().cycle().skip(skip as usize).take(len as usize) + { + result = result.and(try_state_fn(n.clone(), targets.clone())); + } + result + }, + Select::Only(ref pallet_names) => { + let try_state_fns: &[( + &'static str, + fn(BlockNumber, Select) -> Result<(), TryRuntimeError>, + )] = &[for_tuples!( + #( (::name(), Tuple::try_state) ),* + )]; + let mut result = Ok(()); + pallet_names.iter().for_each(|pallet_name| { + if let Some((name, try_state_fn)) = + try_state_fns.iter().find(|(name, _)| name.as_bytes() == pallet_name) + { + result = result.and(try_state_fn(n.clone(), targets.clone())); + } else { + log::warn!( + "Pallet {:?} not found", + sp_std::str::from_utf8(pallet_name).unwrap_or_default() + ); + } + }); + + result + }, + } + } +} From 61883babd392edec619eeffd3fb32768a9e978c9 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Sat, 21 Oct 2023 14:21:23 +0200 Subject: [PATCH 06/24] Cleanup Signed-off-by: Oliver Tale-Yazdi --- .../frame/support/procedural/src/pallet/expand/pallet_struct.rs | 2 -- substrate/frame/support/src/storage/migration.rs | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/substrate/frame/support/procedural/src/pallet/expand/pallet_struct.rs b/substrate/frame/support/procedural/src/pallet/expand/pallet_struct.rs index 32a5aa080aed..c2102f0284db 100644 --- a/substrate/frame/support/procedural/src/pallet/expand/pallet_struct.rs +++ b/substrate/frame/support/procedural/src/pallet/expand/pallet_struct.rs @@ -225,8 +225,6 @@ pub fn expand_pallet_struct(def: &mut Def) -> proc_macro2::TokenStream { } } - - // Implement `PalletInfoAccess` for `Pallet` impl<#type_impl_gen> #frame_support::traits::PalletInfoAccess for #pallet_ident<#type_use_gen> diff --git a/substrate/frame/support/src/storage/migration.rs b/substrate/frame/support/src/storage/migration.rs index cba9ca556eb0..f60409dcb9e9 100644 --- a/substrate/frame/support/src/storage/migration.rs +++ b/substrate/frame/support/src/storage/migration.rs @@ -415,7 +415,7 @@ impl crate::traits::OnRuntime target: crate::LOG_TARGET, "failed to decode the entire state: {}", err ); - // NOTE: This only supports static strings. + // NOTE: This only supports static strings, so we cannot bubble up the exact error. return Err("failed to decode a value from the storage".into()) }, }; From 1cf9ae2e0ddbf788c2331590f63e4308378747e6 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Sat, 21 Oct 2023 14:28:20 +0200 Subject: [PATCH 07/24] Add prdoc Signed-off-by: Oliver Tale-Yazdi --- prdoc/pr_1805.prdoc | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 prdoc/pr_1805.prdoc diff --git a/prdoc/pr_1805.prdoc b/prdoc/pr_1805.prdoc new file mode 100644 index 000000000000..efb0b7565e3d --- /dev/null +++ b/prdoc/pr_1805.prdoc @@ -0,0 +1,23 @@ +title: Introduce state decoding check after runtime upgrades. + +doc: + - audience: Core Dev + description: | + Adds a `TryDecodeEntireState` migration that can be optionally included into the list of migrations of a runtime. It will try to decode the state of all pallets to spot any decoding errors that might have been introduced by missing a migration. + +migrations: + db: [] + + runtime: [] + +crates: + - name: frame-support + semver: minor + - name: frame-support-procedural + semver: minor + - name: kitchensink-runtime + semver: minor + - name: node-template-runtime + semver: minor + +host_functions: [] From 7e4394d4b0b5049c953230794a0b023c0df32f45 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Sat, 21 Oct 2023 15:15:13 +0200 Subject: [PATCH 08/24] Update UI tests Signed-off-by: Oliver Tale-Yazdi --- ...age_ensure_span_are_ok_on_wrong_gen.stderr | 59 +++++++++++++++++++ ...re_span_are_ok_on_wrong_gen_unnamed.stderr | 59 +++++++++++++++++++ 2 files changed, 118 insertions(+) diff --git a/substrate/frame/support/test/tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen.stderr b/substrate/frame/support/test/tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen.stderr index 930af1d7fcb3..7375bcd2f16a 100644 --- a/substrate/frame/support/test/tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen.stderr +++ b/substrate/frame/support/test/tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen.stderr @@ -128,3 +128,62 @@ error[E0277]: the trait bound `Bar: WrapperTypeEncode` is not satisfied = note: required for `Bar` to implement `FullEncode` = note: required for `Bar` to implement `FullCodec` = note: required for `frame_support::pallet_prelude::StorageValue<_GeneratedPrefixForStorageFoo, Bar>` to implement `StorageEntryMetadataBuilder` + +error[E0277]: the trait bound `Bar: WrapperTypeDecode` is not satisfied + --> tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen.rs:18:1 + | +18 | #[frame_support::pallet] + | ^^^^^^^^^^^^^^^^^^^^^^^^ the trait `WrapperTypeDecode` is not implemented for `Bar` + | + = help: the following other types implement trait `WrapperTypeDecode`: + Box + frame_support::sp_runtime::sp_application_crypto::sp_core::Bytes + Rc + Arc + = note: required for `Bar` to implement `Decode` + = note: required for `Bar` to implement `FullCodec` + = note: required for `frame_support::pallet_prelude::StorageValue<_GeneratedPrefixForStorageFoo, Bar>` to implement `TryDecodeEntireStorage` + = note: this error originates in the attribute macro `frame_support::pallet` (in Nightly builds, run with -Z macro-backtrace for more info) + +error[E0277]: the trait bound `Bar: EncodeLike` is not satisfied + --> tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen.rs:18:1 + | +18 | #[frame_support::pallet] + | ^^^^^^^^^^^^^^^^^^^^^^^^ the trait `EncodeLike` is not implemented for `Bar` + | + = help: the following other types implement trait `EncodeLike`: + + + + + + + + + and $N others + = note: required for `Bar` to implement `FullEncode` + = note: required for `Bar` to implement `FullCodec` + = note: required for `frame_support::pallet_prelude::StorageValue<_GeneratedPrefixForStorageFoo, Bar>` to implement `TryDecodeEntireStorage` + = note: this error originates in the attribute macro `frame_support::pallet` (in Nightly builds, run with -Z macro-backtrace for more info) + +error[E0277]: the trait bound `Bar: WrapperTypeEncode` is not satisfied + --> tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen.rs:18:1 + | +18 | #[frame_support::pallet] + | ^^^^^^^^^^^^^^^^^^^^^^^^ the trait `WrapperTypeEncode` is not implemented for `Bar` + | + = help: the following other types implement trait `WrapperTypeEncode`: + Box + bytes::bytes::Bytes + Cow<'a, T> + parity_scale_codec::Ref<'a, T, U> + frame_support::sp_runtime::sp_application_crypto::sp_core::Bytes + Rc + Arc + Vec + and $N others + = note: required for `Bar` to implement `Encode` + = note: required for `Bar` to implement `FullEncode` + = note: required for `Bar` to implement `FullCodec` + = note: required for `frame_support::pallet_prelude::StorageValue<_GeneratedPrefixForStorageFoo, Bar>` to implement `TryDecodeEntireStorage` + = note: this error originates in the attribute macro `frame_support::pallet` (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/substrate/frame/support/test/tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen_unnamed.stderr b/substrate/frame/support/test/tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen_unnamed.stderr index 79798963c8b1..3a0a25712aaf 100644 --- a/substrate/frame/support/test/tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen_unnamed.stderr +++ b/substrate/frame/support/test/tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen_unnamed.stderr @@ -128,3 +128,62 @@ error[E0277]: the trait bound `Bar: WrapperTypeEncode` is not satisfied = note: required for `Bar` to implement `FullEncode` = note: required for `Bar` to implement `FullCodec` = note: required for `frame_support::pallet_prelude::StorageValue<_GeneratedPrefixForStorageFoo, Bar>` to implement `StorageEntryMetadataBuilder` + +error[E0277]: the trait bound `Bar: WrapperTypeDecode` is not satisfied + --> tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen_unnamed.rs:18:1 + | +18 | #[frame_support::pallet] + | ^^^^^^^^^^^^^^^^^^^^^^^^ the trait `WrapperTypeDecode` is not implemented for `Bar` + | + = help: the following other types implement trait `WrapperTypeDecode`: + Box + frame_support::sp_runtime::sp_application_crypto::sp_core::Bytes + Rc + Arc + = note: required for `Bar` to implement `Decode` + = note: required for `Bar` to implement `FullCodec` + = note: required for `frame_support::pallet_prelude::StorageValue<_GeneratedPrefixForStorageFoo, Bar>` to implement `TryDecodeEntireStorage` + = note: this error originates in the attribute macro `frame_support::pallet` (in Nightly builds, run with -Z macro-backtrace for more info) + +error[E0277]: the trait bound `Bar: EncodeLike` is not satisfied + --> tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen_unnamed.rs:18:1 + | +18 | #[frame_support::pallet] + | ^^^^^^^^^^^^^^^^^^^^^^^^ the trait `EncodeLike` is not implemented for `Bar` + | + = help: the following other types implement trait `EncodeLike`: + + + + + + + + + and $N others + = note: required for `Bar` to implement `FullEncode` + = note: required for `Bar` to implement `FullCodec` + = note: required for `frame_support::pallet_prelude::StorageValue<_GeneratedPrefixForStorageFoo, Bar>` to implement `TryDecodeEntireStorage` + = note: this error originates in the attribute macro `frame_support::pallet` (in Nightly builds, run with -Z macro-backtrace for more info) + +error[E0277]: the trait bound `Bar: WrapperTypeEncode` is not satisfied + --> tests/pallet_ui/storage_ensure_span_are_ok_on_wrong_gen_unnamed.rs:18:1 + | +18 | #[frame_support::pallet] + | ^^^^^^^^^^^^^^^^^^^^^^^^ the trait `WrapperTypeEncode` is not implemented for `Bar` + | + = help: the following other types implement trait `WrapperTypeEncode`: + Box + bytes::bytes::Bytes + Cow<'a, T> + parity_scale_codec::Ref<'a, T, U> + frame_support::sp_runtime::sp_application_crypto::sp_core::Bytes + Rc + Arc + Vec + and $N others + = note: required for `Bar` to implement `Encode` + = note: required for `Bar` to implement `FullEncode` + = note: required for `Bar` to implement `FullCodec` + = note: required for `frame_support::pallet_prelude::StorageValue<_GeneratedPrefixForStorageFoo, Bar>` to implement `TryDecodeEntireStorage` + = note: this error originates in the attribute macro `frame_support::pallet` (in Nightly builds, run with -Z macro-backtrace for more info) From 30569acf9706d0ab08851423277b6ffd2e8f87a0 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Sat, 21 Oct 2023 15:15:35 +0200 Subject: [PATCH 09/24] Add to Rococo and Westend runtimes Signed-off-by: Oliver Tale-Yazdi --- polkadot/runtime/rococo/src/lib.rs | 11 ++++++++++- polkadot/runtime/westend/src/lib.rs | 11 ++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/polkadot/runtime/rococo/src/lib.rs b/polkadot/runtime/rococo/src/lib.rs index f4264ea35336..ff738d0f274f 100644 --- a/polkadot/runtime/rococo/src/lib.rs +++ b/polkadot/runtime/rococo/src/lib.rs @@ -1407,7 +1407,10 @@ pub type UncheckedExtrinsic = /// /// This contains the combined migrations of the last 10 releases. It allows to skip runtime /// upgrades in case governance decides to do so. THE ORDER IS IMPORTANT. -pub type Migrations = migrations::Unreleased; +pub type Migrations = ( + migrations::Unreleased, + migrations::Perpetual +); /// The runtime migrations per release. #[allow(deprecated, missing_docs)] @@ -1488,6 +1491,12 @@ pub mod migrations { frame_support::migrations::RemovePallet::DbWeight>, frame_support::migrations::RemovePallet::DbWeight>, ); + + /// Migrations that should run in *every* upgrade. Do not change on release! + pub type Perpetual = ( + // This must be last: + frame_support::storage::migration::EnsureStateDecodes, + ); } /// Executive: handles dispatch to the various modules. diff --git a/polkadot/runtime/westend/src/lib.rs b/polkadot/runtime/westend/src/lib.rs index a7ddfc52ce6c..7da2aa46aa2f 100644 --- a/polkadot/runtime/westend/src/lib.rs +++ b/polkadot/runtime/westend/src/lib.rs @@ -1527,7 +1527,10 @@ impl Get for NominationPoolsMigrationV4OldPallet { /// /// This contains the combined migrations of the last 10 releases. It allows to skip runtime /// upgrades in case governance decides to do so. THE ORDER IS IMPORTANT. -pub type Migrations = migrations::Unreleased; +pub type Migrations = ( + migrations::Unreleased, + migrations::Perpetual +); /// The runtime migrations per release. #[allow(deprecated, missing_docs)] @@ -1558,6 +1561,12 @@ pub mod migrations { pallet_referenda::migration::v1::MigrateV0ToV1, pallet_nomination_pools::migration::versioned_migrations::V6ToV7, ); + + /// Migrations that should run in *every* upgrade. Do not change on release! + pub type Perpetual = ( + // This must be last: + frame_support::storage::migration::EnsureStateDecodes, + ); } /// Unchecked extrinsic type as expected by this runtime. From 30bb3b1726cb453d4a37088e23506585b52efc2c Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Sat, 21 Oct 2023 15:15:54 +0200 Subject: [PATCH 10/24] Add log Signed-off-by: Oliver Tale-Yazdi --- .../procedural/src/pallet/expand/storage.rs | 22 +++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/substrate/frame/support/procedural/src/pallet/expand/storage.rs b/substrate/frame/support/procedural/src/pallet/expand/storage.rs index e8149ecc19cb..03cdca8035ed 100644 --- a/substrate/frame/support/procedural/src/pallet/expand/storage.rs +++ b/substrate/frame/support/procedural/src/pallet/expand/storage.rs @@ -830,7 +830,7 @@ pub fn expand_storages(def: &mut Def) -> proc_macro2::TokenStream { let type_use_gen = &def.type_use_generics(proc_macro2::Span::call_site()); let try_decode_entire_state = { - let storage_names = def + let mut storage_names = def .storages .iter() .filter_map(|storage| { @@ -843,6 +843,7 @@ pub fn expand_storages(def: &mut Def) -> proc_macro2::TokenStream { } }) .collect::>(); + storage_names.sort_by_cached_key(|ident| ident.to_string()); quote::quote!( #[cfg(feature = "try-runtime")] @@ -850,10 +851,23 @@ pub fn expand_storages(def: &mut Def) -> proc_macro2::TokenStream { for #pallet_ident<#type_use_gen> #completed_where_clause { fn try_decode_entire_state() -> Result { - // simply delegate impl to a tuple of all storage items we have. - // + let pallet_name = <::PalletInfo as frame_support::traits::PalletInfo> + ::name::<#pallet_ident<#type_use_gen>>() + .expect("Every active pallet has a name in the runtime; qed"); + + #frame_support::__private::log::debug!(target: "try-decode-state", "trying to decode pallet: {pallet_name}"); + // NOTE: for now, we have to exclude storage items that are feature gated. - <( #( #storage_names ),*) as #frame_support::traits::TryDecodeEntireStorage>::try_decode_entire_state() + let mut decoded = 0usize; + #( + #frame_support::__private::log::debug!(target: "try-decode-state", "trying to decode storage: \ + {pallet_name}::{}", stringify!(#storage_names)); + + decoded += + <#storage_names as #frame_support::traits::TryDecodeEntireStorage>::try_decode_entire_state()?; + )* + + Ok(decoded) } } ) From e2e22e3058dbc5540705397dbabfdf24e4f6c4e9 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Sat, 21 Oct 2023 15:16:04 +0200 Subject: [PATCH 11/24] fmt Signed-off-by: Oliver Tale-Yazdi --- polkadot/runtime/rococo/src/lib.rs | 5 +---- polkadot/runtime/westend/src/lib.rs | 5 +---- .../frame/support/procedural/src/pallet/expand/storage.rs | 4 ++-- 3 files changed, 4 insertions(+), 10 deletions(-) diff --git a/polkadot/runtime/rococo/src/lib.rs b/polkadot/runtime/rococo/src/lib.rs index ff738d0f274f..fa8642dc93a4 100644 --- a/polkadot/runtime/rococo/src/lib.rs +++ b/polkadot/runtime/rococo/src/lib.rs @@ -1407,10 +1407,7 @@ pub type UncheckedExtrinsic = /// /// This contains the combined migrations of the last 10 releases. It allows to skip runtime /// upgrades in case governance decides to do so. THE ORDER IS IMPORTANT. -pub type Migrations = ( - migrations::Unreleased, - migrations::Perpetual -); +pub type Migrations = (migrations::Unreleased, migrations::Perpetual); /// The runtime migrations per release. #[allow(deprecated, missing_docs)] diff --git a/polkadot/runtime/westend/src/lib.rs b/polkadot/runtime/westend/src/lib.rs index 7da2aa46aa2f..af0ddfaafab0 100644 --- a/polkadot/runtime/westend/src/lib.rs +++ b/polkadot/runtime/westend/src/lib.rs @@ -1527,10 +1527,7 @@ impl Get for NominationPoolsMigrationV4OldPallet { /// /// This contains the combined migrations of the last 10 releases. It allows to skip runtime /// upgrades in case governance decides to do so. THE ORDER IS IMPORTANT. -pub type Migrations = ( - migrations::Unreleased, - migrations::Perpetual -); +pub type Migrations = (migrations::Unreleased, migrations::Perpetual); /// The runtime migrations per release. #[allow(deprecated, missing_docs)] diff --git a/substrate/frame/support/procedural/src/pallet/expand/storage.rs b/substrate/frame/support/procedural/src/pallet/expand/storage.rs index 03cdca8035ed..ae3aeec4c466 100644 --- a/substrate/frame/support/procedural/src/pallet/expand/storage.rs +++ b/substrate/frame/support/procedural/src/pallet/expand/storage.rs @@ -856,13 +856,13 @@ pub fn expand_storages(def: &mut Def) -> proc_macro2::TokenStream { .expect("Every active pallet has a name in the runtime; qed"); #frame_support::__private::log::debug!(target: "try-decode-state", "trying to decode pallet: {pallet_name}"); - + // NOTE: for now, we have to exclude storage items that are feature gated. let mut decoded = 0usize; #( #frame_support::__private::log::debug!(target: "try-decode-state", "trying to decode storage: \ {pallet_name}::{}", stringify!(#storage_names)); - + decoded += <#storage_names as #frame_support::traits::TryDecodeEntireStorage>::try_decode_entire_state()?; )* From 25741db0a9ee981f3033595256b80c850eee4c5f Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Tue, 24 Oct 2023 12:57:58 +0200 Subject: [PATCH 12/24] Remove migration struct and call directly from executive Signed-off-by: Oliver Tale-Yazdi --- polkadot/runtime/rococo/src/lib.rs | 8 +-- polkadot/runtime/westend/src/lib.rs | 8 +-- substrate/frame/executive/src/lib.rs | 57 +++++++++++++------ .../frame/support/src/storage/migration.rs | 45 --------------- .../support/src/traits/try_runtime/mod.rs | 5 ++ 5 files changed, 47 insertions(+), 76 deletions(-) diff --git a/polkadot/runtime/rococo/src/lib.rs b/polkadot/runtime/rococo/src/lib.rs index fa8642dc93a4..f4264ea35336 100644 --- a/polkadot/runtime/rococo/src/lib.rs +++ b/polkadot/runtime/rococo/src/lib.rs @@ -1407,7 +1407,7 @@ pub type UncheckedExtrinsic = /// /// This contains the combined migrations of the last 10 releases. It allows to skip runtime /// upgrades in case governance decides to do so. THE ORDER IS IMPORTANT. -pub type Migrations = (migrations::Unreleased, migrations::Perpetual); +pub type Migrations = migrations::Unreleased; /// The runtime migrations per release. #[allow(deprecated, missing_docs)] @@ -1488,12 +1488,6 @@ pub mod migrations { frame_support::migrations::RemovePallet::DbWeight>, frame_support::migrations::RemovePallet::DbWeight>, ); - - /// Migrations that should run in *every* upgrade. Do not change on release! - pub type Perpetual = ( - // This must be last: - frame_support::storage::migration::EnsureStateDecodes, - ); } /// Executive: handles dispatch to the various modules. diff --git a/polkadot/runtime/westend/src/lib.rs b/polkadot/runtime/westend/src/lib.rs index af0ddfaafab0..a7ddfc52ce6c 100644 --- a/polkadot/runtime/westend/src/lib.rs +++ b/polkadot/runtime/westend/src/lib.rs @@ -1527,7 +1527,7 @@ impl Get for NominationPoolsMigrationV4OldPallet { /// /// This contains the combined migrations of the last 10 releases. It allows to skip runtime /// upgrades in case governance decides to do so. THE ORDER IS IMPORTANT. -pub type Migrations = (migrations::Unreleased, migrations::Perpetual); +pub type Migrations = migrations::Unreleased; /// The runtime migrations per release. #[allow(deprecated, missing_docs)] @@ -1558,12 +1558,6 @@ pub mod migrations { pallet_referenda::migration::v1::MigrateV0ToV1, pallet_nomination_pools::migration::versioned_migrations::V6ToV7, ); - - /// Migrations that should run in *every* upgrade. Do not change on release! - pub type Perpetual = ( - // This must be last: - frame_support::storage::migration::EnsureStateDecodes, - ); } /// Unchecked extrinsic type as expected by this runtime. diff --git a/substrate/frame/executive/src/lib.rs b/substrate/frame/executive/src/lib.rs index 1d9afdfa60aa..11ed6626c29e 100644 --- a/substrate/frame/executive/src/lib.rs +++ b/substrate/frame/executive/src/lib.rs @@ -139,9 +139,15 @@ use sp_runtime::{ use sp_std::{marker::PhantomData, prelude::*}; #[cfg(feature = "try-runtime")] -use log; -#[cfg(feature = "try-runtime")] -use sp_runtime::TryRuntimeError; +use ::{ + frame_support::{ + traits::{TryDecodeEntireStorage, TryState}, + StorageNoopGuard, + }, + frame_try_runtime::{TryStateSelect, UpgradeCheckSelect}, + log, + sp_runtime::TryRuntimeError, +}; #[allow(dead_code)] const LOG_TARGET: &str = "runtime::executive"; @@ -229,7 +235,8 @@ impl< + OnIdle> + OnFinalize> + OffchainWorker> - + frame_support::traits::TryState>, + + TryState> + + TryDecodeEntireStorage, COnRuntimeUpgrade: OnRuntimeUpgrade, > Executive where @@ -353,16 +360,12 @@ where /// /// Runs the try-state code both before and after the migration function if `checks` is set to /// `true`. Also, if set to `true`, it runs the `pre_upgrade` and `post_upgrade` hooks. - pub fn try_runtime_upgrade( - checks: frame_try_runtime::UpgradeCheckSelect, - ) -> Result { + pub fn try_runtime_upgrade(checks: UpgradeCheckSelect) -> Result { if checks.try_state() { - let _guard = frame_support::StorageNoopGuard::default(); - , - >>::try_state( + let _guard = StorageNoopGuard::default(); + AllPalletsWithSystem::try_state( frame_system::Pallet::::block_number(), - frame_try_runtime::TryStateSelect::All, + TryStateSelect::All, )?; } @@ -370,14 +373,34 @@ where <(COnRuntimeUpgrade, AllPalletsWithSystem) as OnRuntimeUpgrade>::try_on_runtime_upgrade( checks.pre_and_post(), )?; + // Nothing should modify the state after the migrations ran: + let _guard = StorageNoopGuard::default(); + + // The state must be decodable: + if checks.any() { + match AllPalletsWithSystem::try_decode_entire_state() { + Ok(bytes) => { + log::info!( + target: LOG_TARGET, + "decoded the entire state ({bytes} bytes)", + ); + }, + Err(err) => { + log::info!( + target: LOG_TARGET, + "failed to decode the state: {err}" + ); + + return Err("failed to decode a value from the storage".into()) + }, + }; + } + // Check all storage invariants: if checks.try_state() { - let _guard = frame_support::StorageNoopGuard::default(); - , - >>::try_state( + AllPalletsWithSystem::try_state( frame_system::Pallet::::block_number(), - frame_try_runtime::TryStateSelect::All, + TryStateSelect::All, )?; } diff --git a/substrate/frame/support/src/storage/migration.rs b/substrate/frame/support/src/storage/migration.rs index f60409dcb9e9..568c475bdc69 100644 --- a/substrate/frame/support/src/storage/migration.rs +++ b/substrate/frame/support/src/storage/migration.rs @@ -385,51 +385,6 @@ pub fn move_prefix(from_prefix: &[u8], to_prefix: &[u8]) { } } -/// A phony migration that does nothing, except executing `TryDecodeEntireStorage` on -/// `post_upgrade`, which implies it is only available if `try-state` feature is used. -/// -/// This can be used typically in the top level runtime, where `AllPallets` typically comes from -/// `construct_runtime!`. -pub struct EnsureStateDecodes(sp_std::marker::PhantomData); - -#[cfg(not(feature = "try-runtime"))] -impl crate::traits::OnRuntimeUpgrade for EnsureStateDecodes { - fn on_runtime_upgrade() -> crate::weights::Weight { - Default::default() - } -} - -#[cfg(feature = "try-runtime")] -impl crate::traits::OnRuntimeUpgrade - for EnsureStateDecodes -{ - fn on_runtime_upgrade() -> sp_weights::Weight { - Default::default() - } - - fn post_upgrade(_: Vec) -> Result<(), sp_runtime::TryRuntimeError> { - let decoded = match AllPallets::try_decode_entire_state() { - Ok(bytes) => bytes, - Err(err) => { - log::info!( - target: crate::LOG_TARGET, - "failed to decode the entire state: {}", err - ); - // NOTE: This only supports static strings, so we cannot bubble up the exact error. - return Err("failed to decode a value from the storage".into()) - }, - }; - - log::info!( - target: crate::LOG_TARGET, - "decoded the entire state, total size = {} bytes", - decoded - ); - - Ok(()) - } -} - #[cfg(test)] mod tests { use super::{ diff --git a/substrate/frame/support/src/traits/try_runtime/mod.rs b/substrate/frame/support/src/traits/try_runtime/mod.rs index 4379b9df959b..8438e436146d 100644 --- a/substrate/frame/support/src/traits/try_runtime/mod.rs +++ b/substrate/frame/support/src/traits/try_runtime/mod.rs @@ -110,6 +110,11 @@ impl UpgradeCheckSelect { pub fn try_state(&self) -> bool { matches!(self, Self::All | Self::TryState) } + + /// Whether to run any checks. + pub fn any(&self) -> bool { + !matches!(self, Self::None) + } } #[cfg(feature = "std")] From 4890794c6d63ec156699b3098e73822b05598c34 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Tue, 24 Oct 2023 13:52:15 +0200 Subject: [PATCH 13/24] Fix Signed-off-by: Oliver Tale-Yazdi --- substrate/bin/node-template/runtime/src/lib.rs | 6 ++---- substrate/bin/node/runtime/src/lib.rs | 2 -- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/substrate/bin/node-template/runtime/src/lib.rs b/substrate/bin/node-template/runtime/src/lib.rs index 52bfc9c95f19..8148effe7118 100644 --- a/substrate/bin/node-template/runtime/src/lib.rs +++ b/substrate/bin/node-template/runtime/src/lib.rs @@ -314,11 +314,9 @@ pub type SignedExtra = ( /// All migrations of the runtime, aside from the ones declared in the pallets. /// -/// This can be a tuple of types, each implementing `OnRuntimeUpgrade`. Add other migration types -/// before `EnsureStateDecodes` as needed -- this is only for testing, and -// should come last. +/// This can be a tuple of types, each implementing `OnRuntimeUpgrade`. #[allow(unused_parens)] -type Migrations = (frame_support::migration::EnsureStateDecodes); +type Migrations = (); /// Unchecked extrinsic type as expected by this runtime. pub type UncheckedExtrinsic = diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index fa49a0b842f5..2070e3f12d04 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -2181,8 +2181,6 @@ type Migrations = ( pallet_nomination_pools::migration::v2::MigrateToV2, pallet_alliance::migration::Migration, pallet_contracts::Migration, - // This should always be the last migration item. - frame_support::storage::migration::EnsureStateDecodes, ); type EventRecord = frame_system::EventRecord< From 1c34c2d012c306da45563e90b585258dec00af51 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Tue, 24 Oct 2023 16:49:56 +0200 Subject: [PATCH 14/24] Nicer error print Signed-off-by: Oliver Tale-Yazdi --- substrate/frame/executive/src/lib.rs | 4 ++-- substrate/frame/support/Cargo.toml | 2 +- substrate/frame/support/src/traits/storage.rs | 17 ++++++++++++++--- .../traits/try_runtime/decode_entire_state.rs | 13 +++++++------ 4 files changed, 24 insertions(+), 12 deletions(-) diff --git a/substrate/frame/executive/src/lib.rs b/substrate/frame/executive/src/lib.rs index 11ed6626c29e..89e320dd42c0 100644 --- a/substrate/frame/executive/src/lib.rs +++ b/substrate/frame/executive/src/lib.rs @@ -386,9 +386,9 @@ where ); }, Err(err) => { - log::info!( + log::error!( target: LOG_TARGET, - "failed to decode the state: {err}" + "failed to decode the value at key: {err}", ); return Err("failed to decode a value from the storage".into()) diff --git a/substrate/frame/support/Cargo.toml b/substrate/frame/support/Cargo.toml index 3eb453d9b2ed..a8f05c18ff93 100644 --- a/substrate/frame/support/Cargo.toml +++ b/substrate/frame/support/Cargo.toml @@ -13,6 +13,7 @@ readme = "README.md" targets = ["x86_64-unknown-linux-gnu"] [dependencies] +array-bytes = { version = "6.1", default-features = false } serde = { version = "1.0.188", default-features = false, features = ["alloc", "derive"] } codec = { package = "parity-scale-codec", version = "3.6.1", default-features = false, features = ["derive", "max-encoded-len"] } scale-info = { version = "2.10.0", default-features = false, features = ["derive"] } @@ -52,7 +53,6 @@ aquamarine = { version = "0.3.2" } assert_matches = "1.3.0" pretty_assertions = "1.2.1" frame-system = { path = "../system" } -array-bytes = "6.1" [features] default = [ "std" ] diff --git a/substrate/frame/support/src/traits/storage.rs b/substrate/frame/support/src/traits/storage.rs index fe1b9bf13bb0..91b46ed7d32f 100644 --- a/substrate/frame/support/src/traits/storage.rs +++ b/substrate/frame/support/src/traits/storage.rs @@ -93,9 +93,7 @@ pub trait StorageInstance { } /// Metadata about storage from the runtime. -#[derive( - codec::Encode, codec::Decode, RuntimeDebug, Eq, PartialEq, Clone, scale_info::TypeInfo, -)] +#[derive(codec::Encode, codec::Decode, Eq, PartialEq, Clone, scale_info::TypeInfo)] pub struct StorageInfo { /// Encoded string of pallet name. pub pallet_name: Vec, @@ -109,6 +107,19 @@ pub struct StorageInfo { pub max_size: Option, } +// Uses strings instead of bytes for the debug output. +impl core::fmt::Debug for StorageInfo { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + f.debug_struct("StorageInfo") + .field("pallet_name", &sp_std::str::from_utf8(&self.pallet_name)) + .field("storage_name", &sp_std::str::from_utf8(&self.storage_name)) + .field("prefix", &sp_std::str::from_utf8(&self.prefix)) + .field("max_values", &self.max_values) + .field("max_size", &self.max_size) + .finish() + } +} + /// A trait to give information about storage. /// /// It can be used to calculate PoV worst case size. diff --git a/substrate/frame/support/src/traits/try_runtime/decode_entire_state.rs b/substrate/frame/support/src/traits/try_runtime/decode_entire_state.rs index 73de0be60ed4..0a60cc15098d 100644 --- a/substrate/frame/support/src/traits/try_runtime/decode_entire_state.rs +++ b/substrate/frame/support/src/traits/try_runtime/decode_entire_state.rs @@ -58,19 +58,21 @@ impl TryDecodeEntireStorage for Tuple { #[derive(Debug, Clone, PartialEq, Eq)] pub struct TryDecodeEntireStorageError { /// The key of the undecodable value. - key: Vec, + pub key: Vec, /// The raw value. - raw: Option>, + pub raw: Option>, /// The storage info of the key. - info: StorageInfo, + pub info: StorageInfo, } impl core::fmt::Display for TryDecodeEntireStorageError { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { write!( f, - "Failed to decode value at key {:?} with storage info {:?}. Raw value: {:?}", - self.key, self.info, self.raw, + "Failed to decode value at key: {}. Storage info {:?}. Raw value: {:?}", + array_bytes::bytes2hex("0x", &self.key), + self.info, + self.raw.as_ref().map(|r| array_bytes::bytes2hex("0x", r)), ) } } @@ -89,7 +91,6 @@ fn decode_storage_info(info: StorageInfo) -> Result { let len = bytes.len(); let _ = ::decode_all(&mut bytes.as_ref()).map_err(|_| { - log::error!(target: crate::LOG_TARGET, "failed to decoded {:?}", info,); TryDecodeEntireStorageError { key: key.to_vec(), raw: Some(bytes.to_vec()), From b003d2e74365f375a8afecc3cf64989717536ea8 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Tue, 24 Oct 2023 17:03:27 +0200 Subject: [PATCH 15/24] Fix debug print Signed-off-by: Oliver Tale-Yazdi --- substrate/frame/support/src/traits/storage.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/substrate/frame/support/src/traits/storage.rs b/substrate/frame/support/src/traits/storage.rs index 91b46ed7d32f..477fc44c5cae 100644 --- a/substrate/frame/support/src/traits/storage.rs +++ b/substrate/frame/support/src/traits/storage.rs @@ -107,13 +107,13 @@ pub struct StorageInfo { pub max_size: Option, } -// Uses strings instead of bytes for the debug output. +// Use strings instead of bytes for the debug output. impl core::fmt::Debug for StorageInfo { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { f.debug_struct("StorageInfo") .field("pallet_name", &sp_std::str::from_utf8(&self.pallet_name)) .field("storage_name", &sp_std::str::from_utf8(&self.storage_name)) - .field("prefix", &sp_std::str::from_utf8(&self.prefix)) + .field("prefix", &array_bytes::bytes2hex("0x", &self.prefix)) .field("max_values", &self.max_values) .field("max_size", &self.max_size) .finish() From 3c9907a7f9a13c4bb2793f224e1e37325cf75851 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Tue, 24 Oct 2023 17:45:24 +0200 Subject: [PATCH 16/24] Fix docs Signed-off-by: Oliver Tale-Yazdi --- prdoc/pr_1805.prdoc | 6 +----- substrate/frame/executive/src/lib.rs | 2 +- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/prdoc/pr_1805.prdoc b/prdoc/pr_1805.prdoc index efb0b7565e3d..8a8e6c2fde26 100644 --- a/prdoc/pr_1805.prdoc +++ b/prdoc/pr_1805.prdoc @@ -3,7 +3,7 @@ title: Introduce state decoding check after runtime upgrades. doc: - audience: Core Dev description: | - Adds a `TryDecodeEntireState` migration that can be optionally included into the list of migrations of a runtime. It will try to decode the state of all pallets to spot any decoding errors that might have been introduced by missing a migration. + Adds a check to the try-runtime logic that will verify that all pallet on-chain storage still decodes. This can help to spot missing migrations before they become a problem. The check is enabled as soon as the `--checks` option of the `try-runtime` CLI is not `None`. migrations: db: [] @@ -15,9 +15,5 @@ crates: semver: minor - name: frame-support-procedural semver: minor - - name: kitchensink-runtime - semver: minor - - name: node-template-runtime - semver: minor host_functions: [] diff --git a/substrate/frame/executive/src/lib.rs b/substrate/frame/executive/src/lib.rs index cab256687e69..5f785001ac0f 100644 --- a/substrate/frame/executive/src/lib.rs +++ b/substrate/frame/executive/src/lib.rs @@ -388,7 +388,7 @@ where Err(err) => { log::error!( target: LOG_TARGET, - "failed to decode the value at key: {err}", + "failed to decode the state: {err}", ); return Err("failed to decode a value from the storage".into()) From 022e1e7ea2f9862c088e8144c379bfeb868bd060 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Thu, 26 Oct 2023 12:15:52 +0200 Subject: [PATCH 17/24] Apply suggestions from code review Co-authored-by: Liam Aharon --- substrate/frame/executive/src/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/substrate/frame/executive/src/lib.rs b/substrate/frame/executive/src/lib.rs index 5f785001ac0f..4a8b39b53eb1 100644 --- a/substrate/frame/executive/src/lib.rs +++ b/substrate/frame/executive/src/lib.rs @@ -380,7 +380,7 @@ where if checks.any() { match AllPalletsWithSystem::try_decode_entire_state() { Ok(bytes) => { - log::info!( + log::debug!( target: LOG_TARGET, "decoded the entire state ({bytes} bytes)", ); @@ -388,10 +388,10 @@ where Err(err) => { log::error!( target: LOG_TARGET, - "failed to decode the state: {err}", + "`try_decode_entire_state` failed: {err}", ); - return Err("failed to decode a value from the storage".into()) + return Err("`try_decode_entire_state` failed".into()) }, }; } From 4623145bec683e6036b5ad136839a85714352f52 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Thu, 26 Oct 2023 13:28:47 +0200 Subject: [PATCH 18/24] Return all errors Signed-off-by: Oliver Tale-Yazdi --- substrate/frame/executive/src/lib.rs | 9 +++- .../procedural/src/pallet/expand/storage.rs | 20 +++++++-- .../traits/try_runtime/decode_entire_state.rs | 43 +++++++++++++------ 3 files changed, 52 insertions(+), 20 deletions(-) diff --git a/substrate/frame/executive/src/lib.rs b/substrate/frame/executive/src/lib.rs index 4a8b39b53eb1..7ceb8090b7c6 100644 --- a/substrate/frame/executive/src/lib.rs +++ b/substrate/frame/executive/src/lib.rs @@ -385,12 +385,17 @@ where "decoded the entire state ({bytes} bytes)", ); }, - Err(err) => { + Err(errors) => { log::error!( target: LOG_TARGET, - "`try_decode_entire_state` failed: {err}", + "`try_decode_entire_state` failed with {} errors", + errors.len(), ); + for (i, err) in errors.iter().enumerate() { + log::error!(target: LOG_TARGET, "- {i}. error: {err}"); + } + return Err("`try_decode_entire_state` failed".into()) }, }; diff --git a/substrate/frame/support/procedural/src/pallet/expand/storage.rs b/substrate/frame/support/procedural/src/pallet/expand/storage.rs index ae3aeec4c466..d19012864313 100644 --- a/substrate/frame/support/procedural/src/pallet/expand/storage.rs +++ b/substrate/frame/support/procedural/src/pallet/expand/storage.rs @@ -850,7 +850,7 @@ pub fn expand_storages(def: &mut Def) -> proc_macro2::TokenStream { impl<#type_impl_gen> #frame_support::traits::TryDecodeEntireStorage for #pallet_ident<#type_use_gen> #completed_where_clause { - fn try_decode_entire_state() -> Result { + fn try_decode_entire_state() -> Result> { let pallet_name = <::PalletInfo as frame_support::traits::PalletInfo> ::name::<#pallet_ident<#type_use_gen>>() .expect("Every active pallet has a name in the runtime; qed"); @@ -858,16 +858,28 @@ pub fn expand_storages(def: &mut Def) -> proc_macro2::TokenStream { #frame_support::__private::log::debug!(target: "try-decode-state", "trying to decode pallet: {pallet_name}"); // NOTE: for now, we have to exclude storage items that are feature gated. + let mut errors = #frame_support::__private::sp_std::vec::Vec::new(); let mut decoded = 0usize; + #( #frame_support::__private::log::debug!(target: "try-decode-state", "trying to decode storage: \ {pallet_name}::{}", stringify!(#storage_names)); - decoded += - <#storage_names as #frame_support::traits::TryDecodeEntireStorage>::try_decode_entire_state()?; + match <#storage_names as #frame_support::traits::TryDecodeEntireStorage>::try_decode_entire_state() { + Ok(count) => { + decoded += count; + }, + Err(err) => { + errors.extend(err); + }, + } )* - Ok(decoded) + if errors.is_empty() { + Ok(decoded) + } else { + Err(errors) + } } } ) diff --git a/substrate/frame/support/src/traits/try_runtime/decode_entire_state.rs b/substrate/frame/support/src/traits/try_runtime/decode_entire_state.rs index 0a60cc15098d..ced8bff997e4 100644 --- a/substrate/frame/support/src/traits/try_runtime/decode_entire_state.rs +++ b/substrate/frame/support/src/traits/try_runtime/decode_entire_state.rs @@ -40,17 +40,29 @@ use sp_std::prelude::*; /// decoded bytes if `Ok(_)`. pub trait TryDecodeEntireStorage { /// Decode the entire data under the given storage, returning `Ok(bytes_decoded)` if success. - fn try_decode_entire_state() -> Result; + fn try_decode_entire_state() -> Result>; } #[cfg_attr(all(not(feature = "tuples-96"), not(feature = "tuples-128")), impl_for_tuples(64))] #[cfg_attr(all(feature = "tuples-96", not(feature = "tuples-128")), impl_for_tuples(96))] #[cfg_attr(feature = "tuples-128", impl_for_tuples(128))] impl TryDecodeEntireStorage for Tuple { - fn try_decode_entire_state() -> Result { + fn try_decode_entire_state() -> Result> { + let mut errors = Vec::new(); let mut len = 0usize; - for_tuples!( #( len = len.saturating_add(Tuple::try_decode_entire_state()?); )* ); - Ok(len) + + for_tuples!(#( + match Tuple::try_decode_entire_state() { + Ok(bytes) => len += bytes, + Err(errs) => errors.extend(errs), + } + )*); + + if errors.is_empty() { + Ok(len) + } else { + Err(errors) + } } } @@ -82,7 +94,9 @@ impl core::fmt::Display for TryDecodeEntireStorageError { /// Basically, it decodes and sums up all the values who's key start with `info.prefix`. For values, /// this would be the value itself. For all sorts of maps, this should be all map items in the /// absence of key collision. -fn decode_storage_info(info: StorageInfo) -> Result { +fn decode_storage_info( + info: StorageInfo, +) -> Result> { let mut next_key = info.prefix.clone(); let mut decoded = 0; @@ -91,13 +105,14 @@ fn decode_storage_info(info: StorageInfo) -> Result { let len = bytes.len(); let _ = ::decode_all(&mut bytes.as_ref()).map_err(|_| { - TryDecodeEntireStorageError { + vec![TryDecodeEntireStorageError { key: key.to_vec(), raw: Some(bytes.to_vec()), info: info.clone(), - } + }] })?; - Ok::(len) + + Ok::>(len) }, }; @@ -123,7 +138,7 @@ where QueryKind: QueryKindTrait, OnEmpty: Get + 'static, { - fn try_decode_entire_state() -> Result { + fn try_decode_entire_state() -> Result> { let info = Self::partial_storage_info() .first() .cloned() @@ -143,7 +158,7 @@ where OnEmpty: Get + 'static, MaxValues: Get>, { - fn try_decode_entire_state() -> Result { + fn try_decode_entire_state() -> Result> { let info = Self::partial_storage_info() .first() .cloned() @@ -170,7 +185,7 @@ impl TryDecodeEntireS OnEmpty: Get + 'static, MaxValues: Get>, { - fn try_decode_entire_state() -> Result { + fn try_decode_entire_state() -> Result> { let (map_info, counter_info) = match &Self::partial_storage_info()[..] { [a, b] => (a.clone(), b.clone()), _ => panic!("Counted map has two storage info items; qed"), @@ -204,7 +219,7 @@ impl OnEmpty: Get + 'static, MaxValues: Get>, { - fn try_decode_entire_state() -> Result { + fn try_decode_entire_state() -> Result> { let info = Self::partial_storage_info() .first() .cloned() @@ -223,7 +238,7 @@ where OnEmpty: Get + 'static, MaxValues: Get>, { - fn try_decode_entire_state() -> Result { + fn try_decode_entire_state() -> Result> { let info = Self::partial_storage_info() .first() .cloned() @@ -242,7 +257,7 @@ where OnEmpty: Get + 'static, MaxValues: Get>, { - fn try_decode_entire_state() -> Result { + fn try_decode_entire_state() -> Result> { let (map_info, counter_info) = match &Self::partial_storage_info()[..] { [a, b] => (a.clone(), b.clone()), _ => panic!("Counted NMap has two storage info items; qed"), From d7688b2673d9b5f631937aa895ce9b45fcd583d2 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Thu, 26 Oct 2023 14:06:25 +0200 Subject: [PATCH 19/24] Also try to decode the state in try_execute_block Signed-off-by: Oliver Tale-Yazdi --- substrate/frame/executive/src/lib.rs | 60 ++++++++++++------- .../support/src/traits/try_runtime/mod.rs | 9 ++- 2 files changed, 45 insertions(+), 24 deletions(-) diff --git a/substrate/frame/executive/src/lib.rs b/substrate/frame/executive/src/lib.rs index 7ceb8090b7c6..74216096898b 100644 --- a/substrate/frame/executive/src/lib.rs +++ b/substrate/frame/executive/src/lib.rs @@ -141,7 +141,7 @@ use sp_std::{marker::PhantomData, prelude::*}; #[cfg(feature = "try-runtime")] use ::{ frame_support::{ - traits::{TryDecodeEntireStorage, TryState}, + traits::{TryDecodeEntireStorage, TryDecodeEntireStorageError, TryState}, StorageNoopGuard, }, frame_try_runtime::{TryStateSelect, UpgradeCheckSelect}, @@ -315,11 +315,15 @@ where let _guard = frame_support::StorageNoopGuard::default(); , - >>::try_state(*header.number(), select) + >>::try_state(*header.number(), select.clone()) .map_err(|e| { log::error!(target: LOG_TARGET, "failure: {:?}", e); e })?; + if select.any() { + let res = AllPalletsWithSystem::try_decode_entire_state(); + Self::log_decode_result(res)?; + } drop(_guard); // do some of the checks that would normally happen in `final_checks`, but perhaps skip @@ -378,27 +382,8 @@ where // The state must be decodable: if checks.any() { - match AllPalletsWithSystem::try_decode_entire_state() { - Ok(bytes) => { - log::debug!( - target: LOG_TARGET, - "decoded the entire state ({bytes} bytes)", - ); - }, - Err(errors) => { - log::error!( - target: LOG_TARGET, - "`try_decode_entire_state` failed with {} errors", - errors.len(), - ); - - for (i, err) in errors.iter().enumerate() { - log::error!(target: LOG_TARGET, "- {i}. error: {err}"); - } - - return Err("`try_decode_entire_state` failed".into()) - }, - }; + let res = AllPalletsWithSystem::try_decode_entire_state(); + Self::log_decode_result(res)?; } // Check all storage invariants: @@ -411,6 +396,35 @@ where Ok(weight) } + + /// Logs the result of trying to decode the entire state. + fn log_decode_result( + res: Result>, + ) -> Result<(), TryRuntimeError> { + match res { + Ok(bytes) => { + log::debug!( + target: LOG_TARGET, + "decoded the entire state ({bytes} bytes)", + ); + + Ok(()) + }, + Err(errors) => { + log::error!( + target: LOG_TARGET, + "`try_decode_entire_state` failed with {} errors", + errors.len(), + ); + + for (i, err) in errors.iter().enumerate() { + log::error!(target: LOG_TARGET, "- {i}. error: {err}"); + } + + Err("`try_decode_entire_state` failed".into()) + }, + } + } } impl< diff --git a/substrate/frame/support/src/traits/try_runtime/mod.rs b/substrate/frame/support/src/traits/try_runtime/mod.rs index 8438e436146d..39cb7ad8a4c9 100644 --- a/substrate/frame/support/src/traits/try_runtime/mod.rs +++ b/substrate/frame/support/src/traits/try_runtime/mod.rs @@ -42,6 +42,13 @@ pub enum Select { Only(Vec>), } +impl Select { + /// Whether to run any checks at all. + pub fn any(&self) -> bool { + !matches!(self, Select::None) + } +} + impl Default for Select { fn default() -> Self { Select::None @@ -111,7 +118,7 @@ impl UpgradeCheckSelect { matches!(self, Self::All | Self::TryState) } - /// Whether to run any checks. + /// Whether to run any checks at all. pub fn any(&self) -> bool { !matches!(self, Self::None) } From fbb1e25be22f2a1e5e047723bc84e1a5d77f9926 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Thu, 26 Oct 2023 14:52:30 +0200 Subject: [PATCH 20/24] Tweak error print Signed-off-by: Oliver Tale-Yazdi --- substrate/frame/support/src/traits/storage.rs | 15 +-------------- .../src/traits/try_runtime/decode_entire_state.rs | 7 ++++--- 2 files changed, 5 insertions(+), 17 deletions(-) diff --git a/substrate/frame/support/src/traits/storage.rs b/substrate/frame/support/src/traits/storage.rs index 477fc44c5cae..9e467aea4220 100644 --- a/substrate/frame/support/src/traits/storage.rs +++ b/substrate/frame/support/src/traits/storage.rs @@ -93,7 +93,7 @@ pub trait StorageInstance { } /// Metadata about storage from the runtime. -#[derive(codec::Encode, codec::Decode, Eq, PartialEq, Clone, scale_info::TypeInfo)] +#[derive(Debug, codec::Encode, codec::Decode, Eq, PartialEq, Clone, scale_info::TypeInfo)] pub struct StorageInfo { /// Encoded string of pallet name. pub pallet_name: Vec, @@ -107,19 +107,6 @@ pub struct StorageInfo { pub max_size: Option, } -// Use strings instead of bytes for the debug output. -impl core::fmt::Debug for StorageInfo { - fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { - f.debug_struct("StorageInfo") - .field("pallet_name", &sp_std::str::from_utf8(&self.pallet_name)) - .field("storage_name", &sp_std::str::from_utf8(&self.storage_name)) - .field("prefix", &array_bytes::bytes2hex("0x", &self.prefix)) - .field("max_values", &self.max_values) - .field("max_size", &self.max_size) - .finish() - } -} - /// A trait to give information about storage. /// /// It can be used to calculate PoV worst case size. diff --git a/substrate/frame/support/src/traits/try_runtime/decode_entire_state.rs b/substrate/frame/support/src/traits/try_runtime/decode_entire_state.rs index ced8bff997e4..286eb9863ca0 100644 --- a/substrate/frame/support/src/traits/try_runtime/decode_entire_state.rs +++ b/substrate/frame/support/src/traits/try_runtime/decode_entire_state.rs @@ -67,7 +67,7 @@ impl TryDecodeEntireStorage for Tuple { } /// A value could not be decoded. -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Clone, PartialEq, Eq)] pub struct TryDecodeEntireStorageError { /// The key of the undecodable value. pub key: Vec, @@ -81,9 +81,10 @@ impl core::fmt::Display for TryDecodeEntireStorageError { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { write!( f, - "Failed to decode value at key: {}. Storage info {:?}. Raw value: {:?}", + "Failed to decode storage item `{}::{}` at key: {}. Raw value: {:?}", + &sp_std::str::from_utf8(&self.info.pallet_name).unwrap_or(""), + &sp_std::str::from_utf8(&self.info.storage_name).unwrap_or(""), array_bytes::bytes2hex("0x", &self.key), - self.info, self.raw.as_ref().map(|r| array_bytes::bytes2hex("0x", r)), ) } From 81885ff2368e3dd3470b97cf2f9729f399cbdab4 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Thu, 26 Oct 2023 15:04:20 +0200 Subject: [PATCH 21/24] Fix CI Signed-off-by: Oliver Tale-Yazdi --- .../frame/support/src/traits/try_runtime/decode_entire_state.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/support/src/traits/try_runtime/decode_entire_state.rs b/substrate/frame/support/src/traits/try_runtime/decode_entire_state.rs index 286eb9863ca0..f2bb631d3014 100644 --- a/substrate/frame/support/src/traits/try_runtime/decode_entire_state.rs +++ b/substrate/frame/support/src/traits/try_runtime/decode_entire_state.rs @@ -67,7 +67,7 @@ impl TryDecodeEntireStorage for Tuple { } /// A value could not be decoded. -#[derive(Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq)] pub struct TryDecodeEntireStorageError { /// The key of the undecodable value. pub key: Vec, From 2d8b214374b246511b7b17014e5246aea191fa12 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Fri, 27 Oct 2023 15:37:52 +0200 Subject: [PATCH 22/24] log extended info to debug Signed-off-by: Oliver Tale-Yazdi --- substrate/frame/executive/src/lib.rs | 2 ++ .../support/src/traits/try_runtime/decode_entire_state.rs | 4 +--- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/substrate/frame/executive/src/lib.rs b/substrate/frame/executive/src/lib.rs index 74216096898b..80c45d5abf1a 100644 --- a/substrate/frame/executive/src/lib.rs +++ b/substrate/frame/executive/src/lib.rs @@ -418,7 +418,9 @@ where ); for (i, err) in errors.iter().enumerate() { + // We log the short version to `error` and then the full debug info to `debug`: log::error!(target: LOG_TARGET, "- {i}. error: {err}"); + log::debug!(target: LOG_TARGET, "- {i}. error: {err:?}"); } Err("`try_decode_entire_state` failed".into()) diff --git a/substrate/frame/support/src/traits/try_runtime/decode_entire_state.rs b/substrate/frame/support/src/traits/try_runtime/decode_entire_state.rs index f2bb631d3014..afbf291a0bbb 100644 --- a/substrate/frame/support/src/traits/try_runtime/decode_entire_state.rs +++ b/substrate/frame/support/src/traits/try_runtime/decode_entire_state.rs @@ -81,11 +81,9 @@ impl core::fmt::Display for TryDecodeEntireStorageError { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { write!( f, - "Failed to decode storage item `{}::{}` at key: {}. Raw value: {:?}", + "Failed to decode storage item `{}::{}`", &sp_std::str::from_utf8(&self.info.pallet_name).unwrap_or(""), &sp_std::str::from_utf8(&self.info.storage_name).unwrap_or(""), - array_bytes::bytes2hex("0x", &self.key), - self.raw.as_ref().map(|r| array_bytes::bytes2hex("0x", r)), ) } } From 5472c3e84a6dc5072d5d94fee58d510396a1d511 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Mon, 6 Nov 2023 12:48:47 +0100 Subject: [PATCH 23/24] Prefix runtime log target Signed-off-by: Oliver Tale-Yazdi --- .../frame/support/procedural/src/pallet/expand/storage.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/substrate/frame/support/procedural/src/pallet/expand/storage.rs b/substrate/frame/support/procedural/src/pallet/expand/storage.rs index d19012864313..96c2c8e3120b 100644 --- a/substrate/frame/support/procedural/src/pallet/expand/storage.rs +++ b/substrate/frame/support/procedural/src/pallet/expand/storage.rs @@ -855,14 +855,14 @@ pub fn expand_storages(def: &mut Def) -> proc_macro2::TokenStream { ::name::<#pallet_ident<#type_use_gen>>() .expect("Every active pallet has a name in the runtime; qed"); - #frame_support::__private::log::debug!(target: "try-decode-state", "trying to decode pallet: {pallet_name}"); + #frame_support::__private::log::debug!(target: "runtime::try-decode-state", "trying to decode pallet: {pallet_name}"); // NOTE: for now, we have to exclude storage items that are feature gated. let mut errors = #frame_support::__private::sp_std::vec::Vec::new(); let mut decoded = 0usize; #( - #frame_support::__private::log::debug!(target: "try-decode-state", "trying to decode storage: \ + #frame_support::__private::log::debug!(target: "runtime::try-decode-state", "trying to decode storage: \ {pallet_name}::{}", stringify!(#storage_names)); match <#storage_names as #frame_support::traits::TryDecodeEntireStorage>::try_decode_entire_state() { From 31195395e3d4e9364f47b3b74f47f2a52831ae0f Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Mon, 6 Nov 2023 16:41:51 +0100 Subject: [PATCH 24/24] Resolve unused import Signed-off-by: Oliver Tale-Yazdi --- substrate/frame/executive/src/lib.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/substrate/frame/executive/src/lib.rs b/substrate/frame/executive/src/lib.rs index 0419dc749f19..5f390c77baff 100644 --- a/substrate/frame/executive/src/lib.rs +++ b/substrate/frame/executive/src/lib.rs @@ -363,9 +363,7 @@ where /// Execute all `OnRuntimeUpgrade` of this runtime. /// /// The `checks` param determines whether to execute `pre/post_upgrade` and `try_state` hooks. - pub fn try_runtime_upgrade( - checks: frame_try_runtime::UpgradeCheckSelect, - ) -> Result { + pub fn try_runtime_upgrade(checks: UpgradeCheckSelect) -> Result { let weight = <(COnRuntimeUpgrade, AllPalletsWithSystem) as OnRuntimeUpgrade>::try_on_runtime_upgrade( checks.pre_and_post(),