From b1385852ef2ba45fd6926d75497646debf79e686 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Mon, 17 Jul 2017 06:52:09 -0400 Subject: [PATCH 1/2] Remove factor of 3 from definition of dust. This redefines dust to be the value of an output such that it would cost that value in fees to (create and) spend the output at the dust relay rate. The previous definition was that it would cost 1/3 of the value. The default dust relay rate is correspondingly increased to 3000 sat/kB so the actual default dust output value of 546 satoshis for a non-segwit output remains unchanged. This commit is a refactor only unless a dustrelayfee is passed on the commandline in which case that number now needs to be increased by a factor of 3 to get the same behavior. -dustrelayfee is a hidden command line option. Note: It's not exactly a refactor due to edge case changes in rounding as evidenced by the required change to the unit test. --- src/init.cpp | 2 +- src/policy/policy.cpp | 10 ++++++---- src/policy/policy.h | 4 ++-- src/test/transaction_tests.cpp | 10 +++++----- 4 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 1e8564201..bda0e905b 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -479,7 +479,7 @@ std::string HelpMessage(HelpMessageMode mode) if (showDebug) { strUsage += HelpMessageOpt("-acceptnonstdtxn", strprintf("Relay and mine \"non-standard\" transactions (%sdefault: %u)", "testnet/regtest only; ", defaultChainParams->RequireStandard())); strUsage += HelpMessageOpt("-incrementalrelayfee=", strprintf("Fee rate (in %s/kB) used to define cost of relay, used for mempool limiting and BIP 125 replacement. (default: %s)", CURRENCY_UNIT, FormatMoney(DEFAULT_INCREMENTAL_RELAY_FEE))); - strUsage += HelpMessageOpt("-dustrelayfee=", strprintf("Fee rate (in %s/kB) used to defined dust, the value of an output such that it will cost about 1/3 of its value in fees at this fee rate to spend it. (default: %s)", CURRENCY_UNIT, FormatMoney(DUST_RELAY_TX_FEE))); + strUsage += HelpMessageOpt("-dustrelayfee=", strprintf("Fee rate (in %s/kB) used to defined dust, the value of an output such that it will cost more than its value in fees at this fee rate to spend it. (default: %s)", CURRENCY_UNIT, FormatMoney(DUST_RELAY_TX_FEE))); } strUsage += HelpMessageOpt("-bytespersigop", strprintf(_("Equivalent bytes per sigop in transactions for relay and mining (default: %u)"), DEFAULT_BYTES_PER_SIGOP)); strUsage += HelpMessageOpt("-datacarrier", strprintf(_("Relay and mine data carrier transactions (default: %u)"), DEFAULT_ACCEPT_DATACARRIER)); diff --git a/src/policy/policy.cpp b/src/policy/policy.cpp index 9f2d623e7..605e3e069 100644 --- a/src/policy/policy.cpp +++ b/src/policy/policy.cpp @@ -19,16 +19,18 @@ CAmount GetDustThreshold(const CTxOut& txout, const CFeeRate& dustRelayFeeIn) { // "Dust" is defined in terms of dustRelayFee, // which has units satoshis-per-kilobyte. - // If you'd pay more than 1/3 in fees + // If you'd pay more in fees than the value of the output // to spend something, then we consider it dust. // A typical spendable non-segwit txout is 34 bytes big, and will // need a CTxIn of at least 148 bytes to spend: // so dust is a spendable txout less than - // 546*dustRelayFee/1000 (in satoshis). + // 182*dustRelayFee/1000 (in satoshis). + // 546 satoshis at the default rate of 3000 sat/kB. // A typical spendable segwit txout is 31 bytes big, and will // need a CTxIn of at least 67 bytes to spend: // so dust is a spendable txout less than - // 294*dustRelayFee/1000 (in satoshis). + // 98*dustRelayFee/1000 (in satoshis). + // 294 satoshis at the default rate of 3000 sat/kB. if (txout.scriptPubKey.IsUnspendable()) return 0; @@ -44,7 +46,7 @@ CAmount GetDustThreshold(const CTxOut& txout, const CFeeRate& dustRelayFeeIn) nSize += (32 + 4 + 1 + 107 + 4); // the 148 mentioned above } - return 3 * dustRelayFeeIn.GetFee(nSize); + return dustRelayFeeIn.GetFee(nSize); } bool IsDust(const CTxOut& txout, const CFeeRate& dustRelayFeeIn) diff --git a/src/policy/policy.h b/src/policy/policy.h index 2c2ea9d5b..c06820f84 100644 --- a/src/policy/policy.h +++ b/src/policy/policy.h @@ -40,12 +40,12 @@ static const unsigned int MAX_STANDARD_P2WSH_STACK_ITEMS = 100; static const unsigned int MAX_STANDARD_P2WSH_STACK_ITEM_SIZE = 80; /** The maximum size of a standard witnessScript */ static const unsigned int MAX_STANDARD_P2WSH_SCRIPT_SIZE = 3600; -/** Min feerate for defining dust. Historically this has been the same as the +/** Min feerate for defining dust. Historically this has been based on the * minRelayTxFee, however changing the dust limit changes which transactions are * standard and should be done with care and ideally rarely. It makes sense to * only increase the dust limit after prior releases were already not creating * outputs below the new threshold */ -static const unsigned int DUST_RELAY_TX_FEE = 1000; +static const unsigned int DUST_RELAY_TX_FEE = 3000; /** * Standard script verification flags that standard transactions will comply * with. However scripts violating these flags may still be present in valid diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 39f9f5860..6654634bf 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -692,7 +692,7 @@ BOOST_AUTO_TEST_CASE(test_IsStandard) BOOST_CHECK(IsStandardTx(t, reason)); // Check dust with default relay fee: - CAmount nDustThreshold = 182 * dustRelayFee.GetFeePerK()/1000 * 3; + CAmount nDustThreshold = 182 * dustRelayFee.GetFeePerK()/1000; BOOST_CHECK_EQUAL(nDustThreshold, 546); // dust: t.vout[0].nValue = nDustThreshold - 1; @@ -702,13 +702,13 @@ BOOST_AUTO_TEST_CASE(test_IsStandard) BOOST_CHECK(IsStandardTx(t, reason)); // Check dust with odd relay fee to verify rounding: - // nDustThreshold = 182 * 1234 / 1000 * 3 - dustRelayFee = CFeeRate(1234); + // nDustThreshold = 182 * 3702 / 1000 + dustRelayFee = CFeeRate(3702); // dust: - t.vout[0].nValue = 672 - 1; + t.vout[0].nValue = 673 - 1; BOOST_CHECK(!IsStandardTx(t, reason)); // not dust: - t.vout[0].nValue = 672; + t.vout[0].nValue = 673; BOOST_CHECK(IsStandardTx(t, reason)); dustRelayFee = CFeeRate(DUST_RELAY_TX_FEE); From f4d00e63f7951469c840edede3f675d07249f62a Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Thu, 6 Jul 2017 14:00:46 -0400 Subject: [PATCH 2/2] Add a discard_rate Any change output which would be dust at the discard_rate you are willing to discard completely and add to fee (as well as continuing to pay the fee that would have been needed for creating the change). --- src/wallet/wallet.cpp | 31 +++++++++++++++++++++++++++++-- src/wallet/wallet.h | 3 +++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 13da7bb2c..e230ee0eb 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -57,6 +57,8 @@ CFeeRate CWallet::minTxFee = CFeeRate(DEFAULT_TRANSACTION_MINFEE); */ CFeeRate CWallet::fallbackFee = CFeeRate(DEFAULT_FALLBACK_FEE); +CFeeRate CWallet::m_discard_rate = CFeeRate(DEFAULT_DISCARD_FEE); + const uint256 CMerkleTx::ABANDON_HASH(uint256S("0000000000000000000000000000000000000000000000000000000000000001")); /** @defgroup mapWallet @@ -2501,6 +2503,17 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nC return true; } +static CFeeRate GetDiscardRate(const CBlockPolicyEstimator& estimator) +{ + unsigned int highest_target = estimator.HighestTargetTracked(FeeEstimateHorizon::LONG_HALFLIFE); + CFeeRate discard_rate = estimator.estimateSmartFee(highest_target, nullptr /* FeeCalculation */, false /* conservative */); + // Don't let discard_rate be greater than longest possible fee estimate if we get a valid fee estimate + discard_rate = (discard_rate == CFeeRate(0)) ? CWallet::m_discard_rate : std::min(discard_rate, CWallet::m_discard_rate); + // Discard rate must be at least dustRelayFee + discard_rate = std::max(discard_rate, ::dustRelayFee); + return discard_rate; +} + bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletTx& wtxNew, CReserveKey& reservekey, CAmount& nFeeRet, int& nChangePosInOut, std::string& strFailReason, const CCoinControl& coin_control, bool sign) { @@ -2600,6 +2613,7 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletT CTxOut change_prototype_txout(0, scriptChange); size_t change_prototype_size = GetSerializeSize(change_prototype_txout, SER_DISK, 0); + CFeeRate discard_rate = GetDiscardRate(::feeEstimator); nFeeRet = 0; bool pick_new_inputs = true; CAmount nValueIn = 0; @@ -2667,7 +2681,7 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletT // Never create dust outputs; if we would, just // add the dust to the fee. - if (IsDust(newTxOut, ::dustRelayFee)) + if (IsDust(newTxOut, discard_rate)) { nChangePosInOut = -1; nFeeRet += nChange; @@ -2747,7 +2761,7 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletT // (because of reduced tx size) and so we should add a // change output. Only try this once. CAmount fee_needed_for_change = GetMinimumFee(change_prototype_size, coin_control, ::mempool, ::feeEstimator, nullptr); - CAmount minimum_value_for_change = GetDustThreshold(change_prototype_txout, ::dustRelayFee); + CAmount minimum_value_for_change = GetDustThreshold(change_prototype_txout, discard_rate); CAmount max_excess_fee = fee_needed_for_change + minimum_value_for_change; if (nFeeRet > nFeeNeeded + max_excess_fee && nChangePosInOut == -1 && nSubtractFeeFromAmount == 0 && pick_new_inputs) { pick_new_inputs = false; @@ -3788,6 +3802,9 @@ std::string CWallet::GetWalletHelpString(bool showDebug) strUsage += HelpMessageOpt("-keypool=", strprintf(_("Set key pool size to (default: %u)"), DEFAULT_KEYPOOL_SIZE)); strUsage += HelpMessageOpt("-fallbackfee=", strprintf(_("A fee rate (in %s/kB) that will be used when fee estimation has insufficient data (default: %s)"), CURRENCY_UNIT, FormatMoney(DEFAULT_FALLBACK_FEE))); + strUsage += HelpMessageOpt("-discardfee=", strprintf(_("The fee rate (in %s/kB) used to discard change (to fee) if it would be dust at this fee rate (default: %s) " + "Note: We will always discard up to the dust relay fee and a discard fee above that is limited by the longest target fee estimate"), + CURRENCY_UNIT, FormatMoney(DEFAULT_DISCARD_FEE))); strUsage += HelpMessageOpt("-mintxfee=", strprintf(_("Fees (in %s/kB) smaller than this are considered zero fee for transaction creation (default: %s)"), CURRENCY_UNIT, FormatMoney(DEFAULT_TRANSACTION_MINFEE))); strUsage += HelpMessageOpt("-paytxfee=", strprintf(_("Fee (in %s/kB) to add to transactions you send (default: %s)"), @@ -4113,6 +4130,16 @@ bool CWallet::ParameterInteraction() _("This is the transaction fee you may pay when fee estimates are not available.")); CWallet::fallbackFee = CFeeRate(nFeePerK); } + if (IsArgSet("-discardfee")) + { + CAmount nFeePerK = 0; + if (!ParseMoney(GetArg("-discardfee", ""), nFeePerK)) + return InitError(strprintf(_("Invalid amount for -discardfee=: '%s'"), GetArg("-discardfee", ""))); + if (nFeePerK > HIGH_TX_FEE_PER_KB) + InitWarning(AmountHighWarn("-discardfee") + " " + + _("This is the transaction fee you may discard if change is smaller than dust at this level")); + CWallet::m_discard_rate = CFeeRate(nFeePerK); + } if (IsArgSet("-paytxfee")) { CAmount nFeePerK = 0; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index b716e46a7..a7fef8a80 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -45,6 +45,8 @@ static const unsigned int DEFAULT_KEYPOOL_SIZE = 100; static const CAmount DEFAULT_TRANSACTION_FEE = 0; //! -fallbackfee default static const CAmount DEFAULT_FALLBACK_FEE = 20000; +//! -m_discard_rate default +static const CAmount DEFAULT_DISCARD_FEE = 10000; //! -mintxfee default static const CAmount DEFAULT_TRANSACTION_MINFEE = 1000; //! minimum recommended increment for BIP 125 replacement txs @@ -965,6 +967,7 @@ public: static CFeeRate minTxFee; static CFeeRate fallbackFee; + static CFeeRate m_discard_rate; /** * Estimate the minimum fee considering user set parameters * and the required fee