bitcoinjs / tiny-secp256k1

A tiny secp256k1 native/JS wrapper
MIT License
90 stars 55 forks source link

Fix discrepancies between JS and Native implementations #48

Closed junderw closed 4 years ago

junderw commented 4 years ago

Fixes #5

@yuki-js Please review.

junderw commented 4 years ago

I tested the native implementation manually for acceptable values.

Native impl accepted truthy values ( 0, '', etc. were false, 57, 'abc', etc. were true) So the js impl will match that and allow truthy values and not perform strict type checks on the strict parameter.

yuki-js commented 4 years ago

C++ implementation normalizes if strict. https://github.com/bitcoinjs/tiny-secp256k1/blob/9f07a89d5d9838628e34ac76db9ce64d9d1d3372/native/addon.cpp#L343

junderw commented 4 years ago

C++ implementation normalizes if strict. https://github.com/bitcoinjs/tiny-secp256k1/blob/9f07a89d5d9838628e34ac76db9ce64d9d1d3372/native/addon.cpp#L343

No. It normalizes if NOT strict.

The reason why is because the C library we call is ALWAYS strict (will return false if S value is high).

The normalization function just makes sure the s value is low.

It's very hard to understand..... even for me.

yuki-js commented 4 years ago

Oh, I made a mistake. " if NOT strict" is correct.