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 NetworkValidationError #2508

Merged
merged 2 commits into from Feb 27, 2024

Conversation

tcharding
Copy link
Member

Replaces #2502 because there is going to be way too much arguing on this to bother a newer contributor with.

This PR takes into consideration #2507 but does not improve the issue, it also does not make it worse. I propose to do this and then consider #2507 since this is a step forwards IMO.

Remove the address::Error because its not good. Add a NetworkValidationError and return it from require_network - leaving the door open for resolving Kix's issue in 2507.

The `require_network` function can fail in one way only, add a specific
error for the failure.
@tcharding tcharding changed the title 02 27 address error Add NetworkValidationError Feb 27, 2024
@github-actions github-actions bot added the C-bitcoin PRs modifying the bitcoin crate label Feb 27, 2024
@coveralls
Copy link

Pull Request Test Coverage Report for Build 8057500756

Details

  • -3 of 4 (25.0%) changed or added relevant lines in 2 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.04%) to 83.951%

Changes Missing Coverage Covered Lines Changed/Added Lines %
bitcoin/src/address/error.rs 0 3 0.0%
Files with Coverage Reduction New Missed Lines %
bitcoin/src/address/error.rs 1 5.41%
Totals Coverage Status
Change from base Build 8057343196: 0.04%
Covered Lines: 19438
Relevant Lines: 23154

💛 - Coveralls

Copy link
Collaborator

@Kixunil Kixunil left a comment

Choose a reason for hiding this comment

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

ACK 7e2a81d

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 7e2a81d

@apoelstra apoelstra merged commit 36aa627 into rust-bitcoin:master Feb 27, 2024
187 checks passed
@tcharding tcharding deleted the 02-27-address-error branch February 27, 2024 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bitcoin PRs modifying the bitcoin crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants