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

Expose Address::is_valid_for_network #439

Closed
jurvis opened this issue Jan 2, 2024 · 5 comments · Fixed by #440
Closed

Expose Address::is_valid_for_network #439

jurvis opened this issue Jan 2, 2024 · 5 comments · Fixed by #440
Assignees
Labels
enhancement New feature or request

Comments

@jurvis
Copy link

jurvis commented Jan 2, 2024

Describe the enhancement
rust-bitcoin treats all addresses with the tb prefix as testnet addresses:

When parsing, such addresses are always assumed to be testnet addresses (the same is true for bech32 signet addresses).

However, they may not necessarily be that since regtest and signet share the same prefix. Exposing is_valid_for_network in Address would be more useful for users.

Use case
Implementing a text box/QR scanner that parses a bitcoin address and shows an appropriate error if its network does not align with the user's wallet.

Additional context
NA

@jurvis jurvis added the enhancement New feature or request label Jan 2, 2024
@reez
Copy link
Collaborator

reez commented Jan 3, 2024

I'll start working on this-

@reez reez self-assigned this Jan 3, 2024
@jurvis
Copy link
Author

jurvis commented Jan 3, 2024

@reez thanks, Matthew!

Would be amazing if we also could get that cherry picked into a patch release for 0.30

@reez reez linked a pull request Jan 4, 2024 that will close this issue
8 tasks
@reez
Copy link
Collaborator

reez commented Jan 4, 2024

I have a Draft PR up for this.

Just for the Draft PR I decided to be super exhaustive about the unit tests for clarity on what is_valid_for_network would return in each scenario.

@thunderbiscuit
Copy link
Member

Awesome!

@jurvis I want to make sure I understand your requirements/workflow (because the API doesn't quite do what I thought it did at first glance).

Do you intend to use this to make sure that an address matches a given wallet's network? If that's the case I think this will work in some cases, but in some others will not. For example you might have a signet wallet, scan a testnet address from a testnet faucet, then call the .is_valid_for_network(Network::Signet) method on this address and it would return true even though the address is actually testnet, and would therefore fail at runtime.

Let us know if this does exactly what you need, and if you can expand on how exactly you intend to use it that would be helpful.

As for the patch release, I think we'll be happy to release a patch for your team if this is something you need!

@jurvis
Copy link
Author

jurvis commented Jan 4, 2024

For example you might have a signet wallet, scan a testnet address from a testnet faucet, then call the .is_valid_for_network(Network::Signet) method on this address and it would return true even though the address is actually testnet, and would therefore fail at runtime.

Yup, that seems fine to me. To my knowledge, that's the best we can do? We probably should also document the different expectations between using this API versus network() as well, since it will always return Testnet even when you pass it a Signet address.

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

Successfully merging a pull request may close this issue.

3 participants