This pull request enables full rbf (mempool policy) by default. #28132 was closed recently with this comment.
Rationale:
Full RBF config option was added in July 2022: #25353
It is used regularly: https://mempool.space/rbf#fullrbf
Most mining pools are using it: #28132 (comment)
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For detailed information about the code coverage, see the test coverage report.
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | instagibbs, petertodd, glozow, ariard, murchandamus, achow101 |
| Concept ACK | reardencode, 0xB10C |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
Thanks @reardencode for the review and ACK as its obvious full-rbf should be default. There is some politics involved in it which I wanted to highlight:
I don’t know the reason for deleting this comments and others who reacted with emojis, however it will be better for bitcoin if we can get some ACKs to merge it. I did not mention the vulnerability in rationale because other things are sufficient to make it default.
This also affects coinjoin and I am working on joinstr so less politics would help bitcoin core and everyone involved in bitcoin.
Concept ACK
Given the majority of the hashrate using mempoolfullrbf, Bitcoin Core should adapt its default behavior to match its expectations to the blocks being published.
0@@ -0,0 +1,5 @@
1+Full Replace-By-Fee
Thanks @reardencode for the review and ACK as its obvious full-rbf should be default. There is some politics involved in it which I wanted to highlight:
Sorry for the confusion. To clarify, I had deleted my own comment. I wanted to think a bit more about it before commenting again.
Thanks @reardencode for the review and ACK as its obvious full-rbf should be default. There is some politics involved in it which I wanted to highlight:
Sorry for the confusion. To clarify, I had deleted my own comment. I wanted to think a bit more about it before commenting again.
Its okay, if you refuse to answer something and not forced to do it.
@1440000bytes hello from your friendly neighborhood moderator. This is just a reminder to keep all discussions technical. Pull request comments should be focused on the code in question. Discussions about personal opinions, moderator or maintainer actions (including responses to this comment) can be opened here
Code in question:
0-static constexpr bool DEFAULT_MEMPOOL_FULL_RBF{false};
1+static constexpr bool DEFAULT_MEMPOOL_FULL_RBF{true};
Issue related to this pull request is already open here: https://github.com/bitcoin-core/meta/issues/5
I am free to respond to reviewers and ask them a few questions which they can even refuse to answer.
@1440000bytes hello from your friendly neighborhood moderator. This is just a reminder to keep all discussions technical. > Pull request comments should be focused on the code in question. Discussions about personal opinions, moderator or maintainer actions (including responses to this comment) can be opened [here](https://github.com/bitcoin core/meta/issues) @pinheadmz you or another moderator still has not reply on the meta-comment I made on the moderation rules available here. I understand you can be in a situation of conflict of interest as the example I’m giving to you is related to your current employer to comment so and take a position.
As such from someone who is a code contributor and regular reviewer to this github repository, this is hard to think that moderation can be followed of good faith, when “discussions about personal opinions, moderator or maintainer actions” are not addressed after 2 months of having being mentioned.
Concept ACK.
For the same vector of DoS attack about contracting protocols and multi-party explained in my post on the mailing list in 2021 (“Mempool Funny Games against Multi-Party Funded Transactions).
I believe one of the best long-term solution to avoid new use-case conflict (in the present “0
conf tx acceptance versus multi-party built transaction like coinjoin”) is to minimize the scope
of the transaction-relay policy in the bitcoin core codebase, while this cannot be always followed
when it’s mitigating a DoS vector (e.g MIN_STANDARD_TX_NONWITNESS_SIZE). I made such more or
less experiment to have this approach of having transaction-relay policy rules signaled among
transaction issuers and miners with #29448 and similarly transaction-flags user-defined has been
discussed on the mailing list here.
Historically, there has been some attempt to have the policy rules validated in an external module with a set of patch proposed by jtimon (e.g see 6416, 6420, 6424 and other PRs around the same period). See e.g https://github.com/jtimon/bitcoin/commits/policy-replacement-alt for example of abstractions for “alternative replacement policies”.
About the ongoing issue on moderation, I did a cryptographic timestamp of this github HTML page in the bitcoin blockchain, if this needs to be discussed on another public communication channel.
Maintainers or moderators can for sure mark as off-topic comments on the github repository, though they do no have the proof-of-work to reorganize the mainchain (edited).
FWIW, I had some friends running nodes either do nothing or turn this on. In the last 144 blocks, a default node sees only 64.58% blocks reconstructed with zero round trips which, in my view, means compact blocks is significantly broken for its latency reducing purpose. A node with this enabled sees 94.44% reconstruct without a roundtrip, which is maybe a little low but not broken.
I imagine that if more of the network were running enabled that number might increase further, as it may still be held down due to transactions that fail to propagate well (consistent with that, it seems most nodes with no inbound have low rates regardless of the setting, which presumably wouldn’t be so if most nodes had this setting).
I shared my concerns with another bitcoiner who likes to monitor network health, they replicated, and got similar results (which I presume they’ll share eventually).
33 "-limitancestorcount=50",
34 "-limitancestorsize=101",
35 "-limitdescendantcount=200",
36 "-limitdescendantsize=101",
37 ],
38 # second node has default mempool parameters
This comment is outdated:
0 # second node has default mempool parameters, besides mempoolfullrbf being disabled
ACK 406cdd56e1065fe721f62a0c52fa024455748bbb
I reviewed the code and replicated the test changes to confirm that these are the minimal changes to pass all tests.
If we believe miners only care about the the largest fee/rate then simply providing them with all txns is good enough.
That would be a reason to support this PR, as full-RBF simply provides miners with all txns. Without full RBF enabled by relays, miners would typically only see the “first” transaction.
FWIW, I had some friends running nodes either do nothing or turn this on. In the last 144 blocks, a default node sees only 64.58% blocks reconstructed with zero round trips which, in my view, means compact blocks is significantly broken for its latency reducing purpose. A node with this enabled sees 94.44% reconstruct without a roundtrip, which is maybe a little low but not broken.
This behaviour was originally intended to put pressure on miners to match node policies; it’s not supposed to be an excuse to pressure nodes to adapt to miner policies.
(Personally, I’m ~0 on whether this actually gets merged or not; Knots has had full RBF by default for years, and “first seen” doesn’t really make sense as a policy)
@luke-jr I don’t know that I’d say ‘intended’, particularly since poor propagation either helps or hurts miners depend on their hashrate– it benefits sufficiently large centralized miners when other miners take more time to get their blocks– but regardless of intent it works the way it works.
I would say that to the extent someone is ambivalent with respect to the policy, as you just expressed, it’s preferable to improve block propagation latency since higher latency is a centralization pressure. The existence of the extrapool is intended to decouple policy from block propagation– allowing nodes to keep around some txn they wouldn’t prefer to mine/relay for the benefit of reconstructing blocks, but it only goes so far.
Ultimately it would be best if block relay latency were less fragile– methods have been implemented (the FEC reconstruction of fibre, for example) but never incorporated in widely deployed node software. And even those work particularly well if only a few transactions are missing relative to the node’s mempool and extrapool. So even if more robust block relay was in use, it’s preferable to do whatever lowers the discrepancy, all other things equal (and they’re not always equal, for sure).
I shared my concerns with another bitcoiner who likes to monitor network health, they replicated, and got similar results (which I presume they’ll share eventually).
Correct, I too found enabling mempoolfullrbf to be helpful for compact block reconstruction without requesting transactions.
By analyzing compact block reconstructions in the debug.logs (with debug=cmpctblock enabled) of my monitoring nodes, I noticed that many block reconstructions need an additional round-trip to fetch unknown transactions. While this situation has slightly improved over the last weeks (~50% -> 70+%) as mempool activity died down, this still clearly shows a divergence in policy.
I suspected that the divergence might originate from nearly all pools mining with mempoolfullrbf. I enabled -mempoolfullrbf=1 on the monitoring node/host erin on 2024-07-26. Since then, this node needed request transactions during significantly fewer blocks reconstructions than the other nodes.
Concept ACK
(note that while 75% or so might show up as light green here, doesn’t necessarily mean that 75% is “good”. Ideally, we want this to be close to 100% of blocks reconstructions needing no transaction requests)
0@@ -0,0 +1,5 @@
1+Full Replace-By-Fee
2+===================
3+
4+`mempoolfullrbf=1` is now set by default, which matches the policies of the
non-blocking nit
0`mempoolfullrbf=1` is now set by default, which seems to match the policies of the
29@@ -30,7 +30,7 @@ def run_test(self):
30 self.check_tx_relay()
31
32 self.checkpermission(
33- # default permissions (no specific permissions)
ACK fbd4ca6ea9a3d2f6fe0af3cb5dfe8e1a7fdb4032
Given the weight of evidence on block propagation and the implied miner uptake, I think it’s a conservative flip of the switch.
If the conclusion is that it’s now better for a node to have this “on”, rather than “off”, and this is merged for 28.x, then I think we should also:
If the conclusion is that it’s now better for a node to have this “on”, rather than “off”, and this is merged for
28.x, then I think we should also:* Backport the default change to the relevant release branches. * Ultimately just remove the option (doesn't have to be done here).
We should probably wait with the removal for a bit, but then we could also stop signaling replaceability by default to get rid of a fingerprint.
Reposting the analysis of the full-rbf change on other mempools mechanism, the earlier branch opened by Peter Todd.
On analyzing tangential mempool behaviors hypothetically affected by setting mempoolfullrbf=1 by default:
(a) Wtxid: nsequence signaling is encompassed both in txid and wtxid. After this change and assuming reboot a local mempool can re-accept a bip125 opt-out transaction previously rejected as identical non-witness data might not have change, yet the wtxid being different.
(b) Mempool.dat / persistence: the dump of mempool transaction. Dump of transactions are expected to be altered with mempoolfulrbf=1 as bip125 opt-out transactions are expected to be accepted with this change.
(c) Datacarrier: one op_return data carrying output. The data carriage is implemented in the scriptpubkey while the signaling rbf mechanism is implemented in one of the input. There is no effect expected on data carriage mempool acceptance, unless very exotic usage of sighash malleability, where the txid can be altered yet the hash of the op_return scriptpubkey stays identical.
(d) Dust: the dust satoshi threshold. The dust check is implemented on the output amounts while the signaling rbf mechanism is implemeted in one of the input. There is no effect expected on data carriage mempool acceptance, unless very exotic usage of sighash malleability, where the txid can be altered yet the hash of one of the output amount under the dust threshold stays identical.
(e) Mempool expiry: by default after 14 days transactions are sorted out. Transactions accepted under mempoolfullrbf=0 and still in the mempool until reaching expiration could be replaced by an opt-out transactions. I think this a transient risk of double-spend during the deployment of mempoolfullrbf=1 as a default.
(f) Mempool limits / RBF carve-out. When mempool limits are reached out (25 descendants), there is an exemption to get one more child to be connected on the parent. As bip125 inheritance is not implemented in core, the one more opt-out child could be re-accepted in the mempool. I think this is a transient risk of double-spend during the deployment of mempoolfullrbf=1 as a default.
(g) Package initial acceptance / RBF. In-mempool packages are submitted to descendants and ancestors limits. As bip125 inheritance is not implemented in core, a subset of child could be re-accepted in the mempool. I think this is a transient risk of double-spend during the deployment of mempoolfullrbf=1 as a default.
(h) Mempool Reorg. Signaling mechanism is implemented on the nSequence field. In case of chain reorgs, the pool of disconnected transactions can be re-added in mempool. There is no effect expected on disconnected transactions re-acceptance.
(i) Mempool sigops limits. Script sig ops limits are evaluated on the scriptSig / witness spend. This mempool limit check only happens once a transaction is good for acceptance and scripts have to be validated. An opt-out transaction can be accepted for the same sig ops budget, I think this a transient genuine risk of opportunistic full-node ressource consumption during the deployment of mempoolfullrbf=1 as a default.
(j). Mempool unbroadcast. Transaction delivery is ensured by re-broadcasting transactions until a GETDATA is received. After this change and assuming reboot a local mempool can re-accept a bip125 opt-out transaction previously rejected and keep re-announcing this transaction to non-upgraded peers, provoking a free transaction relay bandwidth. I think this a transient minor risk of bandwidth ressource denial-of-service during the deployment of mempoolfullrbf=1 as a default.
On backporting on earlier versions than 28.0, I don’t know if it is the best approach as you might have temporary double-spend risks in function of how the user is using upgraded bitcoin core software, e.g if the bitcoind process is reboot and there are transactions re-broadcast.
On removing the mempoolfullrbf option definitively, I don’t think this is as easy as you have new use-cases like lightning with channel negotiation options like option_zeroconf to get fast liquidity allocated without waiting for 1st confirmation. It is for sure less safe than waiting for 3 or 6 chain confirmations, however in practice it is contributing to preserve the “instant payment” property of lightning with a low-rate of routing failure.
I would say that to the extent someone is ambivalent with respect to the policy, as you just expressed, it’s preferable to improve block propagation latency since higher latency is a centralization pressure. The existence of the extrapool is intended to decouple policy from block propagation– allowing nodes to keep around some txn they wouldn’t prefer to mine/relay for the benefit of reconstructing blocks, but it only goes so far.
I say “intended” because policy centralization was a concern I had and raised about compact blocks before they were merged, and @sipa explicitly justified CB’s behaviour in this regard as a way for nodes to exert influence on miners in this respect.
Originally, when all the economical pinning issues affecting lightning were found I advocated on the mailing list for more homogeneity in transaction-relay policy among the network of nodes (cf. “Advances in Bitcoin Contracting: Uniform Policy and Package Relay”). Similarly when it was found that bitcoin core didn’t implement bip125 inheritance correctly. For the lightning world, non-propagating transactions due to the usage of timelock is jeopardizing channel funds which is a more severe security risk than non-propagating transactions for simple wallet payment.
Non-propagating transaction is also a denial-of-service risk for usecases like asymmetric peg side-chains where you have consensus failure among validators and the timelock fallback script path have to be used on the chain.
With the experience of mempoolfullrbf deployment and all the conversations that happens on the mailing list (cf. “On
mempool consistency”), I think it turns out there was no common transaction-relay policy that will satisfy equally all the use-cases of that day (lightning, old school payment channel, coin-join, 0-conf acceptance by merchants).
Personally, I think it will be better to have transaction-relay policy mechanism in its own module, moving away from this notion of a consistent transaction-relay policy on the network. Policy asymmetries among transaction-relay nodes are more likely to provide a vector of exploitation at block validation, especially if the bip152 extrapool differs too much.
In a post-subsidy world, where we have use-cases driven by different consensus-validated timelock duration or service-level-agreement on unix-epoch measured quality of service, I think it is better to let use-cases signal with their on-chain fees traffic what is the most convenient transaction-relay policy for them.
In the lack of better heuristics in matters of transaction-relay policy design and deployment, I think it’s better to have the transaction-relay policy default to be harmonized with what can be measured to be run by the majority of miners to keep block propagation latency low, since it’s a centralization vector (and assuming it’s not at odds with the economic majority of on-chain fee paying users or long-term coins holders).
I don’t have a non-invasive measurement methodology of miners mempools settings to observe if mempoolfullrbf=1 is widely adopted by a majority of miners, which is non-redundant with the methodology of petertodd (which sounds to be based on OTS transactions fee-bumping) or 0xB10C and gmaxwell ones on compact block reconstruction success rate.
ACK 590456e3f1043ba0680e0afec9fd7653db1098bb
(including all the slight modifications made since I wrote it)
0@@ -0,0 +1,5 @@
1+Full Replace-By-Fee
2+===================
3+
4+`mempoolfullrbf=1` is now set by default, which matches the policies of the
5+supermajority of hash power.
Or just
0`mempoolfullrbf=1` is now set by default.
Concept ACK fbd4ca6ea9a3d2f6fe0af3cb5dfe8e1a7fdb4032, default options shouldn’t cause significantly reduced block relay efficiency. Frequently rejecting transactions that are likely to confirm makes the node much less useful wrt fee estimation, being aware of payments, etc. Agree that it also wouldn’t make sense to have the option if it’s just a footgun.
I’ve also been collecting data on -mempoolfullrbf={0,1} nodes and also observed
vExtraTxnForCompact) seems consistent with #30493 (comment).Happy to reACK if you take the suggestions, or can do followup.
Enable full rbf (mempool policy) by default and update tests accordingly.
ACK 590456e3f1043ba0680e0afec9fd7653db1098bb
I did my own testing a few weeks ago where I made transactions that did not signal and replaced them to see how many were mined. The methodology was to create a transaction and replace it after 2 minutes. A total of 122 transactions were created, with 118 of the replacements being accepted. This took place on July 10th to 11th over the course of about 30 hours. With a 96.7% success rate, I think that’s pretty good evidence that miners have enabled mempoolfullrbf.
This gist contains the script that I used as well as a log of all of the transactions that were created.
Concept NACK: Opposition to Full-RBF by Default Without Broad Consensus I have several critical concerns regarding the default implementation of Full Replace-By-Fee (RBF):
Deviation from the First-Seen Rule: Bitcoin’s game theory initially hinged on the “first-seen” rule, which is fundamental in preventing malicious double spends by confirming the first transaction broadcast. Moving away from this to a model where transactions can be outbid by higher fees undermines both the security and predictability of transactions, which are essential for maintaining user trust and the overall integrity of the Bitcoin network.
Increased Centralization of Block Production: The last decade has seen a notable increase in the centralization of block production, concentrated among a few large pools. This trend jeopardizes Bitcoin’s democratic and decentralized principles. Implementing changes like RBF, which could be exploited by these centralized entities, further aggravates this problem.
RBF as a Temporary Scaling Measure: Introduced as a stopgap for handling network congestion and scaling issues, RBF falls short as a long-term solution. It overshadows more sustainable approaches such as implementing larger blocks or enhancements like OP_CHECKTEMPLATEVERIFY (OP_CTV). RBF’s temporary nature could potentially obstruct the broader vision of scaling Bitcoin into a viable peer-to-peer currency.
Lack of Broad Social Consensus: The decision to implement a significant protocol change such as RBF without widespread community consensus is deeply concerning. This practice is misaligned with Bitcoin’s governance principles, where substantial changes should be thoroughly debated and agreed upon by the community.
Implications for Bitcoin’s Role as P2P Money: The ongoing reliance on and expansion of RBF features could impede Bitcoin’s ability to scale effectively as practical, peer-to-peer money. It pivots the system towards a competitive fee market, moving away from fostering user empowerment and accessibility—both of which are crucial for Bitcoin’s adoption and functionality as a currency.
Given these points, I strongly recommend a reevaluation of RBF’s role and its implementation strategy. We need to consider more robust and decentralized scaling solutions that adhere to Bitcoin’s original intentions, supporting its development into a reliable and universally accessible digital currency.
