-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Support for optional parts #277
Comments
I think it's valuable to try to minimize the options where possible; avoids a testing combination explosion. E.g. do we really need seperate build options for ecdh-X-only and ecdh-sha256. If things are setup right then unused parts would be excluded at link time, potentially reducing the impact from unneeded parts, and maybe this would permit having fewer options. |
The question is mostly: do we want any optional parts at all, and if so, how do we do that interface-wise? |
I think we do-- I mean, I don't thing we want the range proofs in a default build, or PIR protocols or... |
So far, all pieces of libsecp256k1 functionality have been part of a single offered interface. This made sense as long as all we were doing was implementing a pre-existing ECDSA standard.
However, there are now a few self-defined algorithms implemented and under review (Schnorr/SHA256, ECDH SHA256, ECDH x-only) and a few more externally maintained patches for extra functionality.
I wonder what kind of model we want for these. I feel that it's not optimal to "force" these standards on people using the library for pure ECDSA, but dozens of patch sets is not a maintainable situation either.
Of course, functionality could be compiled optionally, which is pretty easy because of the library's all-internal-implementations-is-in-headers, but there would be a question of headers. secp256k1.h could get optional parts, which get macro processed before install time, or (my preference) have separate .h files (secp256k1/ecdsa.h secp256k1/eckey.h secp256k1/schnorr.h, ...).
Opinions? @gmaxwell @apoelstra @peterdettman @theuni @laanwj?
The text was updated successfully, but these errors were encountered: