-
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
Remove deprecated _ec_privkey_{negate,tweak_add,tweak_mul}
aliases from API
#1593
base: master
Are you sure you want to change the base?
Remove deprecated _ec_privkey_{negate,tweak_add,tweak_mul}
aliases from API
#1593
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.
ACK 5ce9b57.
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
But needs rebase.
I think it's totally fine to remove these now. Would be nice to merge this for the same release as #1639 which is also breaking.
CHANGELOG.md
Outdated
- The previously deprecated function aliases `secp256k1_ec_privkey_negate`, `secp256k1_ec_privkey_tweak_add` and | ||
`secp256k1_ec_privkey_tweak_mul` were removed. The corresponding replacements `secp256k1_ec_seckey_negate`, | ||
`secp256k1_ec_seckey_tweak_add` and `secp256k1_ec_seckey_tweak_mul` have to be used instead. |
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.
nit, slightly shorter without loss of information:
- The previously deprecated function aliases `secp256k1_ec_privkey_negate`, `secp256k1_ec_privkey_tweak_add` and | |
`secp256k1_ec_privkey_tweak_mul` were removed. The corresponding replacements `secp256k1_ec_seckey_negate`, | |
`secp256k1_ec_seckey_tweak_add` and `secp256k1_ec_seckey_tweak_mul` have to be used instead. | |
- Removed previously deprecated function aliases `secp256k1_ec_privkey_negate`, `secp256k1_ec_privkey_tweak_add`, and | |
`secp256k1_ec_privkey_tweak_mul`. Use `secp256k1_ec_seckey_negate`, | |
`secp256k1_ec_seckey_tweak_add`, and `secp256k1_ec_seckey_tweak_mul` instead. |
These function aliases have been described as DEPRECATED in the public API docs already many years ago (see bitcoin-core#701, commit 41fc785), and in addition explicit deprecation warnings are shown by the compiler at least since the first official release 0.2.0 (see PR bitcoin-core#1089, commit fc94a2d), so it should be fine to just remove them by now. Co-authored-by: Tim Ruffing <[email protected]>
5ce9b57
to
37d2c60
Compare
Rebased on master and simplified the changelog as suggested in #1593 (comment). |
These function aliases have been described as DEPRECATED in the public API docs already many years ago (see #701, commit 41fc785), and in addition explicit deprecation warnings are shown by the compiler at least since the first official release 0.2.0 (see PR #1089, commit fc94a2d), so it should be fine to just remove them by now without causing bad surprises to any users.
Note that this PR intentionally doesn't include other deprecated flags/types/aliases by now, as they all have been introduced or marked as deprecated later (though the compiler deprecation warnings have been introduced at the same time). There are no deprecation rules yet and seemingly no removal of deprecated types/functions has ever happened so far, so it might make sense to discuss how to handle deprecation in general and introduce guidelines in the future.