contracts: Change define_env! to expect a Result<T, DispatchError> for every function (#7762)

* Make host functions return TrapReason

This avoids the need to manually store any trap reasons
to the `Runtime` from the host function. This adds the following
benefits:

* It properly composes with the upcoming chain extensions
* Missing to set a trap value is now a compile error

* review: Remove superflous .into()

Author: athei

frame/contracts/src/lib.rs

@@ -378,6 +378,14 @@ decl_error! {
 		/// on the call stack. Those actions are contract self destruction and restoration
 		/// of a tombstone.
 		ReentranceDenied,
+		/// `seal_input` was called twice from the same contract execution context.
+		InputAlreadyRead,
+		/// The subject passed to `seal_random` exceeds the limit.
+		RandomSubjectTooLong,
+		/// The amount of topics passed to `seal_deposit_events` exceeds the limit.
+		TooManyTopics,
+		/// The topics passed to `seal_deposit_events` contains at least one duplicate.
+		DuplicateTopics,
 	}
 }
 

frame/contracts/src/wasm/env_def/macros.rs

@@ -96,7 +96,7 @@ macro_rules! unmarshall_then_body {
 #[inline(always)]
 pub fn constrain_closure<R, F>(f: F) -> F
 where
-	F: FnOnce() -> Result<R, sp_sandbox::HostError>,
+	F: FnOnce() -> Result<R, crate::wasm::runtime::TrapReason>,
 {
 	f
 }
@@ -109,14 +109,20 @@ macro_rules! unmarshall_then_body_then_marshall {
 		>(|| {
 			unmarshall_then_body!($body, $ctx, $args_iter, $( $names : $params ),*)
 		});
-		let r = body()?;
+		let r = body().map_err(|reason| {
+			$ctx.set_trap_reason(reason);
+			sp_sandbox::HostError
+		})?;
 		return Ok(sp_sandbox::ReturnValue::Value({ use $crate::wasm::env_def::ConvertibleToWasm; r.to_typed_value() }))
 	});
 	( $args_iter:ident, $ctx:ident, ( $( $names:ident : $params:ty ),* ) => $body:tt ) => ({
 		let body = $crate::wasm::env_def::macros::constrain_closure::<(), _>(|| {
 			unmarshall_then_body!($body, $ctx, $args_iter, $( $names : $params ),*)
 		});
-		body()?;
+		body().map_err(|reason| {
+			$ctx.set_trap_reason(reason);
+			sp_sandbox::HostError
+		})?;
 		return Ok(sp_sandbox::ReturnValue::Unit)
 	})
 }
@@ -207,15 +213,24 @@ mod tests {
 	use parity_wasm::elements::ValueType;
 	use sp_runtime::traits::Zero;
 	use sp_sandbox::{ReturnValue, Value};
-	use crate::wasm::tests::MockExt;
-	use crate::wasm::Runtime;
-	use crate::exec::Ext;
-	use crate::gas::Gas;
+	use crate::{
+		wasm::{Runtime, runtime::TrapReason, tests::MockExt},
+		exec::Ext,
+		gas::Gas,
+	};
+
+	struct TestRuntime {
+		value: u32,
+	}
+
+	impl TestRuntime {
+		fn set_trap_reason(&mut self, _reason: TrapReason) {}
+	}
 
 	#[test]
 	fn macro_unmarshall_then_body_then_marshall_value_or_trap() {
 		fn test_value(
-			_ctx: &mut u32,
+			_ctx: &mut TestRuntime,
 			args: &[sp_sandbox::Value],
 		) -> Result<ReturnValue, sp_sandbox::HostError> {
 			let mut args = args.iter();
@@ -224,15 +239,15 @@ mod tests {
 				_ctx,
 				(a: u32, b: u32) -> u32 => {
 					if b == 0 {
-						Err(sp_sandbox::HostError)
+						Err(crate::wasm::runtime::TrapReason::Termination)
 					} else {
 						Ok(a / b)
 					}
 				}
 			)
 		}
 
-		let ctx = &mut 0;
+		let ctx = &mut TestRuntime { value: 0 };
 		assert_eq!(
 			test_value(ctx, &[Value::I32(15), Value::I32(3)]).unwrap(),
 			ReturnValue::Value(Value::I32(5)),
@@ -243,24 +258,24 @@ mod tests {
 	#[test]
 	fn macro_unmarshall_then_body_then_marshall_unit() {
 		fn test_unit(
-			ctx: &mut u32,
+			ctx: &mut TestRuntime,
 			args: &[sp_sandbox::Value],
 		) -> Result<ReturnValue, sp_sandbox::HostError> {
 			let mut args = args.iter();
 			unmarshall_then_body_then_marshall!(
 				args,
 				ctx,
 				(a: u32, b: u32) => {
-					*ctx = a + b;
+					ctx.value = a + b;
 					Ok(())
 				}
 			)
 		}
 
-		let ctx = &mut 0;
+		let ctx = &mut TestRuntime { value: 0 };
 		let result = test_unit(ctx, &[Value::I32(2), Value::I32(3)]).unwrap();
 		assert_eq!(result, ReturnValue::Unit);
-		assert_eq!(*ctx, 5);
+		assert_eq!(ctx.value, 5);
 	}
 
 	#[test]
@@ -270,7 +285,7 @@ mod tests {
 			if !amount.is_zero() {
 				Ok(())
 			} else {
-				Err(sp_sandbox::HostError)
+				Err(TrapReason::Termination)
 			}
 		});
 		let _f: fn(&mut Runtime<MockExt>, &[sp_sandbox::Value])
@@ -322,7 +337,7 @@ mod tests {
 				if !amount.is_zero() {
 					Ok(())
 				} else {
-					Err(sp_sandbox::HostError)
+					Err(crate::wasm::runtime::TrapReason::Termination)
 				}
 			},
 		);

frame/contracts/src/wasm/mod.rs

@@ -1537,7 +1537,7 @@ mod tests {
 				&mut gas_meter
 			),
 			Err(ExecError {
-				error: Error::<Test>::ContractTrapped.into(),
+				error: Error::<Test>::TooManyTopics.into(),
 				origin: ErrorOrigin::Caller,
 			})
 		);
@@ -1582,7 +1582,7 @@ mod tests {
 				&mut gas_meter
 			),
 			Err(ExecError {
-				error: Error::<Test>::ContractTrapped.into(),
+				error: Error::<Test>::DuplicateTopics.into(),
 				origin: ErrorOrigin::Caller,
 			})
 		);

frame/contracts/src/wasm/prepare.rs

@@ -491,18 +491,24 @@ mod tests {
 		}
 	}
 
-	// Define test environment for tests. We need ImportSatisfyCheck
-	// implementation from it. So actual implementations doesn't matter.
-	define_env!(TestEnv, <E: Ext>,
-		panic(_ctx) => { unreachable!(); },
+	/// Using unreachable statements triggers unreachable warnings in the generated code
+	#[allow(unreachable_code)]
+	mod env {
+		use super::*;
 
-		// gas is an implementation defined function and a contract can't import it.
-		gas(_ctx, _amount: u32) => { unreachable!(); },
+		// Define test environment for tests. We need ImportSatisfyCheck
+		// implementation from it. So actual implementations doesn't matter.
+		define_env!(Test, <E: Ext>,
+			panic(_ctx) => { unreachable!(); },
 
-		nop(_ctx, _unused: u64) => { unreachable!(); },
+			// gas is an implementation defined function and a contract can't import it.
+			gas(_ctx, _amount: u32) => { unreachable!(); },
 
-		seal_println(_ctx, _ptr: u32, _len: u32) => { unreachable!(); },
-	);
+			nop(_ctx, _unused: u64) => { unreachable!(); },
+
+			seal_println(_ctx, _ptr: u32, _len: u32) => { unreachable!(); },
+		);
+	}
 
 	macro_rules! prepare_test {
 		($name:ident, $wat:expr, $($expected:tt)*) => {
@@ -520,7 +526,7 @@ mod tests {
 					},
 					.. Default::default()
 				};
-				let r = prepare_contract::<TestEnv, crate::tests::Test>(wasm.as_ref(), &schedule);
+				let r = prepare_contract::<env::Test, crate::tests::Test>(wasm.as_ref(), &schedule);
 				assert_matches!(r, $($expected)*);
 			}
 		};
@@ -931,7 +937,7 @@ mod tests {
 			).unwrap();
 			let mut schedule = Schedule::default();
 			schedule.enable_println = true;
-			let r = prepare_contract::<TestEnv, crate::tests::Test>(wasm.as_ref(), &schedule);
+			let r = prepare_contract::<env::Test, crate::tests::Test>(wasm.as_ref(), &schedule);
 			assert_matches!(r, Ok(_));
 		}
 	}