CTxOut::nValue is default-initialized to -1. The dust-threshold for OP_RETURN outputs is 0. Thus, the policy failure would be dust instead of multi-op-return. The test only passes because the dust check is currently not run.
Avoid that confusion by setting the value to 0, to ensure the dust check passes.
The dust-threshold for OP_RETURN outputs is 0.
IMHO the concept of dust makes sense only for outputs that are part of the UTXO set. For provably unspendable outputs, “dust threshold” seems like a misnomer, as any threshold above zero would effectively be an enforcement to burn coins. I think it would be conceptually wrong questionable if GetDustThreshold was ever called for NULL_DATA or any other provably unspendable outputs (even though it includes the sanity check for IsUnspendable() and returns 0 in this case).
If there is ever a minimum nAmount introduced for OP_RETURN outputs, it should be called differently than ‘dust limit’.
That said, I still think the PR itself is an improvement w.r.t. readability and avoiding confusion. (Generally I think scriptPubKey and nValue should always be initialized together). ACK fad140e311028f904635126e3c77352afac1b75e
If there is ever a minimum nAmount introduced for OP_RETURN outputs, it should be called differently than ‘dust limit’.
Agree. Though, I think that GetDustThreshold should be defined and callable for all scriptPubKeys, not only spendable ones. This is also what the current function does. In any case the discussion is theoretical, as only test code is affected.
Labels
Tests