Implement Lean BEEFY (#10882)

Simplified BEEFY worker logic based on the invariant that GRANDPA will always finalize 1st block of each new session, meaning BEEFY worker is guaranteed to receive finality notification for the BEEFY mandatory blocks. Under these conditions the current design is as follows: - session changes are detected based on BEEFY Digest present in BEEFY mandatory blocks, - on each new session new `Rounds` of voting is created, with old rounds being dropped (for gossip rounds, last 3 are still alive so votes are still being gossiped), - after processing finality for a block, the worker votes if a new voting target has become available as a result of said block finality processing, - incoming votes as well as self-created votes are processed and signed commitments are created for completed BEEFY voting rounds, - the worker votes if a new voting target becomes available once a round successfully completes. On worker startup, the current validator set is retrieved from the BEEFY pallet. If it is the genesis validator set, worker starts voting right away considering Block #1 as session start. Otherwise (not genesis), the worker will vote starting with mandatory block of the next session. Later on when we add the BEEFY initial-sync (catch-up) logic, the worker will sync all past mandatory blocks Signed Commitments and will be able to start voting right away. BEEFY mandatory block is the block with header containing the BEEFY `AuthoritiesChange` Digest, this block is guaranteed to be finalized by GRANDPA. This session-boundary block is signed by the ending-session's validator set. Next blocks will be signed by the new session's validator set. This behavior is consistent with what GRANDPA does as well. Also drop the limit N on active gossip rounds. In an adversarial network, a bad actor could create and gossip N invalid votes with round numbers larger than the current correct round number. This would lead to votes for correct rounds to no longer be gossiped. Add unit-tests for all components, including full voter consensus tests. Signed-off-by: Adrian Catangiu <adrian@parity.io> Co-authored-by: Tomasz Drwięga <tomusdrw@users.noreply.github.com> Co-authored-by: David Salami <Wizdave97>
2026-07-18 21:05:40 +00:00 · 2022-03-25 17:31:42 +02:00
parent 52967f17de
commit 498e8c181f
12 changed files with 1812 additions and 565 deletions
@@ -35,9 +35,6 @@ use beefy_primitives::{

 use crate::keystore::BeefyKeystore;

-// Limit BEEFY gossip by keeping only a bound number of voting rounds alive.
-const MAX_LIVE_GOSSIP_ROUNDS: usize = 3;
-
 // Timeout for rebroadcasting messages.
 const REBROADCAST_AFTER: Duration = Duration::from_secs(60 * 5);

@@ -52,13 +49,50 @@ where
 /// A type that represents hash of the message.
 pub type MessageHash = [u8; 8];

-type KnownVotes<B> = BTreeMap<NumberFor<B>, fnv::FnvHashSet<MessageHash>>;
+struct KnownVotes<B: Block> {
+	last_done: Option<NumberFor<B>>,
+	live: BTreeMap<NumberFor<B>, fnv::FnvHashSet<MessageHash>>,
+}
+
+impl<B: Block> KnownVotes<B> {
+	pub fn new() -> Self {
+		Self { last_done: None, live: BTreeMap::new() }
+	}
+
+	/// Create new round votes set if not already present.
+	fn insert(&mut self, round: NumberFor<B>) {
+		self.live.entry(round).or_default();
+	}
+
+	/// Remove `round` and older from live set, update `last_done` accordingly.
+	fn conclude(&mut self, round: NumberFor<B>) {
+		self.live.retain(|&number, _| number > round);
+		self.last_done = self.last_done.max(Some(round));
+	}
+
+	/// Return true if `round` is newer than previously concluded rounds.
+	///
+	/// Latest concluded round is still considered alive to allow proper gossiping for it.
+	fn is_live(&self, round: &NumberFor<B>) -> bool {
+		Some(*round) >= self.last_done
+	}
+
+	/// Add new _known_ `hash` to the round's known votes.
+	fn add_known(&mut self, round: &NumberFor<B>, hash: MessageHash) {
+		self.live.get_mut(round).map(|known| known.insert(hash));
+	}
+
+	/// Check if `hash` is already part of round's known votes.
+	fn is_known(&self, round: &NumberFor<B>, hash: &MessageHash) -> bool {
+		self.live.get(round).map(|known| known.contains(hash)).unwrap_or(false)
+	}
+}

 /// BEEFY gossip validator
 ///
 /// Validate BEEFY gossip messages and limit the number of live BEEFY voting rounds.
 ///
-/// Allows messages from last [`MAX_LIVE_GOSSIP_ROUNDS`] to flow, everything else gets
+/// Allows messages for 'rounds >= last concluded' to flow, everything else gets
 /// rejected/expired.
 ///
 ///All messaging is handled in a single BEEFY global topic.
@@ -78,57 +112,25 @@ where
 	pub fn new() -> GossipValidator<B> {
 		GossipValidator {
 			topic: topic::<B>(),
-			known_votes: RwLock::new(BTreeMap::new()),
+			known_votes: RwLock::new(KnownVotes::new()),
 			next_rebroadcast: Mutex::new(Instant::now() + REBROADCAST_AFTER),
 		}
 	}

 	/// Note a voting round.
 	///
-	/// Noting `round` will keep `round` live.
-	///
-	/// We retain the [`MAX_LIVE_GOSSIP_ROUNDS`] most **recent** voting rounds as live.
-	/// As long as a voting round is live, it will be gossiped to peer nodes.
+	/// Noting round will start a live `round`.
 	pub(crate) fn note_round(&self, round: NumberFor<B>) {
-		debug!(target: "beefy", "🥩 About to note round #{}", round);
-
-		let mut live = self.known_votes.write();
-
-		if !live.contains_key(&round) {
-			live.insert(round, Default::default());
-		}
-
-		if live.len() > MAX_LIVE_GOSSIP_ROUNDS {
-			let to_remove = live.iter().next().map(|x| x.0).copied();
-			if let Some(first) = to_remove {
-				live.remove(&first);
-			}
-		}
+		debug!(target: "beefy", "🥩 About to note gossip round #{}", round);
+		self.known_votes.write().insert(round);
 	}

-	fn add_known(known_votes: &mut KnownVotes<B>, round: &NumberFor<B>, hash: MessageHash) {
-		known_votes.get_mut(round).map(|known| known.insert(hash));
-	}
-
-	// Note that we will always keep the most recent unseen round alive.
-	//
-	// This is a preliminary fix and the detailed description why we are
-	// doing this can be found as part of the issue below
-	//
-	// https://github.com/paritytech/grandpa-bridge-gadget/issues/237
-	//
-	fn is_live(known_votes: &KnownVotes<B>, round: &NumberFor<B>) -> bool {
-		let unseen_round = if let Some(max_known_round) = known_votes.keys().last() {
-			round > max_known_round
-		} else {
-			known_votes.is_empty()
-		};
-
-		known_votes.contains_key(round) || unseen_round
-	}
-
-	fn is_known(known_votes: &KnownVotes<B>, round: &NumberFor<B>, hash: &MessageHash) -> bool {
-		known_votes.get(round).map(|known| known.contains(hash)).unwrap_or(false)
+	/// Conclude a voting round.
+	///
+	/// This can be called once round is complete so we stop gossiping for it.
+	pub(crate) fn conclude_round(&self, round: NumberFor<B>) {
+		debug!(target: "beefy", "🥩 About to drop gossip round #{}", round);
+		self.known_votes.write().conclude(round);
 	}
 }

@@ -152,17 +154,17 @@ where
 			{
 				let known_votes = self.known_votes.read();

-				if !GossipValidator::<B>::is_live(&known_votes, &round) {
+				if !known_votes.is_live(&round) {
 					return ValidationResult::Discard
 				}

-				if GossipValidator::<B>::is_known(&known_votes, &round, &msg_hash) {
+				if known_votes.is_known(&round, &msg_hash) {
 					return ValidationResult::ProcessAndKeep(self.topic)
 				}
 			}

 			if BeefyKeystore::verify(&msg.id, &msg.signature, &msg.commitment.encode()) {
-				GossipValidator::<B>::add_known(&mut *self.known_votes.write(), &round, msg_hash);
+				self.known_votes.write().add_known(&round, msg_hash);
 				return ValidationResult::ProcessAndKeep(self.topic)
 			} else {
 				// TODO: report peer
@@ -182,7 +184,7 @@ where
 			};

 			let round = msg.commitment.block_number;
-			let expired = !GossipValidator::<B>::is_live(&known_votes, &round);
+			let expired = !known_votes.is_live(&round);

 			trace!(target: "beefy", "🥩 Message for round #{} expired: {}", round, expired);

@@ -212,11 +214,11 @@ where

 			let msg = match VoteMessage::<NumberFor<B>, Public, Signature>::decode(&mut data) {
 				Ok(vote) => vote,
-				Err(_) => return true,
+				Err(_) => return false,
 			};

 			let round = msg.commitment.block_number;
-			let allowed = GossipValidator::<B>::is_live(&known_votes, &round);
+			let allowed = known_votes.is_live(&round);

 			debug!(target: "beefy", "🥩 Message for round #{} allowed: {}", round, allowed);

@@ -240,60 +242,58 @@ mod tests {
 	use super::*;

 	#[test]
-	fn note_round_works() {
-		let gv = GossipValidator::<Block>::new();
+	fn known_votes_insert_remove() {
+		let mut kv = KnownVotes::<Block>::new();

-		gv.note_round(1u64);
+		kv.insert(1);
+		kv.insert(1);
+		kv.insert(2);
+		assert_eq!(kv.live.len(), 2);

-		let live = gv.known_votes.read();
-		assert!(GossipValidator::<Block>::is_live(&live, &1u64));
+		let mut kv = KnownVotes::<Block>::new();
+		kv.insert(1);
+		kv.insert(2);
+		kv.insert(3);

-		drop(live);
+		assert!(kv.last_done.is_none());
+		kv.conclude(2);
+		assert_eq!(kv.live.len(), 1);
+		assert!(!kv.live.contains_key(&2));
+		assert_eq!(kv.last_done, Some(2));

-		gv.note_round(3u64);
-		gv.note_round(7u64);
-		gv.note_round(10u64);
+		kv.conclude(1);
+		assert_eq!(kv.last_done, Some(2));

-		let live = gv.known_votes.read();
-
-		assert_eq!(live.len(), MAX_LIVE_GOSSIP_ROUNDS);
-
-		assert!(!GossipValidator::<Block>::is_live(&live, &1u64));
-		assert!(GossipValidator::<Block>::is_live(&live, &3u64));
-		assert!(GossipValidator::<Block>::is_live(&live, &7u64));
-		assert!(GossipValidator::<Block>::is_live(&live, &10u64));
+		kv.conclude(3);
+		assert_eq!(kv.last_done, Some(3));
+		assert!(kv.live.is_empty());
 	}

 	#[test]
-	fn keeps_most_recent_max_rounds() {
+	fn note_and_drop_round_works() {
 		let gv = GossipValidator::<Block>::new();

+		gv.note_round(1u64);
+
+		assert!(gv.known_votes.read().is_live(&1u64));
+
 		gv.note_round(3u64);
 		gv.note_round(7u64);
 		gv.note_round(10u64);
-		gv.note_round(1u64);

-		let live = gv.known_votes.read();
+		assert_eq!(gv.known_votes.read().live.len(), 4);

-		assert_eq!(live.len(), MAX_LIVE_GOSSIP_ROUNDS);
+		gv.conclude_round(7u64);

-		assert!(GossipValidator::<Block>::is_live(&live, &3u64));
-		assert!(!GossipValidator::<Block>::is_live(&live, &1u64));
+		let votes = gv.known_votes.read();

-		drop(live);
-
-		gv.note_round(23u64);
-		gv.note_round(15u64);
-		gv.note_round(20u64);
-		gv.note_round(2u64);
-
-		let live = gv.known_votes.read();
-
-		assert_eq!(live.len(), MAX_LIVE_GOSSIP_ROUNDS);
-
-		assert!(GossipValidator::<Block>::is_live(&live, &15u64));
-		assert!(GossipValidator::<Block>::is_live(&live, &20u64));
-		assert!(GossipValidator::<Block>::is_live(&live, &23u64));
+		// rounds 1 and 3 are outdated, don't gossip anymore
+		assert!(!votes.is_live(&1u64));
+		assert!(!votes.is_live(&3u64));
+		// latest concluded round is still gossiped
+		assert!(votes.is_live(&7u64));
+		// round 10 is alive and in-progress
+		assert!(votes.is_live(&10u64));
 	}

 	#[test]
@@ -304,22 +304,18 @@ mod tests {
 		gv.note_round(7u64);
 		gv.note_round(10u64);

-		let live = gv.known_votes.read();
-
-		assert_eq!(live.len(), MAX_LIVE_GOSSIP_ROUNDS);
-
-		drop(live);
+		assert_eq!(gv.known_votes.read().live.len(), 3);

 		// note round #7 again -> should not change anything
 		gv.note_round(7u64);

-		let live = gv.known_votes.read();
+		let votes = gv.known_votes.read();

-		assert_eq!(live.len(), MAX_LIVE_GOSSIP_ROUNDS);
+		assert_eq!(votes.live.len(), 3);

-		assert!(GossipValidator::<Block>::is_live(&live, &3u64));
-		assert!(GossipValidator::<Block>::is_live(&live, &7u64));
-		assert!(GossipValidator::<Block>::is_live(&live, &10u64));
+		assert!(votes.is_live(&3u64));
+		assert!(votes.is_live(&7u64));
+		assert!(votes.is_live(&10u64));
 	}

 	struct TestContext;
@@ -349,29 +345,32 @@ mod tests {
 		beefy_keystore.sign(&who.public(), &commitment.encode()).unwrap()
 	}

+	fn dummy_vote(block_number: u64) -> VoteMessage<u64, Public, Signature> {
+		let payload = Payload::new(known_payload_ids::MMR_ROOT_ID, MmrRootHash::default().encode());
+		let commitment = Commitment { payload, block_number, validator_set_id: 0 };
+		let signature = sign_commitment(&Keyring::Alice, &commitment);
+
+		VoteMessage { commitment, id: Keyring::Alice.public(), signature }
+	}
+
 	#[test]
 	fn should_avoid_verifying_signatures_twice() {
 		let gv = GossipValidator::<Block>::new();
 		let sender = sc_network::PeerId::random();
 		let mut context = TestContext;

-		let payload = Payload::new(known_payload_ids::MMR_ROOT_ID, MmrRootHash::default().encode());
-		let commitment = Commitment { payload, block_number: 3_u64, validator_set_id: 0 };
-
-		let signature = sign_commitment(&Keyring::Alice, &commitment);
-
-		let vote = VoteMessage { commitment, id: Keyring::Alice.public(), signature };
+		let vote = dummy_vote(3);

 		gv.note_round(3u64);
 		gv.note_round(7u64);
 		gv.note_round(10u64);

-		// first time the cache should be populated.
+		// first time the cache should be populated
 		let res = gv.validate(&mut context, &sender, &vote.encode());

 		assert!(matches!(res, ValidationResult::ProcessAndKeep(_)));
 		assert_eq!(
-			gv.known_votes.read().get(&vote.commitment.block_number).map(|x| x.len()),
+			gv.known_votes.read().live.get(&vote.commitment.block_number).map(|x| x.len()),
 			Some(1)
 		);

@@ -380,17 +379,84 @@ mod tests {

 		assert!(matches!(res, ValidationResult::ProcessAndKeep(_)));

-		// next we should quickly reject if the round is not live.
-		gv.note_round(11_u64);
-		gv.note_round(12_u64);
+		// next we should quickly reject if the round is not live
+		gv.conclude_round(7_u64);

-		assert!(!GossipValidator::<Block>::is_live(
-			&*gv.known_votes.read(),
-			&vote.commitment.block_number
-		));
+		assert!(!gv.known_votes.read().is_live(&vote.commitment.block_number));

 		let res = gv.validate(&mut context, &sender, &vote.encode());

 		assert!(matches!(res, ValidationResult::Discard));
 	}
+
+	#[test]
+	fn messages_allowed_and_expired() {
+		let gv = GossipValidator::<Block>::new();
+		let sender = sc_network::PeerId::random();
+		let topic = Default::default();
+		let intent = MessageIntent::Broadcast;
+
+		// note round 2 and 3, then conclude 2
+		gv.note_round(2u64);
+		gv.note_round(3u64);
+		gv.conclude_round(2u64);
+		let mut allowed = gv.message_allowed();
+		let mut expired = gv.message_expired();
+
+		// check bad vote format
+		assert!(!allowed(&sender, intent, &topic, &mut [0u8; 16]));
+		assert!(expired(topic, &mut [0u8; 16]));
+
+		// inactive round 1 -> expired
+		let vote = dummy_vote(1);
+		let mut encoded_vote = vote.encode();
+		assert!(!allowed(&sender, intent, &topic, &mut encoded_vote));
+		assert!(expired(topic, &mut encoded_vote));
+
+		// active round 2 -> !expired - concluded but still gossiped
+		let vote = dummy_vote(2);
+		let mut encoded_vote = vote.encode();
+		assert!(allowed(&sender, intent, &topic, &mut encoded_vote));
+		assert!(!expired(topic, &mut encoded_vote));
+
+		// in progress round 3 -> !expired
+		let vote = dummy_vote(3);
+		let mut encoded_vote = vote.encode();
+		assert!(allowed(&sender, intent, &topic, &mut encoded_vote));
+		assert!(!expired(topic, &mut encoded_vote));
+
+		// unseen round 4 -> !expired
+		let vote = dummy_vote(3);
+		let mut encoded_vote = vote.encode();
+		assert!(allowed(&sender, intent, &topic, &mut encoded_vote));
+		assert!(!expired(topic, &mut encoded_vote));
+	}
+
+	#[test]
+	fn messages_rebroadcast() {
+		let gv = GossipValidator::<Block>::new();
+		let sender = sc_network::PeerId::random();
+		let topic = Default::default();
+
+		let vote = dummy_vote(1);
+		let mut encoded_vote = vote.encode();
+
+		// re-broadcasting only allowed at `REBROADCAST_AFTER` intervals
+		let intent = MessageIntent::PeriodicRebroadcast;
+		let mut allowed = gv.message_allowed();
+
+		// rebroadcast not allowed so soon after GossipValidator creation
+		assert!(!allowed(&sender, intent, &topic, &mut encoded_vote));
+
+		// hack the inner deadline to be `now`
+		*gv.next_rebroadcast.lock() = Instant::now();
+
+		// still not allowed on old `allowed` closure result
+		assert!(!allowed(&sender, intent, &topic, &mut encoded_vote));
+
+		// renew closure result
+		let mut allowed = gv.message_allowed();
+		// rebroadcast should be allowed now
+		assert!(allowed(&sender, intent, &topic, &mut encoded_vote));
+	}
 }