From d9952dd5e23aceb3aa4dd181fcd3eb51031875af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Tue, 18 Feb 2020 17:56:52 +0100 Subject: [PATCH 001/438] Inspection extension to node CLI (#4697) * Initial inspect. * WiP * Add parsing tests. * Finalize CLI. * Update to latest substrate. * Remove unused imports. * Support ImportParams as well, to get the right pruning setting. * Mention in docs that hash is no 0x. * Move bytes above extrinsics. * Switch to fill helper from sc_cli. * Remove overwrite. * Fix error. * Fix error message. * Remove extra allow. * init_config --- Cargo.lock | 19 ++- bin/node/cli/Cargo.toml | 9 +- bin/node/cli/src/cli.rs | 15 ++- bin/node/cli/src/command.rs | 10 ++ bin/node/cli/src/lib.rs | 1 - bin/node/inspect/Cargo.toml | 17 +++ bin/node/inspect/src/cli.rs | 240 +++++++++++++++++++++++++++++++++++ bin/node/inspect/src/lib.rs | 204 +++++++++++++++++++++++++++++ client/service/src/config.rs | 2 +- primitives/core/Cargo.toml | 2 +- primitives/core/src/lib.rs | 9 ++ 11 files changed, 517 insertions(+), 11 deletions(-) create mode 100644 bin/node/inspect/Cargo.toml create mode 100644 bin/node/inspect/src/cli.rs create mode 100644 bin/node/inspect/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index eca9c8f1fd..b557048f9d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3377,6 +3377,7 @@ dependencies = [ "log 0.4.8", "nix", "node-executor", + "node-inspect", "node-primitives", "node-rpc", "node-runtime", @@ -3463,6 +3464,22 @@ dependencies = [ "wabt", ] +[[package]] +name = "node-inspect" +version = "0.8.0" +dependencies = [ + "derive_more", + "log 0.4.8", + "parity-scale-codec", + "sc-cli", + "sc-client-api", + "sc-service", + "sp-blockchain", + "sp-core", + "sp-runtime", + "structopt", +] + [[package]] name = "node-primitives" version = "2.0.0" @@ -7047,7 +7064,7 @@ dependencies = [ "hash256-std-hasher", "hex", "hex-literal", - "impl-serde 0.2.3", + "impl-serde 0.3.0", "lazy_static", "libsecp256k1", "log 0.4.8", diff --git a/bin/node/cli/Cargo.toml b/bin/node/cli/Cargo.toml index 528d7e5faa..079e8d13e2 100644 --- a/bin/node/cli/Cargo.toml +++ b/bin/node/cli/Cargo.toml @@ -90,6 +90,7 @@ node-executor = { version = "2.0.0", path = "../executor" } # CLI-specific dependencies sc-cli = { version = "0.8.0", optional = true, path = "../../../client/cli" } node-transaction-factory = { version = "0.8.0", optional = true, path = "../transaction-factory" } +node-inspect = { version = "0.8.0", optional = true, path = "../inspect" } # WASM-specific dependencies wasm-bindgen = { version = "0.2.57", optional = true } @@ -110,6 +111,7 @@ nix = "0.17" build-script-utils = { version = "2.0.0", package = "substrate-build-script-utils", path = "../../../utils/build-script-utils" } structopt = { version = "0.3.8", optional = true } node-transaction-factory = { version = "0.8.0", optional = true, path = "../transaction-factory" } +node-inspect = { version = "0.8.0", optional = true, path = "../inspect" } [build-dependencies.sc-cli] version = "0.8.0" @@ -129,12 +131,13 @@ browser = [ "wasm-bindgen-futures", ] cli = [ - "sc-cli", + "node-executor/wasmi-errno", + "node-inspect", "node-transaction-factory", + "sc-cli", "sc-service/rocksdb", - "node-executor/wasmi-errno", - "vergen", "structopt", + "vergen", ] wasmtime = [ "cli", diff --git a/bin/node/cli/src/cli.rs b/bin/node/cli/src/cli.rs index 7844c2c0a5..7dcd02699d 100644 --- a/bin/node/cli/src/cli.rs +++ b/bin/node/cli/src/cli.rs @@ -17,7 +17,7 @@ use sc_cli::{SharedParams, ImportParams, RunCmd}; use structopt::StructOpt; -#[allow(missing_docs)] +/// An overarching CLI command definition. #[derive(Clone, Debug, StructOpt)] #[structopt(settings = &[ structopt::clap::AppSettings::GlobalVersion, @@ -25,7 +25,7 @@ use structopt::StructOpt; structopt::clap::AppSettings::SubcommandsNegateReqs, ])] pub struct Cli { - #[allow(missing_docs)] + /// Possible subcommand with parameters. #[structopt(subcommand)] pub subcommand: Option, #[allow(missing_docs)] @@ -33,10 +33,10 @@ pub struct Cli { pub run: RunCmd, } -#[allow(missing_docs)] +/// Possible subcommands of the main binary. #[derive(Clone, Debug, StructOpt)] pub enum Subcommand { - #[allow(missing_docs)] + /// A set of base subcommands handled by `sc_cli`. #[structopt(flatten)] Base(sc_cli::Subcommand), /// The custom factory subcommmand for manufacturing transactions. @@ -46,6 +46,13 @@ pub enum Subcommand { Only supported for development or local testnet." )] Factory(FactoryCmd), + + /// The custom inspect subcommmand for decoding blocks and extrinsics. + #[structopt( + name = "inspect", + about = "Decode given block or extrinsic using current native runtime." + )] + Inspect(node_inspect::cli::InspectCmd), } /// The `factory` command used to generate transactions. diff --git a/bin/node/cli/src/command.rs b/bin/node/cli/src/command.rs index 61d1051796..ba0f2785c1 100644 --- a/bin/node/cli/src/command.rs +++ b/bin/node/cli/src/command.rs @@ -39,6 +39,16 @@ where load_spec, &version, ), + Some(Subcommand::Inspect(cmd)) => { + cmd.init(&mut config, load_spec, &version)?; + + let client = sc_service::new_full_client::< + node_runtime::Block,node_runtime::RuntimeApi, node_executor::Executor, _, _, + >(&config)?; + let inspect = node_inspect::Inspector::::new(client); + + cmd.run(inspect) + }, Some(Subcommand::Factory(cli_args)) => { sc_cli::init(&cli_args.shared_params, &version)?; sc_cli::init_config(&mut config, &cli_args.shared_params, &version, load_spec)?; diff --git a/bin/node/cli/src/lib.rs b/bin/node/cli/src/lib.rs index f5b915a2be..789d6a6913 100644 --- a/bin/node/cli/src/lib.rs +++ b/bin/node/cli/src/lib.rs @@ -27,7 +27,6 @@ //! hasn't been tested. #![warn(missing_docs)] -#![warn(unused_extern_crates)] pub mod chain_spec; diff --git a/bin/node/inspect/Cargo.toml b/bin/node/inspect/Cargo.toml new file mode 100644 index 0000000000..cbce7e4589 --- /dev/null +++ b/bin/node/inspect/Cargo.toml @@ -0,0 +1,17 @@ +[package] +name = "node-inspect" +version = "0.8.0" +authors = ["Parity Technologies "] +edition = "2018" + +[dependencies] +codec = { package = "parity-scale-codec", version = "1.0.0" } +derive_more = "0.99" +log = "0.4.8" +sc-cli = { version = "0.8.0", path = "../../../client/cli" } +sc-client-api = { version = "2.0.0", path = "../../../client/api" } +sc-service = { version = "0.8", default-features = false, path = "../../../client/service" } +sp-blockchain = { version = "2.0.0", path = "../../../primitives/blockchain" } +sp-core = { version = "2.0.0", path = "../../../primitives/core" } +sp-runtime = { version = "2.0.0", path = "../../../primitives/runtime" } +structopt = "0.3.8" diff --git a/bin/node/inspect/src/cli.rs b/bin/node/inspect/src/cli.rs new file mode 100644 index 0000000000..27afcfff91 --- /dev/null +++ b/bin/node/inspect/src/cli.rs @@ -0,0 +1,240 @@ +// Copyright 2020 Parity Technologies (UK) Ltd. +// This file is part of Substrate. + +// Substrate is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Substrate is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Substrate. If not, see . + +//! Structs to easily compose inspect sub-command for CLI. + +use std::{ + fmt::Debug, + str::FromStr, +}; +use crate::{Inspector, PrettyPrinter}; +use sc_cli::{ImportParams, SharedParams, error}; +use structopt::StructOpt; + +/// The `inspect` command used to print decoded chain data. +#[derive(Debug, StructOpt, Clone)] +pub struct InspectCmd { + #[allow(missing_docs)] + #[structopt(flatten)] + pub command: InspectSubCmd, + + #[allow(missing_docs)] + #[structopt(flatten)] + pub shared_params: SharedParams, + + #[allow(missing_docs)] + #[structopt(flatten)] + pub import_params: ImportParams, +} + +/// A possible inspect sub-commands. +#[derive(Debug, StructOpt, Clone)] +pub enum InspectSubCmd { + /// Decode block with native version of runtime and print out the details. + Block { + /// Address of the block to print out. + /// + /// Can be either a block hash (no 0x prefix) or a number to retrieve existing block, + /// or a 0x-prefixed bytes hex string, representing SCALE encoding of + /// a block. + #[structopt(value_name = "HASH or NUMBER or BYTES")] + input: String, + }, + /// Decode extrinsic with native version of runtime and print out the details. + Extrinsic { + /// Address of an extrinsic to print out. + /// + /// Can be either a block hash (no 0x prefix) or number and the index, in the form + /// of `{block}:{index}` or a 0x-prefixed bytes hex string, + /// representing SCALE encoding of an extrinsic. + #[structopt(value_name = "BLOCK:INDEX or BYTES")] + input: String, + }, +} + +impl InspectCmd { + /// Parse CLI arguments and initialize given config. + pub fn init( + &self, + config: &mut sc_service::config::Configuration, + spec_factory: impl FnOnce(&str) -> Result>, String>, + version: &sc_cli::VersionInfo, + ) -> error::Result<()> where + G: sc_service::RuntimeGenesis, + E: sc_service::ChainSpecExtension, + { + sc_cli::init_config(config, &self.shared_params, version, spec_factory)?; + // make sure to configure keystore + sc_cli::fill_config_keystore_in_memory(config)?; + // and all import params (especially pruning that has to match db meta) + sc_cli::fill_import_params( + config, + &self.import_params, + sc_service::Roles::FULL, + self.shared_params.dev, + )?; + Ok(()) + } + + /// Run the inspect command, passing the inspector. + pub fn run( + self, + inspect: Inspector, + ) -> error::Result<()> where + B: sp_runtime::traits::Block, + B::Hash: FromStr, + P: PrettyPrinter, + { + match self.command { + InspectSubCmd::Block { input } => { + let input = input.parse()?; + let res = inspect.block(input) + .map_err(|e| format!("{}", e))?; + println!("{}", res); + Ok(()) + }, + InspectSubCmd::Extrinsic { input } => { + let input = input.parse()?; + let res = inspect.extrinsic(input) + .map_err(|e| format!("{}", e))?; + println!("{}", res); + Ok(()) + }, + } + } +} + + +/// A block to retrieve. +#[derive(Debug, Clone, PartialEq)] +pub enum BlockAddress { + /// Get block by hash. + Hash(Hash), + /// Get block by number. + Number(Number), + /// Raw SCALE-encoded bytes. + Bytes(Vec), +} + +impl FromStr for BlockAddress { + type Err = String; + + fn from_str(s: &str) -> Result { + // try to parse hash first + if let Ok(hash) = s.parse() { + return Ok(Self::Hash(hash)) + } + + // then number + if let Ok(number) = s.parse() { + return Ok(Self::Number(number)) + } + + // then assume it's bytes (hex-encoded) + sp_core::bytes::from_hex(s) + .map(Self::Bytes) + .map_err(|e| format!( + "Given string does not look like hash or number. It could not be parsed as bytes either: {}", + e + )) + } +} + +/// An extrinsic address to decode and print out. +#[derive(Debug, Clone, PartialEq)] +pub enum ExtrinsicAddress { + /// Extrinsic as part of existing block. + Block(BlockAddress, usize), + /// Raw SCALE-encoded extrinsic bytes. + Bytes(Vec), +} + +impl FromStr for ExtrinsicAddress { + type Err = String; + + fn from_str(s: &str) -> Result { + // first try raw bytes + if let Ok(bytes) = sp_core::bytes::from_hex(s).map(Self::Bytes) { + return Ok(bytes) + } + + // split by a bunch of different characters + let mut it = s.split(|c| c == '.' || c == ':' || c == ' '); + let block = it.next() + .expect("First element of split iterator is never empty; qed") + .parse()?; + + let index = it.next() + .ok_or_else(|| format!("Extrinsic index missing: example \"5:0\""))? + .parse() + .map_err(|e| format!("Invalid index format: {}", e))?; + + Ok(Self::Block(block, index)) + } +} + + +#[cfg(test)] +mod tests { + use super::*; + use sp_core::hash::H160 as Hash; + + #[test] + fn should_parse_block_strings() { + type BlockAddress = super::BlockAddress; + + let b0 = BlockAddress::from_str("3BfC20f0B9aFcAcE800D73D2191166FF16540258"); + let b1 = BlockAddress::from_str("1234"); + let b2 = BlockAddress::from_str("0"); + let b3 = BlockAddress::from_str("0x0012345f"); + + + assert_eq!(b0, Ok(BlockAddress::Hash( + "3BfC20f0B9aFcAcE800D73D2191166FF16540258".parse().unwrap() + ))); + assert_eq!(b1, Ok(BlockAddress::Number(1234))); + assert_eq!(b2, Ok(BlockAddress::Number(0))); + assert_eq!(b3, Ok(BlockAddress::Bytes(vec![0, 0x12, 0x34, 0x5f]))); + } + + #[test] + fn should_parse_extrinsic_address() { + type BlockAddress = super::BlockAddress; + type ExtrinsicAddress = super::ExtrinsicAddress; + + let e0 = ExtrinsicAddress::from_str("1234"); + let b0 = ExtrinsicAddress::from_str("3BfC20f0B9aFcAcE800D73D2191166FF16540258:5"); + let b1 = ExtrinsicAddress::from_str("1234:0"); + let b2 = ExtrinsicAddress::from_str("0 0"); + let b3 = ExtrinsicAddress::from_str("0x0012345f"); + + + assert_eq!(e0, Err("Extrinsic index missing: example \"5:0\"".into())); + assert_eq!(b0, Ok(ExtrinsicAddress::Block( + BlockAddress::Hash("3BfC20f0B9aFcAcE800D73D2191166FF16540258".parse().unwrap()), + 5 + ))); + assert_eq!(b1, Ok(ExtrinsicAddress::Block( + BlockAddress::Number(1234), + 0 + ))); + assert_eq!(b2, Ok(ExtrinsicAddress::Block( + BlockAddress::Number(0), + 0 + ))); + assert_eq!(b3, Ok(ExtrinsicAddress::Bytes(vec![0, 0x12, 0x34, 0x5f]))); + } +} diff --git a/bin/node/inspect/src/lib.rs b/bin/node/inspect/src/lib.rs new file mode 100644 index 0000000000..5c4e18c0a7 --- /dev/null +++ b/bin/node/inspect/src/lib.rs @@ -0,0 +1,204 @@ +// Copyright 2020 Parity Technologies (UK) Ltd. +// This file is part of Substrate. + +// Substrate is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Substrate is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Substrate. If not, see . + +//! A CLI extension for substrate node, adding sub-command to pretty print debug info +//! about blocks and extrinsics. +//! +//! The blocks and extrinsics can either be retrieved from the database (on-chain), +//! or a raw SCALE-encoding can be provided. + +#![warn(missing_docs)] + +pub mod cli; + +use std::{ + fmt, + marker::PhantomData +}; +use codec::{Encode, Decode}; +use sc_client_api::BlockBody; +use sp_blockchain::HeaderBackend; +use sp_core::hexdisplay::HexDisplay; +use sp_runtime::{ + generic::BlockId, + traits::{Block, HashFor, NumberFor, Hash} +}; + +/// A helper type for a generic block input. +pub type BlockAddressFor = cli::BlockAddress< + as Hash>::Output, + NumberFor +>; + +/// A Pretty formatter implementation. +pub trait PrettyPrinter { + /// Nicely format block. + fn fmt_block(&self, fmt: &mut fmt::Formatter, block: &TBlock) -> fmt::Result; + /// Nicely format extrinsic. + fn fmt_extrinsic(&self, fmt: &mut fmt::Formatter, extrinsic: &TBlock::Extrinsic) -> fmt::Result; +} + +/// Default dummy debug printer. +#[derive(Default)] +pub struct DebugPrinter; +impl PrettyPrinter for DebugPrinter { + fn fmt_block(&self, fmt: &mut fmt::Formatter, block: &TBlock) -> fmt::Result { + writeln!(fmt, "Header:")?; + writeln!(fmt, "{:?}", block.header())?; + writeln!(fmt, "Block bytes: {:?}", HexDisplay::from(&block.encode()))?; + writeln!(fmt, "Extrinsics ({})", block.extrinsics().len())?; + for (idx, ex) in block.extrinsics().iter().enumerate() { + writeln!(fmt, "- {}:", idx)?; + >::fmt_extrinsic(self, fmt, ex)?; + } + Ok(()) + } + + fn fmt_extrinsic(&self, fmt: &mut fmt::Formatter, extrinsic: &TBlock::Extrinsic) -> fmt::Result { + writeln!(fmt, " {:?}", extrinsic)?; + writeln!(fmt, " Bytes: {:?}", HexDisplay::from(&extrinsic.encode()))?; + Ok(()) + } +} + +/// Aggregated error for `Inspector` operations. +#[derive(Debug, derive_more::From, derive_more::Display)] +pub enum Error { + /// Could not decode Block or Extrinsic. + Codec(codec::Error), + /// Error accessing blockchain DB. + Blockchain(sp_blockchain::Error), + /// Given block has not been found. + NotFound(String), +} + +impl std::error::Error for Error { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match *self { + Self::Codec(ref e) => Some(e), + Self::Blockchain(ref e) => Some(e), + Self::NotFound(_) => None, + } + } +} + +/// A helper trait to access block headers and bodies. +pub trait ChainAccess: + HeaderBackend + + BlockBody +{} + +impl ChainAccess for T where + TBlock: Block, + T: sp_blockchain::HeaderBackend + sc_client_api::BlockBody, +{} + +/// Blockchain inspector. +pub struct Inspector = DebugPrinter> { + printer: TPrinter, + chain: Box>, + _block: PhantomData, +} + +impl> Inspector { + /// Create new instance of the inspector with default printer. + pub fn new( + chain: impl ChainAccess + 'static, + ) -> Self where TPrinter: Default { + Self::with_printer(chain, Default::default()) + } + + /// Customize pretty-printing of the data. + pub fn with_printer( + chain: impl ChainAccess + 'static, + printer: TPrinter, + ) -> Self { + Inspector { + chain: Box::new(chain) as _, + printer, + _block: Default::default(), + } + } + + /// Get a pretty-printed block. + pub fn block(&self, input: BlockAddressFor) -> Result { + struct BlockPrinter<'a, A, B>(A, &'a B); + impl<'a, A: Block, B: PrettyPrinter> fmt::Display for BlockPrinter<'a, A, B> { + fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { + self.1.fmt_block(fmt, &self.0) + } + } + + let block = self.get_block(input)?; + Ok(format!("{}", BlockPrinter(block, &self.printer))) + } + + fn get_block(&self, input: BlockAddressFor) -> Result { + Ok(match input { + cli::BlockAddress::Bytes(bytes) => { + TBlock::decode(&mut &*bytes)? + }, + cli::BlockAddress::Number(number) => { + let id = BlockId::number(number); + let not_found = format!("Could not find block {:?}", id); + let body = self.chain.block_body(&id)? + .ok_or_else(|| Error::NotFound(not_found.clone()))?; + let header = self.chain.header(id)? + .ok_or_else(|| Error::NotFound(not_found.clone()))?; + TBlock::new(header, body) + }, + cli::BlockAddress::Hash(hash) => { + let id = BlockId::hash(hash); + let not_found = format!("Could not find block {:?}", id); + let body = self.chain.block_body(&id)? + .ok_or_else(|| Error::NotFound(not_found.clone()))?; + let header = self.chain.header(id)? + .ok_or_else(|| Error::NotFound(not_found.clone()))?; + TBlock::new(header, body) + }, + }) + } + + /// Get a pretty-printed extrinsic. + pub fn extrinsic( + &self, + input: cli::ExtrinsicAddress< as Hash>::Output, NumberFor>, + ) -> Result { + struct ExtrinsicPrinter<'a, A: Block, B>(A::Extrinsic, &'a B); + impl<'a, A: Block, B: PrettyPrinter> fmt::Display for ExtrinsicPrinter<'a, A, B> { + fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { + self.1.fmt_extrinsic(fmt, &self.0) + } + } + + let ext = match input { + cli::ExtrinsicAddress::Block(block, index) => { + let block = self.get_block(block)?; + block.extrinsics() + .get(index) + .cloned() + .ok_or_else(|| Error::NotFound(format!( + "Could not find extrinsic {} in block {:?}", index, block + )))? + }, + cli::ExtrinsicAddress::Bytes(bytes) => { + TBlock::Extrinsic::decode(&mut &*bytes)? + } + }; + + Ok(format!("{}", ExtrinsicPrinter(ext, &self.printer))) + } +} diff --git a/client/service/src/config.rs b/client/service/src/config.rs index f4043d533e..2099c600df 100644 --- a/client/service/src/config.rs +++ b/client/service/src/config.rs @@ -137,7 +137,7 @@ pub enum KeystoreConfig { password: Option> }, /// In-memory keystore. Recommended for in-browser nodes. - InMemory + InMemory, } impl KeystoreConfig { diff --git a/primitives/core/Cargo.toml b/primitives/core/Cargo.toml index 3293769023..0d28fe139a 100644 --- a/primitives/core/Cargo.toml +++ b/primitives/core/Cargo.toml @@ -13,7 +13,7 @@ log = { version = "0.4.8", default-features = false } serde = { version = "1.0.101", optional = true, features = ["derive"] } byteorder = { version = "1.3.2", default-features = false } primitive-types = { version = "0.6.2", default-features = false, features = ["codec"] } -impl-serde = { version = "0.2.3", optional = true } +impl-serde = { version = "0.3.0", optional = true } wasmi = { version = "0.6.2", optional = true } hash-db = { version = "0.15.2", default-features = false } hash256-std-hasher = { version = "0.15.2", default-features = false } diff --git a/primitives/core/src/lib.rs b/primitives/core/src/lib.rs index f8c48ee970..01a96d8853 100644 --- a/primitives/core/src/lib.rs +++ b/primitives/core/src/lib.rs @@ -133,6 +133,15 @@ impl Deref for Bytes { fn deref(&self) -> &[u8] { &self.0[..] } } +#[cfg(feature = "std")] +impl sp_std::str::FromStr for Bytes { + type Err = bytes::FromHexError; + + fn from_str(s: &str) -> Result { + bytes::from_hex(s).map(Bytes) + } +} + /// Stores the encoded `RuntimeMetadata` for the native side as opaque type. #[derive(Encode, Decode, PartialEq)] pub struct OpaqueMetadata(Vec); -- GitLab From 646e7fb3809d2efc6f0cace5e34755ab958dd9b4 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Tue, 18 Feb 2020 20:44:43 +0100 Subject: [PATCH 002/438] Fix description of --no-private-ipv4 (#4950) --- client/cli/src/params.rs | 2 +- client/network/src/config.rs | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/client/cli/src/params.rs b/client/cli/src/params.rs index 9ae7bd7748..d6c437f668 100644 --- a/client/cli/src/params.rs +++ b/client/cli/src/params.rs @@ -207,7 +207,7 @@ pub struct NetworkConfigurationParams { #[structopt(long = "port", value_name = "PORT")] pub port: Option, - /// Allow connecting to private IPv4 addresses (as specified in + /// Forbid connecting to private IPv4 addresses (as specified in /// [RFC1918](https://tools.ietf.org/html/rfc1918)), unless the address was passed with /// `--reserved-nodes` or `--bootnodes`. #[structopt(long = "no-private-ipv4")] diff --git a/client/network/src/config.rs b/client/network/src/config.rs index 87c77fee9f..2f920c6663 100644 --- a/client/network/src/config.rs +++ b/client/network/src/config.rs @@ -338,8 +338,9 @@ pub enum TransportConfig { enable_mdns: bool, /// If true, allow connecting to private IPv4 addresses (as defined in - /// [RFC1918](https://tools.ietf.org/html/rfc1918)), unless the address has been passed in - /// [`NetworkConfiguration::reserved_nodes`] or [`NetworkConfiguration::boot_nodes`]. + /// [RFC1918](https://tools.ietf.org/html/rfc1918)). Irrelevant for addresses that have + /// been passed in [`NetworkConfiguration::reserved_nodes`] or + /// [`NetworkConfiguration::boot_nodes`]. allow_private_ipv4: bool, /// Optional external implementation of a libp2p transport. Used in WASM contexts where we -- GitLab From f92b44cce3c0fb31ad96fefec3e3feb4253e7751 Mon Sep 17 00:00:00 2001 From: Stanislav Tkach Date: Wed, 19 Feb 2020 00:25:56 +0200 Subject: [PATCH 003/438] [In Progress] Remove deprecated api (#4973) * Remove deprecated api * Revert changes to wasm-build-runner --- client/finality-grandpa/src/lib.rs | 19 ------------------- client/finality-grandpa/src/observer.rs | 2 +- client/network/src/protocol/specialization.rs | 19 ------------------- 3 files changed, 1 insertion(+), 39 deletions(-) diff --git a/client/finality-grandpa/src/lib.rs b/client/finality-grandpa/src/lib.rs index 45a2400226..e931271df9 100644 --- a/client/finality-grandpa/src/lib.rs +++ b/client/finality-grandpa/src/lib.rs @@ -857,25 +857,6 @@ where } } -#[deprecated(since = "1.1.0", note = "Please switch to run_grandpa_voter.")] -pub fn run_grandpa( - grandpa_params: GrandpaParams, -) -> sp_blockchain::Result + Send + 'static> where - Block::Hash: Ord, - B: Backend + 'static, - E: CallExecutor + Send + Sync + 'static, - N: NetworkT + Send + Sync + Clone + 'static, - SC: SelectChain + 'static, - NumberFor: BlockNumberOps, - DigestFor: Encode, - RA: Send + Sync + 'static, - VR: VotingRule> + Clone + 'static, - X: futures::Future + Clone + Send + Unpin + 'static, - Client: AuxStore, -{ - run_grandpa_voter(grandpa_params) -} - /// When GRANDPA is not initialized we still need to register the finality /// tracker inherent provider which might be expected by the runtime for block /// authoring. Additionally, we register a gossip message validator that diff --git a/client/finality-grandpa/src/observer.rs b/client/finality-grandpa/src/observer.rs index ffe71d573a..77227909dc 100644 --- a/client/finality-grandpa/src/observer.rs +++ b/client/finality-grandpa/src/observer.rs @@ -377,7 +377,7 @@ mod tests { use substrate_test_runtime_client::{TestClientBuilder, TestClientBuilderExt}; use sc_network::PeerId; - use futures::executor::{self, ThreadPool}; + use futures::executor; /// Ensure `Future` implementation of `ObserverWork` is polling its `NetworkBridge`. Regression /// test for bug introduced in d4fbb897c and fixed in b7af8b339. diff --git a/client/network/src/protocol/specialization.rs b/client/network/src/protocol/specialization.rs index af6d5f7a23..6ffa335c8c 100644 --- a/client/network/src/protocol/specialization.rs +++ b/client/network/src/protocol/specialization.rs @@ -41,14 +41,6 @@ pub trait NetworkSpecialization: Send + Sync + 'static { message: Vec ); - /// Called when a network-specific event arrives. - #[deprecated(note = "This method is never called; please use `with_dht_event_tx` when building the service")] - fn on_event(&mut self, _event: Event) {} - - /// Called on abort. - #[deprecated(note = "This method is never called; aborting corresponds to dropping the object")] - fn on_abort(&mut self) { } - /// Called periodically to maintain peers and handle timeouts. fn maintain_peers(&mut self, _ctx: &mut dyn Context) { } @@ -162,17 +154,6 @@ macro_rules! construct_simple_protocol { $( self.$sub_protocol_name.on_message(_ctx, _who, _message); )* } - fn on_event( - &mut self, - _event: $crate::specialization::Event - ) { - $( self.$sub_protocol_name.on_event(_event); )* - } - - fn on_abort(&mut self) { - $( self.$sub_protocol_name.on_abort(); )* - } - fn maintain_peers(&mut self, _ctx: &mut $crate::Context<$block>) { $( self.$sub_protocol_name.maintain_peers(_ctx); )* } -- GitLab From dc92587bea4032e0a0fc96785bfd9aa17c95459e Mon Sep 17 00:00:00 2001 From: Nikolay Volf Date: Wed, 19 Feb 2020 01:34:31 +0300 Subject: [PATCH 004/438] Build block without checking signatures (#4916) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * in executive * in other places * to UnsafeResult * move doc comment * apply suggestions * allow validity mocking for TestXt * add test * augment checkable instead of another trait * fix im online test * blockbuilder dihotomy * review suggestions * update test * Update client/block-builder/src/lib.rs * updae spec_version Co-authored-by: Bastian Köcher --- bin/node-template/runtime/src/lib.rs | 4 + bin/node/runtime/src/lib.rs | 6 +- .../basic-authorship/src/basic_authorship.rs | 4 +- client/block-builder/src/lib.rs | 43 +++++-- client/rpc/src/state/tests.rs | 2 +- client/service/src/lib.rs | 3 +- frame/executive/src/lib.rs | 69 +++++++++--- frame/im-online/src/tests.rs | 4 +- primitives/block-builder/src/lib.rs | 6 +- primitives/runtime/src/generic/mod.rs | 9 ++ .../src/generic/unchecked_extrinsic.rs | 34 ++++-- primitives/runtime/src/testing.rs | 105 +++++++++++++++--- primitives/runtime/src/traits.rs | 10 +- test-utils/runtime/src/lib.rs | 11 +- test-utils/runtime/src/system.rs | 3 +- 15 files changed, 246 insertions(+), 67 deletions(-) diff --git a/bin/node-template/runtime/src/lib.rs b/bin/node-template/runtime/src/lib.rs index a9944199b8..a1bcd157ad 100644 --- a/bin/node-template/runtime/src/lib.rs +++ b/bin/node-template/runtime/src/lib.rs @@ -315,6 +315,10 @@ impl_runtime_apis! { Executive::apply_extrinsic(extrinsic) } + fn apply_trusted_extrinsic(extrinsic: ::Extrinsic) -> ApplyExtrinsicResult { + Executive::apply_trusted_extrinsic(extrinsic) + } + fn finalize_block() -> ::Header { Executive::finalize_block() } diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index a98b1700fe..956fc0bb0e 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -82,7 +82,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { // and set impl_version to 0. If only runtime // implementation changes and behavior does not, then leave spec_version as // is and increment impl_version. - spec_version: 221, + spec_version: 222, impl_version: 0, apis: RUNTIME_API_VERSIONS, }; @@ -686,6 +686,10 @@ impl_runtime_apis! { Executive::apply_extrinsic(extrinsic) } + fn apply_trusted_extrinsic(extrinsic: ::Extrinsic) -> ApplyExtrinsicResult { + Executive::apply_trusted_extrinsic(extrinsic) + } + fn finalize_block() -> ::Header { Executive::finalize_block() } diff --git a/client/basic-authorship/src/basic_authorship.rs b/client/basic-authorship/src/basic_authorship.rs index cab1231e87..a99453544e 100644 --- a/client/basic-authorship/src/basic_authorship.rs +++ b/client/basic-authorship/src/basic_authorship.rs @@ -195,7 +195,7 @@ impl ProposerInner, inherent_data )? { - block_builder.push(extrinsic)?; + block_builder.push_trusted(extrinsic)?; } // proceed with transactions @@ -218,7 +218,7 @@ impl ProposerInner, let pending_tx_data = pending_tx.data().clone(); let pending_tx_hash = pending_tx.hash().clone(); trace!("[{:?}] Pushing to the block.", pending_tx_hash); - match sc_block_builder::BlockBuilder::push(&mut block_builder, pending_tx_data) { + match sc_block_builder::BlockBuilder::push_trusted(&mut block_builder, pending_tx_data) { Ok(()) => { debug!("[{:?}] Pushed to the block.", pending_tx_hash); } diff --git a/client/block-builder/src/lib.rs b/client/block-builder/src/lib.rs index d0eb8b2892..26bc9ecea8 100644 --- a/client/block-builder/src/lib.rs +++ b/client/block-builder/src/lib.rs @@ -121,11 +121,23 @@ where backend, }) } - + /// Push onto the block's list of extrinsics. /// - /// This will ensure the extrinsic can be validly executed (by executing it); + /// This will ensure the extrinsic can be validly executed (by executing it). pub fn push(&mut self, xt: ::Extrinsic) -> Result<(), ApiErrorFor> { + self.push_internal(xt, false) + } + + /// Push onto the block's list of extrinsics. + /// + /// This will treat incoming extrinsic `xt` as untrusted and perform additional checks + /// (currenty checking signature). + pub fn push_trusted(&mut self, xt: ::Extrinsic) -> Result<(), ApiErrorFor> { + self.push_internal(xt, true) + } + + fn push_internal(&mut self, xt: ::Extrinsic, skip_signature: bool) -> Result<(), ApiErrorFor> { let block_id = &self.block_id; let extrinsics = &mut self.extrinsics; @@ -152,12 +164,29 @@ where } }) } else { - self.api.map_api_result(|api| { - match api.apply_extrinsic_with_context( + let use_trusted = skip_signature && self + .api + .has_api_with::>, _>( block_id, - ExecutionContext::BlockConstruction, - xt.clone(), - )? { + |version| version >= 5, + )?; + + self.api.map_api_result(|api| { + let apply_result = if use_trusted { + api.apply_trusted_extrinsic_with_context( + block_id, + ExecutionContext::BlockConstruction, + xt.clone(), + )? + } else { + api.apply_extrinsic_with_context( + block_id, + ExecutionContext::BlockConstruction, + xt.clone(), + )? + }; + + match apply_result { Ok(_) => { extrinsics.push(xt); Ok(()) diff --git a/client/rpc/src/state/tests.rs b/client/rpc/src/state/tests.rs index a0ab11e977..6508d46ddd 100644 --- a/client/rpc/src/state/tests.rs +++ b/client/rpc/src/state/tests.rs @@ -402,7 +402,7 @@ fn should_return_runtime_version() { let result = "{\"specName\":\"test\",\"implName\":\"parity-test\",\"authoringVersion\":1,\ \"specVersion\":1,\"implVersion\":2,\"apis\":[[\"0xdf6acb689907609b\",2],\ - [\"0x37e397fc7c91f5e4\",1],[\"0xd2bc9897eed08f15\",1],[\"0x40fe3ad401f8959a\",4],\ + [\"0x37e397fc7c91f5e4\",1],[\"0xd2bc9897eed08f15\",1],[\"0x40fe3ad401f8959a\",5],\ [\"0xc6e9a76309f39b09\",1],[\"0xdd718d5cc53262d4\",1],[\"0xcbca25e39f142387\",1],\ [\"0xf78b278be53f454c\",2],[\"0xab3c0572291feb8b\",1],[\"0xbc9d89904f5b923f\",1]]}"; diff --git a/client/service/src/lib.rs b/client/service/src/lib.rs index c358e5993e..a2e53616ab 100644 --- a/client/service/src/lib.rs +++ b/client/service/src/lib.rs @@ -705,6 +705,7 @@ mod tests { use futures::executor::block_on; use sp_consensus::SelectChain; use sp_runtime::traits::BlindCheckable; + use sp_runtime::generic::CheckSignature; use substrate_test_runtime_client::{prelude::*, runtime::{Extrinsic, Transfer}}; use sc_transaction_pool::{BasicPool, FullChainApi}; @@ -733,7 +734,7 @@ mod tests { // then assert_eq!(transactions.len(), 1); - assert!(transactions[0].1.clone().check().is_ok()); + assert!(transactions[0].1.clone().check(CheckSignature::Yes).is_ok()); // this should not panic let _ = transactions[0].1.transfer(); } diff --git a/frame/executive/src/lib.rs b/frame/executive/src/lib.rs index 2d1b306531..c112298051 100644 --- a/frame/executive/src/lib.rs +++ b/frame/executive/src/lib.rs @@ -79,13 +79,15 @@ use sp_std::{prelude::*, marker::PhantomData}; use frame_support::weights::{GetDispatchInfo, WeighBlock, DispatchInfo}; use sp_runtime::{ - generic::Digest, ApplyExtrinsicResult, + generic::Digest, + ApplyExtrinsicResult, traits::{ self, Header, Zero, One, Checkable, Applyable, CheckEqual, OnFinalize, OnInitialize, NumberFor, Block as BlockT, OffchainWorker, Dispatchable, Saturating, }, transaction_validity::TransactionValidity, }; +use sp_runtime::generic::CheckSignature; #[allow(deprecated)] use sp_runtime::traits::ValidateUnsigned; use codec::{Codec, Encode}; @@ -255,13 +257,22 @@ where pub fn apply_extrinsic(uxt: Block::Extrinsic) -> ApplyExtrinsicResult { let encoded = uxt.encode(); let encoded_len = encoded.len(); - Self::apply_extrinsic_with_len(uxt, encoded_len, Some(encoded)) + Self::apply_extrinsic_with_len(uxt, encoded_len, Some(encoded), CheckSignature::Yes) + } + + /// Apply extrinsic outside of the block execution function. + /// + /// Same as `apply_extrinsic`, but skips signature checks. + pub fn apply_trusted_extrinsic(uxt: Block::Extrinsic) -> ApplyExtrinsicResult { + let encoded = uxt.encode(); + let encoded_len = encoded.len(); + Self::apply_extrinsic_with_len(uxt, encoded_len, Some(encoded), CheckSignature::No) } /// Apply an extrinsic inside the block execution function. fn apply_extrinsic_no_note(uxt: Block::Extrinsic) { let l = uxt.encode().len(); - match Self::apply_extrinsic_with_len(uxt, l, None) { + match Self::apply_extrinsic_with_len(uxt, l, None, CheckSignature::Yes) { Ok(_) => (), Err(e) => { let err: &'static str = e.into(); panic!(err) }, } @@ -272,9 +283,13 @@ where uxt: Block::Extrinsic, encoded_len: usize, to_note: Option>, + check_signature: CheckSignature, ) -> ApplyExtrinsicResult { // Verify that the signature is good. - let xt = uxt.check(&Default::default())?; + let xt = uxt.check( + check_signature, + &Default::default(), + )?; // We don't need to make sure to `note_extrinsic` only after we know it's going to be // executed to prevent it from leaking in storage since at this point, it will either @@ -322,7 +337,7 @@ where /// Changes made to storage should be discarded. pub fn validate_transaction(uxt: Block::Extrinsic) -> TransactionValidity { let encoded_len = uxt.using_encoded(|d| d.len()); - let xt = uxt.check(&Default::default())?; + let xt = uxt.check(CheckSignature::Yes, &Default::default())?; let dispatch_info = xt.get_dispatch_info(); xt.validate::(dispatch_info, encoded_len) @@ -516,8 +531,8 @@ mod tests { ) } - fn sign_extra(who: u64, nonce: u64, fee: u64) -> Option<(u64, SignedExtra)> { - Some((who, extra(nonce, fee))) + fn sign_extra(who: u64, nonce: u64, fee: u64) -> (u64, SignedExtra) { + (who, extra(nonce, fee)) } #[test] @@ -526,7 +541,7 @@ mod tests { pallet_balances::GenesisConfig:: { balances: vec![(1, 211)], }.assimilate_storage(&mut t).unwrap(); - let xt = sp_runtime::testing::TestXt(sign_extra(1, 0, 0), Call::Balances(BalancesCall::transfer(2, 69))); + let xt = sp_runtime::testing::TestXt::new_signed(sign_extra(1, 0, 0), Call::Balances(BalancesCall::transfer(2, 69))); let weight = xt.get_dispatch_info().weight as u64; let mut t = sp_io::TestExternalities::new(t); t.execute_with(|| { @@ -606,7 +621,7 @@ mod tests { fn bad_extrinsic_not_inserted() { let mut t = new_test_ext(1); // bad nonce check! - let xt = sp_runtime::testing::TestXt(sign_extra(1, 30, 0), Call::Balances(BalancesCall::transfer(33, 69))); + let xt = sp_runtime::testing::TestXt::new_signed(sign_extra(1, 30, 0), Call::Balances(BalancesCall::transfer(33, 69))); t.execute_with(|| { Executive::initialize_block(&Header::new( 1, @@ -624,7 +639,7 @@ mod tests { fn block_weight_limit_enforced() { let mut t = new_test_ext(10000); // given: TestXt uses the encoded len as fixed Len: - let xt = sp_runtime::testing::TestXt(sign_extra(1, 0, 0), Call::Balances(BalancesCall::transfer(33, 0))); + let xt = sp_runtime::testing::TestXt::new_signed(sign_extra(1, 0, 0), Call::Balances(BalancesCall::transfer(33, 0))); let encoded = xt.encode(); let encoded_len = encoded.len() as Weight; let limit = AvailableBlockRatio::get() * MaximumBlockWeight::get() - 175; @@ -641,7 +656,7 @@ mod tests { assert_eq!(>::all_extrinsics_weight(), 175); for nonce in 0..=num_to_exhaust_block { - let xt = sp_runtime::testing::TestXt( + let xt = sp_runtime::testing::TestXt::new_signed( sign_extra(1, nonce.into(), 0), Call::Balances(BalancesCall::transfer(33, 0)), ); let res = Executive::apply_extrinsic(xt); @@ -661,9 +676,9 @@ mod tests { #[test] fn block_weight_and_size_is_stored_per_tx() { - let xt = sp_runtime::testing::TestXt(sign_extra(1, 0, 0), Call::Balances(BalancesCall::transfer(33, 0))); - let x1 = sp_runtime::testing::TestXt(sign_extra(1, 1, 0), Call::Balances(BalancesCall::transfer(33, 0))); - let x2 = sp_runtime::testing::TestXt(sign_extra(1, 2, 0), Call::Balances(BalancesCall::transfer(33, 0))); + let xt = sp_runtime::testing::TestXt::new_signed(sign_extra(1, 0, 0), Call::Balances(BalancesCall::transfer(33, 0))); + let x1 = sp_runtime::testing::TestXt::new_signed(sign_extra(1, 1, 0), Call::Balances(BalancesCall::transfer(33, 0))); + let x2 = sp_runtime::testing::TestXt::new_signed(sign_extra(1, 2, 0), Call::Balances(BalancesCall::transfer(33, 0))); let len = xt.clone().encode().len() as u32; let mut t = new_test_ext(1); t.execute_with(|| { @@ -687,7 +702,7 @@ mod tests { #[test] fn validate_unsigned() { - let xt = sp_runtime::testing::TestXt(None, Call::Balances(BalancesCall::set_balance(33, 69, 69))); + let xt = sp_runtime::testing::TestXt::new_unsigned(Call::Balances(BalancesCall::set_balance(33, 69, 69))); let mut t = new_test_ext(1); t.execute_with(|| { @@ -696,6 +711,28 @@ mod tests { }); } + #[test] + fn apply_trusted_skips_signature_check_but_not_others() { + let xt1 = sp_runtime::testing::TestXt::new_signed(sign_extra(1, 0, 0), Call::Balances(BalancesCall::transfer(33, 0))) + .badly_signed(); + + let mut t = new_test_ext(1); + + t.execute_with(|| { + assert_eq!(Executive::apply_trusted_extrinsic(xt1), Ok(Ok(()))); + }); + + let xt2 = sp_runtime::testing::TestXt::new_signed(sign_extra(1, 0, 0), Call::Balances(BalancesCall::transfer(33, 0))) + .invalid(TransactionValidityError::Invalid(InvalidTransaction::Call)); + + t.execute_with(|| { + assert_eq!( + Executive::apply_trusted_extrinsic(xt2), + Err(TransactionValidityError::Invalid(InvalidTransaction::Call)) + ); + }); + } + #[test] fn can_pay_for_tx_fee_on_full_lock() { let id: LockIdentifier = *b"0 "; @@ -708,7 +745,7 @@ mod tests { 110, lock, ); - let xt = sp_runtime::testing::TestXt( + let xt = sp_runtime::testing::TestXt::new_signed( sign_extra(1, 0, 0), Call::System(SystemCall::remark(vec![1u8])), ); diff --git a/frame/im-online/src/tests.rs b/frame/im-online/src/tests.rs index 808978d403..80056023e5 100644 --- a/frame/im-online/src/tests.rs +++ b/frame/im-online/src/tests.rs @@ -222,7 +222,7 @@ fn should_generate_heartbeats() { assert_eq!(state.read().transactions.len(), 2); // check stuff about the transaction. let ex: Extrinsic = Decode::decode(&mut &*transaction).unwrap(); - let heartbeat = match ex.1 { + let heartbeat = match ex.call { crate::mock::Call::ImOnline(crate::Call::heartbeat(h, _)) => h, e => panic!("Unexpected call: {:?}", e), }; @@ -332,7 +332,7 @@ fn should_not_send_a_report_if_already_online() { assert_eq!(pool_state.read().transactions.len(), 0); // check stuff about the transaction. let ex: Extrinsic = Decode::decode(&mut &*transaction).unwrap(); - let heartbeat = match ex.1 { + let heartbeat = match ex.call { crate::mock::Call::ImOnline(crate::Call::heartbeat(h, _)) => h, e => panic!("Unexpected call: {:?}", e), }; diff --git a/primitives/block-builder/src/lib.rs b/primitives/block-builder/src/lib.rs index f050979bd8..e4e98e1f40 100644 --- a/primitives/block-builder/src/lib.rs +++ b/primitives/block-builder/src/lib.rs @@ -43,7 +43,7 @@ pub mod compatibility_v3 { sp_api::decl_runtime_apis! { /// The `BlockBuilder` api trait that provides the required functionality for building a block. - #[api_version(4)] + #[api_version(5)] pub trait BlockBuilder { /// Compatibility version of `apply_extrinsic` for v3. /// @@ -58,6 +58,10 @@ sp_api::decl_runtime_apis! { /// Returns an inclusion outcome which specifies if this extrinsic is included in /// this block or not. fn apply_extrinsic(extrinsic: ::Extrinsic) -> ApplyExtrinsicResult; + /// Apply the given extrinsic. + /// + /// Same as `apply_extrinsic`, but skips signature verification. + fn apply_trusted_extrinsic(extrinsic: ::Extrinsic) -> ApplyExtrinsicResult; /// Finish the current block. #[renamed("finalise_block", 3)] fn finalize_block() -> ::Header; diff --git a/primitives/runtime/src/generic/mod.rs b/primitives/runtime/src/generic/mod.rs index 5e9928ba19..f6399fff13 100644 --- a/primitives/runtime/src/generic/mod.rs +++ b/primitives/runtime/src/generic/mod.rs @@ -39,6 +39,15 @@ pub use self::digest::{ use crate::codec::Encode; use sp_std::prelude::*; +/// Perform singature check. +#[derive(PartialEq, Eq, Clone, Copy)] +pub enum CheckSignature { + /// Perform. + Yes, + /// Don't perform. + No, +} + fn encode_with_vec_prefix)>(encoder: F) -> Vec { let size = ::sp_std::mem::size_of::(); let reserve = match size { diff --git a/primitives/runtime/src/generic/unchecked_extrinsic.rs b/primitives/runtime/src/generic/unchecked_extrinsic.rs index a516bc1f7f..0db60e32a6 100644 --- a/primitives/runtime/src/generic/unchecked_extrinsic.rs +++ b/primitives/runtime/src/generic/unchecked_extrinsic.rs @@ -24,7 +24,8 @@ use crate::{ self, Member, MaybeDisplay, SignedExtension, Checkable, Extrinsic, ExtrinsicMetadata, IdentifyAccount, }, - generic::CheckedExtrinsic, transaction_validity::{TransactionValidityError, InvalidTransaction}, + generic::{CheckSignature, CheckedExtrinsic}, + transaction_validity::{TransactionValidityError, InvalidTransaction}, }; const TRANSACTION_VERSION: u8 = 4; @@ -120,18 +121,26 @@ where { type Checked = CheckedExtrinsic; - fn check(self, lookup: &Lookup) -> Result { + fn check(self, check_signature: CheckSignature, lookup: &Lookup) -> Result { Ok(match self.signature { Some((signed, signature, extra)) => { let signed = lookup.lookup(signed)?; - let raw_payload = SignedPayload::new(self.function, extra)?; - if !raw_payload.using_encoded(|payload| { - signature.verify(payload, &signed) - }) { - return Err(InvalidTransaction::BadProof.into()) - } - let (function, extra, _) = raw_payload.deconstruct(); + let (function, extra) = if let CheckSignature::No = check_signature { + (self.function, extra) + } else { + let raw_payload = SignedPayload::new(self.function, extra)?; + + if !raw_payload.using_encoded(|payload| { + signature.verify(payload, &signed) + }) { + return Err(InvalidTransaction::BadProof.into()) + } + let (function, extra, _) = raw_payload.deconstruct(); + + (function, extra) + }; + CheckedExtrinsic { signed: Some((signed, extra)), function, @@ -322,6 +331,7 @@ mod tests { use sp_io::hashing::blake2_256; use crate::codec::{Encode, Decode}; use crate::traits::{SignedExtension, IdentifyAccount, IdentityLookup}; + use crate::generic::CheckSignature; use serde::{Serialize, Deserialize}; type TestContext = IdentityLookup; @@ -402,7 +412,7 @@ mod tests { fn unsigned_check_should_work() { let ux = Ex::new_unsigned(vec![0u8; 0]); assert!(!ux.is_signed().unwrap_or(false)); - assert!(>::check(ux, &Default::default()).is_ok()); + assert!(>::check(ux, CheckSignature::Yes, &Default::default()).is_ok()); } #[test] @@ -415,7 +425,7 @@ mod tests { ); assert!(ux.is_signed().unwrap_or(false)); assert_eq!( - >::check(ux, &Default::default()), + >::check(ux, CheckSignature::Yes, &Default::default()), Err(InvalidTransaction::BadProof.into()), ); } @@ -430,7 +440,7 @@ mod tests { ); assert!(ux.is_signed().unwrap_or(false)); assert_eq!( - >::check(ux, &Default::default()), + >::check(ux, CheckSignature::Yes, &Default::default()), Ok(CEx { signed: Some((TEST_ACCOUNT, TestExtra)), function: vec![0u8; 0] }), ); } diff --git a/primitives/runtime/src/testing.rs b/primitives/runtime/src/testing.rs index e3e94c3c9f..be0e36b2d1 100644 --- a/primitives/runtime/src/testing.rs +++ b/primitives/runtime/src/testing.rs @@ -25,11 +25,10 @@ use crate::traits::{ }; #[allow(deprecated)] use crate::traits::ValidateUnsigned; -use crate::{generic, KeyTypeId, ApplyExtrinsicResult}; +use crate::{generic::{self, CheckSignature}, KeyTypeId, ApplyExtrinsicResult}; pub use sp_core::{H256, sr25519}; use sp_core::{crypto::{CryptoType, Dummy, key_types, Public}, U256}; -use crate::transaction_validity::{TransactionValidity, TransactionValidityError}; - +use crate::transaction_validity::{TransactionValidity, TransactionValidityError, InvalidTransaction}; /// Authority Id #[derive(Default, PartialEq, Eq, Clone, Encode, Decode, Debug, Hash, Serialize, Deserialize, PartialOrd, Ord)] pub struct UintAuthorityId(pub u64); @@ -295,12 +294,69 @@ impl<'a, Xt> Deserialize<'a> for Block where Block: Decode { } } -/// Test transaction, tuple of (sender, call, signed_extra) -/// with index only used if sender is some. +/// Test validity. +#[derive(PartialEq, Eq, Clone, Encode, Decode)] +pub enum TestValidity { + /// Valid variant that will pass all checks. + Valid, + /// Variant with invalid signature. + /// + /// Will fail signature check. + SignatureInvalid(TransactionValidityError), + /// Variant with invalid logic. + /// + /// Will fail all checks. + OtherInvalid(TransactionValidityError), +} + +/// Test transaction. /// -/// If sender is some then the transaction is signed otherwise it is unsigned. +/// Used to mock actual transaction. #[derive(PartialEq, Eq, Clone, Encode, Decode)] -pub struct TestXt(pub Option<(u64, Extra)>, pub Call); +pub struct TestXt { + /// Signature with extra. + /// + /// if some, then the transaction is signed. Transaction is unsigned otherwise. + pub signature: Option<(u64, Extra)>, + /// Validity. + /// + /// Instantiate invalid variant and transaction will fail correpsonding checks. + pub validity: TestValidity, + /// Call. + pub call: Call, +} + +impl TestXt { + /// New signed test `TextXt`. + pub fn new_signed(signature: (u64, Extra), call: Call) -> Self { + TestXt { + signature: Some(signature), + validity: TestValidity::Valid, + call, + } + } + + /// New unsigned test `TextXt`. + pub fn new_unsigned(call: Call) -> Self { + TestXt { + signature: None, + validity: TestValidity::Valid, + call, + } + } + + /// Build invalid variant of `TestXt`. + pub fn invalid(mut self, err: TransactionValidityError) -> Self { + self.validity = TestValidity::OtherInvalid(err); + self + } + + /// Build badly signed variant of `TestXt`. + pub fn badly_signed(mut self) -> Self { + self.validity = TestValidity::SignatureInvalid(TransactionValidityError::Invalid(InvalidTransaction::BadProof)); + self + } + } // Non-opaque extrinsics always 0. parity_util_mem::malloc_size_of_is_0!(any: TestXt); @@ -313,24 +369,39 @@ impl Serialize for TestXt where TestXt: E impl Debug for TestXt { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "TestXt({:?}, ...)", self.0.as_ref().map(|x| &x.0)) + write!(f, "TestXt({:?}, {}, ...)", + self.signature.as_ref().map(|x| &x.0), + if let TestValidity::Valid = self.validity { "valid" } else { "invalid" } + ) } } impl Checkable for TestXt { type Checked = Self; - fn check(self, _: &Context) -> Result { Ok(self) } + fn check(self, signature: CheckSignature, _: &Context) -> Result { + match self.validity { + TestValidity::Valid => Ok(self), + TestValidity::SignatureInvalid(e) => + if let CheckSignature::No = signature { + Ok(self) + } else { + Err(e) + }, + TestValidity::OtherInvalid(e) => Err(e), + } + } } + impl traits::Extrinsic for TestXt { type Call = Call; type SignaturePayload = (u64, Extra); fn is_signed(&self) -> Option { - Some(self.0.is_some()) + Some(self.signature.is_some()) } - fn new(c: Call, sig: Option) -> Option { - Some(TestXt(sig, c)) + fn new(call: Call, signature: Option) -> Option { + Some(TestXt { signature, call, validity: TestValidity::Valid }) } } @@ -344,7 +415,7 @@ impl Applyable for TestXt where type Call = Call; type DispatchInfo = Info; - fn sender(&self) -> Option<&Self::AccountId> { self.0.as_ref().map(|x| &x.0) } + fn sender(&self) -> Option<&Self::AccountId> { self.signature.as_ref().map(|x| &x.0) } /// Checks to see if this is a valid *transaction*. It returns information on it if so. #[allow(deprecated)] // Allow ValidateUnsigned @@ -364,14 +435,14 @@ impl Applyable for TestXt where info: Self::DispatchInfo, len: usize, ) -> ApplyExtrinsicResult { - let maybe_who = if let Some((who, extra)) = self.0 { - Extra::pre_dispatch(extra, &who, &self.1, info, len)?; + let maybe_who = if let Some((who, extra)) = self.signature { + Extra::pre_dispatch(extra, &who, &self.call, info, len)?; Some(who) } else { - Extra::pre_dispatch_unsigned(&self.1, info, len)?; + Extra::pre_dispatch_unsigned(&self.call, info, len)?; None }; - Ok(self.1.dispatch(maybe_who.into()).map_err(Into::into)) + Ok(self.call.dispatch(maybe_who.into()).map_err(Into::into)) } } diff --git a/primitives/runtime/src/traits.rs b/primitives/runtime/src/traits.rs index 1b9b9eec70..183df08ab8 100644 --- a/primitives/runtime/src/traits.rs +++ b/primitives/runtime/src/traits.rs @@ -31,7 +31,7 @@ use crate::codec::{Codec, Encode, Decode}; use crate::transaction_validity::{ ValidTransaction, TransactionValidity, TransactionValidityError, UnknownTransaction, }; -use crate::generic::{Digest, DigestItem}; +use crate::generic::{Digest, DigestItem, CheckSignature}; pub use sp_arithmetic::traits::{ AtLeast32Bit, UniqueSaturatedInto, UniqueSaturatedFrom, Saturating, SaturatedConversion, Zero, One, Bounded, CheckedAdd, CheckedSub, CheckedMul, CheckedDiv, @@ -637,7 +637,7 @@ pub trait Checkable: Sized { type Checked; /// Check self, given an instance of Context. - fn check(self, c: &Context) -> Result; + fn check(self, signature: CheckSignature, c: &Context) -> Result; } /// A "checkable" piece of information, used by the standard Substrate Executive in order to @@ -649,15 +649,15 @@ pub trait BlindCheckable: Sized { type Checked; /// Check self. - fn check(self) -> Result; + fn check(self, signature: CheckSignature) -> Result; } // Every `BlindCheckable` is also a `StaticCheckable` for arbitrary `Context`. impl Checkable for T { type Checked = ::Checked; - fn check(self, _c: &Context) -> Result { - BlindCheckable::check(self) + fn check(self, signature: CheckSignature, _c: &Context) -> Result { + BlindCheckable::check(self, signature) } } diff --git a/test-utils/runtime/src/lib.rs b/test-utils/runtime/src/lib.rs index 2505bdde22..138e79cdd5 100644 --- a/test-utils/runtime/src/lib.rs +++ b/test-utils/runtime/src/lib.rs @@ -41,6 +41,7 @@ use sp_runtime::{ BlindCheckable, BlakeTwo256, Block as BlockT, Extrinsic as ExtrinsicT, GetNodeBlockType, GetRuntimeBlockType, Verify, IdentityLookup, }, + generic::CheckSignature, }; use sp_version::RuntimeVersion; pub use sp_core::{hash::H256}; @@ -126,7 +127,7 @@ impl serde::Serialize for Extrinsic { impl BlindCheckable for Extrinsic { type Checked = Self; - fn check(self) -> Result { + fn check(self, _signature: CheckSignature) -> Result { match self { Extrinsic::AuthoritiesChange(new_auth) => Ok(Extrinsic::AuthoritiesChange(new_auth)), Extrinsic::Transfer(transfer, signature) => { @@ -493,6 +494,10 @@ cfg_if! { system::execute_transaction(extrinsic) } + fn apply_trusted_extrinsic(extrinsic: ::Extrinsic) -> ApplyExtrinsicResult { + system::execute_transaction(extrinsic) + } + fn finalize_block() -> ::Header { system::finalize_block() } @@ -680,6 +685,10 @@ cfg_if! { system::execute_transaction(extrinsic) } + fn apply_trusted_extrinsic(extrinsic: ::Extrinsic) -> ApplyExtrinsicResult { + system::execute_transaction(extrinsic) + } + fn finalize_block() -> ::Header { system::finalize_block() } diff --git a/test-utils/runtime/src/system.rs b/test-utils/runtime/src/system.rs index d0a38c7c77..b410d317a1 100644 --- a/test-utils/runtime/src/system.rs +++ b/test-utils/runtime/src/system.rs @@ -29,6 +29,7 @@ use sp_runtime::{ transaction_validity::{ TransactionValidity, ValidTransaction, InvalidTransaction, TransactionValidityError, }, + generic::CheckSignature, }; use codec::{KeyedVec, Encode, Decode}; use frame_system::Trait; @@ -243,7 +244,7 @@ pub fn finalize_block() -> Header { #[inline(always)] fn check_signature(utx: &Extrinsic) -> Result<(), TransactionValidityError> { use sp_runtime::traits::BlindCheckable; - utx.clone().check().map_err(|_| InvalidTransaction::BadProof.into()).map(|_| ()) + utx.clone().check(CheckSignature::Yes).map_err(|_| InvalidTransaction::BadProof.into()).map(|_| ()) } fn execute_transaction_backend(utx: &Extrinsic) -> ApplyExtrinsicResult { -- GitLab From c077a2b5e16c8dc331005871deb9566274a3ffc4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Wed, 19 Feb 2020 10:22:36 +0100 Subject: [PATCH 005/438] Consolidate frame benchmarking into a frame crate (#4977) This prs cleans up some of the frame benchmarking stuff: - Move CLI into `frame-benchmarking-cli`. No frame related CLI should exists in the default Substrate CLI. - Move all traits and types related to frame benchmarking into the `frame-benchmarking` trait. Frame types should be isolated in Frame. --- Cargo.lock | 31 +++++ Cargo.toml | 2 + bin/node/cli/Cargo.toml | 3 + bin/node/cli/src/cli.rs | 7 ++ bin/node/cli/src/command.rs | 7 +- bin/node/executor/Cargo.toml | 1 + bin/node/executor/src/lib.rs | 2 +- bin/node/runtime/Cargo.toml | 2 + bin/node/runtime/src/lib.rs | 28 ++--- client/cli/src/lib.rs | 2 +- client/cli/src/params.rs | 118 ++++-------------- client/service/src/chain_ops.rs | 72 ++--------- frame/balances/Cargo.toml | 2 + frame/balances/src/benchmarking.rs | 18 +-- frame/benchmarking/Cargo.toml | 16 +++ frame/benchmarking/src/lib.rs | 141 ++++++++++++++++++++++ frame/identity/Cargo.toml | 2 + frame/identity/src/benchmarking.rs | 59 ++++----- frame/timestamp/Cargo.toml | 2 + frame/timestamp/src/benchmarking.rs | 21 ++-- primitives/io/src/lib.rs | 24 ---- primitives/runtime/src/lib.rs | 12 -- primitives/runtime/src/traits.rs | 70 ----------- utils/frame/benchmarking-cli/Cargo.toml | 17 +++ utils/frame/benchmarking-cli/src/lib.rs | 152 ++++++++++++++++++++++++ 25 files changed, 483 insertions(+), 328 deletions(-) create mode 100644 frame/benchmarking/Cargo.toml create mode 100644 frame/benchmarking/src/lib.rs create mode 100644 utils/frame/benchmarking-cli/Cargo.toml create mode 100644 utils/frame/benchmarking-cli/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index b557048f9d..f518377d62 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1441,6 +1441,31 @@ dependencies = [ "parity-scale-codec", ] +[[package]] +name = "frame-benchmarking" +version = "2.0.0" +dependencies = [ + "parity-scale-codec", + "sp-api", + "sp-runtime-interface", + "sp-std", +] + +[[package]] +name = "frame-benchmarking-cli" +version = "2.0.0" +dependencies = [ + "frame-benchmarking", + "parity-scale-codec", + "sc-cli", + "sc-client", + "sc-client-db", + "sc-executor", + "sc-service", + "sp-runtime", + "structopt", +] + [[package]] name = "frame-executive" version = "2.0.0" @@ -3369,6 +3394,7 @@ version = "2.0.0" dependencies = [ "assert_cmd", "browser-utils", + "frame-benchmarking-cli", "frame-support", "frame-system", "futures 0.3.4", @@ -3437,6 +3463,7 @@ name = "node-executor" version = "2.0.0" dependencies = [ "criterion 0.3.1", + "frame-benchmarking", "frame-support", "frame-system", "node-primitives", @@ -3530,6 +3557,7 @@ dependencies = [ name = "node-runtime" version = "2.0.0" dependencies = [ + "frame-benchmarking", "frame-executive", "frame-support", "frame-system", @@ -3966,6 +3994,7 @@ dependencies = [ name = "pallet-balances" version = "2.0.0" dependencies = [ + "frame-benchmarking", "frame-support", "frame-system", "pallet-transaction-payment", @@ -4197,6 +4226,7 @@ name = "pallet-identity" version = "2.0.0" dependencies = [ "enumflags2", + "frame-benchmarking", "frame-support", "frame-system", "pallet-balances", @@ -4433,6 +4463,7 @@ dependencies = [ name = "pallet-timestamp" version = "2.0.0" dependencies = [ + "frame-benchmarking", "frame-support", "frame-system", "impl-trait-for-tuples", diff --git a/Cargo.toml b/Cargo.toml index a42a8e24d0..2dc0c8926c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -62,6 +62,7 @@ members = [ "frame/authorship", "frame/babe", "frame/balances", + "frame/benchmarking", "frame/collective", "frame/contracts", "frame/contracts/rpc", @@ -156,6 +157,7 @@ members = [ "utils/browser", "utils/build-script-utils", "utils/fork-tree", + "utils/frame/benchmarking-cli", "utils/frame/rpc/support", "utils/frame/rpc/system", "utils/wasm-builder", diff --git a/bin/node/cli/Cargo.toml b/bin/node/cli/Cargo.toml index 079e8d13e2..d383e2c05a 100644 --- a/bin/node/cli/Cargo.toml +++ b/bin/node/cli/Cargo.toml @@ -89,6 +89,7 @@ node-executor = { version = "2.0.0", path = "../executor" } # CLI-specific dependencies sc-cli = { version = "0.8.0", optional = true, path = "../../../client/cli" } +frame-benchmarking-cli = { version = "2.0.0", optional = true, path = "../../../utils/frame/benchmarking-cli" } node-transaction-factory = { version = "0.8.0", optional = true, path = "../transaction-factory" } node-inspect = { version = "0.8.0", optional = true, path = "../inspect" } @@ -112,6 +113,7 @@ build-script-utils = { version = "2.0.0", package = "substrate-build-script-util structopt = { version = "0.3.8", optional = true } node-transaction-factory = { version = "0.8.0", optional = true, path = "../transaction-factory" } node-inspect = { version = "0.8.0", optional = true, path = "../inspect" } +frame-benchmarking-cli = { version = "2.0.0", optional = true, path = "../../../utils/frame/benchmarking-cli" } [build-dependencies.sc-cli] version = "0.8.0" @@ -135,6 +137,7 @@ cli = [ "node-inspect", "node-transaction-factory", "sc-cli", + "frame-benchmarking-cli", "sc-service/rocksdb", "structopt", "vergen", diff --git a/bin/node/cli/src/cli.rs b/bin/node/cli/src/cli.rs index 7dcd02699d..40f1dcf6f4 100644 --- a/bin/node/cli/src/cli.rs +++ b/bin/node/cli/src/cli.rs @@ -53,6 +53,13 @@ pub enum Subcommand { about = "Decode given block or extrinsic using current native runtime." )] Inspect(node_inspect::cli::InspectCmd), + + /// The custom benchmark subcommmand benchmarking runtime pallets. + #[structopt( + name = "benchmark", + about = "Benchmark runtime pallets." + )] + Benchmark(frame_benchmarking_cli::BenchmarkCmd), } /// The `factory` command used to generate transactions. diff --git a/bin/node/cli/src/command.rs b/bin/node/cli/src/command.rs index ba0f2785c1..5a942d964c 100644 --- a/bin/node/cli/src/command.rs +++ b/bin/node/cli/src/command.rs @@ -43,12 +43,17 @@ where cmd.init(&mut config, load_spec, &version)?; let client = sc_service::new_full_client::< - node_runtime::Block,node_runtime::RuntimeApi, node_executor::Executor, _, _, + node_runtime::Block, node_runtime::RuntimeApi, node_executor::Executor, _, _, >(&config)?; let inspect = node_inspect::Inspector::::new(client); cmd.run(inspect) }, + Some(Subcommand::Benchmark(cmd)) => { + cmd.init(&mut config, load_spec, &version)?; + + cmd.run::<_, _, node_runtime::Block, node_executor::Executor>(config) + }, Some(Subcommand::Factory(cli_args)) => { sc_cli::init(&cli_args.shared_params, &version)?; sc_cli::init_config(&mut config, &cli_args.shared_params, &version, load_spec)?; diff --git a/bin/node/executor/Cargo.toml b/bin/node/executor/Cargo.toml index 1d894e39fa..f55e1dae58 100644 --- a/bin/node/executor/Cargo.toml +++ b/bin/node/executor/Cargo.toml @@ -16,6 +16,7 @@ sp-io = { version = "2.0.0", path = "../../../primitives/io" } sp-state-machine = { version = "0.8", path = "../../../primitives/state-machine" } sp-trie = { version = "2.0.0", path = "../../../primitives/trie" } trie-root = "0.16.0" +frame-benchmarking = { version = "2.0.0", path = "../../../frame/benchmarking" } [dev-dependencies] criterion = "0.3.0" diff --git a/bin/node/executor/src/lib.rs b/bin/node/executor/src/lib.rs index 72f40b7c1f..bcc7f48507 100644 --- a/bin/node/executor/src/lib.rs +++ b/bin/node/executor/src/lib.rs @@ -26,5 +26,5 @@ native_executor_instance!( pub Executor, node_runtime::api::dispatch, node_runtime::native_version, - sp_io::benchmarking::HostFunctions, + frame_benchmarking::benchmarking::HostFunctions, ); diff --git a/bin/node/runtime/Cargo.toml b/bin/node/runtime/Cargo.toml index 3f8e8b6731..8156e4d444 100644 --- a/bin/node/runtime/Cargo.toml +++ b/bin/node/runtime/Cargo.toml @@ -33,6 +33,7 @@ sp-version = { version = "2.0.0", default-features = false, path = "../../../pri # frame dependencies frame-executive = { version = "2.0.0", default-features = false, path = "../../../frame/executive" } +frame-benchmarking = { version = "2.0.0", default-features = false, path = "../../../frame/benchmarking" } frame-support = { version = "2.0.0", default-features = false, path = "../../../frame/support" } frame-system = { version = "2.0.0", default-features = false, path = "../../../frame/system" } frame-system-rpc-runtime-api = { version = "2.0.0", default-features = false, path = "../../../frame/system/rpc/runtime-api/" } @@ -115,6 +116,7 @@ std = [ "sp-session/std", "pallet-sudo/std", "frame-support/std", + "frame-benchmarking/std", "frame-system-rpc-runtime-api/std", "frame-system/std", "pallet-timestamp/std", diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 956fc0bb0e..b60056e045 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -31,14 +31,13 @@ pub use node_primitives::{AccountId, Signature}; use node_primitives::{AccountIndex, Balance, BlockNumber, Hash, Index, Moment}; use sp_api::impl_runtime_apis; use sp_runtime::{ - Permill, Perbill, Percent, ApplyExtrinsicResult, BenchmarkResults, - impl_opaque_keys, generic, create_runtime_str, + Permill, Perbill, Percent, ApplyExtrinsicResult, impl_opaque_keys, generic, create_runtime_str, }; use sp_runtime::curve::PiecewiseLinear; use sp_runtime::transaction_validity::TransactionValidity; use sp_runtime::traits::{ self, BlakeTwo256, Block as BlockT, StaticLookup, SaturatedConversion, - ConvertInto, OpaqueKeys, Benchmarking, + ConvertInto, OpaqueKeys, }; use sp_version::RuntimeVersion; #[cfg(any(feature = "std", test))] @@ -816,28 +815,25 @@ impl_runtime_apis! { } } - impl crate::Benchmark for Runtime { - fn dispatch_benchmark(module: Vec, extrinsic: Vec, steps: u32, repeat: u32) - -> Option> - { + impl frame_benchmarking::Benchmark for Runtime { + fn dispatch_benchmark( + module: Vec, + extrinsic: Vec, + steps: u32, + repeat: u32, + ) -> Option> { + use frame_benchmarking::Benchmarking; + match module.as_slice() { b"pallet-balances" | b"balances" => Balances::run_benchmark(extrinsic, steps, repeat).ok(), b"pallet-identity" | b"identity" => Identity::run_benchmark(extrinsic, steps, repeat).ok(), b"pallet-timestamp" | b"timestamp" => Timestamp::run_benchmark(extrinsic, steps, repeat).ok(), - _ => return None, + _ => None, } } } } -sp_api::decl_runtime_apis! { - pub trait Benchmark - { - fn dispatch_benchmark(module: Vec, extrinsic: Vec, steps: u32, repeat: u32) - -> Option>; - } -} - #[cfg(test)] mod tests { use super::*; diff --git a/client/cli/src/lib.rs b/client/cli/src/lib.rs index 7495ad8e75..6259c0a21b 100644 --- a/client/cli/src/lib.rs +++ b/client/cli/src/lib.rs @@ -57,7 +57,7 @@ use params::{ pub use params::{ SharedParams, ImportParams, ExecutionStrategy, Subcommand, RunCmd, BuildSpecCmd, ExportBlocksCmd, ImportBlocksCmd, CheckBlockCmd, PurgeChainCmd, RevertCmd, - BenchmarkCmd, + WasmExecutionMethod, }; pub use traits::GetSharedParams; use app_dirs::{AppInfo, AppDataType}; diff --git a/client/cli/src/params.rs b/client/cli/src/params.rs index d6c437f668..aaa46c0f63 100644 --- a/client/cli/src/params.rs +++ b/client/cli/src/params.rs @@ -47,29 +47,35 @@ impl Into for ExecutionStrategy { } } -arg_enum! { - /// How to execute Wasm runtime code - #[allow(missing_docs)] - #[derive(Debug, Clone, Copy)] - pub enum WasmExecutionMethod { - // Uses an interpreter. - Interpreted, - // Uses a compiled runtime. - Compiled, +#[allow(missing_docs)] +mod wasm_execution_method { + use super::*; + + arg_enum! { + /// How to execute Wasm runtime code + #[derive(Debug, Clone, Copy)] + pub enum WasmExecutionMethod { + // Uses an interpreter. + Interpreted, + // Uses a compiled runtime. + Compiled, + } } -} -impl WasmExecutionMethod { - /// Returns list of variants that are not disabled by feature flags. - fn enabled_variants() -> Vec<&'static str> { - Self::variants() - .iter() - .cloned() - .filter(|&name| cfg!(feature = "wasmtime") || name != "Compiled") - .collect() + impl WasmExecutionMethod { + /// Returns list of variants that are not disabled by feature flags. + pub fn enabled_variants() -> Vec<&'static str> { + Self::variants() + .iter() + .cloned() + .filter(|&name| cfg!(feature = "wasmtime") || name != "Compiled") + .collect() + } } } +pub use wasm_execution_method::WasmExecutionMethod; + impl Into for WasmExecutionMethod { fn into(self) -> sc_service::config::WasmExecutionMethod { match self { @@ -849,49 +855,6 @@ pub struct PurgeChainCmd { pub shared_params: SharedParams, } -/// The `benchmark` command used to benchmark FRAME Pallets. -#[derive(Debug, StructOpt, Clone)] -pub struct BenchmarkCmd { - /// Select a FRAME Pallet to benchmark. - #[structopt(short, long)] - pub pallet: String, - - /// Select an extrinsic to benchmark. - #[structopt(short, long)] - pub extrinsic: String, - - /// Select how many samples we should take across the variable components. - #[structopt(short, long, default_value = "1")] - pub steps: u32, - - /// Select how many repetitions of this benchmark should run. - #[structopt(short, long, default_value = "1")] - pub repeat: u32, - - #[allow(missing_docs)] - #[structopt(flatten)] - pub shared_params: SharedParams, - - /// The execution strategy that should be used for benchmarks - #[structopt( - long = "execution", - value_name = "STRATEGY", - possible_values = &ExecutionStrategy::variants(), - case_insensitive = true, - )] - pub execution: Option, - - /// Method for executing Wasm runtime code. - #[structopt( - long = "wasm-execution", - value_name = "METHOD", - possible_values = &WasmExecutionMethod::enabled_variants(), - case_insensitive = true, - default_value = "Interpreted" - )] - pub wasm_method: WasmExecutionMethod, -} - /// All core commands that are provided by default. /// /// The core commands are split into multiple subcommands and `Run` is the default subcommand. From @@ -916,9 +879,6 @@ pub enum Subcommand { /// Remove the whole chain data. PurgeChain(PurgeChainCmd), - - /// Run runtime benchmarks. - Benchmark(BenchmarkCmd), } impl Subcommand { @@ -933,7 +893,6 @@ impl Subcommand { CheckBlock(params) => ¶ms.shared_params, Revert(params) => ¶ms.shared_params, PurgeChain(params) => ¶ms.shared_params, - Benchmark(params) => ¶ms.shared_params, } } @@ -960,7 +919,6 @@ impl Subcommand { Subcommand::ImportBlocks(cmd) => cmd.run(config, builder), Subcommand::CheckBlock(cmd) => cmd.run(config, builder), Subcommand::PurgeChain(cmd) => cmd.run(config), - Subcommand::Benchmark(cmd) => cmd.run(config, builder), Subcommand::Revert(cmd) => cmd.run(config, builder), } } @@ -1238,31 +1196,3 @@ impl RevertCmd { Ok(()) } } - -impl BenchmarkCmd { - /// Runs the command and benchmarks the chain. - pub fn run( - self, - config: Configuration, - _builder: B, - ) -> error::Result<()> - where - B: FnOnce(Configuration) -> Result, - G: RuntimeGenesis, - E: ChainSpecExtension, - BC: ServiceBuilderCommand + Unpin, - BB: sp_runtime::traits::Block + Debug, - <<::Header as HeaderT>::Number as std::str::FromStr>::Err: std::fmt::Debug, - ::Hash: std::str::FromStr, - { - let spec = config.chain_spec.expect("chain_spec is always Some"); - let execution_strategy = self.execution.unwrap_or(ExecutionStrategy::Native).into(); - let wasm_method = self.wasm_method.into(); - let pallet = self.pallet; - let extrinsic = self.extrinsic; - let steps = self.steps; - let repeat = self.repeat; - sc_service::chain_ops::benchmark_runtime::(spec, execution_strategy, wasm_method, pallet, extrinsic, steps, repeat)?; - Ok(()) - } -} diff --git a/client/service/src/chain_ops.rs b/client/service/src/chain_ops.rs index 3d77d9c815..30987170f3 100644 --- a/client/service/src/chain_ops.rs +++ b/client/service/src/chain_ops.rs @@ -22,20 +22,17 @@ use crate::error::Error; use sc_chain_spec::{ChainSpec, RuntimeGenesis, Extension}; use log::{warn, info}; use futures::{future, prelude::*}; -use sp_runtime::{ - BuildStorage, BenchmarkResults, - traits::{ - Block as BlockT, NumberFor, One, Zero, Header, SaturatedConversion - } +use sp_runtime::traits::{ + Block as BlockT, NumberFor, One, Zero, Header, SaturatedConversion }; use sp_runtime::generic::{BlockId, SignedBlock}; use codec::{Decode, Encode, IoReader}; -use sc_client::{Client, ExecutionStrategy, StateMachine, LocalCallExecutor}; -#[cfg(feature = "rocksdb")] -use sc_client_db::BenchmarkingState; -use sp_consensus::import_queue::{IncomingBlock, Link, BlockImportError, BlockImportResult, ImportQueue}; -use sp_consensus::BlockOrigin; -use sc_executor::{NativeExecutor, NativeExecutionDispatch, WasmExecutionMethod}; +use sc_client::{Client, LocalCallExecutor}; +use sp_consensus::{ + BlockOrigin, + import_queue::{IncomingBlock, Link, BlockImportError, BlockImportResult, ImportQueue}, +}; +use sc_executor::{NativeExecutor, NativeExecutionDispatch}; use std::{io::{Read, Write, Seek}, pin::Pin}; @@ -49,59 +46,6 @@ pub fn build_spec(spec: ChainSpec, raw: bool) -> error::Result ( - spec: ChainSpec, - strategy: ExecutionStrategy, - wasm_method: WasmExecutionMethod, - pallet: String, - extrinsic: String, - steps: u32, - repeat: u32, -) -> error::Result<()> where - TBl: BlockT, - TExecDisp: NativeExecutionDispatch + 'static, - G: RuntimeGenesis, - E: Extension, -{ - let genesis_storage = spec.build_storage()?; - let mut changes = Default::default(); - let state = BenchmarkingState::::new(genesis_storage)?; - let executor = NativeExecutor::::new( - wasm_method, - None, // heap pages - ); - let result = StateMachine::<_, _, NumberFor, _>::new( - &state, - None, - &mut changes, - &executor, - "Benchmark_dispatch_benchmark", - &(&pallet, &extrinsic, steps, repeat).encode(), - Default::default(), - ).execute(strategy).map_err(|e| format!("Error executing runtime benchmark: {:?}", e))?; - let results = > as Decode>::decode(&mut &result[..]).unwrap_or(None); - if let Some(results) = results { - // Print benchmark metadata - println!("Pallet: {:?}, Extrinsic: {:?}, Steps: {:?}, Repeat: {:?}", pallet, extrinsic, steps, repeat); - // Print the table header - results[0].0.iter().for_each(|param| print!("{:?},", param.0)); - print!("time\n"); - // Print the values - results.iter().for_each(|result| { - let parameters = &result.0; - parameters.iter().for_each(|param| print!("{:?},", param.1)); - print!("{:?}\n", result.1); - }); - info!("Done."); - } else { - info!("No Results."); - } - Ok(()) -} - - impl< TBl, TRtApi, TGen, TCSExt, TBackend, TExecDisp, TFchr, TSc, TImpQu, TFprb, TFpp, TNetP, diff --git a/frame/balances/Cargo.toml b/frame/balances/Cargo.toml index 871290b182..f2bf1069af 100644 --- a/frame/balances/Cargo.toml +++ b/frame/balances/Cargo.toml @@ -11,6 +11,7 @@ codec = { package = "parity-scale-codec", version = "1.0.0", default-features = sp-std = { version = "2.0.0", default-features = false, path = "../../primitives/std" } sp-io = { version = "2.0.0", default-features = false, path = "../../primitives/io" } sp-runtime = { version = "2.0.0", default-features = false, path = "../../primitives/runtime" } +frame-benchmarking = { version = "2.0.0", default-features = false, path = "../benchmarking" } frame-support = { version = "2.0.0", default-features = false, path = "../support" } frame-system = { version = "2.0.0", default-features = false, path = "../system" } @@ -27,6 +28,7 @@ std = [ "sp-std/std", "sp-io/std", "sp-runtime/std", + "frame-benchmarking/std", "frame-support/std", "frame-system/std", ] diff --git a/frame/balances/src/benchmarking.rs b/frame/balances/src/benchmarking.rs index 605561d597..e564824aaa 100644 --- a/frame/balances/src/benchmarking.rs +++ b/frame/balances/src/benchmarking.rs @@ -20,8 +20,10 @@ use super::*; use frame_system::RawOrigin; use sp_io::hashing::blake2_256; -use sp_runtime::{BenchmarkResults, BenchmarkParameter}; -use sp_runtime::traits::{Bounded, Benchmarking, BenchmarkingSetup, Dispatchable}; +use frame_benchmarking::{ + BenchmarkResults, BenchmarkParameter, Benchmarking, BenchmarkingSetup, benchmarking, +}; +use sp_runtime::traits::{Bounded, Dispatchable}; use crate::Module as Balances; @@ -271,8 +273,8 @@ impl Benchmarking for Module { }; // Warm up the DB - sp_io::benchmarking::commit_db(); - sp_io::benchmarking::wipe_db(); + benchmarking::commit_db(); + benchmarking::wipe_db(); let components = , RawOrigin>>::components(&selected_benchmark); // results go here @@ -298,11 +300,11 @@ impl Benchmarking for Module { let (call, caller) = , RawOrigin>>::instance(&selected_benchmark, &c)?; // Commit the externalities to the database, flushing the DB cache. // This will enable worst case scenario for reading from the database. - sp_io::benchmarking::commit_db(); + benchmarking::commit_db(); // Run the benchmark. - let start = sp_io::benchmarking::current_time(); + let start = benchmarking::current_time(); call.dispatch(caller.clone().into())?; - let finish = sp_io::benchmarking::current_time(); + let finish = benchmarking::current_time(); let elapsed = finish - start; sp_std::if_std!{ if let RawOrigin::Signed(who) = caller.clone() { @@ -312,7 +314,7 @@ impl Benchmarking for Module { } results.push((c.clone(), elapsed)); // Wipe the DB back to the genesis state. - sp_io::benchmarking::wipe_db(); + benchmarking::wipe_db(); } } } diff --git a/frame/benchmarking/Cargo.toml b/frame/benchmarking/Cargo.toml new file mode 100644 index 0000000000..c2748e080d --- /dev/null +++ b/frame/benchmarking/Cargo.toml @@ -0,0 +1,16 @@ +[package] +name = "frame-benchmarking" +version = "2.0.0" +authors = ["Parity Technologies "] +edition = "2018" +license = "GPL-3.0" + +[dependencies] +codec = { package = "parity-scale-codec", version = "1.1.2", default-features = false } +sp-api = { version = "2.0.0", path = "../../primitives/api", default-features = false } +sp-runtime-interface = { version = "2.0.0", path = "../../primitives/runtime-interface", default-features = false } +sp-std = { version = "2.0.0", path = "../../primitives/std", default-features = false } + +[features] +default = [ "std" ] +std = [ "sp-runtime-interface/std", "sp-api/std", "codec/std", "sp-std/std" ] diff --git a/frame/benchmarking/src/lib.rs b/frame/benchmarking/src/lib.rs new file mode 100644 index 0000000000..c57cfb4914 --- /dev/null +++ b/frame/benchmarking/src/lib.rs @@ -0,0 +1,141 @@ +// Copyright 2020 Parity Technologies (UK) Ltd. +// This file is part of Substrate. + +// Substrate is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Substrate is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Substrate. If not, see . + +//! Interfaces and types for benchmarking a FRAME runtime. + +#![cfg_attr(not(feature = "std"), no_std)] + +use sp_std::vec::Vec; + +/// An alphabet of possible parameters to use for benchmarking. +#[derive(codec::Encode, codec::Decode, Clone, Copy, PartialEq, Debug)] +#[allow(missing_docs)] +pub enum BenchmarkParameter { + A, B, C, D, E, F, G, H, I, J, K, L, M, N, O, P, Q, R, S, T, U, V, W, X, Y, Z, +} + +/// Results from running benchmarks on a FRAME pallet. +/// Contains duration of the function call in nanoseconds along with the benchmark parameters +/// used for that benchmark result. +pub type BenchmarkResults = (Vec<(BenchmarkParameter, u32)>, u128); + +sp_api::decl_runtime_apis! { + /// Runtime api for benchmarking a FRAME runtime. + pub trait Benchmark { + /// Dispatch the given benchmark. + fn dispatch_benchmark( + module: Vec, + extrinsic: Vec, + steps: u32, + repeat: u32, + ) -> Option>; + } +} + +/// Interface that provides functions for benchmarking the runtime. +#[sp_runtime_interface::runtime_interface] +pub trait Benchmarking { + /// Get the number of nanoseconds passed since the UNIX epoch + /// + /// WARNING! This is a non-deterministic call. Do not use this within + /// consensus critical logic. + fn current_time() -> u128 { + std::time::SystemTime::now().duration_since(std::time::SystemTime::UNIX_EPOCH) + .expect("Unix time doesn't go backwards; qed") + .as_nanos() + } + + /// Reset the trie database to the genesis state. + fn wipe_db(&mut self) { + self.wipe() + } + + /// Commit pending storage changes to the trie database and clear the database cache. + fn commit_db(&mut self) { + self.commit() + } +} + +/// The pallet benchmarking trait. +pub trait Benchmarking { + /// Run the benchmarks for this pallet. + /// + /// Parameters + /// - `extrinsic`: The name of extrinsic function you want to benchmark encoded as bytes. + /// - `steps`: The number of sample points you want to take across the range of parameters. + /// - `repeat`: The number of times you want to repeat a benchmark. + fn run_benchmark(extrinsic: Vec, steps: u32, repeat: u32) -> Result, &'static str>; +} + +/// The required setup for creating a benchmark. +pub trait BenchmarkingSetup { + /// Return the components and their ranges which should be tested in this benchmark. + fn components(&self) -> Vec<(BenchmarkParameter, u32, u32)>; + + /// Set up the storage, and prepare a call and caller to test in a single run of the benchmark. + fn instance(&self, components: &[(BenchmarkParameter, u32)]) -> Result<(Call, RawOrigin), &'static str>; +} + +/// Creates a `SelectedBenchmark` enum implementing `BenchmarkingSetup`. +/// +/// Every variant must implement [`BenchmarkingSetup`]. +/// +/// ```nocompile +/// +/// struct Transfer; +/// impl BenchmarkingSetup for Transfer { ... } +/// +/// struct SetBalance; +/// impl BenchmarkingSetup for SetBalance { ... } +/// +/// selected_benchmark!(Transfer, SetBalance); +/// ``` +#[macro_export] +macro_rules! selected_benchmark { + ( + $( $bench:ident ),* + ) => { + // The list of available benchmarks for this pallet. + enum SelectedBenchmark { + $( $bench, )* + } + + // Allow us to select a benchmark from the list of available benchmarks. + impl $crate::BenchmarkingSetup, RawOrigin> for SelectedBenchmark { + fn components(&self) -> Vec<($crate::BenchmarkParameter, u32, u32)> { + match self { + $( Self::$bench => <$bench as $crate::BenchmarkingSetup< + T, + Call, + RawOrigin, + >>::components(&$bench), )* + } + } + + fn instance(&self, components: &[($crate::BenchmarkParameter, u32)]) + -> Result<(Call, RawOrigin), &'static str> + { + match self { + $( Self::$bench => <$bench as $crate::BenchmarkingSetup< + T, + Call, + RawOrigin, + >>::instance(&$bench, components), )* + } + } + } + }; +} diff --git a/frame/identity/Cargo.toml b/frame/identity/Cargo.toml index 59e8f721c8..c95d230230 100644 --- a/frame/identity/Cargo.toml +++ b/frame/identity/Cargo.toml @@ -12,6 +12,7 @@ enumflags2 = { version = "0.6.2" } sp-std = { version = "2.0.0", default-features = false, path = "../../primitives/std" } sp-io = { version = "2.0.0", default-features = false, path = "../../primitives/io" } sp-runtime = { version = "2.0.0", default-features = false, path = "../../primitives/runtime" } +frame-benchmarking = { version = "2.0.0", default-features = false, path = "../benchmarking" } frame-support = { version = "2.0.0", default-features = false, path = "../support" } frame-system = { version = "2.0.0", default-features = false, path = "../system" } @@ -27,6 +28,7 @@ std = [ "sp-std/std", "sp-io/std", "sp-runtime/std", + "frame-benchmarking/std", "frame-support/std", "frame-system/std", ] diff --git a/frame/identity/src/benchmarking.rs b/frame/identity/src/benchmarking.rs index 11e98101ec..c208d32717 100644 --- a/frame/identity/src/benchmarking.rs +++ b/frame/identity/src/benchmarking.rs @@ -20,8 +20,11 @@ use super::*; use frame_system::RawOrigin; use sp_io::hashing::blake2_256; -use sp_runtime::{BenchmarkResults, BenchmarkParameter, selected_benchmark}; -use sp_runtime::traits::{Bounded, Benchmarking, BenchmarkingSetup, Dispatchable}; +use frame_benchmarking::{ + BenchmarkResults, BenchmarkParameter, selected_benchmark, benchmarking, Benchmarking, + BenchmarkingSetup, +}; +use sp_runtime::traits::{Bounded, Dispatchable}; use crate::Module as Identity; @@ -102,7 +105,7 @@ impl BenchmarkingSetup, RawOrigin> for { // Add r registrars let r = components.iter().find(|&c| c.0 == BenchmarkParameter::R).unwrap().1; - benchmarking::add_registrars::(r)?; + add_registrars::(r)?; // Return the `add_registrar` r + 1 call Ok((crate::Call::::add_registrar(account::("registrar", r + 1)), RawOrigin::Root)) @@ -126,7 +129,7 @@ impl BenchmarkingSetup, RawOrigin> for { // Add r registrars let r = components.iter().find(|&c| c.0 == BenchmarkParameter::R).unwrap().1; - benchmarking::add_registrars::(r)?; + add_registrars::(r)?; // The target user let caller = account::("caller", r); @@ -135,7 +138,7 @@ impl BenchmarkingSetup, RawOrigin> for let _ = T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); // Add an initial identity - let initial_info = benchmarking::create_identity_info::(1); + let initial_info = create_identity_info::(1); Identity::::set_identity(caller_origin.clone(), initial_info)?; // User requests judgement from all the registrars, and they approve @@ -152,7 +155,7 @@ impl BenchmarkingSetup, RawOrigin> for // Create identity info with x additional fields let x = components.iter().find(|&c| c.0 == BenchmarkParameter::X).unwrap().1; // 32 byte data that we reuse below - let info = benchmarking::create_identity_info::(x); + let info = create_identity_info::(x); // Return the `set_identity` call Ok((crate::Call::::set_identity(info), RawOrigin::Signed(caller))) @@ -181,7 +184,7 @@ impl BenchmarkingSetup, RawOrigin> for let _ = T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); // Create their main identity - let info = benchmarking::create_identity_info::(1); + let info = create_identity_info::(1); Identity::::set_identity(caller_origin.clone(), info)?; // Give them s many sub accounts @@ -221,16 +224,16 @@ impl BenchmarkingSetup, RawOrigin> for // Register r registrars let r = components.iter().find(|&c| c.0 == BenchmarkParameter::R).unwrap().1; - benchmarking::add_registrars::(r)?; + add_registrars::(r)?; // Create their main identity with x additional fields let x = components.iter().find(|&c| c.0 == BenchmarkParameter::X).unwrap().1; - let info = benchmarking::create_identity_info::(x); + let info = create_identity_info::(x); Identity::::set_identity(caller_origin.clone(), info)?; // Give them s many sub accounts let s = components.iter().find(|&c| c.0 == BenchmarkParameter::S).unwrap().1; - let _ = benchmarking::add_sub_accounts::(caller.clone(), s)?; + let _ = add_sub_accounts::(caller.clone(), s)?; // User requests judgement from all the registrars, and they approve for i in 0..r { @@ -270,11 +273,11 @@ impl BenchmarkingSetup, RawOrigin> for // Register r registrars let r = components.iter().find(|&c| c.0 == BenchmarkParameter::R).unwrap().1; - benchmarking::add_registrars::(r)?; + add_registrars::(r)?; // Create their main identity with x additional fields let x = components.iter().find(|&c| c.0 == BenchmarkParameter::X).unwrap().1; - let info = benchmarking::create_identity_info::(x); + let info = create_identity_info::(x); Identity::::set_identity(caller_origin.clone(), info)?; // Return the `request_judgement` call @@ -304,11 +307,11 @@ impl BenchmarkingSetup, RawOrigin> for // Register r registrars let r = components.iter().find(|&c| c.0 == BenchmarkParameter::R).unwrap().1; - benchmarking::add_registrars::(r)?; + add_registrars::(r)?; // Create their main identity with x additional fields let x = components.iter().find(|&c| c.0 == BenchmarkParameter::X).unwrap().1; - let info = benchmarking::create_identity_info::(x); + let info = create_identity_info::(x); Identity::::set_identity(caller_origin.clone(), info)?; // Request judgement @@ -337,7 +340,7 @@ impl BenchmarkingSetup, RawOrigin> for // Register r registrars let r = components.iter().find(|&c| c.0 == BenchmarkParameter::R).unwrap().1; - benchmarking::add_registrars::(r)?; + add_registrars::(r)?; // Add caller as registrar Identity::::add_registrar(RawOrigin::Root.into(), caller.clone())?; @@ -366,7 +369,7 @@ impl BenchmarkingSetup, RawOrigin> for // Register r registrars let r = components.iter().find(|&c| c.0 == BenchmarkParameter::R).unwrap().1; - benchmarking::add_registrars::(r)?; + add_registrars::(r)?; // Add caller as registrar Identity::::add_registrar(RawOrigin::Root.into(), caller.clone())?; @@ -395,7 +398,7 @@ impl BenchmarkingSetup, RawOrigin> for // Register r registrars let r = components.iter().find(|&c| c.0 == BenchmarkParameter::R).unwrap().1; - benchmarking::add_registrars::(r)?; + add_registrars::(r)?; // Add caller as registrar Identity::::add_registrar(RawOrigin::Root.into(), caller.clone())?; @@ -428,7 +431,7 @@ impl BenchmarkingSetup, RawOrigin> for { // Add r registrars let r = components.iter().find(|&c| c.0 == BenchmarkParameter::R).unwrap().1; - benchmarking::add_registrars::(r)?; + add_registrars::(r)?; // The user let user = account::("user", r); @@ -438,7 +441,7 @@ impl BenchmarkingSetup, RawOrigin> for // Create their main identity with x additional fields let x = components.iter().find(|&c| c.0 == BenchmarkParameter::X).unwrap().1; - let info = benchmarking::create_identity_info::(x); + let info = create_identity_info::(x); Identity::::set_identity(user_origin.clone(), info)?; // The caller registrar @@ -486,16 +489,16 @@ impl BenchmarkingSetup, RawOrigin> for // Register r registrars let r = components.iter().find(|&c| c.0 == BenchmarkParameter::R).unwrap().1; - benchmarking::add_registrars::(r)?; + add_registrars::(r)?; // Create their main identity with x additional fields let x = components.iter().find(|&c| c.0 == BenchmarkParameter::X).unwrap().1; - let info = benchmarking::create_identity_info::(x); + let info = create_identity_info::(x); Identity::::set_identity(caller_origin.clone(), info)?; // Give them s many sub accounts let s = components.iter().find(|&c| c.0 == BenchmarkParameter::S).unwrap().1; - let _ = benchmarking::add_sub_accounts::(caller.clone(), s)?; + let _ = add_sub_accounts::(caller.clone(), s)?; // User requests judgement from all the registrars, and they approve for i in 0..r { @@ -547,8 +550,8 @@ impl Benchmarking for Module { }; // Warm up the DB - sp_io::benchmarking::commit_db(); - sp_io::benchmarking::wipe_db(); + benchmarking::commit_db(); + benchmarking::wipe_db(); // first one is set_identity. let components = , RawOrigin>>::components(&selected_benchmark); @@ -575,15 +578,15 @@ impl Benchmarking for Module { let (call, caller) = , RawOrigin>>::instance(&selected_benchmark, &c)?; // Commit the externalities to the database, flushing the DB cache. // This will enable worst case scenario for reading from the database. - sp_io::benchmarking::commit_db(); + benchmarking::commit_db(); // Run the benchmark. - let start = sp_io::benchmarking::current_time(); + let start = benchmarking::current_time(); call.dispatch(caller.into())?; - let finish = sp_io::benchmarking::current_time(); + let finish = benchmarking::current_time(); let elapsed = finish - start; results.push((c.clone(), elapsed)); // Wipe the DB back to the genesis state. - sp_io::benchmarking::wipe_db(); + benchmarking::wipe_db(); } } } diff --git a/frame/timestamp/Cargo.toml b/frame/timestamp/Cargo.toml index e68392f9e9..107374799d 100644 --- a/frame/timestamp/Cargo.toml +++ b/frame/timestamp/Cargo.toml @@ -12,6 +12,7 @@ sp-std = { version = "2.0.0", default-features = false, path = "../../primitives sp-io = { version = "2.0.0", default-features = false, path = "../../primitives/io" } sp-runtime = { version = "2.0.0", default-features = false, path = "../../primitives/runtime" } sp-inherents = { version = "2.0.0", default-features = false, path = "../../primitives/inherents" } +frame-benchmarking = { version = "2.0.0", default-features = false, path = "../benchmarking" } frame-support = { version = "2.0.0", default-features = false, path = "../support" } frame-system = { version = "2.0.0", default-features = false, path = "../system" } sp-timestamp = { version = "2.0.0", default-features = false, path = "../../primitives/timestamp" } @@ -28,6 +29,7 @@ std = [ "codec/std", "sp-std/std", "sp-runtime/std", + "frame-benchmarking/std", "frame-support/std", "serde", "frame-system/std", diff --git a/frame/timestamp/src/benchmarking.rs b/frame/timestamp/src/benchmarking.rs index 55d6d7e046..9310d39dfd 100644 --- a/frame/timestamp/src/benchmarking.rs +++ b/frame/timestamp/src/benchmarking.rs @@ -21,8 +21,11 @@ use super::*; use sp_std::prelude::*; use frame_system::RawOrigin; -use sp_runtime::{BenchmarkResults, BenchmarkParameter, selected_benchmark}; -use sp_runtime::traits::{Benchmarking, BenchmarkingSetup, Dispatchable}; +use frame_benchmarking::{ + BenchmarkResults, BenchmarkParameter, selected_benchmark, benchmarking, + Benchmarking, BenchmarkingSetup, +}; +use sp_runtime::traits::Dispatchable; /// Benchmark `set` extrinsic. struct Set; @@ -54,10 +57,10 @@ impl Benchmarking for Module { b"set" => SelectedBenchmark::Set, _ => return Err("Could not find extrinsic."), }; - + // Warm up the DB - sp_io::benchmarking::commit_db(); - sp_io::benchmarking::wipe_db(); + benchmarking::commit_db(); + benchmarking::wipe_db(); let components = , RawOrigin>>::components(&selected_benchmark); let mut results: Vec = Vec::new(); @@ -87,15 +90,15 @@ impl Benchmarking for Module { >>::instance(&selected_benchmark, &c)?; // Commit the externalities to the database, flushing the DB cache. // This will enable worst case scenario for reading from the database. - sp_io::benchmarking::commit_db(); + benchmarking::commit_db(); // Run the benchmark. - let start = sp_io::benchmarking::current_time(); + let start = benchmarking::current_time(); call.dispatch(caller.into())?; - let finish = sp_io::benchmarking::current_time(); + let finish = benchmarking::current_time(); let elapsed = finish - start; results.push((c.clone(), elapsed)); // Wipe the DB back to the genesis state. - sp_io::benchmarking::wipe_db(); + benchmarking::wipe_db(); } } } diff --git a/primitives/io/src/lib.rs b/primitives/io/src/lib.rs index a1e9181f28..5d29fe5c94 100644 --- a/primitives/io/src/lib.rs +++ b/primitives/io/src/lib.rs @@ -771,30 +771,6 @@ pub trait Logging { } } -/// Interface that provides functions for benchmarking the runtime. -#[runtime_interface] -pub trait Benchmarking { - /// Get the number of nanoseconds passed since the UNIX epoch - /// - /// WARNING! This is a non-deterministic call. Do not use this within - /// consensus critical logic. - fn current_time() -> u128 { - std::time::SystemTime::now().duration_since(std::time::SystemTime::UNIX_EPOCH) - .expect("Unix time doesn't go backwards; qed") - .as_nanos() - } - - /// Reset the trie database to the genesis state. - fn wipe_db(&mut self) { - self.wipe() - } - - /// Commit pending storage changes to the trie database and clear the database cache. - fn commit_db(&mut self) { - self.commit() - } -} - /// Wasm-only interface that provides functions for interacting with the sandbox. #[runtime_interface(wasm_only)] pub trait Sandbox { diff --git a/primitives/runtime/src/lib.rs b/primitives/runtime/src/lib.rs index 4d6739bb13..6501dafc0e 100644 --- a/primitives/runtime/src/lib.rs +++ b/primitives/runtime/src/lib.rs @@ -685,18 +685,6 @@ pub fn print(print: impl traits::Printable) { print.print(); } -/// An alphabet of possible parameters to use for benchmarking. -#[derive(Encode, Decode, Clone, Copy, PartialEq, Debug)] -#[allow(missing_docs)] -pub enum BenchmarkParameter { - A, B, C, D, E, F, G, H, I, J, K, L, M, N, O, P, Q, R, S, T, U, V, W, X, Y, Z, -} - -/// Results from running benchmarks on a FRAME pallet. -/// Contains duration of the function call in nanoseconds along with the benchmark parameters -/// used for that benchmark result. -pub type BenchmarkResults = (Vec<(BenchmarkParameter, u32)>, u128); - #[cfg(test)] mod tests { use super::*; diff --git a/primitives/runtime/src/traits.rs b/primitives/runtime/src/traits.rs index 183df08ab8..f6655f68b4 100644 --- a/primitives/runtime/src/traits.rs +++ b/primitives/runtime/src/traits.rs @@ -26,7 +26,6 @@ use std::str::FromStr; #[cfg(feature = "std")] use serde::{Serialize, Deserialize, de::DeserializeOwned}; use sp_core::{self, Hasher, Blake2Hasher, TypeId, RuntimeDebug}; -use crate::BenchmarkParameter; use crate::codec::{Codec, Encode, Decode}; use crate::transaction_validity::{ ValidTransaction, TransactionValidity, TransactionValidityError, UnknownTransaction, @@ -1318,75 +1317,6 @@ pub trait BlockIdTo { ) -> Result>, Self::Error>; } -/// The pallet benchmarking trait. -pub trait Benchmarking { - /// Run the benchmarks for this pallet. - /// - /// Parameters - /// - `extrinsic`: The name of extrinsic function you want to benchmark encoded as bytes. - /// - `steps`: The number of sample points you want to take across the range of parameters. - /// - `repeat`: The number of times you want to repeat a benchmark. - fn run_benchmark(extrinsic: Vec, steps: u32, repeat: u32) -> Result, &'static str>; -} - -/// The required setup for creating a benchmark. -pub trait BenchmarkingSetup { - /// Return the components and their ranges which should be tested in this benchmark. - fn components(&self) -> Vec<(BenchmarkParameter, u32, u32)>; - - /// Set up the storage, and prepare a call and caller to test in a single run of the benchmark. - fn instance(&self, components: &[(BenchmarkParameter, u32)]) -> Result<(Call, RawOrigin), &'static str>; -} - -/// Creates a `SelectedBenchmark` enum implementing `BenchmarkingSetup`. -/// -/// Every variant must implement [`BenchmarkingSetup`](crate::traits::BenchmarkingSetup). -/// -/// ```nocompile -/// -/// struct Transfer; -/// impl BenchmarkingSetup for Transfer { ... } -/// -/// struct SetBalance; -/// impl BenchmarkingSetup for SetBalance { ... } -/// -/// selected_benchmark!(Transfer, SetBalance); -/// ``` -#[macro_export] -macro_rules! selected_benchmark { - ($($bench:ident),*) => { - // The list of available benchmarks for this pallet. - enum SelectedBenchmark { - $( $bench, )* - } - - // Allow us to select a benchmark from the list of available benchmarks. - impl $crate::traits::BenchmarkingSetup, RawOrigin> for SelectedBenchmark { - fn components(&self) -> Vec<(BenchmarkParameter, u32, u32)> { - match self { - $( Self::$bench => <$bench as $crate::traits::BenchmarkingSetup< - T, - Call, - RawOrigin, - >>::components(&$bench), )* - } - } - - fn instance(&self, components: &[(BenchmarkParameter, u32)]) - -> Result<(Call, RawOrigin), &'static str> - { - match self { - $( Self::$bench => <$bench as $crate::traits::BenchmarkingSetup< - T, - Call, - RawOrigin, - >>::instance(&$bench, components), )* - } - } - } - }; -} - #[cfg(test)] mod tests { use super::*; diff --git a/utils/frame/benchmarking-cli/Cargo.toml b/utils/frame/benchmarking-cli/Cargo.toml new file mode 100644 index 0000000000..129104a901 --- /dev/null +++ b/utils/frame/benchmarking-cli/Cargo.toml @@ -0,0 +1,17 @@ +[package] +name = "frame-benchmarking-cli" +version = "2.0.0" +authors = ["Parity Technologies "] +edition = "2018" +license = "GPL-3.0" + +[dependencies] +frame-benchmarking = { version = "2.0.0", path = "../../../frame/benchmarking" } +sc-service = { version = "0.8.0", path = "../../../client/service" } +sc-cli = { version = "0.8.0", path = "../../../client/cli" } +sc-client = { version = "0.8.0", path = "../../../client" } +sc-client-db = { version = "0.8.0", path = "../../../client/db" } +sc-executor = { version = "0.8.0", path = "../../../client/executor" } +sp-runtime = { version = "2.0.0", path = "../../../primitives/runtime" } +structopt = "0.3.8" +codec = { version = "1.1.2", package = "parity-scale-codec" } diff --git a/utils/frame/benchmarking-cli/src/lib.rs b/utils/frame/benchmarking-cli/src/lib.rs new file mode 100644 index 0000000000..a8303beb0f --- /dev/null +++ b/utils/frame/benchmarking-cli/src/lib.rs @@ -0,0 +1,152 @@ +// Copyright 2020 Parity Technologies (UK) Ltd. +// This file is part of Substrate. + +// Substrate is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Substrate is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Substrate. If not, see . + +use sp_runtime::{BuildStorage, traits::{Block as BlockT, Header as HeaderT, NumberFor}}; +use sc_client::StateMachine; +use sc_cli::{ExecutionStrategy, WasmExecutionMethod}; +use sc_client_db::BenchmarkingState; +use sc_service::{RuntimeGenesis, ChainSpecExtension}; +use sc_executor::{NativeExecutor, NativeExecutionDispatch}; +use std::fmt::Debug; +use codec::{Encode, Decode}; +use frame_benchmarking::BenchmarkResults; + +/// The `benchmark` command used to benchmark FRAME Pallets. +#[derive(Debug, structopt::StructOpt, Clone)] +pub struct BenchmarkCmd { + /// Select a FRAME Pallet to benchmark. + #[structopt(short, long)] + pub pallet: String, + + /// Select an extrinsic to benchmark. + #[structopt(short, long)] + pub extrinsic: String, + + /// Select how many samples we should take across the variable components. + #[structopt(short, long, default_value = "1")] + pub steps: u32, + + /// Select how many repetitions of this benchmark should run. + #[structopt(short, long, default_value = "1")] + pub repeat: u32, + + #[allow(missing_docs)] + #[structopt(flatten)] + pub shared_params: sc_cli::SharedParams, + + /// The execution strategy that should be used for benchmarks + #[structopt( + long = "execution", + value_name = "STRATEGY", + possible_values = &ExecutionStrategy::variants(), + case_insensitive = true, + )] + pub execution: Option, + + /// Method for executing Wasm runtime code. + #[structopt( + long = "wasm-execution", + value_name = "METHOD", + possible_values = &WasmExecutionMethod::enabled_variants(), + case_insensitive = true, + default_value = "Interpreted" + )] + pub wasm_method: WasmExecutionMethod, +} + +impl BenchmarkCmd { + /// Parse CLI arguments and initialize given config. + pub fn init( + &self, + config: &mut sc_service::config::Configuration, + spec_factory: impl FnOnce(&str) -> Result>, String>, + version: &sc_cli::VersionInfo, + ) -> sc_cli::error::Result<()> where + G: sc_service::RuntimeGenesis, + E: sc_service::ChainSpecExtension, + { + sc_cli::init_config(config, &self.shared_params, version, spec_factory)?; + // make sure to configure keystore + sc_cli::fill_config_keystore_in_memory(config).map_err(Into::into) + } + + /// Runs the command and benchmarks the chain. + pub fn run( + self, + config: sc_service::Configuration, + ) -> sc_cli::error::Result<()> + where + G: RuntimeGenesis, + E: ChainSpecExtension, + BB: BlockT + Debug, + <<::Header as HeaderT>::Number as std::str::FromStr>::Err: std::fmt::Debug, + ::Hash: std::str::FromStr, + ExecDispatch: NativeExecutionDispatch + 'static, + { + let spec = config.chain_spec.expect("chain_spec is always Some"); + let wasm_method = self.wasm_method.into(); + let strategy = self.execution.unwrap_or(ExecutionStrategy::Native); + + let genesis_storage = spec.build_storage()?; + let mut changes = Default::default(); + let state = BenchmarkingState::::new(genesis_storage)?; + let executor = NativeExecutor::::new( + wasm_method, + None, // heap pages + ); + let result = StateMachine::<_, _, NumberFor, _>::new( + &state, + None, + &mut changes, + &executor, + "Benchmark_dispatch_benchmark", + &(&self.pallet, &self.extrinsic, self.steps, self.repeat).encode(), + Default::default(), + ) + .execute(strategy.into()) + .map_err(|e| format!("Error executing runtime benchmark: {:?}", e))?; + let results = > as Decode>::decode(&mut &result[..]) + .unwrap_or(None); + + if let Some(results) = results { + // Print benchmark metadata + println!( + "Pallet: {:?}, Extrinsic: {:?}, Steps: {:?}, Repeat: {:?}", + self.pallet, + self.extrinsic, + self.steps, + self.repeat, + ); + + // Print the table header + results[0].0.iter().for_each(|param| print!("{:?},", param.0)); + + print!("time\n"); + // Print the values + results.iter().for_each(|result| { + let parameters = &result.0; + parameters.iter().for_each(|param| print!("{:?},", param.1)); + print!("{:?}\n", result.1); + }); + + eprintln!("Done."); + } else { + eprintln!("No Results."); + } + + Ok(()) + } +} -- GitLab From 5a2824d9b20f151466d92f20c767141c30a35b65 Mon Sep 17 00:00:00 2001 From: Alexander Popiak Date: Wed, 19 Feb 2020 10:46:55 +0100 Subject: [PATCH 006/438] Rename remaining occurences of SRML to FRAME (#4932) * rename remaining SRML occurences to FRAME * Some module -> pallet * remove out of date url Co-authored-by: Shawn Tabrizi Co-authored-by: Cecile Tonglet --- bin/node-template/README.md | 2 +- bin/node-template/pallets/template/src/lib.rs | 4 +- .../pallets/template/src/mock.rs | 4 +- bin/node/rpc/src/lib.rs | 4 +- bin/node/runtime/src/constants.rs | 2 +- docs/CONTRIBUTING.adoc | 2 +- docs/README.adoc | 2 +- frame/assets/src/lib.rs | 4 +- frame/authorship/Cargo.toml | 2 +- frame/authorship/src/lib.rs | 2 +- frame/babe/src/lib.rs | 2 +- frame/balances/src/lib.rs | 4 +- frame/example/src/lib.rs | 124 +++++++++--------- frame/executive/src/lib.rs | 8 +- frame/finality-tracker/src/lib.rs | 2 +- frame/generic-asset/src/lib.rs | 10 +- frame/generic-asset/src/mock.rs | 4 +- frame/identity/src/lib.rs | 4 +- frame/membership/src/lib.rs | 4 +- frame/nicks/src/lib.rs | 4 +- frame/recovery/src/mock.rs | 4 +- frame/scored-pool/src/mock.rs | 4 +- frame/session/src/historical.rs | 2 +- frame/session/src/lib.rs | 2 +- frame/society/src/mock.rs | 4 +- frame/support/procedural/src/lib.rs | 2 +- frame/support/src/dispatch.rs | 8 +- frame/support/src/traits.rs | 2 +- frame/system/src/lib.rs | 6 +- frame/timestamp/src/lib.rs | 2 +- frame/utility/src/lib.rs | 4 +- frame/vesting/src/lib.rs | 4 +- primitives/finality-tracker/src/lib.rs | 2 +- primitives/phragmen/src/lib.rs | 4 +- utils/frame/rpc/system/src/lib.rs | 2 +- 35 files changed, 123 insertions(+), 123 deletions(-) diff --git a/bin/node-template/README.md b/bin/node-template/README.md index c411dbeef5..4ae60478fc 100644 --- a/bin/node-template/README.md +++ b/bin/node-template/README.md @@ -1,6 +1,6 @@ # Substrate Node Template -A new SRML-based Substrate node, ready for hacking. +A new FRAME-based Substrate node, ready for hacking. ## Build diff --git a/bin/node-template/pallets/template/src/lib.rs b/bin/node-template/pallets/template/src/lib.rs index aa4d2cbc99..34e055bf54 100644 --- a/bin/node-template/pallets/template/src/lib.rs +++ b/bin/node-template/pallets/template/src/lib.rs @@ -1,12 +1,12 @@ #![cfg_attr(not(feature = "std"), no_std)] -/// A runtime module template with necessary imports +/// A FRAME pallet template with necessary imports /// Feel free to remove or edit this file as needed. /// If you change the name of this file, make sure to update its references in runtime/src/lib.rs /// If you remove this file, you can remove those references -/// For more guidance on Substrate modules, see the example module +/// For more guidance on Substrate FRAME, see the example pallet /// https://github.com/paritytech/substrate/blob/master/frame/example/src/lib.rs use frame_support::{decl_module, decl_storage, decl_event, decl_error, dispatch}; diff --git a/bin/node-template/pallets/template/src/mock.rs b/bin/node-template/pallets/template/src/mock.rs index b3c1098db6..7a23610e4b 100644 --- a/bin/node-template/pallets/template/src/mock.rs +++ b/bin/node-template/pallets/template/src/mock.rs @@ -11,9 +11,9 @@ impl_outer_origin! { pub enum Origin for Test {} } -// For testing the module, we construct most of a mock runtime. This means +// For testing the pallet, we construct most of a mock runtime. This means // first constructing a configuration type (`Test`) which `impl`s each of the -// configuration traits of modules we want to use. +// configuration traits of pallets we want to use. #[derive(Clone, Eq, PartialEq)] pub struct Test; parameter_types! { diff --git a/bin/node/rpc/src/lib.rs b/bin/node/rpc/src/lib.rs index 16e5446bb1..4e1cfa5673 100644 --- a/bin/node/rpc/src/lib.rs +++ b/bin/node/rpc/src/lib.rs @@ -23,9 +23,9 @@ //! need some strong assumptions about the particular runtime. //! //! The RPCs available in this crate however can make some assumptions -//! about how the runtime is constructed and what `SRML` modules +//! about how the runtime is constructed and what FRAME pallets //! are part of it. Therefore all node-runtime-specific RPCs can -//! be placed here or imported from corresponding `SRML` RPC definitions. +//! be placed here or imported from corresponding FRAME RPC definitions. #![warn(missing_docs)] diff --git a/bin/node/runtime/src/constants.rs b/bin/node/runtime/src/constants.rs index b2c880c08b..bf12492f8d 100644 --- a/bin/node/runtime/src/constants.rs +++ b/bin/node/runtime/src/constants.rs @@ -38,7 +38,7 @@ pub mod time { /// a slot being empty). /// This value is only used indirectly to define the unit constants below /// that are expressed in blocks. The rest of the code should use - /// `SLOT_DURATION` instead (like the timestamp module for calculating the + /// `SLOT_DURATION` instead (like the Timestamp pallet for calculating the /// minimum period). /// /// If using BABE with secondary slots (default) then all of the slots will diff --git a/docs/CONTRIBUTING.adoc b/docs/CONTRIBUTING.adoc index c83b686b09..cdd9809fff 100644 --- a/docs/CONTRIBUTING.adoc +++ b/docs/CONTRIBUTING.adoc @@ -27,7 +27,7 @@ Merging pull requests once CI is successful: . Once a PR is ready for review please add the https://github.com/paritytech/substrate/pulls?q=is%3Apr+is%3Aopen+label%3AA0-pleasereview[`pleasereview`] label. Generally PRs should sit with this label for 48 hours in order to garner feedback. It may be merged before if all relevant parties had a look at it. . If the first review is not an approval, swap `A0-pleasereview` to any label `[A3, A7]` to indicate that the PR has received some feedback, but needs further work. For example. https://github.com/paritytech/substrate/labels/A3-inprogress[`A3-inprogress`] is a general indicator that the PR is work in progress and https://github.com/paritytech/substrate/labels/A4-gotissues[`A4-gotissues`] means that it has significant problems that need fixing. Once the work is done, change the label back to `A0-pleasereview`. You might end up swapping a few times back and forth to climb up the A label group. Once a PR is https://github.com/paritytech/substrate/labels/A8-looksgood[`A8-looksgood`], it is ready to merge. -. PRs that break the external API must be tagged with https://github.com/paritytech/substrate/labels/B2-breaksapi[`breaksapi`], when it changes the SRML or consensus of running system with https://github.com/paritytech/substrate/labels/B3-breaksconsensus[`breaksconsensus`] +. PRs that break the external API must be tagged with https://github.com/paritytech/substrate/labels/B2-breaksapi[`breaksapi`], when it changes the FRAME or consensus of running system with https://github.com/paritytech/substrate/labels/B3-breaksconsensus[`breaksconsensus`] . No PR should be merged until all reviews' comments are addressed. *Reviewing pull requests*: diff --git a/docs/README.adoc b/docs/README.adoc index bbc2713fbe..8d762fee05 100644 --- a/docs/README.adoc +++ b/docs/README.adoc @@ -26,7 +26,7 @@ Substrate is designed for use in one of three ways: **2. Modular**: By hacking together pallets built with Substrate FRAME into a new runtime and possibly altering or reconfiguring the Substrate client's block authoring logic. This affords you a very large amount of freedom over your blockchain's logic, letting you change data types, add or remove modules, and crucially, add your own modules. Much can be changed without touching the block authoring logic (since it is generic). If this is the case, then the existing Substrate binary can be used for block authoring and syncing. If the block authoring logic needs to be tweaked, then a new, altered block authoring binary must be built as a separate project and used by validators. This is how the Polkadot relay chain is built and should suffice for almost all circumstances in the near to mid-term. -**3. Generic**: The entire SRML can be ignored and the entire runtime designed and implemented from scratch. If desired, this can be done in a language other than Rust, provided it can target WebAssembly. If the runtime can be made compatible with the existing client's block authoring logic, then you can simply construct a new genesis block from your Wasm blob and launch your chain with the existing Rust-based Substrate client. If not, then you'll need to alter the client's block authoring logic accordingly. This is probably a useless option for most projects right now, but provides complete flexibility allowing for a long-term, far-reaching upgrade path for the Substrate paradigm. +**3. Generic**: The entire FRAME can be ignored and the entire runtime designed and implemented from scratch. If desired, this can be done in a language other than Rust, provided it can target WebAssembly. If the runtime can be made compatible with the existing client's block authoring logic, then you can simply construct a new genesis block from your Wasm blob and launch your chain with the existing Rust-based Substrate client. If not, then you'll need to alter the client's block authoring logic accordingly. This is probably a useless option for most projects right now, but provides complete flexibility allowing for a long-term, far-reaching upgrade path for the Substrate paradigm. === The Basics of Substrate diff --git a/frame/assets/src/lib.rs b/frame/assets/src/lib.rs index 8cdc9b9cc0..ebd1d17bcf 100644 --- a/frame/assets/src/lib.rs +++ b/frame/assets/src/lib.rs @@ -265,9 +265,9 @@ mod tests { pub enum Origin for Test where system = frame_system {} } - // For testing the module, we construct most of a mock runtime. This means + // For testing the pallet, we construct most of a mock runtime. This means // first constructing a configuration type (`Test`) which `impl`s each of the - // configuration traits of modules we want to use. + // configuration traits of pallets we want to use. #[derive(Clone, Eq, PartialEq)] pub struct Test; parameter_types! { diff --git a/frame/authorship/Cargo.toml b/frame/authorship/Cargo.toml index 124d66c1bc..61056820f4 100644 --- a/frame/authorship/Cargo.toml +++ b/frame/authorship/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "pallet-authorship" version = "2.0.0" -description = "Block and Uncle Author tracking for the SRML" +description = "Block and Uncle Author tracking for the FRAME" authors = ["Parity Technologies "] edition = "2018" license = "GPL-3.0" diff --git a/frame/authorship/src/lib.rs b/frame/authorship/src/lib.rs index f2dbd4674f..335e13a7fd 100644 --- a/frame/authorship/src/lib.rs +++ b/frame/authorship/src/lib.rs @@ -14,7 +14,7 @@ // You should have received a copy of the GNU General Public License // along with Substrate. If not, see . -//! Authorship tracking for SRML runtimes. +//! Authorship tracking for FRAME runtimes. //! //! This tracks the current author of the block and recent uncles. diff --git a/frame/babe/src/lib.rs b/frame/babe/src/lib.rs index e4e37a4b7a..aba3a75eb9 100644 --- a/frame/babe/src/lib.rs +++ b/frame/babe/src/lib.rs @@ -159,7 +159,7 @@ decl_storage! { } decl_module! { - /// The BABE SRML module + /// The BABE Pallet pub struct Module for enum Call where origin: T::Origin { /// The number of **slots** that an epoch takes. We couple sessions to /// epochs, i.e. we start a new session once the new epoch begins. diff --git a/frame/balances/src/lib.rs b/frame/balances/src/lib.rs index 2c8ae3e18f..0f5e8b40d5 100644 --- a/frame/balances/src/lib.rs +++ b/frame/balances/src/lib.rs @@ -92,7 +92,7 @@ //! //! The following examples show how to use the Balances module in your custom module. //! -//! ### Examples from the SRML +//! ### Examples from the FRAME //! //! The Contract module uses the `Currency` trait to handle gas payment, and its types inherit from `Currency`: //! @@ -896,7 +896,7 @@ mod imbalances { // This works as long as `increase_total_issuance_by` doesn't use the Imbalance // types (basically for charging fees). // This should eventually be refactored so that the type item that -// depends on the Imbalance type (DustRemoval) is placed in its own SRML module. +// depends on the Imbalance type (DustRemoval) is placed in its own pallet. struct ElevatedTrait, I: Instance>(T, I); impl, I: Instance> Clone for ElevatedTrait { fn clone(&self) -> Self { unimplemented!() } diff --git a/frame/example/src/lib.rs b/frame/example/src/lib.rs index dbdc2efcf8..39f0d25d32 100644 --- a/frame/example/src/lib.rs +++ b/frame/example/src/lib.rs @@ -14,22 +14,22 @@ // You should have received a copy of the GNU General Public License // along with Substrate. If not, see . -//! # Example Module +//! # Example Pallet //! //! -//! The Example: A simple example of a runtime module demonstrating -//! concepts, APIs and structures common to most runtime modules. +//! The Example: A simple example of a FRAME pallet demonstrating +//! concepts, APIs and structures common to most FRAME runtimes. //! -//! Run `cargo doc --package pallet-example --open` to view this module's documentation. +//! Run `cargo doc --package pallet-example --open` to view this pallet's documentation. //! //! ### Documentation Guidelines: //! //! -//! +//! //!