Browse Source

Set the RBF flat to all transactions, and remove the 'use_rbf'

preference from the GUI, because the mempoolfullrbf option in
Bitcoin 0.24 makes RBF signaling pretty meaningless. Fixes #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.
master
ThomasV 3 years ago
parent
commit
e1dc7d1e6f
  1. 2
      electrum/commands.py
  2. 18
      electrum/gui/kivy/uix/dialogs/confirm_tx_dialog.py
  3. 2
      electrum/gui/kivy/uix/screens.py
  4. 11
      electrum/gui/qml/components/Preferences.qml
  5. 10
      electrum/gui/qml/qeconfig.py
  6. 4
      electrum/gui/qml/qewallet.py
  7. 3
      electrum/gui/qt/confirm_tx_dialog.py
  8. 15
      electrum/gui/qt/rbf_dialog.py
  9. 16
      electrum/gui/qt/settings_dialog.py
  10. 8
      electrum/gui/qt/transaction_dialog.py
  11. 14
      electrum/wallet.py

2
electrum/commands.py

@ -675,7 +675,7 @@ class Commands:
@command('wp') @command('wp')
async def payto(self, destination, amount, fee=None, feerate=None, from_addr=None, from_coins=None, change_addr=None, async def payto(self, destination, amount, fee=None, feerate=None, from_addr=None, from_coins=None, change_addr=None,
nocheck=False, unsigned=False, rbf=None, password=None, locktime=None, addtransaction=False, wallet: Abstract_Wallet = None): nocheck=False, unsigned=False, rbf=True, password=None, locktime=None, addtransaction=False, wallet: Abstract_Wallet = None):
"""Create a transaction. """ """Create a transaction. """
self.nocheck = nocheck self.nocheck = nocheck
tx_fee = satoshis(fee) tx_fee = satoshis(fee)

18
electrum/gui/kivy/uix/dialogs/confirm_tx_dialog.py

@ -26,7 +26,6 @@ Builder.load_string('''
message: '' message: ''
warning: '' warning: ''
extra_fee: '' extra_fee: ''
show_final: False
size_hint: 0.8, 0.8 size_hint: 0.8, 0.8
pos_hint: {'top':0.9} pos_hint: {'top':0.9}
method: 0 method: 0
@ -82,17 +81,6 @@ Builder.load_string('''
range: 0, 4 range: 0, 4
step: 1 step: 1
on_value: root.on_slider(self.value) on_value: root.on_slider(self.value)
BoxLayout:
orientation: 'horizontal'
size_hint: 1, 0.2
Label:
text: _('Final')
opacity: int(root.show_final)
CheckBox:
id: final_cb
opacity: int(root.show_final)
disabled: not root.show_final
on_release: root.update_tx()
Label: Label:
text: root.warning text: root.warning
text_size: self.width, None text_size: self.width, None
@ -122,7 +110,7 @@ Builder.load_string('''
class ConfirmTxDialog(FeeSliderDialog, Factory.Popup): class ConfirmTxDialog(FeeSliderDialog, Factory.Popup):
def __init__(self, app: 'ElectrumWindow', amount, make_tx, on_pay, *, show_final=True): def __init__(self, app: 'ElectrumWindow', amount, make_tx, on_pay):
Factory.Popup.__init__(self) Factory.Popup.__init__(self)
FeeSliderDialog.__init__(self, app.electrum_config, self.ids.slider) FeeSliderDialog.__init__(self, app.electrum_config, self.ids.slider)
@ -130,16 +118,14 @@ class ConfirmTxDialog(FeeSliderDialog, Factory.Popup):
self.amount = amount self.amount = amount
self.make_tx = make_tx self.make_tx = make_tx
self.on_pay = on_pay self.on_pay = on_pay
self.show_final = show_final
self.update_slider() self.update_slider()
self.update_text() self.update_text()
self.update_tx() self.update_tx()
def update_tx(self): def update_tx(self):
rbf = not bool(self.ids.final_cb.active) if self.show_final else False
try: try:
# make unsigned transaction # make unsigned transaction
tx = self.make_tx(rbf) tx = self.make_tx()
except NotEnoughFunds: except NotEnoughFunds:
self.warning = _("Not enough funds") self.warning = _("Not enough funds")
self.ids.ok_button.disabled = True self.ids.ok_button.disabled = True

2
electrum/gui/kivy/uix/screens.py

@ -438,7 +438,7 @@ class SendScreen(CScreen, Logger):
outputs = invoice.outputs outputs = invoice.outputs
amount = sum(map(lambda x: x.value, outputs)) if not any(parse_max_spend(x.value) for x in outputs) else '!' amount = sum(map(lambda x: x.value, outputs)) if not any(parse_max_spend(x.value) for x in outputs) else '!'
coins = self.app.wallet.get_spendable_coins(None) coins = self.app.wallet.get_spendable_coins(None)
make_tx = lambda rbf: self.app.wallet.make_unsigned_transaction(coins=coins, outputs=outputs, rbf=rbf) make_tx = lambda: self.app.wallet.make_unsigned_transaction(coins=coins, outputs=outputs)
on_pay = lambda tx: self.app.protected(_('Send payment?'), self.send_tx, (tx, invoice)) on_pay = lambda tx: self.app.protected(_('Send payment?'), self.send_tx, (tx, invoice))
d = ConfirmTxDialog(self.app, amount=amount, make_tx=make_tx, on_pay=on_pay) d = ConfirmTxDialog(self.app, amount=amount, make_tx=make_tx, on_pay=on_pay)
d.open() d.open()

11
electrum/gui/qml/components/Preferences.qml

@ -130,16 +130,6 @@ Pane {
} }
} }
Switch {
id: useRbf
text: qsTr('Use Replace-By-Fee')
Layout.columnSpan: 2
onCheckedChanged: {
if (activeFocus)
Config.useRbf = checked
}
}
Label { Label {
text: qsTr('Default request expiry') text: qsTr('Default request expiry')
Layout.fillWidth: false Layout.fillWidth: false
@ -269,7 +259,6 @@ Pane {
lnRoutingType.currentIndex = Config.useGossip ? 0 : 1 lnRoutingType.currentIndex = Config.useGossip ? 0 : 1
useFallbackAddress.checked = Config.useFallbackAddress useFallbackAddress.checked = Config.useFallbackAddress
enableDebugLogs.checked = Config.enableDebugLogs enableDebugLogs.checked = Config.enableDebugLogs
useRbf.checked = Config.useRbf
useRecoverableChannels.checked = Config.useRecoverableChannels useRecoverableChannels.checked = Config.useRecoverableChannels
} }
} }

10
electrum/gui/qml/qeconfig.py

@ -146,16 +146,6 @@ class QEConfig(AuthMixin, QObject):
self.config.set_key('gui_enable_debug_logs', enable) self.config.set_key('gui_enable_debug_logs', enable)
self.enableDebugLogsChanged.emit() self.enableDebugLogsChanged.emit()
useRbfChanged = pyqtSignal()
@pyqtProperty(bool, notify=useRbfChanged)
def useRbf(self):
return self.config.get('use_rbf', True)
@useRbf.setter
def useRbf(self, useRbf):
self.config.set_key('use_rbf', useRbf)
self.useRbfChanged.emit()
useRecoverableChannelsChanged = pyqtSignal() useRecoverableChannelsChanged = pyqtSignal()
@pyqtProperty(bool, notify=useRecoverableChannelsChanged) @pyqtProperty(bool, notify=useRecoverableChannelsChanged)
def useRecoverableChannels(self): def useRecoverableChannels(self):

4
electrum/gui/qml/qewallet.py

@ -455,9 +455,7 @@ class QEWallet(AuthMixin, QObject, QtEventListener):
# see qt/confirm_tx_dialog qt/main_window # see qt/confirm_tx_dialog qt/main_window
tx = self.wallet.make_unsigned_transaction(coins=coins,outputs=outputs, fee=None) tx = self.wallet.make_unsigned_transaction(coins=coins,outputs=outputs, fee=None)
self._logger.info(str(tx.to_json())) self._logger.info(str(tx.to_json()))
tx.set_rbf(True)
use_rbf = bool(self.wallet.config.get('use_rbf', True))
tx.set_rbf(use_rbf)
self.sign(tx, broadcast=True) self.sign(tx, broadcast=True)
@auth_protect @auth_protect

3
electrum/gui/qt/confirm_tx_dialog.py

@ -113,8 +113,7 @@ class TxEditor:
self.tx = None self.tx = None
self.main_window.show_error(str(e)) self.main_window.show_error(str(e))
raise raise
use_rbf = bool(self.config.get('use_rbf', True)) self.tx.set_rbf(True)
self.tx.set_rbf(use_rbf)
def have_enough_funds_assuming_zero_fees(self) -> bool: def have_enough_funds_assuming_zero_fees(self) -> bool:
try: try:

15
electrum/gui/qt/rbf_dialog.py

@ -47,6 +47,8 @@ class _BaseRBFDialog(WindowModalDialog):
ok_button = OkButton(self) ok_button = OkButton(self)
self.adv_button = QPushButton(_("Show advanced settings")) self.adv_button = QPushButton(_("Show advanced settings"))
self.adv_button.setEnabled(False)
self.adv_button.setVisible(False)
warning_label = WWLabel('\n') warning_label = WWLabel('\n')
warning_label.setStyleSheet(ColorScheme.RED.as_stylesheet()) warning_label.setStyleSheet(ColorScheme.RED.as_stylesheet())
self.feerate_e = FeerateEdit(lambda: 0) self.feerate_e = FeerateEdit(lambda: 0)
@ -115,21 +117,18 @@ class _BaseRBFDialog(WindowModalDialog):
vbox.addWidget(adv_widget) vbox.addWidget(adv_widget)
def _add_advanced_options(self, adv_vbox: QVBoxLayout) -> None: def _add_advanced_options(self, adv_vbox: QVBoxLayout) -> None:
self.cb_rbf = QCheckBox(_('Keep Replace-By-Fee enabled')) pass
self.cb_rbf.setChecked(True)
adv_vbox.addWidget(self.cb_rbf)
def run(self) -> None: def run(self) -> None:
if not self.exec_(): if not self.exec_():
return return
is_rbf = self.cb_rbf.isChecked()
new_fee_rate = self.feerate_e.get_amount() new_fee_rate = self.feerate_e.get_amount()
try: try:
new_tx = self.rbf_func(new_fee_rate) new_tx = self.rbf_func(new_fee_rate)
except Exception as e: except Exception as e:
self.window.show_error(str(e)) self.window.show_error(str(e))
return return
new_tx.set_rbf(is_rbf) new_tx.set_rbf(True)
tx_label = self.wallet.get_label_for_txid(self.txid) tx_label = self.wallet.get_label_for_txid(self.txid)
self.window.show_transaction(new_tx, tx_desc=tx_label) self.window.show_transaction(new_tx, tx_desc=tx_label)
# TODO maybe save tx_label as label for new tx?? # TODO maybe save tx_label as label for new tx??
@ -164,10 +163,8 @@ class BumpFeeDialog(_BaseRBFDialog):
) )
def _add_advanced_options(self, adv_vbox: QVBoxLayout) -> None: def _add_advanced_options(self, adv_vbox: QVBoxLayout) -> None:
self.cb_rbf = QCheckBox(_('Keep Replace-By-Fee enabled')) self.adv_button.setVisible(True)
self.cb_rbf.setChecked(True) self.adv_button.setEnabled(True)
adv_vbox.addWidget(self.cb_rbf)
self.strat_combo = QComboBox() self.strat_combo = QComboBox()
options = [ options = [
_("decrease change, or add new inputs, or decrease any outputs"), _("decrease change, or add new inputs, or decrease any outputs"),

16
electrum/gui/qt/settings_dialog.py

@ -118,21 +118,8 @@ class SettingsDialog(QDialog, QtEventListener):
self.config.set_key('bip21_lightning', bool(x)) self.config.set_key('bip21_lightning', bool(x))
bip21_lightning_cb.stateChanged.connect(on_bip21_lightning) bip21_lightning_cb.stateChanged.connect(on_bip21_lightning)
use_rbf = bool(self.config.get('use_rbf', True)) batch_rbf_cb = QCheckBox(_('Batch unconfirmed transactions'))
use_rbf_cb = QCheckBox(_('Use Replace-By-Fee'))
use_rbf_cb.setChecked(use_rbf)
use_rbf_cb.setToolTip(
_('If you check this box, your transactions will be marked as non-final,') + '\n' + \
_('and you will have the possibility, while they are unconfirmed, to replace them with transactions that pay higher fees.') + '\n' + \
_('Note that some merchants do not accept non-final transactions until they are confirmed.'))
def on_use_rbf(x):
self.config.set_key('use_rbf', bool(x))
batch_rbf_cb.setEnabled(bool(x))
use_rbf_cb.stateChanged.connect(on_use_rbf)
batch_rbf_cb = QCheckBox(_('Batch RBF transactions'))
batch_rbf_cb.setChecked(bool(self.config.get('batch_rbf', False))) batch_rbf_cb.setChecked(bool(self.config.get('batch_rbf', False)))
batch_rbf_cb.setEnabled(use_rbf)
batch_rbf_cb.setToolTip( batch_rbf_cb.setToolTip(
_('If you check this box, your unconfirmed transactions will be consolidated into a single transaction.') + '\n' + \ _('If you check this box, your unconfirmed transactions will be consolidated into a single transaction.') + '\n' + \
_('This will save fees.')) _('This will save fees.'))
@ -505,7 +492,6 @@ class SettingsDialog(QDialog, QtEventListener):
invoices_widgets.append((bip21_lightning_cb, None)) invoices_widgets.append((bip21_lightning_cb, None))
tx_widgets = [] tx_widgets = []
tx_widgets.append((usechange_cb, None)) tx_widgets.append((usechange_cb, None))
tx_widgets.append((use_rbf_cb, None))
tx_widgets.append((batch_rbf_cb, None)) tx_widgets.append((batch_rbf_cb, None))
tx_widgets.append((preview_cb, None)) tx_widgets.append((preview_cb, None))
tx_widgets.append((unconf_cb, None)) tx_widgets.append((unconf_cb, None))

8
electrum/gui/qt/transaction_dialog.py

@ -680,9 +680,6 @@ class BaseTxDialog(QDialog, MessageBoxMixin):
vbox_right.addWidget(self.size_label) vbox_right.addWidget(self.size_label)
self.rbf_label = TxDetailLabel() self.rbf_label = TxDetailLabel()
vbox_right.addWidget(self.rbf_label) vbox_right.addWidget(self.rbf_label)
self.rbf_cb = QCheckBox(_('Replace by fee'))
self.rbf_cb.setChecked(bool(self.config.get('use_rbf', True)))
vbox_right.addWidget(self.rbf_cb)
self.locktime_final_label = TxDetailLabel() self.locktime_final_label = TxDetailLabel()
vbox_right.addWidget(self.locktime_final_label) vbox_right.addWidget(self.locktime_final_label)
@ -713,7 +710,6 @@ class BaseTxDialog(QDialog, MessageBoxMixin):
# set visibility after parenting can be determined by Qt # set visibility after parenting can be determined by Qt
self.rbf_label.setVisible(self.finalized) self.rbf_label.setVisible(self.finalized)
self.rbf_cb.setVisible(not self.finalized)
self.locktime_final_label.setVisible(self.finalized) self.locktime_final_label.setVisible(self.finalized)
self.locktime_setter_widget.setVisible(not self.finalized) self.locktime_setter_widget.setVisible(not self.finalized)
@ -982,11 +978,11 @@ class PreviewTxDialog(BaseTxDialog, TxEditor):
assert self.tx assert self.tx
self.finalized = True self.finalized = True
self.stop_editor_updates() self.stop_editor_updates()
self.tx.set_rbf(self.rbf_cb.isChecked()) self.tx.set_rbf(True)
locktime = self.locktime_e.get_locktime() locktime = self.locktime_e.get_locktime()
if locktime is not None: if locktime is not None:
self.tx.locktime = locktime self.tx.locktime = locktime
for widget in [self.fee_slider, self.fee_combo, self.feecontrol_fields, self.rbf_cb, for widget in [self.fee_slider, self.fee_combo, self.feecontrol_fields,
self.locktime_setter_widget, self.locktime_e]: self.locktime_setter_widget, self.locktime_e]:
widget.setEnabled(False) widget.setEnabled(False)
widget.setVisible(False) widget.setVisible(False)

14
electrum/wallet.py

@ -204,8 +204,7 @@ async def sweep(
locktime = get_locktime_for_new_transaction(network) locktime = get_locktime_for_new_transaction(network)
tx = PartialTransaction.from_io(inputs, outputs, locktime=locktime, version=tx_version) tx = PartialTransaction.from_io(inputs, outputs, locktime=locktime, version=tx_version)
rbf = bool(config.get('use_rbf', True)) tx.set_rbf(True)
tx.set_rbf(rbf)
tx.sign(keypairs) tx.sign(keypairs)
return tx return tx
@ -1420,8 +1419,6 @@ class Abstract_Wallet(ABC, Logger, EventListener):
if not tx: if not tx:
return 2, 'unknown' return 2, 'unknown'
is_final = tx and tx.is_final() is_final = tx and tx.is_final()
if not is_final:
extra.append('rbf')
fee = self.adb.get_tx_fee(tx_hash) fee = self.adb.get_tx_fee(tx_hash)
if fee is not None: if fee is not None:
size = tx.estimated_size() size = tx.estimated_size()
@ -1569,7 +1566,7 @@ class Abstract_Wallet(ABC, Logger, EventListener):
fee=None, fee=None,
change_addr: str = None, change_addr: str = None,
is_sweep=False, is_sweep=False,
rbf=False) -> PartialTransaction: rbf=True) -> PartialTransaction:
"""Can raise NotEnoughFunds or NoDynamicFeeEstimates.""" """Can raise NotEnoughFunds or NoDynamicFeeEstimates."""
if not coins: # any bitcoin tx must have at least 1 input by consensus if not coins: # any bitcoin tx must have at least 1 input by consensus
@ -1668,7 +1665,6 @@ class Abstract_Wallet(ABC, Logger, EventListener):
# Timelock tx to current height. # Timelock tx to current height.
tx.locktime = get_locktime_for_new_transaction(self.network) tx.locktime = get_locktime_for_new_transaction(self.network)
tx.set_rbf(rbf) tx.set_rbf(rbf)
tx.add_info_from_wallet(self) tx.add_info_from_wallet(self)
run_hook('make_unsigned_transaction', self, tx) run_hook('make_unsigned_transaction', self, tx)
@ -1677,7 +1673,7 @@ class Abstract_Wallet(ABC, Logger, EventListener):
def mktx(self, *, def mktx(self, *,
outputs: List[PartialTxOutput], outputs: List[PartialTxOutput],
password=None, fee=None, change_addr=None, password=None, fee=None, change_addr=None,
domain=None, rbf=False, nonlocal_only=False, domain=None, rbf=True, nonlocal_only=False,
tx_version=None, sign=True) -> PartialTransaction: tx_version=None, sign=True) -> PartialTransaction:
coins = self.get_spendable_coins(domain, nonlocal_only=nonlocal_only) coins = self.get_spendable_coins(domain, nonlocal_only=nonlocal_only)
tx = self.make_unsigned_transaction( tx = self.make_unsigned_transaction(
@ -2726,7 +2722,7 @@ class Abstract_Wallet(ABC, Logger, EventListener):
pass pass
def create_transaction(self, outputs, *, fee=None, feerate=None, change_addr=None, domain_addr=None, domain_coins=None, def create_transaction(self, outputs, *, fee=None, feerate=None, change_addr=None, domain_addr=None, domain_coins=None,
unsigned=False, rbf=None, password=None, locktime=None): unsigned=False, rbf=True, password=None, locktime=None):
if fee is not None and feerate is not None: if fee is not None and feerate is not None:
raise Exception("Cannot specify both 'fee' and 'feerate' at the same time!") raise Exception("Cannot specify both 'fee' and 'feerate' at the same time!")
coins = self.get_spendable_coins(domain_addr) coins = self.get_spendable_coins(domain_addr)
@ -2744,8 +2740,6 @@ class Abstract_Wallet(ABC, Logger, EventListener):
change_addr=change_addr) change_addr=change_addr)
if locktime is not None: if locktime is not None:
tx.locktime = locktime tx.locktime = locktime
if rbf is None:
rbf = bool(self.config.get('use_rbf', True))
tx.set_rbf(rbf) tx.set_rbf(rbf)
if not unsigned: if not unsigned:
self.sign_transaction(tx, password) self.sign_transaction(tx, password)

Loading…
Cancel
Save