im-online: send heartbeats at a random period (paritytech#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

frame/babe/src/lib.rs

@@ -31,7 +31,7 @@ use sp_application_crypto::Public;
 use sp_runtime::{
 	generic::DigestItem,
 	traits::{IsMember, One, SaturatedConversion, Saturating, Zero},
-	ConsensusEngineId, KeyTypeId, Percent,
+	ConsensusEngineId, KeyTypeId, Permill,
 };
 use sp_session::{GetSessionNumber, GetValidatorCount};
 use sp_std::prelude::*;
@@ -848,11 +848,11 @@ impl<T: Config> frame_support::traits::EstimateNextSessionRotation<T::BlockNumbe
 		T::EpochDuration::get().saturated_into()
 	}
 
-	fn estimate_current_session_progress(_now: T::BlockNumber) -> (Option<Percent>, Weight) {
+	fn estimate_current_session_progress(_now: T::BlockNumber) -> (Option<Permill>, Weight) {
 		let elapsed = CurrentSlot::<T>::get().saturating_sub(Self::current_epoch_start()) + 1;
 
 		(
-			Some(Percent::from_rational(
+			Some(Permill::from_rational(
 				*elapsed,
 				T::EpochDuration::get(),
 			)),

frame/babe/src/tests.rs

@@ -236,18 +236,18 @@ fn can_estimate_current_epoch_progress() {
 			if Babe::estimate_next_session_rotation(i).0.unwrap() - 1 == i {
 				assert_eq!(
 					Babe::estimate_current_session_progress(i).0.unwrap(),
-					Percent::from_percent(100)
+					Permill::from_percent(100)
 				);
 			} else {
-				assert!(Babe::estimate_current_session_progress(i).0.unwrap() < Percent::from_percent(100));
+				assert!(Babe::estimate_current_session_progress(i).0.unwrap() < Permill::from_percent(100));
 			}
 		}
 
 		// the first block of the new epoch counts towards the epoch progress as well
 		progress_to_block(4);
 		assert_eq!(
 			Babe::estimate_current_session_progress(4).0.unwrap(),
-			Percent::from_percent(33),
+			Permill::from_float(1.0 / 3.0),
 		);
 	})
 }

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);
+
+		// 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 too_early = if let (Some(progress), _) =
+		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,

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,

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);
+	});
+}

frame/session/src/lib.rs

@@ -118,7 +118,7 @@ use sp_std::{prelude::*, marker::PhantomData, ops::{Sub, Rem}};
 use codec::Decode;
 use sp_runtime::{
 	traits::{AtLeast32BitUnsigned, Convert, Member, One, OpaqueKeys, Zero},
-	KeyTypeId, Perbill, Percent, RuntimeAppPublic,
+	KeyTypeId, Perbill, Permill, RuntimeAppPublic,
 };
 use sp_staking::SessionIndex;
 use frame_support::{
@@ -168,7 +168,7 @@ impl<
 		Period::get()
 	}
 
-	fn estimate_current_session_progress(now: BlockNumber) -> (Option<Percent>, Weight) {
+	fn estimate_current_session_progress(now: BlockNumber) -> (Option<Permill>, Weight) {
 		let offset = Offset::get();
 		let period = Period::get();
 
@@ -177,12 +177,12 @@ impl<
 		// (0% is never returned).
 		let progress = if now >= offset {
 			let current = (now - offset) % period.clone() + One::one();
-			Some(Percent::from_rational(
+			Some(Permill::from_rational(
 				current.clone(),
 				period.clone(),
 			))
 		} else {
-			Some(Percent::from_rational(
+			Some(Permill::from_rational(
 				now + One::one(),
 				offset,
 			))

frame/session/src/tests.rs

@@ -274,11 +274,11 @@ fn periodic_session_works() {
 		if P::estimate_next_session_rotation(i).0.unwrap() - 1 == i {
 			assert_eq!(
 				P::estimate_current_session_progress(i).0.unwrap(),
-				Percent::from_percent(100)
+				Permill::from_percent(100)
 			);
 		} else {
 			assert!(
-				P::estimate_current_session_progress(i).0.unwrap() < Percent::from_percent(100)
+				P::estimate_current_session_progress(i).0.unwrap() < Permill::from_percent(100)
 			);
 		}
 	}
@@ -290,7 +290,7 @@ fn periodic_session_works() {
 	assert_eq!(P::estimate_next_session_rotation(3u64).0.unwrap(), 3);
 	assert_eq!(
 		P::estimate_current_session_progress(3u64).0.unwrap(),
-		Percent::from_percent(10),
+		Permill::from_percent(10),
 	);
 
 	for i in (1u64..10).map(|i| 3 + i) {
@@ -302,11 +302,11 @@ fn periodic_session_works() {
 		if P::estimate_next_session_rotation(i).0.unwrap() - 1 == i {
 			assert_eq!(
 				P::estimate_current_session_progress(i).0.unwrap(),
-				Percent::from_percent(100)
+				Permill::from_percent(100)
 			);
 		} else {
 			assert!(
-				P::estimate_current_session_progress(i).0.unwrap() < Percent::from_percent(100)
+				P::estimate_current_session_progress(i).0.unwrap() < Permill::from_percent(100)
 			);
 		}
 	}
@@ -316,14 +316,14 @@ fn periodic_session_works() {
 	assert_eq!(P::estimate_next_session_rotation(13u64).0.unwrap(), 23);
 	assert_eq!(
 		P::estimate_current_session_progress(13u64).0.unwrap(),
-		Percent::from_percent(10)
+		Permill::from_percent(10)
 	);
 
 	assert!(!P::should_end_session(14u64));
 	assert_eq!(P::estimate_next_session_rotation(14u64).0.unwrap(), 23);
 	assert_eq!(
 		P::estimate_current_session_progress(14u64).0.unwrap(),
-		Percent::from_percent(20)
+		Permill::from_percent(20)
 	);
 }
 

frame/support/src/traits/validation.rs

@@ -20,7 +20,7 @@
 use sp_std::prelude::*;
 use codec::{Codec, Decode};
 use sp_runtime::traits::{Convert, Zero};
-use sp_runtime::{BoundToRuntimeAppPublic, ConsensusEngineId, Percent, RuntimeAppPublic};
+use sp_runtime::{BoundToRuntimeAppPublic, ConsensusEngineId, Permill, RuntimeAppPublic};
 use sp_staking::SessionIndex;
 use crate::dispatch::Parameter;
 use crate::weights::Weight;
@@ -126,7 +126,7 @@ pub trait EstimateNextSessionRotation<BlockNumber> {
 	/// Return an estimate of the current session progress.
 	///
 	/// None should be returned if the estimation fails to come to an answer.
-	fn estimate_current_session_progress(now: BlockNumber) -> (Option<Percent>, Weight);
+	fn estimate_current_session_progress(now: BlockNumber) -> (Option<Permill>, Weight);
 
 	/// Return the block number at which the next session rotation is estimated to happen.
 	///
@@ -139,7 +139,7 @@ impl<BlockNumber: Zero> EstimateNextSessionRotation<BlockNumber> for () {
 		Zero::zero()
 	}
 
-	fn estimate_current_session_progress(_: BlockNumber) -> (Option<Percent>, Weight) {
+	fn estimate_current_session_progress(_: BlockNumber) -> (Option<Permill>, Weight) {
 		(None, Zero::zero())
 	}