-
Notifications
You must be signed in to change notification settings - Fork 276
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
restore global-context-less-secure
feature
#407
restore global-context-less-secure
feature
#407
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need this change. I commented on the tracking issue thread to keep the discussion in one place.
Agreed. But I do think we should merge this to put the feature back in place (since it will break compilation to remove it), and then re-remove it for the upcoming major release. |
ACK, removing a feature would be breaking change. |
@elichai can I have an ack here |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK 2a25e5e
…l context (fixup for #407) c1bb316 Make global-context-less-secure actually enable the global context (Elichai Turkel) Pull request description: In #407 we restored the `global-context-less-secure` feature, but it didn't actually do anything because #385 changed all the cfg checks on the whole module to depend on `global-context`, so we need to enable `global-context` in order to make that module compile. so before all this, users could enable *just* `global-context-less-secure` without enabling the `global-context`, and after this PR it will behave the same. (this will not enable the randomization because of: https://github.com/rust-bitcoin/rust-secp256k1/blob/1cf2429b12ff06a04a5c882f2f5d918042629660/src/context.rs#L51) ACKs for top commit: apoelstra: ACK c1bb316 Tree-SHA512: edc7b4916b359a0696cc25f498bc52ad340f981ad6b01b83b68966d6179200bac6acb96f5480157e24c605b5552bdd7b6eb8770bc9a2c5734da3df11c021fb5b
We can't remove a feature in a minor release, and also I believe this feature is actually necessary in some niche applications.