[Qt] show conflicts of unconfirmed transactions in the UI #7826

1. 
   jonasschnelli commented at 3:15 pm on April 6, 2016: contributor

   Currently, the GUI shows only conflicts with transaction in a block.

   This results in displaying replaced transaction (RBF) wrong (the replaced transaction is no longer in the mempool, but still in the wallet-transactionlist, conflicts are not detected).

   This PR will mark unconfirmed transactions that are not in our mempool and have conflicts as conflicted.

2. jonasschnelli added the label GUI on Apr 6, 2016
3. 
   jonasschnelli commented at 3:17 pm on April 6, 2016: contributor

   Another option would be to hide these transactions completely.
4. 
   jonasschnelli commented at 3:21 pm on April 6, 2016: contributor

   Before this PR:

   Sending 49.9BTC and RBF with a 40.0BTC transaction.

   After this PR:

5. in src/qt/transactionrecord.cpp: in 91b63c514b outdated

   271+        LOCK(mempool.cs);
   272+        if (status.inMempool != mempool.exists(hash))
   273+            return true;
   274+    }
   275+
   276+    if (status.cur_num_blocks != chainActive.Height())
   

   ---

   ---

   

   MarcoFalke commented at 3:22 pm on April 6, 2016:

   Nit: I think you can leave those three lines the way it was before: return status.cur_num_blocks != chainActive.Height();

   ---

   

   jonasschnelli commented at 3:23 pm on April 6, 2016:

   Right! Fixed.
6. 
   MarcoFalke commented at 3:23 pm on April 6, 2016: member

   Another option would be to hide these transactions completely.

   I don’t think you want to hide wtx without telling the user/ without having the user told you to do so.

7. jonasschnelli force-pushed on Apr 6, 2016
8. 
   jonasschnelli commented at 3:24 pm on April 6, 2016: contributor

   I don’t think you want to hide wtx without telling the user/ without having the user told you to do so.

   Yes. I agree. IMO the “mark as conflicted” solution is better.

9. 
   MarcoFalke commented at 4:12 pm on April 6, 2016: member

   Hmm, so I have a non-rbf transaction back from august:

   0Status: 0/unconfirmed, not in memory pool
   1Date: 8/19/15 15:51
   

   and sendrawt returns Missing inputs (code -25), but it is not showing the new icon. Need to look into this…

10. 
    jonasschnelli commented at 5:53 pm on April 6, 2016: contributor

    @MarcoFalke: does you non-rbf transaction back from august is in conflict with another transaction in your wallet? Only if it’s so, the conflicted state icon will be shown.
11. 
    MarcoFalke commented at 4:40 pm on April 7, 2016: member

    No, it didn’t conflict within the wallet.

    Tested ACK 614073f

12. 
    MarcoFalke commented at 4:42 pm on April 7, 2016: member

    One could argue to use another status/icon for this e.g. https://raw.githubusercontent.com/jonasschnelli/bitcoin/47196712ec7f3ff2f5f814650d40366cb269fe56/src/qt/res/icons/transaction_replaceable.png with the conflicted icon (cross) in the lower left.
13. jonasschnelli closed this on May 20, 2016

    ---

14. jonasschnelli force-pushed on May 20, 2016
15. [Qt] show mempool conflicts in the UI

    On RPC level, we show possible mempool conflicts already via detecting them over the CWallet::GetConflicts() function.
    Currently, the GUI shows only conflicts with transaction in a block.
    
    This results in displaying replaced transaction (RBF) wrong.
    
    This PR will mark unconfirmed transactions that are _not_ in our mempool and have conflicts as conflicted.

    82962bd8c6
16. jonasschnelli reopened this on May 20, 2016

    ---

17. 
    jonasschnelli commented at 3:51 pm on May 20, 2016: contributor

    Rebased and changed the icon for transactions with conflict but not being in the mempool (RBF):
18. 
    luke-jr commented at 9:45 pm on May 20, 2016: member

    These seem to be two list items referring to the same transaction.
19. 
    jonasschnelli commented at 5:16 am on May 21, 2016: contributor

    @luke-jr: this PR does not focus on hiding out transactions from the wallet (a lot more complexity). At the moment, a replaced RBF transaction is not listed as conflicted (https://github.com/bitcoin/bitcoin/pull/7826#issuecomment-206424972).

    This PR is a tiny fix to improve the GUI RBF attribution.

20. 
    jonasschnelli commented at 5:51 am on May 21, 2016: contributor

    Another solution would be (idea once shortly discusses with @gmaxwell on IRC):

    • directly hide transactions if they are conflicted and not in the mempool (RBF)
    • hide transactions that conflicts with a transaction in a blocks with confirmation >6
    • add option in the settings panel to “display old conflicted transaction” and turn it off by default.
21. 
    sipa commented at 8:55 am on June 5, 2016: member

    @jonasschnelli Any progress here?
22. 
    jonasschnelli commented at 10:09 am on June 5, 2016: contributor

    @sipa: I’m working on a option to auto-hide mempool conflicts (transactions that are not in the blockchain and mempool) (RBF/abandoned txes).
23. 
    jonasschnelli commented at 9:20 am on June 9, 2016: contributor

    I think this solution is fine as it is. The auto-hide option can be written in a separate PR. This PR would be a simple change to allow users to easy identify mempool replacments relevant to the wallet.

    Would be great for 0.13.

24. jonasschnelli added this to the milestone 0.13.0 on Jun 9, 2016
25. 
    laanwj commented at 9:56 am on June 21, 2016: member

    Removing this from the 0.13 milestone as it missed the feature freeze.

    Would be great for 0.13.

    Agree, would have been nice, just like #7817. I think we have a lack of GUI testers/reviewers, as useful GUI changes like this linger way too long.

26. laanwj removed this from the milestone 0.13.0 on Jun 21, 2016
27. 
    jonasschnelli commented at 7:40 am on August 10, 2016: contributor

    This (simple) PR looks for reviewers…
28. 
    MarcoFalke commented at 8:44 am on August 10, 2016: member

    I think the problem is that it may not be obvious for everyone to set up the appropriate transactions to test this. I think it may help to add “proposed steps to reproduce” for harder to test pulls in general.
29. 
    MarcoFalke commented at 8:50 am on August 10, 2016: member

    82962bd Works for standard transactions:

    

    Does not work for non-standard transactions:

    

30. 
    jonasschnelli commented at 9:48 am on January 6, 2017: contributor

    Closing for now due to inactivity…
31. jonasschnelli closed this on Jan 6, 2017

    ---

32. MarcoFalke locked this on Sep 8, 2021

Contributors
jonasschnelli MarcoFalke luke-jr sipa laanwj

Labels
GUI