contracts: add events to ContractResult (paritytech#13807)

* contracts: add events to ContractResult

* contracts: add encoded events to ContractResult

* contracts: add generic Event to ContractResult

* contracts: test bare_call events

* contracts: update bare_call test name

* contracts: add better comments to dry run events

* contracts: fix pallet contracts primitives implementation

* contracts: add EventRecord generic to ContractInstantiateResult

* contracts: make event collection optional

* contracts: impreved notes on `collect_events`

* contracts: update benchmarking calls

* contracts: change bare_call and bare_instantiate to accept enums instead of bools

* contracts: add partial eq to new enums

* contracts: improve comments

* contracts: improve comments

* contracts: fix bare_call benchmarking

* contracts: fix bare call and instantiate in impl_runtime_apis

* contracts: add api versioning to new ContractsApi functions

* contracts: modify api versioning to new ContractsApi functions

* contracts: create new contracts api trait

* contracts: clean up code

* contracts: remove the contract results with events

* contracts: undo contracts api v3

* contracts: remove commented out code

* contracts: add new test bare call result does not return events

* contracts: add code review improvements

* contracts: remove type imports

* contracts: minor code review improvements

Author: juangirini

bin/node/runtime/src/lib.rs

@@ -1880,6 +1880,11 @@ type Migrations = (
 	pallet_contracts::Migration<Runtime>,
 );
 
+type EventRecord = frame_system::EventRecord<
+	<Runtime as frame_system::Config>::RuntimeEvent,
+	<Runtime as frame_system::Config>::Hash,
+>;
+
 /// MMR helper types.
 mod mmr {
 	use super::Runtime;
@@ -2141,7 +2146,7 @@ impl_runtime_apis! {
 		}
 	}
 
-	impl pallet_contracts::ContractsApi<Block, AccountId, Balance, BlockNumber, Hash> for Runtime
+	impl pallet_contracts::ContractsApi<Block, AccountId, Balance, BlockNumber, Hash, EventRecord> for Runtime
 	{
 		fn call(
 			origin: AccountId,
@@ -2150,7 +2155,7 @@ impl_runtime_apis! {
 			gas_limit: Option<Weight>,
 			storage_deposit_limit: Option<Balance>,
 			input_data: Vec<u8>,
-		) -> pallet_contracts_primitives::ContractExecResult<Balance> {
+		) -> pallet_contracts_primitives::ContractExecResult<Balance, EventRecord> {
 			let gas_limit = gas_limit.unwrap_or(RuntimeBlockWeights::get().max_block);
 			Contracts::bare_call(
 				origin,
@@ -2159,7 +2164,8 @@ impl_runtime_apis! {
 				gas_limit,
 				storage_deposit_limit,
 				input_data,
-				true,
+				pallet_contracts::DebugInfo::UnsafeDebug,
+				pallet_contracts::CollectEvents::UnsafeCollect,
 				pallet_contracts::Determinism::Enforced,
 			)
 		}
@@ -2172,7 +2178,7 @@ impl_runtime_apis! {
 			code: pallet_contracts_primitives::Code<Hash>,
 			data: Vec<u8>,
 			salt: Vec<u8>,
-		) -> pallet_contracts_primitives::ContractInstantiateResult<AccountId, Balance>
+		) -> pallet_contracts_primitives::ContractInstantiateResult<AccountId, Balance, EventRecord>
 		{
 			let gas_limit = gas_limit.unwrap_or(RuntimeBlockWeights::get().max_block);
 			Contracts::bare_instantiate(
@@ -2183,7 +2189,8 @@ impl_runtime_apis! {
 				code,
 				data,
 				salt,
-				true
+				pallet_contracts::DebugInfo::UnsafeDebug,
+				pallet_contracts::CollectEvents::UnsafeCollect,
 			)
 		}
 

frame/contracts/primitives/src/lib.rs

@@ -29,11 +29,18 @@ use sp_runtime::{
 use sp_std::prelude::*;
 use sp_weights::Weight;
 
-/// Result type of a `bare_call` or `bare_instantiate` call.
+/// Result type of a `bare_call` or `bare_instantiate` call as well as `ContractsApi::call` and
+/// `ContractsApi::instantiate`.
 ///
 /// It contains the execution result together with some auxiliary information.
+///
+/// #Note
+///
+/// It has been extended to include `events` at the end of the struct while not bumping the
+/// `ContractsApi` version. Therefore when SCALE decoding a `ContractResult` its trailing data
+/// should be ignored to avoid any potential compatibility issues.
 #[derive(Eq, PartialEq, Encode, Decode, RuntimeDebug, TypeInfo)]
-pub struct ContractResult<R, Balance> {
+pub struct ContractResult<R, Balance, EventRecord> {
 	/// How much weight was consumed during execution.
 	pub gas_consumed: Weight,
 	/// How much weight is required as gas limit in order to execute this call.
@@ -71,15 +78,18 @@ pub struct ContractResult<R, Balance> {
 	pub debug_message: Vec<u8>,
 	/// The execution result of the wasm code.
 	pub result: R,
+	/// The events that were emitted during execution. It is an option as event collection is
+	/// optional.
+	pub events: Option<Vec<EventRecord>>,
 }
 
-/// Result type of a `bare_call` call.
-pub type ContractExecResult<Balance> =
-	ContractResult<Result<ExecReturnValue, DispatchError>, Balance>;
+/// Result type of a `bare_call` call as well as `ContractsApi::call`.
+pub type ContractExecResult<Balance, EventRecord> =
+	ContractResult<Result<ExecReturnValue, DispatchError>, Balance, EventRecord>;
 
-/// Result type of a `bare_instantiate` call.
-pub type ContractInstantiateResult<AccountId, Balance> =
-	ContractResult<Result<InstantiateReturnValue<AccountId>, DispatchError>, Balance>;
+/// Result type of a `bare_instantiate` call as well as `ContractsApi::instantiate`.
+pub type ContractInstantiateResult<AccountId, Balance, EventRecord> =
+	ContractResult<Result<InstantiateReturnValue<AccountId>, DispatchError>, Balance, EventRecord>;
 
 /// Result type of a `bare_code_upload` call.
 pub type CodeUploadResult<CodeHash, Balance> =

frame/contracts/src/benchmarking/mod.rs

@@ -951,7 +951,8 @@ benchmarks! {
 			Weight::MAX,
 			None,
 			vec![],
-			true,
+			DebugInfo::UnsafeDebug,
+			CollectEvents::Skip,
 			Determinism::Enforced,
 		)
 		.result?;
@@ -1000,7 +1001,8 @@ benchmarks! {
 			Weight::MAX,
 			None,
 			vec![],
-			true,
+			DebugInfo::UnsafeDebug,
+			CollectEvents::Skip,
 			Determinism::Enforced,
 		)
 		.result?;
@@ -3192,7 +3194,8 @@ benchmarks! {
 			Weight::MAX,
 			None,
 			data,
-			false,
+			DebugInfo::Skip,
+			CollectEvents::Skip,
 			Determinism::Enforced,
 		)
 		.result?;
@@ -3241,7 +3244,8 @@ benchmarks! {
 			Weight::MAX,
 			None,
 			data,
-			false,
+			DebugInfo::Skip,
+			CollectEvents::Skip,
 			Determinism::Enforced,
 		)
 		.result?;

frame/contracts/src/lib.rs

@@ -117,7 +117,7 @@ use frame_support::{
 	weights::Weight,
 	BoundedVec, RuntimeDebugNoBound, WeakBoundedVec,
 };
-use frame_system::{ensure_signed, pallet_prelude::OriginFor, Pallet as System};
+use frame_system::{ensure_signed, pallet_prelude::OriginFor, EventRecord, Pallet as System};
 use pallet_contracts_primitives::{
 	Code, CodeUploadResult, CodeUploadReturnValue, ContractAccessError, ContractExecResult,
 	ContractInstantiateResult, ExecReturnValue, GetStorageResult, InstantiateReturnValue,
@@ -148,6 +148,8 @@ type CodeVec<T> = BoundedVec<u8, <T as Config>::MaxCodeLen>;
 type RelaxedCodeVec<T> = WeakBoundedVec<u8, <T as Config>::MaxCodeLen>;
 type AccountIdLookupOf<T> = <<T as frame_system::Config>::Lookup as StaticLookup>::Source;
 type DebugBufferVec<T> = BoundedVec<u8, <T as Config>::MaxDebugBufferLen>;
+type EventRecordOf<T> =
+	EventRecord<<T as frame_system::Config>::RuntimeEvent, <T as frame_system::Config>::Hash>;
 
 /// The old weight type.
 ///
@@ -971,6 +973,35 @@ struct InstantiateInput<T: Config> {
 	salt: Vec<u8>,
 }
 
+/// Determines whether events should be collected during execution.
+#[derive(PartialEq)]
+pub enum CollectEvents {
+	/// Collect events.
+	///
+	/// # Note
+	///
+	/// Events should only be collected when called off-chain, as this would otherwise
+	/// collect all the Events emitted in the block so far and put them into the PoV.
+	///
+	/// **Never** use this mode for on-chain execution.
+	UnsafeCollect,
+	/// Skip event collection.
+	Skip,
+}
+
+/// Determines whether debug messages will be collected.
+#[derive(PartialEq)]
+pub enum DebugInfo {
+	/// Collect debug messages.
+	/// # Note
+	///
+	/// This should only ever be set to `UnsafeDebug` when executing as an RPC because
+	/// it adds allocations and could be abused to drive the runtime into an OOM panic.
+	UnsafeDebug,
+	/// Skip collection of debug messages.
+	Skip,
+}
+
 /// Return type of private helper functions.
 struct InternalOutput<T: Config, O> {
 	/// The gas meter that was used to execute the call.
@@ -1187,22 +1218,27 @@ impl<T: Config> Pallet<T> {
 	///
 	/// # Note
 	///
-	/// `debug` should only ever be set to `true` when executing as an RPC because
-	/// it adds allocations and could be abused to drive the runtime into an OOM panic.
-	/// If set to `true` it returns additional human readable debugging information.
+	/// If `debug` is set to `DebugInfo::UnsafeDebug` it returns additional human readable debugging
+	/// information.
 	///
-	/// It returns the execution result and the amount of used weight.
+	/// If `collect_events` is set to `CollectEvents::UnsafeCollect` it collects all the Events
+	/// emitted in the block so far and the ones emitted during the execution of this contract.
 	pub fn bare_call(
 		origin: T::AccountId,
 		dest: T::AccountId,
 		value: BalanceOf<T>,
 		gas_limit: Weight,
 		storage_deposit_limit: Option<BalanceOf<T>>,
 		data: Vec<u8>,
-		debug: bool,
+		debug: DebugInfo,
+		collect_events: CollectEvents,
 		determinism: Determinism,
-	) -> ContractExecResult<BalanceOf<T>> {
-		let mut debug_message = if debug { Some(DebugBufferVec::<T>::default()) } else { None };
+	) -> ContractExecResult<BalanceOf<T>, EventRecordOf<T>> {
+		let mut debug_message = if matches!(debug, DebugInfo::UnsafeDebug) {
+			Some(DebugBufferVec::<T>::default())
+		} else {
+			None
+		};
 		let origin = Origin::from_account_id(origin);
 		let common = CommonInput {
 			origin,
@@ -1213,12 +1249,19 @@ impl<T: Config> Pallet<T> {
 			debug_message: debug_message.as_mut(),
 		};
 		let output = CallInput::<T> { dest, determinism }.run_guarded(common);
+		let events = if matches!(collect_events, CollectEvents::UnsafeCollect) {
+			Some(System::<T>::read_events_no_consensus().map(|e| *e).collect())
+		} else {
+			None
+		};
+
 		ContractExecResult {
 			result: output.result.map_err(|r| r.error),
 			gas_consumed: output.gas_meter.gas_consumed(),
 			gas_required: output.gas_meter.gas_required(),
 			storage_deposit: output.storage_deposit,
 			debug_message: debug_message.unwrap_or_default().to_vec(),
+			events,
 		}
 	}
 
@@ -1231,9 +1274,11 @@ impl<T: Config> Pallet<T> {
 	///
 	/// # Note
 	///
-	/// `debug` should only ever be set to `true` when executing as an RPC because
-	/// it adds allocations and could be abused to drive the runtime into an OOM panic.
-	/// If set to `true` it returns additional human readable debugging information.
+	/// If `debug` is set to `DebugInfo::UnsafeDebug` it returns additional human readable debugging
+	/// information.
+	///
+	/// If `collect_events` is set to `CollectEvents::UnsafeCollect` it collects all the Events
+	/// emitted in the block so far.
 	pub fn bare_instantiate(
 		origin: T::AccountId,
 		value: BalanceOf<T>,
@@ -1242,9 +1287,14 @@ impl<T: Config> Pallet<T> {
 		code: Code<CodeHash<T>>,
 		data: Vec<u8>,
 		salt: Vec<u8>,
-		debug: bool,
-	) -> ContractInstantiateResult<T::AccountId, BalanceOf<T>> {
-		let mut debug_message = if debug { Some(DebugBufferVec::<T>::default()) } else { None };
+		debug: DebugInfo,
+		collect_events: CollectEvents,
+	) -> ContractInstantiateResult<T::AccountId, BalanceOf<T>, EventRecordOf<T>> {
+		let mut debug_message = if debug == DebugInfo::UnsafeDebug {
+			Some(DebugBufferVec::<T>::default())
+		} else {
+			None
+		};
 		let common = CommonInput {
 			origin: Origin::from_account_id(origin),
 			value,
@@ -1254,6 +1304,12 @@ impl<T: Config> Pallet<T> {
 			debug_message: debug_message.as_mut(),
 		};
 		let output = InstantiateInput::<T> { code, salt }.run_guarded(common);
+		// collect events if CollectEvents is UnsafeCollect
+		let events = if collect_events == CollectEvents::UnsafeCollect {
+			Some(System::<T>::read_events_no_consensus().map(|e| *e).collect())
+		} else {
+			None
+		};
 		ContractInstantiateResult {
 			result: output
 				.result
@@ -1263,6 +1319,7 @@ impl<T: Config> Pallet<T> {
 			gas_required: output.gas_meter.gas_required(),
 			storage_deposit: output.storage_deposit,
 			debug_message: debug_message.unwrap_or_default().to_vec(),
+			events,
 		}
 	}
 
@@ -1370,11 +1427,12 @@ impl<T: Config> Pallet<T> {
 sp_api::decl_runtime_apis! {
 	/// The API used to dry-run contract interactions.
 	#[api_version(2)]
-	pub trait ContractsApi<AccountId, Balance, BlockNumber, Hash> where
+	pub trait ContractsApi<AccountId, Balance, BlockNumber, Hash, EventRecord> where
 		AccountId: Codec,
 		Balance: Codec,
 		BlockNumber: Codec,
 		Hash: Codec,
+		EventRecord: Codec,
 	{
 		/// Perform a call from a specified account to a given contract.
 		///
@@ -1386,7 +1444,7 @@ sp_api::decl_runtime_apis! {
 			gas_limit: Option<Weight>,
 			storage_deposit_limit: Option<Balance>,
 			input_data: Vec<u8>,
-		) -> ContractExecResult<Balance>;
+		) -> ContractExecResult<Balance, EventRecord>;
 
 		/// Instantiate a new contract.
 		///
@@ -1399,8 +1457,7 @@ sp_api::decl_runtime_apis! {
 			code: Code<Hash>,
 			data: Vec<u8>,
 			salt: Vec<u8>,
-		) -> ContractInstantiateResult<AccountId, Balance>;
-
+		) -> ContractInstantiateResult<AccountId, Balance, EventRecord>;
 
 		/// Upload new code without instantiating a contract from it.
 		///