Skip to content

Conversation

@sdaftuar
Copy link
Member

When responding to a getblocks message, only return inv's as
long as we HAVE_DATA for blocks in the chain, and only for blocks
that we are likely to not delete in the near future.

This fixes the issue mentioned here #6122 (comment).

Copy link
Member

Choose a reason for hiding this comment

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

Can't this check be done only once, before the loop? If it holds for the oldest block in the list, it will also hold for all further ones, I think?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's very likely but not necessarily the case, because blocks can be written out of order to disk (so that we may have an older block but pruned a more recent block), and because we don't guarantee that we always have the last 288 blocks (though we try to, that assumption can break if there's a reorg). So it's possible, say, for a block at height tip-280 to be present but a block at height tip-279 to have been pruned.

Copy link
Member

Choose a reason for hiding this comment

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

Fair enough, and the test is cheap.

When responding to a getblocks message, only return inv's as
long as we HAVE_DATA for blocks in the chain, and only for blocks
that we aren't likely to delete in the near future.
@sdaftuar
Copy link
Member Author

Pushed up a small fix (was using fHavePruned instead of fPruneMode in the test).

@laanwj laanwj added the Bug label May 15, 2015
@morcos
Copy link
Contributor

morcos commented May 15, 2015

tested ACK. Although I don't think this change is needed until #6122 goes through, it will be needed at that point.

@sdaftuar
Copy link
Member Author

Closing, as I don't think this bug can be triggered without enabling block relay for pruning nodes, and this commit is included in #6148 already.

@sdaftuar sdaftuar closed this Jun 16, 2015
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants