Contracts: Only exec parsed code in benchmarks (#3915)

[Weights
compare](https://weights.tasty.limo/compare?unit=weight&ignore_errors=true&threshold=10&method=asymptotic&repo=polkadot-sdk&old=master&new=pg%2Fbench_tweaks&path_pattern=substrate%2Fframe%2F**%2Fsrc%2Fweights.rs%2Cpolkadot%2Fruntime%2F*%2Fsrc%2Fweights%2F**%2F*.rs%2Cpolkadot%2Fbridges%2Fmodules%2F*%2Fsrc%2Fweights.rs%2Ccumulus%2F**%2Fweights%2F*.rs%2Ccumulus%2F**%2Fweights%2Fxcm%2F*.rs%2Ccumulus%2F**%2Fsrc%2Fweights.rs)

Note: Raw weights change does not mean much here, as this PR reduce the
scope of what is benchmarked, they are therefore decreased by a good
margin. One should instead print the Schedule using

cargo test --features runtime-benchmarks bench_print_schedule --
--nocapture
or following the instructions from the
[README](https://github.com/paritytech/polkadot-sdk/tree/pg/bench_tweaks/substrate/frame/contracts#schedule)
for looking at the Schedule of a specific runtime

---------

Co-authored-by: command-bot <>

prdoc/pr_3915.prdoc

@@ -0,0 +1,14 @@
+# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
+# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json
+
+title: "[pallet-contracts] Weights update"
+
+doc:
+  - audience: Runtime Dev
+    description: |
+      Update Host functions benchmarks, instead of benchmarking the whole call extrinsic, this PR solely benchmark the execution of the Host function.
+      Previously, some benchmarks would overestimate the weight as both the parsing and execution of the contract were included in the benchmark.
+
+crates:
+- name: pallet-contracts
+  bump: patch

substrate/frame/contracts/fixtures/contracts/return_with_data.rs

@@ -38,6 +38,10 @@ pub extern "C" fn call() {
 		output: [u8],
 	);
 
+	// Burn some PoV, clear_storage consumes some PoV as in order to clear the storage we need to we
+	// need to read its size first.
+	api::clear_storage_v1(b"");
+
 	let exit_status = uapi::ReturnFlags::from_bits(exit_status[0] as u32).unwrap();
 	api::return_value(exit_status, output);
 }

substrate/frame/contracts/src/benchmarking/call_builder.rs

@@ -0,0 +1,170 @@
+// This file is part of Substrate.
+
+// Copyright (C) Parity Technologies (UK) Ltd.
+// SPDX-License-Identifier: Apache-2.0
+
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// 	http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+use crate::{
+	benchmarking::{Contract, WasmModule},
+	exec::Stack,
+	storage::meter::Meter,
+	wasm::Runtime,
+	BalanceOf, Config, DebugBufferVec, Determinism, ExecReturnValue, GasMeter, Origin, Schedule,
+	TypeInfo, WasmBlob, Weight,
+};
+use codec::{Encode, HasCompact};
+use core::fmt::Debug;
+use sp_core::Get;
+use sp_std::prelude::*;
+
+type StackExt<'a, T> = Stack<'a, T, WasmBlob<T>>;
+
+/// A prepared contract call ready to be executed.
+pub struct PreparedCall<'a, T: Config> {
+	func: wasmi::Func,
+	store: wasmi::Store<Runtime<'a, StackExt<'a, T>>>,
+}
+
+impl<'a, T: Config> PreparedCall<'a, T> {
+	pub fn call(mut self) -> ExecReturnValue {
+		let result = self.func.call(&mut self.store, &[], &mut []);
+		WasmBlob::<T>::process_result(self.store, result).unwrap()
+	}
+}
+
+/// A builder used to prepare a contract call.
+pub struct CallSetup<T: Config> {
+	contract: Contract<T>,
+	dest: T::AccountId,
+	origin: Origin<T>,
+	gas_meter: GasMeter<T>,
+	storage_meter: Meter<T>,
+	schedule: Schedule<T>,
+	value: BalanceOf<T>,
+	debug_message: Option<DebugBufferVec<T>>,
+	determinism: Determinism,
+	data: Vec<u8>,
+}
+
+impl<T> CallSetup<T>
+where
+	T: Config + pallet_balances::Config,
+	<BalanceOf<T> as HasCompact>::Type: Clone + Eq + PartialEq + Debug + TypeInfo + Encode,
+{
+	/// Setup a new call for the given module.
+	pub fn new(module: WasmModule<T>) -> Self {
+		let contract = Contract::<T>::new(module.clone(), vec![]).unwrap();
+		let dest = contract.account_id.clone();
+		let origin = Origin::from_account_id(contract.caller.clone());
+
+		let storage_meter = Meter::new(&origin, None, 0u32.into()).unwrap();
+
+		Self {
+			contract,
+			dest,
+			origin,
+			gas_meter: GasMeter::new(Weight::MAX),
+			storage_meter,
+			schedule: T::Schedule::get(),
+			value: 0u32.into(),
+			debug_message: None,
+			determinism: Determinism::Enforced,
+			data: vec![],
+		}
+	}
+
+	/// Set the meter's storage deposit limit.
+	pub fn set_storage_deposit_limit(&mut self, balance: BalanceOf<T>) {
+		self.storage_meter = Meter::new(&self.origin, Some(balance), 0u32.into()).unwrap();
+	}
+
+	/// Set the call's origin.
+	pub fn set_origin(&mut self, origin: Origin<T>) {
+		self.origin = origin;
+	}
+
+	/// Set the contract's balance.
+	pub fn set_balance(&mut self, value: BalanceOf<T>) {
+		self.contract.set_balance(value);
+	}
+
+	/// Set the call's input data.
+	pub fn set_data(&mut self, value: Vec<u8>) {
+		self.data = value;
+	}
+
+	/// Set the debug message.
+	pub fn enable_debug_message(&mut self) {
+		self.debug_message = Some(Default::default());
+	}
+
+	/// Get the debug message.
+	pub fn debug_message(&self) -> Option<DebugBufferVec<T>> {
+		self.debug_message.clone()
+	}
+
+	/// Get the call's input data.
+	pub fn data(&self) -> Vec<u8> {
+		self.data.clone()
+	}
+
+	/// Get the call's contract.
+	pub fn contract(&self) -> Contract<T> {
+		self.contract.clone()
+	}
+
+	/// Build the call stack.
+	pub fn ext(&mut self) -> (StackExt<'_, T>, WasmBlob<T>) {
+		StackExt::bench_new_call(
+			self.dest.clone(),
+			self.origin.clone(),
+			&mut self.gas_meter,
+			&mut self.storage_meter,
+			&self.schedule,
+			self.value,
+			self.debug_message.as_mut(),
+			self.determinism,
+		)
+	}
+
+	/// Prepare a call to the module.
+	pub fn prepare_call<'a>(
+		ext: &'a mut StackExt<'a, T>,
+		module: WasmBlob<T>,
+		input: Vec<u8>,
+	) -> PreparedCall<'a, T> {
+		let (func, store) = module.bench_prepare_call(ext, input);
+		PreparedCall { func, store }
+	}
+}
+
+#[macro_export]
+macro_rules! call_builder(
+	($func: ident, $module:expr) => {
+		$crate::call_builder!($func, _contract, $module);
+	};
+	($func: ident, $contract: ident, $module:expr) => {
+		let mut setup = CallSetup::<T>::new($module);
+		$crate::call_builder!($func, $contract, setup: setup);
+	};
+    ($func:ident, setup: $setup: ident) => {
+		$crate::call_builder!($func, _contract, setup: $setup);
+	};
+    ($func:ident, $contract: ident, setup: $setup: ident) => {
+		let data = $setup.data();
+		let $contract = $setup.contract();
+		let (mut ext, module) = $setup.ext();
+		let $func = CallSetup::<T>::prepare_call(&mut ext, module, data);
+	};
+);

substrate/frame/contracts/src/exec.rs

@@ -733,6 +733,30 @@ where
 		stack.run(executable, input_data).map(|ret| (account_id, ret))
 	}
 
+	#[cfg(feature = "runtime-benchmarks")]
+	pub fn bench_new_call(
+		dest: T::AccountId,
+		origin: Origin<T>,
+		gas_meter: &'a mut GasMeter<T>,
+		storage_meter: &'a mut storage::meter::Meter<T>,
+		schedule: &'a Schedule<T>,
+		value: BalanceOf<T>,
+		debug_message: Option<&'a mut DebugBufferVec<T>>,
+		determinism: Determinism,
+	) -> (Self, E) {
+		Self::new(
+			FrameArgs::Call { dest, cached_info: None, delegated_call: None },
+			origin,
+			gas_meter,
+			storage_meter,
+			schedule,
+			value,
+			debug_message,
+			determinism,
+		)
+		.unwrap()
+	}
+
 	/// Create a new call stack.
 	fn new(
 		args: FrameArgs<T, E>,

substrate/frame/contracts/src/wasm/mod.rs

@@ -338,26 +338,49 @@ impl<T: Config> CodeInfo<T> {
 	}
 }
 
-impl<T: Config> Executable<T> for WasmBlob<T> {
-	fn from_storage(
-		code_hash: CodeHash<T>,
-		gas_meter: &mut GasMeter<T>,
-	) -> Result<Self, DispatchError> {
-		let code_info = <CodeInfoOf<T>>::get(code_hash).ok_or(Error::<T>::CodeNotFound)?;
-		gas_meter.charge(CodeLoadToken(code_info.code_len))?;
-		let code = <PristineCode<T>>::get(code_hash).ok_or(Error::<T>::CodeNotFound)?;
-		Ok(Self { code, code_info, code_hash })
+use crate::{ExecError, ExecReturnValue};
+use wasmi::Func;
+enum InstanceOrExecReturn<'a, E: Ext> {
+	Instance((Func, Store<Runtime<'a, E>>)),
+	ExecReturn(ExecReturnValue),
+}
+
+type PreExecResult<'a, E> = Result<InstanceOrExecReturn<'a, E>, ExecError>;
+
+impl<T: Config> WasmBlob<T> {
+	/// Sync the frame's gas meter with the engine's one.
+	pub fn process_result<E: Ext<T = T>>(
+		mut store: Store<Runtime<E>>,
+		result: Result<(), wasmi::Error>,
+	) -> ExecResult {
+		let engine_consumed_total = store.fuel_consumed().expect("Fuel metering is enabled; qed");
+		let gas_meter = store.data_mut().ext().gas_meter_mut();
+		let _ = gas_meter.sync_from_executor(engine_consumed_total)?;
+		store.into_data().to_execution_result(result)
 	}
 
-	fn execute<E: Ext<T = T>>(
+	#[cfg(feature = "runtime-benchmarks")]
+	pub fn bench_prepare_call<E: Ext<T = T>>(
 		self,
 		ext: &mut E,
-		function: &ExportedFunction,
 		input_data: Vec<u8>,
-	) -> ExecResult {
+	) -> (Func, Store<Runtime<E>>) {
+		use InstanceOrExecReturn::*;
+		match Self::prepare_execute(self, Runtime::new(ext, input_data), &ExportedFunction::Call)
+			.expect("Benchmark should provide valid module")
+		{
+			Instance((func, store)) => (func, store),
+			ExecReturn(_) => panic!("Expected Instance"),
+		}
+	}
+
+	fn prepare_execute<'a, E: Ext<T = T>>(
+		self,
+		runtime: Runtime<'a, E>,
+		function: &'a ExportedFunction,
+	) -> PreExecResult<'a, E> {
 		let code = self.code.as_slice();
 		// Instantiate the Wasm module to the engine.
-		let runtime = Runtime::new(ext, input_data);
 		let schedule = <T>::Schedule::get();
 		let (mut store, memory, instance) = Self::instantiate::<crate::wasm::runtime::Env, _>(
 			code,
@@ -390,15 +413,6 @@ impl<T: Config> Executable<T> for WasmBlob<T> {
 			.add_fuel(fuel_limit)
 			.expect("We've set up engine to fuel consuming mode; qed");
 
-		// Sync this frame's gas meter with the engine's one.
-		let process_result = |mut store: Store<Runtime<E>>, result| {
-			let engine_consumed_total =
-				store.fuel_consumed().expect("Fuel metering is enabled; qed");
-			let gas_meter = store.data_mut().ext().gas_meter_mut();
-			let _ = gas_meter.sync_from_executor(engine_consumed_total)?;
-			store.into_data().to_execution_result(result)
-		};
-
 		// Start function should already see the correct refcount in case it will be ever inspected.
 		if let &ExportedFunction::Constructor = function {
 			E::increment_refcount(self.code_hash)?;
@@ -417,10 +431,37 @@ impl<T: Config> Executable<T> for WasmBlob<T> {
 						Error::<T>::CodeRejected
 					})?;
 
-				let result = exported_func.call(&mut store, &[], &mut []);
-				process_result(store, result)
+				Ok(InstanceOrExecReturn::Instance((exported_func, store)))
 			},
-			Err(err) => process_result(store, Err(err)),
+			Err(err) => Self::process_result(store, Err(err)).map(InstanceOrExecReturn::ExecReturn),
+		}
+	}
+}
+
+impl<T: Config> Executable<T> for WasmBlob<T> {
+	fn from_storage(
+		code_hash: CodeHash<T>,
+		gas_meter: &mut GasMeter<T>,
+	) -> Result<Self, DispatchError> {
+		let code_info = <CodeInfoOf<T>>::get(code_hash).ok_or(Error::<T>::CodeNotFound)?;
+		gas_meter.charge(CodeLoadToken(code_info.code_len))?;
+		let code = <PristineCode<T>>::get(code_hash).ok_or(Error::<T>::CodeNotFound)?;
+		Ok(Self { code, code_info, code_hash })
+	}
+
+	fn execute<E: Ext<T = T>>(
+		self,
+		ext: &mut E,
+		function: &ExportedFunction,
+		input_data: Vec<u8>,
+	) -> ExecResult {
+		use InstanceOrExecReturn::*;
+		match Self::prepare_execute(self, Runtime::new(ext, input_data), function)? {
+			Instance((func, mut store)) => {
+				let result = func.call(&mut store, &[], &mut []);
+				Self::process_result(store, result)
+			},
+			ExecReturn(exec_return) => Ok(exec_return),
 		}
 	}
 
@@ -1780,6 +1821,7 @@ mod tests {
 	const CODE_GAS_LEFT: &str = r#"
 (module
 	(import "seal1" "gas_left" (func $seal_gas_left (param i32 i32)))
+	(import "seal0" "clear_storage" (func $clear_storage (param i32)))
 	(import "seal0" "seal_return" (func $seal_return (param i32 i32 i32)))
 	(import "env" "memory" (memory 1 1))
 
@@ -1796,6 +1838,9 @@ mod tests {
 	)
 
 	(func (export "call")
+		;; Burn some PoV, clear_storage consumes some PoV as in order to clear the storage we need to we need to read its size first.
+		(call $clear_storage (i32.const 0))
+
 		;; This stores the weight left to the buffer
 		(call $seal_gas_left (i32.const 0) (i32.const 20))
 
@@ -1807,6 +1852,9 @@ mod tests {
 			)
 		)
 
+		;; Burn some PoV, clear_storage consumes some PoV as in order to clear the storage we need to we need to read its size first.
+		(call $clear_storage (i32.const 0))
+
 		;; Return weight left and its encoded value len
 		(call $seal_return (i32.const 0) (i32.const 0) (i32.load (i32.const 20)))