Skip to content

Commit

Permalink
Add execution phases: ApplyInherent and AfterInherent
Browse files Browse the repository at this point in the history 
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
  • Loading branch information
@ggwpez
ggwpez committed Mar 12, 2024
1 parent a756baf commit 8e0bca6
Show file tree
Hide file tree
Showing 3 changed files with 172 additions and 30 deletions.
7 changes: 4 additions & 3 deletions substrate/frame/executive/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ where
return Err(InvalidTransaction::BadMandatory.into())
}

<frame_system::Pallet<System>>::note_applied_extrinsic(&r, dispatch_info);
<frame_system::Pallet<System>>::note_applied_extrinsic(&r, dispatch_info, is_inherent);

Ok(r.map(|_| ()).map_err(|e| e.error))
};
Expand Down Expand Up @@ -601,8 +601,8 @@ where
DispatchClass::Mandatory,
);

frame_system::Pallet::<System>::note_finished_initialize();
<System as frame_system::Config>::PreInherents::pre_inherents();
frame_system::Pallet::<System>::note_finished_initialize();
}

/// Returns if the runtime has been upgraded, based on [`frame_system::LastRuntimeUpgrade`].
Expand Down Expand Up @@ -672,6 +672,7 @@ where
pub fn inherents_applied() {
<frame_system::Pallet<System>>::note_inherents_applied();
<System as frame_system::Config>::PostInherents::post_inherents();
<frame_system::Pallet<System>>::note_post_inherents_applied();

if <System as frame_system::Config>::MultiBlockMigrator::ongoing() {
let used_weight = <System as frame_system::Config>::MultiBlockMigrator::step();
Expand Down Expand Up @@ -806,7 +807,7 @@ where
return Err(InvalidTransaction::BadMandatory.into())
}

<frame_system::Pallet<System>>::note_applied_extrinsic(&r, dispatch_info);
<frame_system::Pallet<System>>::note_applied_extrinsic(&r, dispatch_info, is_inherent);

Ok(r.map(|_| ()).map_err(|e| e.error))
}
Expand Down
159 changes: 139 additions & 20 deletions substrate/frame/executive/src/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,19 @@ use frame_support::{
traits::{fungible, ConstU8, Currency, IsInherent},
weights::{ConstantMultiplier, IdentityFee, RuntimeDbWeight, Weight, WeightMeter, WeightToFee},
};
use frame_system::{pallet_prelude::*, ChainContext, LastRuntimeUpgrade, LastRuntimeUpgradeInfo};
use frame_system::{
pallet_prelude::*, ChainContext, LastRuntimeUpgrade, LastRuntimeUpgradeInfo, Phase,
};
use pallet_balances::Call as BalancesCall;
use pallet_transaction_payment::CurrencyAdapter;

const TEST_KEY: &[u8] = b":test:key:";

fn assert_execution_phase<T: frame_system::Config>(want: &Phase) {
let got = frame_system::ExecutionPhase::<T>::get().unwrap();
assert_eq!(want, &got, "Wrong execution phase");
}

#[frame_support::pallet(dev_mode)]
mod custom {
use super::*;
Expand Down Expand Up @@ -181,27 +188,36 @@ mod custom2 {
!MockedSystemCallbacks::pre_inherent_called(),
"Pre inherent hook goes after on_initialize"
);
assert_execution_phase::<T>(&Phase::Initialization);

Weight::from_parts(0, 0)
}

fn on_idle(_: BlockNumberFor<T>, _: Weight) -> Weight {
assert!(
MockedSystemCallbacks::post_transactions_called(),
"Post transactions hook goes before after on_idle"
"Post transactions hook goes before on_idle"
);
assert_execution_phase::<T>(&Phase::Finalization);

Weight::from_parts(0, 0)
}

fn on_finalize(_: BlockNumberFor<T>) {
assert!(
MockedSystemCallbacks::post_transactions_called(),
"Post transactions hook goes before after on_finalize"
"Post transactions hook goes before on_finalize"
);
assert_execution_phase::<T>(&Phase::Finalization);
}

fn on_runtime_upgrade() -> Weight {
sp_io::storage::set(super::TEST_KEY, "module".as_bytes());
assert!(
!frame_system::ExecutionPhase::<T>::exists(),
"Runtime upgrades do not have a phase"
);

Weight::from_parts(0, 0)
}
}
Expand All @@ -218,6 +234,17 @@ mod custom2 {
assert!(!MockedSystemCallbacks::post_transactions_called());
assert!(System::inherents_applied());

assert!(matches!(
frame_system::ExecutionPhase::<T>::get(),
Some(frame_system::Phase::ApplyExtrinsic(_) | frame_system::Phase::AfterInherent)
));

Ok(())
}

pub fn assert_extrinsic_phase(_: OriginFor<T>, expected: u32) -> DispatchResult {
assert_execution_phase::<T>(&Phase::ApplyExtrinsic(expected));

Ok(())
}

Expand All @@ -242,6 +269,19 @@ mod custom2 {

Ok(())
}

#[pallet::weight((0, DispatchClass::Mandatory))]
pub fn assert_inherent_phase(_: OriginFor<T>, expected: u32) -> DispatchResult {
assert_execution_phase::<T>(&Phase::ApplyInherent(expected));

Ok(())
}

pub fn assert_optional_inherent_phase(_: OriginFor<T>, expected: u32) -> DispatchResult {
assert_execution_phase::<T>(&Phase::ApplyInherent(expected));

Ok(())
}
}

#[pallet::inherent]
Expand All @@ -257,7 +297,13 @@ mod custom2 {
}

fn is_inherent(call: &Self::Call) -> bool {
matches!(call, Call::<T>::inherent {} | Call::<T>::optional_inherent {})
matches!(
call,
Call::<T>::inherent {} |
Call::<T>::optional_inherent {} |
Call::<T>::assert_inherent_phase { .. } |
Call::<T>::assert_optional_inherent_phase { .. }
)
}
}

Expand All @@ -270,6 +316,8 @@ mod custom2 {
match call {
Call::allowed_unsigned { .. } |
Call::optional_inherent { .. } |
Call::assert_inherent_phase { .. } |
Call::assert_optional_inherent_phase { .. } |
Call::inherent { .. } => Ok(()),
_ => Err(UnknownTransaction::NoUnsignedValidator.into()),
}
Expand Down Expand Up @@ -465,6 +513,7 @@ impl PreInherents for MockedSystemCallbacks {
SystemCallbacksCalled::set(1);
// Change the storage to modify the root hash:
frame_support::storage::unhashed::put(b":pre_inherent", b"0");
assert_eq!(frame_system::ExecutionPhase::<Runtime>::get(), Some(Phase::Initialization));
}
}

Expand All @@ -474,6 +523,7 @@ impl PostInherents for MockedSystemCallbacks {
SystemCallbacksCalled::set(2);
// Change the storage to modify the root hash:
frame_support::storage::unhashed::put(b":post_inherent", b"0");
assert_execution_phase::<Runtime>(&Phase::AfterInherent);
}
}

Expand All @@ -483,6 +533,7 @@ impl PostTransactions for MockedSystemCallbacks {
SystemCallbacksCalled::set(3);
// Change the storage to modify the root hash:
frame_support::storage::unhashed::put(b":post_transaction", b"0");
assert_execution_phase::<Runtime>(&Phase::Finalization);
}
}

Expand Down Expand Up @@ -990,6 +1041,7 @@ fn all_weights_are_recorded_correctly() {

let block_number = 1;

frame_system::ExecutionPhase::<Runtime>::kill();
Executive::initialize_block(&Header::new_from_number(block_number));

// Reset the last runtime upgrade info, to make the second call to `on_runtime_upgrade`
Expand All @@ -998,20 +1050,21 @@ fn all_weights_are_recorded_correctly() {
MockedSystemCallbacks::reset();

// All weights that show up in the `initialize_block_impl`
let custom_runtime_upgrade_weight = CustomOnRuntimeUpgrade::on_runtime_upgrade();
let runtime_upgrade_weight =
<AllPalletsWithSystem as OnRuntimeUpgrade>::on_runtime_upgrade();
let on_initialize_weight =
<AllPalletsWithSystem as OnInitialize<u64>>::on_initialize(block_number);
frame_system::ExecutionPhase::<Runtime>::kill();
let custom_ra_weight = CustomOnRuntimeUpgrade::on_runtime_upgrade();

frame_system::ExecutionPhase::<Runtime>::kill();
let ra_weight = <AllPalletsWithSystem as OnRuntimeUpgrade>::on_runtime_upgrade();

frame_system::ExecutionPhase::<Runtime>::put(Phase::Initialization);
let init_weight = <AllPalletsWithSystem as OnInitialize<u64>>::on_initialize(block_number);

let base_block_weight = <Runtime as frame_system::Config>::BlockWeights::get().base_block;

// Weights are recorded correctly
assert_eq!(
frame_system::Pallet::<Runtime>::block_weight().total(),
custom_runtime_upgrade_weight +
runtime_upgrade_weight +
on_initialize_weight +
base_block_weight,
custom_ra_weight + ra_weight + init_weight + base_block_weight,
);
});
}
Expand Down Expand Up @@ -1318,10 +1371,11 @@ fn callbacks_in_block_execution_works() {
callbacks_in_block_execution_works_inner(true);
}

/// Produces a block with `0..15` inherents and `0..15` transactions and runs tests on that.
fn callbacks_in_block_execution_works_inner(mbms_active: bool) {
MbmActive::set(mbms_active);

for (n_in, n_tx) in (0..10usize).zip(0..10usize) {
for (n_in, n_tx) in (0..15usize).zip(0..15usize) {
let mut extrinsics = Vec::new();

let header = new_test_ext(10).execute_with(|| {
Expand All @@ -1331,20 +1385,26 @@ fn callbacks_in_block_execution_works_inner(mbms_active: bool) {

for i in 0..n_in {
let xt = if i % 2 == 0 {
UncheckedXt::new_bare(RuntimeCall::Custom(custom::Call::inherent {}))
UncheckedXt::new_bare(RuntimeCall::Custom2(
custom2::Call::assert_inherent_phase { expected: i as u32 },
))
} else {
UncheckedXt::new_bare(RuntimeCall::Custom2(custom2::Call::optional_inherent {}))
UncheckedXt::new_bare(RuntimeCall::Custom2(
custom2::Call::assert_optional_inherent_phase { expected: i as u32 },
))
};
Executive::apply_extrinsic(xt.clone()).unwrap().unwrap();
extrinsics.push(xt);
}

for t in 0..n_tx {
let xt = UncheckedXt::new_signed(
RuntimeCall::Custom2(custom2::Call::some_call {}),
RuntimeCall::Custom2(custom2::Call::assert_extrinsic_phase {
expected: extrinsics.len() as u32,
}),
1,
1.into(),
tx_ext(t as u64, 0),
tx_ext(t as u64, 1),
);
// Extrinsics can be applied even when MBMs are active. Only the `execute_block`
// will reject it.
Expand All @@ -1354,11 +1414,12 @@ fn callbacks_in_block_execution_works_inner(mbms_active: bool) {

Executive::finalize_block()
});
assert_eq!(SystemCallbacksCalled::get(), 3);
assert!(MockedSystemCallbacks::post_inherent_called());

new_test_ext(10).execute_with(|| {
MockedSystemCallbacks::reset();
let header = std::panic::catch_unwind(|| {
Executive::execute_block(Block::new(header, extrinsics));
Executive::execute_block(Block::new(header.clone(), extrinsics.clone()));
});

match header {
Expand Down Expand Up @@ -1474,3 +1535,61 @@ fn is_inherent_works() {
let ext = UncheckedXt::new_bare(RuntimeCall::Custom2(custom2::Call::allowed_unsigned {}));
assert!(!Runtime::is_inherent(&ext), "Unsigned ext are not automatically inherents");
}

#[test]
fn extrinsic_index_is_correct() {
let in1 = UncheckedXt::new_bare(RuntimeCall::Custom2(custom2::Call::assert_inherent_phase {
expected: 0,
}));
let in2 = UncheckedXt::new_bare(RuntimeCall::Custom2(custom2::Call::assert_inherent_phase {
expected: 1,
}));
let xt1 = UncheckedXt::new_signed(
RuntimeCall::Custom2(custom2::Call::assert_extrinsic_phase { expected: 2 }),
1,
1.into(),
tx_ext(0, 1),
);
let xt2 = UncheckedXt::new_signed(
RuntimeCall::Custom2(custom2::Call::assert_extrinsic_phase { expected: 3 }),
1,
1.into(),
tx_ext(1, 1),
);
let xt3 = UncheckedXt::new_signed(
RuntimeCall::Custom2(custom2::Call::assert_extrinsic_phase { expected: 4 }),
1,
1.into(),
tx_ext(2, 1),
);

let header = new_test_ext(1).execute_with(|| {
Executive::initialize_block(&Header::new_from_number(1));

Executive::apply_extrinsic(in1.clone()).unwrap().unwrap();
Executive::apply_extrinsic(in2.clone()).unwrap().unwrap();
Executive::apply_extrinsic(xt1.clone()).unwrap().unwrap();
Executive::apply_extrinsic(xt2.clone()).unwrap().unwrap();
Executive::apply_extrinsic(xt3.clone()).unwrap().unwrap();

Executive::finalize_block()
});

new_test_ext(1).execute_with(|| {
Executive::execute_block(Block::new(
header.clone(),
vec![in1.clone(), in2.clone(), xt1.clone(), xt2.clone(), xt3.clone()],
));
});

#[cfg(feature = "try-runtime")]
new_test_ext(1).execute_with(|| {
Executive::try_execute_block(
Block::new(header, vec![in1, in2, xt1, xt2, xt3]),
true,
true,
frame_try_runtime::TryStateSelect::All,
)
.unwrap();
});
}
Loading

0 comments on commit 8e0bca6

Please sign in to comment.