Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[pallet-revive] Remove revive events #7164

Merged
merged 6 commits into from
Jan 15, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions prdoc/pr_7164.prdoc
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
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
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
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
Loading