Track BEEFY validator set (#94)

* Track BEEFY validator set

* Add validator_set_id to BeefyWorker

* Make validattor_set_id optional

client/beefy/src/lib.rs

@@ -24,7 +24,10 @@ use futures::{future, FutureExt, Stream, StreamExt};
 use log::{debug, error, info, trace, warn};
 use parking_lot::Mutex;
 
-use beefy_primitives::{BeefyApi, Commitment, ConsensusLog, MmrRootHash, SignedCommitment, BEEFY_ENGINE_ID, KEY_TYPE};
+use beefy_primitives::{
+	BeefyApi, Commitment, ConsensusLog, MmrRootHash, SignedCommitment, ValidatorSet, ValidatorSetId, BEEFY_ENGINE_ID,
+	KEY_TYPE,
+};
 
 use sc_client_api::{Backend as BackendT, BlockchainEvents, FinalityNotification, Finalizer};
 use sc_network_gossip::{
@@ -194,6 +197,7 @@ struct BeefyWorker<Block: BlockT, Id, Signature, FinalityNotifications> {
 	signed_commitment_sender: BeefySignedCommitmentSender<Block, Signature>,
 	best_finalized_block: NumberFor<Block>,
 	best_block_voted_on: NumberFor<Block>,
+	validator_set_id: Option<ValidatorSetId>,
 }
 
 impl<Block, Id, Signature, FinalityNotifications> BeefyWorker<Block, Id, Signature, FinalityNotifications>
@@ -210,6 +214,7 @@ where
 		signed_commitment_sender: BeefySignedCommitmentSender<Block, Signature>,
 		best_finalized_block: NumberFor<Block>,
 		best_block_voted_on: NumberFor<Block>,
+		validator_set_id: Option<ValidatorSetId>,
 	) -> Self {
 		BeefyWorker {
 			local_id,
@@ -221,6 +226,7 @@ where
 			signed_commitment_sender,
 			best_finalized_block,
 			best_block_voted_on,
+			validator_set_id,
 		}
 	}
 }
@@ -259,7 +265,7 @@ where
 	}
 
 	fn handle_finality_notification(&mut self, notification: FinalityNotification<Block>) {
-		debug!(target: "beefy", "Finality notification: {:?}", notification);
+		debug!(target: "beefy", "🥩 Finality notification: {:?}", notification);
 
 		if self.should_vote_on(*notification.header.number()) {
 			let local_id = if let BeefyId::Validator(id) = &self.local_id {
@@ -276,12 +282,22 @@ where
 				return;
 			};
 
-			// TODO: this needs added support for validator set changes (and abstracting the
-			// "thing to sign" would be nice).
+			if let Some(new) = find_authorities_change::<Block, Id>(&notification.header) {
+				debug!(target: "beefy", "🥩 New validator set: {:?}", new);
+				self.validator_set_id = Some(new.id);
+			};
+
+			let current_set_id = if let Some(set_id) = self.validator_set_id {
+				set_id
+			} else {
+				warn!(target: "beefy", "🥩 Unknown validator set id - can't vote for: {:?}", notification.header.hash());
+				return;
+			};
+
 			let commitment = Commitment {
 				payload: mmr_root,
 				block_number: notification.header.number(),
-				validator_set_id: 0,
+				validator_set_id: current_set_id,
 			};
 
 			// TODO #92
@@ -299,7 +315,7 @@ where
 			}) {
 				Ok(sig) => sig,
 				Err(err) => {
-					warn!(target: "beefy", "Error signing: {:?}", err);
+					warn!(target: "beefy", "🥩 Error signing: {:?}", err);
 					return;
 				}
 			};
@@ -316,7 +332,7 @@ where
 				.lock()
 				.gossip_message(topic::<Block>(), message.encode(), false);
 
-			debug!(target: "beefy", "Sent vote message: {:?}", message);
+			debug!(target: "beefy", "🥩 Sent vote message: {:?}", message);
 
 			self.handle_vote(
 				(message.commitment.payload, *message.commitment.block_number),
@@ -333,12 +349,12 @@ where
 
 		if vote_added && self.rounds.is_done(&round) {
 			if let Some(signatures) = self.rounds.drop(&round) {
-				// TODO: this needs added support for validator set changes (and abstracting the
-				// "thing to sign" would be nice).
 				let commitment = Commitment {
 					payload: round.0,
 					block_number: round.1,
-					validator_set_id: 0,
+					validator_set_id: self
+						.validator_set_id
+						.expect("We voted only in case of a valid validator_set_id; qed"),
 				};
 
 				let signed_commitment = SignedCommitment { commitment, signatures };
@@ -353,7 +369,7 @@ where
 	async fn run(mut self) {
 		let mut votes = Box::pin(self.gossip_engine.lock().messages_for(topic::<Block>()).filter_map(
 			|notification| async move {
-				debug!(target: "beefy", "Got vote message: {:?}", notification);
+				debug!(target: "beefy", "🥩 Got vote message: {:?}", notification);
 
 				VoteMessage::<MmrRootHash, NumberFor<Block>, Id, Signature>::decode(&mut &notification.message[..]).ok()
 			},
@@ -382,7 +398,7 @@ where
 					}
 				},
 				_ = gossip_engine.fuse() => {
-					error!(target: "beefy", "Gossip engine has terminated.");
+					error!(target: "beefy", "🥩 Gossip engine has terminated.");
 					return;
 				}
 			}
@@ -453,6 +469,8 @@ pub async fn start_beefy_gadget<Block, Pair, Backend, Client, Network, SyncOracl
 		signed_commitment_sender,
 		best_finalized_block,
 		best_block_voted_on,
+		// TODO #95
+		Some(0),
 	);
 
 	worker.run().await
@@ -470,3 +488,20 @@ where
 		}
 	})
 }
+
+/// Scan the `header` digest log for a BEEFY validator set change. Return either the new
+/// validator set or `None` in case no validator set change has been signaled.
+fn find_authorities_change<B, Id>(header: &B::Header) -> Option<ValidatorSet<Id>>
+where
+	B: BlockT,
+	Id: Codec,
+{
+	let id = OpaqueDigestItemId::Consensus(&BEEFY_ENGINE_ID);
+
+	let filter = |log: ConsensusLog<Id>| match log {
+		ConsensusLog::AuthoritiesChange(validator_set) => Some(validator_set),
+		_ => None,
+	};
+
+	header.digest().convert_first(|l| l.try_to(id).and_then(filter))
+}

frame/beefy/src/lib.rs

@@ -27,7 +27,7 @@ use sp_runtime::{
 };
 use sp_std::prelude::*;
 
-use beefy_primitives::{AuthorityIndex, ConsensusLog, BEEFY_ENGINE_ID};
+use beefy_primitives::{AuthorityIndex, ConsensusLog, ValidatorSet, BEEFY_ENGINE_ID};
 
 #[cfg(test)]
 mod mock;
@@ -102,15 +102,15 @@ impl<T: Config> Pallet<T> {
 		if new != Self::authorities() {
 			<Authorities<T>>::put(&new);
 
-			let next = Self::validator_set_id() + 1u64;
-			<ValidatorSetId<T>>::put(next);
+			let next_id = Self::validator_set_id() + 1u64;
+			<ValidatorSetId<T>>::put(next_id);
 
 			let log: DigestItem<T::Hash> = DigestItem::Consensus(
 				BEEFY_ENGINE_ID,
-				ConsensusLog::AuthoritiesChange {
-					new_validator_set: new,
-					new_validator_set_id: next,
-				}
+				ConsensusLog::AuthoritiesChange(ValidatorSet {
+					validators: new,
+					id: next_id,
+				})
 				.encode(),
 			);
 			<frame_system::Module<T>>::deposit_log(log);

frame/beefy/src/tests.rs

@@ -16,6 +16,7 @@
 
 use std::vec;
 
+use beefy_primitives::ValidatorSet;
 use codec::Encode;
 
 use sp_core::H256;
@@ -69,16 +70,17 @@ fn session_change_updates_authorities() {
 
 		assert!(1 == Beefy::validator_set_id());
 
-		let want = beefy_log(ConsensusLog::AuthoritiesChange {
-			new_validator_set: vec![mock_beefy_id(3), mock_beefy_id(4)],
-			new_validator_set_id: 1,
-		});
+		let want = beefy_log(ConsensusLog::AuthoritiesChange(ValidatorSet {
+			validators: vec![mock_beefy_id(3), mock_beefy_id(4)],
+			id: 1,
+		}));
 
 		let log = System::digest().logs[0].clone();
 
 		assert_eq!(want, log);
 	});
 }
+
 #[test]
 fn session_change_updates_next_authorities() {
 	let want = vec![mock_beefy_id(1), mock_beefy_id(2), mock_beefy_id(3), mock_beefy_id(4)];

primitives/beefy/src/lib.rs

@@ -69,6 +69,15 @@ pub const BEEFY_ENGINE_ID: sp_runtime::ConsensusEngineId = *b"BEEF";
 /// A typedef for validator set id.
 pub type ValidatorSetId = u64;
 
+/// A set of BEEFY authorities, a.k.a. validators.
+#[derive(Decode, Encode, Debug)]
+pub struct ValidatorSet<AuthorityId> {
+	/// Public keys of the validator set elements
+	pub validators: Vec<AuthorityId>,
+	/// Identifier of the validator set
+	pub id: ValidatorSetId,
+}
+
 /// The index of an authority.
 pub type AuthorityIndex = u32;
 
@@ -80,12 +89,7 @@ pub type MmrRootHash = H256;
 pub enum ConsensusLog<AuthorityId: Codec> {
 	/// The authorities have changed.
 	#[codec(index = 1)]
-	AuthoritiesChange {
-		/// Set of new validators to be used
-		new_validator_set: Vec<AuthorityId>,
-		/// Id for this new set of validators
-		new_validator_set_id: ValidatorSetId,
-	},
+	AuthoritiesChange(ValidatorSet<AuthorityId>),
 	/// Disable the authority with given index.
 	#[codec(index = 2)]
 	OnDisabled(AuthorityIndex),