From 01f15a8fb8e97e961e15ca9590405f1f6a214f70 Mon Sep 17 00:00:00 2001 From: Dmitri Ranfft Date: Mon, 7 Aug 2023 15:21:55 +0200 Subject: [PATCH 1/6] Only allow integers in the GUI for autocombine threshold --- .../settings/forms/settingswalletoptionswidget.ui | 2 +- .../pivx/settings/settingswalletoptionswidget.cpp | 15 +++++++++------ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/qt/pivx/settings/forms/settingswalletoptionswidget.ui b/src/qt/pivx/settings/forms/settingswalletoptionswidget.ui index 1bbea14e9..1d6ef6662 100644 --- a/src/qt/pivx/settings/forms/settingswalletoptionswidget.ui +++ b/src/qt/pivx/settings/forms/settingswalletoptionswidget.ui @@ -167,7 +167,7 @@ - 8 + 0 0 diff --git a/src/qt/pivx/settings/settingswalletoptionswidget.cpp b/src/qt/pivx/settings/settingswalletoptionswidget.cpp index 284745db7..11ae7e8b4 100644 --- a/src/qt/pivx/settings/settingswalletoptionswidget.cpp +++ b/src/qt/pivx/settings/settingswalletoptionswidget.cpp @@ -100,20 +100,23 @@ void SettingsWalletOptionsWidget::setSpinBoxStakeSplitThreshold(double val) void SettingsWalletOptionsWidget::onSpinBoxStakeSplitThresholdChanged() { - if (ui->spinBoxStakeSplitThreshold->value() > 0) { - if (ui->spinBoxAutoCombineThreshold->value() >= ui->spinBoxStakeSplitThreshold->value() * 2) { - ui->spinBoxAutoCombineThreshold->setValue(ui->spinBoxStakeSplitThreshold->value() * 2 - 1); + if (ui->spinBoxStakeSplitThreshold->value() > 1) { + if (ui->spinBoxAutoCombineThreshold->value() >= ui->spinBoxStakeSplitThreshold->value() * 2 - 1) { + ui->spinBoxAutoCombineThreshold->setValue(floor(ui->spinBoxStakeSplitThreshold->value() * 2 - 1)); } } } void SettingsWalletOptionsWidget::onSpinBoxAutoCombineThresholdChanged() { - if (ui->spinBoxStakeSplitThreshold->value() > 0) { - if (ui->spinBoxAutoCombineThreshold->value() >= ui->spinBoxStakeSplitThreshold->value() * 2) { - ui->spinBoxAutoCombineThreshold->setValue(ui->spinBoxStakeSplitThreshold->value() * 2 - 1); + if (ui->spinBoxStakeSplitThreshold->value() > 1) { + if (ui->spinBoxAutoCombineThreshold->value() >= ui->spinBoxStakeSplitThreshold->value() * 2 - 1) { + ui->spinBoxAutoCombineThreshold->setValue(floor(ui->spinBoxStakeSplitThreshold->value() * 2 - 1)); } } + + // Enforce only integers. + ui->spinBoxAutoCombineThreshold->setValue(floor(ui->spinBoxAutoCombineThreshold->value())); } void SettingsWalletOptionsWidget::onAutoCombineCheckboxStateChanged() From bd5858044a77836078f37bbfc3793caa205c425e Mon Sep 17 00:00:00 2001 From: Dmitri Ranfft Date: Mon, 7 Aug 2023 15:39:00 +0200 Subject: [PATCH 2/6] Enforce a minimum of 1 in the GUI for autocombine threshold --- src/qt/pivx/settings/forms/settingswalletoptionswidget.ui | 2 +- src/qt/pivx/settings/settingswalletoptionswidget.cpp | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/qt/pivx/settings/forms/settingswalletoptionswidget.ui b/src/qt/pivx/settings/forms/settingswalletoptionswidget.ui index 1d6ef6662..4df7a8f89 100644 --- a/src/qt/pivx/settings/forms/settingswalletoptionswidget.ui +++ b/src/qt/pivx/settings/forms/settingswalletoptionswidget.ui @@ -170,7 +170,7 @@ 0 - 0 + 1 999999 diff --git a/src/qt/pivx/settings/settingswalletoptionswidget.cpp b/src/qt/pivx/settings/settingswalletoptionswidget.cpp index 11ae7e8b4..f802ad77e 100644 --- a/src/qt/pivx/settings/settingswalletoptionswidget.cpp +++ b/src/qt/pivx/settings/settingswalletoptionswidget.cpp @@ -104,6 +104,8 @@ void SettingsWalletOptionsWidget::onSpinBoxStakeSplitThresholdChanged() if (ui->spinBoxAutoCombineThreshold->value() >= ui->spinBoxStakeSplitThreshold->value() * 2 - 1) { ui->spinBoxAutoCombineThreshold->setValue(floor(ui->spinBoxStakeSplitThreshold->value() * 2 - 1)); } + } else { + ui->spinBoxAutoCombineThreshold->setValue(1); } } @@ -113,6 +115,8 @@ void SettingsWalletOptionsWidget::onSpinBoxAutoCombineThresholdChanged() if (ui->spinBoxAutoCombineThreshold->value() >= ui->spinBoxStakeSplitThreshold->value() * 2 - 1) { ui->spinBoxAutoCombineThreshold->setValue(floor(ui->spinBoxStakeSplitThreshold->value() * 2 - 1)); } + } else { + ui->spinBoxAutoCombineThreshold->setValue(1); } // Enforce only integers. From a0f9a0ed046361b4d61217d4985eb18509d012a6 Mon Sep 17 00:00:00 2001 From: Dmitri Ranfft Date: Mon, 7 Aug 2023 16:49:12 +0200 Subject: [PATCH 3/6] Make sure setautocombinerewards only allows integers --- src/wallet/rpcwallet.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index a1961144f..c0ba3f707 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -2969,7 +2969,7 @@ UniValue setautocombinethreshold(const JSONRPCRequest& request) "}\n" "\nExamples:\n" + - HelpExampleCli("setautocombinethreshold", "true 500.12") + HelpExampleRpc("setautocombinethreshold", "true, 500.12")); + HelpExampleCli("setautocombinethreshold", "true 500") + HelpExampleRpc("setautocombinethreshold", "true, 500")); RPCTypeCheck(request.params, {UniValue::VBOOL, UniValue::VNUM}); @@ -2983,6 +2983,8 @@ UniValue setautocombinethreshold(const JSONRPCRequest& request) nThreshold = AmountFromValue(request.params[1]); if (nThreshold < COIN) throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("The threshold value cannot be less than %s", FormatMoney(COIN))); + if (nThreshold % COIN != 0) + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("The threshold value must be an integer")); } CWalletDB walletdb(pwalletMain->strWalletFile); From 0edee622271e43517c3967440254178fc5f848b5 Mon Sep 17 00:00:00 2001 From: Dmitri Ranfft Date: Mon, 7 Aug 2023 21:33:32 +0200 Subject: [PATCH 4/6] Distinguish between old and new wallets in AutoCombineDust --- src/wallet/wallet.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index c400b67cc..88a5802b5 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3242,7 +3242,13 @@ void CWallet::AutoCombineDust(CConnman* connman) return; } - std::map > mapCoinsByAddress = AvailableCoinsByAddress(true, nAutoCombineThreshold * COIN); + // Make sure we don't break the settings saved in the old wallets where the autocombine threshold was saved incorrectly. + CAmount adjustedAutoCombineThreshold = nAutoCombineThreshold; + if (nAutoCombineThreshold < COIN) { + adjustedAutoCombineThreshold = nAutoCombineThreshold * COIN; + } + + std::map > mapCoinsByAddress = AvailableCoinsByAddress(true, adjustedAutoCombineThreshold); //coins are sectioned by address. This combination code only wants to combine inputs that belong to the same address for (std::map >::iterator it = mapCoinsByAddress.begin(); it != mapCoinsByAddress.end(); it++) { @@ -3270,7 +3276,7 @@ void CWallet::AutoCombineDust(CConnman* connman) nTotalRewardsValue += out.Value(); // Combine to the threshold and not way above - if (nTotalRewardsValue > nAutoCombineThreshold * COIN) + if (nTotalRewardsValue > adjustedAutoCombineThreshold) break; // Around 180 bytes per input. We use 190 to be certain @@ -3317,7 +3323,7 @@ void CWallet::AutoCombineDust(CConnman* connman) } //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 && nTotalRewardsValue < adjustedAutoCombineThreshold && nFeeRet > 0) continue; const CWallet::CommitResult& res = CommitTransaction(wtx, keyChange, connman); From 8d5dabdcd0b7b79a032fc611b0dae5b01ca69edd Mon Sep 17 00:00:00 2001 From: Dmitri Ranfft Date: Mon, 7 Aug 2023 21:34:21 +0200 Subject: [PATCH 5/6] Remove trailing whitespace in some files --- src/wallet/rpcwallet.cpp | 20 ++++++++++---------- src/wallet/wallet.cpp | 20 ++++++++++---------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index c0ba3f707..1d912f241 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -1019,7 +1019,7 @@ UniValue getbalance(const JSONRPCRequest& request) " To use this deprecated argument, start __decenomy__d with -deprecatedrpc=accounts. Only include transactions confirmed at least this many times.\n" "3. includeWatchonly (bool, optional, default=false) DEPRECATED. This argument will be removed in v5.0.\n" " To use this deprecated argument, start __decenomy__d with -deprecatedrpc=accounts. Also include balance in watchonly addresses (see 'importaddress')\n" - + "\nResult:\n" "amount (numeric) The total amount in __DSW__ received for this account.\n" @@ -1045,7 +1045,7 @@ UniValue getbalance(const JSONRPCRequest& request) isminefilter filter = ISMINE_SPENDABLE; if (request.params.size() > 2 && request.params[2].get_bool()) filter = filter | ISMINE_WATCH_ONLY; - + return ValueFromAmount(pwalletMain->GetLegacyBalance(filter, nMinDepth, account)); } @@ -1172,7 +1172,7 @@ UniValue sendfrom(const JSONRPCRequest& request) "6. \"comment-to\" (string, optional) An optional comment to store the name of the person or organization \n" " to which you're sending the transaction. This is not part of the transaction, \n" " it is just kept in your wallet.\n" - + "\nResult:\n" "\"transactionid\" (string) The transaction id.\n" @@ -1203,7 +1203,7 @@ UniValue sendfrom(const JSONRPCRequest& request) wtx.mapValue["to"] = request.params[5].get_str(); isminefilter filter = ISMINE_SPENDABLE; - + EnsureWalletIsUnlocked(); // Check funds @@ -1236,7 +1236,7 @@ UniValue sendmany(const JSONRPCRequest& request) " }\n" "3. minconf (numeric, optional, default=1) Only use the balance confirmed at least this many times.\n" "4. \"comment\" (string, optional) A comment\n" - + "\nResult:\n" "\"transactionid\" (string) The transaction id for the send. Only 1 transaction is created regardless of \n" " the number of addresses.\n" @@ -1263,7 +1263,7 @@ UniValue sendmany(const JSONRPCRequest& request) " }\n" "3. minconf (numeric, optional, default=1) Only use the balance confirmed at least this many times.\n" "4. \"comment\" (string, optional) A comment\n" - + "\nResult:\n" "\"transactionid\" (string) The transaction id for the send. Only 1 transaction is created regardless of \n" " the number of addresses.\n" @@ -1320,7 +1320,7 @@ UniValue sendmany(const JSONRPCRequest& request) } isminefilter filter = ISMINE_SPENDABLE; - + EnsureWalletIsUnlocked(); // Check funds @@ -1732,7 +1732,7 @@ UniValue listtransactions(const JSONRPCRequest& request) "2. count (numeric, optional, default=10) The number of transactions to return\n" "3. from (numeric, optional, default=0) The number of transactions to skip\n" "4. includeWatchonly (bool, optional, default=false) Include transactions to watchonly addresses (see 'importaddress')\n" - + "\nResult:\n" "[\n" " {\n" @@ -1783,7 +1783,7 @@ UniValue listtransactions(const JSONRPCRequest& request) "2. count (numeric, optional, default=10) The number of transactions to return\n" "3. from (numeric, optional, default=0) The number of transactions to skip\n" "4. includeWatchonly (bool, optional, default=false) Include transactions to watchonly addresses (see 'importaddress')\n" - + "\nResult:\n" "[\n" " {\n" @@ -1852,7 +1852,7 @@ UniValue listtransactions(const JSONRPCRequest& request) isminefilter filter = ISMINE_SPENDABLE; if ( request.params.size() > 3 && request.params[3].get_bool() ) filter = filter | ISMINE_WATCH_ONLY; - + if (nCount < 0) throw JSONRPCError(RPC_INVALID_PARAMETER, "Negative count"); if (nFrom < 0) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 88a5802b5..2120658b2 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1651,9 +1651,9 @@ CAmount CWallet::GetStakingBalance() const { return std::max(CAmount(0), loopTxsBalance( [](const uint256& id, const CWalletTx& pcoin, CAmount& nTotal) { - if (pcoin.IsTrusted() && - pcoin.GetDepthInMainChain() - >= + if (pcoin.IsTrusted() && + pcoin.GetDepthInMainChain() + >= (Params().GetConsensus().NetworkUpgradeActive(chainActive.Height(), Consensus::UPGRADE_STAKE_MIN_DEPTH_V2) ? Params().GetConsensus().nStakeMinDepthV2 : Params().GetConsensus().nStakeMinDepth) ) { @@ -1810,7 +1810,7 @@ bool CWallet::GetMasternodeVinAndKeys(CTxIn& txinRet, CPubKey& pubKeyRet, CKey& CTxOut txOut = wtx.vout[nOutputIndex]; // Masternode collateral value - if (!CMasternode::CheckMasternodeCollateral(txOut.nValue)) + if (!CMasternode::CheckMasternodeCollateral(txOut.nValue)) { strError = "Invalid collateral tx value"; return error("%s: tx %s, index %d not a masternode collateral", __func__, strTxHash, nOutputIndex); @@ -1863,7 +1863,7 @@ bool CWallet::AvailableCoins(std::vector* pCoins, // --> populates { if (pCoins) pCoins->clear(); const bool fCoinsSelected = (coinControl != nullptr) && coinControl->HasSelected(); - + { LOCK2(cs_main, cs_wallet); for (std::map::const_iterator it = mapWallet.begin(); it != mapWallet.end(); ++it) { @@ -1876,9 +1876,9 @@ bool CWallet::AvailableCoins(std::vector* pCoins, // --> populates continue; // Check min depth requirement for stake inputs - if (nCoinType == STAKEABLE_COINS && - nDepth - < + if (nCoinType == STAKEABLE_COINS && + nDepth + < (Params().GetConsensus().NetworkUpgradeActive(chainActive.Height(), Consensus::UPGRADE_STAKE_MIN_DEPTH_V2) ? Params().GetConsensus().nStakeMinDepthV2 : Params().GetConsensus().nStakeMinDepth)) continue; @@ -2503,7 +2503,7 @@ bool CWallet::CreateCoinStake( if (WITH_LOCK(cs_main, return chainActive.Height()) != pindexPrev->nHeight) return false; // Make sure the wallet is unlocked and shutdown hasn't been requested - if (IsLocked() || ShutdownRequested()) return false; + if (IsLocked() || ShutdownRequested()) return false; nCredit = 0; @@ -2623,7 +2623,7 @@ CWallet::CommitResult CWallet::CommitTransaction(CWalletTx& wtxNew, CReserveKey& AddToWallet(wtxNew); // Notify that old coins are spent - + std::set updated_hashes; for (const CTxIn& txin : wtxNew.vin) { // notify only once From 98a2a0e7fbc1ecb1814f5a11cff6d44f4fbebd80 Mon Sep 17 00:00:00 2001 From: Dmitri Ranfft Date: Wed, 9 Aug 2023 12:43:21 +0200 Subject: [PATCH 6/6] Remove redundant transaction size check --- src/wallet/wallet.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 2120658b2..277a172be 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3253,7 +3253,6 @@ void CWallet::AutoCombineDust(CConnman* connman) //coins are sectioned by address. This combination code only wants to combine inputs that belong to the same address for (std::map >::iterator it = mapCoinsByAddress.begin(); it != mapCoinsByAddress.end(); it++) { std::vector vCoins, vRewardCoins; - bool maxSize = false; vCoins = it->second; // We don't want the tx to be refused for being too large @@ -3282,7 +3281,6 @@ void CWallet::AutoCombineDust(CConnman* connman) // Around 180 bytes per input. We use 190 to be certain txSizeEstimate += 190; if (txSizeEstimate >= MAX_STANDARD_TX_SIZE - 200) { - maxSize = true; break; } } @@ -3323,7 +3321,7 @@ void CWallet::AutoCombineDust(CConnman* connman) } //we don't combine below the threshold unless the fees are 0 to avoid paying fees over fees over fees - if (!maxSize && nTotalRewardsValue < adjustedAutoCombineThreshold && nFeeRet > 0) + if (nTotalRewardsValue < adjustedAutoCombineThreshold && nFeeRet > 0) continue; const CWallet::CommitResult& res = CommitTransaction(wtx, keyChange, connman);