diff --git a/substrate/client/consensus/beefy/src/justification.rs b/substrate/client/consensus/beefy/src/justification.rs index 7f1b9e5237c3..886368c9d7cb 100644 --- a/substrate/client/consensus/beefy/src/justification.rs +++ b/substrate/client/consensus/beefy/src/justification.rs @@ -16,12 +16,11 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -use crate::keystore::BeefyKeystore; -use codec::{DecodeAll, Encode}; +use codec::DecodeAll; use sp_consensus::Error as ConsensusError; use sp_consensus_beefy::{ ecdsa_crypto::{AuthorityId, Signature}, - ValidatorSet, ValidatorSetId, VersionedFinalityProof, + BeefySignatureHasher, KnownSignature, ValidatorSet, ValidatorSetId, VersionedFinalityProof, }; use sp_runtime::traits::{Block as BlockT, NumberFor}; @@ -45,46 +44,31 @@ pub(crate) fn decode_and_verify_finality_proof( ) -> Result, (ConsensusError, u32)> { let proof = >::decode_all(&mut &*encoded) .map_err(|_| (ConsensusError::InvalidJustification, 0))?; - verify_with_validator_set::(target_number, validator_set, &proof).map(|_| proof) + verify_with_validator_set::(target_number, validator_set, &proof)?; + Ok(proof) } /// Verify the Beefy finality proof against the validator set at the block it was generated. -pub(crate) fn verify_with_validator_set( +pub(crate) fn verify_with_validator_set<'a, Block: BlockT>( target_number: NumberFor, - validator_set: &ValidatorSet, - proof: &BeefyVersionedFinalityProof, -) -> Result<(), (ConsensusError, u32)> { - let mut signatures_checked = 0u32; + validator_set: &'a ValidatorSet, + proof: &'a BeefyVersionedFinalityProof, +) -> Result>, (ConsensusError, u32)> { match proof { VersionedFinalityProof::V1(signed_commitment) => { - if signed_commitment.signatures.len() != validator_set.len() || - signed_commitment.commitment.validator_set_id != validator_set.id() || - signed_commitment.commitment.block_number != target_number - { - return Err((ConsensusError::InvalidJustification, 0)) - } - - // Arrangement of signatures in the commitment should be in the same order - // as validators for that set. - let message = signed_commitment.commitment.encode(); - let valid_signatures = validator_set - .validators() - .into_iter() - .zip(signed_commitment.signatures.iter()) - .filter(|(id, signature)| { - signature - .as_ref() - .map(|sig| { - signatures_checked += 1; - BeefyKeystore::verify(*id, sig, &message[..]) - }) - .unwrap_or(false) - }) - .count(); - if valid_signatures >= crate::round::threshold(validator_set.len()) { - Ok(()) + let signatories = signed_commitment + .verify_signatures::<_, BeefySignatureHasher>(target_number, validator_set) + .map_err(|checked_signatures| { + (ConsensusError::InvalidJustification, checked_signatures) + })?; + + if signatories.len() >= crate::round::threshold(validator_set.len()) { + Ok(signatories) } else { - Err((ConsensusError::InvalidJustification, signatures_checked)) + Err(( + ConsensusError::InvalidJustification, + signed_commitment.signature_count() as u32, + )) } }, } @@ -92,6 +76,7 @@ pub(crate) fn verify_with_validator_set( #[cfg(test)] pub(crate) mod tests { + use codec::Encode; use sp_consensus_beefy::{ known_payloads, test_utils::Keyring, Commitment, Payload, SignedCommitment, VersionedFinalityProof, diff --git a/substrate/primitives/consensus/beefy/src/commitment.rs b/substrate/primitives/consensus/beefy/src/commitment.rs index 4fd9e1b0a6ed..8d3a6c6aa90f 100644 --- a/substrate/primitives/consensus/beefy/src/commitment.rs +++ b/substrate/primitives/consensus/beefy/src/commitment.rs @@ -19,8 +19,30 @@ use alloc::{vec, vec::Vec}; use codec::{Decode, Encode, Error, Input}; use core::cmp; use scale_info::TypeInfo; +use sp_application_crypto::RuntimeAppPublic; +use sp_runtime::traits::Hash; + +use crate::{BeefyAuthorityId, Payload, ValidatorSet, ValidatorSetId}; + +/// A commitment signature, accompanied by the id of the validator that it belongs to. +#[derive(Debug)] +pub struct KnownSignature { + /// The signing validator. + pub validator_id: TAuthorityId, + /// The signature. + pub signature: TSignature, +} -use crate::{Payload, ValidatorSetId}; +impl KnownSignature<&TAuthorityId, &TSignature> { + /// Creates a `KnownSignature` from an + /// `KnownSignature<&TAuthorityId, &TSignature>`. + pub fn to_owned(&self) -> KnownSignature { + KnownSignature { + validator_id: self.validator_id.clone(), + signature: self.signature.clone(), + } + } +} /// A commitment signed by GRANDPA validators as part of BEEFY protocol. /// @@ -113,9 +135,49 @@ impl core::fmt::Display impl SignedCommitment { /// Return the number of collected signatures. - pub fn no_of_signatures(&self) -> usize { + pub fn signature_count(&self) -> usize { self.signatures.iter().filter(|x| x.is_some()).count() } + + /// Verify all the commitment signatures against the validator set that was active + /// at the block where the commitment was generated. + /// + /// Returns the valid validator-signature pairs if the commitment can be verified. + pub fn verify_signatures<'a, TAuthorityId, MsgHash>( + &'a self, + target_number: TBlockNumber, + validator_set: &'a ValidatorSet, + ) -> Result>, u32> + where + TBlockNumber: Clone + Encode + PartialEq, + TAuthorityId: RuntimeAppPublic + BeefyAuthorityId, + MsgHash: Hash, + { + if self.signatures.len() != validator_set.len() || + self.commitment.validator_set_id != validator_set.id() || + self.commitment.block_number != target_number + { + return Err(0) + } + + // Arrangement of signatures in the commitment should be in the same order + // as validators for that set. + let encoded_commitment = self.commitment.encode(); + let signatories: Vec<_> = validator_set + .validators() + .into_iter() + .zip(self.signatures.iter()) + .filter_map(|(id, maybe_signature)| { + let signature = maybe_signature.as_ref()?; + match BeefyAuthorityId::verify(id, signature, &encoded_commitment) { + true => Some(KnownSignature { validator_id: id, signature }), + false => None, + } + }) + .collect(); + + Ok(signatories) + } } /// Type to be used to denote placement of signatures @@ -439,13 +501,13 @@ mod tests { commitment, signatures: vec![None, None, Some(sigs.0), Some(sigs.1)], }; - assert_eq!(signed.no_of_signatures(), 2); + assert_eq!(signed.signature_count(), 2); // when signed.signatures[2] = None; // then - assert_eq!(signed.no_of_signatures(), 1); + assert_eq!(signed.signature_count(), 1); } #[test] diff --git a/substrate/primitives/consensus/beefy/src/lib.rs b/substrate/primitives/consensus/beefy/src/lib.rs index 70978ca559dd..6f644c5f790d 100644 --- a/substrate/primitives/consensus/beefy/src/lib.rs +++ b/substrate/primitives/consensus/beefy/src/lib.rs @@ -43,7 +43,7 @@ pub mod witness; #[cfg(feature = "std")] pub mod test_utils; -pub use commitment::{Commitment, SignedCommitment, VersionedFinalityProof}; +pub use commitment::{Commitment, KnownSignature, SignedCommitment, VersionedFinalityProof}; pub use payload::{known_payloads, BeefyPayloadId, Payload, PayloadProvider}; use alloc::vec::Vec;