Address review comments

Signed-off-by: Mihai Ciumeica <mihai@thirdhash.com>

src/wallet/rpcwalletext.cpp

@@ -484,13 +484,13 @@ static void TxWithOutputsToJSON(const CWalletTx &wtx, CWallet *const pwallet,
   UniValue entry(UniValue::VOBJ);
 
   // GetAmounts variables
-  std::list<COutputEntry> listReceived;
-  std::list<COutputEntry> listSent;
+  std::list<COutputEntry> list_received;
+  std::list<COutputEntry> list_sent;
   CAmount fee;
   CAmount amount = 0;
-  std::string strSentAccount;
+  std::string sent_account;
 
-  wtx.GetAmounts(listReceived, listSent, fee, strSentAccount, ISMINE_ALL);
+  wtx.GetAmounts(list_received, list_sent, fee, sent_account, ISMINE_ALL);
 
   if (wtx.IsFromMe(ISMINE_WATCH_ONLY) && !(watchonly & ISMINE_WATCH_ONLY)) {
     return;
@@ -501,38 +501,38 @@ static void TxWithOutputsToJSON(const CWalletTx &wtx, CWallet *const pwallet,
 
   UniValue outputs(UniValue::VARR);
   // common to every type of transaction
-  if (!strSentAccount.empty()) {
-    entry.pushKV("account", strSentAccount);
+  if (!sent_account.empty()) {
+    entry.pushKV("account", sent_account);
   }
   WalletTxToJSON(wtx, entry);
 
-  if (!!listSent.empty()) {
+  if (!list_sent.empty()) {
     entry.pushKV("abandoned", wtx.isAbandoned());
   }
 
-  std::set<int> receivedOutputs;
-  for (const auto &r : listReceived) {
-    receivedOutputs.insert(r.vout);
+  std::set<int> receive_outputs;
+  for (const auto &r : list_received) {
+    receive_outputs.insert(r.vout);
   }
 
   // sent
-  if (!listSent.empty()) {
+  if (!list_sent.empty()) {
     entry.pushKV("fee", ValueFromAmount(-fee));
-    for (const auto &s : listSent) {
+    for (const auto &s : list_sent) {
       UniValue output(UniValue::VOBJ);
       if (!OutputToJSON(output, s, pwallet, wtx, watchonly)) {
         return;
       }
       amount -= s.amount;
-      if (receivedOutputs.count(s.vout) == 0) {
+      if (receive_outputs.count(s.vout) == 0) {
         output.pushKV("amount", ValueFromAmount(-s.amount));
         outputs.push_back(output);
       }
     }
   }
 
   // received
-  for (const auto &r : listReceived) {
+  for (const auto &r : list_received) {
     UniValue output(UniValue::VOBJ);
     if (!OutputToJSON(output, r, pwallet, wtx, watchonly)) {
       return;
@@ -559,7 +559,7 @@ static void TxWithOutputsToJSON(const CWalletTx &wtx, CWallet *const pwallet,
   } else if (!fee) {
     entry.pushKV("category", "receive");
   } else if (amount == 0) {
-    if (listSent.empty()) {
+    if (list_sent.empty()) {
       entry.pushKV("fee", ValueFromAmount(-fee));
     }
     entry.pushKV("category", "internal_transfer");

src/wallet/wallet.cpp

@@ -1685,7 +1685,9 @@ void CWalletTx::GetAmounts(std::list<COutputEntry>& listReceived,
 
     // Compute fee:
     CAmount nDebit = GetDebit(filter);
-    if (nDebit > 0) { // debit>0 means we signed/sent this transaction
+    if (nDebit > 0 && !IsCoinBase()) {
+        // debit>0 means we signed/sent this transaction
+        // Coinbase transactions cannot have fees
         CAmount nValueOut = tx->GetValueOut();
         nFee = nDebit - nValueOut;
     }
@@ -1699,7 +1701,7 @@ void CWalletTx::GetAmounts(std::list<COutputEntry>& listReceived,
         //   2) the output is to us (received)
         if (nDebit > 0) {
             // Don't report 'change' txouts
-            if (pwallet->IsChange(txout)) {
+            if (!IsCoinBase() && pwallet->IsChange(txout)) {
                 continue;
             }
         } else if (!(fIsMine & filter)) {
@@ -1717,6 +1719,20 @@ void CWalletTx::GetAmounts(std::list<COutputEntry>& listReceived,
 
         COutputEntry output = {address, txout.nValue, (int)i};
 
+        if (IsCoinBase()) {
+            // TODO UNIT-E: implement a better way to distinguish reward outputs
+            // For example, current approach does not work in case of stake splitting
+            if (i == tx->vout.size() - 1) {
+                // We consider the last output (and only it) to be a non-reward output
+                if (nDebit > 0 && !(fIsMine & filter)) {
+                    listSent.push_back(output);
+                }
+            } else if (fIsMine & filter) {
+                listReceived.push_back(output);
+            }
+            continue;
+        }
+
         // If we are debited by the transaction, add the output as a "sent" entry
         if (nDebit > 0) {
             listSent.push_back(output);