From 9bacb18ae4d45ec68e6c195dd64ce8772390afcd Mon Sep 17 00:00:00 2001 From: Sebastian Kunert Date: Fri, 19 Jan 2024 14:46:45 +0100 Subject: [PATCH 01/26] Add SignedExtension, Manual reclaim --- Cargo.lock | 2 + .../src/validate_block/implementation.rs | 28 +- .../parachain-template/node/src/command.rs | 3 +- .../parachain-template/node/src/service.rs | 8 +- cumulus/parachains/common/Cargo.toml | 8 +- cumulus/parachains/common/src/lib.rs | 1 + .../common/src/storage_weight_reclaim.rs | 476 ++++++++++++++++++ cumulus/polkadot-parachain/src/command.rs | 3 +- cumulus/polkadot-parachain/src/service.rs | 13 +- cumulus/test/client/src/lib.rs | 17 +- cumulus/test/service/src/lib.rs | 2 +- substrate/frame/system/src/lib.rs | 8 +- .../primitives/weights/src/weight_meter.rs | 20 + 13 files changed, 561 insertions(+), 28 deletions(-) create mode 100644 cumulus/parachains/common/src/storage_weight_reclaim.rs diff --git a/Cargo.lock b/Cargo.lock index 87a2dad75494..db22d82bfacc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11364,6 +11364,7 @@ name = "parachains-common" version = "1.0.0" dependencies = [ "cumulus-primitives-core", + "cumulus-primitives-proof-size-hostfunction", "cumulus-primitives-utility", "frame-support", "frame-system", @@ -11387,6 +11388,7 @@ dependencies = [ "sp-io", "sp-runtime", "sp-std 8.0.0", + "sp-trie", "staging-parachain-info", "staging-xcm", "staging-xcm-builder", diff --git a/cumulus/pallets/parachain-system/src/validate_block/implementation.rs b/cumulus/pallets/parachain-system/src/validate_block/implementation.rs index ce3b724420f1..a07e079fec39 100644 --- a/cumulus/pallets/parachain-system/src/validate_block/implementation.rs +++ b/cumulus/pallets/parachain-system/src/validate_block/implementation.rs @@ -16,7 +16,7 @@ //! The actual implementation of the validate block functionality. -use super::{trie_cache, MemoryOptimizedValidationParams}; +use super::{trie_cache, trie_recorder, MemoryOptimizedValidationParams}; use cumulus_primitives_core::{ relay_chain::Hash as RHash, ParachainBlockData, PersistedValidationData, }; @@ -33,13 +33,15 @@ use sp_core::storage::{ChildInfo, StateVersion}; use sp_externalities::{set_and_run_with_externalities, Externalities}; use sp_io::KillStorageResult; use sp_runtime::traits::{Block as BlockT, Extrinsic, HashingFor, Header as HeaderT}; -use sp_std::prelude::*; -use sp_trie::MemoryDB; +use sp_std::{prelude::*, sync::Arc}; +use sp_trie::{MemoryDB, ProofSizeProvider, TrieRecorderProvider}; +use trie_recorder::SizeOnlyRecorderProvider; type TrieBackend = sp_state_machine::TrieBackend< MemoryDB>, HashingFor, trie_cache::CacheProvider>, + SizeOnlyRecorderProvider>, >; type Ext<'a, B> = sp_state_machine::Ext<'a, HashingFor, TrieBackend>; @@ -48,6 +50,9 @@ fn with_externalities R, R>(f: F) -> R { sp_externalities::with_externalities(f).expect("Environmental externalities not set.") } +/// Recorder instance to be used during this validate_block call. +environmental::environmental!(recorder: trait ProofSizeProvider); + /// Validate the given parachain block. /// /// This function is doing roughly the following: @@ -120,6 +125,7 @@ where sp_std::mem::drop(storage_proof); + let mut recorder = SizeOnlyRecorderProvider::new(); let cache_provider = trie_cache::CacheProvider::new(); // We use the storage root of the `parent_head` to ensure that it is the correct root. // This is already being done above while creating the in-memory db, but let's be paranoid!! @@ -128,6 +134,7 @@ where *parent_header.state_root(), cache_provider, ) + .with_recorder(recorder.clone()) .build(); let _guard = ( @@ -167,9 +174,11 @@ where .replace_implementation(host_default_child_storage_next_key), sp_io::offchain_index::host_set.replace_implementation(host_offchain_index_set), sp_io::offchain_index::host_clear.replace_implementation(host_offchain_index_clear), + cumulus_primitives_proof_size_hostfunction::storage_proof_size::host_storage_proof_size + .replace_implementation(host_storage_proof_size), ); - run_with_externalities::(&backend, || { + run_with_externalities_and_recorder::(&backend, &mut recorder, || { let relay_chain_proof = crate::RelayChainStateProof::new( PSC::SelfParaId::get(), inherent_data.validation_data.relay_parent_storage_root, @@ -190,7 +199,7 @@ where } }); - run_with_externalities::(&backend, || { + run_with_externalities_and_recorder::(&backend, &mut recorder, || { let head_data = HeadData(block.header().encode()); E::execute_block(block); @@ -266,14 +275,15 @@ fn validate_validation_data( } /// Run the given closure with the externalities set. -fn run_with_externalities R>( +fn run_with_externalities_and_recorder R>( backend: &TrieBackend, + recorder: &mut SizeOnlyRecorderProvider>, execute: F, ) -> R { let mut overlay = sp_state_machine::OverlayedChanges::default(); let mut ext = Ext::::new(&mut overlay, backend); - set_and_run_with_externalities(&mut ext, || execute()) + recorder::using(recorder, || set_and_run_with_externalities(&mut ext, || execute())) } fn host_storage_read(key: &[u8], value_out: &mut [u8], value_offset: u32) -> Option { @@ -305,6 +315,10 @@ fn host_storage_clear(key: &[u8]) { with_externalities(|ext| ext.place_storage(key.to_vec(), None)) } +fn host_storage_proof_size() -> u64 { + recorder::with(|rec| rec.estimate_encoded_size()).expect("Recorder is always set; qed") as _ +} + fn host_storage_root(version: StateVersion) -> Vec { with_externalities(|ext| ext.storage_root(version)) } diff --git a/cumulus/parachain-template/node/src/command.rs b/cumulus/parachain-template/node/src/command.rs index 6ddb68a359a7..29ca44bd5bb6 100644 --- a/cumulus/parachain-template/node/src/command.rs +++ b/cumulus/parachain-template/node/src/command.rs @@ -1,5 +1,6 @@ use std::net::SocketAddr; +use cumulus_client_service::storage_proof_size::HostFunctions as ReclaimHostFunctions; use cumulus_primitives_core::ParaId; use frame_benchmarking_cli::{BenchmarkCmd, SUBSTRATE_REFERENCE_HARDWARE}; use log::info; @@ -183,7 +184,7 @@ pub fn run() -> Result<()> { match cmd { BenchmarkCmd::Pallet(cmd) => if cfg!(feature = "runtime-benchmarks") { - runner.sync_run(|config| cmd.run::(config)) + runner.sync_run(|config| cmd.run::(config)) } else { Err("Benchmarking wasn't enabled when building the node. \ You can enable it with `--features runtime-benchmarks`." diff --git a/cumulus/parachain-template/node/src/service.rs b/cumulus/parachain-template/node/src/service.rs index 830b6e82f969..4dd24803e9b1 100644 --- a/cumulus/parachain-template/node/src/service.rs +++ b/cumulus/parachain-template/node/src/service.rs @@ -40,7 +40,10 @@ use substrate_prometheus_endpoint::Registry; pub struct ParachainNativeExecutor; impl sc_executor::NativeExecutionDispatch for ParachainNativeExecutor { - type ExtendHostFunctions = frame_benchmarking::benchmarking::HostFunctions; + type ExtendHostFunctions = ( + cumulus_client_service::storage_proof_size::HostFunctions, + frame_benchmarking::benchmarking::HostFunctions, + ); fn dispatch(method: &str, data: &[u8]) -> Option> { parachain_template_runtime::api::dispatch(method, data) @@ -100,10 +103,11 @@ pub fn new_partial(config: &Configuration) -> Result let executor = ParachainExecutor::new_with_wasm_executor(wasm); let (client, backend, keystore_container, task_manager) = - sc_service::new_full_parts::( + sc_service::new_full_parts_record_import::( config, telemetry.as_ref().map(|(_, telemetry)| telemetry.handle()), executor, + true, )?; let client = Arc::new(client); diff --git a/cumulus/parachains/common/Cargo.toml b/cumulus/parachains/common/Cargo.toml index 61ac91aeb06b..04256b1e32fb 100644 --- a/cumulus/parachains/common/Cargo.toml +++ b/cumulus/parachains/common/Cargo.toml @@ -48,10 +48,12 @@ pallet-collator-selection = { path = "../../pallets/collator-selection", default cumulus-primitives-core = { path = "../../primitives/core", default-features = false } cumulus-primitives-utility = { path = "../../primitives/utility", default-features = false } parachain-info = { package = "staging-parachain-info", path = "../pallets/parachain-info", default-features = false } +cumulus-primitives-proof-size-hostfunction = { path = "../../primitives/proof-size-hostfunction", default-features = false } [dev-dependencies] -pallet-authorship = { path = "../../../substrate/frame/authorship", default-features = false } -sp-io = { path = "../../../substrate/primitives/io", default-features = false } +pallet-authorship = { path = "../../../substrate/frame/authorship", default-features = false} +sp-io = { path = "../../../substrate/primitives/io", default-features = false} +sp-trie = { path = "../../../substrate/primitives/trie", default-features = false } [build-dependencies] substrate-wasm-builder = { path = "../../../substrate/utils/wasm-builder" } @@ -62,6 +64,7 @@ std = [ "codec/std", "cumulus-primitives-core/std", "cumulus-primitives-utility/std", + "cumulus-primitives-proof-size-hostfunction/std", "frame-support/std", "frame-system/std", "log/std", @@ -81,6 +84,7 @@ std = [ "sp-consensus-aura/std", "sp-core/std", "sp-io/std", + "sp-trie/std", "sp-runtime/std", "sp-std/std", "westend-runtime-constants/std", diff --git a/cumulus/parachains/common/src/lib.rs b/cumulus/parachains/common/src/lib.rs index eab5d7f45774..5630cc0c5bb4 100644 --- a/cumulus/parachains/common/src/lib.rs +++ b/cumulus/parachains/common/src/lib.rs @@ -18,6 +18,7 @@ pub mod impls; pub mod message_queue; pub mod rococo; +pub mod storage_weight_reclaim; pub mod westend; pub mod wococo; pub mod xcm_config; diff --git a/cumulus/parachains/common/src/storage_weight_reclaim.rs b/cumulus/parachains/common/src/storage_weight_reclaim.rs new file mode 100644 index 000000000000..16208e327b9a --- /dev/null +++ b/cumulus/parachains/common/src/storage_weight_reclaim.rs @@ -0,0 +1,476 @@ +// 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. + +//! Mechanism to reclaim PoV weight after an extrinsic has been applied. + +use codec::{Decode, Encode}; +use cumulus_primitives_core::Weight; +use cumulus_primitives_proof_size_hostfunction::storage_proof_size::storage_proof_size; +use frame_support::dispatch::{DispatchClass, DispatchInfo, PerDispatchClass, PostDispatchInfo}; +use frame_system::{BlockWeight, Config}; +use scale_info::TypeInfo; +use sp_runtime::{ + traits::{DispatchInfoOf, Dispatchable, PostDispatchInfoOf, SignedExtension}, + transaction_validity::TransactionValidityError, + DispatchResult, +}; +use sp_std::marker::PhantomData; + +const LOG_TARGET: &'static str = "runtime::storage_reclaim"; + +/// Indicates that proof recording was disabled on the node side. +const PROOF_RECORDING_DISABLED: u64 = u64::MAX; + +/// StorageWeightReclaimer is a mechanism for manually reclaiming storage weight. +/// +/// It internally keeps track of the proof size and storage weight at initialization time. At +/// reclaim it computes the real consumed storage weight and refunds excess weight. +/// +/// +/// # Example +/// +/// ```rust +/// use parachains_common::storage_weight_reclaim::StorageWeightReclaimer; +/// +/// // Initialize a StorageWeightReclaimer instance. +/// let mut storage_reclaim = StorageWeightReclaimer::::start(); +/// +/// // Perform some operations that consume storage weight. +/// +/// // Reclaim unused storage weight. +/// if let Some(reclaimed_weight) = storage_reclaim.reclaim() { +/// log::info!("Reclaimed {} weight", reclaimed_weight); +/// } +/// ``` +pub struct StorageWeightReclaimer { + previous_proof_weight: u64, + previous_reported_proof_size: Option, + _phantom: PhantomData, +} + +impl StorageWeightReclaimer { + /// Creates a new `StorageWeightReclaimer` instance and initializes it with the current storage + /// weight and reported proof size from the node. + pub fn start() -> StorageWeightReclaimer { + let previous_proof_weight = BlockWeight::::get().total().proof_size(); + let previous_reported_proof_size = get_proof_size(); + Self { previous_proof_weight, previous_reported_proof_size, _phantom: Default::default() } + } + + /// Check the consumed storage weight and reclaim any consumed excess weight. + pub fn reclaim(&mut self) -> Option { + let current_weight = BlockWeight::::get().total().proof_size(); + let (Some(current_storage_proof_size), Some(initial_proof_size)) = + (get_proof_size(), self.previous_reported_proof_size) + else { + return None; + }; + let used_weight = current_weight.saturating_sub(self.previous_proof_weight); + let used_storage_proof = current_storage_proof_size.saturating_sub(initial_proof_size); + let reclaimable = used_weight.saturating_sub(used_storage_proof); + log::trace!(target: LOG_TARGET, "Reclaiming storage weight. benchmarked_weight: {used_weight}, consumed_weight: {used_storage_proof}, reclaimable: {reclaimable}"); + frame_system::BlockWeight::::mutate(|current| { + current.reduce(Weight::from_parts(0, reclaimable), DispatchClass::Normal); + }); + + self.previous_proof_weight = current_weight.saturating_sub(reclaimable); + self.previous_reported_proof_size = Some(current_storage_proof_size); + Some(Weight::from_parts(0, reclaimable)) + } +} + +/// Returns the current storage proof size from the host side. +/// +/// Returns `None` if proof recording is disabled on the host. +pub fn get_proof_size() -> Option { + let proof_size = storage_proof_size(); + (proof_size != PROOF_RECORDING_DISABLED).then_some(proof_size) +} + +/// Storage weight reclaim mechanism. +/// +/// This extension checks the size of the node-side storage proof +/// before and after executing a given extrinsic. The difference between +/// benchmarked and spent weight can be reclaimed. +#[derive(Encode, Decode, Clone, Eq, PartialEq, Default, TypeInfo)] +#[scale_info(skip_type_params(T))] +pub struct StorageWeightReclaim(PhantomData); + +impl core::fmt::Debug for StorageWeightReclaim { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> Result<(), core::fmt::Error> { + let _ = write!(f, "StorageWeightReclaim"); + Ok(()) + } +} + +impl SignedExtension for StorageWeightReclaim +where + T::RuntimeCall: Dispatchable, +{ + const IDENTIFIER: &'static str = "StorageWeightReclaim"; + + type AccountId = T::AccountId; + type Call = T::RuntimeCall; + type AdditionalSigned = (); + type Pre = Option; + + fn additional_signed( + &self, + ) -> Result + { + Ok(()) + } + + fn pre_dispatch( + self, + _who: &Self::AccountId, + _call: &Self::Call, + _info: &sp_runtime::traits::DispatchInfoOf, + _len: usize, + ) -> Result { + let proof_size = get_proof_size(); + Ok(proof_size) + } + + fn post_dispatch( + pre: Option, + info: &DispatchInfoOf, + _post_info: &PostDispatchInfoOf, + _len: usize, + _result: &DispatchResult, + ) -> Result<(), TransactionValidityError> { + if let Some(Some(pre_dispatch_proof_size)) = pre { + let Some(post_dispatch_proof_size) = get_proof_size() else { + log::debug!(target: LOG_TARGET, "Proof recording enabled during pre-dispatch, now disabled. This should not happen."); + return Ok(()) + }; + let benchmarked_weight = info.weight.proof_size(); + let consumed_weight = post_dispatch_proof_size.saturating_sub(pre_dispatch_proof_size); + + if consumed_weight > benchmarked_weight { + log::debug!(target: LOG_TARGET, "Benchmarked storage weight smaller than consumed storage weight. benchmarked_weight: {benchmarked_weight} consumed_weight: {consumed_weight}"); + return Ok(()) + } + + let reclaimable_storage_part = + benchmarked_weight.saturating_sub(consumed_weight as u64); + log::trace!(target: LOG_TARGET,"Reclaiming storage weight. benchmarked_weight: {benchmarked_weight}, consumed_weight: {consumed_weight}, reclaimable: {reclaimable_storage_part}"); + frame_system::BlockWeight::::mutate(|current| { + current.reduce(Weight::from_parts(0, reclaimable_storage_part), info.class) + }); + } + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use frame_support::{assert_ok, dispatch::DispatchClass, weights::Weight}; + use frame_system::{ + mock::{new_test_ext, Test, CALL}, + BlockWeight, + }; + use sp_std::marker::PhantomData; + use sp_trie::proof_size_extension::ProofSizeExt; + + struct TestRecorder { + return_values: Box<[usize]>, + counter: std::sync::atomic::AtomicUsize, + } + + impl TestRecorder { + fn new(values: &[usize]) -> Self { + TestRecorder { return_values: values.into(), counter: Default::default() } + } + } + + impl sp_trie::ProofSizeProvider for TestRecorder { + fn estimate_encoded_size(&self) -> usize { + let counter = self.counter.fetch_add(1, core::sync::atomic::Ordering::Relaxed); + self.return_values[counter] + } + } + + fn base_block_weight() -> Weight { + ::BlockWeights::get().base_block + } + + fn setup_test_externalities(proof_values: &[usize]) -> sp_io::TestExternalities { + let mut test_ext = new_test_ext(); + let test_recorder = TestRecorder::new(proof_values); + test_ext.register_extension(ProofSizeExt::new(test_recorder)); + test_ext + } + + fn set_current_storage_weight(new_weight: u64) { + BlockWeight::::mutate(|current_weight| { + current_weight.set(Weight::from_parts(0, new_weight), DispatchClass::Normal); + }); + } + + #[test] + fn basic_refund() { + let mut test_ext = setup_test_externalities(&[100, 200]); + + test_ext.execute_with(|| { + // Benchmarked storage weight: 500 + let info = DispatchInfo { weight: Weight::from_parts(0, 500), ..Default::default() }; + let post_info = PostDispatchInfo { + actual_weight: Some(Weight::zero()), + pays_fee: Default::default(), + }; + + set_current_storage_weight(1000); + + let len = 0_usize; + let pre = StorageWeightReclaim::(PhantomData) + .pre_dispatch(&1, CALL, &info, len) + .unwrap(); + assert_eq!(pre, Some(100)); + + // We expect a refund of 400 + assert_ok!(StorageWeightReclaim::::post_dispatch( + Some(pre), + &info, + &post_info, + len, + &Ok(()) + )); + + assert_eq!( + BlockWeight::::get().total(), + Weight::from_parts(base_block_weight().ref_time(), 600) + ); + }) + } + + #[test] + fn does_nothing_without_extension() { + let mut test_ext = new_test_ext(); + + // Proof size extension not registered + test_ext.execute_with(|| { + // Benchmarked storage weight: 500 + let info = DispatchInfo { weight: Weight::from_parts(0, 500), ..Default::default() }; + let post_info = PostDispatchInfo { + actual_weight: Some(Weight::zero()), + pays_fee: Default::default(), + }; + + set_current_storage_weight(1000); + + let len = 0_usize; + let pre = StorageWeightReclaim::(PhantomData) + .pre_dispatch(&1, CALL, &info, len) + .unwrap(); + assert_eq!(pre, None); + + // We expect a refund of 400 + assert_ok!(StorageWeightReclaim::::post_dispatch( + Some(pre), + &info, + &post_info, + len, + &Ok(()) + )); + + assert_eq!( + BlockWeight::::get().total(), + Weight::from_parts(base_block_weight().ref_time(), 1000) + ); + }) + } + + #[test] + fn negative_refund_is_ignored() { + let mut test_ext = setup_test_externalities(&[100, 300]); + + test_ext.execute_with(|| { + // Benchmarked storage weight: 100 + let info = DispatchInfo { weight: Weight::from_parts(0, 100), ..Default::default() }; + let post_info = PostDispatchInfo { + actual_weight: Some(Weight::zero()), + pays_fee: Default::default(), + }; + + set_current_storage_weight(1000); + + let len = 0_usize; + let pre = StorageWeightReclaim::(PhantomData) + .pre_dispatch(&1, CALL, &info, len) + .unwrap(); + assert_eq!(pre, Some(100)); + + // We expect no refund + assert_ok!(StorageWeightReclaim::::post_dispatch( + Some(pre), + &info, + &post_info, + len, + &Ok(()) + )); + + assert_eq!( + BlockWeight::::get().total(), + Weight::from_parts(base_block_weight().ref_time(), 1000) + ); + }) + } + + #[test] + fn test_zero_proof_size() { + let mut test_ext = setup_test_externalities(&[0, 0]); + + test_ext.execute_with(|| { + let info = DispatchInfo { weight: Weight::from_parts(0, 500), ..Default::default() }; + let post_info = PostDispatchInfo::default(); + + let len = 0_usize; + let pre = StorageWeightReclaim::(PhantomData) + .pre_dispatch(&1, CALL, &info, len) + .unwrap(); + assert_eq!(pre, Some(0)); + + assert_ok!(StorageWeightReclaim::::post_dispatch( + Some(pre), + &info, + &post_info, + len, + &Ok(()) + )); + + assert_eq!(BlockWeight::::get().total(), base_block_weight()); + }); + } + + #[test] + fn test_larger_pre_dispatch_proof_size() { + let mut test_ext = setup_test_externalities(&[300, 100]); + + test_ext.execute_with(|| { + let info = DispatchInfo { weight: Weight::from_parts(0, 500), ..Default::default() }; + let post_info = PostDispatchInfo::default(); + + set_current_storage_weight(1313); + + let len = 0_usize; + let pre = StorageWeightReclaim::(PhantomData) + .pre_dispatch(&1, CALL, &info, len) + .unwrap(); + assert_eq!(pre, Some(300)); + + assert_ok!(StorageWeightReclaim::::post_dispatch( + Some(pre), + &info, + &post_info, + len, + &Ok(()) + )); + + assert_eq!( + BlockWeight::::get().total(), + Weight::from_parts(base_block_weight().ref_time(), 813) + ); + }); + } + + #[test] + fn storage_size_reported_correctly() { + let mut test_ext = setup_test_externalities(&[1000]); + test_ext.execute_with(|| { + assert_eq!(get_proof_size(), Some(1000)); + }); + + let mut test_ext = new_test_ext(); + + let test_recorder = TestRecorder::new(&[0]); + + test_ext.register_extension(ProofSizeExt::new(test_recorder)); + + test_ext.execute_with(|| { + assert_eq!(get_proof_size(), Some(0)); + }); + } + + #[test] + fn storage_size_disabled_reported_correctly() { + let mut test_ext = setup_test_externalities(&[PROOF_RECORDING_DISABLED as usize]); + + test_ext.execute_with(|| { + assert_eq!(get_proof_size(), None); + }); + } + + #[test] + fn test_reclaim_helper() { + let mut test_ext = setup_test_externalities(&[1000, 1300, 1800]); + + test_ext.execute_with(|| { + set_current_storage_weight(1000); + let mut reclaim_helper = StorageWeightReclaimer::::start(); + set_current_storage_weight(1500); + let reclaimed = reclaim_helper.reclaim(); + + assert_eq!(reclaimed, Some(Weight::from_parts(0, 200))); + assert_eq!( + BlockWeight::::get().total(), + Weight::from_parts(base_block_weight().ref_time(), 1300) + ); + + set_current_storage_weight(2100); + let reclaimed = reclaim_helper.reclaim(); + assert_eq!(reclaimed, Some(Weight::from_parts(0, 300))); + assert_eq!( + BlockWeight::::get().total(), + Weight::from_parts(base_block_weight().ref_time(), 1800) + ); + }); + } + + #[test] + fn test_reclaim_helper_does_not_reclaim_negative() { + // Benchmarked weight does not change at all + let mut test_ext = setup_test_externalities(&[1000, 1300]); + + test_ext.execute_with(|| { + set_current_storage_weight(1000); + let mut reclaim_helper = StorageWeightReclaimer::::start(); + let reclaimed = reclaim_helper.reclaim(); + + assert_eq!(reclaimed, Some(Weight::from_parts(0, 0))); + assert_eq!( + BlockWeight::::get().total(), + Weight::from_parts(base_block_weight().ref_time(), 1000) + ); + }); + + // Benchmarked weight increases less than storage proof consumes + let mut test_ext = setup_test_externalities(&[1000, 1300]); + + test_ext.execute_with(|| { + set_current_storage_weight(1000); + let mut reclaim_helper = StorageWeightReclaimer::::start(); + set_current_storage_weight(1200); + let reclaimed = reclaim_helper.reclaim(); + + assert_eq!(reclaimed, Some(Weight::from_parts(0, 0))); + assert_eq!( + BlockWeight::::get().total(), + Weight::from_parts(base_block_weight().ref_time(), 1200) + ); + }); + } +} diff --git a/cumulus/polkadot-parachain/src/command.rs b/cumulus/polkadot-parachain/src/command.rs index 04d618f66c75..a73d455da4ee 100644 --- a/cumulus/polkadot-parachain/src/command.rs +++ b/cumulus/polkadot-parachain/src/command.rs @@ -23,6 +23,7 @@ use crate::{ }, service::{new_partial, Block}, }; +use cumulus_client_service::storage_proof_size::HostFunctions as ReclaimHostFunctions; use cumulus_primitives_core::ParaId; use frame_benchmarking_cli::{BenchmarkCmd, SUBSTRATE_REFERENCE_HARDWARE}; use log::info; @@ -584,7 +585,7 @@ pub fn run() -> Result<()> { match cmd { BenchmarkCmd::Pallet(cmd) => if cfg!(feature = "runtime-benchmarks") { - runner.sync_run(|config| cmd.run::(config)) + runner.sync_run(|config| cmd.run::(config)) } else { Err("Benchmarking wasn't enabled when building the node. \ You can enable it with `--features runtime-benchmarks`." diff --git a/cumulus/polkadot-parachain/src/service.rs b/cumulus/polkadot-parachain/src/service.rs index 81d0c9d3980e..db024e2d55f3 100644 --- a/cumulus/polkadot-parachain/src/service.rs +++ b/cumulus/polkadot-parachain/src/service.rs @@ -68,11 +68,15 @@ use substrate_prometheus_endpoint::Registry; use polkadot_primitives::CollatorPair; #[cfg(not(feature = "runtime-benchmarks"))] -type HostFunctions = sp_io::SubstrateHostFunctions; +type HostFunctions = + (sp_io::SubstrateHostFunctions, cumulus_client_service::storage_proof_size::HostFunctions); #[cfg(feature = "runtime-benchmarks")] -type HostFunctions = - (sp_io::SubstrateHostFunctions, frame_benchmarking::benchmarking::HostFunctions); +type HostFunctions = ( + sp_io::SubstrateHostFunctions, + cumulus_client_service::storage_proof_size::HostFunctions, + frame_benchmarking::benchmarking::HostFunctions, +); type ParachainClient = TFullClient>; @@ -274,10 +278,11 @@ where .build(); let (client, backend, keystore_container, task_manager) = - sc_service::new_full_parts::( + sc_service::new_full_parts_record_import::( config, telemetry.as_ref().map(|(_, telemetry)| telemetry.handle()), executor, + true, )?; let client = Arc::new(client); diff --git a/cumulus/test/client/src/lib.rs b/cumulus/test/client/src/lib.rs index df63f683de6b..0593e7d60060 100644 --- a/cumulus/test/client/src/lib.rs +++ b/cumulus/test/client/src/lib.rs @@ -203,13 +203,16 @@ pub fn validate_block( let mut ext_ext = ext.ext(); let heap_pages = HeapAllocStrategy::Static { extra_pages: 1024 }; - let executor = WasmExecutor::::builder() - .with_execution_method(WasmExecutionMethod::default()) - .with_max_runtime_instances(1) - .with_runtime_cache_size(2) - .with_onchain_heap_alloc_strategy(heap_pages) - .with_offchain_heap_alloc_strategy(heap_pages) - .build(); + let executor = WasmExecutor::<( + sp_io::SubstrateHostFunctions, + cumulus_primitives_proof_size_hostfunction::storage_proof_size::HostFunctions, + )>::builder() + .with_execution_method(WasmExecutionMethod::default()) + .with_max_runtime_instances(1) + .with_runtime_cache_size(2) + .with_onchain_heap_alloc_strategy(heap_pages) + .with_offchain_heap_alloc_strategy(heap_pages) + .build(); executor .uncached_call( diff --git a/cumulus/test/service/src/lib.rs b/cumulus/test/service/src/lib.rs index 37ea984ac87a..c838858d17a5 100644 --- a/cumulus/test/service/src/lib.rs +++ b/cumulus/test/service/src/lib.rs @@ -112,7 +112,7 @@ pub type AnnounceBlockFn = Arc>) + Send + Sync>; pub struct RuntimeExecutor; impl sc_executor::NativeExecutionDispatch for RuntimeExecutor { - type ExtendHostFunctions = (); + type ExtendHostFunctions = cumulus_client_service::storage_proof_size::HostFunctions; fn dispatch(method: &str, data: &[u8]) -> Option> { cumulus_test_runtime::api::dispatch(method, data) diff --git a/substrate/frame/system/src/lib.rs b/substrate/frame/system/src/lib.rs index 069217bcee46..7f0365eef74b 100644 --- a/substrate/frame/system/src/lib.rs +++ b/substrate/frame/system/src/lib.rs @@ -146,8 +146,10 @@ use sp_weights::{RuntimeDbWeight, Weight}; use sp_io::TestExternalities; pub mod limits; -#[cfg(test)] -pub(crate) mod mock; + +#[cfg(any(feature = "std", test))] +pub mod mock; + pub mod offchain; mod extensions; @@ -847,7 +849,7 @@ pub mod pallet { #[pallet::storage] #[pallet::whitelist_storage] #[pallet::getter(fn block_weight)] - pub(super) type BlockWeight = StorageValue<_, ConsumedWeight, ValueQuery>; + pub type BlockWeight = StorageValue<_, ConsumedWeight, ValueQuery>; /// Total length (in bytes) for all extrinsics put together, for the current block. #[pallet::storage] diff --git a/substrate/primitives/weights/src/weight_meter.rs b/substrate/primitives/weights/src/weight_meter.rs index 584d22304c3a..1738948e4c3c 100644 --- a/substrate/primitives/weights/src/weight_meter.rs +++ b/substrate/primitives/weights/src/weight_meter.rs @@ -149,6 +149,11 @@ impl WeightMeter { pub fn can_consume(&self, w: Weight) -> bool { self.consumed.checked_add(&w).map_or(false, |t| t.all_lte(self.limit)) } + + /// Reclaim the given weight. + pub fn reclaim_proof_size(&mut self, s: u64) { + self.consumed.saturating_reduce(Weight::from_parts(0, s)); + } } #[cfg(test)] @@ -277,6 +282,21 @@ mod tests { assert_eq!(meter.consumed(), Weight::from_parts(5, 10)); } + #[test] + #[cfg(debug_assertions)] + fn reclaim_works() { + let mut meter = WeightMeter::with_limit(Weight::from_parts(5, 10)); + + meter.consume(Weight::from_parts(5, 10)); + assert_eq!(meter.consumed(), Weight::from_parts(5, 10)); + + meter.reclaim_proof_size(3); + assert_eq!(meter.consumed(), Weight::from_parts(5, 7)); + + meter.reclaim_proof_size(10); + assert_eq!(meter.consumed(), Weight::from_parts(5, 0)); + } + #[test] #[cfg(debug_assertions)] #[should_panic(expected = "Weight counter overflow")] From fca700fa91097d998acbe0b4281a0c9646be65a4 Mon Sep 17 00:00:00 2001 From: Sebastian Kunert Date: Fri, 19 Jan 2024 15:05:37 +0100 Subject: [PATCH 02/26] Remove unnecessary bound --- cumulus/parachains/common/src/storage_weight_reclaim.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cumulus/parachains/common/src/storage_weight_reclaim.rs b/cumulus/parachains/common/src/storage_weight_reclaim.rs index 16208e327b9a..eed813457c52 100644 --- a/cumulus/parachains/common/src/storage_weight_reclaim.rs +++ b/cumulus/parachains/common/src/storage_weight_reclaim.rs @@ -54,13 +54,13 @@ const PROOF_RECORDING_DISABLED: u64 = u64::MAX; /// log::info!("Reclaimed {} weight", reclaimed_weight); /// } /// ``` -pub struct StorageWeightReclaimer { +pub struct StorageWeightReclaimer { previous_proof_weight: u64, previous_reported_proof_size: Option, _phantom: PhantomData, } -impl StorageWeightReclaimer { +impl StorageWeightReclaimer { /// Creates a new `StorageWeightReclaimer` instance and initializes it with the current storage /// weight and reported proof size from the node. pub fn start() -> StorageWeightReclaimer { From f8086456faad3dac248b353a578fbdb911abc9bc Mon Sep 17 00:00:00 2001 From: Sebastian Kunert Date: Mon, 29 Jan 2024 18:01:30 +0100 Subject: [PATCH 03/26] Fix taplo + clippy --- cumulus/parachains/common/Cargo.toml | 8 ++++---- cumulus/parachains/common/src/storage_weight_reclaim.rs | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/cumulus/parachains/common/Cargo.toml b/cumulus/parachains/common/Cargo.toml index 5252bbe662d6..752d494fd0af 100644 --- a/cumulus/parachains/common/Cargo.toml +++ b/cumulus/parachains/common/Cargo.toml @@ -51,8 +51,8 @@ parachain-info = { package = "staging-parachain-info", path = "../pallets/parach cumulus-primitives-proof-size-hostfunction = { path = "../../primitives/proof-size-hostfunction", default-features = false } [dev-dependencies] -pallet-authorship = { path = "../../../substrate/frame/authorship", default-features = false} -sp-io = { path = "../../../substrate/primitives/io", default-features = false} +pallet-authorship = { path = "../../../substrate/frame/authorship", default-features = false } +sp-io = { path = "../../../substrate/primitives/io", default-features = false } sp-trie = { path = "../../../substrate/primitives/trie", default-features = false } [build-dependencies] @@ -63,8 +63,8 @@ default = ["std"] std = [ "codec/std", "cumulus-primitives-core/std", - "cumulus-primitives-utility/std", "cumulus-primitives-proof-size-hostfunction/std", + "cumulus-primitives-utility/std", "frame-support/std", "frame-system/std", "log/std", @@ -83,9 +83,9 @@ std = [ "sp-consensus-aura/std", "sp-core/std", "sp-io/std", - "sp-trie/std", "sp-runtime/std", "sp-std/std", + "sp-trie/std", "westend-runtime-constants?/std", "xcm-executor/std", "xcm/std", diff --git a/cumulus/parachains/common/src/storage_weight_reclaim.rs b/cumulus/parachains/common/src/storage_weight_reclaim.rs index eed813457c52..310755614ca0 100644 --- a/cumulus/parachains/common/src/storage_weight_reclaim.rs +++ b/cumulus/parachains/common/src/storage_weight_reclaim.rs @@ -18,7 +18,7 @@ use codec::{Decode, Encode}; use cumulus_primitives_core::Weight; use cumulus_primitives_proof_size_hostfunction::storage_proof_size::storage_proof_size; -use frame_support::dispatch::{DispatchClass, DispatchInfo, PerDispatchClass, PostDispatchInfo}; +use frame_support::dispatch::{DispatchClass, DispatchInfo, PostDispatchInfo}; use frame_system::{BlockWeight, Config}; use scale_info::TypeInfo; use sp_runtime::{ From 088dd8dcba2299440e7cd450d528840e6fe64bca Mon Sep 17 00:00:00 2001 From: Sebastian Kunert Date: Tue, 30 Jan 2024 10:05:00 +0100 Subject: [PATCH 04/26] More CI fixes --- cumulus/parachains/common/src/storage_weight_reclaim.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cumulus/parachains/common/src/storage_weight_reclaim.rs b/cumulus/parachains/common/src/storage_weight_reclaim.rs index 310755614ca0..e3e6494b025e 100644 --- a/cumulus/parachains/common/src/storage_weight_reclaim.rs +++ b/cumulus/parachains/common/src/storage_weight_reclaim.rs @@ -41,7 +41,7 @@ const PROOF_RECORDING_DISABLED: u64 = u64::MAX; /// /// # Example /// -/// ```rust +/// ```ignore /// use parachains_common::storage_weight_reclaim::StorageWeightReclaimer; /// /// // Initialize a StorageWeightReclaimer instance. From ce33ece19c321752d4ce3db5638d6f5d88e5c3c1 Mon Sep 17 00:00:00 2001 From: Sebastian Kunert Date: Tue, 30 Jan 2024 13:29:01 +0100 Subject: [PATCH 05/26] Provide `reclaim_with_meter` --- .../common/src/storage_weight_reclaim.rs | 45 ++++++++++++++++++- 1 file changed, 43 insertions(+), 2 deletions(-) diff --git a/cumulus/parachains/common/src/storage_weight_reclaim.rs b/cumulus/parachains/common/src/storage_weight_reclaim.rs index e3e6494b025e..b9a9b2bec3c4 100644 --- a/cumulus/parachains/common/src/storage_weight_reclaim.rs +++ b/cumulus/parachains/common/src/storage_weight_reclaim.rs @@ -18,7 +18,10 @@ use codec::{Decode, Encode}; use cumulus_primitives_core::Weight; use cumulus_primitives_proof_size_hostfunction::storage_proof_size::storage_proof_size; -use frame_support::dispatch::{DispatchClass, DispatchInfo, PostDispatchInfo}; +use frame_support::{ + dispatch::{DispatchClass, DispatchInfo, PostDispatchInfo}, + weights::WeightMeter, +}; use frame_system::{BlockWeight, Config}; use scale_info::TypeInfo; use sp_runtime::{ @@ -89,6 +92,16 @@ impl StorageWeightReclaimer { self.previous_reported_proof_size = Some(current_storage_proof_size); Some(Weight::from_parts(0, reclaimable)) } + + /// Check the consumed storage weight and reclaim any consumed excess weight. Adds the reclaimed + /// weight budget back to `weight_meter`. + pub fn reclaim_with_meter(&mut self, weight_meter: &mut WeightMeter) -> Option { + let reclaimed = self.reclaim(); + if let Some(ref weight) = reclaimed { + weight_meter.reclaim_proof_size(weight.proof_size()); + } + reclaimed + } } /// Returns the current storage proof size from the host side. @@ -178,7 +191,11 @@ where #[cfg(test)] mod tests { use super::*; - use frame_support::{assert_ok, dispatch::DispatchClass, weights::Weight}; + use frame_support::{ + assert_ok, + dispatch::DispatchClass, + weights::{Weight, WeightMeter}, + }; use frame_system::{ mock::{new_test_ext, Test, CALL}, BlockWeight, @@ -473,4 +490,28 @@ mod tests { ); }); } + + #[test] + fn test_reclaim_helper_works_with_meter() { + let mut test_ext = setup_test_externalities(&[10, 12]); + + test_ext.execute_with(|| { + let mut remaining_weight_counter = WeightMeter::with_limit(Weight::from_parts(10, 10)); + + set_current_storage_weight(10); + let mut reclaim_helper = StorageWeightReclaimer::::start(); + + //substract benchmarked weight + remaining_weight_counter.consume(Weight::from_parts(0, 5)); + set_current_storage_weight(15); + let reclaimed = reclaim_helper.reclaim_with_meter(&mut remaining_weight_counter); + + assert_eq!(reclaimed, Some(Weight::from_parts(0, 3))); + assert_eq!( + BlockWeight::::get().total(), + Weight::from_parts(base_block_weight().ref_time(), 12) + ); + assert_eq!(remaining_weight_counter.remaining(), Weight::from_parts(10, 8)); + }); + } } From 72b881886870f550ec43f191df9895f8fbe07154 Mon Sep 17 00:00:00 2001 From: Sebastian Kunert Date: Fri, 2 Feb 2024 14:17:40 +0100 Subject: [PATCH 06/26] Reclaimer should not reclaim directly, now only subtracts from meter --- .../common/src/storage_weight_reclaim.rs | 115 ++++++++---------- 1 file changed, 48 insertions(+), 67 deletions(-) diff --git a/cumulus/parachains/common/src/storage_weight_reclaim.rs b/cumulus/parachains/common/src/storage_weight_reclaim.rs index b9a9b2bec3c4..fbabd65923ba 100644 --- a/cumulus/parachains/common/src/storage_weight_reclaim.rs +++ b/cumulus/parachains/common/src/storage_weight_reclaim.rs @@ -19,10 +19,10 @@ use codec::{Decode, Encode}; use cumulus_primitives_core::Weight; use cumulus_primitives_proof_size_hostfunction::storage_proof_size::storage_proof_size; use frame_support::{ - dispatch::{DispatchClass, DispatchInfo, PostDispatchInfo}, + dispatch::{DispatchInfo, PostDispatchInfo}, weights::WeightMeter, }; -use frame_system::{BlockWeight, Config}; +use frame_system::Config; use scale_info::TypeInfo; use sp_runtime::{ traits::{DispatchInfoOf, Dispatchable, PostDispatchInfoOf, SignedExtension}, @@ -45,58 +45,54 @@ const PROOF_RECORDING_DISABLED: u64 = u64::MAX; /// # Example /// /// ```ignore -/// use parachains_common::storage_weight_reclaim::StorageWeightReclaimer; +/// use parachains_common::storage_weight_reclaim::StorageWeightReclaimer; /// -/// // Initialize a StorageWeightReclaimer instance. -/// let mut storage_reclaim = StorageWeightReclaimer::::start(); -/// -/// // Perform some operations that consume storage weight. -/// -/// // Reclaim unused storage weight. -/// if let Some(reclaimed_weight) = storage_reclaim.reclaim() { -/// log::info!("Reclaimed {} weight", reclaimed_weight); -/// } +/// let mut remaining_weight_meter = WeightMeter::with_limit(Weight::from_parts(10, 10)); +/// let mut reclaim_helper = StorageWeightReclaimer::start(&remaining_weight_meter); +/// remaining_weight_meter.try_consume(get_weight_for_work()).is_ok() { +/// do_work(); +/// if let Some(relaimed) = reclaim_helper.reclaim_with_meter(&mut remaining_weight_meter) { +/// log::info!("Reclaimed {} weight", reclaimed_weight); +/// } +/// } /// ``` -pub struct StorageWeightReclaimer { - previous_proof_weight: u64, +pub struct StorageWeightReclaimer { + previous_remaining_proof_weight: u64, previous_reported_proof_size: Option, - _phantom: PhantomData, } -impl StorageWeightReclaimer { +impl StorageWeightReclaimer { /// Creates a new `StorageWeightReclaimer` instance and initializes it with the current storage /// weight and reported proof size from the node. - pub fn start() -> StorageWeightReclaimer { - let previous_proof_weight = BlockWeight::::get().total().proof_size(); + pub fn start(weight_meter: &WeightMeter) -> StorageWeightReclaimer { + let previous_remaining_proof_weight = weight_meter.remaining().proof_size(); let previous_reported_proof_size = get_proof_size(); - Self { previous_proof_weight, previous_reported_proof_size, _phantom: Default::default() } + Self { previous_remaining_proof_weight, previous_reported_proof_size } } - /// Check the consumed storage weight and reclaim any consumed excess weight. - pub fn reclaim(&mut self) -> Option { - let current_weight = BlockWeight::::get().total().proof_size(); + /// Check the consumed storage weight and calculate the consumed excess weight. + fn reclaim(&mut self, remaining_weight: Weight) -> Option { + let current_remaining_weight = remaining_weight.proof_size(); let (Some(current_storage_proof_size), Some(initial_proof_size)) = (get_proof_size(), self.previous_reported_proof_size) else { return None; }; - let used_weight = current_weight.saturating_sub(self.previous_proof_weight); + let used_weight = + self.previous_remaining_proof_weight.saturating_sub(current_remaining_weight); let used_storage_proof = current_storage_proof_size.saturating_sub(initial_proof_size); let reclaimable = used_weight.saturating_sub(used_storage_proof); - log::trace!(target: LOG_TARGET, "Reclaiming storage weight. benchmarked_weight: {used_weight}, consumed_weight: {used_storage_proof}, reclaimable: {reclaimable}"); - frame_system::BlockWeight::::mutate(|current| { - current.reduce(Weight::from_parts(0, reclaimable), DispatchClass::Normal); - }); + log::trace!(target: LOG_TARGET, "Found reclaimable storage weight. benchmarked_weight: {used_weight}, consumed_weight: {used_storage_proof}, reclaimable: {reclaimable}"); - self.previous_proof_weight = current_weight.saturating_sub(reclaimable); + self.previous_remaining_proof_weight = current_remaining_weight.saturating_add(reclaimable); self.previous_reported_proof_size = Some(current_storage_proof_size); Some(Weight::from_parts(0, reclaimable)) } - /// Check the consumed storage weight and reclaim any consumed excess weight. Adds the reclaimed + /// Check the consumed storage weight and add the reclaimed /// weight budget back to `weight_meter`. pub fn reclaim_with_meter(&mut self, weight_meter: &mut WeightMeter) -> Option { - let reclaimed = self.reclaim(); + let reclaimed = self.reclaim(weight_meter.remaining()); if let Some(ref weight) = reclaimed { weight_meter.reclaim_proof_size(weight.proof_size()); } @@ -436,24 +432,17 @@ mod tests { let mut test_ext = setup_test_externalities(&[1000, 1300, 1800]); test_ext.execute_with(|| { - set_current_storage_weight(1000); - let mut reclaim_helper = StorageWeightReclaimer::::start(); - set_current_storage_weight(1500); - let reclaimed = reclaim_helper.reclaim(); + let mut remaining_weight_meter = WeightMeter::with_limit(Weight::from_parts(0, 2000)); + let mut reclaim_helper = StorageWeightReclaimer::start(&remaining_weight_meter); + remaining_weight_meter.consume(Weight::from_parts(0, 500)); + let reclaimed = reclaim_helper.reclaim_with_meter(&mut remaining_weight_meter); assert_eq!(reclaimed, Some(Weight::from_parts(0, 200))); - assert_eq!( - BlockWeight::::get().total(), - Weight::from_parts(base_block_weight().ref_time(), 1300) - ); - set_current_storage_weight(2100); - let reclaimed = reclaim_helper.reclaim(); + remaining_weight_meter.consume(Weight::from_parts(0, 800)); + let reclaimed = reclaim_helper.reclaim_with_meter(&mut remaining_weight_meter); assert_eq!(reclaimed, Some(Weight::from_parts(0, 300))); - assert_eq!( - BlockWeight::::get().total(), - Weight::from_parts(base_block_weight().ref_time(), 1800) - ); + assert_eq!(remaining_weight_meter.remaining(), Weight::from_parts(0, 1200)); }); } @@ -463,31 +452,24 @@ mod tests { let mut test_ext = setup_test_externalities(&[1000, 1300]); test_ext.execute_with(|| { - set_current_storage_weight(1000); - let mut reclaim_helper = StorageWeightReclaimer::::start(); - let reclaimed = reclaim_helper.reclaim(); + let mut remaining_weight_meter = WeightMeter::with_limit(Weight::from_parts(0, 1000)); + let mut reclaim_helper = StorageWeightReclaimer::start(&remaining_weight_meter); + let reclaimed = reclaim_helper.reclaim_with_meter(&mut remaining_weight_meter); assert_eq!(reclaimed, Some(Weight::from_parts(0, 0))); - assert_eq!( - BlockWeight::::get().total(), - Weight::from_parts(base_block_weight().ref_time(), 1000) - ); + assert_eq!(remaining_weight_meter.remaining(), Weight::from_parts(0, 1000)); }); // Benchmarked weight increases less than storage proof consumes let mut test_ext = setup_test_externalities(&[1000, 1300]); test_ext.execute_with(|| { - set_current_storage_weight(1000); - let mut reclaim_helper = StorageWeightReclaimer::::start(); - set_current_storage_weight(1200); - let reclaimed = reclaim_helper.reclaim(); + let mut remaining_weight_meter = WeightMeter::with_limit(Weight::from_parts(0, 1000)); + let mut reclaim_helper = StorageWeightReclaimer::start(&remaining_weight_meter); + remaining_weight_meter.consume(Weight::from_parts(0, 0)); + let reclaimed = reclaim_helper.reclaim_with_meter(&mut remaining_weight_meter); assert_eq!(reclaimed, Some(Weight::from_parts(0, 0))); - assert_eq!( - BlockWeight::::get().total(), - Weight::from_parts(base_block_weight().ref_time(), 1200) - ); }); } @@ -496,22 +478,21 @@ mod tests { let mut test_ext = setup_test_externalities(&[10, 12]); test_ext.execute_with(|| { - let mut remaining_weight_counter = WeightMeter::with_limit(Weight::from_parts(10, 10)); + let mut remaining_weight_meter = WeightMeter::with_limit(Weight::from_parts(10, 10)); set_current_storage_weight(10); - let mut reclaim_helper = StorageWeightReclaimer::::start(); + let mut reclaim_helper = StorageWeightReclaimer::start(&remaining_weight_meter); - //substract benchmarked weight - remaining_weight_counter.consume(Weight::from_parts(0, 5)); - set_current_storage_weight(15); - let reclaimed = reclaim_helper.reclaim_with_meter(&mut remaining_weight_counter); + // Substract benchmarked weight + remaining_weight_meter.consume(Weight::from_parts(0, 5)); + let reclaimed = reclaim_helper.reclaim_with_meter(&mut remaining_weight_meter); assert_eq!(reclaimed, Some(Weight::from_parts(0, 3))); assert_eq!( BlockWeight::::get().total(), - Weight::from_parts(base_block_weight().ref_time(), 12) + Weight::from_parts(base_block_weight().ref_time(), 10) ); - assert_eq!(remaining_weight_counter.remaining(), Weight::from_parts(10, 8)); + assert_eq!(remaining_weight_meter.remaining(), Weight::from_parts(10, 8)); }); } } From ec26a45745776e7beae5a56b8e7f5090a23c578a Mon Sep 17 00:00:00 2001 From: Sebastian Kunert Date: Fri, 2 Feb 2024 17:48:15 +0100 Subject: [PATCH 07/26] Update cumulus/parachains/common/src/storage_weight_reclaim.rs Co-authored-by: Dmitry Markin --- cumulus/parachains/common/src/storage_weight_reclaim.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/cumulus/parachains/common/src/storage_weight_reclaim.rs b/cumulus/parachains/common/src/storage_weight_reclaim.rs index fbabd65923ba..60fb4a2e1b45 100644 --- a/cumulus/parachains/common/src/storage_weight_reclaim.rs +++ b/cumulus/parachains/common/src/storage_weight_reclaim.rs @@ -291,7 +291,6 @@ mod tests { .unwrap(); assert_eq!(pre, None); - // We expect a refund of 400 assert_ok!(StorageWeightReclaim::::post_dispatch( Some(pre), &info, From fb8667404e5b1eaaee3ebeb4c95fca2dc5d79497 Mon Sep 17 00:00:00 2001 From: Sebastian Kunert Date: Thu, 15 Feb 2024 13:50:35 +0100 Subject: [PATCH 08/26] Apply suggestions from code review Co-authored-by: Davide Galassi --- .../common/src/storage_weight_reclaim.rs | 35 ++++++++++++------- 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/cumulus/parachains/common/src/storage_weight_reclaim.rs b/cumulus/parachains/common/src/storage_weight_reclaim.rs index 60fb4a2e1b45..8077dce489f9 100644 --- a/cumulus/parachains/common/src/storage_weight_reclaim.rs +++ b/cumulus/parachains/common/src/storage_weight_reclaim.rs @@ -13,11 +13,13 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! Mechanism to reclaim PoV weight after an extrinsic has been applied. +//! Mechanism to reclaim PoV proof size weight after an extrinsic has been applied. use codec::{Decode, Encode}; use cumulus_primitives_core::Weight; -use cumulus_primitives_proof_size_hostfunction::storage_proof_size::storage_proof_size; +use cumulus_primitives_proof_size_hostfunction::{ + storage_proof_size::storage_proof_size, PROOF_RECORDING_DISABLED, +}; use frame_support::{ dispatch::{DispatchInfo, PostDispatchInfo}, weights::WeightMeter, @@ -33,10 +35,7 @@ use sp_std::marker::PhantomData; const LOG_TARGET: &'static str = "runtime::storage_reclaim"; -/// Indicates that proof recording was disabled on the node side. -const PROOF_RECORDING_DISABLED: u64 = u64::MAX; - -/// StorageWeightReclaimer is a mechanism for manually reclaiming storage weight. +/// `StorageWeightReclaimer` is a mechanism for manually reclaiming storage weight. /// /// It internally keeps track of the proof size and storage weight at initialization time. At /// reclaim it computes the real consumed storage weight and refunds excess weight. @@ -62,8 +61,8 @@ pub struct StorageWeightReclaimer { } impl StorageWeightReclaimer { - /// Creates a new `StorageWeightReclaimer` instance and initializes it with the current storage - /// weight and reported proof size from the node. + /// Creates a new `StorageWeightReclaimer` instance and initializes it with the storage + /// size provided by `weight_meter` and reported proof size from the node. pub fn start(weight_meter: &WeightMeter) -> StorageWeightReclaimer { let previous_remaining_proof_weight = weight_meter.remaining().proof_size(); let previous_reported_proof_size = get_proof_size(); @@ -82,7 +81,10 @@ impl StorageWeightReclaimer { self.previous_remaining_proof_weight.saturating_sub(current_remaining_weight); let used_storage_proof = current_storage_proof_size.saturating_sub(initial_proof_size); let reclaimable = used_weight.saturating_sub(used_storage_proof); - log::trace!(target: LOG_TARGET, "Found reclaimable storage weight. benchmarked_weight: {used_weight}, consumed_weight: {used_storage_proof}, reclaimable: {reclaimable}"); + log::trace!( + target: LOG_TARGET, + "Found reclaimable storage weight. benchmarked: {used_weight}, consumed: {used_storage_proof}" + ); self.previous_remaining_proof_weight = current_remaining_weight.saturating_add(reclaimable); self.previous_reported_proof_size = Some(current_storage_proof_size); @@ -162,20 +164,29 @@ where ) -> Result<(), TransactionValidityError> { if let Some(Some(pre_dispatch_proof_size)) = pre { let Some(post_dispatch_proof_size) = get_proof_size() else { - log::debug!(target: LOG_TARGET, "Proof recording enabled during pre-dispatch, now disabled. This should not happen."); + log::debug!( + target: LOG_TARGET, + "Proof recording enabled during pre-dispatch, now disabled. This should not happen." + ); return Ok(()) }; let benchmarked_weight = info.weight.proof_size(); let consumed_weight = post_dispatch_proof_size.saturating_sub(pre_dispatch_proof_size); if consumed_weight > benchmarked_weight { - log::debug!(target: LOG_TARGET, "Benchmarked storage weight smaller than consumed storage weight. benchmarked_weight: {benchmarked_weight} consumed_weight: {consumed_weight}"); + log::debug!( + target: LOG_TARGET, + "Benchmarked storage weight smaller than consumed storage weight. benchmarked: {benchmarked_weight} consumed: {consumed_weight}" + ); return Ok(()) } let reclaimable_storage_part = benchmarked_weight.saturating_sub(consumed_weight as u64); - log::trace!(target: LOG_TARGET,"Reclaiming storage weight. benchmarked_weight: {benchmarked_weight}, consumed_weight: {consumed_weight}, reclaimable: {reclaimable_storage_part}"); + log::trace!( + target: LOG_TARGET, + "Reclaiming storage weight. benchmarked: {benchmarked_weight}, consumed: {consumed_weight}" + ); frame_system::BlockWeight::::mutate(|current| { current.reduce(Weight::from_parts(0, reclaimable_storage_part), info.class) }); From 00dbfb610eedb32d01d2e45dcd5e3cbcd30a18f7 Mon Sep 17 00:00:00 2001 From: Sebastian Kunert Date: Thu, 15 Feb 2024 15:21:11 +0100 Subject: [PATCH 09/26] Improve naming --- .../common/src/storage_weight_reclaim.rs | 86 ++++++++++--------- .../proof-size-hostfunction/src/lib.rs | 3 +- 2 files changed, 46 insertions(+), 43 deletions(-) diff --git a/cumulus/parachains/common/src/storage_weight_reclaim.rs b/cumulus/parachains/common/src/storage_weight_reclaim.rs index 8077dce489f9..72c2264c4491 100644 --- a/cumulus/parachains/common/src/storage_weight_reclaim.rs +++ b/cumulus/parachains/common/src/storage_weight_reclaim.rs @@ -18,7 +18,7 @@ use codec::{Decode, Encode}; use cumulus_primitives_core::Weight; use cumulus_primitives_proof_size_hostfunction::{ - storage_proof_size::storage_proof_size, PROOF_RECORDING_DISABLED, + storage_proof_size::storage_proof_size, PROOF_RECORDING_DISABLED, }; use frame_support::{ dispatch::{DispatchInfo, PostDispatchInfo}, @@ -50,43 +50,44 @@ const LOG_TARGET: &'static str = "runtime::storage_reclaim"; /// let mut reclaim_helper = StorageWeightReclaimer::start(&remaining_weight_meter); /// remaining_weight_meter.try_consume(get_weight_for_work()).is_ok() { /// do_work(); -/// if let Some(relaimed) = reclaim_helper.reclaim_with_meter(&mut remaining_weight_meter) { +/// if let Some(relaimed_weight) = reclaim_helper.reclaim_with_meter(&mut remaining_weight_meter) { /// log::info!("Reclaimed {} weight", reclaimed_weight); /// } /// } /// ``` pub struct StorageWeightReclaimer { - previous_remaining_proof_weight: u64, + previous_remaining_proof_size: u64, previous_reported_proof_size: Option, } impl StorageWeightReclaimer { /// Creates a new `StorageWeightReclaimer` instance and initializes it with the storage /// size provided by `weight_meter` and reported proof size from the node. - pub fn start(weight_meter: &WeightMeter) -> StorageWeightReclaimer { - let previous_remaining_proof_weight = weight_meter.remaining().proof_size(); + pub fn new(weight_meter: &WeightMeter) -> StorageWeightReclaimer { + let previous_remaining_proof_size = weight_meter.remaining().proof_size(); let previous_reported_proof_size = get_proof_size(); - Self { previous_remaining_proof_weight, previous_reported_proof_size } + Self { previous_remaining_proof_size, previous_reported_proof_size } } /// Check the consumed storage weight and calculate the consumed excess weight. fn reclaim(&mut self, remaining_weight: Weight) -> Option { let current_remaining_weight = remaining_weight.proof_size(); - let (Some(current_storage_proof_size), Some(initial_proof_size)) = + let (Some(current_storage_proof_size), Some(previous_storage_proof_size)) = (get_proof_size(), self.previous_reported_proof_size) else { return None; }; let used_weight = - self.previous_remaining_proof_weight.saturating_sub(current_remaining_weight); - let used_storage_proof = current_storage_proof_size.saturating_sub(initial_proof_size); - let reclaimable = used_weight.saturating_sub(used_storage_proof); + self.previous_remaining_proof_size.saturating_sub(current_remaining_weight); + let reported_used_size = + current_storage_proof_size.saturating_sub(previous_storage_proof_size); + let reclaimable = used_weight.saturating_sub(reported_used_size); log::trace!( target: LOG_TARGET, - "Found reclaimable storage weight. benchmarked: {used_weight}, consumed: {used_storage_proof}" + "Found reclaimable storage weight. benchmarked: {used_weight}, consumed: {reported_used_size}" ); - self.previous_remaining_proof_weight = current_remaining_weight.saturating_add(reclaimable); + self.previous_remaining_proof_size = current_remaining_weight.saturating_add(reclaimable); self.previous_reported_proof_size = Some(current_storage_proof_size); Some(Weight::from_parts(0, reclaimable)) } @@ -162,35 +163,36 @@ where _len: usize, _result: &DispatchResult, ) -> Result<(), TransactionValidityError> { - if let Some(Some(pre_dispatch_proof_size)) = pre { - let Some(post_dispatch_proof_size) = get_proof_size() else { - log::debug!( - target: LOG_TARGET, - "Proof recording enabled during pre-dispatch, now disabled. This should not happen." - ); - return Ok(()) - }; - let benchmarked_weight = info.weight.proof_size(); - let consumed_weight = post_dispatch_proof_size.saturating_sub(pre_dispatch_proof_size); - - if consumed_weight > benchmarked_weight { - log::debug!( - target: LOG_TARGET, - "Benchmarked storage weight smaller than consumed storage weight. benchmarked: {benchmarked_weight} consumed: {consumed_weight}" - ); - return Ok(()) - } - - let reclaimable_storage_part = - benchmarked_weight.saturating_sub(consumed_weight as u64); - log::trace!( + let Some(Some(pre_dispatch_proof_size)) = pre else { + return Ok(()); + }; + + let Some(post_dispatch_proof_size) = get_proof_size() else { + log::debug!( + target: LOG_TARGET, + "Proof recording enabled during pre-dispatch, now disabled. This should not happen." + ); + return Ok(()) + }; + let benchmarked_weight = info.weight.proof_size(); + let consumed_weight = post_dispatch_proof_size.saturating_sub(pre_dispatch_proof_size); + + if consumed_weight > benchmarked_weight { + log::debug!( target: LOG_TARGET, - "Reclaiming storage weight. benchmarked: {benchmarked_weight}, consumed: {consumed_weight}" + "Benchmarked storage weight smaller than consumed storage weight. benchmarked: {benchmarked_weight} consumed: {consumed_weight}" ); - frame_system::BlockWeight::::mutate(|current| { - current.reduce(Weight::from_parts(0, reclaimable_storage_part), info.class) - }); + return Ok(()) } + + let reclaimable_storage_part = benchmarked_weight.saturating_sub(consumed_weight as u64); + log::trace!( + target: LOG_TARGET, + "Reclaiming storage weight. benchmarked: {benchmarked_weight}, consumed: {consumed_weight}" + ); + frame_system::BlockWeight::::mutate(|current| { + current.reduce(Weight::from_parts(0, reclaimable_storage_part), info.class) + }); Ok(()) } } @@ -443,7 +445,7 @@ mod tests { test_ext.execute_with(|| { let mut remaining_weight_meter = WeightMeter::with_limit(Weight::from_parts(0, 2000)); - let mut reclaim_helper = StorageWeightReclaimer::start(&remaining_weight_meter); + let mut reclaim_helper = StorageWeightReclaimer::new(&remaining_weight_meter); remaining_weight_meter.consume(Weight::from_parts(0, 500)); let reclaimed = reclaim_helper.reclaim_with_meter(&mut remaining_weight_meter); @@ -463,7 +465,7 @@ mod tests { test_ext.execute_with(|| { let mut remaining_weight_meter = WeightMeter::with_limit(Weight::from_parts(0, 1000)); - let mut reclaim_helper = StorageWeightReclaimer::start(&remaining_weight_meter); + let mut reclaim_helper = StorageWeightReclaimer::new(&remaining_weight_meter); let reclaimed = reclaim_helper.reclaim_with_meter(&mut remaining_weight_meter); assert_eq!(reclaimed, Some(Weight::from_parts(0, 0))); @@ -475,7 +477,7 @@ mod tests { test_ext.execute_with(|| { let mut remaining_weight_meter = WeightMeter::with_limit(Weight::from_parts(0, 1000)); - let mut reclaim_helper = StorageWeightReclaimer::start(&remaining_weight_meter); + let mut reclaim_helper = StorageWeightReclaimer::new(&remaining_weight_meter); remaining_weight_meter.consume(Weight::from_parts(0, 0)); let reclaimed = reclaim_helper.reclaim_with_meter(&mut remaining_weight_meter); @@ -491,7 +493,7 @@ mod tests { let mut remaining_weight_meter = WeightMeter::with_limit(Weight::from_parts(10, 10)); set_current_storage_weight(10); - let mut reclaim_helper = StorageWeightReclaimer::start(&remaining_weight_meter); + let mut reclaim_helper = StorageWeightReclaimer::new(&remaining_weight_meter); // Substract benchmarked weight remaining_weight_meter.consume(Weight::from_parts(0, 5)); diff --git a/cumulus/primitives/proof-size-hostfunction/src/lib.rs b/cumulus/primitives/proof-size-hostfunction/src/lib.rs index 6da6235e585a..b0bf1628e03a 100644 --- a/cumulus/primitives/proof-size-hostfunction/src/lib.rs +++ b/cumulus/primitives/proof-size-hostfunction/src/lib.rs @@ -35,7 +35,8 @@ pub const PROOF_RECORDING_DISABLED: u64 = u64::MAX; pub trait StorageProofSize { /// Returns the current storage proof size. fn storage_proof_size(&mut self) -> u64 { - self.extension::().map_or(u64::MAX, |e| e.storage_proof_size()) + self.extension::() + .map_or(PROOF_RECORDING_DISABLED, |e| e.storage_proof_size()) } } From 903a026c819d375947fbd783788245e2d0b16ef4 Mon Sep 17 00:00:00 2001 From: Sebastian Kunert Date: Thu, 15 Feb 2024 16:31:18 +0100 Subject: [PATCH 10/26] Add reclaim extension to template runtime --- cumulus/parachain-template/runtime/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/cumulus/parachain-template/runtime/src/lib.rs b/cumulus/parachain-template/runtime/src/lib.rs index d9bc111fcef7..409c8c984ffc 100644 --- a/cumulus/parachain-template/runtime/src/lib.rs +++ b/cumulus/parachain-template/runtime/src/lib.rs @@ -107,6 +107,7 @@ pub type SignedExtra = ( frame_system::CheckNonce, frame_system::CheckWeight, pallet_transaction_payment::ChargeTransactionPayment, + parachains_common::storage_weight_reclaim::StorageWeightReclaim, ); /// Unchecked extrinsic type as expected by this runtime. From d9550e30886fa43ba67f02afc88facaac8088bc9 Mon Sep 17 00:00:00 2001 From: Sebastian Kunert Date: Thu, 15 Feb 2024 16:42:52 +0100 Subject: [PATCH 11/26] Prdoc --- prdoc/pr_3002.prdoc | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 prdoc/pr_3002.prdoc diff --git a/prdoc/pr_3002.prdoc b/prdoc/pr_3002.prdoc new file mode 100644 index 000000000000..1ae380111bea --- /dev/null +++ b/prdoc/pr_3002.prdoc @@ -0,0 +1,26 @@ +# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0 +# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json + +title: PoV Reclaim Runtime Side +author: skunert +topic: runtime +doc: + - audience: Node Dev + description: | + Adds a mechanism to reclaim proof size weight. + 1. Introduces a new `SignedExtension` that reclaims the difference + between benchmarked proof size weight and actual consumed proof size weight. + 2. Introduces a manual mechanism, `StorageWeightReclaimer`, to reclaim excess storage weight for situations + that require manual weight management. The most prominent case is the `on_idle` hook. + 3. Adds the `storage_proof_size` host function to the PVF. Parachain nodes should add it to ensure compatibility. + + To enable proof size reclaiming, add the host `storage_proof_size` host function to the parachain node. Add the + `StorageWeightReclaim` `SignedExtension` to your runtime and enable proof recording during block import. + + +crates: [ ] +host_functions: + - title: `storage_proof_size` + description: | + This host function is used to pass the current size of the storage proof to the runtime. + It was introduced before but becomes relevant now. From a54f6709d37c01cb3fa8160bd96df4c1abfd17df Mon Sep 17 00:00:00 2001 From: Sebastian Kunert Date: Fri, 16 Feb 2024 15:17:48 +0100 Subject: [PATCH 12/26] Reset recorder in PVF --- .../parachain-system/src/validate_block/implementation.rs | 1 + .../parachain-system/src/validate_block/trie_recorder.rs | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/cumulus/pallets/parachain-system/src/validate_block/implementation.rs b/cumulus/pallets/parachain-system/src/validate_block/implementation.rs index a07e079fec39..6aceee7dd568 100644 --- a/cumulus/pallets/parachain-system/src/validate_block/implementation.rs +++ b/cumulus/pallets/parachain-system/src/validate_block/implementation.rs @@ -283,6 +283,7 @@ fn run_with_externalities_and_recorder R>( let mut overlay = sp_state_machine::OverlayedChanges::default(); let mut ext = Ext::::new(&mut overlay, backend); + recorder.reset(); recorder::using(recorder, || set_and_run_with_externalities(&mut ext, || execute())) } diff --git a/cumulus/pallets/parachain-system/src/validate_block/trie_recorder.rs b/cumulus/pallets/parachain-system/src/validate_block/trie_recorder.rs index e73aef70aa49..d956a139a595 100644 --- a/cumulus/pallets/parachain-system/src/validate_block/trie_recorder.rs +++ b/cumulus/pallets/parachain-system/src/validate_block/trie_recorder.rs @@ -97,6 +97,7 @@ pub(crate) struct SizeOnlyRecorderProvider { } impl SizeOnlyRecorderProvider { + /// Create a new instance of [`SizeOnlyRecorderProvider`] pub fn new() -> Self { Self { seen_nodes: Default::default(), @@ -104,6 +105,13 @@ impl SizeOnlyRecorderProvider { recorded_keys: Default::default(), } } + + /// Reset a new instance of [`SizeOnlyRecorderProvider`] + pub fn reset(&mut self) { + self.seen_nodes.borrow_mut().clear(); + *self.encoded_size.borrow_mut() = 0; + self.recorded_keys.borrow_mut().clear(); + } } impl sp_trie::TrieRecorderProvider for SizeOnlyRecorderProvider { From 93c5570ae268e3992ed69f9d2f8653fa64abd325 Mon Sep 17 00:00:00 2001 From: Sebastian Kunert Date: Mon, 19 Feb 2024 14:42:56 +0100 Subject: [PATCH 13/26] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bastian Köcher --- .../src/validate_block/implementation.rs | 2 +- .../src/validate_block/trie_recorder.rs | 4 ++-- .../common/src/storage_weight_reclaim.rs | 22 +++++++------------ prdoc/pr_3002.prdoc | 2 +- 4 files changed, 12 insertions(+), 18 deletions(-) diff --git a/cumulus/pallets/parachain-system/src/validate_block/implementation.rs b/cumulus/pallets/parachain-system/src/validate_block/implementation.rs index 6aceee7dd568..361ee95f3a2c 100644 --- a/cumulus/pallets/parachain-system/src/validate_block/implementation.rs +++ b/cumulus/pallets/parachain-system/src/validate_block/implementation.rs @@ -33,7 +33,7 @@ use sp_core::storage::{ChildInfo, StateVersion}; use sp_externalities::{set_and_run_with_externalities, Externalities}; use sp_io::KillStorageResult; use sp_runtime::traits::{Block as BlockT, Extrinsic, HashingFor, Header as HeaderT}; -use sp_std::{prelude::*, sync::Arc}; +use sp_std::prelude::*; use sp_trie::{MemoryDB, ProofSizeProvider, TrieRecorderProvider}; use trie_recorder::SizeOnlyRecorderProvider; diff --git a/cumulus/pallets/parachain-system/src/validate_block/trie_recorder.rs b/cumulus/pallets/parachain-system/src/validate_block/trie_recorder.rs index d956a139a595..7d1805033252 100644 --- a/cumulus/pallets/parachain-system/src/validate_block/trie_recorder.rs +++ b/cumulus/pallets/parachain-system/src/validate_block/trie_recorder.rs @@ -106,8 +106,8 @@ impl SizeOnlyRecorderProvider { } } - /// Reset a new instance of [`SizeOnlyRecorderProvider`] - pub fn reset(&mut self) { + /// Reset the internal state. + pub fn reset(&self) { self.seen_nodes.borrow_mut().clear(); *self.encoded_size.borrow_mut() = 0; self.recorded_keys.borrow_mut().clear(); diff --git a/cumulus/parachains/common/src/storage_weight_reclaim.rs b/cumulus/parachains/common/src/storage_weight_reclaim.rs index 72c2264c4491..1650e9999f06 100644 --- a/cumulus/parachains/common/src/storage_weight_reclaim.rs +++ b/cumulus/parachains/common/src/storage_weight_reclaim.rs @@ -40,7 +40,6 @@ const LOG_TARGET: &'static str = "runtime::storage_reclaim"; /// It internally keeps track of the proof size and storage weight at initialization time. At /// reclaim it computes the real consumed storage weight and refunds excess weight. /// -/// /// # Example /// /// ```ignore @@ -63,6 +62,7 @@ pub struct StorageWeightReclaimer { impl StorageWeightReclaimer { /// Creates a new `StorageWeightReclaimer` instance and initializes it with the storage /// size provided by `weight_meter` and reported proof size from the node. + #[must_use = "Must call `reclaim_with_meter` to reclaim the weight"] pub fn new(weight_meter: &WeightMeter) -> StorageWeightReclaimer { let previous_remaining_proof_size = weight_meter.remaining().proof_size(); let previous_reported_proof_size = get_proof_size(); @@ -72,11 +72,8 @@ impl StorageWeightReclaimer { /// Check the consumed storage weight and calculate the consumed excess weight. fn reclaim(&mut self, remaining_weight: Weight) -> Option { let current_remaining_weight = remaining_weight.proof_size(); - let (Some(current_storage_proof_size), Some(previous_storage_proof_size)) = - (get_proof_size(), self.previous_reported_proof_size) - else { - return None; - }; + let current_storage_proof_size = get_proof_size()?; + let previous_storage_proof_size = self.previous_reported_proof_size?; let used_weight = self.previous_remaining_proof_size.saturating_sub(current_remaining_weight); let reported_used_size = @@ -95,11 +92,9 @@ impl StorageWeightReclaimer { /// Check the consumed storage weight and add the reclaimed /// weight budget back to `weight_meter`. pub fn reclaim_with_meter(&mut self, weight_meter: &mut WeightMeter) -> Option { - let reclaimed = self.reclaim(weight_meter.remaining()); - if let Some(ref weight) = reclaimed { - weight_meter.reclaim_proof_size(weight.proof_size()); - } - reclaimed + let reclaimed = self.reclaim(weight_meter.remaining())?; + weight_meter.reclaim_proof_size(reclaimed.proof_size()); + Some(reclaimed) } } @@ -152,8 +147,7 @@ where _info: &sp_runtime::traits::DispatchInfoOf, _len: usize, ) -> Result { - let proof_size = get_proof_size(); - Ok(proof_size) + Ok(get_proof_size()) } fn post_dispatch( @@ -178,7 +172,7 @@ where let consumed_weight = post_dispatch_proof_size.saturating_sub(pre_dispatch_proof_size); if consumed_weight > benchmarked_weight { - log::debug!( + log::error!( target: LOG_TARGET, "Benchmarked storage weight smaller than consumed storage weight. benchmarked: {benchmarked_weight} consumed: {consumed_weight}" ); diff --git a/prdoc/pr_3002.prdoc b/prdoc/pr_3002.prdoc index 1ae380111bea..3a282668ce25 100644 --- a/prdoc/pr_3002.prdoc +++ b/prdoc/pr_3002.prdoc @@ -5,7 +5,7 @@ title: PoV Reclaim Runtime Side author: skunert topic: runtime doc: - - audience: Node Dev + - audience: Runtime Dev description: | Adds a mechanism to reclaim proof size weight. 1. Introduces a new `SignedExtension` that reclaims the difference From a257a5241dcf3e9174889335619f7dc85fb5f285 Mon Sep 17 00:00:00 2001 From: Sebastian Kunert Date: Mon, 19 Feb 2024 15:04:32 +0100 Subject: [PATCH 14/26] Review comments --- Cargo.lock | 20 ++++++++++-- Cargo.toml | 1 + .../src/validate_block/implementation.rs | 16 ++++++++-- cumulus/parachain-template/runtime/Cargo.toml | 2 ++ cumulus/parachain-template/runtime/src/lib.rs | 2 +- cumulus/parachains/common/Cargo.toml | 4 --- cumulus/parachains/common/src/lib.rs | 1 - .../storage-weight-reclaim/Cargo.toml | 32 +++++++++++++++++++ .../storage-weight-reclaim/src/lib.rs} | 32 +++++++++---------- 9 files changed, 83 insertions(+), 27 deletions(-) create mode 100644 cumulus/primitives/storage-weight-reclaim/Cargo.toml rename cumulus/{parachains/common/src/storage_weight_reclaim.rs => primitives/storage-weight-reclaim/src/lib.rs} (95%) diff --git a/Cargo.lock b/Cargo.lock index 5871f540fd48..24ab7f1dd8b2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4048,6 +4048,23 @@ dependencies = [ "sp-trie", ] +[[package]] +name = "cumulus-primitives-storage-weight-reclaim" +version = "0.2.0" +dependencies = [ + "cumulus-primitives-core", + "cumulus-primitives-proof-size-hostfunction", + "frame-support", + "frame-system", + "log", + "parity-scale-codec", + "scale-info", + "sp-io", + "sp-runtime", + "sp-std 14.0.0", + "sp-trie", +] + [[package]] name = "cumulus-primitives-timestamp" version = "0.7.0" @@ -11302,6 +11319,7 @@ dependencies = [ "cumulus-pallet-xcm", "cumulus-pallet-xcmp-queue", "cumulus-primitives-core", + "cumulus-primitives-storage-weight-reclaim", "cumulus-primitives-utility", "frame-benchmarking", "frame-executive", @@ -11354,7 +11372,6 @@ name = "parachains-common" version = "7.0.0" dependencies = [ "cumulus-primitives-core", - "cumulus-primitives-proof-size-hostfunction", "cumulus-primitives-utility", "frame-support", "frame-system", @@ -11374,7 +11391,6 @@ dependencies = [ "sp-io", "sp-runtime", "sp-std 14.0.0", - "sp-trie", "staging-parachain-info", "staging-xcm", "staging-xcm-executor", diff --git a/Cargo.toml b/Cargo.toml index d09f19f280cb..6414b506c650 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -127,6 +127,7 @@ members = [ "cumulus/primitives/core", "cumulus/primitives/parachain-inherent", "cumulus/primitives/proof-size-hostfunction", + "cumulus/primitives/storage-weight-reclaim", "cumulus/primitives/timestamp", "cumulus/primitives/utility", "cumulus/test/client", diff --git a/cumulus/pallets/parachain-system/src/validate_block/implementation.rs b/cumulus/pallets/parachain-system/src/validate_block/implementation.rs index 361ee95f3a2c..7374ce126c22 100644 --- a/cumulus/pallets/parachain-system/src/validate_block/implementation.rs +++ b/cumulus/pallets/parachain-system/src/validate_block/implementation.rs @@ -178,7 +178,7 @@ where .replace_implementation(host_storage_proof_size), ); - run_with_externalities_and_recorder::(&backend, &mut recorder, || { + run_with_externalities::(&backend, || { let relay_chain_proof = crate::RelayChainStateProof::new( PSC::SelfParaId::get(), inherent_data.validation_data.relay_parent_storage_root, @@ -274,7 +274,7 @@ fn validate_validation_data( ); } -/// Run the given closure with the externalities set. +/// Run the given closure with the externalities and recorder set. fn run_with_externalities_and_recorder R>( backend: &TrieBackend, recorder: &mut SizeOnlyRecorderProvider>, @@ -283,10 +283,20 @@ fn run_with_externalities_and_recorder R>( let mut overlay = sp_state_machine::OverlayedChanges::default(); let mut ext = Ext::::new(&mut overlay, backend); - recorder.reset(); recorder::using(recorder, || set_and_run_with_externalities(&mut ext, || execute())) } +/// Run the given closure with the externalities set. +fn run_with_externalities R>( + backend: &TrieBackend, + execute: F, +) -> R { + let mut overlay = sp_state_machine::OverlayedChanges::default(); + let mut ext = Ext::::new(&mut overlay, backend); + + set_and_run_with_externalities(&mut ext, || execute()) +} + fn host_storage_read(key: &[u8], value_out: &mut [u8], value_offset: u32) -> Option { match with_externalities(|ext| ext.storage(key)) { Some(value) => { diff --git a/cumulus/parachain-template/runtime/Cargo.toml b/cumulus/parachain-template/runtime/Cargo.toml index 44d96ffc4e62..1873bd0a23eb 100644 --- a/cumulus/parachain-template/runtime/Cargo.toml +++ b/cumulus/parachain-template/runtime/Cargo.toml @@ -73,6 +73,7 @@ cumulus-pallet-xcm = { path = "../../pallets/xcm", default-features = false } cumulus-pallet-xcmp-queue = { path = "../../pallets/xcmp-queue", default-features = false } cumulus-primitives-core = { path = "../../primitives/core", default-features = false } cumulus-primitives-utility = { path = "../../primitives/utility", default-features = false } +cumulus-primitives-storage-weight-reclaim = { path = "../../primitives/storage-weight-reclaim", default-features = false } pallet-collator-selection = { path = "../../pallets/collator-selection", default-features = false } parachains-common = { path = "../../parachains/common", default-features = false } parachain-info = { package = "staging-parachain-info", path = "../../parachains/pallets/parachain-info", default-features = false } @@ -87,6 +88,7 @@ std = [ "cumulus-pallet-xcm/std", "cumulus-pallet-xcmp-queue/std", "cumulus-primitives-core/std", + "cumulus-primitives-storage-weight-reclaim/std", "cumulus-primitives-utility/std", "frame-benchmarking?/std", "frame-executive/std", diff --git a/cumulus/parachain-template/runtime/src/lib.rs b/cumulus/parachain-template/runtime/src/lib.rs index 409c8c984ffc..cee9b33bf37c 100644 --- a/cumulus/parachain-template/runtime/src/lib.rs +++ b/cumulus/parachain-template/runtime/src/lib.rs @@ -107,7 +107,7 @@ pub type SignedExtra = ( frame_system::CheckNonce, frame_system::CheckWeight, pallet_transaction_payment::ChargeTransactionPayment, - parachains_common::storage_weight_reclaim::StorageWeightReclaim, + cumulus_primitives_storage_weight_reclaim::StorageWeightReclaim, ); /// Unchecked extrinsic type as expected by this runtime. diff --git a/cumulus/parachains/common/Cargo.toml b/cumulus/parachains/common/Cargo.toml index 40d9f30ed635..ebc9f822beb2 100644 --- a/cumulus/parachains/common/Cargo.toml +++ b/cumulus/parachains/common/Cargo.toml @@ -42,12 +42,10 @@ pallet-collator-selection = { path = "../../pallets/collator-selection", default cumulus-primitives-core = { path = "../../primitives/core", default-features = false } cumulus-primitives-utility = { path = "../../primitives/utility", default-features = false } parachain-info = { package = "staging-parachain-info", path = "../pallets/parachain-info", default-features = false } -cumulus-primitives-proof-size-hostfunction = { path = "../../primitives/proof-size-hostfunction", default-features = false } [dev-dependencies] pallet-authorship = { path = "../../../substrate/frame/authorship", default-features = false } sp-io = { path = "../../../substrate/primitives/io", default-features = false } -sp-trie = { path = "../../../substrate/primitives/trie", default-features = false } [build-dependencies] substrate-wasm-builder = { path = "../../../substrate/utils/wasm-builder" } @@ -57,7 +55,6 @@ default = ["std"] std = [ "codec/std", "cumulus-primitives-core/std", - "cumulus-primitives-proof-size-hostfunction/std", "cumulus-primitives-utility/std", "frame-support/std", "frame-system/std", @@ -77,7 +74,6 @@ std = [ "sp-io/std", "sp-runtime/std", "sp-std/std", - "sp-trie/std", "xcm-executor/std", "xcm/std", ] diff --git a/cumulus/parachains/common/src/lib.rs b/cumulus/parachains/common/src/lib.rs index f42c5b760a5a..b01d623d2b93 100644 --- a/cumulus/parachains/common/src/lib.rs +++ b/cumulus/parachains/common/src/lib.rs @@ -17,7 +17,6 @@ pub mod impls; pub mod message_queue; -pub mod storage_weight_reclaim; pub mod xcm_config; pub use constants::*; pub use opaque::*; diff --git a/cumulus/primitives/storage-weight-reclaim/Cargo.toml b/cumulus/primitives/storage-weight-reclaim/Cargo.toml new file mode 100644 index 000000000000..bc223104e734 --- /dev/null +++ b/cumulus/primitives/storage-weight-reclaim/Cargo.toml @@ -0,0 +1,32 @@ +[package] +name = "cumulus-primitives-storage-weight-reclaim" +version = "0.2.0" +authors.workspace = true +edition.workspace = true +description = "Utilities to reclaim storage weight." +license = "Apache-2.0" + +[lints] +workspace = true + +[dependencies] +codec = { package = "parity-scale-codec", version = "3.0.0", default-features = false, features = ["derive"] } +log = { workspace = true } +scale-info = { version = "2.10.0", default-features = false, features = ["derive"] } + +frame-support = { path = "../../../substrate/frame/support", default-features = false } +frame-system = { path = "../../../substrate/frame/system", default-features = false } + +sp-runtime = { path = "../../../substrate/primitives/runtime", default-features = false } +sp-std = { path = "../../../substrate/primitives/std", default-features = false } + +cumulus-primitives-core = { path = "../../primitives/core", default-features = false } +cumulus-primitives-proof-size-hostfunction = { path = "../../primitives/proof-size-hostfunction", default-features = false } + +[dev-dependencies] +sp-trie = { path = "../../../substrate/primitives/trie", default-features = false } +sp-io = { path = "../../../substrate/primitives/io", default-features = false } + +[features] +default = ["std"] +std = ["codec/std", "cumulus-primitives-core/std", "cumulus-primitives-proof-size-hostfunction/std", "frame-support/std", "frame-system/std", "log/std", "scale-info/std", "sp-io/std", "sp-runtime/std", "sp-std/std", "sp-trie/std"] diff --git a/cumulus/parachains/common/src/storage_weight_reclaim.rs b/cumulus/primitives/storage-weight-reclaim/src/lib.rs similarity index 95% rename from cumulus/parachains/common/src/storage_weight_reclaim.rs rename to cumulus/primitives/storage-weight-reclaim/src/lib.rs index 1650e9999f06..2d506d977a47 100644 --- a/cumulus/parachains/common/src/storage_weight_reclaim.rs +++ b/cumulus/primitives/storage-weight-reclaim/src/lib.rs @@ -171,21 +171,21 @@ where let benchmarked_weight = info.weight.proof_size(); let consumed_weight = post_dispatch_proof_size.saturating_sub(pre_dispatch_proof_size); - if consumed_weight > benchmarked_weight { - log::error!( - target: LOG_TARGET, - "Benchmarked storage weight smaller than consumed storage weight. benchmarked: {benchmarked_weight} consumed: {consumed_weight}" - ); - return Ok(()) - } - - let reclaimable_storage_part = benchmarked_weight.saturating_sub(consumed_weight as u64); - log::trace!( - target: LOG_TARGET, - "Reclaiming storage weight. benchmarked: {benchmarked_weight}, consumed: {consumed_weight}" - ); + let storage_size_diff = benchmarked_weight.abs_diff(consumed_weight as u64); frame_system::BlockWeight::::mutate(|current| { - current.reduce(Weight::from_parts(0, reclaimable_storage_part), info.class) + if consumed_weight > benchmarked_weight { + log::error!( + target: LOG_TARGET, + "Benchmarked storage weight smaller than consumed storage weight. benchmarked: {benchmarked_weight} consumed: {consumed_weight}" + ); + current.accrue(Weight::from_parts(0, storage_size_diff), info.class) + } else { + log::trace!( + target: LOG_TARGET, + "Reclaiming storage weight. benchmarked: {benchmarked_weight}, consumed: {consumed_weight}" + ); + current.reduce(Weight::from_parts(0, storage_size_diff), info.class) + } }); Ok(()) } @@ -314,7 +314,7 @@ mod tests { } #[test] - fn negative_refund_is_ignored() { + fn negative_refund_is_added_to_weight() { let mut test_ext = setup_test_externalities(&[100, 300]); test_ext.execute_with(|| { @@ -344,7 +344,7 @@ mod tests { assert_eq!( BlockWeight::::get().total(), - Weight::from_parts(base_block_weight().ref_time(), 1000) + Weight::from_parts(base_block_weight().ref_time(), 1100) ); }) } From 9433f16bd31663af04f451227225195aed1cab21 Mon Sep 17 00:00:00 2001 From: Sebastian Kunert Date: Mon, 19 Feb 2024 18:14:44 +0100 Subject: [PATCH 15/26] Use docify --- Cargo.lock | 229 +++++++++--------- .../src/validate_block/implementation.rs | 1 + .../storage-weight-reclaim/Cargo.toml | 1 + .../storage-weight-reclaim/src/lib.rs | 56 +++-- 4 files changed, 154 insertions(+), 133 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 24ab7f1dd8b2..b78aa50afb7b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -191,7 +191,7 @@ checksum = "c0391754c09fab4eae3404d19d0d297aa1c670c1775ab51d8a5312afeca23157" dependencies = [ "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -206,7 +206,7 @@ dependencies = [ "proc-macro-error", "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.49", "syn-solidity", "tiny-keccak", ] @@ -333,7 +333,7 @@ dependencies = [ "proc-macro-error", "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -1218,7 +1218,7 @@ checksum = "16e62a023e7c117e27523144c5d2459f4397fcc3cab0085af8e2224f643a0193" dependencies = [ "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -1235,7 +1235,7 @@ checksum = "a66537f1bb974b254c98ed142ff995236e81b9d0fe4db0575f46612cb15eb0f9" dependencies = [ "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -1417,7 +1417,7 @@ dependencies = [ "regex", "rustc-hash", "shlex", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -2602,19 +2602,19 @@ dependencies = [ "clap_lex 0.2.4", "indexmap 1.9.3", "once_cell", - "strsim", + "strsim 0.10.0", "termcolor", "textwrap", ] [[package]] name = "clap" -version = "4.4.18" +version = "4.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1e578d6ec4194633722ccf9544794b71b1385c3c027efe0c55db226fc880865c" +checksum = "c918d541ef2913577a0f9566e9ce27cb35b6df072075769e0b26cb5a554520da" dependencies = [ "clap_builder", - "clap_derive 4.4.7", + "clap_derive 4.5.0", ] [[package]] @@ -2628,14 +2628,14 @@ dependencies = [ [[package]] name = "clap_builder" -version = "4.4.18" +version = "4.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4df4df40ec50c46000231c914968278b1eb05098cf8f1b3a518a95030e71d1c7" +checksum = "9f3e7391dad68afb0c2ede1bf619f579a3dc9c2ec67f089baa397123a2f3d1eb" dependencies = [ "anstream", "anstyle", - "clap_lex 0.6.0", - "strsim", + "clap_lex 0.7.0", + "strsim 0.11.0", "terminal_size", ] @@ -2645,7 +2645,7 @@ version = "4.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "586a385f7ef2f8b4d86bddaa0c094794e7ccbfe5ffef1f434fe928143fc783a5" dependencies = [ - "clap 4.4.18", + "clap 4.5.1", ] [[package]] @@ -2663,14 +2663,14 @@ dependencies = [ [[package]] name = "clap_derive" -version = "4.4.7" +version = "4.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cf9804afaaf59a91e75b022a30fb7229a7901f60c755489cc61c9b423b836442" +checksum = "307bc0538d5f0f83b8248db3087aa92fe504e4691294d0c96c0eabc33f47ba47" dependencies = [ "heck", "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -2684,9 +2684,9 @@ dependencies = [ [[package]] name = "clap_lex" -version = "0.6.0" +version = "0.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "702fc72eb24e5a1e48ce58027a675bc24edd52096d5397d4aea7c6dd9eca0bd1" +checksum = "98cc8fbded0c607b7ba9dd60cd98df59af97e84d24e49c8557331cfc26d301ce" [[package]] name = "coarsetime" @@ -3382,7 +3382,7 @@ dependencies = [ "anes", "cast", "ciborium", - "clap 4.4.18", + "clap 4.5.1", "criterion-plot", "futures", "is-terminal", @@ -3545,7 +3545,7 @@ dependencies = [ name = "cumulus-client-cli" version = "0.7.0" dependencies = [ - "clap 4.4.18", + "clap 4.5.1", "parity-scale-codec", "sc-chain-spec", "sc-cli", @@ -3903,7 +3903,7 @@ dependencies = [ "proc-macro-crate 3.0.0", "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -4054,6 +4054,7 @@ version = "0.2.0" dependencies = [ "cumulus-primitives-core", "cumulus-primitives-proof-size-hostfunction", + "docify", "frame-support", "frame-system", "log", @@ -4303,7 +4304,7 @@ name = "cumulus-test-service" version = "0.1.0" dependencies = [ "async-trait", - "clap 4.4.18", + "clap 4.5.1", "criterion 0.5.1", "cumulus-client-cli", "cumulus-client-consensus-common", @@ -4426,7 +4427,7 @@ checksum = "83fdaf97f4804dcebfa5862639bc9ce4121e82140bec2a987ac5140294865b5b" dependencies = [ "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -4466,7 +4467,7 @@ dependencies = [ "proc-macro2", "quote", "scratch", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -4483,7 +4484,7 @@ checksum = "50c49547d73ba8dcfd4ad7325d64c6d5391ff4224d498fc39a6f3f49825a530d" dependencies = [ "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -4691,7 +4692,7 @@ checksum = "487585f4d0c6655fe74905e2504d8ad6908e4db67f744eb140876906c2f3175d" dependencies = [ "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -4752,7 +4753,7 @@ dependencies = [ "proc-macro2", "quote", "regex", - "syn 2.0.48", + "syn 2.0.49", "termcolor", "toml 0.8.8", "walkdir", @@ -4977,7 +4978,7 @@ checksum = "5e9a1f9f7d83e59740248a6e14ecf93929ade55027844dfcea78beafccc15745" dependencies = [ "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -4988,7 +4989,7 @@ checksum = "c2ad8cef1d801a4686bfd8919f0b30eac4c8e48968c437a6405ded4fb5272d2b" dependencies = [ "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -5178,7 +5179,7 @@ dependencies = [ "fs-err", "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -5505,7 +5506,7 @@ dependencies = [ "Inflector", "array-bytes 6.1.0", "chrono", - "clap 4.4.18", + "clap 4.5.1", "comfy-table", "frame-benchmarking", "frame-support", @@ -5571,7 +5572,7 @@ dependencies = [ "quote", "scale-info", "sp-arithmetic", - "syn 2.0.48", + "syn 2.0.49", "trybuild", ] @@ -5597,7 +5598,7 @@ dependencies = [ name = "frame-election-solution-type-fuzzer" version = "2.0.0-alpha.5" dependencies = [ - "clap 4.4.18", + "clap 4.5.1", "frame-election-provider-solution-type", "frame-election-provider-support", "frame-support", @@ -5726,7 +5727,7 @@ dependencies = [ "quote", "regex", "sp-crypto-hashing", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -5737,7 +5738,7 @@ dependencies = [ "proc-macro-crate 3.0.0", "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -5746,7 +5747,7 @@ version = "11.0.0" dependencies = [ "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -5979,7 +5980,7 @@ checksum = "87750cf4b7a4c0625b1529e4c543c2182106e4dedc60a2a6455e00d212c489ac" dependencies = [ "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -7893,7 +7894,7 @@ dependencies = [ "macro_magic_core", "macro_magic_macros", "quote", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -7907,7 +7908,7 @@ dependencies = [ "macro_magic_core_macros", "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -7918,7 +7919,7 @@ checksum = "9ea73aa640dc01d62a590d48c0c3521ed739d53b27f919b25c3551e233481654" dependencies = [ "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -7929,7 +7930,7 @@ checksum = "ef9d79ae96aaba821963320eb2b6e34d17df1e5a83d8a1985c29cc5be59577b3" dependencies = [ "macro_magic_core", "quote", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -8098,7 +8099,7 @@ checksum = "68354c5c6bd36d73ff3feceb05efa59b6acb7626617f4962be322a825e61f79a" name = "minimal-node" version = "4.0.0-dev" dependencies = [ - "clap 4.4.18", + "clap 4.5.1", "frame", "futures", "futures-timer", @@ -8561,7 +8562,7 @@ name = "node-bench" version = "0.9.0-dev" dependencies = [ "array-bytes 6.1.0", - "clap 4.4.18", + "clap 4.5.1", "derive_more", "fs_extra", "futures", @@ -8638,7 +8639,7 @@ dependencies = [ name = "node-runtime-generate-bags" version = "3.0.0" dependencies = [ - "clap 4.4.18", + "clap 4.5.1", "generate-bags", "kitchensink-runtime", ] @@ -8647,7 +8648,7 @@ dependencies = [ name = "node-template" version = "4.0.0-dev" dependencies = [ - "clap 4.4.18", + "clap 4.5.1", "frame-benchmarking", "frame-benchmarking-cli", "frame-system", @@ -8691,7 +8692,7 @@ dependencies = [ name = "node-template-release" version = "3.0.0" dependencies = [ - "clap 4.4.18", + "clap 4.5.1", "flate2", "fs_extra", "glob", @@ -9674,7 +9675,7 @@ version = "18.0.0" dependencies = [ "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -10849,7 +10850,7 @@ dependencies = [ "proc-macro2", "quote", "sp-runtime", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -11255,7 +11256,7 @@ dependencies = [ name = "parachain-template-node" version = "0.1.0" dependencies = [ - "clap 4.4.18", + "clap 4.5.1", "color-print", "cumulus-client-cli", "cumulus-client-collator", @@ -11930,7 +11931,7 @@ dependencies = [ "pest_meta", "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -11971,7 +11972,7 @@ checksum = "4359fd9c9171ec6e8c62926d6faaf553a8dc3f64e1507e76da7911b4f6a04405" dependencies = [ "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -12184,7 +12185,7 @@ name = "polkadot-cli" version = "7.0.0" dependencies = [ "cfg-if", - "clap 4.4.18", + "clap 4.5.1", "frame-benchmarking-cli", "futures", "log", @@ -13028,7 +13029,7 @@ dependencies = [ "async-trait", "bridge-hub-rococo-runtime", "bridge-hub-westend-runtime", - "clap 4.4.18", + "clap 4.5.1", "collectives-westend-runtime", "color-print", "contracts-rococo-runtime", @@ -13544,7 +13545,7 @@ dependencies = [ "async-trait", "bincode", "bitvec", - "clap 4.4.18", + "clap 4.5.1", "clap-num", "color-eyre", "colored", @@ -13638,7 +13639,7 @@ version = "1.0.0" dependencies = [ "assert_matches", "async-trait", - "clap 4.4.18", + "clap 4.5.1", "color-eyre", "futures", "futures-timer", @@ -13785,7 +13786,7 @@ dependencies = [ name = "polkadot-voter-bags" version = "7.0.0" dependencies = [ - "clap 4.4.18", + "clap 4.5.1", "generate-bags", "sp-io", "westend-runtime", @@ -13810,7 +13811,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6380dbe1fb03ecc74ad55d841cfc75480222d153ba69ddcb00977866cbdabdb8" dependencies = [ "polkavm-derive-impl 0.5.0", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -13831,7 +13832,7 @@ dependencies = [ "polkavm-common 0.5.0", "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -13843,7 +13844,7 @@ dependencies = [ "polkavm-common 0.8.0", "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -13853,7 +13854,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "15e85319a0d5129dc9f021c62607e0804f5fb777a05cdda44d750ac0732def66" dependencies = [ "polkavm-derive-impl 0.8.0", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -13950,9 +13951,9 @@ dependencies = [ [[package]] name = "portable-atomic" -version = "1.4.2" +version = "1.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f32154ba0af3a075eefa1eda8bb414ee928f62303a54ea85b8d6638ff1a6ee9e" +checksum = "7170ef9988bc169ba16dd36a7fa041e5c4cbeb6a35b76d4c03daded371eae7c0" [[package]] name = "portpicker" @@ -14058,7 +14059,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6c64d9ba0963cdcea2e1b2230fbae2bab30eb25a174be395c41e764bfb65dd62" dependencies = [ "proc-macro2", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -14149,7 +14150,7 @@ checksum = "9b698b0b09d40e9b7c1a47b132d66a8b54bcd20583d9b6d06e4535e383b4405c" dependencies = [ "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -14221,7 +14222,7 @@ checksum = "440f724eba9f6996b75d63681b0a92b06947f1457076d503a4d2e2c8f56442b8" dependencies = [ "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -14321,7 +14322,7 @@ dependencies = [ "itertools 0.11.0", "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -14639,7 +14640,7 @@ checksum = "7f7473c2cfcf90008193dd0e3e16599455cb601a9fce322b5bb55de799664925" dependencies = [ "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -14721,7 +14722,7 @@ checksum = "c08c74e62047bb2de4ff487b251e4a92e24f48745648451635cec7d591162d9f" name = "remote-ext-tests-bags-list" version = "1.0.0" dependencies = [ - "clap 4.4.18", + "clap 4.5.1", "frame-system", "log", "pallet-bags-list-remote-tests", @@ -15549,7 +15550,7 @@ dependencies = [ "proc-macro-crate 3.0.0", "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -15559,7 +15560,7 @@ dependencies = [ "array-bytes 6.1.0", "bip39", "chrono", - "clap 4.4.18", + "clap 4.5.1", "fdlimit", "futures", "futures-timer", @@ -16716,7 +16717,7 @@ dependencies = [ name = "sc-storage-monitor" version = "0.16.0" dependencies = [ - "clap 4.4.18", + "clap 4.5.1", "fs4", "log", "sp-core", @@ -16818,7 +16819,7 @@ dependencies = [ "proc-macro-crate 3.0.0", "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -17202,9 +17203,9 @@ checksum = "f97841a747eef040fcd2e7b3b9a220a7205926e60488e673d9e4926d27772ce5" [[package]] name = "serde" -version = "1.0.195" +version = "1.0.196" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "63261df402c67811e9ac6def069e4786148c4563f4b50fd4bf30aa370d626b02" +checksum = "870026e60fa08c69f064aa766c10f10b1d62db9ccd4d0abb206472bee0ce3b32" dependencies = [ "serde_derive", ] @@ -17229,13 +17230,13 @@ dependencies = [ [[package]] name = "serde_derive" -version = "1.0.195" +version = "1.0.196" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "46fe8f8603d81ba86327b23a2e9cdf49e1255fb94a4c5f297f6ee0547178ea2c" +checksum = "33c85360c95e7d137454dc81d9a4ed2b8efd8fbe19cee57357b32b9771fccb67" dependencies = [ "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -17260,9 +17261,9 @@ dependencies = [ [[package]] name = "serde_json" -version = "1.0.111" +version = "1.0.113" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "176e46fa42316f18edd598015a5166857fc835ec732f5215eac6b7bdbf0a84f4" +checksum = "69801b70b1c3dac963ecb03a364ba0ceda9cf60c71cfe475e99864759c8b8a79" dependencies = [ "itoa", "ryu", @@ -17325,7 +17326,7 @@ checksum = "91d129178576168c589c9ec973feedf7d3126c01ac2bf08795109aa35b69fb8f" dependencies = [ "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -18164,7 +18165,7 @@ dependencies = [ "proc-macro-crate 3.0.0", "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -18557,7 +18558,7 @@ version = "0.0.0" dependencies = [ "quote", "sp-crypto-hashing", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -18575,7 +18576,7 @@ source = "git+https://github.com/paritytech/polkadot-sdk#82912acb33a9030c0ef3bf5 dependencies = [ "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -18584,7 +18585,7 @@ version = "14.0.0" dependencies = [ "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -18744,7 +18745,7 @@ dependencies = [ name = "sp-npos-elections-fuzzer" version = "2.0.0-alpha.5" dependencies = [ - "clap 4.4.18", + "clap 4.5.1", "honggfuzz", "rand", "sp-npos-elections", @@ -18859,7 +18860,7 @@ dependencies = [ "proc-macro-crate 1.3.1", "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -18871,7 +18872,7 @@ dependencies = [ "proc-macro-crate 3.0.0", "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -19143,7 +19144,7 @@ dependencies = [ "proc-macro2", "quote", "sp-version", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -19267,7 +19268,7 @@ checksum = "a8f112729512f8e442d81f95a8a7ddf2b7c6b8a1a6f509a95864142b30cab2d3" name = "staging-chain-spec-builder" version = "2.0.0" dependencies = [ - "clap 4.4.18", + "clap 4.5.1", "log", "sc-chain-spec", "serde_json", @@ -19280,7 +19281,7 @@ version = "3.0.0-dev" dependencies = [ "array-bytes 6.1.0", "assert_cmd", - "clap 4.4.18", + "clap 4.5.1", "clap_complete", "criterion 0.4.0", "frame-benchmarking", @@ -19390,7 +19391,7 @@ dependencies = [ name = "staging-node-inspect" version = "0.12.0" dependencies = [ - "clap 4.4.18", + "clap 4.5.1", "parity-scale-codec", "sc-cli", "sc-client-api", @@ -19544,6 +19545,12 @@ version = "0.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "73473c0e59e6d5812c5dfe2a064a6444949f089e20eec9a2e5506596494e4623" +[[package]] +name = "strsim" +version = "0.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5ee073c9e4cd00e28217186dbe12796d692868f432bf2e97ee73bed0c56dfa01" + [[package]] name = "strum" version = "0.24.1" @@ -19582,14 +19589,14 @@ dependencies = [ "proc-macro2", "quote", "rustversion", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] name = "subkey" version = "9.0.0" dependencies = [ - "clap 4.4.18", + "clap 4.5.1", "sc-cli", ] @@ -19631,7 +19638,7 @@ dependencies = [ name = "substrate-frame-cli" version = "32.0.0" dependencies = [ - "clap 4.4.18", + "clap 4.5.1", "frame-support", "frame-system", "sc-cli", @@ -19980,9 +19987,9 @@ dependencies = [ [[package]] name = "syn" -version = "2.0.48" +version = "2.0.49" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0f3531638e407dfc0814761abb7c00a5b54992b849452a0646b7f65c9f770f3f" +checksum = "915aea9e586f80826ee59f8453c1101f9d1c4b3964cd2460185ee8e299ada496" dependencies = [ "proc-macro2", "quote", @@ -19998,7 +20005,7 @@ dependencies = [ "paste", "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -20112,7 +20119,7 @@ dependencies = [ name = "test-parachain-adder-collator" version = "1.0.0" dependencies = [ - "clap 4.4.18", + "clap 4.5.1", "futures", "futures-timer", "log", @@ -20160,7 +20167,7 @@ dependencies = [ name = "test-parachain-undying-collator" version = "1.0.0" dependencies = [ - "clap 4.4.18", + "clap 4.5.1", "futures", "futures-timer", "log", @@ -20263,7 +20270,7 @@ checksum = "266b2e40bc00e5a6c09c3584011e08b06f123c00362c92b975ba9843aaaa14b8" dependencies = [ "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -20424,7 +20431,7 @@ checksum = "630bdcf245f78637c13ec01ffae6187cca34625e8c63150d424b59e55af2675e" dependencies = [ "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -20631,7 +20638,7 @@ checksum = "5f4f31f56159e98206da9efd823404b79b6ef3143b4a7ab76e67b1751b25a4ab" dependencies = [ "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -20673,7 +20680,7 @@ dependencies = [ "proc-macro-crate 3.0.0", "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -20855,7 +20862,7 @@ version = "0.38.0" dependencies = [ "assert_cmd", "async-trait", - "clap 4.4.18", + "clap 4.5.1", "frame-remote-externalities", "frame-try-runtime", "hex", @@ -21238,7 +21245,7 @@ dependencies = [ "once_cell", "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.49", "wasm-bindgen-shared", ] @@ -21272,7 +21279,7 @@ checksum = "54681b18a46765f095758388f2d0cf16eb8d4169b639ab575a8f5693af210c7b" dependencies = [ "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.49", "wasm-bindgen-backend", "wasm-bindgen-shared", ] @@ -22287,7 +22294,7 @@ dependencies = [ "proc-macro2", "quote", "staging-xcm", - "syn 2.0.48", + "syn 2.0.49", "trybuild", ] @@ -22409,7 +22416,7 @@ checksum = "9ce1b18ccd8e73a9321186f97e46f9f04b778851177567b1975109d26a08d2a6" dependencies = [ "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] @@ -22429,7 +22436,7 @@ checksum = "ce36e65b0d2999d2aafac989fb249189a141aee1f53c612c1f37d72631959f69" dependencies = [ "proc-macro2", "quote", - "syn 2.0.48", + "syn 2.0.49", ] [[package]] diff --git a/cumulus/pallets/parachain-system/src/validate_block/implementation.rs b/cumulus/pallets/parachain-system/src/validate_block/implementation.rs index 7374ce126c22..6b212d814bc4 100644 --- a/cumulus/pallets/parachain-system/src/validate_block/implementation.rs +++ b/cumulus/pallets/parachain-system/src/validate_block/implementation.rs @@ -282,6 +282,7 @@ fn run_with_externalities_and_recorder R>( ) -> R { let mut overlay = sp_state_machine::OverlayedChanges::default(); let mut ext = Ext::::new(&mut overlay, backend); + recorder.reset(); recorder::using(recorder, || set_and_run_with_externalities(&mut ext, || execute())) } diff --git a/cumulus/primitives/storage-weight-reclaim/Cargo.toml b/cumulus/primitives/storage-weight-reclaim/Cargo.toml index bc223104e734..96e2a2647e0a 100644 --- a/cumulus/primitives/storage-weight-reclaim/Cargo.toml +++ b/cumulus/primitives/storage-weight-reclaim/Cargo.toml @@ -22,6 +22,7 @@ sp-std = { path = "../../../substrate/primitives/std", default-features = false cumulus-primitives-core = { path = "../../primitives/core", default-features = false } cumulus-primitives-proof-size-hostfunction = { path = "../../primitives/proof-size-hostfunction", default-features = false } +docify = "0.2.7" [dev-dependencies] sp-trie = { path = "../../../substrate/primitives/trie", default-features = false } diff --git a/cumulus/primitives/storage-weight-reclaim/src/lib.rs b/cumulus/primitives/storage-weight-reclaim/src/lib.rs index 2d506d977a47..bb0078cdcfbe 100644 --- a/cumulus/primitives/storage-weight-reclaim/src/lib.rs +++ b/cumulus/primitives/storage-weight-reclaim/src/lib.rs @@ -41,19 +41,7 @@ const LOG_TARGET: &'static str = "runtime::storage_reclaim"; /// reclaim it computes the real consumed storage weight and refunds excess weight. /// /// # Example -/// -/// ```ignore -/// use parachains_common::storage_weight_reclaim::StorageWeightReclaimer; -/// -/// let mut remaining_weight_meter = WeightMeter::with_limit(Weight::from_parts(10, 10)); -/// let mut reclaim_helper = StorageWeightReclaimer::start(&remaining_weight_meter); -/// remaining_weight_meter.try_consume(get_weight_for_work()).is_ok() { -/// do_work(); -/// if let Some(relaimed_weight) = reclaim_helper.reclaim_with_meter(&mut remaining_weight_meter) { -/// log::info!("Reclaimed {} weight", reclaimed_weight); -/// } -/// } -/// ``` +#[doc = docify::embed!("src/lib.rs", simple_reclaimer_example)] pub struct StorageWeightReclaimer { previous_remaining_proof_size: u64, previous_reported_proof_size: Option, @@ -479,26 +467,50 @@ mod tests { }); } - #[test] - fn test_reclaim_helper_works_with_meter() { - let mut test_ext = setup_test_externalities(&[10, 12]); + /// Just here for doc purposes + fn get_benched_weight() -> Weight { + Weight::from_parts(0, 5) + } - test_ext.execute_with(|| { - let mut remaining_weight_meter = WeightMeter::with_limit(Weight::from_parts(10, 10)); + /// Just here for doc purposes + fn do_work() {} - set_current_storage_weight(10); - let mut reclaim_helper = StorageWeightReclaimer::new(&remaining_weight_meter); + #[docify::export_content(simple_reclaimer_example)] + fn reclaim_with_weight_meter() { + let mut remaining_weight_meter = WeightMeter::with_limit(Weight::from_parts(10, 10)); + + let benched_weight = get_benched_weight(); + + // It is important to instantiate the `StorageWeightReclaimer` before we consume the weight + // for a piece of work from the weight meter. + let mut reclaim_helper = StorageWeightReclaimer::new(&remaining_weight_meter); - // Substract benchmarked weight - remaining_weight_meter.consume(Weight::from_parts(0, 5)); + if remaining_weight_meter.try_consume(benched_weight).is_ok() { + // Perform some work that takes has `benched_weight` storage weight. + do_work(); + + // Reclaimer will detect that we only consumed 2 bytes, so 3 bytes are reclaimed. let reclaimed = reclaim_helper.reclaim_with_meter(&mut remaining_weight_meter); + // We reclaimed 3 bytes of storage size! assert_eq!(reclaimed, Some(Weight::from_parts(0, 3))); assert_eq!( BlockWeight::::get().total(), Weight::from_parts(base_block_weight().ref_time(), 10) ); assert_eq!(remaining_weight_meter.remaining(), Weight::from_parts(10, 8)); + } + } + + #[test] + fn test_reclaim_helper_works_with_meter() { + // The node will report 12 - 10 = 2 consumed storage size between the calls. + let mut test_ext = setup_test_externalities(&[10, 12]); + + test_ext.execute_with(|| { + // Initial storage size is 10. + set_current_storage_weight(10); + reclaim_with_weight_meter(); }); } } From 0f44605b4b2ed634410eaa7eafc6ee40369208d0 Mon Sep 17 00:00:00 2001 From: Sebastian Kunert Date: Mon, 19 Feb 2024 18:52:23 +0100 Subject: [PATCH 16/26] deps --- Cargo.lock | 2 -- cumulus/parachains/common/Cargo.toml | 4 ---- 2 files changed, 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8ff5ff44c676..b33cb40edc0c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11421,9 +11421,7 @@ dependencies = [ "pallet-collator-selection", "pallet-message-queue", "pallet-xcm", - "parity-scale-codec", "polkadot-primitives", - "scale-info", "sp-consensus-aura", "sp-core", "sp-io", diff --git a/cumulus/parachains/common/Cargo.toml b/cumulus/parachains/common/Cargo.toml index ebc9f822beb2..e26f421c08f3 100644 --- a/cumulus/parachains/common/Cargo.toml +++ b/cumulus/parachains/common/Cargo.toml @@ -13,9 +13,7 @@ workspace = true targets = ["x86_64-unknown-linux-gnu"] [dependencies] -codec = { package = "parity-scale-codec", version = "3.0.0", features = ["derive"], default-features = false } log = { workspace = true } -scale-info = { version = "2.10.0", default-features = false, features = ["derive"] } # Substrate frame-support = { path = "../../../substrate/frame/support", default-features = false } @@ -53,7 +51,6 @@ substrate-wasm-builder = { path = "../../../substrate/utils/wasm-builder" } [features] default = ["std"] std = [ - "codec/std", "cumulus-primitives-core/std", "cumulus-primitives-utility/std", "frame-support/std", @@ -68,7 +65,6 @@ std = [ "pallet-xcm/std", "parachain-info/std", "polkadot-primitives/std", - "scale-info/std", "sp-consensus-aura/std", "sp-core/std", "sp-io/std", From b308598903c845c6c984b89e6fa1ba7c0c03a052 Mon Sep 17 00:00:00 2001 From: Sebastian Kunert Date: Tue, 20 Feb 2024 10:11:35 +0100 Subject: [PATCH 17/26] Revert "deps" This reverts commit 0f44605b4b2ed634410eaa7eafc6ee40369208d0. --- Cargo.lock | 2 ++ cumulus/parachains/common/Cargo.toml | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index b33cb40edc0c..8ff5ff44c676 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11421,7 +11421,9 @@ dependencies = [ "pallet-collator-selection", "pallet-message-queue", "pallet-xcm", + "parity-scale-codec", "polkadot-primitives", + "scale-info", "sp-consensus-aura", "sp-core", "sp-io", diff --git a/cumulus/parachains/common/Cargo.toml b/cumulus/parachains/common/Cargo.toml index e26f421c08f3..ebc9f822beb2 100644 --- a/cumulus/parachains/common/Cargo.toml +++ b/cumulus/parachains/common/Cargo.toml @@ -13,7 +13,9 @@ workspace = true targets = ["x86_64-unknown-linux-gnu"] [dependencies] +codec = { package = "parity-scale-codec", version = "3.0.0", features = ["derive"], default-features = false } log = { workspace = true } +scale-info = { version = "2.10.0", default-features = false, features = ["derive"] } # Substrate frame-support = { path = "../../../substrate/frame/support", default-features = false } @@ -51,6 +53,7 @@ substrate-wasm-builder = { path = "../../../substrate/utils/wasm-builder" } [features] default = ["std"] std = [ + "codec/std", "cumulus-primitives-core/std", "cumulus-primitives-utility/std", "frame-support/std", @@ -65,6 +68,7 @@ std = [ "pallet-xcm/std", "parachain-info/std", "polkadot-primitives/std", + "scale-info/std", "sp-consensus-aura/std", "sp-core/std", "sp-io/std", From 872f17053e5b2c0caabc5cdc04e5056e1fe51a77 Mon Sep 17 00:00:00 2001 From: Sebastian Kunert Date: Tue, 20 Feb 2024 12:04:08 +0100 Subject: [PATCH 18/26] Add small test for reset --- .../parachain-system/src/validate_block/trie_recorder.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cumulus/pallets/parachain-system/src/validate_block/trie_recorder.rs b/cumulus/pallets/parachain-system/src/validate_block/trie_recorder.rs index 7d1805033252..48310670c074 100644 --- a/cumulus/pallets/parachain-system/src/validate_block/trie_recorder.rs +++ b/cumulus/pallets/parachain-system/src/validate_block/trie_recorder.rs @@ -289,6 +289,9 @@ mod tests { reference_recorder.estimate_encoded_size(), recorder_for_test.estimate_encoded_size() ); + + recorder_for_test.reset(); + assert_eq!(recorder_for_test.estimate_encoded_size(), 0) } } } From 5386108e2a5cdd51476a25f28cf8f2c89e3b443b Mon Sep 17 00:00:00 2001 From: Sebastian Kunert Date: Tue, 20 Feb 2024 15:51:45 +0100 Subject: [PATCH 19/26] Comments --- .../src/validate_block/implementation.rs | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/cumulus/pallets/parachain-system/src/validate_block/implementation.rs b/cumulus/pallets/parachain-system/src/validate_block/implementation.rs index 6b212d814bc4..e8bd84cdcf4b 100644 --- a/cumulus/pallets/parachain-system/src/validate_block/implementation.rs +++ b/cumulus/pallets/parachain-system/src/validate_block/implementation.rs @@ -34,7 +34,7 @@ use sp_externalities::{set_and_run_with_externalities, Externalities}; use sp_io::KillStorageResult; use sp_runtime::traits::{Block as BlockT, Extrinsic, HashingFor, Header as HeaderT}; use sp_std::prelude::*; -use sp_trie::{MemoryDB, ProofSizeProvider, TrieRecorderProvider}; +use sp_trie::{MemoryDB, ProofSizeProvider}; use trie_recorder::SizeOnlyRecorderProvider; type TrieBackend = sp_state_machine::TrieBackend< @@ -50,7 +50,7 @@ fn with_externalities R, R>(f: F) -> R { sp_externalities::with_externalities(f).expect("Environmental externalities not set.") } -/// Recorder instance to be used during this validate_block call. +// Recorder instance to be used during this validate_block call. environmental::environmental!(recorder: trait ProofSizeProvider); /// Validate the given parachain block. @@ -178,7 +178,7 @@ where .replace_implementation(host_storage_proof_size), ); - run_with_externalities::(&backend, || { + run_with_externalities_and_recorder::(&backend, &mut recorder, || { let relay_chain_proof = crate::RelayChainStateProof::new( PSC::SelfParaId::get(), inherent_data.validation_data.relay_parent_storage_root, @@ -274,7 +274,7 @@ fn validate_validation_data( ); } -/// Run the given closure with the externalities and recorder set. +/// Run the given closure with the externalities set. fn run_with_externalities_and_recorder R>( backend: &TrieBackend, recorder: &mut SizeOnlyRecorderProvider>, @@ -287,17 +287,6 @@ fn run_with_externalities_and_recorder R>( recorder::using(recorder, || set_and_run_with_externalities(&mut ext, || execute())) } -/// Run the given closure with the externalities set. -fn run_with_externalities R>( - backend: &TrieBackend, - execute: F, -) -> R { - let mut overlay = sp_state_machine::OverlayedChanges::default(); - let mut ext = Ext::::new(&mut overlay, backend); - - set_and_run_with_externalities(&mut ext, || execute()) -} - fn host_storage_read(key: &[u8], value_out: &mut [u8], value_offset: u32) -> Option { match with_externalities(|ext| ext.storage(key)) { Some(value) => { From 6af533b264fc8997f54203607c290db83de36647 Mon Sep 17 00:00:00 2001 From: Sebastian Kunert Date: Wed, 21 Feb 2024 12:12:46 +0100 Subject: [PATCH 20/26] Incorporate unspent value and add more tests --- .../storage-weight-reclaim/src/lib.rs | 220 +++++++++++++++--- 1 file changed, 192 insertions(+), 28 deletions(-) diff --git a/cumulus/primitives/storage-weight-reclaim/src/lib.rs b/cumulus/primitives/storage-weight-reclaim/src/lib.rs index bb0078cdcfbe..c00c177d3468 100644 --- a/cumulus/primitives/storage-weight-reclaim/src/lib.rs +++ b/cumulus/primitives/storage-weight-reclaim/src/lib.rs @@ -141,7 +141,7 @@ where fn post_dispatch( pre: Option, info: &DispatchInfoOf, - _post_info: &PostDispatchInfoOf, + post_info: &PostDispatchInfoOf, _len: usize, _result: &DispatchResult, ) -> Result<(), TransactionValidityError> { @@ -159,18 +159,26 @@ where let benchmarked_weight = info.weight.proof_size(); let consumed_weight = post_dispatch_proof_size.saturating_sub(pre_dispatch_proof_size); - let storage_size_diff = benchmarked_weight.abs_diff(consumed_weight as u64); + // Unspent weight according to the `actual_weight` from `PostDispatchInfo` + // This unspent weight will be refunded by the `CheckWeight` extension, so we need to + // account for that. + let unspent = post_info.calc_unspent(info).proof_size(); + let storage_size_diff = + benchmarked_weight.saturating_sub(unspent).abs_diff(consumed_weight as u64); + + // This value will be reclaimed by [`frame_system::CheckWeight`], so we need to calculate + // that in. frame_system::BlockWeight::::mutate(|current| { if consumed_weight > benchmarked_weight { log::error!( target: LOG_TARGET, - "Benchmarked storage weight smaller than consumed storage weight. benchmarked: {benchmarked_weight} consumed: {consumed_weight}" + "Benchmarked storage weight smaller than consumed storage weight. benchmarked: {benchmarked_weight} consumed: {consumed_weight} unspent: {unspent}" ); current.accrue(Weight::from_parts(0, storage_size_diff), info.class) } else { log::trace!( target: LOG_TARGET, - "Reclaiming storage weight. benchmarked: {benchmarked_weight}, consumed: {consumed_weight}" + "Reclaiming storage weight. benchmarked: {benchmarked_weight}, consumed: {consumed_weight} unspent: {unspent}" ); current.reduce(Weight::from_parts(0, storage_size_diff), info.class) } @@ -189,7 +197,7 @@ mod tests { }; use frame_system::{ mock::{new_test_ext, Test, CALL}, - BlockWeight, + BlockWeight, CheckWeight, }; use sp_std::marker::PhantomData; use sp_trie::proof_size_extension::ProofSizeExt; @@ -231,24 +239,23 @@ mod tests { #[test] fn basic_refund() { - let mut test_ext = setup_test_externalities(&[100, 200]); + // The real cost will be 100 bytes of storage size + let mut test_ext = setup_test_externalities(&[0, 100]); test_ext.execute_with(|| { + set_current_storage_weight(1000); + // Benchmarked storage weight: 500 let info = DispatchInfo { weight: Weight::from_parts(0, 500), ..Default::default() }; - let post_info = PostDispatchInfo { - actual_weight: Some(Weight::zero()), - pays_fee: Default::default(), - }; - - set_current_storage_weight(1000); + let post_info = PostDispatchInfo::default(); let len = 0_usize; let pre = StorageWeightReclaim::(PhantomData) .pre_dispatch(&1, CALL, &info, len) .unwrap(); - assert_eq!(pre, Some(100)); + assert_eq!(pre, Some(0)); + assert_ok!(CheckWeight::::post_dispatch(None, &info, &post_info, 0, &Ok(()))); // We expect a refund of 400 assert_ok!(StorageWeightReclaim::::post_dispatch( Some(pre), @@ -271,14 +278,11 @@ mod tests { // Proof size extension not registered test_ext.execute_with(|| { + set_current_storage_weight(1000); + // Benchmarked storage weight: 500 let info = DispatchInfo { weight: Weight::from_parts(0, 500), ..Default::default() }; - let post_info = PostDispatchInfo { - actual_weight: Some(Weight::zero()), - pays_fee: Default::default(), - }; - - set_current_storage_weight(1000); + let post_info = PostDispatchInfo::default(); let len = 0_usize; let pre = StorageWeightReclaim::(PhantomData) @@ -286,6 +290,7 @@ mod tests { .unwrap(); assert_eq!(pre, None); + assert_ok!(CheckWeight::::post_dispatch(None, &info, &post_info, 0, &Ok(()))); assert_ok!(StorageWeightReclaim::::post_dispatch( Some(pre), &info, @@ -306,14 +311,10 @@ mod tests { let mut test_ext = setup_test_externalities(&[100, 300]); test_ext.execute_with(|| { + set_current_storage_weight(1000); // Benchmarked storage weight: 100 let info = DispatchInfo { weight: Weight::from_parts(0, 100), ..Default::default() }; - let post_info = PostDispatchInfo { - actual_weight: Some(Weight::zero()), - pays_fee: Default::default(), - }; - - set_current_storage_weight(1000); + let post_info = PostDispatchInfo::default(); let len = 0_usize; let pre = StorageWeightReclaim::(PhantomData) @@ -322,6 +323,7 @@ mod tests { assert_eq!(pre, Some(100)); // We expect no refund + assert_ok!(CheckWeight::::post_dispatch(None, &info, &post_info, 0, &Ok(()))); assert_ok!(StorageWeightReclaim::::post_dispatch( Some(pre), &info, @@ -351,6 +353,7 @@ mod tests { .unwrap(); assert_eq!(pre, Some(0)); + assert_ok!(CheckWeight::::post_dispatch(None, &info, &post_info, 0, &Ok(()))); assert_ok!(StorageWeightReclaim::::post_dispatch( Some(pre), &info, @@ -368,17 +371,18 @@ mod tests { let mut test_ext = setup_test_externalities(&[300, 100]); test_ext.execute_with(|| { + set_current_storage_weight(1300); + let info = DispatchInfo { weight: Weight::from_parts(0, 500), ..Default::default() }; let post_info = PostDispatchInfo::default(); - set_current_storage_weight(1313); - let len = 0_usize; let pre = StorageWeightReclaim::(PhantomData) .pre_dispatch(&1, CALL, &info, len) .unwrap(); assert_eq!(pre, Some(300)); + assert_ok!(CheckWeight::::post_dispatch(None, &info, &post_info, 0, &Ok(()))); assert_ok!(StorageWeightReclaim::::post_dispatch( Some(pre), &info, @@ -389,11 +393,171 @@ mod tests { assert_eq!( BlockWeight::::get().total(), - Weight::from_parts(base_block_weight().ref_time(), 813) + Weight::from_parts(base_block_weight().ref_time(), 800) ); }); } + #[test] + fn test_incorporates_check_weight_unspent_weight() { + let mut test_ext = setup_test_externalities(&[100, 300]); + + test_ext.execute_with(|| { + set_current_storage_weight(1000); + + // Benchmarked storage weight: 300 + let info = DispatchInfo { weight: Weight::from_parts(100, 300), ..Default::default() }; + + // Actual weight is 50 + let post_info = PostDispatchInfo { + actual_weight: Some(Weight::from_parts(50, 250)), + pays_fee: Default::default(), + }; + + let len = 0_usize; + let pre = StorageWeightReclaim::(PhantomData) + .pre_dispatch(&1, CALL, &info, len) + .unwrap(); + assert_eq!(pre, Some(100)); + + // The `CheckWeight` extension will refunt `actual_weight` from `PostDispatchInfo` + // we always need to call `post_dispatch` to verify that they interoperate correctly. + assert_ok!(CheckWeight::::post_dispatch(None, &info, &post_info, 0, &Ok(()))); + assert_ok!(StorageWeightReclaim::::post_dispatch( + Some(pre), + &info, + &post_info, + len, + &Ok(()) + )); + + assert_eq!( + BlockWeight::::get().total(), + Weight::from_parts(base_block_weight().ref_time(), 900) + ); + }) + } + + #[test] + fn test_incorporates_check_weight_unspent_weight_on_negative() { + let mut test_ext = setup_test_externalities(&[100, 300]); + + test_ext.execute_with(|| { + set_current_storage_weight(1000); + // Benchmarked storage weight: 50 + let info = DispatchInfo { weight: Weight::from_parts(100, 50), ..Default::default() }; + + // Actual weight is 25 + let post_info = PostDispatchInfo { + actual_weight: Some(Weight::from_parts(50, 25)), + pays_fee: Default::default(), + }; + + let len = 0_usize; + let pre = StorageWeightReclaim::(PhantomData) + .pre_dispatch(&1, CALL, &info, len) + .unwrap(); + assert_eq!(pre, Some(100)); + + // The `CheckWeight` extension will refunt `actual_weight` from `PostDispatchInfo` + // we always need to call `post_dispatch` to verify that they interoperate correctly. + assert_ok!(CheckWeight::::post_dispatch(None, &info, &post_info, 0, &Ok(()))); + assert_ok!(StorageWeightReclaim::::post_dispatch( + Some(pre), + &info, + &post_info, + len, + &Ok(()) + )); + + assert_eq!( + BlockWeight::::get().total(), + Weight::from_parts(base_block_weight().ref_time(), 1150) + ); + }) + } + + #[test] + fn test_incorporates_check_weight_unspent_weight_reverse_order() { + let mut test_ext = setup_test_externalities(&[100, 300]); + + test_ext.execute_with(|| { + set_current_storage_weight(1000); + + // Benchmarked storage weight: 300 + let info = DispatchInfo { weight: Weight::from_parts(100, 300), ..Default::default() }; + + // Actual weight is 50 + let post_info = PostDispatchInfo { + actual_weight: Some(Weight::from_parts(50, 250)), + pays_fee: Default::default(), + }; + + let len = 0_usize; + let pre = StorageWeightReclaim::(PhantomData) + .pre_dispatch(&1, CALL, &info, len) + .unwrap(); + assert_eq!(pre, Some(100)); + + assert_ok!(StorageWeightReclaim::::post_dispatch( + Some(pre), + &info, + &post_info, + len, + &Ok(()) + )); + // `CheckWeight` gets called after `StorageWeightReclaim` this time. + // The `CheckWeight` extension will refunt `actual_weight` from `PostDispatchInfo` + // we always need to call `post_dispatch` to verify that they interoperate correctly. + assert_ok!(CheckWeight::::post_dispatch(None, &info, &post_info, 0, &Ok(()))); + + assert_eq!( + BlockWeight::::get().total(), + Weight::from_parts(base_block_weight().ref_time(), 900) + ); + }) + } + + #[test] + fn test_incorporates_check_weight_unspent_weight_on_negative_reverse_order() { + let mut test_ext = setup_test_externalities(&[100, 300]); + + test_ext.execute_with(|| { + set_current_storage_weight(1000); + // Benchmarked storage weight: 50 + let info = DispatchInfo { weight: Weight::from_parts(100, 50), ..Default::default() }; + + // Actual weight is 25 + let post_info = PostDispatchInfo { + actual_weight: Some(Weight::from_parts(50, 25)), + pays_fee: Default::default(), + }; + + let len = 0_usize; + let pre = StorageWeightReclaim::(PhantomData) + .pre_dispatch(&1, CALL, &info, len) + .unwrap(); + assert_eq!(pre, Some(100)); + + assert_ok!(StorageWeightReclaim::::post_dispatch( + Some(pre), + &info, + &post_info, + len, + &Ok(()) + )); + // `CheckWeight` gets called after `StorageWeightReclaim` this time. + // The `CheckWeight` extension will refunt `actual_weight` from `PostDispatchInfo` + // we always need to call `post_dispatch` to verify that they interoperate correctly. + assert_ok!(CheckWeight::::post_dispatch(None, &info, &post_info, 0, &Ok(()))); + + assert_eq!( + BlockWeight::::get().total(), + Weight::from_parts(base_block_weight().ref_time(), 1150) + ); + }) + } + #[test] fn storage_size_reported_correctly() { let mut test_ext = setup_test_externalities(&[1000]); From 03074da55119c3c772bf691d5e5bba816ed11fa6 Mon Sep 17 00:00:00 2001 From: Sebastian Kunert Date: Wed, 21 Feb 2024 15:45:35 +0100 Subject: [PATCH 21/26] Format features & small fixes --- Cargo.lock | 1 + .../src/validate_block/implementation.rs | 2 +- .../primitives/storage-weight-reclaim/Cargo.toml | 14 +++++++++++++- cumulus/test/runtime/Cargo.toml | 2 ++ cumulus/test/runtime/src/lib.rs | 1 + 5 files changed, 18 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1e77c13efd9f..cb5aef55135a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4269,6 +4269,7 @@ version = "0.1.0" dependencies = [ "cumulus-pallet-parachain-system", "cumulus-primitives-core", + "cumulus-primitives-storage-weight-reclaim", "frame-executive", "frame-support", "frame-system", diff --git a/cumulus/pallets/parachain-system/src/validate_block/implementation.rs b/cumulus/pallets/parachain-system/src/validate_block/implementation.rs index e8bd84cdcf4b..ecab7a9a0931 100644 --- a/cumulus/pallets/parachain-system/src/validate_block/implementation.rs +++ b/cumulus/pallets/parachain-system/src/validate_block/implementation.rs @@ -274,7 +274,7 @@ fn validate_validation_data( ); } -/// Run the given closure with the externalities set. +/// Run the given closure with the externalities and recorder set. fn run_with_externalities_and_recorder R>( backend: &TrieBackend, recorder: &mut SizeOnlyRecorderProvider>, diff --git a/cumulus/primitives/storage-weight-reclaim/Cargo.toml b/cumulus/primitives/storage-weight-reclaim/Cargo.toml index 96e2a2647e0a..2be91ddee3b0 100644 --- a/cumulus/primitives/storage-weight-reclaim/Cargo.toml +++ b/cumulus/primitives/storage-weight-reclaim/Cargo.toml @@ -30,4 +30,16 @@ sp-io = { path = "../../../substrate/primitives/io", default-features = false } [features] default = ["std"] -std = ["codec/std", "cumulus-primitives-core/std", "cumulus-primitives-proof-size-hostfunction/std", "frame-support/std", "frame-system/std", "log/std", "scale-info/std", "sp-io/std", "sp-runtime/std", "sp-std/std", "sp-trie/std"] +std = [ + "codec/std", + "cumulus-primitives-core/std", + "cumulus-primitives-proof-size-hostfunction/std", + "frame-support/std", + "frame-system/std", + "log/std", + "scale-info/std", + "sp-io/std", + "sp-runtime/std", + "sp-std/std", + "sp-trie/std", +] diff --git a/cumulus/test/runtime/Cargo.toml b/cumulus/test/runtime/Cargo.toml index 5902a62512be..6ec2d8e162de 100644 --- a/cumulus/test/runtime/Cargo.toml +++ b/cumulus/test/runtime/Cargo.toml @@ -39,6 +39,7 @@ sp-version = { path = "../../../substrate/primitives/version", default-features # Cumulus cumulus-pallet-parachain-system = { path = "../../pallets/parachain-system", default-features = false, features = ["parameterized-consensus-hook"] } cumulus-primitives-core = { path = "../../primitives/core", default-features = false } +cumulus-primitives-storage-weight-reclaim = { path = "../../primitives/storage-weight-reclaim" } [build-dependencies] substrate-wasm-builder = { path = "../../../substrate/utils/wasm-builder", optional = true } @@ -49,6 +50,7 @@ std = [ "codec/std", "cumulus-pallet-parachain-system/std", "cumulus-primitives-core/std", + "cumulus-primitives-storage-weight-reclaim/std", "frame-executive/std", "frame-support/std", "frame-system-rpc-runtime-api/std", diff --git a/cumulus/test/runtime/src/lib.rs b/cumulus/test/runtime/src/lib.rs index 6068f895c83b..5fb314109844 100644 --- a/cumulus/test/runtime/src/lib.rs +++ b/cumulus/test/runtime/src/lib.rs @@ -331,6 +331,7 @@ pub type SignedExtra = ( frame_system::CheckNonce, frame_system::CheckWeight, pallet_transaction_payment::ChargeTransactionPayment, + cumulus_primitives_storage_weight_reclaim::StorageWeightReclaim, ); /// Unchecked extrinsic type as expected by this runtime. pub type UncheckedExtrinsic = From f6bb7967564143287386a9feadb0e0e54c881a4f Mon Sep 17 00:00:00 2001 From: Sebastian Kunert Date: Thu, 22 Feb 2024 09:25:16 +0100 Subject: [PATCH 22/26] Upgrade try-runtime & fix test-service --- .gitlab/pipeline/check.yml | 4 +++- Cargo.lock | 1 + cumulus/test/service/Cargo.toml | 1 + cumulus/test/service/src/lib.rs | 1 + 4 files changed, 6 insertions(+), 1 deletion(-) diff --git a/.gitlab/pipeline/check.yml b/.gitlab/pipeline/check.yml index cdb5d1b05d09..4d71a473372d 100644 --- a/.gitlab/pipeline/check.yml +++ b/.gitlab/pipeline/check.yml @@ -108,8 +108,10 @@ check-toml-format: export RUST_LOG=remote-ext=debug,runtime=debug echo "---------- Downloading try-runtime CLI ----------" - curl -sL https://github.com/paritytech/try-runtime-cli/releases/download/v0.5.0/try-runtime-x86_64-unknown-linux-musl -o try-runtime + curl -sL https://github.com/paritytech/try-runtime-cli/releases/download/v0.5.4/try-runtime-x86_64-unknown-linux-musl -o try-runtime chmod +x ./try-runtime + echo "Using try-runtime-cli version:" + ./try-runtime --version echo "---------- Building ${PACKAGE} runtime ----------" time cargo build --release --locked -p "$PACKAGE" --features try-runtime diff --git a/Cargo.lock b/Cargo.lock index cb5aef55135a..2cbd83aaf6bc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4312,6 +4312,7 @@ dependencies = [ "cumulus-client-service", "cumulus-pallet-parachain-system", "cumulus-primitives-core", + "cumulus-primitives-storage-weight-reclaim", "cumulus-relay-chain-inprocess-interface", "cumulus-relay-chain-interface", "cumulus-relay-chain-minimal-node", diff --git a/cumulus/test/service/Cargo.toml b/cumulus/test/service/Cargo.toml index b26f0b9967cf..27273f4e0a8d 100644 --- a/cumulus/test/service/Cargo.toml +++ b/cumulus/test/service/Cargo.toml @@ -81,6 +81,7 @@ cumulus-relay-chain-minimal-node = { path = "../../client/relay-chain-minimal-no cumulus-client-pov-recovery = { path = "../../client/pov-recovery" } cumulus-test-relay-sproof-builder = { path = "../relay-sproof-builder" } cumulus-pallet-parachain-system = { path = "../../pallets/parachain-system", default-features = false, features = ["parameterized-consensus-hook"] } +cumulus-primitives-storage-weight-reclaim = { path = "../../primitives/storage-weight-reclaim" } pallet-timestamp = { path = "../../../substrate/frame/timestamp" } [dev-dependencies] diff --git a/cumulus/test/service/src/lib.rs b/cumulus/test/service/src/lib.rs index 6247d9a12533..3854f02572c0 100644 --- a/cumulus/test/service/src/lib.rs +++ b/cumulus/test/service/src/lib.rs @@ -894,6 +894,7 @@ pub fn construct_extrinsic( frame_system::CheckNonce::::from(nonce), frame_system::CheckWeight::::new(), pallet_transaction_payment::ChargeTransactionPayment::::from(tip), + cumulus_primitives_storage_weight_reclaim::StorageWeightReclaim::, ); let raw_payload = runtime::SignedPayload::from_raw( function.clone(), From eb79558edeb34d4d4239ab6b450337bba4c45de9 Mon Sep 17 00:00:00 2001 From: Sebastian Kunert Date: Thu, 22 Feb 2024 12:59:45 +0100 Subject: [PATCH 23/26] Test-service once more --- Cargo.lock | 1 + cumulus/primitives/storage-weight-reclaim/src/lib.rs | 7 +++++++ cumulus/test/client/Cargo.toml | 1 + cumulus/test/client/src/lib.rs | 3 ++- cumulus/test/service/src/lib.rs | 4 ++-- 5 files changed, 13 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2cbd83aaf6bc..09d097f8b367 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4225,6 +4225,7 @@ dependencies = [ "cumulus-primitives-core", "cumulus-primitives-parachain-inherent", "cumulus-primitives-proof-size-hostfunction", + "cumulus-primitives-storage-weight-reclaim", "cumulus-test-relay-sproof-builder", "cumulus-test-runtime", "cumulus-test-service", diff --git a/cumulus/primitives/storage-weight-reclaim/src/lib.rs b/cumulus/primitives/storage-weight-reclaim/src/lib.rs index c00c177d3468..eaa77755e47b 100644 --- a/cumulus/primitives/storage-weight-reclaim/src/lib.rs +++ b/cumulus/primitives/storage-weight-reclaim/src/lib.rs @@ -103,6 +103,13 @@ pub fn get_proof_size() -> Option { #[scale_info(skip_type_params(T))] pub struct StorageWeightReclaim(PhantomData); +impl StorageWeightReclaim { + /// Create a new `StorageWeightReclaim` instance. + pub fn new() -> Self { + Self(Default::default()) + } +} + impl core::fmt::Debug for StorageWeightReclaim { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> Result<(), core::fmt::Error> { let _ = write!(f, "StorageWeightReclaim"); diff --git a/cumulus/test/client/Cargo.toml b/cumulus/test/client/Cargo.toml index 7190172101cb..028733ce2355 100644 --- a/cumulus/test/client/Cargo.toml +++ b/cumulus/test/client/Cargo.toml @@ -41,6 +41,7 @@ cumulus-test-relay-sproof-builder = { path = "../relay-sproof-builder" } cumulus-primitives-core = { path = "../../primitives/core" } cumulus-primitives-proof-size-hostfunction = { path = "../../primitives/proof-size-hostfunction" } cumulus-primitives-parachain-inherent = { path = "../../primitives/parachain-inherent" } +cumulus-primitives-storage-weight-reclaim = { path = "../../primitives/storage-weight-reclaim" } [features] runtime-benchmarks = [ diff --git a/cumulus/test/client/src/lib.rs b/cumulus/test/client/src/lib.rs index 0593e7d60060..62481a3bf0b7 100644 --- a/cumulus/test/client/src/lib.rs +++ b/cumulus/test/client/src/lib.rs @@ -151,6 +151,7 @@ pub fn generate_extrinsic_with_pair( frame_system::CheckNonce::::from(nonce), frame_system::CheckWeight::::new(), pallet_transaction_payment::ChargeTransactionPayment::::from(tip), + cumulus_primitives_storage_weight_reclaim::StorageWeightReclaim::::new(), ); let function = function.into(); @@ -158,7 +159,7 @@ pub fn generate_extrinsic_with_pair( let raw_payload = SignedPayload::from_raw( function.clone(), extra.clone(), - ((), VERSION.spec_version, genesis_block, current_block_hash, (), (), ()), + ((), VERSION.spec_version, genesis_block, current_block_hash, (), (), (), ()), ); let signature = raw_payload.using_encoded(|e| origin.sign(e)); diff --git a/cumulus/test/service/src/lib.rs b/cumulus/test/service/src/lib.rs index 3854f02572c0..3554a383f219 100644 --- a/cumulus/test/service/src/lib.rs +++ b/cumulus/test/service/src/lib.rs @@ -894,12 +894,12 @@ pub fn construct_extrinsic( frame_system::CheckNonce::::from(nonce), frame_system::CheckWeight::::new(), pallet_transaction_payment::ChargeTransactionPayment::::from(tip), - cumulus_primitives_storage_weight_reclaim::StorageWeightReclaim::, + cumulus_primitives_storage_weight_reclaim::StorageWeightReclaim::::new(), ); let raw_payload = runtime::SignedPayload::from_raw( function.clone(), extra.clone(), - ((), runtime::VERSION.spec_version, genesis_block, current_block_hash, (), (), ()), + ((), runtime::VERSION.spec_version, genesis_block, current_block_hash, (), (), (), ()), ); let signature = raw_payload.using_encoded(|e| caller.sign(e)); runtime::UncheckedExtrinsic::new_signed( From b8012bee8d762697aed71a6bba37d8c479ca90fb Mon Sep 17 00:00:00 2001 From: Sebastian Kunert Date: Thu, 22 Feb 2024 14:55:19 +0100 Subject: [PATCH 24/26] Fix PrDoc and feature flags --- cumulus/primitives/proof-size-hostfunction/src/lib.rs | 1 + cumulus/primitives/storage-weight-reclaim/src/lib.rs | 2 ++ cumulus/test/runtime/Cargo.toml | 2 +- prdoc/pr_3002.prdoc | 7 +++++-- 4 files changed, 9 insertions(+), 3 deletions(-) diff --git a/cumulus/primitives/proof-size-hostfunction/src/lib.rs b/cumulus/primitives/proof-size-hostfunction/src/lib.rs index b0bf1628e03a..8ebc58ea450d 100644 --- a/cumulus/primitives/proof-size-hostfunction/src/lib.rs +++ b/cumulus/primitives/proof-size-hostfunction/src/lib.rs @@ -18,6 +18,7 @@ #![cfg_attr(not(feature = "std"), no_std)] +#[cfg(feature = "std")] use sp_externalities::ExternalitiesExt; use sp_runtime_interface::runtime_interface; diff --git a/cumulus/primitives/storage-weight-reclaim/src/lib.rs b/cumulus/primitives/storage-weight-reclaim/src/lib.rs index eaa77755e47b..f161f5866f33 100644 --- a/cumulus/primitives/storage-weight-reclaim/src/lib.rs +++ b/cumulus/primitives/storage-weight-reclaim/src/lib.rs @@ -15,6 +15,8 @@ //! Mechanism to reclaim PoV proof size weight after an extrinsic has been applied. +#![cfg_attr(not(feature = "std"), no_std)] + use codec::{Decode, Encode}; use cumulus_primitives_core::Weight; use cumulus_primitives_proof_size_hostfunction::{ diff --git a/cumulus/test/runtime/Cargo.toml b/cumulus/test/runtime/Cargo.toml index 6ec2d8e162de..449a8b819bc0 100644 --- a/cumulus/test/runtime/Cargo.toml +++ b/cumulus/test/runtime/Cargo.toml @@ -39,7 +39,7 @@ sp-version = { path = "../../../substrate/primitives/version", default-features # Cumulus cumulus-pallet-parachain-system = { path = "../../pallets/parachain-system", default-features = false, features = ["parameterized-consensus-hook"] } cumulus-primitives-core = { path = "../../primitives/core", default-features = false } -cumulus-primitives-storage-weight-reclaim = { path = "../../primitives/storage-weight-reclaim" } +cumulus-primitives-storage-weight-reclaim = { path = "../../primitives/storage-weight-reclaim", default-features = false } [build-dependencies] substrate-wasm-builder = { path = "../../../substrate/utils/wasm-builder", optional = true } diff --git a/prdoc/pr_3002.prdoc b/prdoc/pr_3002.prdoc index 3a282668ce25..511a07e39c47 100644 --- a/prdoc/pr_3002.prdoc +++ b/prdoc/pr_3002.prdoc @@ -18,9 +18,12 @@ doc: `StorageWeightReclaim` `SignedExtension` to your runtime and enable proof recording during block import. -crates: [ ] +crates: + - name: "cumulus-primitives-storage-weight-reclaim" host_functions: - - title: `storage_proof_size` + - name: "storage_proof_size" description: | This host function is used to pass the current size of the storage proof to the runtime. It was introduced before but becomes relevant now. + Note: This host function is intended to be used through `cumulus_primitives_storage_weight_reclaim::get_proof_size`. + Direct usage is not recommended. From fdfa2661f19e21d36a1b7e0c094439819d648489 Mon Sep 17 00:00:00 2001 From: Sebastian Kunert Date: Fri, 23 Feb 2024 10:43:57 +0100 Subject: [PATCH 25/26] Enable import proof recording in cumulus test client --- .../pallets/parachain-system/src/validate_block/tests.rs | 2 +- cumulus/test/client/src/lib.rs | 2 +- substrate/test-utils/client/src/lib.rs | 9 +++++++++ 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/cumulus/pallets/parachain-system/src/validate_block/tests.rs b/cumulus/pallets/parachain-system/src/validate_block/tests.rs index f17ac6007a09..a9fb65e11089 100644 --- a/cumulus/pallets/parachain-system/src/validate_block/tests.rs +++ b/cumulus/pallets/parachain-system/src/validate_block/tests.rs @@ -59,7 +59,7 @@ fn call_validate_block( } fn create_test_client() -> (Client, Header) { - let client = TestClientBuilder::new().build(); + let client = TestClientBuilder::new().enable_import_proof_recording().build(); let genesis_header = client .header(client.chain_info().genesis_hash) diff --git a/cumulus/test/client/src/lib.rs b/cumulus/test/client/src/lib.rs index 62481a3bf0b7..c46f4da7f678 100644 --- a/cumulus/test/client/src/lib.rs +++ b/cumulus/test/client/src/lib.rs @@ -151,7 +151,7 @@ pub fn generate_extrinsic_with_pair( frame_system::CheckNonce::::from(nonce), frame_system::CheckWeight::::new(), pallet_transaction_payment::ChargeTransactionPayment::::from(tip), - cumulus_primitives_storage_weight_reclaim::StorageWeightReclaim::::new(), + cumulus_primitives_storage_weight_reclaim::StorageWeightReclaim::::new(), ); let function = function.into(); diff --git a/substrate/test-utils/client/src/lib.rs b/substrate/test-utils/client/src/lib.rs index e3f06e275635..d283b24f286a 100644 --- a/substrate/test-utils/client/src/lib.rs +++ b/substrate/test-utils/client/src/lib.rs @@ -72,6 +72,7 @@ pub struct TestClientBuilder, bad_blocks: BadBlocks, enable_offchain_indexing_api: bool, + enable_import_proof_recording: bool, no_genesis: bool, } @@ -120,6 +121,7 @@ impl bad_blocks: None, enable_offchain_indexing_api: false, no_genesis: false, + enable_import_proof_recording: false, } } @@ -165,6 +167,12 @@ impl self } + /// Enable proof recording on import. + pub fn enable_import_proof_recording(mut self) -> Self { + self.enable_import_proof_recording = true; + self + } + /// Disable writing genesis. pub fn set_no_genesis(mut self) -> Self { self.no_genesis = true; @@ -202,6 +210,7 @@ impl }; let client_config = ClientConfig { + enable_import_proof_recording: self.enable_import_proof_recording, offchain_indexing_api: self.enable_offchain_indexing_api, no_genesis: self.no_genesis, ..Default::default() From 408f7c7ea219ad983b6e37c5a64fdc341b566e1a Mon Sep 17 00:00:00 2001 From: Sebastian Kunert Date: Fri, 23 Feb 2024 13:57:02 +0100 Subject: [PATCH 26/26] Use cumulus-test-runtime for tests --- Cargo.lock | 3 +- .../storage-weight-reclaim/Cargo.toml | 3 +- .../storage-weight-reclaim/src/lib.rs | 118 +++++++----------- substrate/frame/system/src/lib.rs | 5 +- 4 files changed, 52 insertions(+), 77 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0f04a7b51fc8..b74a5a96e4f4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4049,10 +4049,11 @@ dependencies = [ [[package]] name = "cumulus-primitives-storage-weight-reclaim" -version = "0.2.0" +version = "1.0.0" dependencies = [ "cumulus-primitives-core", "cumulus-primitives-proof-size-hostfunction", + "cumulus-test-runtime", "docify", "frame-support", "frame-system", diff --git a/cumulus/primitives/storage-weight-reclaim/Cargo.toml b/cumulus/primitives/storage-weight-reclaim/Cargo.toml index 2be91ddee3b0..4835fb5192b8 100644 --- a/cumulus/primitives/storage-weight-reclaim/Cargo.toml +++ b/cumulus/primitives/storage-weight-reclaim/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "cumulus-primitives-storage-weight-reclaim" -version = "0.2.0" +version = "1.0.0" authors.workspace = true edition.workspace = true description = "Utilities to reclaim storage weight." @@ -27,6 +27,7 @@ docify = "0.2.7" [dev-dependencies] sp-trie = { path = "../../../substrate/primitives/trie", default-features = false } sp-io = { path = "../../../substrate/primitives/io", default-features = false } +cumulus-test-runtime = { path = "../../test/runtime" } [features] default = ["std"] diff --git a/cumulus/primitives/storage-weight-reclaim/src/lib.rs b/cumulus/primitives/storage-weight-reclaim/src/lib.rs index f161f5866f33..5dddc92e3955 100644 --- a/cumulus/primitives/storage-weight-reclaim/src/lib.rs +++ b/cumulus/primitives/storage-weight-reclaim/src/lib.rs @@ -204,13 +204,27 @@ mod tests { dispatch::DispatchClass, weights::{Weight, WeightMeter}, }; - use frame_system::{ - mock::{new_test_ext, Test, CALL}, - BlockWeight, CheckWeight, - }; + use frame_system::{BlockWeight, CheckWeight}; + use sp_runtime::{AccountId32, BuildStorage}; use sp_std::marker::PhantomData; use sp_trie::proof_size_extension::ProofSizeExt; + type Test = cumulus_test_runtime::Runtime; + const CALL: &::RuntimeCall = + &cumulus_test_runtime::RuntimeCall::System(frame_system::Call::set_heap_pages { + pages: 0u64, + }); + const ALICE: AccountId32 = AccountId32::new([1u8; 32]); + const LEN: usize = 0; + + pub fn new_test_ext() -> sp_io::TestExternalities { + let ext: sp_io::TestExternalities = cumulus_test_runtime::RuntimeGenesisConfig::default() + .build_storage() + .unwrap() + .into(); + ext + } + struct TestRecorder { return_values: Box<[usize]>, counter: std::sync::atomic::AtomicUsize, @@ -229,10 +243,6 @@ mod tests { } } - fn base_block_weight() -> Weight { - ::BlockWeights::get().base_block - } - fn setup_test_externalities(proof_values: &[usize]) -> sp_io::TestExternalities { let mut test_ext = new_test_ext(); let test_recorder = TestRecorder::new(proof_values); @@ -258,9 +268,8 @@ mod tests { let info = DispatchInfo { weight: Weight::from_parts(0, 500), ..Default::default() }; let post_info = PostDispatchInfo::default(); - let len = 0_usize; let pre = StorageWeightReclaim::(PhantomData) - .pre_dispatch(&1, CALL, &info, len) + .pre_dispatch(&ALICE, CALL, &info, LEN) .unwrap(); assert_eq!(pre, Some(0)); @@ -270,14 +279,11 @@ mod tests { Some(pre), &info, &post_info, - len, + LEN, &Ok(()) )); - assert_eq!( - BlockWeight::::get().total(), - Weight::from_parts(base_block_weight().ref_time(), 600) - ); + assert_eq!(BlockWeight::::get().total().proof_size(), 600); }) } @@ -293,9 +299,8 @@ mod tests { let info = DispatchInfo { weight: Weight::from_parts(0, 500), ..Default::default() }; let post_info = PostDispatchInfo::default(); - let len = 0_usize; let pre = StorageWeightReclaim::(PhantomData) - .pre_dispatch(&1, CALL, &info, len) + .pre_dispatch(&ALICE, CALL, &info, LEN) .unwrap(); assert_eq!(pre, None); @@ -304,14 +309,11 @@ mod tests { Some(pre), &info, &post_info, - len, + LEN, &Ok(()) )); - assert_eq!( - BlockWeight::::get().total(), - Weight::from_parts(base_block_weight().ref_time(), 1000) - ); + assert_eq!(BlockWeight::::get().total().proof_size(), 1000); }) } @@ -325,9 +327,8 @@ mod tests { let info = DispatchInfo { weight: Weight::from_parts(0, 100), ..Default::default() }; let post_info = PostDispatchInfo::default(); - let len = 0_usize; let pre = StorageWeightReclaim::(PhantomData) - .pre_dispatch(&1, CALL, &info, len) + .pre_dispatch(&ALICE, CALL, &info, LEN) .unwrap(); assert_eq!(pre, Some(100)); @@ -337,14 +338,11 @@ mod tests { Some(pre), &info, &post_info, - len, + LEN, &Ok(()) )); - assert_eq!( - BlockWeight::::get().total(), - Weight::from_parts(base_block_weight().ref_time(), 1100) - ); + assert_eq!(BlockWeight::::get().total().proof_size(), 1100); }) } @@ -356,9 +354,8 @@ mod tests { let info = DispatchInfo { weight: Weight::from_parts(0, 500), ..Default::default() }; let post_info = PostDispatchInfo::default(); - let len = 0_usize; let pre = StorageWeightReclaim::(PhantomData) - .pre_dispatch(&1, CALL, &info, len) + .pre_dispatch(&ALICE, CALL, &info, LEN) .unwrap(); assert_eq!(pre, Some(0)); @@ -367,11 +364,11 @@ mod tests { Some(pre), &info, &post_info, - len, + LEN, &Ok(()) )); - assert_eq!(BlockWeight::::get().total(), base_block_weight()); + assert_eq!(BlockWeight::::get().total().proof_size(), 0); }); } @@ -385,9 +382,8 @@ mod tests { let info = DispatchInfo { weight: Weight::from_parts(0, 500), ..Default::default() }; let post_info = PostDispatchInfo::default(); - let len = 0_usize; let pre = StorageWeightReclaim::(PhantomData) - .pre_dispatch(&1, CALL, &info, len) + .pre_dispatch(&ALICE, CALL, &info, LEN) .unwrap(); assert_eq!(pre, Some(300)); @@ -396,14 +392,11 @@ mod tests { Some(pre), &info, &post_info, - len, + LEN, &Ok(()) )); - assert_eq!( - BlockWeight::::get().total(), - Weight::from_parts(base_block_weight().ref_time(), 800) - ); + assert_eq!(BlockWeight::::get().total().proof_size(), 800); }); } @@ -423,9 +416,8 @@ mod tests { pays_fee: Default::default(), }; - let len = 0_usize; let pre = StorageWeightReclaim::(PhantomData) - .pre_dispatch(&1, CALL, &info, len) + .pre_dispatch(&ALICE, CALL, &info, LEN) .unwrap(); assert_eq!(pre, Some(100)); @@ -436,14 +428,11 @@ mod tests { Some(pre), &info, &post_info, - len, + LEN, &Ok(()) )); - assert_eq!( - BlockWeight::::get().total(), - Weight::from_parts(base_block_weight().ref_time(), 900) - ); + assert_eq!(BlockWeight::::get().total().proof_size(), 900); }) } @@ -462,9 +451,8 @@ mod tests { pays_fee: Default::default(), }; - let len = 0_usize; let pre = StorageWeightReclaim::(PhantomData) - .pre_dispatch(&1, CALL, &info, len) + .pre_dispatch(&ALICE, CALL, &info, LEN) .unwrap(); assert_eq!(pre, Some(100)); @@ -475,14 +463,11 @@ mod tests { Some(pre), &info, &post_info, - len, + LEN, &Ok(()) )); - assert_eq!( - BlockWeight::::get().total(), - Weight::from_parts(base_block_weight().ref_time(), 1150) - ); + assert_eq!(BlockWeight::::get().total().proof_size(), 1150); }) } @@ -502,9 +487,8 @@ mod tests { pays_fee: Default::default(), }; - let len = 0_usize; let pre = StorageWeightReclaim::(PhantomData) - .pre_dispatch(&1, CALL, &info, len) + .pre_dispatch(&ALICE, CALL, &info, LEN) .unwrap(); assert_eq!(pre, Some(100)); @@ -512,7 +496,7 @@ mod tests { Some(pre), &info, &post_info, - len, + LEN, &Ok(()) )); // `CheckWeight` gets called after `StorageWeightReclaim` this time. @@ -520,10 +504,7 @@ mod tests { // we always need to call `post_dispatch` to verify that they interoperate correctly. assert_ok!(CheckWeight::::post_dispatch(None, &info, &post_info, 0, &Ok(()))); - assert_eq!( - BlockWeight::::get().total(), - Weight::from_parts(base_block_weight().ref_time(), 900) - ); + assert_eq!(BlockWeight::::get().total().proof_size(), 900); }) } @@ -542,9 +523,8 @@ mod tests { pays_fee: Default::default(), }; - let len = 0_usize; let pre = StorageWeightReclaim::(PhantomData) - .pre_dispatch(&1, CALL, &info, len) + .pre_dispatch(&ALICE, CALL, &info, LEN) .unwrap(); assert_eq!(pre, Some(100)); @@ -552,7 +532,7 @@ mod tests { Some(pre), &info, &post_info, - len, + LEN, &Ok(()) )); // `CheckWeight` gets called after `StorageWeightReclaim` this time. @@ -560,10 +540,7 @@ mod tests { // we always need to call `post_dispatch` to verify that they interoperate correctly. assert_ok!(CheckWeight::::post_dispatch(None, &info, &post_info, 0, &Ok(()))); - assert_eq!( - BlockWeight::::get().total(), - Weight::from_parts(base_block_weight().ref_time(), 1150) - ); + assert_eq!(BlockWeight::::get().total().proof_size(), 1150); }) } @@ -667,10 +644,7 @@ mod tests { // We reclaimed 3 bytes of storage size! assert_eq!(reclaimed, Some(Weight::from_parts(0, 3))); - assert_eq!( - BlockWeight::::get().total(), - Weight::from_parts(base_block_weight().ref_time(), 10) - ); + assert_eq!(BlockWeight::::get().total().proof_size(), 10); assert_eq!(remaining_weight_meter.remaining(), Weight::from_parts(10, 8)); } } diff --git a/substrate/frame/system/src/lib.rs b/substrate/frame/system/src/lib.rs index 7f0365eef74b..1405c7303f87 100644 --- a/substrate/frame/system/src/lib.rs +++ b/substrate/frame/system/src/lib.rs @@ -146,9 +146,8 @@ use sp_weights::{RuntimeDbWeight, Weight}; use sp_io::TestExternalities; pub mod limits; - -#[cfg(any(feature = "std", test))] -pub mod mock; +#[cfg(test)] +pub(crate) mod mock; pub mod offchain;