Advance pindexLastCommonBlock for blocks in chainActive #6233

pull sdaftuar wants to merge 1 commits into bitcoin:master from sdaftuar:fix-lastcommonblock-pruning changing 1 files +3 −2
  1. sdaftuar commented at 7:26 PM on June 4, 2015: member

    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).

  2. Advance pindexLastCommonBlock for blocks in chainActive
    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.
    3e9143386a
  3. laanwj added the label Bug on Jun 5, 2015
  4. laanwj added the label Priority High on Jun 5, 2015
  5. laanwj added the label P2P on Jun 5, 2015
  6. sipa commented at 2:01 PM on June 14, 2015: member

    Untested ACK

  7. jtimon commented at 1:07 PM on June 21, 2015: contributor

    ut ACK

  8. sipa commented at 1:21 PM on June 21, 2015: member

    Been running on bitcoin.sipa.be for a week, no problems.

  9. sdaftuar commented at 12:55 PM on June 25, 2015: member

    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.)

  10. laanwj commented at 2:33 PM on June 25, 2015: member

    utACK

  11. laanwj merged this on Jun 25, 2015
  12. laanwj closed this on Jun 25, 2015

  13. laanwj referenced this in commit e34a8acd9f on Jun 25, 2015
  14. laanwj referenced this in commit a587606525 on Jun 25, 2015
  15. laanwj commented at 2:42 PM on June 25, 2015: member

    Cherry-picked to 0.11 as a587606525242f76684208191436999ceadb8b56

  16. MarcoFalke locked this on Sep 8, 2021
Labels

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-14 12:16 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me