search

btcsuite / btcd

An alternative full node bitcoin implementation written in Go (golang)
https://github.com/btcsuite/btcd/blob/master/README.md
ISC License
6.1k stars 2.31k forks source link

mempool.IsDust comment and argument name are out of date #2060

Closed benma closed 7 months ago

benma commented 7 months ago

https://github.com/btcsuite/btcd/blob/d15dd7108765d4ae810dae6c5446b81ee76f3fbf/mempool/policy.go#L263-L266

The 1/3 factor is out of date and was removed in https://github.com/bitcoin/bitcoin/commit/b1385852ef2ba45fd6926d75497646debf79e686.

Furhermore, Bitcoin Core is using dustrelayfee instead of minrelayfee, which by default is 3x larger (3000 instead of 1000). See

https://github.com/bitcoin/bitcoin/blob/8243762700bc6e7876ae5d4fc000500858b99f66/src/policy/policy.cpp#L26

https://github.com/bitcoin/bitcoin/blob/8243762700bc6e7876ae5d4fc000500858b99f66/src/policy/policy.h#L55

So another constant DustRelayTxFee = 3000 should be added below

https://github.com/btcsuite/btcd/blob/d15dd7108765d4ae810dae6c5446b81ee76f3fbf/mempool/mempool.go#L145

Roasbeef commented 7 months ago

Hi @benma, dust is policy. We haven't 100% kept up with policy, as it can change at a whim when the bitcoind developers to decide to (with caveats ofc). The min relay fee can be set with a command line option.

btcd is also behind in other areas like the latest versions of package relay, package based transaction selection for mining, and a few other mempool changes as well.

Ultimately, we'll accept PRs to convert policy more closely, though it's the case today that with things like Full RBF vs not, policy can deviate dramatically due to user configuration (eg: the mempool size limit affects the smallest fee accepted, which impacts the feefilter, which can cause transactions to not get relayed).

Crypt-iQ commented 7 months ago

Currently the dust produced by btcd and bitcoind match up because GetDustThreshold divides by 3 and MinRelayTxFee is by default 1000. Seems like all we'd have to do is just add the constant and not divide by 3 in GetDustThreshold but that would break callers of GetDustThreshold

benma commented 7 months ago

@Crypt-iQ thanks for clarifying, I missed that, not sure how :flushed: The function comment there even says it's multiplying by 3 to account for the dustrelayfee:

https://github.com/btcsuite/btcd/blob/d15dd7108765d4ae810dae6c5446b81ee76f3fbf/mempool/policy.go#L175C1-L177C50

I think I'll close this - sorry for the noise!