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

Add Bitcoin-core regtest bech32 addresses support #14

Merged
merged 4 commits into from
Jun 25, 2018

Conversation

D4nte
Copy link
Contributor

@D4nte D4nte commented Jun 21, 2018

Bitcoin-core addresses on regtest start with "bcrt1".
While this is not part of BIP173, adding the support allows library users to use bitcoin-core regtest to end-to-end test their rust software.

Please note that I would like to do a similar pull request for repo rust-bitcoin. However, rust-bech32-bitcoin/master contains breaking changes which means that rust-bitcoin/master cannot compile with the current bech32/master.

I had to create a branch based from commit 4d08819 to implement my changes and use them with rust-bitcoin/master.

If you are happy with integrating this pull request, could you please assist in creating a release 0.5.2? (0.5.1 + this pull request). I cannot see a dedicated branch/tag for 0.5.1 in this repo.

Many thanks for considering my request and in providing this very useful library!

bitcoin-core addresses on regtest start with "bcrt1".
While this is not part of BIP173, adding the support allows
user to use bitcoin-core regtest to end-to-end test their rust
software using this library.
@sgeisler
Copy link
Contributor

Hi, thanks for the PR. I will have a look at rust-bitcoin later to see how much work it would be to integrate the current version of rust-bech32-bitcoin.

Imo this issue is also related to #7 (and I'd say somehow blocked if it's not too urgent to have regtest support) since you had to introduce new enum variants everywhere otherwise.

Regarding the tags, it should be easy to find the commits changing the version in the Cargo.toml, so maybe I can reconstruct some of the tags. We should begin tagging new version again imo.

The version number for this change has to be 0.6.0 since adding a variant to an enum which would break some formerly exhaustive matches. It would be nice if you could change the version number in the Cargo.toml accordingly.

@clarkmoody
Copy link
Member

clarkmoody commented Jun 21, 2018

I suggest renaming to Regtest as the enum value, since we already have Testnet that refers to Bitcoin's testnet, and bcrt is used in multiple implementations, not only Bitcoin Core.

@sgeisler
Copy link
Contributor

If you are happy with integrating this pull request, could you please assist in creating a release 0.5.2? (0.5.1 + this pull request). I cannot see a dedicated branch/tag for 0.5.1 in this repo.

After looking at the code and noticing that rust-bech32-bitcoin's current version already is at v0.6.1 I understand what you meant with v0.5.2. But I'd still disagree to use a patch version increase for a breaking API change (even though for versions <1.0.0 everything is technically allowed).

I've opened rust-bitcoin/rust-bitcoin#100 to make rust-bitcoin compatible with v0.6.1, so your PR could be released as v0.7.0 and be included in rust-bitcoin/rust-bitcoin#100 before merging it.

@D4nte
Copy link
Contributor Author

D4nte commented Jun 25, 2018

All requested changes done. Let me know if there is anything else.

I'll see if I can help with rust-bitcoin :)

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.

👍

@clarkmoody clarkmoody merged commit 687fc10 into rust-bitcoin:master Jun 25, 2018
@clarkmoody
Copy link
Member

Published 0.7.0 to crates.io

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants