spesmilo / electrum

Electrum Bitcoin Wallet
https://electrum.org
MIT License
7.26k stars 3.04k forks source link

Remove RBF from user visible settings #8088

Closed llamasoft closed 1 year ago

llamasoft commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

On December 6, 2022 4:49:05 AM CST, ThomasV @.***> wrote:

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.

I think that's an good approach. Ironically, one that full-rbf can help. 😂

ecdsa commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

Bitrefill treats RBF differently, waits for a confirmation, but does not outright reject RBF. Though given what they are writing re: exchange rate risk, they may do so in the future.

When you say that BitPay was rejecting RBF, you mean they would actually cancel an order and give you a refund?

On December 7, 2022 8:39:13 AM CST, ThomasV @.***> wrote:

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?

-- Reply to this email directly or view it on GitHub: https://github.com/spesmilo/electrum/issues/8088#issuecomment-1341063174 You are receiving this because you commented.

Message ID: @.***>

ecdsa commented 1 year ago

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 commented 1 year ago

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 https://github.com/spesmilo/electrum/issues/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)

ecdsa commented 1 year ago

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.

pscopic commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

I did some testing recently and at the moment Bitrefill doesn't seem to accept zeroconf at all. How exactly have you been able to get it to work?

On June 18, 2023 5:34:42 AM EDT, PS Copic @.***> wrote:

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.