From 5721e5569e10453588e3869162f0f4740b828a17 Mon Sep 17 00:00:00 2001 From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com> Date: Tue, 19 Nov 2024 10:12:17 +0100 Subject: [PATCH 1/6] `TransactionPool` API uses `async_trait` (#6528) This PR refactors `TransactionPool` API to use `async_trait`, replacing the` Pin>` pattern. This should improve readability and maintainability. The change is not altering any functionality. --------- Co-authored-by: GitHub Action --- Cargo.lock | 2 + prdoc/pr_6528.prdoc | 18 ++ substrate/bin/node/bench/Cargo.toml | 1 + substrate/bin/node/bench/src/construct.rs | 48 ++--- substrate/client/rpc-spec-v2/Cargo.toml | 1 + .../src/transaction/tests/middleware_pool.rs | 96 ++++----- substrate/client/rpc/src/author/mod.rs | 17 +- substrate/client/service/src/lib.rs | 16 +- .../client/transaction-pool/api/src/lib.rs | 55 ++---- .../fork_aware_txpool/fork_aware_txpool.rs | 184 ++++++++---------- .../src/fork_aware_txpool/mod.rs | 12 +- substrate/client/transaction-pool/src/lib.rs | 4 +- .../single_state_txpool.rs | 95 ++++----- .../src/transaction_pool_wrapper.rs | 52 ++--- 14 files changed, 268 insertions(+), 333 deletions(-) create mode 100644 prdoc/pr_6528.prdoc diff --git a/Cargo.lock b/Cargo.lock index 182d8f6bacad..02d7da8f7657 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11086,6 +11086,7 @@ name = "node-bench" version = "0.9.0-dev" dependencies = [ "array-bytes", + "async-trait", "clap 4.5.13", "derive_more 0.99.17", "fs_extra", @@ -23245,6 +23246,7 @@ version = "0.34.0" dependencies = [ "array-bytes", "assert_matches", + "async-trait", "futures", "futures-util", "hex", diff --git a/prdoc/pr_6528.prdoc b/prdoc/pr_6528.prdoc new file mode 100644 index 000000000000..477ad76c947f --- /dev/null +++ b/prdoc/pr_6528.prdoc @@ -0,0 +1,18 @@ +title: 'TransactionPool API uses async_trait' +doc: +- audience: Node Dev + description: |- + This PR refactors `TransactionPool` API to use `async_trait`, replacing the` Pin>` pattern. This should improve readability and maintainability. + + The change is not altering any functionality. +crates: +- name: sc-rpc-spec-v2 + bump: minor +- name: sc-service + bump: minor +- name: sc-transaction-pool-api + bump: major +- name: sc-transaction-pool + bump: major +- name: sc-rpc + bump: minor diff --git a/substrate/bin/node/bench/Cargo.toml b/substrate/bin/node/bench/Cargo.toml index 8c6556da682c..447f947107c1 100644 --- a/substrate/bin/node/bench/Cargo.toml +++ b/substrate/bin/node/bench/Cargo.toml @@ -15,6 +15,7 @@ workspace = true # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] +async-trait = { workspace = true } array-bytes = { workspace = true, default-features = true } clap = { features = ["derive"], workspace = true } log = { workspace = true, default-features = true } diff --git a/substrate/bin/node/bench/src/construct.rs b/substrate/bin/node/bench/src/construct.rs index bed6e3d914c2..22129c6a1d69 100644 --- a/substrate/bin/node/bench/src/construct.rs +++ b/substrate/bin/node/bench/src/construct.rs @@ -24,14 +24,14 @@ //! DO NOT depend on user input). Thus transaction generation should be //! based on randomized data. -use futures::Future; use std::{borrow::Cow, collections::HashMap, pin::Pin, sync::Arc}; +use async_trait::async_trait; use node_primitives::Block; use node_testing::bench::{BenchDb, BlockType, DatabaseType, KeyTypes}; use sc_transaction_pool_api::{ - ImportNotificationStream, PoolFuture, PoolStatus, ReadyTransactions, TransactionFor, - TransactionSource, TransactionStatusStreamFor, TxHash, + ImportNotificationStream, PoolStatus, ReadyTransactions, TransactionFor, TransactionSource, + TransactionStatusStreamFor, TxHash, }; use sp_consensus::{Environment, Proposer}; use sp_inherents::InherentDataProvider; @@ -224,54 +224,47 @@ impl ReadyTransactions for TransactionsIterator { fn report_invalid(&mut self, _tx: &Self::Item) {} } +#[async_trait] impl sc_transaction_pool_api::TransactionPool for Transactions { type Block = Block; type Hash = node_primitives::Hash; type InPoolTransaction = PoolTransaction; type Error = sc_transaction_pool_api::error::Error; - /// Returns a future that imports a bunch of unverified transactions to the pool. - fn submit_at( + /// Asynchronously imports a bunch of unverified transactions to the pool. + async fn submit_at( &self, _at: Self::Hash, _source: TransactionSource, _xts: Vec>, - ) -> PoolFuture>, Self::Error> { + ) -> Result>, Self::Error> { unimplemented!() } - /// Returns a future that imports one unverified transaction to the pool. - fn submit_one( + /// Asynchronously imports one unverified transaction to the pool. + async fn submit_one( &self, _at: Self::Hash, _source: TransactionSource, _xt: TransactionFor, - ) -> PoolFuture, Self::Error> { + ) -> Result, Self::Error> { unimplemented!() } - fn submit_and_watch( + async fn submit_and_watch( &self, _at: Self::Hash, _source: TransactionSource, _xt: TransactionFor, - ) -> PoolFuture>>, Self::Error> { + ) -> Result>>, Self::Error> { unimplemented!() } - fn ready_at( + async fn ready_at( &self, _at: Self::Hash, - ) -> Pin< - Box< - dyn Future< - Output = Box> + Send>, - > + Send, - >, - > { - let iter: Box> + Send> = - Box::new(TransactionsIterator(self.0.clone().into_iter())); - Box::pin(futures::future::ready(iter)) + ) -> Box> + Send> { + Box::new(TransactionsIterator(self.0.clone().into_iter())) } fn ready(&self) -> Box> + Send> { @@ -306,18 +299,11 @@ impl sc_transaction_pool_api::TransactionPool for Transactions { unimplemented!() } - fn ready_at_with_timeout( + async fn ready_at_with_timeout( &self, _at: Self::Hash, _timeout: std::time::Duration, - ) -> Pin< - Box< - dyn Future< - Output = Box> + Send>, - > + Send - + '_, - >, - > { + ) -> Box> + Send> { unimplemented!() } } diff --git a/substrate/client/rpc-spec-v2/Cargo.toml b/substrate/client/rpc-spec-v2/Cargo.toml index 58dd8b830beb..daa805912fb9 100644 --- a/substrate/client/rpc-spec-v2/Cargo.toml +++ b/substrate/client/rpc-spec-v2/Cargo.toml @@ -44,6 +44,7 @@ rand = { workspace = true, default-features = true } schnellru = { workspace = true } [dev-dependencies] +async-trait = { workspace = true } jsonrpsee = { workspace = true, features = ["server", "ws-client"] } serde_json = { workspace = true, default-features = true } tokio = { features = ["macros"], workspace = true, default-features = true } diff --git a/substrate/client/rpc-spec-v2/src/transaction/tests/middleware_pool.rs b/substrate/client/rpc-spec-v2/src/transaction/tests/middleware_pool.rs index adcc987f9c39..a543969a89b8 100644 --- a/substrate/client/rpc-spec-v2/src/transaction/tests/middleware_pool.rs +++ b/substrate/client/rpc-spec-v2/src/transaction/tests/middleware_pool.rs @@ -16,16 +16,16 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . +use async_trait::async_trait; use codec::Encode; -use futures::Future; use sc_transaction_pool::BasicPool; use sc_transaction_pool_api::{ - ImportNotificationStream, PoolFuture, PoolStatus, ReadyTransactions, TransactionFor, - TransactionPool, TransactionSource, TransactionStatusStreamFor, TxHash, + ImportNotificationStream, PoolStatus, ReadyTransactions, TransactionFor, TransactionPool, + TransactionSource, TransactionStatusStreamFor, TxHash, }; use crate::hex_string; -use futures::{FutureExt, StreamExt}; +use futures::StreamExt; use sp_runtime::traits::Block as BlockT; use std::{collections::HashMap, pin::Pin, sync::Arc}; @@ -77,67 +77,64 @@ impl MiddlewarePool { } } +#[async_trait] impl TransactionPool for MiddlewarePool { type Block = as TransactionPool>::Block; type Hash = as TransactionPool>::Hash; type InPoolTransaction = as TransactionPool>::InPoolTransaction; type Error = as TransactionPool>::Error; - fn submit_at( + async fn submit_at( &self, at: ::Hash, source: TransactionSource, xts: Vec>, - ) -> PoolFuture, Self::Error>>, Self::Error> { - self.inner_pool.submit_at(at, source, xts) + ) -> Result, Self::Error>>, Self::Error> { + self.inner_pool.submit_at(at, source, xts).await } - fn submit_one( + async fn submit_one( &self, at: ::Hash, source: TransactionSource, xt: TransactionFor, - ) -> PoolFuture, Self::Error> { - self.inner_pool.submit_one(at, source, xt) + ) -> Result, Self::Error> { + self.inner_pool.submit_one(at, source, xt).await } - fn submit_and_watch( + async fn submit_and_watch( &self, at: ::Hash, source: TransactionSource, xt: TransactionFor, - ) -> PoolFuture>>, Self::Error> { - let pool = self.inner_pool.clone(); - let sender = self.sender.clone(); + ) -> Result>>, Self::Error> { let transaction = hex_string(&xt.encode()); + let sender = self.sender.clone(); - async move { - let watcher = match pool.submit_and_watch(at, source, xt).await { - Ok(watcher) => watcher, - Err(err) => { - let _ = sender.send(MiddlewarePoolEvent::PoolError { - transaction: transaction.clone(), - err: err.to_string(), - }); - return Err(err); - }, - }; - - let watcher = watcher.map(move |status| { - let sender = sender.clone(); - let transaction = transaction.clone(); - - let _ = sender.send(MiddlewarePoolEvent::TransactionStatus { - transaction, - status: status.clone(), + let watcher = match self.inner_pool.submit_and_watch(at, source, xt).await { + Ok(watcher) => watcher, + Err(err) => { + let _ = sender.send(MiddlewarePoolEvent::PoolError { + transaction: transaction.clone(), + err: err.to_string(), }); + return Err(err); + }, + }; + + let watcher = watcher.map(move |status| { + let sender = sender.clone(); + let transaction = transaction.clone(); - status + let _ = sender.send(MiddlewarePoolEvent::TransactionStatus { + transaction, + status: status.clone(), }); - Ok(watcher.boxed()) - } - .boxed() + status + }); + + Ok(watcher.boxed()) } fn remove_invalid(&self, hashes: &[TxHash]) -> Vec> { @@ -164,17 +161,11 @@ impl TransactionPool for MiddlewarePool { self.inner_pool.ready_transaction(hash) } - fn ready_at( + async fn ready_at( &self, at: ::Hash, - ) -> Pin< - Box< - dyn Future< - Output = Box> + Send>, - > + Send, - >, - > { - self.inner_pool.ready_at(at) + ) -> Box> + Send> { + self.inner_pool.ready_at(at).await } fn ready(&self) -> Box> + Send> { @@ -185,18 +176,11 @@ impl TransactionPool for MiddlewarePool { self.inner_pool.futures() } - fn ready_at_with_timeout( + async fn ready_at_with_timeout( &self, at: ::Hash, _timeout: std::time::Duration, - ) -> Pin< - Box< - dyn Future< - Output = Box> + Send>, - > + Send - + '_, - >, - > { - self.inner_pool.ready_at(at) + ) -> Box> + Send> { + self.inner_pool.ready_at(at).await } } diff --git a/substrate/client/rpc/src/author/mod.rs b/substrate/client/rpc/src/author/mod.rs index 731f4df2f6f3..6afc871e565a 100644 --- a/substrate/client/rpc/src/author/mod.rs +++ b/substrate/client/rpc/src/author/mod.rs @@ -29,7 +29,6 @@ use crate::{ }; use codec::{Decode, Encode}; -use futures::TryFutureExt; use jsonrpsee::{core::async_trait, types::ErrorObject, Extensions, PendingSubscriptionSink}; use sc_rpc_api::check_if_safe; use sc_transaction_pool_api::{ @@ -191,14 +190,16 @@ where }, }; - let submit = self.pool.submit_and_watch(best_block_hash, TX_SOURCE, dxt).map_err(|e| { - e.into_pool_error() - .map(error::Error::from) - .unwrap_or_else(|e| error::Error::Verification(Box::new(e))) - }); - + let pool = self.pool.clone(); let fut = async move { - let stream = match submit.await { + let submit = + pool.submit_and_watch(best_block_hash, TX_SOURCE, dxt).await.map_err(|e| { + e.into_pool_error() + .map(error::Error::from) + .unwrap_or_else(|e| error::Error::Verification(Box::new(e))) + }); + + let stream = match submit { Ok(stream) => stream, Err(err) => { let _ = pending.reject(ErrorObject::from(err)).await; diff --git a/substrate/client/service/src/lib.rs b/substrate/client/service/src/lib.rs index 5cfd80cef910..9c01d7288a81 100644 --- a/substrate/client/service/src/lib.rs +++ b/substrate/client/service/src/lib.rs @@ -528,13 +528,17 @@ where }; let start = std::time::Instant::now(); - let import_future = self.pool.submit_one( - self.client.info().best_hash, - sc_transaction_pool_api::TransactionSource::External, - uxt, - ); + let pool = self.pool.clone(); + let client = self.client.clone(); Box::pin(async move { - match import_future.await { + match pool + .submit_one( + client.info().best_hash, + sc_transaction_pool_api::TransactionSource::External, + uxt, + ) + .await + { Ok(_) => { let elapsed = start.elapsed(); debug!(target: sc_transaction_pool::LOG_TARGET, "import transaction: {elapsed:?}"); diff --git a/substrate/client/transaction-pool/api/src/lib.rs b/substrate/client/transaction-pool/api/src/lib.rs index 3ac1a79a0c28..6f771e9479bd 100644 --- a/substrate/client/transaction-pool/api/src/lib.rs +++ b/substrate/client/transaction-pool/api/src/lib.rs @@ -23,7 +23,7 @@ pub mod error; use async_trait::async_trait; use codec::Codec; -use futures::{Future, Stream}; +use futures::Stream; use serde::{de::DeserializeOwned, Deserialize, Serialize}; use sp_core::offchain::TransactionPoolExt; use sp_runtime::traits::{Block as BlockT, Member}; @@ -208,9 +208,6 @@ pub type LocalTransactionFor

= <

::Block as BlockT> /// Transaction's index within the block in which it was included. pub type TxIndex = usize; -/// Typical future type used in transaction pool api. -pub type PoolFuture = std::pin::Pin> + Send>>; - /// In-pool transaction interface. /// /// The pool is container of transactions that are implementing this trait. @@ -238,6 +235,7 @@ pub trait InPoolTransaction { } /// Transaction pool interface. +#[async_trait] pub trait TransactionPool: Send + Sync { /// Block type. type Block: BlockT; @@ -253,46 +251,40 @@ pub trait TransactionPool: Send + Sync { // *** RPC - /// Returns a future that imports a bunch of unverified transactions to the pool. - fn submit_at( + /// Asynchronously imports a bunch of unverified transactions to the pool. + async fn submit_at( &self, at: ::Hash, source: TransactionSource, xts: Vec>, - ) -> PoolFuture, Self::Error>>, Self::Error>; + ) -> Result, Self::Error>>, Self::Error>; - /// Returns a future that imports one unverified transaction to the pool. - fn submit_one( + /// Asynchronously imports one unverified transaction to the pool. + async fn submit_one( &self, at: ::Hash, source: TransactionSource, xt: TransactionFor, - ) -> PoolFuture, Self::Error>; + ) -> Result, Self::Error>; - /// Returns a future that imports a single transaction and starts to watch their progress in the + /// Asynchronously imports a single transaction and starts to watch their progress in the /// pool. - fn submit_and_watch( + async fn submit_and_watch( &self, at: ::Hash, source: TransactionSource, xt: TransactionFor, - ) -> PoolFuture>>, Self::Error>; + ) -> Result>>, Self::Error>; // *** Block production / Networking /// Get an iterator for ready transactions ordered by priority. /// - /// Guarantees to return only when transaction pool got updated at `at` block. - /// Guarantees to return immediately when `None` is passed. - fn ready_at( + /// Guaranteed to resolve only when transaction pool got updated at `at` block. + /// Guaranteed to resolve immediately when `None` is passed. + async fn ready_at( &self, at: ::Hash, - ) -> Pin< - Box< - dyn Future< - Output = Box> + Send>, - > + Send, - >, - >; + ) -> Box> + Send>; /// Get an iterator for ready transactions ordered by priority. fn ready(&self) -> Box> + Send>; @@ -322,22 +314,15 @@ pub trait TransactionPool: Send + Sync { /// Return specific ready transaction by hash, if there is one. fn ready_transaction(&self, hash: &TxHash) -> Option>; - /// Returns set of ready transaction at given block within given timeout. + /// Asynchronously returns a set of ready transaction at given block within given timeout. /// - /// If the timeout is hit during method execution then the best effort set of ready transactions - /// for given block, without executing full maintain process is returned. - fn ready_at_with_timeout( + /// If the timeout is hit during method execution, then the best effort (without executing full + /// maintain process) set of ready transactions for given block is returned. + async fn ready_at_with_timeout( &self, at: ::Hash, timeout: std::time::Duration, - ) -> Pin< - Box< - dyn Future< - Output = Box> + Send>, - > + Send - + '_, - >, - >; + ) -> Box> + Send>; } /// An iterator of ready transactions. diff --git a/substrate/client/transaction-pool/src/fork_aware_txpool/fork_aware_txpool.rs b/substrate/client/transaction-pool/src/fork_aware_txpool/fork_aware_txpool.rs index a342d35b2844..065d0cb3a274 100644 --- a/substrate/client/transaction-pool/src/fork_aware_txpool/fork_aware_txpool.rs +++ b/substrate/client/transaction-pool/src/fork_aware_txpool/fork_aware_txpool.rs @@ -33,7 +33,7 @@ use crate::{ enactment_state::{EnactmentAction, EnactmentState}, fork_aware_txpool::revalidation_worker, graph::{self, base_pool::Transaction, ExtrinsicFor, ExtrinsicHash, IsValidator, Options}, - PolledIterator, ReadyIteratorFor, LOG_TARGET, + ReadyIteratorFor, LOG_TARGET, }; use async_trait::async_trait; use futures::{ @@ -45,8 +45,8 @@ use futures::{ use parking_lot::Mutex; use prometheus_endpoint::Registry as PrometheusRegistry; use sc_transaction_pool_api::{ - ChainEvent, ImportNotificationStream, MaintainedTransactionPool, PoolFuture, PoolStatus, - TransactionFor, TransactionPool, TransactionSource, TransactionStatusStreamFor, TxHash, + ChainEvent, ImportNotificationStream, MaintainedTransactionPool, PoolStatus, TransactionFor, + TransactionPool, TransactionSource, TransactionStatusStreamFor, TxHash, }; use sp_blockchain::{HashAndNumber, TreeRoute}; use sp_core::traits::SpawnEssentialNamed; @@ -375,14 +375,13 @@ where /// /// Pruning is just rebuilding the underlying transactions graph, no validations are executed, /// so this process shall be fast. - pub fn ready_at_light(&self, at: Block::Hash) -> PolledIterator { + pub async fn ready_at_light(&self, at: Block::Hash) -> ReadyIteratorFor { let start = Instant::now(); let api = self.api.clone(); log::trace!(target: LOG_TARGET, "fatp::ready_at_light {:?}", at); let Ok(block_number) = self.api.resolve_block_number(at) else { - let empty: ReadyIteratorFor = Box::new(std::iter::empty()); - return Box::pin(async { empty }) + return Box::new(std::iter::empty()) }; let best_result = { @@ -401,57 +400,53 @@ where ) }; - Box::pin(async move { - if let Ok((Some(best_tree_route), Some(best_view))) = best_result { - let tmp_view: View = View::new_from_other( - &best_view, - &HashAndNumber { hash: at, number: block_number }, - ); - - let mut all_extrinsics = vec![]; + if let Ok((Some(best_tree_route), Some(best_view))) = best_result { + let tmp_view: View = + View::new_from_other(&best_view, &HashAndNumber { hash: at, number: block_number }); - for h in best_tree_route.enacted() { - let extrinsics = api - .block_body(h.hash) - .await - .unwrap_or_else(|e| { - log::warn!(target: LOG_TARGET, "Compute ready light transactions: error request: {}", e); - None - }) - .unwrap_or_default() - .into_iter() - .map(|t| api.hash_and_length(&t).0); - all_extrinsics.extend(extrinsics); - } + let mut all_extrinsics = vec![]; - let before_count = tmp_view.pool.validated_pool().status().ready; - let tags = tmp_view - .pool - .validated_pool() - .extrinsics_tags(&all_extrinsics) + for h in best_tree_route.enacted() { + let extrinsics = api + .block_body(h.hash) + .await + .unwrap_or_else(|e| { + log::warn!(target: LOG_TARGET, "Compute ready light transactions: error request: {}", e); + None + }) + .unwrap_or_default() .into_iter() - .flatten() - .flatten() - .collect::>(); - let _ = tmp_view.pool.validated_pool().prune_tags(tags); - - let after_count = tmp_view.pool.validated_pool().status().ready; - log::debug!(target: LOG_TARGET, - "fatp::ready_at_light {} from {} before: {} to be removed: {} after: {} took:{:?}", - at, - best_view.at.hash, - before_count, - all_extrinsics.len(), - after_count, - start.elapsed() - ); - Box::new(tmp_view.pool.validated_pool().ready()) - } else { - let empty: ReadyIteratorFor = Box::new(std::iter::empty()); - log::debug!(target: LOG_TARGET, "fatp::ready_at_light {} -> empty, took:{:?}", at, start.elapsed()); - empty + .map(|t| api.hash_and_length(&t).0); + all_extrinsics.extend(extrinsics); } - }) + + let before_count = tmp_view.pool.validated_pool().status().ready; + let tags = tmp_view + .pool + .validated_pool() + .extrinsics_tags(&all_extrinsics) + .into_iter() + .flatten() + .flatten() + .collect::>(); + let _ = tmp_view.pool.validated_pool().prune_tags(tags); + + let after_count = tmp_view.pool.validated_pool().status().ready; + log::debug!(target: LOG_TARGET, + "fatp::ready_at_light {} from {} before: {} to be removed: {} after: {} took:{:?}", + at, + best_view.at.hash, + before_count, + all_extrinsics.len(), + after_count, + start.elapsed() + ); + Box::new(tmp_view.pool.validated_pool().ready()) + } else { + let empty: ReadyIteratorFor = Box::new(std::iter::empty()); + log::debug!(target: LOG_TARGET, "fatp::ready_at_light {} -> empty, took:{:?}", at, start.elapsed()); + empty + } } /// Waits for the set of ready transactions for a given block up to a specified timeout. @@ -464,18 +459,18 @@ where /// maintain. /// /// Returns a future resolving to a ready iterator of transactions. - fn ready_at_with_timeout_internal( + async fn ready_at_with_timeout_internal( &self, at: Block::Hash, timeout: std::time::Duration, - ) -> PolledIterator { + ) -> ReadyIteratorFor { log::debug!(target: LOG_TARGET, "fatp::ready_at_with_timeout at {:?} allowed delay: {:?}", at, timeout); let timeout = futures_timer::Delay::new(timeout); let (view_already_exists, ready_at) = self.ready_at_internal(at); if view_already_exists { - return ready_at; + return ready_at.await; } let maybe_ready = async move { @@ -493,18 +488,19 @@ where }; let fall_back_ready = self.ready_at_light(at); - Box::pin(async { - let (maybe_ready, fall_back_ready) = - futures::future::join(maybe_ready.boxed(), fall_back_ready.boxed()).await; - maybe_ready.unwrap_or(fall_back_ready) - }) + let (maybe_ready, fall_back_ready) = + futures::future::join(maybe_ready, fall_back_ready).await; + maybe_ready.unwrap_or(fall_back_ready) } - fn ready_at_internal(&self, at: Block::Hash) -> (bool, PolledIterator) { + fn ready_at_internal( + &self, + at: Block::Hash, + ) -> (bool, Pin> + Send>>) { let mut ready_poll = self.ready_poll.lock(); if let Some((view, inactive)) = self.view_store.get_view_at(at, true) { - log::debug!(target: LOG_TARGET, "fatp::ready_at {at:?} (inactive:{inactive:?})"); + log::debug!(target: LOG_TARGET, "fatp::ready_at_internal {at:?} (inactive:{inactive:?})"); let iterator: ReadyIteratorFor = Box::new(view.pool.validated_pool().ready()); return (true, async move { iterator }.boxed()); } @@ -519,7 +515,7 @@ where }) .boxed(); log::debug!(target: LOG_TARGET, - "fatp::ready_at {at:?} pending keys: {:?}", + "fatp::ready_at_internal {at:?} pending keys: {:?}", ready_poll.pollers.keys() ); (false, pending) @@ -573,6 +569,7 @@ fn reduce_multiview_result(input: HashMap>>) -> Vec TransactionPool for ForkAwareTxPool where Block: BlockT, @@ -590,12 +587,12 @@ where /// /// The internal limits of the pool are checked. The results of submissions to individual views /// are reduced to single result. Refer to `reduce_multiview_result` for more details. - fn submit_at( + async fn submit_at( &self, _: ::Hash, source: TransactionSource, xts: Vec>, - ) -> PoolFuture, Self::Error>>, Self::Error> { + ) -> Result, Self::Error>>, Self::Error> { let view_store = self.view_store.clone(); log::debug!(target: LOG_TARGET, "fatp::submit_at count:{} views:{}", xts.len(), self.active_views_count()); log_xt_trace!(target: LOG_TARGET, xts.iter().map(|xt| self.tx_hash(xt)), "[{:?}] fatp::submit_at"); @@ -603,7 +600,7 @@ where let mempool_results = self.mempool.extend_unwatched(source, &xts); if view_store.is_empty() { - return future::ready(Ok(mempool_results)).boxed() + return Ok(mempool_results) } let to_be_submitted = mempool_results @@ -616,11 +613,10 @@ where .report(|metrics| metrics.submitted_transactions.inc_by(to_be_submitted.len() as _)); let mempool = self.mempool.clone(); - async move { - let results_map = view_store.submit(source, to_be_submitted.into_iter()).await; - let mut submission_results = reduce_multiview_result(results_map).into_iter(); + let results_map = view_store.submit(source, to_be_submitted.into_iter()).await; + let mut submission_results = reduce_multiview_result(results_map).into_iter(); - Ok(mempool_results + Ok(mempool_results .into_iter() .map(|result| { result.and_then(|xt_hash| { @@ -633,60 +629,50 @@ where }) }) .collect::>()) - } - .boxed() } /// Submits a single transaction and returns a future resolving to the submission results. /// /// Actual transaction submission process is delegated to the `submit_at` function. - fn submit_one( + async fn submit_one( &self, _at: ::Hash, source: TransactionSource, xt: TransactionFor, - ) -> PoolFuture, Self::Error> { + ) -> Result, Self::Error> { log::trace!(target: LOG_TARGET, "[{:?}] fatp::submit_one views:{}", self.tx_hash(&xt), self.active_views_count()); - let result_future = self.submit_at(_at, source, vec![xt]); - async move { - let result = result_future.await; - match result { - Ok(mut v) => - v.pop().expect("There is exactly one element in result of submit_at. qed."), - Err(e) => Err(e), - } + match self.submit_at(_at, source, vec![xt]).await { + Ok(mut v) => + v.pop().expect("There is exactly one element in result of submit_at. qed."), + Err(e) => Err(e), } - .boxed() } /// Submits a transaction and starts to watch its progress in the pool, returning a stream of /// status updates. /// /// Actual transaction submission process is delegated to the `ViewStore` internal instance. - fn submit_and_watch( + async fn submit_and_watch( &self, at: ::Hash, source: TransactionSource, xt: TransactionFor, - ) -> PoolFuture>>, Self::Error> { + ) -> Result>>, Self::Error> { log::trace!(target: LOG_TARGET, "[{:?}] fatp::submit_and_watch views:{}", self.tx_hash(&xt), self.active_views_count()); let xt = Arc::from(xt); let xt_hash = match self.mempool.push_watched(source, xt.clone()) { Ok(xt_hash) => xt_hash, - Err(e) => return future::ready(Err(e)).boxed(), + Err(e) => return Err(e), }; self.metrics.report(|metrics| metrics.submitted_transactions.inc()); let view_store = self.view_store.clone(); let mempool = self.mempool.clone(); - async move { - view_store - .submit_and_watch(at, source, xt) - .await - .inspect_err(|_| mempool.remove(xt_hash)) - } - .boxed() + view_store + .submit_and_watch(at, source, xt) + .await + .inspect_err(|_| mempool.remove(xt_hash)) } /// Intended to remove transactions identified by the given hashes, and any dependent @@ -757,9 +743,9 @@ where } /// Returns an iterator for ready transactions at a specific block, ordered by priority. - fn ready_at(&self, at: ::Hash) -> PolledIterator { + async fn ready_at(&self, at: ::Hash) -> ReadyIteratorFor { let (_, result) = self.ready_at_internal(at); - result + result.await } /// Returns an iterator for ready transactions, ordered by priority. @@ -782,12 +768,12 @@ where /// /// If the timeout expires before the maintain process is accomplished, a best-effort /// set of transactions is returned (refer to `ready_at_light`). - fn ready_at_with_timeout( + async fn ready_at_with_timeout( &self, at: ::Hash, timeout: std::time::Duration, - ) -> PolledIterator { - self.ready_at_with_timeout_internal(at, timeout) + ) -> ReadyIteratorFor { + self.ready_at_with_timeout_internal(at, timeout).await } } diff --git a/substrate/client/transaction-pool/src/fork_aware_txpool/mod.rs b/substrate/client/transaction-pool/src/fork_aware_txpool/mod.rs index 9f979e216b6d..5f7294a24fd7 100644 --- a/substrate/client/transaction-pool/src/fork_aware_txpool/mod.rs +++ b/substrate/client/transaction-pool/src/fork_aware_txpool/mod.rs @@ -201,12 +201,12 @@ //! required to accomplish it. //! //! ### Providing ready transactions: `ready_at` -//! The [`ready_at`] function returns a [future][`crate::PolledIterator`] that resolves to the -//! [ready transactions iterator][`ReadyTransactions`]. The block builder shall wait either for the -//! future to be resolved or for timeout to be hit. To avoid building empty blocks in case of -//! timeout, the waiting for timeout functionality was moved into the transaction pool, and new API -//! function was added: [`ready_at_with_timeout`]. This function also provides a fall back ready -//! iterator which is result of [light maintain](#light-maintain). +//! The asynchronous [`ready_at`] function resolves to the [ready transactions +//! iterator][`ReadyTransactions`]. The block builder shall wait either for the future to be +//! resolved or for timeout to be hit. To avoid building empty blocks in case of timeout, the +//! waiting for timeout functionality was moved into the transaction pool, and new API function was +//! added: [`ready_at_with_timeout`]. This function also provides a fall back ready iterator which +//! is result of [light maintain](#light-maintain). //! //! New function internally waits either for [maintain](#maintain) process triggered for requested //! block to be accomplished or for the timeout. If timeout hits then the result of [light diff --git a/substrate/client/transaction-pool/src/lib.rs b/substrate/client/transaction-pool/src/lib.rs index 888d25d3a0d2..3d3d596c291f 100644 --- a/substrate/client/transaction-pool/src/lib.rs +++ b/substrate/client/transaction-pool/src/lib.rs @@ -30,7 +30,7 @@ mod single_state_txpool; mod transaction_pool_wrapper; use common::{api, enactment_state}; -use std::{future::Future, pin::Pin, sync::Arc}; +use std::sync::Arc; pub use api::FullChainApi; pub use builder::{Builder, TransactionPoolHandle, TransactionPoolOptions, TransactionPoolType}; @@ -50,8 +50,6 @@ type BoxedReadyIterator = Box< type ReadyIteratorFor = BoxedReadyIterator, graph::ExtrinsicFor>; -type PolledIterator = Pin> + Send>>; - /// Log target for transaction pool. /// /// It can be used by other components for logging functionality strictly related to txpool (e.g. diff --git a/substrate/client/transaction-pool/src/single_state_txpool/single_state_txpool.rs b/substrate/client/transaction-pool/src/single_state_txpool/single_state_txpool.rs index 0826b95cf070..b29630b563bb 100644 --- a/substrate/client/transaction-pool/src/single_state_txpool/single_state_txpool.rs +++ b/substrate/client/transaction-pool/src/single_state_txpool/single_state_txpool.rs @@ -29,9 +29,8 @@ use crate::{ error, log_xt::log_xt_trace, }, - graph, - graph::{ExtrinsicHash, IsValidator}, - PolledIterator, ReadyIteratorFor, LOG_TARGET, + graph::{self, ExtrinsicHash, IsValidator}, + ReadyIteratorFor, LOG_TARGET, }; use async_trait::async_trait; use futures::{channel::oneshot, future, prelude::*, Future, FutureExt}; @@ -39,8 +38,8 @@ use parking_lot::Mutex; use prometheus_endpoint::Registry as PrometheusRegistry; use sc_transaction_pool_api::{ error::Error as TxPoolError, ChainEvent, ImportNotificationStream, MaintainedTransactionPool, - PoolFuture, PoolStatus, TransactionFor, TransactionPool, TransactionSource, - TransactionStatusStreamFor, TxHash, + PoolStatus, TransactionFor, TransactionPool, TransactionSource, TransactionStatusStreamFor, + TxHash, }; use sp_blockchain::{HashAndNumber, TreeRoute}; use sp_core::traits::SpawnEssentialNamed; @@ -224,26 +223,19 @@ where &self.api } - fn ready_at_with_timeout_internal( + async fn ready_at_with_timeout_internal( &self, at: Block::Hash, timeout: std::time::Duration, - ) -> PolledIterator { - let timeout = futures_timer::Delay::new(timeout); - let ready_maintained = self.ready_at(at); - let ready_current = self.ready(); - - let ready = async { - select! { - ready = ready_maintained => ready, - _ = timeout => ready_current - } - }; - - Box::pin(ready) + ) -> ReadyIteratorFor { + select! { + ready = self.ready_at(at)=> ready, + _ = futures_timer::Delay::new(timeout)=> self.ready() + } } } +#[async_trait] impl TransactionPool for BasicPool where Block: BlockT, @@ -255,12 +247,12 @@ where graph::base_pool::Transaction, graph::ExtrinsicFor>; type Error = PoolApi::Error; - fn submit_at( + async fn submit_at( &self, at: ::Hash, source: TransactionSource, xts: Vec>, - ) -> PoolFuture, Self::Error>>, Self::Error> { + ) -> Result, Self::Error>>, Self::Error> { let pool = self.pool.clone(); let xts = xts.into_iter().map(Arc::from).collect::>(); @@ -268,38 +260,32 @@ where .report(|metrics| metrics.submitted_transactions.inc_by(xts.len() as u64)); let number = self.api.resolve_block_number(at); - async move { - let at = HashAndNumber { hash: at, number: number? }; - Ok(pool.submit_at(&at, source, xts).await) - } - .boxed() + let at = HashAndNumber { hash: at, number: number? }; + Ok(pool.submit_at(&at, source, xts).await) } - fn submit_one( + async fn submit_one( &self, at: ::Hash, source: TransactionSource, xt: TransactionFor, - ) -> PoolFuture, Self::Error> { + ) -> Result, Self::Error> { let pool = self.pool.clone(); let xt = Arc::from(xt); self.metrics.report(|metrics| metrics.submitted_transactions.inc()); let number = self.api.resolve_block_number(at); - async move { - let at = HashAndNumber { hash: at, number: number? }; - pool.submit_one(&at, source, xt).await - } - .boxed() + let at = HashAndNumber { hash: at, number: number? }; + pool.submit_one(&at, source, xt).await } - fn submit_and_watch( + async fn submit_and_watch( &self, at: ::Hash, source: TransactionSource, xt: TransactionFor, - ) -> PoolFuture>>, Self::Error> { + ) -> Result>>, Self::Error> { let pool = self.pool.clone(); let xt = Arc::from(xt); @@ -307,13 +293,10 @@ where let number = self.api.resolve_block_number(at); - async move { - let at = HashAndNumber { hash: at, number: number? }; - let watcher = pool.submit_and_watch(&at, source, xt).await?; + let at = HashAndNumber { hash: at, number: number? }; + let watcher = pool.submit_and_watch(&at, source, xt).await?; - Ok(watcher.into_stream().boxed()) - } - .boxed() + Ok(watcher.into_stream().boxed()) } fn remove_invalid(&self, hashes: &[TxHash]) -> Vec> { @@ -343,9 +326,9 @@ where self.pool.validated_pool().ready_by_hash(hash) } - fn ready_at(&self, at: ::Hash) -> PolledIterator { + async fn ready_at(&self, at: ::Hash) -> ReadyIteratorFor { let Ok(at) = self.api.resolve_block_number(at) else { - return async { Box::new(std::iter::empty()) as Box<_> }.boxed() + return Box::new(std::iter::empty()) as Box<_> }; let status = self.status(); @@ -354,25 +337,23 @@ where // There could be transaction being added because of some re-org happening at the relevant // block, but this is relative unlikely. if status.ready == 0 && status.future == 0 { - return async { Box::new(std::iter::empty()) as Box<_> }.boxed() + return Box::new(std::iter::empty()) as Box<_> } if self.ready_poll.lock().updated_at() >= at { log::trace!(target: LOG_TARGET, "Transaction pool already processed block #{}", at); let iterator: ReadyIteratorFor = Box::new(self.pool.validated_pool().ready()); - return async move { iterator }.boxed() + return iterator } - self.ready_poll - .lock() - .add(at) - .map(|received| { - received.unwrap_or_else(|e| { - log::warn!(target: LOG_TARGET, "Error receiving pending set: {:?}", e); - Box::new(std::iter::empty()) - }) + let result = self.ready_poll.lock().add(at).map(|received| { + received.unwrap_or_else(|e| { + log::warn!(target: LOG_TARGET, "Error receiving pending set: {:?}", e); + Box::new(std::iter::empty()) }) - .boxed() + }); + + result.await } fn ready(&self) -> ReadyIteratorFor { @@ -384,12 +365,12 @@ where pool.futures().cloned().collect::>() } - fn ready_at_with_timeout( + async fn ready_at_with_timeout( &self, at: ::Hash, timeout: std::time::Duration, - ) -> PolledIterator { - self.ready_at_with_timeout_internal(at, timeout) + ) -> ReadyIteratorFor { + self.ready_at_with_timeout_internal(at, timeout).await } } diff --git a/substrate/client/transaction-pool/src/transaction_pool_wrapper.rs b/substrate/client/transaction-pool/src/transaction_pool_wrapper.rs index 4e1b53833b8f..e373c0278d80 100644 --- a/substrate/client/transaction-pool/src/transaction_pool_wrapper.rs +++ b/substrate/client/transaction-pool/src/transaction_pool_wrapper.rs @@ -22,16 +22,16 @@ use crate::{ builder::FullClientTransactionPool, graph::{base_pool::Transaction, ExtrinsicFor, ExtrinsicHash}, - ChainApi, FullChainApi, + ChainApi, FullChainApi, ReadyIteratorFor, }; use async_trait::async_trait; use sc_transaction_pool_api::{ ChainEvent, ImportNotificationStream, LocalTransactionFor, LocalTransactionPool, - MaintainedTransactionPool, PoolFuture, PoolStatus, ReadyTransactions, TransactionFor, - TransactionPool, TransactionSource, TransactionStatusStreamFor, TxHash, + MaintainedTransactionPool, PoolStatus, ReadyTransactions, TransactionFor, TransactionPool, + TransactionSource, TransactionStatusStreamFor, TxHash, }; use sp_runtime::traits::Block as BlockT; -use std::{collections::HashMap, future::Future, pin::Pin, sync::Arc}; +use std::{collections::HashMap, pin::Pin, sync::Arc}; /// The wrapper for actual object providing implementation of TransactionPool. /// @@ -49,6 +49,7 @@ where + 'static, Client::Api: sp_transaction_pool::runtime_api::TaggedTransactionQueue; +#[async_trait] impl TransactionPool for TransactionPoolWrapper where Block: BlockT, @@ -68,44 +69,38 @@ where >; type Error = as ChainApi>::Error; - fn submit_at( + async fn submit_at( &self, at: ::Hash, source: TransactionSource, xts: Vec>, - ) -> PoolFuture, Self::Error>>, Self::Error> { - self.0.submit_at(at, source, xts) + ) -> Result, Self::Error>>, Self::Error> { + self.0.submit_at(at, source, xts).await } - fn submit_one( + async fn submit_one( &self, at: ::Hash, source: TransactionSource, xt: TransactionFor, - ) -> PoolFuture, Self::Error> { - self.0.submit_one(at, source, xt) + ) -> Result, Self::Error> { + self.0.submit_one(at, source, xt).await } - fn submit_and_watch( + async fn submit_and_watch( &self, at: ::Hash, source: TransactionSource, xt: TransactionFor, - ) -> PoolFuture>>, Self::Error> { - self.0.submit_and_watch(at, source, xt) + ) -> Result>>, Self::Error> { + self.0.submit_and_watch(at, source, xt).await } - fn ready_at( + async fn ready_at( &self, at: ::Hash, - ) -> Pin< - Box< - dyn Future< - Output = Box> + Send>, - > + Send, - >, - > { - self.0.ready_at(at) + ) -> ReadyIteratorFor> { + self.0.ready_at(at).await } fn ready(&self) -> Box> + Send> { @@ -140,19 +135,12 @@ where self.0.ready_transaction(hash) } - fn ready_at_with_timeout( + async fn ready_at_with_timeout( &self, at: ::Hash, timeout: std::time::Duration, - ) -> Pin< - Box< - dyn Future< - Output = Box> + Send>, - > + Send - + '_, - >, - > { - self.0.ready_at_with_timeout(at, timeout) + ) -> ReadyIteratorFor> { + self.0.ready_at_with_timeout(at, timeout).await } } From 5e8348f03f49cc32ad48912e186c04b0f1212b16 Mon Sep 17 00:00:00 2001 From: Tobi Demeco <50408393+TDemeco@users.noreply.github.com> Date: Tue, 19 Nov 2024 06:29:15 -0300 Subject: [PATCH 2/6] sp-trie: correctly avoid panicking when decoding bad compact proofs (#6502) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description Opening another PR because I added a test to check for my fix pushed in #6486 and realized that for some reason I completely forgot how to code and did not fix the underlying issue, since out-of-bounds indexing could still happen even with the check I added. This one should fix that and, as an added bonus, has a simple test used as an integrity check to make sure future changes don't accidently revert this fix. Now `sp-trie` should definitely not panic when faced with bad `CompactProof`s. Sorry about that 😅 This, like #6486, is related to issue #6485 ## Integration No changes have to be done downstream, and as such the version bump should be minor. --------- Co-authored-by: Bastian Köcher --- prdoc/pr_6502.prdoc | 10 ++++++++++ substrate/primitives/trie/src/node_codec.rs | 8 ++++---- substrate/primitives/trie/src/storage_proof.rs | 10 +++++++++- 3 files changed, 23 insertions(+), 5 deletions(-) create mode 100644 prdoc/pr_6502.prdoc diff --git a/prdoc/pr_6502.prdoc b/prdoc/pr_6502.prdoc new file mode 100644 index 000000000000..3e2467ed5524 --- /dev/null +++ b/prdoc/pr_6502.prdoc @@ -0,0 +1,10 @@ +title: "sp-trie: correctly avoid panicking when decoding bad compact proofs" + +doc: + - audience: "Runtime Dev" + description: | + "Fixed the check introduced in [PR #6486](https://github.com/paritytech/polkadot-sdk/pull/6486). Now `sp-trie` correctly avoids panicking when decoding bad compact proofs." + +crates: +- name: sp-trie + bump: patch diff --git a/substrate/primitives/trie/src/node_codec.rs b/substrate/primitives/trie/src/node_codec.rs index 27da0c6334a2..400f57f3b1bf 100644 --- a/substrate/primitives/trie/src/node_codec.rs +++ b/substrate/primitives/trie/src/node_codec.rs @@ -110,8 +110,8 @@ where NodeHeader::Null => Ok(NodePlan::Empty), NodeHeader::HashedValueBranch(nibble_count) | NodeHeader::Branch(_, nibble_count) => { let padding = nibble_count % nibble_ops::NIBBLE_PER_BYTE != 0; - // data should be at least the size of the offset - if data.len() < input.offset { + // data should be at least of size offset + 1 + if data.len() < input.offset + 1 { return Err(Error::BadFormat) } // check that the padding is valid (if any) @@ -158,8 +158,8 @@ where }, NodeHeader::HashedValueLeaf(nibble_count) | NodeHeader::Leaf(nibble_count) => { let padding = nibble_count % nibble_ops::NIBBLE_PER_BYTE != 0; - // data should be at least the size of the offset - if data.len() < input.offset { + // data should be at least of size offset + 1 + if data.len() < input.offset + 1 { return Err(Error::BadFormat) } // check that the padding is valid (if any) diff --git a/substrate/primitives/trie/src/storage_proof.rs b/substrate/primitives/trie/src/storage_proof.rs index a9f6298742f6..bf0dc72e650b 100644 --- a/substrate/primitives/trie/src/storage_proof.rs +++ b/substrate/primitives/trie/src/storage_proof.rs @@ -232,7 +232,8 @@ pub mod tests { use super::*; use crate::{tests::create_storage_proof, StorageProof}; - type Layout = crate::LayoutV1; + type Hasher = sp_core::Blake2Hasher; + type Layout = crate::LayoutV1; const TEST_DATA: &[(&[u8], &[u8])] = &[(b"key1", &[1; 64]), (b"key2", &[2; 64]), (b"key3", &[3; 64]), (b"key11", &[4; 64])]; @@ -245,4 +246,11 @@ pub mod tests { Err(StorageProofError::DuplicateNodes) )); } + + #[test] + fn invalid_compact_proof_does_not_panic_when_decoding() { + let invalid_proof = CompactProof { encoded_nodes: vec![vec![135]] }; + let result = invalid_proof.to_memory_db::(None); + assert!(result.is_err()); + } } From 293d4a59a9c39e69e083724d45d3cec34ca7b6f2 Mon Sep 17 00:00:00 2001 From: Ermal Kaleci Date: Tue, 19 Nov 2024 10:34:03 +0100 Subject: [PATCH 3/6] [pallet-revive] Update delegate_call to accept address and weight (#6111) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Enhance the `delegate_call` function to accept an `address` target parameter instead of a `code_hash`. This allows direct identification of the target contract using the provided address. Additionally, introduce parameters for specifying a customizable `ref_time` limit and `proof_size` limit, thereby improving flexibility and control during contract interactions. --------- Co-authored-by: Alexander Theißen --- prdoc/pr_6111.prdoc | 17 ++ .../fixtures/contracts/delegate_call.rs | 8 +- .../contracts/delegate_call_deposit_limit.rs | 46 +++++ .../contracts/delegate_call_simple.rs | 6 +- .../contracts/locking_delegate_dependency.rs | 3 +- .../frame/revive/src/benchmarking/mod.rs | 25 ++- substrate/frame/revive/src/exec.rs | 172 +++++++++++++----- substrate/frame/revive/src/tests.rs | 112 +++++++++++- substrate/frame/revive/src/wasm/runtime.rs | 50 ++--- substrate/frame/revive/uapi/src/host.rs | 13 +- .../frame/revive/uapi/src/host/riscv32.rs | 51 ++++-- 11 files changed, 391 insertions(+), 112 deletions(-) create mode 100644 prdoc/pr_6111.prdoc create mode 100644 substrate/frame/revive/fixtures/contracts/delegate_call_deposit_limit.rs diff --git a/prdoc/pr_6111.prdoc b/prdoc/pr_6111.prdoc new file mode 100644 index 000000000000..4ada3031c805 --- /dev/null +++ b/prdoc/pr_6111.prdoc @@ -0,0 +1,17 @@ +title: "[pallet-revive] Update delegate_call to accept address and weight" + +doc: + - audience: Runtime Dev + description: | + Enhance the `delegate_call` function to accept an `address` target parameter instead of a `code_hash`. + This allows direct identification of the target contract using the provided address. + Additionally, introduce parameters for specifying a customizable `ref_time` limit and `proof_size` limit, + thereby improving flexibility and control during contract interactions. + +crates: + - name: pallet-revive + bump: major + - name: pallet-revive-fixtures + bump: patch + - name: pallet-revive-uapi + bump: major diff --git a/substrate/frame/revive/fixtures/contracts/delegate_call.rs b/substrate/frame/revive/fixtures/contracts/delegate_call.rs index 9fd155408af3..3cf74acf1321 100644 --- a/substrate/frame/revive/fixtures/contracts/delegate_call.rs +++ b/substrate/frame/revive/fixtures/contracts/delegate_call.rs @@ -28,7 +28,11 @@ pub extern "C" fn deploy() {} #[no_mangle] #[polkavm_derive::polkavm_export] pub extern "C" fn call() { - input!(code_hash: &[u8; 32],); + input!( + address: &[u8; 20], + ref_time: u64, + proof_size: u64, + ); let mut key = [0u8; 32]; key[0] = 1u8; @@ -42,7 +46,7 @@ pub extern "C" fn call() { assert!(value[0] == 2u8); let input = [0u8; 0]; - api::delegate_call(uapi::CallFlags::empty(), code_hash, &input, None).unwrap(); + api::delegate_call(uapi::CallFlags::empty(), address, ref_time, proof_size, None, &input, None).unwrap(); api::get_storage(StorageFlags::empty(), &key, value).unwrap(); assert!(value[0] == 1u8); diff --git a/substrate/frame/revive/fixtures/contracts/delegate_call_deposit_limit.rs b/substrate/frame/revive/fixtures/contracts/delegate_call_deposit_limit.rs new file mode 100644 index 000000000000..55203d534c9b --- /dev/null +++ b/substrate/frame/revive/fixtures/contracts/delegate_call_deposit_limit.rs @@ -0,0 +1,46 @@ +// 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. + +#![no_std] +#![no_main] + +use common::{input, u256_bytes}; +use uapi::{HostFn, HostFnImpl as api, StorageFlags}; + +#[no_mangle] +#[polkavm_derive::polkavm_export] +pub extern "C" fn deploy() {} + +#[no_mangle] +#[polkavm_derive::polkavm_export] +pub extern "C" fn call() { + input!( + address: &[u8; 20], + deposit_limit: u64, + ); + + let input = [0u8; 0]; + api::delegate_call(uapi::CallFlags::empty(), address, 0, 0, Some(&u256_bytes(deposit_limit)), &input, None).unwrap(); + + let mut key = [0u8; 32]; + key[0] = 1u8; + + let mut value = [0u8; 32]; + + api::get_storage(StorageFlags::empty(), &key, &mut &mut value[..]).unwrap(); + assert!(value[0] == 1u8); +} diff --git a/substrate/frame/revive/fixtures/contracts/delegate_call_simple.rs b/substrate/frame/revive/fixtures/contracts/delegate_call_simple.rs index 20f8ec3364ee..a8501dad4692 100644 --- a/substrate/frame/revive/fixtures/contracts/delegate_call_simple.rs +++ b/substrate/frame/revive/fixtures/contracts/delegate_call_simple.rs @@ -28,9 +28,9 @@ pub extern "C" fn deploy() {} #[no_mangle] #[polkavm_derive::polkavm_export] pub extern "C" fn call() { - input!(code_hash: &[u8; 32],); + input!(address: &[u8; 20],); - // Delegate call into passed code hash. + // Delegate call into passed address. let input = [0u8; 0]; - api::delegate_call(uapi::CallFlags::empty(), code_hash, &input, None).unwrap(); + api::delegate_call(uapi::CallFlags::empty(), address, 0, 0, None, &input, None).unwrap(); } diff --git a/substrate/frame/revive/fixtures/contracts/locking_delegate_dependency.rs b/substrate/frame/revive/fixtures/contracts/locking_delegate_dependency.rs index 54c7c7f3d5e2..3d7702c6537a 100644 --- a/substrate/frame/revive/fixtures/contracts/locking_delegate_dependency.rs +++ b/substrate/frame/revive/fixtures/contracts/locking_delegate_dependency.rs @@ -30,6 +30,7 @@ const ALICE_FALLBACK: [u8; 20] = [1u8; 20]; fn load_input(delegate_call: bool) { input!( action: u32, + address: &[u8; 20], code_hash: &[u8; 32], ); @@ -51,7 +52,7 @@ fn load_input(delegate_call: bool) { } if delegate_call { - api::delegate_call(uapi::CallFlags::empty(), code_hash, &[], None).unwrap(); + api::delegate_call(uapi::CallFlags::empty(), address, 0, 0, None, &[], None).unwrap(); } } diff --git a/substrate/frame/revive/src/benchmarking/mod.rs b/substrate/frame/revive/src/benchmarking/mod.rs index 40ad3a3aed17..9c4d817a07de 100644 --- a/substrate/frame/revive/src/benchmarking/mod.rs +++ b/substrate/frame/revive/src/benchmarking/mod.rs @@ -1555,25 +1555,36 @@ mod benchmarks { #[benchmark(pov_mode = Measured)] fn seal_delegate_call() -> Result<(), BenchmarkError> { - let hash = Contract::::with_index(1, WasmModule::dummy(), vec![])?.info()?.code_hash; + let Contract { account_id: address, .. } = + Contract::::with_index(1, WasmModule::dummy(), vec![]).unwrap(); + + let address_bytes = address.encode(); + let address_len = address_bytes.len() as u32; + + let deposit: BalanceOf = (u32::MAX - 100).into(); + let deposit_bytes = Into::::into(deposit).encode(); let mut setup = CallSetup::::default(); + setup.set_storage_deposit_limit(deposit); setup.set_origin(Origin::from_account_id(setup.contract().account_id.clone())); let (mut ext, _) = setup.ext(); let mut runtime = crate::wasm::Runtime::<_, [u8]>::new(&mut ext, vec![]); - let mut memory = memory!(hash.encode(),); + let mut memory = memory!(address_bytes, deposit_bytes,); let result; #[block] { result = runtime.bench_delegate_call( memory.as_mut_slice(), - 0, // flags - 0, // code_hash_ptr - 0, // input_data_ptr - 0, // input_data_len - SENTINEL, // output_ptr + 0, // flags + 0, // address_ptr + 0, // ref_time_limit + 0, // proof_size_limit + address_len, // deposit_ptr + 0, // input_data_ptr + 0, // input_data_len + SENTINEL, // output_ptr 0, ); } diff --git a/substrate/frame/revive/src/exec.rs b/substrate/frame/revive/src/exec.rs index fdb45f045bbc..49c08166483e 100644 --- a/substrate/frame/revive/src/exec.rs +++ b/substrate/frame/revive/src/exec.rs @@ -210,7 +210,13 @@ pub trait Ext: sealing::Sealed { /// Execute code in the current frame. /// /// Returns the code size of the called contract. - fn delegate_call(&mut self, code: H256, input_data: Vec) -> Result<(), ExecError>; + fn delegate_call( + &mut self, + gas_limit: Weight, + deposit_limit: U256, + address: H160, + input_data: Vec, + ) -> Result<(), ExecError>; /// Instantiate a contract from the given code. /// @@ -581,18 +587,30 @@ struct Frame { allows_reentry: bool, /// If `true` subsequent calls cannot modify storage. read_only: bool, - /// The caller of the currently executing frame which was spawned by `delegate_call`. - delegate_caller: Option>, + /// The delegate call info of the currently executing frame which was spawned by + /// `delegate_call`. + delegate: Option>, /// The output of the last executed call frame. last_frame_output: ExecReturnValue, } +/// This structure is used to represent the arguments in a delegate call frame in order to +/// distinguish who delegated the call and where it was delegated to. +struct DelegateInfo { + /// The caller of the contract. + pub caller: Origin, + /// The address of the contract the call was delegated to. + pub callee: H160, +} + /// Used in a delegate call frame arguments in order to override the executable and caller. struct DelegatedCall { /// The executable which is run instead of the contracts own `executable`. executable: E, /// The caller of the contract. caller: Origin, + /// The address of the contract the call was delegated to. + callee: H160, } /// Parameter passed in when creating a new `Frame`. @@ -898,8 +916,7 @@ where read_only: bool, origin_is_caller: bool, ) -> Result, E)>, ExecError> { - let (account_id, contract_info, executable, delegate_caller, entry_point) = match frame_args - { + let (account_id, contract_info, executable, delegate, entry_point) = match frame_args { FrameArgs::Call { dest, cached_info, delegated_call } => { let contract = if let Some(contract) = cached_info { contract @@ -914,8 +931,8 @@ where }; let (executable, delegate_caller) = - if let Some(DelegatedCall { executable, caller }) = delegated_call { - (executable, Some(caller)) + if let Some(DelegatedCall { executable, caller, callee }) = delegated_call { + (executable, Some(DelegateInfo { caller, callee })) } else { (E::from_storage(contract.code_hash, gas_meter)?, None) }; @@ -931,8 +948,8 @@ where use sp_runtime::Saturating; address::create1( &deployer, - // the Nonce from the origin has been incremented pre-dispatch, so we need - // to subtract 1 to get the nonce at the time of the call. + // the Nonce from the origin has been incremented pre-dispatch, so we + // need to subtract 1 to get the nonce at the time of the call. if origin_is_caller { account_nonce.saturating_sub(1u32.into()).saturated_into() } else { @@ -956,7 +973,7 @@ where }; let frame = Frame { - delegate_caller, + delegate, value_transferred, contract_info: CachedContract::Cached(contract_info), account_id, @@ -1025,7 +1042,7 @@ where let frame = self.top_frame(); let entry_point = frame.entry_point; let delegated_code_hash = - if frame.delegate_caller.is_some() { Some(*executable.code_hash()) } else { None }; + if frame.delegate.is_some() { Some(*executable.code_hash()) } else { None }; // The output of the caller frame will be replaced by the output of this run. // It is also not accessible from nested frames. @@ -1103,7 +1120,13 @@ where frame.nested_storage.enforce_limit(contract)?; } - let frame = self.top_frame(); + let frame = self.top_frame_mut(); + + // If a special limit was set for the sub-call, we enforce it here. + // The sub-call will be rolled back in case the limit is exhausted. + let contract = frame.contract_info.as_contract(); + frame.nested_storage.enforce_subcall_limit(contract)?; + let account_id = T::AddressMapper::to_address(&frame.account_id); match (entry_point, delegated_code_hash) { (ExportedFunction::Constructor, _) => { @@ -1112,15 +1135,7 @@ where return Err(Error::::TerminatedInConstructor.into()); } - // If a special limit was set for the sub-call, we enforce it here. - // This is needed because contract constructor might write to storage. - // The sub-call will be rolled back in case the limit is exhausted. - let frame = self.top_frame_mut(); - let contract = frame.contract_info.as_contract(); - frame.nested_storage.enforce_subcall_limit(contract)?; - let caller = T::AddressMapper::to_address(self.caller().account_id()?); - // Deposit an instantiation event. Contracts::::deposit_event(Event::Instantiated { deployer: caller, @@ -1134,12 +1149,6 @@ where }); }, (ExportedFunction::Call, None) => { - // If a special limit was set for the sub-call, we enforce it here. - // The sub-call will be rolled back in case the limit is exhausted. - let frame = self.top_frame_mut(); - let contract = frame.contract_info.as_contract(); - frame.nested_storage.enforce_subcall_limit(contract)?; - let caller = self.caller(); Contracts::::deposit_event(Event::Called { caller: caller.clone(), @@ -1468,11 +1477,20 @@ where result } - fn delegate_call(&mut self, code_hash: H256, input_data: Vec) -> Result<(), ExecError> { + fn delegate_call( + &mut self, + gas_limit: Weight, + deposit_limit: U256, + address: H160, + input_data: Vec, + ) -> Result<(), ExecError> { // We reset the return data now, so it is cleared out even if no new frame was executed. // This is for example the case for unknown code hashes or creating the frame fails. *self.last_frame_output_mut() = Default::default(); + let code_hash = ContractInfoOf::::get(&address) + .ok_or(Error::::CodeNotFound) + .map(|c| c.code_hash)?; let executable = E::from_storage(code_hash, self.gas_meter_mut())?; let top_frame = self.top_frame_mut(); let contract_info = top_frame.contract_info().clone(); @@ -1482,11 +1500,15 @@ where FrameArgs::Call { dest: account_id, cached_info: Some(contract_info), - delegated_call: Some(DelegatedCall { executable, caller: self.caller().clone() }), + delegated_call: Some(DelegatedCall { + executable, + caller: self.caller().clone(), + callee: address, + }), }, value, - Weight::zero(), - BalanceOf::::zero(), + gas_limit, + deposit_limit.try_into().map_err(|_| Error::::BalanceConversionFailed)?, self.is_read_only(), )?; self.run(executable.expect(FRAME_ALWAYS_EXISTS_ON_INSTANTIATE), input_data) @@ -1601,7 +1623,7 @@ where } fn caller(&self) -> Origin { - if let Some(caller) = &self.top_frame().delegate_caller { + if let Some(DelegateInfo { caller, .. }) = &self.top_frame().delegate { caller.clone() } else { self.frames() @@ -1655,7 +1677,13 @@ where return Err(Error::::InvalidImmutableAccess.into()); } - let address = T::AddressMapper::to_address(self.account_id()); + // Immutable is read from contract code being executed + let address = self + .top_frame() + .delegate + .as_ref() + .map(|d| d.callee) + .unwrap_or(T::AddressMapper::to_address(self.account_id())); Ok(>::get(address).ok_or_else(|| Error::::InvalidImmutableAccess)?) } @@ -1917,7 +1945,7 @@ mod tests { AddressMapper, Error, }; use assert_matches::assert_matches; - use frame_support::{assert_err, assert_ok, parameter_types}; + use frame_support::{assert_err, assert_noop, assert_ok, parameter_types}; use frame_system::{AccountInfo, EventRecord, Phase}; use pallet_revive_uapi::ReturnFlags; use pretty_assertions::assert_eq; @@ -2185,18 +2213,20 @@ mod tests { let delegate_ch = MockLoader::insert(Call, move |ctx, _| { assert_eq!(ctx.ext.value_transferred(), U256::from(value)); - let _ = ctx.ext.delegate_call(success_ch, Vec::new())?; + let _ = + ctx.ext.delegate_call(Weight::zero(), U256::zero(), CHARLIE_ADDR, Vec::new())?; Ok(ExecReturnValue { flags: ReturnFlags::empty(), data: Vec::new() }) }); ExtBuilder::default().build().execute_with(|| { place_contract(&BOB, delegate_ch); + place_contract(&CHARLIE, success_ch); set_balance(&ALICE, 100); let balance = get_balance(&BOB_FALLBACK); let origin = Origin::from_account_id(ALICE); let mut storage_meter = storage::meter::Meter::new(&origin, 0, 55).unwrap(); - let _ = MockStack::run_call( + assert_ok!(MockStack::run_call( origin, BOB_ADDR, &mut GasMeter::::new(GAS_LIMIT), @@ -2204,14 +2234,63 @@ mod tests { value.into(), vec![], None, - ) - .unwrap(); + )); assert_eq!(get_balance(&ALICE), 100 - value); assert_eq!(get_balance(&BOB_FALLBACK), balance + value); }); } + #[test] + fn delegate_call_missing_contract() { + let missing_ch = MockLoader::insert(Call, move |_ctx, _| { + Ok(ExecReturnValue { flags: ReturnFlags::empty(), data: Vec::new() }) + }); + + let delegate_ch = MockLoader::insert(Call, move |ctx, _| { + let _ = + ctx.ext.delegate_call(Weight::zero(), U256::zero(), CHARLIE_ADDR, Vec::new())?; + Ok(ExecReturnValue { flags: ReturnFlags::empty(), data: Vec::new() }) + }); + + ExtBuilder::default().build().execute_with(|| { + place_contract(&BOB, delegate_ch); + set_balance(&ALICE, 100); + + let origin = Origin::from_account_id(ALICE); + let mut storage_meter = storage::meter::Meter::new(&origin, 0, 55).unwrap(); + + // contract code missing + assert_noop!( + MockStack::run_call( + origin.clone(), + BOB_ADDR, + &mut GasMeter::::new(GAS_LIMIT), + &mut storage_meter, + U256::zero(), + vec![], + None, + ), + ExecError { + error: Error::::CodeNotFound.into(), + origin: ErrorOrigin::Callee, + } + ); + + // add missing contract code + place_contract(&CHARLIE, missing_ch); + assert_ok!(MockStack::run_call( + origin, + BOB_ADDR, + &mut GasMeter::::new(GAS_LIMIT), + &mut storage_meter, + U256::zero(), + vec![], + None, + )); + }); + } + #[test] fn changes_are_reverted_on_failing_call() { // This test verifies that changes are reverted on a call which fails (or equally, returns @@ -4615,7 +4694,12 @@ mod tests { // An unknown code hash to fail the delegate_call on the first condition. *ctx.ext.last_frame_output_mut() = output_revert(); assert_eq!( - ctx.ext.delegate_call(invalid_code_hash, Default::default()), + ctx.ext.delegate_call( + Weight::zero(), + U256::zero(), + H160([0xff; 20]), + Default::default() + ), Err(Error::::CodeNotFound.into()) ); assert_eq!(ctx.ext.last_frame_output(), &Default::default()); @@ -4732,14 +4816,12 @@ mod tests { Ok(vec![2]), ); - // In a delegate call, we should witness the caller immutable data + // Also in a delegate call, we should witness the callee immutable data assert_eq!( - ctx.ext.delegate_call(charlie_ch, Vec::new()).map(|_| ctx - .ext - .last_frame_output() - .data - .clone()), - Ok(vec![1]) + ctx.ext + .delegate_call(Weight::zero(), U256::zero(), CHARLIE_ADDR, Vec::new()) + .map(|_| ctx.ext.last_frame_output().data.clone()), + Ok(vec![2]) ); exec_success() diff --git a/substrate/frame/revive/src/tests.rs b/substrate/frame/revive/src/tests.rs index a35e4d908601..177b8dff706b 100644 --- a/substrate/frame/revive/src/tests.rs +++ b/substrate/frame/revive/src/tests.rs @@ -1127,7 +1127,7 @@ fn deploy_and_call_other_contract() { #[test] fn delegate_call() { let (caller_wasm, _caller_code_hash) = compile_module("delegate_call").unwrap(); - let (callee_wasm, callee_code_hash) = compile_module("delegate_call_lib").unwrap(); + let (callee_wasm, _callee_code_hash) = compile_module("delegate_call_lib").unwrap(); ExtBuilder::default().existential_deposit(500).build().execute_with(|| { let _ = ::Currency::set_balance(&ALICE, 1_000_000); @@ -1137,12 +1137,91 @@ fn delegate_call() { builder::bare_instantiate(Code::Upload(caller_wasm)) .value(300_000) .build_and_unwrap_contract(); - // Only upload 'callee' code - assert_ok!(Contracts::upload_code(RuntimeOrigin::signed(ALICE), callee_wasm, 100_000,)); + + // Instantiate the 'callee' + let Contract { addr: callee_addr, .. } = + builder::bare_instantiate(Code::Upload(callee_wasm)) + .value(100_000) + .build_and_unwrap_contract(); assert_ok!(builder::call(caller_addr) .value(1337) - .data(callee_code_hash.as_ref().to_vec()) + .data((callee_addr, 0u64, 0u64).encode()) + .build()); + }); +} + +#[test] +fn delegate_call_with_weight_limit() { + let (caller_wasm, _caller_code_hash) = compile_module("delegate_call").unwrap(); + let (callee_wasm, _callee_code_hash) = compile_module("delegate_call_lib").unwrap(); + + ExtBuilder::default().existential_deposit(500).build().execute_with(|| { + let _ = ::Currency::set_balance(&ALICE, 1_000_000); + + // Instantiate the 'caller' + let Contract { addr: caller_addr, .. } = + builder::bare_instantiate(Code::Upload(caller_wasm)) + .value(300_000) + .build_and_unwrap_contract(); + + // Instantiate the 'callee' + let Contract { addr: callee_addr, .. } = + builder::bare_instantiate(Code::Upload(callee_wasm)) + .value(100_000) + .build_and_unwrap_contract(); + + // fails, not enough weight + assert_err!( + builder::bare_call(caller_addr) + .value(1337) + .data((callee_addr, 100u64, 100u64).encode()) + .build() + .result, + Error::::ContractTrapped, + ); + + assert_ok!(builder::call(caller_addr) + .value(1337) + .data((callee_addr, 500_000_000u64, 100_000u64).encode()) + .build()); + }); +} + +#[test] +fn delegate_call_with_deposit_limit() { + let (caller_pvm, _caller_code_hash) = compile_module("delegate_call_deposit_limit").unwrap(); + let (callee_pvm, _callee_code_hash) = compile_module("delegate_call_lib").unwrap(); + + ExtBuilder::default().existential_deposit(500).build().execute_with(|| { + let _ = ::Currency::set_balance(&ALICE, 1_000_000); + + // Instantiate the 'caller' + let Contract { addr: caller_addr, .. } = + builder::bare_instantiate(Code::Upload(caller_pvm)) + .value(300_000) + .build_and_unwrap_contract(); + + // Instantiate the 'callee' + let Contract { addr: callee_addr, .. } = + builder::bare_instantiate(Code::Upload(callee_pvm)) + .value(100_000) + .build_and_unwrap_contract(); + + // Delegate call will write 1 storage and deposit of 2 (1 item) + 32 (bytes) is required. + // Fails, not enough deposit + assert_err!( + builder::bare_call(caller_addr) + .value(1337) + .data((callee_addr, 33u64).encode()) + .build() + .result, + Error::::StorageDepositLimitExhausted, + ); + + assert_ok!(builder::call(caller_addr) + .value(1337) + .data((callee_addr, 34u64).encode()) .build()); }); } @@ -3666,6 +3745,12 @@ fn locking_delegate_dependency_works() { .map(|c| sp_core::H256(sp_io::hashing::keccak_256(c))) .collect(); + let hash2addr = |code_hash: &H256| { + let mut addr = H160::zero(); + addr.as_bytes_mut().copy_from_slice(&code_hash.as_ref()[..20]); + addr + }; + // Define inputs with various actions to test locking / unlocking delegate_dependencies. // See the contract for more details. let noop_input = (0u32, callee_hashes[0]); @@ -3675,17 +3760,19 @@ fn locking_delegate_dependency_works() { // Instantiate the caller contract with the given input. let instantiate = |input: &(u32, H256)| { + let (action, code_hash) = input; builder::bare_instantiate(Code::Upload(wasm_caller.clone())) .origin(RuntimeOrigin::signed(ALICE_FALLBACK)) - .data(input.encode()) + .data((action, hash2addr(code_hash), code_hash).encode()) .build() }; // Call contract with the given input. let call = |addr_caller: &H160, input: &(u32, H256)| { + let (action, code_hash) = input; builder::bare_call(*addr_caller) .origin(RuntimeOrigin::signed(ALICE_FALLBACK)) - .data(input.encode()) + .data((action, hash2addr(code_hash), code_hash).encode()) .build() }; const ED: u64 = 2000; @@ -3702,7 +3789,7 @@ fn locking_delegate_dependency_works() { // Upload all the delegated codes (they all have the same size) let mut deposit = Default::default(); for code in callee_codes.iter() { - let CodeUploadReturnValue { deposit: deposit_per_code, .. } = + let CodeUploadReturnValue { deposit: deposit_per_code, code_hash } = Contracts::bare_upload_code( RuntimeOrigin::signed(ALICE_FALLBACK), code.clone(), @@ -3710,6 +3797,9 @@ fn locking_delegate_dependency_works() { ) .unwrap(); deposit = deposit_per_code; + // Mock contract info by using first 20 bytes of code_hash as address. + let addr = hash2addr(&code_hash); + ContractInfoOf::::set(&addr, ContractInfo::new(&addr, 0, code_hash).ok()); } // Instantiate should now work. @@ -3746,7 +3836,11 @@ fn locking_delegate_dependency_works() { // Locking self should fail. assert_err!( - call(&addr_caller, &(1u32, self_code_hash)).result, + builder::bare_call(addr_caller) + .origin(RuntimeOrigin::signed(ALICE_FALLBACK)) + .data((1u32, &addr_caller, self_code_hash).encode()) + .build() + .result, Error::::CannotAddSelfAsDelegateDependency ); @@ -3785,7 +3879,7 @@ fn locking_delegate_dependency_works() { assert_err!( builder::bare_call(addr_caller) .storage_deposit_limit(dependency_deposit - 1) - .data(lock_delegate_dependency_input.encode()) + .data((1u32, hash2addr(&callee_hashes[0]), callee_hashes[0]).encode()) .build() .result, Error::::StorageDepositLimitExhausted diff --git a/substrate/frame/revive/src/wasm/runtime.rs b/substrate/frame/revive/src/wasm/runtime.rs index 8310fe701013..3e2c83db1ebd 100644 --- a/substrate/frame/revive/src/wasm/runtime.rs +++ b/substrate/frame/revive/src/wasm/runtime.rs @@ -536,16 +536,17 @@ macro_rules! charge_gas { /// The kind of call that should be performed. enum CallType { /// Execute another instantiated contract - Call { callee_ptr: u32, value_ptr: u32, deposit_ptr: u32, weight: Weight }, - /// Execute deployed code in the context (storage, account ID, value) of the caller contract - DelegateCall { code_hash_ptr: u32 }, + Call { value_ptr: u32 }, + /// Execute another contract code in the context (storage, account ID, value) of the caller + /// contract + DelegateCall, } impl CallType { fn cost(&self) -> RuntimeCosts { match self { CallType::Call { .. } => RuntimeCosts::CallBase, - CallType::DelegateCall { .. } => RuntimeCosts::DelegateCallBase, + CallType::DelegateCall => RuntimeCosts::DelegateCallBase, } } } @@ -987,6 +988,9 @@ impl<'a, E: Ext, M: ?Sized + Memory> Runtime<'a, E, M> { memory: &mut M, flags: CallFlags, call_type: CallType, + callee_ptr: u32, + deposit_ptr: u32, + weight: Weight, input_data_ptr: u32, input_data_len: u32, output_ptr: u32, @@ -994,6 +998,10 @@ impl<'a, E: Ext, M: ?Sized + Memory> Runtime<'a, E, M> { ) -> Result { self.charge_gas(call_type.cost())?; + let callee = memory.read_h160(callee_ptr)?; + let deposit_limit = + if deposit_ptr == SENTINEL { U256::zero() } else { memory.read_u256(deposit_ptr)? }; + let input_data = if flags.contains(CallFlags::CLONE_INPUT) { let input = self.input_data.as_ref().ok_or(Error::::InputForwarded)?; charge_gas!(self, RuntimeCosts::CallInputCloned(input.len() as u32))?; @@ -1006,13 +1014,7 @@ impl<'a, E: Ext, M: ?Sized + Memory> Runtime<'a, E, M> { }; let call_outcome = match call_type { - CallType::Call { callee_ptr, value_ptr, deposit_ptr, weight } => { - let callee = memory.read_h160(callee_ptr)?; - let deposit_limit = if deposit_ptr == SENTINEL { - U256::zero() - } else { - memory.read_u256(deposit_ptr)? - }; + CallType::Call { value_ptr } => { let read_only = flags.contains(CallFlags::READ_ONLY); let value = memory.read_u256(value_ptr)?; if value > 0u32.into() { @@ -1033,13 +1035,11 @@ impl<'a, E: Ext, M: ?Sized + Memory> Runtime<'a, E, M> { read_only, ) }, - CallType::DelegateCall { code_hash_ptr } => { + CallType::DelegateCall => { if flags.intersects(CallFlags::ALLOW_REENTRY | CallFlags::READ_ONLY) { return Err(Error::::InvalidCallFlags.into()); } - - let code_hash = memory.read_h256(code_hash_ptr)?; - self.ext.delegate_call(code_hash, input_data) + self.ext.delegate_call(weight, deposit_limit, callee, input_data) }, }; @@ -1252,12 +1252,10 @@ pub mod env { self.call( memory, CallFlags::from_bits(flags).ok_or(Error::::InvalidCallFlags)?, - CallType::Call { - callee_ptr, - value_ptr, - deposit_ptr, - weight: Weight::from_parts(ref_time_limit, proof_size_limit), - }, + CallType::Call { value_ptr }, + callee_ptr, + deposit_ptr, + Weight::from_parts(ref_time_limit, proof_size_limit), input_data_ptr, input_data_len, output_ptr, @@ -1272,7 +1270,10 @@ pub mod env { &mut self, memory: &mut M, flags: u32, - code_hash_ptr: u32, + address_ptr: u32, + ref_time_limit: u64, + proof_size_limit: u64, + deposit_ptr: u32, input_data_ptr: u32, input_data_len: u32, output_ptr: u32, @@ -1281,7 +1282,10 @@ pub mod env { self.call( memory, CallFlags::from_bits(flags).ok_or(Error::::InvalidCallFlags)?, - CallType::DelegateCall { code_hash_ptr }, + CallType::DelegateCall, + address_ptr, + deposit_ptr, + Weight::from_parts(ref_time_limit, proof_size_limit), input_data_ptr, input_data_len, output_ptr, diff --git a/substrate/frame/revive/uapi/src/host.rs b/substrate/frame/revive/uapi/src/host.rs index cb52cf93540b..6b3a8b07f040 100644 --- a/substrate/frame/revive/uapi/src/host.rs +++ b/substrate/frame/revive/uapi/src/host.rs @@ -323,7 +323,13 @@ pub trait HostFn: private::Sealed { /// # Parameters /// /// - `flags`: See [`CallFlags`] for a documentation of the supported flags. - /// - `code_hash`: The hash of the code to be executed. + /// - `address`: The address of the code to be executed. Should be decodable as an + /// `T::AccountId`. Traps otherwise. + /// - `ref_time_limit`: how much *ref_time* Weight to devote to the execution. + /// - `proof_size_limit`: how much *proof_size* Weight to devote to the execution. + /// - `deposit_limit`: The storage deposit limit for delegate call. Passing `None` means setting + /// no specific limit for the call, which implies storage usage up to the limit of the parent + /// call. /// - `input`: The input data buffer used to call the contract. /// - `output`: A reference to the output data buffer to write the call output buffer. If `None` /// is provided then the output buffer is not copied. @@ -338,7 +344,10 @@ pub trait HostFn: private::Sealed { /// - [CodeNotFound][`crate::ReturnErrorCode::CodeNotFound] fn delegate_call( flags: CallFlags, - code_hash: &[u8; 32], + address: &[u8; 20], + ref_time_limit: u64, + proof_size_limit: u64, + deposit_limit: Option<&[u8; 32]>, input_data: &[u8], output: Option<&mut &mut [u8]>, ) -> Result; diff --git a/substrate/frame/revive/uapi/src/host/riscv32.rs b/substrate/frame/revive/uapi/src/host/riscv32.rs index 199a0abc3ddc..e8b27057ed18 100644 --- a/substrate/frame/revive/uapi/src/host/riscv32.rs +++ b/substrate/frame/revive/uapi/src/host/riscv32.rs @@ -59,14 +59,7 @@ mod sys { out_len_ptr: *mut u32, ) -> ReturnCode; pub fn call(ptr: *const u8) -> ReturnCode; - pub fn delegate_call( - flags: u32, - code_hash_ptr: *const u8, - input_data_ptr: *const u8, - input_data_len: u32, - out_ptr: *mut u8, - out_len_ptr: *mut u32, - ) -> ReturnCode; + pub fn delegate_call(ptr: *const u8) -> ReturnCode; pub fn instantiate(ptr: *const u8) -> ReturnCode; pub fn terminate(beneficiary_ptr: *const u8); pub fn input(out_ptr: *mut u8, out_len_ptr: *mut u32); @@ -306,24 +299,42 @@ impl HostFn for HostFnImpl { fn delegate_call( flags: CallFlags, - code_hash: &[u8; 32], + address: &[u8; 20], + ref_time_limit: u64, + proof_size_limit: u64, + deposit_limit: Option<&[u8; 32]>, input: &[u8], mut output: Option<&mut &mut [u8]>, ) -> Result { let (output_ptr, mut output_len) = ptr_len_or_sentinel(&mut output); - let ret_code = { - unsafe { - sys::delegate_call( - flags.bits(), - code_hash.as_ptr(), - input.as_ptr(), - input.len() as u32, - output_ptr, - &mut output_len, - ) - } + let deposit_limit_ptr = ptr_or_sentinel(&deposit_limit); + #[repr(packed)] + #[allow(dead_code)] + struct Args { + flags: u32, + address: *const u8, + ref_time_limit: u64, + proof_size_limit: u64, + deposit_limit: *const u8, + input: *const u8, + input_len: u32, + output: *mut u8, + output_len: *mut u32, + } + let args = Args { + flags: flags.bits(), + address: address.as_ptr(), + ref_time_limit, + proof_size_limit, + deposit_limit: deposit_limit_ptr, + input: input.as_ptr(), + input_len: input.len() as _, + output: output_ptr, + output_len: &mut output_len as *mut _, }; + let ret_code = { unsafe { sys::delegate_call(&args as *const Args as *const _) } }; + if let Some(ref mut output) = output { extract_from_slice(output, output_len as usize); } From 0449b214accd0f0fbf7ea3e8f3a8d8b7f99445e4 Mon Sep 17 00:00:00 2001 From: tmpolaczyk <44604217+tmpolaczyk@users.noreply.github.com> Date: Tue, 19 Nov 2024 10:43:09 +0100 Subject: [PATCH 4/6] Fix metrics not shutting down if there are open connections (#6220) Fix prometheus metrics not shutting down if there are open connections. I fixed the same issue in the past but it broke again after a dependecy upgrade. See also: https://github.com/paritytech/polkadot-sdk/pull/1637 --- prdoc/pr_6220.prdoc | 10 ++++++++++ substrate/utils/prometheus/Cargo.toml | 2 +- substrate/utils/prometheus/src/lib.rs | 2 ++ 3 files changed, 13 insertions(+), 1 deletion(-) create mode 100644 prdoc/pr_6220.prdoc diff --git a/prdoc/pr_6220.prdoc b/prdoc/pr_6220.prdoc new file mode 100644 index 000000000000..6a5ee4fa59be --- /dev/null +++ b/prdoc/pr_6220.prdoc @@ -0,0 +1,10 @@ +title: Fix metrics not shutting down if there are open connections + +doc: + - audience: Runtime Dev + description: | + Fix prometheus metrics not shutting down if there are open connections + +crates: +- name: substrate-prometheus-endpoint + bump: patch diff --git a/substrate/utils/prometheus/Cargo.toml b/substrate/utils/prometheus/Cargo.toml index 9bdec3cb8183..b8dfd6fb2bee 100644 --- a/substrate/utils/prometheus/Cargo.toml +++ b/substrate/utils/prometheus/Cargo.toml @@ -18,7 +18,7 @@ targets = ["x86_64-unknown-linux-gnu"] [dependencies] http-body-util = { workspace = true } hyper = { features = ["http1", "server"], workspace = true } -hyper-util = { features = ["server-auto", "tokio"], workspace = true } +hyper-util = { features = ["server-auto", "server-graceful", "tokio"], workspace = true } log = { workspace = true, default-features = true } prometheus = { workspace = true } thiserror = { workspace = true } diff --git a/substrate/utils/prometheus/src/lib.rs b/substrate/utils/prometheus/src/lib.rs index 35597cad03d8..5edac2e6650f 100644 --- a/substrate/utils/prometheus/src/lib.rs +++ b/substrate/utils/prometheus/src/lib.rs @@ -102,6 +102,7 @@ async fn init_prometheus_with_listener( log::info!(target: "prometheus", "〽️ Prometheus exporter started at {}", listener.local_addr()?); let server = hyper_util::server::conn::auto::Builder::new(hyper_util::rt::TokioExecutor::new()); + let graceful = hyper_util::server::graceful::GracefulShutdown::new(); loop { let io = match listener.accept().await { @@ -120,6 +121,7 @@ async fn init_prometheus_with_listener( hyper::service::service_fn(move |req| request_metrics(req, registry.clone())), ) .into_owned(); + let conn = graceful.watch(conn); tokio::spawn(async move { if let Err(err) = conn.await { From 8d4138f77106a6af49920ad84f3283f696f3f905 Mon Sep 17 00:00:00 2001 From: Maciej Date: Tue, 19 Nov 2024 14:40:25 +0000 Subject: [PATCH 5/6] Validator Re-Enabling (#5724) Aims to implement Stage 3 of Validator Disbling as outlined here: https://github.com/paritytech/polkadot-sdk/issues/4359 Features: - [x] New Disabling Strategy (Staking level) - [x] Re-enabling logic (Session level) - [x] More generic disabling decision output - [x] New Disabling Events Testing & Security: - [x] Unit tests - [x] Mock tests - [x] Try-runtime checks - [x] Try-runtime tested on westend snap - [x] Try-runtime CI tests - [ ] Re-enabling Zombienet Test (?) - [ ] SRLabs Audit Closes #4745 Closes #2418 --------- Co-authored-by: ordian Co-authored-by: Ankan <10196091+Ank4n@users.noreply.github.com> Co-authored-by: Tsvetomir Dimitrov --- .../src/validate_block/trie_cache.rs | 5 +- .../src/validate_block/trie_recorder.rs | 5 +- polkadot/runtime/test-runtime/src/lib.rs | 2 +- polkadot/runtime/westend/src/lib.rs | 3 +- prdoc/pr_5724.prdoc | 37 ++ substrate/bin/node/runtime/src/lib.rs | 2 +- .../test-staking-e2e/src/lib.rs | 27 +- .../test-staking-e2e/src/mock.rs | 3 +- substrate/frame/session/src/lib.rs | 21 +- substrate/frame/staking/CHANGELOG.md | 12 + substrate/frame/staking/src/lib.rs | 175 ++++++- substrate/frame/staking/src/migrations.rs | 76 +++ substrate/frame/staking/src/mock.rs | 6 +- substrate/frame/staking/src/pallet/impls.rs | 17 +- substrate/frame/staking/src/pallet/mod.rs | 17 +- substrate/frame/staking/src/slashing.rs | 51 +- substrate/frame/staking/src/tests.rs | 442 +++++++++++++++++- substrate/primitives/staking/src/offence.rs | 25 + .../state-machine/src/trie_backend.rs | 20 +- substrate/primitives/trie/src/recorder.rs | 5 +- 20 files changed, 864 insertions(+), 87 deletions(-) create mode 100644 prdoc/pr_5724.prdoc diff --git a/cumulus/pallets/parachain-system/src/validate_block/trie_cache.rs b/cumulus/pallets/parachain-system/src/validate_block/trie_cache.rs index 035541fb17b1..36efd3decf77 100644 --- a/cumulus/pallets/parachain-system/src/validate_block/trie_cache.rs +++ b/cumulus/pallets/parachain-system/src/validate_block/trie_cache.rs @@ -85,7 +85,10 @@ impl CacheProvider { } impl TrieCacheProvider for CacheProvider { - type Cache<'a> = TrieCache<'a, H> where H: 'a; + type Cache<'a> + = TrieCache<'a, H> + where + H: 'a; fn as_trie_db_cache(&self, storage_root: ::Out) -> Self::Cache<'_> { TrieCache { 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 4a478d047f1b..8dc2f20dd390 100644 --- a/cumulus/pallets/parachain-system/src/validate_block/trie_recorder.rs +++ b/cumulus/pallets/parachain-system/src/validate_block/trie_recorder.rs @@ -115,7 +115,10 @@ impl SizeOnlyRecorderProvider { } impl sp_trie::TrieRecorderProvider for SizeOnlyRecorderProvider { - type Recorder<'a> = SizeOnlyRecorder<'a, H> where H: 'a; + type Recorder<'a> + = SizeOnlyRecorder<'a, H> + where + H: 'a; fn drain_storage_proof(self) -> Option { None diff --git a/polkadot/runtime/test-runtime/src/lib.rs b/polkadot/runtime/test-runtime/src/lib.rs index d2ed5abb6ed1..69ce187dce40 100644 --- a/polkadot/runtime/test-runtime/src/lib.rs +++ b/polkadot/runtime/test-runtime/src/lib.rs @@ -395,7 +395,7 @@ impl pallet_staking::Config for Runtime { type BenchmarkingConfig = polkadot_runtime_common::StakingBenchmarkingConfig; type EventListeners = (); type WeightInfo = (); - type DisablingStrategy = pallet_staking::UpToLimitDisablingStrategy; + type DisablingStrategy = pallet_staking::UpToLimitWithReEnablingDisablingStrategy; } parameter_types! { diff --git a/polkadot/runtime/westend/src/lib.rs b/polkadot/runtime/westend/src/lib.rs index 993010cbce66..7a5562cc98c1 100644 --- a/polkadot/runtime/westend/src/lib.rs +++ b/polkadot/runtime/westend/src/lib.rs @@ -755,7 +755,7 @@ impl pallet_staking::Config for Runtime { type BenchmarkingConfig = polkadot_runtime_common::StakingBenchmarkingConfig; type EventListeners = (NominationPools, DelegatedStaking); type WeightInfo = weights::pallet_staking::WeightInfo; - type DisablingStrategy = pallet_staking::UpToLimitDisablingStrategy; + type DisablingStrategy = pallet_staking::UpToLimitWithReEnablingDisablingStrategy; } impl pallet_fast_unstake::Config for Runtime { @@ -1836,6 +1836,7 @@ pub mod migrations { >, parachains_shared::migration::MigrateToV1, parachains_scheduler::migration::MigrateV2ToV3, + pallet_staking::migrations::v16::MigrateV15ToV16, // permanent pallet_xcm::migration::MigrateToLatestXcmVersion, ); diff --git a/prdoc/pr_5724.prdoc b/prdoc/pr_5724.prdoc new file mode 100644 index 000000000000..be9d21c214a8 --- /dev/null +++ b/prdoc/pr_5724.prdoc @@ -0,0 +1,37 @@ +# 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: Validator Re-Enabling (master PR) + +doc: + - audience: Runtime Dev + description: | + Implementation of the Stage 3 for the New Disabling Strategy: https://github.com/paritytech/polkadot-sdk/issues/4359 + + This PR changes when an active validator node gets disabled for comitting offences. + When Byzantine Threshold Validators (1/3) are already disabled instead of no longer + disabling the highest offenders will be disabled potentially re-enabling low offenders. + + - audience: Node Operator + description: | + Implementation of the Stage 3 for the New Disabling Strategy: https://github.com/paritytech/polkadot-sdk/issues/4359 + + This PR changes when an active validator node gets disabled within parachain consensus (reduced responsibilities and + reduced rewards) for comitting offences. This should not affect active validators on a day-to-day basis and will only + be relevant when the network is under attack or there is a wide spread malfunction causing slashes. In that case + lowest offenders might get eventually re-enabled (back to normal responsibilities and normal rewards). + +migrations: + db: [] + runtime: + - reference: pallet-staking + description: | + Migrating `DisabledValidators` from `Vec` to `Vec<(u32, PerBill)>` where the PerBill represents the severity + of the offence in terms of the % slash. + +crates: + - name: pallet-staking + bump: minor + + - name: pallet-session + bump: minor diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index 914b51fb5621..e68e04840776 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -742,7 +742,7 @@ impl pallet_staking::Config for Runtime { type EventListeners = NominationPools; type WeightInfo = pallet_staking::weights::SubstrateWeight; type BenchmarkingConfig = StakingBenchmarkingConfig; - type DisablingStrategy = pallet_staking::UpToLimitDisablingStrategy; + type DisablingStrategy = pallet_staking::UpToLimitWithReEnablingDisablingStrategy; } impl pallet_fast_unstake::Config for Runtime { diff --git a/substrate/frame/election-provider-multi-phase/test-staking-e2e/src/lib.rs b/substrate/frame/election-provider-multi-phase/test-staking-e2e/src/lib.rs index 41928905ed95..26a6345e145f 100644 --- a/substrate/frame/election-provider-multi-phase/test-staking-e2e/src/lib.rs +++ b/substrate/frame/election-provider-multi-phase/test-staking-e2e/src/lib.rs @@ -147,30 +147,35 @@ fn mass_slash_doesnt_enter_emergency_phase() { let active_set_size_before_slash = Session::validators().len(); - // Slash more than 1/3 of the active validators - let mut slashed = slash_half_the_active_set(); + // assuming half is above the disabling limit (default 1/3), otherwise test will break + let slashed = slash_half_the_active_set(); let active_set_size_after_slash = Session::validators().len(); // active set should stay the same before and after the slash assert_eq!(active_set_size_before_slash, active_set_size_after_slash); - // Slashed validators are disabled up to a limit - slashed.truncate( - pallet_staking::UpToLimitDisablingStrategy::::disable_limit( - active_set_size_after_slash, - ), - ); - // Find the indices of the disabled validators let active_set = Session::validators(); - let expected_disabled = slashed + let potentially_disabled = slashed .into_iter() .map(|d| active_set.iter().position(|a| *a == d).unwrap() as u32) .collect::>(); + // Ensure that every actually disabled validator is also in the potentially disabled set + // (not necessarily the other way around) + let disabled = Session::disabled_validators(); + for d in disabled.iter() { + assert!(potentially_disabled.contains(d)); + } + + // Ensure no more than disabling limit of validators (default 1/3) is disabled + let disabling_limit = pallet_staking::UpToLimitWithReEnablingDisablingStrategy::< + SLASHING_DISABLING_FACTOR, + >::disable_limit(active_set_size_before_slash); + assert!(disabled.len() == disabling_limit); + assert_eq!(pallet_staking::ForceEra::::get(), pallet_staking::Forcing::NotForcing); - assert_eq!(Session::disabled_validators(), expected_disabled); }); } diff --git a/substrate/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs b/substrate/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs index b182ddec77ab..eaab848c1694 100644 --- a/substrate/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs +++ b/substrate/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs @@ -304,7 +304,8 @@ impl pallet_staking::Config for Runtime { type MaxUnlockingChunks = MaxUnlockingChunks; type EventListeners = Pools; type WeightInfo = pallet_staking::weights::SubstrateWeight; - type DisablingStrategy = pallet_staking::UpToLimitDisablingStrategy; + type DisablingStrategy = + pallet_staking::UpToLimitWithReEnablingDisablingStrategy; type BenchmarkingConfig = pallet_staking::TestBenchmarkingConfig; } diff --git a/substrate/frame/session/src/lib.rs b/substrate/frame/session/src/lib.rs index 325758d54dd8..e8b4a355f49a 100644 --- a/substrate/frame/session/src/lib.rs +++ b/substrate/frame/session/src/lib.rs @@ -127,8 +127,8 @@ use frame_support::{ dispatch::DispatchResult, ensure, traits::{ - EstimateNextNewSession, EstimateNextSessionRotation, FindAuthor, Get, OneSessionHandler, - ValidatorRegistration, ValidatorSet, + Defensive, EstimateNextNewSession, EstimateNextSessionRotation, FindAuthor, Get, + OneSessionHandler, ValidatorRegistration, ValidatorSet, }, weights::Weight, Parameter, @@ -735,6 +735,23 @@ impl Pallet { }) } + /// Re-enable the validator of index `i`, returns `false` if the validator was already enabled. + pub fn enable_index(i: u32) -> bool { + if i >= Validators::::decode_len().defensive_unwrap_or(0) as u32 { + return false + } + + // If the validator is not disabled, return false. + DisabledValidators::::mutate(|disabled| { + if let Ok(index) = disabled.binary_search(&i) { + disabled.remove(index); + true + } else { + false + } + }) + } + /// Disable the validator identified by `c`. (If using with the staking pallet, /// this would be their *stash* account.) /// diff --git a/substrate/frame/staking/CHANGELOG.md b/substrate/frame/staking/CHANGELOG.md index 113b7a6200b6..064a7d4a48f4 100644 --- a/substrate/frame/staking/CHANGELOG.md +++ b/substrate/frame/staking/CHANGELOG.md @@ -7,6 +7,18 @@ on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). We maintain a single integer version number for staking pallet to keep track of all storage migrations. +## [v16] + + +### Added + +- New default implementation of `DisablingStrategy` - `UpToLimitWithReEnablingDisablingStrategy`. + Same as `UpToLimitDisablingStrategy` except when a limit (1/3 default) is reached. When limit is + reached the offender is only disabled if his offence is greater or equal than some other already + disabled offender. The smallest possible offender is re-enabled to make space for the new greater + offender. A limit should thus always be respected. +- `DisabledValidators` changed format to include severity of the offence. + ## [v15] ### Added diff --git a/substrate/frame/staking/src/lib.rs b/substrate/frame/staking/src/lib.rs index a4a6e71af0df..6361663b2b1c 100644 --- a/substrate/frame/staking/src/lib.rs +++ b/substrate/frame/staking/src/lib.rs @@ -324,7 +324,7 @@ use sp_runtime::{ Perbill, Perquintill, Rounding, RuntimeDebug, Saturating, }; use sp_staking::{ - offence::{Offence, OffenceError, ReportOffence}, + offence::{Offence, OffenceError, OffenceSeverity, ReportOffence}, EraIndex, ExposurePage, OnStakingUpdate, Page, PagedExposureMetadata, SessionIndex, StakingAccount, }; @@ -849,6 +849,9 @@ pub trait SessionInterface { /// Disable the validator at the given index, returns `false` if the validator was already /// disabled or the index is out of bounds. fn disable_validator(validator_index: u32) -> bool; + /// Re-enable a validator that was previously disabled. Returns `false` if the validator was + /// already enabled or the index is out of bounds. + fn enable_validator(validator_index: u32) -> bool; /// Get the validators from session. fn validators() -> Vec; /// Prune historical session tries up to but not including the given index. @@ -873,6 +876,10 @@ where >::disable_index(validator_index) } + fn enable_validator(validator_index: u32) -> bool { + >::enable_index(validator_index) + } + fn validators() -> Vec<::AccountId> { >::validators() } @@ -886,6 +893,9 @@ impl SessionInterface for () { fn disable_validator(_: u32) -> bool { true } + fn enable_validator(_: u32) -> bool { + true + } fn validators() -> Vec { Vec::new() } @@ -1271,19 +1281,47 @@ impl BenchmarkingConfig for TestBenchmarkingConfig { /// Controls validator disabling pub trait DisablingStrategy { - /// Make a disabling decision. Returns the index of the validator to disable or `None` if no new - /// validator should be disabled. + /// Make a disabling decision. Returning a [`DisablingDecision`] fn decision( offender_stash: &T::AccountId, + offender_slash_severity: OffenceSeverity, slash_era: EraIndex, - currently_disabled: &Vec, - ) -> Option; + currently_disabled: &Vec<(u32, OffenceSeverity)>, + ) -> DisablingDecision; } -/// Implementation of [`DisablingStrategy`] which disables validators from the active set up to a -/// threshold. `DISABLING_LIMIT_FACTOR` is the factor of the maximum disabled validators in the -/// active set. E.g. setting this value to `3` means no more than 1/3 of the validators in the -/// active set can be disabled in an era. +/// Helper struct representing a decision coming from a given [`DisablingStrategy`] implementing +/// `decision` +/// +/// `disable` is the index of the validator to disable, +/// `reenable` is the index of the validator to re-enable. +#[derive(Debug)] +pub struct DisablingDecision { + pub disable: Option, + pub reenable: Option, +} + +/// Calculate the disabling limit based on the number of validators and the disabling limit factor. +/// +/// This is a sensible default implementation for the disabling limit factor for most disabling +/// strategies. +/// +/// Disabling limit factor n=2 -> 1/n = 1/2 = 50% of validators can be disabled +fn factor_based_disable_limit(validators_len: usize, disabling_limit_factor: usize) -> usize { + validators_len + .saturating_sub(1) + .checked_div(disabling_limit_factor) + .unwrap_or_else(|| { + defensive!("DISABLING_LIMIT_FACTOR should not be 0"); + 0 + }) +} + +/// Implementation of [`DisablingStrategy`] using factor_based_disable_limit which disables +/// validators from the active set up to a threshold. `DISABLING_LIMIT_FACTOR` is the factor of the +/// maximum disabled validators in the active set. E.g. setting this value to `3` means no more than +/// 1/3 of the validators in the active set can be disabled in an era. +/// /// By default a factor of 3 is used which is the byzantine threshold. pub struct UpToLimitDisablingStrategy; @@ -1291,13 +1329,7 @@ impl UpToLimitDisablingStrategy usize { - validators_len - .saturating_sub(1) - .checked_div(DISABLING_LIMIT_FACTOR) - .unwrap_or_else(|| { - defensive!("DISABLING_LIMIT_FACTOR should not be 0"); - 0 - }) + factor_based_disable_limit(validators_len, DISABLING_LIMIT_FACTOR) } } @@ -1306,9 +1338,10 @@ impl DisablingStrategy { fn decision( offender_stash: &T::AccountId, + _offender_slash_severity: OffenceSeverity, slash_era: EraIndex, - currently_disabled: &Vec, - ) -> Option { + currently_disabled: &Vec<(u32, OffenceSeverity)>, + ) -> DisablingDecision { let active_set = T::SessionInterface::validators(); // We don't disable more than the limit @@ -1318,7 +1351,7 @@ impl DisablingStrategy "Won't disable: reached disabling limit {:?}", Self::disable_limit(active_set.len()) ); - return None + return DisablingDecision { disable: None, reenable: None } } // We don't disable for offences in previous eras @@ -1329,18 +1362,116 @@ impl DisablingStrategy CurrentEra::::get().unwrap_or_default(), slash_era ); - return None + return DisablingDecision { disable: None, reenable: None } } let offender_idx = if let Some(idx) = active_set.iter().position(|i| i == offender_stash) { idx as u32 } else { log!(debug, "Won't disable: offender not in active set",); - return None + return DisablingDecision { disable: None, reenable: None } }; log!(debug, "Will disable {:?}", offender_idx); - Some(offender_idx) + DisablingDecision { disable: Some(offender_idx), reenable: None } + } +} + +/// Implementation of [`DisablingStrategy`] which disables validators from the active set up to a +/// limit (factor_based_disable_limit) and if the limit is reached and the new offender is higher +/// (bigger punishment/severity) then it re-enables the lowest offender to free up space for the new +/// offender. +/// +/// This strategy is not based on cumulative severity of offences but only on the severity of the +/// highest offence. Offender first committing a 25% offence and then a 50% offence will be treated +/// the same as an offender committing 50% offence. +/// +/// An extension of [`UpToLimitDisablingStrategy`]. +pub struct UpToLimitWithReEnablingDisablingStrategy; + +impl + UpToLimitWithReEnablingDisablingStrategy +{ + /// Disabling limit calculated from the total number of validators in the active set. When + /// reached re-enabling logic might kick in. + pub fn disable_limit(validators_len: usize) -> usize { + factor_based_disable_limit(validators_len, DISABLING_LIMIT_FACTOR) + } +} + +impl DisablingStrategy + for UpToLimitWithReEnablingDisablingStrategy +{ + fn decision( + offender_stash: &T::AccountId, + offender_slash_severity: OffenceSeverity, + slash_era: EraIndex, + currently_disabled: &Vec<(u32, OffenceSeverity)>, + ) -> DisablingDecision { + let active_set = T::SessionInterface::validators(); + + // We don't disable for offences in previous eras + if ActiveEra::::get().map(|e| e.index).unwrap_or_default() > slash_era { + log!( + debug, + "Won't disable: current_era {:?} > slash_era {:?}", + Pallet::::current_era().unwrap_or_default(), + slash_era + ); + return DisablingDecision { disable: None, reenable: None } + } + + // We don't disable validators that are not in the active set + let offender_idx = if let Some(idx) = active_set.iter().position(|i| i == offender_stash) { + idx as u32 + } else { + log!(debug, "Won't disable: offender not in active set",); + return DisablingDecision { disable: None, reenable: None } + }; + + // Check if offender is already disabled + if let Some((_, old_severity)) = + currently_disabled.iter().find(|(idx, _)| *idx == offender_idx) + { + if offender_slash_severity > *old_severity { + log!(debug, "Offender already disabled but with lower severity, will disable again to refresh severity of {:?}", offender_idx); + return DisablingDecision { disable: Some(offender_idx), reenable: None }; + } else { + log!(debug, "Offender already disabled with higher or equal severity"); + return DisablingDecision { disable: None, reenable: None }; + } + } + + // We don't disable more than the limit (but we can re-enable a smaller offender to make + // space) + if currently_disabled.len() >= Self::disable_limit(active_set.len()) { + log!( + debug, + "Reached disabling limit {:?}, checking for re-enabling", + Self::disable_limit(active_set.len()) + ); + + // Find the smallest offender to re-enable that is not higher than + // offender_slash_severity + if let Some((smallest_idx, _)) = currently_disabled + .iter() + .filter(|(_, severity)| *severity <= offender_slash_severity) + .min_by_key(|(_, severity)| *severity) + { + log!(debug, "Will disable {:?} and re-enable {:?}", offender_idx, smallest_idx); + return DisablingDecision { + disable: Some(offender_idx), + reenable: Some(*smallest_idx), + } + } else { + log!(debug, "No smaller offender found to re-enable"); + return DisablingDecision { disable: None, reenable: None } + } + } else { + // If we are not at the limit, just disable the new offender and dont re-enable anyone + log!(debug, "Will disable {:?}", offender_idx); + return DisablingDecision { disable: Some(offender_idx), reenable: None } + } } } diff --git a/substrate/frame/staking/src/migrations.rs b/substrate/frame/staking/src/migrations.rs index 5c9cf8613213..9dfa93c70b32 100644 --- a/substrate/frame/staking/src/migrations.rs +++ b/substrate/frame/staking/src/migrations.rs @@ -60,6 +60,79 @@ impl Default for ObsoleteReleases { #[storage_alias] type StorageVersion = StorageValue, ObsoleteReleases, ValueQuery>; +/// Migrating `DisabledValidators` from `Vec` to `Vec<(u32, OffenceSeverity)>` to track offense +/// severity for re-enabling purposes. +pub mod v16 { + use super::*; + use sp_staking::offence::OffenceSeverity; + + pub struct VersionUncheckedMigrateV15ToV16(core::marker::PhantomData); + impl UncheckedOnRuntimeUpgrade for VersionUncheckedMigrateV15ToV16 { + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, sp_runtime::TryRuntimeError> { + let old_disabled_validators = v15::DisabledValidators::::get(); + Ok(old_disabled_validators.encode()) + } + + fn on_runtime_upgrade() -> Weight { + // Migrating `DisabledValidators` from `Vec` to `Vec<(u32, OffenceSeverity)>`. + // Using max severity (PerBill 100%) for the migration which effectively makes it so + // offenders before the migration will not be re-enabled this era unless there are + // other 100% offenders. + let max_offence = OffenceSeverity(Perbill::from_percent(100)); + // Inject severity + let migrated = v15::DisabledValidators::::take() + .into_iter() + .map(|v| (v, max_offence)) + .collect::>(); + + DisabledValidators::::set(migrated); + + log!(info, "v16 applied successfully."); + T::DbWeight::get().reads_writes(1, 1) + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade(state: Vec) -> Result<(), TryRuntimeError> { + // Decode state to get old_disabled_validators in a format of Vec + let old_disabled_validators = + Vec::::decode(&mut state.as_slice()).expect("Failed to decode state"); + let new_disabled_validators = DisabledValidators::::get(); + + // Compare lengths + frame_support::ensure!( + old_disabled_validators.len() == new_disabled_validators.len(), + "DisabledValidators length mismatch" + ); + + // Compare contents + let new_disabled_validators = + new_disabled_validators.into_iter().map(|(v, _)| v).collect::>(); + frame_support::ensure!( + old_disabled_validators == new_disabled_validators, + "DisabledValidator ids mismatch" + ); + + // Verify severity + let max_severity = OffenceSeverity(Perbill::from_percent(100)); + let new_disabled_validators = DisabledValidators::::get(); + for (_, severity) in new_disabled_validators { + frame_support::ensure!(severity == max_severity, "Severity mismatch"); + } + + Ok(()) + } + } + + pub type MigrateV15ToV16 = VersionedMigration< + 15, + 16, + VersionUncheckedMigrateV15ToV16, + Pallet, + ::DbWeight, + >; +} + /// Migrating `OffendingValidators` from `Vec<(u32, bool)>` to `Vec` pub mod v15 { use super::*; @@ -67,6 +140,9 @@ pub mod v15 { // The disabling strategy used by staking pallet type DefaultDisablingStrategy = UpToLimitDisablingStrategy; + #[storage_alias] + pub(crate) type DisabledValidators = StorageValue, Vec, ValueQuery>; + pub struct VersionUncheckedMigrateV14ToV15(core::marker::PhantomData); impl UncheckedOnRuntimeUpgrade for VersionUncheckedMigrateV14ToV15 { fn on_runtime_upgrade() -> Weight { diff --git a/substrate/frame/staking/src/mock.rs b/substrate/frame/staking/src/mock.rs index 2d3446d2dabc..df8cb38e8b37 100644 --- a/substrate/frame/staking/src/mock.rs +++ b/substrate/frame/staking/src/mock.rs @@ -258,7 +258,8 @@ impl OnStakingUpdate for EventListenerMock { } } -// Disabling threshold for `UpToLimitDisablingStrategy` +// Disabling threshold for `UpToLimitDisablingStrategy` and +// `UpToLimitWithReEnablingDisablingStrategy`` pub(crate) const DISABLING_LIMIT_FACTOR: usize = 3; #[derive_impl(crate::config_preludes::TestDefaultConfig)] @@ -284,7 +285,8 @@ impl crate::pallet::pallet::Config for Test { type HistoryDepth = HistoryDepth; type MaxControllersInDeprecationBatch = MaxControllersInDeprecationBatch; type EventListeners = EventListenerMock; - type DisablingStrategy = pallet_staking::UpToLimitDisablingStrategy; + type DisablingStrategy = + pallet_staking::UpToLimitWithReEnablingDisablingStrategy; } pub struct WeightedNominationsQuota; diff --git a/substrate/frame/staking/src/pallet/impls.rs b/substrate/frame/staking/src/pallet/impls.rs index 972d0f3d47b9..2ae925d03643 100644 --- a/substrate/frame/staking/src/pallet/impls.rs +++ b/substrate/frame/staking/src/pallet/impls.rs @@ -510,7 +510,7 @@ impl Pallet { } // disable all offending validators that have been disabled for the whole era - for index in >::get() { + for (index, _) in >::get() { T::SessionInterface::disable_validator(index); } } @@ -1497,6 +1497,12 @@ where continue } + Self::deposit_event(Event::::SlashReported { + validator: stash.clone(), + fraction: *slash_fraction, + slash_era, + }); + let unapplied = slashing::compute_slash::(slashing::SlashParams { stash, slash: *slash_fraction, @@ -1507,12 +1513,6 @@ where reward_proportion, }); - Self::deposit_event(Event::::SlashReported { - validator: stash.clone(), - fraction: *slash_fraction, - slash_era, - }); - if let Some(mut unapplied) = unapplied { let nominators_len = unapplied.others.len() as u64; let reporters_len = details.reporters.len() as u64; @@ -2303,9 +2303,10 @@ impl Pallet { Ok(()) } + // Sorted by index fn ensure_disabled_validators_sorted() -> Result<(), TryRuntimeError> { ensure!( - DisabledValidators::::get().windows(2).all(|pair| pair[0] <= pair[1]), + DisabledValidators::::get().windows(2).all(|pair| pair[0].0 <= pair[1].0), "DisabledValidators is not sorted" ); Ok(()) diff --git a/substrate/frame/staking/src/pallet/mod.rs b/substrate/frame/staking/src/pallet/mod.rs index d33b863a521a..b3f8c18f704c 100644 --- a/substrate/frame/staking/src/pallet/mod.rs +++ b/substrate/frame/staking/src/pallet/mod.rs @@ -38,6 +38,7 @@ use sp_runtime::{ }; use sp_staking::{ + offence::OffenceSeverity, EraIndex, Page, SessionIndex, StakingAccount::{self, Controller, Stash}, StakingInterface, @@ -69,7 +70,7 @@ pub mod pallet { use super::*; /// The in-code storage version. - const STORAGE_VERSION: StorageVersion = StorageVersion::new(15); + const STORAGE_VERSION: StorageVersion = StorageVersion::new(16); #[pallet::pallet] #[pallet::storage_version(STORAGE_VERSION)] @@ -704,11 +705,15 @@ pub mod pallet { /// implementor of [`DisablingStrategy`] defines if a validator should be disabled which /// implicitly means that the implementor also controls the max number of disabled validators. /// - /// The vec is always kept sorted so that we can find whether a given validator has previously - /// offended using binary search. + /// The vec is always kept sorted based on the u32 index so that we can find whether a given + /// validator has previously offended using binary search. + /// + /// Additionally, each disabled validator is associated with an `OffenceSeverity` which + /// represents how severe is the offence that got the validator disabled. #[pallet::storage] #[pallet::unbounded] - pub type DisabledValidators = StorageValue<_, Vec, ValueQuery>; + pub type DisabledValidators = + StorageValue<_, Vec<(u32, OffenceSeverity)>, ValueQuery>; /// The threshold for when users can start calling `chill_other` for other validators / /// nominators. The threshold is compared to the actual number of validators / nominators @@ -849,6 +854,10 @@ pub mod pallet { ForceEra { mode: Forcing }, /// Report of a controller batch deprecation. ControllerBatchDeprecated { failures: u32 }, + /// Validator has been disabled. + ValidatorDisabled { stash: T::AccountId }, + /// Validator has been re-enabled. + ValidatorReenabled { stash: T::AccountId }, } #[pallet::error] diff --git a/substrate/frame/staking/src/slashing.rs b/substrate/frame/staking/src/slashing.rs index 9fb782265b8b..ae76b0707dcb 100644 --- a/substrate/frame/staking/src/slashing.rs +++ b/substrate/frame/staking/src/slashing.rs @@ -65,7 +65,7 @@ use sp_runtime::{ traits::{Saturating, Zero}, DispatchResult, RuntimeDebug, }; -use sp_staking::{EraIndex, StakingInterface}; +use sp_staking::{offence::OffenceSeverity, EraIndex, StakingInterface}; /// The proportion of the slashing reward to be paid out on the first slashing detection. /// This is f_1 in the paper. @@ -321,17 +321,48 @@ fn kick_out_if_recent(params: SlashParams) { } /// Inform the [`DisablingStrategy`] implementation about the new offender and disable the list of -/// validators provided by [`make_disabling_decision`]. +/// validators provided by [`decision`]. fn add_offending_validator(params: &SlashParams) { DisabledValidators::::mutate(|disabled| { - if let Some(offender) = - T::DisablingStrategy::decision(params.stash, params.slash_era, &disabled) - { - // Add the validator to `DisabledValidators` and disable it. Do nothing if it is - // already disabled. - if let Err(index) = disabled.binary_search_by_key(&offender, |index| *index) { - disabled.insert(index, offender); - T::SessionInterface::disable_validator(offender); + let new_severity = OffenceSeverity(params.slash); + let decision = + T::DisablingStrategy::decision(params.stash, new_severity, params.slash_era, &disabled); + + if let Some(offender_idx) = decision.disable { + // Check if the offender is already disabled + match disabled.binary_search_by_key(&offender_idx, |(index, _)| *index) { + // Offender is already disabled, update severity if the new one is higher + Ok(index) => { + let (_, old_severity) = &mut disabled[index]; + if new_severity > *old_severity { + *old_severity = new_severity; + } + }, + Err(index) => { + // Offender is not disabled, add to `DisabledValidators` and disable it + disabled.insert(index, (offender_idx, new_severity)); + // Propagate disablement to session level + T::SessionInterface::disable_validator(offender_idx); + // Emit event that a validator got disabled + >::deposit_event(super::Event::::ValidatorDisabled { + stash: params.stash.clone(), + }); + }, + } + } + + if let Some(reenable_idx) = decision.reenable { + // Remove the validator from `DisabledValidators` and re-enable it. + if let Ok(index) = disabled.binary_search_by_key(&reenable_idx, |(index, _)| *index) { + disabled.remove(index); + // Propagate re-enablement to session level + T::SessionInterface::enable_validator(reenable_idx); + // Emit event that a validator got re-enabled + let reenabled_stash = + T::SessionInterface::validators()[reenable_idx as usize].clone(); + >::deposit_event(super::Event::::ValidatorReenabled { + stash: reenabled_stash, + }); } } }); diff --git a/substrate/frame/staking/src/tests.rs b/substrate/frame/staking/src/tests.rs index ffa317618f1f..6c2335e1aac8 100644 --- a/substrate/frame/staking/src/tests.rs +++ b/substrate/frame/staking/src/tests.rs @@ -3402,6 +3402,7 @@ fn slash_kicks_validators_not_nominators_and_disables_nominator_for_kicked_valid fraction: Perbill::from_percent(10), slash_era: 1 }, + Event::ValidatorDisabled { stash: 11 }, Event::Slashed { staker: 11, amount: 100 }, Event::Slashed { staker: 101, amount: 12 }, ] @@ -3474,11 +3475,13 @@ fn non_slashable_offence_disables_validator() { fraction: Perbill::from_percent(0), slash_era: 1 }, + Event::ValidatorDisabled { stash: 11 }, Event::SlashReported { validator: 21, fraction: Perbill::from_percent(25), slash_era: 1 }, + Event::ValidatorDisabled { stash: 21 }, Event::Slashed { staker: 21, amount: 250 }, Event::Slashed { staker: 101, amount: 94 } ] @@ -3506,6 +3509,7 @@ fn slashing_independent_of_disabling_validator() { let now = ActiveEra::::get().unwrap().index; + // --- Disable without a slash --- // offence with no slash associated on_offence_in_era( &[OffenceDetails { offender: (11, exposure_11.clone()), reporters: vec![] }], @@ -3516,7 +3520,18 @@ fn slashing_independent_of_disabling_validator() { // nomination remains untouched. assert_eq!(Nominators::::get(101).unwrap().targets, vec![11, 21]); - // offence that slashes 25% of the bond + // first validator is disabled but not slashed + assert!(is_disabled(11)); + + // --- Slash without disabling --- + // offence that slashes 50% of the bond (setup for next slash) + on_offence_in_era( + &[OffenceDetails { offender: (11, exposure_11.clone()), reporters: vec![] }], + &[Perbill::from_percent(50)], + now, + ); + + // offence that slashes 25% of the bond but does not disable on_offence_in_era( &[OffenceDetails { offender: (21, exposure_21.clone()), reporters: vec![] }], &[Perbill::from_percent(25)], @@ -3526,6 +3541,10 @@ fn slashing_independent_of_disabling_validator() { // nomination remains untouched. assert_eq!(Nominators::::get(101).unwrap().targets, vec![11, 21]); + // second validator is slashed but not disabled + assert!(!is_disabled(21)); + assert!(is_disabled(11)); + assert_eq!( staking_events_since_last_call(), vec![ @@ -3536,6 +3555,14 @@ fn slashing_independent_of_disabling_validator() { fraction: Perbill::from_percent(0), slash_era: 1 }, + Event::ValidatorDisabled { stash: 11 }, + Event::SlashReported { + validator: 11, + fraction: Perbill::from_percent(50), + slash_era: 1 + }, + Event::Slashed { staker: 11, amount: 500 }, + Event::Slashed { staker: 101, amount: 62 }, Event::SlashReported { validator: 21, fraction: Perbill::from_percent(25), @@ -3545,11 +3572,6 @@ fn slashing_independent_of_disabling_validator() { Event::Slashed { staker: 101, amount: 94 } ] ); - - // first validator is disabled but not slashed - assert!(is_disabled(11)); - // second validator is slashed but not disabled - assert!(!is_disabled(21)); }); } @@ -3563,7 +3585,7 @@ fn offence_threshold_doesnt_trigger_new_era() { assert_eq_uvec!(Session::validators(), vec![11, 21, 31, 41]); assert_eq!( - UpToLimitDisablingStrategy::::disable_limit( + UpToLimitWithReEnablingDisablingStrategy::::disable_limit( Session::validators().len() ), 1 @@ -3578,7 +3600,7 @@ fn offence_threshold_doesnt_trigger_new_era() { on_offence_now( &[OffenceDetails { offender: (11, exposure_11.clone()), reporters: vec![] }], - &[Perbill::zero()], + &[Perbill::from_percent(50)], ); // 11 should be disabled because the byzantine threshold is 1 @@ -8277,11 +8299,14 @@ mod byzantine_threshold_disabling_strategy { use crate::{ tests::Test, ActiveEra, ActiveEraInfo, DisablingStrategy, UpToLimitDisablingStrategy, }; - use sp_staking::EraIndex; + use sp_runtime::Perbill; + use sp_staking::{offence::OffenceSeverity, EraIndex}; // Common test data - the stash of the offending validator, the era of the offence and the // active set const OFFENDER_ID: ::AccountId = 7; + const MAX_OFFENDER_SEVERITY: OffenceSeverity = OffenceSeverity(Perbill::from_percent(100)); + const MIN_OFFENDER_SEVERITY: OffenceSeverity = OffenceSeverity(Perbill::from_percent(0)); const SLASH_ERA: EraIndex = 1; const ACTIVE_SET: [::ValidatorId; 7] = [1, 2, 3, 4, 5, 6, 7]; const OFFENDER_VALIDATOR_IDX: u32 = 6; // the offender is with index 6 in the active set @@ -8293,48 +8318,431 @@ mod byzantine_threshold_disabling_strategy { pallet_session::Validators::::put(ACTIVE_SET.to_vec()); ActiveEra::::put(ActiveEraInfo { index: 2, start: None }); - let disable_offender = + let disabling_decision = >::decision( &OFFENDER_ID, + MAX_OFFENDER_SEVERITY, SLASH_ERA, &initially_disabled, ); - assert!(disable_offender.is_none()); + assert!(disabling_decision.disable.is_none() && disabling_decision.reenable.is_none()); }); } #[test] fn dont_disable_beyond_byzantine_threshold() { sp_io::TestExternalities::default().execute_with(|| { - let initially_disabled = vec![1, 2]; + let initially_disabled = vec![(1, MIN_OFFENDER_SEVERITY), (2, MAX_OFFENDER_SEVERITY)]; pallet_session::Validators::::put(ACTIVE_SET.to_vec()); - let disable_offender = + let disabling_decision = >::decision( &OFFENDER_ID, + MAX_OFFENDER_SEVERITY, SLASH_ERA, &initially_disabled, ); - assert!(disable_offender.is_none()); + assert!(disabling_decision.disable.is_none() && disabling_decision.reenable.is_none()); }); } #[test] fn disable_when_below_byzantine_threshold() { sp_io::TestExternalities::default().execute_with(|| { - let initially_disabled = vec![1]; + let initially_disabled = vec![(1, MAX_OFFENDER_SEVERITY)]; pallet_session::Validators::::put(ACTIVE_SET.to_vec()); - let disable_offender = + let disabling_decision = >::decision( &OFFENDER_ID, + MAX_OFFENDER_SEVERITY, + SLASH_ERA, + &initially_disabled, + ); + + assert_eq!(disabling_decision.disable, Some(OFFENDER_VALIDATOR_IDX)); + }); + } +} + +mod disabling_strategy_with_reenabling { + use crate::{ + tests::Test, ActiveEra, ActiveEraInfo, DisablingStrategy, + UpToLimitWithReEnablingDisablingStrategy, + }; + use sp_runtime::Perbill; + use sp_staking::{offence::OffenceSeverity, EraIndex}; + + // Common test data - the stash of the offending validator, the era of the offence and the + // active set + const OFFENDER_ID: ::AccountId = 7; + const MAX_OFFENDER_SEVERITY: OffenceSeverity = OffenceSeverity(Perbill::from_percent(100)); + const LOW_OFFENDER_SEVERITY: OffenceSeverity = OffenceSeverity(Perbill::from_percent(0)); + const SLASH_ERA: EraIndex = 1; + const ACTIVE_SET: [::ValidatorId; 7] = [1, 2, 3, 4, 5, 6, 7]; + const OFFENDER_VALIDATOR_IDX: u32 = 6; // the offender is with index 6 in the active set + + #[test] + fn dont_disable_for_ancient_offence() { + sp_io::TestExternalities::default().execute_with(|| { + let initially_disabled = vec![]; + pallet_session::Validators::::put(ACTIVE_SET.to_vec()); + ActiveEra::::put(ActiveEraInfo { index: 2, start: None }); + + let disabling_decision = + >::decision( + &OFFENDER_ID, + MAX_OFFENDER_SEVERITY, + SLASH_ERA, + &initially_disabled, + ); + + assert!(disabling_decision.disable.is_none() && disabling_decision.reenable.is_none()); + }); + } + + #[test] + fn disable_when_below_byzantine_threshold() { + sp_io::TestExternalities::default().execute_with(|| { + let initially_disabled = vec![(0, MAX_OFFENDER_SEVERITY)]; + pallet_session::Validators::::put(ACTIVE_SET.to_vec()); + + let disabling_decision = + >::decision( + &OFFENDER_ID, + MAX_OFFENDER_SEVERITY, + SLASH_ERA, + &initially_disabled, + ); + + // Disable Offender and do not re-enable anyone + assert_eq!(disabling_decision.disable, Some(OFFENDER_VALIDATOR_IDX)); + assert_eq!(disabling_decision.reenable, None); + }); + } + + #[test] + fn reenable_arbitrary_on_equal_severity() { + sp_io::TestExternalities::default().execute_with(|| { + let initially_disabled = vec![(0, MAX_OFFENDER_SEVERITY), (1, MAX_OFFENDER_SEVERITY)]; + pallet_session::Validators::::put(ACTIVE_SET.to_vec()); + + let disabling_decision = + >::decision( + &OFFENDER_ID, + MAX_OFFENDER_SEVERITY, + SLASH_ERA, + &initially_disabled, + ); + + assert!(disabling_decision.disable.is_some() && disabling_decision.reenable.is_some()); + // Disable 7 and enable 1 + assert_eq!(disabling_decision.disable.unwrap(), OFFENDER_VALIDATOR_IDX); + assert_eq!(disabling_decision.reenable.unwrap(), 0); + }); + } + + #[test] + fn do_not_reenable_higher_offenders() { + sp_io::TestExternalities::default().execute_with(|| { + let initially_disabled = vec![(0, MAX_OFFENDER_SEVERITY), (1, MAX_OFFENDER_SEVERITY)]; + pallet_session::Validators::::put(ACTIVE_SET.to_vec()); + + let disabling_decision = + >::decision( + &OFFENDER_ID, + LOW_OFFENDER_SEVERITY, SLASH_ERA, &initially_disabled, ); - assert_eq!(disable_offender, Some(OFFENDER_VALIDATOR_IDX)); + assert!(disabling_decision.disable.is_none() && disabling_decision.reenable.is_none()); + }); + } + + #[test] + fn reenable_lower_offenders() { + sp_io::TestExternalities::default().execute_with(|| { + let initially_disabled = vec![(0, LOW_OFFENDER_SEVERITY), (1, LOW_OFFENDER_SEVERITY)]; + pallet_session::Validators::::put(ACTIVE_SET.to_vec()); + + let disabling_decision = + >::decision( + &OFFENDER_ID, + MAX_OFFENDER_SEVERITY, + SLASH_ERA, + &initially_disabled, + ); + + assert!(disabling_decision.disable.is_some() && disabling_decision.reenable.is_some()); + // Disable 7 and enable 1 + assert_eq!(disabling_decision.disable.unwrap(), OFFENDER_VALIDATOR_IDX); + assert_eq!(disabling_decision.reenable.unwrap(), 0); + }); + } + + #[test] + fn reenable_lower_offenders_unordered() { + sp_io::TestExternalities::default().execute_with(|| { + let initially_disabled = vec![(0, MAX_OFFENDER_SEVERITY), (1, LOW_OFFENDER_SEVERITY)]; + pallet_session::Validators::::put(ACTIVE_SET.to_vec()); + + let disabling_decision = + >::decision( + &OFFENDER_ID, + MAX_OFFENDER_SEVERITY, + SLASH_ERA, + &initially_disabled, + ); + + assert!(disabling_decision.disable.is_some() && disabling_decision.reenable.is_some()); + // Disable 7 and enable 1 + assert_eq!(disabling_decision.disable.unwrap(), OFFENDER_VALIDATOR_IDX); + assert_eq!(disabling_decision.reenable.unwrap(), 1); + }); + } + + #[test] + fn update_severity() { + sp_io::TestExternalities::default().execute_with(|| { + let initially_disabled = + vec![(OFFENDER_VALIDATOR_IDX, LOW_OFFENDER_SEVERITY), (0, MAX_OFFENDER_SEVERITY)]; + pallet_session::Validators::::put(ACTIVE_SET.to_vec()); + + let disabling_decision = + >::decision( + &OFFENDER_ID, + MAX_OFFENDER_SEVERITY, + SLASH_ERA, + &initially_disabled, + ); + + assert!(disabling_decision.disable.is_some() && disabling_decision.reenable.is_none()); + // Disable 7 "again" AKA update their severity + assert_eq!(disabling_decision.disable.unwrap(), OFFENDER_VALIDATOR_IDX); + }); + } + + #[test] + fn update_cannot_lower_severity() { + sp_io::TestExternalities::default().execute_with(|| { + let initially_disabled = + vec![(OFFENDER_VALIDATOR_IDX, MAX_OFFENDER_SEVERITY), (0, MAX_OFFENDER_SEVERITY)]; + pallet_session::Validators::::put(ACTIVE_SET.to_vec()); + + let disabling_decision = + >::decision( + &OFFENDER_ID, + LOW_OFFENDER_SEVERITY, + SLASH_ERA, + &initially_disabled, + ); + + assert!(disabling_decision.disable.is_none() && disabling_decision.reenable.is_none()); + }); + } + + #[test] + fn no_accidental_reenablement_on_repeated_offence() { + sp_io::TestExternalities::default().execute_with(|| { + let initially_disabled = + vec![(OFFENDER_VALIDATOR_IDX, MAX_OFFENDER_SEVERITY), (0, LOW_OFFENDER_SEVERITY)]; + pallet_session::Validators::::put(ACTIVE_SET.to_vec()); + + let disabling_decision = + >::decision( + &OFFENDER_ID, + MAX_OFFENDER_SEVERITY, + SLASH_ERA, + &initially_disabled, + ); + + assert!(disabling_decision.disable.is_none() && disabling_decision.reenable.is_none()); + }); + } +} + +#[test] +fn reenable_lower_offenders_mock() { + ExtBuilder::default() + .validator_count(7) + .set_status(41, StakerStatus::Validator) + .set_status(51, StakerStatus::Validator) + .set_status(201, StakerStatus::Validator) + .set_status(202, StakerStatus::Validator) + .build_and_execute(|| { + mock::start_active_era(1); + assert_eq_uvec!(Session::validators(), vec![11, 21, 31, 41, 51, 201, 202]); + + let exposure_11 = Staking::eras_stakers(Staking::active_era().unwrap().index, &11); + let exposure_21 = Staking::eras_stakers(Staking::active_era().unwrap().index, &21); + let exposure_31 = Staking::eras_stakers(Staking::active_era().unwrap().index, &31); + + // offence with a low slash + on_offence_now( + &[OffenceDetails { offender: (11, exposure_11.clone()), reporters: vec![] }], + &[Perbill::from_percent(10)], + ); + on_offence_now( + &[OffenceDetails { offender: (21, exposure_21.clone()), reporters: vec![] }], + &[Perbill::from_percent(20)], + ); + + // it does NOT affect the nominator. + assert_eq!(Staking::nominators(101).unwrap().targets, vec![11, 21]); + + // both validators should be disabled + assert!(is_disabled(11)); + assert!(is_disabled(21)); + + // offence with a higher slash + on_offence_now( + &[OffenceDetails { offender: (31, exposure_31.clone()), reporters: vec![] }], + &[Perbill::from_percent(50)], + ); + + // First offender is no longer disabled + assert!(!is_disabled(11)); + // Mid offender is still disabled + assert!(is_disabled(21)); + // New offender is disabled + assert!(is_disabled(31)); + + assert_eq!( + staking_events_since_last_call(), + vec![ + Event::StakersElected, + Event::EraPaid { era_index: 0, validator_payout: 11075, remainder: 33225 }, + Event::SlashReported { + validator: 11, + fraction: Perbill::from_percent(10), + slash_era: 1 + }, + Event::ValidatorDisabled { stash: 11 }, + Event::Slashed { staker: 11, amount: 100 }, + Event::Slashed { staker: 101, amount: 12 }, + Event::SlashReported { + validator: 21, + fraction: Perbill::from_percent(20), + slash_era: 1 + }, + Event::ValidatorDisabled { stash: 21 }, + Event::Slashed { staker: 21, amount: 200 }, + Event::Slashed { staker: 101, amount: 75 }, + Event::SlashReported { + validator: 31, + fraction: Perbill::from_percent(50), + slash_era: 1 + }, + Event::ValidatorDisabled { stash: 31 }, + Event::ValidatorReenabled { stash: 11 }, + Event::Slashed { staker: 31, amount: 250 }, + ] + ); + }); +} + +#[test] +fn do_not_reenable_higher_offenders_mock() { + ExtBuilder::default() + .validator_count(7) + .set_status(41, StakerStatus::Validator) + .set_status(51, StakerStatus::Validator) + .set_status(201, StakerStatus::Validator) + .set_status(202, StakerStatus::Validator) + .build_and_execute(|| { + mock::start_active_era(1); + assert_eq_uvec!(Session::validators(), vec![11, 21, 31, 41, 51, 201, 202]); + + let exposure_11 = Staking::eras_stakers(Staking::active_era().unwrap().index, &11); + let exposure_21 = Staking::eras_stakers(Staking::active_era().unwrap().index, &21); + let exposure_31 = Staking::eras_stakers(Staking::active_era().unwrap().index, &31); + + // offence with a major slash + on_offence_now( + &[OffenceDetails { offender: (11, exposure_11.clone()), reporters: vec![] }], + &[Perbill::from_percent(50)], + ); + on_offence_now( + &[OffenceDetails { offender: (21, exposure_21.clone()), reporters: vec![] }], + &[Perbill::from_percent(50)], + ); + + // both validators should be disabled + assert!(is_disabled(11)); + assert!(is_disabled(21)); + + // offence with a minor slash + on_offence_now( + &[OffenceDetails { offender: (31, exposure_31.clone()), reporters: vec![] }], + &[Perbill::from_percent(10)], + ); + + // First and second offenders are still disabled + assert!(is_disabled(11)); + assert!(is_disabled(21)); + // New offender is not disabled as limit is reached and his prio is lower + assert!(!is_disabled(31)); + + assert_eq!( + staking_events_since_last_call(), + vec![ + Event::StakersElected, + Event::EraPaid { era_index: 0, validator_payout: 11075, remainder: 33225 }, + Event::SlashReported { + validator: 11, + fraction: Perbill::from_percent(50), + slash_era: 1 + }, + Event::ValidatorDisabled { stash: 11 }, + Event::Slashed { staker: 11, amount: 500 }, + Event::Slashed { staker: 101, amount: 62 }, + Event::SlashReported { + validator: 21, + fraction: Perbill::from_percent(50), + slash_era: 1 + }, + Event::ValidatorDisabled { stash: 21 }, + Event::Slashed { staker: 21, amount: 500 }, + Event::Slashed { staker: 101, amount: 187 }, + Event::SlashReported { + validator: 31, + fraction: Perbill::from_percent(10), + slash_era: 1 + }, + Event::Slashed { staker: 31, amount: 50 }, + ] + ); + }); +} + +#[cfg(all(feature = "try-runtime", test))] +mod migration_tests { + use super::*; + use frame_support::traits::UncheckedOnRuntimeUpgrade; + use migrations::{v15, v16}; + + #[test] + fn migrate_v15_to_v16_with_try_runtime() { + ExtBuilder::default().validator_count(7).build_and_execute(|| { + // Initial setup: Create old `DisabledValidators` in the form of `Vec` + let old_disabled_validators = vec![1u32, 2u32]; + v15::DisabledValidators::::put(old_disabled_validators.clone()); + + // Run pre-upgrade checks + let pre_upgrade_result = v16::VersionUncheckedMigrateV15ToV16::::pre_upgrade(); + assert!(pre_upgrade_result.is_ok()); + let pre_upgrade_state = pre_upgrade_result.unwrap(); + + // Run the migration + v16::VersionUncheckedMigrateV15ToV16::::on_runtime_upgrade(); + + // Run post-upgrade checks + let post_upgrade_result = + v16::VersionUncheckedMigrateV15ToV16::::post_upgrade(pre_upgrade_state); + assert!(post_upgrade_result.is_ok()); }); } } diff --git a/substrate/primitives/staking/src/offence.rs b/substrate/primitives/staking/src/offence.rs index 2c2ebc1fc971..e73e8efe5839 100644 --- a/substrate/primitives/staking/src/offence.rs +++ b/substrate/primitives/staking/src/offence.rs @@ -242,3 +242,28 @@ impl OffenceReportSystem for () { Ok(()) } } + +/// Wrapper type representing the severity of an offence. +/// +/// As of now the only meaningful value taken into account +/// when deciding the severity of an offence is the associated +/// slash amount `Perbill`. +/// +/// For instance used for the purposes of distinguishing who should be +/// prioritized for disablement. +#[derive( + Clone, Copy, PartialEq, Eq, Encode, Decode, sp_runtime::RuntimeDebug, scale_info::TypeInfo, +)] +pub struct OffenceSeverity(pub Perbill); + +impl PartialOrd for OffenceSeverity { + fn partial_cmp(&self, other: &Self) -> Option { + self.0.partial_cmp(&other.0) + } +} + +impl Ord for OffenceSeverity { + fn cmp(&self, other: &Self) -> core::cmp::Ordering { + self.0.cmp(&other.0) + } +} diff --git a/substrate/primitives/state-machine/src/trie_backend.rs b/substrate/primitives/state-machine/src/trie_backend.rs index f91ce5d2e52f..8d4dfd34240d 100644 --- a/substrate/primitives/state-machine/src/trie_backend.rs +++ b/substrate/primitives/state-machine/src/trie_backend.rs @@ -73,7 +73,10 @@ pub trait TrieCacheProvider { #[cfg(feature = "std")] impl TrieCacheProvider for LocalTrieCache { - type Cache<'a> = TrieCache<'a, H> where H: 'a; + type Cache<'a> + = TrieCache<'a, H> + where + H: 'a; fn as_trie_db_cache(&self, storage_root: H::Out) -> Self::Cache<'_> { self.as_trie_db_cache(storage_root) @@ -90,7 +93,10 @@ impl TrieCacheProvider for LocalTrieCache { #[cfg(feature = "std")] impl TrieCacheProvider for &LocalTrieCache { - type Cache<'a> = TrieCache<'a, H> where Self: 'a; + type Cache<'a> + = TrieCache<'a, H> + where + Self: 'a; fn as_trie_db_cache(&self, storage_root: H::Out) -> Self::Cache<'_> { (*self).as_trie_db_cache(storage_root) @@ -139,7 +145,10 @@ impl trie_db::TrieCache> for UnimplementedCacheProvider< #[cfg(not(feature = "std"))] impl TrieCacheProvider for UnimplementedCacheProvider { - type Cache<'a> = UnimplementedCacheProvider where H: 'a; + type Cache<'a> + = UnimplementedCacheProvider + where + H: 'a; fn as_trie_db_cache(&self, _storage_root: ::Out) -> Self::Cache<'_> { unimplemented!() @@ -176,7 +185,10 @@ impl trie_db::TrieRecorder for UnimplementedRecorderProvider< #[cfg(not(feature = "std"))] impl TrieRecorderProvider for UnimplementedRecorderProvider { - type Recorder<'a> = UnimplementedRecorderProvider where H: 'a; + type Recorder<'a> + = UnimplementedRecorderProvider + where + H: 'a; fn drain_storage_proof(self) -> Option { unimplemented!() diff --git a/substrate/primitives/trie/src/recorder.rs b/substrate/primitives/trie/src/recorder.rs index 2886577eddc6..4ec13066ded7 100644 --- a/substrate/primitives/trie/src/recorder.rs +++ b/substrate/primitives/trie/src/recorder.rs @@ -252,7 +252,10 @@ pub struct TrieRecorder<'a, H: Hasher> { } impl crate::TrieRecorderProvider for Recorder { - type Recorder<'a> = TrieRecorder<'a, H> where H: 'a; + type Recorder<'a> + = TrieRecorder<'a, H> + where + H: 'a; fn drain_storage_proof(self) -> Option { Some(Recorder::drain_storage_proof(self)) From 09757a4164d44f224b44291f1e01b1e813a52220 Mon Sep 17 00:00:00 2001 From: Kazunobu Ndong <33208377+ndkazu@users.noreply.github.com> Date: Wed, 20 Nov 2024 04:08:46 +0900 Subject: [PATCH 6/6] Migrate pallet-democracy benchmarks to benchmark v2 syntax (#6509) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description Migrates pallet-democracy benchmarks to benchmark v2 syntax This is Part of https://github.com/paritytech/polkadot-sdk/issues/6202 --------- Co-authored-by: Bastian Köcher Co-authored-by: command-bot <> Co-authored-by: Dmitry Markin Co-authored-by: Alexandru Vasile <60601340+lexnv@users.noreply.github.com> --- prdoc/pr_6509.prdoc | 13 + substrate/frame/democracy/src/benchmarking.rs | 548 +++++++++++------- substrate/frame/democracy/src/weights.rs | 302 +++++----- 3 files changed, 498 insertions(+), 365 deletions(-) create mode 100644 prdoc/pr_6509.prdoc diff --git a/prdoc/pr_6509.prdoc b/prdoc/pr_6509.prdoc new file mode 100644 index 000000000000..74215fe0084c --- /dev/null +++ b/prdoc/pr_6509.prdoc @@ -0,0 +1,13 @@ +# 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: Migrate pallet-democracy benchmark to v2 + +doc: + - audience: Runtime Dev + description: | + "Part of issue #6202." + +crates: +- name: pallet-democracy + bump: patch diff --git a/substrate/frame/democracy/src/benchmarking.rs b/substrate/frame/democracy/src/benchmarking.rs index ee36e9212f52..f9c810e56192 100644 --- a/substrate/frame/democracy/src/benchmarking.rs +++ b/substrate/frame/democracy/src/benchmarking.rs @@ -17,9 +17,11 @@ //! Democracy pallet benchmarking. +#![cfg(feature = "runtime-benchmarks")] + use super::*; -use frame_benchmarking::v1::{account, benchmarks, whitelist_account, BenchmarkError}; +use frame_benchmarking::v2::*; use frame_support::{ assert_noop, assert_ok, traits::{Currency, EnsureOrigin, Get, OnInitialize, UnfilteredDispatchable}, @@ -94,11 +96,15 @@ fn note_preimage() -> T::Hash { hash } -benchmarks! { - propose { +#[benchmarks] +mod benchmarks { + use super::*; + + #[benchmark] + fn propose() -> Result<(), BenchmarkError> { let p = T::MaxProposals::get(); - for i in 0 .. (p - 1) { + for i in 0..(p - 1) { add_proposal::(i)?; } @@ -106,18 +112,22 @@ benchmarks! { let proposal = make_proposal::(0); let value = T::MinimumDeposit::get(); whitelist_account!(caller); - }: _(RawOrigin::Signed(caller), proposal, value) - verify { + + #[extrinsic_call] + _(RawOrigin::Signed(caller), proposal, value); + assert_eq!(PublicProps::::get().len(), p as usize, "Proposals not created."); + Ok(()) } - second { + #[benchmark] + fn second() -> Result<(), BenchmarkError> { let caller = funded_account::("caller", 0); add_proposal::(0)?; // Create s existing "seconds" // we must reserve one deposit for the `proposal` and one for our benchmarked `second` call. - for i in 0 .. T::MaxDeposits::get() - 2 { + for i in 0..T::MaxDeposits::get() - 2 { let seconder = funded_account::("seconder", i); Democracy::::second(RawOrigin::Signed(seconder).into(), 0)?; } @@ -125,20 +135,32 @@ benchmarks! { let deposits = DepositOf::::get(0).ok_or("Proposal not created")?; assert_eq!(deposits.0.len(), (T::MaxDeposits::get() - 1) as usize, "Seconds not recorded"); whitelist_account!(caller); - }: _(RawOrigin::Signed(caller), 0) - verify { + + #[extrinsic_call] + _(RawOrigin::Signed(caller), 0); + let deposits = DepositOf::::get(0).ok_or("Proposal not created")?; - assert_eq!(deposits.0.len(), (T::MaxDeposits::get()) as usize, "`second` benchmark did not work"); + assert_eq!( + deposits.0.len(), + (T::MaxDeposits::get()) as usize, + "`second` benchmark did not work" + ); + Ok(()) } - vote_new { + #[benchmark] + fn vote_new() -> Result<(), BenchmarkError> { let caller = funded_account::("caller", 0); let account_vote = account_vote::(100u32.into()); // We need to create existing direct votes - for i in 0 .. T::MaxVotes::get() - 1 { + for i in 0..T::MaxVotes::get() - 1 { let ref_index = add_referendum::(i).0; - Democracy::::vote(RawOrigin::Signed(caller.clone()).into(), ref_index, account_vote)?; + Democracy::::vote( + RawOrigin::Signed(caller.clone()).into(), + ref_index, + account_vote, + )?; } let votes = match VotingOf::::get(&caller) { Voting::Direct { votes, .. } => votes, @@ -148,23 +170,32 @@ benchmarks! { let ref_index = add_referendum::(T::MaxVotes::get() - 1).0; whitelist_account!(caller); - }: vote(RawOrigin::Signed(caller.clone()), ref_index, account_vote) - verify { + + #[extrinsic_call] + vote(RawOrigin::Signed(caller.clone()), ref_index, account_vote); + let votes = match VotingOf::::get(&caller) { Voting::Direct { votes, .. } => votes, _ => return Err("Votes are not direct".into()), }; + assert_eq!(votes.len(), T::MaxVotes::get() as usize, "Vote was not recorded."); + Ok(()) } - vote_existing { + #[benchmark] + fn vote_existing() -> Result<(), BenchmarkError> { let caller = funded_account::("caller", 0); let account_vote = account_vote::(100u32.into()); // We need to create existing direct votes for i in 0..T::MaxVotes::get() { let ref_index = add_referendum::(i).0; - Democracy::::vote(RawOrigin::Signed(caller.clone()).into(), ref_index, account_vote)?; + Democracy::::vote( + RawOrigin::Signed(caller.clone()).into(), + ref_index, + account_vote, + )?; } let votes = match VotingOf::::get(&caller) { Voting::Direct { votes, .. } => votes, @@ -179,43 +210,50 @@ benchmarks! { // This tests when a user changes a vote whitelist_account!(caller); - }: vote(RawOrigin::Signed(caller.clone()), ref_index, new_vote) - verify { + + #[extrinsic_call] + vote(RawOrigin::Signed(caller.clone()), ref_index, new_vote); + let votes = match VotingOf::::get(&caller) { Voting::Direct { votes, .. } => votes, _ => return Err("Votes are not direct".into()), }; assert_eq!(votes.len(), T::MaxVotes::get() as usize, "Vote was incorrectly added"); - let referendum_info = ReferendumInfoOf::::get(ref_index) - .ok_or("referendum doesn't exist")?; - let tally = match referendum_info { + let referendum_info = + ReferendumInfoOf::::get(ref_index).ok_or("referendum doesn't exist")?; + let tally = match referendum_info { ReferendumInfo::Ongoing(r) => r.tally, _ => return Err("referendum not ongoing".into()), }; assert_eq!(tally.nays, 1000u32.into(), "changed vote was not recorded"); + Ok(()) } - emergency_cancel { - let origin = - T::CancellationOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; + #[benchmark] + fn emergency_cancel() -> Result<(), BenchmarkError> { + let origin = T::CancellationOrigin::try_successful_origin() + .map_err(|_| BenchmarkError::Weightless)?; let (ref_index, _, preimage_hash) = add_referendum::(0); assert_ok!(Democracy::::referendum_status(ref_index)); - }: _(origin, ref_index) - verify { + + #[extrinsic_call] + _(origin as T::RuntimeOrigin, ref_index); // Referendum has been canceled - assert_noop!( - Democracy::::referendum_status(ref_index), - Error::::ReferendumInvalid, + assert_noop!(Democracy::::referendum_status(ref_index), Error::::ReferendumInvalid,); + assert_last_event::( + crate::Event::MetadataCleared { + owner: MetadataOwner::Referendum(ref_index), + hash: preimage_hash, + } + .into(), ); - assert_last_event::(crate::Event::MetadataCleared { - owner: MetadataOwner::Referendum(ref_index), - hash: preimage_hash, - }.into()); + Ok(()) } - blacklist { + #[benchmark] + fn blacklist() -> Result<(), BenchmarkError> { // Place our proposal at the end to make sure it's worst case. - for i in 0 .. T::MaxProposals::get() - 1 { + for i in 0..T::MaxProposals::get() - 1 { add_proposal::(i)?; } // We should really add a lot of seconds here, but we're not doing it elsewhere. @@ -231,21 +269,24 @@ benchmarks! { )); let origin = T::BlacklistOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; - }: _(origin, hash, Some(ref_index)) - verify { + #[extrinsic_call] + _(origin as T::RuntimeOrigin, hash, Some(ref_index)); + // Referendum has been canceled - assert_noop!( - Democracy::::referendum_status(ref_index), - Error::::ReferendumInvalid + assert_noop!(Democracy::::referendum_status(ref_index), Error::::ReferendumInvalid); + assert_has_event::( + crate::Event::MetadataCleared { + owner: MetadataOwner::Referendum(ref_index), + hash: preimage_hash, + } + .into(), ); - assert_has_event::(crate::Event::MetadataCleared { - owner: MetadataOwner::Referendum(ref_index), - hash: preimage_hash, - }.into()); + Ok(()) } // Worst case scenario, we external propose a previously blacklisted proposal - external_propose { + #[benchmark] + fn external_propose() -> Result<(), BenchmarkError> { let origin = T::ExternalOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; let proposal = make_proposal::(0); @@ -258,33 +299,42 @@ benchmarks! { .try_into() .unwrap(); Blacklist::::insert(proposal.hash(), (BlockNumberFor::::zero(), addresses)); - }: _(origin, proposal) - verify { + #[extrinsic_call] + _(origin as T::RuntimeOrigin, proposal); + // External proposal created ensure!(NextExternal::::exists(), "External proposal didn't work"); + Ok(()) } - external_propose_majority { + #[benchmark] + fn external_propose_majority() -> Result<(), BenchmarkError> { let origin = T::ExternalMajorityOrigin::try_successful_origin() .map_err(|_| BenchmarkError::Weightless)?; let proposal = make_proposal::(0); - }: _(origin, proposal) - verify { + #[extrinsic_call] + _(origin as T::RuntimeOrigin, proposal); + // External proposal created ensure!(NextExternal::::exists(), "External proposal didn't work"); + Ok(()) } - external_propose_default { + #[benchmark] + fn external_propose_default() -> Result<(), BenchmarkError> { let origin = T::ExternalDefaultOrigin::try_successful_origin() .map_err(|_| BenchmarkError::Weightless)?; let proposal = make_proposal::(0); - }: _(origin, proposal) - verify { + #[extrinsic_call] + _(origin as T::RuntimeOrigin, proposal); + // External proposal created ensure!(NextExternal::::exists(), "External proposal didn't work"); + Ok(()) } - fast_track { + #[benchmark] + fn fast_track() -> Result<(), BenchmarkError> { let origin_propose = T::ExternalDefaultOrigin::try_successful_origin() .expect("ExternalDefaultOrigin has no successful origin required for the benchmark"); let proposal = make_proposal::(0); @@ -295,23 +345,30 @@ benchmarks! { assert_ok!(Democracy::::set_metadata( origin_propose, MetadataOwner::External, - Some(preimage_hash))); + Some(preimage_hash) + )); // NOTE: Instant origin may invoke a little bit more logic, but may not always succeed. let origin_fast_track = T::FastTrackOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; let voting_period = T::FastTrackVotingPeriod::get(); let delay = 0u32; - }: _(origin_fast_track, proposal_hash, voting_period, delay.into()) - verify { + #[extrinsic_call] + _(origin_fast_track as T::RuntimeOrigin, proposal_hash, voting_period, delay.into()); + assert_eq!(ReferendumCount::::get(), 1, "referendum not created"); - assert_last_event::(crate::Event::MetadataTransferred { - prev_owner: MetadataOwner::External, - owner: MetadataOwner::Referendum(0), - hash: preimage_hash, - }.into()); + assert_last_event::( + crate::Event::MetadataTransferred { + prev_owner: MetadataOwner::External, + owner: MetadataOwner::Referendum(0), + hash: preimage_hash, + } + .into(), + ); + Ok(()) } - veto_external { + #[benchmark] + fn veto_external() -> Result<(), BenchmarkError> { let proposal = make_proposal::(0); let proposal_hash = proposal.hash(); @@ -323,28 +380,32 @@ benchmarks! { assert_ok!(Democracy::::set_metadata( origin_propose, MetadataOwner::External, - Some(preimage_hash)) - ); + Some(preimage_hash) + )); let mut vetoers: BoundedVec = Default::default(); - for i in 0 .. (T::MaxBlacklisted::get() - 1) { + for i in 0..(T::MaxBlacklisted::get() - 1) { vetoers.try_push(account::("vetoer", i, SEED)).unwrap(); } vetoers.sort(); Blacklist::::insert(proposal_hash, (BlockNumberFor::::zero(), vetoers)); - let origin = T::VetoOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; + let origin = + T::VetoOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; ensure!(NextExternal::::get().is_some(), "no external proposal"); - }: _(origin, proposal_hash) - verify { + #[extrinsic_call] + _(origin as T::RuntimeOrigin, proposal_hash); + assert!(NextExternal::::get().is_none()); let (_, new_vetoers) = Blacklist::::get(&proposal_hash).ok_or("no blacklist")?; assert_eq!(new_vetoers.len(), T::MaxBlacklisted::get() as usize, "vetoers not added"); + Ok(()) } - cancel_proposal { + #[benchmark] + fn cancel_proposal() -> Result<(), BenchmarkError> { // Place our proposal at the end to make sure it's worst case. - for i in 0 .. T::MaxProposals::get() { + for i in 0..T::MaxProposals::get() { add_proposal::(i)?; } // Add metadata to the first proposal. @@ -353,31 +414,41 @@ benchmarks! { assert_ok!(Democracy::::set_metadata( RawOrigin::Signed(proposer).into(), MetadataOwner::Proposal(0), - Some(preimage_hash))); + Some(preimage_hash) + )); let cancel_origin = T::CancelProposalOrigin::try_successful_origin() .map_err(|_| BenchmarkError::Weightless)?; - }: _(cancel_origin, 0) - verify { - assert_last_event::(crate::Event::MetadataCleared { - owner: MetadataOwner::Proposal(0), - hash: preimage_hash, - }.into()); + #[extrinsic_call] + _(cancel_origin as T::RuntimeOrigin, 0); + + assert_last_event::( + crate::Event::MetadataCleared { + owner: MetadataOwner::Proposal(0), + hash: preimage_hash, + } + .into(), + ); + Ok(()) } - cancel_referendum { + #[benchmark] + fn cancel_referendum() -> Result<(), BenchmarkError> { let (ref_index, _, preimage_hash) = add_referendum::(0); - }: _(RawOrigin::Root, ref_index) - verify { - assert_last_event::(crate::Event::MetadataCleared { - owner: MetadataOwner::Referendum(0), - hash: preimage_hash, - }.into()); - } + #[extrinsic_call] + _(RawOrigin::Root, ref_index); - #[extra] - on_initialize_external { - let r in 0 .. REFERENDUM_COUNT_HINT; + assert_last_event::( + crate::Event::MetadataCleared { + owner: MetadataOwner::Referendum(0), + hash: preimage_hash, + } + .into(), + ); + Ok(()) + } + #[benchmark(extra)] + fn on_initialize_external(r: Linear<0, REFERENDUM_COUNT_HINT>) -> Result<(), BenchmarkError> { for i in 0..r { add_referendum::(i); } @@ -397,14 +468,17 @@ benchmarks! { let block_number = T::LaunchPeriod::get(); - }: { Democracy::::on_initialize(block_number) } - verify { + #[block] + { + Democracy::::on_initialize(block_number); + } + // One extra because of next external assert_eq!(ReferendumCount::::get(), r + 1, "referenda not created"); ensure!(!NextExternal::::exists(), "External wasn't taken"); // All but the new next external should be finished - for i in 0 .. r { + for i in 0..r { if let Some(value) = ReferendumInfoOf::::get(i) { match value { ReferendumInfo::Finished { .. } => (), @@ -412,12 +486,13 @@ benchmarks! { } } } + Ok(()) } - #[extra] - on_initialize_public { - let r in 0 .. (T::MaxVotes::get() - 1); - + #[benchmark(extra)] + fn on_initialize_public( + r: Linear<0, { T::MaxVotes::get() - 1 }>, + ) -> Result<(), BenchmarkError> { for i in 0..r { add_referendum::(i); } @@ -430,13 +505,16 @@ benchmarks! { let block_number = T::LaunchPeriod::get(); - }: { Democracy::::on_initialize(block_number) } - verify { + #[block] + { + Democracy::::on_initialize(block_number); + } + // One extra because of next public assert_eq!(ReferendumCount::::get(), r + 1, "proposal not accepted"); // All should be finished - for i in 0 .. r { + for i in 0..r { if let Some(value) = ReferendumInfoOf::::get(i) { match value { ReferendumInfo::Finished { .. } => (), @@ -444,12 +522,12 @@ benchmarks! { } } } + Ok(()) } // No launch no maturing referenda. - on_initialize_base { - let r in 0 .. (T::MaxVotes::get() - 1); - + #[benchmark] + fn on_initialize_base(r: Linear<0, { T::MaxVotes::get() - 1 }>) -> Result<(), BenchmarkError> { for i in 0..r { add_referendum::(i); } @@ -464,22 +542,28 @@ benchmarks! { assert_eq!(ReferendumCount::::get(), r, "referenda not created"); assert_eq!(LowestUnbaked::::get(), 0, "invalid referenda init"); - }: { Democracy::::on_initialize(1u32.into()) } - verify { + #[block] + { + Democracy::::on_initialize(1u32.into()); + } + // All should be on going - for i in 0 .. r { + for i in 0..r { if let Some(value) = ReferendumInfoOf::::get(i) { match value { - ReferendumInfo::Finished { .. } => return Err("Referendum has been finished".into()), + ReferendumInfo::Finished { .. } => + return Err("Referendum has been finished".into()), ReferendumInfo::Ongoing(_) => (), } } } + Ok(()) } - on_initialize_base_with_launch_period { - let r in 0 .. (T::MaxVotes::get() - 1); - + #[benchmark] + fn on_initialize_base_with_launch_period( + r: Linear<0, { T::MaxVotes::get() - 1 }>, + ) -> Result<(), BenchmarkError> { for i in 0..r { add_referendum::(i); } @@ -496,22 +580,26 @@ benchmarks! { let block_number = T::LaunchPeriod::get(); - }: { Democracy::::on_initialize(block_number) } - verify { + #[block] + { + Democracy::::on_initialize(block_number); + } + // All should be on going - for i in 0 .. r { + for i in 0..r { if let Some(value) = ReferendumInfoOf::::get(i) { match value { - ReferendumInfo::Finished { .. } => return Err("Referendum has been finished".into()), + ReferendumInfo::Finished { .. } => + return Err("Referendum has been finished".into()), ReferendumInfo::Ongoing(_) => (), } } } + Ok(()) } - delegate { - let r in 0 .. (T::MaxVotes::get() - 1); - + #[benchmark] + fn delegate(r: Linear<0, { T::MaxVotes::get() - 1 }>) -> Result<(), BenchmarkError> { let initial_balance: BalanceOf = 100u32.into(); let delegated_balance: BalanceOf = 1000u32.into(); @@ -538,7 +626,11 @@ benchmarks! { // We need to create existing direct votes for the `new_delegate` for i in 0..r { let ref_index = add_referendum::(i).0; - Democracy::::vote(RawOrigin::Signed(new_delegate.clone()).into(), ref_index, account_vote)?; + Democracy::::vote( + RawOrigin::Signed(new_delegate.clone()).into(), + ref_index, + account_vote, + )?; } let votes = match VotingOf::::get(&new_delegate) { Voting::Direct { votes, .. } => votes, @@ -546,8 +638,15 @@ benchmarks! { }; assert_eq!(votes.len(), r as usize, "Votes were not recorded."); whitelist_account!(caller); - }: _(RawOrigin::Signed(caller.clone()), new_delegate_lookup, Conviction::Locked1x, delegated_balance) - verify { + + #[extrinsic_call] + _( + RawOrigin::Signed(caller.clone()), + new_delegate_lookup, + Conviction::Locked1x, + delegated_balance, + ); + let (target, balance) = match VotingOf::::get(&caller) { Voting::Delegating { target, balance, .. } => (target, balance), _ => return Err("Votes are not direct".into()), @@ -559,11 +658,11 @@ benchmarks! { _ => return Err("Votes are not direct".into()), }; assert_eq!(delegations.capital, delegated_balance, "delegation was not recorded."); + Ok(()) } - undelegate { - let r in 0 .. (T::MaxVotes::get() - 1); - + #[benchmark] + fn undelegate(r: Linear<0, { T::MaxVotes::get() - 1 }>) -> Result<(), BenchmarkError> { let initial_balance: BalanceOf = 100u32.into(); let delegated_balance: BalanceOf = 1000u32.into(); @@ -590,7 +689,7 @@ benchmarks! { Democracy::::vote( RawOrigin::Signed(the_delegate.clone()).into(), ref_index, - account_vote + account_vote, )?; } let votes = match VotingOf::::get(&the_delegate) { @@ -599,31 +698,38 @@ benchmarks! { }; assert_eq!(votes.len(), r as usize, "Votes were not recorded."); whitelist_account!(caller); - }: _(RawOrigin::Signed(caller.clone())) - verify { + + #[extrinsic_call] + _(RawOrigin::Signed(caller.clone())); + // Voting should now be direct match VotingOf::::get(&caller) { Voting::Direct { .. } => (), _ => return Err("undelegation failed".into()), } + Ok(()) } - clear_public_proposals { + #[benchmark] + fn clear_public_proposals() -> Result<(), BenchmarkError> { add_proposal::(0)?; - }: _(RawOrigin::Root) + #[extrinsic_call] + _(RawOrigin::Root); - // Test when unlock will remove locks - unlock_remove { - let r in 0 .. (T::MaxVotes::get() - 1); + Ok(()) + } + // Test when unlock will remove locks + #[benchmark] + fn unlock_remove(r: Linear<0, { T::MaxVotes::get() - 1 }>) -> Result<(), BenchmarkError> { let locker = funded_account::("locker", 0); let locker_lookup = T::Lookup::unlookup(locker.clone()); // Populate votes so things are locked let base_balance: BalanceOf = 100u32.into(); let small_vote = account_vote::(base_balance); // Vote and immediately unvote - for i in 0 .. r { + for i in 0..r { let ref_index = add_referendum::(i).0; Democracy::::vote(RawOrigin::Signed(locker.clone()).into(), ref_index, small_vote)?; Democracy::::remove_vote(RawOrigin::Signed(locker.clone()).into(), ref_index)?; @@ -631,23 +737,25 @@ benchmarks! { let caller = funded_account::("caller", 0); whitelist_account!(caller); - }: unlock(RawOrigin::Signed(caller), locker_lookup) - verify { + + #[extrinsic_call] + unlock(RawOrigin::Signed(caller), locker_lookup); + // Note that we may want to add a `get_lock` api to actually verify let voting = VotingOf::::get(&locker); assert_eq!(voting.locked_balance(), BalanceOf::::zero()); + Ok(()) } // Test when unlock will set a new value - unlock_set { - let r in 0 .. (T::MaxVotes::get() - 1); - + #[benchmark] + fn unlock_set(r: Linear<0, { T::MaxVotes::get() - 1 }>) -> Result<(), BenchmarkError> { let locker = funded_account::("locker", 0); let locker_lookup = T::Lookup::unlookup(locker.clone()); // Populate votes so things are locked let base_balance: BalanceOf = 100u32.into(); let small_vote = account_vote::(base_balance); - for i in 0 .. r { + for i in 0..r { let ref_index = add_referendum::(i).0; Democracy::::vote(RawOrigin::Signed(locker.clone()).into(), ref_index, small_vote)?; } @@ -670,8 +778,10 @@ benchmarks! { let caller = funded_account::("caller", 0); whitelist_account!(caller); - }: unlock(RawOrigin::Signed(caller), locker_lookup) - verify { + + #[extrinsic_call] + unlock(RawOrigin::Signed(caller), locker_lookup); + let votes = match VotingOf::::get(&locker) { Voting::Direct { votes, .. } => votes, _ => return Err("Votes are not direct".into()), @@ -681,17 +791,21 @@ benchmarks! { let voting = VotingOf::::get(&locker); // Note that we may want to add a `get_lock` api to actually verify assert_eq!(voting.locked_balance(), if r > 0 { base_balance } else { 0u32.into() }); + Ok(()) } - remove_vote { - let r in 1 .. T::MaxVotes::get(); - + #[benchmark] + fn remove_vote(r: Linear<1, { T::MaxVotes::get() }>) -> Result<(), BenchmarkError> { let caller = funded_account::("caller", 0); let account_vote = account_vote::(100u32.into()); - for i in 0 .. r { + for i in 0..r { let ref_index = add_referendum::(i).0; - Democracy::::vote(RawOrigin::Signed(caller.clone()).into(), ref_index, account_vote)?; + Democracy::::vote( + RawOrigin::Signed(caller.clone()).into(), + ref_index, + account_vote, + )?; } let votes = match VotingOf::::get(&caller) { @@ -702,26 +816,32 @@ benchmarks! { let ref_index = r - 1; whitelist_account!(caller); - }: _(RawOrigin::Signed(caller.clone()), ref_index) - verify { + + #[extrinsic_call] + _(RawOrigin::Signed(caller.clone()), ref_index); + let votes = match VotingOf::::get(&caller) { Voting::Direct { votes, .. } => votes, _ => return Err("Votes are not direct".into()), }; assert_eq!(votes.len(), (r - 1) as usize, "Vote was not removed"); + Ok(()) } // Worst case is when target == caller and referendum is ongoing - remove_other_vote { - let r in 1 .. T::MaxVotes::get(); - + #[benchmark] + fn remove_other_vote(r: Linear<1, { T::MaxVotes::get() }>) -> Result<(), BenchmarkError> { let caller = funded_account::("caller", r); let caller_lookup = T::Lookup::unlookup(caller.clone()); let account_vote = account_vote::(100u32.into()); - for i in 0 .. r { + for i in 0..r { let ref_index = add_referendum::(i).0; - Democracy::::vote(RawOrigin::Signed(caller.clone()).into(), ref_index, account_vote)?; + Democracy::::vote( + RawOrigin::Signed(caller.clone()).into(), + ref_index, + account_vote, + )?; } let votes = match VotingOf::::get(&caller) { @@ -732,68 +852,71 @@ benchmarks! { let ref_index = r - 1; whitelist_account!(caller); - }: _(RawOrigin::Signed(caller.clone()), caller_lookup, ref_index) - verify { + + #[extrinsic_call] + _(RawOrigin::Signed(caller.clone()), caller_lookup, ref_index); + let votes = match VotingOf::::get(&caller) { Voting::Direct { votes, .. } => votes, _ => return Err("Votes are not direct".into()), }; assert_eq!(votes.len(), (r - 1) as usize, "Vote was not removed"); + Ok(()) } - set_external_metadata { + #[benchmark] + fn set_external_metadata() -> Result<(), BenchmarkError> { let origin = T::ExternalOrigin::try_successful_origin() .expect("ExternalOrigin has no successful origin required for the benchmark"); - assert_ok!( - Democracy::::external_propose(origin.clone(), make_proposal::(0)) - ); + assert_ok!(Democracy::::external_propose(origin.clone(), make_proposal::(0))); let owner = MetadataOwner::External; let hash = note_preimage::(); - }: set_metadata(origin, owner.clone(), Some(hash)) - verify { - assert_last_event::(crate::Event::MetadataSet { - owner, - hash, - }.into()); + + #[extrinsic_call] + set_metadata(origin as T::RuntimeOrigin, owner.clone(), Some(hash)); + + assert_last_event::(crate::Event::MetadataSet { owner, hash }.into()); + Ok(()) } - clear_external_metadata { + #[benchmark] + fn clear_external_metadata() -> Result<(), BenchmarkError> { let origin = T::ExternalOrigin::try_successful_origin() .expect("ExternalOrigin has no successful origin required for the benchmark"); - assert_ok!( - Democracy::::external_propose(origin.clone(), make_proposal::(0)) - ); + assert_ok!(Democracy::::external_propose(origin.clone(), make_proposal::(0))); let owner = MetadataOwner::External; - let proposer = funded_account::("proposer", 0); + let _proposer = funded_account::("proposer", 0); let hash = note_preimage::(); assert_ok!(Democracy::::set_metadata(origin.clone(), owner.clone(), Some(hash))); - }: set_metadata(origin, owner.clone(), None) - verify { - assert_last_event::(crate::Event::MetadataCleared { - owner, - hash, - }.into()); + + #[extrinsic_call] + set_metadata(origin as T::RuntimeOrigin, owner.clone(), None); + + assert_last_event::(crate::Event::MetadataCleared { owner, hash }.into()); + Ok(()) } - set_proposal_metadata { + #[benchmark] + fn set_proposal_metadata() -> Result<(), BenchmarkError> { // Place our proposal at the end to make sure it's worst case. - for i in 0 .. T::MaxProposals::get() { + for i in 0..T::MaxProposals::get() { add_proposal::(i)?; } let owner = MetadataOwner::Proposal(0); let proposer = funded_account::("proposer", 0); let hash = note_preimage::(); - }: set_metadata(RawOrigin::Signed(proposer).into(), owner.clone(), Some(hash)) - verify { - assert_last_event::(crate::Event::MetadataSet { - owner, - hash, - }.into()); + + #[extrinsic_call] + set_metadata(RawOrigin::Signed(proposer), owner.clone(), Some(hash)); + + assert_last_event::(crate::Event::MetadataSet { owner, hash }.into()); + Ok(()) } - clear_proposal_metadata { + #[benchmark] + fn clear_proposal_metadata() -> Result<(), BenchmarkError> { // Place our proposal at the end to make sure it's worst case. - for i in 0 .. T::MaxProposals::get() { + for i in 0..T::MaxProposals::get() { add_proposal::(i)?; } let proposer = funded_account::("proposer", 0); @@ -802,33 +925,36 @@ benchmarks! { assert_ok!(Democracy::::set_metadata( RawOrigin::Signed(proposer.clone()).into(), owner.clone(), - Some(hash))); - }: set_metadata(RawOrigin::Signed(proposer).into(), owner.clone(), None) - verify { - assert_last_event::(crate::Event::MetadataCleared { - owner, - hash, - }.into()); + Some(hash) + )); + + #[extrinsic_call] + set_metadata::(RawOrigin::Signed(proposer), owner.clone(), None); + + assert_last_event::(crate::Event::MetadataCleared { owner, hash }.into()); + Ok(()) } - set_referendum_metadata { + #[benchmark] + fn set_referendum_metadata() -> Result<(), BenchmarkError> { // create not ongoing referendum. ReferendumInfoOf::::insert( 0, ReferendumInfo::Finished { end: BlockNumberFor::::zero(), approved: true }, ); let owner = MetadataOwner::Referendum(0); - let caller = funded_account::("caller", 0); + let _caller = funded_account::("caller", 0); let hash = note_preimage::(); - }: set_metadata(RawOrigin::Root.into(), owner.clone(), Some(hash)) - verify { - assert_last_event::(crate::Event::MetadataSet { - owner, - hash, - }.into()); + + #[extrinsic_call] + set_metadata::(RawOrigin::Root, owner.clone(), Some(hash)); + + assert_last_event::(crate::Event::MetadataSet { owner, hash }.into()); + Ok(()) } - clear_referendum_metadata { + #[benchmark] + fn clear_referendum_metadata() -> Result<(), BenchmarkError> { // create not ongoing referendum. ReferendumInfoOf::::insert( 0, @@ -838,17 +964,13 @@ benchmarks! { let hash = note_preimage::(); MetadataOf::::insert(owner.clone(), hash); let caller = funded_account::("caller", 0); - }: set_metadata(RawOrigin::Signed(caller).into(), owner.clone(), None) - verify { - assert_last_event::(crate::Event::MetadataCleared { - owner, - hash, - }.into()); + + #[extrinsic_call] + set_metadata::(RawOrigin::Signed(caller), owner.clone(), None); + + assert_last_event::(crate::Event::MetadataCleared { owner, hash }.into()); + Ok(()) } - impl_benchmark_test_suite!( - Democracy, - crate::tests::new_test_ext(), - crate::tests::Test - ); + impl_benchmark_test_suite!(Democracy, crate::tests::new_test_ext(), crate::tests::Test); } diff --git a/substrate/frame/democracy/src/weights.rs b/substrate/frame/democracy/src/weights.rs index 0a2200a78b5d..765ee57f0eb3 100644 --- a/substrate/frame/democracy/src/weights.rs +++ b/substrate/frame/democracy/src/weights.rs @@ -18,27 +18,25 @@ //! Autogenerated weights for `pallet_democracy` //! //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 32.0.0 -//! DATE: 2024-11-08, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]` +//! DATE: 2024-11-19, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]` //! WORST CASE MAP SIZE: `1000000` //! HOSTNAME: `runner-wiukf8gn-project-674-concurrent-0`, CPU: `Intel(R) Xeon(R) CPU @ 2.60GHz` //! WASM-EXECUTION: `Compiled`, CHAIN: `Some("dev")`, DB CACHE: `1024` // Executed Command: -// ./target/production/substrate-node +// target/production/substrate-node // benchmark // pallet -// --chain=dev // --steps=50 // --repeat=20 -// --pallet=pallet_democracy -// --no-storage-info -// --no-median-slopes -// --no-min-squares // --extrinsic=* // --wasm-execution=compiled // --heap-pages=4096 -// --output=./substrate/frame/democracy/src/weights.rs +// --json-file=/builds/parity/mirrors/polkadot-sdk/.git/.artifacts/bench.json +// --pallet=pallet_democracy +// --chain=dev // --header=./substrate/HEADER-APACHE2 +// --output=./substrate/frame/democracy/src/weights.rs // --template=./substrate/.maintain/frame-weight-template.hbs #![cfg_attr(rustfmt, rustfmt_skip)] @@ -96,8 +94,8 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `4834` // Estimated: `18187` - // Minimum execution time: 48_991_000 picoseconds. - Weight::from_parts(50_476_000, 18187) + // Minimum execution time: 49_681_000 picoseconds. + Weight::from_parts(51_578_000, 18187) .saturating_add(T::DbWeight::get().reads(3_u64)) .saturating_add(T::DbWeight::get().writes(3_u64)) } @@ -107,8 +105,8 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `3589` // Estimated: `6695` - // Minimum execution time: 43_129_000 picoseconds. - Weight::from_parts(45_076_000, 6695) + // Minimum execution time: 45_001_000 picoseconds. + Weight::from_parts(45_990_000, 6695) .saturating_add(T::DbWeight::get().reads(1_u64)) .saturating_add(T::DbWeight::get().writes(1_u64)) } @@ -124,8 +122,8 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `3503` // Estimated: `7260` - // Minimum execution time: 63_761_000 picoseconds. - Weight::from_parts(65_424_000, 7260) + // Minimum execution time: 65_095_000 picoseconds. + Weight::from_parts(67_484_000, 7260) .saturating_add(T::DbWeight::get().reads(4_u64)) .saturating_add(T::DbWeight::get().writes(3_u64)) } @@ -141,8 +139,8 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `3525` // Estimated: `7260` - // Minimum execution time: 66_543_000 picoseconds. - Weight::from_parts(69_537_000, 7260) + // Minimum execution time: 66_877_000 picoseconds. + Weight::from_parts(68_910_000, 7260) .saturating_add(T::DbWeight::get().reads(4_u64)) .saturating_add(T::DbWeight::get().writes(3_u64)) } @@ -156,8 +154,8 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `399` // Estimated: `3666` - // Minimum execution time: 28_934_000 picoseconds. - Weight::from_parts(29_982_000, 3666) + // Minimum execution time: 29_312_000 picoseconds. + Weight::from_parts(30_040_000, 3666) .saturating_add(T::DbWeight::get().reads(3_u64)) .saturating_add(T::DbWeight::get().writes(3_u64)) } @@ -179,8 +177,8 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `5943` // Estimated: `18187` - // Minimum execution time: 108_004_000 picoseconds. - Weight::from_parts(110_779_000, 18187) + // Minimum execution time: 107_932_000 picoseconds. + Weight::from_parts(108_940_000, 18187) .saturating_add(T::DbWeight::get().reads(8_u64)) .saturating_add(T::DbWeight::get().writes(7_u64)) } @@ -192,8 +190,8 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `3449` // Estimated: `6703` - // Minimum execution time: 17_630_000 picoseconds. - Weight::from_parts(18_419_000, 6703) + // Minimum execution time: 17_703_000 picoseconds. + Weight::from_parts(18_188_000, 6703) .saturating_add(T::DbWeight::get().reads(2_u64)) .saturating_add(T::DbWeight::get().writes(1_u64)) } @@ -203,8 +201,8 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `0` // Estimated: `0` - // Minimum execution time: 2_572_000 picoseconds. - Weight::from_parts(2_810_000, 0) + // Minimum execution time: 2_672_000 picoseconds. + Weight::from_parts(2_814_000, 0) .saturating_add(T::DbWeight::get().writes(1_u64)) } /// Storage: `Democracy::NextExternal` (r:0 w:1) @@ -213,8 +211,8 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `0` // Estimated: `0` - // Minimum execution time: 2_628_000 picoseconds. - Weight::from_parts(2_724_000, 0) + // Minimum execution time: 2_584_000 picoseconds. + Weight::from_parts(2_846_000, 0) .saturating_add(T::DbWeight::get().writes(1_u64)) } /// Storage: `Democracy::NextExternal` (r:1 w:1) @@ -229,8 +227,8 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `319` // Estimated: `3518` - // Minimum execution time: 24_624_000 picoseconds. - Weight::from_parts(25_518_000, 3518) + // Minimum execution time: 24_603_000 picoseconds. + Weight::from_parts(25_407_000, 3518) .saturating_add(T::DbWeight::get().reads(3_u64)) .saturating_add(T::DbWeight::get().writes(5_u64)) } @@ -244,8 +242,8 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `3552` // Estimated: `6703` - // Minimum execution time: 31_786_000 picoseconds. - Weight::from_parts(32_786_000, 6703) + // Minimum execution time: 31_721_000 picoseconds. + Weight::from_parts(32_785_000, 6703) .saturating_add(T::DbWeight::get().reads(3_u64)) .saturating_add(T::DbWeight::get().writes(3_u64)) } @@ -261,8 +259,8 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `5854` // Estimated: `18187` - // Minimum execution time: 87_352_000 picoseconds. - Weight::from_parts(89_670_000, 18187) + // Minimum execution time: 86_981_000 picoseconds. + Weight::from_parts(89_140_000, 18187) .saturating_add(T::DbWeight::get().reads(4_u64)) .saturating_add(T::DbWeight::get().writes(4_u64)) } @@ -274,8 +272,8 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `304` // Estimated: `3518` - // Minimum execution time: 17_561_000 picoseconds. - Weight::from_parts(18_345_000, 3518) + // Minimum execution time: 17_465_000 picoseconds. + Weight::from_parts(18_018_000, 3518) .saturating_add(T::DbWeight::get().reads(1_u64)) .saturating_add(T::DbWeight::get().writes(2_u64)) } @@ -290,10 +288,10 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `277 + r * (86 ±0)` // Estimated: `1489 + r * (2676 ±0)` - // Minimum execution time: 6_801_000 picoseconds. - Weight::from_parts(8_524_132, 1489) - // Standard Error: 10_028 - .saturating_add(Weight::from_parts(4_073_619, 0).saturating_mul(r.into())) + // Minimum execution time: 6_746_000 picoseconds. + Weight::from_parts(7_381_932, 1489) + // Standard Error: 10_311 + .saturating_add(Weight::from_parts(4_107_935, 0).saturating_mul(r.into())) .saturating_add(T::DbWeight::get().reads(2_u64)) .saturating_add(T::DbWeight::get().reads((1_u64).saturating_mul(r.into()))) .saturating_add(T::DbWeight::get().writes(1_u64)) @@ -316,10 +314,10 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `277 + r * (86 ±0)` // Estimated: `18187 + r * (2676 ±0)` - // Minimum execution time: 10_160_000 picoseconds. - Weight::from_parts(11_472_067, 18187) - // Standard Error: 10_730 - .saturating_add(Weight::from_parts(4_104_654, 0).saturating_mul(r.into())) + // Minimum execution time: 9_766_000 picoseconds. + Weight::from_parts(9_788_895, 18187) + // Standard Error: 11_913 + .saturating_add(Weight::from_parts(4_130_441, 0).saturating_mul(r.into())) .saturating_add(T::DbWeight::get().reads(5_u64)) .saturating_add(T::DbWeight::get().reads((1_u64).saturating_mul(r.into()))) .saturating_add(T::DbWeight::get().writes(1_u64)) @@ -338,10 +336,10 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `863 + r * (108 ±0)` // Estimated: `19800 + r * (2676 ±0)` - // Minimum execution time: 49_741_000 picoseconds. - Weight::from_parts(53_544_421, 19800) - // Standard Error: 11_984 - .saturating_add(Weight::from_parts(5_123_946, 0).saturating_mul(r.into())) + // Minimum execution time: 48_992_000 picoseconds. + Weight::from_parts(55_524_560, 19800) + // Standard Error: 11_278 + .saturating_add(Weight::from_parts(4_987_109, 0).saturating_mul(r.into())) .saturating_add(T::DbWeight::get().reads(5_u64)) .saturating_add(T::DbWeight::get().reads((1_u64).saturating_mul(r.into()))) .saturating_add(T::DbWeight::get().writes(4_u64)) @@ -357,10 +355,10 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `526 + r * (108 ±0)` // Estimated: `13530 + r * (2676 ±0)` - // Minimum execution time: 24_977_000 picoseconds. - Weight::from_parts(22_449_729, 13530) - // Standard Error: 10_846 - .saturating_add(Weight::from_parts(5_058_209, 0).saturating_mul(r.into())) + // Minimum execution time: 23_828_000 picoseconds. + Weight::from_parts(23_638_577, 13530) + // Standard Error: 10_946 + .saturating_add(Weight::from_parts(4_971_245, 0).saturating_mul(r.into())) .saturating_add(T::DbWeight::get().reads(2_u64)) .saturating_add(T::DbWeight::get().reads((1_u64).saturating_mul(r.into()))) .saturating_add(T::DbWeight::get().writes(2_u64)) @@ -373,8 +371,8 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `0` // Estimated: `0` - // Minimum execution time: 2_700_000 picoseconds. - Weight::from_parts(3_028_000, 0) + // Minimum execution time: 2_759_000 picoseconds. + Weight::from_parts(2_850_000, 0) .saturating_add(T::DbWeight::get().writes(1_u64)) } /// Storage: `Democracy::VotingOf` (r:1 w:1) @@ -390,10 +388,10 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `596` // Estimated: `7260` - // Minimum execution time: 31_183_000 picoseconds. - Weight::from_parts(43_105_470, 7260) - // Standard Error: 3_096 - .saturating_add(Weight::from_parts(98_571, 0).saturating_mul(r.into())) + // Minimum execution time: 30_804_000 picoseconds. + Weight::from_parts(42_750_018, 7260) + // Standard Error: 3_300 + .saturating_add(Weight::from_parts(99_997, 0).saturating_mul(r.into())) .saturating_add(T::DbWeight::get().reads(4_u64)) .saturating_add(T::DbWeight::get().writes(3_u64)) } @@ -410,10 +408,10 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `597 + r * (22 ±0)` // Estimated: `7260` - // Minimum execution time: 39_672_000 picoseconds. - Weight::from_parts(44_120_387, 7260) - // Standard Error: 1_890 - .saturating_add(Weight::from_parts(130_089, 0).saturating_mul(r.into())) + // Minimum execution time: 39_946_000 picoseconds. + Weight::from_parts(44_500_306, 7260) + // Standard Error: 1_914 + .saturating_add(Weight::from_parts(116_987, 0).saturating_mul(r.into())) .saturating_add(T::DbWeight::get().reads(4_u64)) .saturating_add(T::DbWeight::get().writes(3_u64)) } @@ -426,10 +424,10 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `761 + r * (26 ±0)` // Estimated: `7260` - // Minimum execution time: 21_396_000 picoseconds. - Weight::from_parts(26_151_983, 7260) - // Standard Error: 2_052 - .saturating_add(Weight::from_parts(131_709, 0).saturating_mul(r.into())) + // Minimum execution time: 21_677_000 picoseconds. + Weight::from_parts(25_329_290, 7260) + // Standard Error: 1_998 + .saturating_add(Weight::from_parts(157_800, 0).saturating_mul(r.into())) .saturating_add(T::DbWeight::get().reads(2_u64)) .saturating_add(T::DbWeight::get().writes(2_u64)) } @@ -442,10 +440,10 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `761 + r * (26 ±0)` // Estimated: `7260` - // Minimum execution time: 21_425_000 picoseconds. - Weight::from_parts(26_335_367, 7260) - // Standard Error: 2_170 - .saturating_add(Weight::from_parts(130_502, 0).saturating_mul(r.into())) + // Minimum execution time: 21_777_000 picoseconds. + Weight::from_parts(26_635_600, 7260) + // Standard Error: 2_697 + .saturating_add(Weight::from_parts(135_641, 0).saturating_mul(r.into())) .saturating_add(T::DbWeight::get().reads(2_u64)) .saturating_add(T::DbWeight::get().writes(2_u64)) } @@ -461,8 +459,8 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `351` // Estimated: `3556` - // Minimum execution time: 19_765_000 picoseconds. - Weight::from_parts(20_266_000, 3556) + // Minimum execution time: 19_914_000 picoseconds. + Weight::from_parts(20_450_000, 3556) .saturating_add(T::DbWeight::get().reads(3_u64)) .saturating_add(T::DbWeight::get().writes(1_u64)) } @@ -474,8 +472,8 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `319` // Estimated: `3518` - // Minimum execution time: 16_560_000 picoseconds. - Weight::from_parts(17_277_000, 3518) + // Minimum execution time: 16_212_000 picoseconds. + Weight::from_parts(16_745_000, 3518) .saturating_add(T::DbWeight::get().reads(2_u64)) .saturating_add(T::DbWeight::get().writes(1_u64)) } @@ -491,8 +489,8 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `4883` // Estimated: `18187` - // Minimum execution time: 47_711_000 picoseconds. - Weight::from_parts(48_669_000, 18187) + // Minimum execution time: 47_225_000 picoseconds. + Weight::from_parts(47_976_000, 18187) .saturating_add(T::DbWeight::get().reads(3_u64)) .saturating_add(T::DbWeight::get().writes(1_u64)) } @@ -504,8 +502,8 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `4855` // Estimated: `18187` - // Minimum execution time: 43_809_000 picoseconds. - Weight::from_parts(45_698_000, 18187) + // Minimum execution time: 43_140_000 picoseconds. + Weight::from_parts(43_924_000, 18187) .saturating_add(T::DbWeight::get().reads(2_u64)) .saturating_add(T::DbWeight::get().writes(1_u64)) } @@ -519,8 +517,8 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `106` // Estimated: `3556` - // Minimum execution time: 14_736_000 picoseconds. - Weight::from_parts(15_191_000, 3556) + // Minimum execution time: 14_614_000 picoseconds. + Weight::from_parts(15_376_000, 3556) .saturating_add(T::DbWeight::get().reads(2_u64)) .saturating_add(T::DbWeight::get().writes(1_u64)) } @@ -532,8 +530,8 @@ impl WeightInfo for SubstrateWeight { // Proof Size summary in bytes: // Measured: `335` // Estimated: `3666` - // Minimum execution time: 22_803_000 picoseconds. - Weight::from_parts(23_732_000, 3666) + // Minimum execution time: 22_588_000 picoseconds. + Weight::from_parts(23_267_000, 3666) .saturating_add(T::DbWeight::get().reads(2_u64)) .saturating_add(T::DbWeight::get().writes(1_u64)) } @@ -553,8 +551,8 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `4834` // Estimated: `18187` - // Minimum execution time: 48_991_000 picoseconds. - Weight::from_parts(50_476_000, 18187) + // Minimum execution time: 49_681_000 picoseconds. + Weight::from_parts(51_578_000, 18187) .saturating_add(RocksDbWeight::get().reads(3_u64)) .saturating_add(RocksDbWeight::get().writes(3_u64)) } @@ -564,8 +562,8 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `3589` // Estimated: `6695` - // Minimum execution time: 43_129_000 picoseconds. - Weight::from_parts(45_076_000, 6695) + // Minimum execution time: 45_001_000 picoseconds. + Weight::from_parts(45_990_000, 6695) .saturating_add(RocksDbWeight::get().reads(1_u64)) .saturating_add(RocksDbWeight::get().writes(1_u64)) } @@ -581,8 +579,8 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `3503` // Estimated: `7260` - // Minimum execution time: 63_761_000 picoseconds. - Weight::from_parts(65_424_000, 7260) + // Minimum execution time: 65_095_000 picoseconds. + Weight::from_parts(67_484_000, 7260) .saturating_add(RocksDbWeight::get().reads(4_u64)) .saturating_add(RocksDbWeight::get().writes(3_u64)) } @@ -598,8 +596,8 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `3525` // Estimated: `7260` - // Minimum execution time: 66_543_000 picoseconds. - Weight::from_parts(69_537_000, 7260) + // Minimum execution time: 66_877_000 picoseconds. + Weight::from_parts(68_910_000, 7260) .saturating_add(RocksDbWeight::get().reads(4_u64)) .saturating_add(RocksDbWeight::get().writes(3_u64)) } @@ -613,8 +611,8 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `399` // Estimated: `3666` - // Minimum execution time: 28_934_000 picoseconds. - Weight::from_parts(29_982_000, 3666) + // Minimum execution time: 29_312_000 picoseconds. + Weight::from_parts(30_040_000, 3666) .saturating_add(RocksDbWeight::get().reads(3_u64)) .saturating_add(RocksDbWeight::get().writes(3_u64)) } @@ -636,8 +634,8 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `5943` // Estimated: `18187` - // Minimum execution time: 108_004_000 picoseconds. - Weight::from_parts(110_779_000, 18187) + // Minimum execution time: 107_932_000 picoseconds. + Weight::from_parts(108_940_000, 18187) .saturating_add(RocksDbWeight::get().reads(8_u64)) .saturating_add(RocksDbWeight::get().writes(7_u64)) } @@ -649,8 +647,8 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `3449` // Estimated: `6703` - // Minimum execution time: 17_630_000 picoseconds. - Weight::from_parts(18_419_000, 6703) + // Minimum execution time: 17_703_000 picoseconds. + Weight::from_parts(18_188_000, 6703) .saturating_add(RocksDbWeight::get().reads(2_u64)) .saturating_add(RocksDbWeight::get().writes(1_u64)) } @@ -660,8 +658,8 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `0` // Estimated: `0` - // Minimum execution time: 2_572_000 picoseconds. - Weight::from_parts(2_810_000, 0) + // Minimum execution time: 2_672_000 picoseconds. + Weight::from_parts(2_814_000, 0) .saturating_add(RocksDbWeight::get().writes(1_u64)) } /// Storage: `Democracy::NextExternal` (r:0 w:1) @@ -670,8 +668,8 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `0` // Estimated: `0` - // Minimum execution time: 2_628_000 picoseconds. - Weight::from_parts(2_724_000, 0) + // Minimum execution time: 2_584_000 picoseconds. + Weight::from_parts(2_846_000, 0) .saturating_add(RocksDbWeight::get().writes(1_u64)) } /// Storage: `Democracy::NextExternal` (r:1 w:1) @@ -686,8 +684,8 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `319` // Estimated: `3518` - // Minimum execution time: 24_624_000 picoseconds. - Weight::from_parts(25_518_000, 3518) + // Minimum execution time: 24_603_000 picoseconds. + Weight::from_parts(25_407_000, 3518) .saturating_add(RocksDbWeight::get().reads(3_u64)) .saturating_add(RocksDbWeight::get().writes(5_u64)) } @@ -701,8 +699,8 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `3552` // Estimated: `6703` - // Minimum execution time: 31_786_000 picoseconds. - Weight::from_parts(32_786_000, 6703) + // Minimum execution time: 31_721_000 picoseconds. + Weight::from_parts(32_785_000, 6703) .saturating_add(RocksDbWeight::get().reads(3_u64)) .saturating_add(RocksDbWeight::get().writes(3_u64)) } @@ -718,8 +716,8 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `5854` // Estimated: `18187` - // Minimum execution time: 87_352_000 picoseconds. - Weight::from_parts(89_670_000, 18187) + // Minimum execution time: 86_981_000 picoseconds. + Weight::from_parts(89_140_000, 18187) .saturating_add(RocksDbWeight::get().reads(4_u64)) .saturating_add(RocksDbWeight::get().writes(4_u64)) } @@ -731,8 +729,8 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `304` // Estimated: `3518` - // Minimum execution time: 17_561_000 picoseconds. - Weight::from_parts(18_345_000, 3518) + // Minimum execution time: 17_465_000 picoseconds. + Weight::from_parts(18_018_000, 3518) .saturating_add(RocksDbWeight::get().reads(1_u64)) .saturating_add(RocksDbWeight::get().writes(2_u64)) } @@ -747,10 +745,10 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `277 + r * (86 ±0)` // Estimated: `1489 + r * (2676 ±0)` - // Minimum execution time: 6_801_000 picoseconds. - Weight::from_parts(8_524_132, 1489) - // Standard Error: 10_028 - .saturating_add(Weight::from_parts(4_073_619, 0).saturating_mul(r.into())) + // Minimum execution time: 6_746_000 picoseconds. + Weight::from_parts(7_381_932, 1489) + // Standard Error: 10_311 + .saturating_add(Weight::from_parts(4_107_935, 0).saturating_mul(r.into())) .saturating_add(RocksDbWeight::get().reads(2_u64)) .saturating_add(RocksDbWeight::get().reads((1_u64).saturating_mul(r.into()))) .saturating_add(RocksDbWeight::get().writes(1_u64)) @@ -773,10 +771,10 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `277 + r * (86 ±0)` // Estimated: `18187 + r * (2676 ±0)` - // Minimum execution time: 10_160_000 picoseconds. - Weight::from_parts(11_472_067, 18187) - // Standard Error: 10_730 - .saturating_add(Weight::from_parts(4_104_654, 0).saturating_mul(r.into())) + // Minimum execution time: 9_766_000 picoseconds. + Weight::from_parts(9_788_895, 18187) + // Standard Error: 11_913 + .saturating_add(Weight::from_parts(4_130_441, 0).saturating_mul(r.into())) .saturating_add(RocksDbWeight::get().reads(5_u64)) .saturating_add(RocksDbWeight::get().reads((1_u64).saturating_mul(r.into()))) .saturating_add(RocksDbWeight::get().writes(1_u64)) @@ -795,10 +793,10 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `863 + r * (108 ±0)` // Estimated: `19800 + r * (2676 ±0)` - // Minimum execution time: 49_741_000 picoseconds. - Weight::from_parts(53_544_421, 19800) - // Standard Error: 11_984 - .saturating_add(Weight::from_parts(5_123_946, 0).saturating_mul(r.into())) + // Minimum execution time: 48_992_000 picoseconds. + Weight::from_parts(55_524_560, 19800) + // Standard Error: 11_278 + .saturating_add(Weight::from_parts(4_987_109, 0).saturating_mul(r.into())) .saturating_add(RocksDbWeight::get().reads(5_u64)) .saturating_add(RocksDbWeight::get().reads((1_u64).saturating_mul(r.into()))) .saturating_add(RocksDbWeight::get().writes(4_u64)) @@ -814,10 +812,10 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `526 + r * (108 ±0)` // Estimated: `13530 + r * (2676 ±0)` - // Minimum execution time: 24_977_000 picoseconds. - Weight::from_parts(22_449_729, 13530) - // Standard Error: 10_846 - .saturating_add(Weight::from_parts(5_058_209, 0).saturating_mul(r.into())) + // Minimum execution time: 23_828_000 picoseconds. + Weight::from_parts(23_638_577, 13530) + // Standard Error: 10_946 + .saturating_add(Weight::from_parts(4_971_245, 0).saturating_mul(r.into())) .saturating_add(RocksDbWeight::get().reads(2_u64)) .saturating_add(RocksDbWeight::get().reads((1_u64).saturating_mul(r.into()))) .saturating_add(RocksDbWeight::get().writes(2_u64)) @@ -830,8 +828,8 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `0` // Estimated: `0` - // Minimum execution time: 2_700_000 picoseconds. - Weight::from_parts(3_028_000, 0) + // Minimum execution time: 2_759_000 picoseconds. + Weight::from_parts(2_850_000, 0) .saturating_add(RocksDbWeight::get().writes(1_u64)) } /// Storage: `Democracy::VotingOf` (r:1 w:1) @@ -847,10 +845,10 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `596` // Estimated: `7260` - // Minimum execution time: 31_183_000 picoseconds. - Weight::from_parts(43_105_470, 7260) - // Standard Error: 3_096 - .saturating_add(Weight::from_parts(98_571, 0).saturating_mul(r.into())) + // Minimum execution time: 30_804_000 picoseconds. + Weight::from_parts(42_750_018, 7260) + // Standard Error: 3_300 + .saturating_add(Weight::from_parts(99_997, 0).saturating_mul(r.into())) .saturating_add(RocksDbWeight::get().reads(4_u64)) .saturating_add(RocksDbWeight::get().writes(3_u64)) } @@ -867,10 +865,10 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `597 + r * (22 ±0)` // Estimated: `7260` - // Minimum execution time: 39_672_000 picoseconds. - Weight::from_parts(44_120_387, 7260) - // Standard Error: 1_890 - .saturating_add(Weight::from_parts(130_089, 0).saturating_mul(r.into())) + // Minimum execution time: 39_946_000 picoseconds. + Weight::from_parts(44_500_306, 7260) + // Standard Error: 1_914 + .saturating_add(Weight::from_parts(116_987, 0).saturating_mul(r.into())) .saturating_add(RocksDbWeight::get().reads(4_u64)) .saturating_add(RocksDbWeight::get().writes(3_u64)) } @@ -883,10 +881,10 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `761 + r * (26 ±0)` // Estimated: `7260` - // Minimum execution time: 21_396_000 picoseconds. - Weight::from_parts(26_151_983, 7260) - // Standard Error: 2_052 - .saturating_add(Weight::from_parts(131_709, 0).saturating_mul(r.into())) + // Minimum execution time: 21_677_000 picoseconds. + Weight::from_parts(25_329_290, 7260) + // Standard Error: 1_998 + .saturating_add(Weight::from_parts(157_800, 0).saturating_mul(r.into())) .saturating_add(RocksDbWeight::get().reads(2_u64)) .saturating_add(RocksDbWeight::get().writes(2_u64)) } @@ -899,10 +897,10 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `761 + r * (26 ±0)` // Estimated: `7260` - // Minimum execution time: 21_425_000 picoseconds. - Weight::from_parts(26_335_367, 7260) - // Standard Error: 2_170 - .saturating_add(Weight::from_parts(130_502, 0).saturating_mul(r.into())) + // Minimum execution time: 21_777_000 picoseconds. + Weight::from_parts(26_635_600, 7260) + // Standard Error: 2_697 + .saturating_add(Weight::from_parts(135_641, 0).saturating_mul(r.into())) .saturating_add(RocksDbWeight::get().reads(2_u64)) .saturating_add(RocksDbWeight::get().writes(2_u64)) } @@ -918,8 +916,8 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `351` // Estimated: `3556` - // Minimum execution time: 19_765_000 picoseconds. - Weight::from_parts(20_266_000, 3556) + // Minimum execution time: 19_914_000 picoseconds. + Weight::from_parts(20_450_000, 3556) .saturating_add(RocksDbWeight::get().reads(3_u64)) .saturating_add(RocksDbWeight::get().writes(1_u64)) } @@ -931,8 +929,8 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `319` // Estimated: `3518` - // Minimum execution time: 16_560_000 picoseconds. - Weight::from_parts(17_277_000, 3518) + // Minimum execution time: 16_212_000 picoseconds. + Weight::from_parts(16_745_000, 3518) .saturating_add(RocksDbWeight::get().reads(2_u64)) .saturating_add(RocksDbWeight::get().writes(1_u64)) } @@ -948,8 +946,8 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `4883` // Estimated: `18187` - // Minimum execution time: 47_711_000 picoseconds. - Weight::from_parts(48_669_000, 18187) + // Minimum execution time: 47_225_000 picoseconds. + Weight::from_parts(47_976_000, 18187) .saturating_add(RocksDbWeight::get().reads(3_u64)) .saturating_add(RocksDbWeight::get().writes(1_u64)) } @@ -961,8 +959,8 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `4855` // Estimated: `18187` - // Minimum execution time: 43_809_000 picoseconds. - Weight::from_parts(45_698_000, 18187) + // Minimum execution time: 43_140_000 picoseconds. + Weight::from_parts(43_924_000, 18187) .saturating_add(RocksDbWeight::get().reads(2_u64)) .saturating_add(RocksDbWeight::get().writes(1_u64)) } @@ -976,8 +974,8 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `106` // Estimated: `3556` - // Minimum execution time: 14_736_000 picoseconds. - Weight::from_parts(15_191_000, 3556) + // Minimum execution time: 14_614_000 picoseconds. + Weight::from_parts(15_376_000, 3556) .saturating_add(RocksDbWeight::get().reads(2_u64)) .saturating_add(RocksDbWeight::get().writes(1_u64)) } @@ -989,8 +987,8 @@ impl WeightInfo for () { // Proof Size summary in bytes: // Measured: `335` // Estimated: `3666` - // Minimum execution time: 22_803_000 picoseconds. - Weight::from_parts(23_732_000, 3666) + // Minimum execution time: 22_588_000 picoseconds. + Weight::from_parts(23_267_000, 3666) .saturating_add(RocksDbWeight::get().reads(2_u64)) .saturating_add(RocksDbWeight::get().writes(1_u64)) }