8000 Remove old deprecated code by tcharding · Pull Request #1163 · rust-bitcoin/rust-bitcoin · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Remove old deprecated code #1163

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

Merged
merged 5 commits into from
Aug 11, 2022

Conversation

tcharding
Copy link
Member
@tcharding tcharding commented Aug 2, 2022

I'm not sure what the exact policy we decided on for deprecating is but we have code deprecated since v0.24.0 and v.026.2, both of these seem reasonable safe to remove.

Removal of SighashCompontents uncovered the fact that we never moved the BIP 143 test vector code over to the new SighashCache.

  • Patch 1: Trivial preparatory cleanup
  • Patch 2: Re-write bip143 test vectors using sighash::SighashCache
  • Patch 3: Trivial, use SighashCache instead of SigHashCache
  • Patch 4: Remove SighashComponents struct deprecated in 0.24.0
  • Patch 5: Remove MerkleBlock methods deprecated in 0.26.2

@tcharding tcharding force-pushed the 08-03-remove-deprecated branch from 4a9f81b to 5f3c8ff Compare August 2, 2022 23:12
@tcharding
Copy link
Member Author

Draft while I actually do it properly.

@tcharding tcharding marked this pull request as draft August 3, 2022 00:17
In preparation for adding unit tests to the `sighash` module clean up
the import statements.

Refactor only, no logic changes.
We have a bunch of unit tests that verify the BIP 143 test vectors.
Three of these use the deprecated `SighashComponents` struct. We should
implement these tests using the new `SighashCache`. In order to do so we
have to move the tests to the `sighash` module so they can get access to
the private `segwit_cache()` function, and verify the cache internal
state against the test vectors.

Re-write the BIP 143 test vector code using `SighashCache` and remove
the versions using `SighashComponents`.

Done in preparation for removing the deprecated `SighashComponents`.
The `bip143::SigHashCache` is deprecated in favour of
`sighash::SighashCache` however we still use it in `bip143` unit tests.

Use the new `sighash::SighashCache` in `bip143` unit tests.
This struct was deprecated in v0.24.0, we can safely remove it now.
We have two methods that have been deprecated since 0.26.2,  we can
safely remove them now.
@tcharding tcharding force-pushed the 08-03-remove-deprecated branch from 5f3c8ff to dfb8ef9 Compare August 3, 2022 02:34
@tcharding tcharding marked this pull request as ready for review August 3, 2022 02:37
@Kixunil
Copy link
Collaborator
Kixunil commented Aug 3, 2022

what the exact policy

AFAIK there's none but anything is good enough IMO. We do unannounced changes too. 🤷‍♂️ .

@apoelstra
Copy link
Member

I'll ACK this but in the interest of getting the release out I think we'll wait til after 0.29.0 to merge

Copy link
Member
@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK dfb8ef9

@tcharding
Copy link
Member Author

Oh yes, I expected that. I'll add 0.30 milestone and try to remember to add labels/milestones to my PRs. Thanks.

@tcharding tcharding added this to the 0.30.0 milestone Aug 3, 2022
@tcharding tcharding added API break This PR requires a version bump for the next release release notes mention labels Aug 3, 2022
Copy link
Member
@sanket1729 sanket1729 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

8000

ACK dfb8ef9

Copy link
Member
@sanket1729 sanket1729 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK dfb8ef9

@apoelstra apoelstra merged commit cc5b6ef into rust-bitcoin:master Aug 11, 2022
@tcharding tcharding deleted the 08-03-remove-deprecated branch August 15, 2022 04:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API break This PR requires a version bump for the next release release notes mention
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0