refactor: move to coincurve

[a1denvalu3]

Migrate from secp256k1-py to coincurve bindings for the secp256k1 library.

fixes #592

[TheRealCheebs]

dumb comment: but it's nice when things fit nicely. 😄

[TheRealCheebs]

This PR addresses #592

[callebtc]

Is there a PublicKeyCompressed in coincurve? Should be a tiny bit faster.

[a1denvalu3]

I followed https://ofek.dev/coincurve/api/ and it doesn't appear to have it.

[TheRealCheebs]

awesome addition of benchmarks!

[callebtc]

nice benchmarks! this confirms that coincurve doesn't cause any performance issues. seems like it is more than 2x slower for the main BDHKE, but faster for schnorr sigs. that's a hefty price to pay. btw, I think the code for the benchmarks doesn't need to be included in the PR.

here are the results

[a1denvalu3]

Benchmarks show a drastic drop in performance. I'm questioning whether we should go ahead with this regardless.

[TheRealCheebs]

The original motivation behind this move was because coincurve was better maintained than secp256k1 and others like CLN used it for their python plugins? That is at least what was in the initial issue/suggestion from July 2024. Are there other driving factors that came up in the last 18 months?

[a1denvalu3]

The original motivation behind this move was because coincurve was better maintained than secp256k1-py

Yes this was the motivation, but we also want to keep performance in mind.

[TheRealCheebs]

I'm changing my take. secp256k1-py was last released November 2021. That was a bit more than a year before the first official release (0.2.0) of the secp256k1 library its composed of. They are now at 0.7.0, so a considerable amount of work has continued. And that work is missing from secp256k1-py. On the other hand, coincurve 21 explicitly upgraded the internal libsecp256k1 to version 0.6.0. It sounds like coincurve has moved towards removing all external runtime dependencies in v21 to reduce attack vectors. Its testing against modern python versions and has made a bunch of other security focused changes.

It seems like it's a big hit, especially when we look at the benchmarks, but I guess I wonder how it feels in the "real world". And I would personally be happy to trade some performance for safety in this space.

edit:
And there will continue to be things like #806 if we continue with secp256k1 (which feel less than ideal).

[a1denvalu3]

Benchmarks are now moved to a different branch

[prusnak]

Benchmarks show a drastic drop in performance

Any ideas why this is happening? Both packages just wrap the native C code. Is the performance hit in the C code (old libsecp256k1 vs new one) or is it because of binding code in Python?

[TheRealCheebs]

@prusnak From a line of Gemini questioning. It was pointing to Python-to-C translation layer and object lifecycle management as the slowdown. And it was stating that bench-marking is "over-blowing" the slowness vs what anyone would see in real world use cases.

1. Object Instantiation vs. Static Calls

The primary difference is that secp256k1-py often allowed for "rawer" access to functions, whereas coincurve is strictly object-oriented.

The Hit: Every time you create a PrivateKey or PublicKey object in coincurve, Python has to allocate memory, run the init method, and perform validation checks (like ensuring the key is within the valid curve range).

The Scale: If you are signing 10,000 messages and creating a new PrivateKey object for each one instead of reusing a single instance, the overhead of Python object creation will far outweigh the microseconds spent on the actual C-level ECDSA math.

2. Defensive Validation (The "Robustness" Cost)
   coincurve is designed to be "safest by default."

Point Validation: When you load a public key, coincurve doesn't just treat it as a blob of bytes. It calls secp256k1_ec_pubkey_parse to ensure the point actually lies on the curve.

Normalization: It often performs extra checks to ensure signatures are in the "Lower-S" form (to prevent malleability), which is a requirement for modern blockchains like Ethereum but adds a few extra logic gates in the wrapper code.

Comparison: secp256k1-py frequently bypassed these checks if you used the lower-level functional interface, essentially "trusting" that your input data was already sanitized.

3. CFFI vs. C-Extensions (The Bridge)
   Both libraries use CFFI (C Foreign Function Interface), but they use it differently:

Type Marshalling: coincurve does more "translation" between Python types (like bytes) and C types (like unsigned char*). It ensures that memory is correctly pinned and that pointers are handled safely to avoid segfaults.

The Context Object: libsecp256k1 requires a "context" (a pre-computed table of values) to run fast. coincurve manages a global context for you. While this is generally a performance win, the internal logic to ensure that context is thread-safe and properly initialized adds a tiny layer of "bookkeeping" to every call.

And the areas to clean up if we see real world performance hits would be around

Reuse Objects: Never call PrivateKey(seed) inside a loop. Create the object once and call .sign() repeatedly.

Batch Verification: If you are verifying many signatures, look for batching opportunities. coincurve is optimized for repeated use of the same PublicKey object.

Avoid Re-parsing: If you have public keys in byte format, parse them into PublicKey objects once and store the objects in a cache rather than re-parsing the bytes every time you need to verify a signature.

It looks like Coincurve is the definitive standard now for python bindings to the secp256k1 curve, and I think the trade offs between safety and robustness/maintained code between the two should trump the performance hits we see when thrown in sandboxed benchmarks.

[prusnak]

Point Validation: When you load a public key, coincurve doesn't just treat it as a blob of bytes. It calls secp256k1_ec_pubkey_parse to ensure the point actually lies on the curve.

I think this might be the cause of most of the performance issue we probably.

If you are signing 10,000 messages and creating a new PrivateKey object for each one instead of reusing a single instance, the overhead of Python object creation will far outweigh the microseconds spent on the actual C-level ECDSA math.

+1

And it was stating that bench-marking is "over-blowing" the slowness vs what anyone would see in real world use cases.

+1

It looks like Coincurve is the definitive standard now for python bindings to the secp256k1 curve, and I think the trade offs between safety and robustness/maintained code between the two should trump the performance hits we see when thrown in sandboxed benchmarks.

+1

[callebtc]

LGTM