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

move libsecp256k1 calls to separate package #9006

Open
ecdsa opened this issue Apr 15, 2024 · 2 comments
Open

move libsecp256k1 calls to separate package #9006

ecdsa opened this issue Apr 15, 2024 · 2 comments

Comments

@ecdsa
Copy link
Member

ecdsa commented Apr 15, 2024

Currently, some projects such as python-nostr use cffi based python packages such as secp256k1or coincurve. In contrast, Electrum uses ctypes in order to call libsecp256k1 functions in ecc.py and ecc_fast.py. It would make sense to create a standalone python package for that. That package could be used by other projects, and it would free those projects from importing cffi related dependencies.

Note that some of the methods in ecc.py use cryptography (encrypt_message, verify_message). I think we should not import cryptography in the new package, but rather move those methods to electum/crypto.py.

Not sure how to name the new package. IMaybe python-libsecp256k1 ?

@SomberNight
Copy link
Member

SomberNight commented Apr 15, 2024

Not sure how to name the new package. IMaybe python-libsecp256k1 ?

I strongly think it should have a distinct name. For example, coincurve is a good name because you can easily distinguish it from rusty's secp bindings. python-libsecp256k1 is not a name that can be easily distinguished from rusty's bindings. We could call it electrum-secp256k1. If for some reason we did not want electrum in the name, we could call it secp256k1-ctypes.

That package could be used by other projects, and it would free those projects from importing cffi related dependencies.
Note that some of the methods in ecc.py use cryptography (encrypt_message, verify_message). I think we should not import cryptography in the new package, but rather move those methods to electum/crypto.py.

Right, to make the new package useful, I think it should

  • depend only on the python stdlib (ctypes, hashlib, etc) and the libsecp256k1 C library (as a .so/.dll shared library)
  • "just" move the current ecc.py and ecc_fast.py modules from electrum into the new package, and have electrum depend on the new package
    • some parts of ecc.py might have to be moved to bitcoin.py, crypto.py, etc
    • e.g. for ECPubkey.encrypt_message (and decrypt_message also), it would make sense not to put them into the new package. Instead, there could be a new module in electrum, which adds a subclass of ECPubkey, which adds the encrypt_message method. e.g. There would be electrum_secp256k1.ecc.ECPubkey and electrum.ecc.ECPubkey(electrum_secp256k1.ecc.ECPubkey), and the base class would not have an encrypt_message method, but the child class would. (but this is just an idea, there are other ways ofc)
    • e.g. some of the "usermessage" stuff could go to bitcoin.py, as it is more to do with the bitcoin-specific "signmessage" than libsecp or ecc
  • the big question is what to do about wheels
    • electrum-secp256k1 should be distributed on PyPI
    • electrum would depend on electrum-secp256k1 and install it from PyPI
      • or maybe electrum could use electrum-secp256k1 as a git submodule
    • I think as a first iteration, and to keep things simple, there could be no binary wheels, only an sdist for electrum-secp256k1 on PyPI
    • just like with electrum right now, the user would be expected to provide the .so file
    • contrib/make_libsecp256k1.sh should probably also be moved to the new package
      • although it might be needed for the build scripts in electrum as well...
    • even if we decided to distribute binary wheels for electrum-secp256k1, it might be best to not use them as part of electrum's build process (force pip to use sdist and build from source at that time)
      • note that the wine build does not have a C compiler atm
      • still, consider Linux users doing pip install . on an electrum tar.gz or git clone. If we provide binary wheels for the secp package, we probably want a high bar of release security there too...

@ecdsa
Copy link
Member Author

ecdsa commented Apr 15, 2024

  • we can use the name electrum-secp256k1
  • at this point, we do not know whether other projects will use that package. Therefore, I think we should not spend resources distributing wheels.
  • in the case of python-nostr, the cffi-related dependencies won't be removed, because python-nostr also uses cryptography. So, the only thing that would get removed is secp256k1. That might not be compelling enough for them to use it. However, if we decide to distribute python-nostr in an Electrum plugin, we will want something that is pure python. Thus, we might have to fork it.
  • Before we start all this, it would probably be wise to wait until we have a working plugin using python-nostr. At this point, there is no real need for this.

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

3 participants