From ca7ab55cd8e1ecfca3b5562dc111e2f09c37abc0 Mon Sep 17 00:00:00 2001 From: Alin Dima Date: Mon, 6 Nov 2023 18:30:09 +0200 Subject: [PATCH 01/12] minor: overseer availability-distribution message declaration update (#2179) availability-distribution subsystem is not sending availability-recovery messages. Update the overseer declaration to reflect this --- polkadot/node/overseer/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/polkadot/node/overseer/src/lib.rs b/polkadot/node/overseer/src/lib.rs index 6802426d3c70..5207bb830d8c 100644 --- a/polkadot/node/overseer/src/lib.rs +++ b/polkadot/node/overseer/src/lib.rs @@ -498,7 +498,6 @@ pub struct Overseer { #[subsystem(AvailabilityDistributionMessage, sends: [ AvailabilityStoreMessage, - AvailabilityRecoveryMessage, ChainApiMessage, RuntimeApiMessage, NetworkBridgeTxMessage, From 63f265e60d37ef3061b9412839451b26892edbb5 Mon Sep 17 00:00:00 2001 From: Piet <75956460+PieWol@users.noreply.github.com> Date: Mon, 6 Nov 2023 19:40:14 +0100 Subject: [PATCH 02/12] TryDecodeEntireState check for storage types and pallets (#1805) ### This PR is a port of this [PR for substrate](https://github.com/paritytech/substrate/pull/13013) by @kianenigma Add infrastructure needed to have a Pallet::decode_entire_state(), which makes sure all "typed" storage items defined in the pallet are decode-able. This is not enforced in any way at the moment. Teams who wish to integrate/use this in the try-runtime feature flag should add frame_support::storage::migration::EnsureStateDecodes as the LAST ITEM of the runtime's custom migrations, and pass it to frame-executive. This will make it usable in try-runtime on-runtime-upgrade. This now catches cases like https://github.com/paritytech/polkadot-sdk/pull/1969: ```pre ERROR runtime::executive] failed to decode the value at key: Failed to decode value at key: 0x94eadf0156a8ad5156507773d0471e4ab8ebad86f546c7e0b135a4212aace339. Storage info StorageInfo { pallet_name: Ok("ParaScheduler"), storage_name: Ok("AvailabilityCores"), prefix: Err(Utf8Error { valid_up_to: 0, error_len: Some(1) }), max_values: Some(1), max_size: None }. Raw value: Some("0x0c010101010101") ``` ... or: ![image](https://github.com/paritytech/polkadot-sdk/assets/10380170/73052d4f-4da5-4b21-a8dd-b17004e5965e) Closes #241 --------- Signed-off-by: Oliver Tale-Yazdi Co-authored-by: Oliver Tale-Yazdi Co-authored-by: Liam Aharon --- prdoc/pr_1805.prdoc | 19 + .../bin/node-template/runtime/src/lib.rs | 7 + substrate/frame/executive/src/lib.rs | 72 ++- substrate/frame/glutton/src/lib.rs | 5 +- substrate/frame/support/Cargo.toml | 2 +- .../procedural/src/pallet/expand/event.rs | 3 +- .../procedural/src/pallet/expand/storage.rs | 59 +++ .../procedural/src/pallet/parse/storage.rs | 2 +- .../support/src/storage/generator/mod.rs | 6 +- .../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 | 5 +- substrate/frame/support/src/traits/storage.rs | 4 +- .../traits/try_runtime/decode_entire_state.rs | 498 ++++++++++++++++++ .../{try_runtime.rs => try_runtime/mod.rs} | 17 + ...age_ensure_span_are_ok_on_wrong_gen.stderr | 59 +++ ...re_span_are_ok_on_wrong_gen_unnamed.stderr | 59 +++ 19 files changed, 807 insertions(+), 36 deletions(-) create mode 100644 prdoc/pr_1805.prdoc create mode 100644 substrate/frame/support/src/traits/try_runtime/decode_entire_state.rs rename substrate/frame/support/src/traits/{try_runtime.rs => try_runtime/mod.rs} (94%) diff --git a/prdoc/pr_1805.prdoc b/prdoc/pr_1805.prdoc new file mode 100644 index 000000000000..8a8e6c2fde26 --- /dev/null +++ b/prdoc/pr_1805.prdoc @@ -0,0 +1,19 @@ +title: Introduce state decoding check after runtime upgrades. + +doc: + - audience: Core Dev + description: | + 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: [] + + runtime: [] + +crates: + - name: frame-support + semver: minor + - name: frame-support-procedural + semver: minor + +host_functions: [] diff --git a/substrate/bin/node-template/runtime/src/lib.rs b/substrate/bin/node-template/runtime/src/lib.rs index 62c24081cbea..6aa4cb70fde1 100644 --- a/substrate/bin/node-template/runtime/src/lib.rs +++ b/substrate/bin/node-template/runtime/src/lib.rs @@ -313,6 +313,12 @@ 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`. +#[allow(unused_parens)] +type Migrations = (); + /// Unchecked extrinsic type as expected by this runtime. pub type UncheckedExtrinsic = generic::UncheckedExtrinsic; @@ -325,6 +331,7 @@ pub type Executive = frame_executive::Executive< frame_system::ChainContext, Runtime, AllPalletsWithSystem, + Migrations, >; #[cfg(feature = "runtime-benchmarks")] diff --git a/substrate/frame/executive/src/lib.rs b/substrate/frame/executive/src/lib.rs index 1ca9629fd420..5f390c77baff 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, TryDecodeEntireStorageError, 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 @@ -308,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 @@ -352,26 +363,61 @@ 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(), )?; + // Nothing should modify the state after the migrations ran: + let _guard = StorageNoopGuard::default(); + // The state must be decodable: + if checks.any() { + let res = AllPalletsWithSystem::try_decode_entire_state(); + Self::log_decode_result(res)?; + } + + // 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, )?; } 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() { + // 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()) + }, + } + } } impl< 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/Cargo.toml b/substrate/frame/support/Cargo.toml index f8c8edc05104..b8e21e60761a 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/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..96c2c8e3120b 100644 --- a/substrate/frame/support/procedural/src/pallet/expand/storage.rs +++ b/substrate/frame/support/procedural/src/pallet/expand/storage.rs @@ -822,12 +822,69 @@ 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 mut 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::>(); + storage_names.sort_by_cached_key(|ident| ident.to_string()); + + 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> { + 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: "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: "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() { + Ok(count) => { + decoded += count; + }, + Err(err) => { + errors.extend(err); + }, + } + )* + + if errors.is_empty() { + Ok(decoded) + } else { + Err(errors) + } + } + } + ) + }; + quote::quote!( impl<#type_impl_gen> #pallet_ident<#type_use_gen> #completed_where_clause @@ -853,5 +910,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/types/counted_map.rs b/substrate/frame/support/src/storage/types/counted_map.rs index 75fbdf2617d1..04e69751c16a 100644 --- a/substrate/frame/support/src/storage/types/counted_map.rs +++ b/substrate/frame/support/src/storage/types/counted_map.rs @@ -119,7 +119,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`]. @@ -423,14 +427,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() } } @@ -1207,7 +1211,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 c2c2197aceee..279894ee9736 100644 --- a/substrate/frame/support/src/storage/types/counted_nmap.rs +++ b/substrate/frame/support/src/storage/types/counted_nmap.rs @@ -114,8 +114,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`]. @@ -472,7 +474,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 1d995d93e882..9dd6f4066e43 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 34cc3e24956b..263091dd2523 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 frame_support::storage::StorageDecodeNonDedupLength; @@ -72,7 +72,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 4c692f848fde..12cc84027c48 100644 --- a/substrate/frame/support/src/traits.rs +++ b/substrate/frame/support/src/traits.rs @@ -126,4 +126,7 @@ 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, TryDecodeEntireStorageError, TryState, + UpgradeCheckSelect, +}; diff --git a/substrate/frame/support/src/traits/storage.rs b/substrate/frame/support/src/traits/storage.rs index fe1b9bf13bb0..9e467aea4220 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(Debug, codec::Encode, codec::Decode, Eq, PartialEq, Clone, scale_info::TypeInfo)] pub struct StorageInfo { /// Encoded string of pallet name. pub pallet_name: Vec, 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 new file mode 100644 index 000000000000..afbf291a0bbb --- /dev/null +++ b/substrate/frame/support/src/traits/try_runtime/decode_entire_state.rs @@ -0,0 +1,498 @@ +// 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. + +//! Types to check that the entire storage can be decoded. + +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_core::Get; +use sp_std::prelude::*; + +/// 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 errors = Vec::new(); + let mut len = 0usize; + + 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) + } + } +} + +/// A value could not be decoded. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct TryDecodeEntireStorageError { + /// The key of the undecodable value. + pub key: Vec, + /// The raw value. + pub raw: Option>, + /// The storage info of the key. + 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 storage item `{}::{}`", + &sp_std::str::from_utf8(&self.info.pallet_name).unwrap_or(""), + &sp_std::str::from_utf8(&self.info.storage_name).unwrap_or(""), + ) + } +} + +/// 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(|_| { + vec![TryDecodeEntireStorageError { + key: key.to_vec(), + raw: Some(bytes.to_vec()), + info: info.clone(), + }] + })?; + + 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) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::{ + storage::types::{self, CountedStorageMapInstance, CountedStorageNMapInstance, Key}, + Blake2_128Concat, + }; + + type H = Blake2_128Concat; + + macro_rules! build_prefix { + ($name:ident) => { + 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/src/traits/try_runtime.rs b/substrate/frame/support/src/traits/try_runtime/mod.rs similarity index 94% rename from substrate/frame/support/src/traits/try_runtime.rs rename to substrate/frame/support/src/traits/try_runtime/mod.rs index e7a1fe109fc2..bec2dbf549a1 100644 --- a/substrate/frame/support/src/traits/try_runtime.rs +++ b/substrate/frame/support/src/traits/try_runtime/mod.rs @@ -17,6 +17,11 @@ //! 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; @@ -37,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 @@ -105,6 +117,11 @@ impl UpgradeCheckSelect { pub fn try_state(&self) -> bool { matches!(self, Self::All | Self::TryState) } + + /// Whether to run any checks at all. + pub fn any(&self) -> bool { + !matches!(self, Self::None) + } } #[cfg(feature = "std")] 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 2ad7a430d820a0459ed68c8908b7c80e1a29b1ec Mon Sep 17 00:00:00 2001 From: Liam Aharon Date: Tue, 7 Nov 2023 15:12:40 +1100 Subject: [PATCH 03/12] Initialise on-chain `StorageVersion` for pallets added after genesis (#1297) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Original PR https://github.com/paritytech/substrate/pull/14641 --- Closes https://github.com/paritytech/polkadot-sdk/issues/109 ### Problem Quoting from the above issue: > When adding a pallet to chain after genesis we currently don't set the StorageVersion. So, when calling on_chain_storage_version it returns 0 while the pallet is maybe already at storage version 9 when it was added to the chain. This could lead to issues when running migrations. ### Solution - Create a new trait `BeforeAllRuntimeMigrations` with a single method `fn before_all_runtime_migrations() -> Weight` trait with a noop default implementation - Modify `Executive` to call `BeforeAllRuntimeMigrations::before_all_runtime_migrations` for all pallets before running any other hooks - Implement `BeforeAllRuntimeMigrations` in the pallet proc macro to initialize the on-chain version to the current pallet version if the pallet has no storage set (indicating it has been recently added to the runtime and needs to have its version initialised). ### Other changes in this PR - Abstracted repeated boilerplate to access the `pallet_name` in the pallet expand proc macro. ### FAQ #### Why create a new hook instead of adding this logic to the pallet `pre_upgrade`? `Executive` currently runs `COnRuntimeUpgrade` (custom migrations) before `AllPalletsWithSystem` migrations. We need versions to be initialized before the `COnRuntimeUpgrade` migrations are run, because `COnRuntimeUpgrade` migrations may use the on-chain version for critical logic. e.g. `VersionedRuntimeUpgrade` uses it to decide whether or not to execute. We cannot reorder `COnRuntimeUpgrade` and `AllPalletsWithSystem` so `AllPalletsWithSystem` runs first, because `AllPalletsWithSystem` have some logic in their `post_upgrade` hooks to verify that the on-chain version and current pallet version match. A common use case of `COnRuntimeUpgrade` migrations is to perform a migration which will result in the versions matching, so if they were reordered these `post_upgrade` checks would fail. #### Why init the on-chain version for pallets without a current storage version? We must init the on-chain version for pallets even if they don't have a defined storage version so if there is a future version bump, the on-chain version is not automatically set to that new version without a proper migration. e.g. bad scenario: 1. A pallet with no 'current version' is added to the runtime 2. Later, the pallet is upgraded with the 'current version' getting set to 1 and a migration is added to Executive Migrations to migrate the storage from 0 to 1 a. Runtime upgrade occurs b. `before_all` hook initializes the on-chain version to 1 c. `on_runtime_upgrade` of the migration executes, and sees the on-chain version is already 1 therefore think storage is already migrated and does not execute the storage migration Now, on-chain version is 1 but storage is still at version 0. By always initializing the on-chain version when the pallet is added to the runtime we avoid that scenario. --------- Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Co-authored-by: Bastian Köcher --- substrate/frame/executive/src/lib.rs | 16 +++- .../procedural/src/pallet/expand/hooks.rs | 93 +++++++++++++------ substrate/frame/support/src/lib.rs | 2 +- substrate/frame/support/src/migrations.rs | 4 +- substrate/frame/support/src/traits.rs | 4 +- substrate/frame/support/src/traits/hooks.rs | 27 ++++++ .../frame/support/src/traits/metadata.rs | 17 ++++ substrate/frame/support/test/tests/pallet.rs | 60 +++++++++++- 8 files changed, 182 insertions(+), 41 deletions(-) diff --git a/substrate/frame/executive/src/lib.rs b/substrate/frame/executive/src/lib.rs index 5f390c77baff..e2c906c1bf6c 100644 --- a/substrate/frame/executive/src/lib.rs +++ b/substrate/frame/executive/src/lib.rs @@ -121,8 +121,8 @@ use frame_support::{ dispatch::{DispatchClass, DispatchInfo, GetDispatchInfo, PostDispatchInfo}, pallet_prelude::InvalidTransaction, traits::{ - EnsureInherentsAreFirst, ExecuteBlock, OffchainWorker, OnFinalize, OnIdle, OnInitialize, - OnRuntimeUpgrade, + BeforeAllRuntimeMigrations, EnsureInherentsAreFirst, ExecuteBlock, OffchainWorker, + OnFinalize, OnIdle, OnInitialize, OnRuntimeUpgrade, }, weights::Weight, }; @@ -194,6 +194,7 @@ impl< Context: Default, UnsignedValidator, AllPalletsWithSystem: OnRuntimeUpgrade + + BeforeAllRuntimeMigrations + OnInitialize> + OnIdle> + OnFinalize> @@ -231,6 +232,7 @@ impl< Context: Default, UnsignedValidator, AllPalletsWithSystem: OnRuntimeUpgrade + + BeforeAllRuntimeMigrations + OnInitialize> + OnIdle> + OnFinalize> @@ -364,7 +366,9 @@ where /// /// The `checks` param determines whether to execute `pre/post_upgrade` and `try_state` hooks. pub fn try_runtime_upgrade(checks: UpgradeCheckSelect) -> Result { - let weight = + let before_all_weight = + ::before_all_runtime_migrations(); + let try_on_runtime_upgrade_weight = <(COnRuntimeUpgrade, AllPalletsWithSystem) as OnRuntimeUpgrade>::try_on_runtime_upgrade( checks.pre_and_post(), )?; @@ -385,7 +389,7 @@ where )?; } - Ok(weight) + Ok(before_all_weight.saturating_add(try_on_runtime_upgrade_weight)) } /// Logs the result of trying to decode the entire state. @@ -429,6 +433,7 @@ impl< Context: Default, UnsignedValidator, AllPalletsWithSystem: OnRuntimeUpgrade + + BeforeAllRuntimeMigrations + OnInitialize> + OnIdle> + OnFinalize> @@ -445,7 +450,10 @@ where { /// Execute all `OnRuntimeUpgrade` of this runtime, and return the aggregate weight. pub fn execute_on_runtime_upgrade() -> Weight { + let before_all_weight = + ::before_all_runtime_migrations(); <(COnRuntimeUpgrade, AllPalletsWithSystem) as OnRuntimeUpgrade>::on_runtime_upgrade() + .saturating_add(before_all_weight) } /// Start the execution of a particular block. diff --git a/substrate/frame/support/procedural/src/pallet/expand/hooks.rs b/substrate/frame/support/procedural/src/pallet/expand/hooks.rs index aaad4dd2be0e..5044d4285bb6 100644 --- a/substrate/frame/support/procedural/src/pallet/expand/hooks.rs +++ b/substrate/frame/support/procedural/src/pallet/expand/hooks.rs @@ -34,6 +34,38 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { let type_use_gen = &def.type_use_generics(span); let pallet_ident = &def.pallet_struct.pallet; let frame_system = &def.frame_system; + let pallet_name = quote::quote! { + < + ::PalletInfo + as + #frame_support::traits::PalletInfo + >::name::().unwrap_or("") + }; + + let initialize_on_chain_storage_version = if let Some(current_version) = + &def.pallet_struct.storage_version + { + quote::quote! { + #frame_support::__private::log::info!( + target: #frame_support::LOG_TARGET, + "🐥 New pallet {:?} detected in the runtime. Initializing the on-chain storage version to match the storage version defined in the pallet: {:?}", + #pallet_name, + #current_version + ); + #current_version.put::(); + } + } else { + quote::quote! { + let default_version = #frame_support::traits::StorageVersion::new(0); + #frame_support::__private::log::info!( + target: #frame_support::LOG_TARGET, + "🐥 New pallet {:?} detected in the runtime. The pallet has no defined storage version, so the on-chain version is being initialized to {:?}.", + #pallet_name, + default_version + ); + default_version.put::(); + } + }; let log_runtime_upgrade = if has_runtime_upgrade { // a migration is defined here. @@ -42,7 +74,7 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { target: #frame_support::LOG_TARGET, "⚠️ {} declares internal migrations (which *might* execute). \ On-chain `{:?}` vs current storage version `{:?}`", - pallet_name, + #pallet_name, ::on_chain_storage_version(), ::current_storage_version(), ); @@ -53,7 +85,7 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { #frame_support::__private::log::debug!( target: #frame_support::LOG_TARGET, "✅ no migration for {}", - pallet_name, + #pallet_name, ); } }; @@ -78,16 +110,10 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { let current_version = ::current_storage_version(); if on_chain_version != current_version { - let pallet_name = < - ::PalletInfo - as - #frame_support::traits::PalletInfo - >::name::().unwrap_or(""); - #frame_support::__private::log::error!( target: #frame_support::LOG_TARGET, "{}: On chain storage version {:?} doesn't match current storage version {:?}.", - pallet_name, + #pallet_name, on_chain_version, current_version, ); @@ -100,17 +126,11 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { let on_chain_version = ::on_chain_storage_version(); if on_chain_version != #frame_support::traits::StorageVersion::new(0) { - let pallet_name = < - ::PalletInfo - as - #frame_support::traits::PalletInfo - >::name::().unwrap_or(""); - #frame_support::__private::log::error!( target: #frame_support::LOG_TARGET, "{}: On chain storage version {:?} is set to non zero, \ while the pallet is missing the `#[pallet::storage_version(VERSION)]` attribute.", - pallet_name, + #pallet_name, on_chain_version, ); @@ -173,6 +193,32 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { } } + impl<#type_impl_gen> + #frame_support::traits::BeforeAllRuntimeMigrations + for #pallet_ident<#type_use_gen> #where_clause + { + fn before_all_runtime_migrations() -> #frame_support::weights::Weight { + use #frame_support::traits::{Get, PalletInfoAccess}; + use #frame_support::__private::hashing::twox_128; + use #frame_support::storage::unhashed::contains_prefixed_key; + #frame_support::__private::sp_tracing::enter_span!( + #frame_support::__private::sp_tracing::trace_span!("before_all") + ); + + // Check if the pallet has any keys set, including the storage version. If there are + // no keys set, the pallet was just added to the runtime and needs to have its + // version initialized. + let pallet_hashed_prefix = ::name_hash(); + let exists = contains_prefixed_key(&pallet_hashed_prefix); + if !exists { + #initialize_on_chain_storage_version + ::DbWeight::get().reads_writes(1, 1) + } else { + ::DbWeight::get().reads(1) + } + } + } + impl<#type_impl_gen> #frame_support::traits::OnRuntimeUpgrade for #pallet_ident<#type_use_gen> #where_clause @@ -183,11 +229,6 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { ); // log info about the upgrade. - let pallet_name = < - ::PalletInfo - as - #frame_support::traits::PalletInfo - >::name::().unwrap_or(""); #log_runtime_upgrade < @@ -258,16 +299,10 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { n: #frame_system::pallet_prelude::BlockNumberFor::, _s: #frame_support::traits::TryStateSelect ) -> Result<(), #frame_support::sp_runtime::TryRuntimeError> { - let pallet_name = < - ::PalletInfo - as - #frame_support::traits::PalletInfo - >::name::().expect("No name found for the pallet! This usually means that the pallet wasn't added to `construct_runtime!`."); - #frame_support::__private::log::info!( target: #frame_support::LOG_TARGET, "🩺 Running {:?} try-state checks", - pallet_name, + #pallet_name, ); < Self as #frame_support::traits::Hooks< @@ -277,7 +312,7 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { #frame_support::__private::log::error!( target: #frame_support::LOG_TARGET, "❌ {:?} try_state checks failed: {:?}", - pallet_name, + #pallet_name, err ); diff --git a/substrate/frame/support/src/lib.rs b/substrate/frame/support/src/lib.rs index d49588686851..a01f3a01593a 100644 --- a/substrate/frame/support/src/lib.rs +++ b/substrate/frame/support/src/lib.rs @@ -47,7 +47,7 @@ pub mod __private { pub use sp_core::{OpaqueMetadata, Void}; pub use sp_core_hashing_proc_macro; pub use sp_inherents; - pub use sp_io::{self, storage::root as storage_root}; + pub use sp_io::{self, hashing, storage::root as storage_root}; pub use sp_metadata_ir as metadata_ir; #[cfg(feature = "std")] pub use sp_runtime::{bounded_btree_map, bounded_vec}; diff --git a/substrate/frame/support/src/migrations.rs b/substrate/frame/support/src/migrations.rs index d6ae6c6291b1..22471e883a70 100644 --- a/substrate/frame/support/src/migrations.rs +++ b/substrate/frame/support/src/migrations.rs @@ -119,7 +119,7 @@ impl< let on_chain_version = Pallet::on_chain_storage_version(); if on_chain_version == FROM { log::info!( - "🚚 Pallet {:?} migrating storage version from {:?} to {:?}.", + "🚚 Pallet {:?} VersionedMigration migrating storage version from {:?} to {:?}.", Pallet::name(), FROM, TO @@ -134,7 +134,7 @@ impl< weight.saturating_add(DbWeight::get().reads_writes(1, 1)) } else { log::warn!( - "🚚 Pallet {:?} migration {}->{} can be removed; on-chain is already at {:?}.", + "🚚 Pallet {:?} VersionedMigration migration {}->{} can be removed; on-chain is already at {:?}.", Pallet::name(), FROM, TO, diff --git a/substrate/frame/support/src/traits.rs b/substrate/frame/support/src/traits.rs index 12cc84027c48..6362e750d2ab 100644 --- a/substrate/frame/support/src/traits.rs +++ b/substrate/frame/support/src/traits.rs @@ -84,8 +84,8 @@ mod hooks; #[allow(deprecated)] pub use hooks::GenesisBuild; pub use hooks::{ - BuildGenesisConfig, Hooks, IntegrityTest, OnFinalize, OnGenesis, OnIdle, OnInitialize, - OnRuntimeUpgrade, OnTimestampSet, + BeforeAllRuntimeMigrations, BuildGenesisConfig, Hooks, IntegrityTest, OnFinalize, OnGenesis, + OnIdle, OnInitialize, OnRuntimeUpgrade, OnTimestampSet, }; pub mod schedule; diff --git a/substrate/frame/support/src/traits/hooks.rs b/substrate/frame/support/src/traits/hooks.rs index 6acecb8928d8..20788ce932d8 100644 --- a/substrate/frame/support/src/traits/hooks.rs +++ b/substrate/frame/support/src/traits/hooks.rs @@ -99,6 +99,19 @@ pub trait OnGenesis { fn on_genesis() {} } +/// Implemented by pallets, allows defining logic to run prior to any [`OnRuntimeUpgrade`] logic. +/// +/// This hook is intended to be used internally in FRAME and not be exposed to FRAME developers. +/// +/// It is defined as a seperate trait from [`OnRuntimeUpgrade`] precisely to not pollute the public +/// API. +pub trait BeforeAllRuntimeMigrations { + /// Something that should happen before runtime migrations are executed. + fn before_all_runtime_migrations() -> Weight { + Weight::zero() + } +} + /// See [`Hooks::on_runtime_upgrade`]. pub trait OnRuntimeUpgrade { /// See [`Hooks::on_runtime_upgrade`]. @@ -150,10 +163,24 @@ pub trait OnRuntimeUpgrade { } } +#[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 BeforeAllRuntimeMigrations for Tuple { + /// Implements the default behavior of + /// [`BeforeAllRuntimeMigrations::before_all_runtime_migrations`] for tuples. + fn before_all_runtime_migrations() -> Weight { + let mut weight = Weight::zero(); + for_tuples!( #( weight = weight.saturating_add(Tuple::before_all_runtime_migrations()); )* ); + weight + } +} + #[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 OnRuntimeUpgrade for Tuple { + /// Implements the default behavior of [`OnRuntimeUpgrade::on_runtime_upgrade`] for tuples. fn on_runtime_upgrade() -> Weight { let mut weight = Weight::zero(); for_tuples!( #( weight = weight.saturating_add(Tuple::on_runtime_upgrade()); )* ); diff --git a/substrate/frame/support/src/traits/metadata.rs b/substrate/frame/support/src/traits/metadata.rs index bd29b6009161..0af8d06719fe 100644 --- a/substrate/frame/support/src/traits/metadata.rs +++ b/substrate/frame/support/src/traits/metadata.rs @@ -222,6 +222,23 @@ impl StorageVersion { crate::storage::unhashed::get_or_default(&key) } + + /// Returns if the storage version key for the given pallet exists in storage. + /// + /// See [`STORAGE_VERSION_STORAGE_KEY_POSTFIX`] on how this key is built. + /// + /// # Panics + /// + /// This function will panic iff `Pallet` can not be found by `PalletInfo`. + /// In a runtime that is put together using + /// [`construct_runtime!`](crate::construct_runtime) this should never happen. + /// + /// It will also panic if this function isn't executed in an externalities + /// provided environment. + pub fn exists() -> bool { + let key = Self::storage_key::

(); + crate::storage::unhashed::exists(&key) + } } impl PartialEq for StorageVersion { diff --git a/substrate/frame/support/test/tests/pallet.rs b/substrate/frame/support/test/tests/pallet.rs index 83ae5b9253ce..00e7adafb0b7 100644 --- a/substrate/frame/support/test/tests/pallet.rs +++ b/substrate/frame/support/test/tests/pallet.rs @@ -21,7 +21,7 @@ use frame_support::{ dispatch_context::with_context, pallet_prelude::{StorageInfoTrait, ValueQuery}, parameter_types, - storage::unhashed, + storage::{unhashed, unhashed::contains_prefixed_key}, traits::{ ConstU32, GetCallIndex, GetCallName, GetStorageVersion, OnFinalize, OnGenesis, OnInitialize, OnRuntimeUpgrade, PalletError, PalletInfoAccess, StorageVersion, @@ -2330,6 +2330,58 @@ fn test_storage_alias() { }) } +#[test] +fn pallet_on_chain_storage_version_initializes_correctly() { + type Executive = frame_executive::Executive< + Runtime, + Block, + frame_system::ChainContext, + Runtime, + AllPalletsWithSystem, + >; + + // Simple example of a pallet with current version 10 being added to the runtime for the first + // time. + TestExternalities::default().execute_with(|| { + let current_version = Example::current_storage_version(); + + // Check the pallet has no storage items set. + let pallet_hashed_prefix = twox_128(Example::name().as_bytes()); + let exists = contains_prefixed_key(&pallet_hashed_prefix); + assert_eq!(exists, false); + + // [`frame_support::traits::BeforeAllRuntimeMigrations`] hook should initialize the storage + // version. + Executive::execute_on_runtime_upgrade(); + + // Check that the storage version was initialized to the current version + let on_chain_version_after = StorageVersion::get::(); + assert_eq!(on_chain_version_after, current_version); + }); + + // Pallet with no current storage version should have the on-chain version initialized to 0. + TestExternalities::default().execute_with(|| { + // Example4 current_storage_version is NoStorageVersionSet. + + // Check the pallet has no storage items set. + let pallet_hashed_prefix = twox_128(Example4::name().as_bytes()); + let exists = contains_prefixed_key(&pallet_hashed_prefix); + assert_eq!(exists, false); + + // Confirm the storage version is implicitly 0. + let on_chain_version_before = StorageVersion::get::(); + assert_eq!(on_chain_version_before, StorageVersion::new(0)); + + // [`frame_support::traits::BeforeAllRuntimeMigrations`] initializes the storage version. + Executive::execute_on_runtime_upgrade(); + + // Check that the storage version now exists and was initialized to 0. + let on_chain_version_after = StorageVersion::get::(); + assert_eq!(StorageVersion::exists::(), true); + assert_eq!(on_chain_version_after, StorageVersion::new(0)); + }); +} + #[cfg(feature = "try-runtime")] #[test] fn post_runtime_upgrade_detects_storage_version_issues() { @@ -2382,8 +2434,10 @@ fn post_runtime_upgrade_detects_storage_version_issues() { >; TestExternalities::default().execute_with(|| { - // Call `on_genesis` to put the storage version of `Example` into the storage. - Example::on_genesis(); + // Set the on-chain version to one less than the current version for `Example`, simulating a + // forgotten migration + StorageVersion::new(9).put::(); + // The version isn't changed, we should detect it. assert!( Executive::try_runtime_upgrade(UpgradeCheckSelect::PreAndPost).unwrap_err() == From b8a21fe080adf8f34db592109946d285cbd26d7c Mon Sep 17 00:00:00 2001 From: Alexandru Gheorghe <49718502+alexggh@users.noreply.github.com> Date: Tue, 7 Nov 2023 12:10:19 +0200 Subject: [PATCH 04/12] zombienet_tests: Fix genesis error in 0006-parachains-max-tranche0.toml (#2191) There was a race in merging between https://github.com/paritytech/polkadot-sdk/pull/1256 and https://github.com/paritytech/polkadot-sdk/pull/1178, so this newly added tests wasn't updated with the new path for the configuration, so fix that. Signed-off-by: Alexandru Gheorghe --- .../functional/0006-parachains-max-tranche0.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/polkadot/zombienet_tests/functional/0006-parachains-max-tranche0.toml b/polkadot/zombienet_tests/functional/0006-parachains-max-tranche0.toml index 68ac90f5a7e1..bef54cb8ca41 100644 --- a/polkadot/zombienet_tests/functional/0006-parachains-max-tranche0.toml +++ b/polkadot/zombienet_tests/functional/0006-parachains-max-tranche0.toml @@ -2,7 +2,7 @@ timeout = 1000 bootnode = true -[relaychain.genesis.runtime.configuration.config] +[relaychain.genesis.runtimeGenesis.patch.configuration.config] max_validators_per_core = 1 needed_approvals = 7 relay_vrf_modulo_samples = 5 From ec9d9e03f604c40ca356be50f168fe51cb66433f Mon Sep 17 00:00:00 2001 From: Xiliang Chen Date: Tue, 7 Nov 2023 23:46:57 +1300 Subject: [PATCH 05/12] mark pallet-asset-rate optional in polkadot-runtime-common (#2187) Part of #2186 The only usage of pallet-asset-rate is guarded by `runtime-benchmarks` feature. I don't want ORML to be forced to include this pallet in deps for no good reason. --- polkadot/runtime/common/Cargo.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/polkadot/runtime/common/Cargo.toml b/polkadot/runtime/common/Cargo.toml index a6642f9c9c77..0882e555aafe 100644 --- a/polkadot/runtime/common/Cargo.toml +++ b/polkadot/runtime/common/Cargo.toml @@ -39,7 +39,7 @@ pallet-timestamp = { path = "../../../substrate/frame/timestamp", default-featur pallet-vesting = { path = "../../../substrate/frame/vesting", default-features = false } pallet-transaction-payment = { path = "../../../substrate/frame/transaction-payment", default-features = false } pallet-treasury = { path = "../../../substrate/frame/treasury", default-features = false } -pallet-asset-rate = { path = "../../../substrate/frame/asset-rate", default-features = false } +pallet-asset-rate = { path = "../../../substrate/frame/asset-rate", default-features = false, optional = true } pallet-election-provider-multi-phase = { path = "../../../substrate/frame/election-provider-multi-phase", default-features = false } frame-election-provider-support = { path = "../../../substrate/frame/election-provider-support", default-features = false } @@ -80,7 +80,7 @@ std = [ "inherents/std", "libsecp256k1/std", "log/std", - "pallet-asset-rate/std", + "pallet-asset-rate?/std", "pallet-authorship/std", "pallet-balances/std", "pallet-election-provider-multi-phase/std", From d35e3e046357106ee63185de0281daf6d7573104 Mon Sep 17 00:00:00 2001 From: vuittont60 <81072379+vuittont60@users.noreply.github.com> Date: Tue, 7 Nov 2023 20:11:06 +0800 Subject: [PATCH 06/12] docs: fix typos (#2193) --- bridges/zombienet/README.md | 2 +- cumulus/parachains/runtimes/assets/README.md | 2 +- polkadot/roadmap/implementers-guide/src/disputes-flow.md | 2 +- .../src/node/backing/statement-distribution-legacy.md | 2 +- .../src/node/backing/statement-distribution.md | 2 +- .../src/node/disputes/dispute-distribution.md | 2 +- .../src/node/utility/candidate-validation.md | 2 +- .../src/node/utility/pvf-host-and-workers.md | 2 +- .../implementers-guide/src/node/utility/pvf-prechecker.md | 2 +- polkadot/roadmap/implementers-guide/src/protocol-approval.md | 2 +- polkadot/roadmap/implementers-guide/src/runtime/hrmp.md | 2 +- polkadot/roadmap/implementers-guide/src/runtime/inclusion.md | 2 +- polkadot/roadmap/implementers-guide/src/runtime/paras.md | 2 +- polkadot/roadmap/implementers-guide/src/runtime/scheduler.md | 2 +- polkadot/roadmap/implementers-guide/src/types/backing.md | 2 +- substrate/bin/utils/subkey/README.md | 4 ++-- substrate/client/consensus/beefy/README.md | 4 ++-- substrate/client/transaction-pool/README.md | 2 +- substrate/docs/CHANGELOG.md | 2 +- substrate/docs/Upgrading-2.0-to-3.0.md | 2 +- substrate/frame/root-testing/README.md | 2 +- substrate/primitives/maybe-compressed-blob/README.md | 2 +- substrate/test-utils/runtime/res/README.md | 2 +- 23 files changed, 25 insertions(+), 25 deletions(-) diff --git a/bridges/zombienet/README.md b/bridges/zombienet/README.md index 2da3093f4f01..7f7de770814b 100644 --- a/bridges/zombienet/README.md +++ b/bridges/zombienet/README.md @@ -2,7 +2,7 @@ This folder contains [zombienet](https://github.com/paritytech/zombienet/) based integration tests for both onchain and offchain bridges code. Due to some -[technical diffuculties](https://github.com/paritytech/parity-bridges-common/pull/2649#issue-1965339051), we +[technical difficulties](https://github.com/paritytech/parity-bridges-common/pull/2649#issue-1965339051), we are using native zombienet provider, which means that you need to build some binaries locally. To start those tests, you need to: diff --git a/cumulus/parachains/runtimes/assets/README.md b/cumulus/parachains/runtimes/assets/README.md index 78145395cbf9..05466e537f42 100644 --- a/cumulus/parachains/runtimes/assets/README.md +++ b/cumulus/parachains/runtimes/assets/README.md @@ -8,7 +8,7 @@ Asset Hub allows users to: - Deploy promise-backed assets, both fungible and non-fungible, with a DOT/KSM deposit. - Set admin roles to manage assets and asset classes. - Register assets as "self-sufficient" if the Relay Chain agrees, i.e. gain the ability for an - asset to justify the existance of accounts sans DOT/KSM. + asset to justify the existence of accounts sans DOT/KSM. - Pay transaction fees using sufficient assets. - Transfer (and approve transfer) assets. - Interact with the chain via its transactional API or XCM. diff --git a/polkadot/roadmap/implementers-guide/src/disputes-flow.md b/polkadot/roadmap/implementers-guide/src/disputes-flow.md index f9fd8dcce351..b5cc5611c6ff 100644 --- a/polkadot/roadmap/implementers-guide/src/disputes-flow.md +++ b/polkadot/roadmap/implementers-guide/src/disputes-flow.md @@ -52,7 +52,7 @@ stateDiagram-v2 IncomingRequestDisputeAvailabilityData --> RespondUnavailable IncomingRequestDisputeAvailabilityData --> DisputeDataAvail DisputeDataAvail --> RespondWithDisputeAvailabilityData: Send - VoteGossipReceived --> Track: implies source peer has
dispute availablity data + VoteGossipReceived --> Track: implies source peer has
dispute availability data ``` --- diff --git a/polkadot/roadmap/implementers-guide/src/node/backing/statement-distribution-legacy.md b/polkadot/roadmap/implementers-guide/src/node/backing/statement-distribution-legacy.md index e10a55010b91..710055144665 100644 --- a/polkadot/roadmap/implementers-guide/src/node/backing/statement-distribution-legacy.md +++ b/polkadot/roadmap/implementers-guide/src/node/backing/statement-distribution-legacy.md @@ -149,7 +149,7 @@ The receiver of such a message needs to request the actual payload via request/r This is necessary as distribution of a large payload (mega bytes) via gossip would make the network collapse and timely distribution of statements would no longer be possible. By using request/response it is ensured that each peer only -transferes large data once. We only take good care to detect an overloaded peer early and immediately move on to a +transfers large data once. We only take good care to detect an overloaded peer early and immediately move on to a different peer for fetching the data. This mechanism should result in a good load distribution and therefore a rather optimal distribution path. diff --git a/polkadot/roadmap/implementers-guide/src/node/backing/statement-distribution.md b/polkadot/roadmap/implementers-guide/src/node/backing/statement-distribution.md index 24f2785f8294..86a1bf121413 100644 --- a/polkadot/roadmap/implementers-guide/src/node/backing/statement-distribution.md +++ b/polkadot/roadmap/implementers-guide/src/node/backing/statement-distribution.md @@ -3,7 +3,7 @@ This subsystem is responsible for distributing signed statements that we have generated and forwarding statements generated by our peers. Received candidate receipts and statements are passed to the [Candidate Backing subsystem](candidate-backing.md) to handle producing local statements. On receiving -`StatementDistributionMessage::Share`, this subsystem distributes the message across the network with redundency to +`StatementDistributionMessage::Share`, this subsystem distributes the message across the network with redundancy to ensure a fast backing process. ## Overview diff --git a/polkadot/roadmap/implementers-guide/src/node/disputes/dispute-distribution.md b/polkadot/roadmap/implementers-guide/src/node/disputes/dispute-distribution.md index 4547e02352ec..e916d246060e 100644 --- a/polkadot/roadmap/implementers-guide/src/node/disputes/dispute-distribution.md +++ b/polkadot/roadmap/implementers-guide/src/node/disputes/dispute-distribution.md @@ -297,7 +297,7 @@ is `500ms` and above `RATE_LIMIT` is `100ms`. 1/3 of validators are malicious, so for 1000 this means around 330 malicious actors worst case. All those actors can send a message every `100ms`, that is 10 per second. This -means at the begining of an attack they can open up around 3300 batches. Each +means at the beginning of an attack they can open up around 3300 batches. Each containing two votes. So memory usage is still negligible. In reality it is even less, as we also demand 10 new votes to trickle in per batch in order to keep it alive, every `500ms`. Hence for the first second, each batch requires 20 votes diff --git a/polkadot/roadmap/implementers-guide/src/node/utility/candidate-validation.md b/polkadot/roadmap/implementers-guide/src/node/utility/candidate-validation.md index 376fa187de1b..e252ec237b79 100644 --- a/polkadot/roadmap/implementers-guide/src/node/utility/candidate-validation.md +++ b/polkadot/roadmap/implementers-guide/src/node/utility/candidate-validation.md @@ -31,7 +31,7 @@ all the necessary parameters to the validation function. These are: * The [`ValidationData`](../../types/candidate.md#validationdata). * The [`PoV`](../../types/availability.md#proofofvalidity). -The second category is for PVF pre-checking. This is primarly used by the [PVF pre-checker](pvf-prechecker.md) +The second category is for PVF pre-checking. This is primarily used by the [PVF pre-checker](pvf-prechecker.md) subsystem. ### Determining Parameters diff --git a/polkadot/roadmap/implementers-guide/src/node/utility/pvf-host-and-workers.md b/polkadot/roadmap/implementers-guide/src/node/utility/pvf-host-and-workers.md index 0cefeb1f77ca..52129f9eb80a 100644 --- a/polkadot/roadmap/implementers-guide/src/node/utility/pvf-host-and-workers.md +++ b/polkadot/roadmap/implementers-guide/src/node/utility/pvf-host-and-workers.md @@ -44,7 +44,7 @@ execution request: We use timeouts for both preparation and execution jobs to limit the amount of time they can take. As the time for a job can vary depending on the machine and load on the machine, this can potentially lead to disputes where some validators -successfuly execute a PVF and others don't. +successfully execute a PVF and others don't. One dispute mitigation we have in place is a more lenient timeout for preparation during execution than during pre-checking. The rationale is that the diff --git a/polkadot/roadmap/implementers-guide/src/node/utility/pvf-prechecker.md b/polkadot/roadmap/implementers-guide/src/node/utility/pvf-prechecker.md index b722bdc6539a..f0de50f2267b 100644 --- a/polkadot/roadmap/implementers-guide/src/node/utility/pvf-prechecker.md +++ b/polkadot/roadmap/implementers-guide/src/node/utility/pvf-prechecker.md @@ -24,7 +24,7 @@ relevant. When a PVF just becomes relevant, the subsystem will send a message to the [Candidate Validation] subsystem asking for the pre-check. -Upon receving a message from the candidate-validation subsystem, the pre-checker will note down that the PVF has its +Upon receiving a message from the candidate-validation subsystem, the pre-checker will note down that the PVF has its judgement and will also sign and submit a [`PvfCheckStatement`][PvfCheckStatement] via the [`submit_pvf_check_statement` runtime API][PVF pre-checking runtime API]. In case, a judgement was received for a PVF that is no longer in view it is ignored. diff --git a/polkadot/roadmap/implementers-guide/src/protocol-approval.md b/polkadot/roadmap/implementers-guide/src/protocol-approval.md index 4fac84591456..70bc0233d65a 100644 --- a/polkadot/roadmap/implementers-guide/src/protocol-approval.md +++ b/polkadot/roadmap/implementers-guide/src/protocol-approval.md @@ -330,7 +330,7 @@ We prefer doing approval checkers assignments under `RelayVRFModulo` or `RelayVR assignments benefit security the most. We suggest assigning at least 16 checkers under `RelayVRFModulo` or `RelayVRFModuloCompact` although assignment levels have never been properly analyzed. -Our delay criteria `RelayVRFDelay` and `RelayEquivocation` both have two primary paramaters, expected checkers per +Our delay criteria `RelayVRFDelay` and `RelayEquivocation` both have two primary parameters, expected checkers per tranche and the zeroth delay tranche width. We require expected checkers per tranche to be less than three because otherwise an adversary with 1/3 stake could force diff --git a/polkadot/roadmap/implementers-guide/src/runtime/hrmp.md b/polkadot/roadmap/implementers-guide/src/runtime/hrmp.md index aa31491d72f6..69d33ca8670d 100644 --- a/polkadot/roadmap/implementers-guide/src/runtime/hrmp.md +++ b/polkadot/roadmap/implementers-guide/src/runtime/hrmp.md @@ -108,7 +108,7 @@ HrmpEgressChannelsIndex: map ParaId => Vec; /// Invariant: cannot be non-empty if the corresponding channel in `HrmpChannels` is `None`. HrmpChannelContents: map HrmpChannelId => Vec; /// Maintains a mapping that can be used to answer the question: -/// What paras sent a message at the given block number for a given reciever. +/// What paras sent a message at the given block number for a given receiver. /// Invariants: /// - The inner `Vec` is never empty. /// - The inner `Vec` cannot store two same `ParaId`. diff --git a/polkadot/roadmap/implementers-guide/src/runtime/inclusion.md b/polkadot/roadmap/implementers-guide/src/runtime/inclusion.md index f73df209981a..f6a32a01d502 100644 --- a/polkadot/roadmap/implementers-guide/src/runtime/inclusion.md +++ b/polkadot/roadmap/implementers-guide/src/runtime/inclusion.md @@ -133,7 +133,7 @@ All failed checks should lead to an unrecoverable error making the block invalid [`UpwardMessage`s](../types/messages.md#upward-message) from the [`CandidateCommitments`](../types/candidate.md#candidate-commitments). 1. call `Dmp::prune_dmq` with the para id of the candidate and the candidate's `processed_downward_messages`. - 1. call `Hrmp::prune_hrmp` with the para id of the candiate and the candidate's `hrmp_watermark`. + 1. call `Hrmp::prune_hrmp` with the para id of the candidate and the candidate's `hrmp_watermark`. 1. call `Hrmp::queue_outbound_hrmp` with the para id of the candidate and the list of horizontal messages taken from the commitment, 1. Call `Paras::note_new_head` using the `HeadData` from the receipt and `relay_parent_number`. diff --git a/polkadot/roadmap/implementers-guide/src/runtime/paras.md b/polkadot/roadmap/implementers-guide/src/runtime/paras.md index ac9b1520c3df..e6e0b53512fe 100644 --- a/polkadot/roadmap/implementers-guide/src/runtime/paras.md +++ b/polkadot/roadmap/implementers-guide/src/runtime/paras.md @@ -189,7 +189,7 @@ UpgradeGoAheadSignal: map hasher(twox_64_concat) ParaId => Option)`: indicate previosuly-occupied cores which are to be considered returned +- `free_cores(Vec<(CoreIndex, FreedReason)>)`: indicate previously-occupied cores which are to be considered returned and why they are being returned. - All freed lease holding parachain cores should be assigned to their respective parachain - All freed on-demand parachain cores whose reason for freeing was `FreedReason::Concluded` should have the claim diff --git a/polkadot/roadmap/implementers-guide/src/types/backing.md b/polkadot/roadmap/implementers-guide/src/types/backing.md index 7e43325ec5cd..bfe182383fd5 100644 --- a/polkadot/roadmap/implementers-guide/src/types/backing.md +++ b/polkadot/roadmap/implementers-guide/src/types/backing.md @@ -30,7 +30,7 @@ work, we extract a signed wrapper. ```rust,ignore /// A signed type which encapsulates the common desire to sign some data and validate a signature. /// -/// Note that the internal fields are not public; they are all accessable by immutable getters. +/// Note that the internal fields are not public; they are all accessible by immutable getters. /// This reduces the chance that they are accidentally mutated, invalidating the signature. struct Signed { /// The payload is part of the signed data. The rest is the signing context, diff --git a/substrate/bin/utils/subkey/README.md b/substrate/bin/utils/subkey/README.md index 60e5a9ca9350..3232c8295872 100644 --- a/substrate/bin/utils/subkey/README.md +++ b/substrate/bin/utils/subkey/README.md @@ -74,7 +74,7 @@ The output above shows a **secret phrase** (also called **mnemonic phrase**) and **Private Key**). Those 2 secrets are the pieces of information you MUST keep safe and secret. All the other information below can be derived from those secrets. -The output above also show the **public key** and the **Account ID**. Those are the independant from the network where +The output above also show the **public key** and the **Account ID**. Those are the independent from the network where you will use the key. The **SS58 address** (or **Public Address**) of a new account is a reprensentation of the public keys of an account for @@ -163,7 +163,7 @@ This time, we properly recovered `5He5pZpc7AJ8evPuab37vJF6KkFDqq9uDq2WXh877Qw6ia ### Inspecting a key -If you have *some data* about a key, `subkey inpsect` will help you discover more information about it. +If you have *some data* about a key, `subkey inspect` will help you discover more information about it. If you have **secrets** that you would like to verify for instance, you can use: diff --git a/substrate/client/consensus/beefy/README.md b/substrate/client/consensus/beefy/README.md index 1a5a9667fdbb..13f88303a972 100644 --- a/substrate/client/consensus/beefy/README.md +++ b/substrate/client/consensus/beefy/README.md @@ -104,7 +104,7 @@ shortcuts: ## Mental Model BEEFY should be considered as an extra voting round done by GRANDPA validators for the current -best finalized block. Similarily to how GRANDPA is lagging behind best produced (non-finalized) +best finalized block. Similarly to how GRANDPA is lagging behind best produced (non-finalized) block, BEEFY is going to lag behind best GRANDPA (finalized) block. ``` @@ -302,7 +302,7 @@ periodically on the global topic. Let's now dive into description of the message ## Misbehavior -Similarily to other PoS protocols, BEEFY considers casting two different votes in the same round a +Similarly to other PoS protocols, BEEFY considers casting two different votes in the same round a misbehavior. I.e. for a particular `round_number`, the validator produces signatures for 2 different `Commitment`s and broadcasts them. This is called **equivocation**. diff --git a/substrate/client/transaction-pool/README.md b/substrate/client/transaction-pool/README.md index b34b623b26e0..b55dc6482d64 100644 --- a/substrate/client/transaction-pool/README.md +++ b/substrate/client/transaction-pool/README.md @@ -227,7 +227,7 @@ transactions that are prepared for block inclusion. Propagation is best effort, especially for block authors and is not directly incentivised. However the networking protocol might penalise peers that send invalid or useless transactions so we should be nice to others. Also see below a proposal -to instead of gossiping everyting have other peers request transactions they +to instead of gossiping everything have other peers request transactions they are interested in. Since the pool is expected to store more transactions than what can fit diff --git a/substrate/docs/CHANGELOG.md b/substrate/docs/CHANGELOG.md index 8a1b245a5b0c..042b7b8a2f01 100644 --- a/substrate/docs/CHANGELOG.md +++ b/substrate/docs/CHANGELOG.md @@ -45,7 +45,7 @@ board](https://github.com/paritytech/substrate/discussions). * Allow validators to block and kick their nominator set. (#7930) * Decouple Staking and Election - Part1: Support traits (#7908) * Introduces account existence providers reference counting (#7363) -* contracts: Cap the surcharge reward by the amount of rent that way payed by a contract (#7870) +* contracts: Cap the surcharge reward by the amount of rent that way paid by a contract (#7870) * Use checked math when calculating storage size (#7885) * Fix clear prefix check to avoid erasing child trie roots. (#7848) * contracts: Collect rent for the first block during deployment (#7847) diff --git a/substrate/docs/Upgrading-2.0-to-3.0.md b/substrate/docs/Upgrading-2.0-to-3.0.md index 3f2a3e7c5be2..1be41a34ef34 100644 --- a/substrate/docs/Upgrading-2.0-to-3.0.md +++ b/substrate/docs/Upgrading-2.0-to-3.0.md @@ -113,7 +113,7 @@ And update the overall definition for weights on frame and a few related types a ```diff= -const AVERAGE_ON_INITIALIZE_WEIGHT: Perbill = Perbill::from_percent(10); -+/// We assume that ~10% of the block weight is consumed by `on_initalize` handlers. ++/// We assume that ~10% of the block weight is consumed by `on_initialize` handlers. +/// This is used to limit the maximal weight of a single extrinsic. +const AVERAGE_ON_INITIALIZE_RATIO: Perbill = Perbill::from_percent(10); +/// We allow `Normal` extrinsics to fill up the block up to 75%, the rest can be used diff --git a/substrate/frame/root-testing/README.md b/substrate/frame/root-testing/README.md index aa231e3ef20a..a0eeda5cc6aa 100644 --- a/substrate/frame/root-testing/README.md +++ b/substrate/frame/root-testing/README.md @@ -1,5 +1,5 @@ # Root Testing Pallet -Pallet that contains extrinsics that can be usefull in testing. +Pallet that contains extrinsics that can be useful in testing. NOTE: This pallet should only be used for testing purposes and should not be used in production runtimes! diff --git a/substrate/primitives/maybe-compressed-blob/README.md b/substrate/primitives/maybe-compressed-blob/README.md index b5bb869c30e4..a2f6a1c53432 100644 --- a/substrate/primitives/maybe-compressed-blob/README.md +++ b/substrate/primitives/maybe-compressed-blob/README.md @@ -1,3 +1,3 @@ -Handling of blobs, typicaly validation code, which may be compressed. +Handling of blobs, typically validation code, which may be compressed. License: Apache-2.0 diff --git a/substrate/test-utils/runtime/res/README.md b/substrate/test-utils/runtime/res/README.md index 6d6ae55c3463..fdf94fd9d876 100644 --- a/substrate/test-utils/runtime/res/README.md +++ b/substrate/test-utils/runtime/res/README.md @@ -20,5 +20,5 @@ comparing keys, not the values. `renamed_authorities`. -`default_genesis_config_invalid.json` is just an imcomplete copy of +`default_genesis_config_invalid.json` is just an incomplete copy of `default_genesis_config.json` with `babe::authorities` field removed. From 46c421bfbd5257428fc8069457e2d373d830a5c6 Mon Sep 17 00:00:00 2001 From: Bill Laboon Date: Tue, 7 Nov 2023 16:20:36 +0100 Subject: [PATCH 07/12] Fix "slashaed" typo (#2205) # Description This merely fixes a typo in the documentation, replacing the typo "slashaed" with "slashed". Since external entities use the comments for explanations of events, this will then be shown externally. I noticed this when reviewing [this event](https://polkadot.subscan.io/extrinsic/0xb6bc1e3abde0c2ed9c500c74cfc64cdb8179e5d9af97f4bf53242ce4cdd15a1d?event=18064194-6) on Subscan. This is not related to any other issues or PRs. --- substrate/frame/referenda/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/referenda/src/lib.rs b/substrate/frame/referenda/src/lib.rs index be21375f526f..8912f9ad2173 100644 --- a/substrate/frame/referenda/src/lib.rs +++ b/substrate/frame/referenda/src/lib.rs @@ -305,7 +305,7 @@ pub mod pallet { /// The amount placed by the account. amount: BalanceOf, }, - /// A deposit has been slashaed. + /// A deposit has been slashed. DepositSlashed { /// The account who placed the deposit. who: T::AccountId, From 2c3b432c7de19b8cd7f3f734e109218f9c471af5 Mon Sep 17 00:00:00 2001 From: Sebastian Kunert Date: Tue, 7 Nov 2023 17:21:24 +0100 Subject: [PATCH 08/12] Disable incoming light-client connections for minimal relay node (#2202) When running with `--relay-chain-rpc-url` we received multiple reports of high traffic that disappears when `--in-peers-light 0` is set. Indeed it does not make much sense for light clients to connect to the minimal node since it is not running the block announce protocol and the request/response protocol for light clients. This is intended to alleviate the traffic issues for now. closes #1896 probably related https://github.com/paritytech/cumulus/issues/2563 --- .../relay-chain-minimal-node/src/network.rs | 25 ++++++++++++++++--- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/cumulus/client/relay-chain-minimal-node/src/network.rs b/cumulus/client/relay-chain-minimal-node/src/network.rs index f39d7a26dd88..813dca47a039 100644 --- a/cumulus/client/relay-chain-minimal-node/src/network.rs +++ b/cumulus/client/relay-chain-minimal-node/src/network.rs @@ -19,7 +19,8 @@ use sp_runtime::traits::{Block as BlockT, NumberFor}; use sc_network::{ config::{ - NonDefaultSetConfig, NonReservedPeerMode, NotificationHandshake, ProtocolId, SetConfig, + NetworkConfiguration, NonDefaultSetConfig, NonReservedPeerMode, NotificationHandshake, + ProtocolId, SetConfig, }, peer_store::PeerStore, NetworkService, @@ -35,7 +36,7 @@ use std::{iter, sync::Arc}; /// Build the network service, the network status sinks and an RPC sender. pub(crate) fn build_collator_network( config: &Configuration, - network_config: FullNetworkConfiguration, + mut full_network_config: FullNetworkConfiguration, spawn_handle: SpawnTaskHandle, genesis_hash: Hash, best_header: Header, @@ -53,8 +54,12 @@ pub(crate) fn build_collator_network( genesis_hash, ); + // Since this node has no syncing, we do not want light-clients to connect to it. + // Here we set any potential light-client slots to 0. + adjust_network_config_light_in_peers(&mut full_network_config.network_config); + let peer_store = PeerStore::new( - network_config + full_network_config .network_config .boot_nodes .iter() @@ -75,7 +80,7 @@ pub(crate) fn build_collator_network( }) }, fork_id: None, - network_config, + network_config: full_network_config, peer_store: peer_store_handle, genesis_hash, protocol_id, @@ -114,6 +119,18 @@ pub(crate) fn build_collator_network( Ok((network_service, network_starter, Box::new(SyncOracle {}))) } +fn adjust_network_config_light_in_peers(config: &mut NetworkConfiguration) { + let light_client_in_peers = (config.default_peers_set.in_peers + + config.default_peers_set.out_peers) + .saturating_sub(config.default_peers_set_num_full); + if light_client_in_peers > 0 { + tracing::debug!(target: crate::LOG_TARGET, "Detected {light_client_in_peers} peer slots for light clients. Since this minimal node does support\ + neither syncing nor light-client request/response, we are setting them to 0."); + } + config.default_peers_set.in_peers = + config.default_peers_set.in_peers.saturating_sub(light_client_in_peers); +} + struct SyncOracle; impl sp_consensus::SyncOracle for SyncOracle { From 0d350b67e716f1c52058f19a191a5aeec1a62c03 Mon Sep 17 00:00:00 2001 From: Francisco Aguirre Date: Wed, 8 Nov 2023 06:39:40 +0100 Subject: [PATCH 09/12] XCM builder pattern (#2107) Added a proc macro to be able to write XCMs using the builder pattern. This means we go from having to do this: ```rust let message: Xcm<()> = Xcm(vec![ WithdrawAsset(assets), BuyExecution { fees: asset, weight_limit: Unlimited }, DepositAsset { assets, beneficiary }, ]); ``` to this: ```rust let message: Xcm<()> = Xcm::builder() .withdraw_asset(assets) .buy_execution(asset, Unlimited), .deposit_asset(assets, beneficiary) .build(); ``` --------- Co-authored-by: Keith Yeung Co-authored-by: command-bot <> --- Cargo.lock | 1 + polkadot/xcm/procedural/Cargo.toml | 5 + .../xcm/procedural/src/builder_pattern.rs | 115 ++++++++++++++++++ polkadot/xcm/procedural/src/lib.rs | 13 ++ polkadot/xcm/procedural/tests/ui.rs | 32 +++++ .../procedural/tests/ui/builder_pattern.rs | 25 ++++ .../tests/ui/builder_pattern.stderr | 5 + polkadot/xcm/src/v3/mod.rs | 9 +- polkadot/xcm/xcm-simulator/example/src/lib.rs | 19 +++ prdoc/pr_2107.prdoc | 24 ++++ 10 files changed, 247 insertions(+), 1 deletion(-) create mode 100644 polkadot/xcm/procedural/src/builder_pattern.rs create mode 100644 polkadot/xcm/procedural/tests/ui.rs create mode 100644 polkadot/xcm/procedural/tests/ui/builder_pattern.rs create mode 100644 polkadot/xcm/procedural/tests/ui/builder_pattern.stderr create mode 100644 prdoc/pr_2107.prdoc diff --git a/Cargo.lock b/Cargo.lock index 764d4ed01aa5..70b0d43120f1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -21176,6 +21176,7 @@ dependencies = [ "proc-macro2", "quote", "syn 2.0.38", + "trybuild", ] [[package]] diff --git a/polkadot/xcm/procedural/Cargo.toml b/polkadot/xcm/procedural/Cargo.toml index 56df0d94f586..33c2a94be0e4 100644 --- a/polkadot/xcm/procedural/Cargo.toml +++ b/polkadot/xcm/procedural/Cargo.toml @@ -1,9 +1,11 @@ [package] name = "xcm-procedural" +description = "Procedural macros for XCM" authors.workspace = true edition.workspace = true license.workspace = true version = "1.0.0" +publish = true [lib] proc-macro = true @@ -13,3 +15,6 @@ proc-macro2 = "1.0.56" quote = "1.0.28" syn = "2.0.38" Inflector = "0.11.4" + +[dev-dependencies] +trybuild = { version = "1.0.74", features = ["diff"] } diff --git a/polkadot/xcm/procedural/src/builder_pattern.rs b/polkadot/xcm/procedural/src/builder_pattern.rs new file mode 100644 index 000000000000..ebad54e972b6 --- /dev/null +++ b/polkadot/xcm/procedural/src/builder_pattern.rs @@ -0,0 +1,115 @@ +// Copyright (C) Parity Technologies (UK) Ltd. +// This file is part of Polkadot. + +// Polkadot is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Polkadot is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Polkadot. If not, see . + +//! Derive macro for creating XCMs with a builder pattern + +use inflector::Inflector; +use proc_macro::TokenStream; +use proc_macro2::TokenStream as TokenStream2; +use quote::{format_ident, quote}; +use syn::{ + parse_macro_input, Data, DeriveInput, Error, Expr, ExprLit, Fields, Lit, Meta, MetaNameValue, +}; + +pub fn derive(input: TokenStream) -> TokenStream { + let input = parse_macro_input!(input as DeriveInput); + let builder_impl = match &input.data { + Data::Enum(data_enum) => generate_methods_for_enum(input.ident, data_enum), + _ => + return Error::new_spanned(&input, "Expected the `Instruction` enum") + .to_compile_error() + .into(), + }; + let output = quote! { + pub struct XcmBuilder(Vec>); + impl Xcm { + pub fn builder() -> XcmBuilder { + XcmBuilder::(Vec::new()) + } + } + #builder_impl + }; + output.into() +} + +fn generate_methods_for_enum(name: syn::Ident, data_enum: &syn::DataEnum) -> TokenStream2 { + let methods = data_enum.variants.iter().map(|variant| { + let variant_name = &variant.ident; + let method_name_string = &variant_name.to_string().to_snake_case(); + let method_name = syn::Ident::new(&method_name_string, variant_name.span()); + let docs: Vec<_> = variant + .attrs + .iter() + .filter_map(|attr| match &attr.meta { + Meta::NameValue(MetaNameValue { + value: Expr::Lit(ExprLit { lit: Lit::Str(literal), .. }), + .. + }) if attr.path().is_ident("doc") => Some(literal.value()), + _ => None, + }) + .map(|doc| syn::parse_str::(&format!("/// {}", doc)).unwrap()) + .collect(); + let method = match &variant.fields { + Fields::Unit => { + quote! { + pub fn #method_name(mut self) -> Self { + self.0.push(#name::::#variant_name); + self + } + } + }, + Fields::Unnamed(fields) => { + let arg_names: Vec<_> = fields + .unnamed + .iter() + .enumerate() + .map(|(index, _)| format_ident!("arg{}", index)) + .collect(); + let arg_types: Vec<_> = fields.unnamed.iter().map(|field| &field.ty).collect(); + quote! { + pub fn #method_name(mut self, #(#arg_names: #arg_types),*) -> Self { + self.0.push(#name::::#variant_name(#(#arg_names),*)); + self + } + } + }, + Fields::Named(fields) => { + let arg_names: Vec<_> = fields.named.iter().map(|field| &field.ident).collect(); + let arg_types: Vec<_> = fields.named.iter().map(|field| &field.ty).collect(); + quote! { + pub fn #method_name(mut self, #(#arg_names: #arg_types),*) -> Self { + self.0.push(#name::::#variant_name { #(#arg_names),* }); + self + } + } + }, + }; + quote! { + #(#docs)* + #method + } + }); + let output = quote! { + impl XcmBuilder { + #(#methods)* + + pub fn build(self) -> Xcm { + Xcm(self.0) + } + } + }; + output +} diff --git a/polkadot/xcm/procedural/src/lib.rs b/polkadot/xcm/procedural/src/lib.rs index 2ebccadf50be..83cc6cdf98ff 100644 --- a/polkadot/xcm/procedural/src/lib.rs +++ b/polkadot/xcm/procedural/src/lib.rs @@ -18,6 +18,7 @@ use proc_macro::TokenStream; +mod builder_pattern; mod v2; mod v3; mod weight_info; @@ -47,3 +48,15 @@ pub fn impl_conversion_functions_for_junctions_v3(input: TokenStream) -> TokenSt .unwrap_or_else(syn::Error::into_compile_error) .into() } + +/// This is called on the `Instruction` enum, not on the `Xcm` struct, +/// and allows for the following syntax for building XCMs: +/// let message = Xcm::builder() +/// .withdraw_asset(assets) +/// .buy_execution(fees, weight_limit) +/// .deposit_asset(assets, beneficiary) +/// .build(); +#[proc_macro_derive(Builder)] +pub fn derive_builder(input: TokenStream) -> TokenStream { + builder_pattern::derive(input) +} diff --git a/polkadot/xcm/procedural/tests/ui.rs b/polkadot/xcm/procedural/tests/ui.rs new file mode 100644 index 000000000000..a6ec35d0862a --- /dev/null +++ b/polkadot/xcm/procedural/tests/ui.rs @@ -0,0 +1,32 @@ +// Copyright (C) Parity Technologies (UK) Ltd. +// This file is part of Polkadot. + +// Polkadot is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Polkadot is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Polkadot. If not, see . + +//! UI tests for XCM procedural macros + +#[cfg(not(feature = "disable-ui-tests"))] +#[test] +fn ui() { + // Only run the ui tests when `RUN_UI_TESTS` is set. + if std::env::var("RUN_UI_TESTS").is_err() { + return; + } + + // As trybuild is using `cargo check`, we don't need the real WASM binaries. + std::env::set_var("SKIP_WASM_BUILD", "1"); + + let t = trybuild::TestCases::new(); + t.compile_fail("tests/ui/*.rs"); +} diff --git a/polkadot/xcm/procedural/tests/ui/builder_pattern.rs b/polkadot/xcm/procedural/tests/ui/builder_pattern.rs new file mode 100644 index 000000000000..e4bcda572ad7 --- /dev/null +++ b/polkadot/xcm/procedural/tests/ui/builder_pattern.rs @@ -0,0 +1,25 @@ +// Copyright (C) Parity Technologies (UK) Ltd. +// This file is part of Polkadot. + +// Polkadot is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Polkadot is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Polkadot. If not, see . + +//! Test error when attaching the derive builder macro to something +//! other than the XCM `Instruction` enum. + +use xcm_procedural::Builder; + +#[derive(Builder)] +struct SomeStruct; + +fn main() {} diff --git a/polkadot/xcm/procedural/tests/ui/builder_pattern.stderr b/polkadot/xcm/procedural/tests/ui/builder_pattern.stderr new file mode 100644 index 000000000000..439b40f31cae --- /dev/null +++ b/polkadot/xcm/procedural/tests/ui/builder_pattern.stderr @@ -0,0 +1,5 @@ +error: Expected the `Instruction` enum + --> tests/ui/builder_pattern.rs:23:1 + | +23 | struct SomeStruct; + | ^^^^^^^^^^^^^^^^^^ diff --git a/polkadot/xcm/src/v3/mod.rs b/polkadot/xcm/src/v3/mod.rs index aa5bfe169ac1..7ec3f6f40af2 100644 --- a/polkadot/xcm/src/v3/mod.rs +++ b/polkadot/xcm/src/v3/mod.rs @@ -404,7 +404,14 @@ impl XcmContext { /// /// This is the inner XCM format and is version-sensitive. Messages are typically passed using the /// outer XCM format, known as `VersionedXcm`. -#[derive(Derivative, Encode, Decode, TypeInfo, xcm_procedural::XcmWeightInfoTrait)] +#[derive( + Derivative, + Encode, + Decode, + TypeInfo, + xcm_procedural::XcmWeightInfoTrait, + xcm_procedural::Builder, +)] #[derivative(Clone(bound = ""), Eq(bound = ""), PartialEq(bound = ""), Debug(bound = ""))] #[codec(encode_bound())] #[codec(decode_bound())] diff --git a/polkadot/xcm/xcm-simulator/example/src/lib.rs b/polkadot/xcm/xcm-simulator/example/src/lib.rs index 85b8ad1c5cb7..03e7c19a9148 100644 --- a/polkadot/xcm/xcm-simulator/example/src/lib.rs +++ b/polkadot/xcm/xcm-simulator/example/src/lib.rs @@ -649,4 +649,23 @@ mod tests { ); }); } + + #[test] + fn builder_pattern_works() { + let asset: MultiAsset = (Here, 100u128).into(); + let beneficiary: MultiLocation = AccountId32 { id: [0u8; 32], network: None }.into(); + let message: Xcm<()> = Xcm::builder() + .withdraw_asset(asset.clone().into()) + .buy_execution(asset.clone(), Unlimited) + .deposit_asset(asset.clone().into(), beneficiary) + .build(); + assert_eq!( + message, + Xcm(vec![ + WithdrawAsset(asset.clone().into()), + BuyExecution { fees: asset.clone(), weight_limit: Unlimited }, + DepositAsset { assets: asset.into(), beneficiary }, + ]) + ); + } } diff --git a/prdoc/pr_2107.prdoc b/prdoc/pr_2107.prdoc new file mode 100644 index 000000000000..0e33680555ac --- /dev/null +++ b/prdoc/pr_2107.prdoc @@ -0,0 +1,24 @@ +# Schema: Parity PR Documentation Schema (prdoc) +# See doc at https://github.com/paritytech/prdoc + +title: Add a builder pattern to create XCM programs + +doc: + - audience: Core Dev + description: | + XCMs can now be built using a builder pattern like so: + Xcm::builder() + .withdraw_asset(assets) + .buy_execution(fees, weight_limit) + .deposit_asset(assets, beneficiary) + .build(); + +migrations: + db: [] + + runtime: [] + +crates: + - name: xcm + +host_functions: [] From ba4c74d9110bbb77024578a953841b28ee1fd452 Mon Sep 17 00:00:00 2001 From: Adrian Catangiu Date: Wed, 8 Nov 2023 11:40:57 +0200 Subject: [PATCH 10/12] [testnets][xcm-emulator] add bridge-hub-westend and hook it up to emulator (#2204) `bridge-hub-westend-runtime` was added to cumulus/parachains, but wasn't hooked up to xcm-emulator to run tests against it. This commit addresses that ^. Signed-off-by: Adrian Catangiu --- Cargo.lock | 30 ++++++++ Cargo.toml | 1 + .../bridges/bridge-hub-westend/Cargo.toml | 41 ++++++++++ .../bridges/bridge-hub-westend/src/lib.rs | 67 +++++++++++++++++ .../bridge-hub-westend/src/tests/example.rs | 74 +++++++++++++++++++ .../bridge-hub-westend/src/tests/mod.rs | 17 +++++ .../bridge-hub-westend/src/tests/teleport.rs | 30 ++++++++ .../emulated/common/Cargo.toml | 2 + .../emulated/common/src/constants.rs | 57 ++++++++++++++ .../emulated/common/src/lib.rs | 27 ++++++- .../bridge-hubs/bridge-hub-rococo/Cargo.toml | 2 +- 11 files changed, 345 insertions(+), 3 deletions(-) create mode 100644 cumulus/parachains/integration-tests/emulated/bridges/bridge-hub-westend/Cargo.toml create mode 100644 cumulus/parachains/integration-tests/emulated/bridges/bridge-hub-westend/src/lib.rs create mode 100644 cumulus/parachains/integration-tests/emulated/bridges/bridge-hub-westend/src/tests/example.rs create mode 100644 cumulus/parachains/integration-tests/emulated/bridges/bridge-hub-westend/src/tests/mod.rs create mode 100644 cumulus/parachains/integration-tests/emulated/bridges/bridge-hub-westend/src/tests/teleport.rs diff --git a/Cargo.lock b/Cargo.lock index 70b0d43120f1..06ab9ed7e2fc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2298,6 +2298,35 @@ dependencies = [ "staging-xcm-executor", ] +[[package]] +name = "bridge-hub-westend-integration-tests" +version = "1.0.0" +dependencies = [ + "asset-test-utils", + "bp-messages", + "bridge-hub-westend-runtime", + "cumulus-pallet-dmp-queue", + "cumulus-pallet-xcmp-queue", + "frame-support", + "frame-system", + "integration-tests-common", + "pallet-assets", + "pallet-balances", + "pallet-bridge-messages", + "pallet-message-queue", + "pallet-xcm", + "parachains-common", + "parity-scale-codec", + "polkadot-core-primitives", + "polkadot-parachain-primitives", + "polkadot-runtime-parachains", + "sp-core", + "sp-weights", + "staging-xcm", + "staging-xcm-executor", + "xcm-emulator", +] + [[package]] name = "bridge-hub-westend-runtime" version = "0.1.0" @@ -6717,6 +6746,7 @@ dependencies = [ "bridge-hub-kusama-runtime", "bridge-hub-polkadot-runtime", "bridge-hub-rococo-runtime", + "bridge-hub-westend-runtime", "bridge-runtime-common", "collectives-polkadot-runtime", "cumulus-pallet-parachain-system", diff --git a/Cargo.toml b/Cargo.toml index ccb952cc963e..2344a24a5d95 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -68,6 +68,7 @@ members = [ "cumulus/parachains/integration-tests/emulated/assets/asset-hub-rococo", "cumulus/parachains/integration-tests/emulated/assets/asset-hub-westend", "cumulus/parachains/integration-tests/emulated/bridges/bridge-hub-rococo", + "cumulus/parachains/integration-tests/emulated/bridges/bridge-hub-westend", "cumulus/parachains/integration-tests/emulated/common", "cumulus/parachains/pallets/collective-content", "cumulus/parachains/pallets/parachain-info", diff --git a/cumulus/parachains/integration-tests/emulated/bridges/bridge-hub-westend/Cargo.toml b/cumulus/parachains/integration-tests/emulated/bridges/bridge-hub-westend/Cargo.toml new file mode 100644 index 000000000000..af5408b888b1 --- /dev/null +++ b/cumulus/parachains/integration-tests/emulated/bridges/bridge-hub-westend/Cargo.toml @@ -0,0 +1,41 @@ +[package] +name = "bridge-hub-westend-integration-tests" +version = "1.0.0" +authors.workspace = true +edition.workspace = true +license = "Apache-2.0" +description = "Bridge Hub Westend runtime integration tests with xcm-emulator" +publish = false + +[dependencies] +codec = { package = "parity-scale-codec", version = "3.4.0", default-features = false } + +# Substrate +frame-support = { path = "../../../../../../substrate/frame/support", default-features = false} +frame-system = { path = "../../../../../../substrate/frame/system", default-features = false} +sp-core = { path = "../../../../../../substrate/primitives/core", default-features = false} +sp-weights = { path = "../../../../../../substrate/primitives/weights", default-features = false} +pallet-balances = { path = "../../../../../../substrate/frame/balances", default-features = false} +pallet-assets = { path = "../../../../../../substrate/frame/assets", default-features = false} +pallet-message-queue = { path = "../../../../../../substrate/frame/message-queue", default-features = false } + +# Polkadot +polkadot-core-primitives = { path = "../../../../../../polkadot/core-primitives", default-features = false} +polkadot-parachain-primitives = { path = "../../../../../../polkadot/parachain", default-features = false} +polkadot-runtime-parachains = { path = "../../../../../../polkadot/runtime/parachains" } +xcm = { package = "staging-xcm", path = "../../../../../../polkadot/xcm", default-features = false} +pallet-xcm = { path = "../../../../../../polkadot/xcm/pallet-xcm", default-features = false} +xcm-executor = { package = "staging-xcm-executor", path = "../../../../../../polkadot/xcm/xcm-executor", default-features = false} + +# Cumulus +asset-test-utils = { path = "../../../../../parachains/runtimes/assets/test-utils", default-features = false } +parachains-common = { path = "../../../../common" } +cumulus-pallet-xcmp-queue = { path = "../../../../../pallets/xcmp-queue", default-features = false} +cumulus-pallet-dmp-queue = { path = "../../../../../pallets/dmp-queue", default-features = false} +pallet-bridge-messages = { path = "../../../../../../bridges/modules/messages", default-features = false} +bp-messages = { path = "../../../../../../bridges/primitives/messages", default-features = false} +bridge-hub-westend-runtime = { path = "../../../../../parachains/runtimes/bridge-hubs/bridge-hub-westend", default-features = false } + +# Local +xcm-emulator = { path = "../../../../../xcm/xcm-emulator", default-features = false} +integration-tests-common = { path = "../../common", default-features = false} diff --git a/cumulus/parachains/integration-tests/emulated/bridges/bridge-hub-westend/src/lib.rs b/cumulus/parachains/integration-tests/emulated/bridges/bridge-hub-westend/src/lib.rs new file mode 100644 index 000000000000..8b17ce53eb00 --- /dev/null +++ b/cumulus/parachains/integration-tests/emulated/bridges/bridge-hub-westend/src/lib.rs @@ -0,0 +1,67 @@ +// 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. + +pub use bp_messages::LaneId; +pub use frame_support::assert_ok; +pub use integration_tests_common::{ + constants::{ + bridge_hub_westend::ED as BRIDGE_HUB_WESTEND_ED, westend::ED as WESTEND_ED, + PROOF_SIZE_THRESHOLD, REF_TIME_THRESHOLD, XCM_V3, + }, + test_parachain_is_trusted_teleporter, + xcm_helpers::{xcm_transact_paid_execution, xcm_transact_unpaid_execution}, + AssetHubRococo, AssetHubWestend, AssetHubWestendReceiver, BridgeHubRococo, BridgeHubWestend, + BridgeHubWestendPallet, BridgeHubWestendSender, PenpalWestendA, Westend, WestendPallet, +}; +pub use parachains_common::{AccountId, Balance}; +pub use xcm::{ + prelude::{AccountId32 as AccountId32Junction, *}, + v3::{ + Error, + NetworkId::{Rococo as RococoId, Westend as WestendId}, + }, +}; +pub use xcm_emulator::{ + assert_expected_events, bx, helpers::weight_within_threshold, Chain, Parachain as Para, + RelayChain as Relay, Test, TestArgs, TestContext, TestExt, +}; + +pub const ASSET_ID: u32 = 1; +pub const ASSET_MIN_BALANCE: u128 = 1000; +pub const ASSETS_PALLET_ID: u8 = 50; + +pub type RelayToSystemParaTest = Test; +pub type SystemParaToRelayTest = Test; +pub type SystemParaToParaTest = Test; + +/// Returns a `TestArgs` instance to de used for the Relay Chain accross integraton tests +pub fn relay_test_args(amount: Balance) -> TestArgs { + TestArgs { + dest: Westend::child_location_of(AssetHubWestend::para_id()), + beneficiary: AccountId32Junction { + network: None, + id: AssetHubWestendReceiver::get().into(), + } + .into(), + amount, + assets: (Here, amount).into(), + asset_id: None, + fee_asset_item: 0, + weight_limit: WeightLimit::Unlimited, + } +} + +#[cfg(test)] +mod tests; diff --git a/cumulus/parachains/integration-tests/emulated/bridges/bridge-hub-westend/src/tests/example.rs b/cumulus/parachains/integration-tests/emulated/bridges/bridge-hub-westend/src/tests/example.rs new file mode 100644 index 000000000000..e15a0dedab9e --- /dev/null +++ b/cumulus/parachains/integration-tests/emulated/bridges/bridge-hub-westend/src/tests/example.rs @@ -0,0 +1,74 @@ +// 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. + +use crate::*; + +#[test] +fn example() { + // Init tests variables + // XcmPallet send arguments + let sudo_origin = ::RuntimeOrigin::root(); + let destination = Westend::child_location_of(BridgeHubWestend::para_id()).into(); + let weight_limit = WeightLimit::Unlimited; + let check_origin = None; + + let remote_xcm = Xcm(vec![ClearOrigin]); + + let xcm = VersionedXcm::from(Xcm(vec![ + UnpaidExecution { weight_limit, check_origin }, + ExportMessage { + network: RococoId, + destination: X1(Parachain(AssetHubRococo::para_id().into())), + xcm: remote_xcm, + }, + ])); + + // Westend Global Consensus + // Send XCM message from Relay Chain to Bridge Hub source Parachain + Westend::execute_with(|| { + assert_ok!(::XcmPallet::send( + sudo_origin, + bx!(destination), + bx!(xcm), + )); + + type RuntimeEvent = ::RuntimeEvent; + + assert_expected_events!( + Westend, + vec![ + RuntimeEvent::XcmPallet(pallet_xcm::Event::Sent { .. }) => {}, + ] + ); + }); + // Receive XCM message in Bridge Hub source Parachain + BridgeHubWestend::execute_with(|| { + type RuntimeEvent = ::RuntimeEvent; + + assert_expected_events!( + BridgeHubWestend, + vec![ + RuntimeEvent::MessageQueue(pallet_message_queue::Event::Processed { + success: true, + .. + }) => {}, + RuntimeEvent::BridgeRococoMessages(pallet_bridge_messages::Event::MessageAccepted { + lane_id: LaneId([0, 0, 0, 2]), + nonce: 1, + }) => {}, + ] + ); + }); +} diff --git a/cumulus/parachains/integration-tests/emulated/bridges/bridge-hub-westend/src/tests/mod.rs b/cumulus/parachains/integration-tests/emulated/bridges/bridge-hub-westend/src/tests/mod.rs new file mode 100644 index 000000000000..1eef05c6b928 --- /dev/null +++ b/cumulus/parachains/integration-tests/emulated/bridges/bridge-hub-westend/src/tests/mod.rs @@ -0,0 +1,17 @@ +// 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. + +mod example; +mod teleport; diff --git a/cumulus/parachains/integration-tests/emulated/bridges/bridge-hub-westend/src/tests/teleport.rs b/cumulus/parachains/integration-tests/emulated/bridges/bridge-hub-westend/src/tests/teleport.rs new file mode 100644 index 000000000000..8dff6c292955 --- /dev/null +++ b/cumulus/parachains/integration-tests/emulated/bridges/bridge-hub-westend/src/tests/teleport.rs @@ -0,0 +1,30 @@ +// 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. + +use crate::*; +use bridge_hub_westend_runtime::xcm_config::XcmConfig; + +#[test] +fn teleport_to_other_system_parachains_works() { + let amount = BRIDGE_HUB_WESTEND_ED * 100; + let native_asset: MultiAssets = (Parent, amount).into(); + + test_parachain_is_trusted_teleporter!( + BridgeHubWestend, // Origin + XcmConfig, // XCM configuration + vec![AssetHubWestend], // Destinations + (native_asset, amount) + ); +} diff --git a/cumulus/parachains/integration-tests/emulated/common/Cargo.toml b/cumulus/parachains/integration-tests/emulated/common/Cargo.toml index 6688c80eb9ad..16a37e94be53 100644 --- a/cumulus/parachains/integration-tests/emulated/common/Cargo.toml +++ b/cumulus/parachains/integration-tests/emulated/common/Cargo.toml @@ -52,6 +52,7 @@ collectives-polkadot-runtime = { path = "../../../runtimes/collectives/collectiv bridge-hub-kusama-runtime = { path = "../../../runtimes/bridge-hubs/bridge-hub-kusama" } bridge-hub-polkadot-runtime = { path = "../../../runtimes/bridge-hubs/bridge-hub-polkadot" } bridge-hub-rococo-runtime = { path = "../../../runtimes/bridge-hubs/bridge-hub-rococo" } +bridge-hub-westend-runtime = { path = "../../../runtimes/bridge-hubs/bridge-hub-westend" } xcm-emulator = { default-features = false, path = "../../../../xcm/xcm-emulator" } cumulus-pallet-xcmp-queue = { default-features = false, path = "../../../../pallets/xcmp-queue" } cumulus-pallet-parachain-system = { path = "../../../../pallets/parachain-system" } @@ -68,6 +69,7 @@ runtime-benchmarks = [ "bridge-hub-kusama-runtime/runtime-benchmarks", "bridge-hub-polkadot-runtime/runtime-benchmarks", "bridge-hub-rococo-runtime/runtime-benchmarks", + "bridge-hub-westend-runtime/runtime-benchmarks", "bridge-runtime-common/runtime-benchmarks", "collectives-polkadot-runtime/runtime-benchmarks", "cumulus-pallet-parachain-system/runtime-benchmarks", diff --git a/cumulus/parachains/integration-tests/emulated/common/src/constants.rs b/cumulus/parachains/integration-tests/emulated/common/src/constants.rs index 86027229c59a..1dffbccbba34 100644 --- a/cumulus/parachains/integration-tests/emulated/common/src/constants.rs +++ b/cumulus/parachains/integration-tests/emulated/common/src/constants.rs @@ -1160,3 +1160,60 @@ pub mod bridge_hub_rococo { } } } + +// Bridge Hub Westend +pub mod bridge_hub_westend { + use super::*; + pub const PARA_ID: u32 = 1013; + pub const ED: Balance = parachains_common::westend::currency::EXISTENTIAL_DEPOSIT; + + pub fn genesis() -> Storage { + let genesis_config = serde_json::json!({ + "balances": { + "balances": accounts::init_balances() + .iter() + .cloned() + .map(|k| (k, ED * 4096)) + .collect::>(), + }, + "parachainInfo": { + "parachainId": cumulus_primitives_core::ParaId::from(PARA_ID), + }, + "collatorSelection": { + "invulnerables": collators::invulnerables() + .iter() + .cloned() + .map(|(acc, _)| acc) + .collect::>(), + "candidacyBond": ED * 16, + }, + "session": { + "keys": collators::invulnerables() + .into_iter() + .map(|(acc, aura)| { + ( + acc.clone(), // account id + acc, // validator id + bridge_hub_westend_runtime::SessionKeys { aura }, // session keys + ) + }) + .collect::>(), + }, + "polkadotXcm": { + "safeXcmVersion": Some(SAFE_XCM_VERSION), + }, + "bridgeRococoGrandpa": { + "owner": Some(get_account_id_from_seed::(accounts::BOB)), + }, + "bridgeRococoMessages": { + "owner": Some(get_account_id_from_seed::(accounts::BOB)), + } + }); + + build_genesis_storage( + genesis_config, + bridge_hub_westend_runtime::WASM_BINARY + .expect("WASM binary was not built, please build it!"), + ) + } +} diff --git a/cumulus/parachains/integration-tests/emulated/common/src/lib.rs b/cumulus/parachains/integration-tests/emulated/common/src/lib.rs index 2cb90650b943..1debd50d609b 100644 --- a/cumulus/parachains/integration-tests/emulated/common/src/lib.rs +++ b/cumulus/parachains/integration-tests/emulated/common/src/lib.rs @@ -20,8 +20,8 @@ pub mod xcm_helpers; use constants::{ accounts::{ALICE, BOB}, - asset_hub_rococo, asset_hub_westend, asset_hub_wococo, bridge_hub_rococo, penpal, rococo, - westend, + asset_hub_rococo, asset_hub_westend, asset_hub_wococo, bridge_hub_rococo, bridge_hub_westend, + penpal, rococo, westend, }; use impls::{RococoWococoMessageHandler, WococoRococoMessageHandler}; pub use paste; @@ -118,6 +118,23 @@ decl_test_parachains! { AssetConversion: asset_hub_westend_runtime::AssetConversion, } }, + pub struct BridgeHubWestend { + genesis = bridge_hub_westend::genesis(), + on_init = { + bridge_hub_westend_runtime::AuraExt::on_initialize(1); + }, + runtime = bridge_hub_westend_runtime, + core = { + XcmpMessageHandler: bridge_hub_westend_runtime::XcmpQueue, + LocationToAccountId: bridge_hub_westend_runtime::xcm_config::LocationToAccountId, + ParachainInfo: bridge_hub_westend_runtime::ParachainInfo, + MessageProcessor: DefaultParaMessageProcessor, + }, + pallets = { + PolkadotXcm: bridge_hub_westend_runtime::PolkadotXcm, + Balances: bridge_hub_westend_runtime::Balances, + } + }, pub struct PenpalWestendA { genesis = penpal::genesis(penpal::PARA_ID_A), on_init = { @@ -257,6 +274,7 @@ decl_test_networks! { relay_chain = Westend, parachains = vec![ AssetHubWestend, + BridgeHubWestend, PenpalWestendA, ], bridge = () @@ -323,6 +341,10 @@ impl_assert_events_helpers_for_parachain!(AssetHubRococo); // PenpalWestendA implementation impl_assert_events_helpers_for_parachain!(PenpalWestendA); +// BridgeHubWestend implementation +impl_accounts_helpers_for_parachain!(BridgeHubWestend); +impl_assert_events_helpers_for_parachain!(BridgeHubWestend); + // BridgeHubRococo implementation impl_accounts_helpers_for_parachain!(BridgeHubRococo); impl_assert_events_helpers_for_parachain!(BridgeHubRococo); @@ -343,6 +365,7 @@ decl_test_sender_receiver_accounts_parameter_types! { // Bridged Hubs BridgeHubRococo { sender: ALICE, receiver: BOB }, BridgeHubWococo { sender: ALICE, receiver: BOB }, + BridgeHubWestend { sender: ALICE, receiver: BOB }, // Penpals PenpalWestendA { sender: ALICE, receiver: BOB }, PenpalRococoA { sender: ALICE, receiver: BOB }, diff --git a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/Cargo.toml b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/Cargo.toml index 76ff63db4dac..671d38e808fc 100644 --- a/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/Cargo.toml +++ b/cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/Cargo.toml @@ -29,6 +29,7 @@ pallet-aura = { path = "../../../../../substrate/frame/aura", default-features = pallet-authorship = { path = "../../../../../substrate/frame/authorship", default-features = false} pallet-balances = { path = "../../../../../substrate/frame/balances", default-features = false} pallet-session = { path = "../../../../../substrate/frame/session", default-features = false} +pallet-message-queue = { path = "../../../../../substrate/frame/message-queue", default-features = false } pallet-multisig = { path = "../../../../../substrate/frame/multisig", default-features = false} pallet-timestamp = { path = "../../../../../substrate/frame/timestamp", default-features = false} pallet-transaction-payment = { path = "../../../../../substrate/frame/transaction-payment", default-features = false} @@ -62,7 +63,6 @@ xcm-executor = { package = "staging-xcm-executor", path = "../../../../../polkad # Cumulus cumulus-pallet-aura-ext = { path = "../../../../pallets/aura-ext", default-features = false } -pallet-message-queue = { path = "../../../../../substrate/frame/message-queue", default-features = false } cumulus-pallet-dmp-queue = { path = "../../../../pallets/dmp-queue", default-features = false } cumulus-pallet-parachain-system = { path = "../../../../pallets/parachain-system", default-features = false, features = ["parameterized-consensus-hook",] } cumulus-pallet-session-benchmarking = { path = "../../../../pallets/session-benchmarking", default-features = false} From 8af08e7626be57ee64e1429b99552511ac76110c Mon Sep 17 00:00:00 2001 From: benluelo <57334811+benluelo@users.noreply.github.com> Date: Wed, 8 Nov 2023 10:51:40 +0000 Subject: [PATCH 11/12] feat(frame-support-procedural): add `automaticaly_derived` attr to `NoBound` derives (#2197) fixes #2196 --- substrate/frame/support/procedural/src/no_bound/clone.rs | 1 + substrate/frame/support/procedural/src/no_bound/debug.rs | 1 + substrate/frame/support/procedural/src/no_bound/default.rs | 1 + substrate/frame/support/procedural/src/no_bound/partial_eq.rs | 1 + substrate/frame/support/test/tests/derive_no_bound.rs | 3 +++ 5 files changed, 7 insertions(+) diff --git a/substrate/frame/support/procedural/src/no_bound/clone.rs b/substrate/frame/support/procedural/src/no_bound/clone.rs index 2c9037984f59..8e57a10d34c6 100644 --- a/substrate/frame/support/procedural/src/no_bound/clone.rs +++ b/substrate/frame/support/procedural/src/no_bound/clone.rs @@ -98,6 +98,7 @@ pub fn derive_clone_no_bound(input: proc_macro::TokenStream) -> proc_macro::Toke quote::quote!( const _: () = { + #[automatically_derived] impl #impl_generics ::core::clone::Clone for #name #ty_generics #where_clause { fn clone(&self) -> Self { #impl_ diff --git a/substrate/frame/support/procedural/src/no_bound/debug.rs b/substrate/frame/support/procedural/src/no_bound/debug.rs index 88f5dfe7bec4..dd14b9cd564c 100644 --- a/substrate/frame/support/procedural/src/no_bound/debug.rs +++ b/substrate/frame/support/procedural/src/no_bound/debug.rs @@ -112,6 +112,7 @@ pub fn derive_debug_no_bound(input: proc_macro::TokenStream) -> proc_macro::Toke quote::quote!( const _: () = { + #[automatically_derived] impl #impl_generics ::core::fmt::Debug for #input_ident #ty_generics #where_clause { fn fmt(&self, fmt: &mut ::core::fmt::Formatter) -> ::core::fmt::Result { #impl_ diff --git a/substrate/frame/support/procedural/src/no_bound/default.rs b/substrate/frame/support/procedural/src/no_bound/default.rs index ddaab26c4415..0524247d24ed 100644 --- a/substrate/frame/support/procedural/src/no_bound/default.rs +++ b/substrate/frame/support/procedural/src/no_bound/default.rs @@ -149,6 +149,7 @@ pub fn derive_default_no_bound(input: proc_macro::TokenStream) -> proc_macro::To quote!( const _: () = { + #[automatically_derived] impl #impl_generics ::core::default::Default for #name #ty_generics #where_clause { fn default() -> Self { #impl_ diff --git a/substrate/frame/support/procedural/src/no_bound/partial_eq.rs b/substrate/frame/support/procedural/src/no_bound/partial_eq.rs index 1a4a4e50b39e..7cf5701b193b 100644 --- a/substrate/frame/support/procedural/src/no_bound/partial_eq.rs +++ b/substrate/frame/support/procedural/src/no_bound/partial_eq.rs @@ -128,6 +128,7 @@ pub fn derive_partial_eq_no_bound(input: proc_macro::TokenStream) -> proc_macro: quote::quote!( const _: () = { + #[automatically_derived] impl #impl_generics ::core::cmp::PartialEq for #name #ty_generics #where_clause { fn eq(&self, other: &Self) -> bool { #impl_ diff --git a/substrate/frame/support/test/tests/derive_no_bound.rs b/substrate/frame/support/test/tests/derive_no_bound.rs index dc78027f22eb..af2633dc53ca 100644 --- a/substrate/frame/support/test/tests/derive_no_bound.rs +++ b/substrate/frame/support/test/tests/derive_no_bound.rs @@ -136,6 +136,7 @@ struct StructNoGenerics { field2: u64, } +#[allow(dead_code)] #[derive(DebugNoBound, CloneNoBound, EqNoBound, PartialEqNoBound, DefaultNoBound)] enum EnumNoGenerics { #[default] @@ -162,6 +163,7 @@ enum Enum { } // enum that will have a named default. +#[allow(dead_code)] #[derive(DebugNoBound, CloneNoBound, EqNoBound, PartialEqNoBound, DefaultNoBound)] enum Enum2 { #[default] @@ -175,6 +177,7 @@ enum Enum2 { VariantUnit2, } +#[allow(dead_code)] // enum that will have a unit default. #[derive(DebugNoBound, CloneNoBound, EqNoBound, PartialEqNoBound, DefaultNoBound)] enum Enum3 { From 9e1039e932739b9e3ce3aaa690bd922559a345c1 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Wed, 8 Nov 2023 12:59:55 +0100 Subject: [PATCH 12/12] Add `sudo::remove_key` (#2165) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Changes: - Adds a new call `remove_key` to the sudo pallet to permanently remove the sudo key. - Remove some clones and general maintenance --------- Signed-off-by: Oliver Tale-Yazdi Co-authored-by: Bastian Köcher Co-authored-by: command-bot <> --- .../runtime/rococo/src/weights/pallet_sudo.rs | 55 ++++++---- .../westend/src/weights/pallet_sudo.rs | 58 +++++----- prdoc/pr_2165.prdoc | 17 +++ substrate/frame/sudo/src/benchmarking.rs | 44 +++++--- substrate/frame/sudo/src/lib.rs | 92 ++++++++-------- substrate/frame/sudo/src/mock.rs | 4 +- substrate/frame/sudo/src/tests.rs | 28 ++--- substrate/frame/sudo/src/weights.rs | 102 +++++++++++------- 8 files changed, 240 insertions(+), 160 deletions(-) create mode 100644 prdoc/pr_2165.prdoc diff --git a/polkadot/runtime/rococo/src/weights/pallet_sudo.rs b/polkadot/runtime/rococo/src/weights/pallet_sudo.rs index 83215af5b8ae..694174954fc7 100644 --- a/polkadot/runtime/rococo/src/weights/pallet_sudo.rs +++ b/polkadot/runtime/rococo/src/weights/pallet_sudo.rs @@ -17,24 +17,25 @@ //! Autogenerated weights for `pallet_sudo` //! //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev -//! DATE: 2023-05-26, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]` +//! DATE: 2023-11-07, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]` //! WORST CASE MAP SIZE: `1000000` -//! HOSTNAME: `bm5`, CPU: `Intel(R) Core(TM) i7-7700K CPU @ 4.20GHz` -//! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("rococo-dev"), DB CACHE: 1024 +//! HOSTNAME: `runner-yprdrvc7-project-674-concurrent-0`, CPU: `Intel(R) Xeon(R) CPU @ 2.60GHz` +//! WASM-EXECUTION: `Compiled`, CHAIN: `Some("rococo-dev")`, DB CACHE: 1024 // Executed Command: -// ./target/production/polkadot +// target/production/polkadot // benchmark // pallet -// --chain=rococo-dev // --steps=50 // --repeat=20 -// --pallet=pallet_sudo // --extrinsic=* -// --execution=wasm // --wasm-execution=compiled -// --header=./file_header.txt -// --output=./runtime/rococo/src/weights/ +// --heap-pages=4096 +// --json-file=/builds/parity/mirrors/polkadot-sdk/.git/.artifacts/bench.json +// --pallet=pallet_sudo +// --chain=rococo-dev +// --header=./polkadot/file_header.txt +// --output=./polkadot/runtime/rococo/src/weights/ #![cfg_attr(rustfmt, rustfmt_skip)] #![allow(unused_parens)] @@ -47,38 +48,50 @@ use core::marker::PhantomData; /// Weight functions for `pallet_sudo`. pub struct WeightInfo(PhantomData); impl pallet_sudo::WeightInfo for WeightInfo { - /// Storage: Sudo Key (r:1 w:1) - /// Proof: Sudo Key (max_values: Some(1), max_size: Some(32), added: 527, mode: MaxEncodedLen) + /// Storage: `Sudo::Key` (r:1 w:1) + /// Proof: `Sudo::Key` (`max_values`: Some(1), `max_size`: Some(32), added: 527, mode: `MaxEncodedLen`) fn set_key() -> Weight { // Proof Size summary in bytes: // Measured: `132` // Estimated: `1517` - // Minimum execution time: 13_047_000 picoseconds. - Weight::from_parts(13_325_000, 0) + // Minimum execution time: 8_432_000 picoseconds. + Weight::from_parts(8_757_000, 0) .saturating_add(Weight::from_parts(0, 1517)) .saturating_add(T::DbWeight::get().reads(1)) .saturating_add(T::DbWeight::get().writes(1)) } - /// Storage: Sudo Key (r:1 w:0) - /// Proof: Sudo Key (max_values: Some(1), max_size: Some(32), added: 527, mode: MaxEncodedLen) + /// Storage: `Sudo::Key` (r:1 w:0) + /// Proof: `Sudo::Key` (`max_values`: Some(1), `max_size`: Some(32), added: 527, mode: `MaxEncodedLen`) fn sudo() -> Weight { // Proof Size summary in bytes: // Measured: `132` // Estimated: `1517` - // Minimum execution time: 13_250_000 picoseconds. - Weight::from_parts(13_544_000, 0) + // Minimum execution time: 9_167_000 picoseconds. + Weight::from_parts(9_397_000, 0) .saturating_add(Weight::from_parts(0, 1517)) .saturating_add(T::DbWeight::get().reads(1)) } - /// Storage: Sudo Key (r:1 w:0) - /// Proof: Sudo Key (max_values: Some(1), max_size: Some(32), added: 527, mode: MaxEncodedLen) + /// Storage: `Sudo::Key` (r:1 w:0) + /// Proof: `Sudo::Key` (`max_values`: Some(1), `max_size`: Some(32), added: 527, mode: `MaxEncodedLen`) fn sudo_as() -> Weight { // Proof Size summary in bytes: // Measured: `132` // Estimated: `1517` - // Minimum execution time: 13_424_000 picoseconds. - Weight::from_parts(13_801_000, 0) + // Minimum execution time: 9_133_000 picoseconds. + Weight::from_parts(9_573_000, 0) .saturating_add(Weight::from_parts(0, 1517)) .saturating_add(T::DbWeight::get().reads(1)) } + /// Storage: `Sudo::Key` (r:1 w:1) + /// Proof: `Sudo::Key` (`max_values`: Some(1), `max_size`: Some(32), added: 527, mode: `MaxEncodedLen`) + fn remove_key() -> Weight { + // Proof Size summary in bytes: + // Measured: `132` + // Estimated: `1517` + // Minimum execution time: 7_374_000 picoseconds. + Weight::from_parts(7_702_000, 0) + .saturating_add(Weight::from_parts(0, 1517)) + .saturating_add(T::DbWeight::get().reads(1)) + .saturating_add(T::DbWeight::get().writes(1)) + } } diff --git a/polkadot/runtime/westend/src/weights/pallet_sudo.rs b/polkadot/runtime/westend/src/weights/pallet_sudo.rs index 8a220173ee57..e9ab3ad37a4c 100644 --- a/polkadot/runtime/westend/src/weights/pallet_sudo.rs +++ b/polkadot/runtime/westend/src/weights/pallet_sudo.rs @@ -17,27 +17,25 @@ //! Autogenerated weights for `pallet_sudo` //! //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev -//! DATE: 2023-06-14, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]` +//! DATE: 2023-11-07, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]` //! WORST CASE MAP SIZE: `1000000` -//! HOSTNAME: `runner--ss9ysm1-project-163-concurrent-0`, CPU: `Intel(R) Xeon(R) CPU @ 2.60GHz` -//! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("westend-dev"), DB CACHE: 1024 +//! HOSTNAME: `runner-yprdrvc7-project-674-concurrent-0`, CPU: `Intel(R) Xeon(R) CPU @ 2.60GHz` +//! WASM-EXECUTION: `Compiled`, CHAIN: `Some("westend-dev")`, DB CACHE: 1024 // Executed Command: -// ./target/production/polkadot +// target/production/polkadot // benchmark // pallet -// --chain=westend-dev // --steps=50 // --repeat=20 -// --no-storage-info -// --no-median-slopes -// --no-min-squares -// --pallet=pallet_sudo // --extrinsic=* -// --execution=wasm // --wasm-execution=compiled -// --header=./file_header.txt -// --output=./runtime/westend/src/weights/ +// --heap-pages=4096 +// --json-file=/builds/parity/mirrors/polkadot-sdk/.git/.artifacts/bench.json +// --pallet=pallet_sudo +// --chain=westend-dev +// --header=./polkadot/file_header.txt +// --output=./polkadot/runtime/westend/src/weights/ #![cfg_attr(rustfmt, rustfmt_skip)] #![allow(unused_parens)] @@ -50,38 +48,50 @@ use core::marker::PhantomData; /// Weight functions for `pallet_sudo`. pub struct WeightInfo(PhantomData); impl pallet_sudo::WeightInfo for WeightInfo { - /// Storage: Sudo Key (r:1 w:1) - /// Proof: Sudo Key (max_values: Some(1), max_size: Some(32), added: 527, mode: MaxEncodedLen) + /// Storage: `Sudo::Key` (r:1 w:1) + /// Proof: `Sudo::Key` (`max_values`: Some(1), `max_size`: Some(32), added: 527, mode: `MaxEncodedLen`) fn set_key() -> Weight { // Proof Size summary in bytes: // Measured: `132` // Estimated: `1517` - // Minimum execution time: 12_360_000 picoseconds. - Weight::from_parts(12_803_000, 0) + // Minimum execution time: 8_750_000 picoseconds. + Weight::from_parts(9_102_000, 0) .saturating_add(Weight::from_parts(0, 1517)) .saturating_add(T::DbWeight::get().reads(1)) .saturating_add(T::DbWeight::get().writes(1)) } - /// Storage: Sudo Key (r:1 w:0) - /// Proof: Sudo Key (max_values: Some(1), max_size: Some(32), added: 527, mode: MaxEncodedLen) + /// Storage: `Sudo::Key` (r:1 w:0) + /// Proof: `Sudo::Key` (`max_values`: Some(1), `max_size`: Some(32), added: 527, mode: `MaxEncodedLen`) fn sudo() -> Weight { // Proof Size summary in bytes: // Measured: `132` // Estimated: `1517` - // Minimum execution time: 12_158_000 picoseconds. - Weight::from_parts(12_506_000, 0) + // Minimum execution time: 9_607_000 picoseconds. + Weight::from_parts(10_139_000, 0) .saturating_add(Weight::from_parts(0, 1517)) .saturating_add(T::DbWeight::get().reads(1)) } - /// Storage: Sudo Key (r:1 w:0) - /// Proof: Sudo Key (max_values: Some(1), max_size: Some(32), added: 527, mode: MaxEncodedLen) + /// Storage: `Sudo::Key` (r:1 w:0) + /// Proof: `Sudo::Key` (`max_values`: Some(1), `max_size`: Some(32), added: 527, mode: `MaxEncodedLen`) fn sudo_as() -> Weight { // Proof Size summary in bytes: // Measured: `132` // Estimated: `1517` - // Minimum execution time: 12_286_000 picoseconds. - Weight::from_parts(12_664_000, 0) + // Minimum execution time: 9_886_000 picoseconds. + Weight::from_parts(10_175_000, 0) .saturating_add(Weight::from_parts(0, 1517)) .saturating_add(T::DbWeight::get().reads(1)) } + /// Storage: `Sudo::Key` (r:1 w:1) + /// Proof: `Sudo::Key` (`max_values`: Some(1), `max_size`: Some(32), added: 527, mode: `MaxEncodedLen`) + fn remove_key() -> Weight { + // Proof Size summary in bytes: + // Measured: `132` + // Estimated: `1517` + // Minimum execution time: 7_843_000 picoseconds. + Weight::from_parts(8_152_000, 0) + .saturating_add(Weight::from_parts(0, 1517)) + .saturating_add(T::DbWeight::get().reads(1)) + .saturating_add(T::DbWeight::get().writes(1)) + } } diff --git a/prdoc/pr_2165.prdoc b/prdoc/pr_2165.prdoc new file mode 100644 index 000000000000..31cb691c43aa --- /dev/null +++ b/prdoc/pr_2165.prdoc @@ -0,0 +1,17 @@ +title: Add sudo::remove_key + +doc: + - audience: Core Dev + description: | + Pallet `Sudo` now has the ability to remove the sudo key via `remove_key`. This is a less-invasive way of rendering the sudo pallet useless without needing a code upgrade. + +migrations: + db: [] + + runtime: [] + +crates: + - name: pallet-sudo + semver: minor + +host_functions: [] diff --git a/substrate/frame/sudo/src/benchmarking.rs b/substrate/frame/sudo/src/benchmarking.rs index 6a365c1873c1..e64233fe7480 100644 --- a/substrate/frame/sudo/src/benchmarking.rs +++ b/substrate/frame/sudo/src/benchmarking.rs @@ -22,41 +22,40 @@ use crate::Pallet; use frame_benchmarking::v2::*; use frame_system::RawOrigin; -const SEED: u32 = 0; - -fn assert_last_event(generic_event: ::RuntimeEvent) { - frame_system::Pallet::::assert_last_event(generic_event.into()); +fn assert_last_event(generic_event: crate::Event) { + let re: ::RuntimeEvent = generic_event.into(); + frame_system::Pallet::::assert_last_event(re.into()); } -#[benchmarks( where ::RuntimeCall: From>)] +#[benchmarks(where ::RuntimeCall: From>)] mod benchmarks { use super::*; #[benchmark] fn set_key() { let caller: T::AccountId = whitelisted_caller(); - Key::::put(caller.clone()); + Key::::put(&caller); - let new_sudoer: T::AccountId = account("sudoer", 0, SEED); + let new_sudoer: T::AccountId = account("sudoer", 0, 0); let new_sudoer_lookup = T::Lookup::unlookup(new_sudoer.clone()); #[extrinsic_call] _(RawOrigin::Signed(caller.clone()), new_sudoer_lookup); - assert_last_event::(Event::KeyChanged { old_sudoer: Some(caller) }.into()); + assert_last_event::(Event::KeyChanged { old: Some(caller), new: new_sudoer }); } #[benchmark] fn sudo() { let caller: T::AccountId = whitelisted_caller(); - Key::::put(caller.clone()); + Key::::put(&caller); - let call: ::RuntimeCall = frame_system::Call::remark { remark: vec![] }.into(); + let call = frame_system::Call::remark { remark: vec![] }.into(); #[extrinsic_call] - _(RawOrigin::Signed(caller.clone()), Box::new(call.clone())); + _(RawOrigin::Signed(caller), Box::new(call)); - assert_last_event::(Event::Sudid { sudo_result: Ok(()) }.into()) + assert_last_event::(Event::Sudid { sudo_result: Ok(()) }) } #[benchmark] @@ -64,15 +63,26 @@ mod benchmarks { let caller: T::AccountId = whitelisted_caller(); Key::::put(caller.clone()); - let call: ::RuntimeCall = frame_system::Call::remark { remark: vec![] }.into(); + let call = frame_system::Call::remark { remark: vec![] }.into(); + + let who: T::AccountId = account("as", 0, 0); + let who_lookup = T::Lookup::unlookup(who); + + #[extrinsic_call] + _(RawOrigin::Signed(caller), who_lookup, Box::new(call)); + + assert_last_event::(Event::SudoAsDone { sudo_result: Ok(()) }) + } - let who: T::AccountId = account("as", 0, SEED); - let who_lookup = T::Lookup::unlookup(who.clone()); + #[benchmark] + fn remove_key() { + let caller: T::AccountId = whitelisted_caller(); + Key::::put(&caller); #[extrinsic_call] - _(RawOrigin::Signed(caller), who_lookup, Box::new(call.clone())); + _(RawOrigin::Signed(caller.clone())); - assert_last_event::(Event::SudoAsDone { sudo_result: Ok(()) }.into()) + assert_last_event::(Event::KeyRemoved {}); } impl_benchmark_test_suite!(Pallet, crate::mock::new_bench_ext(), crate::mock::Test); diff --git a/substrate/frame/sudo/src/lib.rs b/substrate/frame/sudo/src/lib.rs index 36de44d9d729..d556c5eb6ae6 100644 --- a/substrate/frame/sudo/src/lib.rs +++ b/substrate/frame/sudo/src/lib.rs @@ -146,7 +146,7 @@ type AccountIdLookupOf = <::Lookup as StaticLookup pub mod pallet { use super::{DispatchResult, *}; use frame_support::pallet_prelude::*; - use frame_system::pallet_prelude::*; + use frame_system::{pallet_prelude::*, RawOrigin}; /// Default preludes for [`Config`]. pub mod config_preludes { @@ -190,11 +190,6 @@ pub mod pallet { #[pallet::call] impl Pallet { /// Authenticates the sudo key and dispatches a function call with `Root` origin. - /// - /// The dispatch origin for this call must be _Signed_. - /// - /// ## Complexity - /// - O(1). #[pallet::call_index(0)] #[pallet::weight({ let dispatch_info = call.get_dispatch_info(); @@ -207,12 +202,11 @@ pub mod pallet { origin: OriginFor, call: Box<::RuntimeCall>, ) -> DispatchResultWithPostInfo { - // This is a public call, so we ensure that the origin is some signed account. - let sender = ensure_signed(origin)?; - ensure!(Self::key().map_or(false, |k| sender == k), Error::::RequireSudo); + Self::ensure_sudo(origin)?; - let res = call.dispatch_bypass_filter(frame_system::RawOrigin::Root.into()); + let res = call.dispatch_bypass_filter(RawOrigin::Root.into()); Self::deposit_event(Event::Sudid { sudo_result: res.map(|_| ()).map_err(|e| e.error) }); + // Sudo user does not pay a fee. Ok(Pays::No.into()) } @@ -222,9 +216,6 @@ pub mod pallet { /// Sudo user to specify the weight of the call. /// /// The dispatch origin for this call must be _Signed_. - /// - /// ## Complexity - /// - O(1). #[pallet::call_index(1)] #[pallet::weight((*weight, call.get_dispatch_info().class))] pub fn sudo_unchecked_weight( @@ -232,37 +223,30 @@ pub mod pallet { call: Box<::RuntimeCall>, weight: Weight, ) -> DispatchResultWithPostInfo { - // This is a public call, so we ensure that the origin is some signed account. - let sender = ensure_signed(origin)?; + Self::ensure_sudo(origin)?; let _ = weight; // We don't check the weight witness since it is a root call. - ensure!(Self::key().map_or(false, |k| sender == k), Error::::RequireSudo); - let res = call.dispatch_bypass_filter(frame_system::RawOrigin::Root.into()); + let res = call.dispatch_bypass_filter(RawOrigin::Root.into()); Self::deposit_event(Event::Sudid { sudo_result: res.map(|_| ()).map_err(|e| e.error) }); + // Sudo user does not pay a fee. Ok(Pays::No.into()) } /// Authenticates the current sudo key and sets the given AccountId (`new`) as the new sudo /// key. - /// - /// The dispatch origin for this call must be _Signed_. - /// - /// ## Complexity - /// - O(1). #[pallet::call_index(2)] #[pallet::weight(T::WeightInfo::set_key())] pub fn set_key( origin: OriginFor, new: AccountIdLookupOf, ) -> DispatchResultWithPostInfo { - // This is a public call, so we ensure that the origin is some signed account. - let sender = ensure_signed(origin)?; - ensure!(Self::key().map_or(false, |k| sender == k), Error::::RequireSudo); + Self::ensure_sudo(origin)?; + let new = T::Lookup::lookup(new)?; + Self::deposit_event(Event::KeyChanged { old: Key::::get(), new: new.clone() }); + Key::::put(new); - Self::deposit_event(Event::KeyChanged { old_sudoer: Key::::get() }); - Key::::put(&new); // Sudo user does not pay a fee. Ok(Pays::No.into()) } @@ -271,9 +255,6 @@ pub mod pallet { /// a given account. /// /// The dispatch origin for this call must be _Signed_. - /// - /// ## Complexity - /// - O(1). #[pallet::call_index(3)] #[pallet::weight({ let dispatch_info = call.get_dispatch_info(); @@ -287,17 +268,29 @@ pub mod pallet { who: AccountIdLookupOf, call: Box<::RuntimeCall>, ) -> DispatchResultWithPostInfo { - // This is a public call, so we ensure that the origin is some signed account. - let sender = ensure_signed(origin)?; - ensure!(Self::key().map_or(false, |k| sender == k), Error::::RequireSudo); + Self::ensure_sudo(origin)?; let who = T::Lookup::lookup(who)?; - - let res = call.dispatch_bypass_filter(frame_system::RawOrigin::Signed(who).into()); - + let res = call.dispatch_bypass_filter(RawOrigin::Signed(who).into()); Self::deposit_event(Event::SudoAsDone { sudo_result: res.map(|_| ()).map_err(|e| e.error), }); + + // Sudo user does not pay a fee. + Ok(Pays::No.into()) + } + + /// Permanently removes the sudo key. + /// + /// **This cannot be un-done.** + #[pallet::call_index(4)] + #[pallet::weight(T::WeightInfo::remove_key())] + pub fn remove_key(origin: OriginFor) -> DispatchResultWithPostInfo { + Self::ensure_sudo(origin)?; + + Self::deposit_event(Event::KeyRemoved {}); + Key::::kill(); + // Sudo user does not pay a fee. Ok(Pays::No.into()) } @@ -313,9 +306,13 @@ pub mod pallet { }, /// The sudo key has been updated. KeyChanged { - /// The old sudo key if one was previously set. - old_sudoer: Option, + /// The old sudo key (if one was previously set). + old: Option, + /// The new sudo key (if one was set). + new: T::AccountId, }, + /// The key was permanently removed. + KeyRemoved, /// A [sudo_as](Pallet::sudo_as) call just took place. SudoAsDone { /// The result of the call made by the sudo user. @@ -324,9 +321,9 @@ pub mod pallet { } #[pallet::error] - /// Error for the Sudo pallet + /// Error for the Sudo pallet. pub enum Error { - /// Sender must be the Sudo account + /// Sender must be the Sudo account. RequireSudo, } @@ -345,8 +342,19 @@ pub mod pallet { #[pallet::genesis_build] impl BuildGenesisConfig for GenesisConfig { fn build(&self) { - if let Some(ref key) = self.key { - Key::::put(key); + Key::::set(self.key.clone()); + } + } + + impl Pallet { + /// Ensure that the caller is the sudo key. + pub(crate) fn ensure_sudo(origin: OriginFor) -> DispatchResult { + let sender = ensure_signed(origin)?; + + if Self::key().map_or(false, |k| k == sender) { + Ok(()) + } else { + Err(Error::::RequireSudo.into()) } } } diff --git a/substrate/frame/sudo/src/mock.rs b/substrate/frame/sudo/src/mock.rs index 427bda6d99e4..6f123b7c82b2 100644 --- a/substrate/frame/sudo/src/mock.rs +++ b/substrate/frame/sudo/src/mock.rs @@ -156,7 +156,9 @@ pub fn new_test_ext(root_key: u64) -> sp_io::TestExternalities { sudo::GenesisConfig:: { key: Some(root_key) } .assimilate_storage(&mut t) .unwrap(); - t.into() + let mut ext: sp_io::TestExternalities = t.into(); + ext.execute_with(|| System::set_block_number(1)); + ext } #[cfg(feature = "runtime-benchmarks")] diff --git a/substrate/frame/sudo/src/tests.rs b/substrate/frame/sudo/src/tests.rs index 6963ba2e6a05..13dc069ddef1 100644 --- a/substrate/frame/sudo/src/tests.rs +++ b/substrate/frame/sudo/src/tests.rs @@ -59,9 +59,6 @@ fn sudo_basics() { #[test] fn sudo_emits_events_correctly() { new_test_ext(1).execute_with(|| { - // Set block number to 1 because events are not emitted on block 0. - System::set_block_number(1); - // Should emit event to indicate success when called with the root `key` and `call` is `Ok`. let call = Box::new(RuntimeCall::Logger(LoggerCall::privileged_i32_log { i: 42, @@ -118,9 +115,6 @@ fn sudo_unchecked_weight_basics() { #[test] fn sudo_unchecked_weight_emits_events_correctly() { new_test_ext(1).execute_with(|| { - // Set block number to 1 because events are not emitted on block 0. - System::set_block_number(1); - // Should emit event to indicate success when called with the root `key` and `call` is `Ok`. let call = Box::new(RuntimeCall::Logger(LoggerCall::privileged_i32_log { i: 42, @@ -154,15 +148,24 @@ fn set_key_basics() { #[test] fn set_key_emits_events_correctly() { new_test_ext(1).execute_with(|| { - // Set block number to 1 because events are not emitted on block 0. - System::set_block_number(1); - // A root `key` can change the root `key`. assert_ok!(Sudo::set_key(RuntimeOrigin::signed(1), 2)); - System::assert_has_event(TestEvent::Sudo(Event::KeyChanged { old_sudoer: Some(1) })); + System::assert_has_event(TestEvent::Sudo(Event::KeyChanged { old: Some(1), new: 2 })); // Double check. assert_ok!(Sudo::set_key(RuntimeOrigin::signed(2), 4)); - System::assert_has_event(TestEvent::Sudo(Event::KeyChanged { old_sudoer: Some(2) })); + System::assert_has_event(TestEvent::Sudo(Event::KeyChanged { old: Some(2), new: 4 })); + }); +} + +#[test] +fn remove_key_works() { + new_test_ext(1).execute_with(|| { + assert_ok!(Sudo::remove_key(RuntimeOrigin::signed(1))); + assert!(Sudo::key().is_none()); + System::assert_has_event(TestEvent::Sudo(Event::KeyRemoved {})); + + assert_noop!(Sudo::remove_key(RuntimeOrigin::signed(1)), Error::::RequireSudo); + assert_noop!(Sudo::set_key(RuntimeOrigin::signed(1), 1), Error::::RequireSudo); }); } @@ -201,9 +204,6 @@ fn sudo_as_basics() { #[test] fn sudo_as_emits_events_correctly() { new_test_ext(1).execute_with(|| { - // Set block number to 1 because events are not emitted on block 0. - System::set_block_number(1); - // A non-privileged function will work when passed to `sudo_as` with the root `key`. let call = Box::new(RuntimeCall::Logger(LoggerCall::non_privileged_log { i: 42, diff --git a/substrate/frame/sudo/src/weights.rs b/substrate/frame/sudo/src/weights.rs index 0cdd0c8a81f2..10d1a9f7a513 100644 --- a/substrate/frame/sudo/src/weights.rs +++ b/substrate/frame/sudo/src/weights.rs @@ -15,32 +15,29 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! Autogenerated weights for pallet_sudo. +//! Autogenerated weights for `pallet_sudo` //! //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev -//! DATE: 2023-06-16, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]` +//! DATE: 2023-11-07, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]` //! WORST CASE MAP SIZE: `1000000` -//! HOSTNAME: `runner-e8ezs4ez-project-145-concurrent-0`, CPU: `Intel(R) Xeon(R) CPU @ 2.60GHz` -//! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 1024 +//! HOSTNAME: `runner-yprdrvc7-project-674-concurrent-0`, CPU: `Intel(R) Xeon(R) CPU @ 2.60GHz` +//! WASM-EXECUTION: `Compiled`, CHAIN: `Some("dev")`, DB CACHE: `1024` // Executed Command: -// ./target/production/substrate +// target/production/substrate-node // benchmark // pallet -// --chain=dev // --steps=50 // --repeat=20 -// --pallet=pallet_sudo -// --no-storage-info -// --no-median-slopes -// --no-min-squares // --extrinsic=* -// --execution=wasm // --wasm-execution=compiled // --heap-pages=4096 -// --output=./frame/sudo/src/weights.rs -// --header=./HEADER-APACHE2 -// --template=./.maintain/frame-weight-template.hbs +// --json-file=/builds/parity/mirrors/polkadot-sdk/.git/.artifacts/bench.json +// --pallet=pallet_sudo +// --chain=dev +// --header=./substrate/HEADER-APACHE2 +// --output=./substrate/frame/sudo/src/weights.rs +// --template=./substrate/.maintain/frame-weight-template.hbs #![cfg_attr(rustfmt, rustfmt_skip)] #![allow(unused_parens)] @@ -50,80 +47,103 @@ use frame_support::{traits::Get, weights::{Weight, constants::RocksDbWeight}}; use core::marker::PhantomData; -/// Weight functions needed for pallet_sudo. +/// Weight functions needed for `pallet_sudo`. pub trait WeightInfo { fn set_key() -> Weight; fn sudo() -> Weight; fn sudo_as() -> Weight; + fn remove_key() -> Weight; } -/// Weights for pallet_sudo using the Substrate node and recommended hardware. +/// Weights for `pallet_sudo` using the Substrate node and recommended hardware. pub struct SubstrateWeight(PhantomData); impl WeightInfo for SubstrateWeight { - /// Storage: Sudo Key (r:1 w:1) - /// Proof: Sudo Key (max_values: Some(1), max_size: Some(32), added: 527, mode: MaxEncodedLen) + /// Storage: `Sudo::Key` (r:1 w:1) + /// Proof: `Sudo::Key` (`max_values`: Some(1), `max_size`: Some(32), added: 527, mode: `MaxEncodedLen`) fn set_key() -> Weight { // Proof Size summary in bytes: // Measured: `165` // Estimated: `1517` - // Minimum execution time: 12_918_000 picoseconds. - Weight::from_parts(13_403_000, 1517) + // Minimum execution time: 9_600_000 picoseconds. + Weight::from_parts(10_076_000, 1517) .saturating_add(T::DbWeight::get().reads(1_u64)) .saturating_add(T::DbWeight::get().writes(1_u64)) } - /// Storage: Sudo Key (r:1 w:0) - /// Proof: Sudo Key (max_values: Some(1), max_size: Some(32), added: 527, mode: MaxEncodedLen) + /// Storage: `Sudo::Key` (r:1 w:0) + /// Proof: `Sudo::Key` (`max_values`: Some(1), `max_size`: Some(32), added: 527, mode: `MaxEncodedLen`) fn sudo() -> Weight { // Proof Size summary in bytes: // Measured: `165` // Estimated: `1517` - // Minimum execution time: 12_693_000 picoseconds. - Weight::from_parts(13_001_000, 1517) + // Minimum execution time: 10_453_000 picoseconds. + Weight::from_parts(10_931_000, 1517) .saturating_add(T::DbWeight::get().reads(1_u64)) } - /// Storage: Sudo Key (r:1 w:0) - /// Proof: Sudo Key (max_values: Some(1), max_size: Some(32), added: 527, mode: MaxEncodedLen) + /// Storage: `Sudo::Key` (r:1 w:0) + /// Proof: `Sudo::Key` (`max_values`: Some(1), `max_size`: Some(32), added: 527, mode: `MaxEncodedLen`) fn sudo_as() -> Weight { // Proof Size summary in bytes: // Measured: `165` // Estimated: `1517` - // Minimum execution time: 12_590_000 picoseconds. - Weight::from_parts(12_994_000, 1517) + // Minimum execution time: 10_202_000 picoseconds. + Weight::from_parts(10_800_000, 1517) + .saturating_add(T::DbWeight::get().reads(1_u64)) + } + /// Storage: `Sudo::Key` (r:1 w:1) + /// Proof: `Sudo::Key` (`max_values`: Some(1), `max_size`: Some(32), added: 527, mode: `MaxEncodedLen`) + fn remove_key() -> Weight { + // Proof Size summary in bytes: + // Measured: `165` + // Estimated: `1517` + // Minimum execution time: 8_555_000 picoseconds. + Weight::from_parts(8_846_000, 1517) .saturating_add(T::DbWeight::get().reads(1_u64)) + .saturating_add(T::DbWeight::get().writes(1_u64)) } } -// For backwards compatibility and tests +// For backwards compatibility and tests. impl WeightInfo for () { - /// Storage: Sudo Key (r:1 w:1) - /// Proof: Sudo Key (max_values: Some(1), max_size: Some(32), added: 527, mode: MaxEncodedLen) + /// Storage: `Sudo::Key` (r:1 w:1) + /// Proof: `Sudo::Key` (`max_values`: Some(1), `max_size`: Some(32), added: 527, mode: `MaxEncodedLen`) fn set_key() -> Weight { // Proof Size summary in bytes: // Measured: `165` // Estimated: `1517` - // Minimum execution time: 12_918_000 picoseconds. - Weight::from_parts(13_403_000, 1517) + // Minimum execution time: 9_600_000 picoseconds. + Weight::from_parts(10_076_000, 1517) .saturating_add(RocksDbWeight::get().reads(1_u64)) .saturating_add(RocksDbWeight::get().writes(1_u64)) } - /// Storage: Sudo Key (r:1 w:0) - /// Proof: Sudo Key (max_values: Some(1), max_size: Some(32), added: 527, mode: MaxEncodedLen) + /// Storage: `Sudo::Key` (r:1 w:0) + /// Proof: `Sudo::Key` (`max_values`: Some(1), `max_size`: Some(32), added: 527, mode: `MaxEncodedLen`) fn sudo() -> Weight { // Proof Size summary in bytes: // Measured: `165` // Estimated: `1517` - // Minimum execution time: 12_693_000 picoseconds. - Weight::from_parts(13_001_000, 1517) + // Minimum execution time: 10_453_000 picoseconds. + Weight::from_parts(10_931_000, 1517) .saturating_add(RocksDbWeight::get().reads(1_u64)) } - /// Storage: Sudo Key (r:1 w:0) - /// Proof: Sudo Key (max_values: Some(1), max_size: Some(32), added: 527, mode: MaxEncodedLen) + /// Storage: `Sudo::Key` (r:1 w:0) + /// Proof: `Sudo::Key` (`max_values`: Some(1), `max_size`: Some(32), added: 527, mode: `MaxEncodedLen`) fn sudo_as() -> Weight { // Proof Size summary in bytes: // Measured: `165` // Estimated: `1517` - // Minimum execution time: 12_590_000 picoseconds. - Weight::from_parts(12_994_000, 1517) + // Minimum execution time: 10_202_000 picoseconds. + Weight::from_parts(10_800_000, 1517) + .saturating_add(RocksDbWeight::get().reads(1_u64)) + } + /// Storage: `Sudo::Key` (r:1 w:1) + /// Proof: `Sudo::Key` (`max_values`: Some(1), `max_size`: Some(32), added: 527, mode: `MaxEncodedLen`) + fn remove_key() -> Weight { + // Proof Size summary in bytes: + // Measured: `165` + // Estimated: `1517` + // Minimum execution time: 8_555_000 picoseconds. + Weight::from_parts(8_846_000, 1517) .saturating_add(RocksDbWeight::get().reads(1_u64)) + .saturating_add(RocksDbWeight::get().writes(1_u64)) } }