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

Remove RBF from user visible settings #8088

Closed
llamasoft opened this issue Dec 4, 2022 · 16 comments
Closed

Remove RBF from user visible settings #8088

llamasoft opened this issue Dec 4, 2022 · 16 comments
Labels
topic-wallet 👛 related to wallet.py, or maybe address_synchronizer.py/coinchooser.py user-interface 🔲

Comments

@llamasoft
Copy link

Bitcoin 24.0 adds a new mempoolfullrbf option that allows for replace-by-fee even if the original transaction didn't set the RBF flag.

Currently, the get_tx_info and dscancel functions only consider a double-spend-cancel viable if the transaction has the RBF flag set (i.e. if not tx.is_final()).
As the linked release notes mention, full RBF will now be a mainstream feature even though many alternate implementations have supported it for a while. Either is_final() should always return False or it should depend on if the transaction has been confirmed or not.

@junderw
Copy link
Contributor

junderw commented Dec 5, 2022

From a usability perspective, what you really want to avoid is:

  1. Electrum says we can cancel.
  2. Electrum fails to cancel at a high rate due to lack of adoption of the mempoolfullrbf flag (currently defaults to off)

Maybe introduce the logic change as a simple flag that advanced users can toggle, and the default can be switched to "always return False" after the success rate is high enough (enough of the network adopts the mempoolfullrbf flag)

@ecdsa
Copy link
Member

ecdsa commented Dec 5, 2022

I think there is no need to update the current logic for the moment.
As @junderw pointed out, lack of adoption would create usability issues.
Electrum has RBF by default, that should be enough for most use cases.

@SomberNight
Copy link
Member

Maybe introduce the logic change as a simple flag that advanced users can toggle

We could add a config key that allows RBF-ing non-signalling txs.

@petertodd
Copy link

Counterpoint re: usability issues: the times when users are most likely to need this, mempool congestion, are times when due to txs being dropped full-rbf doesn't need any miner support to be fairly often succesful. In fact, you don't even need to be connected to full-rbf peers as a dropped tx isn't being double-spent from their perspective.

@ecdsa
Copy link
Member

ecdsa commented Dec 6, 2022

We could add a config key that allows RBF-ing non-signalling txs.

I do not think it is the right approach to add new stuff every time we want to fix a rare issue.
Lets stop trying to please everyone all the time.

I would prefer if we removed the current RBF checkbox in the UI, and we made all our transactions RBF signalling.
That would simplify the UX, instead of making it more complex.

The reason why we allow users to disable RBF is because some merchants want non-rbf transactions.
That is the real issue. In my opinion, we should not let those merchants dictate what we do.

@petertodd
Copy link

petertodd commented Dec 6, 2022 via email

@ecdsa
Copy link
Member

ecdsa commented Dec 7, 2022

ok, let me reopen this. with full-RBF, the RBF setting does not make sense anymore, it makes sense to remove it from the GUI.

@ecdsa ecdsa reopened this Dec 7, 2022
@ecdsa ecdsa changed the title "Double Spend Cancel" logic needs to be updated for new Bitcoin 24.0 feature Remove RBF from user visible settings Dec 7, 2022
@ecdsa
Copy link
Member

ecdsa commented Dec 7, 2022

I think the main merchant that caused problems in the past was BitPay. Are they still rejecting RBF payments today? Are there other payment processors still doing that?

edit: actually, I do not think they are rejecting RBF payments. they just don't accept before they are confirmed.

@petertodd
Copy link

petertodd commented Dec 7, 2022 via email

@ecdsa
Copy link
Member

ecdsa commented Dec 7, 2022

I am referring to this page: https://github.com/bitpay/jsonPaymentProtocol/blob/master/bip70.md
I am not sure what they mean by that, and if this documentation is outdated or not.
There is a 5 years old reddit thread about it https://www.reddit.com/r/Bitcoin/comments/86olrl/bitpay_wants_wallets_to_disable_rbf_accept/

@SomberNight
Copy link
Member

SomberNight commented Dec 10, 2022

I would prefer if we removed the current RBF checkbox in the UI, and we made all our transactions RBF signalling.
That would simplify the UX, instead of making it more complex.

Sure, that's fine, at this point I think we can do that.
However, note it does not address OP's point with opening this issue:

replace-by-fee even if the original transaction didn't set the RBF flag.

Even if we make sure all of the transactions we create signal RBF, if a user restores a wallet with pre-existing unconfirmed txs created elsewhere, the question is whether we want to allow RBF-ing those.
Also, note that we don't always signal RBF atm, e.g. for channel funding txs (see #7072).

RBF-ing a non-signalling tx is highly probabilistic atm. First and foremost, I am sure many electrum servers are backed by bitcoinds with default mempoolfullrbf=false, sending an error to the client if it tried to RBF such a tx. Even when using an electrum server that has mempoolfullrbf=true, propagation in the p2p network is not guaranteed and is totally opaque to the client (even to bitcoind).
So, atm, if we wanted to allow RBF-ing non-signalling txs, we would either need to hide it behind an advanced option, or give proper warnings and explanation to the user.

(note: power-users can atm RBF non-signalling txs by using the Qt console to call wallet.adb.remove_transaction(txid) and then creating a replacement normally (but making sure it conflicts with the old tx) - this is what I had done multiple times over the years when needing this, and also instructed some users to do it, but this is obviously not a nice solution)

@SomberNight SomberNight added user-interface 🔲 topic-wallet 👛 related to wallet.py, or maybe address_synchronizer.py/coinchooser.py labels Dec 10, 2022
@ecdsa
Copy link
Member

ecdsa commented Dec 10, 2022

All right. I don't know if bumping the fee of transactions imported from non-Electrum wallets is what the OP had in mind, but I consider that as out of scope. If we have an opportunity to simplify the UX, we should take it. Nothing beats the satisfaction of removing a few check boxes from the GUI.

Regarding channel funding, we can keep using the RBF flag as a way to prevent an accidental fee bumping from another instance of the same wallet. As you point out, that solution is not going to work with channel v2, so we will need to address that separately.

@ecdsa ecdsa closed this as completed in e1dc7d1 Dec 10, 2022
Dargon789 pushed a commit to Dargon789/electrum that referenced this issue Jan 27, 2023
preference from the GUI, because the mempoolfullrbf option in
Bitcoin 0.24 makes RBF signaling pretty meaningless. Fixes spesmilo#8088.

Note: RBF remains disabled for channel funding transactions.
In that case, the flag is actually only used as a semaphore
between different instances of the same wallet.
@pscopic
Copy link

pscopic commented May 29, 2023

All right. I don't know if bumping the fee of transactions imported from non-Electrum wallets is what the OP had in mind, but I consider that as out of scope. If we have an opportunity to simplify the UX, we should take it. Nothing beats the satisfaction of removing a few check boxes from the GUI.

Regarding channel funding, we can keep using the RBF flag as a way to prevent an accidental fee bumping from another instance of the same wallet. As you point out, that solution is not going to work with channel v2, so we will need to address that separately.

Unfortunately, the latest release with the RBF checkbox removed has made it practically unusable with the popular bitrefill merchant during the high mempool times that have been pervasive lately. Instead of processing immediately with a minimum sat/byte rate they provide when RBF is disabled it instead requires one confirmation. Been waiting 40 minutes so far for my transaction to process. The only alternative is to bump the fee up to levels that makes the transaction itself impractical.

@petertodd
Copy link

@pscopic Your objection doesn't make sense. Bitrefill doesn't immediately accept unconfirmed transactions without very high fees. Eg at this moment, mempool.space is estimating 50sat/vB is required to get accepted into the next block, while bitrefill requests a tx fee of >70sat/vB for immediate acceptance.

Anyway, Electrum supports Lightning. If you want instant confirmations, use it.

@pscopic
Copy link

pscopic commented Jun 18, 2023

Bitrefill doesn't immediately accept unconfirmed transactions without very high fees.

Thanks for your reply, and you're right they do charge a surtax for immediately accepting unconfirmed transactions that can average 40-50% more than the sat/byte to get accepted to the next block listed on mempool.space. Despite that, setting my sat/byte to that premium can still take hours before I receive that first confirmation without the ability to turn rbf off.

And while I take your point to use lightning, there doesn't appear to be any decent updated documentation on how to use the android electrum lightning feature despite the recent changes to the UI. If you know otherwise please let me know as the bitcointalk docs for android lightning are terribly antiquated. As it stands it appears to add a level of complexity that runs counter to the intent of removing the rbf checkbox in the interests of simplifying the UI.

@petertodd
Copy link

petertodd commented Jun 21, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic-wallet 👛 related to wallet.py, or maybe address_synchronizer.py/coinchooser.py user-interface 🔲
Projects
None yet
Development

No branches or pull requests

6 participants