Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

BLS Core Crypto attempt #2 #13618

Merged
Lederstrumpf merged 49 commits into from
May 9, 2023
Merged

BLS Core Crypto attempt #2 #13618

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
b845210
Cherry pick all crypto related changes from pull-request #13311
drskalman Mar 15, 2023
081ced0
Merge branch 'master' into davxyn-skalman-core-bls-crypto
davxy Mar 20, 2023
f411d74
Merge branch 'master' into davxyn-skalman-core-bls-crypto
davxy Mar 20, 2023
ee61ddb
Import some stuff just if 'full_crypto' is on
davxy Mar 21, 2023
a1e0f13
Remove copyright year
davxy Mar 21, 2023
7023c14
Cleanup
davxy Mar 21, 2023
d92186d
First generic BLS draft
davxy Mar 21, 2023
409b2f4
Finalize generic implementation
davxy Mar 23, 2023
7177bb1
Restore tests
davxy Mar 23, 2023
9b320cd
Merge pull request #13 from davxy/davxyn-skalman-core-bls-crypto-generic
davxy Mar 23, 2023
58d4122
Fix rust docs
davxy Mar 23, 2023
1e331c0
Merge branch 'master' into davxyn-skalman-core-bls-crypto
davxy Mar 23, 2023
a630ead
Fix after master merge
davxy Mar 23, 2023
8a34867
Merge branch 'master' into davxyn-skalman-core-bls-crypto
davxy Mar 24, 2023
5c47d33
Fix after master merge
davxy Mar 24, 2023
fe65fce
Use double bls with G1 as signature group and verify individual signa…
drskalman Mar 28, 2023
b278ba7
Fix inclusions and types used within substrate
davxy Mar 29, 2023
3d50a72
Remove unused cruft
davxy Mar 29, 2023
994a805
Restore usage of upstream crates
davxy Mar 29, 2023
d728537
Fix test
davxy Mar 29, 2023
fbb2f47
Reduce the diff by aligning Cargo.lock to master
davxy Mar 29, 2023
bc52143
Application-crypto provides bls381
davxy Mar 29, 2023
eb8a7ab
Merge branch 'master' into davxyn-skalman-core-bls-crypto
davxy Mar 29, 2023
0acdf5c
Implement bls381 for local keystore
davxy Mar 29, 2023
26d9ce2
Merge branch 'master' into davxyn-skalman-core-bls-crypto
davxy Mar 30, 2023
9de1515
Use new generic keystore features
davxy Mar 30, 2023
267538f
import DoublePublickey[Scheme] from the bls-like root to be less conf…
drskalman Apr 4, 2023
e3c2af8
Merge branch 'master' into davxyn-skalman-core-bls-crypto
andresilva Apr 12, 2023
2a0a0ca
fix compilation
andresilva Apr 12, 2023
de48262
Apply suggestions from code review
davxy Apr 13, 2023
7d709da
Clean leftovers
davxy Apr 13, 2023
a2b7308
- update bls test vector after applying spec change recommendation.
drskalman Apr 13, 2023
478da9b
Merge branch 'davxyn-skalman-core-bls-crypto' of https://github.com/w…
drskalman Apr 13, 2023
426c5ae
Different hard junction ids for different bls12 types
davxy Apr 13, 2023
da36af4
update to new bls-like
drskalman Apr 13, 2023
856881a
Merge branch 'davxyn-skalman-core-bls-crypto' of https://github.com/w…
drskalman Apr 13, 2023
0c7a5f7
bls-like → w3f-bls
drskalman Apr 16, 2023
9218d9c
Make clippy happy
davxy Apr 16, 2023
58c2d94
update test vector after replacing hash and crop with hash to field.
drskalman Apr 19, 2023
6041fec
cargo fmt
drskalman Apr 19, 2023
f9b0da0
Merge branch 'master' into davxyn-skalman-core-bls-crypto
drskalman Apr 19, 2023
758a169
Merge remote-tracking branch 'upstream/master' into HEAD
Lederstrumpf Apr 25, 2023
b11659d
account for #13972
Lederstrumpf Apr 25, 2023
3a9c5ed
hide BLS behind "bls_non_production" feature flag
Lederstrumpf Apr 25, 2023
0bfbcfd
Merge remote-tracking branch 'upstream/master' into davxyn-skalman-co…
Lederstrumpf May 4, 2023
5a1a9cc
Remove Cargo.lock entries duplicated in merge
Lederstrumpf May 4, 2023
2e48a99
add bls377 to primitives/keystore and client/keystore
drskalman May 9, 2023
270b32e
rename feature `bls_non_production` to `bls-experimental`
drskalman May 9, 2023
35b9167
Merge branch 'master' into davxyn-skalman-core-bls-crypto
drskalman May 9, 2023
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
Prev Previous commit
Next Next commit
Merge branch 'master' into davxyn-skalman-core-bls-crypto
  • Loading branch information
@davxy
davxy committed Mar 29, 2023
commit eb8a7ab99ab370ea86dc582f795a99362aa2049c
1 change: 0 additions & 1 deletion Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions primitives/application-crypto/src/bls381.rs
View file
Open in desktop
Copy link
Contributor

@Lederstrumpf Lederstrumpf Apr 11, 2023

Choose a reason for hiding this comment

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

at some point, we were discussing adding bls(381), but hiding it behind a feature flag (not full_crypto, but a dedicated one to avoid production use). I may have missed some decisions/discussions in-between - you're now intentionally achieving the same by not implementing RuntimePublic?

Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ use sp_std::vec::Vec;
pub use sp_core::bls::bls381::*;

mod app {
use sp_core::testing::BLS377;
use sp_core::testing::BLS381;

crate::app_crypto!(super, BLS377);
crate::app_crypto!(super, BLS381);

impl crate::traits::BoundToRuntimeAppPublic for Public {
type Public = Self;
Expand Down
6 changes: 1 addition & 5 deletions primitives/core/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,8 @@ bitflags = "1.3"
array-bytes = { version = "4.1", optional = true }
ed25519-zebra = { version = "3.1.0", default-features = false, optional = true }
blake2 = { version = "0.10.4", default-features = false, optional = true }
schnorrkel = { version = "0.9.1", features = [
"preaudit_deprecated",
"u64_backend",
], default-features = false, optional = true }
schnorrkel = { version = "0.9.1", features = ["preaudit_deprecated", "u64_backend"], default-features = false, optional = true }
bls-like = {git = "https://github.com/w3f/bls", default-features = false}
Copy link

@burdges burdges Mar 29, 2023

Choose a reason for hiding this comment

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

Imho, we should not depend upon the EngineBLS trait here. It's completely useless. It'll also cause issues for portability and maintenance.

Copy link
Member

@davxy davxy Mar 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

bls crypto type within substrate has been defined to be generic.
This allowed to easily have both bls12-381 and bls12-377 with the minimum effort and code duplication.
To have such kind of generic we bounded the generic type to be an EngineBLS trait.
AFAIK is the only bound that allow us to do provide a generic implementation.
What you suggest as an alternative bound?

Copy link

@burdges burdges Mar 29, 2023

Choose a reason for hiding this comment

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

We should not imho use the bls-like crate in production. I started it as an experiment, and much has been added, but it was never cleaned up, so much of what is there no longer serves any purpose, and is generally a confusing poorly structured mess. https://github.com/w3f/ring-vrf/tree/master/nugget_bls was written to do exactly what beefy requires.

Copy link
Contributor Author

@drskalman drskalman Mar 29, 2023

Choose a reason for hiding this comment

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

I made this repo https://github.com/w3f/BCLS-BLS I'll start isolating the bls logic from nugget and bls-like to here and eventually relpace the backend.

Nonetheless I don't want this to become a blocker on this pull requset as, given that the output of the BLS-like matches the reference in the paper, BEEFY's integration is blocked on this, production ready BLS BEEFY is also blocked on apk proof adoption of BW6 on BLS12-381 and we are exposing no host functio.

Therefore, I don't think blocking the pull request due to some opinion about an abstraction in the backend is justified.

Copy link

@burdges burdges Mar 30, 2023

Choose a reason for hiding this comment

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

Or just use nugget_bls after changing whatever needs changing? At minimum it helps consolidate our code which should be hardened for non-constant-time elliptic curves.

I've found an unrelated annoyance with merlin, so I'll do some ark_transcript crate based on shake128 btw.

sha2 = { version = "0.10.0", default-features = false, optional = true }
hex = { version = "0.4", default-features = false, optional = true}
libsecp256k1 = { version = "0.7", default-features = false, features = ["static-context"], optional = true }
merlin = { version = "2.0", default-features = false, optional = true }
Expand Down
7 changes: 2 additions & 5 deletions primitives/core/src/bls.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -509,8 +509,7 @@ impl<T: BlsBound> CryptoType for Pair<T> {
mod test {
use super::*;
use crate::crypto::DEV_PHRASE;
use bls377::Pair;
use bls_like::TinyBLS377;
use bls377::{Pair, Signature};
Copy link
Contributor

@Lederstrumpf Lederstrumpf Apr 11, 2023

Choose a reason for hiding this comment

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

nit: could duplicate these tests for bls381

use hex_literal::hex;

#[test]
Expand Down Expand Up @@ -666,9 +665,7 @@ mod test {

#[test]
fn signature_serialization_doesnt_panic() {
fn deserialize_signature(
text: &str,
) -> Result<Signature<TinyBLS377>, serde_json::error::Error> {
fn deserialize_signature(text: &str) -> Result<Signature, serde_json::error::Error> {
serde_json::from_str(text)
}
assert!(deserialize_signature("Not valid json.").is_err());
Expand Down
You are viewing a condensed version of this merge commit. You can view the full changes here.