Skip to content

Commit

Permalink
[contracts] Add per local weight for function call (paritytech#12806)
Browse files Browse the repository at this point in the history
* Add per local weight for function call

* ".git/.scripts/bench-bot.sh" pallet dev pallet_contracts

* Update frame/contracts/src/benchmarking/mod.rs

Co-authored-by: Alexander Theißen <alex.theissen@me.com>

* apply suggestions from code review

* ".git/.scripts/bench-bot.sh" pallet dev pallet_contracts

* Update frame/contracts/src/benchmarking/mod.rs

Co-authored-by: Alexander Theißen <alex.theissen@me.com>

* tune the benchmark

* ".git/.scripts/bench-bot.sh" pallet dev pallet_contracts

* fix benches

* ".git/.scripts/bench-bot.sh" pallet dev pallet_contracts

Co-authored-by: command-bot <>
Co-authored-by: Alexander Theißen <alex.theissen@me.com>
  • Loading branch information
2 people authored and ark0f committed Feb 27, 2023
1 parent 610d7d9 commit d89e59b
Show file tree
Hide file tree
Showing 7 changed files with 1,383 additions and 1,267 deletions.
13 changes: 11 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion frame/contracts/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ codec = { package = "parity-scale-codec", version = "3.0.0", default-features =
] }
scale-info = { version = "2.1.1", default-features = false, features = ["derive"] }
log = { version = "0.4", default-features = false }
wasm-instrument = { version = "0.3", default-features = false }
wasm-instrument = { version = "0.4", default-features = false }
serde = { version = "1", optional = true, features = ["derive"] }
smallvec = { version = "1", default-features = false, features = [
"const_generics",
Expand Down
16 changes: 10 additions & 6 deletions frame/contracts/src/benchmarking/code.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,14 @@ use frame_support::traits::Get;
use sp_core::crypto::UncheckedFrom;
use sp_runtime::traits::Hash;
use sp_std::{borrow::ToOwned, prelude::*};
use wasm_instrument::parity_wasm::{
builder,
elements::{
self, BlockType, CustomSection, External, FuncBody, Instruction, Instructions, Module,
Section, ValueType,
use wasm_instrument::{
gas_metering,
parity_wasm::{
builder,
elements::{
self, BlockType, CustomSection, External, FuncBody, Instruction, Instructions, Module,
Section, ValueType,
},
},
};

Expand Down Expand Up @@ -541,7 +544,8 @@ where
fn inject_gas_metering<T: Config>(module: Module) -> Module {
let schedule = T::Schedule::get();
let gas_rules = schedule.rules(&module, Determinism::Deterministic);
wasm_instrument::gas_metering::inject(module, &gas_rules, "seal0").unwrap()
let backend = gas_metering::host_function::Injector::new("seal0", "gas");
gas_metering::inject(module, backend, &gas_rules).unwrap()
}

fn inject_stack_metering<T: Config>(module: Module) -> Module {
Expand Down
24 changes: 21 additions & 3 deletions frame/contracts/src/benchmarking/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2429,10 +2429,28 @@ benchmarks! {
sbox.invoke();
}

// w_per_local = w_bench
instr_call_per_local {
let l in 0 .. T::Schedule::get().limits.locals;
let mut aux_body = body::plain(vec![
Instruction::End,
]);
body::inject_locals(&mut aux_body, l);
let mut sbox = Sandbox::from(&WasmModule::<T>::from(ModuleDefinition {
aux_body: Some(aux_body),
call_body: Some(body::repeated(INSTR_BENCHMARK_BATCH_SIZE, &[
Instruction::Call(2), // call aux
])),
.. Default::default()
}));
}: {
sbox.invoke();
}

// w_local_get = w_bench - 1 * w_param
instr_local_get {
let r in 0 .. INSTR_BENCHMARK_BATCHES;
let max_locals = T::Schedule::get().limits.stack_height.unwrap_or(512);
let max_locals = T::Schedule::get().limits.locals;
let mut call_body = body::repeated_dyn(r * INSTR_BENCHMARK_BATCH_SIZE, vec![
RandomGetLocal(0, max_locals),
Regular(Instruction::Drop),
Expand All @@ -2449,7 +2467,7 @@ benchmarks! {
// w_local_set = w_bench - 1 * w_param
instr_local_set {
let r in 0 .. INSTR_BENCHMARK_BATCHES;
let max_locals = T::Schedule::get().limits.stack_height.unwrap_or(512);
let max_locals = T::Schedule::get().limits.locals;
let mut call_body = body::repeated_dyn(r * INSTR_BENCHMARK_BATCH_SIZE, vec![
RandomI64Repeated(1),
RandomSetLocal(0, max_locals),
Expand All @@ -2466,7 +2484,7 @@ benchmarks! {
// w_local_tee = w_bench - 2 * w_param
instr_local_tee {
let r in 0 .. INSTR_BENCHMARK_BATCHES;
let max_locals = T::Schedule::get().limits.stack_height.unwrap_or(512);
let max_locals = T::Schedule::get().limits.locals;
let mut call_body = body::repeated_dyn(r * INSTR_BENCHMARK_BATCH_SIZE, vec![
RandomI64Repeated(1),
RandomTeeLocal(0, max_locals),
Expand Down
13 changes: 13 additions & 0 deletions frame/contracts/src/schedule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,12 @@ pub struct Limits {
/// the linear memory limit `memory_pages` applies to them.
pub globals: u32,

/// Maximum number of locals a function can have.
///
/// As wasm engine initializes each of the local, we need to limit their number to confine
/// execution costs.
pub locals: u32,

/// Maximum numbers of parameters a function can have.
///
/// Those need to be limited to prevent a potentially exploitable interaction with
Expand Down Expand Up @@ -212,6 +218,7 @@ pub struct InstructionWeights<T: Config> {
pub call: u32,
pub call_indirect: u32,
pub call_indirect_per_param: u32,
pub call_per_local: u32,
pub local_get: u32,
pub local_set: u32,
pub local_tee: u32,
Expand Down Expand Up @@ -522,6 +529,7 @@ impl Default for Limits {
// No stack limit required because we use a runtime resident execution engine.
stack_height: None,
globals: 256,
locals: 1024,
parameters: 128,
memory_pages: 16,
// 4k function pointers (This is in count not bytes).
Expand Down Expand Up @@ -552,6 +560,7 @@ impl<T: Config> Default for InstructionWeights<T> {
call: cost_instr!(instr_call, 2),
call_indirect: cost_instr!(instr_call_indirect, 3),
call_indirect_per_param: cost_instr!(instr_call_indirect_per_param, 1),
call_per_local: cost_instr!(instr_call_per_local, 1),
local_get: cost_instr!(instr_local_get, 1),
local_set: cost_instr!(instr_local_set, 1),
local_tee: cost_instr!(instr_local_tee, 2),
Expand Down Expand Up @@ -792,6 +801,10 @@ impl<'a, T: Config> gas_metering::Rules for ScheduleRules<'a, T> {
// The cost for growing is therefore already included in the instruction cost.
gas_metering::MemoryGrowCost::Free
}

fn call_per_local_cost(&self) -> u32 {
self.schedule.instruction_weights.call_per_local
}
}

#[cfg(test)]
Expand Down
65 changes: 59 additions & 6 deletions frame/contracts/src/wasm/prepare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,9 @@ use codec::{Encode, MaxEncodedLen};
use sp_core::crypto::UncheckedFrom;
use sp_runtime::{traits::Hash, DispatchError};
use sp_std::prelude::*;
use wasm_instrument::parity_wasm::elements::{
self, External, Internal, MemoryType, Type, ValueType,
use wasm_instrument::{
gas_metering,
parity_wasm::elements::{self, External, Internal, MemoryType, Type, ValueType},
};
use wasmi::StackLimits;
use wasmparser::{Validator, WasmFeatures};
Expand Down Expand Up @@ -132,6 +133,19 @@ impl<'a, T: Config> ContractModule<'a, T> {
Ok(())
}

fn ensure_local_variable_limit(&self, limit: u32) -> Result<(), &'static str> {
if let Some(code_section) = self.module.code_section() {
for func_body in code_section.bodies() {
let locals_count: u32 =
func_body.locals().iter().map(|val_type| val_type.count()).sum();
if locals_count > limit {
return Err("single function declares too many locals")
}
}
}
Ok(())
}

/// Ensures that no floating point types are in use.
fn ensure_no_floating_types(&self) -> Result<(), &'static str> {
if let Some(global_section) = self.module.global_section() {
Expand Down Expand Up @@ -197,9 +211,9 @@ impl<'a, T: Config> ContractModule<'a, T> {

fn inject_gas_metering(self, determinism: Determinism) -> Result<Self, &'static str> {
let gas_rules = self.schedule.rules(&self.module, determinism);
let contract_module =
wasm_instrument::gas_metering::inject(self.module, &gas_rules, "seal0")
.map_err(|_| "gas instrumentation failed")?;
let backend = gas_metering::host_function::Injector::new("seal0", "gas");
let contract_module = gas_metering::inject(self.module, backend, &gas_rules)
.map_err(|_| "gas instrumentation failed")?;
Ok(ContractModule { module: contract_module, schedule: self.schedule })
}

Expand Down Expand Up @@ -422,6 +436,7 @@ where
contract_module.ensure_no_internal_memory()?;
contract_module.ensure_table_size_limit(schedule.limits.table_size)?;
contract_module.ensure_global_variable_limit(schedule.limits.globals)?;
contract_module.ensure_local_variable_limit(schedule.limits.locals)?;
contract_module.ensure_parameter_limit(schedule.limits.parameters)?;
contract_module.ensure_br_table_size_limit(schedule.limits.br_table_size)?;

Expand Down Expand Up @@ -636,7 +651,8 @@ mod tests {
let wasm = wat::parse_str($wat).unwrap().try_into().unwrap();
let schedule = Schedule {
limits: Limits {
globals: 3,
globals: 3,
locals: 3,
parameters: 3,
memory_pages: 16,
table_size: 3,
Expand Down Expand Up @@ -736,6 +752,43 @@ mod tests {
);
}

mod locals {
use super::*;

prepare_test!(
local_number_valid,
r#"
(module
(func
(local i32)
(local i32)
(local i32)
)
(func (export "call"))
(func (export "deploy"))
)
"#,
Ok(_)
);

prepare_test!(
local_number_too_high,
r#"
(module
(func
(local i32)
(local i32)
(local i32)
(local i32)
)
(func (export "call"))
(func (export "deploy"))
)
"#,
Err("single function declares too many locals")
);
}

mod memories {
use super::*;

Expand Down
Loading

0 comments on commit d89e59b

Please sign in to comment.