Skip to content

Implement verifychain method and test #155

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 1 commit into from
May 11, 2025

Conversation

GideonBature
Copy link
Contributor

@GideonBature GideonBature commented May 9, 2025

This is based on PR #116 on implementing one method at a time for easier review: This is the third method implementation: verifychain which returns a (bool type). Once this is approved, I’ll proceed with the next one.

Copy link
Member

@tcharding tcharding left a comment

Choose a reason for hiding this comment

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

Except the docs thing everything here looks good - too easy!

@GideonBature
Copy link
Contributor Author

Except the docs thing everything here looks good - too easy!

removed the unnecessary lines of the rust docs, leaving just a single line, as you suggested....

@tcharding
Copy link
Member

tcharding commented May 10, 2025

While reviewing this I saw that we have the integration test for gettxoutproof in the wrong place (it shows up in the diff of this patch). Want to fix that?

FTR I'm as fallible as the next bloke, if you see mistakes please clean em up - its super appreciated. This repo is even more necessary to keep clean than most because its so big, boring, boilerplate. In general you can either do them as separate PRs, or if the PR you are working is going to include the mistake in the diff just put the fix as a separate patch at the front of the PR i.e., do it first in a separate patch (commit).

Copy link
Member

@tcharding tcharding left a comment

Choose a reason for hiding this comment

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

ACK ae286f1

@tcharding tcharding merged commit 8feadd4 into rust-bitcoin:master May 11, 2025
28 checks passed
@tcharding
Copy link
Member

Thanks man, you are killing it!

@GideonBature
Copy link
Contributor Author

Thanks man, you are killing it!

Thank you.... I am learning from you...

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.

2 participants