search

phoreproject / bls

Go implementation of the BLS12-381 pairing
Apache License 2.0
89 stars 31 forks source link

Align uncompressed serialization w/zkcrypto spec #2

Closed nmarley closed 5 years ago

nmarley commented 5 years ago

Hi! I've been following this code base for a couple weeks now, and love the work. (Esp. the bugfix for pubkeys w/0 at the beginning, which was driving me crazy for the last week.)

It looks like you are implementing the spec here:

https://github.com/zkcrypto/pairing/tree/master/src/bls12_381

If this is the case, it looks like the serialization for the latest commit (uncompressed pubkeys) doesn't match exactly. This PR is an attempt to help it line up w/that spec.

Since all coords use a max of 381 bits, there should be 3 bits at the beginning of the serialization even for uncompressed pubkeys/sigs ((48 * 8) - 381), so the extra byte shouldn't be necessary.

The interface I've added to G1Affine and G2Affine allows quick serialization and deserialization of bytes which represent the coords, but also allows to easily swap G1 and G2 for implementations such as https://github.com/Chia-Network/bls-signatures/blob/master/SPEC.md (which I am working on).

Also, I believe in the near future it should be fairly easy to combine DeserializePubKey for both compressed and uncompressed based on the size of the byte slice. I will probably work on that in a few hours, and hopefully this PR is acceptable for this repo.

nmarley commented 5 years ago

Note: this work to combine serialization methods is based on this commit and done here: https://github.com/nmarley/go-bls12-381/tree/combine-deserialize-methods

If this PR gets merged then I will open a PR for that branch also.

meyer9 commented 5 years ago

Awesome! I'll look through this. The uncompressed pubkey serialization and deserialization was mostly just me testing. I didn't know there was actually a spec for that yet. Also, I'm going to be merging in an optimized version of the library soon which has a much different interface for Fq, so you may want to base your future commits on that (fqrepr) branch.

nmarley commented 5 years ago

Thanks for reviewing!

I believe using slices is more idiomatic due to their flexibility. I rarely see fixed-size arrays being returned from function calls. I also think it would be more difficult to switch G1/G2 if arrays are used.

meyer9 commented 5 years ago

Hmm... I think slices might be a good idea if this library were to support both the options for G1/G2 as public keys, but I'm not sure if I see a benefit to using slices if we restrict public keys to using only one or the other. Maybe this part should be removed from this library all together as it's application specific whether to use G1/G2 for public keys.