search

bitcoinjs / tiny-secp256k1

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

Find low r grinding #22

Closed junderw closed 5 years ago

junderw commented 6 years ago

Implementing the usage of grinding for low R values. Ref: https://github.com/bitcoin/bitcoin/pull/13666 @achow101 @sipa @gmaxwell

Note: Buffer.prototype.writeUIntLE added a bounds check for byteLength in node v10, so 6 bytes is the largest value we can write.

Currently since npm test runs tests on native JS and CPP as well, we are sure that they are consistent.

My only reservation is the hackey CPP (if we need to grind over 255 times, it breaks... but flipping a coin 255 times and getting all heads is such a small chance... I wonder if it is worth it to fix the hackey-ness for the sake of correctness? Would this introduce some sort of bug? I would like if some people can review this)

Also, I wonder if this is even necessary... in the spirit of "following the Core implementation" I have created it... but wonder if it's worth it.

junderw commented 6 years ago

Decided to make the lowR grinding optional... as we might actually decide to turn it off down the road. (we sign hundreds of inputs at a time, and having an extra 50% of time added on to that would be a pain... hehe)

We can then offer an optional flag in bitcoinjs-lib and default it to do whatever Core does / is widely accepted.

dcousens commented 6 years ago

@junderw appreciate the optional :+1:

junderw commented 5 years ago

Closed in favor of #23