[RPC] [Wallet] AutoCombineRewards fixes and Improvements

src/rpc/client.cpp

@@ -128,6 +128,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", "getunconfirmedbalance", &getunconfirmedbalance, false, false, true},
         {"wallet", "getwalletinfo", &getwalletinfo, false, false, true},

src/rpc/server.h

@@ -249,6 +249,7 @@ extern UniValue reservebalance(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

@@ -2262,38 +2262,107 @@ UniValue getstakesplitthreshold(const UniValue& params, bool fHelp)
     return int(pwalletMain->nStakeSplitThreshold);
 }
 
+UniValue getautocombineinfo(const UniValue& params, bool fHelp)
+{
+    if (fHelp || params.size() != 0)
+        throw runtime_error(
+            "getautocombineinfo\n"
+            "Returns the autocombinerewards settings\n"
+            "\nResult:\n"
+            "1. enabled   (string) The feature turned \"on\" or \"off\".\n"
+            "2. threshold (numeric) If enabled on, returns autocombine threshold.\n"
+            "3. frequency (variable) If enabled, returns frequency in blocks, or \"nextblock\" if one time. "
+                                    "If one time already run, \"startup\" is returned\n");
+
+    UniValue obj(UniValue::VOBJ);
+    obj.push_back(Pair("enabled", pwalletMain->fCombineDust ? "on" : "off"));
+    if (pwalletMain->fCombineDust) {
+        obj.push_back(Pair("threshold",
+                           int(pwalletMain->nAutoCombineThreshold)));
+        if (pwalletMain->nAutoCombineBlockFrequency == 0) {
+            obj.push_back(Pair("frequency", "nextblock"));
+        } else {
+            obj.push_back(Pair("frequency",
+                           int(pwalletMain->nAutoCombineBlockFrequency)));
+        }
+    }
+    else {
+        if (pwalletMain->nAutoCombineBlockFrequency == 0) {
+            obj.push_back(Pair("threshold", int(pwalletMain->nAutoCombineThreshold)));
+            obj.push_back(Pair("frequency", "startup"));
+        }
+    }
+
+    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 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   (string) The feature turned \"on\" or \"off\".\n"
+            "2. threshold (numeric) If enabled on, returns autocombine threshold.\n"
+            "3. frequency (variable) If enabled, returns frequency in blocks, or \"nextblock\" if one time.\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 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 ? "on" : "off"));
+    if (pwalletMain->fCombineDust) {
+        obj.push_back(Pair("threshold", int(pwalletMain->nAutoCombineThreshold)));
+        if (pwalletMain->nAutoCombineBlockFrequency == 0) {
+            obj.push_back(Pair("frequency", "nextblock"));
+        } else {
+            obj.push_back(Pair("frequency", int(pwalletMain->nAutoCombineBlockFrequency)));
+        }
+    }
+
+    return obj;
 }
 
 UniValue printMultiSend()

src/wallet/wallet.cpp

@@ -4190,6 +4190,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;
+    }
+
     map<CBitcoinAddress, 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
@@ -4216,9 +4228,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
@@ -4264,15 +4277,16 @@ void CWallet::AutoCombineDust()
         }
 
         //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

@@ -279,6 +279,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface
     //Auto Combine Inputs
     bool fCombineDust;
     CAmount nAutoCombineThreshold;
+    int nAutoCombineBlockFrequency;
 
     CWallet()
     {
@@ -330,6 +331,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface
         //Auto Combine Dust
         fCombineDust = false;
         nAutoCombineThreshold = 0;
+        nAutoCombineBlockFrequency = 15;
     }
 
     int getZeromintPercentage()

src/wallet/walletdb.cpp

@@ -278,13 +278,22 @@ bool CWalletDB::EraseMSDisabledAddresses(std::vector<std::string> vDisabledAddre
     }
     return ret;
 }
-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::WriteDefaultKey(const CPubKey& vchPubKey)
@@ -700,10 +709,23 @@ bool ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, CW
             ssValue >> strDisabledAddress;
             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

@@ -125,7 +125,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 WriteDefaultKey(const CPubKey& vchPubKey);