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

blockchain: Add IsAncestor method to blockNode #2153

Merged
merged 2 commits into from
Apr 10, 2024

Conversation

kcalvinalvin
Copy link
Collaborator

@kcalvinalvin kcalvinalvin commented Apr 2, 2024

IsAncestor() provides functionality for testing if a block node is an ancestor of anther block node.

Cherry-picked from 0d6a93e687e3470608cccfc58cc4a2ddb5ea8262.

It's a part of #2154 as this method is used in utreexod code for testing invalidateblock and reconsiderblock methods on blockchain.

@coveralls
Copy link

coveralls commented Apr 2, 2024

Pull Request Test Coverage Report for Build 8597216282

Details

  • 20 of 24 (83.33%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.01%) to 56.879%

Changes Missing Coverage Covered Lines Changed/Added Lines %
blockchain/blockindex.go 20 24 83.33%
Totals Coverage Status
Change from base Build 8532284036: 0.01%
Covered Lines: 29441
Relevant Lines: 51761

💛 - Coveralls

Copy link
Member

@Roasbeef Roasbeef left a comment

Choose a reason for hiding this comment

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

Nice addition! Mostly straight forward to follow, though I'm concerned that the equality check is done using pointers, rather than the value itself. Eg: if we make a new instance or copy of otherNode (that's already in the index, and is an ancestor of node), then from my reading we'll return false, when we should return true.

blockchain/blockindex.go Show resolved Hide resolved
blockchain/blockindex.go Outdated Show resolved Hide resolved
blockchain/blockindex.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@Crypt-iQ Crypt-iQ left a comment

Choose a reason for hiding this comment

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

Looks good once laolu's comments addressed. One nit in the test

blockchain/chain_test.go Outdated Show resolved Hide resolved
Helper function for the added IsAncestor in the follow up commit.
Returns true if all the fields (except for parent and ancestor) are
equal.
IsAncestor() provides functionality for testing if a block node is
an ancestor of anther block node.
Copy link
Member

@Roasbeef Roasbeef left a comment

Choose a reason for hiding this comment

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

LGTM 📣

@Roasbeef Roasbeef merged commit 5d50f7c into btcsuite:master Apr 10, 2024
3 checks passed
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.

4 participants