remove gas() host fn, continue clean-up

save

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

@@ -681,15 +681,15 @@ fn expand_functions(def: &EnvDef, expand_blocks: bool, host_state: TokenStream2)
 				let gas_before = {
 					let engine_consumed = __caller__.fuel_consumed().expect("Fuel metering is enabled; qed");
 					let reftime_consumed = engine_consumed.saturating_mul(<E::T as Config>::Schedule::get().instruction_weights.base as u64);
-				   __caller__.data_mut().ext.gas_meter().sync_reftime(reftime_consumed).map_err(#into_trap).map_err(#into_host)?
+				   __caller__.data_mut().ext.gas_meter_mut().sync_reftime(reftime_consumed).map_err(#into_trap).map_err(#into_host)?
 				};
 			}
 		} else {
 			quote! { }
 		};
 		let sync_gas_after = if expand_blocks {
 			quote! {
-				let gas_after = __caller__.data().ext.gas_meter_immut().gas_left().ref_time();
+				let gas_after = __caller__.data().ext.gas_meter().gas_left().ref_time();
 				let host_consumed = gas_before.saturating_sub(gas_after);
 				let fuel_consumed = host_consumed.checked_div(<E::T as Config>::Schedule::get().instruction_weights.base as u64).ok_or(Error::<E::T>::InvalidSchedule).map_err(#into_trap).map_err(#into_host)?;
 				__caller__.consume_fuel(fuel_consumed).map_err(|_| TrapReason::from(Error::<E::T>::OutOfGas)).map_err(#into_host)?;

frame/contracts/src/exec.rs

@@ -135,7 +135,7 @@ pub trait Ext: sealing::Sealed {
 
 	/// Call (possibly transferring some amount of funds) into the specified account.
 	///
-	/// Returns the original code size of the called contract.
+	/// Returns the code size of the called contract.
 	fn call(
 		&mut self,
 		gas_limit: Weight,
@@ -148,7 +148,7 @@ pub trait Ext: sealing::Sealed {
 
 	/// Execute code in the current frame.
 	///
-	/// Returns the original code size of the called contract.
+	/// Returns the code size of the called contract.
 	fn delegate_call(
 		&mut self,
 		code: CodeHash<Self::T>,
@@ -159,7 +159,7 @@ pub trait Ext: sealing::Sealed {
 	///
 	/// Returns the original code size of the called contract.
 	/// The newly created account will be associated with `code`. `value` specifies the amount of
-	/// value transferred from this to the newly created account.
+	/// value transferred from the caller to the newly created account.
 	fn instantiate(
 		&mut self,
 		gas_limit: Weight,
@@ -263,11 +263,11 @@ pub trait Ext: sealing::Sealed {
 	/// Get a reference to the schedule used by the current call.
 	fn schedule(&self) -> &Schedule<Self::T>;
 
-	/// Get a mutable reference to the nested gas meter.
-	fn gas_meter(&mut self) -> &mut GasMeter<Self::T>;
+	/// Get an immutable reference to the nested gas meter.
+	fn gas_meter(&self) -> &GasMeter<Self::T>;
 
 	/// Get a mutable reference to the nested gas meter.
-	fn gas_meter_immut(&self) -> &GasMeter<Self::T>;
+	fn gas_meter_mut(&mut self) -> &mut GasMeter<Self::T>;
 
 	/// Append a string to the debug buffer.
 	///
@@ -368,7 +368,7 @@ pub trait Executable<T: Config>: Sized {
 	/// The code hash of the executable.
 	fn code_hash(&self) -> &CodeHash<T>;
 
-	/// Size of the instrumented code in bytes.
+	/// Size of the conract code in bytes.
 	fn code_len(&self) -> u32;
 
 	/// The code does not contain any instructions which could lead to indeterminism.
@@ -1208,7 +1208,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())?;
+		let executable = E::from_storage(code_hash, self.schedule, 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();
@@ -1235,7 +1235,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())?;
+		let executable = E::from_storage(code_hash, self.schedule, self.gas_meter_mut())?;
 		let nonce = self.next_nonce();
 		let executable = self.push_frame(
 			FrameArgs::Instantiate {
@@ -1387,12 +1387,12 @@ where
 		self.schedule
 	}
 
-	fn gas_meter(&mut self) -> &mut GasMeter<Self::T> {
-		&mut self.top_frame_mut().nested_gas
+	fn gas_meter(&self) -> &GasMeter<Self::T> {
+		&self.top_frame().nested_gas
 	}
 
-	fn gas_meter_immut(&self) -> &GasMeter<Self::T> {
-		&self.top_frame().nested_gas
+	fn gas_meter_mut(&mut self) -> &mut GasMeter<Self::T> {
+		&mut self.top_frame_mut().nested_gas
 	}
 
 	fn append_debug_buffer(&mut self, msg: &str) -> bool {

frame/contracts/src/gas.rs

@@ -186,11 +186,9 @@ impl<T: Config> GasMeter<T> {
 	/// the Wasm engine gas meter. Passed `consumed` value is scaled by multiplying it by the
 	/// weight of a basic operation, as such an operation in wasmi engine costs 1.
 	///
-	/// This is used for gas synchronizations with the engine both on enter and on exit
-	/// of every host function.
+	/// This is used for gas synchronizations with the engine in every host function.
 	///
 	/// Returns the updated `ref_time` value for the gas left in the meter.
-	/// TODO : add scaling
 	/// Normally this would never fail, as engine should fail first when out of gas.
 	pub fn sync_reftime(&mut self, consumed: u64) -> Result<u64, DispatchError> {
 		let ref_time = self.gas_left.ref_time_mut();

frame/contracts/src/lib.rs

@@ -279,9 +279,7 @@ pub mod pallet {
 		/// The address generator used to generate the addresses of contracts.
 		type AddressGenerator: AddressGenerator<Self>;
 
-		/// The maximum length of a contract code in bytes. This limit applies to the instrumented
-		/// version of the code. Therefore `instantiate_with_code` can fail even when supplying
-		/// a wasm binary below this maximum size.
+		/// The maximum length of a contract code in bytes.
 		///
 		/// The value should be chosen carefully taking into the account the overall memory limit
 		/// your runtime has, as well as the [maximum allowed callstack
@@ -335,15 +333,15 @@ pub mod pallet {
 			// 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 both the original and instrumented wasm blobs for each
-			// contract, hence we add up `MaxCodeLen*2` more to the safety margin.
+			// Next, the pallet keeps the Wasm blob for each
+			// contract, hence we add up one `MaxCodeLen` more 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`.
 			//
-			// That being said, for every contract executed in runtime, at least `MaxCodeLen*18*4`
+			// 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
 			// each contract (stack frame) should also be counted.
 			//
@@ -352,7 +350,7 @@ pub mod pallet {
 			//
 			// This gives us the following formula:
 			//
-			// `(MaxCodeLen * 18 * 4 + MAX_STACK_SIZE + max_heap_size) * max_call_depth <
+			// `(MaxCodeLen * 17 * 4 + MAX_STACK_SIZE + max_heap_size) * max_call_depth <
 			// max_runtime_mem/2`
 			//
 			// Hence the upper limit for the `MaxCodeLen` can be defined as follows:
@@ -361,7 +359,7 @@ pub mod pallet {
 				.saturating_div(max_call_depth)
 				.saturating_sub(max_heap_size)
 				.saturating_sub(MAX_STACK_SIZE)
-				.saturating_div(18 * 4);
+				.saturating_div(17 * 4);
 
 			assert!(
 				T::MaxCodeLen::get() < code_len_limit,
@@ -472,7 +470,7 @@ pub mod pallet {
 		///
 		/// If the code does not already exist a deposit is reserved from the caller
 		/// and unreserved only when [`Self::remove_code`] is called. The size of the reserve
-		/// depends on the instrumented size of the the supplied `code`.
+		/// depends on the size of the supplied `code`.
 		///
 		/// If the code already exists in storage it will still return `Ok` and upgrades
 		/// the in storage version to the current
@@ -621,7 +619,7 @@ pub mod pallet {
 		///
 		/// Instantiation is executed as follows:
 		///
-		/// - The supplied `code` is instrumented, deployed, and a `code_hash` is created for that
+		/// - The supplied `code` is deployed, and a `code_hash` is created for that
 		///   code.
 		/// - If the `code_hash` already exists on the chain the underlying `code` will be shared.
 		/// - The destination address is computed based on the sender, code_hash and the salt.
@@ -847,7 +845,7 @@ pub mod pallet {
 		/// or via RPC an `Ok` will be returned. In this case the caller needs to inspect the flags
 		/// to determine whether a reversion has taken place.
 		ContractReverted,
-		/// The contract's code was found to be invalid during validation or instrumentation.
+		/// The contract's code was found to be invalid during validation.
 		///
 		/// The most likely cause of this is that an API was used which is not supported by the
 		/// node. This happens if an older node is used with a new version of ink!. Try updating
@@ -860,11 +858,7 @@ pub mod pallet {
 		Indeterministic,
 	}
 
-	/// A mapping from an original code hash to the original code, untouched by instrumentation.
-	#[pallet::storage]
-	pub(crate) type PristineCode<T: Config> = StorageMap<_, Identity, CodeHash<T>, CodeVec<T>>;
-
-	/// A mapping between an original code hash and instrumented wasm code, ready for execution.
+	/// A mapping between an original code hash and Wasm code, ready for execution.
 	#[pallet::storage]
 	pub(crate) type CodeStorage<T: Config> =
 		StorageMap<_, Identity, CodeHash<T>, PrefabWasmModule<T>>;
@@ -1324,7 +1318,7 @@ impl<T: Config> Pallet<T> {
 		ContractInfo::<T>::load_code_hash(account)
 	}
 
-	/// Store code for benchmarks which does not check nor instrument the code.
+	/// Store code for benchmarks which does not validate the code.
 	#[cfg(feature = "runtime-benchmarks")]
 	fn store_code_raw(
 		code: Vec<u8>,

frame/contracts/src/wasm/mod.rs

@@ -271,9 +271,6 @@ impl<T: Config> PrefabWasmModule<T> {
 		<OwnerInfoOf<T>>::mutate(&code_hash, |owner_info| {
 			match owner_info {
 				// Instantiate existing contract.
-				//
-				// No need to update the `CodeStorage` as any re-instrumentation eagerly saves
-				// the re-instrumented code.
 				Some(owner_info) if instantiated => {
 					owner_info.refcount = owner_info.refcount.checked_add(1).expect(
 						"
@@ -286,23 +283,15 @@ impl<T: Config> PrefabWasmModule<T> {
 					);
 					Ok(())
 				},
-				// Re-upload existing contract without executing it.
-				//
-				// We are careful here to just overwrite the code to not include it into the PoV.
-				// We do this because the uploaded code was instrumented with the latest schedule
-				// and hence we persist those changes. Otherwise the next execution will pay again
-				// for the instrumentation.
-				Some(_) => {
-					<CodeStorage<T>>::insert(&code_hash, module);
-					Ok(())
-				},
+				// Contract code is already stored in storage. Nothing to be done here.
+				Some(_) => Ok(()),
 				// Upload a new contract.
 				//
 				// We need to write all data structures and collect the deposit.
 				None => {
 					let mut new_owner_info = module.owner_info.take().expect(
 						"If an executable isn't in storage it was uploaded.
-				If it was uploaded the owner info was generated and attached. qed
+				If it was uploaded, the owner info was generated and attached. qed
 				",
 					);
 					// This `None` case happens only in freshly uploaded modules. This means that
@@ -392,11 +381,11 @@ impl<T: Config> PrefabWasmModule<T> {
 		store_code(executable, false)
 	}
 
-	/// Create the module without checking nor instrumenting the passed code.
+	/// Create the module without checking the passed code.
 	///
 	/// # Note
 	///
-	/// This is useful for benchmarking where we don't want instrumentation to skew
+	/// This is useful for benchmarking where we don't want validation of the module to skew
 	/// our results. This also does not collect any deposit from the `owner`. Also useful
 	/// during testing when we want to deploy codes that do not pass the instantiation checks.
 	#[cfg(any(test, feature = "runtime-benchmarks"))]
@@ -744,7 +733,7 @@ mod tests {
 		fn schedule(&self) -> &Schedule<Self::T> {
 			&self.schedule
 		}
-		fn gas_meter(&mut self) -> &mut GasMeter<Self::T> {
+		fn gas_meter_mut(&mut self) -> &mut GasMeter<Self::T> {
 			&mut self.gas_meter
 		}
 		fn append_debug_buffer(&mut self, msg: &str) -> bool {

frame/contracts/src/wasm/prepare.rs

@@ -284,7 +284,7 @@ impl ContractModule {
 	}
 
 	fn into_wasm_code(self) -> Result<Vec<u8>, &'static str> {
-		elements::serialize(self.0).map_err(|_| "error serializing instrumented module")
+		elements::serialize(self.0).map_err(|_| "error serializing contract module")
 	}
 }
 
@@ -399,7 +399,7 @@ where
 		)
 		.map_err(|err| {
 			log::debug!(target: LOG_TARGET, "{}", err);
-			(Error::<T>::CodeRejected.into(), "new code rejected after instrumentation")
+			(Error::<T>::CodeRejected.into(), "new code rejected on wasmi instantiation")
 		})?;
 	}
 
@@ -457,7 +457,7 @@ where
 /// Alternate (possibly unsafe) preparation functions used only for benchmarking and testing.
 ///
 /// For benchmarking we need to construct special contracts that might not pass our
-/// sanity checks or need to skip instrumentation for correct results. We hide functions
+/// sanity checks. We hide functions
 /// allowing this behind a feature that is only set during benchmarking or testing to
 /// prevent usage in production code.
 #[cfg(any(test, feature = "runtime-benchmarks"))]

frame/contracts/src/wasm/runtime.rs

@@ -174,9 +174,6 @@ impl HostError for TrapReason {}
 #[cfg_attr(test, derive(Debug, PartialEq, Eq))]
 #[derive(Copy, Clone)]
 pub enum RuntimeCosts {
-	/// Charge the gas meter with the cost of a metering block. The charged costs are
-	/// the supplied cost of the block plus the overhead of the metering itself.
-	MeteringBlock(u64),
 	/// Weight charged for copying data from the sandbox.
 	CopyFromContract(u32),
 	/// Weight charged for copying data to the sandbox.
@@ -277,7 +274,6 @@ impl RuntimeCosts {
 	fn token<T: Config>(&self, s: &HostFnWeights<T>) -> RuntimeToken {
 		use self::RuntimeCosts::*;
 		let weight = match *self {
-			MeteringBlock(amount) => s.gas.saturating_add(Weight::from_parts(amount, 0)),
 			CopyFromContract(len) => s.return_per_byte.saturating_mul(len.into()),
 			CopyToContract(len) => s.input_per_byte.saturating_mul(len.into()),
 			Caller => s.caller,
@@ -369,7 +365,7 @@ impl RuntimeCosts {
 macro_rules! charge_gas {
 	($runtime:expr, $costs:expr) => {{
 		let token = $costs.token(&$runtime.ext.schedule().host_fn_weights);
-		$runtime.ext.gas_meter().charge(token)
+		$runtime.ext.gas_meter_mut().charge(token)
 	}};
 }
 
@@ -548,7 +544,7 @@ impl<'a, E: Ext + 'a> Runtime<'a, E> {
 	/// refunded to match the actual amount.
 	pub fn adjust_gas(&mut self, charged: ChargedAmount, actual_costs: RuntimeCosts) {
 		let token = actual_costs.token(&self.ext.schedule().host_fn_weights);
-		self.ext.gas_meter().adjust_gas(charged, token);
+		self.ext.gas_meter_mut().adjust_gas(charged, token);
 	}
 
 	/// Read designated chunk from the sandbox memory.
@@ -1016,19 +1012,6 @@ impl<'a, E: Ext + 'a> Runtime<'a, E> {
 // for every function.
 #[define_env(doc)]
 pub mod env {
-	/// Account for used gas. Traps if gas used is greater than gas limit.
-	///
-	/// NOTE: This is a implementation defined call and is NOT a part of the public API.
-	/// This call is supposed to be called only by instrumentation injected code.
-	/// It deals only with the *ref_time* Weight.
-	///
-	/// - `amount`: How much gas is used.
-	/// TODO remove this host fn
-	fn gas(ctx: _, _memory: _, amount: u64) -> Result<(), TrapReason> {
-		ctx.charge_gas(RuntimeCosts::MeteringBlock(amount))?;
-		Ok(())
-	}
-
 	/// Set the value at the given key in the contract storage.
 	///
 	/// Equivalent to the newer [`seal1`][`super::api_doc::Version1::set_storage`] version with the