search

status-im / specs

Specifications for Status clients.
https://specs.status.im/
MIT License
14 stars 14 forks source link

Added public key compression specs #137

Closed Samyoul closed 4 years ago

Samyoul commented 4 years ago

What has changed?

I've added detailed specs for the implementation of public key compression and decompression. The specifications detail the use of the following multiformat features:

multiformat is used to ensure that the implementation has as much flexibility and robustness as feasible.

Why make the change?

The usage of key de/compression is outside the typical usage of public keys and requires a degree of background knowledge to correctly implement. The purpose of this specification change is to provide this needed background knowledge.

Please also see

oskarth commented 4 years ago

Thanks for the spec PR! Is there any chance we could people from Core (or more broadly, Status client implementers like desktop) more involved in these spec reviews as well? Currently I only see @cammellos as part of Core on reviews, where the rest are part of Nimbus/Vac.

cc @andremedeiros @hesterbruikman @iurimatias

arnetheduck commented 4 years ago

There's a few things I don't understand here:

arnetheduck commented 4 years ago

also, where did the BLS serializations come from? the process of standardizing them has not settled yet cc @mratsim

Samyoul commented 4 years ago

@arnetheduck

  • where do these keys expected to appear? ie are they a pure UX feature or do the keys appear on the wire?

Yes, this functionality is (currently) purely for UX purposes, see my comment here https://github.com/status-im/status-go/issues/1937#issuecomment-638987966 and the issue on https://github.com/status-im/status-react/issues/10325

  • what are the BLS keys used for
  • why both G1/G2 field variants?

In my implementation I added BLS key handling for the sake of providing functionality that was simple to implement and potentially useful in the future. But on reflection I can't justify the mandatory use of BLS keys in the spec. I'll make them a RECOMMENDATION in the case that there are UX reasons for using BLS keys.

also, where did the BLS serializations come from? the process of standardizing them has not settled yet

The BLS serialisation came from the zkcrypto spec https://github.com/zkcrypto/pairing/tree/master/src/bls12_381#serialization

arnetheduck commented 4 years ago

In my implementation I added BLS key handling for the sake of providing functionality that was simple to implement and potentially useful in the future

YAGNI / it's going to be obsolete by the time of that future

Samyoul commented 4 years ago

@cammellos and @gravityblast would you mind giving me your thoughts on this spec submission?

@decanus would you mind re-reviewing the changes made to this submission?

@oskarth do you feel that the rationale given is sufficient and gives sufficiently specific information about the places that would use key "compression"?

Thank you

oskarth commented 4 years ago

@Samyoul seems good to me, thanks!

Samyoul commented 4 years ago

@decanus would you mind unblocking the merge of this PR? Thank you. 🙂