From 0ae1e63c14dd386a241ec01d68b4cc1a70cedbd7 Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Thu, 25 Jul 2024 14:06:08 +0300 Subject: [PATCH 01/18] Fix report_future_block_voting weight --- substrate/frame/beefy/src/lib.rs | 50 +++++++++++++++++++------------- 1 file changed, 30 insertions(+), 20 deletions(-) diff --git a/substrate/frame/beefy/src/lib.rs b/substrate/frame/beefy/src/lib.rs index 353ba876c7ed..5bd7b100dd4b 100644 --- a/substrate/frame/beefy/src/lib.rs +++ b/substrate/frame/beefy/src/lib.rs @@ -19,21 +19,33 @@ extern crate alloc; +mod default_weights; +mod equivocation; +#[cfg(test)] +mod mock; +#[cfg(test)] +mod tests; + use alloc::{boxed::Box, vec::Vec}; use codec::{Encode, MaxEncodedLen}; +use log; use frame_support::{ dispatch::{DispatchResultWithPostInfo, Pays}, pallet_prelude::*, traits::{Get, OneSessionHandler}, - weights::Weight, + weights::{constants::RocksDbWeight as DbWeight, Weight}, BoundedSlice, BoundedVec, Parameter, }; use frame_system::{ ensure_none, ensure_signed, pallet_prelude::{BlockNumberFor, HeaderFor, OriginFor}, }; -use log; +use sp_consensus_beefy::{ + AncestryHelper, AuthorityIndex, BeefyAuthorityId, ConsensusLog, DoubleVotingProof, + ForkVotingProof, FutureBlockVotingProof, OnNewValidatorSet, ValidatorSet, BEEFY_ENGINE_ID, + GENESIS_AUTHORITY_SET_ID, +}; use sp_runtime::{ generic::DigestItem, traits::{IsMember, Member, One}, @@ -42,24 +54,10 @@ use sp_runtime::{ use sp_session::{GetSessionNumber, GetValidatorCount}; use sp_staking::{offence::OffenceReportSystem, SessionIndex}; -use sp_consensus_beefy::{ - AncestryHelper, AuthorityIndex, BeefyAuthorityId, ConsensusLog, DoubleVotingProof, - ForkVotingProof, FutureBlockVotingProof, OnNewValidatorSet, ValidatorSet, BEEFY_ENGINE_ID, - GENESIS_AUTHORITY_SET_ID, -}; - -mod default_weights; -mod equivocation; -#[cfg(test)] -mod mock; -#[cfg(test)] -mod tests; - +use crate::equivocation::EquivocationEvidenceFor; pub use crate::equivocation::{EquivocationOffence, EquivocationReportSystem, TimeSlot}; pub use pallet::*; -use crate::equivocation::EquivocationEvidenceFor; - const LOG_TARGET: &str = "runtime::beefy"; #[frame_support::pallet] @@ -358,7 +356,7 @@ pub mod pallet { /// and validate the given key ownership proof against the extracted offender. /// If both are valid, the offence will be reported. #[pallet::call_index(5)] - #[pallet::weight(T::WeightInfo::report_fork_voting( + #[pallet::weight(T::WeightInfo::report_future_block_voting( key_owner_proof.validator_count(), T::MaxNominators::get(), ))] @@ -389,7 +387,7 @@ pub mod pallet { /// if the block author is defined it will be defined as the equivocation /// reporter. #[pallet::call_index(6)] - #[pallet::weight(T::WeightInfo::report_fork_voting( + #[pallet::weight(T::WeightInfo::report_future_block_voting( key_owner_proof.validator_count(), T::MaxNominators::get(), ))] @@ -740,15 +738,27 @@ pub trait WeightInfo { validator_count: u32, max_nominators_per_validator: u32, ) -> Weight; + fn report_double_voting(validator_count: u32, max_nominators_per_validator: u32) -> Weight { Self::report_voting_equivocation(2, validator_count, max_nominators_per_validator) } + fn report_fork_voting(validator_count: u32, max_nominators_per_validator: u32) -> Weight; + fn report_future_block_voting( validator_count: u32, max_nominators_per_validator: u32, ) -> Weight { - Self::report_voting_equivocation(1, validator_count, max_nominators_per_validator) + // checking if the report is for a future block + DbWeight::get() + .reads(1) + // check and report the equivocated vote + .saturating_add(Self::report_voting_equivocation( + 1, + validator_count, + max_nominators_per_validator, + )) } + fn set_new_genesis() -> Weight; } From 15a18ec685b6329204c6d6cc481aea653057a1ad Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Fri, 26 Jul 2024 16:40:14 +0300 Subject: [PATCH 02/18] Add empty benchmarks for pallet-beefy-mmr --- Cargo.lock | 2 + polkadot/runtime/rococo/src/lib.rs | 2 + polkadot/runtime/westend/src/lib.rs | 2 + substrate/bin/node/runtime/src/lib.rs | 1 + substrate/frame/beefy-mmr/Cargo.toml | 3 + substrate/frame/beefy-mmr/src/benchmarking.rs | 34 ++++++++++ substrate/frame/beefy-mmr/src/lib.rs | 34 ++++++++-- substrate/frame/beefy-mmr/src/mock.rs | 1 + substrate/frame/beefy-mmr/src/weights.rs | 65 +++++++++++++++++++ substrate/frame/beefy/src/lib.rs | 9 +-- substrate/frame/beefy/src/mock.rs | 15 ++++- .../primitives/consensus/beefy/Cargo.toml | 2 + .../primitives/consensus/beefy/src/lib.rs | 10 +++ 13 files changed, 170 insertions(+), 10 deletions(-) create mode 100644 substrate/frame/beefy-mmr/src/benchmarking.rs create mode 100644 substrate/frame/beefy-mmr/src/weights.rs diff --git a/Cargo.lock b/Cargo.lock index 7466975fa428..360001d39642 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10064,6 +10064,7 @@ version = "28.0.0" dependencies = [ "array-bytes", "binary-merkle-tree", + "frame-benchmarking", "frame-support", "frame-system", "log", @@ -20040,6 +20041,7 @@ dependencies = [ "sp-keystore", "sp-mmr-primitives", "sp-runtime", + "sp-weights", "strum 0.26.2", "w3f-bls", ] diff --git a/polkadot/runtime/rococo/src/lib.rs b/polkadot/runtime/rococo/src/lib.rs index 2f23b889916b..ad279c38a708 100644 --- a/polkadot/runtime/rococo/src/lib.rs +++ b/polkadot/runtime/rococo/src/lib.rs @@ -1335,6 +1335,7 @@ impl pallet_beefy_mmr::Config for Runtime { type BeefyAuthorityToMerkleLeaf = pallet_beefy_mmr::BeefyEcdsaToEthereum; type LeafExtra = H256; type BeefyDataProvider = ParaHeadsRootProvider; + type WeightInfo = (); } impl paras_sudo_wrapper::Config for Runtime {} @@ -1735,6 +1736,7 @@ mod benches { // Substrate [pallet_balances, Balances] [pallet_balances, NisCounterpartBalances] + [pallet_beefy_mmr, MmrLeaf] [frame_benchmarking::baseline, Baseline::] [pallet_bounties, Bounties] [pallet_child_bounties, ChildBounties] diff --git a/polkadot/runtime/westend/src/lib.rs b/polkadot/runtime/westend/src/lib.rs index eddb7cbd21ea..e6348f2379c0 100644 --- a/polkadot/runtime/westend/src/lib.rs +++ b/polkadot/runtime/westend/src/lib.rs @@ -456,6 +456,7 @@ impl pallet_beefy_mmr::Config for Runtime { type BeefyAuthorityToMerkleLeaf = pallet_beefy_mmr::BeefyEcdsaToEthereum; type LeafExtra = H256; type BeefyDataProvider = ParaHeadsRootProvider; + type WeightInfo = (); } parameter_types! { @@ -1842,6 +1843,7 @@ mod benches { // Substrate [pallet_bags_list, VoterList] [pallet_balances, Balances] + [pallet_beefy_mmr, BeefyMmrLeaf] [pallet_conviction_voting, ConvictionVoting] [pallet_election_provider_multi_phase, ElectionProviderMultiPhase] [frame_election_provider_support, ElectionProviderBench::] diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index fd8597563a02..d91a3ee76084 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -1611,6 +1611,7 @@ impl pallet_beefy_mmr::Config for Runtime { type BeefyAuthorityToMerkleLeaf = pallet_beefy_mmr::BeefyEcdsaToEthereum; type LeafExtra = Vec; type BeefyDataProvider = (); + type WeightInfo = (); } parameter_types! { diff --git a/substrate/frame/beefy-mmr/Cargo.toml b/substrate/frame/beefy-mmr/Cargo.toml index 672a7a68bc7e..d67ac20ee922 100644 --- a/substrate/frame/beefy-mmr/Cargo.toml +++ b/substrate/frame/beefy-mmr/Cargo.toml @@ -18,6 +18,7 @@ log = { workspace = true } scale-info = { features = ["derive"], workspace = true } serde = { optional = true, workspace = true, default-features = true } binary-merkle-tree = { workspace = true } +frame-benchmarking = { optional = true, workspace = true } frame-support = { workspace = true } frame-system = { workspace = true } pallet-beefy = { workspace = true } @@ -40,6 +41,7 @@ std = [ "array-bytes", "binary-merkle-tree/std", "codec/std", + "frame-benchmarking/std", "frame-support/std", "frame-system/std", "log/std", @@ -65,6 +67,7 @@ try-runtime = [ "sp-runtime/try-runtime", ] runtime-benchmarks = [ + "frame-benchmarking/runtime-benchmarks", "frame-support/runtime-benchmarks", "frame-system/runtime-benchmarks", "pallet-mmr/runtime-benchmarks", diff --git a/substrate/frame/beefy-mmr/src/benchmarking.rs b/substrate/frame/beefy-mmr/src/benchmarking.rs new file mode 100644 index 000000000000..da69b39169b2 --- /dev/null +++ b/substrate/frame/beefy-mmr/src/benchmarking.rs @@ -0,0 +1,34 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Beefy pallet benchmarking. + +#![cfg(feature = "runtime-benchmarks")] + +use super::*; +use frame_benchmarking::v2::*; + +#[benchmarks] +mod benchmarks { + use super::*; + + #[benchmark] + fn n_items_proof_is_non_canonical(n: Linear<1, 64>) { + #[block] + {} + } +} diff --git a/substrate/frame/beefy-mmr/src/lib.rs b/substrate/frame/beefy-mmr/src/lib.rs index 195bbfbf2f29..177d76dbbdaa 100644 --- a/substrate/frame/beefy-mmr/src/lib.rs +++ b/substrate/frame/beefy-mmr/src/lib.rs @@ -35,7 +35,10 @@ extern crate alloc; -use sp_runtime::traits::{Convert, Header, Member}; +use sp_runtime::{ + generic::OpaqueDigestItemId, + traits::{Convert, Header, Member}, +}; use alloc::vec::Vec; use codec::Decode; @@ -43,19 +46,25 @@ use pallet_mmr::{primitives::AncestryProof, LeafDataProvider, ParentNumberAndHas use sp_consensus_beefy::{ known_payloads, mmr::{BeefyAuthoritySet, BeefyDataProvider, BeefyNextAuthoritySet, MmrLeaf, MmrLeafVersion}, - AncestryHelper, Commitment, ConsensusLog, ValidatorSet as BeefyValidatorSet, + AncestryHelper, AncestryHelperWeightInfo, Commitment, ConsensusLog, + ValidatorSet as BeefyValidatorSet, }; -use frame_support::{crypto::ecdsa::ECDSAExt, traits::Get}; +use frame_support::{ + crypto::ecdsa::ECDSAExt, pallet_prelude::Weight, traits::Get, + weights::constants::RocksDbWeight as DbWeight, +}; use frame_system::pallet_prelude::{BlockNumberFor, HeaderFor}; pub use pallet::*; -use sp_runtime::generic::OpaqueDigestItemId; +use weights::WeightInfo; +mod benchmarking; #[cfg(test)] mod mock; #[cfg(test)] mod tests; +mod weights; /// A BEEFY consensus digest item with MMR root hash. pub struct DepositBeefyDigest(core::marker::PhantomData); @@ -126,6 +135,8 @@ pub mod pallet { /// Retrieve arbitrary data that should be added to the mmr leaf type BeefyDataProvider: BeefyDataProvider; + + type WeightInfo: WeightInfo; } /// Details of current BEEFY authority set. @@ -263,6 +274,21 @@ where } } +impl AncestryHelperWeightInfo> for Pallet +where + T: pallet_mmr::Config, +{ + fn extract_validation_context(_header: &HeaderFor) -> Weight { + // We're only reading from `BlockHash` and then searching in a small vec, + // which should be insignificant. + DbWeight::get().reads(1) + } + + fn is_non_canonical(proof: &>>::Proof) -> Weight { + ::WeightInfo::n_items_proof_is_non_canonical(proof.items.len() as u32) + } +} + impl Pallet { /// Return the currently active BEEFY authority set proof. pub fn authority_set_proof() -> BeefyAuthoritySet> { diff --git a/substrate/frame/beefy-mmr/src/mock.rs b/substrate/frame/beefy-mmr/src/mock.rs index 1102f9677aaa..5af83260c062 100644 --- a/substrate/frame/beefy-mmr/src/mock.rs +++ b/substrate/frame/beefy-mmr/src/mock.rs @@ -122,6 +122,7 @@ impl pallet_beefy_mmr::Config for Test { type LeafExtra = Vec; type BeefyDataProvider = DummyDataProvider; + type WeightInfo = (); } pub struct DummyDataProvider; diff --git a/substrate/frame/beefy-mmr/src/weights.rs b/substrate/frame/beefy-mmr/src/weights.rs new file mode 100644 index 000000000000..b72663d0ca4c --- /dev/null +++ b/substrate/frame/beefy-mmr/src/weights.rs @@ -0,0 +1,65 @@ + +//! Autogenerated weights for `pallet_beefy_mmr` +//! +//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 32.0.0 +//! DATE: 2024-07-26, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]` +//! WORST CASE MAP SIZE: `1000000` +//! HOSTNAME: `serban-ROG-Zephyrus`, CPU: `AMD Ryzen 9 7945HX with Radeon Graphics` +//! WASM-EXECUTION: `Compiled`, CHAIN: `None`, DB CACHE: `1024` + +// Executed Command: +// target/release/polkadot +// benchmark +// pallet +// --runtime +// ./target/release/wbuild/rococo-runtime/rococo_runtime.wasm +// --steps=50 +// --repeat=20 +// --pallet=pallet_beefy_mmr +// --extrinsic=* +// --wasm-execution=Compiled +// --heap-pages=4096 +// --output=./substrate/frame/beefy-mmr/src/weights.rs +// --template=./substrate/.maintain/frame-weight-template.hbs + +#![cfg_attr(rustfmt, rustfmt_skip)] +#![allow(unused_parens)] +#![allow(unused_imports)] +#![allow(missing_docs)] + +use frame_support::{traits::Get, weights::{Weight, constants::RocksDbWeight}}; +use core::marker::PhantomData; + +/// Weight functions needed for `pallet_beefy_mmr`. +pub trait WeightInfo { + fn n_items_proof_is_non_canonical(n: u32, ) -> Weight; +} + +/// Weights for `pallet_beefy_mmr` using the Substrate node and recommended hardware. +pub struct SubstrateWeight(PhantomData); +impl WeightInfo for SubstrateWeight { + /// The range of component `n` is `[1, 64]`. + fn n_items_proof_is_non_canonical(n: u32, ) -> Weight { + // Proof Size summary in bytes: + // Measured: `0` + // Estimated: `0` + // Minimum execution time: 0_000 picoseconds. + Weight::from_parts(1_187_650, 0) + // Standard Error: 180 + .saturating_add(Weight::from_parts(137, 0).saturating_mul(n.into())) + } +} + +// For backwards compatibility and tests. +impl WeightInfo for () { + /// The range of component `n` is `[1, 64]`. + fn n_items_proof_is_non_canonical(n: u32, ) -> Weight { + // Proof Size summary in bytes: + // Measured: `0` + // Estimated: `0` + // Minimum execution time: 0_000 picoseconds. + Weight::from_parts(1_187_650, 0) + // Standard Error: 180 + .saturating_add(Weight::from_parts(137, 0).saturating_mul(n.into())) + } +} diff --git a/substrate/frame/beefy/src/lib.rs b/substrate/frame/beefy/src/lib.rs index 5bd7b100dd4b..ab1a7df961e6 100644 --- a/substrate/frame/beefy/src/lib.rs +++ b/substrate/frame/beefy/src/lib.rs @@ -42,9 +42,9 @@ use frame_system::{ pallet_prelude::{BlockNumberFor, HeaderFor, OriginFor}, }; use sp_consensus_beefy::{ - AncestryHelper, AuthorityIndex, BeefyAuthorityId, ConsensusLog, DoubleVotingProof, - ForkVotingProof, FutureBlockVotingProof, OnNewValidatorSet, ValidatorSet, BEEFY_ENGINE_ID, - GENESIS_AUTHORITY_SET_ID, + AncestryHelper, AncestryHelperWeightInfo, AuthorityIndex, BeefyAuthorityId, ConsensusLog, + DoubleVotingProof, ForkVotingProof, FutureBlockVotingProof, OnNewValidatorSet, ValidatorSet, + BEEFY_ENGINE_ID, GENESIS_AUTHORITY_SET_ID, }; use sp_runtime::{ generic::DigestItem, @@ -100,7 +100,8 @@ pub mod pallet { type OnNewValidatorSet: OnNewValidatorSet<::BeefyId>; /// Hook for checking commitment canonicity. - type AncestryHelper: AncestryHelper>; + type AncestryHelper: AncestryHelper> + + AncestryHelperWeightInfo>; /// Weights for this pallet. type WeightInfo: WeightInfo; diff --git a/substrate/frame/beefy/src/mock.rs b/substrate/frame/beefy/src/mock.rs index b423fa0bda89..b1aaef449c5d 100644 --- a/substrate/frame/beefy/src/mock.rs +++ b/substrate/frame/beefy/src/mock.rs @@ -21,12 +21,13 @@ use std::vec; use frame_election_provider_support::{ bounds::{ElectionBounds, ElectionBoundsBuilder}, - onchain, SequentialPhragmen, + onchain, SequentialPhragmen, Weight, }; use frame_support::{ construct_runtime, derive_impl, parameter_types, traits::{ConstU32, ConstU64, KeyOwnerProofSystem, OnFinalize, OnInitialize}, }; +use frame_system::pallet_prelude::HeaderFor; use pallet_session::historical as pallet_session_historical; use sp_core::{crypto::KeyTypeId, ConstU128}; use sp_runtime::{ @@ -43,7 +44,7 @@ use sp_state_machine::BasicExternalities; use crate as pallet_beefy; pub use sp_consensus_beefy::{ecdsa_crypto::AuthorityId as BeefyId, ConsensusLog, BEEFY_ENGINE_ID}; -use sp_consensus_beefy::{AncestryHelper, Commitment}; +use sp_consensus_beefy::{AncestryHelper, AncestryHelperWeightInfo, Commitment}; impl_opaque_keys! { pub struct MockSessionKeys { @@ -131,6 +132,16 @@ impl AncestryHelper
for MockAncestryHelper { } } +impl AncestryHelperWeightInfo
for MockAncestryHelper { + fn extract_validation_context(_header: &Header) -> Weight { + unimplemented!() + } + + fn is_non_canonical(_proof: &>>::Proof) -> Weight { + unimplemented!() + } +} + impl pallet_beefy::Config for Test { type BeefyId = BeefyId; type MaxAuthorities = ConstU32<100>; diff --git a/substrate/primitives/consensus/beefy/Cargo.toml b/substrate/primitives/consensus/beefy/Cargo.toml index f31aa5756ba2..57ddab9a70ce 100644 --- a/substrate/primitives/consensus/beefy/Cargo.toml +++ b/substrate/primitives/consensus/beefy/Cargo.toml @@ -26,6 +26,7 @@ sp-io = { workspace = true } sp-mmr-primitives = { workspace = true } sp-runtime = { workspace = true } sp-keystore = { workspace = true } +sp-weights = { workspace = true } strum = { features = ["derive"], workspace = true } lazy_static = { optional = true, workspace = true } @@ -48,6 +49,7 @@ std = [ "sp-keystore/std", "sp-mmr-primitives/std", "sp-runtime/std", + "sp-weights/std", "strum/std", ] diff --git a/substrate/primitives/consensus/beefy/src/lib.rs b/substrate/primitives/consensus/beefy/src/lib.rs index 6ec4a727e276..9b9352905063 100644 --- a/substrate/primitives/consensus/beefy/src/lib.rs +++ b/substrate/primitives/consensus/beefy/src/lib.rs @@ -56,6 +56,7 @@ use sp_runtime::{ traits::{Hash, Header as HeaderT, Keccak256, NumberFor}, OpaqueValue, }; +use sp_weights::Weight; /// Key type for BEEFY module. pub const KEY_TYPE: sp_core::crypto::KeyTypeId = sp_application_crypto::key_types::BEEFY; @@ -460,6 +461,15 @@ pub trait AncestryHelper { ) -> bool; } +/// Weight information for the logic in `AncestryHelper`. +pub trait AncestryHelperWeightInfo: AncestryHelper
{ + /// Weight info for the `AncestryHelper::extract_validation_context()` method. + fn extract_validation_context(header: &Header) -> Weight; + + /// Weight info for the `AncestryHelper::is_non_canonical()` method. + fn is_non_canonical(proof: &>::Proof) -> Weight; +} + /// An opaque type used to represent the key ownership proof at the runtime API /// boundary. The inner value is an encoded representation of the actual key /// ownership proof which will be parameterized when defining the runtime. At From 91b34dad177e37ae0209b3adde0facf32644b674 Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Mon, 29 Jul 2024 19:35:22 +0300 Subject: [PATCH 03/18] [MMR] Make offchain set/get work in benchmarks --- .../merkle-mountain-range/src/mmr/storage.rs | 25 ++++++++++++++----- 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/substrate/frame/merkle-mountain-range/src/mmr/storage.rs b/substrate/frame/merkle-mountain-range/src/mmr/storage.rs index a39089801484..948a148c74a4 100644 --- a/substrate/frame/merkle-mountain-range/src/mmr/storage.rs +++ b/substrate/frame/merkle-mountain-range/src/mmr/storage.rs @@ -22,7 +22,6 @@ use codec::Encode; use core::iter::Peekable; use log::{debug, trace}; use sp_core::offchain::StorageKind; -use sp_io::offchain_index; use sp_mmr_primitives::{mmr_lib, mmr_lib::helper, utils::NodesUtils}; use crate::{ @@ -47,6 +46,22 @@ pub struct RuntimeStorage; /// DOES NOT support adding new items to the MMR. pub struct OffchainStorage; +impl OffchainStorage { + fn get(key: &[u8]) -> Option> { + sp_io::offchain::local_storage_get(StorageKind::PERSISTENT, &key) + } + + #[cfg(not(feature = "runtime-benchmarks"))] + fn set(key: &[u8], value: &[u8]) { + sp_io::offchain_index::set(key, value); + } + + #[cfg(feature = "runtime-benchmarks")] + fn set(key: &[u8], value: &[u8]) { + sp_io::offchain::local_storage_set(StorageKind::PERSISTENT, key, value); + } +} + /// A storage layer for MMR. /// /// There are two different implementations depending on the use case. @@ -78,7 +93,7 @@ where pos, ancestor_leaf_idx, key ); // Try to retrieve the element from Off-chain DB. - if let Some(elem) = sp_io::offchain::local_storage_get(StorageKind::PERSISTENT, &key) { + if let Some(elem) = OffchainStorage::get(&key) { return Ok(codec::Decode::decode(&mut &*elem).ok()) } @@ -93,8 +108,7 @@ where pos, ancestor_leaf_idx, ancestor_parent_hash, temp_key ); // Retrieve the element from Off-chain DB. - Ok(sp_io::offchain::local_storage_get(StorageKind::PERSISTENT, &temp_key) - .and_then(|v| codec::Decode::decode(&mut &*v).ok())) + Ok(OffchainStorage::get(&temp_key).and_then(|v| codec::Decode::decode(&mut &*v).ok())) } } @@ -203,8 +217,7 @@ where target: "runtime::mmr::offchain", "offchain db set: pos {} parent_hash {:?} key {:?}", pos, parent_hash, temp_key ); - // Indexing API is used to store the full node content. - offchain_index::set(&temp_key, &encoded_node); + OffchainStorage::set(&temp_key, &encoded_node); } } From 8e0ea6fde3d1f1c6221bd1bf6e4adec7f2ca2374 Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Fri, 26 Jul 2024 11:14:09 +0300 Subject: [PATCH 04/18] Ancestry proof benchmarks --- substrate/frame/beefy-mmr/src/benchmarking.rs | 81 ++++++++++++++- substrate/frame/beefy-mmr/src/lib.rs | 27 +++-- substrate/frame/beefy-mmr/src/tests.rs | 6 +- substrate/frame/beefy-mmr/src/weights.rs | 99 ++++++++++++++++--- substrate/frame/beefy/src/mock.rs | 2 +- .../frame/merkle-mountain-range/src/lib.rs | 8 ++ .../merkle-mountain-range/src/mmr/mmr.rs | 46 ++++++++- .../primitives/consensus/beefy/src/lib.rs | 2 +- .../merkle-mountain-range/src/utils.rs | 11 ++- 9 files changed, 245 insertions(+), 37 deletions(-) diff --git a/substrate/frame/beefy-mmr/src/benchmarking.rs b/substrate/frame/beefy-mmr/src/benchmarking.rs index da69b39169b2..680b560b8748 100644 --- a/substrate/frame/beefy-mmr/src/benchmarking.rs +++ b/substrate/frame/beefy-mmr/src/benchmarking.rs @@ -20,15 +20,92 @@ #![cfg(feature = "runtime-benchmarks")] use super::*; +use crate::Pallet as BeefyMmr; +use codec::Encode; use frame_benchmarking::v2::*; +use frame_support::traits::Hooks; +use frame_system::{Config as SystemConfig, Pallet as System}; +use pallet_mmr::{Nodes, Pallet as Mmr}; +use sp_consensus_beefy::Payload; +use sp_runtime::traits::Zero; + +pub trait Config: + pallet_mmr::Config + crate::Config +{ +} + +impl Config for T where + T: pallet_mmr::Config + crate::Config +{ +} + +fn init_block(block_num: u32) { + let block_num = block_num.into(); + System::::initialize(&block_num, &::Hash::default(), &Default::default()); + Mmr::::on_initialize(block_num); +} #[benchmarks] mod benchmarks { use super::*; #[benchmark] - fn n_items_proof_is_non_canonical(n: Linear<1, 64>) { + fn extract_validation_context() { + init_block::(0); + let header = System::::finalize(); + frame_system::BlockHash::::insert(BlockNumberFor::::zero(), header.hash()); + + let validation_context; + #[block] + { + validation_context = + as AncestryHelper>>::extract_validation_context(header); + } + + assert!(validation_context.is_some()); + } + + #[benchmark] + fn read_peak() { + init_block::(0); + + let peak; + #[block] + { + peak = Nodes::::get(0) + } + + assert!(peak.is_some()); + } + + /// Generate ancestry proofs with `n` nodes and benchmark the verification logic. + /// These proofs are inflated, containing all the leafs, so we won't read any peak during + /// the verification. We need to account for the peaks separately. + #[benchmark] + fn n_items_proof_is_non_canonical(n: Linear<2, 512>) { + for block_num in 1..=n { + init_block::(block_num); + } + let proof = Mmr::::generate_mock_ancestry_proof().unwrap(); + assert_eq!(proof.items.len(), n as usize); + + let is_non_canonical; #[block] - {} + { + is_non_canonical = as AncestryHelper>>::is_non_canonical( + &Commitment { + payload: Payload::from_single_entry( + known_payloads::MMR_ROOT_ID, + MerkleRootOf::::default().encode(), + ), + block_number: n.into(), + validator_set_id: 0, + }, + proof, + Mmr::::mmr_root(), + ); + }; + + assert_eq!(is_non_canonical, true); } } diff --git a/substrate/frame/beefy-mmr/src/lib.rs b/substrate/frame/beefy-mmr/src/lib.rs index 177d76dbbdaa..f41e2f027ffc 100644 --- a/substrate/frame/beefy-mmr/src/lib.rs +++ b/substrate/frame/beefy-mmr/src/lib.rs @@ -38,11 +38,12 @@ extern crate alloc; use sp_runtime::{ generic::OpaqueDigestItemId, traits::{Convert, Header, Member}, + SaturatedConversion, }; use alloc::vec::Vec; use codec::Decode; -use pallet_mmr::{primitives::AncestryProof, LeafDataProvider, ParentNumberAndHash}; +use pallet_mmr::{primitives::AncestryProof, LeafDataProvider, NodesUtils, ParentNumberAndHash}; use sp_consensus_beefy::{ known_payloads, mmr::{BeefyAuthoritySet, BeefyDataProvider, BeefyNextAuthoritySet, MmrLeaf, MmrLeafVersion}, @@ -50,10 +51,7 @@ use sp_consensus_beefy::{ ValidatorSet as BeefyValidatorSet, }; -use frame_support::{ - crypto::ecdsa::ECDSAExt, pallet_prelude::Weight, traits::Get, - weights::constants::RocksDbWeight as DbWeight, -}; +use frame_support::{crypto::ecdsa::ECDSAExt, pallet_prelude::Weight, traits::Get}; use frame_system::pallet_prelude::{BlockNumberFor, HeaderFor}; pub use pallet::*; @@ -278,14 +276,23 @@ impl AncestryHelperWeightInfo> for Pallet where T: pallet_mmr::Config, { - fn extract_validation_context(_header: &HeaderFor) -> Weight { - // We're only reading from `BlockHash` and then searching in a small vec, - // which should be insignificant. - DbWeight::get().reads(1) + fn extract_validation_context() -> Weight { + ::WeightInfo::extract_validation_context() } fn is_non_canonical(proof: &>>::Proof) -> Weight { - ::WeightInfo::n_items_proof_is_non_canonical(proof.items.len() as u32) + let mmr_utils = NodesUtils::new(proof.leaf_count); + let num_peaks = mmr_utils.number_of_peaks(); + + // The approximated cost of verifying an ancestry proof with `n` nodes. + // We add the previous peaks to the total number of nodes, + // since they have to be processed as well. + ::WeightInfo::n_items_proof_is_non_canonical( + proof.items.len().saturating_add(proof.prev_peaks.len()).saturated_into(), + ) + // `n_items_proof_is_non_canonical()` uses inflated proofs that contain all the leafs, + // where no peak needs to be read. So we need to also add the cost of reading the peaks. + .saturating_add(::WeightInfo::read_peak().saturating_mul(num_peaks)) } } diff --git a/substrate/frame/beefy-mmr/src/tests.rs b/substrate/frame/beefy-mmr/src/tests.rs index f99835a1dc0a..226b0b698777 100644 --- a/substrate/frame/beefy-mmr/src/tests.rs +++ b/substrate/frame/beefy-mmr/src/tests.rs @@ -40,8 +40,6 @@ fn init_block(block: u64, maybe_parent_hash: Option) { System::initialize(&block, &parent_hash, &Default::default()); Session::on_initialize(block); Mmr::on_initialize(block); - Beefy::on_initialize(block); - BeefyMmr::on_initialize(block); } pub fn beefy_log(log: ConsensusLog) -> DigestItem { @@ -268,7 +266,7 @@ fn is_non_canonical_should_work_correctly() { ext.register_extension(OffchainWorkerExt::new(offchain)); ext.execute_with(|| { - let valid_proof = Mmr::generate_ancestry_proof(250, None).unwrap(); + let valid_proof = BeefyMmr::generate_proof(250, None).unwrap(); let mut invalid_proof = valid_proof.clone(); invalid_proof.items.push((300, Default::default())); @@ -343,7 +341,7 @@ fn is_non_canonical_should_work_correctly() { // - should return false, if the commitment is targeting the canonical chain // - should return true if the commitment is NOT targeting the canonical chain for prev_block_number in 1usize..=500 { - let proof = Mmr::generate_ancestry_proof(prev_block_number as u64, None).unwrap(); + let proof = BeefyMmr::generate_proof(prev_block_number as u64, None).unwrap(); assert_eq!( BeefyMmr::is_non_canonical( diff --git a/substrate/frame/beefy-mmr/src/weights.rs b/substrate/frame/beefy-mmr/src/weights.rs index b72663d0ca4c..d870752d248b 100644 --- a/substrate/frame/beefy-mmr/src/weights.rs +++ b/substrate/frame/beefy-mmr/src/weights.rs @@ -1,8 +1,24 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. //! Autogenerated weights for `pallet_beefy_mmr` //! //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 32.0.0 -//! DATE: 2024-07-26, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]` +//! DATE: 2024-07-30, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]` //! WORST CASE MAP SIZE: `1000000` //! HOSTNAME: `serban-ROG-Zephyrus`, CPU: `AMD Ryzen 9 7945HX with Radeon Graphics` //! WASM-EXECUTION: `Compiled`, CHAIN: `None`, DB CACHE: `1024` @@ -20,6 +36,7 @@ // --wasm-execution=Compiled // --heap-pages=4096 // --output=./substrate/frame/beefy-mmr/src/weights.rs +// --header=./substrate/HEADER-APACHE2 // --template=./substrate/.maintain/frame-weight-template.hbs #![cfg_attr(rustfmt, rustfmt_skip)] @@ -32,34 +49,86 @@ use core::marker::PhantomData; /// Weight functions needed for `pallet_beefy_mmr`. pub trait WeightInfo { + fn extract_validation_context() -> Weight; + fn read_peak() -> Weight; fn n_items_proof_is_non_canonical(n: u32, ) -> Weight; } /// Weights for `pallet_beefy_mmr` using the Substrate node and recommended hardware. pub struct SubstrateWeight(PhantomData); impl WeightInfo for SubstrateWeight { - /// The range of component `n` is `[1, 64]`. + /// Storage: `System::BlockHash` (r:1 w:0) + /// Proof: `System::BlockHash` (`max_values`: None, `max_size`: Some(44), added: 2519, mode: `MaxEncodedLen`) + fn extract_validation_context() -> Weight { + // Proof Size summary in bytes: + // Measured: `69` + // Estimated: `3509` + // Minimum execution time: 4_749_000 picoseconds. + Weight::from_parts(5_936_000, 3509) + .saturating_add(T::DbWeight::get().reads(1_u64)) + } + /// Storage: `Mmr::Nodes` (r:1 w:0) + /// Proof: `Mmr::Nodes` (`max_values`: None, `max_size`: Some(40), added: 2515, mode: `MaxEncodedLen`) + fn read_peak() -> Weight { + // Proof Size summary in bytes: + // Measured: `109` + // Estimated: `3505` + // Minimum execution time: 3_561_000 picoseconds. + Weight::from_parts(3_562_000, 3505) + .saturating_add(T::DbWeight::get().reads(1_u64)) + } + /// Storage: `Mmr::RootHash` (r:1 w:0) + /// Proof: `Mmr::RootHash` (`max_values`: Some(1), `max_size`: Some(32), added: 527, mode: `MaxEncodedLen`) + /// Storage: `Mmr::NumberOfLeaves` (r:1 w:0) + /// Proof: `Mmr::NumberOfLeaves` (`max_values`: Some(1), `max_size`: Some(8), added: 503, mode: `MaxEncodedLen`) + /// The range of component `n` is `[2, 512]`. fn n_items_proof_is_non_canonical(n: u32, ) -> Weight { // Proof Size summary in bytes: - // Measured: `0` - // Estimated: `0` - // Minimum execution time: 0_000 picoseconds. - Weight::from_parts(1_187_650, 0) - // Standard Error: 180 - .saturating_add(Weight::from_parts(137, 0).saturating_mul(n.into())) + // Measured: `101` + // Estimated: `1517` + // Minimum execution time: 7_124_000 picoseconds. + Weight::from_parts(15_547_453, 1517) + // Standard Error: 1_661 + .saturating_add(Weight::from_parts(986_446, 0).saturating_mul(n.into())) + .saturating_add(T::DbWeight::get().reads(2_u64)) } } // For backwards compatibility and tests. impl WeightInfo for () { - /// The range of component `n` is `[1, 64]`. + /// Storage: `System::BlockHash` (r:1 w:0) + /// Proof: `System::BlockHash` (`max_values`: None, `max_size`: Some(44), added: 2519, mode: `MaxEncodedLen`) + fn extract_validation_context() -> Weight { + // Proof Size summary in bytes: + // Measured: `69` + // Estimated: `3509` + // Minimum execution time: 4_749_000 picoseconds. + Weight::from_parts(5_936_000, 3509) + .saturating_add(RocksDbWeight::get().reads(1_u64)) + } + /// Storage: `Mmr::Nodes` (r:1 w:0) + /// Proof: `Mmr::Nodes` (`max_values`: None, `max_size`: Some(40), added: 2515, mode: `MaxEncodedLen`) + fn read_peak() -> Weight { + // Proof Size summary in bytes: + // Measured: `109` + // Estimated: `3505` + // Minimum execution time: 3_561_000 picoseconds. + Weight::from_parts(3_562_000, 3505) + .saturating_add(RocksDbWeight::get().reads(1_u64)) + } + /// Storage: `Mmr::RootHash` (r:1 w:0) + /// Proof: `Mmr::RootHash` (`max_values`: Some(1), `max_size`: Some(32), added: 527, mode: `MaxEncodedLen`) + /// Storage: `Mmr::NumberOfLeaves` (r:1 w:0) + /// Proof: `Mmr::NumberOfLeaves` (`max_values`: Some(1), `max_size`: Some(8), added: 503, mode: `MaxEncodedLen`) + /// The range of component `n` is `[2, 512]`. fn n_items_proof_is_non_canonical(n: u32, ) -> Weight { // Proof Size summary in bytes: - // Measured: `0` - // Estimated: `0` - // Minimum execution time: 0_000 picoseconds. - Weight::from_parts(1_187_650, 0) - // Standard Error: 180 - .saturating_add(Weight::from_parts(137, 0).saturating_mul(n.into())) + // Measured: `101` + // Estimated: `1517` + // Minimum execution time: 7_124_000 picoseconds. + Weight::from_parts(15_547_453, 1517) + // Standard Error: 1_661 + .saturating_add(Weight::from_parts(986_446, 0).saturating_mul(n.into())) + .saturating_add(RocksDbWeight::get().reads(2_u64)) } } diff --git a/substrate/frame/beefy/src/mock.rs b/substrate/frame/beefy/src/mock.rs index b1aaef449c5d..5c79d8f7d7d7 100644 --- a/substrate/frame/beefy/src/mock.rs +++ b/substrate/frame/beefy/src/mock.rs @@ -133,7 +133,7 @@ impl AncestryHelper
for MockAncestryHelper { } impl AncestryHelperWeightInfo
for MockAncestryHelper { - fn extract_validation_context(_header: &Header) -> Weight { + fn extract_validation_context() -> Weight { unimplemented!() } diff --git a/substrate/frame/merkle-mountain-range/src/lib.rs b/substrate/frame/merkle-mountain-range/src/lib.rs index 0ab44711bcf5..e2eb552e0ee1 100644 --- a/substrate/frame/merkle-mountain-range/src/lib.rs +++ b/substrate/frame/merkle-mountain-range/src/lib.rs @@ -439,6 +439,14 @@ impl, I: 'static> Pallet { mmr.generate_ancestry_proof(prev_leaf_count) } + #[cfg(feature = "runtime-benchmarks")] + pub fn generate_mock_ancestry_proof() -> Result>, Error> + { + let leaf_count = Self::block_num_to_leaf_count(>::block_number())?; + let mmr: ModuleMmr = mmr::Mmr::new(leaf_count); + mmr.generate_mock_ancestry_proof() + } + pub fn verify_ancestry_proof( root: HashOf, ancestry_proof: primitives::AncestryProof>, diff --git a/substrate/frame/merkle-mountain-range/src/mmr/mmr.rs b/substrate/frame/merkle-mountain-range/src/mmr/mmr.rs index 2b46357c5072..f9a4580b9bb3 100644 --- a/substrate/frame/merkle-mountain-range/src/mmr/mmr.rs +++ b/substrate/frame/merkle-mountain-range/src/mmr/mmr.rs @@ -156,7 +156,7 @@ where } /// Return the internal size of the MMR (number of nodes). - #[cfg(test)] + #[cfg(any(test, feature = "runtime-benchmarks"))] pub fn size(&self) -> NodeIndex { self.mmr.mmr_size() } @@ -252,4 +252,48 @@ where .collect(), }) } + + /// Generate an inflated ancestry proof for the latest leaf in the MMR. + /// + /// The generated proof contains all the leafs in the MMR, so this way we can generate a proof + /// with exactly `leaf_count` items. + #[cfg(feature = "runtime-benchmarks")] + pub fn generate_mock_ancestry_proof( + &self, + ) -> Result>, Error> { + use crate::ModuleMmr; + use alloc::vec; + use sp_mmr_primitives::mmr_lib::helper; + + let mmr: ModuleMmr = Mmr::new(self.leaves); + let store = >::default(); + + let mut prev_peaks = vec![]; + for peak_pos in helper::get_peaks(mmr.size()) { + let peak = store + .get_elem(peak_pos) + .map_err(|_| Error::GenerateProof)? + .ok_or(Error::GenerateProof)? + .hash(); + prev_peaks.push(peak); + } + + let mut proof_items = vec![]; + for leaf_idx in 0..self.leaves { + let leaf_pos = NodesUtils::leaf_index_to_leaf_node_index(leaf_idx); + let leaf = store + .get_elem(leaf_pos) + .map_err(|_| Error::GenerateProof)? + .ok_or(Error::GenerateProof)? + .hash(); + proof_items.push((leaf_pos, leaf)); + } + + Ok(sp_mmr_primitives::AncestryProof { + prev_peaks, + prev_leaf_count: self.leaves, + leaf_count: self.leaves, + items: proof_items, + }) + } } diff --git a/substrate/primitives/consensus/beefy/src/lib.rs b/substrate/primitives/consensus/beefy/src/lib.rs index 9b9352905063..e977fb0ea25f 100644 --- a/substrate/primitives/consensus/beefy/src/lib.rs +++ b/substrate/primitives/consensus/beefy/src/lib.rs @@ -464,7 +464,7 @@ pub trait AncestryHelper { /// Weight information for the logic in `AncestryHelper`. pub trait AncestryHelperWeightInfo: AncestryHelper
{ /// Weight info for the `AncestryHelper::extract_validation_context()` method. - fn extract_validation_context(header: &Header) -> Weight; + fn extract_validation_context() -> Weight; /// Weight info for the `AncestryHelper::is_non_canonical()` method. fn is_non_canonical(proof: &>::Proof) -> Weight; diff --git a/substrate/primitives/merkle-mountain-range/src/utils.rs b/substrate/primitives/merkle-mountain-range/src/utils.rs index 72674e24a272..2460af4b800f 100644 --- a/substrate/primitives/merkle-mountain-range/src/utils.rs +++ b/substrate/primitives/merkle-mountain-range/src/utils.rs @@ -91,7 +91,7 @@ impl NodesUtils { Self::leaf_node_index_to_leaf_index(rightmost_leaf_pos) } - // Translate a _leaf_ `NodeIndex` to its `LeafIndex`. + /// Translate a _leaf_ `NodeIndex` to its `LeafIndex`. fn leaf_node_index_to_leaf_index(pos: NodeIndex) -> LeafIndex { if pos == 0 { return 0 @@ -100,8 +100,13 @@ impl NodesUtils { (pos + peaks.len() as u64) >> 1 } - // Starting from any node position get position of rightmost leaf; this is the leaf - // responsible for the addition of node `pos`. + /// Translate a `LeafIndex` to its _leaf_ `NodeIndex`. + pub fn leaf_index_to_leaf_node_index(leaf_index: NodeIndex) -> LeafIndex { + helper::leaf_index_to_pos(leaf_index) + } + + /// Starting from any node position get position of rightmost leaf; this is the leaf + /// responsible for the addition of node `pos`. fn rightmost_leaf_node_index_from_pos(pos: NodeIndex) -> NodeIndex { pos - (helper::pos_height_in_tree(pos) as u64) } From e6599ea1f61540b31bd827a9d72ef18e924e5c48 Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Tue, 30 Jul 2024 14:42:09 +0300 Subject: [PATCH 05/18] Compute report_fork_voting() weight --- substrate/frame/beefy/src/default_weights.rs | 5 --- substrate/frame/beefy/src/lib.rs | 37 +++++++++++++++++--- substrate/frame/beefy/src/tests.rs | 10 ++++-- 3 files changed, 39 insertions(+), 13 deletions(-) diff --git a/substrate/frame/beefy/src/default_weights.rs b/substrate/frame/beefy/src/default_weights.rs index 70dd3bb02bf1..6b83015459d6 100644 --- a/substrate/frame/beefy/src/default_weights.rs +++ b/substrate/frame/beefy/src/default_weights.rs @@ -57,11 +57,6 @@ impl crate::WeightInfo for () { .saturating_add(DbWeight::get().reads(2)) } - // TODO: Calculate - fn report_fork_voting(_validator_count: u32, _max_nominators_per_validator: u32) -> Weight { - Weight::MAX - } - fn set_new_genesis() -> Weight { DbWeight::get().writes(1) } diff --git a/substrate/frame/beefy/src/lib.rs b/substrate/frame/beefy/src/lib.rs index ab1a7df961e6..821e7c939b55 100644 --- a/substrate/frame/beefy/src/lib.rs +++ b/substrate/frame/beefy/src/lib.rs @@ -294,9 +294,10 @@ pub mod pallet { /// and validate the given key ownership proof against the extracted offender. /// If both are valid, the offence will be reported. #[pallet::call_index(3)] - #[pallet::weight(T::WeightInfo::report_fork_voting( + #[pallet::weight(T::WeightInfo::report_fork_voting::( key_owner_proof.validator_count(), T::MaxNominators::get(), + &equivocation_proof.ancestry_proof ))] pub fn report_fork_voting( origin: OriginFor, @@ -328,9 +329,10 @@ pub mod pallet { /// if the block author is defined it will be defined as the equivocation /// reporter. #[pallet::call_index(4)] - #[pallet::weight(T::WeightInfo::report_fork_voting( + #[pallet::weight(T::WeightInfo::report_fork_voting::( key_owner_proof.validator_count(), T::MaxNominators::get(), + &equivocation_proof.ancestry_proof ))] pub fn report_fork_voting_unsigned( origin: OriginFor, @@ -740,11 +742,36 @@ pub trait WeightInfo { max_nominators_per_validator: u32, ) -> Weight; + fn set_new_genesis() -> Weight; +} + +pub(crate) trait WeightInfoExt: WeightInfo { fn report_double_voting(validator_count: u32, max_nominators_per_validator: u32) -> Weight { Self::report_voting_equivocation(2, validator_count, max_nominators_per_validator) } - fn report_fork_voting(validator_count: u32, max_nominators_per_validator: u32) -> Weight; + fn report_fork_voting( + validator_count: u32, + max_nominators_per_validator: u32, + ancestry_proof: &>>::Proof, + ) -> Weight { + let _weight = >>::extract_validation_context() + .saturating_add( + >>::is_non_canonical( + ancestry_proof, + ), + ) + .saturating_add(Self::report_voting_equivocation( + 1, + validator_count, + max_nominators_per_validator, + )); + + // TODO: return `_weight` here. + // We return `Weight::MAX` currently in order to disallow this extrinsic for the moment. + // We need to check that the proof is optimal. + Weight::MAX + } fn report_future_block_voting( validator_count: u32, @@ -760,6 +787,6 @@ pub trait WeightInfo { max_nominators_per_validator, )) } - - fn set_new_genesis() -> Weight; } + +impl WeightInfoExt for T where T: WeightInfo {} diff --git a/substrate/frame/beefy/src/tests.rs b/substrate/frame/beefy/src/tests.rs index a63b3532b698..d75237205cac 100644 --- a/substrate/frame/beefy/src/tests.rs +++ b/substrate/frame/beefy/src/tests.rs @@ -35,7 +35,7 @@ use sp_consensus_beefy::{ use sp_runtime::DigestItem; use sp_session::MembershipProof; -use crate::{self as beefy, mock::*, Call, Config, Error, WeightInfo}; +use crate::{self as beefy, mock::*, Call, Config, Error, WeightInfoExt}; fn init_block(block: u64) { System::set_block_number(block); @@ -765,7 +765,9 @@ fn report_double_voting_has_valid_weight() { // the weight depends on the size of the validator set, // but there's a lower bound of 100 validators. assert!((1..=100) - .map(|validators| ::WeightInfo::report_double_voting(validators, 1000)) + .map(|validators| <::WeightInfo as WeightInfoExt>::report_double_voting( + validators, 1000 + )) .collect::>() .windows(2) .all(|w| w[0] == w[1])); @@ -773,7 +775,9 @@ fn report_double_voting_has_valid_weight() { // after 100 validators the weight should keep increasing // with every extra validator. assert!((100..=1000) - .map(|validators| ::WeightInfo::report_double_voting(validators, 1000)) + .map(|validators| <::WeightInfo as WeightInfoExt>::report_double_voting( + validators, 1000 + )) .collect::>() .windows(2) .all(|w| w[0].ref_time() < w[1].ref_time())); From 3f8272e0d910da9222761486b3af8eb010f54c46 Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Tue, 30 Jul 2024 18:09:49 +0300 Subject: [PATCH 06/18] Remove impl_benchmark_test_suite!() for the MMR pallet Otherwise the tests will fail with errors related to acessing the offchain DB. That's related to the changes in pre previous commits. I couldn't find another way to fix this. --- substrate/frame/merkle-mountain-range/src/benchmarking.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/substrate/frame/merkle-mountain-range/src/benchmarking.rs b/substrate/frame/merkle-mountain-range/src/benchmarking.rs index 07afd9529eb2..34a8d5912107 100644 --- a/substrate/frame/merkle-mountain-range/src/benchmarking.rs +++ b/substrate/frame/merkle-mountain-range/src/benchmarking.rs @@ -38,6 +38,4 @@ benchmarks_instance_pallet! { } verify { assert_eq!(crate::NumberOfLeaves::::get(), leaves); } - - impl_benchmark_test_suite!(Pallet, crate::tests::new_test_ext(), crate::mock::Test); } From 928201fab7c7044a17cfc91af1409bb97cc80a3e Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Tue, 13 Aug 2024 10:24:58 +0300 Subject: [PATCH 07/18] Update TODO comment Co-authored-by: Branislav Kontur --- substrate/frame/beefy/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/beefy/src/lib.rs b/substrate/frame/beefy/src/lib.rs index 821e7c939b55..cf690a9df339 100644 --- a/substrate/frame/beefy/src/lib.rs +++ b/substrate/frame/beefy/src/lib.rs @@ -767,7 +767,7 @@ pub(crate) trait WeightInfoExt: WeightInfo { max_nominators_per_validator, )); - // TODO: return `_weight` here. + // TODO: https://github.com/paritytech/polkadot-sdk/issues/4523 - return `_weight` here. // We return `Weight::MAX` currently in order to disallow this extrinsic for the moment. // We need to check that the proof is optimal. Weight::MAX From efad090be4227e625ae27be4da1b65ba039d9b3f Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Tue, 13 Aug 2024 13:50:30 +0300 Subject: [PATCH 08/18] Generate weights locally --- polkadot/runtime/rococo/src/lib.rs | 82 +++++++++-------- polkadot/runtime/rococo/src/weights/mod.rs | 1 + .../rococo/src/weights/pallet_beefy_mmr.rs | 88 +++++++++++++++++++ polkadot/runtime/westend/src/lib.rs | 18 ++-- polkadot/runtime/westend/src/weights/mod.rs | 1 + .../westend/src/weights/pallet_beefy_mmr.rs | 88 +++++++++++++++++++ substrate/frame/beefy-mmr/src/lib.rs | 2 +- 7 files changed, 228 insertions(+), 52 deletions(-) create mode 100644 polkadot/runtime/rococo/src/weights/pallet_beefy_mmr.rs create mode 100644 polkadot/runtime/westend/src/weights/pallet_beefy_mmr.rs diff --git a/polkadot/runtime/rococo/src/lib.rs b/polkadot/runtime/rococo/src/lib.rs index ad279c38a708..30b915d32deb 100644 --- a/polkadot/runtime/rococo/src/lib.rs +++ b/polkadot/runtime/rococo/src/lib.rs @@ -1335,7 +1335,7 @@ impl pallet_beefy_mmr::Config for Runtime { type BeefyAuthorityToMerkleLeaf = pallet_beefy_mmr::BeefyEcdsaToEthereum; type LeafExtra = H256; type BeefyDataProvider = ParaHeadsRootProvider; - type WeightInfo = (); + type WeightInfo = weights::pallet_beefy_mmr::WeightInfo; } impl paras_sudo_wrapper::Config for Runtime {} @@ -1630,47 +1630,45 @@ pub mod migrations { /// Unreleased migrations. Add new ones here: pub type Unreleased = ( - pallet_society::migrations::MigrateToV2, - parachains_configuration::migration::v7::MigrateToV7, - assigned_slots::migration::v1::MigrateToV1, - parachains_scheduler::migration::MigrateV1ToV2, - parachains_configuration::migration::v8::MigrateToV8, - parachains_configuration::migration::v9::MigrateToV9, - paras_registrar::migration::MigrateToV1, - pallet_referenda::migration::v1::MigrateV0ToV1, - pallet_referenda::migration::v1::MigrateV0ToV1, - - // Unlock & unreserve Gov1 funds - - pallet_elections_phragmen::migrations::unlock_and_unreserve_all_funds::UnlockAndUnreserveAllFunds, - pallet_democracy::migrations::unlock_and_unreserve_all_funds::UnlockAndUnreserveAllFunds, - pallet_tips::migrations::unreserve_deposits::UnreserveDeposits, - - // Delete all Gov v1 pallet storage key/values. - - frame_support::migrations::RemovePallet::DbWeight>, - frame_support::migrations::RemovePallet::DbWeight>, - frame_support::migrations::RemovePallet::DbWeight>, - frame_support::migrations::RemovePallet::DbWeight>, - frame_support::migrations::RemovePallet::DbWeight>, - frame_support::migrations::RemovePallet::DbWeight>, - - pallet_grandpa::migrations::MigrateV4ToV5, - parachains_configuration::migration::v10::MigrateToV10, - - // Migrate Identity pallet for Usernames - pallet_identity::migration::versioned::V0ToV1, - parachains_configuration::migration::v11::MigrateToV11, - // This needs to come after the `parachains_configuration` above as we are reading the configuration. - coretime::migration::MigrateToCoretime, - parachains_configuration::migration::v12::MigrateToV12, - parachains_on_demand::migration::MigrateV0ToV1, - - // permanent - pallet_xcm::migration::MigrateToLatestXcmVersion, - - parachains_inclusion::migration::MigrateToV1, - ); + pallet_society::migrations::MigrateToV2, + parachains_configuration::migration::v7::MigrateToV7, + assigned_slots::migration::v1::MigrateToV1, + parachains_scheduler::migration::MigrateV1ToV2, + parachains_configuration::migration::v8::MigrateToV8, + parachains_configuration::migration::v9::MigrateToV9, + paras_registrar::migration::MigrateToV1, + pallet_referenda::migration::v1::MigrateV0ToV1, + pallet_referenda::migration::v1::MigrateV0ToV1, + + // Unlock & unreserve Gov1 funds + + pallet_elections_phragmen::migrations::unlock_and_unreserve_all_funds::UnlockAndUnreserveAllFunds, + pallet_democracy::migrations::unlock_and_unreserve_all_funds::UnlockAndUnreserveAllFunds, + pallet_tips::migrations::unreserve_deposits::UnreserveDeposits, + + // Delete all Gov v1 pallet storage key/values. + + frame_support::migrations::RemovePallet::DbWeight>, + frame_support::migrations::RemovePallet::DbWeight>, + frame_support::migrations::RemovePallet::DbWeight>, + frame_support::migrations::RemovePallet::DbWeight>, + frame_support::migrations::RemovePallet::DbWeight>, + frame_support::migrations::RemovePallet::DbWeight>, + pallet_grandpa::migrations::MigrateV4ToV5, + parachains_configuration::migration::v10::MigrateToV10, + + // Migrate Identity pallet for Usernames + pallet_identity::migration::versioned::V0ToV1, + parachains_configuration::migration::v11::MigrateToV11, + // This needs to come after the `parachains_configuration` above as we are reading the configuration. + coretime::migration::MigrateToCoretime, + parachains_configuration::migration::v12::MigrateToV12, + parachains_on_demand::migration::MigrateV0ToV1, + + // permanent + pallet_xcm::migration::MigrateToLatestXcmVersion, + parachains_inclusion::migration::MigrateToV1, + ); } /// Executive: handles dispatch to the various modules. diff --git a/polkadot/runtime/rococo/src/weights/mod.rs b/polkadot/runtime/rococo/src/weights/mod.rs index 0512a393a6c4..cd3f4689f562 100644 --- a/polkadot/runtime/rococo/src/weights/mod.rs +++ b/polkadot/runtime/rococo/src/weights/mod.rs @@ -19,6 +19,7 @@ pub mod frame_system; pub mod pallet_asset_rate; pub mod pallet_balances_balances; pub mod pallet_balances_nis_counterpart_balances; +pub mod pallet_beefy_mmr; pub mod pallet_bounties; pub mod pallet_child_bounties; pub mod pallet_conviction_voting; diff --git a/polkadot/runtime/rococo/src/weights/pallet_beefy_mmr.rs b/polkadot/runtime/rococo/src/weights/pallet_beefy_mmr.rs new file mode 100644 index 000000000000..9ea0395c98f4 --- /dev/null +++ b/polkadot/runtime/rococo/src/weights/pallet_beefy_mmr.rs @@ -0,0 +1,88 @@ +// Copyright (C) Parity Technologies (UK) Ltd. +// This file is part of Polkadot. + +// Polkadot is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Polkadot is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Polkadot. If not, see . + +//! Autogenerated weights for `pallet_beefy_mmr` +//! +//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 32.0.0 +//! DATE: 2024-08-13, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]` +//! WORST CASE MAP SIZE: `1000000` +//! HOSTNAME: `serban-ROG-Zephyrus`, CPU: `AMD Ryzen 9 7945HX with Radeon Graphics` +//! WASM-EXECUTION: `Compiled`, CHAIN: `Some("rococo-dev")`, DB CACHE: 1024 + +// Executed Command: +// target/production/polkadot +// benchmark +// pallet +// --steps=50 +// --repeat=20 +// --extrinsic=* +// --wasm-execution=compiled +// --heap-pages=4096 +// --pallet=pallet_beefy_mmr +// --chain=rococo-dev +// --header=./polkadot/file_header.txt +// --output=./polkadot/runtime/rococo/src/weights/ + +#![cfg_attr(rustfmt, rustfmt_skip)] +#![allow(unused_parens)] +#![allow(unused_imports)] +#![allow(missing_docs)] + +use frame_support::{traits::Get, weights::Weight}; +use core::marker::PhantomData; + +/// Weight functions for `pallet_beefy_mmr`. +pub struct WeightInfo(PhantomData); +impl pallet_beefy_mmr::WeightInfo for WeightInfo { + /// Storage: `System::BlockHash` (r:1 w:0) + /// Proof: `System::BlockHash` (`max_values`: None, `max_size`: Some(44), added: 2519, mode: `MaxEncodedLen`) + fn extract_validation_context() -> Weight { + // Proof Size summary in bytes: + // Measured: `92` + // Estimated: `3509` + // Minimum execution time: 4_749_000 picoseconds. + Weight::from_parts(4_749_000, 0) + .saturating_add(Weight::from_parts(0, 3509)) + .saturating_add(T::DbWeight::get().reads(1)) + } + /// Storage: `Mmr::Nodes` (r:1 w:0) + /// Proof: `Mmr::Nodes` (`max_values`: None, `max_size`: Some(40), added: 2515, mode: `MaxEncodedLen`) + fn read_peak() -> Weight { + // Proof Size summary in bytes: + // Measured: `234` + // Estimated: `3505` + // Minimum execution time: 3_562_000 picoseconds. + Weight::from_parts(3_562_000, 0) + .saturating_add(Weight::from_parts(0, 3505)) + .saturating_add(T::DbWeight::get().reads(1)) + } + /// Storage: `Mmr::RootHash` (r:1 w:0) + /// Proof: `Mmr::RootHash` (`max_values`: Some(1), `max_size`: Some(32), added: 527, mode: `MaxEncodedLen`) + /// Storage: `Mmr::NumberOfLeaves` (r:1 w:0) + /// Proof: `Mmr::NumberOfLeaves` (`max_values`: Some(1), `max_size`: Some(8), added: 503, mode: `MaxEncodedLen`) + /// The range of component `n` is `[2, 512]`. + fn n_items_proof_is_non_canonical(n: u32, ) -> Weight { + // Proof Size summary in bytes: + // Measured: `226` + // Estimated: `1517` + // Minimum execution time: 7_123_000 picoseconds. + Weight::from_parts(14_261_804, 0) + .saturating_add(Weight::from_parts(0, 1517)) + // Standard Error: 1_247 + .saturating_add(Weight::from_parts(891_144, 0).saturating_mul(n.into())) + .saturating_add(T::DbWeight::get().reads(2)) + } +} diff --git a/polkadot/runtime/westend/src/lib.rs b/polkadot/runtime/westend/src/lib.rs index 6f8f6af43e40..aa446b03368d 100644 --- a/polkadot/runtime/westend/src/lib.rs +++ b/polkadot/runtime/westend/src/lib.rs @@ -461,7 +461,7 @@ impl pallet_beefy_mmr::Config for Runtime { type BeefyAuthorityToMerkleLeaf = pallet_beefy_mmr::BeefyEcdsaToEthereum; type LeafExtra = H256; type BeefyDataProvider = ParaHeadsRootProvider; - type WeightInfo = (); + type WeightInfo = weights::pallet_beefy_mmr::WeightInfo; } parameter_types! { @@ -602,20 +602,20 @@ impl pallet_election_provider_multi_phase::MinerConfig for Runtime { type MaxWeight = OffchainSolutionWeightLimit; type Solution = NposCompactSolution16; type MaxVotesPerVoter = < - ::DataProvider - as - frame_election_provider_support::ElectionDataProvider - >::MaxVotesPerVoter; + ::DataProvider + as + frame_election_provider_support::ElectionDataProvider + >::MaxVotesPerVoter; type MaxWinners = MaxActiveValidators; // The unsigned submissions have to respect the weight of the submit_unsigned call, thus their // weight estimate function is wired to this call's weight. fn solution_weight(v: u32, t: u32, a: u32, d: u32) -> Weight { < - ::WeightInfo - as - pallet_election_provider_multi_phase::WeightInfo - >::submit_unsigned(v, t, a, d) + ::WeightInfo + as + pallet_election_provider_multi_phase::WeightInfo + >::submit_unsigned(v, t, a, d) } } diff --git a/polkadot/runtime/westend/src/weights/mod.rs b/polkadot/runtime/westend/src/weights/mod.rs index 2248e421e639..cb6e2c85e363 100644 --- a/polkadot/runtime/westend/src/weights/mod.rs +++ b/polkadot/runtime/westend/src/weights/mod.rs @@ -20,6 +20,7 @@ pub mod frame_system; pub mod pallet_asset_rate; pub mod pallet_bags_list; pub mod pallet_balances; +pub mod pallet_beefy_mmr; pub mod pallet_conviction_voting; pub mod pallet_election_provider_multi_phase; pub mod pallet_fast_unstake; diff --git a/polkadot/runtime/westend/src/weights/pallet_beefy_mmr.rs b/polkadot/runtime/westend/src/weights/pallet_beefy_mmr.rs new file mode 100644 index 000000000000..496944e51d38 --- /dev/null +++ b/polkadot/runtime/westend/src/weights/pallet_beefy_mmr.rs @@ -0,0 +1,88 @@ +// Copyright (C) Parity Technologies (UK) Ltd. +// This file is part of Polkadot. + +// Polkadot is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Polkadot is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Polkadot. If not, see . + +//! Autogenerated weights for `pallet_beefy_mmr` +//! +//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 32.0.0 +//! DATE: 2024-08-13, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]` +//! WORST CASE MAP SIZE: `1000000` +//! HOSTNAME: `serban-ROG-Zephyrus`, CPU: `AMD Ryzen 9 7945HX with Radeon Graphics` +//! WASM-EXECUTION: `Compiled`, CHAIN: `Some("westend-dev")`, DB CACHE: 1024 + +// Executed Command: +// target/production/polkadot +// benchmark +// pallet +// --steps=50 +// --repeat=20 +// --extrinsic=* +// --wasm-execution=compiled +// --heap-pages=4096 +// --pallet=pallet_beefy_mmr +// --chain=westend-dev +// --header=./polkadot/file_header.txt +// --output=./polkadot/runtime/westend/src/weights/ + +#![cfg_attr(rustfmt, rustfmt_skip)] +#![allow(unused_parens)] +#![allow(unused_imports)] +#![allow(missing_docs)] + +use frame_support::{traits::Get, weights::Weight}; +use core::marker::PhantomData; + +/// Weight functions for `pallet_beefy_mmr`. +pub struct WeightInfo(PhantomData); +impl pallet_beefy_mmr::WeightInfo for WeightInfo { + /// Storage: `System::BlockHash` (r:1 w:0) + /// Proof: `System::BlockHash` (`max_values`: None, `max_size`: Some(44), added: 2519, mode: `MaxEncodedLen`) + fn extract_validation_context() -> Weight { + // Proof Size summary in bytes: + // Measured: `92` + // Estimated: `3509` + // Minimum execution time: 4_749_000 picoseconds. + Weight::from_parts(4_750_000, 0) + .saturating_add(Weight::from_parts(0, 3509)) + .saturating_add(T::DbWeight::get().reads(1)) + } + /// Storage: `Mmr::Nodes` (r:1 w:0) + /// Proof: `Mmr::Nodes` (`max_values`: None, `max_size`: Some(40), added: 2515, mode: `MaxEncodedLen`) + fn read_peak() -> Weight { + // Proof Size summary in bytes: + // Measured: `201` + // Estimated: `3505` + // Minimum execution time: 3_562_000 picoseconds. + Weight::from_parts(4_749_000, 0) + .saturating_add(Weight::from_parts(0, 3505)) + .saturating_add(T::DbWeight::get().reads(1)) + } + /// Storage: `Mmr::RootHash` (r:1 w:0) + /// Proof: `Mmr::RootHash` (`max_values`: Some(1), `max_size`: Some(32), added: 527, mode: `MaxEncodedLen`) + /// Storage: `Mmr::NumberOfLeaves` (r:1 w:0) + /// Proof: `Mmr::NumberOfLeaves` (`max_values`: Some(1), `max_size`: Some(8), added: 503, mode: `MaxEncodedLen`) + /// The range of component `n` is `[2, 512]`. + fn n_items_proof_is_non_canonical(n: u32, ) -> Weight { + // Proof Size summary in bytes: + // Measured: `193` + // Estimated: `1517` + // Minimum execution time: 7_123_000 picoseconds. + Weight::from_parts(11_878_187, 0) + .saturating_add(Weight::from_parts(0, 1517)) + // Standard Error: 1_296 + .saturating_add(Weight::from_parts(855_192, 0).saturating_mul(n.into())) + .saturating_add(T::DbWeight::get().reads(2)) + } +} diff --git a/substrate/frame/beefy-mmr/src/lib.rs b/substrate/frame/beefy-mmr/src/lib.rs index f41e2f027ffc..73119c3faa9b 100644 --- a/substrate/frame/beefy-mmr/src/lib.rs +++ b/substrate/frame/beefy-mmr/src/lib.rs @@ -55,7 +55,7 @@ use frame_support::{crypto::ecdsa::ECDSAExt, pallet_prelude::Weight, traits::Get use frame_system::pallet_prelude::{BlockNumberFor, HeaderFor}; pub use pallet::*; -use weights::WeightInfo; +pub use weights::WeightInfo; mod benchmarking; #[cfg(test)] From e1a11408a9e0f328e1d1da7f43a81d57b91fcb0a Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Tue, 13 Aug 2024 14:07:03 +0300 Subject: [PATCH 09/18] Add prdoc --- prdoc/pr_5188.prdoc | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 prdoc/pr_5188.prdoc diff --git a/prdoc/pr_5188.prdoc b/prdoc/pr_5188.prdoc new file mode 100644 index 000000000000..b2ab9ff6653b --- /dev/null +++ b/prdoc/pr_5188.prdoc @@ -0,0 +1,32 @@ +# 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: Added benchmarks for BEEFY fork voting + +doc: + - audience: + - Runtime Dev + - Runtime User + description: | + This PR adds benchmarks for `report_fork_voting` and `report_future_voting` extrinsics to `pallet-beefy`. + `report_future_voting` can be called now. `report_fork_voting` can't be called yet. Even though we have added + the formula for computing its weight, we still use `Weight::MAX`. We will set the proper weight in a future PR. + In order to do this we need to also check that the ancestry proof is optimal. + The PR adds a `WeightInfo` associated trait to the `pallet_beefy_mmr::Config` and defines benchmarks for + `pallet_beefy_mmr`. + +crates: + - name: pallet-mmr + bump: minor + - name: sp-mmr-primitives + bump: minor + - name: sp-consensus-beefy + bump: minor + - name: rococo-runtime + bump: minor + - name: pallet-beefy + bump: major + - name: pallet-beefy-mmr + bump: major + - name: westend-runtime + bump: minor From 9270d80c28b4bc273d33956e483066bed62bb027 Mon Sep 17 00:00:00 2001 From: command-bot <> Date: Tue, 13 Aug 2024 11:58:43 +0000 Subject: [PATCH 10/18] ".git/.scripts/commands/bench/bench.sh" --subcommand=pallet --runtime=rococo --target_dir=polkadot --pallet=pallet_beefy_mmr --- .../rococo/src/weights/pallet_beefy_mmr.rs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/polkadot/runtime/rococo/src/weights/pallet_beefy_mmr.rs b/polkadot/runtime/rococo/src/weights/pallet_beefy_mmr.rs index 9ea0395c98f4..317c9149ec6c 100644 --- a/polkadot/runtime/rococo/src/weights/pallet_beefy_mmr.rs +++ b/polkadot/runtime/rococo/src/weights/pallet_beefy_mmr.rs @@ -19,7 +19,7 @@ //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 32.0.0 //! DATE: 2024-08-13, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]` //! WORST CASE MAP SIZE: `1000000` -//! HOSTNAME: `serban-ROG-Zephyrus`, CPU: `AMD Ryzen 9 7945HX with Radeon Graphics` +//! HOSTNAME: `runner-696hpswk-project-674-concurrent-0`, CPU: `Intel(R) Xeon(R) CPU @ 2.60GHz` //! WASM-EXECUTION: `Compiled`, CHAIN: `Some("rococo-dev")`, DB CACHE: 1024 // Executed Command: @@ -31,6 +31,7 @@ // --extrinsic=* // --wasm-execution=compiled // --heap-pages=4096 +// --json-file=/builds/parity/mirrors/polkadot-sdk/.git/.artifacts/bench.json // --pallet=pallet_beefy_mmr // --chain=rococo-dev // --header=./polkadot/file_header.txt @@ -53,8 +54,8 @@ impl pallet_beefy_mmr::WeightInfo for WeightInfo { // Proof Size summary in bytes: // Measured: `92` // Estimated: `3509` - // Minimum execution time: 4_749_000 picoseconds. - Weight::from_parts(4_749_000, 0) + // Minimum execution time: 7_116_000 picoseconds. + Weight::from_parts(7_343_000, 0) .saturating_add(Weight::from_parts(0, 3509)) .saturating_add(T::DbWeight::get().reads(1)) } @@ -64,8 +65,8 @@ impl pallet_beefy_mmr::WeightInfo for WeightInfo { // Proof Size summary in bytes: // Measured: `234` // Estimated: `3505` - // Minimum execution time: 3_562_000 picoseconds. - Weight::from_parts(3_562_000, 0) + // Minimum execution time: 5_652_000 picoseconds. + Weight::from_parts(5_963_000, 0) .saturating_add(Weight::from_parts(0, 3505)) .saturating_add(T::DbWeight::get().reads(1)) } @@ -78,11 +79,11 @@ impl pallet_beefy_mmr::WeightInfo for WeightInfo { // Proof Size summary in bytes: // Measured: `226` // Estimated: `1517` - // Minimum execution time: 7_123_000 picoseconds. - Weight::from_parts(14_261_804, 0) + // Minimum execution time: 11_953_000 picoseconds. + Weight::from_parts(15_978_891, 0) .saturating_add(Weight::from_parts(0, 1517)) - // Standard Error: 1_247 - .saturating_add(Weight::from_parts(891_144, 0).saturating_mul(n.into())) + // Standard Error: 1_780 + .saturating_add(Weight::from_parts(1_480_582, 0).saturating_mul(n.into())) .saturating_add(T::DbWeight::get().reads(2)) } } From be8a25a780f04e0b0365bdc10c6e5c56c23e2518 Mon Sep 17 00:00:00 2001 From: command-bot <> Date: Tue, 13 Aug 2024 11:59:00 +0000 Subject: [PATCH 11/18] ".git/.scripts/commands/bench/bench.sh" --subcommand=pallet --runtime=westend --target_dir=polkadot --pallet=pallet_beefy_mmr --- .../westend/src/weights/pallet_beefy_mmr.rs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/polkadot/runtime/westend/src/weights/pallet_beefy_mmr.rs b/polkadot/runtime/westend/src/weights/pallet_beefy_mmr.rs index 496944e51d38..5be207e3fcff 100644 --- a/polkadot/runtime/westend/src/weights/pallet_beefy_mmr.rs +++ b/polkadot/runtime/westend/src/weights/pallet_beefy_mmr.rs @@ -19,7 +19,7 @@ //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 32.0.0 //! DATE: 2024-08-13, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]` //! WORST CASE MAP SIZE: `1000000` -//! HOSTNAME: `serban-ROG-Zephyrus`, CPU: `AMD Ryzen 9 7945HX with Radeon Graphics` +//! HOSTNAME: `runner-696hpswk-project-674-concurrent-0`, CPU: `Intel(R) Xeon(R) CPU @ 2.60GHz` //! WASM-EXECUTION: `Compiled`, CHAIN: `Some("westend-dev")`, DB CACHE: 1024 // Executed Command: @@ -31,6 +31,7 @@ // --extrinsic=* // --wasm-execution=compiled // --heap-pages=4096 +// --json-file=/builds/parity/mirrors/polkadot-sdk/.git/.artifacts/bench.json // --pallet=pallet_beefy_mmr // --chain=westend-dev // --header=./polkadot/file_header.txt @@ -53,8 +54,8 @@ impl pallet_beefy_mmr::WeightInfo for WeightInfo { // Proof Size summary in bytes: // Measured: `92` // Estimated: `3509` - // Minimum execution time: 4_749_000 picoseconds. - Weight::from_parts(4_750_000, 0) + // Minimum execution time: 7_850_000 picoseconds. + Weight::from_parts(8_169_000, 0) .saturating_add(Weight::from_parts(0, 3509)) .saturating_add(T::DbWeight::get().reads(1)) } @@ -64,8 +65,8 @@ impl pallet_beefy_mmr::WeightInfo for WeightInfo { // Proof Size summary in bytes: // Measured: `201` // Estimated: `3505` - // Minimum execution time: 3_562_000 picoseconds. - Weight::from_parts(4_749_000, 0) + // Minimum execution time: 6_852_000 picoseconds. + Weight::from_parts(7_448_000, 0) .saturating_add(Weight::from_parts(0, 3505)) .saturating_add(T::DbWeight::get().reads(1)) } @@ -78,11 +79,11 @@ impl pallet_beefy_mmr::WeightInfo for WeightInfo { // Proof Size summary in bytes: // Measured: `193` // Estimated: `1517` - // Minimum execution time: 7_123_000 picoseconds. - Weight::from_parts(11_878_187, 0) + // Minimum execution time: 12_860_000 picoseconds. + Weight::from_parts(17_158_162, 0) .saturating_add(Weight::from_parts(0, 1517)) - // Standard Error: 1_296 - .saturating_add(Weight::from_parts(855_192, 0).saturating_mul(n.into())) + // Standard Error: 1_732 + .saturating_add(Weight::from_parts(1_489_410, 0).saturating_mul(n.into())) .saturating_add(T::DbWeight::get().reads(2)) } } From 4a71e7208c1f60949c21f8dcc6ba9ddce05e86c0 Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Tue, 13 Aug 2024 15:27:57 +0300 Subject: [PATCH 12/18] Add pallet_beefy_mmr benchmarks to kitchensink --- substrate/bin/node/runtime/src/lib.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index 1b3d1228800a..f1650820a240 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -820,17 +820,17 @@ impl pallet_election_provider_multi_phase::MinerConfig for Runtime { type MaxWeight = MinerMaxWeight; type Solution = NposSolution16; type MaxVotesPerVoter = - <::DataProvider as ElectionDataProvider>::MaxVotesPerVoter; + <::DataProvider as ElectionDataProvider>::MaxVotesPerVoter; type MaxWinners = MaxActiveValidators; // The unsigned submissions have to respect the weight of the submit_unsigned call, thus their // weight estimate function is wired to this call's weight. fn solution_weight(v: u32, t: u32, a: u32, d: u32) -> Weight { < - ::WeightInfo - as - pallet_election_provider_multi_phase::WeightInfo - >::submit_unsigned(v, t, a, d) + ::WeightInfo + as + pallet_election_provider_multi_phase::WeightInfo + >::submit_unsigned(v, t, a, d) } } @@ -2206,7 +2206,7 @@ impl EnsureOriginWithArg for DynamicParamet match key { RuntimeParametersKey::Storage(_) => { frame_system::ensure_root(origin.clone()).map_err(|_| origin)?; - return Ok(()) + return Ok(()); }, } } @@ -2586,6 +2586,7 @@ mod benches { [pallet_babe, Babe] [pallet_bags_list, VoterList] [pallet_balances, Balances] + [pallet_beefy_mmr, MmrLeaf] [pallet_bounties, Bounties] [pallet_broker, Broker] [pallet_child_bounties, ChildBounties] @@ -3320,7 +3321,7 @@ mod tests { #[test] fn perbill_as_onchain_accuracy() { type OnChainAccuracy = - <::Solution as NposSolution>::Accuracy; + <::Solution as NposSolution>::Accuracy; let maximum_chain_accuracy: Vec> = (0..MaxNominations::get()) .map(|_| >::from(OnChainAccuracy::one().deconstruct())) .collect(); From d1fcb6e0de4539738c0ca626202f7b99f5f60f8d Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Tue, 13 Aug 2024 16:21:18 +0300 Subject: [PATCH 13/18] bot bench substrate-pallet --pallet=pallet_beefy_mmr --- substrate/frame/beefy-mmr/src/weights.rs | 62 ++++++++++++------------ 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/substrate/frame/beefy-mmr/src/weights.rs b/substrate/frame/beefy-mmr/src/weights.rs index d870752d248b..c292f25400cc 100644 --- a/substrate/frame/beefy-mmr/src/weights.rs +++ b/substrate/frame/beefy-mmr/src/weights.rs @@ -18,25 +18,25 @@ //! Autogenerated weights for `pallet_beefy_mmr` //! //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 32.0.0 -//! DATE: 2024-07-30, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]` +//! DATE: 2024-08-13, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]` //! WORST CASE MAP SIZE: `1000000` -//! HOSTNAME: `serban-ROG-Zephyrus`, CPU: `AMD Ryzen 9 7945HX with Radeon Graphics` -//! WASM-EXECUTION: `Compiled`, CHAIN: `None`, DB CACHE: `1024` +//! HOSTNAME: `runner-696hpswk-project-674-concurrent-0`, CPU: `Intel(R) Xeon(R) CPU @ 2.60GHz` +//! WASM-EXECUTION: `Compiled`, CHAIN: `Some("dev")`, DB CACHE: `1024` // Executed Command: -// target/release/polkadot +// target/production/substrate-node // benchmark // pallet -// --runtime -// ./target/release/wbuild/rococo-runtime/rococo_runtime.wasm // --steps=50 // --repeat=20 -// --pallet=pallet_beefy_mmr // --extrinsic=* -// --wasm-execution=Compiled +// --wasm-execution=compiled // --heap-pages=4096 -// --output=./substrate/frame/beefy-mmr/src/weights.rs +// --json-file=/builds/parity/mirrors/polkadot-sdk/.git/.artifacts/bench.json +// --pallet=pallet_beefy_mmr +// --chain=dev // --header=./substrate/HEADER-APACHE2 +// --output=./substrate/frame/beefy-mmr/src/weights.rs // --template=./substrate/.maintain/frame-weight-template.hbs #![cfg_attr(rustfmt, rustfmt_skip)] @@ -61,20 +61,20 @@ impl WeightInfo for SubstrateWeight { /// Proof: `System::BlockHash` (`max_values`: None, `max_size`: Some(44), added: 2519, mode: `MaxEncodedLen`) fn extract_validation_context() -> Weight { // Proof Size summary in bytes: - // Measured: `69` + // Measured: `92` // Estimated: `3509` - // Minimum execution time: 4_749_000 picoseconds. - Weight::from_parts(5_936_000, 3509) + // Minimum execution time: 7_461_000 picoseconds. + Weight::from_parts(7_669_000, 3509) .saturating_add(T::DbWeight::get().reads(1_u64)) } /// Storage: `Mmr::Nodes` (r:1 w:0) /// Proof: `Mmr::Nodes` (`max_values`: None, `max_size`: Some(40), added: 2515, mode: `MaxEncodedLen`) fn read_peak() -> Weight { // Proof Size summary in bytes: - // Measured: `109` + // Measured: `333` // Estimated: `3505` - // Minimum execution time: 3_561_000 picoseconds. - Weight::from_parts(3_562_000, 3505) + // Minimum execution time: 6_137_000 picoseconds. + Weight::from_parts(6_423_000, 3505) .saturating_add(T::DbWeight::get().reads(1_u64)) } /// Storage: `Mmr::RootHash` (r:1 w:0) @@ -84,12 +84,12 @@ impl WeightInfo for SubstrateWeight { /// The range of component `n` is `[2, 512]`. fn n_items_proof_is_non_canonical(n: u32, ) -> Weight { // Proof Size summary in bytes: - // Measured: `101` + // Measured: `325` // Estimated: `1517` - // Minimum execution time: 7_124_000 picoseconds. - Weight::from_parts(15_547_453, 1517) - // Standard Error: 1_661 - .saturating_add(Weight::from_parts(986_446, 0).saturating_mul(n.into())) + // Minimum execution time: 10_687_000 picoseconds. + Weight::from_parts(14_851_626, 1517) + // Standard Error: 1_455 + .saturating_add(Weight::from_parts(961_703, 0).saturating_mul(n.into())) .saturating_add(T::DbWeight::get().reads(2_u64)) } } @@ -100,20 +100,20 @@ impl WeightInfo for () { /// Proof: `System::BlockHash` (`max_values`: None, `max_size`: Some(44), added: 2519, mode: `MaxEncodedLen`) fn extract_validation_context() -> Weight { // Proof Size summary in bytes: - // Measured: `69` + // Measured: `92` // Estimated: `3509` - // Minimum execution time: 4_749_000 picoseconds. - Weight::from_parts(5_936_000, 3509) + // Minimum execution time: 7_461_000 picoseconds. + Weight::from_parts(7_669_000, 3509) .saturating_add(RocksDbWeight::get().reads(1_u64)) } /// Storage: `Mmr::Nodes` (r:1 w:0) /// Proof: `Mmr::Nodes` (`max_values`: None, `max_size`: Some(40), added: 2515, mode: `MaxEncodedLen`) fn read_peak() -> Weight { // Proof Size summary in bytes: - // Measured: `109` + // Measured: `333` // Estimated: `3505` - // Minimum execution time: 3_561_000 picoseconds. - Weight::from_parts(3_562_000, 3505) + // Minimum execution time: 6_137_000 picoseconds. + Weight::from_parts(6_423_000, 3505) .saturating_add(RocksDbWeight::get().reads(1_u64)) } /// Storage: `Mmr::RootHash` (r:1 w:0) @@ -123,12 +123,12 @@ impl WeightInfo for () { /// The range of component `n` is `[2, 512]`. fn n_items_proof_is_non_canonical(n: u32, ) -> Weight { // Proof Size summary in bytes: - // Measured: `101` + // Measured: `325` // Estimated: `1517` - // Minimum execution time: 7_124_000 picoseconds. - Weight::from_parts(15_547_453, 1517) - // Standard Error: 1_661 - .saturating_add(Weight::from_parts(986_446, 0).saturating_mul(n.into())) + // Minimum execution time: 10_687_000 picoseconds. + Weight::from_parts(14_851_626, 1517) + // Standard Error: 1_455 + .saturating_add(Weight::from_parts(961_703, 0).saturating_mul(n.into())) .saturating_add(RocksDbWeight::get().reads(2_u64)) } } From 6b171885123426712a30d3d9bce236183e84f859 Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Tue, 13 Aug 2024 16:44:04 +0300 Subject: [PATCH 14/18] Revert "Add pallet_beefy_mmr benchmarks to kitchensink" This reverts commit 4a71e7208c1f60949c21f8dcc6ba9ddce05e86c0. --- substrate/bin/node/runtime/src/lib.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index f1650820a240..1b3d1228800a 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -820,17 +820,17 @@ impl pallet_election_provider_multi_phase::MinerConfig for Runtime { type MaxWeight = MinerMaxWeight; type Solution = NposSolution16; type MaxVotesPerVoter = - <::DataProvider as ElectionDataProvider>::MaxVotesPerVoter; + <::DataProvider as ElectionDataProvider>::MaxVotesPerVoter; type MaxWinners = MaxActiveValidators; // The unsigned submissions have to respect the weight of the submit_unsigned call, thus their // weight estimate function is wired to this call's weight. fn solution_weight(v: u32, t: u32, a: u32, d: u32) -> Weight { < - ::WeightInfo - as - pallet_election_provider_multi_phase::WeightInfo - >::submit_unsigned(v, t, a, d) + ::WeightInfo + as + pallet_election_provider_multi_phase::WeightInfo + >::submit_unsigned(v, t, a, d) } } @@ -2206,7 +2206,7 @@ impl EnsureOriginWithArg for DynamicParamet match key { RuntimeParametersKey::Storage(_) => { frame_system::ensure_root(origin.clone()).map_err(|_| origin)?; - return Ok(()); + return Ok(()) }, } } @@ -2586,7 +2586,6 @@ mod benches { [pallet_babe, Babe] [pallet_bags_list, VoterList] [pallet_balances, Balances] - [pallet_beefy_mmr, MmrLeaf] [pallet_bounties, Bounties] [pallet_broker, Broker] [pallet_child_bounties, ChildBounties] @@ -3321,7 +3320,7 @@ mod tests { #[test] fn perbill_as_onchain_accuracy() { type OnChainAccuracy = - <::Solution as NposSolution>::Accuracy; + <::Solution as NposSolution>::Accuracy; let maximum_chain_accuracy: Vec> = (0..MaxNominations::get()) .map(|_| >::from(OnChainAccuracy::one().deconstruct())) .collect(); From 9fab84aa2c3eb72395821d77e9804c576dd117ee Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Tue, 13 Aug 2024 19:45:27 +0300 Subject: [PATCH 15/18] Call local_storage_set only in offline context --- substrate/frame/merkle-mountain-range/src/mmr/storage.rs | 6 +++++- substrate/primitives/io/src/lib.rs | 5 +++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/substrate/frame/merkle-mountain-range/src/mmr/storage.rs b/substrate/frame/merkle-mountain-range/src/mmr/storage.rs index 948a148c74a4..cdaa497f3e9c 100644 --- a/substrate/frame/merkle-mountain-range/src/mmr/storage.rs +++ b/substrate/frame/merkle-mountain-range/src/mmr/storage.rs @@ -58,7 +58,11 @@ impl OffchainStorage { #[cfg(feature = "runtime-benchmarks")] fn set(key: &[u8], value: &[u8]) { - sp_io::offchain::local_storage_set(StorageKind::PERSISTENT, key, value); + if sp_io::offchain::is_offchain_db_ext_available() { + sp_io::offchain::local_storage_set(StorageKind::PERSISTENT, key, value); + } else { + sp_io::offchain_index::set(key, value); + } } } diff --git a/substrate/primitives/io/src/lib.rs b/substrate/primitives/io/src/lib.rs index 38cf72a0c790..8a7875cb9217 100644 --- a/substrate/primitives/io/src/lib.rs +++ b/substrate/primitives/io/src/lib.rs @@ -1343,6 +1343,11 @@ sp_externalities::decl_extension! { /// These functions are being made available to the runtime and are called by the runtime. #[runtime_interface] pub trait Offchain { + /// Checks if the offchain database extension is available. + fn is_offchain_db_ext_available(&mut self) -> bool { + self.extension::().is_some() + } + /// Returns if the local node is a potential validator. /// /// Even if this function returns `true`, it does not mean that any keys are configured From 1454f87ede244265a77635a572460189ef31435d Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Tue, 13 Aug 2024 19:45:45 +0300 Subject: [PATCH 16/18] improvements --- substrate/bin/node/runtime/src/lib.rs | 1 + substrate/frame/beefy-mmr/src/benchmarking.rs | 14 ++++++++++---- substrate/frame/beefy-mmr/src/mock.rs | 8 +++++++- substrate/frame/beefy-mmr/src/tests.rs | 15 +-------------- .../merkle-mountain-range/src/benchmarking.rs | 2 ++ 5 files changed, 21 insertions(+), 19 deletions(-) diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index 1b3d1228800a..ff3f7e26baf2 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -2586,6 +2586,7 @@ mod benches { [pallet_babe, Babe] [pallet_bags_list, VoterList] [pallet_balances, Balances] + [pallet_beefy_mmr, MmrLeaf] [pallet_bounties, Bounties] [pallet_broker, Broker] [pallet_child_bounties, ChildBounties] diff --git a/substrate/frame/beefy-mmr/src/benchmarking.rs b/substrate/frame/beefy-mmr/src/benchmarking.rs index 680b560b8748..b87802fad5c0 100644 --- a/substrate/frame/beefy-mmr/src/benchmarking.rs +++ b/substrate/frame/beefy-mmr/src/benchmarking.rs @@ -27,7 +27,7 @@ use frame_support::traits::Hooks; use frame_system::{Config as SystemConfig, Pallet as System}; use pallet_mmr::{Nodes, Pallet as Mmr}; use sp_consensus_beefy::Payload; -use sp_runtime::traits::Zero; +use sp_runtime::traits::One; pub trait Config: pallet_mmr::Config + crate::Config @@ -51,9 +51,9 @@ mod benchmarks { #[benchmark] fn extract_validation_context() { - init_block::(0); + init_block::(1); let header = System::::finalize(); - frame_system::BlockHash::::insert(BlockNumberFor::::zero(), header.hash()); + frame_system::BlockHash::::insert(BlockNumberFor::::one(), header.hash()); let validation_context; #[block] @@ -67,7 +67,7 @@ mod benchmarks { #[benchmark] fn read_peak() { - init_block::(0); + init_block::(1); let peak; #[block] @@ -108,4 +108,10 @@ mod benchmarks { assert_eq!(is_non_canonical, true); } + + impl_benchmark_test_suite!( + Pallet, + crate::mock::new_test_ext(Default::default()), + crate::mock::Test + ); } diff --git a/substrate/frame/beefy-mmr/src/mock.rs b/substrate/frame/beefy-mmr/src/mock.rs index 5af83260c062..6756c618d706 100644 --- a/substrate/frame/beefy-mmr/src/mock.rs +++ b/substrate/frame/beefy-mmr/src/mock.rs @@ -37,6 +37,7 @@ use crate as pallet_beefy_mmr; pub use sp_consensus_beefy::{ ecdsa_crypto::AuthorityId as BeefyId, mmr::BeefyDataProvider, ConsensusLog, BEEFY_ENGINE_ID, }; +use sp_core::offchain::{testing::TestOffchainExt, OffchainDbExt, OffchainWorkerExt}; impl_opaque_keys! { pub struct MockSessionKeys { @@ -192,5 +193,10 @@ pub fn new_test_ext_raw_authorities(authorities: Vec<(u64, BeefyId)>) -> TestExt .assimilate_storage(&mut t) .unwrap(); - t.into() + let mut ext: TestExternalities = t.into(); + let (offchain, _offchain_state) = TestOffchainExt::with_offchain_db(ext.offchain_db()); + ext.register_extension(OffchainDbExt::new(offchain.clone())); + ext.register_extension(OffchainWorkerExt::new(offchain)); + + ext } diff --git a/substrate/frame/beefy-mmr/src/tests.rs b/substrate/frame/beefy-mmr/src/tests.rs index 226b0b698777..b126a01012b4 100644 --- a/substrate/frame/beefy-mmr/src/tests.rs +++ b/substrate/frame/beefy-mmr/src/tests.rs @@ -24,10 +24,7 @@ use sp_consensus_beefy::{ AncestryHelper, Commitment, Payload, ValidatorSet, }; -use sp_core::{ - offchain::{testing::TestOffchainExt, OffchainDbExt, OffchainWorkerExt}, - H256, -}; +use sp_core::H256; use sp_io::TestExternalities; use sp_runtime::{traits::Keccak256, DigestItem}; @@ -209,11 +206,6 @@ fn should_update_authorities() { fn extract_validation_context_should_work_correctly() { let mut ext = new_test_ext(vec![1, 2]); - // Register offchain ext. - let (offchain, _offchain_state) = TestOffchainExt::with_offchain_db(ext.offchain_db()); - ext.register_extension(OffchainDbExt::new(offchain.clone())); - ext.register_extension(OffchainWorkerExt::new(offchain)); - ext.execute_with(|| { init_block(1, None); let h1 = System::finalize(); @@ -260,11 +252,6 @@ fn is_non_canonical_should_work_correctly() { }); ext.persist_offchain_overlay(); - // Register offchain ext. - let (offchain, _offchain_state) = TestOffchainExt::with_offchain_db(ext.offchain_db()); - ext.register_extension(OffchainDbExt::new(offchain.clone())); - ext.register_extension(OffchainWorkerExt::new(offchain)); - ext.execute_with(|| { let valid_proof = BeefyMmr::generate_proof(250, None).unwrap(); let mut invalid_proof = valid_proof.clone(); diff --git a/substrate/frame/merkle-mountain-range/src/benchmarking.rs b/substrate/frame/merkle-mountain-range/src/benchmarking.rs index 34a8d5912107..07afd9529eb2 100644 --- a/substrate/frame/merkle-mountain-range/src/benchmarking.rs +++ b/substrate/frame/merkle-mountain-range/src/benchmarking.rs @@ -38,4 +38,6 @@ benchmarks_instance_pallet! { } verify { assert_eq!(crate::NumberOfLeaves::::get(), leaves); } + + impl_benchmark_test_suite!(Pallet, crate::tests::new_test_ext(), crate::mock::Test); } From 4d525a19a8600a66d34f6dc385f3d391ce0e19e4 Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Wed, 14 Aug 2024 08:24:47 +0300 Subject: [PATCH 17/18] Update prdoc --- prdoc/pr_5188.prdoc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/prdoc/pr_5188.prdoc b/prdoc/pr_5188.prdoc index b2ab9ff6653b..b93897a5eceb 100644 --- a/prdoc/pr_5188.prdoc +++ b/prdoc/pr_5188.prdoc @@ -30,3 +30,5 @@ crates: bump: major - name: westend-runtime bump: minor + - name: sp-io + bump: minor From c7149cc0bdfda7d13a804b89a58d8d17eaf366c5 Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Wed, 14 Aug 2024 17:32:07 +0300 Subject: [PATCH 18/18] Remove is_offchain_db_ext_available() --- prdoc/pr_5188.prdoc | 2 -- substrate/frame/beefy-mmr/src/benchmarking.rs | 12 ++++++++++++ substrate/frame/merkle-mountain-range/src/lib.rs | 5 +++++ .../frame/merkle-mountain-range/src/mmr/storage.rs | 8 ++++---- substrate/primitives/io/src/lib.rs | 5 ----- 5 files changed, 21 insertions(+), 11 deletions(-) diff --git a/prdoc/pr_5188.prdoc b/prdoc/pr_5188.prdoc index b93897a5eceb..b2ab9ff6653b 100644 --- a/prdoc/pr_5188.prdoc +++ b/prdoc/pr_5188.prdoc @@ -30,5 +30,3 @@ crates: bump: major - name: westend-runtime bump: minor - - name: sp-io - bump: minor diff --git a/substrate/frame/beefy-mmr/src/benchmarking.rs b/substrate/frame/beefy-mmr/src/benchmarking.rs index b87802fad5c0..135f95eabb99 100644 --- a/substrate/frame/beefy-mmr/src/benchmarking.rs +++ b/substrate/frame/beefy-mmr/src/benchmarking.rs @@ -51,6 +51,10 @@ mod benchmarks { #[benchmark] fn extract_validation_context() { + if !cfg!(feature = "test") { + pallet_mmr::UseLocalStorage::::set(true); + } + init_block::(1); let header = System::::finalize(); frame_system::BlockHash::::insert(BlockNumberFor::::one(), header.hash()); @@ -67,6 +71,10 @@ mod benchmarks { #[benchmark] fn read_peak() { + if !cfg!(feature = "test") { + pallet_mmr::UseLocalStorage::::set(true); + } + init_block::(1); let peak; @@ -83,6 +91,10 @@ mod benchmarks { /// the verification. We need to account for the peaks separately. #[benchmark] fn n_items_proof_is_non_canonical(n: Linear<2, 512>) { + if !cfg!(feature = "test") { + pallet_mmr::UseLocalStorage::::set(true); + } + for block_num in 1..=n { init_block::(block_num); } diff --git a/substrate/frame/merkle-mountain-range/src/lib.rs b/substrate/frame/merkle-mountain-range/src/lib.rs index e2eb552e0ee1..7dfe95c83361 100644 --- a/substrate/frame/merkle-mountain-range/src/lib.rs +++ b/substrate/frame/merkle-mountain-range/src/lib.rs @@ -240,6 +240,11 @@ pub mod pallet { pub type Nodes, I: 'static = ()> = StorageMap<_, Identity, NodeIndex, HashOf, OptionQuery>; + /// Helper flag used in the runtime benchmarks for the initial setup. + #[cfg(feature = "runtime-benchmarks")] + #[pallet::storage] + pub type UseLocalStorage = StorageValue<_, bool, ValueQuery>; + #[pallet::hooks] impl, I: 'static> Hooks> for Pallet { fn on_initialize(_n: BlockNumberFor) -> Weight { diff --git a/substrate/frame/merkle-mountain-range/src/mmr/storage.rs b/substrate/frame/merkle-mountain-range/src/mmr/storage.rs index cdaa497f3e9c..02852388b417 100644 --- a/substrate/frame/merkle-mountain-range/src/mmr/storage.rs +++ b/substrate/frame/merkle-mountain-range/src/mmr/storage.rs @@ -52,13 +52,13 @@ impl OffchainStorage { } #[cfg(not(feature = "runtime-benchmarks"))] - fn set(key: &[u8], value: &[u8]) { + fn set, I: 'static>(key: &[u8], value: &[u8]) { sp_io::offchain_index::set(key, value); } #[cfg(feature = "runtime-benchmarks")] - fn set(key: &[u8], value: &[u8]) { - if sp_io::offchain::is_offchain_db_ext_available() { + fn set, I: 'static>(key: &[u8], value: &[u8]) { + if crate::pallet::UseLocalStorage::::get() { sp_io::offchain::local_storage_set(StorageKind::PERSISTENT, key, value); } else { sp_io::offchain_index::set(key, value); @@ -221,7 +221,7 @@ where target: "runtime::mmr::offchain", "offchain db set: pos {} parent_hash {:?} key {:?}", pos, parent_hash, temp_key ); - OffchainStorage::set(&temp_key, &encoded_node); + OffchainStorage::set::(&temp_key, &encoded_node); } } diff --git a/substrate/primitives/io/src/lib.rs b/substrate/primitives/io/src/lib.rs index 8a7875cb9217..38cf72a0c790 100644 --- a/substrate/primitives/io/src/lib.rs +++ b/substrate/primitives/io/src/lib.rs @@ -1343,11 +1343,6 @@ sp_externalities::decl_extension! { /// These functions are being made available to the runtime and are called by the runtime. #[runtime_interface] pub trait Offchain { - /// Checks if the offchain database extension is available. - fn is_offchain_db_ext_available(&mut self) -> bool { - self.extension::().is_some() - } - /// Returns if the local node is a potential validator. /// /// Even if this function returns `true`, it does not mean that any keys are configured