Split Operational and Normal weight (#5954)

* Split operational and normal weight

* Update other parts of the code

* relatively minor changes to add/fix behavior

* Limit reported block weight for fee adjustment

* fix test

* explicit match

* Move common logic out

* Update frame/system/src/lib.rs

Co-authored-by: Gavin Wood <gavin@parity.io>

* API improvement @bkchr

Co-authored-by: Gavin Wood <gavin@parity.io>

substrate/bin/node/runtime/src/impls.rs

@@ -70,8 +70,8 @@ pub struct TargetedFeeAdjustment<T>(sp_std::marker::PhantomData<T>);
 
 impl<T: Get<Perquintill>> Convert<Fixed128, Fixed128> for TargetedFeeAdjustment<T> {
 	fn convert(multiplier: Fixed128) -> Fixed128 {
-		let block_weight = System::all_extrinsics_weight();
 		let max_weight = MaximumBlockWeight::get();
+		let block_weight = System::all_extrinsics_weight().total().min(max_weight);
 		let target_weight = (T::get() * max_weight) as u128;
 		let block_weight = block_weight as u128;
 
@@ -132,11 +132,12 @@ mod tests {
 
 	// poc reference implementation.
 	fn fee_multiplier_update(block_weight: Weight, previous: Fixed128) -> Fixed128  {
-		let block_weight = block_weight as f64;
-		let v: f64 = 0.00004;
-
 		// maximum tx weight
 		let m = max() as f64;
+		// block weight always truncated to max weight
+		let block_weight = (block_weight as f64).min(m);
+		let v: f64 = 0.00004;
+
 		// Ideal saturation in terms of weight
 		let ss = target() as f64;
 		// Current saturation in terms of weight

substrate/frame/executive/src/lib.rs

@@ -116,7 +116,7 @@
 
 use sp_std::{prelude::*, marker::PhantomData};
 use frame_support::{
-	storage::StorageValue, weights::{GetDispatchInfo, DispatchInfo},
+	storage::StorageValue, weights::{GetDispatchInfo, DispatchInfo, DispatchClass},
 	traits::{OnInitialize, OnFinalize, OnRuntimeUpgrade, OffchainWorker},
 };
 use sp_runtime::{
@@ -235,7 +235,7 @@ where
 			let mut weight = <frame_system::Module::<System> as OnRuntimeUpgrade>::on_runtime_upgrade();
 			weight = weight.saturating_add(COnRuntimeUpgrade::on_runtime_upgrade());
 			weight = weight.saturating_add(<AllModules as OnRuntimeUpgrade>::on_runtime_upgrade());
-			<frame_system::Module<System>>::register_extra_weight_unchecked(weight);
+			<frame_system::Module<System>>::register_extra_weight_unchecked(weight, DispatchClass::Mandatory);
 		}
 		<frame_system::Module<System>>::initialize(
 			block_number,
@@ -247,7 +247,7 @@ where
 		<frame_system::Module<System> as OnInitialize<System::BlockNumber>>::on_initialize(*block_number);
 		let weight = <AllModules as OnInitialize<System::BlockNumber>>::on_initialize(*block_number)
 			.saturating_add(<System::BlockExecutionWeight as frame_support::traits::Get<_>>::get());
-		<frame_system::Module::<System>>::register_extra_weight_unchecked(weight);
+		<frame_system::Module::<System>>::register_extra_weight_unchecked(weight, DispatchClass::Mandatory);
 
 		frame_system::Module::<System>::note_finished_initialize();
 	}
@@ -785,7 +785,7 @@ mod tests {
 				Digest::default(),
 			));
 			// Base block execution weight + `on_initialize` weight from the custom module.
-			assert_eq!(<frame_system::Module<Runtime>>::all_extrinsics_weight(), base_block_weight);
+			assert_eq!(<frame_system::Module<Runtime>>::all_extrinsics_weight().total(), base_block_weight);
 
 			for nonce in 0..=num_to_exhaust_block {
 				let xt = TestXt::new(
@@ -795,7 +795,7 @@ mod tests {
 				if nonce != num_to_exhaust_block {
 					assert!(res.is_ok());
 					assert_eq!(
-						<frame_system::Module<Runtime>>::all_extrinsics_weight(),
+						<frame_system::Module<Runtime>>::all_extrinsics_weight().total(),
 						//--------------------- on_initialize + block_execution + extrinsic_base weight
 						(encoded_len + 5) * (nonce + 1) + base_block_weight,
 					);
@@ -815,7 +815,7 @@ mod tests {
 		let len = xt.clone().encode().len() as u32;
 		let mut t = new_test_ext(1);
 		t.execute_with(|| {
-			assert_eq!(<frame_system::Module<Runtime>>::all_extrinsics_weight(), 0);
+			assert_eq!(<frame_system::Module<Runtime>>::all_extrinsics_weight().total(), 0);
 			assert_eq!(<frame_system::Module<Runtime>>::all_extrinsics_len(), 0);
 
 			assert!(Executive::apply_extrinsic(xt.clone()).unwrap().is_ok());
@@ -824,14 +824,14 @@ mod tests {
 
 			// default weight for `TestXt` == encoded length.
 			assert_eq!(
-				<frame_system::Module<Runtime>>::all_extrinsics_weight(),
+				<frame_system::Module<Runtime>>::all_extrinsics_weight().total(),
 				3 * (len as Weight + <Runtime as frame_system::Trait>::ExtrinsicBaseWeight::get()),
 			);
 			assert_eq!(<frame_system::Module<Runtime>>::all_extrinsics_len(), 3 * len);
 
 			let _ = <frame_system::Module<Runtime>>::finalize();
 
-			assert_eq!(<frame_system::Module<Runtime>>::all_extrinsics_weight(), 0);
+			assert_eq!(<frame_system::Module<Runtime>>::all_extrinsics_weight().total(), 0);
 			assert_eq!(<frame_system::Module<Runtime>>::all_extrinsics_len(), 0);
 		});
 	}
@@ -903,7 +903,7 @@ mod tests {
 			// NOTE: might need updates over time if new weights are introduced.
 			// For now it only accounts for the base block execution weight and
 			// the `on_initialize` weight defined in the custom test module.
-			assert_eq!(<frame_system::Module<Runtime>>::all_extrinsics_weight(), 175 + 10);
+			assert_eq!(<frame_system::Module<Runtime>>::all_extrinsics_weight().total(), 175 + 10);
 		})
 	}
 

substrate/frame/system/src/lib.rs

@@ -357,6 +357,60 @@ impl From<sp_version::RuntimeVersion> for LastRuntimeUpgradeInfo {
 	}
 }
 
+/// An object to track the currently used extrinsic weight in a block.
+#[derive(Clone, Eq, PartialEq, Default, RuntimeDebug, Encode, Decode)]
+pub struct ExtrinsicsWeight {
+	normal: Weight,
+	operational: Weight,
+}
+
+impl ExtrinsicsWeight {
+	/// Returns the total weight consumed by all extrinsics in the block.
+	pub fn total(&self) -> Weight {
+		self.normal.saturating_add(self.operational)
+	}
+
+	/// Add some weight of a specific dispatch class, saturating at the numeric bounds of `Weight`.
+	pub fn add(&mut self, weight: Weight, class: DispatchClass) {
+		let value = self.get_mut(class);
+		*value = value.saturating_add(weight);
+	}
+
+	/// Try to add some weight of a specific dispatch class, returning Err(()) if overflow would occur.
+	pub fn checked_add(&mut self, weight: Weight, class: DispatchClass) -> Result<(), ()> {
+		let value = self.get_mut(class);
+		*value = value.checked_add(weight).ok_or(())?;
+		Ok(())
+	}
+
+	/// Subtract some weight of a specific dispatch class, saturating at the numeric bounds of `Weight`.
+	pub fn sub(&mut self, weight: Weight, class: DispatchClass) {
+		let value = self.get_mut(class);
+		*value = value.saturating_sub(weight);
+	}
+
+	/// Get the current weight of a specific dispatch class.
+	pub fn get(&self, class: DispatchClass) -> Weight {
+		match class {
+			DispatchClass::Operational => self.operational,
+			DispatchClass::Normal | DispatchClass::Mandatory => self.normal,
+		}
+	}
+
+	/// Get a mutable reference to the current weight of a specific dispatch class.
+	fn get_mut(&mut self, class: DispatchClass) -> &mut Weight {
+		match class {
+			DispatchClass::Operational => &mut self.operational,
+			DispatchClass::Normal | DispatchClass::Mandatory => &mut self.normal,
+		}
+	}
+
+	/// Set the weight of a specific dispatch class.
+	pub fn put(&mut self, new: Weight, class: DispatchClass) {
+		*self.get_mut(class) = new;
+	}
+}
+
 decl_storage! {
 	trait Store for Module<T: Trait> as System {
 		/// The full account information for a particular account ID.
@@ -366,8 +420,8 @@ decl_storage! {
 		/// Total extrinsics count for the current block.
 		ExtrinsicCount: Option<u32>;
 
-		/// Total weight for all extrinsics put together, for the current block.
-		AllExtrinsicsWeight: Option<Weight>;
+		/// Total weight for all extrinsics for the current block.
+		AllExtrinsicsWeight: ExtrinsicsWeight;
 
 		/// Total length (in bytes) for all extrinsics put together, for the current block.
 		AllExtrinsicsLen: Option<u32>;
@@ -915,9 +969,9 @@ impl<T: Trait> Module<T> {
 		ExtrinsicCount::get().unwrap_or_default()
 	}
 
-	/// Gets a total weight of all executed extrinsics.
-	pub fn all_extrinsics_weight() -> Weight {
-		AllExtrinsicsWeight::get().unwrap_or_default()
+	/// Gets the weight of all executed extrinsics.
+	pub fn all_extrinsics_weight() -> ExtrinsicsWeight {
+		AllExtrinsicsWeight::get()
 	}
 
 	pub fn all_extrinsics_len() -> u32 {
@@ -939,12 +993,10 @@ impl<T: Trait> Module<T> {
 	/// of block weight is more than the block weight limit. This is what the _unchecked_.
 	///
 	/// Another potential use-case could be for the `on_initialize` and `on_finalize` hooks.
-	///
-	/// If no previous weight exists, the function initializes the weight to zero.
-	pub fn register_extra_weight_unchecked(weight: Weight) {
-		let current_weight = AllExtrinsicsWeight::get().unwrap_or_default();
-		let next_weight = current_weight.saturating_add(weight);
-		AllExtrinsicsWeight::put(next_weight);
+	pub fn register_extra_weight_unchecked(weight: Weight, class: DispatchClass) {
+		AllExtrinsicsWeight::mutate(|current_weight| {
+			current_weight.add(weight, class);
+		});
 	}
 
 	/// Start the execution of a particular block.
@@ -1065,7 +1117,9 @@ impl<T: Trait> Module<T> {
 	/// Set the current block weight. This should only be used in some integration tests.
 	#[cfg(any(feature = "std", test))]
 	pub fn set_block_limits(weight: Weight, len: usize) {
-		AllExtrinsicsWeight::put(weight);
+		AllExtrinsicsWeight::mutate(|current_weight| {
+			current_weight.put(weight, DispatchClass::Normal)
+		});
 		AllExtrinsicsLen::put(len as u32);
 	}
 
@@ -1298,24 +1352,49 @@ impl<T: Trait + Send + Sync> CheckWeight<T> where
 	/// Upon successes, it returns the new block weight as a `Result`.
 	fn check_weight(
 		info: &DispatchInfoOf<T::Call>,
-	) -> Result<Weight, TransactionValidityError> {
-		let current_weight = Module::<T>::all_extrinsics_weight();
+	) -> Result<ExtrinsicsWeight, TransactionValidityError> {
 		let maximum_weight = T::MaximumBlockWeight::get();
-		let limit = Self::get_dispatch_limit_ratio(info.class) * maximum_weight;
-		if info.class == DispatchClass::Mandatory {
+		let mut all_weight = Module::<T>::all_extrinsics_weight();
+		match info.class {
 			// If we have a dispatch that must be included in the block, it ignores all the limits.
-			let extrinsic_weight = info.weight.saturating_add(T::ExtrinsicBaseWeight::get());
-			let next_weight = current_weight.saturating_add(extrinsic_weight);
-			Ok(next_weight)
-		} else {
-			let extrinsic_weight = info.weight.checked_add(T::ExtrinsicBaseWeight::get())
-				.ok_or(InvalidTransaction::ExhaustsResources)?;
-			let next_weight = current_weight.checked_add(extrinsic_weight)
-				.ok_or(InvalidTransaction::ExhaustsResources)?;
-			if next_weight > limit {
-				Err(InvalidTransaction::ExhaustsResources.into())
-			} else {
-				Ok(next_weight)
+			DispatchClass::Mandatory => {
+				let extrinsic_weight = info.weight.saturating_add(T::ExtrinsicBaseWeight::get());
+				all_weight.add(extrinsic_weight, DispatchClass::Mandatory);
+				Ok(all_weight)
+			},
+			// If we have a normal dispatch, we follow all the normal rules and limits.
+			DispatchClass::Normal => {
+				let normal_limit = Self::get_dispatch_limit_ratio(DispatchClass::Normal) * maximum_weight;
+				let extrinsic_weight = info.weight.checked_add(T::ExtrinsicBaseWeight::get())
+					.ok_or(InvalidTransaction::ExhaustsResources)?;
+				all_weight.checked_add(extrinsic_weight, DispatchClass::Normal)
+					.map_err(|_| InvalidTransaction::ExhaustsResources)?;
+				if all_weight.get(DispatchClass::Normal) > normal_limit {
+					Err(InvalidTransaction::ExhaustsResources.into())
+				} else {
+					Ok(all_weight)
+				}
+			},
+			// If we have an operational dispatch, allow it if we have not used our full
+			// "operational space" (independent of existing fullness).
+			DispatchClass::Operational => {
+				let operational_limit = Self::get_dispatch_limit_ratio(DispatchClass::Operational) * maximum_weight;
+				let normal_limit = Self::get_dispatch_limit_ratio(DispatchClass::Normal) * maximum_weight;
+				let operational_space = operational_limit.saturating_sub(normal_limit);
+
+				let extrinsic_weight = info.weight.checked_add(T::ExtrinsicBaseWeight::get())
+					.ok_or(InvalidTransaction::ExhaustsResources)?;
+				all_weight.checked_add(extrinsic_weight, DispatchClass::Operational)
+					.map_err(|_| InvalidTransaction::ExhaustsResources)?;
+
+				// If it would fit in normally, its okay
+				if all_weight.total() <= maximum_weight ||
+				// If we have not used our operational space
+				all_weight.get(DispatchClass::Operational) <= operational_space {
+					Ok(all_weight)
+				} else {
+					Err(InvalidTransaction::ExhaustsResources.into())
+				}
 			}
 		}
 	}
@@ -1452,8 +1531,8 @@ impl<T: Trait + Send + Sync> SignedExtension for CheckWeight<T> where
 
 		let unspent = post_info.calc_unspent(info);
 		if unspent > 0 {
-			AllExtrinsicsWeight::mutate(|weight| {
-				*weight = weight.map(|w| w.saturating_sub(unspent));
+			AllExtrinsicsWeight::mutate(|current_weight| {
+				current_weight.sub(unspent, info.class);
 			})
 		}
 
@@ -1713,7 +1792,7 @@ pub(crate) mod tests {
 	use sp_std::cell::RefCell;
 	use sp_core::H256;
 	use sp_runtime::{traits::{BlakeTwo256, IdentityLookup, SignedExtension}, testing::Header, DispatchError};
-	use frame_support::{impl_outer_origin, parameter_types, assert_ok};
+	use frame_support::{impl_outer_origin, parameter_types, assert_ok, assert_noop};
 
 	impl_outer_origin! {
 		pub enum Origin for Test where system = super {}
@@ -1810,7 +1889,7 @@ pub(crate) mod tests {
 	fn new_test_ext() -> sp_io::TestExternalities {
 		let mut ext: sp_io::TestExternalities = GenesisConfig::default().build_storage::<Test>().unwrap().into();
 		// Add to each test the initial weight of a block
-		ext.execute_with(|| System::register_extra_weight_unchecked(<Test as Trait>::BlockExecutionWeight::get()));
+		ext.execute_with(|| System::register_extra_weight_unchecked(<Test as Trait>::BlockExecutionWeight::get(), DispatchClass::Mandatory));
 		ext
 	}
 
@@ -2036,7 +2115,9 @@ pub(crate) mod tests {
 			let len = 0_usize;
 
 			let reset_check_weight = |i, f, s| {
-				AllExtrinsicsWeight::put(s);
+				AllExtrinsicsWeight::mutate(|current_weight| {
+					current_weight.put(s, DispatchClass::Normal)
+				});
 				let r = CheckWeight::<Test>(PhantomData).pre_dispatch(&1, CALL, i, len);
 				if f { assert!(r.is_err()) } else { assert!(r.is_ok()) }
 			};
@@ -2056,17 +2137,19 @@ pub(crate) mod tests {
 			let len = 0_usize;
 
 			// We allow 75% for normal transaction, so we put 25% - extrinsic base weight
-			AllExtrinsicsWeight::put(256 - <Test as Trait>::ExtrinsicBaseWeight::get());
+			AllExtrinsicsWeight::mutate(|current_weight| {
+				current_weight.put(256 - <Test as Trait>::ExtrinsicBaseWeight::get(), DispatchClass::Normal)
+			});
 
 			let pre = CheckWeight::<Test>(PhantomData).pre_dispatch(&1, CALL, &info, len).unwrap();
-			assert_eq!(AllExtrinsicsWeight::get().unwrap(), info.weight + 256);
+			assert_eq!(AllExtrinsicsWeight::get().total(), info.weight + 256);
 
 			assert!(
 				CheckWeight::<Test>::post_dispatch(pre, &info, &post_info, len, &Ok(()))
 				.is_ok()
 			);
 			assert_eq!(
-				AllExtrinsicsWeight::get().unwrap(),
+				AllExtrinsicsWeight::get().total(),
 				post_info.actual_weight.unwrap() + 256,
 			);
 		})
@@ -2079,11 +2162,13 @@ pub(crate) mod tests {
 			let post_info = PostDispatchInfo { actual_weight: Some(700), };
 			let len = 0_usize;
 
-			AllExtrinsicsWeight::put(128);
+			AllExtrinsicsWeight::mutate(|current_weight| {
+				current_weight.put(128, DispatchClass::Normal)
+			});
 
 			let pre = CheckWeight::<Test>(PhantomData).pre_dispatch(&1, CALL, &info, len).unwrap();
 			assert_eq!(
-				AllExtrinsicsWeight::get().unwrap(),
+				AllExtrinsicsWeight::get().total(),
 				info.weight + 128 + <Test as Trait>::ExtrinsicBaseWeight::get(),
 			);
 
@@ -2092,7 +2177,7 @@ pub(crate) mod tests {
 				.is_ok()
 			);
 			assert_eq!(
-				AllExtrinsicsWeight::get().unwrap(),
+				AllExtrinsicsWeight::get().total(),
 				info.weight + 128 + <Test as Trait>::ExtrinsicBaseWeight::get(),
 			);
 		})
@@ -2105,11 +2190,11 @@ pub(crate) mod tests {
 			let len = 0_usize;
 
 			// Initial weight from `BlockExecutionWeight`
-			assert_eq!(System::all_extrinsics_weight(), <Test as Trait>::BlockExecutionWeight::get());
+			assert_eq!(System::all_extrinsics_weight().total(), <Test as Trait>::BlockExecutionWeight::get());
 			let r = CheckWeight::<Test>(PhantomData).pre_dispatch(&1, CALL, &free, len);
 			assert!(r.is_ok());
 			assert_eq!(
-				System::all_extrinsics_weight(),
+				System::all_extrinsics_weight().total(),
 				<Test as Trait>::ExtrinsicBaseWeight::get() + <Test as Trait>::BlockExecutionWeight::get()
 			);
 		})
@@ -2126,17 +2211,17 @@ pub(crate) mod tests {
 			let len = 0_usize;
 
 			assert_ok!(CheckWeight::<Test>::do_pre_dispatch(&max, len));
-			assert_eq!(System::all_extrinsics_weight(), Weight::max_value());
-			assert!(System::all_extrinsics_weight() > <Test as Trait>::MaximumBlockWeight::get());
+			assert_eq!(System::all_extrinsics_weight().total(), Weight::max_value());
+			assert!(System::all_extrinsics_weight().total() > <Test as Trait>::MaximumBlockWeight::get());
 		});
 	}
 
 	#[test]
 	fn register_extra_weight_unchecked_doesnt_care_about_limits() {
 		new_test_ext().execute_with(|| {
-			System::register_extra_weight_unchecked(Weight::max_value());
-			assert_eq!(System::all_extrinsics_weight(), Weight::max_value());
-			assert!(System::all_extrinsics_weight() > <Test as Trait>::MaximumBlockWeight::get());
+			System::register_extra_weight_unchecked(Weight::max_value(), DispatchClass::Normal);
+			assert_eq!(System::all_extrinsics_weight().total(), Weight::max_value());
+			assert!(System::all_extrinsics_weight().total() > <Test as Trait>::MaximumBlockWeight::get());
 		});
 	}
 
@@ -2154,10 +2239,45 @@ pub(crate) mod tests {
 			let len = 0_usize;
 
 			assert_ok!(CheckWeight::<Test>::do_pre_dispatch(&max_normal, len));
-			assert_eq!(System::all_extrinsics_weight(), 768);
+			assert_eq!(System::all_extrinsics_weight().total(), 768);
 			assert_ok!(CheckWeight::<Test>::do_pre_dispatch(&rest_operational, len));
 			assert_eq!(<Test as Trait>::MaximumBlockWeight::get(), 1024);
-			assert_eq!(System::all_extrinsics_weight(), <Test as Trait>::MaximumBlockWeight::get());
+			assert_eq!(System::all_extrinsics_weight().total(), <Test as Trait>::MaximumBlockWeight::get());
+		});
+	}
+
+	#[test]
+	fn dispatch_order_does_not_effect_weight_logic() {
+		new_test_ext().execute_with(|| {
+			// We switch the order of `full_block_with_normal_and_operational`
+			let max_normal = DispatchInfo { weight: 753, ..Default::default() };
+			let rest_operational = DispatchInfo { weight: 251, class: DispatchClass::Operational, ..Default::default() };
+
+			let len = 0_usize;
+
+			assert_ok!(CheckWeight::<Test>::do_pre_dispatch(&rest_operational, len));
+			// Extra 15 here from block execution + base extrinsic weight
+			assert_eq!(System::all_extrinsics_weight().total(), 266);
+			assert_ok!(CheckWeight::<Test>::do_pre_dispatch(&max_normal, len));
+			assert_eq!(<Test as Trait>::MaximumBlockWeight::get(), 1024);
+			assert_eq!(System::all_extrinsics_weight().total(), <Test as Trait>::MaximumBlockWeight::get());
+		});
+	}
+
+	#[test]
+	fn operational_works_on_full_block() {
+		new_test_ext().execute_with(|| {
+			// An on_initialize takes up the whole block! (Every time!)
+			System::register_extra_weight_unchecked(Weight::max_value(), DispatchClass::Mandatory);
+			let dispatch_normal = DispatchInfo { weight: 251, class: DispatchClass::Normal, ..Default::default() };
+			let dispatch_operational = DispatchInfo { weight: 251, class: DispatchClass::Operational, ..Default::default() };
+			let len = 0_usize;
+
+			assert_noop!(CheckWeight::<Test>::do_pre_dispatch(&dispatch_normal, len), InvalidTransaction::ExhaustsResources);
+			// Thank goodness we can still do an operational transaction to possibly save the blockchain.
+			assert_ok!(CheckWeight::<Test>::do_pre_dispatch(&dispatch_operational, len));
+			// Not too much though
+			assert_noop!(CheckWeight::<Test>::do_pre_dispatch(&dispatch_operational, len), InvalidTransaction::ExhaustsResources);
 		});
 	}
 
@@ -2170,7 +2290,9 @@ pub(crate) mod tests {
 			let normal_limit = normal_weight_limit();
 
 			// given almost full block
-			AllExtrinsicsWeight::put(normal_limit);
+			AllExtrinsicsWeight::mutate(|current_weight| {
+				current_weight.put(normal_limit, DispatchClass::Normal)
+			});
 			// will not fit.
 			assert!(CheckWeight::<Test>(PhantomData).pre_dispatch(&1, CALL, &normal, len).is_err());
 			// will fit.