im-online: send heartbeats at a random period (#8819)

* im-online: send heartbeats at a random period

* support: use permill to represent session progress

* im-online: increase probability of heartbeating with session progress

* babe, session: fix tests

* babe: fix test

substrate/frame/im-online/src/lib.rs

@@ -81,8 +81,8 @@ use sp_std::prelude::*;
 use sp_std::convert::TryInto;
 use sp_runtime::{
 	offchain::storage::StorageValueRef,
-	traits::{AtLeast32BitUnsigned, Convert, Saturating},
-	Perbill, Percent, RuntimeDebug,
+	traits::{AtLeast32BitUnsigned, Convert, Saturating, TrailingZeroInput},
+	Perbill, Permill, PerThing, RuntimeDebug, SaturatedConversion,
 };
 use sp_staking::{
 	SessionIndex,
@@ -571,23 +571,46 @@ impl<T: Config> Pallet<T> {
 	pub(crate) fn send_heartbeats(
 		block_number: T::BlockNumber,
 	) -> OffchainResult<T, impl Iterator<Item = OffchainResult<T, ()>>> {
-		const HALF_SESSION: Percent = Percent::from_percent(50);
+		const START_HEARTBEAT_RANDOM_PERIOD: Permill = Permill::from_percent(10);
+		const START_HEARTBEAT_FINAL_PERIOD: Permill = Permill::from_percent(80);
 
-		let too_early = if let (Some(progress), _) =
+		// this should give us a residual probability of 1/SESSION_LENGTH of sending an heartbeat,
+		// i.e. all heartbeats spread uniformly, over most of the session. as the session progresses
+		// the probability of sending an heartbeat starts to increase exponentially.
+		let random_choice = |progress: Permill| {
+			// given session progress `p` and session length `l`
+			// the threshold formula is: p^6 + 1/l
+			let session_length = T::NextSessionRotation::average_session_length();
+			let residual = Permill::from_rational(1u32, session_length.saturated_into());
+			let threshold: Permill = progress.saturating_pow(6).saturating_add(residual);
+
+			let seed = sp_io::offchain::random_seed();
+			let random = <u32>::decode(&mut TrailingZeroInput::new(seed.as_ref()))
+				.expect("input is padded with zeroes; qed");
+			let random = Permill::from_parts(random % Permill::ACCURACY);
+
+			random <= threshold
+		};
+
+		let should_heartbeat = if let (Some(progress), _) =
 			T::NextSessionRotation::estimate_current_session_progress(block_number)
 		{
-			// we try to get an estimate of the current session progress first since it
-			// should provide more accurate results and send the heartbeat if we're halfway
-			// through the session.
-			progress < HALF_SESSION
+			// we try to get an estimate of the current session progress first since it should
+			// provide more accurate results. we will start an early heartbeat period where we'll
+			// randomly pick whether to heartbeat. after 80% of the session has elapsed, if we
+			// haven't sent an heartbeat yet we'll send one unconditionally. the idea is to prevent
+			// all nodes from sending the heartbeats at the same block and causing a temporary (but
+			// deterministic) spike in transactions.
+			progress >= START_HEARTBEAT_FINAL_PERIOD
+				|| progress >= START_HEARTBEAT_RANDOM_PERIOD && random_choice(progress)
 		} else {
 			// otherwise we fallback to using the block number calculated at the beginning
 			// of the session that should roughly correspond to the middle of the session
 			let heartbeat_after = <HeartbeatAfter<T>>::get();
-			block_number < heartbeat_after
+			block_number >= heartbeat_after
 		};
 
-		if too_early {
+		if !should_heartbeat {
 			return Err(OffchainErr::TooEarly);
 		}
 
@@ -607,7 +630,6 @@ impl<T: Config> Pallet<T> {
 		)
 	}
 
-
 	fn send_single_heartbeat(
 		authority_index: u32,
 		key: T::AuthorityId,

substrate/frame/im-online/src/mock.rs

@@ -26,7 +26,7 @@ use pallet_session::historical as pallet_session_historical;
 use sp_core::H256;
 use sp_runtime::testing::{Header, TestXt, UintAuthorityId};
 use sp_runtime::traits::{BlakeTwo256, ConvertInto, IdentityLookup};
-use sp_runtime::{Perbill, Percent};
+use sp_runtime::{Perbill, Permill};
 use sp_staking::{
 	offence::{OffenceError, ReportOffence},
 	SessionIndex,
@@ -182,7 +182,7 @@ impl pallet_authorship::Config for Runtime {
 }
 
 thread_local! {
-	pub static MOCK_CURRENT_SESSION_PROGRESS: RefCell<Option<Option<Percent>>> = RefCell::new(None);
+	pub static MOCK_CURRENT_SESSION_PROGRESS: RefCell<Option<Option<Permill>>> = RefCell::new(None);
 }
 
 thread_local! {
@@ -199,7 +199,7 @@ impl frame_support::traits::EstimateNextSessionRotation<u64> for TestNextSession
 		mock.unwrap_or(pallet_session::PeriodicSessions::<Period, Offset>::average_session_length())
 	}
 
-	fn estimate_current_session_progress(now: u64) -> (Option<Percent>, Weight) {
+	fn estimate_current_session_progress(now: u64) -> (Option<Permill>, Weight) {
 		let (estimate, weight) =
 			pallet_session::PeriodicSessions::<Period, Offset>::estimate_current_session_progress(
 				now,

substrate/frame/im-online/src/tests.rs

@@ -433,10 +433,92 @@ fn should_handle_non_linear_session_progress() {
 		assert!(ImOnline::send_heartbeats(5).ok().is_some());
 
 		// if we have a valid current session progress then we'll heartbeat as soon
-		// as we're past 50% of the session regardless of the block number
+		// as we're past 80% of the session regardless of the block number
 		MOCK_CURRENT_SESSION_PROGRESS
-			.with(|p| *p.borrow_mut() = Some(Some(Percent::from_percent(51))));
+			.with(|p| *p.borrow_mut() = Some(Some(Permill::from_percent(81))));
 
 		assert!(ImOnline::send_heartbeats(2).ok().is_some());
 	});
 }
+
+#[test]
+fn test_does_not_heartbeat_early_in_the_session() {
+	let mut ext = new_test_ext();
+	let (offchain, _state) = TestOffchainExt::new();
+	let (pool, _) = TestTransactionPoolExt::new();
+	ext.register_extension(OffchainDbExt::new(offchain.clone()));
+	ext.register_extension(OffchainWorkerExt::new(offchain));
+	ext.register_extension(TransactionPoolExt::new(pool));
+
+	ext.execute_with(|| {
+		// mock current session progress as being 5%. we only randomly start
+		// heartbeating after 10% of the session has elapsed.
+		MOCK_CURRENT_SESSION_PROGRESS.with(|p| *p.borrow_mut() = Some(Some(Permill::from_float(0.05))));
+		assert_eq!(
+			ImOnline::send_heartbeats(2).err(),
+			Some(OffchainErr::TooEarly),
+		);
+	});
+}
+
+#[test]
+fn test_probability_of_heartbeating_increases_with_session_progress() {
+	let mut ext = new_test_ext();
+	let (offchain, state) = TestOffchainExt::new();
+	let (pool, _) = TestTransactionPoolExt::new();
+	ext.register_extension(OffchainDbExt::new(offchain.clone()));
+	ext.register_extension(OffchainWorkerExt::new(offchain));
+	ext.register_extension(TransactionPoolExt::new(pool));
+
+	ext.execute_with(|| {
+		let set_test = |progress, random: f64| {
+			// the average session length is 100 blocks, therefore the residual
+			// probability of sending a heartbeat is 1%
+			MOCK_AVERAGE_SESSION_LENGTH.with(|p| *p.borrow_mut() = Some(100));
+			MOCK_CURRENT_SESSION_PROGRESS.with(|p| *p.borrow_mut() =
+				Some(Some(Permill::from_float(progress))));
+
+			let mut seed = [0u8; 32];
+			let encoded = ((random * Permill::ACCURACY as f64) as u32).encode();
+			seed[0..4].copy_from_slice(&encoded);
+			state.write().seed = seed;
+		};
+
+		let assert_too_early = |progress, random| {
+			set_test(progress, random);
+			assert_eq!(
+				ImOnline::send_heartbeats(2).err(),
+				Some(OffchainErr::TooEarly),
+			);
+		};
+
+		let assert_heartbeat_ok = |progress, random| {
+			set_test(progress, random);
+			assert!(ImOnline::send_heartbeats(2).ok().is_some());
+		};
+
+		assert_too_early(0.05, 1.0);
+
+		assert_too_early(0.1, 0.1);
+		assert_too_early(0.1, 0.011);
+		assert_heartbeat_ok(0.1, 0.010);
+
+		assert_too_early(0.4, 0.015);
+		assert_heartbeat_ok(0.4, 0.014);
+
+		assert_too_early(0.5, 0.026);
+		assert_heartbeat_ok(0.5, 0.025);
+
+		assert_too_early(0.6, 0.057);
+		assert_heartbeat_ok(0.6, 0.056);
+
+		assert_too_early(0.65, 0.086);
+		assert_heartbeat_ok(0.65, 0.085);
+
+		assert_too_early(0.7, 0.13);
+		assert_heartbeat_ok(0.7, 0.12);
+
+		assert_too_early(0.75, 0.19);
+		assert_heartbeat_ok(0.75, 0.18);
+	});
+}