[RPC] [Wallet] AutoCombineRewards fixes and Improvements

src/rpc/client.cpp

@@ -136,6 +136,7 @@ static const CRPCConvertParam vRPCConvertParams[] =
         {"setstakesplitthreshold", 0},
         {"autocombinerewards", 0},
         {"autocombinerewards", 1},
+        {"autocombinerewards", 2},
         {"getzerocoinbalance", 0},
         {"listmintedzerocoins", 0},
         {"listmintedzerocoins", 1},

src/rpc/server.cpp

@@ -418,6 +418,7 @@ static const CRPCCommand vRPCCommands[] =
         {"wallet", "getreceivedbyaddress", &getreceivedbyaddress, false, false, true},
         {"wallet", "getstakingstatus", &getstakingstatus, false, false, true},
         {"wallet", "getstakesplitthreshold", &getstakesplitthreshold, false, false, true},
+        {"wallet", "getautocombineinfo", &getautocombineinfo, false, false, true},
         {"wallet", "gettransaction", &gettransaction, false, false, true},
         {"wallet", "abandontransaction", &abandontransaction, false, false, true},
         {"wallet", "getunconfirmedbalance", &getunconfirmedbalance, false, false, true},

src/rpc/server.h

@@ -264,6 +264,7 @@ extern UniValue getnetworkinfo(const UniValue& params, bool fHelp);
 extern UniValue setstakesplitthreshold(const UniValue& params, bool fHelp);
 extern UniValue getstakesplitthreshold(const UniValue& params, bool fHelp);
 extern UniValue multisend(const UniValue& params, bool fHelp);
+extern UniValue getautocombineinfo(const UniValue& params, bool fHelp);
 extern UniValue autocombinerewards(const UniValue& params, bool fHelp);
 extern UniValue getzerocoinbalance(const UniValue& params, bool fHelp);
 extern UniValue listmintedzerocoins(const UniValue& params, bool fHelp);

src/wallet/rpcwallet.cpp

@@ -3034,6 +3034,8 @@ UniValue setstakesplitthreshold(const UniValue& params, bool fHelp)
             "{\n"
             "  \"threshold\": n,        (numeric) Threshold value set\n"
             "  \"saved\": true|false    (boolean) 'true' if successfully saved to the wallet file\n"
+            "  \"warning\":             (string)  A human readable warning, if any are present.\n"
+
             "}\n"
 
             "\nExamples:\n" +
@@ -3043,6 +3045,16 @@ UniValue setstakesplitthreshold(const UniValue& params, bool fHelp)
 
     CAmount nStakeSplitThreshold = AmountFromValue(params[0]);
 
+    std::string strComment;
+
+    // Warn if this will conflict with AutoCombineRewards
+    if (((pwalletMain->nAutoCombineThreshold > nStakeSplitThreshold) || !pwalletMain->nAutoCombineThreshold)
+          && (pwalletMain->fCombineDust)) {
+        strComment = "***Warning: Threshold level may conflict with autocombinerewards threshold!***";
+    } else {
+        strComment = "(none)";
+    }
+
     CWalletDB walletdb(pwalletMain->strWalletFile);
     LOCK(pwalletMain->cs_wallet);
     {
@@ -3057,6 +3069,7 @@ UniValue setstakesplitthreshold(const UniValue& params, bool fHelp)
         } else
             result.push_back(Pair("saved", "false"));
 
+        result.push_back(Pair("warning", strComment));
         return result;
     }
 }
@@ -3077,38 +3090,146 @@ UniValue getstakesplitthreshold(const UniValue& params, bool fHelp)
     return ValueFromAmount(pwalletMain->nStakeSplitThreshold);
 }
 
+UniValue getautocombineinfo(const UniValue& params, bool fHelp)
+{
+    if (fHelp || params.size() != 0)
+        throw std::runtime_error(
+            "getautocombineinfo\n"
+            "Returns the autocombinerewards settings\n"
+            "\nResult:\n"
+            "1. enabled   (boolean) The feature is enabled (true) or disabled (false).\n"
+            "2. threshold (numeric) If enabled, returns autocombine threshold.\n"
+            "3. frequency (numeric) If enabled, returns frequency in blocks.\n"
+            "4. comment   (string)  A human readable state of the settings.\n"
+            "5. warning   (string)  A human readable warning, if any are present.\n");
+
+    UniValue obj(UniValue::VOBJ);
+    obj.push_back(Pair("enabled", pwalletMain->fCombineDust));
+    obj.push_back(Pair("threshold", int(pwalletMain->nAutoCombineThreshold)));
+    obj.push_back(Pair("frequency", int(pwalletMain->nAutoCombineBlockFrequency)));
+
+    std::string strComment;
+    if (pwalletMain->nAutoCombineThreshold == 0)
+        strComment = "Enabled for maximum combine, ";
+    else
+        strComment = "Enabled for ";
+
+    if (pwalletMain->fCombineDust) {
+        if (pwalletMain->nAutoCombineBlockFrequency == 0) {
+            strComment += "one shot on next block";
+        } else {
+            strComment += "repeat on frequency";
+        }
+    } else {
+        if (pwalletMain->nAutoCombineBlockFrequency == 0) {
+            strComment += "one shot on startup";
+        } else {
+            strComment = "Disabled";
+        }
+    }
+    obj.push_back(Pair("comment", strComment));
+
+    // Warn if this will conflict with StakeSplitThreshold
+    if (((pwalletMain->nAutoCombineThreshold > pwalletMain->nStakeSplitThreshold) || !pwalletMain->nAutoCombineThreshold)
+          && (pwalletMain->fCombineDust)) {
+        strComment = "***Warning: Threshold level may conflict with stakesplitthreshold!***";
+    } else {
+        strComment = "(none)";
+    }
+    obj.push_back(Pair("warning", strComment));
+
+    return obj;
+}
+
 UniValue autocombinerewards(const UniValue& params, bool fHelp)
 {
-    bool fEnable;
-    if (params.size() >= 1)
+    bool fEnable = false;
+
+    if (params.size() >= 1) {
         fEnable = params[0].get_bool();
+    }
 
-    if (fHelp || params.size() < 1 || (fEnable && params.size() != 2) || params.size() > 2)
+    if (fHelp || params.size() < 1 || (fEnable && params.size() < 2) || params.size() > 3)
         throw std::runtime_error(
-            "autocombinerewards enable ( threshold )\n"
-            "\nWallet will automatically monitor for any coins with value below the threshold amount, and combine them if they reside with the same PIVX address\n"
-            "When autocombinerewards runs it will create a transaction, and therefore will be subject to transaction fees.\n"
+            "autocombinerewards enable ( threshold ) ( frequency )\n"
+            "\nWallet will automatically monitor for UTXOs with values below the threshold amount, "
+            "and combine them into transactions sized to the threshold amount, if they reside with "
+            "the same PIVX address.\n"
+            "\nA threshold value of \"0\" will combine all the UTXOs, up to the maximum possible "
+            "within the maximum transaction size of a block.\n"
+            "\nA frequency value of \"0\" will run the combine once, on the next available block, "
+            "and once again on each wallet startup.\n"
+            "\nWhen autocombinerewards runs it will create a transaction, and therefore will be subject "
+            "to transaction fees.  Transactions will be limited to a full combine of the threshold "
+            "amount unless the transaction fees are zero.\n"
 
             "\nArguments:\n"
-            "1. enable          (boolean, required) Enable auto combine (true) or disable (false)\n"
-            "2. threshold       (numeric, optional) Threshold amount (default: 0)\n"
+            "1. enable    (boolean, required) Enable auto combine (true) or disable (false).\n"
+            "2. threshold (numeric, optional) (required for enable) target total PIV to combine into one UTXO.\n"
+            "3. frequency (numeric, optional) Frequency (in blocks) for autocombine to run (default: 15)\n"
+
+            "\nResult:\n"
+            "1. enabled   (boolean) The feature is enabled (true) or disabled (false).\n"
+            "2. threshold (numeric) If enabled, returns autocombine threshold.\n"
+            "3. frequency (numeric) If enabled, returns frequency in blocks.\n"
+            "4. comment   (string)  A human readable state of the settings.\n"
+            "5. warning   (string)  A human readable warning, if any are present.\n"
 
             "\nExamples:\n" +
-            HelpExampleCli("autocombinerewards", "true 500") + HelpExampleRpc("autocombinerewards", "true 500"));
+            HelpExampleCli("autocombinerewards", "true 500 15") + HelpExampleRpc("autocombinerewards", "true 500 15"));
 
     CWalletDB walletdb(pwalletMain->strWalletFile);
     CAmount nThreshold = 0;
+    int nBlockFrequency = 15;
 
-    if (fEnable)
+    if (fEnable) {
         nThreshold = params[1].get_int();
+        if (params.size() > 2) {
+            nBlockFrequency = params[2].get_int();
+            if (nBlockFrequency < 0)
+                nBlockFrequency = 1;
+        }
+    }
 
     pwalletMain->fCombineDust = fEnable;
     pwalletMain->nAutoCombineThreshold = nThreshold;
+    pwalletMain->nAutoCombineBlockFrequency = nBlockFrequency;
 
-    if (!walletdb.WriteAutoCombineSettings(fEnable, nThreshold))
+    if (!walletdb.WriteAutoCombineSettings(fEnable, nThreshold, nBlockFrequency))
         throw std::runtime_error("Changed settings in wallet but failed to save to database\n");
 
-    return NullUniValue;
+    UniValue obj(UniValue::VOBJ);
+    obj.push_back(Pair("enabled", pwalletMain->fCombineDust));
+    if (pwalletMain->fCombineDust) {
+        std::string strComment;
+
+        obj.push_back(Pair("threshold", int(pwalletMain->nAutoCombineThreshold)));
+        obj.push_back(Pair("frequency", int(pwalletMain->nAutoCombineBlockFrequency)));
+
+        if (pwalletMain->nAutoCombineThreshold == 0) {
+            strComment = "Enabled for maximum combine, ";
+        } else {
+            strComment = "Enabled for ";
+        }
+
+        if (pwalletMain->nAutoCombineBlockFrequency == 0) {
+            strComment += "one shot on next block";
+        } else {
+            strComment += "repeat on frequency";
+        }
+        obj.push_back(Pair("comment", strComment));
+
+        // Warn if this will conflict with StakeSplitThreshold
+        if ((pwalletMain->nAutoCombineThreshold > pwalletMain->nStakeSplitThreshold) || !pwalletMain->nAutoCombineThreshold) {
+            strComment = "***Warning: Threshold level may conflict with stakesplitthreshold!***";
+        } else {
+            strComment = "(none)";
+        }
+        obj.push_back(Pair("warning", strComment));
+
+    }
+
+    return obj;
 }
 
 UniValue printMultiSend()

src/wallet/wallet.cpp

@@ -3361,6 +3361,18 @@ void CWallet::AutoCombineDust()
         return;
     }
 
+    if (nAutoCombineBlockFrequency != 0) {
+        // If the block height hasn't exceeded our frequency; or is not a multiple of our frequency.
+        if ((nAutoCombineBlockFrequency > chainActive.Tip()->nHeight) ||
+            (chainActive.Tip()->nHeight % nAutoCombineBlockFrequency)) {
+            return;
+        }
+    } else {
+        // If nAutoCombineBlockFrequency is 0, it's the special onetime case
+        // so let it rip but turn it off so it doesn't rip again.
+        fCombineDust = 0;
+    }
+
     std::map<CBitcoinAddress, std::vector<COutput> > mapCoinsByAddress = AvailableCoinsByAddress(true, nAutoCombineThreshold * COIN);
 
     //coins are sectioned by address. This combination code only wants to combine inputs that belong to the same address
@@ -3391,9 +3403,10 @@ void CWallet::AutoCombineDust()
             coinControl->Select(outpt);
             vRewardCoins.push_back(out);
             nTotalRewardsValue += out.Value();
-
-            // Combine to the threshold and not way above
-            if (nTotalRewardsValue > nAutoCombineThreshold * COIN)
+            // Combine until our total is enough above the threshold to remain above after adjustments
+            // Unless no threshold is set; in which case we want to keep going until we hit MAX_STANDARD_TX_SIZE
+            if (nAutoCombineThreshold &&
+                ((nTotalRewardsValue - nTotalRewardsValue / 10) > nAutoCombineThreshold * COIN))
                 break;
 
             // Around 180 bytes per input. We use 190 to be certain
@@ -3408,13 +3421,16 @@ void CWallet::AutoCombineDust()
         if (!coinControl->HasSelected())
             continue;
 
-        //we cannot combine one coin with itself
-        if (vRewardCoins.size() <= 1)
+        //we cannot combine one coin with itself, nor do we want to constantly combine the previous
+        //combine with the change.
+        if (vRewardCoins.size() <= 2)
             continue;
 
         std::vector<std::pair<CScript, CAmount> > vecSend;
         CScript scriptPubKey = GetScriptForDestination(it->first.Get());
-        vecSend.push_back(std::make_pair(scriptPubKey, nTotalRewardsValue));
+
+        // 10% safety margin to avoid "Insufficient funds" errors
+        vecSend.push_back(std::make_pair(scriptPubKey, nTotalRewardsValue - (nTotalRewardsValue / 10)));
 
         //Send change to same address
         CTxDestination destMyAddress;
@@ -3430,24 +3446,22 @@ void CWallet::AutoCombineDust()
         std::string strErr;
         CAmount nFeeRet = 0;
 
-        // 10% safety margin to avoid "Insufficient funds" errors
-        vecSend[0].second = nTotalRewardsValue - (nTotalRewardsValue / 10);
-
         if (!CreateTransaction(vecSend, wtx, keyChange, nFeeRet, strErr, coinControl, ALL_COINS, false, CAmount(0))) {
             LogPrintf("AutoCombineDust createtransaction failed, reason: %s\n", strErr);
             continue;
         }
 
         //we don't combine below the threshold unless the fees are 0 to avoid paying fees over fees over fees
-        if (!maxSize && nTotalRewardsValue < nAutoCombineThreshold * COIN && nFeeRet > 0)
+        if (!maxSize && vecSend[0].second < nAutoCombineThreshold * COIN && nFeeRet > 0)
             continue;
 
         if (!CommitTransaction(wtx, keyChange)) {
             LogPrintf("AutoCombineDust transaction commit failed\n");
             continue;
         }
 
-        LogPrintf("AutoCombineDust sent transaction\n");
+        LogPrintf("AutoCombineDust sent transaction. Fee=%d, Total Value=%d Sending=%d\n",
+                  nFeeRet, nTotalRewardsValue, vecSend[0].second);
 
         delete coinControl;
     }

src/wallet/wallet.h

@@ -308,6 +308,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface
     //Auto Combine Inputs
     bool fCombineDust;
     CAmount nAutoCombineThreshold;
+    int nAutoCombineBlockFrequency;
 
     CWallet();
     CWallet(std::string strWalletFileIn);

src/wallet/walletdb.cpp

@@ -249,13 +249,22 @@ bool CWalletDB::EraseMSDisabledAddresses(std::vector<std::string> vDisabledAddre
     */
 }
 
-bool CWalletDB::WriteAutoCombineSettings(bool fEnable, CAmount nCombineThreshold)
+bool CWalletDB::WriteAutoCombineSettings(bool fEnable, CAmount nCombineThreshold, int nBlockFrequency)
 {
     nWalletDBUpdated++;
-    std::pair<bool, CAmount> pSettings;
-    pSettings.first = fEnable;
-    pSettings.second = nCombineThreshold;
-    return Write(std::string("autocombinesettings"), pSettings, true);
+    // Overwrite the old format with a special flag
+    std::pair<bool, CAmount> pSettingsOld;
+    pSettingsOld.first = fEnable;
+    pSettingsOld.second = -1; // Negatives doesn't make sense, so we can use them as flags
+    if (Write(std::string("autocombinesettings"), pSettingsOld, true)) {
+        // Now add the new format as v2
+        std::pair<bool, CAmount> enabledMS1(fEnable, nCombineThreshold);
+        std::pair<std::pair<bool, CAmount>,int> pSettings(enabledMS1, nBlockFrequency);
+        return Write(std::string("autocombinesettingsV2"), pSettings, true);
+    } else {
+        // report the old format write failed
+        return false;
+    }
 }
 
 bool CWalletDB::ReadPool(int64_t nPool, CKeyPool& keypool)
@@ -674,10 +683,23 @@ bool ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, CW
             pwallet->vDisabledAddresses.push_back(strDisabledAddress);
             */
         } else if (strType == "autocombinesettings") {
+            // Old Format
             std::pair<bool, CAmount> pSettings;
             ssValue >> pSettings;
-            pwallet->fCombineDust = pSettings.first;
-            pwallet->nAutoCombineThreshold = pSettings.second;
+            if (pSettings.second >= 0) {
+                // Convert the old format
+                pwallet->fCombineDust = pSettings.first;
+                pwallet->nAutoCombineThreshold = pSettings.second;
+                pwallet->nAutoCombineBlockFrequency = 15; // Default
+                LogPrintf("autocombinerewards settings are stale, refresh your settings for the new format\n");
+            }
+        } else if (strType == "autocombinesettingsV2") {
+            // New Format
+            std::pair<std::pair<bool, CAmount>,int> pSettings;
+            ssValue >> pSettings;
+            pwallet->fCombineDust = pSettings.first.first;
+            pwallet->nAutoCombineThreshold = pSettings.first.second;
+            pwallet->nAutoCombineBlockFrequency = pSettings.second;
         } else if (strType == "destdata") {
             std::string strAddress, strKey, strValue;
             ssKey >> strAddress;

src/wallet/walletdb.h

@@ -142,7 +142,7 @@ class CWalletDB : public CDB
     bool WriteMSettings(bool fMultiSendStake, bool fMultiSendMasternode, int nLastMultiSendHeight);
     bool WriteMSDisabledAddresses(std::vector<std::string> vDisabledAddresses);
     bool EraseMSDisabledAddresses(std::vector<std::string> vDisabledAddresses);
-    bool WriteAutoCombineSettings(bool fEnable, CAmount nCombineThreshold);
+    bool WriteAutoCombineSettings(bool fEnable, CAmount nCombineThreshold, int nBlockFrequency);
 
     bool ReadPool(int64_t nPool, CKeyPool& keypool);
     bool WritePool(int64_t nPool, const CKeyPool& keypool);