From 26ea6e1e485b8ed13fff161be4fc41d448296946 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> Date: Sat, 24 Sep 2022 17:17:44 +0200 Subject: [PATCH] Add base-weight to `System::Extrinsic*` events (#12329) * Add base-weight to events Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Fix test Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> --- substrate/bin/node/executor/tests/basic.rs | 13 +- .../system/src/extensions/check_weight.rs | 12 +- substrate/frame/system/src/lib.rs | 8 +- substrate/frame/system/src/mock.rs | 1 + substrate/frame/system/src/tests.rs | 312 ++++++++++-------- 5 files changed, 198 insertions(+), 148 deletions(-) diff --git a/substrate/bin/node/executor/tests/basic.rs b/substrate/bin/node/executor/tests/basic.rs index 99a9b83596a..fc4e138faaf 100644 --- a/substrate/bin/node/executor/tests/basic.rs +++ b/substrate/bin/node/executor/tests/basic.rs @@ -311,10 +311,19 @@ fn full_native_block_import_works() { let mut alice_last_known_balance: Balance = Default::default(); let mut fees = t.execute_with(|| transfer_fee(&xt())); - let transfer_weight = default_transfer_call().get_dispatch_info().weight; + let transfer_weight = default_transfer_call().get_dispatch_info().weight.saturating_add( + <Runtime as frame_system::Config>::BlockWeights::get() + .get(DispatchClass::Normal) + .base_extrinsic, + ); let timestamp_weight = pallet_timestamp::Call::set::<Runtime> { now: Default::default() } .get_dispatch_info() - .weight; + .weight + .saturating_add( + <Runtime as frame_system::Config>::BlockWeights::get() + .get(DispatchClass::Mandatory) + .base_extrinsic, + ); executor_call(&mut t, "Core_execute_block", &block1.0, true).0.unwrap(); diff --git a/substrate/frame/system/src/extensions/check_weight.rs b/substrate/frame/system/src/extensions/check_weight.rs index 466231b8455..15a88913cd3 100644 --- a/substrate/frame/system/src/extensions/check_weight.rs +++ b/substrate/frame/system/src/extensions/check_weight.rs @@ -342,7 +342,7 @@ mod tests { .get(DispatchClass::Operational) .max_total .unwrap_or_else(|| weights.max_block); - let base_weight = weights.get(DispatchClass::Normal).base_extrinsic; + let base_weight = weights.get(DispatchClass::Operational).base_extrinsic; let weight = operational_limit - base_weight; let okay = @@ -378,11 +378,11 @@ mod tests { // Max normal is 768 (75%) // 10 is taken for block execution weight // So normal extrinsic can be 758 weight (-5 for base extrinsic weight) - // And Operational can be 256 to produce a full block (-5 for base) + // And Operational can be 246 to produce a full block (-10 for base) let max_normal = DispatchInfo { weight: Weight::from_ref_time(753), ..Default::default() }; let rest_operational = DispatchInfo { - weight: Weight::from_ref_time(251), + weight: Weight::from_ref_time(246), class: DispatchClass::Operational, ..Default::default() }; @@ -406,7 +406,7 @@ mod tests { let max_normal = DispatchInfo { weight: Weight::from_ref_time(753), ..Default::default() }; let rest_operational = DispatchInfo { - weight: Weight::from_ref_time(251), + weight: Weight::from_ref_time(246), class: DispatchClass::Operational, ..Default::default() }; @@ -414,7 +414,7 @@ mod tests { let len = 0_usize; assert_ok!(CheckWeight::<Test>::do_pre_dispatch(&rest_operational, len)); - // Extra 15 here from block execution + base extrinsic weight + // Extra 20 here from block execution + base extrinsic weight assert_eq!(System::block_weight().total(), Weight::from_ref_time(266)); assert_ok!(CheckWeight::<Test>::do_pre_dispatch(&max_normal, len)); assert_eq!(block_weight_limit(), Weight::from_ref_time(1024)); @@ -433,7 +433,7 @@ mod tests { ..Default::default() }; let dispatch_operational = DispatchInfo { - weight: Weight::from_ref_time(251), + weight: Weight::from_ref_time(246), class: DispatchClass::Operational, ..Default::default() }; diff --git a/substrate/frame/system/src/lib.rs b/substrate/frame/system/src/lib.rs index 36360f8fae2..dc74157da79 100644 --- a/substrate/frame/system/src/lib.rs +++ b/substrate/frame/system/src/lib.rs @@ -1509,9 +1509,15 @@ impl<T: Config> Pallet<T> { } /// To be called immediately after an extrinsic has been applied. + /// + /// Emits an `ExtrinsicSuccess` or `ExtrinsicFailed` event depending on the outcome. + /// The emitted event contains the post-dispatch corrected weight including + /// the base-weight for its dispatch class. pub fn note_applied_extrinsic(r: &DispatchResultWithPostInfo, mut info: DispatchInfo) { - info.weight = extract_actual_weight(r, &info); + info.weight = extract_actual_weight(r, &info) + .saturating_add(T::BlockWeights::get().get(info.class).base_extrinsic); info.pays_fee = extract_actual_pays_fee(r, &info); + Self::deposit_event(match r { Ok(_) => Event::ExtrinsicSuccess { dispatch_info: info }, Err(err) => { diff --git a/substrate/frame/system/src/mock.rs b/substrate/frame/system/src/mock.rs index 1c0511787eb..b6fc1216120 100644 --- a/substrate/frame/system/src/mock.rs +++ b/substrate/frame/system/src/mock.rs @@ -67,6 +67,7 @@ parameter_types! { weights.max_total = Some(NORMAL_DISPATCH_RATIO * MAX_BLOCK_WEIGHT); }) .for_class(DispatchClass::Operational, |weights| { + weights.base_extrinsic = Weight::from_ref_time(10); weights.max_total = Some(MAX_BLOCK_WEIGHT); weights.reserved = Some( MAX_BLOCK_WEIGHT - NORMAL_DISPATCH_RATIO * MAX_BLOCK_WEIGHT diff --git a/substrate/frame/system/src/tests.rs b/substrate/frame/system/src/tests.rs index 92563f4ad17..c42131c4502 100644 --- a/substrate/frame/system/src/tests.rs +++ b/substrate/frame/system/src/tests.rs @@ -169,6 +169,10 @@ fn deposit_event_should_work() { }] ); + let normal_base = <Test as crate::Config>::BlockWeights::get() + .get(DispatchClass::Normal) + .base_extrinsic; + System::reset_events(); System::initialize(&2, &[0u8; 32].into(), &Default::default()); System::deposit_event(SysEvent::NewAccount { account: 32 }); @@ -194,14 +198,17 @@ fn deposit_event_should_work() { }, EventRecord { phase: Phase::ApplyExtrinsic(0), - event: SysEvent::ExtrinsicSuccess { dispatch_info: Default::default() }.into(), + event: SysEvent::ExtrinsicSuccess { + dispatch_info: DispatchInfo { weight: normal_base, ..Default::default() } + } + .into(), topics: vec![] }, EventRecord { phase: Phase::ApplyExtrinsic(1), event: SysEvent::ExtrinsicFailed { dispatch_error: DispatchError::BadOrigin.into(), - dispatch_info: Default::default() + dispatch_info: DispatchInfo { weight: normal_base, ..Default::default() } } .into(), topics: vec![] @@ -223,6 +230,9 @@ fn deposit_event_uses_actual_weight_and_pays_fee() { System::initialize(&1, &[0u8; 32].into(), &Default::default()); System::note_finished_initialize(); + let normal_base = <Test as crate::Config>::BlockWeights::get() + .get(DispatchClass::Normal) + .base_extrinsic; let pre_info = DispatchInfo { weight: Weight::from_ref_time(1000), ..Default::default() }; System::note_applied_extrinsic(&Ok(Some(300).into()), pre_info); System::note_applied_extrinsic(&Ok(Some(1000).into()), pre_info); @@ -267,144 +277,168 @@ fn deposit_event_uses_actual_weight_and_pays_fee() { }), pre_info, ); + // Also works for operational. + let operational_base = <Test as crate::Config>::BlockWeights::get() + .get(DispatchClass::Operational) + .base_extrinsic; + assert!(normal_base != operational_base, "Test pre-condition violated"); + let pre_info = DispatchInfo { + weight: Weight::from_ref_time(1000), + class: DispatchClass::Operational, + ..Default::default() + }; + System::note_applied_extrinsic(&Ok(Some(300).into()), pre_info); - assert_eq!( - System::events(), - vec![ - EventRecord { - phase: Phase::ApplyExtrinsic(0), - event: SysEvent::ExtrinsicSuccess { - dispatch_info: DispatchInfo { - weight: Weight::from_ref_time(300), - ..Default::default() - }, - } - .into(), - topics: vec![] - }, - EventRecord { - phase: Phase::ApplyExtrinsic(1), - event: SysEvent::ExtrinsicSuccess { - dispatch_info: DispatchInfo { - weight: Weight::from_ref_time(1000), - ..Default::default() - }, - } - .into(), - topics: vec![] - }, - EventRecord { - phase: Phase::ApplyExtrinsic(2), - event: SysEvent::ExtrinsicSuccess { - dispatch_info: DispatchInfo { - weight: Weight::from_ref_time(1000), - ..Default::default() - }, - } - .into(), - topics: vec![] - }, - EventRecord { - phase: Phase::ApplyExtrinsic(3), - event: SysEvent::ExtrinsicSuccess { - dispatch_info: DispatchInfo { - weight: Weight::from_ref_time(1000), - pays_fee: Pays::Yes, - ..Default::default() - }, - } - .into(), - topics: vec![] - }, - EventRecord { - phase: Phase::ApplyExtrinsic(4), - event: SysEvent::ExtrinsicSuccess { - dispatch_info: DispatchInfo { - weight: Weight::from_ref_time(1000), - pays_fee: Pays::No, - ..Default::default() - }, - } - .into(), - topics: vec![] - }, - EventRecord { - phase: Phase::ApplyExtrinsic(5), - event: SysEvent::ExtrinsicSuccess { - dispatch_info: DispatchInfo { - weight: Weight::from_ref_time(1000), - pays_fee: Pays::No, - ..Default::default() - }, - } - .into(), - topics: vec![] - }, - EventRecord { - phase: Phase::ApplyExtrinsic(6), - event: SysEvent::ExtrinsicSuccess { - dispatch_info: DispatchInfo { - weight: Weight::from_ref_time(500), - pays_fee: Pays::No, - ..Default::default() - }, - } - .into(), - topics: vec![] - }, - EventRecord { - phase: Phase::ApplyExtrinsic(7), - event: SysEvent::ExtrinsicFailed { - dispatch_error: DispatchError::BadOrigin.into(), - dispatch_info: DispatchInfo { - weight: Weight::from_ref_time(999), - ..Default::default() - }, - } - .into(), - topics: vec![] - }, - EventRecord { - phase: Phase::ApplyExtrinsic(8), - event: SysEvent::ExtrinsicFailed { - dispatch_error: DispatchError::BadOrigin.into(), - dispatch_info: DispatchInfo { - weight: Weight::from_ref_time(1000), - pays_fee: Pays::Yes, - ..Default::default() - }, - } - .into(), - topics: vec![] - }, - EventRecord { - phase: Phase::ApplyExtrinsic(9), - event: SysEvent::ExtrinsicFailed { - dispatch_error: DispatchError::BadOrigin.into(), - dispatch_info: DispatchInfo { - weight: Weight::from_ref_time(800), - pays_fee: Pays::Yes, - ..Default::default() - }, - } - .into(), - topics: vec![] - }, - EventRecord { - phase: Phase::ApplyExtrinsic(10), - event: SysEvent::ExtrinsicFailed { - dispatch_error: DispatchError::BadOrigin.into(), - dispatch_info: DispatchInfo { - weight: Weight::from_ref_time(800), - pays_fee: Pays::No, - ..Default::default() - }, - } - .into(), - topics: vec![] - }, - ] - ); + let got = System::events(); + let want = vec![ + EventRecord { + phase: Phase::ApplyExtrinsic(0), + event: SysEvent::ExtrinsicSuccess { + dispatch_info: DispatchInfo { + weight: Weight::from_ref_time(300).saturating_add(normal_base), + ..Default::default() + }, + } + .into(), + topics: vec![], + }, + EventRecord { + phase: Phase::ApplyExtrinsic(1), + event: SysEvent::ExtrinsicSuccess { + dispatch_info: DispatchInfo { + weight: Weight::from_ref_time(1000).saturating_add(normal_base), + ..Default::default() + }, + } + .into(), + topics: vec![], + }, + EventRecord { + phase: Phase::ApplyExtrinsic(2), + event: SysEvent::ExtrinsicSuccess { + dispatch_info: DispatchInfo { + weight: Weight::from_ref_time(1000).saturating_add(normal_base), + ..Default::default() + }, + } + .into(), + topics: vec![], + }, + EventRecord { + phase: Phase::ApplyExtrinsic(3), + event: SysEvent::ExtrinsicSuccess { + dispatch_info: DispatchInfo { + weight: Weight::from_ref_time(1000).saturating_add(normal_base), + pays_fee: Pays::Yes, + ..Default::default() + }, + } + .into(), + topics: vec![], + }, + EventRecord { + phase: Phase::ApplyExtrinsic(4), + event: SysEvent::ExtrinsicSuccess { + dispatch_info: DispatchInfo { + weight: Weight::from_ref_time(1000).saturating_add(normal_base), + pays_fee: Pays::No, + ..Default::default() + }, + } + .into(), + topics: vec![], + }, + EventRecord { + phase: Phase::ApplyExtrinsic(5), + event: SysEvent::ExtrinsicSuccess { + dispatch_info: DispatchInfo { + weight: Weight::from_ref_time(1000).saturating_add(normal_base), + pays_fee: Pays::No, + ..Default::default() + }, + } + .into(), + topics: vec![], + }, + EventRecord { + phase: Phase::ApplyExtrinsic(6), + event: SysEvent::ExtrinsicSuccess { + dispatch_info: DispatchInfo { + weight: Weight::from_ref_time(500).saturating_add(normal_base), + pays_fee: Pays::No, + ..Default::default() + }, + } + .into(), + topics: vec![], + }, + EventRecord { + phase: Phase::ApplyExtrinsic(7), + event: SysEvent::ExtrinsicFailed { + dispatch_error: DispatchError::BadOrigin.into(), + dispatch_info: DispatchInfo { + weight: Weight::from_ref_time(999).saturating_add(normal_base), + ..Default::default() + }, + } + .into(), + topics: vec![], + }, + EventRecord { + phase: Phase::ApplyExtrinsic(8), + event: SysEvent::ExtrinsicFailed { + dispatch_error: DispatchError::BadOrigin.into(), + dispatch_info: DispatchInfo { + weight: Weight::from_ref_time(1000).saturating_add(normal_base), + pays_fee: Pays::Yes, + ..Default::default() + }, + } + .into(), + topics: vec![], + }, + EventRecord { + phase: Phase::ApplyExtrinsic(9), + event: SysEvent::ExtrinsicFailed { + dispatch_error: DispatchError::BadOrigin.into(), + dispatch_info: DispatchInfo { + weight: Weight::from_ref_time(800).saturating_add(normal_base), + pays_fee: Pays::Yes, + ..Default::default() + }, + } + .into(), + topics: vec![], + }, + EventRecord { + phase: Phase::ApplyExtrinsic(10), + event: SysEvent::ExtrinsicFailed { + dispatch_error: DispatchError::BadOrigin.into(), + dispatch_info: DispatchInfo { + weight: Weight::from_ref_time(800).saturating_add(normal_base), + pays_fee: Pays::No, + ..Default::default() + }, + } + .into(), + topics: vec![], + }, + EventRecord { + phase: Phase::ApplyExtrinsic(11), + event: SysEvent::ExtrinsicSuccess { + dispatch_info: DispatchInfo { + weight: Weight::from_ref_time(300).saturating_add(operational_base), + class: DispatchClass::Operational, + ..Default::default() + }, + } + .into(), + topics: vec![], + }, + ]; + for (i, event) in want.into_iter().enumerate() { + assert_eq!(got[i], event, "Event mismatch at index {}", i); + } }); } -- GitLab