PR review - change on_initialize to on_idle

bridges/modules/xcm-bridge-hub-router/src/benchmarking.rs

@@ -40,7 +40,7 @@ mod benchmarks {
 	use super::*;
 
 	#[benchmark]
-	fn on_initialize_when_bridge_state_removed() -> Result<(), BenchmarkError> {
+	fn on_idle_when_bridge_state_removed() -> Result<(), BenchmarkError> {
 		let bridge_id =
 			T::BridgeIdResolver::resolve_for_dest(&T::ensure_bridged_target_destination()?)
 				.ok_or(BenchmarkError::Weightless)?;
@@ -54,7 +54,7 @@ mod benchmarks {
 
 		#[block]
 		{
-			let _ = crate::Pallet::<T, I>::on_initialize(Zero::zero());
+			let _ = crate::Pallet::<T, I>::on_idle(Zero::zero(), Weight::MAX);
 		}
 
 		assert!(Bridges::<T, I>::get(bridge_id).is_none());
@@ -63,7 +63,7 @@ mod benchmarks {
 	}
 
 	#[benchmark]
-	fn on_initialize_when_bridge_state_updated() -> Result<(), BenchmarkError> {
+	fn on_indle_when_bridge_state_updated() -> Result<(), BenchmarkError> {
 		let bridge_id =
 			T::BridgeIdResolver::resolve_for_dest(&T::ensure_bridged_target_destination()?)
 				.ok_or(BenchmarkError::Weightless)?;
@@ -78,7 +78,7 @@ mod benchmarks {
 
 		#[block]
 		{
-			let _ = crate::Pallet::<T, I>::on_initialize(Zero::zero());
+			let _ = crate::Pallet::<T, I>::on_idle(Zero::zero(), Weight::MAX);
 		}
 
 		assert!(

bridges/modules/xcm-bridge-hub-router/src/lib.rs

@@ -106,6 +106,7 @@ pub const LOG_TARGET: &str = "runtime::bridge-xcm-router";
 pub mod pallet {
 	use super::*;
 	use frame_support::pallet_prelude::*;
+	use frame_support::weights::WeightMeter;
 	use frame_system::pallet_prelude::*;
 
 	/// Default implementations of [`DefaultConfig`], which can be used to implement [`Config`].
@@ -182,23 +183,29 @@ pub mod pallet {
 
 	#[pallet::hooks]
 	impl<T: Config<I>, I: 'static> Hooks<BlockNumberFor<T>> for Pallet<T, I> {
-		fn on_initialize(_n: BlockNumberFor<T>) -> Weight {
-			let mut weight_used = Weight::zero();
+		fn on_idle(_n: BlockNumberFor<T>, remaining_weight: Weight) -> Weight {
+			let mut meter = WeightMeter::with_limit(remaining_weight);
 
-			// Iterate all uncongested bridges
+			// Iterate all congested bridges
 			let mut bridges_to_update = Vec::new();
 			let mut bridges_to_remove = Vec::new();
 			for (bridge_id, mut bridge_state) in Bridges::<T, I>::iter() {
-				weight_used.saturating_accrue(T::DbWeight::get().reads(1));
+				meter.consume(T::DbWeight::get().reads(1));
 
 				// If no longer congested, we can start decreasing the fee factor.
 				if !bridge_state.is_congested {
 					let previous_factor = bridge_state.delivery_fee_factor;
 						let new_factor = previous_factor / EXPONENTIAL_FEE_BASE;
 					if new_factor >= MINIMAL_DELIVERY_FEE_FACTOR {
 						bridge_state.delivery_fee_factor = new_factor;
+						if meter.try_consume(T::WeightInfo::on_idle_when_bridge_state_updated()).is_err() {
+							break;
+						}
 						bridges_to_update.push((bridge_id, previous_factor, bridge_state));
 					} else {
+						if meter.try_consume(T::WeightInfo::on_idle_when_bridge_state_removed()).is_err() {
+							break;
+						}
 						bridges_to_remove.push((bridge_id, previous_factor));
 					}
 				}
@@ -217,8 +224,6 @@ pub mod pallet {
 					new_value: 0.into(),
 					bridge_id,
 				});
-				weight_used
-					.saturating_accrue(T::WeightInfo::on_initialize_when_bridge_state_removed());
 			}
 			// update
 			for (bridge_id, previous_value, bridge_state) in bridges_to_update.into_iter() {
@@ -236,11 +241,9 @@ pub mod pallet {
 					new_value,
 					bridge_id,
 				});
-				weight_used
-					.saturating_accrue(T::WeightInfo::on_initialize_when_bridge_state_updated());
 			}
 
-			weight_used
+			meter.consumed()
 		}
 	}
 
@@ -539,6 +542,7 @@ mod tests {
 		run_test(|| {
 			let dest = Location::new(2, [GlobalConsensus(BridgedNetworkId::get())]);
 			let initial_fee_factor = FixedU128::from_rational(125, 100);
+			let mut remaining_weight = Weight::MAX;
 
 			// make bridge uncongested + update fee factor
 			let bridge_id = set_bridge_state_for::<TestRuntime, ()>(
@@ -550,8 +554,14 @@ mod tests {
 			let mut last_delivery_fee_factor = initial_fee_factor;
 			while let Some(bridge_state) = get_bridge_state_for::<TestRuntime, ()>(&dest) {
 				last_delivery_fee_factor = bridge_state.delivery_fee_factor;
-				XcmBridgeHubRouter::on_initialize(One::one());
+				remaining_weight = XcmBridgeHubRouter::on_idle(One::one(), remaining_weight.clone());
+
+				// avoid infinite loops (decreasing is expected)
+				if let Some(bridge_state) = get_bridge_state_for::<TestRuntime, ()>(&dest) {
+					assert!(bridge_state.delivery_fee_factor < last_delivery_fee_factor);
+				}
 			}
+			assert!(remaining_weight.all_lt(Weight::MAX));
 
 			// check emitted event
 			// (first one for updating)

bridges/modules/xcm-bridge-hub-router/src/weights.rs

@@ -50,8 +50,8 @@ use sp_std::marker::PhantomData;
 
 /// Weight functions needed for pallet_xcm_bridge_hub_router.
 pub trait WeightInfo {
-	fn on_initialize_when_bridge_state_removed() -> Weight;
-	fn on_initialize_when_bridge_state_updated() -> Weight;
+	fn on_idle_when_bridge_state_removed() -> Weight;
+	fn on_idle_when_bridge_state_updated() -> Weight;
 	fn report_bridge_status() -> Weight;
 }
 
@@ -60,7 +60,7 @@ impl WeightInfo for () {
 	/// Storage: `ToUnknownXcmRouter::Bridges` (r:2 w:1)
 	/// Proof: `ToUnknownXcmRouter::Bridges` (`max_values`: None, `max_size`: Some(65), added: 2540,
 	/// mode: `MaxEncodedLen`)
-	fn on_initialize_when_bridge_state_removed() -> Weight {
+	fn on_idle_when_bridge_state_removed() -> Weight {
 		// Proof Size summary in bytes:
 		//  Measured:  `204`
 		//  Estimated: `6070`
@@ -73,7 +73,7 @@ impl WeightInfo for () {
 	/// Storage: `ToUnknownXcmRouter::Bridges` (r:2 w:1)
 	/// Proof: `ToUnknownXcmRouter::Bridges` (`max_values`: None, `max_size`: Some(65), added: 2540,
 	/// mode: `MaxEncodedLen`)
-	fn on_initialize_when_bridge_state_updated() -> Weight {
+	fn on_idle_when_bridge_state_updated() -> Weight {
 		// Proof Size summary in bytes:
 		//  Measured:  `204`
 		//  Estimated: `6070`

cumulus/parachains/runtimes/assets/asset-hub-rococo/src/weights/pallet_xcm_bridge_hub_router.rs

@@ -50,7 +50,7 @@ pub struct WeightInfo<T>(PhantomData<T>);
 impl<T: frame_system::Config> pallet_xcm_bridge_hub_router::WeightInfo for WeightInfo<T> {
 	/// Storage: `ToWestendXcmRouter::Bridges` (r:2 w:1)
 	/// Proof: `ToWestendXcmRouter::Bridges` (`max_values`: None, `max_size`: Some(65), added: 2540, mode: `MaxEncodedLen`)
-	fn on_initialize_when_bridge_state_removed() -> Weight {
+	fn on_idle_when_bridge_state_removed() -> Weight {
 		// Proof Size summary in bytes:
 		//  Measured:  `204`
 		//  Estimated: `6070`
@@ -62,7 +62,7 @@ impl<T: frame_system::Config> pallet_xcm_bridge_hub_router::WeightInfo for Weigh
 	}
 	/// Storage: `ToWestendXcmRouter::Bridges` (r:2 w:1)
 	/// Proof: `ToWestendXcmRouter::Bridges` (`max_values`: None, `max_size`: Some(65), added: 2540, mode: `MaxEncodedLen`)
-	fn on_initialize_when_bridge_state_updated() -> Weight {
+	fn on_idle_when_bridge_state_updated() -> Weight {
 		// Proof Size summary in bytes:
 		//  Measured:  `204`
 		//  Estimated: `6070`

cumulus/parachains/runtimes/assets/asset-hub-westend/src/weights/pallet_xcm_bridge_hub_router.rs

@@ -50,7 +50,7 @@ pub struct WeightInfo<T>(PhantomData<T>);
 impl<T: frame_system::Config> pallet_xcm_bridge_hub_router::WeightInfo for WeightInfo<T> {
 	/// Storage: `ToRococoXcmRouter::Bridges` (r:2 w:1)
 	/// Proof: `ToRococoXcmRouter::Bridges` (`max_values`: None, `max_size`: Some(65), added: 2540, mode: `MaxEncodedLen`)
-	fn on_initialize_when_bridge_state_removed() -> Weight {
+	fn on_idle_when_bridge_state_removed() -> Weight {
 		// Proof Size summary in bytes:
 		//  Measured:  `204`
 		//  Estimated: `6070`
@@ -62,7 +62,7 @@ impl<T: frame_system::Config> pallet_xcm_bridge_hub_router::WeightInfo for Weigh
 	}
 	/// Storage: `ToRococoXcmRouter::Bridges` (r:2 w:1)
 	/// Proof: `ToRococoXcmRouter::Bridges` (`max_values`: None, `max_size`: Some(65), added: 2540, mode: `MaxEncodedLen`)
-	fn on_initialize_when_bridge_state_updated() -> Weight {
+	fn on_idle_when_bridge_state_updated() -> Weight {
 		// Proof Size summary in bytes:
 		//  Measured:  `204`
 		//  Estimated: `6070`