python-graphenelib
python-graphenelib copied to clipboard
Migrate from secp256k1 to secp256k1prp.
secp256k1
is a python wrapper around a secp256k1
C library, which speeds up and provides various crypto functions to private key generation.
secp256k1prp
is a fork, which wraps a similar C library, secp256k1-zkp
, from bitshares-core, which adds support for Pederesen commitments / Range Proofs (hence, "prp").
As of time of this writing secp256k1prp
is also more maintained than secp256k1
, with pip wheel files provided for all major python platforms (wheel files contain the pre-compiled C library, for a specific platform, so the end-user doesn't have to compile it himself), see here: https://pypi.org/project/secp256k1prp/#files
This PR simply replaces secp256k1
with secp256k1prp
, but it could possibly look for either.
I would really love to add this patch. However, the issue is that secp256k1
is generally known, well maintained and trusted. Not saying that I don't trust you, but in order to be able to merge this patch, I'd need to be sure that the secp256k1 stuff is untouched and thus need ti review the entire fork. The impact on tampering with secp256k1
whould be much more massive.
How about using some sort of trigger that allows to switch the actual implementation from outside.
A simple USE_sec256k1prp=True
for instance would do the trick.
I merely want to ensure that devs that don't need it, can stick with the regular sec256k1
implementation.
That makes total sense, I'll do it.
Regarding reviewing my fork, it's not that hard: the python-wrapper code I changed is nothing drastic, just version bumping and some build option adjustments. See for yourself! https://github.com/ludbb/secp256k1-py/compare/master...jhtitor:master
Now, there are cffi bindings to the C code, and again, you may plainly see, that nothing suspicious is going on here, it's a 1:1 mapping.
Then, in setup.py
, you will find this:
# Version of libsecp256k1 to download if none exists in the `libsecp256k1`
# directory
LIB_TARBALL_URL = "https://github.com/sipa/secp256k1-zkp/archive/35932bb24e83257b737a8ab4da0816972f4c252a.tar.gz"
That is a reference to a very specific commit to the zkp library (mind you -- the same library that is used in bitshares-core), the commit is https://github.com/sipa/secp256k1-zkp/commit/35932bb24e83257b737a8ab4da0816972f4c252a . It is authored by Gregory Maxwell and is the defacto method for pedersen range proofs. There are other forks of the library, that also add this, but I've picked this commit specifically, because gmaxwell is a know and trusted member of the crypto community, and we basically live off his code anyways. (And I had the very same "fears" as you, I didn't want some random implementation, I wanted the implementation).
Also, for argument sake, if I turn evil, and release a bad future version, all this won't help. So I think maybe you could fork my repo and maintain it yourself? I've included build scripts to make windows and manylinux wheel files, it's pretty sweet.
That being said, I don't see myself ever touching this code again, it's stable, the pip files are built, and there's nothing more to do, really.