Enable full RBF on all networks by default, but defer full RBF enablement on mainnet until 1st May 2023. This enables easier testing of the behaviour on test networks and provides time for services relying on RBF being opt-in only to adapt to that policy going away.
Full RBF support can still be entirely disabled for an individual node by setting -mempoolfullrbf=0.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Reviewers, this pull request conflicts with the following ones:
mempoolfullrbf=1 by default by ariard)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.
Concept ACK
While there is a default set in Core I believe full RBF should be the default. As the PR description states Core users are free to change from whatever the default is and alternative implementations are free to set a different default. Assuming the default is changed in Core I also think it is right to signpost this in advance to give the ecosystem time to prepare for it as argued on the mailing list.
693@@ -693,6 +694,7 @@ static RPCHelpMan getmempoolinfo()
694 {RPCResult::Type::NUM, "incrementalrelayfee", "minimum fee rate increment for mempool limiting or replacement in " + CURRENCY_UNIT + "/kvB"},
695 {RPCResult::Type::NUM, "unbroadcastcount", "Current number of transactions that haven't passed initial broadcast yet"},
696 {RPCResult::Type::BOOL, "fullrbf", "True if the mempool accepts RBF without replaceability signaling inspection"},
697+ {RPCResult::Type::NUM, "defer_fullrbf", "If non-zero, fullrbf will be considered false until this timestamp is reached"},
0 {RPCResult::Type::NUM_TIME, "defer_fullrbf", "If non-zero, \"fullrbf\" will be considered false until this timestamp is reached, expressed in " + UNIX_EPOCH_TIME},
-mempoolfullrbf on mainnet until the deadline with no option to override. My first impression reading the description was that this simply changes the mainnet default to true after the deadline, which I believe would be a better approach. Temporarily disabling the option already received a few NACKs in #26287.
-0 on this.
This doesn’t seem to me to be much different from just having the next release default to enabled. The next release is going to be in ~6 months anyways, by which point the default would be to enable. Not many people run pre-release builds so I wouldn’t expect this to make much of a difference on testnets. Additionally, enabling full RBF now (before the next release) by merging #26305 would have the same effects on testnet, and would have the same effect of signaling that opt-in RBF will be going away. If the goal is to signal that opt-in RBF is going away soon, I think that has already been sufficiently signaled via the bitcoin-dev discussion as well.
With #25600, #26305 and this current PR I think we have all the options represented for fullrbf deployment or enablement over mainet. From my understanding of the mailing list discussions, it sounds there is a broad agreement over full-rbf deployment even by zero-conf services operators. However, the remaining open question sounds to be more on the deployment timeframe. It should be noted the difference between this current PR and #26305 are really lightweight, if Core binds to its release schedule of a new version every 6 months. At the same time it provides a clear time point for zero-conf services operators, independently of the aleas of release candidates, on the assumption next release is tagged before 1st May 2023.
This left as a remaining wonder if there is any objection among zero-conf services operators over a ~6 months timeframe to upgrade or harden their services. I don’t know if some zero-conf services operators relying on PoS (e.g Bitcoin ATMs) might need longer deployment cycles.
This enables easier testing of the behaviour on test networks and provides time for services relying on RBF being opt-in only to adapt to that policy going away.
https://github.com/bitcoin/bitcoin/pull/26323/commits/7cbcf5b61dcaa34b152217bb3c7518186c025bd2 will make it easier to test bugs in bitcoin core and every project that uses bitcoin core. I am sure there will be some vulnerabilities so this is a step in right direction and reviews, testings etc. are always helpful.
This doesn’t seem to me to be much different from just having the next release default to enabled.
Note that this is intended for inclusion in 24.0 not just master and eventual inclusion in 25.0
Getting a little bewildered by all the PRs. Make full RBF default on bitcoin-inquisition first if we want testnets/signets to have it on by default before Core. And discuss the release schedule of when Core might change the default on an issue and/or on the mailing list?
I don’t believe in added code (complexity) for issues that can be resolved in alternative repos and through communication with the ecosystem. Hence I guess that makes me an Approach ACK on #26305 and Approach NACK on this PR.
This isn’t a consensus change. Coding up “activations” and flag days etc seems like overkill to me. As others have said previously if your security model relies on (default) policy that is a weakness of your security model. A reliance on consensus is not a weakness of your security model. We think changing the default policy in Core could partially bolster Lightning and Layer 2’s current security model but it certainly doesn’t resolve it entirely. Hence a default policy change should not and does not need to be treated like a consensus rule change.
I did slightly longer form discussion on the email thread https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-October/021021.html
General approach ACK, but I see no particular rush for a 24.0 backport vs kicking it back another 6 months and not bothering with a backport for 24.0. Moving on is a win for the community, even if we are rejecting each others’ premises at this juncture on 0-conf security.
To me, this seems like a more aggressive approach compared to #26305. If this PR gets into 24.0, this means that full-RBF transactions will percolate through the network starting May 1, 2023, without any further chance to adapt (because regardless of what we do in 25.0, a sufficient number of 24.0 nodes will be in mainnet). I’m not sure if the discussion on the mailing list has advanced far enough to warrant locking in this decision now (timelines haven’t really been discussed much), and I also wonder if this is the kind of change we should be making this late in a release cycle, after rc2. With #26305, we’d have more time to re-evaluate whether to make full-rbf the default for 25.0 or wait a bit longer.
While this PR “prevents” node operators from enabling full-RBF right now, I don’t think that this is such a strong argument, because I would imagine that most of the node operators who would choose to support fullrbf right now are also technically proficient enough to recompile bitcoin core after removing the barrier this PR introduces with a trivial one-line patch of backdating m_defer_full_rbf. Also, if a significant percentage of node operators (at least ~5-10% seem to be necessary to make an impact) really care about full-RBF that much that they’d actively activate it now (which I doubt, I believe that almost everyone will just go with the default for now), plus a significant percentage of hash power actively chooses to mine these transactions, then it wouldn’t be on the devs to try stopping them.
mempoolfullrbf setting available for the applications who would like to experiment with replacement without handling replaceability signaling.
Getting a little bewildered by all the PRs. Make full RBF default on bitcoin-inquisition first if we want testnets/signets to have it on by default before Core. And discuss the release schedule of when Core might change the default on an issue and/or on the mailing list?
bitcoin-inquisition only works for signet. All bitcoin projects and libraries do not support signet and sometimes you have to use testnet for testing.
I don’t believe in added code (complexity) for issues that can be resolved in alternative repos and through communication with the ecosystem. Hence I guess that makes me an Approach ACK on #26305 and Approach NACK on this PR.
Flag day date could be discussed and changed before merging this pull request. At least there would be a fixed date as there is no release schedule afaik or needs to be adjusted later because of default RBF policy.
If enabled, full RBF is deferred until 1st May 2023 for mainnet. Full
RBF support can still be entirely disabled by setting -mempoolfullrbf=0.
Enable full RBF on all networks by default. This enables
easier testing of the behaviour on test networks, without
causing any immediate changes on mainnet due to the deferral
imposed by the previous commit.
41@@ -42,7 +42,7 @@ class MempoolAcceptanceTest(BitcoinTestFramework):
42 def set_test_params(self):
43 self.num_nodes = 1
44 self.extra_args = [[
45- '-txindex','-permitbaremultisig=0',
46+ '-txindex','-permitbaremultisig=0', '-mempoolfullrbf=0'
Seems easier and clearer to just fixup the test?
0diff --git a/test/functional/mempool_accept.py b/test/functional/mempool_accept.py
1index 02ec18140c..be9cea95c1 100755
2--- a/test/functional/mempool_accept.py
3+++ b/test/functional/mempool_accept.py
4@@ -140,7 +140,14 @@ class MempoolAcceptanceTest(BitcoinTestFramework):
5 tx = tx_from_hex(raw_tx_0)
6 tx.vout[0].nValue -= int(4 * fee * COIN) # Set more fee
7 self.check_mempool_result(
8- result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': 'txn-mempool-conflict'}],
9+ result_expected=[{
10+ 'txid': tx.rehash(),
11+ 'allowed': True,
12+ 'vsize': 104,
13+ 'fees': {
14+ 'base': Decimal('0.00004200')
15+ },
16+ }],
17 rawtxs=[tx.serialize().hex()],
18 maxfeerate=0,
19 )
20diff --git a/test/functional/p2p_permissions.py b/test/functional/p2p_permissions.py
21index 453a0920cc..52caceee24 100755
22--- a/test/functional/p2p_permissions.py
23+++ b/test/functional/p2p_permissions.py
24@@ -112,8 +112,7 @@ class P2PPermissionsTests(BitcoinTestFramework):
25 }], outputs=[{
26 ADDRESS_BCRT1_P2WSH_OP_TRUE: 5,
27 }],
28- replaceable=False),
29- )
30+ ))
31 tx.wit.vtxinwit = [CTxInWitness()]
32 tx.wit.vtxinwit[0].scriptWitness.stack = [CScript([OP_TRUE])]
33 txid = tx.rehash()
34@@ -130,13 +129,13 @@ class P2PPermissionsTests(BitcoinTestFramework):
35 self.log.debug("Check that node[1] will not send an invalid tx to node[0]")
36 tx.vout[0].nValue += 1
37 txid = tx.rehash()
38- # Send the transaction twice. The first time, it'll be rejected by ATMP because it conflicts
39- # with a mempool transaction. The second time, it'll be in the m_recent_rejects filter.
40+ # Send the transaction twice. The first time, it'll be rejected by the
41+ # mempool. The second time, it'll be in the m_recent_rejects filter.
42 p2p_rebroadcast_wallet.send_txs_and_test(
43 [tx],
44 self.nodes[1],
45 success=False,
46- reject_reason='{} from peer=0 was not accepted: txn-mempool-conflict'.format(txid)
47+ reject_reason=f"{txid} from peer=0 was not accepted: insufficient fee",
48 )
49
50 p2p_rebroadcast_wallet.send_txs_and_test(
Concept NACK for reasons already stated amply by myself and others on the mailing list.
I don’t mind a time-based change to the default (though even that seems overengineered), but ignoring an explicitly-enabled full RBF is just dumb. Especially to simply satisfy a false sense of “security” that doesn’t really exist.
Concept NACK for reasons already stated amply by myself and others on the mailing list.
I don’t mind a time-based change to the default (though even that seems overengineered), but ignoring an explicitly-enabled full RBF is just dumb. Especially to simply satisfy a false sense of “security” that doesn’t really exist.
#26305 is even worse because:
Only options left for users is to not run v 25.0 if that pull request is merged or use the config option to disable it. Others (majority) will anyway run other versions of core, so I am not even sure what kind of security guarantee does this change provide.
#26305 is even worse because:
Just to be clear, as advocated in the opening description of #26305, in reaction to the mailing list discussion, idea was to have all the full-rbf deployment options available. I think we have between this current PR, #26305 and #25600 and give a chance for everyone to express an opinion on real code. I hope we’ll be able to browse the trade-offs of all the options during upcoming IRC meeting of this week, and give more time to sync with the mailing list discussions.
If you have more ideas of another way forward to deploy full-rbf, I would invite you to open a code proposal. Or if you have conceptual reserves on the idea of full-rbf deployed by Bitcoin Core, I would invite you to pursue the conversation on the mailing list.
95@@ -96,6 +96,9 @@ class CMainParams : public CChainParams {
96 consensus.nMinimumChainWork = uint256S("0x00000000000000000000000000000000000000003404ba0801921119f903495e");
97 consensus.defaultAssumeValid = uint256S("0x00000000000000000009c97098b5295f7e5f183ac811fb5d1534040adb93cabd"); // 751565
98
99+ // Defer full RBF activation
100+ m_defer_full_rbf = 1682899200; // 1 May 2023 00:00:00 +0000
0 m_defer_full_rbf = 1698796800; // 1 Nov 2023 00:00:00 +0000
If you have more ideas of another way forward to deploy full-rbf, I would invite you to open a code proposal. Or if you have conceptual reserves on the idea of full-rbf deployed by Bitcoin Core, I would invite you to pursue the conversation on the mailing list. @ariard This is the only change I would like in this pull request and prefer this over other 2 for fullrbf(default) in bitcoin core.
Already sent an email to bitcoin-dev, not updated yet.
I suspect (b) is has a long time horizon, as most services suck it up and take the DoS currently anyways, ala Wasabi.
(a) seems to range from “NACK”, to “6 months” to “when you’ve solved free option problem I have due to my business model”. I’d personally focus on concrete numbers given.
I’m agreeing on with other opinions here – at best a development timeline should be informed from engineering and operational resources required on 0confs business/merchants. Requesting to high resources on a short-time frame presents a merchant ecosystem centralization risk. An outcome we might be careful to avoid.
On (b), we have to account for all the coinjoins services (Joinmarket/Wasabi/Whirlpool), the new collaborative construction scheme like https://github.com/ln-vortex/ln-vortex. Beyond dual-funding/splicing are expected to be supported and deployed soon by some LSP such as Phoenix or some LDK users.
I feel this PR is conflating two things:
-mempoolfullrbf option on mainnet now; addressed by making the option ignored for a certain amount of time.I believe these should be addressed separately. If (1) was worth addressing (which I am not convinced of), we should simply revert #25353. Having an exposed command-line option which has no effect for the time being seems strictly worse to me than just not having that option at all until said later date.
If (2) is worth addressing (which I’m more in favor of, though I’m not convinced we’re ready to pick a timeframe yet), it can be done by making the default of the option time-dependent, or simply planning to change the default in a future release.
Having an exposed command-line option which has no effect for the time being seems strictly worse to me than just not having that option at all until said later date.
Having an option that’s inactive until some future date means that, at that future date, you can use that option at that time without first having to do an additional software upgrade. Without the option, you also have to do an upgrade. Maybe a software upgrade is trivial enough that that’s not worth worrying about, but that still seems to be a trivial counterexample to having the option being “strictly worse” that not?
If you kept the other logic from this PR, but removed just the command line option, the behaviour would be “opt-in RBF until 1st May, then full RBF” – having the option just allows you to change that locally to “opt-in RBF until 1st May, then opt-in RBF after as well”. I’m not sure that’s a very useful option to have for mainnet; but it’s at least a useful option to have for testing purposes in the meantime. It could perhaps also be a useful safety-net for mainnet if 99% of users decide full RBF is a bad idea and want to disable it asap without also having to do a software update.
If [lack of a clear rollout schedule for adoption of the full RBF policy on mainnet] is worth addressing (which I’m more in favor of, though I’m not convinced we’re ready to pick a timeframe yet),
Where I’m coming from is that I think core making the option available on mainnet is picking a timeframe: effectively release date plus a few weeks if the default is true; or release date plus a few months if the default is false. (Obviously, I might be wrong in thinking that). I don’t think that’s a good or a bad thing in particular, just a reality of being the most popular node software.
It smells funny to add things to chainparams for something that definitely isn’t consensus and we haven’t done that before (?)
Yes, chain params are parameters that depend on the chain. Consensus params are also chain params, but not every consensus param is a chain param. There are also other default policy settings (fRequireStandard) in chain params, so I don’t think there is a better place to put them?
I agree with the separation from #26323 (comment), and i’m leaning toward Concept NACK on both fronts:
true, at least for now.
I’m writing here with the hope of adding some clarity to the issues that this PR tries to solve and, more specifically, why it is that an opt-in deployment would be dangerous for zero-conf apps. Please let me know if this is the wrong venue for such clarifications.
During the weekend, I realized I’ve been trying to convey why, after an opt-in deployment, an attack on zero-conf apps is possible, but not why it’s probable. In particular, it may be helpful to share some concrete data points that might not be obvious from the outside. I’ll only talk about Muun since it’s what I have detailed data for.
Today, Muun users have all their funds on-chain and possess all the private keys needed to move those funds independently. When a user makes an outgoing lightning payment in Muun, the mobile wallet uses the on-chain funds to make a submarine swap. More specifically, the wallet will broadcast an on-chain transaction with an HTLC output.
This setup was optimized for user security (no need for watchtowers, static non-freezable backup, on-chain payments don’t need a trusted swap, etc.) but made lightning payments more expensive since they have to hit the chain every time. During high-fee scenarios, these swaps can get so expensive that users will stop making payments, which led us to make the fee for the on-chain HTLC transaction as low as possible. Currently, an HTLC transaction may take up to a week to confirm.
To provide the instantness that users expect from a lightning payment, we perform a zero-conf risk analysis, as described on the mailing list, before making the lightning payment needed to complete the submarine swap. The double-spend attack on Muun that would be made much easier with a full-RBF deployment is the double-spending of the HTLC transaction (or one of its unconfirmed ancestors).
To make the independent recovery process as easy as possible for users that may not be familiar with the bitcoin protocol, we built an open-source recovery tool. The tool will find and sweep all user funds without collaboration from Muun servers.
To attempt a double-spend attack on Muun, you can just run the recovery tool to sweep your funds while making a lightning payment inside the mobile wallet. It’s a relatively easy attack to perform, which doesn’t even require coding skills. We receive multiple malicious double-spend attempts every day. For example, this is an (unsuccessful) double-spend attempt from today [0].
In practice, the zero-conf risk analysis is highly effective, catching most of these attempts.
In the event of a non-empty mempool, even if a single-digit percentage of the hashing power adopts full-RBF, the chances of a double-spend attack succeeding are high. Given that the HTLC transaction needs a low fee to be viable (from a UX perspective), any of our current attackers that manage to get their transaction relayed to these miners will probably succeed in getting it mined before the HTLC transaction is mined.
I understand the desire to gradually deploy a feature with complex side effects, such as full-RBF, instead of a big-bang deployment like this PR proposes. Such a deployment would give us time to study and understand the side effects before committing to a full deployment.
However, there are two issues with that approach:
The current PR addresses both concerns.
As some may know, it’s still early days for non-custodial lightning wallets on mobile devices (even more so when we first deployed lightning payments to production in 2019). Current state-of-the-art off-chain setups for mobile devices:
Understanding that there are many open security issues, we decided to take an iterative approach to lightning, preferring to optimize for user security along the way, even if sometimes that meant trading it off for Muun’s security. In particular, we always knew that zero-conf would not last forever, so we’ve been working for some time on transitioning from our current submarine swap architecture to an off-chain architecture that doesn’t compromise user security.
I’m sure we will all eventually converge on a similar setup. It has been incredibly valuable to have multiple wallets exploring different approaches to map the mobile lightning design space.
[0]
original htlc txid: 0f121365e69442b8883aec9277b45b13f68a46fac87468d2f99b6fe99a56d2f6
original htlc raw tx:
00100000000010191ab6cdf87dd72e12a777e96714a97a8bd47c2b324932534c19f3f564207c3680000000000ffffffff02d3220000000000002251202d998180198df8c761291f9f71aa246403ecf1e26520929e3eb1357923dcea1b874c1f00000000002200202cdae88db47726db6c1779e5266ec839cd45d8bc7d6a5211492b42b28dd9253d014189a6ff7388124ea84822a46aac2933d44d193a7d9ab7f3ade3fffeb0f3092203d3250fcdd2cf18111de6d781f9028ed55a37d811aa83ee8ce61e09b36cfb34cd0100000000
double spending txid: ea1cb2468e4bba4f3105ab799019166f514d8d1bff1505f730f452791c0c60be
double spending raw tx:
00200000000010191ab6cdf87dd72e12a777e96714a97a8bd47c2b324932534c19f3f564207c3680000000000ffffffff015b681f00000000001600148ec70195d3bfc349c36c1769631ac080c8532a5f0141f1710f098ecf4c4be486b06f8c6058f5454040805442416f31eb7d4279e530046e824eae3abb6f3edea2a53311edd6b72ac99c08f5e134ed686ca46ff9679e300100000000
In practice, the zero-conf risk analysis is highly effective, catching most of these attempts.
You’ve made a specific kind of attempt easy, and are catching those. But if someone really wanted to do it, you probably couldn’t detect those.
In the event of a non-empty mempool, even if a single-digit percentage of the hashing power adopts full-RBF, the chances of a double-spend attack succeeding are high.
Having this option does not change that risk. Miners who wish to use full RBF can (and frankly, should) already use one of the other options for full RBF that have existed for years.
To make the independent recovery process as easy as possible for users that may not be familiar with the bitcoin protocol, we built an open-source recovery tool. The tool will find and sweep all user funds without collaboration from Muun servers.
To attempt a double-spend attack on Muun, you can just run the recovery tool to sweep your funds while making a lightning payment inside the mobile wallet.
I have no clue why something should be changed in a bitcoin core pull request for projects that are vulnerable and could be made secure without changing anything in bitcoin core.
Will Bitcoin Core contributors make changes to secure an app if I build a vulnerable app on LN but has some users?
I have no clue why something should be changed in a bitcoin core pull request for projects that are vulnerable and could be made secure without changing anything in bitcoin core.
The project becomes vulnerable once bitcoin core pull request #25353 is shipped. I’m actually arguing for not changing bitcoin core now, or changing it in a way that gives a transition period to applications affected by the policy change.
This is simply false. #25353 makes no changes to the network. It’s simply an option, disabled by default, for a policy that has been easily available for years.
True, I misspoke. The project becomes vulnerable if #25353 is shipped AND minimally adopted by miners (ie. they turn the flag on). If no miner adopts it, then the project doesn’t become vulnerable. If the flag is adopted, it does (indirectly) change the network.
Dario, i understand your concern. However i don’t think your request is oriented toward the right persons. If you are relying on first-seen-safe in your application and trusting miners for not including a replacement transaction, then you should probably (try to) expose your rationale to them to (try to) convince the ones who currently do so to stop and the ones who don’t not to start.
Your request (and language used in your comment like “gradual deployment for the network”) seems to be based on the assumption that Bitcoin Core contributors control what code miners and nodes on the network are running. Aside from the poor logical conclusion this reasoning leads to, i don’t think that trivially preventing an option from being set in an open source project is going to make your application able to rely on first-seen-safe behaviour.
On the other hand setting the mempoolfullrbf option to true, that this PR is also doing, is indeed likely to be an effective change of relay rules on the network (because Bitcoin Core is the most used implementation and defaults have proven to be sticky). I don’t think we should do that (yet), precisely because of your report.
Changing the default to true (since the historically behaviour has been false) puts us in a situation where we need to arbitrage between two uses of the Bitcoin network. Ideally, i wish we’d change the default only once the network has proven to be transitioning.
TL;DR: we can ask the network to keep not transitioning to non-signaling RBF, but i don’t think we should either 1) try to ineffectively prevent it from transitioning by shipping an anti-feature 2) force it to transition by setting the default to true while a substantial part of users are still relying on first-seen-safe.
This is for the part related to this PR. I have comments on the broader non-signaling RBF discussion that has been ongoing (especially with regard to relying on first-seen-safe and how the network currently behaves) but i’ll keep them for the mailing list if i get the time).
I empathise with 0confs application like Muun, which aims to offer the best Lightning experience possible, in the light of the early development state of many critical piece of infrastructures (watchtower, backups, fees managements, etc). However, I’m renewing with my recent comments in #26287 and agreeing with the reasoning exposed that as a project, Bitcoin Core contributors can’t mandate the full-node softwares run on the mining hosts. Opt-in full-rbf patches have been existent for years, and the software engineering bar to enable such functionality (few lines of codes) is so low that #25353 can only be considered as a marginal change in the Bitcoin full-node landscape. Moreover, the “cat is out of the bag” and reverting the change won’t prevent mining node operators wishing to improve their income by accepting high-fee bidders replacement candidates.
I think too the effort would be more well allocated on reaching out to miners to explain the activation of full-rbf will downgrade the reliability of 0confs services/applications, unprepared to the changes and communicate a deprecation period that would enable as smooth as we can upgrade. One valuable insights from the mailing list discussions, a short full-rbf deployment timeline might favour only 0confs team with the manpower to adapt, leading to the centralisation of the payment ecosystem. On that ground of preserving decentralisation, and therefore making the payment ecosystem more competitive in the long-term with higher volume, a miner could have an incentive to defer the turning on of full-rbf setting. All that said, I don’t know if this reasoning is strong enough, and relying on social norms in a permissionless and pseudonymous ecosystem like the mining one doesn’t sound robust, I think. Still hope we can converge on a deployment timeline 6/12/18 mo, offering visibility and predictability to all ecosystem stakeholders.