beefy: initialize voter from genesis and fix initial sync

client/beefy/src/import.rs

@@ -17,12 +17,11 @@
 // along with this program. If not, see <https://www.gnu.org/licenses/>.
 
 use beefy_primitives::{crypto::Signature, BeefyApi, VersionedFinalityProof, BEEFY_ENGINE_ID};
-use codec::Encode;
-use log::error;
+use log::debug;
 use std::{collections::HashMap, sync::Arc};
 
 use sp_api::{ProvideRuntimeApi, TransactionFor};
-use sp_blockchain::{well_known_cache_keys, HeaderBackend};
+use sp_blockchain::well_known_cache_keys;
 use sp_consensus::Error as ConsensusError;
 use sp_runtime::{
 	generic::BlockId,
@@ -96,34 +95,6 @@ where
 
 		decode_and_verify_commitment::<Block>(&encoded[..], number, &validator_set)
 	}
-
-	/// Import BEEFY justification: Send it to worker for processing and also append it to backend.
-	///
-	/// This function assumes:
-	/// - `justification` is verified and valid,
-	/// - the block referred by `justification` has been imported _and_ finalized.
-	fn import_beefy_justification_unchecked(
-		&self,
-		number: NumberFor<Block>,
-		justification: VersionedFinalityProof<NumberFor<Block>, Signature>,
-	) {
-		// Append the justification to the block in the backend.
-		if let Err(e) = self.backend.append_justification(
-			BlockId::Number(number),
-			(BEEFY_ENGINE_ID, justification.encode()),
-		) {
-			error!(target: "beefy", "🥩 Error {:?} on appending justification: {:?}", e, justification);
-		}
-		// Send the justification to the BEEFY voter for processing.
-		match justification {
-			// TODO #11838: Should not unpack, these channels should also use
-			// `VersionedFinalityProof`.
-			VersionedFinalityProof::V1(signed_commitment) => self
-				.justification_sender
-				.notify(|| Ok::<_, ()>(signed_commitment))
-				.expect("forwards closure result; the closure always returns Ok; qed."),
-		};
-	}
 }
 
 #[async_trait::async_trait]
@@ -151,42 +122,36 @@ where
 		let hash = block.post_hash();
 		let number = *block.header.number();
 
-		let beefy_proof = block
-			.justifications
-			.as_mut()
-			.and_then(|just| {
-				let decoded = just
-					.get(BEEFY_ENGINE_ID)
-					.map(|encoded| self.decode_and_verify(encoded, number, hash));
-				// Remove BEEFY justification from the list before giving to `inner`;
-				// we will append it to backend ourselves at the end if all goes well.
-				just.remove(BEEFY_ENGINE_ID);
-				decoded
-			})
-			.transpose()
-			.unwrap_or(None);
+		let beefy_encoded = block.justifications.as_mut().and_then(|just| {
+			let encoded = just.get(BEEFY_ENGINE_ID).cloned();
+			// Remove BEEFY justification from the list before giving to `inner`;
+			// we will append it to backend ourselves at the end if all goes well.
+			just.remove(BEEFY_ENGINE_ID);
+			encoded
+		});
 
 		// Run inner block import.
 		let inner_import_result = self.inner.import_block(block, new_cache).await?;
 
-		match (beefy_proof, &inner_import_result) {
-			(Some(proof), ImportResult::Imported(_)) => {
-				let status = self.backend.blockchain().info();
-				if number <= status.finalized_number &&
-					Some(hash) ==
-						self.backend
-							.blockchain()
-							.hash(number)
-							.map_err(|e| ConsensusError::ClientImport(e.to_string()))?
-				{
+		match (beefy_encoded, &inner_import_result) {
+			(Some(encoded), ImportResult::Imported(_)) => {
+				if let Ok(proof) = self.decode_and_verify(&encoded, number, hash) {
 					// The proof is valid and the block is imported and final, we can import.
-					self.import_beefy_justification_unchecked(number, proof);
+					debug!(target: "beefy", "🥩 import justif {:?} for block number {:?}.", proof, number);
+					// Send the justification to the BEEFY voter for processing.
+					match proof {
+						// TODO #11838: Should not unpack, these channels should also use
+						// `VersionedFinalityProof`.
+						VersionedFinalityProof::V1(signed_commitment) => self
+							.justification_sender
+							.notify(|| Ok::<_, ()>(signed_commitment))
+							.expect("forwards closure result; the closure always returns Ok; qed."),
+					};
 				} else {
-					error!(
+					debug!(
 						target: "beefy",
-						"🥩 Cannot import justification: {:?} for, not yet final, block number {:?}",
-						proof,
-						number,
+						"🥩 error decoding justification: {:?} for imported block {:?}",
+						encoded, number,
 					);
 				}
 			},

client/beefy/src/round.rs

@@ -147,13 +147,8 @@ where
 		trace!(target: "beefy", "🥩 Round #{} done: {}", round.1, done);
 
 		if done {
-			// remove this and older (now stale) rounds
 			let signatures = self.rounds.remove(round)?.votes;
-			self.rounds.retain(|&(_, number), _| number > round.1);
-			self.mandatory_done = self.mandatory_done || round.1 == self.session_start;
-			self.best_done = self.best_done.max(Some(round.1));
-			debug!(target: "beefy", "🥩 Concluded round #{}", round.1);
-
+			self.conclude(round.1);
 			Some(
 				self.validators()
 					.iter()
@@ -165,9 +160,12 @@ where
 		}
 	}
 
-	#[cfg(test)]
-	pub(crate) fn test_set_mandatory_done(&mut self, done: bool) {
-		self.mandatory_done = done;
+	pub(crate) fn conclude(&mut self, round_num: NumberFor<B>) {
+		// Remove this and older (now stale) rounds.
+		self.rounds.retain(|&(_, number), _| number > round_num);
+		self.mandatory_done = self.mandatory_done || round_num == self.session_start;
+		self.best_done = self.best_done.max(Some(round_num));
+		debug!(target: "beefy", "🥩 Concluded round #{}", round_num);
 	}
 }
 
@@ -178,9 +176,19 @@ mod tests {
 
 	use beefy_primitives::{crypto::Public, ValidatorSet};
 
-	use super::{threshold, RoundTracker, Rounds};
+	use super::{threshold, Block as BlockT, Hash, RoundTracker, Rounds};
 	use crate::keystore::tests::Keyring;
 
+	impl<P, B> Rounds<P, B>
+	where
+		P: Ord + Hash + Clone,
+		B: BlockT,
+	{
+		pub(crate) fn test_set_mandatory_done(&mut self, done: bool) {
+			self.mandatory_done = done;
+		}
+	}
+
 	#[test]
 	fn round_tracker() {
 		let mut rt = RoundTracker::default();

client/beefy/src/tests.rs

@@ -145,7 +145,7 @@ impl BeefyTestNet {
 		})
 	}
 
-	pub(crate) fn generate_blocks(
+	pub(crate) fn generate_blocks_and_sync(
 		&mut self,
 		count: usize,
 		session_length: u64,
@@ -168,6 +168,7 @@ impl BeefyTestNet {
 
 			block
 		});
+		self.block_until_sync();
 	}
 }
 
@@ -524,8 +525,7 @@ fn beefy_finalizing_blocks() {
 	runtime.spawn(initialize_beefy(&mut net, beefy_peers, min_block_delta));
 
 	// push 42 blocks including `AuthorityChange` digests every 10 blocks.
-	net.generate_blocks(42, session_len, &validator_set, true);
-	net.block_until_sync();
+	net.generate_blocks_and_sync(42, session_len, &validator_set, true);
 
 	let net = Arc::new(Mutex::new(net));
 
@@ -563,8 +563,7 @@ fn lagging_validators() {
 	runtime.spawn(initialize_beefy(&mut net, beefy_peers, min_block_delta));
 
 	// push 62 blocks including `AuthorityChange` digests every 30 blocks.
-	net.generate_blocks(62, session_len, &validator_set, true);
-	net.block_until_sync();
+	net.generate_blocks_and_sync(62, session_len, &validator_set, true);
 
 	let net = Arc::new(Mutex::new(net));
 
@@ -640,8 +639,7 @@ fn correct_beefy_payload() {
 	runtime.spawn(initialize_beefy(&mut net, bad_peers, min_block_delta));
 
 	// push 10 blocks
-	net.generate_blocks(12, session_len, &validator_set, false);
-	net.block_until_sync();
+	net.generate_blocks_and_sync(12, session_len, &validator_set, false);
 
 	let net = Arc::new(Mutex::new(net));
 	// with 3 good voters and 1 bad one, consensus should happen and best blocks produced.