search

decentralized-identity / bbs-signature

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

Update fixtures for new h2s and sign #248

Closed BasileiosKal closed 1 year ago

BasileiosKal commented 1 year ago

Update fixtures to accommodate the updates in #243

Also change the number of bytes that will be "hashed" to e and s to 32 instead of 48 to avoid an extra, unnecessary hash.

(note that those bytes will be "extended" to 48 pseudo random bytes inside hash_to_scalar either way).

Not ideal still (we call expand_message twice while it could be once). This will be fixed if we decide to rm s so IMO the current procedure is fine for now.

tmarkovski commented 1 year ago

I’m having issues with the calculate_domain procedure in this PR. Specifically, step 4. dom_array = (Q_1, Q_2, L, H_1, ..., H_L) lists the L argument coming after Q_1 and Q_2, but this doesn't work against the fixtures. If I change that to (L, Q_1, Q_2, H_1, ..., H_L) such that L is at position 1, then the fixtures pass. I don't know which option is the correct one, but it seems the spec and the fixtures are out of agreement.

BasileiosKal commented 1 year ago

Hey @tmarkovski! Thanks for catching this 🙌 Updated the document to match the test vectors (i.e., use dom_array = (L, Q_1, Q_2, H_1, ..., H_L)). Not sure where the inconsistency was introduced but IMO it's this way.

BasileiosKal commented 1 year ago

Discussed on WG call on 6th of Mar. Test vectors are cross validated. Will close after reviews and @tmarkovski issue is resolved!

Wind4Greg commented 1 year ago

Reminder as mentioned on March 6th call all test vectors are validating against my implementation at https://github.com/Wind4Greg/grotto-bbs-signatures/tree/spec-update.

tmarkovski commented 1 year ago

Issue is resolved, test vectors pass and implementation matches the spec.

BasileiosKal commented 1 year ago

Discussed on WG call! Approved. Test vectors cross validated by multiple implementations! Merging. Thanks everyone!

christianpaquin commented 1 year ago

I started to update my implementation to match the updated spec. Hopefully I'll be close to done by our next meeting