From 0541cb0083d4435efff1654902c55c49c7b9b054 Mon Sep 17 00:00:00 2001 From: Skalman Date: Fri, 27 Oct 2023 12:21:49 -0400 Subject: [PATCH 01/10] Fix `BeefyAuthorityId::verify` for `ecdsa_bls_crypto` not to use `ecdsa_bls::verify` --- .../primitives/consensus/beefy/src/lib.rs | 62 ++++++++++++++----- 1 file changed, 48 insertions(+), 14 deletions(-) diff --git a/substrate/primitives/consensus/beefy/src/lib.rs b/substrate/primitives/consensus/beefy/src/lib.rs index 5bdf8ce010a1..802567369e7c 100644 --- a/substrate/primitives/consensus/beefy/src/lib.rs +++ b/substrate/primitives/consensus/beefy/src/lib.rs @@ -133,7 +133,7 @@ pub mod bls_crypto { ::Output: Into<[u8; 32]>, { fn verify(&self, signature: &::Signature, msg: &[u8]) -> bool { - // `w3f-bls` library uses IETF hashing standard and as such does not exposes + // `w3f-bls` library uses IETF hashing standard and as such does not expose // a choice of hash to field function. // We are directly calling into the library to avoid introducing new host call. // and because BeefyAuthorityId::verify is being called in the runtime so we don't have @@ -157,7 +157,7 @@ pub mod bls_crypto { pub mod ecdsa_bls_crypto { use super::{BeefyAuthorityId, Hash, RuntimeAppPublic, KEY_TYPE}; use sp_application_crypto::{app_crypto, ecdsa_bls377}; - use sp_core::{crypto::Wraps, ecdsa_bls377::Pair as EcdsaBlsPair, Pair as _}; + use sp_core::{bls377, crypto::Wraps, ecdsa, Pair as _}; app_crypto!(ecdsa_bls377, KEY_TYPE); @@ -172,12 +172,30 @@ pub mod ecdsa_bls_crypto { ::Output: Into<[u8; 32]>, { fn verify(&self, signature: &::Signature, msg: &[u8]) -> bool { - // `w3f-bls` library uses IETF hashing standard and as such does not exposes - // a choice of hash to field function. - // We are directly calling into the library to avoid introducing new host call. - // and because BeefyAuthorityId::verify is being called in the runtime so we don't have + // We can not call simply call + // `EcdsaBlsPair::verify(signature.as_inner_ref(), msg, self.as_inner_ref())` + // because that invokes ecdsa default verification which perfoms blake2 hash + // which we don't want. As such we need to re-implement the verification + // of signatures part by part instead of relying on paired crypto + let public: &[u8] = self.as_inner_ref().as_ref(); + let signature: &[u8] = signature.as_inner_ref().as_ref(); + let msg_hash = ::hash(msg).into(); + + let ecdsa_public = + ecdsa::Public::try_from(&public[0..ecdsa::PUBLIC_KEY_SERIALIZED_SIZE]); + let ecdsa_signature = + ecdsa::Signature::try_from(&signature[0..ecdsa::SIGNATURE_SERIALIZED_SIZE]); + + let bls_public = bls377::Public::try_from(&public[ecdsa::PUBLIC_KEY_SERIALIZED_SIZE..]); + let bls_signature = + bls377::Signature::try_from(&signature[ecdsa::SIGNATURE_SERIALIZED_SIZE..]); - EcdsaBlsPair::verify(signature.as_inner_ref(), msg, self.as_inner_ref()) + return match (ecdsa_public, ecdsa_signature, bls_public, bls_signature) { + (Ok(ecdsa_public), Ok(ecdsa_signature), Ok(bls_public), Ok(bls_signature)) => + ecdsa::Pair::verify_prehashed(&ecdsa_signature, &msg_hash, &ecdsa_public) && + bls377::Pair::verify(&bls_signature, msg, &bls_public), + _ => false, + }; } } } @@ -257,6 +275,7 @@ pub enum ConsensusLog { /// /// A vote message is a direct vote created by a BEEFY node on every voting round /// and is gossiped to its peers. +/// TODO: Remove `Signature` generic type, instead get it from `Id::Signature`. #[derive(Clone, Debug, Decode, Encode, PartialEq, TypeInfo)] pub struct VoteMessage { /// Commit to information extracted from a finalized block @@ -505,15 +524,30 @@ mod tests { #[cfg(feature = "bls-experimental")] fn ecdsa_bls_beefy_verify_works() { let msg = &b"test-message"[..]; - let (pair, _) = ecdsa_bls_crypto::Pair::generate(); - - let signature: ecdsa_bls_crypto::Signature = pair.as_inner_ref().sign(&msg).into(); + let (ecdsa_pair, _) = ecdsa_crypto::Pair::generate(); + let (bls_pair, _) = bls_crypto::Pair::generate(); + + let ecdsa_keccak_256_signature = ecdsa_pair.as_inner_ref().sign_prehashed(&keccak_256(msg)); + let bls_signature = bls_pair.sign(&msg); + + let public_vec: Vec = [ + ecdsa_pair.public().as_inner_ref().as_ref(), + bls_pair.public().as_inner_ref().as_ref(), + ] + .concat(); + + let public = ecdsa_bls_crypto::Public::try_from(&public_vec[..]).unwrap(); + let signature_slice: Vec = [ + >::as_ref(&ecdsa_keccak_256_signature), + >::as_ref(&bls_signature.as_inner_ref()), + ] + .concat(); + let signature = ecdsa_bls_crypto::Signature::try_from(&signature_slice[..]).unwrap(); // Verification works if same hashing function is used when signing and verifying. - assert!(BeefyAuthorityId::::verify(&pair.public(), &signature, msg)); + assert!(BeefyAuthorityId::::verify(&public, &signature, msg)); - // Other public key doesn't work - let (other_pair, _) = ecdsa_bls_crypto::Pair::generate(); - assert!(!BeefyAuthorityId::::verify(&other_pair.public(), &signature, msg,)); + // Verification doesn't works if we verify function provided by pair_crypto implementation + assert!(!ecdsa_bls_crypto::Pair::verify(&signature, msg, &public)); } } From 099b86d3c743d4d23b4150b94fbc23bb5a0c40b6 Mon Sep 17 00:00:00 2001 From: Skalman Date: Fri, 27 Oct 2023 12:31:06 -0400 Subject: [PATCH 02/10] Fix comment in `primitives/consensus/beefy/src/lib.rs` --- substrate/primitives/consensus/beefy/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/substrate/primitives/consensus/beefy/src/lib.rs b/substrate/primitives/consensus/beefy/src/lib.rs index 802567369e7c..1af40b2c4442 100644 --- a/substrate/primitives/consensus/beefy/src/lib.rs +++ b/substrate/primitives/consensus/beefy/src/lib.rs @@ -172,11 +172,11 @@ pub mod ecdsa_bls_crypto { ::Output: Into<[u8; 32]>, { fn verify(&self, signature: &::Signature, msg: &[u8]) -> bool { - // We can not call simply call + // We can not simply call // `EcdsaBlsPair::verify(signature.as_inner_ref(), msg, self.as_inner_ref())` // because that invokes ecdsa default verification which perfoms blake2 hash // which we don't want. As such we need to re-implement the verification - // of signatures part by part instead of relying on paired crypto + // of signatures part by part instead of relying on `paired_crypto` let public: &[u8] = self.as_inner_ref().as_ref(); let signature: &[u8] = signature.as_inner_ref().as_ref(); let msg_hash = ::hash(msg).into(); From 033ff5701780d7537ccfe14d66b181252db928b5 Mon Sep 17 00:00:00 2001 From: Skalman Date: Tue, 31 Oct 2023 10:10:36 -0400 Subject: [PATCH 03/10] - implement `sign/verify_with_hasher` for `ecdsa_bls377` `Pair` with test - update `substrate/primitives/consensus/beefy/src/lib.rs` accordingly --- .../primitives/consensus/beefy/src/lib.rs | 60 +++++---------- .../primitives/core/src/paired_crypto.rs | 74 ++++++++++++++++++- 2 files changed, 88 insertions(+), 46 deletions(-) diff --git a/substrate/primitives/consensus/beefy/src/lib.rs b/substrate/primitives/consensus/beefy/src/lib.rs index 1af40b2c4442..c78abdeb1ccc 100644 --- a/substrate/primitives/consensus/beefy/src/lib.rs +++ b/substrate/primitives/consensus/beefy/src/lib.rs @@ -157,7 +157,7 @@ pub mod bls_crypto { pub mod ecdsa_bls_crypto { use super::{BeefyAuthorityId, Hash, RuntimeAppPublic, KEY_TYPE}; use sp_application_crypto::{app_crypto, ecdsa_bls377}; - use sp_core::{bls377, crypto::Wraps, ecdsa, Pair as _}; + use sp_core::{crypto::Wraps, ecdsa_bls377::Pair as EcdsaBlsPair}; app_crypto!(ecdsa_bls377, KEY_TYPE); @@ -175,27 +175,12 @@ pub mod ecdsa_bls_crypto { // We can not simply call // `EcdsaBlsPair::verify(signature.as_inner_ref(), msg, self.as_inner_ref())` // because that invokes ecdsa default verification which perfoms blake2 hash - // which we don't want. As such we need to re-implement the verification - // of signatures part by part instead of relying on `paired_crypto` - let public: &[u8] = self.as_inner_ref().as_ref(); - let signature: &[u8] = signature.as_inner_ref().as_ref(); - let msg_hash = ::hash(msg).into(); - - let ecdsa_public = - ecdsa::Public::try_from(&public[0..ecdsa::PUBLIC_KEY_SERIALIZED_SIZE]); - let ecdsa_signature = - ecdsa::Signature::try_from(&signature[0..ecdsa::SIGNATURE_SERIALIZED_SIZE]); - - let bls_public = bls377::Public::try_from(&public[ecdsa::PUBLIC_KEY_SERIALIZED_SIZE..]); - let bls_signature = - bls377::Signature::try_from(&signature[ecdsa::SIGNATURE_SERIALIZED_SIZE..]); - - return match (ecdsa_public, ecdsa_signature, bls_public, bls_signature) { - (Ok(ecdsa_public), Ok(ecdsa_signature), Ok(bls_public), Ok(bls_signature)) => - ecdsa::Pair::verify_prehashed(&ecdsa_signature, &msg_hash, &ecdsa_public) && - bls377::Pair::verify(&bls_signature, msg, &bls_public), - _ => false, - }; + // which we don't want. + EcdsaBlsPair::verify_with_hasher::( + signature.as_inner_ref(), + msg, + self.as_inner_ref(), + ) } } } @@ -524,30 +509,19 @@ mod tests { #[cfg(feature = "bls-experimental")] fn ecdsa_bls_beefy_verify_works() { let msg = &b"test-message"[..]; - let (ecdsa_pair, _) = ecdsa_crypto::Pair::generate(); - let (bls_pair, _) = bls_crypto::Pair::generate(); - - let ecdsa_keccak_256_signature = ecdsa_pair.as_inner_ref().sign_prehashed(&keccak_256(msg)); - let bls_signature = bls_pair.sign(&msg); - - let public_vec: Vec = [ - ecdsa_pair.public().as_inner_ref().as_ref(), - bls_pair.public().as_inner_ref().as_ref(), - ] - .concat(); - - let public = ecdsa_bls_crypto::Public::try_from(&public_vec[..]).unwrap(); - let signature_slice: Vec = [ - >::as_ref(&ecdsa_keccak_256_signature), - >::as_ref(&bls_signature.as_inner_ref()), - ] - .concat(); - let signature = ecdsa_bls_crypto::Signature::try_from(&signature_slice[..]).unwrap(); + let (pair, _) = ecdsa_bls_crypto::Pair::generate(); + + let signature: ecdsa_bls_crypto::Signature = + pair.as_inner_ref().sign_with_hasher::(&msg).into(); // Verification works if same hashing function is used when signing and verifying. - assert!(BeefyAuthorityId::::verify(&public, &signature, msg)); + assert!(BeefyAuthorityId::::verify(&pair.public(), &signature, msg)); // Verification doesn't works if we verify function provided by pair_crypto implementation - assert!(!ecdsa_bls_crypto::Pair::verify(&signature, msg, &public)); + assert!(!ecdsa_bls_crypto::Pair::verify(&signature, msg, &pair.public())); + + // Other public key doesn't work + let (other_pair, _) = ecdsa_bls_crypto::Pair::generate(); + assert!(!BeefyAuthorityId::::verify(&other_pair.public(), &signature, msg,)); } } diff --git a/substrate/primitives/core/src/paired_crypto.rs b/substrate/primitives/core/src/paired_crypto.rs index a97b657e7578..88167bb8a2fe 100644 --- a/substrate/primitives/core/src/paired_crypto.rs +++ b/substrate/primitives/core/src/paired_crypto.rs @@ -39,7 +39,11 @@ use sp_std::convert::TryFrom; /// ECDSA and BLS12-377 paired crypto scheme #[cfg(feature = "bls-experimental")] pub mod ecdsa_bls377 { - use crate::{bls377, crypto::CryptoTypeId, ecdsa}; + use crate::{ + bls377, + crypto::{CryptoTypeId, Pair as PairT, UncheckedFrom}, + ecdsa, + }; /// An identifier used to match public keys against BLS12-377 keys pub const CRYPTO_ID: CryptoTypeId = CryptoTypeId(*b"ecb7"); @@ -71,6 +75,60 @@ pub mod ecdsa_bls377 { impl super::CryptoType for Pair { type Pair = Pair; } + + #[cfg(feature = "full_crypto")] + impl Pair { + /// hashes the `message` with the specified `MsgHasher` and then signs it using ECDSA + /// algorithm. It does not affect the behavoir of BLS12-377 component. It generates + /// BLS12-377 Signature according to IETF standard and disregard the hasher for the + /// BLS12-377 component + pub fn sign_with_hasher(&self, message: &[u8]) -> Signature + where + ::Out: Into<[u8; 32]>, + { + let msg_hash = ::hash(message).into(); + + let mut raw: [u8; SIGNATURE_LEN] = [0u8; SIGNATURE_LEN]; + raw[..ecdsa::SIGNATURE_SERIALIZED_SIZE] + .copy_from_slice(self.left.sign_prehashed(&msg_hash).as_ref()); + raw[ecdsa::SIGNATURE_SERIALIZED_SIZE..] + .copy_from_slice(self.right.sign(message).as_ref()); + ::Signature::unchecked_from(raw) + } + + /// hashes the `message` with the specified `MsgHasher` and then verifys if the resulting + /// hash was signed by the provided ECDSA public key.. It does not affect the behavoir of + /// BLS12-377 component. It Verify the BLS12-377 signature as it was hashed and signed + /// according to IETF standrad + pub fn verify_with_hasher( + sig: &Signature, + message: &[u8], + public: &Public, + ) -> bool + where + ::Out: Into<[u8; 32]>, + { + let msg_hash = ::hash(message).into(); + + let Ok(left_pub) = public.0[..ecdsa::PUBLIC_KEY_SERIALIZED_SIZE].try_into() else { + return false + }; + let Ok(left_sig) = sig.0[0..ecdsa::SIGNATURE_SERIALIZED_SIZE].try_into() else { + return false + }; + if !ecdsa::Pair::verify_prehashed(&left_sig, &msg_hash, &left_pub) { + return false + } + + let Ok(right_pub) = public.0[ecdsa::PUBLIC_KEY_SERIALIZED_SIZE..].try_into() else { + return false + }; + let Ok(right_sig) = sig.0[ecdsa::SIGNATURE_SERIALIZED_SIZE..].try_into() else { + return false + }; + bls377::Pair::verify(&right_sig, message.as_ref(), &right_pub) + } + } } /// Secure seed length. @@ -455,12 +513,12 @@ where #[cfg(all(test, feature = "bls-experimental"))] mod test { use super::*; - use crate::crypto::DEV_PHRASE; + use crate::{crypto::DEV_PHRASE, KeccakHasher}; use ecdsa_bls377::{Pair, Signature}; use crate::{bls377, ecdsa}; - #[test] + #[test] fn test_length_of_paired_ecdsa_and_bls377_public_key_and_signature_is_correct() { assert_eq!( ::Public::LEN, @@ -617,6 +675,16 @@ mod test { assert_eq!(cmp, public); } + #[test] + fn sign_and_verify_with_haser_works() { + let pair = + Pair::from_seed(&(b"12345678901234567890123456789012".as_slice().try_into().unwrap())); + let message = b"Something important"; + let signature = pair.sign_with_hasher::(&message[..]); + + assert!(Pair::verify_with_hasher::(&signature, &message[..], &pair.public())); + } + #[test] fn signature_serialization_works() { let pair = From 76831b7b87250e3db10e66bc0e357d9b33b48d64 Mon Sep 17 00:00:00 2001 From: drskalman <35698397+drskalman@users.noreply.github.com> Date: Wed, 1 Nov 2023 14:00:44 -0400 Subject: [PATCH 04/10] Fix typos and improve comment quality Co-authored-by: Davide Galassi --- substrate/primitives/consensus/beefy/src/lib.rs | 2 +- substrate/primitives/core/src/paired_crypto.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/substrate/primitives/consensus/beefy/src/lib.rs b/substrate/primitives/consensus/beefy/src/lib.rs index c78abdeb1ccc..738a491f04fb 100644 --- a/substrate/primitives/consensus/beefy/src/lib.rs +++ b/substrate/primitives/consensus/beefy/src/lib.rs @@ -260,7 +260,7 @@ pub enum ConsensusLog { /// /// A vote message is a direct vote created by a BEEFY node on every voting round /// and is gossiped to its peers. -/// TODO: Remove `Signature` generic type, instead get it from `Id::Signature`. +// TODO: Remove `Signature` generic type, instead get it from `Id::Signature`. #[derive(Clone, Debug, Decode, Encode, PartialEq, TypeInfo)] pub struct VoteMessage { /// Commit to information extracted from a finalized block diff --git a/substrate/primitives/core/src/paired_crypto.rs b/substrate/primitives/core/src/paired_crypto.rs index 88167bb8a2fe..03567a8b7317 100644 --- a/substrate/primitives/core/src/paired_crypto.rs +++ b/substrate/primitives/core/src/paired_crypto.rs @@ -676,7 +676,7 @@ mod test { } #[test] - fn sign_and_verify_with_haser_works() { + fn sign_and_verify_with_hasher_works() { let pair = Pair::from_seed(&(b"12345678901234567890123456789012".as_slice().try_into().unwrap())); let message = b"Something important"; From c860dd0a1810a0bee3e54c77bc0971ef84119e3d Mon Sep 17 00:00:00 2001 From: drskalman <35698397+drskalman@users.noreply.github.com> Date: Thu, 9 Nov 2023 12:58:03 -0500 Subject: [PATCH 05/10] Improve documention and comments Co-authored-by: Robert Hambrock --- substrate/primitives/consensus/beefy/src/lib.rs | 2 +- substrate/primitives/core/src/paired_crypto.rs | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/substrate/primitives/consensus/beefy/src/lib.rs b/substrate/primitives/consensus/beefy/src/lib.rs index 738a491f04fb..2684baac407b 100644 --- a/substrate/primitives/consensus/beefy/src/lib.rs +++ b/substrate/primitives/consensus/beefy/src/lib.rs @@ -517,7 +517,7 @@ mod tests { // Verification works if same hashing function is used when signing and verifying. assert!(BeefyAuthorityId::::verify(&pair.public(), &signature, msg)); - // Verification doesn't works if we verify function provided by pair_crypto implementation + // Verification doesn't work if we verify function provided by pair_crypto implementation assert!(!ecdsa_bls_crypto::Pair::verify(&signature, msg, &pair.public())); // Other public key doesn't work diff --git a/substrate/primitives/core/src/paired_crypto.rs b/substrate/primitives/core/src/paired_crypto.rs index 03567a8b7317..684b8dffa343 100644 --- a/substrate/primitives/core/src/paired_crypto.rs +++ b/substrate/primitives/core/src/paired_crypto.rs @@ -80,7 +80,7 @@ pub mod ecdsa_bls377 { impl Pair { /// hashes the `message` with the specified `MsgHasher` and then signs it using ECDSA /// algorithm. It does not affect the behavoir of BLS12-377 component. It generates - /// BLS12-377 Signature according to IETF standard and disregard the hasher for the + /// BLS12-377 Signature according to IETF standard and disregards the hasher for the /// BLS12-377 component pub fn sign_with_hasher(&self, message: &[u8]) -> Signature where @@ -96,10 +96,10 @@ pub mod ecdsa_bls377 { ::Signature::unchecked_from(raw) } - /// hashes the `message` with the specified `MsgHasher` and then verifys if the resulting - /// hash was signed by the provided ECDSA public key.. It does not affect the behavoir of - /// BLS12-377 component. It Verify the BLS12-377 signature as it was hashed and signed - /// according to IETF standrad + /// Hashes the `message` with the specified `MsgHasher` and then verifies whether the resulting + /// hash was signed by the provided ECDSA public key. It does not affect the behavior of the + /// BLS12-377 component. It verifies whether the BLS12-377 signature was hashed and signed + /// according to IETF standard pub fn verify_with_hasher( sig: &Signature, message: &[u8], From 3b8ededc87fb5203904cf4725b16407163705d40 Mon Sep 17 00:00:00 2001 From: Skalman Date: Mon, 13 Nov 2023 11:09:31 -0500 Subject: [PATCH 06/10] add reasoning on why in BEEFY we want the ECDSA signature on Keccak hash of the message instead of Blake2 --- substrate/primitives/consensus/beefy/src/lib.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/substrate/primitives/consensus/beefy/src/lib.rs b/substrate/primitives/consensus/beefy/src/lib.rs index 2684baac407b..3fd33e934c69 100644 --- a/substrate/primitives/consensus/beefy/src/lib.rs +++ b/substrate/primitives/consensus/beefy/src/lib.rs @@ -175,7 +175,14 @@ pub mod ecdsa_bls_crypto { // We can not simply call // `EcdsaBlsPair::verify(signature.as_inner_ref(), msg, self.as_inner_ref())` // because that invokes ecdsa default verification which perfoms blake2 hash - // which we don't want. + // which we don't want. This is because ECDSA signatures are meant to be verified + // on Ethereum network where Keccak hasher is siginificantly cheaper than Blake2b. + // See Figure 3 of [OnSc21](https://www.scitepress.org/Papers/2021/106066/106066.pdf) + // for comparison. + // + // - [OnSc21] Onica, E., & Schifirneţ, C. (2021). Towards efficient hashing in + // ethereum smart contracts. Proceedings of the 16th International + // Conference on Software Technologies, () EcdsaBlsPair::verify_with_hasher::( signature.as_inner_ref(), msg, From 8cfccd8696f39a2727a5952d55d8491d0648d3ff Mon Sep 17 00:00:00 2001 From: Skalman Date: Mon, 13 Nov 2023 11:45:32 -0500 Subject: [PATCH 07/10] fmt --- substrate/primitives/core/src/paired_crypto.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/substrate/primitives/core/src/paired_crypto.rs b/substrate/primitives/core/src/paired_crypto.rs index 684b8dffa343..aa9994695573 100644 --- a/substrate/primitives/core/src/paired_crypto.rs +++ b/substrate/primitives/core/src/paired_crypto.rs @@ -96,10 +96,10 @@ pub mod ecdsa_bls377 { ::Signature::unchecked_from(raw) } - /// Hashes the `message` with the specified `MsgHasher` and then verifies whether the resulting - /// hash was signed by the provided ECDSA public key. It does not affect the behavior of the - /// BLS12-377 component. It verifies whether the BLS12-377 signature was hashed and signed - /// according to IETF standard + /// Hashes the `message` with the specified `MsgHasher` and then verifies whether the + /// resulting hash was signed by the provided ECDSA public key. It does not affect the + /// behavior of the BLS12-377 component. It verifies whether the BLS12-377 signature was + /// hashed and signed according to IETF standard pub fn verify_with_hasher( sig: &Signature, message: &[u8], From a15a60da935a52317d8e9975de233a9b9b9a0629 Mon Sep 17 00:00:00 2001 From: drskalman <35698397+drskalman@users.noreply.github.com> Date: Mon, 13 Nov 2023 13:08:43 -0500 Subject: [PATCH 08/10] Improve documentation Co-authored-by: Davide Galassi --- substrate/primitives/consensus/beefy/src/lib.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/substrate/primitives/consensus/beefy/src/lib.rs b/substrate/primitives/consensus/beefy/src/lib.rs index 3fd33e934c69..f79ee3ba0c9d 100644 --- a/substrate/primitives/consensus/beefy/src/lib.rs +++ b/substrate/primitives/consensus/beefy/src/lib.rs @@ -174,15 +174,11 @@ pub mod ecdsa_bls_crypto { fn verify(&self, signature: &::Signature, msg: &[u8]) -> bool { // We can not simply call // `EcdsaBlsPair::verify(signature.as_inner_ref(), msg, self.as_inner_ref())` - // because that invokes ecdsa default verification which perfoms blake2 hash + // because that invokes ECDSA default verification which perfoms Blake2b hash // which we don't want. This is because ECDSA signatures are meant to be verified - // on Ethereum network where Keccak hasher is siginificantly cheaper than Blake2b. + // on Ethereum network where Keccak hasher is significantly cheaper than Blake2b. // See Figure 3 of [OnSc21](https://www.scitepress.org/Papers/2021/106066/106066.pdf) // for comparison. - // - // - [OnSc21] Onica, E., & Schifirneţ, C. (2021). Towards efficient hashing in - // ethereum smart contracts. Proceedings of the 16th International - // Conference on Software Technologies, () EcdsaBlsPair::verify_with_hasher::( signature.as_inner_ref(), msg, From 1228d41785544d8fda76ffa9dd23a1a2e7600970 Mon Sep 17 00:00:00 2001 From: Davide Galassi Date: Tue, 14 Nov 2023 08:50:41 +0100 Subject: [PATCH 09/10] Mostly doc nitpicks --- .../primitives/consensus/beefy/src/lib.rs | 3 +- .../primitives/core/src/paired_crypto.rs | 38 ++++++++++--------- 2 files changed, 22 insertions(+), 19 deletions(-) diff --git a/substrate/primitives/consensus/beefy/src/lib.rs b/substrate/primitives/consensus/beefy/src/lib.rs index f79ee3ba0c9d..26b1cc375956 100644 --- a/substrate/primitives/consensus/beefy/src/lib.rs +++ b/substrate/primitives/consensus/beefy/src/lib.rs @@ -167,8 +167,9 @@ pub mod ecdsa_bls_crypto { /// Signature for a BEEFY authority using (ECDSA,BLS) as its crypto. pub type AuthoritySignature = Signature; - impl BeefyAuthorityId for AuthorityId + impl BeefyAuthorityId for AuthorityId where + MsgHash: Hash, ::Output: Into<[u8; 32]>, { fn verify(&self, signature: &::Signature, msg: &[u8]) -> bool { diff --git a/substrate/primitives/core/src/paired_crypto.rs b/substrate/primitives/core/src/paired_crypto.rs index aa9994695573..edf6156f268e 100644 --- a/substrate/primitives/core/src/paired_crypto.rs +++ b/substrate/primitives/core/src/paired_crypto.rs @@ -39,6 +39,8 @@ use sp_std::convert::TryFrom; /// ECDSA and BLS12-377 paired crypto scheme #[cfg(feature = "bls-experimental")] pub mod ecdsa_bls377 { + #[cfg(feature = "full_crypto")] + use crate::Hasher; use crate::{ bls377, crypto::{CryptoTypeId, Pair as PairT, UncheckedFrom}, @@ -78,15 +80,17 @@ pub mod ecdsa_bls377 { #[cfg(feature = "full_crypto")] impl Pair { - /// hashes the `message` with the specified `MsgHasher` and then signs it using ECDSA - /// algorithm. It does not affect the behavoir of BLS12-377 component. It generates - /// BLS12-377 Signature according to IETF standard and disregards the hasher for the - /// BLS12-377 component - pub fn sign_with_hasher(&self, message: &[u8]) -> Signature + /// Hashes the `message` with the specified [`Hasher`] before signing sith the ECDSA secret + /// component. + /// + /// The hasher does not affect the BLS12-377 component. This generates BLS12-377 Signature + /// according to IETF standard. + pub fn sign_with_hasher(&self, message: &[u8]) -> Signature where - ::Out: Into<[u8; 32]>, + H: Hasher, + H::Out: Into<[u8; 32]>, { - let msg_hash = ::hash(message).into(); + let msg_hash = H::hash(message).into(); let mut raw: [u8; SIGNATURE_LEN] = [0u8; SIGNATURE_LEN]; raw[..ecdsa::SIGNATURE_SERIALIZED_SIZE] @@ -96,19 +100,17 @@ pub mod ecdsa_bls377 { ::Signature::unchecked_from(raw) } - /// Hashes the `message` with the specified `MsgHasher` and then verifies whether the - /// resulting hash was signed by the provided ECDSA public key. It does not affect the - /// behavior of the BLS12-377 component. It verifies whether the BLS12-377 signature was - /// hashed and signed according to IETF standard - pub fn verify_with_hasher( - sig: &Signature, - message: &[u8], - public: &Public, - ) -> bool + /// Hashes the `message` with the specified [`Hasher`] before verifying with the ECDSA + /// public component. + /// + /// The hasher does not affect the the BLS12-377 component. This verifies whether the + /// BLS12-377 signature was hashed and signed according to IETF standard + pub fn verify_with_hasher(sig: &Signature, message: &[u8], public: &Public) -> bool where - ::Out: Into<[u8; 32]>, + H: Hasher, + H::Out: Into<[u8; 32]>, { - let msg_hash = ::hash(message).into(); + let msg_hash = H::hash(message).into(); let Ok(left_pub) = public.0[..ecdsa::PUBLIC_KEY_SERIALIZED_SIZE].try_into() else { return false From 16e26cee9d97289728dee0ea006d12e114240a43 Mon Sep 17 00:00:00 2001 From: Davide Galassi Date: Tue, 14 Nov 2023 08:53:45 +0100 Subject: [PATCH 10/10] Last trivial nits :-) --- substrate/primitives/consensus/beefy/src/lib.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/substrate/primitives/consensus/beefy/src/lib.rs b/substrate/primitives/consensus/beefy/src/lib.rs index 26b1cc375956..e31c53237be2 100644 --- a/substrate/primitives/consensus/beefy/src/lib.rs +++ b/substrate/primitives/consensus/beefy/src/lib.rs @@ -167,10 +167,10 @@ pub mod ecdsa_bls_crypto { /// Signature for a BEEFY authority using (ECDSA,BLS) as its crypto. pub type AuthoritySignature = Signature; - impl BeefyAuthorityId for AuthorityId + impl BeefyAuthorityId for AuthorityId where - MsgHash: Hash, - ::Output: Into<[u8; 32]>, + H: Hash, + H::Output: Into<[u8; 32]>, { fn verify(&self, signature: &::Signature, msg: &[u8]) -> bool { // We can not simply call @@ -180,7 +180,7 @@ pub mod ecdsa_bls_crypto { // on Ethereum network where Keccak hasher is significantly cheaper than Blake2b. // See Figure 3 of [OnSc21](https://www.scitepress.org/Papers/2021/106066/106066.pdf) // for comparison. - EcdsaBlsPair::verify_with_hasher::( + EcdsaBlsPair::verify_with_hasher::( signature.as_inner_ref(), msg, self.as_inner_ref(),