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

Use newly released bech32 API #1951

Merged

Conversation

tcharding
Copy link
Member

@tcharding tcharding commented Jul 21, 2023

Depend on the newly released version of bech32, BOOM!

bitcoin/src/address.rs Outdated Show resolved Hide resolved
bitcoin/src/address.rs Outdated Show resolved Hide resolved
@apoelstra
Copy link
Member

Overall looking pretty good :)

@tcharding
Copy link
Member Author

Overall looking pretty good :)

Sweet huh, way better than earlier iterations.

@tcharding tcharding changed the title 07 21 use bech32 segwit iter WIP: Use new bech32 API (v2) Jul 22, 2023
@tcharding tcharding force-pushed the 07-21-use-bech32-segwit-iter branch from 4321612 to 5c90027 Compare July 23, 2023 00:44
@apoelstra
Copy link
Member

5c90027 looking pretty good :)

@tcharding tcharding force-pushed the 07-21-use-bech32-segwit-iter branch from 5c90027 to bb23ef8 Compare August 2, 2023 00:08
@tcharding tcharding changed the title WIP: Use new bech32 API (v2) WIP: Use new bech32 API Aug 2, 2023
@tcharding tcharding changed the title WIP: Use new bech32 API WIP: Use new unreleased bech32 API Aug 2, 2023
@tcharding tcharding force-pushed the 07-21-use-bech32-segwit-iter branch 2 times, most recently from 536d5ad to 44d0782 Compare August 3, 2023 04:23
bitcoin/src/address.rs Outdated Show resolved Hide resolved
@tcharding tcharding force-pushed the 07-21-use-bech32-segwit-iter branch from 44d0782 to d4e40b3 Compare August 4, 2023 01:17
@tcharding tcharding changed the title WIP: Use new unreleased bech32 API Use newly released bech32 API Aug 21, 2023
@tcharding tcharding marked this pull request as ready for review August 21, 2023 05:36
@tcharding tcharding added the P-high High priority label Aug 21, 2023
clarkmoody
clarkmoody previously approved these changes Aug 22, 2023
Copy link
Member

@clarkmoody clarkmoody left a comment

Choose a reason for hiding this comment

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

ACK 92be241

bitcoin/src/address/mod.rs Show resolved Hide resolved
@tcharding
Copy link
Member Author

Rebase only, no other changes.

@tcharding
Copy link
Member Author

Rebased to pick up recent pinning fix, no other changes.

@tcharding
Copy link
Member Author

This is only the last patch, it is non-trivial and requires 2 maintainer acks to merge, though as mentioned elsewhere I think @clarkmoody's ack is worth something, especially since he is the original author and maintainer of rust-bech32, on which this PR depends.

apoelstra added a commit that referenced this pull request Sep 20, 2023
52f2332 Remove docs from witness version conversion functions (Tobin C. Harding)
47d6d78 Remove bip 173/350 test vectors (Tobin C. Harding)
e0eaeaa Split ParseError out of Error (Tobin C. Harding)
0f536e8 Add new UnknownAddressTypeError for parsing address type (Tobin C. Harding)
e2014cb Import error variants within dislay impl (Tobin C. Harding)
9d7791f Remove unnecessary self:: from error import (Tobin C. Harding)
b2e485e Split the address error code out into a separate module (Tobin C. Harding)
f34ca0c Move address.rs to address/mod.rs (Tobin C. Harding)

Pull request description:

  In preparation for depending on the recently released version of `rust-bech32` do a bunch of preparatory fixes.

  1. Improve `address` module error handling as we are doing else where at the moment
  2. Remove bip 173 and 350 test vector tests, these are fully covered in bech32
  3. Trim down the docs on `WitnessVersion`

  This PR is the first 8 patches of #1951

ACKs for top commit:
  sanket1729:
    ACK 52f2332
  apoelstra:
    ACK 52f2332

Tree-SHA512: 67a4ea4020b4e5c9c8396e4195e06dbd1d11335788f9e52f60abbc0b399e37e5dacc9bb7fa4e88221670322fa3c3407ade059d5c709f96e2df97240f4524e08c
Use the new bech32 iterator API that Andrew and I wrote.
@tcharding
Copy link
Member Author

Went back over this while rebasing because it was done a long time ago, noticed that there were some error variants that are now unused. No other changes.

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 e4c7e01

@apoelstra
Copy link
Member

Nice!! I finally reviewed this in detail.

Copy link
Member

@clarkmoody clarkmoody left a comment

Choose a reason for hiding this comment

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

ACK e4c7e01

@apoelstra apoelstra merged commit 675fd54 into rust-bitcoin:master Sep 21, 2023
29 checks passed
@tcharding tcharding deleted the 07-21-use-bech32-segwit-iter branch September 22, 2023 02:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-high High priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants