zecfoundation
/
zcashd
mirror of https://github.com/ZcashFoundation/zcashd.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Fix the dust threshold rate to three times 100 zats/1000 bytes. 

(We express it that way rather than 300 zats/1000 bytes, because the
threshold is always rounded to an integer and then multiplied by 3.)

Bitcoin Core added the concept of "dust" in bitcoin/bitcoin#2577.
At that point the dust threshold 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 three times 100 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.

An earlier commit in this PR changed relaying policy to be more strict
about enforcing minRelayTxFee. The commit just before this one also
allowed `-minrelaytxfee=0`, which we are going to use 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 this PR.

Since we can no longer modify the dust threshold, we remove a check
from transaction_tests.cpp that relied on doing so.

This change also indirectly fixes a false-positive assertion error that
would occur in `SpendableInputs::LimitToAmount` if we allowed the dust
threshold to be zero.

Signed-off-by: Daira Emma Hopwood <daira@jacaranda.org>
This commit is contained in:
Daira Emma Hopwood 2023-04-12 16:58:24 +01:00
parent 5e87cb480f
commit e2f99f5133
7 changed files with 79 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/policy/policy.cpp
View File

@ -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;
}

48
src/policy/policy.h
View File

@ -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);

17
src/primitives/transaction.cpp
View File

@ -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);

21
src/primitives/transaction.h
View File

@ -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)

16
src/test/transaction_tests.cpp
View File

@ -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));

11
src/wallet/wallet.cpp
View File

@ -5558,7 +5558,7 @@ bool CWallet::CreateTransaction(const vector<CRecipient>& vecSend, CWalletTx& wt
}
}
if (txout.IsDust(::minRelayTxFee))
if (txout.IsDust())
{
if (recipient.fSubtractFeeFromAmount && nFeeRet > 0)
{
@ -5630,16 +5630,16 @@ bool CWallet::CreateTransaction(const vector<CRecipient>& 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<CRecipient>& 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<OutputPool>& 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);

2
src/wallet/wallet_tx_builder.cpp
View File

@ -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(