Skip to content

Commit

Permalink
[pallet-revive] Remove revive events (#7164)
Browse files Browse the repository at this point in the history 
Remove all pallet::events except for the `ContractEmitted` event that is
emitted by contracts

---------

Co-authored-by: command-bot <>
Co-authored-by: Alexander Theißen <[email protected]>
  • Loading branch information
@pgherveou @athei
pgherveou and athei authored Jan 15, 2025
1 parent ece32e3 commit 5be6587
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 500 deletions.
8 changes: 8 additions & 0 deletions prdoc/pr_7164.prdoc
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
title: '[pallet-revive] Remove revive events'
doc:
- audience: Runtime Dev
description: Remove all pallet::events except for the `ContractEmitted` event that
is emitted by contracts
crates:
- name: pallet-revive
bump: major
103 changes: 13 additions & 90 deletions substrate/frame/revive/src/exec.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1092,34 +1092,11 @@ where
.enforce_limit(contract)
.map_err(|e| ExecError { error: e, origin: ErrorOrigin::Callee })?;

let account_id = T::AddressMapper::to_address(&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());
}

let caller = T::AddressMapper::to_address(self.caller().account_id()?);
// Deposit an instantiation event.
Contracts::<T>::deposit_event(Event::Instantiated {
deployer: caller,
contract: account_id,
});
},
(ExportedFunction::Call, Some(code_hash)) => {
Contracts::<T>::deposit_event(Event::DelegateCalled {
contract: account_id,
code_hash,
});
},
(ExportedFunction::Call, None) => {
let caller = self.caller();
Contracts::<T>::deposit_event(Event::Called {
caller: caller.clone(),
contract: account_id,
});
},
// It is not allowed to terminate a contract inside its constructor.
if entry_point == ExportedFunction::Constructor &&
matches!(frame.contract_info, CachedContract::Terminated)
{
return Err(Error::<T>::TerminatedInConstructor.into());
}

Ok(output)
Expand Down Expand Up @@ -1526,10 +1503,6 @@ where
.charge_deposit(frame.account_id.clone(), StorageDeposit::Refund(*deposit));
}

Contracts::<T>::deposit_event(Event::Terminated {
contract: account_address,
beneficiary: *beneficiary,
});
Ok(())
}

Expand Down Expand Up @@ -1782,11 +1755,6 @@ where

Self::increment_refcount(hash)?;
Self::decrement_refcount(prev_hash);
Contracts::<Self::T>::deposit_event(Event::ContractCodeUpdated {
contract: T::AddressMapper::to_address(&frame.account_id),
new_code_hash: hash,
old_code_hash: prev_hash,
});
Ok(())
}

Expand Down Expand Up @@ -2933,13 +2901,6 @@ mod tests {
ContractInfo::<Test>::load_code_hash(&instantiated_contract_id).unwrap(),
dummy_ch
);
assert_eq!(
&events(),
&[Event::Instantiated {
deployer: ALICE_ADDR,
contract: instantiated_contract_address
}]
);
});
}

Expand Down Expand Up @@ -3055,19 +3016,6 @@ mod tests {
ContractInfo::<Test>::load_code_hash(&instantiated_contract_id).unwrap(),
dummy_ch
);
assert_eq!(
&events(),
&[
Event::Instantiated {
deployer: BOB_ADDR,
contract: instantiated_contract_address
},
Event::Called {
caller: Origin::from_account_id(ALICE),
contract: BOB_ADDR
},
]
);
});
}

Expand Down Expand Up @@ -3119,13 +3067,6 @@ mod tests {
),
Ok(_)
);

// The contract wasn't instantiated so we don't expect to see an instantiation
// event here.
assert_eq!(
&events(),
&[Event::Called { caller: Origin::from_account_id(ALICE), contract: BOB_ADDR },]
);
});
}

Expand Down Expand Up @@ -3465,24 +3406,14 @@ 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_FALLBACK,
hash: remark_hash
}),
topics: vec![],
},
EventRecord {
phase: Phase::Initialization,
event: MetaEvent::Contracts(crate::Event::Called {
caller: Origin::from_account_id(ALICE),
contract: BOB_ADDR,
}),
topics: vec![],
},
]
vec![EventRecord {
phase: Phase::Initialization,
event: MetaEvent::System(frame_system::Event::Remarked {
sender: BOB_FALLBACK,
hash: remark_hash
}),
topics: vec![],
},]
);
});
}
Expand Down Expand Up @@ -3571,14 +3502,6 @@ mod tests {
},),
topics: vec![],
},
EventRecord {
phase: Phase::Initialization,
event: MetaEvent::Contracts(crate::Event::Called {
caller: Origin::from_account_id(ALICE),
contract: BOB_ADDR,
}),
topics: vec![],
},
]
);
});
Expand Down
72 changes: 0 additions & 72 deletions substrate/frame/revive/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -379,25 +379,6 @@ pub mod pallet {

#[pallet::event]
pub enum Event<T: Config> {
/// Contract deployed by address at the specified address.
Instantiated { deployer: H160, contract: H160 },

/// Contract has been removed.
///
/// # Note
///
/// The only way for a contract to be removed and emitting this event is by calling
/// `seal_terminate`.
Terminated {
/// The contract that was terminated.
contract: H160,
/// The account that received the contracts remaining balance
beneficiary: H160,
},

/// Code with the specified hash has been stored.
CodeStored { code_hash: H256, deposit_held: BalanceOf<T>, uploader: H160 },

/// A custom event emitted by the contract.
ContractEmitted {
/// The contract that emitted the event.
Expand All @@ -409,54 +390,6 @@ pub mod pallet {
/// Number of topics is capped by [`limits::NUM_EVENT_TOPICS`].
topics: Vec<H256>,
},

/// A code with the specified hash was removed.
CodeRemoved { code_hash: H256, deposit_released: BalanceOf<T>, remover: H160 },

/// A contract's code was updated.
ContractCodeUpdated {
/// The contract that has been updated.
contract: H160,
/// New code hash that was set for the contract.
new_code_hash: H256,
/// Previous code hash of the contract.
old_code_hash: H256,
},

/// 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 caller of the `contract`.
caller: Origin<T>,
/// The contract that was called.
contract: H160,
},

/// 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: H160,
/// The code hash that was delegate called.
code_hash: H256,
},

/// Some funds have been transferred and held as storage deposit.
StorageDepositTransferredAndHeld { from: H160, to: H160, amount: BalanceOf<T> },

/// Some storage deposit funds have been transferred and released.
StorageDepositTransferredAndReleased { from: H160, to: H160, amount: BalanceOf<T> },
}

#[pallet::error]
Expand Down Expand Up @@ -985,11 +918,6 @@ pub mod pallet {
};
<ExecStack<T, WasmBlob<T>>>::increment_refcount(code_hash)?;
<ExecStack<T, WasmBlob<T>>>::decrement_refcount(contract.code_hash);
Self::deposit_event(Event::ContractCodeUpdated {
contract: dest,
new_code_hash: code_hash,
old_code_hash: contract.code_hash,
});
contract.code_hash = code_hash;
Ok(())
})
Expand Down
16 changes: 2 additions & 14 deletions substrate/frame/revive/src/storage/meter.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@
//! This module contains functions to meter the storage deposit.
use crate::{
address::AddressMapper, storage::ContractInfo, AccountIdOf, BalanceOf, CodeInfo, Config, Error,
Event, HoldReason, Inspect, Origin, Pallet, StorageDeposit as Deposit, System, LOG_TARGET,
storage::ContractInfo, AccountIdOf, BalanceOf, CodeInfo, Config, Error, HoldReason, Inspect,
Origin, Pallet, StorageDeposit as Deposit, System, LOG_TARGET,
};
use alloc::vec::Vec;
use core::{fmt::Debug, marker::PhantomData};
Expand Down Expand Up @@ -516,12 +516,6 @@ impl<T: Config> Ext<T> for ReservingExt {
Preservation::Preserve,
Fortitude::Polite,
)?;

Pallet::<T>::deposit_event(Event::StorageDepositTransferredAndHeld {
from: T::AddressMapper::to_address(origin),
to: T::AddressMapper::to_address(contract),
amount: *amount,
});
},
Deposit::Refund(amount) => {
let transferred = T::Currency::transfer_on_hold(
Expand All @@ -534,12 +528,6 @@ impl<T: Config> Ext<T> for ReservingExt {
Fortitude::Polite,
)?;

Pallet::<T>::deposit_event(Event::StorageDepositTransferredAndReleased {
from: T::AddressMapper::to_address(contract),
to: T::AddressMapper::to_address(origin),
amount: transferred,
});

if transferred < *amount {
// This should never happen, if it does it means that there is a bug in the
// runtime logic. In the rare case this happens we try to refund as much as we
Expand Down
Loading

0 comments on commit 5be6587

Please sign in to comment.