TS implementation suggestion for #23

plume-sig / zk-nullifier-sig

Implementation of PLUME: nullifier friendly signature scheme on ECDSA

MIT License

133 stars 22 forks source link

TS implementation suggestion for #23 #34

Closed skaunov closed 1 year ago

skaunov commented 1 year ago

This one could do #23 in TS part. I really need some approve that I got things correctly before moving on to Circom part.

Also pay attention to the note in the initial commit for this issue, pls. I suspect that Rust implementation actually should panic when SEC1 decoding gives number greq then the field order. So the inconsistency might be much deeper.

wrapping around field instead of curve order
difference in decoding of the hash into an element

skaunov commented 1 year ago

[ ] think about breaking the interface with switching to bigint: remove excessive conversions, closer to the Rust implementation which initiated this issue, interface isn't stable anyway, but I'd double check this switch doesn't introduce potential exploitation

Divide-By-0 commented 1 year ago

It's entirely possible this is a great bug you've found, I hadn't considered that. Is the bug in all of the implementations or just the rust one?

skaunov commented 1 year ago

wraps correctly though on `c` reception, didn't check I'm still in the beginning of my ZK journey, so I'd appreciate an explanation of what `c` is. (Here, a link, in an email, in discussions, ...) X) If _it is_ a hash semantically, than maybe its type should be changed in the crates.

Divide-By-0 commented 1 year ago

Right. C is described in both the plume blog post and the slides, both of which are linked from the repo readme! Blog post will be easiest to understand.

skaunov commented 1 year ago

Closing this in favor of solution from the other end (taking Rust implementations to be consistent with TS).