Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

reset events before apply runtime upgrade #10620

Merged
merged 6 commits into from
Jan 12, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
4 changes: 2 additions & 2 deletions frame/aura/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
use crate::mock::{new_test_ext, Aura, MockDisabledValidators, System};
use codec::Encode;
use frame_support::traits::OnInitialize;
use frame_system::InitKind;
use sp_consensus_aura::{Slot, AURA_ENGINE_ID};
use sp_runtime::{Digest, DigestItem};

Expand All @@ -45,7 +44,8 @@ fn disabled_validators_cannot_author_blocks() {
let pre_digest =
Digest { logs: vec![DigestItem::PreRuntime(AURA_ENGINE_ID, slot.encode())] };

System::initialize(&42, &System::parent_hash(), &pre_digest, InitKind::Full);
System::reset_events();
System::initialize(&42, &System::parent_hash(), &pre_digest);

// let's disable the validator
MockDisabledValidators::disable_validator(1);
Expand Down
7 changes: 4 additions & 3 deletions frame/babe/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ use frame_support::{
parameter_types,
traits::{ConstU128, ConstU32, ConstU64, GenesisBuild, KeyOwnerProofSystem, OnInitialize},
};
use frame_system::InitKind;
use pallet_session::historical as pallet_session_historical;
use pallet_staking::EraIndex;
use sp_consensus_babe::{AuthorityId, AuthorityPair, Slot};
Expand Down Expand Up @@ -255,7 +254,8 @@ pub fn go_to_block(n: u64, s: u64) {

let pre_digest = make_secondary_plain_pre_digest(0, s.into());

System::initialize(&n, &parent_hash, &pre_digest, InitKind::Full);
System::reset_events();
System::initialize(&n, &parent_hash, &pre_digest);

Babe::on_initialize(n);
Session::on_initialize(n);
Expand Down Expand Up @@ -421,7 +421,8 @@ pub fn generate_equivocation_proof(
let make_header = || {
let parent_hash = System::parent_hash();
let pre_digest = make_secondary_plain_pre_digest(offender_authority_index, slot);
System::initialize(&current_block, &parent_hash, &pre_digest, InitKind::Full);
System::reset_events();
System::initialize(&current_block, &parent_hash, &pre_digest);
System::set_block_number(current_block);
Timestamp::set_timestamp(current_block);
System::finalize()
Expand Down
8 changes: 5 additions & 3 deletions frame/executive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,11 @@ where
parent_hash: &System::Hash,
digest: &Digest,
) {
// Reset events before apply runtime upgrade hook.
// This is required to preserve events from runtime upgrade hook.
// This means the format of all the event related storages must always be compatible.
<frame_system::Pallet<System>>::reset_events();
xlc marked this conversation as resolved.
Show resolved Hide resolved

let mut weight = 0;
if Self::runtime_upgraded() {
weight = weight.saturating_add(Self::execute_on_runtime_upgrade());
Expand All @@ -302,7 +307,6 @@ where
block_number,
parent_hash,
digest,
frame_system::InitKind::Full,
);
weight = weight.saturating_add(<AllPalletsWithSystem as OnInitialize<
System::BlockNumber,
Expand Down Expand Up @@ -510,7 +514,6 @@ where
&(frame_system::Pallet::<System>::block_number() + One::one()),
&block_hash,
&Default::default(),
frame_system::InitKind::Inspection,
);

enter_span! { sp_tracing::Level::TRACE, "validate_transaction" };
Expand Down Expand Up @@ -545,7 +548,6 @@ where
header.number(),
header.parent_hash(),
&digests,
frame_system::InitKind::Inspection,
);

// Frame system only inserts the parent hash into the block hashes as normally we don't know
Expand Down
2 changes: 1 addition & 1 deletion frame/merkle-mountain-range/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,11 @@ fn new_block() -> u64 {
let hash = H256::repeat_byte(number as u8);
LEAF_DATA.with(|r| r.borrow_mut().a = number);

frame_system::Pallet::<Test>::reset_events();
frame_system::Pallet::<Test>::initialize(
&number,
&hash,
&Default::default(),
frame_system::InitKind::Full,
);
MMR::on_initialize(number)
}
Expand Down
3 changes: 2 additions & 1 deletion frame/randomness-collective-flip/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,8 @@ mod tests {
let mut parent_hash = System::parent_hash();

for i in 1..(blocks + 1) {
System::initialize(&i, &parent_hash, &Default::default(), frame_system::InitKind::Full);
System::reset_events();
System::initialize(&i, &parent_hash, &Default::default());
CollectiveFlip::on_initialize(i);

let header = System::finalize();
Expand Down
30 changes: 0 additions & 30 deletions frame/system/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -944,27 +944,6 @@ where
}
}

/// A type of block initialization to perform.
pub enum InitKind {
/// Leave inspectable storage entries in state.
///
/// i.e. `Events` are not being reset.
/// Should only be used for off-chain calls,
/// regular block execution should clear those.
Inspection,

/// Reset also inspectable storage entries.
///
/// This should be used for regular block execution.
Full,
}

impl Default for InitKind {
fn default() -> Self {
InitKind::Full
}
}

/// Reference status; can be either referenced or unreferenced.
#[derive(RuntimeDebug)]
pub enum RefStatus {
Expand Down Expand Up @@ -1306,7 +1285,6 @@ impl<T: Config> Pallet<T> {
number: &T::BlockNumber,
parent_hash: &T::Hash,
digest: &generic::Digest,
kind: InitKind,
) {
// populate environment
ExecutionPhase::<T>::put(Phase::Initialization);
Expand All @@ -1318,13 +1296,6 @@ impl<T: Config> Pallet<T> {

// Remove previous block data from storage
BlockWeight::<T>::kill();

// Kill inspectable storage entries in state when `InitKind::Full`.
if let InitKind::Full = kind {
<Events<T>>::kill();
EventCount::<T>::kill();
<EventTopics<T>>::remove_all(None);
}
}

/// Remove temporary "environment" entries in storage, compute the storage root and return the
Expand Down Expand Up @@ -1446,7 +1417,6 @@ impl<T: Config> Pallet<T> {

/// Reset events. Can be used as an alternative to
/// `initialize` for tests that don't need to bother with the other environment entries.
xlc marked this conversation as resolved.
Show resolved Hide resolved
#[cfg(any(feature = "std", feature = "runtime-benchmarks", test))]
pub fn reset_events() {
<Events<T>>::kill();
EventCount::<T>::kill();
Expand Down
21 changes: 14 additions & 7 deletions frame/system/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,8 @@ fn provider_required_to_support_consumer() {
#[test]
fn deposit_event_should_work() {
new_test_ext().execute_with(|| {
System::initialize(&1, &[0u8; 32].into(), &Default::default(), InitKind::Full);
System::reset_events();
System::initialize(&1, &[0u8; 32].into(), &Default::default());
System::note_finished_extrinsics();
System::deposit_event(SysEvent::CodeUpdated);
System::finalize();
Expand All @@ -167,7 +168,8 @@ fn deposit_event_should_work() {
}]
);

System::initialize(&2, &[0u8; 32].into(), &Default::default(), InitKind::Full);
System::reset_events();
System::initialize(&2, &[0u8; 32].into(), &Default::default());
System::deposit_event(SysEvent::NewAccount { account: 32 });
System::note_finished_initialize();
System::deposit_event(SysEvent::KilledAccount { account: 42 });
Expand Down Expand Up @@ -216,7 +218,8 @@ fn deposit_event_should_work() {
#[test]
fn deposit_event_uses_actual_weight() {
new_test_ext().execute_with(|| {
System::initialize(&1, &[0u8; 32].into(), &Default::default(), InitKind::Full);
System::reset_events();
System::initialize(&1, &[0u8; 32].into(), &Default::default());
System::note_finished_initialize();

let pre_info = DispatchInfo { weight: 1000, ..Default::default() };
Expand Down Expand Up @@ -275,7 +278,8 @@ fn deposit_event_topics() {
new_test_ext().execute_with(|| {
const BLOCK_NUMBER: u64 = 1;

System::initialize(&BLOCK_NUMBER, &[0u8; 32].into(), &Default::default(), InitKind::Full);
System::reset_events();
System::initialize(&BLOCK_NUMBER, &[0u8; 32].into(), &Default::default());
System::note_finished_extrinsics();

let topics = vec![H256::repeat_byte(1), H256::repeat_byte(2), H256::repeat_byte(3)];
Expand Down Expand Up @@ -333,7 +337,8 @@ fn prunes_block_hash_mappings() {
new_test_ext().execute_with(|| {
// simulate import of 15 blocks
for n in 1..=15 {
System::initialize(&n, &[n as u8 - 1; 32].into(), &Default::default(), InitKind::Full);
System::reset_events();
System::initialize(&n, &[n as u8 - 1; 32].into(), &Default::default());

System::finalize();
}
Expand Down Expand Up @@ -464,7 +469,8 @@ fn events_not_emitted_during_genesis() {
#[test]
fn extrinsics_root_is_calculated_correctly() {
new_test_ext().execute_with(|| {
System::initialize(&1, &[0u8; 32].into(), &Default::default(), InitKind::Full);
System::reset_events();
System::initialize(&1, &[0u8; 32].into(), &Default::default());
System::note_finished_initialize();
System::note_extrinsic(vec![1]);
System::note_applied_extrinsic(&Ok(().into()), Default::default());
Expand All @@ -481,7 +487,8 @@ fn extrinsics_root_is_calculated_correctly() {
#[test]
fn runtime_updated_digest_emitted_when_heap_pages_changed() {
new_test_ext().execute_with(|| {
System::initialize(&1, &[0u8; 32].into(), &Default::default(), InitKind::Full);
System::reset_events();
System::initialize(&1, &[0u8; 32].into(), &Default::default());
System::set_heap_pages(RawOrigin::Root.into(), 5).unwrap();
assert_runtime_updated_digest(1);
});
Expand Down