contracts: Run start function (#1367)

Fixes #116 

Start function wasn't allowed in a contract. Now it is allowed and is
being run.

It was disallowed because it is not used by Rust and supporting it made
the code more complex. However, not running the start function violates
the wasm standard. This makes life harder for some languages (see linked
ticket).

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

@@ -49,7 +49,7 @@ use frame_support::{
 use sp_core::Get;
 use sp_runtime::{DispatchError, RuntimeDebug};
 use sp_std::prelude::*;
-use wasmi::{Instance, Linker, Memory, MemoryType, StackLimits, Store};
+use wasmi::{InstancePre, Linker, Memory, MemoryType, StackLimits, Store};
 
 const BYTES_PER_PAGE: usize = 64 * 1024;
 
@@ -164,7 +164,30 @@ impl<T: Config> WasmBlob<T> {
 	///
 	/// Applies all necessary checks before removing the code.
 	pub fn remove(origin: &T::AccountId, code_hash: CodeHash<T>) -> DispatchResult {
-		Self::try_remove_code(origin, code_hash)
+		<CodeInfoOf<T>>::try_mutate_exists(&code_hash, |existing| {
+			if let Some(code_info) = existing {
+				ensure!(code_info.refcount == 0, <Error<T>>::CodeInUse);
+				ensure!(&code_info.owner == origin, BadOrigin);
+				let _ = T::Currency::release(
+					&HoldReason::CodeUploadDepositReserve.into(),
+					&code_info.owner,
+					code_info.deposit,
+					BestEffort,
+				);
+				let deposit_released = code_info.deposit;
+				let remover = code_info.owner.clone();
+
+				*existing = None;
+				<PristineCode<T>>::remove(&code_hash);
+				<Pallet<T>>::deposit_event(
+					vec![code_hash],
+					Event::CodeRemoved { code_hash, deposit_released, remover },
+				);
+				Ok(())
+			} else {
+				Err(<Error<T>>::CodeNotFound.into())
+			}
+		})
 	}
 
 	/// Creates and returns an instance of the supplied code.
@@ -179,7 +202,7 @@ impl<T: Config> WasmBlob<T> {
 		determinism: Determinism,
 		stack_limits: StackLimits,
 		allow_deprecated: AllowDeprecatedInterface,
-	) -> Result<(Store<H>, Memory, Instance), &'static str>
+	) -> Result<(Store<H>, Memory, InstancePre), &'static str>
 	where
 		E: Environment<H>,
 	{
@@ -217,9 +240,7 @@ impl<T: Config> WasmBlob<T> {
 
 		let instance = linker
 			.instantiate(&mut store, &contract.module)
-			.map_err(|_| "can't instantiate module with provided definitions")?
-			.ensure_no_start(&mut store)
-			.map_err(|_| "start function is forbidden but found in the module")?;
+			.map_err(|_| "can't instantiate module with provided definitions")?;
 
 		Ok((store, memory, instance))
 	}
@@ -261,45 +282,6 @@ impl<T: Config> WasmBlob<T> {
 		})
 	}
 
-	/// Try to remove code together with all associated information.
-	fn try_remove_code(origin: &T::AccountId, code_hash: CodeHash<T>) -> DispatchResult {
-		<CodeInfoOf<T>>::try_mutate_exists(&code_hash, |existing| {
-			if let Some(code_info) = existing {
-				ensure!(code_info.refcount == 0, <Error<T>>::CodeInUse);
-				ensure!(&code_info.owner == origin, BadOrigin);
-				let _ = T::Currency::release(
-					&HoldReason::CodeUploadDepositReserve.into(),
-					&code_info.owner,
-					code_info.deposit,
-					BestEffort,
-				);
-				let deposit_released = code_info.deposit;
-				let remover = code_info.owner.clone();
-
-				*existing = None;
-				<PristineCode<T>>::remove(&code_hash);
-				<Pallet<T>>::deposit_event(
-					vec![code_hash],
-					Event::CodeRemoved { code_hash, deposit_released, remover },
-				);
-				Ok(())
-			} else {
-				Err(<Error<T>>::CodeNotFound.into())
-			}
-		})
-	}
-
-	/// Load code with the given code hash.
-	fn load_code(
-		code_hash: CodeHash<T>,
-		gas_meter: &mut GasMeter<T>,
-	) -> Result<(CodeVec<T>, CodeInfo<T>), 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((code, code_info))
-	}
-
 	/// Create the module without checking the passed code.
 	///
 	/// # Note
@@ -318,12 +300,6 @@ impl<T: Config> WasmBlob<T> {
 }
 
 impl<T: Config> CodeInfo<T> {
-	/// Return the refcount of the module.
-	#[cfg(test)]
-	pub fn refcount(&self) -> u64 {
-		self.refcount
-	}
-
 	#[cfg(test)]
 	pub fn new(owner: T::AccountId) -> Self {
 		CodeInfo {
@@ -335,6 +311,16 @@ impl<T: Config> CodeInfo<T> {
 		}
 	}
 
+	/// Returns reference count of the module.
+	pub fn refcount(&self) -> u64 {
+		self.refcount
+	}
+
+	/// Return mutable reference to the refcount of the module.
+	pub fn refcount_mut(&mut self) -> &mut u64 {
+		&mut self.refcount
+	}
+
 	/// Returns the deposit of the module.
 	pub fn deposit(&self) -> BalanceOf<T> {
 		self.deposit
@@ -346,29 +332,12 @@ impl<T: Config> Executable<T> for WasmBlob<T> {
 		code_hash: CodeHash<T>,
 		gas_meter: &mut GasMeter<T>,
 	) -> Result<Self, DispatchError> {
-		let (code, code_info) = Self::load_code(code_hash, gas_meter)?;
+		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 increment_refcount(code_hash: CodeHash<T>) -> Result<(), DispatchError> {
-		<CodeInfoOf<T>>::mutate(code_hash, |existing| -> Result<(), DispatchError> {
-			if let Some(info) = existing {
-				info.refcount = info.refcount.saturating_add(1);
-				Ok(())
-			} else {
-				Err(Error::<T>::CodeNotFound.into())
-			}
-		})
-	}
-
-	fn decrement_refcount(code_hash: CodeHash<T>) {
-		<CodeInfoOf<T>>::mutate(code_hash, |existing| {
-			if let Some(info) = existing {
-				info.refcount = info.refcount.saturating_sub(1);
-			}
-		});
-	}
-
 	fn execute<E: Ext<T = T>>(
 		self,
 		ext: &mut E,
@@ -410,25 +379,38 @@ impl<T: Config> Executable<T> for WasmBlob<T> {
 			.add_fuel(fuel_limit)
 			.expect("We've set up engine to fuel consuming mode; qed");
 
-		let exported_func = instance
-			.get_export(&store, function.identifier())
-			.and_then(|export| export.into_func())
-			.ok_or_else(|| {
-				log::error!(target: LOG_TARGET, "failed to find entry point");
-				Error::<T>::CodeRejected
-			})?;
+		// 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();
+			gas_meter.charge_fuel(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 {
-			WasmBlob::<T>::increment_refcount(self.code_hash)?;
+			E::increment_refcount(self.code_hash)?;
 		}
 
-		let result = exported_func.call(&mut store, &[], &mut []);
-		let engine_consumed_total = store.fuel_consumed().expect("Fuel metering is enabled; qed");
-		// Sync this frame's gas meter with the engine's one.
-		let gas_meter = store.data_mut().ext().gas_meter_mut();
-		gas_meter.charge_fuel(engine_consumed_total)?;
+		// Any abort in start function (includes `return` + `terminate`) will make us skip the
+		// call into the subsequent exported function. This means that calling `return` returns data
+		// from the whole contract execution.
+		match instance.start(&mut store) {
+			Ok(instance) => {
+				let exported_func = instance
+					.get_export(&store, function.identifier())
+					.and_then(|export| export.into_func())
+					.ok_or_else(|| {
+						log::error!(target: LOG_TARGET, "failed to find entry point");
+						Error::<T>::CodeRejected
+					})?;
 
-		store.into_data().to_execution_result(result)
+				let result = exported_func.call(&mut store, &[], &mut []);
+				process_result(store, result)
+			},
+			Err(err) => process_result(store, Err(err)),
+		}
 	}
 
 	fn code_hash(&self) -> &CodeHash<T> {
@@ -740,7 +722,10 @@ mod tests {
 		fn nonce(&mut self) -> u64 {
 			995
 		}
-
+		fn increment_refcount(_code_hash: CodeHash<Self::T>) -> Result<(), DispatchError> {
+			Ok(())
+		}
+		fn decrement_refcount(_code_hash: CodeHash<Self::T>) {}
 		fn add_delegate_dependency(
 			&mut self,
 			code: CodeHash<Self::T>,
@@ -748,7 +733,6 @@ mod tests {
 			self.delegate_dependencies.borrow_mut().insert(code);
 			Ok(())
 		}
-
 		fn remove_delegate_dependency(
 			&mut self,
 			code: &CodeHash<Self::T>,
@@ -790,11 +774,20 @@ mod tests {
 		executable.execute(ext.borrow_mut(), entry_point, input_data)
 	}
 
-	/// Execute the supplied code.
+	/// Execute the `call` function within the supplied code.
 	fn execute<E: BorrowMut<MockExt>>(wat: &str, input_data: Vec<u8>, ext: E) -> ExecResult {
 		execute_internal(wat, input_data, ext, &ExportedFunction::Call, true, false)
 	}
 
+	/// Execute the `deploy` function within the supplied code.
+	fn execute_instantiate<E: BorrowMut<MockExt>>(
+		wat: &str,
+		input_data: Vec<u8>,
+		ext: E,
+	) -> ExecResult {
+		execute_internal(wat, input_data, ext, &ExportedFunction::Constructor, true, false)
+	}
+
 	/// Execute the supplied code with disabled unstable functions.
 	///
 	/// In our test config unstable functions are disabled so that we can test them.
@@ -1878,32 +1871,47 @@ mod tests {
 		assert_ok!(execute(CODE_VALUE_TRANSFERRED, vec![], MockExt::default()));
 	}
 
-	const START_FN_ILLEGAL: &str = r#"
+	const START_FN_DOES_RUN: &str = r#"
 (module
-	(import "seal0" "seal_return" (func $seal_return (param i32 i32 i32)))
+	(import "seal0" "seal_deposit_event" (func $seal_deposit_event (param i32 i32 i32 i32)))
 	(import "env" "memory" (memory 1 1))
 
 	(start $start)
 	(func $start
-		(unreachable)
+		(call $seal_deposit_event
+			(i32.const 0) ;; Pointer to the start of topics buffer
+			(i32.const 0) ;; The length of the topics buffer.
+			(i32.const 0) ;; Pointer to the start of the data buffer
+			(i32.const 13) ;; Length of the buffer
+		)
 	)
 
-	(func (export "call")
-		(unreachable)
-	)
+	(func (export "call"))
 
-	(func (export "deploy")
-		(unreachable)
-	)
+	(func (export "deploy"))
 
-	(data (i32.const 8) "\01\02\03\04")
+	(data (i32.const 0) "\00\01\2A\00\00\00\00\00\00\00\E5\14\00")
 )
 "#;
 
 	#[test]
-	fn start_fn_illegal() {
-		let output = execute(START_FN_ILLEGAL, vec![], MockExt::default());
-		assert_err!(output, <Error<Test>>::CodeRejected,);
+	fn start_fn_does_run_on_call() {
+		let mut ext = MockExt::default();
+		execute(START_FN_DOES_RUN, vec![], &mut ext).unwrap();
+		assert_eq!(
+			ext.events[0].1,
+			[0x00_u8, 0x01, 0x2a, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xe5, 0x14, 0x00]
+		);
+	}
+
+	#[test]
+	fn start_fn_does_run_on_deploy() {
+		let mut ext = MockExt::default();
+		execute_instantiate(START_FN_DOES_RUN, vec![], &mut ext).unwrap();
+		assert_eq!(
+			ext.events[0].1,
+			[0x00_u8, 0x01, 0x2a, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xe5, 0x14, 0x00]
+		);
 	}
 
 	const CODE_TIMESTAMP_NOW: &str = r#"