Keep mempool consistent during block-reorgs #5945

1. 
   gavinandresen commented at 5:22 PM on March 25, 2015: contributor

   This fixes a subtle bug involving block re-orgs and non-standard transactions.

   Start with a block containing a non-standard transaction, and one or more transactions spending it in the memory pool.

   Then re-org away from that block to another chain that does not contain the non-standard transaction.

   Result before this fix: the dependent transactions get stuck in the mempool without their parent, putting the mempool in an inconsistent state.

   Tested with a new unit test.

   Thanks to Alex Morcos for finding the bug.

2. gavinandresen force-pushed on Mar 25, 2015
3. gavinandresen cross-referenced this on Mar 25, 2015 from issue Limit amount of memory used in large re-orgs by gavinandresen
4. 
   morcos commented at 6:31 PM on March 25, 2015: member

   I tested this. ACK.
   I think it has the side effect of making reorgs a little faster too!

5. in src/txmempool.cpp:None in c22adce0c1 outdated

    450 | +                if (it == mapNextTx.end())
    451 | +                    continue;
    452 | +                txToRemove.push_back(it->second.ptx->GetHash());
    453 | +            }
    454 | +        }
    455 |          while (!txToRemove.empty())
   

   ---

   ---

   

   petertodd commented at 12:02 AM on March 26, 2015:

   This could use some comments saying what this block of code is intending to actually do; not obvious right now.

   ---

   

   gavinandresen commented at 3:58 PM on March 26, 2015:

   Comment added.

6. laanwj added the label Mempool on Mar 26, 2015
7. laanwj added the label Bug on Mar 26, 2015
8. gavinandresen force-pushed on Mar 26, 2015
9. Keep mempool consistent during block-reorgs

   This fixes a subtle bug involving block re-orgs and non-standard transactions.
   
   Start with a block containing a non-standard transaction, and
   one or more transactions spending it in the memory pool.
   
   Then re-org away from that block to another chain that does
   not contain the non-standard transaction.
   
   Result before this fix: the dependent transactions get stuck
   in the mempool without their parent, putting the mempool
   in an inconsistent state.
   
   Tested with a new unit test.

   ad9e86dca1
10. 
    petertodd commented at 12:43 AM on March 27, 2015: contributor

    Very much untested and rushed ACK

11. 
    laanwj commented at 11:32 AM on April 1, 2015: member

    Going to test this.

12. 
    sipa commented at 5:50 PM on April 1, 2015: member

    Untested ACK.

13. gmaxwell added this to the milestone 0.10.0 on Apr 2, 2015
14. 
    gmaxwell commented at 8:41 PM on April 5, 2015: contributor

    I was observed two crashes in 0.10rc1, each 24 hours apart with checkmempool=1 (and a non-default fee policy) during reorgs that appeared to be likely due to the bug this fixes. I've now run three days with this pull applied and no crashes, and there have been several reorgs during that time.

15. 
    laanwj commented at 7:48 AM on April 6, 2015: member

    Tested ACK

16. laanwj merged this on Apr 6, 2015
17. laanwj closed this on Apr 6, 2015

    ---

18. laanwj referenced this in commit 27e8d224e9 on Apr 6, 2015
19. gavinandresen referenced this in commit 8bb4abc19a on Apr 6, 2015
20. 
    laanwj commented at 7:57 AM on April 6, 2015: member

    Backported to 0.10 branch as 8bb4abc19ae0e76424e69a19ff6fc31e5b044333

21. gavinandresen referenced this in commit 1c62e84099 on Apr 6, 2015
22. gavinandresen deleted the branch on Apr 24, 2015
23. reddink referenced this in commit 231af74642 on Jul 11, 2020
24. reddink referenced this in commit c66f68a39f on Jul 14, 2020
25. bitcoin locked this on Sep 8, 2021

Contributors

gavinandresenmorcospetertoddlaanwjsipagmaxwell

Milestone
0.10.0