Fix quantization in referenda alarm (paritytech#12815)

* Fix quantization in referenda alarm * Formatting * alarm interval, test (paritytech#12818) Co-authored-by: Muharem Ismailov <[email protected]>
agryaznov · Dec 2, 2022 · c17c7d8 · c17c7d8
1 parent 5ae8a3b
commit c17c7d8
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 18 deletions.
diff --git a/frame/referenda/src/lib.rs b/frame/referenda/src/lib.rs
@@ -755,8 +755,11 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		when: T::BlockNumber,
 	) -> Option<(T::BlockNumber, ScheduleAddressOf<T, I>)> {
 		let alarm_interval = T::AlarmInterval::get().max(One::one());
-		let when = when.saturating_add(alarm_interval).saturating_sub(One::one()) /
-			(alarm_interval.saturating_mul(alarm_interval)).max(One::one());
+		// Alarm must go off no earlier than `when`.
+		// This rounds `when` upwards to the next multiple of `alarm_interval`.
+		let when = (when.saturating_add(alarm_interval.saturating_sub(One::one())) /
+			alarm_interval)
+			.saturating_mul(alarm_interval);
 		let maybe_result = T::Scheduler::schedule(
 			DispatchTime::At(when),
 			None,
@@ -863,9 +866,6 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		// Set an alarm call for the next block to nudge the track along.
 		let now = frame_system::Pallet::<T>::block_number();
 		let next_block = now + One::one();
-		let alarm_interval = T::AlarmInterval::get().max(One::one());
-		let when = (next_block + alarm_interval - One::one()) / alarm_interval * alarm_interval;
-
 		let call = match T::Preimages::bound(CallOf::<T, I>::from(Call::one_fewer_deciding {
 			track,
 		})) {
@@ -875,19 +875,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 				return
 			},
 		};
-		let maybe_result = T::Scheduler::schedule(
-			DispatchTime::At(when),
-			None,
-			128u8,
-			frame_system::RawOrigin::Root.into(),
-			call,
-		);
-		debug_assert!(
-			maybe_result.is_ok(),
-			"Unable to schedule a new alarm at #{:?} (now: #{:?})?!",
-			when,
-			now
-		);
+		Self::set_alarm(call, next_block);
 	}
 
 	/// Ensure that a `service_referendum` alarm happens for the referendum `index` at `alarm`.

diff --git a/frame/referenda/src/tests.rs b/frame/referenda/src/tests.rs
@@ -265,6 +265,27 @@ fn queueing_works() {
 	});
 }
 
+#[test]
+fn alarm_interval_works() {
+	new_test_ext().execute_with(|| {
+		let call =
+			<Test as Config>::Preimages::bound(CallOf::<Test, ()>::from(Call::nudge_referendum {
+				index: 0,
+			}))
+			.unwrap();
+		for n in 0..10 {
+			let interval = n * n;
+			let now = 100 * (interval + 1);
+			System::set_block_number(now);
+			AlarmInterval::set(interval);
+			let when = now + 1;
+			let (actual, _) = Referenda::set_alarm(call.clone(), when).unwrap();
+			assert!(actual >= when);
+			assert!(actual - interval <= when);
+		}
+	});
+}
+
 #[test]
 fn auto_timeout_should_happen_with_nothing_but_submit() {
 	new_test_ext().execute_with(|| {