mempool: Clear vTxHashes when mapTx is cleared #24145

1. 
   Bushstar commented at 9:34 am on January 25, 2022: contributor

   vTxHashes is a vector of all entries in mapTx, if you clear one you should clear the other, lest someone try to use the txiter in vTxHashes which would result in a segfault.
2. fanquake added the label Mempool on Jan 25, 2022
3. 
   glozow commented at 11:05 am on January 25, 2022: member

   light code review ACK 2902780

   Perhaps someone with more historical insight should look at this as well. I can’t find a reason for vTxHashes to not be cleared. It looks like it was introduced this way in 811902649d6aaddd886cb39b83aa69adf7b441bd.

4. 
   MarcoFalke commented at 3:10 pm on January 26, 2022: member

   It might be better to remove the whole function instead. See #20030 and then #19909
5. 
   glozow commented at 4:01 pm on January 26, 2022: member

   It might be better to remove the whole function instead. See #20030 and then #19909

   Oh, agree. Is #19909 up for grabs?

6. 
   MarcoFalke commented at 4:04 pm on January 26, 2022: member

   #19909 is the trivial part, #20030 is the tougher one to review even though it only changes on line. I can reopen it later today.
7. 
   luke-jr commented at 10:54 pm on January 29, 2022: member

   Regardless of removing the whole function later, this seems to be a good idea and should be reviewed/merged for backport benefit.
8. in src/txmempool.cpp:707 in 2902780093 outdated

   704@@ -705,6 +705,7 @@ void CTxMemPool::removeForBlock(const std::vector<CTransactionRef>& vtx, unsigne
   705 void CTxMemPool::_clear()
   706 {
   707     mapTx.clear();
   708+    vTxHashes.clear();
   

   ---

   ---

   

   luke-jr commented at 10:54 pm on January 29, 2022:

   Just for the sake of not having invalid iterators even temporarily, I prefer this before mapTx is cleared.

   ---

   

   Bushstar commented at 7:53 am on January 31, 2022:

   Updated to move vTxHashes clear before mapTx clear.
9. luke-jr approved
10. 
    luke-jr commented at 10:55 pm on January 29, 2022: member

    utACK, 1 nit
11. Clear vTxHashes when mapTx is cleared 9d65ad365c
12. Bushstar force-pushed on Jan 31, 2022
13. luke-jr approved
14. 
    luke-jr commented at 8:15 am on January 31, 2022: member

    re-utACK
15. 
    laanwj commented at 12:00 pm on April 6, 2022: member

    Code review ACK 9d65ad365c539557e969a35d22723d92e0b422fe This makes sense for consistency. As long as there is a clear function it makes sense to clear everything and not forget this. Also agree on the long-run goal of removing it in favor of safer approaches.
16. laanwj renamed this:
    Clear vTxHashes when mapTx is cleared
    mempool: Clear vTxHashes when mapTx is cleared
    on Apr 6, 2022
17. laanwj merged this on Apr 6, 2022
18. laanwj closed this on Apr 6, 2022

    ---

19. sidhujag referenced this in commit d29293f6aa on Apr 6, 2022
20. glozow referenced this in commit 03254c2229 on Jan 4, 2023
21. DrahtBot locked this on Apr 6, 2023

Contributors
Bushstar glozow MarcoFalke luke-jr laanwj

Labels
Mempool