Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[24.x] Remove mempoolfullrbf option #26456

Closed
wants to merge 1 commit into from

Conversation

achow101
Copy link
Member

@achow101 achow101 commented Nov 4, 2022

Reverts #25353 for 24.x only. Alternative to #26438 and #26287.

Note that this is not a backport. This is not intended to be merged into master, nor is it a followup to #26438. This is a standalone PR for 24.x only.

It's clear that the discussion around the mempoolfullrbf option is holding up the 24.0 release. Release candidates have been taking longer than usual with the hope that a resolution will be found before the next RC. But it seems that we are deadlocked on this issue. I believe that the release maintainer is hesitant to tag the release while this discussion is ongoing.

In the interest of getting the 24.0 release out sooner, I am opening this PR to remove the mempoolfullrbf option from 24.0 only. The option will remain in master so that we can continue to discuss it. This maintains the current status quo - the option is new, and similar options have not been in releases for several years.

While our general policy is that only backports of bug fixes are merged into release branches after feature freeze, I think it is reasonable to make an exception considering the magnitude of the disagreement mempoolfullrbf is causing. Maintaining the status quo allows the discussion to resolve in its own timeframe without holding up the release.

@achow101
Copy link
Member Author

achow101 commented Nov 4, 2022

To be clear, I am in favor of releasing 24.0 with the mempoolrbf option. However I am more in favor of releasing 24.0 sooner rather than later, hence this PR. There are a lot of things in 24.0 that I would like to see released, and several things with more urgency too, IMO. We need to come to a consensus on whether we will release 24.0 with the option, and once it's out the door, we can go back to arguing about whether to have it in the long term.

@petertodd
Copy link
Contributor

Still strong NACK

The stated rational for removing this default false option boils down to protecting zeroconf. We should not be setting a precedent that we're so concerned about zeroconf that merely giving an option to users is unacceptably dangerous.

Time is a concern here: John Carvallo has stated that his company is "working on tools to make 0conf acceptance easier/safer, but some devs would rather try and break a Bitcoin use case to protect their niche designs.". He of course has been one of the most vocal in opposition to having this option. We do not want to be in a position where by delaying the release of this option, we have even more political pressure in the future.

@petertodd
Copy link
Contributor

Also, I should point out that most of the discussion around #26438, particularly the many NACKs, equally applies to this pull-req. The context of those NACKs is the status quo that v24.0 will be released with the mempoolfullrbf, and those reviewers are well aware of that fact and are protesting that proposed removal from the v24.0 release.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NACK

It's clear that the discussion around the mempoolfullrbf option is holding up the 24.0 release. Release candidates have been taking longer than usual with the hope that a resolution will be found before the next RC. But it seems that we are deadlocked on this issue. I believe that the release maintainer is hesitant to tag the release while this discussion is ongoing.

There is no deadlock. #26438 should have been closed by now after so many NACKs from different developers.

@maflcko maflcko added this to the 24.0 milestone Nov 5, 2022
@dergoegge
Copy link
Member

Concept ACK

It's kinda dumb that the full RBF discussion is stalling the release. We have not had mempoolfullrbf for ten+ years, surely not having it for one or two more releases is not a problem. mempoolfullrbf has also not been shipped yet, so "removing" it does no harm what so ever. The amount of time being wasted on this is absolutely wild.

#25353 would probably not have been merged if there had been as much contention as there currently is around the issue. I think removing the option from the release branch but keeping it on master is an OK compromise. The discussion can keep going on for another release cycle if need be but meanwhile we can get out bug fixes and features in v24.0.

Not shipping mempoolfullrbf in 24.0 is not a concession to never shipping it and I personally hope that we see full rbf widely deployed on the network at some point.

If you want the mempoolfullrbf option, just run rc3!

@michaelfolkson
Copy link
Contributor

Concept NACK

In the interest of getting the 24.0 release out sooner, I am opening this PR to remove the mempoolfullrbf option from 24.0 only. The option will remain in master so that we can continue to discuss it. This maintains the current status quo - the option is new, and similar options have not been in releases for several years.

If I learnt anything from the drawn out Taproot activation debacle is that punting decisions on topics that have been discussed ad nauseam further down the line when there isn't clear consensus achieves nothing apart from wasting time and resulting in strange inferior compromises. There won't be clear consensus on this for 25.0 either or 26.0 or 27.0. #25353 was merged in July and there has been plenty of time to discuss it since. This is an option that is off by default and our experience with off by default options is that most users leave them off. Even if they turn them on it is perfectly valid to run different mempool policy. If we are going to do it do it for this release. If we aren't going to do it then don't do it ever. This project shouldn't just be a giant discussion board for a mempool policy option.

@petertodd
Copy link
Contributor

petertodd commented Nov 5, 2022 via email

Copy link
Member

@darosior darosior left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concept ACK 760d0d7 for reasons stated here: #26287 (comment).

@petertodd
Copy link
Contributor

@michaelfolkson

If I learnt anything from the drawn out Taproot activation debacle is that punting decisions on topics that have been discussed ad nauseam further down the line when there isn't clear consensus achieves nothing apart from wasting time and resulting in strange inferior compromises. There won't be clear consensus on this for 25.0 either or 26.0 or 27.0.

Agreed. But unlike Taproot and other soft forks, we at least have a clear consensus on a perfectly valid way for the wider community to express their option: the mempoolfullrbf flag.

My advice to maintainers is simple: if you release the flag, all the discussion about whether or not full-rbf should happen can happen outside of GitHub.

@ghost
Copy link

ghost commented Nov 5, 2022

If you want the mempoolfullrbf option, just run rc3!

Any users reading comments in this PR, please don't do this as there could still be bugs.

@luke-jr
Copy link
Member

luke-jr commented Nov 5, 2022

NACK to reverting OR holding up the release.

A controversial policy option is not a reason to deny people the option. People who don't want it, can just leave it off.

@harding
Copy link
Contributor

harding commented Nov 5, 2022

Concept ACK for two main reasons:

  1. As a matter of general policy, I believe it is better to release on time (or slightly late) without a new feature than to either delay the release for the new feature or release a new feature that has outstanding concerns by a significant number of well-informed good actors.

  2. As I argued extensively on #26287, I believe we should provide one release cycle worth of advanced warning in the release notes when we are contemplating a change that may significantly adversely affect downstream users.


I was trying to think of previous occasions where the project removed code during the RC process due to controversy, so here's a quick summary with references of what I think is the closest match to our current circumstances.

A brief history of making the BIP61 P2P reject message an option, disabling it by default, and then removing it across the 0.17, 0.18, 0.19, and 0.20 releases:

  • 2018-05-01: #13134 opened to add an enablebip61 configuration option and default it to true (the existing behavior). Merged 22 days later. Released in 0.17 five months later.

  • 2018-08-24: #14054 opened to default enablebip61 to false (a change in default behavior). Merged 27 days later.

  • 2019-03-01: version 0.18 with branched off master (per #14438)

  • 2019-03-06: an announcement of the planned disablement was posted to the Bitcoin-Dev mailing list. Several people protest the change.

  • 2019-03-14: #15602 opened directly against the 0.18 branch to revert enablebip61 back to true but adding a release note saying, "BIP 61 reject messages are now deprecated. [...] BIP 61 messages will be disabled by default in a future version, before being removed entirely." Merged the same day and released in 0.18 six weeks later.

  • 2019-11-24: version 0.19 released with enablebip61=0

  • 2019-10-18 (date out of order): #15347 merged into master (after 0.19 was branched off) to fully remove BIP61 reject message support, including the enablebip61 option.

In comparing that situation to the current one, I imagine those in favor of mempoolfullrbf will note that enablebip61 was included in 0.17 without issue. It's not clear to me whether that was because its preservation of existing behavior was unopposed or because nobody who would've opposed it knew about the change before its release.

What I appreciate about the previous situation is that the project at that time met the surprise of downstream users to a non-emergency change in an RC with a quick reversion but a clear statement of future intent---an intent which was ultimately realized. The total delay this added to the plan of removing BIP61 was just six months.

We're not currently in a position where I think any of us can predict the future state of full-rbf within Bitcoin Core, but I think we can take this opportunity to release 24.0 quickly without a controversial change and with communication to downstream users that there may be rbf-related changes in the next major release (and that the consequences of those potential changes may manifest even earlier due to activity outside of the Bitcoin Core project).

@luke-jr
Copy link
Member

luke-jr commented Nov 6, 2022

BTW, bitcoincore.org has for years linked to a schedule to switch to full RBF mid-2016. I'm sure there have been other mentions of this as a long-term plan. I don't think it's correct to imply there was no announcements until 24 rc1.

https://bitcoincore.org/en/faq/optin_rbf/

https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2015-June/009253.html

@greenaddress
Copy link
Contributor

Strong NACK

For the same reasons listed in the other PR

@michaelfolkson
Copy link
Contributor

outstanding concerns by a significant number of well-informed good actors

@harding: You need to be clearer here on who you consider well informed good actors. (e.g. I consider Suhas a well informed good actor with an unfortunately very last minute intervention but I also consider Peter, Luke, Andrew as well informed good actors who are all in favor of releasing 24.0 with this option) There are a small minority commenting who will be against enabling this option or any additional RBF mempool policy support (e.g. in v3 transaction relay) in any future version: 25.0, 26.0, 27.0 etc. So a valid concern a significant number of commenters have is that either it is never released because of this small minority or worse some strange compromise is proposed with that small minority (and I've observed this tendency with you a number of times now) which ends up being inferior to the initial simpler solution.

@harding
Copy link
Contributor

harding commented Nov 6, 2022

@luke-jr

BTW, bitcoincore.org has for years linked to a schedule to switch to full RBF mid-2016

Yes, it links to a schedule that was obsolete months before opt-in RBF was proposed and which says, "Prior to the activation deadline, supporting nodes and miners will support first-seen-safe(1) replace-by-fee (FSS-RBF) mempool behavior", which clearly hasn't happened in Bitcoin Core yet.[1]

I'm sure there have been other mentions of this as a long-term plan. I don't think it's correct to imply there was no announcements until 24 rc1.

There were certainly plenty of announcements of individual developers or teams of developers working towards the goal of full-rbf, but it seems like some people are checking the release notes for these types of announcements---which is the most logical place to find them, especially since the project does almost no other communication as a unified team---and so we should be using that mechanism to communicate this upcoming change.

@michaelfolkson

[...] some strange compromise is proposed ([by] you a number of times now) which ends up being inferior to the initial simpler solution.

Ooh, juicy ad hominem! How many times exactly? Can you please rank them in order of impact? I want to frame the list and put it on my brag wall.

[1] For those not around circa 2015, here's a quick summary of what FSS-RBF meant (and what full-rbf meant back then): https://bitcoinops.org/en/newsletters/2022/10/19/#fn:full-rbf

@michaelfolkson
Copy link
Contributor

Ooh, juicy ad hominem!

It is not ad hominem for any individual's past record to be assessed. How else does one come to a judgment on who is a "well-informed good actor" (your words)? Just assume everyone is and if you don't think they are it becomes "juicy and ad hominem"? I think you are a well informed good actor but there is a repeated pattern of seeking short term inferior compromises in my view. BIP 9 MTP Speedy Trial, the coin flip thing, a "transitory" mainnet activation of CTV etc. I would much rather the maintainers look at the discussion and make an informed decision perhaps drafting a statement on why they made that decision rather than compromise or punt decisions to future releases when they will be just as contentious. Release the option or take the code out and wait for v3 transaction relay (which will also be opposed by some). I'll support either.

@gruve-p
Copy link
Contributor

gruve-p commented Nov 6, 2022

Concept NACK

Same reasons as #26287 (comment)

@petertodd
Copy link
Contributor

outstanding concerns by a significant number of well-informed good actors

Yeah, @harding, I'm going to second @michaelfolkson's request for who exactly you think are the good actors here, and who you don't think is worth listening to.

We do after all have clear NACKs from three different wallets with many years of history in Bitcoin, who all have good reasons for full-rbf to exist.

@petertodd
Copy link
Contributor

2. As I argued extensively on #26287, I believe we should provide one release cycle worth of advanced warning in the release notes when we are contemplating a change that may significantly adversely affect downstream users.

@harding We are contemplating the release of a default-off option. Users and miners choose to use it.

Are you saying that we should be so concerned about zeroconf that a default off option that could potentially affect the three companies that spoke out - one of which doesn't even have a product yet, and claims to be releasing one soon - that we should delay that release by many months?

As @luke-jr noted, it's been made clear for years that zeroconf is a dangerous practice and theres no guarantee . Acceptance of it is minimal for the obvious reason that people relying on it get scammed. Frankly, pulling this feature just sends the message that zeroconf is safe enough to rely on it. That's exactly the message that people like @BitcoinErrorLog are trying to promote right now.

Now, let's get to scheduling: if you pull this option, I fully expect @BitcoinErrorLog to rush his product to the market and in 6 months on the next release, complain that we have to delay everything yet again for his sake. This ridiculous process can and will go on forever.

@harding
Copy link
Contributor

harding commented Nov 6, 2022

@petertodd

exactly [who do] you think are the good actors here

I think there are a significant number of well-informed good actors on all sides of this debate. My point is not to weigh the quantity or quality of one side against another but merely to recognize that there are "outstanding concerns" and to state my belief that "it is better to release on time (or slightly late) without a new feature than to either delay the release for the new feature or release a new feature that has outstanding concerns".

Are you saying that we should be so concerned about [...] a default off option that could potentially affect the three companies

Yes! We should be concerned about the things our downstream users find concerning. I know that argument cuts both ways on this issue---as those responding to your advocacy have shown, there are users concerned about not having this option. But not having an option is the exact same behavior we had in the prior release; it's non-inclusion will be a surprise only to those who knew it was included in master and the RCs, and (ironically) the point of having a development branch and RCs is to allow immature code to be replaced---we offer no guarantees of feature finality on master or RCs. We do have policies and precedents for announcing changes in releases that may adversely affect some users, such as in the case of a deprecation, and I'm stating in my concept ACK that I feel it's more important to follow those policies than it is to release a new feature.

pulling this feature just sends the message that zeroconf is safe enough to rely on it.

I think a warning that future releases may include a mempoolfullrbf option, along with a reminder that zeroconf is unsafe, sends a clear message that nobody should be relying on it. For #26287, I wrote a proposed release note doing just that; I'm happy to update it to the current circumstances.

if you pull this option, I fully expect [people in the future to] complain that we have to delay everything yet again for [their] sake. This ridiculous process can and will go on forever.

Although I find the present debate frustrating at times, I'm amazed at the quantity and quality of information produced by advocates on all sides in just the past few weeks. I wouldn't have thought a default-to-previous-behavior option would warrant concern, or realized how high a percentage of nodes needed the same policy to ensure reliable propagation, or seen the downsides of preferential peering, or contemplated non-replacement as a DoS-resistant policy useful in many cases, or discovered that full-rbf doesn't make any existing coinjoin or contracting protocol more reliable when opt-in rbf is available, or considered the perspective of RBF wallets authors who allow importing stuck transactions from other wallets, or been exposed to several other interesting and important technical arguments.

I guess if your end goal is to enable full-rbf or kill zeroconf, or conversely to limit rbf or preserve zeroconf, what has happened in the last month looks like a ridiculous zig-zag of a process, but to me it looks like exactly the process we want---technical experts investigating all of the nuances of a subject so that developers (and ultimately users) can make fully informed choices. If new and important arguments are still being formulated in six months by the 25.0 release, then maybe we should delay again---but if all of the arguments at that time are well-known among experts, I have confidence in the contributors to this project that they'll make the best choice available at that time.

@SkanderHelali
Copy link

NACK for the same reasons listed here #26438 (comment).

@newbiesolominer
Copy link

NACK for the same reason as I said in the other pull-req.

#26438 (comment)

@michaelfolkson Bringing up how hard decisions can be is good. Release the option and let nodes and miners decide. Thank you.

@petertodd
Copy link
Contributor

@harding

exactly [who do] you think are the good actors here

I think there are a significant number of well-informed good actors on all sides of this debate. My point is not to weigh the quantity or quality of one side against another but merely to recognize that there are "outstanding concerns" and to state my belief that "it is better to release on time (or slightly late) without a new feature than to either delay the release for the new feature or release a new feature that has outstanding concerns".

There will always be outstanding concerns. On the dev mailing list, we even had Bitrefill essentially make the argument that opt-in-rbf should be removed, and @BitcoinErrorLog has fairly explicitly made that argument.

So that leaves us with weighing competing concerns. Who do you think are the "good actors" exactly? When we're blocking the release of a default-off option that is widely desired in the community - an action that obviously prevents the community by just voicing their support by simply running with the option enabled - we should have some transparency on who calls the shots.

Are you saying that we should be so concerned about [...] a default off option that could potentially affect the three companies

Yes! We should be concerned about the things our downstream users find concerning. I know that argument cuts both ways on this issue---as those responding to your advocacy have shown, there are users concerned about not having this option. But not having an option is the exact same behavior we had in the prior release; it's non-inclusion will be a surprise only to those who knew it was included in master and the RCs, and (ironically) the point of having a development branch and RCs is to allow immature code to be replaced---we offer no guarantees of feature finality on master or RCs. We do have policies and precedents for announcing changes in releases that may adversely affect some users, such as in the case of a deprecation, and I'm stating in my concept ACK that I feel it's more important to follow those policies than it is to release a new feature.

The fact is for multiple months it was considered likely that RBF in some form would be shipped in v24.0. That wasn't controversial except of course to the tiny group who believe so strongly in zeroconf that they even argue replacement in general should be removed.

I will say it explicitly: the opinions of that group should not count in this decision. From a technology point of view they're nuts.

That leaves us with a reasonable political compromise: let the community decide. Which was why I and many others were fine with a default-off.

Now at the last minute we're pulling that feature... for what reason exactly? Other than "controversy" I haven't seen any coherent argument as to why a default off option should not be released.

pulling this feature just sends the message that zeroconf is safe enough to rely on it.

I think a warning that future releases may include a mempoolfullrbf option, along with a reminder that zeroconf is unsafe, sends a clear message that nobody should be relying on it. For #26287, I wrote a proposed release note doing just that; I'm happy to update it to the current circumstances.

Quoting #26438 (comment)

yes. I disagree with him. zeroconf is not safe nor unsafe. its just
zeroconf. how is it unsafe? because it can fall out of mempool? so can a
1conf.

It is clear that at least some people are of the mistaken impression that zeroconf is safer than it is. Pulling the default-off mempoolfullrbf option is telling those people that you were keeping zeroconf safe. No amount of "clear messages" in release notes that few people read is going to stop that message.

if you pull this option, I fully expect [people in the future to] complain that we have to delay everything yet again for [their] sake. This ridiculous process can and will go on forever.

Although I find the present debate frustrating at times, I'm amazed at the quantity and quality of information produced by advocates on all sides in just the past few weeks.

I'm not.

I wouldn't have thought a default-to-previous-behavior option would warrant concern

Yes, no sane person would have thought that... and sure enough, the Core dev members who are advocating otherwise haven't given us clear explanations of why they're blocking this option. What I do see is the possibility that their work on v3 transactions is influencing the decision, because v3 transactions aren't clearly secure. Obviously, if that's the case that gives us even more reason to release this option to learn more about the incentives involved.

I guess if your end goal is to enable full-rbf or kill zeroconf, or conversely to limit rbf or preserve zeroconf, what has happened in the last month looks like a ridiculous zig-zag of a process, but to me it looks like exactly the process we want---technical experts investigating all of the nuances of a subject so that developers (and ultimately users) can make fully informed choices. If new and important arguments are still being formulated in six months by the 25.0 release, then maybe we should delay again---but if all of the arguments at that time are well-known among experts, I have confidence in the contributors to this project that they'll make the best choice available at that time.

I have far less confidence now than I did a month ago. No-one has been able to articulate a clear reason why they're trying to block this default-off option. You can point to "concerns" and "debate" all you want. But the fact is, we have Core devs trying to block people from using an option with clear use-cases, and a wider political significance.

What it does smell like is that the v3 transaction proposal is kinda broken, and @sdaftuar thinks that by blocking full-rbf, the brokenness of them will be less obvious and they won't have wasted as much work. Not only is that technical nonsense - it's fine if only a subset of miners choose to follow the v3 rules, as it'll still achieve the goal of pinning protection - it's also an abuse of the political side of decision making.

Stop trying to block full-rbf. Let the community decide.

@yancyribbens
Copy link
Contributor

Hopefully the irony isn't lost on how the reason this PR was created was because of a deadlock, and in response there is more deadlock.

I haven't seen any coherent argument as to why a default off option should not be released.

A node operator can choose to run a patch to enable Full RBF, so you could make the argument that it doesn't matter if this option is shipped or not. Although, one important distinction, if something goes wrong, the finger pointing won't be at Core but at node operators that chose to run a one off patch. @ariard authored #25353 and later was in support of NACK #26287 (comment) which should say something.

I raised a question on the mailing list that's gone unanswered. While there hasn't been any coherent argument to why a default off option should be released, there hasn't been a coherent argument to include it other than to break zero-conf. Also, the post by @sdaftuar #26287 (comment) went largely unanswered.

I'm in favor of not forcing something until it's ready.  While I consider those who argue in favor of keeping the option to be experts on the matter,  I don't see any immediate consequences of punting until the next release; allowing everyone to have a better understanding and better prepare.

@petertodd
Copy link
Contributor

petertodd commented Nov 7, 2022 via email

@esneider
Copy link
Contributor

esneider commented Nov 7, 2022

@petertodd

Pulling the default-off mempoolfullrbf option is telling those people that you were keeping zeroconf safe. No amount of "clear messages" in release notes that few people read is going to stop that message.

Muun is currently working all-hands-on-deck to stop relying on zero-conf as soon as possible, so you could say that some part of the message was received.

For my part, whatever the final changeset ends up being, I'll personally contact as many affected apps as I can to let them know (there are a bunch of apps affected by some of the side effects from full-RBF that haven't participated in the discussion).

I know some core devs have been doing the same during the last few weeks.

The fact is for multiple months it was considered likely that RBF in some form would be shipped in v24.0. That wasn't controversial except of course to the tiny group who believe so strongly in zeroconf that they even argue replacement in general should be removed.

I will say it explicitly: the opinions of that group should not count in this decision. From a technology point of view they're nuts.

That leaves us with a reasonable political compromise: let the community decide. Which was why I and many others were fine with a default-off.

Now at the last minute we're pulling that feature... for what reason exactly? Other than "controversy" I haven't seen any coherent argument as to why a default off option should not be released.

Given that I never argued against a full-RBF network or replacements in general, I hope this counts for something.

Back in June, when the default-false flag was announced on the mailing list, it was widely believed that it wouldn't have enough adoption to get the network to the percolation threshold needed to transition the network fully into a full-RBF policy.

What wasn't clear at the point (neither in the initial communications nor in the following github and mailing list discussions) is that with minimal miner adoption, some kinds of applications would be immediately disrupted (ie. exposed to substantially more risk than before). After seeing the events of the last weeks, I'm sure that, had they been aware of this, several core developers would have wanted to at least give a heads up to the affected applications.

Given that we were pro full-RBF and thought we would have time until a default-true deployment to stop relying on zero-conf, we didn't speak against the default-false flag at the time.

At the very least, you could say that there is new information about the immediacy of the impact on affected applications and the surface of the problem for affected applications (eg. the free american call option problem brought by Sergej from Bitrefill).

There will always be outstanding concerns. On the dev mailing list, we even had Bitrefill essentially make the argument that opt-in-rbf should be removed, and @BitcoinErrorLog has fairly explicitly made that argument.

So that leaves us with weighing competing concerns. Who do you think are the "good actors" exactly? When we're blocking the release of a default-off option that is widely desired in the community - an action that obviously prevents the community by just voicing their support by simply running with the option enabled

Notice that the original intention of the default-false flag #25353 wasn't to design a voting mechanism for whether the node operators want to transition (or not) to a full-RBF network.

As a voting mechanism, the flag suffers from many problems:

  • there's no clear activation threshold
  • it's easy to manipulate (eg. sybil attack)
  • it's opaque and hard to quantify where the current count is at
  • nodes voting for or against full-RBF may transition the network to an intermediate state without enough adoption to reach the percolation threshold, which isn't what either party is voting for:
    • Some full-RBF-enabled nodes can sometimes use full-RBF, while others can't.
    • From a full-RBF-enabled node's point of view, after broadcasting a full-RBF replacement, it's hard to determine whether it will work or not.
    • From a full-RBF-enabled node's point of view, it's hard to determine whether someone sending you money is using full-RBF.

@harding
Copy link
Contributor

harding commented Nov 7, 2022

@petertodd

What it does smell like is that the v3 transaction proposal is kinda broken, and @sdaftuar thinks that by blocking full-rbf, the brokenness of them will be less obvious and they won't have wasted as much work. [...] an abuse of the political side of decision making.

I'm deeply saddened that your decade worth of passionate advocacy for RBF, an effort I've long admired (and sometimes contributed to in small ways), has led to you to accuse one of Bitcoin's most thoughtful protocol developers of obfuscation and abuse of process.

I don't believe your accusation is warranted in any way, and I hope that you'll realize your mistake and apologize to Suhas (even if only in private).

I think the above is an indication that this argument is becoming more emotional than technical, and so I don't feel any need to contribute further to it---hopefully enough arguments and evidence have been presented to allow the maintainers to make the best possible decision when they are ready.

@petertodd
Copy link
Contributor

@esneider

Pulling the default-off mempoolfullrbf option is telling those people that you were keeping zeroconf safe. No amount of "clear messages" in release notes that few people read is going to stop that message.

Muun is currently working all-hands-on-deck to stop relying on zero-conf as soon as possible, so you could say that some part of the message was received.

I noticed recently that Muun appears to have made this problem even worse than before: previously, if I sent a large Lightning transaction, Muun would delay the transaction with a message saying that it was waiting for confirmation. Lately in testing, I haven't seen this message.

Am I correct in saying that Muun has removed/increased some kind of limit here that previously limited exposure to zeroconf risks?

Back in June, when the default-false flag was announced on the mailing list, it was widely believed that it wouldn't have enough adoption to get the network to the percolation threshold needed to transition the network fully into a full-RBF policy.

I can not agree with that statement. Patches to do preferential peering have existed for years. I personally maintained one for a while. The percolation threshold does not meaningfully prevent full-rbf txs from getting relayed to miners. Your understanding of the issue in June was simply flawed, and flawed in a way that I do not think was common.

What wasn't clear at the point (neither in the initial communications nor in the following github and mailing list discussions) is that with minimal miner adoption, some kinds of applications would be immediately disrupted (ie. exposed to substantially more risk than before). After seeing the events of the last weeks, I'm sure that, had they been aware of this, several core developers would have wanted to at least give a heads up to the affected applications.

Again, because preferential peering patches have existed for years - and because people can obviously just coordinate to peer with each other - it has always been clear that with minimal miner adopting of full-rbf, some kinds of applications would be immediately disrupted.

Furthermore, miner adoption of any divergent mempool policy has this effect. The most obvious one being min relay fees (and the close cousin, the max mempool size limit). It is precisely that divergence that allowed myself and others to over and over again make successful double-spends with rbf. Those double spends have been widely documented, eg: https://petertodd.org/2016/are-wallets-ready-for-rbf

So that leaves us with weighing competing concerns. Who do you think are the "good actors" exactly? When we're blocking the release of a default-off option that is widely desired in the community - an action that obviously prevents the community by just voicing their support by simply running with the option enabled

Notice that the original intention of the default-false flag #25353 wasn't to design a voting mechanism for whether the node operators want to transition (or not) to a full-RBF network.

As a voting mechanism, the flag suffers from many problems:

* there's no clear activation threshold

There's a very clear one: how easy it is to double-spend with varying degrees of effort.

* it's easy to manipulate (eg. sybil attack)

This configuration option does not expose a service bit or other easily observable network feature. There is nothing to sybil attack.

Equally from a different perspective, if someone wants to make a large number of nodes relay full-rbf, removing a default-off configuration option changes absolutely nothing: turning full-rbf on is literally a one-line patch. Anyone technologically sophisticated enough to be doing a genuine sybil attack will have the ability to turn that feature on themselves.

* it's opaque and hard to quantify where the current count is at

According to some commenters that's a good thing, as they believe that full-rbf nodes will get attacked.

Anyway, ease of double-spending is the obvious and natural way of measuring support. Indeed, there is no need to "measure" support directly.

* nodes voting for or against full-RBF may transition the network to an intermediate state without enough adoption to reach the percolation threshold, which isn't what either party is voting for:

As you yourself admit, if an intermediate state is somehow a problem, fixing that by running a few more full-rbf nodes is easily accomplished.

  * Some full-RBF-enabled nodes can sometimes use full-RBF, while others can't.

Full-RBF nodes that definitely need it to work can easily connect to a node with preferential peering. This is not a problem.

  * From a full-RBF-enabled node's point of view, after broadcasting a full-RBF replacement, it's hard to determine whether it will work or not.

This isn't a reason to prevent people from choosing to run full-rbf nodes.

  * From a full-RBF-enabled node's point of view, it's hard to determine whether someone sending you money is using full-RBF.

Why is this even an issue? Everyone has to assume that an unconfirmed transaction may be double-spent.

Sorry, but I believe you have had serious misunderstandings of how the Bitcoin protocol works. I think your misunderstandings are illustrative of why it would be a serious mistake to remove this default-off option: we'd be sending a message that zeroconf is safe and we had made a meaningful difference in making it safer.

Also, I do not believe that creating conditions where Muun wallet feels it must switch to its previous behavior of waiting for a confirmation is a showstopper: users can easily migrate to a different wallet like Phoenix that actually implements the Lightning protocol in the wallet, as expected.

@petertodd
Copy link
Contributor

@harding

I'm deeply saddened that your decade worth of passionate advocacy for RBF, an effort I've long admired (and sometimes contributed to in small ways), has led to you to accuse one of Bitcoin's most thoughtful protocol developers of obfuscation and abuse of process.

I am saddened that @sdaftuar has put himself in such a position.

The observation is not just mine. In private communication I've had a number of people with the technical chops to follow the discussion - people who aren't involved in Bitcoin Core development - tell me that something has gone seriously off the rails with this sudden opposition to a default-off option. It's an example of poor political judgement around an obviously political issue, and external observers can easily see it.

Look, getting carried away with support for the v3 project is human and forgivable. But it's much easier to forgive if the people involved correct this mistake and drop these pull-reqs.

@BitcoinErrorLog
Copy link

Fwiw, despite all the attention and attribution Peter is making to me, I was not the originator of this fresh debate. Muun brought it to my attention, along with Bitrefill and colleagues.

I have been part of merchant efforts to explore and formalize best practices for 0conf use cases for 4 years, from "turbo channels" thru to our new wallet, Bitkit. Before I worked at Bitrefill I was ignorant and did not see how users benefit from 0conf, nor did I understand the various ways a merchant can limit their exposure to 0conf risks. The risks 0conf-accepting merchants accept are known and manageable and a very useful service to Bitcoiners, generating txns, fees, utility, etc.

Further, 0conf risk-mitigations make Lightning complexity bearable for users at the app level, when buying channels, and LN implementations have all recently added support, finally, for 0conf channels. I anticipate this will help greatly with adoption and UX of wallets.

This is all thanks to first-seen policy in mempools. This policy has "somehow" helped us discover that such behaviors may be incentive-compatible. Even if this changes, we do not know how many days or decades it may take, and opt-in rbf is fine as-was for 7 years as well.

However temporary you may believe this harmony to be, it is no reason to disrupt it. None of us can actually predict the future nor claim to have a perfect grasp of incentives across all interactions. What we do know is this is working and Bitcoiners are benefiting.

We also know that full-rbf can be implemented at the wallet level easily. We even have designs for doing so in our app if we can extend bip21 to pass incidental feerate and rbf-off flags as a standard for merchant invoicing. These are quite elegant and achievable goals.

I am not sure how Peter thinks our product roadmapping works, or what product he thinks we will "rush" out in 6 months, or why any of that would be a threat to anyone or anything, but such claims are irrelevant and inaccurate. What I want is to build on the Bitcoin we have without it shifting under my feet at the whim of self-righteous equivocators with pet designs for Bitcoin Core.

I am advocating to protect what we have and to avoid influencing delicate policy decisions. Rbf and 0conf can coexist under first-seen and I find the full-rbf plan to be unfair as it opts txns into behaviors without their choice. I understand that this could still happen without Core assistance, but that fact is that in 7 years it has not, and we have had no issues with rbf or with 0conf.

Thanks for your consideration and sorry for yet more text for you to read on this topic. I am here to protect Bitcoin and ensure maximal usage in any appropriate way I can.

@achow101
Copy link
Member Author

achow101 commented Nov 8, 2022

Closing in line with #26438 (comment)

@achow101 achow101 closed this Nov 8, 2022
@yancyribbens
Copy link
Contributor

After reading more of the arguments, I think leaving this option in is the correct decision. I think an important point is that even if a transaction does not signal replacement, node and miners are really just working on what is the most profitable with the highest incentive. Therefore, to build software the assumes an unconfirmed transaction is somehow safe from replacement is faulty.

@bitcoin bitcoin locked as resolved and limited conversation to collaborators Nov 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet