seal: Rework contracts API (#6573)

* Transition getter functions to not use scratch buffer

* Remove scratch buffer from ext_get_storage

* Remove scratch buffer from ext_call

* Remove scratch buffer from ext_instantiate

* Add ext_input and remove scratch buffer

* Rework error handling (changes RPC exposed data)

* ext_return passes a flags field instead of a return code
	* Flags is only for seal and not for the caller
	* flags: u32 replaced status_code: u8 in RPC exposed type
* API functions use a unified error type (ReturnCode)
* ext_transfer now traps on error to be consistent with call and instantiate

* Remove the no longer used `Dispatched` event

* Updated inline documentation

* Prevent skipping of copying the output for getter API

* Return gas_consumed from the RPC contracts call interface

* Updated COMPLEXTITY.md

* Rename ext_gas_price to ext_weight_to_fee

* Align comments with spaces

* Removed no longer used `ExecError`

* Remove possible panic in `from_typed_value`

* Use a struct as associated data for SpecialTrap::Return

* Fix nits in COMPLEXITY.md

* Renamed SpecialTrap to TrapReason

* Fix test

* Finish renaming special_trap -> trap_reason

* Remove no longer used get_runtime_storage

* fixup! Remove no longer used get_runtime_storage

* Removed tabs for comment aligment

substrate/frame/contracts/src/exec.rs

@@ -19,11 +19,12 @@ use super::{CodeHash, Config, ContractAddressFor, Event, RawEvent, Trait,
 use crate::gas::{Gas, GasMeter, Token};
 use crate::rent;
 use crate::storage;
+use bitflags::bitflags;
 
 use sp_std::prelude::*;
 use sp_runtime::traits::{Bounded, Zero, Convert};
 use frame_support::{
-	storage::unhashed, dispatch::DispatchError,
+	dispatch::DispatchError,
 	traits::{ExistenceRequirement, Currency, Time, Randomness},
 	weights::Weight,
 };
@@ -37,58 +38,31 @@ pub type StorageKey = [u8; 32];
 /// A type that represents a topic of an event. At the moment a hash is used.
 pub type TopicOf<T> = <T as frame_system::Trait>::Hash;
 
-/// A status code return to the source of a contract call or instantiation indicating success or
-/// failure. A code of 0 indicates success and that changes are applied. All other codes indicate
-/// failure and that changes are reverted. The particular code in the case of failure is opaque and
-/// may be interpreted by the calling contract.
-pub type StatusCode = u8;
-
-/// The status code indicating success.
-pub const STATUS_SUCCESS: StatusCode = 0;
+bitflags! {
+	/// Flags used by a contract to customize exit behaviour.
+	pub struct ReturnFlags: u32 {
+		/// If this bit is set all changes made by the contract exection are rolled back.
+		const REVERT = 0x0000_0001;
+	}
+}
 
 /// Output of a contract call or instantiation which ran to completion.
 #[cfg_attr(test, derive(PartialEq, Eq, Debug))]
 pub struct ExecReturnValue {
-	pub status: StatusCode,
+	/// Flags passed along by `ext_return`. Empty when `ext_return` was never called.
+	pub flags: ReturnFlags,
+	/// Buffer passed along by `ext_return`. Empty when `ext_return` was never called.
 	pub data: Vec<u8>,
 }
 
 impl ExecReturnValue {
-	/// Returns whether the call or instantiation exited with a successful status code.
+	/// We understand the absense of a revert flag as success.
 	pub fn is_success(&self) -> bool {
-		self.status == STATUS_SUCCESS
+		!self.flags.contains(ReturnFlags::REVERT)
 	}
 }
 
-/// An error indicating some failure to execute a contract call or instantiation. This can include
-/// VM-specific errors during execution (eg. division by 0, OOB access, failure to satisfy some
-/// precondition of a system call, etc.) or errors with the orchestration (eg. out-of-gas errors, a
-/// non-existent destination contract, etc.).
-#[cfg_attr(test, derive(sp_runtime::RuntimeDebug))]
-pub struct ExecError {
-	pub reason: DispatchError,
-	/// This is an allocated buffer that may be reused. The buffer must be cleared explicitly
-	/// before reuse.
-	pub buffer: Vec<u8>,
-}
-
-pub type ExecResult = Result<ExecReturnValue, ExecError>;
-
-/// Evaluate an expression of type Result<_, &'static str> and either resolve to the value if Ok or
-/// wrap the error string into an ExecutionError with the provided buffer and return from the
-/// enclosing function. This macro is used instead of .map_err(..)? in order to avoid taking
-/// ownership of buffer unless there is an error.
-#[macro_export]
-macro_rules! try_or_exec_error {
-	($e:expr, $buffer:expr) => {
-		match $e {
-			Ok(val) => val,
-			Err(reason) => return Err(
-				$crate::exec::ExecError { reason: reason.into(), buffer: $buffer }
-			),
-		}
-	}
-}
+pub type ExecResult = Result<ExecReturnValue, DispatchError>;
 
 /// An interface that provides access to the external environment in which the
 /// smart-contract is executed.
@@ -118,7 +92,7 @@ pub trait Ext {
 		value: BalanceOf<Self::T>,
 		gas_meter: &mut GasMeter<Self::T>,
 		input_data: Vec<u8>,
-	) -> Result<(AccountIdOf<Self::T>, ExecReturnValue), ExecError>;
+	) -> Result<(AccountIdOf<Self::T>, ExecReturnValue), DispatchError>;
 
 	/// Transfer some amount of funds into the specified account.
 	fn transfer(
@@ -208,11 +182,6 @@ pub trait Ext {
 	/// Returns the maximum allowed size of a storage item.
 	fn max_value_size(&self) -> u32;
 
-	/// Returns the value of runtime under the given key.
-	///
-	/// Returns `None` if the value doesn't exist.
-	fn get_runtime_storage(&self, key: &[u8]) -> Option<Vec<u8>>;
-
 	/// Returns the price for the specified amount of weight.
 	fn get_weight_price(&self, weight: Weight) -> BalanceOf<Self::T>;
 }
@@ -331,20 +300,14 @@ where
 		input_data: Vec<u8>,
 	) -> ExecResult {
 		if self.depth == self.config.max_depth as usize {
-			return Err(ExecError {
-				reason: "reached maximum depth, cannot make a call".into(),
-				buffer: input_data,
-			});
+			Err("reached maximum depth, cannot make a call")?
 		}
 
 		if gas_meter
 			.charge(self.config, ExecFeeToken::Call)
 			.is_out_of_gas()
 		{
-			return Err(ExecError {
-				reason: "not enough gas to pay base call fee".into(),
-				buffer: input_data,
-			});
+			Err("not enough gas to pay base call fee")?
 		}
 
 		// Assumption: `collect_rent` doesn't collide with overlay because
@@ -354,10 +317,7 @@ where
 
 		// Calls to dead contracts always fail.
 		if let Some(ContractInfo::Tombstone(_)) = contract_info {
-			return Err(ExecError {
-				reason: "contract has been evicted".into(),
-				buffer: input_data,
-			});
+			Err("contract has been evicted")?
 		};
 
 		let caller = self.self_account.clone();
@@ -365,27 +325,21 @@ where
 
 		self.with_nested_context(dest.clone(), dest_trie_id, |nested| {
 			if value > BalanceOf::<T>::zero() {
-				try_or_exec_error!(
-					transfer(
-						gas_meter,
-						TransferCause::Call,
-						&caller,
-						&dest,
-						value,
-						nested,
-					),
-					input_data
-				);
+				transfer(
+					gas_meter,
+					TransferCause::Call,
+					&caller,
+					&dest,
+					value,
+					nested,
+				)?
 			}
 
 			// If code_hash is not none, then the destination account is a live contract, otherwise
 			// it is a regular account since tombstone accounts have already been rejected.
 			match storage::code_hash::<T>(&dest) {
 				Ok(dest_code_hash) => {
-					let executable = try_or_exec_error!(
-						nested.loader.load_main(&dest_code_hash),
-						input_data
-					);
+					let executable = nested.loader.load_main(&dest_code_hash)?;
 					let output = nested.vm
 						.execute(
 							&executable,
@@ -395,7 +349,7 @@ where
 						)?;
 					Ok(output)
 				}
-				Err(storage::ContractAbsentError) => Ok(ExecReturnValue { status: STATUS_SUCCESS, data: Vec::new() }),
+				Err(storage::ContractAbsentError) => Ok(ExecReturnValue { flags: ReturnFlags::empty(), data: Vec::new() }),
 			}
 		})
 	}
@@ -406,22 +360,16 @@ where
 		gas_meter: &mut GasMeter<T>,
 		code_hash: &CodeHash<T>,
 		input_data: Vec<u8>,
-	) -> Result<(T::AccountId, ExecReturnValue), ExecError> {
+	) -> Result<(T::AccountId, ExecReturnValue), DispatchError> {
 		if self.depth == self.config.max_depth as usize {
-			return Err(ExecError {
-				reason: "reached maximum depth, cannot instantiate".into(),
-				buffer: input_data,
-			});
+			Err("reached maximum depth, cannot instantiate")?
 		}
 
 		if gas_meter
 			.charge(self.config, ExecFeeToken::Instantiate)
 			.is_out_of_gas()
 		{
-			return Err(ExecError {
-				reason: "not enough gas to pay base instantiate fee".into(),
-				buffer: input_data,
-			});
+			Err("not enough gas to pay base instantiate fee")?
 		}
 
 		let caller = self.self_account.clone();
@@ -437,36 +385,27 @@ where
 		let dest_trie_id = <T as Trait>::TrieIdGenerator::trie_id(&dest);
 
 		let output = self.with_nested_context(dest.clone(), Some(dest_trie_id), |nested| {
-			try_or_exec_error!(
-				storage::place_contract::<T>(
-					&dest,
-					nested
-						.self_trie_id
-						.clone()
-						.expect("the nested context always has to have self_trie_id"),
-					code_hash.clone()
-				),
-				input_data
-			);
+			storage::place_contract::<T>(
+				&dest,
+				nested
+					.self_trie_id
+					.clone()
+					.expect("the nested context always has to have self_trie_id"),
+				code_hash.clone()
+			)?;
 
 			// Send funds unconditionally here. If the `endowment` is below existential_deposit
 			// then error will be returned here.
-			try_or_exec_error!(
-				transfer(
-					gas_meter,
-					TransferCause::Instantiate,
-					&caller,
-					&dest,
-					endowment,
-					nested,
-				),
-				input_data
-			);
+			transfer(
+				gas_meter,
+				TransferCause::Instantiate,
+				&caller,
+				&dest,
+				endowment,
+				nested,
+			)?;
 
-			let executable = try_or_exec_error!(
-				nested.loader.load_init(&code_hash),
-				input_data
-			);
+			let executable = nested.loader.load_init(&code_hash)?;
 			let output = nested.vm
 				.execute(
 					&executable,
@@ -477,10 +416,7 @@ where
 
 			// Error out if insufficient remaining balance.
 			if T::Currency::free_balance(&dest) < nested.config.existential_deposit {
-				return Err(ExecError {
-					reason: "insufficient remaining balance".into(),
-					buffer: output.data,
-				});
+				Err("insufficient remaining balance")?
 			}
 
 			// Deposit an instantiation event.
@@ -518,7 +454,7 @@ where
 		frame_support::storage::with_transaction(|| {
 			let output = func(&mut nested);
 			match output {
-				Ok(ref rv) if rv.is_success() => Commit(output),
+				Ok(ref rv) if !rv.flags.contains(ReturnFlags::REVERT) => Commit(output),
 				_ => Rollback(output),
 			}
 		})
@@ -681,7 +617,7 @@ where
 		endowment: BalanceOf<T>,
 		gas_meter: &mut GasMeter<T>,
 		input_data: Vec<u8>,
-	) -> Result<(AccountIdOf<T>, ExecReturnValue), ExecError> {
+	) -> Result<(AccountIdOf<T>, ExecReturnValue), DispatchError> {
 		self.ctx.instantiate(endowment, gas_meter, code_hash, input_data)
 	}
 
@@ -839,10 +775,6 @@ where
 		self.ctx.config.max_value_size
 	}
 
-	fn get_runtime_storage(&self, key: &[u8]) -> Option<Vec<u8>> {
-		unhashed::get_raw(&key)
-	}
-
 	fn get_weight_price(&self, weight: Weight) -> BalanceOf<Self::T> {
 		T::WeightPrice::convert(weight)
 	}
@@ -867,11 +799,11 @@ fn deposit_event<T: Trait>(
 mod tests {
 	use super::{
 		BalanceOf, Event, ExecFeeToken, ExecResult, ExecutionContext, Ext, Loader,
-		RawEvent, TransferFeeKind, TransferFeeToken, Vm,
+		RawEvent, TransferFeeKind, TransferFeeToken, Vm, ReturnFlags,
 	};
 	use crate::{
 		gas::GasMeter, tests::{ExtBuilder, Test, MetaEvent},
-		exec::{ExecReturnValue, ExecError, STATUS_SUCCESS}, CodeHash, Config,
+		exec::ExecReturnValue, CodeHash, Config,
 		gas::Gas,
 		storage,
 	};
@@ -980,7 +912,7 @@ mod tests {
 	}
 
 	fn exec_success() -> ExecResult {
-		Ok(ExecReturnValue { status: STATUS_SUCCESS, data: Vec::new() })
+		Ok(ExecReturnValue { flags: ReturnFlags::empty(), data: Vec::new() })
 	}
 
 	#[test]
@@ -1096,7 +1028,7 @@ mod tests {
 		let vm = MockVm::new();
 		let mut loader = MockLoader::empty();
 		let return_ch = loader.insert(
-			|_| Ok(ExecReturnValue { status: 1, data: Vec::new() })
+			|_| Ok(ExecReturnValue { flags: ReturnFlags::REVERT, data: Vec::new() })
 		);
 
 		ExtBuilder::default().build().execute_with(|| {
@@ -1228,10 +1160,7 @@ mod tests {
 
 			assert_matches!(
 				result,
-				Err(ExecError {
-					reason: DispatchError::Module { message: Some("InsufficientBalance"), .. },
-					buffer: _,
-				})
+				Err(DispatchError::Module { message: Some("InsufficientBalance"), .. })
 			);
 			assert_eq!(get_balance(&origin), 0);
 			assert_eq!(get_balance(&dest), 0);
@@ -1248,7 +1177,7 @@ mod tests {
 		let vm = MockVm::new();
 		let mut loader = MockLoader::empty();
 		let return_ch = loader.insert(
-			|_| Ok(ExecReturnValue { status: STATUS_SUCCESS, data: vec![1, 2, 3, 4] })
+			|_| Ok(ExecReturnValue { flags: ReturnFlags::empty(), data: vec![1, 2, 3, 4] })
 		);
 
 		ExtBuilder::default().build().execute_with(|| {
@@ -1279,7 +1208,7 @@ mod tests {
 		let vm = MockVm::new();
 		let mut loader = MockLoader::empty();
 		let return_ch = loader.insert(
-			|_| Ok(ExecReturnValue { status: 1, data: vec![1, 2, 3, 4] })
+			|_| Ok(ExecReturnValue { flags: ReturnFlags::REVERT, data: vec![1, 2, 3, 4] })
 		);
 
 		ExtBuilder::default().build().execute_with(|| {
@@ -1370,10 +1299,7 @@ mod tests {
 				// Verify that we've got proper error and set `reached_bottom`.
 				assert_matches!(
 					r,
-					Err(ExecError {
-						reason: DispatchError::Other("reached maximum depth, cannot make a call"),
-						buffer: _,
-					})
+					Err(DispatchError::Other("reached maximum depth, cannot make a call"))
 				);
 				*reached_bottom = true;
 			} else {
@@ -1517,7 +1443,7 @@ mod tests {
 
 		let mut loader = MockLoader::empty();
 		let dummy_ch = loader.insert(
-			|_| Ok(ExecReturnValue { status: STATUS_SUCCESS, data: vec![80, 65, 83, 83] })
+			|_| Ok(ExecReturnValue { flags: ReturnFlags::empty(), data: vec![80, 65, 83, 83] })
 		);
 
 		ExtBuilder::default().existential_deposit(15).build().execute_with(|| {
@@ -1550,7 +1476,7 @@ mod tests {
 
 		let mut loader = MockLoader::empty();
 		let dummy_ch = loader.insert(
-			|_| Ok(ExecReturnValue { status: 1, data: vec![70, 65, 73, 76] })
+			|_| Ok(ExecReturnValue { flags: ReturnFlags::REVERT, data: vec![70, 65, 73, 76] })
 		);
 
 		ExtBuilder::default().existential_deposit(15).build().execute_with(|| {
@@ -1627,7 +1553,7 @@ mod tests {
 
 		let mut loader = MockLoader::empty();
 		let dummy_ch = loader.insert(
-			|_| Err(ExecError { reason: "It's a trap!".into(), buffer: Vec::new() })
+			|_| Err("It's a trap!".into())
 		);
 		let instantiator_ch = loader.insert({
 			let dummy_ch = dummy_ch.clone();
@@ -1640,7 +1566,7 @@ mod tests {
 						ctx.gas_meter,
 						vec![]
 					),
-					Err(ExecError { reason: DispatchError::Other("It's a trap!"), buffer: _ })
+					Err(DispatchError::Other("It's a trap!"))
 				);
 
 				exec_success()
@@ -1691,10 +1617,7 @@ mod tests {
 						&terminate_ch,
 						vec![],
 					),
-					Err(ExecError {
-						reason: DispatchError::Other("insufficient remaining balance"),
-						buffer
-					}) if buffer == Vec::<u8>::new()
+					Err(DispatchError::Other("insufficient remaining balance"))
 				);
 
 				assert_eq!(

substrate/frame/contracts/src/gas.rs

@@ -178,8 +178,8 @@ impl<T: Trait> GasMeter<T> {
 		}
 	}
 
-	/// Returns how much gas left from the initial budget.
-	fn gas_spent(&self) -> Gas {
+	/// Returns how much gas was used.
+	pub fn gas_spent(&self) -> Gas {
 		self.gas_limit - self.gas_left
 	}
 

substrate/frame/contracts/src/lib.rs

@@ -93,7 +93,7 @@ use crate::exec::ExecutionContext;
 use crate::wasm::{WasmLoader, WasmVm};
 
 pub use crate::gas::{Gas, GasMeter};
-pub use crate::exec::{ExecResult, ExecReturnValue, ExecError, StatusCode};
+pub use crate::exec::{ExecResult, ExecReturnValue};
 
 #[cfg(feature = "std")]
 use serde::{Serialize, Deserialize};
@@ -410,7 +410,11 @@ decl_error! {
 		/// Tombstones don't match.
 		InvalidTombstone,
 		/// An origin TrieId written in the current block.
-		InvalidContractOrigin
+		InvalidContractOrigin,
+		/// The executed contract exhausted its gas limit.
+		OutOfGas,
+		/// The output buffer supplied to a contract API call was too small.
+		OutputBufferTooSmall,
 	}
 }
 
@@ -515,7 +519,7 @@ decl_module! {
 			let result = Self::execute_wasm(origin, &mut gas_meter, |ctx, gas_meter| {
 				ctx.call(dest, value, gas_meter, data)
 			});
-			gas_meter.into_dispatch_result(result.map_err(|e| e.reason))
+			gas_meter.into_dispatch_result(result)
 		}
 
 		/// Instantiates a new contract from the `codehash` generated by `put_code`, optionally transferring some balance.
@@ -543,7 +547,7 @@ decl_module! {
 				ctx.instantiate(endowment, gas_meter, &code_hash, data)
 					.map(|(_address, output)| output)
 			});
-			gas_meter.into_dispatch_result(result.map_err(|e| e.reason))
+			gas_meter.into_dispatch_result(result)
 		}
 
 		/// Allows block producers to claim a small reward for evicting a contract. If a block producer
@@ -587,17 +591,22 @@ impl<T: Trait> Module<T> {
 	///
 	/// This function is similar to `Self::call`, but doesn't perform any address lookups and better
 	/// suitable for calling directly from Rust.
+	///
+	/// It returns the exection result and the amount of used weight.
 	pub fn bare_call(
 		origin: T::AccountId,
 		dest: T::AccountId,
 		value: BalanceOf<T>,
 		gas_limit: Gas,
 		input_data: Vec<u8>,
-	) -> ExecResult {
+	) -> (ExecResult, Gas) {
 		let mut gas_meter = GasMeter::new(gas_limit);
-		Self::execute_wasm(origin, &mut gas_meter, |ctx, gas_meter| {
-			ctx.call(dest, value, gas_meter, input_data)
-		})
+		(
+			Self::execute_wasm(origin, &mut gas_meter, |ctx, gas_meter| {
+				ctx.call(dest, value, gas_meter, input_data)
+			}),
+			gas_meter.gas_spent(),
+		)
 	}
 
 	/// Query storage of a specified contract under a specified key.
@@ -673,10 +682,6 @@ decl_event! {
 		/// Triggered when the current schedule is updated.
 		ScheduleUpdated(u32),
 
-		/// A call was dispatched from the given account. The bool signals whether it was
-		/// successful execution or not.
-		Dispatched(AccountId, bool),
-
 		/// An event deposited upon execution of a contract from the account.
 		ContractExecution(AccountId, Vec<u8>),
 	}

substrate/frame/contracts/src/tests.rs

@@ -17,6 +17,7 @@
 use crate::{
 	BalanceOf, ContractAddressFor, ContractInfo, ContractInfoOf, GenesisConfig, Module,
 	RawAliveContractInfo, RawEvent, Trait, TrieId, Schedule, TrieIdGenerator, gas::Gas,
+	Error,
 };
 use assert_matches::assert_matches;
 use hex_literal::*;
@@ -478,7 +479,7 @@ fn run_out_of_gas() {
 					67_500_000,
 					vec![],
 				),
-				"ran out of gas during contract execution"
+				Error::<Test>::OutOfGas,
 			);
 		});
 }
@@ -1169,7 +1170,7 @@ fn restoration(test_different_storage: bool, test_restore_to_with_dirty_storage:
 					DJANGO,
 					0,
 					GAS_LIMIT,
-					vec![],
+					set_rent_code_hash.as_ref().to_vec(),
 				)
 			};
 
@@ -1294,7 +1295,7 @@ fn storage_max_value_limit() {
 				Origin::signed(ALICE),
 				BOB,
 				0,
-				GAS_LIMIT,
+				GAS_LIMIT * 2, // we are copying a huge buffer
 				Encode::encode(&self::MaxValueSize::get()),
 			));
 
@@ -1594,8 +1595,8 @@ fn crypto_hashes() {
 					0,
 					GAS_LIMIT,
 					params,
-				).unwrap();
-				assert_eq!(result.status, 0);
+				).0.unwrap();
+				assert!(result.is_success());
 				let expected = hash_fn(input.as_ref());
 				assert_eq!(&result.data[..*expected_size], &*expected);
 			}

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

@@ -227,11 +227,7 @@ impl<'a> ContractModule<'a> {
 			};
 
 			// Then check the signature.
-			// Both "call" and "deploy" has a [] -> [] or [] -> [i32] function type.
-			//
-			// The [] -> [] signature predates the [] -> [i32] signature and is supported for
-			// backwards compatibility. This will likely be removed once ink! is updated to
-			// generate modules with the new function signatures.
+			// Both "call" and "deploy" has a () -> () function type.
 			let func_ty_idx = func_entries.get(fn_idx as usize)
 				.ok_or_else(|| "export refers to non-existent function")?
 				.type_ref();