Rust format

[davxy]

as per title

[burdges]

We should mark this crate as deprecated btw

[davxy]

We should mark this crate as deprecated btw

But this crate is used by the new bls implementation for Substrate.

https://github.com/paritytech/substrate/pull/13618

What should we use then?

CC @@drskalman

[burdges]

We should use https://github.com/w3f/ring-vrf/tree/master/nugget_bls instead.

We started the bls-like crate in this repository long ago when we did not even know if we wanted signatures or public keys on G1 or G2, but the flexible EngineBLS approach here became obsolete when we selected the mixed G1+G2 trick: https://github.com/w3f/ring-vrf/blob/master/nugget_bls/src/lib.rs#L244

[drskalman]

The crate implements the scheme described in the paper. Any other implementation could be easily replace the backend as long as it implements the same scheme . So there is little use to delay the BLS and BEEFY integration everytime there is some new BLS library is implemented.

We should go ahead with the implementation base on this library so we can merge BLS on BEEFY as soon as possile, and then when nugget_bls is ready (moved to independent reposity, removed merlin dependency etc) we could discussed best backend for BLS in substrate. It should not be much work to replace backend as long as they conform to spec.

[burdges]

The crate implements the scheme described in the paper.

This is incorrect.

There should be two DLEQ proofs being used in the protocol there, first the public key itself is a cross-curve proof, and second pre-aggregation BLS signatures should be thin VRF proofs. It appears neither of these proofs appear in this crate.

Aggregation is slightly different too, in that nugget merges two equations, and one of those is seemingly not even supplied here.

So there is little use to delay the BLS and BEEFY integration everytime there is some new BLS library is implemented.

I suspect the real delays shall stem from people being confused by all the cruft I left in this crate.

Also EngineBLS shall complicate porting and maintenance, so let's avoid ever putting it into production.

[burdges]

The crate implements the scheme described in the paper.

This is incorrect.

There should be two DLEQ proofs being used in the protocol there, first the public key itself is a cross-curve proof, and second pre-aggregation BLS signatures should be thin VRF proofs. It appears neither of these proofs appear in this crate. Aggregation is slightly different too.

So there is little use to delay the BLS and BEEFY integration everytime there is some new BLS library is implemented.

I suspect the real delays shall stem from people being confused by all the cruft I left in this crate.

Also EngineBLS shall complicate porting and maintenance, so let's avoid ever putting it into production.

[drskalman]

We should mark this crate as deprecated btw

I'm would disagree. The flexibility of this library, helps us to implement, benchmark and compare various BLS schemes quite easily as we did for the paper and there are more benchmarks that @AlistairStewart has asked for and it is likely that are more other schemes in future that we need to benchmark.

Also EngineBLS shall complicate porting and maintenance

The flexibility of the library is partially due to EngineBLS abstraction.

The aggregation of public keys in the signature group, the verification based on it and the Chaum-Pedersen signatures were implemented in skalman-double-puclic-key-verify branch back in October when we wrote the paper. I have not merged it into master because substrate doesn't have support for hash type of 144 and 160 bytes and I didn't wanted to block @davxy from advancing. I'll add those to parity common and merge.

Any deviation from the paper, should be discussed, specified and documented to prevent future confusion and complication.

All we currently need in order to merge the BEFFY BLS support, is sign and verify for non-aggregated BLS signatures and I'd focus on those only to be able move BEEFY's pull request forward.

[burdges]

It's nice EngineBLS helped for the paper. I do not believe EngineBLS could ever be helpful in production, given EngineBLS::SignatureGroup = G1 is always optimal. Also.. It definitely caused much pain in the past. It'll likely do so again as arkworks traits evolve. And it'll just break whenever future optimizations change the pairing semantics.

I didn't notice your branch before sorry, but yes https://github.com/w3f/bls/blob/skalman-double-puclic-key-verify/src/chaum_pedersen_signature.rs works, but it the public key more than a signature. Abstractly, EngineBLS::PublicKeyGroup no longer makes sense. We always have EngineBLS::SignatureGroup = G1 but our public keys are much more G1 than G2, like that's the whole point. Also, the PoK in master right now serves no purpose.

If you glance at https://github.com/w3f/ring-vrf/blob/master/nugget_bls/src/lib.rs then you'll see PublicKeys are DLEQ proofs, Signatures are DLEQ proofs, and AggregateSignatures do exactly what's desired.