Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate BEEFY BLS crypto to bls12-381 curve #4931

Merged
merged 15 commits into from
Jul 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 11 additions & 11 deletions substrate/client/consensus/beefy/src/keystore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use log::warn;

use sp_application_crypto::{key_types::BEEFY as BEEFY_KEY_TYPE, AppCrypto, RuntimeAppPublic};
#[cfg(feature = "bls-experimental")]
use sp_core::ecdsa_bls377;
use sp_core::ecdsa_bls381;
use sp_core::{ecdsa, keccak_256};

use sp_keystore::KeystorePtr;
Expand Down Expand Up @@ -100,13 +100,13 @@ impl<AuthorityId: AuthorityIdBound> BeefyKeystore<AuthorityId> {
},

#[cfg(feature = "bls-experimental")]
ecdsa_bls377::CRYPTO_ID => {
let public: ecdsa_bls377::Public =
ecdsa_bls377::Public::try_from(public.as_slice()).unwrap();
ecdsa_bls381::CRYPTO_ID => {
let public: ecdsa_bls381::Public =
ecdsa_bls381::Public::try_from(public.as_slice()).unwrap();
let sig = store
.ecdsa_bls377_sign_with_keccak256(BEEFY_KEY_TYPE, &public, &message)
.ecdsa_bls381_sign_with_keccak256(BEEFY_KEY_TYPE, &public, &message)
.map_err(|e| error::Error::Keystore(e.to_string()))?
.ok_or_else(|| error::Error::Signature("bls377_sign() failed".to_string()))?;
.ok_or_else(|| error::Error::Signature("bls381_sign() failed".to_string()))?;
let sig_ref: &[u8] = sig.as_ref();
sig_ref.to_vec()
},
Expand Down Expand Up @@ -146,8 +146,8 @@ impl<AuthorityId: AuthorityIdBound> BeefyKeystore<AuthorityId> {
}),

#[cfg(feature = "bls-experimental")]
ecdsa_bls377::CRYPTO_ID => store
.ecdsa_bls377_public_keys(BEEFY_KEY_TYPE)
ecdsa_bls381::CRYPTO_ID => store
.ecdsa_bls381_public_keys(BEEFY_KEY_TYPE)
.drain(..)
.map(|pk| AuthorityId::try_from(pk.as_ref()))
.collect::<Result<Vec<_>, _>>()
Expand Down Expand Up @@ -254,9 +254,9 @@ pub mod tests {
AuthorityId::decode(&mut pk.as_ref()).unwrap()
},
#[cfg(feature = "bls-experimental")]
ecdsa_bls377::CRYPTO_ID => {
ecdsa_bls381::CRYPTO_ID => {
let pk = store
.ecdsa_bls377_generate_new(key_type, optional_seed.as_deref())
.ecdsa_bls381_generate_new(key_type, optional_seed.as_deref())
.ok()
.unwrap();
AuthorityId::decode(&mut pk.as_ref()).unwrap()
Expand Down Expand Up @@ -452,7 +452,7 @@ pub mod tests {
#[cfg(feature = "bls-experimental")]
#[test]
fn sign_error_for_ecdsa_n_bls() {
sign_error::<ecdsa_bls_crypto::AuthorityId>("bls377_sign() failed");
sign_error::<ecdsa_bls_crypto::AuthorityId>("bls381_sign() failed");
}

#[test]
Expand Down
56 changes: 15 additions & 41 deletions substrate/client/keystore/src/local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ use sp_core::bandersnatch;
}

sp_keystore::bls_experimental_enabled! {
use sp_core::{bls377, bls381, ecdsa_bls377, KeccakHasher};
use sp_core::{bls381, ecdsa_bls381, KeccakHasher};
}

use crate::{Error, Result};
Expand Down Expand Up @@ -357,68 +357,42 @@ impl Keystore for LocalKeystore {
self.sign::<bls381::Pair>(key_type, public, msg)
}

fn bls377_public_keys(&self, key_type: KeyTypeId) -> Vec<bls377::Public> {
self.public_keys::<bls377::Pair>(key_type)
fn ecdsa_bls381_public_keys(&self, key_type: KeyTypeId) -> Vec<ecdsa_bls381::Public> {
self.public_keys::<ecdsa_bls381::Pair>(key_type)
}

/// Generate a new pair compatible with the 'bls377' signature scheme.
/// Generate a new pair of paired-keys compatible with the '(ecdsa,bls381)' signature scheme.
///
/// If `[seed]` is `Some` then the key will be ephemeral and stored in memory.
fn bls377_generate_new(
fn ecdsa_bls381_generate_new(
&self,
key_type: KeyTypeId,
seed: Option<&str>,
) -> std::result::Result<bls377::Public, TraitError> {
self.generate_new::<bls377::Pair>(key_type, seed)
) -> std::result::Result<ecdsa_bls381::Public, TraitError> {
self.generate_new::<ecdsa_bls381::Pair>(key_type, seed)
}

fn bls377_sign(
fn ecdsa_bls381_sign(
&self,
key_type: KeyTypeId,
public: &bls377::Public,
public: &ecdsa_bls381::Public,
msg: &[u8],
) -> std::result::Result<Option<bls377::Signature>, TraitError> {
self.sign::<bls377::Pair>(key_type, public, msg)
) -> std::result::Result<Option<ecdsa_bls381::Signature>, TraitError> {
self.sign::<ecdsa_bls381::Pair>(key_type, public, msg)
}

fn ecdsa_bls377_public_keys(&self, key_type: KeyTypeId) -> Vec<ecdsa_bls377::Public> {
self.public_keys::<ecdsa_bls377::Pair>(key_type)
}

/// Generate a new pair of paired-keys compatible with the '(ecdsa,bls377)' signature scheme.
///
/// If `[seed]` is `Some` then the key will be ephemeral and stored in memory.
fn ecdsa_bls377_generate_new(
&self,
key_type: KeyTypeId,
seed: Option<&str>,
) -> std::result::Result<ecdsa_bls377::Public, TraitError> {
self.generate_new::<ecdsa_bls377::Pair>(key_type, seed)
}

fn ecdsa_bls377_sign(
&self,
key_type: KeyTypeId,
public: &ecdsa_bls377::Public,
msg: &[u8],
) -> std::result::Result<Option<ecdsa_bls377::Signature>, TraitError> {
self.sign::<ecdsa_bls377::Pair>(key_type, public, msg)
}

fn ecdsa_bls377_sign_with_keccak256(
fn ecdsa_bls381_sign_with_keccak256(
&self,
key_type: KeyTypeId,
public: &ecdsa_bls377::Public,
public: &ecdsa_bls381::Public,
msg: &[u8],
) -> std::result::Result<Option<ecdsa_bls377::Signature>, TraitError> {
) -> std::result::Result<Option<ecdsa_bls381::Signature>, TraitError> {
let sig = self.0
.read()
.key_pair_by_type::<ecdsa_bls377::Pair>(public, key_type)?
.key_pair_by_type::<ecdsa_bls381::Pair>(public, key_type)?
.map(|pair| pair.sign_with_hasher::<KeccakHasher>(msg));
Ok(sig)
}


}
}

Expand Down
55 changes: 0 additions & 55 deletions substrate/primitives/application-crypto/src/bls377.rs

This file was deleted.

30 changes: 30 additions & 0 deletions substrate/primitives/application-crypto/src/bls381.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@
// limitations under the License.

//! BLS12-381 crypto applications.
use crate::{KeyTypeId, RuntimePublic};

use alloc::vec::Vec;

pub use sp_core::bls::bls381::*;

Expand All @@ -26,3 +29,30 @@ mod app {
#[cfg(feature = "full_crypto")]
pub use app::Pair as AppPair;
pub use app::{Public as AppPublic, Signature as AppSignature};

impl RuntimePublic for Public {
type Signature = Signature;

/// Dummy implementation. Returns an empty vector.
fn all(_key_type: KeyTypeId) -> Vec<Self> {
Vec::new()
}

fn generate_pair(key_type: KeyTypeId, seed: Option<Vec<u8>>) -> Self {
sp_io::crypto::bls381_generate(key_type, seed)
}

/// Dummy implementation. Returns `None`.
fn sign<M: AsRef<[u8]>>(&self, _key_type: KeyTypeId, _msg: &M) -> Option<Self::Signature> {
None
}

/// Dummy implementation. Returns `false`.
fn verify<M: AsRef<[u8]>>(&self, _msg: &M, _signature: &Self::Signature) -> bool {
false
}

fn to_raw_vec(&self) -> Vec<u8> {
sp_core::crypto::ByteArray::to_raw_vec(self)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@
// See the License for the specific language governing permissions and
// limitations under the License.

//! ECDSA and BLS12-377 paired crypto applications.
//! ECDSA and BLS12-381 paired crypto applications.

use crate::{KeyTypeId, RuntimePublic};
use alloc::vec::Vec;

pub use sp_core::paired_crypto::ecdsa_bls377::*;
pub use sp_core::paired_crypto::ecdsa_bls381::*;

mod app {
crate::app_crypto!(super, sp_core::testing::ECDSA_BLS377);
crate::app_crypto!(super, sp_core::testing::ECDSA_BLS381);
}

#[cfg(feature = "full_crypto")]
Expand All @@ -39,7 +39,7 @@ impl RuntimePublic for Public {
}

fn generate_pair(key_type: KeyTypeId, seed: Option<Vec<u8>>) -> Self {
sp_io::crypto::ecdsa_bls377_generate(key_type, seed)
sp_io::crypto::ecdsa_bls381_generate(key_type, seed)
}

/// Dummy implementation. Returns `None`.
Expand Down
4 changes: 1 addition & 3 deletions substrate/primitives/application-crypto/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,10 @@ pub use serde;
#[cfg(feature = "bandersnatch-experimental")]
pub mod bandersnatch;
#[cfg(feature = "bls-experimental")]
pub mod bls377;
#[cfg(feature = "bls-experimental")]
pub mod bls381;
pub mod ecdsa;
#[cfg(feature = "bls-experimental")]
pub mod ecdsa_bls377;
pub mod ecdsa_bls381;
pub mod ed25519;
pub mod sr25519;
mod traits;
Expand Down
2 changes: 1 addition & 1 deletion substrate/primitives/consensus/beefy/src/commitment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,7 @@ mod tests {
assert_eq!(
encoded,
array_bytes::hex2bytes_unchecked(
"046d68343048656c6c6f20576f726c642105000000000000000000000000000000000000000000000004300400000008558455ad81279df0795cc985580e4fb75d72d948d1107b2ac80a09abed4da8480c746cc321f2319a5e99a830e314d10dd3cd68ce3dc0c33c86e99bcb7816f9ba015dd1c9b2237e54baa93d232cdf83a430b58a5efbc2f86ca1bab173a315ff6f15bef161425750c028055e9a23947b73002889a8b22168628438875a8ef25d76db998a80187b50719471286f054f3b3809b77a0cd87d7fe9c1a9d5d562683e25a70610f0804e92340549a43a7159b77b0c2d6e1f8105c337a86cdd9aaacdc496577f3db8c55ef9e6fd48f2c5c05a2274707491635d8ba3df64f324575b7b2a34487bca2324b6a0046395a71681be3d0c2a001074884b6998c82331bd57ffa0a02cbfd02483c765b9216eab6a1fc119206236bf7971be68acaebff7400edee943240006a6096c9cfa65e9eb4e67f025c27112d14b4574fb208c439500f45cf3a8060f6cf009044f3141cce0364a7c2710a19b1bdf4abf27f86e5e3db08bddd35a7d12"
"046d68343048656c6c6f20576f726c642105000000000000000000000000000000000000000000000004300400000008558455ad81279df0795cc985580e4fb75d72d948d1107b2ac80a09abed4da8480c746cc321f2319a5e99a830e314d10dd3cd68ce3dc0c33c86e99bcb7816f9ba0182022df4689ef25499205f7154a1a62eb2d6d5c4a3657efed321e2c277998130d1b01a264c928afb79534cb0fa9dcf79f67ed4e6bf2de576bb936146f2fa60fa56b8651677cc764ea4fe317c62294c2a0c5966e439653eed0572fded5e2461c888518e0769718dcce9f3ff612fb89d262d6e1f8105c337a86cdd9aaacdc496577f3db8c55ef9e6fd48f2c5c05a2274707491635d8ba3df64f324575b7b2a34487bca2324b6a0046395a71681be3d0c2a00a90973bea76fac3a4e2d76a25ec3926d6a5a20aacee15ec0756cd268088ed5612b67b4a49349cee70bc1185078d17c7f7df9d944e8be30022d9680d0437c4ba4600d74050692e8ee9b96e37df2a39d1cb4b4af4b6a058342dd9e8c7481a3a0b8975ad8614c953e950253aa327698d842"
)
);
}
Expand Down
12 changes: 6 additions & 6 deletions substrate/primitives/consensus/beefy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,10 +142,10 @@ pub mod ecdsa_crypto {
#[cfg(feature = "bls-experimental")]
pub mod bls_crypto {
use super::{AuthorityIdBound, BeefyAuthorityId, Hash, RuntimeAppPublic, KEY_TYPE};
use sp_application_crypto::{app_crypto, bls377};
use sp_core::{bls377::Pair as BlsPair, crypto::Wraps, Pair as _};
use sp_application_crypto::{app_crypto, bls381};
use sp_core::{bls381::Pair as BlsPair, crypto::Wraps, Pair as _};

app_crypto!(bls377, KEY_TYPE);
app_crypto!(bls381, KEY_TYPE);

/// Identity of a BEEFY authority using BLS as its crypto.
pub type AuthorityId = Public;
Expand Down Expand Up @@ -184,10 +184,10 @@ pub mod bls_crypto {
#[cfg(feature = "bls-experimental")]
pub mod ecdsa_bls_crypto {
use super::{AuthorityIdBound, BeefyAuthorityId, Hash, RuntimeAppPublic, KEY_TYPE};
use sp_application_crypto::{app_crypto, ecdsa_bls377};
use sp_core::{crypto::Wraps, ecdsa_bls377::Pair as EcdsaBlsPair};
use sp_application_crypto::{app_crypto, ecdsa_bls381};
use sp_core::{crypto::Wraps, ecdsa_bls381::Pair as EcdsaBlsPair};

app_crypto!(ecdsa_bls377, KEY_TYPE);
app_crypto!(ecdsa_bls381, KEY_TYPE);

/// Identity of a BEEFY authority using (ECDSA,BLS) as its crypto.
pub type AuthorityId = Public;
Expand Down
8 changes: 4 additions & 4 deletions substrate/primitives/consensus/beefy/src/witness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ mod tests {
#[cfg(feature = "bls-experimental")]
use w3f_bls::{
single_pop_aggregator::SignatureAggregatorAssumingPoP, Message, SerializableToBytes,
Signed, TinyBLS377,
Signed, TinyBLS381,
};

type TestCommitment = Commitment<u128>;
Expand Down Expand Up @@ -198,15 +198,15 @@ mod tests {
// from signed take a function as the aggregator
TestBlsSignedCommitmentWitness::from_signed::<_, _>(signed, |sigs| {
// we are going to aggregate the signatures here
let mut aggregatedsigs: SignatureAggregatorAssumingPoP<TinyBLS377> =
let mut aggregatedsigs: SignatureAggregatorAssumingPoP<TinyBLS381> =
Copy link
Member

@davxy davxy Jul 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only a test, but would be nice to encapsulate the BLS backend in primitives/core bls module.
Maybe we can construct a stand alone function exposing the aggregated signature there?
Is required only for this test?

Also this test doesn't have any assertion.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I implemented such a thing during the Prague hackathon: w3f@53d90d3#diff-8b079a4b5fd964b13e8f4c7da10f3a924d015f1dec8f4bad2059eb3ab20ca94fR246 I asked @AlistairStewart if I should port those efforts here. He disagreed. He said that there was a plan to publish signature accumulation (either Merkle root or BLS aggregation) in BEEFY messages but that plan was scrapped. That is why you see "witness" only shows up in test. The current plan is that the aggregation should entirely happens by the relayer/prover.

Having said that, I'm happy to do whatever gets this pull request approved >:-). I could remove the test entirely or if you really insists that the aggregation code shows up in the test or in the backend, I'd think Al won't object if the approval of the pull request hanging on this, his concern was mostly time spend on something we won't use I assume.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was mostly a nitpick. What about the experimental feature and not exposing the bls12-377 stuff in keystore and sp-io?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll remove bls12-377 from sp-io and application crypto and remove as much as needed to make it compile. I'd keep the experimental feature for now. I'd remove it in the next pull request when I implement proof of possession check, without that BLS aggregations are not secure.

SignatureAggregatorAssumingPoP::new(Message::new(b"", b"mock payload"));

for sig in sigs {
match sig {
Some(sig) => {
let serialized_sig : Vec<u8> = (*sig.1).to_vec();
aggregatedsigs.add_signature(
&w3f_bls::Signature::<TinyBLS377>::from_bytes(
&w3f_bls::Signature::<TinyBLS381>::from_bytes(
serialized_sig.as_slice()
).unwrap()
);
Expand All @@ -219,7 +219,7 @@ mod tests {

// We can't use BlsSignature::try_from because it expected 112Bytes (CP (64) + BLS 48)
// single signature while we are having a BLS aggregated signature corresponding to no CP.
w3f_bls::Signature::<TinyBLS377>::from_bytes(witness.signature_accumulator.as_slice())
w3f_bls::Signature::<TinyBLS381>::from_bytes(witness.signature_accumulator.as_slice())
.unwrap();
}

Expand Down
Loading
Loading