BLS Core Crypto attempt #2

[drskalman]

We have cherry picked all crypto related changes from pull-request #13311 in order to facilitate its review and merging. This is a starting point. The goal is to make as much as possible of the bls crypto code generic on the choice of the curve (BLS12-377 and BLS12-381). The signature scheme should follows the scheme described in https://eprint.iacr.org/2022/1611 for individual and aggregated signatures.

---

This PR provides:

• generic BLS12 implementation in sp-core::crypto module
• concrete BLS12-377 and BLS12-381 implementations using the generic one
• BLS12-381 in sp-application-crypto
• BLS12-381 support in the Keystore trait methods
• BLS related host functions were removed from sp-io.
  • as far as I understood verification is eventually required in the runtime to verify equivocations. Given the sporadic nature of the operation probably is better to do that in wasm and prevent introducing host functions (that can't be removed once introduced)
  • an unimplemented! call has been inserted in the RuntimeAppPublic implementation (implementation is still required)

[davxy]

@drskalman please have a look

[davxy]

@drskalman there are a couple of tests that fail (are failing with the original code as well).

The tests are in sp-core::bls:

• test_vector_should_work
• test_vector_by_string_should_work

Are these supposed to be successful?
Where are the test vectors taken?

[davxy]

@drskalman @burdges

Naming

(marginal... but somehow important) what is an appropriate name for this primitive? Plain bls seems pretty generic and not 100% accurate since this doesn't introduce a plain bls signature scheme.

Maybe a more appropriate name should be found for the implementation?

edit: Is the chosen name nugget?

[acatangiu]

So I'm not sure which reason is "strong" enough to halt moving forward at this stage.

I was not suggesting we stop moving forward with this PR (and future BEEFY ones), but I am interested in having clear communication on the decisions we make and their alternatives and consequences.

Like I said, looking at this PR, it was not clear why we choose one or the other - I myself cannot have a preference as it is outside my area of knowledge, but I expect any decision on contested alternatives to be explained well enough for most to understand said decision.

Having said that, I did get the gist of the decision from #13618 (comment)

[burdges]

edit: Is the chosen name nugget?

Yes, the create was named after the name selected for that paper.

[Lederstrumpf]

Chatted with @drskalman on this again today.
In my mind, what's primarily at odds here (aside from bls-like vs nugget-bls) are the following two contentions:

1. This PR being in limbo blocks other work on BEEFY, and we should resolve this.
2. Introducing a BLS interface that may change significantly, especially if migrating to nugget-bls, could result in technical debt. This PR doesn't expose a runtime I/O host interface nor the RuntimePublic trait, but this doesn't prevent, say, a parachain team, from implementing the missing pieces prematurely, and requiring storage migrations etc. down the line.

To reconcile both and simultaneously give us more time to converge wrt. nugget-bls, I've gated the bls primitives behind a new feature flag bls_non_production, as previously hinted towards here.

I'm not married to the flag's name - other options include non_production_bls_crypto for consistency with the full_crypto feature, or experimental_bls_crypto, and I welcome other suggestions, so far as the name discourages premature usage.

[Lederstrumpf]

before continuing the catch-up game with master:
@burdges @davxy @drskalman (since Adrian's 👍'd & André's ooo): Any feedback on the above?

i.e.

1. good with the feature flag in general?
2. good with the name bls_non_production?

Otherwise, please suggest alternatives.
Let's try to resolve this soon.

[drskalman]

1. good with the feature flag in general?
2. good with the name bls_non_production?

I'm fine with "bls_non_production" flag (both for its name and its existance) if it helps us merge. If it makes trouble later down the line we could always reconsider it. @davxy probably knows better about its implication in the beefy primitive's code.

[drskalman]

Plain bls seems pretty generic and not 100% accurate since this doesn't introduce a plain bls signature scheme.

The 'bls' part o

[drskalman]

Plain bls seems pretty generic and not 100% accurate since this doesn't introduce a plain bls signature scheme.

The 'bls' part of the signature and the proof of possession proposed in the paper and implemented in w3f-bls (but not the one implemented in the ring-vrf/nugget_bls) are complying with IETF standard for BLS signatures. We just provide this extra parts (the extra Signature and the extra public key in G1) to make the verification more efficient. Nonetheless, a verifier which implements the plain BLS scheme in the IETF standard, is still able to verify, both the individual and aggregated signatures and the proof of possession.

There were also talks about making the extra part optional in Substrate as in if isn't there then, just perform plain bls verification instead.

So maybe calling it Plain 'bls' isn't far fetched and call it something else might gives the impression that we are not complying with the standards.

Having said that, I'm not particularly opposed to calling the crypto module nugget-bls to indicates that we are also generating the extra bits for efficiency, but we'll be in the situation of substrate nugget-bls being incompatible ring-vrf/nugget_bls. But it might not be a big deal. I'll leave it to @davxy to decide.

[davxy]

So, my understanding is that this can act plain BLS signature but ships an extra schnorr proof in G1 that doesn't use pairings to verify (signature size = bls-sign-size + 2 * secret-key-size).

My 2 cents:

• I would go ahead and merge this if it comply with what Beefy requires now[^1]
• would be really awesome if in the future we could swap this with nugget-bls (if in the end we find out it is superior for whatever reason)
• no_std compat of verification: if I understood correctly nugget-bls at the moment builds in no-std but panics. We currently require verification to run on no-std

[1] Just to be clear, I've not followed the Beefy developments and but I've just provided this generic interface to the w3f-bls backend, so maybe @acatangiu @Lederstrumpf @andresilva are more expert than me to tell if there are some requirement not satisfied.
Currently as you may notice, the verify is not forwarded to the host and should be performed in wasm. Not a big deal (IMO) since the crate is no-std and the operation should be very sporadic.

All in all, @Lederstrumpf I think we can merge this gated by the feature flag (I suggest bls-experimental as feature name)

[drskalman]

Now that is clear that APK proofs do not work out-of-the-box for Ethereum (to Polkadot bridge) (due to the fact that each committee sign different messages) and we need a new proof system for that, @swasilyev and @AlistairStewart see little value in generating the Polkadots-to-X proofs using BLS12-381 and rather using BLS377 instead given the following benefits:

• Faster proof generation in Polkadot ecosystem.
• no need for a new powers of tau cermony.
• no need to modify current APK proofs code.
• no need for new FFT strategy.

So I've added the bls377 to application-crypto and the keystore and we are going to used it in beefy.

[Lederstrumpf]

I suggest bls-experimental as feature name

ack name change to bls-experimental

So I've added the bls377 to application-crypto and the keystore and we are going to used it in beefy.

ack

I'm fine with "bls_non_production" flag (both for its name and its existance) if it helps us merge. If it makes trouble later down the line we could always reconsider it.

agreed, and let me be clear that the purpose of this flag is to be a temporary gatekeeper until we've settled on a production bls lib & API.

Just to be clear, I've not followed the Beefy developments and but I've just provided this generic interface to the w3f-bls backend, so maybe @acatangiu @Lederstrumpf @andresilva are more expert than me to tell if there are some requirement not satisfied.

We haven't had any related changes to BEEFY since we've coordinated requirements with @drskalman.

Thanks to @drskalman & @davxy for all your patience & effort in pushing this PR!
Let's merge 👉 👈