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

Signing with libsecp256k1 by default ? #9

Closed
dgpv opened this issue Oct 18, 2018 · 9 comments
Closed

Signing with libsecp256k1 by default ? #9

dgpv opened this issue Oct 18, 2018 · 9 comments

Comments

@dgpv
Copy link

dgpv commented Oct 18, 2018

currently, to sign with libsecp256k1, you need to check if it is available, with is_libsec256k1_available(), and then enable it with use_libsecp256k1_for_signing(True)

This was done for compatibility, as libsecp256k1 may be not available. Creating the code for making the ECC library pluggable, as suggested in petertodd#123 (comment), will require time and resources that we are not ready to expend.

In practice, we always enable libsecp256k1 for signing. We could just switch to use it by default, but this would mean that if the library is not available, the code will crash with ImportError exception.

@HelloZeroNet
Copy link

HelloZeroNet commented Nov 21, 2018

I made some modification to enable libsecp256k1 support in SignMessage function (PR:#12), but the early benchmarks are confusing on Win10, Python 3.7.1 64bit:

  • OpenSSL 1.0.2p compiled by indyproject: 3.22s
  • secp256k1 from coincurve pip (600k): 3.27s
  • secp256k1 extracted from secp256k1_vc120 nupkg (86k): 3.4s

Any idea why is it slower than using openssl and what the reason of the size difference in the dll?

Benchmark code:

from bitcoin.wallet import CBitcoinSecret, P2PKHBitcoinAddress
from bitcoin.signmessage import BitcoinMessage, VerifyMessage, SignMessage

address = "1N2XWu5soeppX2qUjvrf81rpdbShKJrjTr"
message = BitcoinMessage("hello")
key = CBitcoinSecret("5JsunC55XGVqFQj5kPGK4MWgTL26jKbnPhjnmchSNPo75XXCwtk")
signature = "HGbib2kv9gm9IJjDt1FXbXFczZi35u0rZR3iPUIt5GglDDCeIQ7v8eYXVNIaLoJRI4URGZrhwmsYQ9aVtRTnTfQ="  

use_libsecp256k1_for_signing(False)
signed = SignMessage(key, message)
s = time.time()
for i in range(1000):
    signed = SignMessage(key, message)

print("Signmessage (openssl) x 1000: %.2fs" % (time.time() - s))

use_libsecp256k1_for_signing(True)
signed = SignMessage(key, message)
s = time.time()
for i in range(1000):
    signed = SignMessage(key, message)

print("Signmessage (secp256k1) x 1000: %.2fs" % (time.time() - s)) 

@dgpv
Copy link
Author

dgpv commented Nov 21, 2018

reason of the size difference in the dll

debug/release build of the dll ?

On linux, when I increased number of iterations to 10000, there's about 5% difference

Signmessage (openssl) x 10000: 20.29s
Signmessage (secp256k1) x 10000: 21.37s

libsecp256k1 is faster for signature validation (optimized for it, to make tx validation fast), but may not necessary be faster for signature creation.

The difference may also be caused by the difference in python code paths, but I do not see any places that could be suspect, and do not feel like investigating/profiling it further.

I'm OK with just using openssl for sign_compact, because it is used only in sign_message(), and libsecp256k1 signing was added for RFC6979 deterministic signature generation for transactions, not for speed.

@HelloZeroNet
Copy link

I did the same test using coincurve and the signature speed is more than 10 times faster:

  • Verify x1000: 0.140s
  • Sign x1000: 0.227s

The code I used: https://gist.github.com/HelloZeroNet/c82198335b430b51d86f9d6484fd91c2

Do you have and idea why is the significant difference?

@dgpv
Copy link
Author

dgpv commented Nov 22, 2018

probably because coincurve do much more in C code, just calling secp256k1_ecdsa_recoverable_signature() and then secp256k1_ecdsa_recoverable_signature_serialize_compact()

versus python-bitcoinlib's version that do cec_key.recover() in python, for example

@dgpv
Copy link
Author

dgpv commented Nov 28, 2018

signing messages in the same way as in coincurve is now implemented in branch https://github.com/Simplexum/python-bitcointx/tree/sign_libsecp256k1_default

(pull request #14, I will merge it into master later, it needs more testing before this)

But now the speed for message signing is comparable with coincurve.

Signmessage (secp256k1) x 1000: 0.07s

@dgpv
Copy link
Author

dgpv commented Nov 29, 2018

When secp256k1_ecdsa_verify() is used in CECKey.verify(), the speedup is roughly 4x versus openssl's ECDSA_verify()

@HelloZeroNet
Copy link

I can confirm the performance is close to coincurve now:

Verifymessage (secp256k) x 1000: 0.17s 
Verifymessage (openssl) x 1000: 0.93s 
Verifymessage (coincurve) x 1000: 0.16s

Do you think if adding recover_compact_with_openssl function and make coincurve dll/so optional could be possible?

@dgpv
Copy link
Author

dgpv commented Nov 29, 2018

The changes in #14 do not use coincurve - just libsecp256k1 directly through ctypes. There's no need for additional dependency.

recover_compact_with_openssl() is an old, slow function that uses openssl - I just thought that the code might still be useful somehow, so did not delete it right away. Might do in the future, though. The main recover_compact() uses direct calls to libsecp256k1.

The changes make libsecp256k1 a hard dependency - bitcointx.core.key module won't load without libsecp256k1 present. This is the change that this issue was created for, in the first palce.

@dgpv
Copy link
Author

dgpv commented Nov 30, 2018

Pull request #14 merged.

libsecp256k1 is now required dependency, and is used for signing and verifying.

@dgpv dgpv closed this as completed Nov 30, 2018
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