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

PoC: Add internals module #153

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jonasnick
Copy link
Contributor

The idea with exposing internal functions is that instead of (only) having experimental modules, we could have experimental libraries that depend on libsecp and live outside of this repo. This has a couple of advantages, such as being able to phase out experimental modules, which simplifies versioning. One downside, however, is that the benefits of libsecp being a single compilation unit is being lost. But another big benefit is that complicated, stateful protocols (such as threshold sigs) can be written in rust (for example), using libsecp internals.

I only played with exposing scalars so far, but the module could also provide field.h, group.h, ecmult, ecmult_gen, etc. It would have no API or ABI guarantees. One idea mentioned by @rustyrussell is to make each call fail unless secp256k1_experimental_enable(version) has been called (with the exact version of the library).

@real-or-random
Copy link
Collaborator

Nice to see a concrete proposal.

The approach to make this a module is pretty lightweight.

I believe there's agreement that we don't want to provide any API/ABI stability. That implies that callers need to depend on the exact version. With that mind, I wonder why the approach in the PR will be better than vendoring a specific version.

Normally the advantage is that people are able create library X and library Y independently and both X and Y can depend on secp256k1(-zkp) and link to it. But if X and Y need to depend on the exact same version of secp256k1(-zkp), this advantage is rather moot, and they're not really independent.

Another advantage you mention is that we can have foreign language bindings for the internals. But if that is the primary reason, wouldn't it be more straight-forward to create those bindings directly, e.g., for Rust create a rust-secp256k1-internals crate that vendors secp256k1(-zkp), or have an internals feature in rust-secp256k1?

@jonasnick
Copy link
Contributor Author

they're not really independent.

They're still independent in that they can have separate maintainers, a different focus and more adequate versioning. Also, users are able to pick exactly what they need.

wouldn't it be more straight-forward to create those bindings directly

What do you mean exactly by more directly? Or rather, in what way is an internals module not direct?

@elsirion
Copy link

One idea mentioned by @rustyrussell is to make each call fail unless secp256k1_experimental_enable(version) has been called (with the exact version of the library).

Would it be reasonable to give the internal API its own version? That way the check could be SemVer-aware and also allow the use of newer versions if there were no breaking changes. Even if in practice the respective language bindings just vendor a specific version this could still be a good fail-safe to check for API compatibility. Also tracking changes in an internal API changelog would be very useful for downstream users.

Regarding the exposed scalar functionality I think it would be interesting to expose multiplication so that e.g. Shamir Secret Sharing can be implemented. On the same note having secp256k1_scalar_set_u64 available would probably also be nice (although that seems covered by secp256k1_internals_scalar_set_b32 in a more clunky way).

@Kixunil
Copy link

Kixunil commented Feb 10, 2022

Could the check be compile-time? E.g. by suffixing all public functions with semver major number.

theStack added a commit to theStack/bitcoin that referenced this pull request Mar 14, 2024
note that this is intentionally added to the main module (secp256k1.{h,c}) in
order to keep things simple and avoid bothering with the build system

source: BlockstreamResearch/secp256k1-zkp#153
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

Successfully merging this pull request may close these issues.

4 participants