frame-benchmarking-cli: Remove native dispatch requirement (#14474)

bkchr · web-flow · commit 444bc4f89740 · 2023-06-29T17:56:25.000+02:00
* frame-benchmarking-cli: Remove native dispatch requirement

No need for this, we can just use the `WasmExecutor` directly.

* Fixes

* Pass benchmarking host functions

* Ensure we can pass custom host functions
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/bin/node-template/node/src/command.rs b/bin/node-template/node/src/command.rs
@@ -124,7 +124,7 @@ pub fn run() -> sc_cli::Result<()> {
 							)
 						}
 
-						cmd.run::<Block, service::ExecutorDispatch>(config)
+						cmd.run::<Block, ()>(config)
 					},
 					BenchmarkCmd::Block(cmd) => {
 						let PartialComponents { client, .. } = service::new_partial(&config)?;
diff --git a/bin/node/cli/Cargo.toml b/bin/node/cli/Cargo.toml
@@ -58,6 +58,7 @@ sp-keystore = { version = "0.27.0", path = "../../../primitives/keystore" }
 sp-consensus = { version = "0.10.0-dev", path = "../../../primitives/consensus/common" }
 sp-transaction-storage-proof = { version = "4.0.0-dev", path = "../../../primitives/transaction-storage-proof" }
 sp-io = { path = "../../../primitives/io" }
+sp-statement-store = { path = "../../../primitives/statement-store" }
 
 # client dependencies
 sc-client-api = { version = "4.0.0-dev", path = "../../../client/api" }
diff --git a/bin/node/cli/src/command.rs b/bin/node/cli/src/command.rs
@@ -117,7 +117,7 @@ pub fn run() -> Result<()> {
 							)
 						}
 
-						cmd.run::<Block, ExecutorDispatch>(config)
+						cmd.run::<Block, sp_statement_store::runtime_api::HostFunctions>(config)
 					},
 					BenchmarkCmd::Block(cmd) => {
 						// ensure that we keep the task manager alive
diff --git a/client/consensus/babe/src/lib.rs b/client/consensus/babe/src/lib.rs
@@ -1152,10 +1152,10 @@ where
 			// Verification for imported blocks is skipped in two cases:
 			// 1. When importing blocks below the last finalized block during network initial
 			//    synchronization.
-			// 2. When importing whole state we don't calculate epoch descriptor, but rather
-			//    read it from the state after import. We also skip all verifications
-			//    because there's no parent state and we trust the sync module to verify
-			//    that the state is correct and finalized.
+			// 2. When importing whole state we don't calculate epoch descriptor, but rather read it
+			//    from the state after import. We also skip all verifications because there's no
+			//    parent state and we trust the sync module to verify that the state is correct and
+			//    finalized.
 			return Ok(block)
 		}
 
diff --git a/client/consensus/beefy/src/worker.rs b/client/consensus/beefy/src/worker.rs
@@ -78,11 +78,11 @@ pub(crate) struct VoterOracle<B: Block> {
 	///
 	/// There are three voter states coresponding to three queue states:
 	/// 1. voter uninitialized: queue empty,
-	/// 2. up-to-date - all mandatory blocks leading up to current GRANDPA finalized:
-	///    queue has ONE element, the 'current session' where `mandatory_done == true`,
+	/// 2. up-to-date - all mandatory blocks leading up to current GRANDPA finalized: queue has ONE
+	///    element, the 'current session' where `mandatory_done == true`,
 	/// 3. lagging behind GRANDPA: queue has [1, N] elements, where all `mandatory_done == false`.
-	///    In this state, everytime a session gets its mandatory block BEEFY finalized, it's
-	///    popped off the queue, eventually getting to state `2. up-to-date`.
+	///    In this state, everytime a session gets its mandatory block BEEFY finalized, it's popped
+	///    off the queue, eventually getting to state `2. up-to-date`.
 	sessions: VecDeque<Rounds<B>>,
 	/// Min delta in block numbers between two blocks, BEEFY should vote on.
 	min_block_delta: u32,
diff --git a/client/network/src/protocol/notifications/behaviour.rs b/client/network/src/protocol/notifications/behaviour.rs
@@ -84,19 +84,17 @@ use std::{
 /// the API of this behaviour and towards the peerset manager is aggregated in
 /// the following way:
 ///
-///   1. The enabled/disabled status is the same across all connections, as
-///      decided by the peerset manager.
-///   2. `send_packet` and `write_notification` always send all data over
-///      the same connection to preserve the ordering provided by the transport,
-///      as long as that connection is open. If it closes, a second open
-///      connection may take over, if one exists, but that case should be no
-///      different than a single connection failing and being re-established
-///      in terms of potential reordering and dropped messages. Messages can
-///      be received on any connection.
-///   3. The behaviour reports `NotificationsOut::CustomProtocolOpen` when the
-///      first connection reports `NotifsHandlerOut::OpenResultOk`.
-///   4. The behaviour reports `NotificationsOut::CustomProtocolClosed` when the
-///      last connection reports `NotifsHandlerOut::ClosedResult`.
+///   1. The enabled/disabled status is the same across all connections, as decided by the peerset
+///      manager.
+///   2. `send_packet` and `write_notification` always send all data over the same connection to
+///      preserve the ordering provided by the transport, as long as that connection is open. If it
+///      closes, a second open connection may take over, if one exists, but that case should be no
+///      different than a single connection failing and being re-established in terms of potential
+///      reordering and dropped messages. Messages can be received on any connection.
+///   3. The behaviour reports `NotificationsOut::CustomProtocolOpen` when the first connection
+///      reports `NotifsHandlerOut::OpenResultOk`.
+///   4. The behaviour reports `NotificationsOut::CustomProtocolClosed` when the last connection
+///      reports `NotifsHandlerOut::ClosedResult`.
 ///
 /// In this way, the number of actual established connections to the peer is
 /// an implementation detail of this behaviour. Note that, in practice and at
diff --git a/client/network/src/service/traits.rs b/client/network/src/service/traits.rs
@@ -402,9 +402,9 @@ pub trait NetworkNotification {
 	/// a receiver. With a `NotificationSender` at hand, sending a notification is done in two
 	/// steps:
 	///
-	/// 1.  [`NotificationSender::ready`] is used to wait for the sender to become ready
+	/// 1. [`NotificationSender::ready`] is used to wait for the sender to become ready
 	/// for another notification, yielding a [`NotificationSenderReady`] token.
-	/// 2.  [`NotificationSenderReady::send`] enqueues the notification for sending. This operation
+	/// 2. [`NotificationSenderReady::send`] enqueues the notification for sending. This operation
 	/// can only fail if the underlying notification substream or connection has suddenly closed.
 	///
 	/// An error is returned by [`NotificationSenderReady::send`] if there exists no open
diff --git a/client/rpc-spec-v2/src/chain_head/event.rs b/client/rpc-spec-v2/src/chain_head/event.rs
@@ -192,12 +192,10 @@ pub struct Finalized<Hash> {
 /// The event generated by the `follow` method.
 ///
 /// The events are generated in the following order:
-/// 1. Initialized - generated only once to signal the
-///      latest finalized block
+/// 1. Initialized - generated only once to signal the latest finalized block
 /// 2. NewBlock - a new block was added.
-/// 3. BestBlockChanged - indicate that the best block
-///      is now the one from this event. The block was
-///      announced priorly with the `NewBlock` event.
+/// 3. BestBlockChanged - indicate that the best block is now the one from this event. The block was
+///    announced priorly with the `NewBlock` event.
 /// 4. Finalized - State the finalized and pruned blocks.
 #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]
diff --git a/frame/benchmarking/src/lib.rs b/frame/benchmarking/src/lib.rs
@@ -246,22 +246,21 @@ pub use v1::*;
 /// of impls and structs required by the benchmarking engine. Additionally, a benchmark
 /// function is also generated that resembles the function definition you provide, with a few
 /// modifications:
-/// 1. The function name is transformed from i.e. `original_name` to `_original_name` so as not
-///    to collide with the struct `original_name` that is created for some of the benchmarking
-///    engine impls.
-/// 2. Appropriate `T: Config` and `I` (if this is an instance benchmark) generics are added to
-///    the function automatically during expansion, so you should not add these manually on
-///    your function definition (but you may make use of `T` and `I` anywhere within your
-///    benchmark function, in any of the three sections (setup, call, verification).
+/// 1. The function name is transformed from i.e. `original_name` to `_original_name` so as not to
+///    collide with the struct `original_name` that is created for some of the benchmarking engine
+///    impls.
+/// 2. Appropriate `T: Config` and `I` (if this is an instance benchmark) generics are added to the
+///    function automatically during expansion, so you should not add these manually on your
+///    function definition (but you may make use of `T` and `I` anywhere within your benchmark
+///    function, in any of the three sections (setup, call, verification).
 /// 3. Arguments such as `u: Linear<10, 100>` are converted to `u: u32` to make the function
 ///    directly callable.
-/// 4. A `verify: bool` param is added as the last argument. Specifying `true` will result in
-///    the verification section of your function executing, while a value of `false` will skip
+/// 4. A `verify: bool` param is added as the last argument. Specifying `true` will result in the
+///    verification section of your function executing, while a value of `false` will skip
 ///    verification.
 /// 5. If you specify a return type on the function definition, it must conform to the [rules
-///    below](#support-for-result-benchmarkerror-and-the--operator), and the last statement of
-///    the function definition must resolve to something compatible with `Result<(),
-///    BenchmarkError>`.
+///    below](#support-for-result-benchmarkerror-and-the--operator), and the last statement of the
+///    function definition must resolve to something compatible with `Result<(), BenchmarkError>`.
 ///
 /// The reason we generate an actual function as part of the expansion is to allow the compiler
 /// to enforce several constraints that would otherwise be difficult to enforce and to reduce
diff --git a/frame/contracts/src/schedule.rs b/frame/contracts/src/schedule.rs
@@ -145,21 +145,20 @@ impl Limits {
 /// There there is one field for each wasm instruction that describes the weight to
 /// execute one instruction of that name. There are a few exceptions:
 ///
-/// 1. If there is a i64 and a i32 variant of an instruction we use the weight
-///    of the former for both.
-/// 2. The following instructions are free of charge because they merely structure the
-///    wasm module and cannot be spammed without making the module invalid (and rejected):
-///    End, Unreachable, Return, Else
-/// 3. The following instructions cannot be benchmarked because they are removed by any
-///    real world execution engine as a preprocessing step and therefore don't yield a
-///    meaningful benchmark result. However, in contrast to the instructions mentioned
-///    in 2. they can be spammed. We price them with the same weight as the "default"
-///    instruction (i64.const): Block, Loop, Nop
-/// 4. We price both i64.const and drop as InstructionWeights.i64const / 2. The reason
-///    for that is that we cannot benchmark either of them on its own but we need their
-///    individual values to derive (by subtraction) the weight of all other instructions
-///    that use them as supporting instructions. Supporting means mainly pushing arguments
-///    and dropping return values in order to maintain a valid module.
+/// 1. If there is a i64 and a i32 variant of an instruction we use the weight of the former for
+///    both.
+/// 2. The following instructions are free of charge because they merely structure the wasm module
+///    and cannot be spammed without making the module invalid (and rejected): End, Unreachable,
+///    Return, Else
+/// 3. The following instructions cannot be benchmarked because they are removed by any real world
+///    execution engine as a preprocessing step and therefore don't yield a meaningful benchmark
+///    result. However, in contrast to the instructions mentioned in 2. they can be spammed. We
+///    price them with the same weight as the "default" instruction (i64.const): Block, Loop, Nop
+/// 4. We price both i64.const and drop as InstructionWeights.i64const / 2. The reason for that is
+///    that we cannot benchmark either of them on its own but we need their individual values to
+///    derive (by subtraction) the weight of all other instructions that use them as supporting
+///    instructions. Supporting means mainly pushing arguments and dropping return values in order
+///    to maintain a valid module.
 #[cfg_attr(feature = "std", derive(Serialize, Deserialize))]
 #[derive(Clone, Encode, Decode, PartialEq, Eq, ScheduleDebug, TypeInfo)]
 #[scale_info(skip_type_params(T))]
diff --git a/frame/im-online/src/lib.rs b/frame/im-online/src/lib.rs
@@ -656,7 +656,7 @@ impl<T: Config> Pallet<T> {
 		//
 		// At index `idx`:
 		// 1. A (ImOnline) public key to be used by a validator at index `idx` to send im-online
-		//          heartbeats.
+		//    heartbeats.
 		let authorities = Keys::<T>::get();
 
 		// local keystore
diff --git a/frame/recovery/src/lib.rs b/frame/recovery/src/lib.rs
@@ -48,34 +48,30 @@
 //! ### Recovery Life Cycle
 //!
 //! The intended life cycle of a successful recovery takes the following steps:
-//! 1. The account owner calls `create_recovery` to set up a recovery configuration
-//!    for their account.
-//! 2. At some later time, the account owner loses access to their account and wants
-//!    to recover it. Likely, they will need to create a new account and fund it with
-//!    enough balance to support the transaction fees and the deposit for the
-//!    recovery process.
-//! 3. Using this new account, they call `initiate_recovery`.
-//! 4. Then the account owner would contact their configured friends to vouch for
-//!    the recovery attempt. The account owner would provide their old account id
-//!    and the new account id, and friends would call `vouch_recovery` with those
-//!    parameters.
-//! 5. Once a threshold number of friends have vouched for the recovery attempt,
-//!    the account owner needs to wait until the delay period has passed, starting
-//!    when they initiated the recovery process.
-//! 6. Now the account owner is able to call `claim_recovery`, which subsequently
-//!    allows them to call `as_recovered` and directly make calls on-behalf-of the lost
+//! 1. The account owner calls `create_recovery` to set up a recovery configuration for their
 //!    account.
-//! 7. Using the now recovered account, the account owner can call `close_recovery`
-//!    on the recovery process they opened, reclaiming the recovery deposit they
-//!    placed.
+//! 2. At some later time, the account owner loses access to their account and wants to recover it.
+//!    Likely, they will need to create a new account and fund it with enough balance to support the
+//!    transaction fees and the deposit for the recovery process.
+//! 3. Using this new account, they call `initiate_recovery`.
+//! 4. Then the account owner would contact their configured friends to vouch for the recovery
+//!    attempt. The account owner would provide their old account id and the new account id, and
+//!    friends would call `vouch_recovery` with those parameters.
+//! 5. Once a threshold number of friends have vouched for the recovery attempt, the account owner
+//!    needs to wait until the delay period has passed, starting when they initiated the recovery
+//!    process.
+//! 6. Now the account owner is able to call `claim_recovery`, which subsequently allows them to
+//!    call `as_recovered` and directly make calls on-behalf-of the lost account.
+//! 7. Using the now recovered account, the account owner can call `close_recovery` on the recovery
+//!    process they opened, reclaiming the recovery deposit they placed.
 //! 8. Then the account owner should then call `remove_recovery` to remove the recovery
-//!    configuration on the recovered account and reclaim the recovery configuration
-//!    deposit they placed.
-//! 9. Using `as_recovered`, the account owner is able to call any other pallets
-//!    to clean up their state and reclaim any reserved or locked funds. They
-//!    can then transfer all funds from the recovered account to the new account.
-//! 10. When the recovered account becomes reaped (i.e. its free and reserved
-//!     balance drops to zero), the final recovery link is removed.
+//!    configuration on the recovered account and reclaim the recovery configuration deposit they
+//!    placed.
+//! 9. Using `as_recovered`, the account owner is able to call any other pallets to clean up their
+//!    state and reclaim any reserved or locked funds. They can then transfer all funds from the
+//!    recovered account to the new account.
+//! 10. When the recovered account becomes reaped (i.e. its free and reserved balance drops to
+//!     zero), the final recovery link is removed.
 //!
 //! ### Malicious Recovery Attempts
 //!
diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs
@@ -2708,13 +2708,13 @@ pub mod pallet_prelude {
 ///     - query the metadata using the `state_getMetadata` RPC and curl, or use `subsee -p
 ///       <PALLET_NAME> > meta.json`
 /// 2. Generate the template upgrade for the pallet provided by `decl_storage` with the
-///     environment variable `PRINT_PALLET_UPGRADE`: `PRINT_PALLET_UPGRADE=1 cargo check -p
-///     my_pallet`. This template can be used as it contains all information for storages,
-///     genesis config and genesis build.
+///    environment variable `PRINT_PALLET_UPGRADE`: `PRINT_PALLET_UPGRADE=1 cargo check -p
+///    my_pallet`. This template can be used as it contains all information for storages,
+///    genesis config and genesis build.
 /// 3. Reorganize the pallet to have the trait `Config`, `decl_*` macros,
-///     [`ValidateUnsigned`](`pallet_prelude::ValidateUnsigned`),
-///     [`ProvideInherent`](`pallet_prelude::ProvideInherent`), and Origin` all together in one
-///     file. Suggested order:
+///    [`ValidateUnsigned`](`pallet_prelude::ValidateUnsigned`),
+///    [`ProvideInherent`](`pallet_prelude::ProvideInherent`), and Origin` all together in one
+///    file. Suggested order:
 ///     * `Config`,
 ///     * `decl_module`,
 ///     * `decl_event`,
@@ -2774,8 +2774,8 @@ pub mod pallet_prelude {
 /// 8. **migrate error**: rewrite it with attribute
 ///    [`#[pallet::error]`](#error-palleterror-optional).
 /// 9. **migrate storage**: `decl_storage` provide an upgrade template (see 3.). All storages,
-///     genesis config, genesis build and default implementation of genesis config can be
-///     taken from it directly.
+///    genesis config, genesis build and default implementation of genesis config can be taken
+///    from it directly.
 ///
 ///     Otherwise here is the manual process:
 ///
diff --git a/primitives/npos-elections/src/balancing.rs b/primitives/npos-elections/src/balancing.rs
@@ -19,7 +19,7 @@
 //!
 //! Given a committee `A` and an edge weight vector `w`, a balanced solution is one that
 //!
-//! 1.  it maximizes the sum of member supports, i.e `Argmax { sum(support(c)) }`. for all `c` in
+//! 1. it maximizes the sum of member supports, i.e `Argmax { sum(support(c)) }`. for all `c` in
 //! `A`.
 //! 2. it minimizes the sum of supports squared, i.e `Argmin { sum(support(c).pow(2)) }` for all `c`
 //! in `A`.
diff --git a/primitives/runtime/src/offchain/http.rs b/primitives/runtime/src/offchain/http.rs
@@ -343,10 +343,10 @@ impl Response {
 /// A buffered byte iterator over response body.
 ///
 /// Note that reading the body may return `None` in following cases:
-/// 1. Either the deadline you've set is reached (check via `#error`;
-/// 	   In such case you can resume the reader by setting a new deadline)
-/// 2. Or because of IOError. In such case the reader is not resumable and will keep
-///    returning `None`.
+/// 1. Either the deadline you've set is reached (check via `#error`; In such case you can resume
+///    the reader by setting a new deadline)
+/// 2. Or because of IOError. In such case the reader is not resumable and will keep returning
+///    `None`.
 /// 3. The body has been returned. The reader will keep returning `None`.
 #[derive(Clone)]
 pub struct ResponseBody {
diff --git a/primitives/staking/src/offence.rs b/primitives/staking/src/offence.rs
diff --git a/utils/frame/benchmarking-cli/Cargo.toml b/utils/frame/benchmarking-cli/Cargo.toml
diff --git a/utils/frame/benchmarking-cli/src/pallet/command.rs b/utils/frame/benchmarking-cli/src/pallet/command.rs
diff --git a/utils/wasm-builder/src/builder.rs b/utils/wasm-builder/src/builder.rs

Original file line number	Diff line number	Diff line change
`@@ -124,7 +124,7 @@ pub fn run() -> sc_cli::Result<()> {`
`124`	`124`	`)`
`125`	`125`	`}`
`126`	`126`
`127`		`- cmd.run::<Block, service::ExecutorDispatch>(config)`
	`127`	`+ cmd.run::<Block, ()>(config)`
`128`	`128`	`},`
`129`	`129`	`BenchmarkCmd::Block(cmd) => {`
`130`	`130`	`let PartialComponents { client, .. } = service::new_partial(&config)?;`
Original file line number	Diff line number	Diff line change
`@@ -117,7 +117,7 @@ pub fn run() -> Result<()> {`
`117`	`117`	`)`
`118`	`118`	`}`
`119`	`119`
`120`		`- cmd.run::<Block, ExecutorDispatch>(config)`
	`120`	`+ cmd.run::<Block, sp_statement_store::runtime_api::HostFunctions>(config)`
`121`	`121`	`},`
`122`	`122`	`BenchmarkCmd::Block(cmd) => {`
`123`	`123`	`// ensure that we keep the task manager alive`