update docs

frame/contracts/src/exec.rs

@@ -870,7 +870,6 @@ where
 			self.initial_transfer()?;
 
 			// Call into the Wasm blob.
-			// We pass `reftime_left` into the execution engine to initialize its gas metering.
 			let output = executable
 				.execute(self, &entry_point, input_data)
 				.map_err(|e| ExecError { error: e.error, origin: ErrorOrigin::Callee })?;

frame/contracts/src/gas.rs

@@ -155,9 +155,7 @@ impl<T: Config> GasMeter<T> {
 	/// Amount is calculated by the given `token`.
 	///
 	/// Returns `OutOfGas` if there is not enough gas or addition of the specified
-	/// amount of gas has lead to overflow. On success returns `Proceed`.
-	///
-	/// Any charged amount less than base instruction weight is rounded up to it.
+	/// amount of gas has lead to overflow.
 	///
 	/// NOTE that amount isn't consumed if there is not enough gas. This is considered
 	/// safe because we always charge gas before performing any resource-spending action.
@@ -187,15 +185,15 @@ impl<T: Config> GasMeter<T> {
 		self.gas_left = self.gas_left.saturating_add(adjustment).min(self.gas_limit);
 	}
 
-	/// This method is used for gas syncs with the engine in every host function.
+	/// This method is used for gas syncs with the engine at start of every host function call.
 	///
 	/// Updates internal `engine_comsumed` tracker of engine fuel consumption.
 	///
 	/// Charges self with the `ref_time` Weight corresponding to `wasmi_fuel` consumed on the engine
 	/// side. Passed value is scaled by multiplying it by the weight of a basic operation, as such
 	/// an operation in wasmi engine costs 1.
 	///
-	/// Returns the updated `gas_left` Weight value from the meter.
+	/// Returns the updated `gas_left` `Weight` value from the meter.
 	/// Normally this would never fail, as engine should fail first when out of gas.
 	pub fn sync_fuel(&mut self, wasmi_fuel: u64) -> Result<Weight, DispatchError> {
 		if !wasmi_fuel.is_zero() {

frame/contracts/src/schedule.rs

@@ -138,34 +138,14 @@ impl Limits {
 	}
 }
 
-/// Describes the weight for all categories of supported wasm instructions.
-///
-/// There there is one field for each wasm instruction that describes the weight to
-/// execute one instruction of that name. There are a few exceptions:
-///
-/// 1. If there is a i64 and a i32 variant of an instruction we use the weight
-///    of the former for both.
-/// 2. The following instructions are free of charge because they merely structure the
-///    wasm module and cannot be spammed without making the module invalid (and rejected):
-///    End, Unreachable, Return, Else
-/// 3. The following instructions cannot be benchmarked because they are removed by any
-///    real world execution engine as a preprocessing step and therefore don't yield a
-///    meaningful benchmark result. However, in contrast to the instructions mentioned
-///    in 2. they can be spammed. We price them with the same weight as the "default"
-///    instruction (i64.const): Block, Loop, Nop
-/// 4. We price both i64.const and drop as InstructionWeights.i64const / 2. The reason
-///    for that is that we cannot benchmark either of them on its own but we need their
-///    individual values to derive (by subtraction) the weight of all other instructions
-///    that use them as supporting instructions. Supporting means mainly pushing arguments
-///    and dropping return values in order to maintain a valid module.
-/// TODO update doc
+/// Gas metering of Wasm executed instructions is being done on the engine side.
+/// This struct holds a reference value used to gas units scaling between host and engine.
 #[cfg_attr(feature = "std", derive(Serialize, Deserialize))]
 #[derive(Clone, Encode, Decode, PartialEq, Eq, ScheduleDebug, TypeInfo)]
 #[scale_info(skip_type_params(T))]
 pub struct InstructionWeights<T: Config> {
-	/// Base instruction ref_time weight.
-	/// Used to gas units scaling between host and engine.
-	/// Should match to wasmi's 1 fuel (see <https://github.com/paritytech/wasmi/issues/701>).
+	/// Base instruction `ref_time` Weight.
+	/// Should match to wasmi's `1` fuel (see <https://github.com/paritytech/wasmi/issues/701>).
 	pub base: u32,
 	/// The type parameter is used in the default implementation.
 	#[codec(skip)]
@@ -424,6 +404,8 @@ impl Default for Limits {
 }
 
 impl<T: Config> Default for InstructionWeights<T> {
+	/// We price both `i64.const` and `drop` as `instr_i64const / 2`. The reason
+	/// for that is that we cannot benchmark either of them on its own.
 	fn default() -> Self {
 		Self { base: cost_instr!(instr_i64const, 1), _phantom: PhantomData }
 	}

frame/contracts/src/wasm/mod.rs

@@ -56,40 +56,40 @@ use wasmi::{
 	Module, StackLimits, Store,
 };
 
-/// TODO: docs
+/// Validated Wasm module ready for execution.
+/// This data structure is immutable once created and stored.
 #[derive(Encode, Decode, scale_info::TypeInfo)]
 #[codec(mel_bound())]
 #[scale_info(skip_type_params(T))]
 pub struct WasmBlob<T: Config> {
 	code: CodeVec<T>,
-	// This isn't needed for contract execution and does not get loaded from storage by default.
-	// It is `Some` if and only if this struct was generated from code.
+	// This isn't needed for contract execution and is not stored alongside it.
 	#[codec(skip)]
 	pub owner_info: OwnerInfo<T>,
 }
 
-/// Contract metadata, such as:
-/// - owner of the contract,
+/// Contract code related data, such as:
+/// - owner of the contract, i.e. account uploaded its code,
 /// - storage deposit amount,
-/// - reference count
-/// - determinism marker
-/// TODO: rename this struct
+/// - reference count,
+/// - determinism marker.
+/// TODO: rename this struct to `CodeInfo`?
 ///
 /// It is stored in a separate storage entry to avoid loading the code when not necessary.
 #[derive(Clone, Encode, Decode, scale_info::TypeInfo, MaxEncodedLen)]
 #[codec(mel_bound())]
 #[scale_info(skip_type_params(T))]
 pub struct OwnerInfo<T: Config> {
-	/// The account that has deployed the contract and hence is allowed to remove it.
+	/// The account that has uploaded the contract code and hence is allowed to remove it.
 	owner: AccountIdOf<T>,
-	/// The amount of balance that was deposited by the owner in order to deploy it.
+	/// The amount of balance that was deposited by the owner in order to store it on-chain.
 	#[codec(compact)]
 	deposit: BalanceOf<T>,
-	/// The number of contracts that use this as their code.
+	/// The number of instantiated contracts that use this as their code.
 	#[codec(compact)]
 	refcount: u64,
-	/// Marks if the code might contain non deterministic features and is therefore never allowed
-	/// to be run on chain. Specifically, such a code can never be instantiated into a contract
+	/// Marks if the code might contain non-deterministic features and is therefore never allowed
+	/// to be run on-chain. Specifically, such a code can never be instantiated into a contract
 	/// and can just be used through a delegate call.
 	determinism: Determinism,
 }
@@ -139,7 +139,7 @@ impl<T: Config> Token<T> for CodeToken {
 		// In case of `Load` 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 not know the
+		// 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)
@@ -195,8 +195,8 @@ impl<T: Config> WasmBlob<T> {
 	/// Creates and returns an instance of the supplied code.
 	///
 	/// This is either used for later executing a contract or for validation of a contract.
-	/// When validating we pass `()` a `host_state`. Please note that such a dummy instance must
-	/// **never** be called/executed since it will panic the executor.
+	/// When validating we pass `()` as `host_state`. Please note that such a dummy instance must
+	/// **never** be called/executed, since it will panic the executor.
 	pub fn instantiate<E, H>(
 		code: &[u8],
 		host_state: H,
@@ -231,9 +231,8 @@ impl<T: Config> WasmBlob<T> {
 			},
 			allow_deprecated,
 		)?;
-
-		// Here we allocate this memory in the _store_. It allocates _inital_ val, but allows it to
-		// grow up to max number of mem pages, if neccesary.
+		// Here we allocate this memory in the _store_. It allocates _inital_ value, but allows it
+		// to grow up to maximum number of memory pages, if neccesary.
 		let memory =
 			Memory::new(&mut store, MemoryType::new(memory_limits.0, Some(memory_limits.1))?)
 				.expect(
@@ -252,8 +251,8 @@ impl<T: Config> WasmBlob<T> {
 
 	/// Put the module blob into storage.
 	///
-	/// Increments the refcount of the in-storage `prefab_module` if it already exists in storage
-	/// under the specified `code_hash`.
+	/// Increments the reference count of the in-storage `WasmBlob`, if it already exists in
+	/// storage.
 	fn store_code(mut module: Self, instantiated: bool) -> DispatchResult {
 		let code_hash = &module.code_hash();
 		<OwnerInfoOf<T>>::mutate(code_hash, |stored_owner_info| {
@@ -275,7 +274,7 @@ impl<T: Config> WasmBlob<T> {
 				Some(_) => Ok(()),
 				// Upload a new contract code.
 				//
-				// We need to store the code and its owner info, and collect the deposit.
+				// We need to store the code and its owner_info, and collect the deposit.
 				None => {
 					// This `None` case happens only in freshly uploaded modules. This means that
 					// the `owner` is always the origin of the current transaction.
@@ -326,12 +325,12 @@ impl<T: Config> WasmBlob<T> {
 		Ok(code)
 	}
 
-	/// Decrement the refcount of a code in-storage by one.
+	/// Decrement the reference count of a stored code by one.
 	///
 	/// # Note
 	///
-	/// A contract whose refcount dropped to zero isn't automatically removed. A `remove_code`
-	/// transaction must be submitted by the original uploader to do so.
+	/// A contract whose reference count dropped to zero isn't automatically removed. A
+	/// `remove_code` transaction must be submitted by the original uploader to do so.
 	fn decrement_refcount(code_hash: CodeHash<T>) {
 		<OwnerInfoOf<T>>::mutate(code_hash, |existing| {
 			if let Some(info) = existing {
@@ -340,11 +339,12 @@ impl<T: Config> WasmBlob<T> {
 		});
 	}
 
-	/// Increment the refcount of a code in-storage by one.
+	/// Increment the reference count of a of a stored code by one.
 	///
 	/// # Errors
 	///
-	/// [`Error::CodeNotFound`] is returned if the specified `code_hash` does not exist.
+	/// [`Error::CodeNotFound`] is returned if no stored code found having the specified
+	/// `code_hash`.
 	fn increment_refcount(code_hash: CodeHash<T>) -> Result<(), DispatchError> {
 		<OwnerInfoOf<T>>::mutate(code_hash, |existing| -> Result<(), DispatchError> {
 			if let Some(info) = existing {
@@ -400,10 +400,10 @@ impl<T: Config> Executable<T> for WasmBlob<T> {
 	) -> Result<Self, DispatchError> {
 		let code = Self::load_code(code_hash, gas_meter)?;
 		let code_hash = T::Hashing::hash(&code);
-		// We store owner_info at the same time as contract code,
+		// We store `owner_info` at the same time as contract code,
 		// therefore this query shouldn't really fail.
-		// We consider its failure equal to CodeNotFound, as contract code without
-		// owner_info is unusable in this pallet.
+		// We consider its failure equal to `CodeNotFound`, as contract code without
+		// `owner_info` is unusable in this pallet.
 		let owner_info = <OwnerInfoOf<T>>::get(code_hash).ok_or(Error::<T>::CodeNotFound)?;
 
 		Ok(Self { code, owner_info })
@@ -424,8 +424,9 @@ impl<T: Config> Executable<T> for WasmBlob<T> {
 		input_data: Vec<u8>,
 	) -> ExecResult {
 		let code = self.code.as_slice();
-		// Extract memory limits from the module.
 		let contract_module = prepare::ContractModule::new(code)?;
+		// Extract memory limits from the module.
+		// This also checks that module's memory import satisfies the schedule.
 		let memory_limits =
 			prepare::get_memory_limits(contract_module.scan_imports::<T>(&[])?, ext.schedule())?;
 		// Instantiate the Wasm module to the engine.
@@ -447,7 +448,7 @@ impl<T: Config> Executable<T> for WasmBlob<T> {
 		store.data_mut().set_memory(memory);
 
 		// Set fuel limit for the wasmi execution.
-		// We normalize it by the base instruction weight, as its cost in wasmi engine is 1.
+		// We normalize it by the base instruction weight, as its cost in wasmi engine is `1`.
 		let fuel_limit = store
 			.data_mut()
 			.ext()

frame/contracts/src/wasm/prepare.rs

@@ -314,8 +314,7 @@ pub fn get_memory_limits<T: Config>(
 
 /// Check that given `code` satisfies constraints required for the contract Wasm module.
 ///
-/// On success it returns back the code together with its `(initial, maximum)`
-/// memory limits. The memory requirement was also validated against the `schedule`.
+/// On success it returns back the code.
 fn validate<E, T>(
 	code: &[u8],
 	schedule: &Schedule<T>,
@@ -367,7 +366,8 @@ where
 		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)?;
-		// We do it here just to check that module imported memory satisfies the Schedule limits
+		// Extract memory limits from the module.
+		// This also checks that module's memory import satisfies the schedule.
 		let memory_limits = get_memory_limits(contract_module.scan_imports::<T>(&[])?, schedule)?;
 
 		let code = contract_module.into_wasm_code()?;
@@ -381,8 +381,8 @@ where
 
 	// This will make sure that the module can be actually run within wasmi:
 	//
-	// - Doesn't use any unknown imports.
-	// - Doesn't explode the wasmi bytecode generation.
+	// - It doesn't use any unknown imports.
+	// - It doesn't explode the wasmi bytecode generation.
 	if matches!(try_instantiate, TryInstantiate::Instantiate) {
 		// We don't actually ever run any code so we can get away with a minimal stack which
 		// reduces the amount of memory that needs to be zeroed.
@@ -405,12 +405,11 @@ where
 
 /// Validates the given binary `code` is a valid Wasm module satisfying following constraints:
 ///
-/// - the module doesn't define an internal memory instance;
-/// - imported memory (if any) doesn't reserve more memory than permitted by the `schedule`;
-/// - all imported functions from the external environment matches defined by `env` module.
+/// - The module doesn't define an internal memory instance.
+/// - Imported memory (if any) doesn't reserve more memory than permitted by the `schedule`.
+/// - All imported functions from the external environment match defined by `env` module.
 ///
-/// TODO: re-phrase
-/// Also constructs contract owner_info (metadata?) by calculating the storage deposit.
+/// Also constructs contract `owner_info` by calculating the storage deposit.
 pub fn prepare<E, T>(
 	code: CodeVec<T>,
 	schedule: &Schedule<T>,
@@ -433,7 +432,7 @@ where
 		(<Error<T>>::CodeTooLarge.into(), "preparation altered the code")
 	);
 
-	// Calculate deposit for storing contract code and owner info in two different storage items.
+	// Calculate deposit for storing contract code and `owner_info` in two different storage items.
 	let bytes_added = code.len().saturating_add(<OwnerInfo<T>>::max_encoded_len()) as u32;
 	let deposit = Diff { bytes_added, items_added: 2, ..Default::default() }
 		.update_contract::<T>(None)
@@ -447,14 +446,13 @@ 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. We hide functions
-/// allowing this behind a feature that is only set during benchmarking or testing to
-/// prevent usage in production code.
+/// 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"))]
 pub mod benchmarking {
 	use super::*;
 
-	/// Prepare function that does not check the passed in code.
+	/// Prepare function that does not perform most checks on the passed in code.
 	pub fn prepare<T: Config>(
 		code: Vec<u8>,
 		schedule: &Schedule<T>,

frame/contracts/src/wasm/runtime.rs

@@ -147,7 +147,6 @@ pub struct ReturnData {
 /// as a quick way to terminate the application (all other variants).
 #[derive(RuntimeDebug)]
 pub enum TrapReason {
-	//	InstructionTrap(DispatchError),
 	/// The supervisor trapped the contract because of an error condition occurred during
 	/// execution in privileged code.
 	SupervisorError(DispatchError),
@@ -488,15 +487,15 @@ impl<'a, E: Ext + 'a> Runtime<'a, E> {
 		match sandbox_result {
 			// Contract returned from main function -> no data was returned.
 			Ok(_) => Ok(ExecReturnValue { flags: ReturnFlags::empty(), data: Vec::new() }),
-			// OutOfGas when host asks engine to consume more than left in the _store_.
+			// `OutOfGas` when host asks engine to consume more than left in the _store_.
 			// We should never get this case, as gas meter is being charged (and hence raises error)
 			// first.
 			Err(wasmi::Error::Store(_)) => Err(Error::<E::T>::OutOfGas.into()),
 			// Contract either trapped or some host function aborted the execution.
 			Err(wasmi::Error::Trap(trap)) => {
 				if let Some(code) = trap.trap_code() {
 					match code {
-						// OutOfGas during engine execution.
+						// `OutOfGas` during engine execution.
 						OutOfFuel => return Err(Error::<E::T>::OutOfGas.into()),
 						_ => (),
 					}