address review comments

frame/contracts/proc-macro/src/lib.rs

@@ -626,13 +626,11 @@ fn expand_functions(def: &EnvDef, expand_blocks: bool, host_state: TokenStream2)
 			quote! {
 				let result = #body;
 				if ::log::log_enabled!(target: "runtime::contracts::strace", ::log::Level::Trace) {
-					{
 						use sp_std::fmt::Write;
 						let mut w = sp_std::Writer::default();
 						let _ = core::write!(&mut w, #trace_fmt_str, #( #trace_fmt_args, )* result);
 						let msg = core::str::from_utf8(&w.inner()).unwrap_or_default();
 						ctx.ext().append_debug_buffer(msg);
-					}
 				}
 				result
 			}

frame/contracts/src/benchmarking/code.rs

@@ -358,31 +358,6 @@ impl<T: Config> WasmModule<T> {
 		.into()
 	}
 
-	#[allow(dead_code)]
-	pub fn unary_instr(instr: Instruction, repeat: u32) -> Self {
-		use body::DynInstr::{RandomI64Repeated, Regular};
-		ModuleDefinition {
-			call_body: Some(body::repeated_dyn(
-				repeat,
-				vec![RandomI64Repeated(1), Regular(instr), Regular(Instruction::Drop)],
-			)),
-			..Default::default()
-		}
-		.into()
-	}
-
-	#[allow(dead_code)]
-	pub fn binary_instr(instr: Instruction, repeat: u32) -> Self {
-		use body::DynInstr::{RandomI64Repeated, Regular};
-		ModuleDefinition {
-			call_body: Some(body::repeated_dyn(
-				repeat,
-				vec![RandomI64Repeated(2), Regular(instr), Regular(Instruction::Drop)],
-			)),
-			..Default::default()
-		}
-		.into()
-	}
 }
 
 /// Mechanisms to generate a function body that can be used inside a `ModuleDefinition`.
@@ -392,7 +367,6 @@ pub mod body {
 	/// When generating contract code by repeating a wasm sequence, it's sometimes necessary
 	/// to change those instructions on each repetition. The variants of this enum describe
 	/// various ways in which this can happen.
-	#[allow(dead_code)]
 	pub enum DynInstr {
 		/// Insert the associated instruction.
 		Regular(Instruction),

frame/contracts/src/exec.rs

@@ -331,7 +331,6 @@ pub trait Executable<T: Config>: Sized {
 	/// Charges size base load weight from the gas meter.
 	fn from_storage(
 		code_hash: CodeHash<T>,
-		schedule: &Schedule<T>,
 		gas_meter: &mut GasMeter<T>,
 	) -> Result<Self, DispatchError>;
 
@@ -348,7 +347,6 @@ pub trait Executable<T: Config>: Sized {
 	fn remove_user(code_hash: CodeHash<T>);
 
 	/// Execute the specified exported function and return the result.
-	/// `reftime_limit` is passed to the execution engine to account for gas.
 	///
 	/// When the specified function is `Constructor` the executable is stored and its
 	/// refcount incremented.
@@ -367,7 +365,7 @@ pub trait Executable<T: Config>: Sized {
 	/// The code hash of the executable.
 	fn code_hash(&self) -> CodeHash<T>;
 
-	/// Size of the conract code in bytes.
+	/// Size of the contract code in bytes.
 	fn code_len(&self) -> u32;
 
 	/// The code does not contain any instructions which could lead to indeterminism.
@@ -707,7 +705,6 @@ where
 			Weight::zero(),
 			storage_meter,
 			BalanceOf::<T>::zero(),
-			schedule,
 			determinism,
 		)?;
 
@@ -740,7 +737,6 @@ where
 		gas_limit: Weight,
 		storage_meter: &mut storage::meter::GenericMeter<T, S>,
 		deposit_limit: BalanceOf<T>,
-		schedule: &Schedule<T>,
 		determinism: Determinism,
 	) -> Result<(Frame<T>, E, Option<u64>), ExecError> {
 		let (account_id, contract_info, executable, delegate_caller, entry_point, nonce) =
@@ -756,7 +752,7 @@ where
 						if let Some(DelegatedCall { executable, caller }) = delegated_call {
 							(executable, Some(caller))
 						} else {
-							(E::from_storage(contract.code_hash, schedule, gas_meter)?, None)
+							(E::from_storage(contract.code_hash, gas_meter)?, None)
 						};
 
 					(dest, contract, executable, delegate_caller, ExportedFunction::Call, None)
@@ -836,7 +832,6 @@ where
 			gas_limit,
 			nested_storage,
 			deposit_limit,
-			self.schedule,
 			self.determinism,
 		)?;
 		self.frames.push(frame);
@@ -1198,7 +1193,7 @@ where
 		code_hash: CodeHash<Self::T>,
 		input_data: Vec<u8>,
 	) -> Result<ExecReturnValue, ExecError> {
-		let executable = E::from_storage(code_hash, self.schedule, self.gas_meter_mut())?;
+		let executable = E::from_storage(code_hash, self.gas_meter_mut())?;
 		let top_frame = self.top_frame_mut();
 		let contract_info = top_frame.contract_info().clone();
 		let account_id = top_frame.account_id.clone();
@@ -1225,7 +1220,7 @@ where
 		input_data: Vec<u8>,
 		salt: &[u8],
 	) -> Result<(AccountIdOf<T>, ExecReturnValue), ExecError> {
-		let executable = E::from_storage(code_hash, self.schedule, self.gas_meter_mut())?;
+		let executable = E::from_storage(code_hash, self.gas_meter_mut())?;
 		let nonce = self.next_nonce();
 		let executable = self.push_frame(
 			FrameArgs::Instantiate {
@@ -1432,7 +1427,7 @@ where
 
 	fn set_code_hash(&mut self, hash: CodeHash<Self::T>) -> Result<(), DispatchError> {
 		let frame = top_frame_mut!(self);
-		if !E::from_storage(hash, self.schedule, &mut frame.nested_gas)?.is_deterministic() {
+		if !E::from_storage(hash, &mut frame.nested_gas)?.is_deterministic() {
 			return Err(<Error<T>>::Indeterministic.into())
 		}
 		E::add_user(hash)?;
@@ -1600,7 +1595,6 @@ mod tests {
 	impl Executable<Test> for MockExecutable {
 		fn from_storage(
 			code_hash: CodeHash<Test>,
-			_schedule: &Schedule<Test>,
 			_gas_meter: &mut GasMeter<Test>,
 		) -> Result<Self, DispatchError> {
 			Loader::mutate(|loader| {
@@ -1962,7 +1956,7 @@ mod tests {
 			let min_balance = <Test as Config>::Currency::minimum_balance();
 			let mut gas_meter = GasMeter::<Test>::new(GAS_LIMIT);
 			let executable =
-				MockExecutable::from_storage(input_data_ch, &schedule, &mut gas_meter).unwrap();
+				MockExecutable::from_storage(input_data_ch, &mut gas_meter).unwrap();
 			set_balance(&ALICE, min_balance * 10_000);
 			let contract_origin = Origin::from_account_id(ALICE);
 			let mut storage_meter =
@@ -2376,7 +2370,7 @@ mod tests {
 			let schedule = <Test as Config>::Schedule::get();
 			let mut gas_meter = GasMeter::<Test>::new(GAS_LIMIT);
 			let executable =
-				MockExecutable::from_storage(dummy_ch, &schedule, &mut gas_meter).unwrap();
+				MockExecutable::from_storage(dummy_ch, &mut gas_meter).unwrap();
 			let contract_origin = Origin::from_account_id(ALICE);
 			let mut storage_meter =
 				storage::meter::Meter::new(&contract_origin, Some(0), 0).unwrap();
@@ -2409,7 +2403,7 @@ mod tests {
 			let min_balance = <Test as Config>::Currency::minimum_balance();
 			let mut gas_meter = GasMeter::<Test>::new(GAS_LIMIT);
 			let executable =
-				MockExecutable::from_storage(dummy_ch, &schedule, &mut gas_meter).unwrap();
+				MockExecutable::from_storage(dummy_ch, &mut gas_meter).unwrap();
 			set_balance(&ALICE, min_balance * 1000);
 			let contract_origin = Origin::from_account_id(ALICE);
 			let mut storage_meter =
@@ -2455,7 +2449,7 @@ mod tests {
 			let min_balance = <Test as Config>::Currency::minimum_balance();
 			let mut gas_meter = GasMeter::<Test>::new(GAS_LIMIT);
 			let executable =
-				MockExecutable::from_storage(dummy_ch, &schedule, &mut gas_meter).unwrap();
+				MockExecutable::from_storage(dummy_ch, &mut gas_meter).unwrap();
 			set_balance(&ALICE, min_balance * 1000);
 			let contract_origin = Origin::from_account_id(ALICE);
 			let mut storage_meter =
@@ -2624,7 +2618,7 @@ mod tests {
 			let schedule = <Test as Config>::Schedule::get();
 			let mut gas_meter = GasMeter::<Test>::new(GAS_LIMIT);
 			let executable =
-				MockExecutable::from_storage(terminate_ch, &schedule, &mut gas_meter).unwrap();
+				MockExecutable::from_storage(terminate_ch, &mut gas_meter).unwrap();
 			set_balance(&ALICE, 10_000);
 			let contract_origin = Origin::from_account_id(ALICE);
 			let mut storage_meter =
@@ -2726,7 +2720,7 @@ mod tests {
 			let schedule = <Test as Config>::Schedule::get();
 			let min_balance = <Test as Config>::Currency::minimum_balance();
 			let mut gas_meter = GasMeter::<Test>::new(GAS_LIMIT);
-			let executable = MockExecutable::from_storage(code, &schedule, &mut gas_meter).unwrap();
+			let executable = MockExecutable::from_storage(code, &mut gas_meter).unwrap();
 			set_balance(&ALICE, min_balance * 10_000);
 			let contract_origin = Origin::from_account_id(ALICE);
 			let mut storage_meter =
@@ -3151,13 +3145,13 @@ mod tests {
 			let min_balance = <Test as Config>::Currency::minimum_balance();
 			let mut gas_meter = GasMeter::<Test>::new(GAS_LIMIT);
 			let fail_executable =
-				MockExecutable::from_storage(fail_code, &schedule, &mut gas_meter).unwrap();
+				MockExecutable::from_storage(fail_code, &mut gas_meter).unwrap();
 			let success_executable =
-				MockExecutable::from_storage(success_code, &schedule, &mut gas_meter).unwrap();
+				MockExecutable::from_storage(success_code, &mut gas_meter).unwrap();
 			let succ_fail_executable =
-				MockExecutable::from_storage(succ_fail_code, &schedule, &mut gas_meter).unwrap();
+				MockExecutable::from_storage(succ_fail_code, &mut gas_meter).unwrap();
 			let succ_succ_executable =
-				MockExecutable::from_storage(succ_succ_code, &schedule, &mut gas_meter).unwrap();
+				MockExecutable::from_storage(succ_succ_code, &mut gas_meter).unwrap();
 			set_balance(&ALICE, min_balance * 10_000);
 			let contract_origin = Origin::from_account_id(ALICE);
 			let mut storage_meter =

frame/contracts/src/gas.rs

@@ -230,11 +230,6 @@ impl<T: Config> GasMeter<T> {
 		self.gas_left
 	}
 
-	/// Returns current tracked engine fuel consumption.
-	pub fn engine_consumed(&self) -> u64 {
-		self.engine_consumed
-	}
-
 	/// Turn this GasMeter into a DispatchResult that contains the actually used gas.
 	pub fn into_dispatch_result<R, E>(
 		self,

frame/contracts/src/lib.rs

@@ -360,17 +360,17 @@ pub mod pallet {
 
 			// Check that given configured `MaxCodeLen`, runtime heap memory limit can't be broken.
 			//
-			// In worst case, the decoded wasm contract code would be `x16` times larger than the
+			// In worst case, the decoded Wasm contract code would be `x16` times larger than the
 			// encoded one. This is because even a single-byte wasm instruction has 16-byte size in
 			// wasmi. This gives us `MaxCodeLen*16` safety margin.
 			//
 			// Next, the pallet keeps the Wasm blob for each
-			// contract, hence we add up one `MaxCodeLen` more to the safety margin.
+			// contract, hence we add up `MaxCodeLen` to the safety margin.
 			//
 			// Finally, the inefficiencies of the freeing-bump allocator
 			// being used in the client for the runtime memory allocations, could lead to possible
 			// memory allocations for contract code grow up to `x4` times in some extreme cases,
-			// which gives us total multiplier of `18*4` for `MaxCodeLen`.
+			// which gives us total multiplier of `17*4` for `MaxCodeLen`.
 			//
 			// That being said, for every contract executed in runtime, at least `MaxCodeLen*17*4`
 			// memory should be available. Note that maximum allowed heap memory and stack size per
@@ -1233,7 +1233,7 @@ impl<T: Config> Invokable<T> for InstantiateInput<T> {
 					(executable.open_deposit(code_info), executable)
 				},
 				Code::Existing(hash) =>
-					(Default::default(), WasmBlob::from_storage(*hash, &schedule, &mut gas_meter)?),
+					(Default::default(), WasmBlob::from_storage(*hash, &mut gas_meter)?),
 			};
 			let contract_origin = Origin::from_account_id(origin.clone());
 			let mut storage_meter = StorageMeter::new(

frame/contracts/src/migration/v12.rs

@@ -199,7 +199,7 @@ impl<T: Config> Migrate for Migration<T> {
 			} else {
 				log::warn!(
 					target: LOG_TARGET,
-					"new deposit: {:?} is more than old deposit: {:?}",
+					"new deposit: {:?} >= old deposit: {:?}",
 					&info.deposit,
 					&old.deposit
 				);
@@ -289,8 +289,7 @@ impl<T: Config> Migrate for Migration<T> {
 		PristineCode::<T>::iter().for_each(|(_k, v)| {
 			storage_code += v.len() as u32;
 		});
-		let old_deposit = state.1;
-		let storage_module = state.2;
+		let (_, old_deposit, storage_module) = state;
 		// CodeInfoOf::max_encoded_len == OwnerInfoOf::max_encoded_len + 1
 		// I.e. code info adds up 1 byte per record.
 		let info_bytes_added = items.clone();

frame/contracts/src/tests.rs

@@ -123,7 +123,7 @@ pub mod test_utils {
 		<<Test as SysConfig>::Hashing as Hash>::hash_of(s)
 	}
 	pub fn expected_deposit(code_len: usize) -> u64 {
-		// For onwer info, the deposit for max_encoded_len is taken.
+		// For code_info, the deposit for max_encoded_len is taken.
 		let code_info_len = CodeInfo::<Test>::max_encoded_len() as u64;
 		// Calculate deposit to be reserved.
 		// We add 2 storage items: one for code, other for code_info

frame/contracts/src/wasm/mod.rs

@@ -125,26 +125,20 @@ impl ExportedFunction {
 	}
 }
 
-/// Costs for operations that are related to code handling.
+/// Cost of code loading from storage.
 #[cfg_attr(test, derive(Debug, PartialEq, Eq))]
 #[derive(Clone, Copy)]
-enum CodeToken {
-	/// Weight for loading a contract per byte.
-	Load(u32),
-}
+struct CodeLoadToken(u32);
 
-impl<T: Config> Token<T> for CodeToken {
+impl<T: Config> Token<T> for CodeLoadToken {
 	fn weight(&self) -> Weight {
-		use self::CodeToken::*;
-		// In case of `Load` we already covered the general costs of
+		// When loading the contract, we already covered the general costs of
 		// calling the storage but still need to account for the actual size of the
 		// contract code. This is why we subtract `T::*::(0)`. We need to do this at this
 		// point because when charging the general weight for calling the contract we don't know the
 		// size of the contract.
-		match *self {
-			Load(len) => T::WeightInfo::call_with_code_per_byte(len)
-				.saturating_sub(T::WeightInfo::call_with_code_per_byte(0)),
-		}
+		T::WeightInfo::call_with_code_per_byte(self.0)
+			.saturating_sub(T::WeightInfo::call_with_code_per_byte(0))
 	}
 }
 
@@ -316,11 +310,11 @@ impl<T: Config> WasmBlob<T> {
 		gas_meter: &mut GasMeter<T>,
 	) -> Result<CodeVec<T>, DispatchError> {
 		let max_code_len = T::MaxCodeLen::get();
-		let charged = gas_meter.charge(CodeToken::Load(max_code_len))?;
+		let charged = gas_meter.charge(CodeLoadToken(max_code_len))?;
 
 		let code = <PristineCode<T>>::get(code_hash).ok_or(Error::<T>::CodeNotFound)?;
 		let code_len = code.len() as u32;
-		gas_meter.adjust_gas(charged, CodeToken::Load(code_len));
+		gas_meter.adjust_gas(charged, CodeLoadToken(code_len));
 
 		Ok(code)
 	}
@@ -395,7 +389,6 @@ impl<T: Config> CodeInfo<T> {
 impl<T: Config> Executable<T> for WasmBlob<T> {
 	fn from_storage(
 		code_hash: CodeHash<T>,
-		_schedule: &Schedule<T>,
 		gas_meter: &mut GasMeter<T>,
 	) -> Result<Self, DispatchError> {
 		let code = Self::load_code(code_hash, gas_meter)?;

frame/contracts/src/wasm/runtime.rs

@@ -67,7 +67,6 @@ pub trait Environment<HostState> {
 }
 
 /// Type of a storage key.
-#[allow(dead_code)]
 enum KeyType {
 	/// Legacy fix sized key `[u8;32]`.
 	Fix,