Advance pindexLastCommonBlock for blocks in chainActive#6233
Merged
laanwj merged 1 commit intobitcoin:masterfrom Jun 25, 2015
Merged
Advance pindexLastCommonBlock for blocks in chainActive#6233laanwj merged 1 commit intobitcoin:masterfrom
laanwj merged 1 commit intobitcoin:masterfrom
Conversation
This prevents an edge case where a block downloaded and pruned in-between successive calls to FindNextBlocksToDownload could cause the block to be unnecessarily re-requested.
Member
|
Untested ACK |
Contributor
|
ut ACK |
Member
|
Been running on bitcoin.sipa.be for a week, no problems. |
Member
Author
|
Thoughts on merging this into 0.11? This is unfortunately difficult to test, but I did try to recreate the conditions that produced the infinite download bug I previously triggered, and verified that with this code no blocks were downloaded more than once. Not a conclusive test of the code however. (To be clear, I don't believe there is a potential infinite download bug in master anymore, that risk was eliminated already, but I think it's possible for a block to be downloaded twice.) |
Member
|
utACK |
laanwj
added a commit
that referenced
this pull request
Jun 25, 2015
3e91433 Advance pindexLastCommonBlock for blocks in chainActive (Suhas Daftuar)
Member
|
Cherry-picked to 0.11 as a587606 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This prevents an edge case where a block downloaded and pruned
in-between successive calls to FindNextBlocksToDownload could
cause the block to be unnecessarily re-requested.
The issue is that pindexLastCommonBlock is only updated on calls
to FindNextBlocksToDownload, by starting at its prior value and then
advancing forward until we encounter a block for which we don't
HAVE_DATA (which we then try to request).
During initial block download, it's possible for the tip to advance by
many blocks during a single call to ActivateBestChain (since blocks
arrive out of order, when a block comes in that completes a long
sequence, then we'll see many tip updates in a row). If a long enough
sequence of tip updates occurs, then it's possible that the block
that just arrived could be pruned at the end of processing, before
FindNextBlocksToDownload can be called for any other peers. This
in turn would cause us to re-request the block from one of those peers,
since we'd no longer HAVE_DATA for it.
Since blocks in chainActive never need to be re-downloaded, this
pull should prevent pruning nodes from ever re-requesting blocks in this
situation, while still allowing for pruning nodes to redownload blocks
that may have been previously pruned but are needed for a reorg (this
is exercised in
pruning.py, which this pull passes).