rpc: Enhance consolidateunspent and fix fee calculation

src/qt/forms/sendcoinsdialog.ui

@@ -6,8 +6,8 @@
    <rect>
     <x>0</x>
     <y>0</y>
-    <width>850</width>
-    <height>400</height>
+    <width>899</width>
+    <height>456</height>
    </rect>
   </property>
   <property name="windowTitle">
@@ -85,7 +85,7 @@
          </layout>
         </item>
         <item>
-         <layout class="QHBoxLayout" name="horizontalLayoutCoinControl2" stretch="0,0,0,0">
+         <layout class="QHBoxLayout" name="horizontalLayoutCoinControl2" stretch="0,0,0,0,0">
           <property name="spacing">
            <number>8</number>
           </property>
@@ -122,6 +122,13 @@
             </property>
            </widget>
           </item>
+          <item>
+           <widget class="QPushButton" name="coinControlResetPushButton">
+            <property name="text">
+             <string>Reset</string>
+            </property>
+           </widget>
+          </item>
           <item>
            <spacer name="coinControlHorizontalSpacer">
             <property name="orientation">
@@ -547,8 +554,8 @@
        <rect>
         <x>0</x>
         <y>0</y>
-        <width>830</width>
-        <height>167</height>
+        <width>869</width>
+        <height>112</height>
        </rect>
       </property>
       <layout class="QVBoxLayout" name="verticalLayout_2">

src/qt/sendcoinsdialog.cpp

@@ -36,6 +36,7 @@ SendCoinsDialog::SendCoinsDialog(QWidget *parent) :
     // Coin Control
     ui->coinControlChangeEdit->setFont(GUIUtil::bitcoinAddressFont());
     connect(ui->coinControlPushButton, SIGNAL(clicked()), this, SLOT(coinControlButtonClicked()));
+    connect(ui->coinControlResetPushButton, SIGNAL(clicked()), this, SLOT(coinControlResetButtonClicked()));
     connect(ui->coinControlChangeCheckBox, SIGNAL(stateChanged(int)), this, SLOT(coinControlChangeChecked(int)));
     connect(ui->coinControlChangeEdit, SIGNAL(textEdited(const QString &)), this, SLOT(coinControlChangeEdited(const QString &)));
 
@@ -436,6 +437,12 @@ void SendCoinsDialog::coinControlButtonClicked()
     coinControlUpdateLabels();
 }
 
+void SendCoinsDialog::coinControlResetButtonClicked()
+{
+    CoinControlDialog::coinControl->SetNull();
+    coinControlUpdateLabels();
+}
+
 // Coin Control: checkbox custom change address
 void SendCoinsDialog::coinControlChangeChecked(int state)
 {

src/qt/sendcoinsdialog.h

@@ -52,6 +52,7 @@ private slots:
     void updateDisplayUnit();
     void coinControlFeatureChanged(bool);
     void coinControlButtonClicked();
+    void coinControlResetButtonClicked();
     void coinControlChangeChecked(int);
     void coinControlChangeEdited(const QString &);
     void coinControlUpdateLabels();

src/rpc/client.cpp

@@ -112,6 +112,8 @@ static const CRPCConvertParam vRPCConvertParams[] =
     { "consolidatemsunspent"   , 4 },
     { "consolidatemsunspent"   , 5 },
     { "consolidatemsunspent"   , 6 },
+    { "consolidateunspent"     , 3 },
+    { "consolidateunspent"     , 4 },
     { "getbalance"             , 1 },
     { "getbalance"             , 2 },
     { "getbalancedetail"       , 0 },

src/rpc/rawtransaction.cpp

@@ -21,6 +21,8 @@
 #include "wallet/coincontrol.h"
 #include "wallet/wallet.h"
 
+#include <queue>
+
 using namespace GRC;
 using namespace std;
 
@@ -488,19 +490,38 @@ UniValue listunspent(const UniValue& params, bool fHelp)
 
 UniValue consolidateunspent(const UniValue& params, bool fHelp)
 {
-    if (fHelp || params.size() < 1 || params.size() > 3)
+    if (fHelp || params.size() < 1 || params.size() > 5)
         throw runtime_error(
-                "consolidateunspent <address> [UTXO size [maximum number of inputs]]\n"
+                "consolidateunspent <address> [UTXO size] [maximum number of inputs] [sweep all addresses] [sweep change]\n"
+                "\n"
+                "<address>:                  The Gridcoin address target for consolidation.\n"
+                "\n"
+                "[UTXO size]:                Optional parameter for target consolidation output size.\n"
+                "\n"
+                "[maximum number of inputs]: Defaults to 50, clamped to 200 maximum to prevent transaction failures.\n"
+                "\n"
+                "[sweep all addresses]:      Boolean to indicate whether all addresses should be used for inputs to the\n"
+                "                            consolidation. If true, the source of the consolidation is all addresses and\n"
+                "                            the output will be to the specified address, otherwise inputs will only be\n"
+                "                            sourced from the same address.\n"
+                "\n"
+                "[sweep change]:             Boolean to indicate whether change associated with the address should be\n"
+                "                            consolidated. If [sweep all addresses] is true then this is also forced true.\n"
+                "\n"
+                "consolidateunspent performs a single transaction to consolidate UTXOs to/on a given address. The optional\n"
+                "parameter of UTXO size will result in consolidating UTXOs to generate the largest output possible less\n"
+                "than that size or the total value of the specified maximum number of smallest inputs, whichever is less.\n"
                 "\n"
-                "Performs a single transaction to consolidate UTXOs on\n"
-                "a given address. The optional parameter of UTXO size will result\n"
-                "in consolidating UTXOs to generate an output less than that size or\n"
-                "the total value of the specified maximum number of smallest inputs,\n"
-                "whichever is less.\n"
+                "The script is designed to be run repeatedly and will become a no-op if the UTXO's are consolidated such\n"
+                "that no more meet the specified criteria. This is ideal for automated periodic scripting.\n"
                 "\n"
-                "The script is designed to be run repeatedly and will become a no-op\n"
-                "if the UTXO's are consolidated such that no more meet the specified\n"
-                "criteria. This is ideal for automated periodic scripting.\n");
+                "To consolidate the entire wallet to one address do something like:\n"
+                "\n"
+                "consolidate unspent <address> <amount equal or larger than balance> 200 true repeatedly until there are\n"
+                "no more UTXOs to consolidate.\n"
+                "\n"
+                "In all cases the address MUST exist in your wallet beforehand. If doing this for the purpose of creating\n"
+                "a new smaller wallet, create a new address beforehand to serve as the target of the consolidation.\n");
 
     UniValue result(UniValue::VOBJ);
 
@@ -514,16 +535,28 @@ UniValue consolidateunspent(const UniValue& params, bool fHelp)
     // about 3x that. The GUI will not be responsive during the transaction due to locking.
     unsigned int nInputNumberLimit = 50;
 
+    bool sweep_all_addresses = false;
+
+    // Note this value is ignored if sweep_all_addresses is set to true.
+    bool sweep_change = false;
+
     if (params.size() > 1) nConsolidateLimit = AmountFromValue(params[1]);
+
     if (params.size() > 2) nInputNumberLimit = params[2].get_int();
 
+    if (params.size() > 3) sweep_all_addresses = params[3].get_bool();
+
+    if (params.size() > 4 && !sweep_all_addresses) sweep_change = params[4].get_bool();
+
     // Clamp InputNumberLimit to 200. Above 200 risks an invalid transaction due to the size.
-    nInputNumberLimit = std::min(nInputNumberLimit, (unsigned int) 200);
+    nInputNumberLimit = std::min<unsigned int>(nInputNumberLimit, 200);
 
     if (!OptimizeAddress.IsValid())
+    {
         throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, string("Invalid Gridcoin address: ") + sAddress);
+    }
 
-    // Set the consolidation transaction address to the same as the inputs to consolidate.
+    // Set the consolidation transaction address to the specified output address.
     CScript scriptDestPubKey;
     scriptDestPubKey.SetDestination(OptimizeAddress.Get());
 
@@ -534,33 +567,107 @@ UniValue consolidateunspent(const UniValue& params, bool fHelp)
     // more UTXO's will have the same nValue.
     std::multimap<int64_t, COutput> mInputs;
 
-    // Have to lock both main and wallet to prevent deadlocks.
+    // Have to lock both main and wallet.
     LOCK2(cs_main, pwalletMain->cs_wallet);
 
     // Get the current UTXO's.
     pwalletMain->AvailableCoins(vecInputs, false, NULL, false);
 
-    // Filter outputs by matching address and insert into sorted multimap.
+    // Filter outputs in the wallet that are the candidate inputs by matching address and insert into sorted multimap.
     for (auto const& out : vecInputs)
     {
-        CTxDestination outaddress;
+        CTxDestination out_address;
+
         int64_t nOutValue = out.tx->vout[out.i].nValue;
 
-        if (!ExtractDestination(out.tx->vout[out.i].scriptPubKey, outaddress)) continue;
+        // If we can't resolve the address, then move on.
+        if (!ExtractDestination(out.tx->vout[out.i].scriptPubKey, out_address)) continue;
 
-        if (CBitcoinAddress(outaddress) == OptimizeAddress)
+        // If the UTXO matches the consolidation address or all sweep_all_addresses is true then add to the inputs
+        // map for consolidation. Note that the value of sweep_change is ignored and all change will be swept.
+        if (CBitcoinAddress(out_address) == OptimizeAddress || sweep_all_addresses)
+        {
             mInputs.insert(std::make_pair(nOutValue, out));
-    }
+        }
+        // If sweep_change is true... and ...
+        // If the UTXO is change (we already know it "ISMINE") then iterate through the inputs to the transaction
+        // that created the change. This loop has the effect of matching the change to the address of the first
+        // input's address of the transaction that created the change. It is possible that the change could have been
+        // created from a transaction whose inputs can from multiple addresses within one's wallet. In this case,
+        // a choice has to be made. This is similar to listaddressgroupings and is a decent approach...
+        else if (sweep_change && pwalletMain->IsChange(out.tx->vout[out.i]))
+        {
+            // Iterate through the inputs of the transaction that created the change. Note that this has to be implemented
+            // as a work queue, because change can stake, and so the input here could still be change, etc. You have to
+            // continue walking up the tree of transactions until you get to a non-change source address. If the non-change
+            // source address matches the consolidation address, the UTXO is included.
 
-    CWalletTx wtxNew;
+            // The work queue.
+            std::queue<std::vector<CTxIn>> vin_queue;
 
-    // For min fee calculation.
-    CTransaction txDummy;
+            // The inital population of the queue is the input vector of the transaction that created the change UTXO.
+            vin_queue.push(out.tx->vin);
+
+
+            // Keep track of the vin vectors processed on the queue and limit to a reasonable value of 25 to prevent
+            // excessively long execution times. This introduces the possibility of failing to resolve a change parent
+            // address that has been through many stakes, but I am concerned about the processing time.
+            unsigned int vins_emplaced = 0;
+
+            while (!vin_queue.empty() && vins_emplaced <= 25)
+            {
+                // Pull the first unit of work.
+                std::vector<CTxIn> v_change_input = vin_queue.front();
+                vin_queue.pop();
+
+                for (const auto& change_input : v_change_input)
+                {
+                    // Get the COutPoint of this transaction input.
+                    COutPoint prevout = change_input.prevout;
+
+                    CTxDestination change_input_address;
+
+                    // Get the transaction that generated this output, which was the input to
+                    // the transaction that created the change.
+                    CWalletTx tx_change_input = pwalletMain->mapWallet[prevout.hash];
+
+                    // Get the corresponding output of that transaction (this is the same as the input to the
+                    // referenced transaction). We need this to resolve the address.
+                    CTxOut prev_ctxout = tx_change_input.vout[prevout.n];
+
+                    // If this is still change then place the input vector of this transaction onto the queue.
+                    if (pwalletMain->IsChange(prev_ctxout))
+                    {
+                        vin_queue.emplace(tx_change_input.vin);
+                        ++vins_emplaced;
+                    }
+
+                    // If not change, then get the address of that output and if it matches the OptimizeAddress add the UTXO
+                    // to the inputs map for consolidation.
+                    if (ExtractDestination(prev_ctxout.scriptPubKey, change_input_address))
+                    {
+                        if (CBitcoinAddress(change_input_address) == OptimizeAddress)
+                        {
+                            // Insert the ORIGINAL change UTXO into the input map for the consolidation.
+                            mInputs.insert(std::make_pair(nOutValue, out));
+
+                            // We do not need/want to add it more than once, and we do not need to continue processing the
+                            // queue if a linkage is found.
+                            break;
+                        }
+                    }
+                } // for (const auto& change_input : v_change_input)
+            } // while (!vin_queue.empty())
+        } // else if (pwalletMain->IsChange(out.tx->vout[out.i]))
+    } // for (auto const& out : vecInputs)
+
+    CWalletTx wtxNew;
 
     set<pair<const CWalletTx*,unsigned int>> setCoins;
 
     unsigned int iInputCount = 0;
-    int64_t nValue = 0;
+    int64_t nAmount = 0;
+    unsigned int nBytesInputs = 0;
 
     // Construct the inputs to the consolidation transaction. Either all of the inputs from above, or 200,
     // or when the total reaches/exceeds nConsolidateLimit, whichever is more limiting. The map allows us
@@ -569,7 +676,7 @@ UniValue consolidateunspent(const UniValue& params, bool fHelp)
     {
         int64_t nUTXOValue = out.second.tx->vout[out.second.i].nValue;
 
-        if (iInputCount == nInputNumberLimit || ((nValue + nUTXOValue) > nConsolidateLimit && nConsolidateLimit != 0)) break;
+        if (iInputCount == nInputNumberLimit || ((nAmount + nUTXOValue) > nConsolidateLimit && nConsolidateLimit != 0)) break;
 
         // This has been moved after the break to change the behavior so that the
         // consolidation is limited to the set of UTXO's SMALLER than the nConsolidateLimit
@@ -589,13 +696,33 @@ UniValue consolidateunspent(const UniValue& params, bool fHelp)
         // to solve both is to include a "change" UTXO to true up the mismatch. This is
         // overly complex and not worth the implementation time.
 
-        nValue += nUTXOValue;
+        nAmount += nUTXOValue;
 
-        LogPrint(BCLog::LogFlags::VERBOSE, "INFO: consolidateunspent: input value = %f, confirmations = %" PRId64, ((double) out.first) / (double) COIN, out.second.nDepth);
+        LogPrint(BCLog::LogFlags::VERBOSE, "INFO: consolidateunspent: input value = %f, confirmations = %" PRId64,
+                 ((double) out.first) / (double) COIN, out.second.nDepth);
 
         setCoins.insert(make_pair(out.second.tx, out.second.i));
 
         ++iInputCount;
+
+        // For fee calculation. This is similar to the calculation in coincontroldialog.cpp.
+        CTxDestination address;
+
+        if(ExtractDestination(out.second.tx->vout[out.second.i].scriptPubKey, address))
+        {
+            CPubKey pubkey;
+            CKeyID *keyid = boost::get<CKeyID>(&address);
+            if (keyid && pwalletMain->GetPubKey(*keyid, pubkey))
+            {
+                nBytesInputs += (pubkey.IsCompressed() ? 148 : 180);
+            }
+            // in all error cases, simply assume 148 here
+            else
+            {
+                nBytesInputs += 148;
+            }
+        }
+        else nBytesInputs += 148;
     }
 
     // If number of inputs that meet criteria is less than two, then do nothing.
@@ -612,16 +739,18 @@ UniValue consolidateunspent(const UniValue& params, bool fHelp)
 
     // Fee calculation to avoid change.
 
+    CTransaction txDummy;
+
     // Bytes
-    // --------- The inputs to the tx - The one output.
-    int64_t nBytes = iInputCount * 148 + 34 + 10;
+    // ----- The inputs to the tx - The one output.
+    int64_t nBytes = nBytesInputs + 2 * 34 + 10;
 
     // Min Fee
     int64_t nMinFee = txDummy.GetMinFee(1, GMF_SEND, nBytes);
 
-    int64_t nFee = nTransactionFee * (1 + nBytes / 1000);
+    int64_t nFee = nTransactionFee * (1 + (int64_t) nBytes / 1000);
 
-    int64_t nFeeRequired = max(nMinFee, nFee);
+    int64_t nFeeRequired = std::max(nMinFee, nFee);
 
 
     if (pwalletMain->IsLocked())
@@ -643,7 +772,7 @@ UniValue consolidateunspent(const UniValue& params, bool fHelp)
     // Reduce the out value for the transaction by nFeeRequired from the total of the inputs to provide a fee
     // to the staker. The fee has been calculated so that no change should be produced from the CreateTransaction
     // call. Just in case, the input address is specified as the return address via coincontrol.
-    vecSend.push_back(std::make_pair(scriptDestPubKey, nValue - nFeeRequired));
+    vecSend.push_back(std::make_pair(scriptDestPubKey, nAmount - nFeeRequired));
 
     CCoinControl coinControl;
 
@@ -653,20 +782,23 @@ UniValue consolidateunspent(const UniValue& params, bool fHelp)
     if (!pwalletMain->CreateTransaction(vecSend, setCoins, wtxNew, reservekey, nFeeRequired, &coinControl))
     {
         string strError;
-        if (nValue + nFeeRequired > pwalletMain->GetBalance())
-            strError = strprintf(_("Error: This transaction requires a transaction fee of at least %s because of its amount, complexity, or use of recently received funds  "), FormatMoney(nFeeRequired));
+        if (nAmount + nFeeRequired > pwalletMain->GetBalance())
+            strError = strprintf(_("Error: This transaction requires a transaction fee of at least %s because of its amount,"
+                                   " complexity, or use of recently received funds "), FormatMoney(nFeeRequired));
         else
             strError = _("Error: Transaction creation failed  ");
         LogPrintf("consolidateunspent: %s", strError);
         return strError;
     }
 
     if (!pwalletMain->CommitTransaction(wtxNew, reservekey))
-        return _("Error: The transaction was rejected.  This might happen if some of the coins in your wallet were already spent, such as if you used a copy of wallet.dat and coins were spent in the copy but not marked as spent here.");
+        return _("Error: The transaction was rejected.  This might happen if some of the coins in your wallet were already"
+                 " spent, such as if you used a copy of wallet.dat and coins were spent in the copy but not marked as spent"
+                 " here.");
 
     result.pushKV("result", true);
     result.pushKV("UTXOs consolidated", (uint64_t) iInputCount);
-    result.pushKV("Output UTXO value", (double)(nValue - nFeeRequired) / COIN);
+    result.pushKV("Output UTXO value", (double)(nAmount - nFeeRequired) / COIN);
 
     return result;
 }