slots: fix slot lenience methods (#5742)

* slots: extract slot lenience from babe and aura

* slots: add tests for slot lenience

* slots: fix comment in test

substrate/client/consensus/aura/src/lib.rs

@@ -299,27 +299,28 @@ impl<B, C, E, I, P, Error, SO> sc_consensus_slots::SimpleSlotWorker<B> for AuraW
 	fn proposing_remaining_duration(
 		&self,
 		head: &B::Header,
-		slot_info: &SlotInfo
+		slot_info: &SlotInfo,
 	) -> Option<std::time::Duration> {
-		// never give more than 20 times more lenience.
-		const BACKOFF_CAP: u64 = 20;
-
 		let slot_remaining = self.slot_remaining_duration(slot_info);
+
 		let parent_slot = match find_pre_digest::<B, P>(head) {
 			Err(_) => return Some(slot_remaining),
 			Ok(d) => d,
 		};
 
-		// we allow a lenience of the number of slots since the head of the
+		if let Some(slot_lenience) =
-		// chain was produced, minus 1 (since there is always a difference of at least 1)
+			sc_consensus_slots::slot_lenience_exponential(parent_slot, slot_info)
-		//
+		{
-		// linear back-off.
+			debug!(target: "aura",
-		// in normal cases we only attempt to issue blocks up to the end of the slot.
+				"No block for {} slots. Applying linear lenience of {}s",
-		// when the chain has been stalled for a few slots, we give more lenience.
+				slot_info.number.saturating_sub(parent_slot + 1),
-		let slot_lenience = slot_info.number.saturating_sub(parent_slot + 1);
+				slot_lenience.as_secs(),
-		let slot_lenience = std::cmp::min(slot_lenience, BACKOFF_CAP);
+			);
-		let slot_lenience = Duration::from_secs(slot_lenience * slot_info.duration);
+
-		Some(slot_lenience + slot_remaining)
+			Some(slot_remaining + slot_lenience)
+		} else {
+			Some(slot_remaining)
+		}
 	}
 }
 

substrate/client/consensus/babe/src/lib.rs

@@ -510,38 +510,28 @@ impl<B, C, E, I, Error, SO> sc_consensus_slots::SimpleSlotWorker<B> for BabeWork
 	fn proposing_remaining_duration(
 		&self,
 		head: &B::Header,
-		slot_info: &SlotInfo
+		slot_info: &SlotInfo,
 	) -> Option<std::time::Duration> {
-		// never give more than 2^this times the lenience.
-		const BACKOFF_CAP: u64 = 8;
-
-		// how many slots it takes before we double the lenience.
-		const BACKOFF_STEP: u64 = 2;
-
 		let slot_remaining = self.slot_remaining_duration(slot_info);
+
 		let parent_slot = match find_pre_digest::<B>(head) {
 			Err(_) => return Some(slot_remaining),
 			Ok(d) => d.slot_number(),
 		};
 
-		// we allow a lenience of the number of slots since the head of the
+		if let Some(slot_lenience) =
-		// chain was produced, minus 1 (since there is always a difference of at least 1)
+			sc_consensus_slots::slot_lenience_exponential(parent_slot, slot_info)
-		//
+		{
-		// exponential back-off.
+			debug!(target: "babe",
-		// in normal cases we only attempt to issue blocks up to the end of the slot.
+				"No block for {} slots. Applying exponential lenience of {}s",
-		// when the chain has been stalled for a few slots, we give more lenience.
+				slot_info.number.saturating_sub(parent_slot + 1),
-		let slot_lenience = slot_info.number.saturating_sub(parent_slot + 1);
+				slot_lenience.as_secs(),
+			);
 
-		let slot_lenience = std::cmp::min(slot_lenience, BACKOFF_CAP);
+			Some(slot_remaining + slot_lenience)
-		let slot_duration = slot_info.duration << (slot_lenience / BACKOFF_STEP);
+		} else {
-
+			Some(slot_remaining)
-		if slot_lenience >= 1 {
-			debug!(target: "babe", "No block for {} slots. Applying 2^({}/{}) lenience",
-				slot_lenience, slot_lenience, BACKOFF_STEP);
 		}
-
-		let slot_lenience = Duration::from_secs(slot_duration);
-		Some(slot_lenience + slot_remaining)
 	}
 }
 

substrate/client/consensus/slots/src/lib.rs

@@ -483,3 +483,120 @@ impl<T: Clone> SlotDuration<T> {
 		self.0.clone()
 	}
 }
+
+/// Calculate a slot duration lenience based on the number of missed slots from current
+/// to parent. If the number of skipped slots is greated than 0 this method will apply
+/// an exponential backoff of at most `2^7 * slot_duration`, if no slots were skipped
+/// this method will return `None.`
+pub fn slot_lenience_exponential(parent_slot: u64, slot_info: &SlotInfo) -> Option<Duration> {
+	// never give more than 2^this times the lenience.
+	const BACKOFF_CAP: u64 = 7;
+
+	// how many slots it takes before we double the lenience.
+	const BACKOFF_STEP: u64 = 2;
+
+	// we allow a lenience of the number of slots since the head of the
+	// chain was produced, minus 1 (since there is always a difference of at least 1)
+	//
+	// exponential back-off.
+	// in normal cases we only attempt to issue blocks up to the end of the slot.
+	// when the chain has been stalled for a few slots, we give more lenience.
+	let skipped_slots = slot_info.number.saturating_sub(parent_slot + 1);
+
+	if skipped_slots == 0 {
+		None
+	} else {
+		let slot_lenience = skipped_slots / BACKOFF_STEP;
+		let slot_lenience = std::cmp::min(slot_lenience, BACKOFF_CAP);
+		let slot_lenience = 1 << slot_lenience;
+		Some(Duration::from_millis(slot_lenience * slot_info.duration))
+	}
+}
+
+/// Calculate a slot duration lenience based on the number of missed slots from current
+/// to parent. If the number of skipped slots is greated than 0 this method will apply
+/// a linear backoff of at most `20 * slot_duration`, if no slots were skipped
+/// this method will return `None.`
+pub fn slot_lenience_linear(parent_slot: u64, slot_info: &SlotInfo) -> Option<Duration> {
+	// never give more than 20 times more lenience.
+	const BACKOFF_CAP: u64 = 20;
+
+	// we allow a lenience of the number of slots since the head of the
+	// chain was produced, minus 1 (since there is always a difference of at least 1)
+	//
+	// linear back-off.
+	// in normal cases we only attempt to issue blocks up to the end of the slot.
+	// when the chain has been stalled for a few slots, we give more lenience.
+	let skipped_slots = slot_info.number.saturating_sub(parent_slot + 1);
+
+	if skipped_slots == 0 {
+		None
+	} else {
+		let slot_lenience = std::cmp::min(skipped_slots, BACKOFF_CAP);
+		Some(Duration::from_millis(slot_lenience * slot_info.duration))
+	}
+}
+
+#[cfg(test)]
+mod test {
+	use std::time::{Duration, Instant};
+
+	const SLOT_DURATION: Duration = Duration::from_millis(6000);
+
+	fn slot(n: u64) -> super::slots::SlotInfo {
+		super::slots::SlotInfo {
+			number: n,
+			last_number: n - 1,
+			duration: SLOT_DURATION.as_millis() as u64,
+			timestamp: Default::default(),
+			inherent_data: Default::default(),
+			ends_at: Instant::now(),
+		}
+	}
+
+	#[test]
+	fn linear_slot_lenience() {
+		// if no slots are skipped there should be no lenience
+		assert_eq!(super::slot_lenience_linear(1, &slot(2)), None);
+
+		// otherwise the lenience is incremented linearly with
+		// the number of skipped slots.
+		for n in 3..=22 {
+			assert_eq!(
+				super::slot_lenience_linear(1, &slot(n)),
+				Some(SLOT_DURATION * (n - 2) as u32),
+			);
+		}
+
+		// but we cap it to a maximum of 20 slots
+		assert_eq!(
+			super::slot_lenience_linear(1, &slot(23)),
+			Some(SLOT_DURATION * 20),
+		);
+	}
+
+	#[test]
+	fn exponential_slot_lenience() {
+		// if no slots are skipped there should be no lenience
+		assert_eq!(super::slot_lenience_exponential(1, &slot(2)), None);
+
+		// otherwise the lenience is incremented exponentially every two slots
+		for n in 3..=17 {
+			assert_eq!(
+				super::slot_lenience_exponential(1, &slot(n)),
+				Some(SLOT_DURATION * 2u32.pow((n / 2 - 1) as u32)),
+			);
+		}
+
+		// but we cap it to a maximum of 14 slots
+		assert_eq!(
+			super::slot_lenience_exponential(1, &slot(18)),
+			Some(SLOT_DURATION * 2u32.pow(7)),
+		);
+
+		assert_eq!(
+			super::slot_lenience_exponential(1, &slot(19)),
+			Some(SLOT_DURATION * 2u32.pow(7)),
+		);
+	}
+}