decentralized-identity / bbs-signature

The BBS Signature Scheme
https://identity.foundation/bbs-signature/draft-irtf-cfrg-bbs-signatures.html
Apache License 2.0
78 stars 26 forks source link

Clarified scalar operations #184

Closed christianpaquin closed 2 years ago

christianpaquin commented 2 years ago

Added explicit mod r and moved the scalar multiplications before additions to avoid op-priority confusions.

BasileiosKal commented 2 years ago

LGTM.

Just a note. maybe we are encouraging people to do the xy + z operations and then mod r?? which I don't think is a good idea in practice (it should be ((x y mod r) + z) mod r ??).

Another option would be to define that all operations between scalars happen in the multiplicative group mod r?? That would be the more "mathematically correct" description, although I’m uncertain if it would be the clearer one.

I'm definitely overthinking this though. I think we should move with merging this 😄

christianpaquin commented 2 years ago

Just a note. maybe we are encouraging people to do the xy + z operations and then mod r?? which I don't think is a good idea in practice (it should be ((x y mod r) + z) mod r ??).

It's true, but it also becomes cumbersome. Your suggestion for a note somewhere that all calculations in that field is mod r. I simply wanted to avoid someone leaving a larger scalar and then trying to multiply that to the elliptic curve point; result would be validate but would be very inefficient. A simple mod r at the end indicates it's ok to apply the modulo at the end of the equation (confirming my interpretation of the spec).

tplooker commented 2 years ago

Multiple reviews and approvals, merging.