Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement low R signing #248

Open
brianddk opened this issue Oct 21, 2020 · 5 comments
Open

Implement low R signing #248

brianddk opened this issue Oct 21, 2020 · 5 comments

Comments

@brianddk
Copy link

brianddk commented Oct 21, 2020

Bitcoin Core v0.17.0 introduced R-value grinding to ensure that all DER signatures are 71 bytes. In involves looping through nonce values till you produce a smaller sig and deterministically stopping at the first one.

Electrum recently rolled it out so the code could be lifted from there.

@dgpv
Copy link
Contributor

dgpv commented Oct 21, 2020

I've implemented for bitcointx: Simplexum#54

For bitcoinlib that might be more involved because it still uses openssl by default

@brianddk
Copy link
Author

brianddk commented Oct 21, 2020

@dgpv Awesome. Is your codebase close enough issue a PR, or are you soliciting others to do a little copypasta to bring the changes upstream?

For bitcoinlib that might be more involved because it still uses openssl by default

Well the OpenSSL libs already have malleable signatures, so you don't even need a sign call that supports nonce, just loop the sig call till you get a small one. Not sure if the OpenSSL sigs get that small though.

But yes... I always call use_libsecp256k1_for_signing(True) to ensure I don't get the OpenSSL lib.

@dgpv
Copy link
Contributor

dgpv commented Oct 21, 2020

My codebase in this area is very different now. secp256k1 is used exclusively and not optionally. key.sign() in bitcointx is still descended from _sign_with_libsecp256k1 that I submitted to bitcoinlib long ago, and it is likely that adapting the code from bitcointx that I referenced would be easier than adapting from electrum or writing anew.

to bring the changes upstream?

Note that I do not consider bitcoinlib 'upstream' to bitcointx - it diverged significantly both in code and in overall approach ("swiss army knife" vs "specialized and strict"). But watching PRs and issues in bitcoinlib gives ideas for how to improve bitcointx, and of course anyone is free to get the code from bitcointx and port to bitcoinlib when it makes sense.

@dgpv
Copy link
Contributor

dgpv commented Oct 21, 2020

just loop the sig call till you get a small one. Not sure if the OpenSSL sigs get that small though.

The code in Bitcoin Core and electrum is not grinding for smaller size. It specifically grinding for 'low R' value, and the smaller size is the consequence. AFAIU, you could have a signature that is 70 bytes long, but still has high R. If you just grind for size, it is likely that the lib will sometimes produce signatures that are distinguishable from signatures made with Core or electrum. This might have privacy implications.

@brianddk
Copy link
Author

anyone is free to get the code from bitcointx and port to bitcoinlib when it makes sense.

@dgpv Thanks... If no one else picks up the change, I'll be happy to draft the PR. Low level object marshaling in Python is a rather new realm for me, so it might be slow going.

If you just grind for size, it is likely that the lib will sometimes produce signatures that are distinguishable from signatures made with Core or electrum. This might have privacy implications.

Yep... I meant to imply "grind till you get a good enough R". At least that is what I would suggest and will try to fold into the PR.

@ghost ghost mentioned this issue Apr 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants