Skip idle_scheduler Calls When Not Producing blocks

modules/evm/src/bench/mock.rs

@@ -34,7 +34,7 @@ pub use primitives::{
 };
 use sp_core::{H160, H256};
 use sp_runtime::{
-	traits::{BlakeTwo256, IdentityLookup},
+	traits::{BlakeTwo256, BlockNumberProvider, IdentityLookup},
 	AccountId32,
 };
 
@@ -141,13 +141,26 @@ define_combined_task! {
 
 parameter_types!(
 	pub MinimumWeightRemainInBlock: Weight = u64::MIN;
+	pub const DisableBlockThreshold: BlockNumber = 6;
 );
 
+pub struct MockBlockNumberProvider;
+
+impl BlockNumberProvider for MockBlockNumberProvider {
+	type BlockNumber = u32;
+
+	fn current_block_number() -> Self::BlockNumber {
+		Zero::zero()
+	}
+}
+
 impl module_idle_scheduler::Config for Runtime {
 	type Event = Event;
 	type WeightInfo = ();
 	type Task = ScheduledTasks;
 	type MinimumWeightRemainInBlock = MinimumWeightRemainInBlock;
+	type RelayChainBlockNumberProvider = MockBlockNumberProvider;
+	type DisableBlockThreshold = DisableBlockThreshold;
 }
 
 pub struct GasToWeight;

modules/evm/src/mock.rs

@@ -32,7 +32,7 @@ use primitives::{define_combined_task, Amount, BlockNumber, CurrencyId, ReserveI
 use sp_core::{H160, H256};
 use sp_runtime::{
 	testing::Header,
-	traits::{BlakeTwo256, IdentityLookup},
+	traits::{BlakeTwo256, BlockNumberProvider, IdentityLookup},
 	AccountId32,
 };
 use std::{collections::BTreeMap, str::FromStr};
@@ -140,13 +140,26 @@ define_combined_task! {
 
 parameter_types!(
 	pub MinimumWeightRemainInBlock: Weight = u64::MIN;
+	pub DisableBlockThreshold: BlockNumber = u32::MAX;
 );
 
+pub struct MockBlockNumberProvider;
+
+impl BlockNumberProvider for MockBlockNumberProvider {
+	type BlockNumber = u32;
+
+	fn current_block_number() -> Self::BlockNumber {
+		Zero::zero()
+	}
+}
+
 impl module_idle_scheduler::Config for Runtime {
 	type Event = Event;
 	type WeightInfo = ();
 	type Task = ScheduledTasks;
 	type MinimumWeightRemainInBlock = MinimumWeightRemainInBlock;
+	type RelayChainBlockNumberProvider = MockBlockNumberProvider;
+	type DisableBlockThreshold = DisableBlockThreshold;
 }
 
 pub struct GasToWeight;

modules/idle-scheduler/src/lib.rs

@@ -23,14 +23,15 @@
 #![cfg_attr(not(feature = "std"), no_std)]
 #![allow(clippy::unused_unit)]
 #![allow(unused_must_use)]
-use acala_primitives::{task::TaskResult, Nonce};
+use acala_primitives::{task::TaskResult, BlockNumber, Nonce};
 use codec::FullCodec;
+use frame_support::log;
 use frame_support::pallet_prelude::*;
 use frame_system::pallet_prelude::*;
 pub use module_support::{DispatchableTask, IdleScheduler};
 use scale_info::TypeInfo;
 use sp_runtime::{
-	traits::{One, Zero},
+	traits::{BlockNumberProvider, One},
 	ArithmeticError,
 };
 use sp_std::{cmp::PartialEq, fmt::Debug, prelude::*};
@@ -58,6 +59,15 @@ pub mod module {
 		/// The minimum weight that should remain before idle tasks are dispatched.
 		#[pallet::constant]
 		type MinimumWeightRemainInBlock: Get<Weight>;
+
+		/// Gets RelayChain Block Number
+		type RelayChainBlockNumberProvider: BlockNumberProvider<BlockNumber = BlockNumber>;
+
+		/// Number of Relay Chain blocks skipped to disable `on_idle` dispatching scheduled tasks
+		/// this shuts down idle-scheduler when block production is slower than this number of
+		/// relaychain blocks
+		#[pallet::constant]
+		type DisableBlockThreshold: Get<BlockNumber>;
 	}
 
 	#[pallet::event]
@@ -76,14 +86,50 @@ pub mod module {
 	#[pallet::getter(fn next_task_id)]
 	pub type NextTaskId<T: Config> = StorageValue<_, Nonce, ValueQuery>;
 
+	///
+	#[pallet::storage]
+	#[pallet::getter(fn previous_relay_block)]
+	pub type PreviousRelayBlockNumber<T: Config> = StorageValue<_, BlockNumber, ValueQuery>;
+
 	#[pallet::pallet]
 	#[pallet::without_storage_info]
 	pub struct Pallet<T>(_);
 
 	#[pallet::hooks]
 	impl<T: Config> Hooks<T::BlockNumber> for Pallet<T> {
+		fn on_initialize(_n: T::BlockNumber) -> Weight {
+			// This is the previous relay block because `on_initialize` is executed
+			// before the inherent that sets the new relay chain block number
+			let previous_relay_block: BlockNumber = T::RelayChainBlockNumberProvider::current_block_number();
+
+			PreviousRelayBlockNumber::<T>::put(previous_relay_block);
+			T::WeightInfo::on_initialize()
+		}
+
 		fn on_idle(_n: T::BlockNumber, remaining_weight: Weight) -> Weight {
-			Self::do_dispatch_tasks(remaining_weight)
+			// Checks if we have skipped enough relay blocks without block production to skip dispatching
+			// scheduled tasks
+			let current_relay_block_number: BlockNumber = T::RelayChainBlockNumberProvider::current_block_number();
+			let previous_relay_block_number = PreviousRelayBlockNumber::<T>::take();
+			if current_relay_block_number.saturating_sub(previous_relay_block_number) >= T::DisableBlockThreshold::get()
+			{
+				log::debug!(
+					target: "idle-scheduler",
+					"Relaychain produced blocks without finalizing parachain blocks. Idle-scheduler will not execute.\ncurrent relay block number: {:?}\nprevious relay block number: {:?}",
+					current_relay_block_number,
+					previous_relay_block_number
+				);
+				// something is not correct so exaust all remaining weight (note: any on_idle hooks after
+				// IdleScheduler won't execute)
+				remaining_weight
+			} else {
+				Self::do_dispatch_tasks(remaining_weight)
+			}
+		}
+
+		fn on_finalize(_n: T::BlockNumber) {
+			// Don't commit to storage, needed for the case block is full and `on_idle` isn't called
+			PreviousRelayBlockNumber::<T>::kill();
 		}
 	}
 
@@ -116,9 +162,10 @@ impl<T: Config> Pallet<T> {
 
 	/// Keep dispatching tasks in Storage, until insufficient weight remains.
 	pub fn do_dispatch_tasks(total_weight: Weight) -> Weight {
-		let mut weight_remaining = total_weight;
+		let mut weight_remaining = total_weight.saturating_sub(T::WeightInfo::on_idle_base());
 		if weight_remaining <= T::MinimumWeightRemainInBlock::get() {
-			return Zero::zero();
+			// return total weight so no `on_idle` hook will execute after IdleScheduler
+			return total_weight;
 		}
 
 		let mut completed_tasks: Vec<(Nonce, TaskResult)> = vec![];
@@ -128,6 +175,7 @@ impl<T: Config> Pallet<T> {
 			weight_remaining = weight_remaining.saturating_sub(result.used_weight);
 			if result.finished {
 				completed_tasks.push((id, result));
+				weight_remaining = weight_remaining.saturating_sub(T::WeightInfo::clear_tasks());
 			}
 
 			// If remaining weight falls below the minimmum, break from the loop.
@@ -136,6 +184,13 @@ impl<T: Config> Pallet<T> {
 			}
 		}
 
+		Self::remove_completed_tasks(completed_tasks);
+
+		total_weight.saturating_sub(weight_remaining)
+	}
+
+	// Removes completed tasks and deposits events
+	pub fn remove_completed_tasks(completed_tasks: Vec<(Nonce, TaskResult)>) {
 		// Deposit event and remove completed tasks.
 		for (id, result) in completed_tasks {
 			Self::deposit_event(Event::<T>::TaskDispatched {
@@ -144,8 +199,6 @@ impl<T: Config> Pallet<T> {
 			});
 			Tasks::<T>::remove(id);
 		}
-
-		total_weight.saturating_sub(weight_remaining)
 	}
 }
 

modules/idle-scheduler/src/mock.rs

@@ -25,11 +25,14 @@ use acala_primitives::{define_combined_task, task::TaskResult};
 use frame_support::weights::Weight;
 use frame_support::{construct_runtime, parameter_types, traits::Everything};
 use module_support::DispatchableTask;
+pub use sp_runtime::offchain::storage::StorageValueRef;
 
+use super::*;
 use codec::{Decode, Encode};
 use scale_info::TypeInfo;
 
 pub const BASE_WEIGHT: Weight = 1_000_000;
+pub const RELAY_BLOCK_KEY: [u8; 32] = [0; 32];
 
 parameter_types!(
 	pub const BlockHashCount: u32 = 250;
@@ -65,13 +68,27 @@ impl frame_system::Config for Runtime {
 
 parameter_types!(
 	pub const MinimumWeightRemainInBlock: Weight = 100_000_000_000;
+	pub const DisableBlockThreshold: BlockNumber = 6;
 );
 
+pub struct MockBlockNumberProvider;
+
+impl BlockNumberProvider for MockBlockNumberProvider {
+	type BlockNumber = u32;
+
+	fn current_block_number() -> Self::BlockNumber {
+		// gets a local mock storage value
+		u32::decode(&mut &sp_io::storage::get(&RELAY_BLOCK_KEY).unwrap()[..]).unwrap()
+	}
+}
+
 impl module_idle_scheduler::Config for Runtime {
 	type Event = Event;
 	type WeightInfo = ();
 	type Task = ScheduledTasks;
 	type MinimumWeightRemainInBlock = MinimumWeightRemainInBlock;
+	type RelayChainBlockNumberProvider = MockBlockNumberProvider;
+	type DisableBlockThreshold = DisableBlockThreshold;
 }
 
 // Mock dispatachable tasks
@@ -137,6 +154,7 @@ impl ExtBuilder {
 
 		let mut ext = sp_io::TestExternalities::new(t);
 		ext.execute_with(|| System::set_block_number(1));
+		ext.execute_with(|| sp_io::storage::set(&RELAY_BLOCK_KEY, &0_u32.encode()));
 		ext
 	}
 }

modules/idle-scheduler/src/tests.rs

@@ -69,8 +69,9 @@ fn can_process_tasks_up_to_weight_limit() {
 			ScheduledTasks::HomaLiteTask(HomaLiteTask::OnIdle)
 		));
 
-		// Given enough weights for only 2 tasks: MinimumWeightRemainInBlock::get() + BASE_WEIGHT*2
-		IdleScheduler::on_idle(0, 100_002_000_000);
+		// Given enough weights for only 2 tasks: MinimumWeightRemainInBlock::get() + BASE_WEIGHT*2 +
+		// on_idle_base()
+		IdleScheduler::on_idle(0, 100_002_000_000 + <()>::on_idle_base() + (<()>::clear_tasks() * 2));
 
 		// Due to hashing, excution is not guaranteed to be in order.
 		assert_eq!(
@@ -80,13 +81,13 @@ fn can_process_tasks_up_to_weight_limit() {
 		assert_eq!(Tasks::<Runtime>::get(1), None);
 		assert_eq!(Tasks::<Runtime>::get(2), None);
 
-		IdleScheduler::on_idle(0, 100_000_000_000);
+		IdleScheduler::on_idle(0, 100_000_000_000 + <()>::on_idle_base());
 		assert_eq!(
 			Tasks::<Runtime>::get(0),
 			Some(ScheduledTasks::BalancesTask(BalancesTask::OnIdle))
 		);
 
-		IdleScheduler::on_idle(0, 100_001_000_000);
+		IdleScheduler::on_idle(0, 100_001_000_000 + <()>::on_idle_base());
 		assert_eq!(Tasks::<Runtime>::get(0), None);
 	});
 }
@@ -104,3 +105,26 @@ fn can_increment_next_task_id() {
 		assert_eq!(NextTaskId::<Runtime>::get(), 1);
 	});
 }
+
+#[test]
+fn on_idle_works() {
+	ExtBuilder::default().build().execute_with(|| {
+		IdleScheduler::on_initialize(0);
+		assert_ok!(IdleScheduler::schedule_task(
+			Origin::root(),
+			ScheduledTasks::BalancesTask(BalancesTask::OnIdle)
+		));
+		// simulate relay block number jumping 10 blocks
+		sp_io::storage::set(&RELAY_BLOCK_KEY, &10_u32.encode());
+		assert_eq!(IdleScheduler::on_idle(System::block_number(), u64::MAX), u64::MAX);
+
+		System::set_block_number(1);
+		IdleScheduler::on_initialize(1);
+		// On_initialize is called it will execute, as now relay block number is the same
+		assert_eq!(
+			IdleScheduler::on_idle(System::block_number(), u64::MAX),
+			BASE_WEIGHT + <()>::on_idle_base() + <()>::clear_tasks()
+		);
+		assert!(!PreviousRelayBlockNumber::<Runtime>::exists());
+	});
+}