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. 
   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!
4. in src/txmempool.cpp: 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 0: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.
5. laanwj added the label Mempool on Mar 26, 2015
6. laanwj added the label Bug on Mar 26, 2015
7. gavinandresen force-pushed on Mar 26, 2015
8. 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
9. 
   petertodd commented at 0:43 am on March 27, 2015: contributor

   Very much untested and rushed ACK
10. 
    laanwj commented at 11:32 am on April 1, 2015: member

    Going to test this.
11. 
    sipa commented at 5:50 pm on April 1, 2015: member

    Untested ACK.
12. gmaxwell added this to the milestone 0.10.0 on Apr 2, 2015
13. 
    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.
14. 
    laanwj commented at 7:48 am on April 6, 2015: member

    Tested ACK
15. laanwj merged this on Apr 6, 2015
16. laanwj closed this on Apr 6, 2015

    ---

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

    Backported to 0.10 branch as 8bb4abc19ae0e76424e69a19ff6fc31e5b044333
20. gavinandresen referenced this in commit 1c62e84099 on Apr 6, 2015
21. gavinandresen deleted the branch on Apr 24, 2015
22. reddink referenced this in commit 231af74642 on Jul 11, 2020
23. reddink referenced this in commit c66f68a39f on Jul 14, 2020
24. MarcoFalke locked this on Sep 8, 2021

Contributors
gavinandresen morcos petertodd laanwj sipa gmaxwell

Labels
Bug Mempool

Milestone
0.10.0