From daa41ea5ccf2edc60613fb754177355af39a6ffa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Mon, 20 Dec 2021 14:57:32 +0100 Subject: [PATCH] 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 | 3 +- substrate/client/beefy/src/worker.rs | 4 +- substrate/primitives/beefy/src/commitment.rs | 93 +++++++++++--------- substrate/primitives/beefy/src/lib.rs | 3 +- substrate/primitives/beefy/src/witness.rs | 18 ++-- 5 files changed, 64 insertions(+), 57 deletions(-) diff --git a/substrate/client/beefy/src/notification.rs b/substrate/client/beefy/src/notification.rs index f394ae6c84..7caaa54352 100644 --- a/substrate/client/beefy/src/notification.rs +++ b/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 = beefy_primitives::SignedCommitment>; +pub type SignedCommitment = + beefy_primitives::SignedCommitment, beefy_primitives::crypto::Signature>; /// Stream of signed commitments returned when subscribing. type SignedCommitmentStream = TracingUnboundedReceiver>; diff --git a/substrate/client/beefy/src/worker.rs b/substrate/client/beefy/src/worker.rs index fa48e64c12..bf29a50f19 100644 --- a/substrate/client/beefy/src/worker.rs +++ b/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() diff --git a/substrate/primitives/beefy/src/commitment.rs b/substrate/primitives/beefy/src/commitment.rs index ec1ceeded8..63c6520d86 100644 --- a/substrate/primitives/beefy/src/commitment.rs +++ b/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 { +pub struct SignedCommitment { /// The commitment signatures are collected for. pub commitment: Commitment, /// 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>, + pub signatures: Vec>, } -impl SignedCommitment { +impl SignedCommitment { /// 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 { +struct CompactSignedCommitment { /// The commitment, unchanged compared to regular [`SignedCommitment`]. - commitment: TCommitment, + commitment: Commitment, /// 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 { /// 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, + signatures_compact: Vec, } -impl<'a, TBlockNumber> CompactSignedCommitment<&'a Commitment> { +impl<'a, TBlockNumber: Clone, TSignature> CompactSignedCommitment { /// Packs a `SignedCommitment` into the compressed `CompactSignedCommitment` format for /// efficient network transport. - fn pack(signed_commitment: &'a SignedCommitment) -> Self { + fn pack(signed_commitment: &'a SignedCommitment) -> 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 = + 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 = vec![]; - - for signature in signatures { - match signature { - Some(value) => signatures_compact.push(value.clone()), - None => (), - } - } - - let mut bits: Vec = - 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> { 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>, - ) -> SignedCommitment { + temporary_signatures: CompactSignedCommitment, + ) -> SignedCommitment { let CompactSignedCommitment { commitment, signatures_from, @@ -247,7 +248,7 @@ impl<'a, TBlockNumber> CompactSignedCommitment<&'a Commitment> { bits.truncate(validator_set_len as usize); let mut next_signature = signatures_compact.into_iter(); - let signatures: Vec> = bits + let signatures: Vec> = bits .iter() .map(|&x| if x == 1 { next_signature.next() } else { None }) .collect(); @@ -256,9 +257,10 @@ impl<'a, TBlockNumber> CompactSignedCommitment<&'a Commitment> { } } -impl Encode for SignedCommitment +impl Encode for SignedCommitment where - TBlockNumber: Encode, + TBlockNumber: Encode + Clone, + TSignature: Encode, { fn using_encoded R>(&self, f: F) -> R { let temp = CompactSignedCommitment::pack(self); @@ -266,9 +268,10 @@ where } } -impl Decode for SignedCommitment +impl Decode for SignedCommitment where - TBlockNumber: Decode, + TBlockNumber: Decode + Clone, + TSignature: Decode, { fn decode(input: &mut I) -> Result { let temp = CompactSignedCommitment::decode(input)?; @@ -276,14 +279,18 @@ where } } -/// A [SignedCommitment] with a version number. This variant will be appended -/// to the block justifications for the block for which the signed commitment -/// has been generated. +/// A [SignedCommitment] with a version number. +/// +/// 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 { +pub enum VersionedFinalityProof { #[codec(index = 1)] /// Current active version - V1(SignedCommitment), + V1(SignedCommitment), } #[cfg(test)] @@ -298,8 +305,8 @@ mod tests { use crate::{crypto, KEY_TYPE}; type TestCommitment = Commitment; - type TestSignedCommitment = SignedCommitment; - type TestVersionedCommitment = VersionedCommitment; + type TestSignedCommitment = SignedCommitment; + type TestVersionedFinalityProof = VersionedFinalityProof; // 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)); } diff --git a/substrate/primitives/beefy/src/lib.rs b/substrate/primitives/beefy/src/lib.rs index cb3cf601a7..20f6b9d0d8 100644 --- a/substrate/primitives/beefy/src/lib.rs +++ b/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}; diff --git a/substrate/primitives/beefy/src/witness.rs b/substrate/primitives/beefy/src/witness.rs index 3ead08bdd7..102a6be09c 100644 --- a/substrate/primitives/beefy/src/witness.rs +++ b/substrate/primitives/beefy/src/witness.rs @@ -25,10 +25,7 @@ use sp_std::prelude::*; -use crate::{ - commitment::{Commitment, SignedCommitment}, - crypto::Signature, -}; +use crate::commitment::{Commitment, SignedCommitment}; /// A light form of [SignedCommitment]. /// @@ -60,12 +57,12 @@ impl SignedCommitmentWitness( - signed: SignedCommitment, + pub fn from_signed( + signed: SignedCommitment, merkelize: TMerkelize, - ) -> (Self, Vec>) + ) -> (Self, Vec>) where - TMerkelize: FnOnce(&[Option]) -> TMerkleRoot, + TMerkelize: FnOnce(&[Option]) -> 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; - type TestSignedCommitment = SignedCommitment; - type TestSignedCommitmentWitness = SignedCommitmentWitness>>; + type TestSignedCommitment = SignedCommitment; + type TestSignedCommitmentWitness = + SignedCommitmentWitness>>; // The mock signatures are equivalent to the ones produced by the BEEFY keystore fn mock_signatures() -> (crypto::Signature, crypto::Signature) {