Emit events for inter contract calls (#12136)

* Add topics to contract events

* Add `Call` events

* Fix compilation for no_std

* Added docs

substrate/frame/contracts/src/exec.rs

@@ -34,7 +34,7 @@ use pallet_contracts_primitives::ExecReturnValue;
 use smallvec::{Array, SmallVec};
 use sp_core::{crypto::UncheckedFrom, ecdsa::Public as ECDSAPublic};
 use sp_io::{crypto::secp256k1_ecdsa_recover_compressed, hashing::blake2_256};
-use sp_runtime::traits::Convert;
+use sp_runtime::traits::{Convert, Hash};
 use sp_std::{marker::PhantomData, mem, prelude::*};
 
 pub type AccountIdOf<T> = <T as frame_system::Config>::AccountId;
@@ -784,16 +784,19 @@ where
 	///
 	/// This can be either a call or an instantiate.
 	fn run(&mut self, executable: E, input_data: Vec<u8>) -> Result<ExecReturnValue, ExecError> {
-		let entry_point = self.top_frame().entry_point;
+		let frame = self.top_frame();
+		let entry_point = frame.entry_point;
+		let delegated_code_hash =
+			if frame.delegate_caller.is_some() { Some(*executable.code_hash()) } else { None };
 		let do_transaction = || {
 			// We need to charge the storage deposit before the initial transfer so that
 			// it can create the account in case the initial transfer is < ed.
 			if entry_point == ExportedFunction::Constructor {
-				let top_frame = top_frame_mut!(self);
-				top_frame.nested_storage.charge_instantiate(
+				let frame = top_frame_mut!(self);
+				frame.nested_storage.charge_instantiate(
 					&self.origin,
-					&top_frame.account_id,
-					top_frame.contract_info.get(&top_frame.account_id),
+					&frame.account_id,
+					frame.contract_info.get(&frame.account_id),
 				)?;
 			}
 
@@ -805,23 +808,42 @@ where
 				.execute(self, &entry_point, input_data)
 				.map_err(|e| ExecError { error: e.error, origin: ErrorOrigin::Callee })?;
 
-			// Additional work needs to be performed in case of an instantiation.
-			if !output.did_revert() && entry_point == ExportedFunction::Constructor {
-				let frame = self.top_frame();
+			// Avoid useless work that would be reverted anyways.
+			if output.did_revert() {
+				return Ok(output)
+			}
 
-				// It is not allowed to terminate a contract inside its constructor.
-				if matches!(frame.contract_info, CachedContract::Terminated) {
-					return Err(Error::<T>::TerminatedInConstructor.into())
-				}
+			let frame = self.top_frame();
+			let account_id = &frame.account_id;
+			match (entry_point, delegated_code_hash) {
+				(ExportedFunction::Constructor, _) => {
+					// It is not allowed to terminate a contract inside its constructor.
+					if matches!(frame.contract_info, CachedContract::Terminated) {
+						return Err(Error::<T>::TerminatedInConstructor.into())
+					}
 
-				// Deposit an instantiation event.
-				deposit_event::<T>(
-					vec![],
-					Event::Instantiated {
-						deployer: self.caller().clone(),
-						contract: frame.account_id.clone(),
-					},
-				);
+					// Deposit an instantiation event.
+					Contracts::<T>::deposit_event(
+						vec![T::Hashing::hash_of(self.caller()), T::Hashing::hash_of(account_id)],
+						Event::Instantiated {
+							deployer: self.caller().clone(),
+							contract: account_id.clone(),
+						},
+					);
+				},
+				(ExportedFunction::Call, Some(code_hash)) => {
+					Contracts::<T>::deposit_event(
+						vec![T::Hashing::hash_of(account_id), T::Hashing::hash_of(&code_hash)],
+						Event::DelegateCalled { contract: account_id.clone(), code_hash },
+					);
+				},
+				(ExportedFunction::Call, None) => {
+					let caller = self.caller();
+					Contracts::<T>::deposit_event(
+						vec![T::Hashing::hash_of(caller), T::Hashing::hash_of(account_id)],
+						Event::Called { caller: caller.clone(), contract: account_id.clone() },
+					);
+				},
 			}
 
 			Ok(output)
@@ -850,6 +872,7 @@ where
 			// has changed.
 			Err(error) => (false, Err(error.into())),
 		};
+
 		self.pop_frame(success);
 		output
 	}
@@ -1134,10 +1157,13 @@ where
 		)?;
 		ContractInfoOf::<T>::remove(&frame.account_id);
 		E::remove_user(info.code_hash);
-		Contracts::<T>::deposit_event(Event::Terminated {
-			contract: frame.account_id.clone(),
-			beneficiary: beneficiary.clone(),
-		});
+		Contracts::<T>::deposit_event(
+			vec![T::Hashing::hash_of(&frame.account_id), T::Hashing::hash_of(&beneficiary)],
+			Event::Terminated {
+				contract: frame.account_id.clone(),
+				beneficiary: beneficiary.clone(),
+			},
+		);
 		Ok(())
 	}
 
@@ -1242,7 +1268,7 @@ where
 	}
 
 	fn deposit_event(&mut self, topics: Vec<T::Hash>, data: Vec<u8>) {
-		deposit_event::<Self::T>(
+		Contracts::<Self::T>::deposit_event(
 			topics,
 			Event::ContractEmitted { contract: self.top_frame().account_id.clone(), data },
 		);
@@ -1304,22 +1330,18 @@ where
 		let prev_hash = top_frame.contract_info().code_hash;
 		E::remove_user(prev_hash);
 		top_frame.contract_info().code_hash = hash;
-		Contracts::<Self::T>::deposit_event(Event::ContractCodeUpdated {
-			contract: top_frame.account_id.clone(),
-			new_code_hash: hash,
-			old_code_hash: prev_hash,
-		});
+		Contracts::<Self::T>::deposit_event(
+			vec![T::Hashing::hash_of(&top_frame.account_id), hash, prev_hash],
+			Event::ContractCodeUpdated {
+				contract: top_frame.account_id.clone(),
+				new_code_hash: hash,
+				old_code_hash: prev_hash,
+			},
+		);
 		Ok(())
 	}
 }
 
-fn deposit_event<T: Config>(topics: Vec<T::Hash>, event: Event<T>) {
-	<frame_system::Pallet<T>>::deposit_event_indexed(
-		&topics,
-		<T as Config>::Event::from(event).into(),
-	)
-}
-
 mod sealing {
 	use super::*;
 
@@ -1347,7 +1369,7 @@ mod tests {
 		gas::GasMeter,
 		storage::Storage,
 		tests::{
-			test_utils::{get_balance, place_contract, set_balance},
+			test_utils::{get_balance, hash, place_contract, set_balance},
 			Call, Event as MetaEvent, ExtBuilder, Test, TestFilter, ALICE, BOB, CHARLIE, GAS_LIMIT,
 		},
 		Error,
@@ -2237,7 +2259,10 @@ mod tests {
 			);
 			assert_eq!(
 				&events(),
-				&[Event::Instantiated { deployer: BOB, contract: instantiated_contract_address }]
+				&[
+					Event::Instantiated { deployer: BOB, contract: instantiated_contract_address },
+					Event::Called { caller: ALICE, contract: BOB },
+				]
 			);
 		});
 	}
@@ -2289,7 +2314,7 @@ mod tests {
 
 			// The contract wasn't instantiated so we don't expect to see an instantiation
 			// event here.
-			assert_eq!(&events(), &[]);
+			assert_eq!(&events(), &[Event::Called { caller: ALICE, contract: BOB },]);
 		});
 	}
 
@@ -2591,14 +2616,24 @@ mod tests {
 			let remark_hash = <Test as frame_system::Config>::Hashing::hash(b"Hello World");
 			assert_eq!(
 				System::events(),
-				vec![EventRecord {
-					phase: Phase::Initialization,
-					event: MetaEvent::System(frame_system::Event::Remarked {
-						sender: BOB,
-						hash: remark_hash
-					}),
-					topics: vec![],
-				},]
+				vec![
+					EventRecord {
+						phase: Phase::Initialization,
+						event: MetaEvent::System(frame_system::Event::Remarked {
+							sender: BOB,
+							hash: remark_hash
+						}),
+						topics: vec![],
+					},
+					EventRecord {
+						phase: Phase::Initialization,
+						event: MetaEvent::Contracts(crate::Event::Called {
+							caller: ALICE,
+							contract: BOB,
+						}),
+						topics: vec![hash(&ALICE), hash(&BOB)],
+					},
+				]
 			);
 		});
 	}
@@ -2684,6 +2719,14 @@ mod tests {
 						},),
 						topics: vec![],
 					},
+					EventRecord {
+						phase: Phase::Initialization,
+						event: MetaEvent::Contracts(crate::Event::Called {
+							caller: ALICE,
+							contract: BOB,
+						}),
+						topics: vec![hash(&ALICE), hash(&BOB)],
+					},
 				]
 			);
 		});

substrate/frame/contracts/src/lib.rs

@@ -631,11 +631,14 @@ pub mod pallet {
 				};
 				<PrefabWasmModule<T>>::add_user(code_hash)?;
 				<PrefabWasmModule<T>>::remove_user(contract.code_hash);
-				Self::deposit_event(Event::ContractCodeUpdated {
-					contract: dest.clone(),
-					new_code_hash: code_hash,
-					old_code_hash: contract.code_hash,
-				});
+				Self::deposit_event(
+					vec![T::Hashing::hash_of(&dest), code_hash, contract.code_hash],
+					Event::ContractCodeUpdated {
+						contract: dest.clone(),
+						new_code_hash: code_hash,
+						old_code_hash: contract.code_hash,
+					},
+				);
 				contract.code_hash = code_hash;
 				Ok(())
 			})
@@ -643,7 +646,6 @@ pub mod pallet {
 	}
 
 	#[pallet::event]
-	#[pallet::generate_deposit(pub(super) fn deposit_event)]
 	pub enum Event<T: Config> {
 		/// Contract deployed by address at the specified address.
 		Instantiated { deployer: T::AccountId, contract: T::AccountId },
@@ -685,6 +687,35 @@ pub mod pallet {
 			/// Previous code hash of the contract.
 			old_code_hash: T::Hash,
 		},
+
+		/// A contract was called either by a plain account or another contract.
+		///
+		/// # Note
+		///
+		/// Please keep in mind that like all events this is only emitted for successful
+		/// calls. This is because on failure all storage changes including events are
+		/// rolled back.
+		Called {
+			/// The account that called the `contract`.
+			caller: T::AccountId,
+			/// The contract that was called.
+			contract: T::AccountId,
+		},
+
+		/// A contract delegate called a code hash.
+		///
+		/// # Note
+		///
+		/// Please keep in mind that like all events this is only emitted for successful
+		/// calls. This is because on failure all storage changes including events are
+		/// rolled back.
+		DelegateCalled {
+			/// The contract that performed the delegate call and hence in whose context
+			/// the `code_hash` is executed.
+			contract: T::AccountId,
+			/// The code hash that was delegate called.
+			code_hash: CodeHash<T>,
+		},
 	}
 
 	#[pallet::error]
@@ -1084,4 +1115,11 @@ where
 		};
 		InternalInstantiateOutput { result: try_exec(), gas_meter, storage_deposit }
 	}
+
+	fn deposit_event(topics: Vec<T::Hash>, event: Event<T>) {
+		<frame_system::Pallet<T>>::deposit_event_indexed(
+			&topics,
+			<T as Config>::Event::from(event).into(),
+		)
+	}
 }

substrate/frame/contracts/src/tests.rs

@@ -15,6 +15,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+use self::test_utils::hash;
 use crate::{
 	chain_extension::{
 		ChainExtension, Environment, Ext, InitState, RegisteredChainExtension,
@@ -74,8 +75,9 @@ frame_support::construct_runtime!(
 
 #[macro_use]
 pub mod test_utils {
-	use super::{Balances, Test};
+	use super::{Balances, Hash, SysConfig, Test};
 	use crate::{exec::AccountIdOf, storage::Storage, CodeHash, Config, ContractInfoOf, Nonce};
+	use codec::Encode;
 	use frame_support::traits::Currency;
 
 	pub fn place_contract(address: &AccountIdOf<Test>, code_hash: CodeHash<Test>) {
@@ -95,6 +97,9 @@ pub mod test_utils {
 	pub fn get_balance(who: &AccountIdOf<Test>) -> u64 {
 		Balances::free_balance(who)
 	}
+	pub fn hash<S: Encode>(s: &S) -> <<Test as SysConfig>::Hashing as Hash>::Output {
+		<<Test as SysConfig>::Hashing as Hash>::hash_of(s)
+	}
 	macro_rules! assert_return_code {
 		( $x:expr , $y:expr $(,)? ) => {{
 			assert_eq!(u32::from_le_bytes($x.data[..].try_into().unwrap()), $y as u32);
@@ -571,7 +576,7 @@ fn instantiate_and_call_and_deposit_event() {
 						deployer: ALICE,
 						contract: addr.clone()
 					}),
-					topics: vec![],
+					topics: vec![hash(&ALICE), hash(&addr)],
 				},
 			]
 		);
@@ -857,7 +862,7 @@ fn deploy_and_call_other_contract() {
 						deployer: caller_addr.clone(),
 						contract: callee_addr.clone(),
 					}),
-					topics: vec![],
+					topics: vec![hash(&caller_addr), hash(&callee_addr)],
 				},
 				EventRecord {
 					phase: Phase::Initialization,
@@ -868,6 +873,22 @@ fn deploy_and_call_other_contract() {
 					}),
 					topics: vec![],
 				},
+				EventRecord {
+					phase: Phase::Initialization,
+					event: Event::Contracts(crate::Event::Called {
+						caller: caller_addr.clone(),
+						contract: callee_addr.clone(),
+					}),
+					topics: vec![hash(&caller_addr), hash(&callee_addr)],
+				},
+				EventRecord {
+					phase: Phase::Initialization,
+					event: Event::Contracts(crate::Event::Called {
+						caller: ALICE,
+						contract: caller_addr.clone(),
+					}),
+					topics: vec![hash(&ALICE), hash(&caller_addr)],
+				},
 			]
 		);
 	});
@@ -1067,6 +1088,14 @@ fn cannot_self_destruct_by_refund_after_slash() {
 					}),
 					topics: vec![],
 				},
+				EventRecord {
+					phase: Phase::Initialization,
+					event: Event::Contracts(crate::Event::Called {
+						caller: ALICE,
+						contract: addr.clone(),
+					}),
+					topics: vec![hash(&ALICE), hash(&addr)],
+				},
 				EventRecord {
 					phase: Phase::Initialization,
 					event: Event::Balances(pallet_balances::Event::ReserveRepatriated {
@@ -1174,7 +1203,15 @@ fn self_destruct_works() {
 						contract: addr.clone(),
 						beneficiary: DJANGO
 					}),
-					topics: vec![],
+					topics: vec![hash(&addr), hash(&DJANGO)],
+				},
+				EventRecord {
+					phase: Phase::Initialization,
+					event: Event::Contracts(crate::Event::Called {
+						caller: ALICE,
+						contract: addr.clone(),
+					}),
+					topics: vec![hash(&ALICE), hash(&addr)],
 				},
 				EventRecord {
 					phase: Phase::Initialization,
@@ -2621,7 +2658,7 @@ fn upload_code_works() {
 				EventRecord {
 					phase: Phase::Initialization,
 					event: Event::Contracts(crate::Event::CodeStored { code_hash }),
-					topics: vec![],
+					topics: vec![code_hash],
 				},
 			]
 		);
@@ -2700,7 +2737,7 @@ fn remove_code_works() {
 				EventRecord {
 					phase: Phase::Initialization,
 					event: Event::Contracts(crate::Event::CodeStored { code_hash }),
-					topics: vec![],
+					topics: vec![code_hash],
 				},
 				EventRecord {
 					phase: Phase::Initialization,
@@ -2713,7 +2750,7 @@ fn remove_code_works() {
 				EventRecord {
 					phase: Phase::Initialization,
 					event: Event::Contracts(crate::Event::CodeRemoved { code_hash }),
-					topics: vec![],
+					topics: vec![code_hash],
 				},
 			]
 		);
@@ -2755,7 +2792,7 @@ fn remove_code_wrong_origin() {
 				EventRecord {
 					phase: Phase::Initialization,
 					event: Event::Contracts(crate::Event::CodeStored { code_hash }),
-					topics: vec![],
+					topics: vec![code_hash],
 				},
 			]
 		);
@@ -2886,7 +2923,7 @@ fn instantiate_with_zero_balance_works() {
 				EventRecord {
 					phase: Phase::Initialization,
 					event: Event::Contracts(crate::Event::CodeStored { code_hash }),
-					topics: vec![],
+					topics: vec![code_hash],
 				},
 				EventRecord {
 					phase: Phase::Initialization,
@@ -2894,7 +2931,7 @@ fn instantiate_with_zero_balance_works() {
 						deployer: ALICE,
 						contract: addr.clone(),
 					}),
-					topics: vec![],
+					topics: vec![hash(&ALICE), hash(&addr)],
 				},
 			]
 		);
@@ -2986,7 +3023,7 @@ fn instantiate_with_below_existential_deposit_works() {
 				EventRecord {
 					phase: Phase::Initialization,
 					event: Event::Contracts(crate::Event::CodeStored { code_hash }),
-					topics: vec![],
+					topics: vec![code_hash],
 				},
 				EventRecord {
 					phase: Phase::Initialization,
@@ -2994,7 +3031,7 @@ fn instantiate_with_below_existential_deposit_works() {
 						deployer: ALICE,
 						contract: addr.clone(),
 					}),
-					topics: vec![],
+					topics: vec![hash(&ALICE), hash(&addr)],
 				},
 			]
 		);
@@ -3074,6 +3111,14 @@ fn storage_deposit_works() {
 					}),
 					topics: vec![],
 				},
+				EventRecord {
+					phase: Phase::Initialization,
+					event: Event::Contracts(crate::Event::Called {
+						caller: ALICE,
+						contract: addr.clone(),
+					}),
+					topics: vec![hash(&ALICE), hash(&addr)],
+				},
 				EventRecord {
 					phase: Phase::Initialization,
 					event: Event::Balances(pallet_balances::Event::Transfer {
@@ -3091,6 +3136,14 @@ fn storage_deposit_works() {
 					}),
 					topics: vec![],
 				},
+				EventRecord {
+					phase: Phase::Initialization,
+					event: Event::Contracts(crate::Event::Called {
+						caller: ALICE,
+						contract: addr.clone(),
+					}),
+					topics: vec![hash(&ALICE), hash(&addr)],
+				},
 				EventRecord {
 					phase: Phase::Initialization,
 					event: Event::Balances(pallet_balances::Event::Transfer {
@@ -3108,6 +3161,14 @@ fn storage_deposit_works() {
 					}),
 					topics: vec![],
 				},
+				EventRecord {
+					phase: Phase::Initialization,
+					event: Event::Contracts(crate::Event::Called {
+						caller: ALICE,
+						contract: addr.clone(),
+					}),
+					topics: vec![hash(&ALICE), hash(&addr)],
+				},
 				EventRecord {
 					phase: Phase::Initialization,
 					event: Event::Balances(pallet_balances::Event::ReserveRepatriated {
@@ -3193,11 +3254,11 @@ fn set_code_extrinsic() {
 			vec![EventRecord {
 				phase: Phase::Initialization,
 				event: Event::Contracts(pallet_contracts::Event::ContractCodeUpdated {
-					contract: addr,
+					contract: addr.clone(),
 					new_code_hash,
 					old_code_hash: code_hash,
 				}),
-				topics: vec![],
+				topics: vec![hash(&addr), new_code_hash, code_hash],
 			},]
 		);
 	});
@@ -3226,7 +3287,7 @@ fn call_after_killed_account_needs_funding() {
 
 		// Destroy the account of the contract by slashing.
 		// Slashing can actually happen if the contract takes part in staking.
-		// It is a corner case and we except the destruction of the account.
+		// It is a corner case and we accept the destruction of the account.
 		let _ = <Test as Config>::Currency::slash(
 			&addr,
 			<Test as Config>::Currency::total_balance(&addr),
@@ -3284,6 +3345,14 @@ fn call_after_killed_account_needs_funding() {
 					}),
 					topics: vec![],
 				},
+				EventRecord {
+					phase: Phase::Initialization,
+					event: Event::Contracts(crate::Event::Called {
+						caller: ALICE,
+						contract: addr.clone(),
+					}),
+					topics: vec![hash(&ALICE), hash(&addr)],
+				},
 				EventRecord {
 					phase: Phase::Initialization,
 					event: Event::System(frame_system::Event::NewAccount { account: addr.clone() }),
@@ -3306,6 +3375,14 @@ fn call_after_killed_account_needs_funding() {
 					}),
 					topics: vec![],
 				},
+				EventRecord {
+					phase: Phase::Initialization,
+					event: Event::Contracts(crate::Event::Called {
+						caller: ALICE,
+						contract: addr.clone(),
+					}),
+					topics: vec![hash(&ALICE), hash(&addr)],
+				},
 			]
 		);
 	});
@@ -3454,6 +3531,8 @@ fn set_code_hash() {
 		// upload new code
 		assert_ok!(Contracts::upload_code(Origin::signed(ALICE), new_wasm.clone(), None));
 
+		System::reset_events();
+
 		// First call sets new code_hash and returns 1
 		let result = Contracts::bare_call(
 			ALICE,
@@ -3477,16 +3556,34 @@ fn set_code_hash() {
 
 		// Checking for the last event only
 		assert_eq!(
-			System::events().pop().unwrap(),
-			EventRecord {
-				phase: Phase::Initialization,
-				event: Event::Contracts(crate::Event::ContractCodeUpdated {
-					contract: contract_addr.clone(),
-					new_code_hash,
-					old_code_hash: code_hash,
-				}),
-				topics: vec![],
-			},
+			&System::events(),
+			&[
+				EventRecord {
+					phase: Phase::Initialization,
+					event: Event::Contracts(crate::Event::ContractCodeUpdated {
+						contract: contract_addr.clone(),
+						new_code_hash,
+						old_code_hash: code_hash,
+					}),
+					topics: vec![hash(&contract_addr), new_code_hash, code_hash],
+				},
+				EventRecord {
+					phase: Phase::Initialization,
+					event: Event::Contracts(crate::Event::Called {
+						caller: ALICE,
+						contract: contract_addr.clone(),
+					}),
+					topics: vec![hash(&ALICE), hash(&contract_addr)],
+				},
+				EventRecord {
+					phase: Phase::Initialization,
+					event: Event::Contracts(crate::Event::Called {
+						caller: ALICE,
+						contract: contract_addr.clone(),
+					}),
+					topics: vec![hash(&ALICE), hash(&contract_addr)],
+				},
+			],
 		);
 	});
 }

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

@@ -42,6 +42,7 @@ use frame_support::{
 };
 use sp_core::crypto::UncheckedFrom;
 use sp_runtime::traits::BadOrigin;
+use sp_std::vec;
 
 /// Put the instrumented module in storage.
 ///
@@ -96,7 +97,7 @@ where
 			<PristineCode<T>>::insert(&code_hash, orig_code);
 			<OwnerInfoOf<T>>::insert(&code_hash, owner_info);
 			*existing = Some(module);
-			<Pallet<T>>::deposit_event(Event::CodeStored { code_hash });
+			<Pallet<T>>::deposit_event(vec![code_hash], Event::CodeStored { code_hash });
 			Ok(())
 		},
 	})
@@ -133,7 +134,10 @@ pub fn increment_refcount<T: Config>(code_hash: CodeHash<T>) -> Result<(), Dispa
 }
 
 /// Try to remove code together with all associated information.
-pub fn try_remove<T: Config>(origin: &T::AccountId, code_hash: CodeHash<T>) -> DispatchResult {
+pub fn try_remove<T: Config>(origin: &T::AccountId, code_hash: CodeHash<T>) -> DispatchResult
+where
+	T::AccountId: UncheckedFrom<T::Hash> + AsRef<[u8]>,
+{
 	<OwnerInfoOf<T>>::try_mutate_exists(&code_hash, |existing| {
 		if let Some(owner_info) = existing {
 			ensure!(owner_info.refcount == 0, <Error<T>>::CodeInUse);
@@ -142,7 +146,7 @@ pub fn try_remove<T: Config>(origin: &T::AccountId, code_hash: CodeHash<T>) -> D
 			*existing = None;
 			<PristineCode<T>>::remove(&code_hash);
 			<CodeStorage<T>>::remove(&code_hash);
-			<Pallet<T>>::deposit_event(Event::CodeRemoved { code_hash });
+			<Pallet<T>>::deposit_event(vec![code_hash], Event::CodeRemoved { code_hash });
 			Ok(())
 		} else {
 			Err(<Error<T>>::CodeNotFound.into())