diff --git a/src/qt/coincontroldialog.cpp b/src/qt/coincontroldialog.cpp index db14a5cb8..99ff54e89 100644 --- a/src/qt/coincontroldialog.cpp +++ b/src/qt/coincontroldialog.cpp @@ -543,7 +543,7 @@ void CoinControlDialog::updateLabels(WalletModel *model, QDialog* dialog) nChange -= nPayFee; // Never create dust outputs; if we would, just add the dust to the fee. - if (nChange > 0 && nChange < MIN_CHANGE) + if (nChange > 0 && nChange < CWallet::GetMinChange()) { CTxOut txout(nChange, (CScript)std::vector(24, 0)); if (txout.IsDust(CWallet::discardThreshold)) diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 9d4788726..c6aeb943a 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -201,31 +201,31 @@ BOOST_AUTO_TEST_CASE(coin_selection_tests) // empty the wallet and start again, now with fractions of a coin, to test small change avoidance empty_wallet(); - add_coin(MIN_CHANGE * 1 / 10); - add_coin(MIN_CHANGE * 2 / 10); - add_coin(MIN_CHANGE * 3 / 10); - add_coin(MIN_CHANGE * 4 / 10); - add_coin(MIN_CHANGE * 5 / 10); + add_coin(CWallet::GetMinChange() * 1 / 10); + add_coin(CWallet::GetMinChange() * 2 / 10); + add_coin(CWallet::GetMinChange() * 3 / 10); + add_coin(CWallet::GetMinChange() * 4 / 10); + add_coin(CWallet::GetMinChange() * 5 / 10); - // try making 1 * MIN_CHANGE from the 1.5 * MIN_CHANGE - // we'll get change smaller than MIN_CHANGE whatever happens, so can expect MIN_CHANGE exactly - BOOST_CHECK( wallet.SelectCoinsMinConf(MIN_CHANGE, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, MIN_CHANGE); + // try making 1 * GetMinChange() from the 1.5 * GetMinChange() + // we'll get change smaller than GetMinChange() whatever happens, so can expect GetMinChange() exactly + BOOST_CHECK( wallet.SelectCoinsMinConf(CWallet::GetMinChange(), 1, 1, 0, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK_EQUAL(nValueRet, CWallet::GetMinChange()); // but if we add a bigger output, small change is avoided - add_coin(1111*MIN_CHANGE); + add_coin(1111*CWallet::GetMinChange()); // try making 1 from 0.1 + 0.2 + 0.3 + 0.4 + 0.5 + 1111 = 1112.5 - BOOST_CHECK( wallet.SelectCoinsMinConf(1 * MIN_CHANGE, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 1 * MIN_CHANGE); // we should get the exact amount + BOOST_CHECK( wallet.SelectCoinsMinConf(1 * CWallet::GetMinChange(), 1, 1, 0, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK_EQUAL(nValueRet, 1 * CWallet::GetMinChange()); // we should get the exact amount // if we add more small output: - add_coin(MIN_CHANGE * 6 / 10); - add_coin(MIN_CHANGE * 7 / 10); + add_coin(CWallet::GetMinChange() * 6 / 10); + add_coin(CWallet::GetMinChange() * 7 / 10); - // and try again to make 1.0 * MIN_CHANGE - BOOST_CHECK( wallet.SelectCoinsMinConf(1 * MIN_CHANGE, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 1 * MIN_CHANGE); // we should get the exact amount + // and try again to make 1.0 * GetMinChange() + BOOST_CHECK( wallet.SelectCoinsMinConf(1 * CWallet::GetMinChange(), 1, 1, 0, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK_EQUAL(nValueRet, 1 * CWallet::GetMinChange()); // we should get the exact amount // run the 'mtgox' test (see http://blockexplorer.com/tx/29a3efd3ef04f9153d47a990bd7b048a4b2d213daaa5fb8ed670fb85f13bdbcf) // they tried to consolidate 10 50k outputs into one 500k output, and ended up with 50k in change @@ -237,43 +237,43 @@ BOOST_AUTO_TEST_CASE(coin_selection_tests) BOOST_CHECK_EQUAL(nValueRet, 500000 * COIN); // we should get the exact amount BOOST_CHECK_EQUAL(setCoinsRet.size(), 10U); // in ten outputs - // if there's not enough in the smaller coins to make at least 1 * MIN_CHANGE change (0.5+0.6+0.7 < 1.0+1.0), + // if there's not enough in the smaller coins to make at least 1 * GetMinChange() change (0.5+0.6+0.7 < 1.0+1.0), // we need to try finding an exact subset anyway // sometimes it will fail, and so we use the next biggest output: empty_wallet(); - add_coin(MIN_CHANGE * 5 / 10); - add_coin(MIN_CHANGE * 6 / 10); - add_coin(MIN_CHANGE * 7 / 10); - add_coin(1111 * MIN_CHANGE); - BOOST_CHECK( wallet.SelectCoinsMinConf(1 * MIN_CHANGE, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 1111 * MIN_CHANGE); // we get the bigger output + add_coin(CWallet::GetMinChange() * 5 / 10); + add_coin(CWallet::GetMinChange() * 6 / 10); + add_coin(CWallet::GetMinChange() * 7 / 10); + add_coin(1111 * CWallet::GetMinChange()); + BOOST_CHECK( wallet.SelectCoinsMinConf(1 * CWallet::GetMinChange(), 1, 1, 0, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK_EQUAL(nValueRet, 1111 * CWallet::GetMinChange()); // we get the bigger output BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); // but sometimes it's possible, and we use an exact subset (0.4 + 0.6 = 1.0) empty_wallet(); - add_coin(MIN_CHANGE * 4 / 10); - add_coin(MIN_CHANGE * 6 / 10); - add_coin(MIN_CHANGE * 8 / 10); - add_coin(1111 * MIN_CHANGE); - BOOST_CHECK( wallet.SelectCoinsMinConf(MIN_CHANGE, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, MIN_CHANGE); // we should get the exact amount + add_coin(CWallet::GetMinChange() * 4 / 10); + add_coin(CWallet::GetMinChange() * 6 / 10); + add_coin(CWallet::GetMinChange() * 8 / 10); + add_coin(1111 * CWallet::GetMinChange()); + BOOST_CHECK( wallet.SelectCoinsMinConf(CWallet::GetMinChange(), 1, 1, 0, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK_EQUAL(nValueRet, CWallet::GetMinChange()); // we should get the exact amount BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); // in two outputs 0.4+0.6 // test avoiding small change empty_wallet(); - add_coin(MIN_CHANGE * 5 / 100); - add_coin(MIN_CHANGE * 1); - add_coin(MIN_CHANGE * 100); + add_coin(CWallet::GetMinChange() * 5 / 100); + add_coin(CWallet::GetMinChange() * 1); + add_coin(CWallet::GetMinChange() * 100); // trying to make 100.01 from these three outputs - BOOST_CHECK(wallet.SelectCoinsMinConf(MIN_CHANGE * 10001 / 100, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, MIN_CHANGE * 10105 / 100); // we should get all outputs + BOOST_CHECK(wallet.SelectCoinsMinConf(CWallet::GetMinChange() * 10001 / 100, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK_EQUAL(nValueRet, CWallet::GetMinChange() * 10105 / 100); // we should get all outputs BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); // but if we try to make 99.9, we should take the bigger of the two small outputs to avoid small change - BOOST_CHECK(wallet.SelectCoinsMinConf(MIN_CHANGE * 9990 / 100, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 101 * MIN_CHANGE); + BOOST_CHECK(wallet.SelectCoinsMinConf(CWallet::GetMinChange() * 9990 / 100, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); + BOOST_CHECK_EQUAL(nValueRet, 101 * CWallet::GetMinChange()); BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); // test with many inputs @@ -283,9 +283,9 @@ BOOST_AUTO_TEST_CASE(coin_selection_tests) for (uint16_t j = 0; j < 676; j++) add_coin(amt); BOOST_CHECK(wallet.SelectCoinsMinConf(20*CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - if (amt - 20*CENT < MIN_CHANGE) { + if (amt - 20*CENT < CWallet::GetMinChange()) { // needs more than one input: - uint16_t returnSize = std::ceil((20.0 * CENT + MIN_CHANGE)/amt); + uint16_t returnSize = std::ceil((20.0 * CENT + CWallet::GetMinChange())/amt); CAmount returnValue = amt * returnSize; BOOST_CHECK_EQUAL(nValueRet, returnValue); BOOST_CHECK_EQUAL(setCoinsRet.size(), returnSize); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 9ec616b3a..1cc7afffa 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2179,6 +2179,14 @@ static void ApproximateBestSubset(vector vCoins, set >& setCoinsRet, CAmount& nValueRet) const { @@ -2218,7 +2226,7 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const int nConfMin nValueRet += coin.first; return true; } - else if (n < nTargetValue + MIN_CHANGE) + else if (n < nTargetValue + GetMinChange()) { vValue.push_back(coin); nTotalLower += n; @@ -2255,13 +2263,13 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const int nConfMin CAmount nBest; ApproximateBestSubset(vValue, nTotalLower, nTargetValue, vfBest, nBest); - if (nBest != nTargetValue && nTotalLower >= nTargetValue + MIN_CHANGE) - ApproximateBestSubset(vValue, nTotalLower, nTargetValue + MIN_CHANGE, vfBest, nBest); + if (nBest != nTargetValue && nTotalLower >= nTargetValue + GetMinChange()) + ApproximateBestSubset(vValue, nTotalLower, nTargetValue + GetMinChange(), vfBest, nBest); // If we have a bigger coin and (either the stochastic approximation didn't find a good solution, // or the next bigger coin is closer), return the bigger coin if (coinLowestLarger.second.first && - ((nBest != nTargetValue && nBest < nTargetValue + MIN_CHANGE) || coinLowestLarger.first <= nBest)) + ((nBest != nTargetValue && nBest < nTargetValue + GetMinChange()) || coinLowestLarger.first <= nBest)) { setCoinsRet.insert(coinLowestLarger.second); nValueRet += coinLowestLarger.first; @@ -2735,7 +2743,16 @@ bool CWallet::CreateTransaction(const vector& vecSend, CWalletTx& wt CAmount additionalFeeNeeded = nFeeNeeded - nFeeRet; vector::iterator change_position = txNew.vout.begin()+nChangePosInOut; // Only reduce change if remaining amount is still a large enough output. - if (change_position->nValue >= MIN_FINAL_CHANGE + additionalFeeNeeded) { + /* Dogecoin: this has been changed from a static MIN_FINAL_CHANGE that + * followed DEFAULT_DISCARD_THRESHOLD to instead use the configurable + * discard threshold. + * + * Note: + * If GetMinChange() ever becomes configurable or otherwise changes to no + * longer be derived from DEFAULT_DISCARD_THRESHOLD, then this check + * must be adapted. + */ + if (change_position->nValue >= discardThreshold + additionalFeeNeeded) { change_position->nValue -= additionalFeeNeeded; nFeeRet += additionalFeeNeeded; break; // Done, able to increase fee from change diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index cef25b502..206ce39ad 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -74,11 +74,10 @@ static const CAmount DEFAULT_DISCARD_THRESHOLD = COIN; * This way, replacements for fee bumps are transient rather than persisted. */ static const CAmount WALLET_INCREMENTAL_RELAY_FEE = RECOMMENDED_MIN_TX_FEE / 10; - /* * Dogecoin: Creating change outputs at exactly the dustlimit is counter- * productive because it leaves no space to bump the fee up, so we make the - * MIN_CHANGE parameter higher than the DEFAULT_DISCARD_THRESHOLD parameter. + * minimum change higher than the discard threshold. * * When RBF is not a default policy, we need to scale for both that and CPFP, * to have a facility for those that did not manually enable RBF, yet need to @@ -93,20 +92,18 @@ static const CAmount WALLET_INCREMENTAL_RELAY_FEE = RECOMMENDED_MIN_TX_FEE / 10; * or transaction size, we assume that most transactions are < 1kb, leading * to the following when planning for a replacements with 2x original fee: * - * RBF: MIN_CHANGE = DEFAULT_DISCARD_THRESHOLD + min fee or - * CPFP: MIN_CHANGE = DEFAULT_DISCARD_THRESHOLD + 2 * min fee * 0.147 + min fee + * RBF: min change = discardThreshold + minTxFee(1000) or + * CPFP: min change = discardThreshold + 2 * minTxFee(147) + minTxFee(1000) * * Where the CPFP requirement is higher than the RBF one to lead to the same * result. * - * This can be rounded up to the nearest multiple of RECOMMENDED_MIN_TX_FEE as: + * This can be rounded up to the nearest multiple of minTxFee(1000) as: * - * MIN_CHANGE = DEFAULT_DISCARD_THRESHOLD + 2 * RECOMMENDED_MIN_TX_FEE + * min change = discardThreshold + 2 * minTxFee(1000) */ -//! target minimum change amount -static const CAmount MIN_CHANGE = DEFAULT_DISCARD_THRESHOLD + 2 * RECOMMENDED_MIN_TX_FEE; -//! final minimum change amount after paying for fees -static const CAmount MIN_FINAL_CHANGE = DEFAULT_DISCARD_THRESHOLD; +//! target minimum change fee multiplier +static const CAmount MIN_CHANGE_FEE_MULTIPLIER = 2; //! Default for -spendzeroconfchange static const bool DEFAULT_SPEND_ZEROCONF_CHANGE = true; @@ -798,6 +795,12 @@ public: static CFeeRate minTxFee; static CFeeRate fallbackFee; static CAmount discardThreshold; + + /** + * Minimum change as a function of discardThreshold + */ + static CAmount GetMinChange(); + /** * Estimate the minimum fee considering user set parameters * and the required fee