Update block-tester #5422

1. 
   TheBlueMatt commented at 11:23 am on December 4, 2014: member

   Built on top of #5267.
2. TheBlueMatt renamed this:
   Remove useless removed-tx tracking from mempool remove calls
   Update block-tester and Remove useless removed-tx tracking from mempool remove calls
   on Dec 4, 2014
3. 
   TheBlueMatt commented at 9:59 pm on December 4, 2014: member

   #5267 no longer contains the pull-tester update, which is still a part of this.
4. 
   morcos commented at 10:38 pm on December 4, 2014: member

   I tried out a variation of @dgenr8’s comments in regtest by creating 2 coinbases and using them as inputs for a transaction, but then creating a block with a different tx which double spends one of those inputs. The qt client correctly shows that the original transaction is conflicted, but what it doesn’t do is realize that one of the inputs spent by the original transaction is now available again and therefore it shows the wrong balance. This is a result of MarkDirty() not getting called on the inputs for the previous transaction anymore.
5. TheBlueMatt force-pushed on Dec 5, 2014
6. 
   dgenr8 commented at 1:52 am on December 5, 2014: contributor

   The visible status update problem I saw may be an existing wallet problem. The wallet gui accesses the mempool directly to set the transaction status icon, so it will notice the removed transaction next time it checks for it in the mempool. But it tries to do this only when it thinks the transaction is actually showing in the gui, and there is also logic that presumes status changes will only happen when a block has been added since the last gui update.

   What I want to test is the tx not showing visually when the block arrives, then being exposed later by user action such as switching to the history tab. I won’t be able to do this for several days though.

   The report of @morcos is consistent, because the isDirty update only happens on explicit SyncWithWallets (it does not directly access the mempool).

7. TheBlueMatt force-pushed on Dec 5, 2014
8. TheBlueMatt force-pushed on Dec 5, 2014
9. laanwj added the label Tests on Dec 5, 2014
10. TheBlueMatt force-pushed on Dec 5, 2014
11. TheBlueMatt force-pushed on Dec 5, 2014
12. TheBlueMatt force-pushed on Dec 5, 2014
13. TheBlueMatt force-pushed on Dec 5, 2014
14. 
    morcos commented at 8:20 pm on December 5, 2014: member

    I think you might be in the middle of changing this around, but here is the little test I put together. I don’t know the code well enough to understand what’s happening, but not only does the GUI show an incorrect balance in this test, but the rpc getbalance returns the wrong value and fails the assertion. However, if you change node0 in my test to a bitcoind instead of a bitcoin-qt, then the rpc getbalance works properly. That seems odd to me.
15. 
    TheBlueMatt commented at 10:48 pm on December 5, 2014: member

    @morcos wow…that makes it sound like the only reason the balances were previously consistent is because of a race condition…(+/- the fact that cs_main is held, so it may be safe still…). Anyway, thanks for the testing @dgenr8 and @morcos…wanna test again with the latest MarkDirty() additions?
16. 
    TheBlueMatt commented at 10:48 pm on December 5, 2014: member

    I’m still working on the test changes, which maybe should just be a separate pull request, but..oh well.
17. 
    TheBlueMatt commented at 10:50 pm on December 5, 2014: member

    @morcos Thanks, I’ll try to take a look later today and merge that in…might be a bit before I can, though.
18. TheBlueMatt force-pushed on Dec 5, 2014
19. TheBlueMatt force-pushed on Dec 6, 2014
20. 
    TheBlueMatt commented at 8:35 am on December 6, 2014: member

    OK, thanks a ton @dgenr8 and @morcos, that part of the change is now in #5433.
21. TheBlueMatt renamed this:
    Update block-tester and Remove useless removed-tx tracking from mempool remove calls
    Update block-tester
    on Dec 6, 2014
22. TheBlueMatt force-pushed on Dec 20, 2014
23. TheBlueMatt force-pushed on Dec 20, 2014
24. TheBlueMatt force-pushed on Dec 20, 2014
25. 
    TheBlueMatt commented at 8:34 am on December 20, 2014: member

    I apologize this took so long, (see the various fixes commit to see why it took so many travis round-trips…), but this should now be good to go…
26. 
    TheBlueMatt commented at 8:34 am on December 20, 2014: member

    Note that this will delay travis results somewhat, I’d think but hopefully not too much.
27. 
    fanquake commented at 12:25 pm on January 23, 2015: member

    Needs rebase.
28. New pull tester which adds a rather simple large-reorg test 1129bed867
29. Various fixes to make travis work with the larger blocktester

    Travis needed a number of things, including disabling the new
    tests if we're running with DEBUG_LOCKORDER (to prevent timeout),
    logging output to a file instead of stderr to avoid running travis
    over its max-log limit, disabling the new tests entirely on
    Windows (wine), and reducing dbcache for the new tests (to avoid
    Travis OOMing us).

    b19cab0e6d
30. TRAVIS: Run dmesg on signal 137 (prints basic OOM kill info) ebe3e4cb3d
31. TheBlueMatt force-pushed on Jan 27, 2015
32. 
    TheBlueMatt commented at 3:41 pm on January 27, 2015: member

    Rebased, thanks.
33. 
    jgarzik commented at 6:42 pm on July 23, 2015: contributor

    Status?
34. 
    laanwj commented at 10:07 pm on October 1, 2015: member

    Closing due to inactivity
35. laanwj closed this on Oct 1, 2015

    ---

36. MarcoFalke locked this on Sep 8, 2021

Contributors
TheBlueMatt morcos dgenr8 fanquake jgarzik laanwj

Labels
Tests