From 09a16a30a62c51aad43e3935822b53d5528f494e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andr=C3=A9=20Silva?=
 <123550+andresilva@users.noreply.github.com>
Date: Mon, 18 May 2020 12:53:33 +0100
Subject: [PATCH] grandpa: minor cleanup for gossip round start instant (#5976)

* grandpa: move gossip round start instant to LocalView

* grandpa: round duration is 2 gossip periods
---
 .../src/communication/gossip.rs               | 54 +++++++++++++------
 1 file changed, 38 insertions(+), 16 deletions(-)

diff --git a/substrate/client/finality-grandpa/src/communication/gossip.rs b/substrate/client/finality-grandpa/src/communication/gossip.rs
index 7fe17e974bb..c96301ede8f 100644
--- a/substrate/client/finality-grandpa/src/communication/gossip.rs
+++ b/substrate/client/finality-grandpa/src/communication/gossip.rs
@@ -110,7 +110,7 @@ const CATCH_UP_THRESHOLD: u64 = 2;
 const PROPAGATION_ALL: u32 = 4; //in rounds;
 const PROPAGATION_ALL_AUTHORITIES: u32 = 2; //in rounds;
 const PROPAGATION_SOME_NON_AUTHORITIES: u32 = 3; //in rounds;
-const ROUND_DURATION: u32 = 4; // measured in gossip durations
+const ROUND_DURATION: u32 = 2; // measured in gossip durations
 
 const MIN_LUCKY: usize = 5;
 
@@ -181,15 +181,27 @@ impl<N: Ord> View<N> {
 	}
 }
 
-/// A local view of protocol state. Only differs from `View` in that we also
-/// track the round and set id at which the last commit was observed.
+/// A local view of protocol state. Similar to `View` but we additionally track
+/// the round and set id at which the last commit was observed, and the instant
+/// at which the current round started.
 struct LocalView<N> {
 	round: Round,
 	set_id: SetId,
 	last_commit: Option<(N, Round, SetId)>,
+	round_start: Instant,
 }
 
 impl<N> LocalView<N> {
+	/// Creates a new `LocalView` at the given set id and round.
+	fn new(set_id: SetId, round: Round) -> LocalView<N> {
+		LocalView {
+			set_id,
+			round,
+			last_commit: None,
+			round_start: Instant::now(),
+		}
+	}
+
 	/// Converts the local view to a `View` discarding round and set id
 	/// information about the last commit.
 	fn as_view(&self) -> View<&N> {
@@ -205,9 +217,16 @@ impl<N> LocalView<N> {
 		if set_id != self.set_id {
 			self.set_id = set_id;
 			self.round = Round(1);
+ 			self.round_start = Instant::now();
 		}
 	}
 
+	/// Updates the current round.
+	fn update_round(&mut self, round: Round) {
+		self.round = round;
+		self.round_start = Instant::now();
+	}
+
 	/// Returns the height of the block that the last observed commit finalizes.
 	fn last_commit_height(&self) -> Option<&N> {
 		self.last_commit.as_ref().map(|(number, _, _)| number)
@@ -656,7 +675,6 @@ struct Inner<Block: BlockT> {
 	local_view: Option<LocalView<NumberFor<Block>>>,
 	peers: Peers<NumberFor<Block>>,
 	live_topics: KeepTopics<Block>,
-	round_start: Instant,
 	authorities: Vec<AuthorityId>,
 	config: crate::Config,
 	next_rebroadcast: Instant,
@@ -689,7 +707,6 @@ impl<Block: BlockT> Inner<Block> {
 			local_view: None,
 			peers: Peers::default(),
 			live_topics: KeepTopics::new(),
-			round_start: Instant::now(),
 			next_rebroadcast: Instant::now() + REBROADCAST_AFTER,
 			authorities: Vec::new(),
 			pending_catch_up: PendingCatchUp::None,
@@ -715,10 +732,9 @@ impl<Block: BlockT> Inner<Block> {
 			debug!(target: "afg", "Voter {} noting beginning of round {:?} to network.",
 				self.config.name(), (round, set_id));
 
-			local_view.round = round;
+			local_view.update_round(round);
 
 			self.live_topics.push(round, set_id);
-			self.round_start = Instant::now();
 			self.peers.reshuffle();
 		}
 		self.multicast_neighbor_packet()
@@ -729,11 +745,10 @@ impl<Block: BlockT> Inner<Block> {
 	fn note_set(&mut self, set_id: SetId, authorities: Vec<AuthorityId>) -> MaybeMessage<Block> {
 		{
 			let local_view = match self.local_view {
-				ref mut x @ None => x.get_or_insert(LocalView {
-					round: Round(1),
+ 				ref mut x @ None => x.get_or_insert(LocalView::new(
 					set_id,
-					last_commit: None,
-				}),
+					Round(1),
+				)),
 				Some(ref mut v) => if v.set_id == set_id {
 					if self.authorities != authorities {
 						debug!(target: "afg",
@@ -1121,8 +1136,10 @@ impl<Block: BlockT> Inner<Block> {
 	/// underlying gossip layer, which should happen every 30 seconds.
 	fn round_message_allowed<N>(&self, who: &PeerId, peer: &PeerInfo<N>) -> bool {
 		let round_duration = self.config.gossip_duration * ROUND_DURATION;
-		let round_elapsed = self.round_start.elapsed();
-
+		let round_elapsed = match self.local_view {
+			Some(ref local_view) => local_view.round_start.elapsed(),
+			None => return false,
+		};
 
 		if !self.config.is_authority
 			&& round_elapsed < round_duration * PROPAGATION_ALL
@@ -1185,7 +1202,10 @@ impl<Block: BlockT> Inner<Block> {
 	/// underlying gossip layer, which should happen every 30 seconds.
 	fn global_message_allowed<N>(&self, who: &PeerId, peer: &PeerInfo<N>) -> bool {
 		let round_duration = self.config.gossip_duration * ROUND_DURATION;
-		let round_elapsed = self.round_start.elapsed();
+		let round_elapsed = match self.local_view {
+			Some(ref local_view) => local_view.round_start.elapsed(),
+			None => return false,
+		};
 
 		match peer.roles {
 			ObservedRole::OurSentry | ObservedRole::OurGuardedAuthority => true,
@@ -2320,7 +2340,8 @@ mod tests {
 
 		let test = |num_round, peers| {
 			// rewind n round durations
-			val.inner.write().round_start = Instant::now() - round_duration * num_round;
+			val.inner.write().local_view.as_mut().unwrap().round_start =
+				Instant::now() - round_duration * num_round;
 			let mut message_allowed = val.message_allowed();
 
 			move || {
@@ -2448,7 +2469,8 @@ mod tests {
 		}
 
 		{
-			val.inner.write().round_start = Instant::now() - round_duration * 4;
+			val.inner.write().local_view.as_mut().unwrap().round_start =
+				Instant::now() - round_duration * 4;
 			let mut message_allowed = val.message_allowed();
 			// on the fourth round duration we should allow messages to authorities
 			// (on the second we would do `sqrt(authorities)`)
-- 
GitLab