From dc661accd280089a8d4b1a6b4568f03ffe0ac0e1 Mon Sep 17 00:00:00 2001 From: CaveSpectre11 <36988814+CaveSpectre11@users.noreply.github.com> Date: Sat, 15 Jun 2019 18:10:11 -0400 Subject: [PATCH 1/2] [RPC] AutoCombineRewards fixes and improvements Repair autocombinerewards issues, add the ability to configure the frequency that it is run, the ability to combine up to the maximum transaction size allowed, and backwards compatability to the old wallet format. --- src/rpc/client.cpp | 1 + src/rpc/server.cpp | 1 + src/rpc/server.h | 1 + src/wallet/rpcwallet.cpp | 109 ++++++++++++++++++++++++++++++++++----- src/wallet/wallet.cpp | 36 +++++++++---- src/wallet/wallet.h | 1 + src/wallet/walletdb.cpp | 36 ++++++++++--- src/wallet/walletdb.h | 2 +- 8 files changed, 156 insertions(+), 31 deletions(-) diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 7ea69ecdf9e6..6869b825f086 100644 --- a/src/rpc/client.cpp +++ b/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}, diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index 136522c1de81..ee847b109c8f 100644 --- a/src/rpc/server.cpp +++ b/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}, diff --git a/src/rpc/server.h b/src/rpc/server.h index 6a74bed697fb..93b9d1968b43 100644 --- a/src/rpc/server.h +++ b/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); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 9d7580662918..b22ad1d2b47c 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3077,38 +3077,123 @@ UniValue getstakesplitthreshold(const UniValue& params, bool fHelp) return ValueFromAmount(pwalletMain->nStakeSplitThreshold); } +UniValue getautocombineinfo(const UniValue& params, bool fHelp) +{ + std::string strComment; + + 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"); + + 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))); + + if (pwalletMain->nAutoCombineThreshold == 0) + strComment = "Enabled for maximum combine, "; + else + strComment = "Enabled for "; + + if (pwalletMain->fCombineDust) { + if (pwalletMain->nAutoCombineBlockFrequency == 0) { + obj.push_back(Pair("comment", strComment + "one shot on next block")); + } else { + obj.push_back(Pair("comment", strComment + "repeat on frequency")); + } + } else { + if (pwalletMain->nAutoCombineBlockFrequency == 0) { + obj.push_back(Pair("comment", strComment + "one shot on startup")); + } else { + obj.push_back(Pair("comment", "Disabled")); + } + } + + 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" "\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; + + if (pwalletMain->nAutoCombineThreshold == 0) + strComment = "Enabled for maximum combine, "; + else + strComment = "Enabled for "; + + obj.push_back(Pair("threshold", int(pwalletMain->nAutoCombineThreshold))); + obj.push_back(Pair("frequency", int(pwalletMain->nAutoCombineBlockFrequency))); + if (pwalletMain->nAutoCombineBlockFrequency == 0) { + obj.push_back(Pair("comment", strComment + "one shot on next block")); + } else { + obj.push_back(Pair("comment", strComment + "repeat on frequency")); + } + } + + return obj; } UniValue printMultiSend() diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 3bc771cf7230..b2f7af669509 100644 --- a/src/wallet/wallet.cpp +++ b/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 > 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 > 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,16 +3446,13 @@ 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)) { @@ -3447,7 +3460,8 @@ void CWallet::AutoCombineDust() 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; } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 75aa9db894da..7c1b05ac3d31 100644 --- a/src/wallet/wallet.h +++ b/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); diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index a9832b2f1062..f851852d8a1a 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -249,13 +249,22 @@ bool CWalletDB::EraseMSDisabledAddresses(std::vector vDisabledAddre */ } -bool CWalletDB::WriteAutoCombineSettings(bool fEnable, CAmount nCombineThreshold) +bool CWalletDB::WriteAutoCombineSettings(bool fEnable, CAmount nCombineThreshold, int nBlockFrequency) { nWalletDBUpdated++; - std::pair pSettings; - pSettings.first = fEnable; - pSettings.second = nCombineThreshold; - return Write(std::string("autocombinesettings"), pSettings, true); + // Overwrite the old format with a special flag + std::pair 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 enabledMS1(fEnable, nCombineThreshold); + std::pair,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 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,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; diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index dcd5c70008c6..07e0bba7000c 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -142,7 +142,7 @@ class CWalletDB : public CDB bool WriteMSettings(bool fMultiSendStake, bool fMultiSendMasternode, int nLastMultiSendHeight); bool WriteMSDisabledAddresses(std::vector vDisabledAddresses); bool EraseMSDisabledAddresses(std::vector 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); From 3b119e9ab19e4ff06d07460be97d143935a6f19d Mon Sep 17 00:00:00 2001 From: Cave Spectre Date: Sat, 20 Jul 2019 22:06:22 -0400 Subject: [PATCH 2/2] [rpc] Add logic to warn if potential conflict between autocombine and stakesplit --- src/wallet/rpcwallet.cpp | 62 +++++++++++++++++++++++++++++++--------- 1 file changed, 49 insertions(+), 13 deletions(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index b22ad1d2b47c..e6d70d7ad8c3 100644 --- a/src/wallet/rpcwallet.cpp +++ b/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; } } @@ -3079,8 +3092,6 @@ UniValue getstakesplitthreshold(const UniValue& params, bool fHelp) UniValue getautocombineinfo(const UniValue& params, bool fHelp) { - std::string strComment; - if (fHelp || params.size() != 0) throw std::runtime_error( "getautocombineinfo\n" @@ -3089,13 +3100,15 @@ UniValue getautocombineinfo(const UniValue& params, bool fHelp) "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"); + "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 @@ -3103,17 +3116,27 @@ UniValue getautocombineinfo(const UniValue& params, bool fHelp) if (pwalletMain->fCombineDust) { if (pwalletMain->nAutoCombineBlockFrequency == 0) { - obj.push_back(Pair("comment", strComment + "one shot on next block")); + strComment += "one shot on next block"; } else { - obj.push_back(Pair("comment", strComment + "repeat on frequency")); + strComment += "repeat on frequency"; } } else { if (pwalletMain->nAutoCombineBlockFrequency == 0) { - obj.push_back(Pair("comment", strComment + "one shot on startup")); + strComment += "one shot on startup"; } else { - obj.push_back(Pair("comment", "Disabled")); + 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; } @@ -3150,6 +3173,7 @@ UniValue autocombinerewards(const UniValue& params, bool fHelp) "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 15") + HelpExampleRpc("autocombinerewards", "true 500 15")); @@ -3179,18 +3203,30 @@ UniValue autocombinerewards(const UniValue& params, bool fHelp) if (pwalletMain->fCombineDust) { std::string strComment; - if (pwalletMain->nAutoCombineThreshold == 0) + 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 + } else { strComment = "Enabled for "; + } - obj.push_back(Pair("threshold", int(pwalletMain->nAutoCombineThreshold))); - obj.push_back(Pair("frequency", int(pwalletMain->nAutoCombineBlockFrequency))); if (pwalletMain->nAutoCombineBlockFrequency == 0) { - obj.push_back(Pair("comment", strComment + "one shot on next block")); + 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 { - obj.push_back(Pair("comment", strComment + "repeat on frequency")); + strComment = "(none)"; } + obj.push_back(Pair("warning", strComment)); + } return obj;