diff --git a/substrate/frame/contracts/src/exec.rs b/substrate/frame/contracts/src/exec.rs index 81e1aef92c..4ae336a907 100644 --- a/substrate/frame/contracts/src/exec.rs +++ b/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 = ::AccountId; @@ -784,16 +784,19 @@ where /// /// This can be either a call or an instantiate. fn run(&mut self, executable: E, input_data: Vec) -> Result { - 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::::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::::TerminatedInConstructor.into()) + } - // Deposit an instantiation event. - deposit_event::( - vec![], - Event::Instantiated { - deployer: self.caller().clone(), - contract: frame.account_id.clone(), - }, - ); + // Deposit an instantiation event. + Contracts::::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::::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::::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::::remove(&frame.account_id); E::remove_user(info.code_hash); - Contracts::::deposit_event(Event::Terminated { - contract: frame.account_id.clone(), - beneficiary: beneficiary.clone(), - }); + Contracts::::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, data: Vec) { - deposit_event::( + Contracts::::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::::deposit_event(Event::ContractCodeUpdated { - contract: top_frame.account_id.clone(), - new_code_hash: hash, - old_code_hash: prev_hash, - }); + Contracts::::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(topics: Vec, event: Event) { - >::deposit_event_indexed( - &topics, - ::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 = ::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)], + }, ] ); }); diff --git a/substrate/frame/contracts/src/lib.rs b/substrate/frame/contracts/src/lib.rs index 7fda4bca36..3c63ad8601 100644 --- a/substrate/frame/contracts/src/lib.rs +++ b/substrate/frame/contracts/src/lib.rs @@ -631,11 +631,14 @@ pub mod pallet { }; >::add_user(code_hash)?; >::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 { /// 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, + }, } #[pallet::error] @@ -1084,4 +1115,11 @@ where }; InternalInstantiateOutput { result: try_exec(), gas_meter, storage_deposit } } + + fn deposit_event(topics: Vec, event: Event) { + >::deposit_event_indexed( + &topics, + ::Event::from(event).into(), + ) + } } diff --git a/substrate/frame/contracts/src/tests.rs b/substrate/frame/contracts/src/tests.rs index f4fa6bb3d7..f65f845f7b 100644 --- a/substrate/frame/contracts/src/tests.rs +++ b/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, code_hash: CodeHash) { @@ -95,6 +97,9 @@ pub mod test_utils { pub fn get_balance(who: &AccountIdOf) -> u64 { Balances::free_balance(who) } + pub fn hash(s: &S) -> <::Hashing as Hash>::Output { + <::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 _ = ::Currency::slash( &addr, ::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)], + }, + ], ); }); } diff --git a/substrate/frame/contracts/src/wasm/code_cache.rs b/substrate/frame/contracts/src/wasm/code_cache.rs index d482d5c4bb..eb5551e342 100644 --- a/substrate/frame/contracts/src/wasm/code_cache.rs +++ b/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 >::insert(&code_hash, orig_code); >::insert(&code_hash, owner_info); *existing = Some(module); - >::deposit_event(Event::CodeStored { code_hash }); + >::deposit_event(vec![code_hash], Event::CodeStored { code_hash }); Ok(()) }, }) @@ -133,7 +134,10 @@ pub fn increment_refcount(code_hash: CodeHash) -> Result<(), Dispa } /// Try to remove code together with all associated information. -pub fn try_remove(origin: &T::AccountId, code_hash: CodeHash) -> DispatchResult { +pub fn try_remove(origin: &T::AccountId, code_hash: CodeHash) -> DispatchResult +where + T::AccountId: UncheckedFrom + AsRef<[u8]>, +{ >::try_mutate_exists(&code_hash, |existing| { if let Some(owner_info) = existing { ensure!(owner_info.refcount == 0, >::CodeInUse); @@ -142,7 +146,7 @@ pub fn try_remove(origin: &T::AccountId, code_hash: CodeHash) -> D *existing = None; >::remove(&code_hash); >::remove(&code_hash); - >::deposit_event(Event::CodeRemoved { code_hash }); + >::deposit_event(vec![code_hash], Event::CodeRemoved { code_hash }); Ok(()) } else { Err(>::CodeNotFound.into())