diff --git a/src/policy/policy.cpp b/src/policy/policy.cpp index 967801e78..e44f9ac23 100644 --- a/src/policy/policy.cpp +++ b/src/policy/policy.cpp @@ -119,7 +119,7 @@ bool IsStandardTx(const CTransaction& tx, std::string& reason, const CChainParam else if ((whichType == TX_MULTISIG) && (!fIsBareMultisigStd)) { reason = "bare-multisig"; return false; - } else if (txout.IsDust(::minRelayTxFee)) { + } else if (txout.IsDust()) { reason = "dust"; return false; } diff --git a/src/policy/policy.h b/src/policy/policy.h index cd84e35f2..a2feb15b1 100644 --- a/src/policy/policy.h +++ b/src/policy/policy.h @@ -41,6 +41,54 @@ static const unsigned int STANDARD_SCRIPT_VERIFY_FLAGS = MANDATORY_SCRIPT_VERIFY /** For convenience, standard but not mandatory verify flags. */ static const unsigned int STANDARD_NOT_MANDATORY_VERIFY_FLAGS = STANDARD_SCRIPT_VERIFY_FLAGS & ~MANDATORY_SCRIPT_VERIFY_FLAGS; +/** + * The rate used to calculate the dust threshold, in zatoshis per 1000 bytes. + * This will effectively be multiplied by 3 (it is necessary to express it this + * way to ensure that rounding will be calculated the same as before #6542). + * + * Bitcoin Core added the concept of "dust" in bitcoin/bitcoin#2577. At that + * point the dust threshold rate was tied to three times the minRelayTxFee rate, + * with the motivation that if you'd pay more than a third of the minimum relay + * fee to spend something, it should be considered dust. This was implemented + * as a standard rule rejecting dust outputs. + * + * This motivation will not apply after ZIP 317 block construction is implemented: + * at that point the ZIP 317 marginal fee will be 5000 zats per logical action, + * but the dust threshold rate will still be 300 zats per 1000 bytes. Those costs + * would only coincide if the marginal size per logical action were + * 5000/300 * 1000 ~= 16667 bytes, and in practice the marginal size for any + * kind of input is much smaller than that. + * + * However, to avoid interoperability problems (older wallets creating transactions + * that newer nodes will reject because they view the outputs as dust), we will have + * to coordinate any increase in the dust threshold carefully. + * + * More history: in Zcash the minRelayTxFee rate was 5000 zats/1000 bytes at launch, + * changed to 1000 zats/1000 bytes in zcashd v1.0.3 and to 100 zats/1000 bytes in + * zcashd v1.0.7-1 (#2141). The relaying problem for shielded transactions (#1969) + * that prompted the latter change was fixed more thoroughly by the addition of + * `CFeeRate::GetFeeForRelay` in #4916, ensuring that a transaction paying + * `DEFAULT_FEE` can always be relayed. At the same time the default fee was set + * to 1000 zats, per ZIP 313. + * + * #6542 changed relaying policy to be more strict about enforcing minRelayTxFee. + * It also allowed `-minrelaytxfee=0`, which we are using to avoid some test + * breakage. But if the dust threshold rate were still set to three times the + * minRelayTxFee rate, then setting `-minrelaytxfee=0` would have the side effect + * of setting the dust threshold to zero, which is not intended. + * + * Bitcoin Core took a different approach to disentangling the dust threshold from + * the relay threshold, adding a `-dustrelayfee` option (bitcoin/bitcoin#9380). + * We don't want to do that because it is likely that we will change the dust policy + * again, and adding a user-visible config option might conflict with that. Also, + * it isn't a good idea for the dust threshold rate to be configurable per node; + * it's a standard rule parameter and should only be changed with network-wide + * coordination (if it is increased then wallets have to change before nodes, and + * vice versa if it is decreased). So for now we set it to a constant that matches + * the behaviour before #6542. + */ +static const unsigned int ONE_THIRD_DUST_THRESHOLD_RATE = 100; + // Sanity check the magic numbers when we change them static_assert(DEFAULT_BLOCK_MAX_SIZE <= MAX_BLOCK_SIZE); diff --git a/src/primitives/transaction.cpp b/src/primitives/transaction.cpp index c748f128a..2b3b0a835 100644 --- a/src/primitives/transaction.cpp +++ b/src/primitives/transaction.cpp @@ -5,6 +5,7 @@ // file COPYING or https://www.opensource.org/licenses/mit-license.php . #include "primitives/transaction.h" +#include "policy/policy.h" #include "hash.h" #include "tinyformat.h" @@ -123,6 +124,22 @@ CTxOut::CTxOut(const CAmount& nValueIn, CScript scriptPubKeyIn) scriptPubKey = scriptPubKeyIn; } +CAmount CTxOut::GetDustThreshold() const +{ + // See the comment on ONE_THIRD_DUST_THRESHOLD_RATE in policy.h. + static const CFeeRate oneThirdDustThresholdRate {ONE_THIRD_DUST_THRESHOLD_RATE}; + + if (scriptPubKey.IsUnspendable()) + return 0; + + // A typical spendable txout is 34 bytes, and will need a txin of at + // least 148 bytes to spend. With ONE_THIRD_DUST_THRESHOLD_RATE == 100, + // the dust threshold for such a txout would be + // 3*floor(100*(34 + 148)/1000) zats = 54 zats. + size_t nSize = GetSerializeSize(*this, SER_DISK, 0) + 148u; + return 3*oneThirdDustThresholdRate.GetFee(nSize); +} + uint256 CTxOut::GetHash() const { return SerializeHash(*this); diff --git a/src/primitives/transaction.h b/src/primitives/transaction.h index f569f8039..0b9b01ffb 100644 --- a/src/primitives/transaction.h +++ b/src/primitives/transaction.h @@ -625,26 +625,11 @@ public: uint256 GetHash() const; - CAmount GetDustThreshold(const CFeeRate &minRelayTxFee) const - { - // "Dust" is defined in terms of CTransaction::minRelayTxFee, - // which has units satoshis-per-kilobyte. - // If you'd pay more than 1/3 in fees - // to spend something, then we consider it dust. - // A typical spendable 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 - // 54*minRelayTxFee/1000 (in satoshis) - if (scriptPubKey.IsUnspendable()) - return 0; + CAmount GetDustThreshold() const; - size_t nSize = GetSerializeSize(*this, SER_DISK, 0) + 148u; - return 3*minRelayTxFee.GetFee(nSize); - } - - bool IsDust(const CFeeRate &minRelayTxFee) const + bool IsDust() const { - return (nValue < GetDustThreshold(minRelayTxFee)); + return nValue < GetDustThreshold(); } friend bool operator==(const CTxOut& a, const CTxOut& b) diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index e1c468984..ea9ac0caf 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -690,8 +690,9 @@ BOOST_AUTO_TEST_CASE(test_IsStandard) string reason; BOOST_CHECK(IsStandardTx(t, reason, chainparams)); - // Check dust with default relay fee: - CAmount nDustThreshold = 182 * minRelayTxFee.GetFeePerK()/1000 * 3; + // Check dust threshold: + CFeeRate oneThirdDustThresholdRate{ONE_THIRD_DUST_THRESHOLD_RATE}; + CAmount nDustThreshold = (34 + 148) * oneThirdDustThresholdRate.GetFeePerK()/1000 * 3; BOOST_CHECK_EQUAL(nDustThreshold, 54); // dust: t.vout[0].nValue = nDustThreshold - 1; @@ -700,17 +701,6 @@ BOOST_AUTO_TEST_CASE(test_IsStandard) t.vout[0].nValue = nDustThreshold; BOOST_CHECK(IsStandardTx(t, reason, chainparams)); - // Check dust with odd relay fee to verify rounding: - // nDustThreshold = 182 * 1234 / 1000 * 3 - minRelayTxFee = CFeeRate(1234); - // dust: - t.vout[0].nValue = 672 - 1; - BOOST_CHECK(!IsStandardTx(t, reason, chainparams)); - // not dust: - t.vout[0].nValue = 672; - BOOST_CHECK(IsStandardTx(t, reason, chainparams)); - minRelayTxFee = CFeeRate(DEFAULT_MIN_RELAY_TX_FEE); - t.vout[0].scriptPubKey = CScript() << OP_1; BOOST_CHECK(!IsStandardTx(t, reason, chainparams)); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index a149394c1..629c54f7c 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -5558,7 +5558,7 @@ bool CWallet::CreateTransaction(const vector& vecSend, CWalletTx& wt } } - if (txout.IsDust(::minRelayTxFee)) + if (txout.IsDust()) { if (recipient.fSubtractFeeFromAmount && nFeeRet > 0) { @@ -5630,16 +5630,16 @@ bool CWallet::CreateTransaction(const vector& vecSend, CWalletTx& wt // We do not move dust-change to fees, because the sender would end up paying more than requested. // This would be against the purpose of the all-inclusive feature. // So instead we raise the change and deduct from the recipient. - if (nSubtractFeeFromAmount > 0 && newTxOut.IsDust(::minRelayTxFee)) + if (nSubtractFeeFromAmount > 0 && newTxOut.IsDust()) { - CAmount nDust = newTxOut.GetDustThreshold(::minRelayTxFee) - newTxOut.nValue; + CAmount nDust = newTxOut.GetDustThreshold() - newTxOut.nValue; newTxOut.nValue += nDust; // raise change until no more dust for (unsigned int i = 0; i < vecSend.size(); i++) // subtract from first recipient { if (vecSend[i].fSubtractFeeFromAmount) { txNew.vout[i].nValue -= nDust; - if (txNew.vout[i].IsDust(::minRelayTxFee)) + if (txNew.vout[i].IsDust()) { strFailReason = _("The transaction amount is too small to send after the fee has been deducted"); return false; @@ -5651,7 +5651,7 @@ bool CWallet::CreateTransaction(const vector& vecSend, CWalletTx& wt // Never create dust outputs; if we would, just // add the dust to the fee. - if (newTxOut.IsDust(::minRelayTxFee)) + if (newTxOut.IsDust()) { nFeeRet += nChange; reservekey.ReturnKey(); @@ -7753,6 +7753,7 @@ bool SpendableInputs::LimitToAmount( const CAmount dustThreshold, const std::set& recipientPools) { + // dustThreshold cannot be zero because it is no longer configured via `-minrelaytxfee`. assert(amountRequired >= 0 && dustThreshold > 0); // Calling this method twice is a programming error. assert(!limited); diff --git a/src/wallet/wallet_tx_builder.cpp b/src/wallet/wallet_tx_builder.cpp index 1576d1ba2..6bead7085 100644 --- a/src/wallet/wallet_tx_builder.cpp +++ b/src/wallet/wallet_tx_builder.cpp @@ -191,7 +191,7 @@ CAmount WalletTxBuilder::DefaultDustThreshold() const { CKey secret{CKey::TestOnlyRandomKey(true)}; CScript scriptPubKey = GetScriptForDestination(secret.GetPubKey().GetID()); CTxOut txout(CAmount(1), scriptPubKey); - return txout.GetDustThreshold(minRelayFee); + return txout.GetDustThreshold(); } SpendableInputs WalletTxBuilder::FindAllSpendableInputs(