BLS Compression c_flag

[kirk-baird]

Hey,

I was just looking for clarification on the use of the c_flag in BLS G1 and G2 compression.
From reading:
https://github.com/ethereum/eth2.0-specs/blob/master/specs/bls_signature.md#g1-points
it was my interpretation that the c_flag / c_flag1 should always be set to 1.

In looking through the test_bls.yml I noticed that for both G1 (public keys) and G2 (Signatures) don't have the leading bit set to 1.

Specifically in relation to case03 and case04 output I notice that neither of these have a leading 1 bit.

Now I know the c_flag doesn't impact the decompressed (x, y) values and I'm not certain of its function so I thought I'd raise this to clarify.

Cheers Kirk

=======================================================
Edit:
Upon further inspection I see in case02, the 7th input {domain: 0x01, msg: 5656...}
Output is: 0x996ebb76ef93b5c1b9b4adfa60d8526cbf64a3d681dc1aa4d0579c1961f28f0ec14622d6f5688e6d29a4873778b4f0fe
Which has the 2^383 bit set to 1 when the y value is odd.
Should it not be the 2^381 bit (i.e. a_flag)?

Again no impact on the point values/signing/verifying just mentioning for consensus.

[benjaminion]

Yes, I'm currently integrating these tests into our implementation and have found the same issues (both with c_flag1 and a_flag1) for the compressed output. Uncompressed output is fine.

In addition, I think the flag(s) are being applied to the real part of the X coordinate (i.e. x2) rather than the imaginary part (x1) as required by the spec. Admittedly, the spec could be clearer on this point.

[benjaminion]

For reference, the summary of issues I found with the test data is here: Consensys/teku#401 (comment)

Reproduced below for reference:

 * The point compression used is broken.
 * Specifically:
 *   > The c flag is missing
 *   > The a flag is inverted and is at the wrong bit position
 *   > The compressed coordinate ordering is inverted: should be [Im, Re], but is [Re, Im]
 *   > The a flag doesn't always match the Y-coordinate, which matters when interpreting
 *     compressed input data, such as for the aggregation tests.
 * Also
 *   > Our yaml ingester doesn't like the test case 07 input data format (a missing "-")
 * Finally, note that the spec and test data will likely be changing.

See here and here for more on the Y co-ord selection issue.

[kirk-baird]

Excellent thanks :)

[ChihChengLiang]

@kirk-baird I'm working on the broken bit flags. We'll have an update in BLS signature library and generate the correct tests soon ethereum/eth2.0-test-generators#22.
@benjaminion will investigate and fix the [Im, Re] ordering and the case 07 yaml format soon.

[ChihChengLiang]

Hi @kirk-baird @benjaminion, the to be generated test would look like this https://gist.github.com/ChihChengLiang/328bc1db5d2a47950e5364c11f23052a . Could you please confirm if it works?

[benjaminion]

@ChihChengLiang - this is a thing of beauty. All the tests ran perfectly with no need to modify the input at all. So, assuming that you and I didn't independently make the same mistakes, it LGTM 😃

Thank you very much for this ❤️

[kirk-baird]

Works for me too 😃

Thanks heaps for this!!

[ChihChengLiang]

BLS tests updated. Waiting for another confirmation from Nim status-im/nim-blscurve#20 to close this issue.

[mratsim]

Closing, it's working fine on our side :).