From a2163420f4317f852ee3064dae39ead4edf90d20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Thei=C3=9Fen?= Date: Wed, 29 Jul 2020 12:32:27 +0200 Subject: [PATCH] seal: Add benchmarks for dispatchables (#6715) * seal: Fix syntax that confuses rust-analyzer * seal: Add benchmarks for Dispatchables These are only the benchmarks for the dispatchables of the pallet. Those are not listed in the Schedule because we do not want to pull the Schedule from storage before dispatching. This OK because those costs are not related to actual contract execution. Those costs (instruction costs, ext_* costs) will be benchmarked seperatly and entered into the default Schedule. * seal: Add a maximum code size * Fix comments from review * Removed SEED constant --- substrate/Cargo.lock | 3 +- substrate/bin/node/runtime/Cargo.toml | 1 + substrate/bin/node/runtime/src/lib.rs | 1 + substrate/frame/contracts/Cargo.toml | 29 +- .../contracts/fixtures/benchmarks/dummy.wat | 4 + substrate/frame/contracts/src/benchmarking.rs | 271 ++++++++++++++++++ substrate/frame/contracts/src/lib.rs | 13 +- substrate/frame/contracts/src/tests.rs | 8 +- substrate/frame/contracts/src/wasm/mod.rs | 3 +- substrate/frame/contracts/src/wasm/prepare.rs | 5 +- 10 files changed, 315 insertions(+), 23 deletions(-) create mode 100644 substrate/frame/contracts/fixtures/benchmarks/dummy.wat create mode 100644 substrate/frame/contracts/src/benchmarking.rs diff --git a/substrate/Cargo.lock b/substrate/Cargo.lock index ef15e57e80..0fe8e4919e 100644 --- a/substrate/Cargo.lock +++ b/substrate/Cargo.lock @@ -4262,6 +4262,7 @@ version = "2.0.0-rc5" dependencies = [ "assert_matches", "bitflags", + "frame-benchmarking", "frame-support", "frame-system", "hex-literal", @@ -4279,8 +4280,8 @@ dependencies = [ "sp-runtime", "sp-sandbox", "sp-std", - "wabt", "wasmi-validation", + "wat", ] [[package]] diff --git a/substrate/bin/node/runtime/Cargo.toml b/substrate/bin/node/runtime/Cargo.toml index def3378643..35ed740045 100644 --- a/substrate/bin/node/runtime/Cargo.toml +++ b/substrate/bin/node/runtime/Cargo.toml @@ -152,6 +152,7 @@ runtime-benchmarks = [ "pallet-babe/runtime-benchmarks", "pallet-balances/runtime-benchmarks", "pallet-collective/runtime-benchmarks", + "pallet-contracts/runtime-benchmarks", "pallet-democracy/runtime-benchmarks", "pallet-elections-phragmen/runtime-benchmarks", "pallet-grandpa/runtime-benchmarks", diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index ad748c4574..9dae66a127 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -1162,6 +1162,7 @@ impl_runtime_apis! { add_benchmark!(params, batches, pallet_babe, Babe); add_benchmark!(params, batches, pallet_balances, Balances); add_benchmark!(params, batches, pallet_collective, Council); + add_benchmark!(params, batches, pallet_contracts, Contracts); add_benchmark!(params, batches, pallet_democracy, Democracy); add_benchmark!(params, batches, pallet_elections_phragmen, Elections); add_benchmark!(params, batches, pallet_grandpa, Grandpa); diff --git a/substrate/frame/contracts/Cargo.toml b/substrate/frame/contracts/Cargo.toml index f3ac96d68e..74655b9528 100644 --- a/substrate/frame/contracts/Cargo.toml +++ b/substrate/frame/contracts/Cargo.toml @@ -12,29 +12,31 @@ description = "FRAME pallet for WASM contracts" targets = ["x86_64-unknown-linux-gnu"] [dependencies] -serde = { version = "1.0.101", optional = true, features = ["derive"] } -pwasm-utils = { version = "0.12.0", default-features = false } +bitflags = "1.0" codec = { package = "parity-scale-codec", version = "1.3.4", default-features = false, features = ["derive"] } +frame-benchmarking = { version = "2.0.0-rc5", default-features = false, path = "../benchmarking", optional = true } +frame-support = { version = "2.0.0-rc5", default-features = false, path = "../support" } +frame-system = { version = "2.0.0-rc5", default-features = false, path = "../system" } +pallet-contracts-primitives = { version = "2.0.0-rc5", default-features = false, path = "common" } parity-wasm = { version = "0.41.0", default-features = false } -wasmi-validation = { version = "0.3.0", default-features = false } +pwasm-utils = { version = "0.12.0", default-features = false } +serde = { version = "1.0.101", optional = true, features = ["derive"] } sp-core = { version = "2.0.0-rc5", default-features = false, path = "../../primitives/core" } sp-runtime = { version = "2.0.0-rc5", default-features = false, path = "../../primitives/runtime" } sp-io = { version = "2.0.0-rc5", default-features = false, path = "../../primitives/io" } sp-std = { version = "2.0.0-rc5", default-features = false, path = "../../primitives/std" } sp-sandbox = { version = "0.8.0-rc5", default-features = false, path = "../../primitives/sandbox" } -frame-support = { version = "2.0.0-rc5", default-features = false, path = "../support" } -frame-system = { version = "2.0.0-rc5", default-features = false, path = "../system" } -pallet-contracts-primitives = { version = "2.0.0-rc5", default-features = false, path = "common" } -bitflags = "1.0" +wasmi-validation = { version = "0.3.0", default-features = false } +wat = { version = "1.0", optional = true, default-features = false } [dev-dependencies] -wabt = "0.9.2" assert_matches = "1.3.0" hex-literal = "0.2.1" -pretty_assertions = "0.6.1" pallet-balances = { version = "2.0.0-rc5", path = "../balances" } pallet-timestamp = { version = "2.0.0-rc5", path = "../timestamp" } pallet-randomness-collective-flip = { version = "2.0.0-rc5", path = "../randomness-collective-flip" } +pretty_assertions = "0.6.1" +wat = "1.0" [features] default = ["std"] @@ -53,3 +55,12 @@ std = [ "wasmi-validation/std", "pallet-contracts-primitives/std", ] +runtime-benchmarks = [ + "frame-benchmarking", + "wat", + # We are linking the wat crate which uses std and therefore brings with it the + # std panic handler. Therefore we need to disable out own panic handlers. Mind that + # we still override the std memory allocator. + "sp-io/disable_panic_handler", + "sp-io/disable_oom", +] diff --git a/substrate/frame/contracts/fixtures/benchmarks/dummy.wat b/substrate/frame/contracts/fixtures/benchmarks/dummy.wat new file mode 100644 index 0000000000..b878d26ef9 --- /dev/null +++ b/substrate/frame/contracts/fixtures/benchmarks/dummy.wat @@ -0,0 +1,4 @@ +(module + (func (export "call")) + (func (export "deploy")) +) diff --git a/substrate/frame/contracts/src/benchmarking.rs b/substrate/frame/contracts/src/benchmarking.rs new file mode 100644 index 0000000000..29f992643e --- /dev/null +++ b/substrate/frame/contracts/src/benchmarking.rs @@ -0,0 +1,271 @@ +// This file is part of Substrate. + +// Copyright (C) 2020 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Benchmarks for the contracts pallet + +#![cfg(feature = "runtime-benchmarks")] + +use crate::*; +use crate::Module as Contracts; + +use frame_benchmarking::{benchmarks, account}; +use frame_system::{Module as System, RawOrigin}; +use parity_wasm::elements::FuncBody; +use sp_runtime::traits::Hash; + +macro_rules! load_module { + ($name:expr) => {{ + let code = include_bytes!(concat!("../fixtures/benchmarks/", $name, ".wat")); + compile_module::(code) + }}; +} + +fn compile_module(code: &[u8]) -> (Vec, ::Output) { + let code = sp_std::str::from_utf8(code).expect("Invalid utf8 in wat file."); + let binary = wat::parse_str(code).expect("Failed to compile wat file."); + let hash = T::Hashing::hash(&binary); + (binary, hash) +} + +fn funding() -> BalanceOf { + T::Currency::minimum_balance() * 10_000.into() +} + +fn create_funded_user(string: &'static str, n: u32) -> T::AccountId { + let user = account(string, n, 0); + T::Currency::make_free_balance_be(&user, funding::()); + user +} + +fn contract_with_call_body(body: FuncBody) -> (Vec, ::Output) { + use parity_wasm::elements::{ + Instructions, Instruction::End, + }; + let contract = parity_wasm::builder::ModuleBuilder::new() + // deploy function (idx 0) + .function() + .signature().with_params(vec![]).with_return_type(None).build() + .body().with_instructions(Instructions::new(vec![End])).build() + .build() + // call function (idx 1) + .function() + .signature().with_params(vec![]).with_return_type(None).build() + .with_body(body) + .build() + .export().field("deploy").internal().func(0).build() + .export().field("call").internal().func(1).build() + .build(); + let bytes = contract.to_bytes().unwrap(); + let hash = T::Hashing::hash(&bytes); + (bytes, hash) +} + +fn expanded_contract(target_bytes: u32) -> (Vec, ::Output) { + use parity_wasm::elements::{ + Instruction::{self, If, I32Const, Return, End}, + BlockType, Instructions, + }; + // Base size of a contract is 47 bytes and each expansion adds 6 bytes. + // We do one expansion less to account for the code section and function body + // size fields inside the binary wasm module representation which are leb128 encoded + // and therefore grow in size when the contract grows. We are not allowed to overshoot + // because of the maximum code size that is enforced by `put_code`. + let expansions = (target_bytes.saturating_sub(47) / 6).saturating_sub(1) as usize; + const EXPANSION: [Instruction; 4] = [ + I32Const(0), + If(BlockType::NoResult), + Return, + End, + ]; + let instructions = Instructions::new( + EXPANSION + .iter() + .cycle() + .take(EXPANSION.len() * expansions) + .cloned() + .chain(sp_std::iter::once(End)) + .collect() + ); + contract_with_call_body::(FuncBody::new(Vec::new(), instructions)) +} + +fn advance_block(num: ::BlockNumber) { + let now = System::::block_number(); + System::::set_block_number(now + num); +} + +benchmarks! { + _ { + } + + // This extrinsic is pretty much constant as it is only a simple setter. + update_schedule { + let schedule = Schedule { + version: 1, + .. Default::default() + }; + }: _(RawOrigin::Root, schedule) + + // This constructs a contract that is maximal expensive to instrument. + // It creates a maximum number of metering blocks per byte. + put_code { + let n in 0 .. Contracts::::current_schedule().max_code_size; + let caller = create_funded_user::("caller", 0); + let (binary, hash) = expanded_contract::(n); + }: _(RawOrigin::Signed(caller), binary) + + // Instantiate uses a dummy contract constructor to measure the overhead of the instantiate. + // The size of the data has no influence on the costs of this extrinsic as long as the contract + // won't call `ext_input` in its constructor to copy the data to contract memory. + // The dummy contract used here does not do this. The costs for the data copy is billed as + // part of `ext_input`. + instantiate { + let data = vec![0u8; 128]; + let endowment = Config::::subsistence_threshold_uncached(); + let caller = create_funded_user::("caller", 0); + let (binary, hash) = load_module!("dummy"); + Contracts::::put_code(RawOrigin::Signed(caller.clone()).into(), binary.to_vec()) + .unwrap(); + + }: _( + RawOrigin::Signed(caller.clone()), + endowment, + Weight::max_value(), + hash, + data + ) + verify { + assert_eq!( + funding::() - endowment, + T::Currency::free_balance(&caller), + ) + } + + // We just call a dummy contract to measure to overhead of the call extrinsic. + // As for instantiate the size of the data does not influence the costs. + call { + let data = vec![0u8; 128]; + let endowment = Config::::subsistence_threshold_uncached(); + let value = T::Currency::minimum_balance() * 100.into(); + let caller = create_funded_user::("caller", 0); + let (binary, hash) = load_module!("dummy"); + let addr = T::DetermineContractAddress::contract_address_for(&hash, &[], &caller); + Contracts::::put_code(RawOrigin::Signed(caller.clone()).into(), binary.to_vec()) + .unwrap(); + Contracts::::instantiate( + RawOrigin::Signed(caller.clone()).into(), + endowment, + Weight::max_value(), + hash, + vec![], + ).unwrap(); + }: _( + RawOrigin::Signed(caller.clone()), + T::Lookup::unlookup(addr), + value, + Weight::max_value(), + data + ) + verify { + assert_eq!( + funding::() - endowment - value, + T::Currency::free_balance(&caller), + ) + } + + // We benchmark the costs for sucessfully evicting an empty contract. + // The actual costs are depending on how many storage items the evicted contract + // does have. However, those costs are not to be payed by the sender but + // will be distributed over multiple blocks using a scheduler. Otherwise there is + // no incentive to remove large contracts when the removal is more expensive than + // the reward for removing them. + claim_surcharge { + let endowment = Config::::subsistence_threshold_uncached(); + let value = T::Currency::minimum_balance() * 100.into(); + let caller = create_funded_user::("caller", 0); + let (binary, hash) = load_module!("dummy"); + let addr = T::DetermineContractAddress::contract_address_for(&hash, &[], &caller); + Contracts::::put_code(RawOrigin::Signed(caller.clone()).into(), binary.to_vec()) + .unwrap(); + Contracts::::instantiate( + RawOrigin::Signed(caller.clone()).into(), + endowment, + Weight::max_value(), + hash, + vec![], + ).unwrap(); + + // instantiate should leave us with an alive contract + ContractInfoOf::::get(addr.clone()).unwrap().get_alive().unwrap(); + + // generate some rent + advance_block::(::SignedClaimHandicap::get() + 1.into()); + + }: _(RawOrigin::Signed(caller.clone()), addr.clone(), None) + verify { + // the claim surcharge should have evicted the contract + ContractInfoOf::::get(addr.clone()).unwrap().get_tombstone().unwrap(); + + // the caller should get the reward for being a good snitch + assert_eq!( + funding::() - endowment + ::SurchargeReward::get(), + T::Currency::free_balance(&caller), + ); + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::tests::{ExtBuilder, Test}; + use frame_support::assert_ok; + + #[test] + fn update_schedule() { + ExtBuilder::default().build().execute_with(|| { + assert_ok!(test_benchmark_update_schedule::()); + }); + } + + #[test] + fn put_code() { + ExtBuilder::default().build().execute_with(|| { + assert_ok!(test_benchmark_put_code::()); + }); + } + + #[test] + fn instantiate() { + ExtBuilder::default().build().execute_with(|| { + assert_ok!(test_benchmark_instantiate::()); + }); + } + + #[test] + fn call() { + ExtBuilder::default().build().execute_with(|| { + assert_ok!(test_benchmark_call::()); + }); + } + + #[test] + fn claim_surcharge() { + ExtBuilder::default().build().execute_with(|| { + assert_ok!(test_benchmark_claim_surcharge::()); + }); + } +} diff --git a/substrate/frame/contracts/src/lib.rs b/substrate/frame/contracts/src/lib.rs index 003853102d..6d0b481dd0 100644 --- a/substrate/frame/contracts/src/lib.rs +++ b/substrate/frame/contracts/src/lib.rs @@ -85,6 +85,7 @@ mod storage; mod exec; mod wasm; mod rent; +mod benchmarking; #[cfg(test)] mod tests; @@ -107,7 +108,7 @@ use sp_runtime::{ RuntimeDebug, }; use frame_support::{ - decl_module, decl_event, decl_storage, decl_error, + decl_module, decl_event, decl_storage, decl_error, ensure, parameter_types, storage::child::ChildInfo, dispatch::{DispatchResult, DispatchResultWithPostInfo}, traits::{OnUnbalanced, Currency, Get, Time, Randomness}, @@ -420,6 +421,8 @@ decl_error! { /// for a tombstone to be created. Use `ext_terminate` to remove a contract without /// leaving a tombstone behind. InsufficientBalance, + /// The code supplied to `put_code` exceeds the limit specified in the current schedule. + CodeTooLarge, } } @@ -495,6 +498,7 @@ decl_module! { ) -> DispatchResult { ensure_signed(origin)?; let schedule = >::current_schedule(); + ensure!(code.len() as u32 <= schedule.max_code_size, Error::::CodeTooLarge); let result = wasm::save_code::(code, &schedule); if let Ok(code_hash) = result { Self::deposit_event(RawEvent::CodeStored(code_hash)); @@ -619,7 +623,7 @@ impl Module { address: T::AccountId, key: [u8; 32], ) -> sp_std::result::Result>, ContractAccessError> { - let contract_info = >::get(&address) + let contract_info = ContractInfoOf::::get(&address) .ok_or(ContractAccessError::DoesntExist)? .get_alive() .ok_or(ContractAccessError::IsTombstone)?; @@ -826,6 +830,10 @@ pub struct Schedule { /// The maximum length of a subject used for PRNG generation. pub max_subject_len: u32, + + /// The maximum length of a contract code in bytes. This limit applies to the uninstrumented + // and pristine form of the code as supplied to `put_code`. + pub max_code_size: u32, } // 500 (2 instructions per nano second on 2GHZ) * 1000x slowdown through wasmi @@ -857,6 +865,7 @@ impl Default for Schedule { max_table_size: 16 * 1024, enable_println: false, max_subject_len: 32, + max_code_size: 512 * 1024, } } } diff --git a/substrate/frame/contracts/src/tests.rs b/substrate/frame/contracts/src/tests.rs index 0d2a2f7a31..a2d85bb313 100644 --- a/substrate/frame/contracts/src/tests.rs +++ b/substrate/frame/contracts/src/tests.rs @@ -269,16 +269,12 @@ impl ExtBuilder { /// The fixture files are located under the `fixtures/` directory. fn compile_module( fixture_name: &str, -) -> Result<(Vec, ::Output), wabt::Error> +) -> wat::Result<(Vec, ::Output)> where T: frame_system::Trait, { - use std::fs; - let fixture_path = ["fixtures/", fixture_name, ".wat"].concat(); - let module_wat_source = - fs::read_to_string(&fixture_path).expect(&format!("Unable to find {} fixture", fixture_name)); - let wasm_binary = wabt::wat2wasm(module_wat_source)?; + let wasm_binary = wat::parse_file(fixture_path)?; let code_hash = T::Hashing::hash(&wasm_binary); Ok((wasm_binary, code_hash)) } diff --git a/substrate/frame/contracts/src/wasm/mod.rs b/substrate/frame/contracts/src/wasm/mod.rs index 500c0f4dcc..68dbae896b 100644 --- a/substrate/frame/contracts/src/wasm/mod.rs +++ b/substrate/frame/contracts/src/wasm/mod.rs @@ -157,7 +157,6 @@ mod tests { use crate::tests::{Test, Call}; use crate::wasm::prepare::prepare_contract; use crate::{CodeHash, BalanceOf}; - use wabt; use hex_literal::hex; use assert_matches::assert_matches; use sp_runtime::DispatchError; @@ -459,7 +458,7 @@ mod tests { ) -> ExecResult { use crate::exec::Vm; - let wasm = wabt::wat2wasm(wat).unwrap(); + let wasm = wat::parse_str(wat).unwrap(); let schedule = crate::Schedule::default(); let prefab_module = prepare_contract::(&wasm, &schedule).unwrap(); diff --git a/substrate/frame/contracts/src/wasm/prepare.rs b/substrate/frame/contracts/src/wasm/prepare.rs index 03f33f2dc6..2ffbe3bbdf 100644 --- a/substrate/frame/contracts/src/wasm/prepare.rs +++ b/substrate/frame/contracts/src/wasm/prepare.rs @@ -391,7 +391,6 @@ mod tests { use super::*; use crate::exec::Ext; use std::fmt; - use wabt; use assert_matches::assert_matches; impl fmt::Debug for PrefabWasmModule { @@ -417,7 +416,7 @@ mod tests { ($name:ident, $wat:expr, $($expected:tt)*) => { #[test] fn $name() { - let wasm = wabt::Wat2Wasm::new().validate(false).convert($wat).unwrap(); + let wasm = wat::parse_str($wat).unwrap(); let schedule = Schedule::default(); let r = prepare_contract::(wasm.as_ref(), &schedule); assert_matches!(r, $($expected)*); @@ -694,7 +693,7 @@ mod tests { #[test] fn ext_println_debug_enabled() { - let wasm = wabt::Wat2Wasm::new().validate(false).convert( + let wasm = wat::parse_str( r#" (module (import "env" "ext_println" (func $ext_println (param i32 i32)))