With a total ordering on mempool transactions, we are now able to calculate a
transaction's mining score at all times. Use this to improve the RBF logic:
- we no longer enforce a "no new unconfirmed parents" rule
- we now require that the mempool's feerate diagram must improve in order
to accept a replacement
- the topology restrictions for conflicts in the package rbf setting have been
eliminated
Revert the temporary change to mempool_ephemeral_dust.py that were previously
made due to RBF validation checks being reordered.
Co-authored-by: Gregory Sanders <gsanders87@gmail.com>, glozow <gloriajzhao@gmail.com>
ba84a25dee [doc] update mempool-replacements.md for incremental relay feerate change (glozow)
18720bc5d5 [doc] release note for min feerate changes (glozow)
6da5de58ca [policy] lower default minrelaytxfee and incrementalrelayfee to 100sat/kvB (glozow)
2e515d2897 [prep/test] make wallet_fundrawtransaction's minrelaytxfee assumption explicit (glozow)
457cfb61b5 [prep/util] help MockMempoolMinFee handle more precise feerates (glozow)
3eab8b7240 [prep/test] replace magic number 1000 with respective feerate vars (glozow)
5f2df0ef78 [miner] lower default -blockmintxfee to 1sat/kvB (glozow)
d6213d6aa1 [doc] assert that default min relay feerate and incremental are the same (glozow)
1fbee5d7b6 [test] explicitly check default -minrelaytxfee and -incrementalrelayfee (glozow)
72dc18467d [test] RBF rule 4 for various incrementalrelayfee settings (glozow)
85f498893f [test] check bypass of minrelay for various minrelaytxfee settings (glozow)
e5f896bb1f [test] check miner doesn't select 0fee transactions (glozow)
Pull request description:
ML post for discussion about the general concept, how this impacts the wider ecosystem, philosophy about minimum feerates, etc: https://delvingbitcoin.org/t/changing-the-minimum-relay-feerate/1886
This PR is inspired by #13922 and #32959 to lower the minimum relay feerate in response to bitcoin's exchange rate changes in the last ~10 years. It lowers the default `-minrelaytxfee` and `-incrementalrelayfee`, and knocks `-blockmintxfee` down to the minimum nonzero setting. Also adds some tests for the settings and pulls in #32750.
The minimum relay feerate is a DoS protection rule, representing a price on the network bandwidth used to relay transactions that have no PoW. While relay nodes don't all collect fees, the assumption is that if nodes on the network use their resources to relay this transaction, it will reach a miner and the attacker's money will be spent once it is mined. The incremental relay feerate is similar: it's used to price the relay of replacement transactions (the additional fees need to cover the new transactions at this feerate) and evicted transactions (following a trim, the new mempool minimum feerate is the package feerate of what was removed + incremental).
Also note that many nodes on the network have elected to relay/mine lower feerate transactions. Miners (some say up to 85%) are choosing to mine these low feerate transactions instead of leaving block space unfilled, but these blocks have extremely poor compact block reconstruction rates with nodes that rejected or didn't hear about those transactions earlier.
- https://github.com/bitcoin/bitcoin/pull/33106#issuecomment-3155627414
- https://x.com/caesrcd/status/1947022514267230302
- https://mempool.space/block/00000000000000000001305770e0aa279dcd8ba8be18c3d5cf736a26f77e06fd
- https://mempool.space/block/00000000000000000001b491649ec030aa8e003e1f4f9d3b24bb99ba16f91e97
- https://x.com/mononautical/status/1949452586391855121
While it wouldn't make sense to loosen DoS restrictions recklessly in response to these events, I think the current price is higher than necessary, and this motivates us changing the default soon. Since the minimum relay feerate defines an amount as too small based on what it costs the attacker, it makes sense to consider BTC's conversion rate to what resources you can buy in the "real world."
Going off of [this comment](https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3095260286) and [this comment](https://github.com/bitcoin/bitcoin/pull/33106#issuecomment-3142444090)
- Let's say an attacker wants to use/exhaust the network's bandwidth, and has the choice between renting resources from a commercial provider and getting the network to "spam" itself it by sending unconfirmed transactions. We'd like the latter to be more expensive than the former.
- The bandwidth for relaying a transaction across the network is roughly its serialized size (plus relay overhead) x number of nodes. A 1000vB transaction is 1000-4000B serialized. With 100k nodes, that's 0.1-0.4GB
- If the going rate for ec2 bandwidth is 10c/GB, that's like 1-4c per kvB of transaction data
- Then a 1000vB transaction should pay at least 4c
- $0.04 USD is 40 satoshis at 100k USD/BTC
- Baking in some margin for changes in USD/BTC conversion rate, number of nodes (and thus bandwidth), and commercial service costs, I think 50-100 satoshis is on the conservative end but in the right ballpark
- At least 97% of the recent sub-1sat/vB transactions would be accepted with a new threshold of 0.1sat/vB: https://github.com/bitcoin/bitcoin/pull/33106#issuecomment-3156213089
List of feerates that are changed and why:
- min relay feerate: significant conversion rate changes, see above
- incremental relay feerate: should follow min relay feerate, see above
- block minimum feerate: shouldn’t be above min relay feerate, otherwise the node accepts transactions it will never mine. I've knocked it down to the bare minimum of 1sat/kvB. Now that we no longer have coin age priority (removed in v0.15), I think we can leave it to the `CheckFeeRate` policy rule to enforce a minimum entry price, and the block assembly code should just fill up the block with whatever it finds in mempool.
List of feerates that are not changed and why:
- dust feerate: this feerate cannot be changed as flexibly as the minrelay feerate. A much longer record of low feerate transactions being mined is needed to motivate a decrease there.
- maxfeerate (RPC, wallet): I think the conversion rate is relevant as well, but out of scope for this PR
- minimum feerate returned by fee estimator: should be done later. In the past, we've excluded new policy defaults from fee estimation until we feel confident they represent miner policy (e.g. #9519). Also, the fee estimator itself doesn't have support for sub-1sat/vB yet.
- all wallet feerates (mintxfee, fallbackfee, discardfee, consolidatefeerate, WALLET_INCREMENTAL_RELAY_FEE, etc.): should be done later. Our standard procedure is to do wallet changes at least 1 release after policy changes.
ACKs for top commit:
achow101:
ACK ba84a25dee
gmaxwell:
ACK ba84a25dee
jsarenik:
Tested ACK ba84a25dee
darosior:
ACK ba84a25dee
ajtowns:
ACK ba84a25dee
davidgumberg:
crACK ba84a25dee
w0xlt:
ACK ba84a25dee
caesrcd:
reACK ba84a25dee
ismaelsadeeq:
re-ACK ba84a25dee
Tree-SHA512: b4c35e8b506b1184db466551a7e2e48bb1e535972a8dbcaa145ce3a8bfdcc70a8807dc129460f129a9d31024174d34077154a387c32f1a3e6831f6fa5e9c399e
Let's say an attacker wants to use/exhaust the network's bandwidth, and
has the choice between renting resources from a commercial provider and
getting the network to "spam" itself it by sending unconfirmed
transactions. We'd like the latter to be more expensive than the former.
The bandwidth for relaying a transaction across the network is roughly
its serialized size (plus relay overhead) x number of nodes. A 1000vB
transaction is 1000-4000B serialized. With 100k nodes, that's 0.1-0.4GB
If the going rate for commercial services is 10c/GB, that's like 1-4c per kvB
of transaction data, so a 1000vB transaction should pay at least $0.04.
At a price of 120k USD/BTC, 100sat is about $0.12. This price allows us
to tolerate a large decrease in the conversion rate or increase in the
number of nodes.
The Consensus Cleanup soft fork proposal includes enforcing that coinbase transactions set their
locktime field to the block height, minus 1 (as well as their nSequence such as to not disable the
timelock). If such a fork were to be activated by Bitcoin users, miners need to be ready to produce
compliant blocks at the risk of losing substantial amounts mining would-be invalid blocks. As miners
are unfamously slow to upgrade, it's good to make this change as early as possible.
Although Bitcoin Core's GBT implementation does not provide the "coinbasetxn" field, and mining
pool software crafts the coinbase on its own, updating the Bitcoin Core mining code is a first step
toward convincing pools to update their (often closed source) code. A possible followup is also to
introduce new fields to GBT. In addition, this first step also makes it possible to test future
Consensus Cleanup changes.
The changes to the seemingly-unrelated RBF tests is because these tests assert an error message
which may vary depending on the txid of the transactions used in the test. This commit changes the
coinbase transaction structure and therefore impact the txid of transactions in all tests.
The change to the "Bad snapshot" error message in the assumeutxo functional test is because this
specific test case reads into the txid of the next transaction in the snapshot and asserts the error
message based it gets on deserializing this txid as a coin for the previous transaction. As this
commit changes this txid it impacts the deserialization error raised.
Rather than individually calling addUnchecked for each transaction added in a
changeset (after removing all the to-be-removed transactions), instead we can
take advantage of boost::multi_index's splicing features to extract and insert
entries directly from the staging multi_index into mapTx.
This has the immediate advantage of saving allocation overhead for mempool
entries which have already been allocated once. This also means that the memory
locations of mempool entries will not change when transactions go from staging
to the main mempool.
Additionally, eliminate addUnchecked and require all new transactions to enter
the mempool via a CTxMemPoolChangeSet.
It's preferable to use type-safe transaction identifiers to avoid
confusing txid and wtxid. The next commit will add a reference to this
set; we use this opportunity to change it to Txid ahead of time instead
of adding new uses of uint256.
Since the kernel library no longer depends on the system file, move it
to the common library instead in accordance to the diagram in
doc/design/libraries.md.
Test each component of the RBF policy in isolation. Unlike the RBF
functional tests, these do not rely on things like RPC results, mempool
submission, etc.
