Display coinjoins properly in QT #7101

1. 
   raedah commented at 8:02 am on November 26, 2015: none

   Big diff from modifying the main conditional, but actually only a few lines changed. This resolves #6814
2. display tx details correctly when part of a coinjoin f01b195a00
3. change main conditional block 73496aaa0b
4. in src/qt/transactionrecord.cpp: in 73496aaa0b

   41@@ -42,7 +42,80 @@ QList<TransactionRecord> TransactionRecord::decomposeTransaction(const CWallet *
   42     uint256 hash = wtx.GetHash();
   43     std::map<std::string, std::string> mapValue = wtx.mapValue;
   44 
   45-    if (nNet > 0 || wtx.IsCoinBase())
   46+    bool involvesWatchAddress = false;
   

   ---

   ---

   

   jonasschnelli commented at 8:04 am on November 26, 2015:

   This looks strange?! Do you only want to include bool involvesWatchAddress = false; into the if condition? If so, i think a intent would be nice.

   ---

   

   raedah commented at 6:16 pm on November 26, 2015:

   that if is in red. its gone. Note that only a few lines were changed in this file, but because of indentation, the diff looks huge.

   ---

   

   jonasschnelli commented at 6:58 pm on November 26, 2015:

   Argh. Again I got fooled by the colors. Never mind my previous comment.
5. 
   jonasschnelli commented at 8:05 am on November 26, 2015: contributor

   Thanks! I’m also working on a fix for coin join transaction, will test and improve this.
6. laanwj added the label Wallet on Nov 26, 2015
7. jonasschnelli added the label GUI on Nov 26, 2015
8. 
   raedah commented at 8:21 pm on November 28, 2015: none

   This fixes the transaction list so that it shows 1 row per transaction instead of 2, and for the amount shows the transaction Net value. More importantly, this fixes the details view so that Debits and Credits are shown accurately, allowing one to discern what Debits and Credits were theirs in this transaction.

   What additions are you planning?

9. 
   jonasschnelli commented at 7:43 am on November 30, 2015: contributor

   @raedah: I think we should visually distinct coin-join transactions. Maybe adding in/out types for sub.type = TransactionRecord::MixedXXX; and use a different icon or row background (slightly different color) could make sense.
10. 
    raedah commented at 7:49 am on December 3, 2015: none

    I suppose a coinjoin transaction would be identified as a transaction which has inputs which are yours but also has inputs that are not yours. My cpp is intro level, so I can’t write this new logic that is not yet in the code.
11. 
    raedah commented at 11:37 pm on December 15, 2015: none

    I dont see any more changes incoming. Can this be merged as is so it can get some testing?
12. 
    jonasschnelli commented at 7:20 am on December 16, 2015: contributor

    This is not how merge things. I really think we first need a clear concept how we display CJ in the UI.

    Also we mostly only merge PRs that are tested through multiple contributors (tested ACKs).

13. 
    raedah commented at 10:49 pm on December 25, 2015: none

    There is already a definition that coinjoins fall under. It was already in the code as ‘Mixed debit transaction’. This update fixes the logic to remove the assumption that Net positive transactions only have credits and no debits. Right now none of the debit information is being shown in the details view, which makes it extra difficult to try and discern what is happening with failed transactions (double spends) because bitcoin-qt doesnt tell you what your transaction inputs were, so its not obvious where to check an address balance to see if your coins moved or not. This patch fixes that.
14. in src/qt/transactiondesc.cpp: in 73496aaa0b

    144-    {
    145-        //
    146-        // Credit
    147-        //
    148-        strHTML += "<b>" + tr("Credit") + ":</b> " + BitcoinUnits::formatHtmlWithUnit(unit, nNet) + "<br>";
    149-    }
    

    ---

    ---

    

    luke-jr commented at 6:30 pm on February 12, 2016:

    I don’t see how this removal isn’t a regression…
15. 
    laanwj commented at 12:44 pm on February 15, 2016: member

    There seems to be wide disagreement on these changes, and there is no active development going on here, so I’m going to close this. For anyone picking this up, please open a new pull request or ask to reopen this one.
16. laanwj closed this on Feb 15, 2016

    ---

17. raedah deleted the branch on Feb 21, 2016
18. MarcoFalke locked this on Sep 8, 2021

Contributors
raedah jonasschnelli luke-jr laanwj

Labels
GUI Wallet