client/beefy: request justifs from peers further in consensus (#13343)

For on-demand justifications, peer selection is based on witnessed
gossip votes. This commit changes the condition for selecting a peer
to request justification for `block` from
"last voted on >= `block`" to "peer last voted on strict > `block`".

When allowing `>=` we see nodes continuously spamming unsuccessful
on-demand requests to nodes which are still voting on a block without
having a justification available.

One way to fix the spam would be to add some rate-limiting or backoff
period when requesting justifications.

The other solution (present in this commit) is to simply request
justifications from peers that are voting on future blocks so we know
they're _guaranteed_ to have the wanted mandatory justification
available to send back.

Signed-off-by: acatangiu <adrian@parity.io>

substrate/client/beefy/src/communication/peers.rs

@@ -57,11 +57,11 @@ impl<B: Block> KnownPeers<B> {
 		self.live.remove(peer);
 	}
 
-	/// Return _filtered and cloned_ list of peers that have voted on `block` or higher.
-	pub fn at_least_at_block(&self, block: NumberFor<B>) -> VecDeque<PeerId> {
+	/// Return _filtered and cloned_ list of peers that have voted on higher than `block`.
+	pub fn further_than(&self, block: NumberFor<B>) -> VecDeque<PeerId> {
 		self.live
 			.iter()
-			.filter_map(|(k, v)| (v.last_voted_on >= block).then_some(k))
+			.filter_map(|(k, v)| (v.last_voted_on > block).then_some(k))
 			.cloned()
 			.collect()
 	}
@@ -92,22 +92,22 @@ mod tests {
 		assert!(peers.contains(&bob));
 		assert!(peers.contains(&charlie));
 
-		// Get peers at block >= 5
-		let at_5 = peers.at_least_at_block(5);
+		// Get peers at block > 4
+		let further_than_4 = peers.further_than(4);
 		// Should be Bob and Charlie
-		assert_eq!(at_5.len(), 2);
-		assert!(at_5.contains(&bob));
-		assert!(at_5.contains(&charlie));
+		assert_eq!(further_than_4.len(), 2);
+		assert!(further_than_4.contains(&bob));
+		assert!(further_than_4.contains(&charlie));
 
 		// 'Tracked' Alice seen voting for 10.
 		peers.note_vote_for(alice, 10);
 
-		// Get peers at block >= 9
-		let at_9 = peers.at_least_at_block(9);
+		// Get peers at block > 9
+		let further_than_9 = peers.further_than(9);
 		// Should be Charlie and Alice
-		assert_eq!(at_9.len(), 2);
-		assert!(at_9.contains(&charlie));
-		assert!(at_9.contains(&alice));
+		assert_eq!(further_than_9.len(), 2);
+		assert!(further_than_9.contains(&charlie));
+		assert!(further_than_9.contains(&alice));
 
 		// Remove Alice
 		peers.remove(&alice);
@@ -115,9 +115,9 @@ mod tests {
 		assert!(!peers.contains(&alice));
 
 		// Get peers at block >= 9
-		let at_9 = peers.at_least_at_block(9);
+		let further_than_9 = peers.further_than(9);
 		// Now should be just Charlie
-		assert_eq!(at_9.len(), 1);
-		assert!(at_9.contains(&charlie));
+		assert_eq!(further_than_9.len(), 1);
+		assert!(further_than_9.contains(&charlie));
 	}
 }

substrate/client/beefy/src/communication/request_response/outgoing_requests_engine.rs

@@ -80,7 +80,7 @@ impl<B: Block> OnDemandJustificationsEngine<B> {
 
 	fn reset_peers_cache_for_block(&mut self, block: NumberFor<B>) {
 		// TODO (issue #12296): replace peer selection with generic one that involves all protocols.
-		self.peers_cache = self.live_peers.lock().at_least_at_block(block);
+		self.peers_cache = self.live_peers.lock().further_than(block);
 	}
 
 	fn try_next_peer(&mut self) -> Option<PeerId> {
@@ -199,7 +199,6 @@ impl<B: Block> OnDemandJustificationsEngine<B> {
 		let (peer, req_info, resp) = match &mut self.state {
 			State::Idle => {
 				futures::pending!();
-				// Doesn't happen as 'futures::pending!()' is an 'await' barrier that never passes.
 				return None
 			},
 			State::AwaitingResponse(peer, req_info, receiver) => {