Alter BEEFY primitives to prepare for potential BLS integration (#10466)

* Generalize signature.

* Fix tests.

* Introduce VersionedFinalityProof.

* cargo +nightly fmt --all

* Rework packing a tad.

substrate/client/beefy/src/notification.rs

@@ -24,7 +24,8 @@ use sp_runtime::traits::{Block, NumberFor};
 use parking_lot::Mutex;
 
 /// Stream of signed commitments returned when subscribing.
-pub type SignedCommitment<Block> = beefy_primitives::SignedCommitment<NumberFor<Block>>;
+pub type SignedCommitment<Block> =
+	beefy_primitives::SignedCommitment<NumberFor<Block>, beefy_primitives::crypto::Signature>;
 
 /// Stream of signed commitments returned when subscribing.
 type SignedCommitmentStream<Block> = TracingUnboundedReceiver<SignedCommitment<Block>>;

substrate/client/beefy/src/worker.rs

@@ -37,7 +37,7 @@ use sp_runtime::{
 use beefy_primitives::{
 	crypto::{AuthorityId, Public, Signature},
 	known_payload_ids, BeefyApi, Commitment, ConsensusLog, MmrRootHash, Payload, SignedCommitment,
-	ValidatorSet, VersionedCommitment, VoteMessage, BEEFY_ENGINE_ID, GENESIS_AUTHORITY_SET_ID,
+	ValidatorSet, VersionedFinalityProof, VoteMessage, BEEFY_ENGINE_ID, GENESIS_AUTHORITY_SET_ID,
 };
 
 use crate::{
@@ -330,7 +330,7 @@ where
 						BlockId::Number(round.1),
 						(
 							BEEFY_ENGINE_ID,
-							VersionedCommitment::V1(signed_commitment.clone()).encode(),
+							VersionedFinalityProof::V1(signed_commitment.clone()).encode(),
 						),
 					)
 					.is_err()

substrate/primitives/beefy/src/commitment.rs

@@ -18,7 +18,7 @@
 use codec::{Decode, Encode, Error, Input};
 use sp_std::{cmp, prelude::*};
 
-use crate::{crypto::Signature, ValidatorSetId};
+use crate::ValidatorSetId;
 
 /// Id of different payloads in the [`Commitment`] data
 pub type BeefyPayloadId = [u8; 2];
@@ -139,17 +139,17 @@ where
 /// please take a look at custom [`Encode`] and [`Decode`] implementations and
 /// `CompactSignedCommitment` struct.
 #[derive(Clone, Debug, PartialEq, Eq)]
-pub struct SignedCommitment<TBlockNumber> {
+pub struct SignedCommitment<TBlockNumber, TSignature> {
 	/// The commitment signatures are collected for.
 	pub commitment: Commitment<TBlockNumber>,
 	/// GRANDPA validators' signatures for the commitment.
 	///
 	/// The length of this `Vec` must match number of validators in the current set (see
 	/// [Commitment::validator_set_id]).
-	pub signatures: Vec<Option<Signature>>,
+	pub signatures: Vec<Option<TSignature>>,
 }
 
-impl<TBlockNumber> SignedCommitment<TBlockNumber> {
+impl<TBlockNumber, TSignature> SignedCommitment<TBlockNumber, TSignature> {
 	/// Return the number of collected signatures.
 	pub fn no_of_signatures(&self) -> usize {
 		self.signatures.iter().filter(|x| x.is_some()).count()
@@ -163,9 +163,9 @@ const CONTAINER_BIT_SIZE: usize = 8;
 
 /// Compressed representation of [`SignedCommitment`], used for encoding efficiency.
 #[derive(Clone, Debug, PartialEq, Eq, Encode, Decode)]
-struct CompactSignedCommitment<TCommitment> {
+struct CompactSignedCommitment<TBlockNumber, TSignature> {
 	/// The commitment, unchanged compared to regular [`SignedCommitment`].
-	commitment: TCommitment,
+	commitment: Commitment<TBlockNumber>,
 	/// A bitfield representing presence of a signature coming from a validator at some index.
 	///
 	/// The bit at index `0` is set to `1` in case we have a signature coming from a validator at
@@ -183,33 +183,29 @@ struct CompactSignedCommitment<TCommitment> {
 	/// Note that in order to associate a `Signature` from this `Vec` with a validator, one needs
 	/// to look at the `signatures_from` bitfield, since some validators might have not produced a
 	/// signature.
-	signatures_compact: Vec<Signature>,
+	signatures_compact: Vec<TSignature>,
 }
 
-impl<'a, TBlockNumber> CompactSignedCommitment<&'a Commitment<TBlockNumber>> {
+impl<'a, TBlockNumber: Clone, TSignature> CompactSignedCommitment<TBlockNumber, &'a TSignature> {
 	/// Packs a `SignedCommitment` into the compressed `CompactSignedCommitment` format for
 	/// efficient network transport.
-	fn pack(signed_commitment: &'a SignedCommitment<TBlockNumber>) -> Self {
+	fn pack(signed_commitment: &'a SignedCommitment<TBlockNumber, TSignature>) -> Self {
 		let SignedCommitment { commitment, signatures } = signed_commitment;
 		let validator_set_len = signatures.len() as u32;
+
+		let signatures_compact: Vec<&'a TSignature> =
+			signatures.iter().filter_map(|x| x.as_ref()).collect();
+		let bits = {
+			let mut bits: Vec<u8> =
+				signatures.iter().map(|x| if x.is_some() { 1 } else { 0 }).collect();
+			// Resize with excess bits for placement purposes
+			let excess_bits_len =
+				CONTAINER_BIT_SIZE - (validator_set_len as usize % CONTAINER_BIT_SIZE);
+			bits.resize(bits.len() + excess_bits_len, 0);
+			bits
+		};
+
 		let mut signatures_from: BitField = vec![];
-		let mut signatures_compact: Vec<Signature> = vec![];
-
-		for signature in signatures {
-			match signature {
-				Some(value) => signatures_compact.push(value.clone()),
-				None => (),
-			}
-		}
-
-		let mut bits: Vec<u8> =
-			signatures.iter().map(|x| if x.is_some() { 1 } else { 0 }).collect();
-
-		// Resize with excess bits for placement purposes
-		let excess_bits_len =
-			CONTAINER_BIT_SIZE - (validator_set_len as usize % CONTAINER_BIT_SIZE);
-		bits.resize(bits.len() + excess_bits_len, 0);
-
 		let chunks = bits.chunks(CONTAINER_BIT_SIZE);
 		for chunk in chunks {
 			let mut iter = chunk.iter().copied();
@@ -223,13 +219,18 @@ impl<'a, TBlockNumber> CompactSignedCommitment<&'a Commitment<TBlockNumber>> {
 			signatures_from.push(v);
 		}
 
-		Self { commitment, signatures_from, validator_set_len, signatures_compact }
+		Self {
+			commitment: commitment.clone(),
+			signatures_from,
+			validator_set_len,
+			signatures_compact,
+		}
 	}
 
 	/// Unpacks a `CompactSignedCommitment` into the uncompressed `SignedCommitment` form.
 	fn unpack(
-		temporary_signatures: CompactSignedCommitment<Commitment<TBlockNumber>>,
+		temporary_signatures: CompactSignedCommitment<TBlockNumber, TSignature>,
-	) -> SignedCommitment<TBlockNumber> {
+	) -> SignedCommitment<TBlockNumber, TSignature> {
 		let CompactSignedCommitment {
 			commitment,
 			signatures_from,
@@ -247,7 +248,7 @@ impl<'a, TBlockNumber> CompactSignedCommitment<&'a Commitment<TBlockNumber>> {
 		bits.truncate(validator_set_len as usize);
 
 		let mut next_signature = signatures_compact.into_iter();
-		let signatures: Vec<Option<Signature>> = bits
+		let signatures: Vec<Option<TSignature>> = bits
 			.iter()
 			.map(|&x| if x == 1 { next_signature.next() } else { None })
 			.collect();
@@ -256,9 +257,10 @@ impl<'a, TBlockNumber> CompactSignedCommitment<&'a Commitment<TBlockNumber>> {
 	}
 }
 
-impl<TBlockNumber> Encode for SignedCommitment<TBlockNumber>
+impl<TBlockNumber, TSignature> Encode for SignedCommitment<TBlockNumber, TSignature>
 where
-	TBlockNumber: Encode,
+	TBlockNumber: Encode + Clone,
+	TSignature: Encode,
 {
 	fn using_encoded<R, F: FnOnce(&[u8]) -> R>(&self, f: F) -> R {
 		let temp = CompactSignedCommitment::pack(self);
@@ -266,9 +268,10 @@ where
 	}
 }
 
-impl<TBlockNumber> Decode for SignedCommitment<TBlockNumber>
+impl<TBlockNumber, TSignature> Decode for SignedCommitment<TBlockNumber, TSignature>
 where
-	TBlockNumber: Decode,
+	TBlockNumber: Decode + Clone,
+	TSignature: Decode,
 {
 	fn decode<I: Input>(input: &mut I) -> Result<Self, Error> {
 		let temp = CompactSignedCommitment::decode(input)?;
@@ -276,14 +279,18 @@ where
 	}
 }
 
-/// A [SignedCommitment] with a version number. This variant will be appended
+/// A [SignedCommitment] with a version number.
-/// to the block justifications for the block for which the signed commitment
+///
-/// has been generated.
+/// This variant will be appended to the block justifications for the block
+/// for which the signed commitment has been generated.
+///
+/// Note that this enum is subject to change in the future with introduction
+/// of additional cryptographic primitives to BEEFY.
 #[derive(Clone, Debug, PartialEq, codec::Encode, codec::Decode)]
-pub enum VersionedCommitment<N> {
+pub enum VersionedFinalityProof<N, S> {
 	#[codec(index = 1)]
 	/// Current active version
-	V1(SignedCommitment<N>),
+	V1(SignedCommitment<N, S>),
 }
 
 #[cfg(test)]
@@ -298,8 +305,8 @@ mod tests {
 	use crate::{crypto, KEY_TYPE};
 
 	type TestCommitment = Commitment<u128>;
-	type TestSignedCommitment = SignedCommitment<u128>;
+	type TestSignedCommitment = SignedCommitment<u128, crypto::Signature>;
-	type TestVersionedCommitment = VersionedCommitment<u128>;
+	type TestVersionedFinalityProof = VersionedFinalityProof<u128, crypto::Signature>;
 
 	// The mock signatures are equivalent to the ones produced by the BEEFY keystore
 	fn mock_signatures() -> (crypto::Signature, crypto::Signature) {
@@ -435,14 +442,14 @@ mod tests {
 			signatures: vec![None, None, Some(sigs.0), Some(sigs.1)],
 		};
 
-		let versioned = TestVersionedCommitment::V1(signed.clone());
+		let versioned = TestVersionedFinalityProof::V1(signed.clone());
 
 		let encoded = codec::Encode::encode(&versioned);
 
 		assert_eq!(1, encoded[0]);
 		assert_eq!(encoded[1..], codec::Encode::encode(&signed));
 
-		let decoded = TestVersionedCommitment::decode(&mut &*encoded);
+		let decoded = TestVersionedFinalityProof::decode(&mut &*encoded);
 
 		assert_eq!(decoded, Ok(versioned));
 	}

substrate/primitives/beefy/src/lib.rs

@@ -36,7 +36,8 @@ pub mod mmr;
 pub mod witness;
 
 pub use commitment::{
-	known_payload_ids, BeefyPayloadId, Commitment, Payload, SignedCommitment, VersionedCommitment,
+	known_payload_ids, BeefyPayloadId, Commitment, Payload, SignedCommitment,
+	VersionedFinalityProof,
 };
 
 use codec::{Codec, Decode, Encode};

substrate/primitives/beefy/src/witness.rs

@@ -25,10 +25,7 @@
 
 use sp_std::prelude::*;
 
-use crate::{
+use crate::commitment::{Commitment, SignedCommitment};
-	commitment::{Commitment, SignedCommitment},
-	crypto::Signature,
-};
 
 /// A light form of [SignedCommitment].
 ///
@@ -60,12 +57,12 @@ impl<TBlockNumber, TMerkleRoot> SignedCommitmentWitness<TBlockNumber, TMerkleRoo
 	/// and a merkle root of all signatures.
 	///
 	/// Returns the full list of signatures along with the witness.
-	pub fn from_signed<TMerkelize>(
+	pub fn from_signed<TMerkelize, TSignature>(
-		signed: SignedCommitment<TBlockNumber>,
+		signed: SignedCommitment<TBlockNumber, TSignature>,
 		merkelize: TMerkelize,
-	) -> (Self, Vec<Option<Signature>>)
+	) -> (Self, Vec<Option<TSignature>>)
 	where
-		TMerkelize: FnOnce(&[Option<Signature>]) -> TMerkleRoot,
+		TMerkelize: FnOnce(&[Option<TSignature>]) -> TMerkleRoot,
 	{
 		let SignedCommitment { commitment, signatures } = signed;
 		let signed_by = signatures.iter().map(|s| s.is_some()).collect();
@@ -87,8 +84,9 @@ mod tests {
 	use crate::{crypto, known_payload_ids, Payload, KEY_TYPE};
 
 	type TestCommitment = Commitment<u128>;
-	type TestSignedCommitment = SignedCommitment<u128>;
+	type TestSignedCommitment = SignedCommitment<u128, crypto::Signature>;
-	type TestSignedCommitmentWitness = SignedCommitmentWitness<u128, Vec<Option<Signature>>>;
+	type TestSignedCommitmentWitness =
+		SignedCommitmentWitness<u128, Vec<Option<crypto::Signature>>>;
 
 	// The mock signatures are equivalent to the ones produced by the BEEFY keystore
 	fn mock_signatures() -> (crypto::Signature, crypto::Signature) {