Make mempool-removed tracking much more explicit #5433

1. 
   TheBlueMatt commented at 8:32 am on December 6, 2014: member

   Based on #5267. The conflicted transaction tracking during mempool remove is actually critical for balance stability, as @dgenr8 and @morcos pointed out previously…but for a very subtle reason. When we called SyncTransaction, it was doing a ton of work…just to uncache the balance. Instead, this pull replaces the SyncTransaction call with a new wallet signal, which only provides the transaction hash, making the removed tracking a bit lighter weight.

   Also, it explains what is actually happening and adds a python test-case to test it out, instead of it being entirely opaque and marginally fragile.

2. TheBlueMatt force-pushed on Dec 6, 2014
3. TheBlueMatt force-pushed on Dec 6, 2014
4. 
   dgenr8 commented at 4:18 pm on December 6, 2014: contributor

   Looks good, I plan to test. If you can call the new signal from txmempool directly during remove, then you can also get rid of the to-be-removed list.

   And I’ll just mention that the wallet relying on the volatile and policy-dependent mempool for conflict detection is something that could be improved (in the wallet).

5. TheBlueMatt renamed this:
   Mempoolconflict
   Make mempool-removed tracking much more explicit
   on Dec 6, 2014
6. TheBlueMatt force-pushed on Dec 6, 2014
7. TheBlueMatt force-pushed on Dec 7, 2014
8. 
   morcos commented at 7:24 pm on December 8, 2014: member

   I did some testing with the mempool-removed tracking part of this pull, and the caching of credit balances now seems to be properly updated.
9. TheBlueMatt force-pushed on Dec 8, 2014
10. TheBlueMatt force-pushed on Dec 20, 2014
11. TheBlueMatt force-pushed on Dec 20, 2014
12. TheBlueMatt force-pushed on Dec 20, 2014
13. TheBlueMatt force-pushed on Dec 21, 2014
14. TheBlueMatt force-pushed on Dec 21, 2014
15. TheBlueMatt force-pushed on Dec 21, 2014
16. TheBlueMatt force-pushed on Dec 21, 2014
17. TheBlueMatt force-pushed on Dec 22, 2014
18. TheBlueMatt force-pushed on Dec 22, 2014
19. TheBlueMatt force-pushed on Dec 23, 2014
20. TheBlueMatt force-pushed on Dec 23, 2014
21. 
    TheBlueMatt commented at 2:49 am on December 23, 2014: member

    After all the travis round-tripping, it appears the LogPrints in the DEBUG_LOCKORDER routines were killing the ability to restart bitcoind, so those are now gone and this is reviewable.
22. 
    sipa commented at 4:56 pm on January 4, 2015: member

    Untested ACK.
23. laanwj added the label Mempool on Jan 8, 2015
24. 
    jtimon commented at 11:40 pm on May 15, 2015: contributor

    Needs rebase
25. Make mempool-removed tracking much more explicit

    ...with comments and a new signal to wallets

    7357c1ad70
26. Add conflicted-txn python test 423abaa453
27. TheBlueMatt force-pushed on Aug 22, 2015
28. 
    TheBlueMatt commented at 1:59 am on August 22, 2015: member

    Rebased.
29. 
    jtimon commented at 4:42 pm on August 23, 2015: contributor

    ut ACK
30. 
    dcousens commented at 0:28 am on August 24, 2015: contributor

    concept ACK
31. Fix test to use new framework updates 85f11ed248
32. 
    TheBlueMatt commented at 9:51 pm on August 28, 2015: member

    Also fixed the test cases after rebase.
33. 
    MarcoFalke commented at 8:10 am on October 30, 2015: member

    @TheBlueMatt Needs rebase
34. TheBlueMatt closed this on Nov 5, 2015

    ---

35. 
    dcousens commented at 8:52 pm on November 5, 2015: contributor

    @TheBlueMatt not worth rebasing?
36. 
    TheBlueMatt commented at 9:59 pm on November 5, 2015: member

    Meh.
37. MarcoFalke locked this on Sep 8, 2021

Contributors
TheBlueMatt dgenr8 morcos sipa jtimon dcousens MarcoFalke

Labels
Mempool