refactor: Avoid locking tx pool cs thrice #13786

1. 
   MarcoFalke commented at 3:08 pm on July 28, 2018: member

   addUnchecked is (outside the tests) only called by ATMP, which already takes the tx pool read lock. So locking it twice more in both addUnchecked methods seems redundant.

   Similarly CalculateMemPoolAncestors is (beside once in the wallet) only called in contexts, where the tx pool lock is already taken. So remove the lock there as well.

2. MarcoFalke added the label Refactoring on Jul 28, 2018
3. 
   DrahtBot commented at 3:24 pm on July 28, 2018: member

   • #10443 (Add fee_est tool for debugging fee estimation code by ryanofsky)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

4. MarcoFalke force-pushed on Jul 28, 2018
5. 
   promag commented at 8:35 pm on July 28, 2018: member

   Tested ACK fa6b4b5. Verified all OP claims.

   CalculateMemPoolAncestors is called in: https://github.com/bitcoin/bitcoin/blob/ef4fac0ea5b4891f4529e4b59dfd1f7aeb3009b5/src/miner.h#L173 https://github.com/bitcoin/bitcoin/blob/ef4fac0ea5b4891f4529e4b59dfd1f7aeb3009b5/src/policy/rbf.h#L26 https://github.com/bitcoin/bitcoin/blob/ef4fac0ea5b4891f4529e4b59dfd1f7aeb3009b5/src/txmempool.h#L680 which already have the annotation.

6. refactor: Avoid locking tx pool cs thrice fa5ed4f8d2
7. MarcoFalke force-pushed on Jul 29, 2018
8. 
   MarcoFalke commented at 12:05 pm on July 29, 2018: member

   Rebased
9. in src/test/mempool_tests.cpp:58 in fa5ed4f8d2

   54@@ -55,6 +55,7 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest)
   55 
   56 
   57     CTxMemPool testPool;
   58+    LOCK(testPool.cs);
   

   ---

   ---

   

   Empact commented at 4:34 pm on July 30, 2018:

   nit: could move to 65 since removeRecursive locks separately
10. in src/test/policyestimator_tests.cpp:21 in fa5ed4f8d2

    17@@ -18,6 +18,7 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
    18 {
    19     CBlockPolicyEstimator feeEst;
    20     CTxMemPool mpool(&feeEst);
    21+    LOCK(mpool.cs);
    

    ---

    ---

    

    Empact commented at 4:35 pm on July 30, 2018:

    nit: could move to 55
11. 
    Empact commented at 4:36 pm on July 30, 2018: member

    utACK fa5ed4f
12. 
    kallewoof commented at 6:57 pm on July 30, 2018: member

    utACK fa5ed4f8d2c4cb3507bcc2460725d483f2e5789c
13. MarcoFalke referenced this in commit 84d5a6210c on Jul 30, 2018
14. MarcoFalke merged this on Jul 30, 2018
15. MarcoFalke closed this on Jul 30, 2018

    ---

16. MarcoFalke deleted the branch on Jul 30, 2018
17. jasonbcox referenced this in commit 5822a6e6ef on Oct 11, 2019
18. PastaPastaPasta referenced this in commit 76bd594609 on Feb 2, 2021
19. PastaPastaPasta referenced this in commit 4a7387fba9 on Feb 4, 2021
20. MarcoFalke locked this on Sep 8, 2021

Contributors
MarcoFalke DrahtBot promag Empact kallewoof

Labels
Refactoring