From 2f6d60c715e3c2637a5fe491d375a493ced43bba Mon Sep 17 00:00:00 2001 From: ThomasV Date: Sun, 26 Feb 2023 10:15:25 +0100 Subject: [PATCH] Move transaction related settings into transaction editor. That way, users can see the effects settings directly on their transaction. This changes the API of make_tx: - get_coins is called inside make_tx, so that inputs can be changed dynamically - make_tx takes an optional parameter: unconfirmed_only, passed to get_coins - ConfirmTxDialog detects if we can pay by disabling confirmed_only or lowering fee --- electrum/gui/qt/confirm_tx_dialog.py | 107 ++++++++++++++++++++++---- electrum/gui/qt/main_window.py | 9 +-- electrum/gui/qt/rbf_dialog.py | 6 +- electrum/gui/qt/send_tab.py | 31 ++++---- electrum/gui/qt/settings_dialog.py | 69 +---------------- electrum/gui/qt/transaction_dialog.py | 11 ++- electrum/wallet.py | 2 +- 7 files changed, 124 insertions(+), 111 deletions(-) diff --git a/electrum/gui/qt/confirm_tx_dialog.py b/electrum/gui/qt/confirm_tx_dialog.py index b928cc70a..98be69853 100644 --- a/electrum/gui/qt/confirm_tx_dialog.py +++ b/electrum/gui/qt/confirm_tx_dialog.py @@ -374,12 +374,58 @@ class TxEditor(WindowModalDialog): def create_top_bar(self, text): self.pref_menu = QMenu() - self.m1 = self.pref_menu.addAction('Show inputs/outputs', self.toggle_io_visibility) - self.m1.setCheckable(True) - self.m2 = self.pref_menu.addAction('Edit fees', self.toggle_fee_details) - self.m2.setCheckable(True) - self.m3 = self.pref_menu.addAction('Edit Locktime', self.toggle_locktime) - self.m3.setCheckable(True) + self.pref_menu.setToolTipsVisible(True) + def add_pref_action(b, action, text, tooltip): + m = self.pref_menu.addAction(text, action) + m.setCheckable(True) + m.setChecked(b) + m.setToolTip(tooltip) + return m + add_pref_action( + self.config.get('show_tx_io', False), + self.toggle_io_visibility, + _('Show inputs and outputs'), '') + add_pref_action( + self.config.get('show_tx_fee_details', False), + self.toggle_fee_details, + _('Edit fees manually'), '') + add_pref_action( + self.config.get('show_tx_locktime', False), + self.toggle_locktime, + _('Edit Locktime'), '') + self.pref_menu.addSeparator() + add_pref_action( + self.wallet.use_change, + self.toggle_use_change, + _('Use change addresses'), + _('Using change addresses makes it more difficult for other people to track your transactions.')) + self.use_multi_change_menu = add_pref_action( + self.wallet.multiple_change, self.toggle_multiple_change, + _('Use multiple change addresses',), + '\n'.join([ + _('In some cases, use up to 3 change addresses in order to break ' + 'up large coin amounts and obfuscate the recipient address.'), + _('This may result in higher transactions fees.') + ])) + self.use_multi_change_menu.setEnabled(self.wallet.use_change) + add_pref_action( + self.config.get('batch_rbf', False), + self.toggle_batch_rbf, + _('Batch unconfirmed transactions'), + _('If you check this box, your unconfirmed transactions will be consolidated into a single transaction.') + '\n' + \ + _('This will save fees, but might have unwanted effects in terms of privacy')) + add_pref_action( + self.config.get('confirmed_only', False), + self.toggle_confirmed_only, + _('Spend only confirmed coins'), + _('Spend only confirmed inputs.')) + add_pref_action( + self.config.get('coin_chooser_output_rounding', True), + self.toggle_confirmed_only, + _('Enable output value rounding'), + _('Set the value of the change output so that it has similar precision to the other outputs.') + '\n' + \ + _('This might improve your privacy somewhat.') + '\n' + \ + _('If enabled, at most 100 satoshis might be lost due to this, per transaction.')) self.pref_button = QToolButton() self.pref_button.setIcon(read_QIcon("preferences.png")) self.pref_button.setMenu(self.pref_menu) @@ -397,6 +443,27 @@ class TxEditor(WindowModalDialog): self.resize(size) self.resize(size) + def toggle_use_change(self): + self.wallet.use_change = not self.wallet.use_change + self.wallet.db.put('use_change', self.wallet.use_change) + self.use_multi_change_menu.setEnabled(self.wallet.use_change) + self.trigger_update() + + def toggle_multiple_change(self): + self.wallet.multiple_change = not self.wallet.multiple_change + self.wallet.db.put('multiple_change', self.wallet.multiple_change) + self.trigger_update() + + def toggle_batch_rbf(self): + b = not self.config.get('batch_rbf', False) + self.config.set_key('batch_rbf', b) + self.trigger_update() + + def toggle_confirmed_only(self): + b = not self.config.get('confirmed_only', False) + self.config.set_key('confirmed_only', b) + self.trigger_update() + def toggle_io_visibility(self): b = not self.config.get('show_tx_io', False) self.config.set_key('show_tx_io', b) @@ -417,7 +484,6 @@ class TxEditor(WindowModalDialog): def set_io_visible(self, b): self.io_widget.setVisible(b) - self.m1.setChecked(b) def set_fee_edit_visible(self, b): detailed = [self.feerounding_icon, self.feerate_e, self.fee_e] @@ -427,14 +493,12 @@ class TxEditor(WindowModalDialog): w.hide() for w in (detailed if b else basic): w.show() - self.m2.setChecked(b) def set_locktime_visible(self, b): for w in [ self.locktime_e, self.locktime_label]: w.setVisible(b) - self.m3.setChecked(b) def run(self): cancelled = not self.exec_() @@ -452,8 +516,19 @@ class TxEditor(WindowModalDialog): def _update_widgets(self): self._update_amount_label() if self.not_enough_funds: - self.error = self.main_window.send_tab.get_text_not_enough_funds_mentioning_frozen() + self.error = _('Not enough funds.') + confirmed_only = self.config.get('confirmed_only', False) + if confirmed_only and self.can_pay_assuming_zero_fees(confirmed_only=False): + self.error += ' ' + _('Change your settings to allow spending unconfirmed coins.') + elif self.can_pay_assuming_zero_fees(confirmed_only=confirmed_only): + self.error += ' ' + _('You need to set a lower fee.') + else: + self.error += '' + else: + self.error = '' if not self.tx: + if self.not_enough_funds: + self.io_widget.update(None) self.set_feerounding_visibility(False) else: self.check_tx_fee_warning() @@ -467,7 +542,6 @@ class TxEditor(WindowModalDialog): self._update_send_button() self._update_message() - def check_tx_fee_warning(self): # side effects: self.error, self.message fee = self.tx.get_fee() @@ -538,8 +612,9 @@ class ConfirmTxDialog(TxEditor): def update_tx(self, *, fallback_to_zero_fee: bool = False): fee_estimator = self.get_fee_estimator() + confirmed_only = self.config.get('confirmed_only', False) try: - self.tx = self.make_tx(fee_estimator) + self.tx = self.make_tx(fee_estimator, confirmed_only=confirmed_only) self.not_enough_funds = False self.no_dynfee_estimates = False error = '' @@ -549,7 +624,7 @@ class ConfirmTxDialog(TxEditor): self.tx = None if fallback_to_zero_fee: try: - self.tx = self.make_tx(0) + self.tx = self.make_tx(0, confirmed_only=confirmed_only) except BaseException: return else: @@ -558,7 +633,7 @@ class ConfirmTxDialog(TxEditor): self.no_dynfee_estimates = True self.tx = None try: - self.tx = self.make_tx(0) + self.tx = self.make_tx(0, confirmed_only=confirmed_only) except NotEnoughFunds: self.not_enough_funds = True return @@ -570,10 +645,10 @@ class ConfirmTxDialog(TxEditor): raise self.tx.set_rbf(True) - def have_enough_funds_assuming_zero_fees(self) -> bool: + def can_pay_assuming_zero_fees(self, confirmed_only) -> bool: # called in send_tab.py try: - tx = self.make_tx(0) + tx = self.make_tx(0, confirmed_only=confirmed_only) except NotEnoughFunds: return False else: diff --git a/electrum/gui/qt/main_window.py b/electrum/gui/qt/main_window.py index f71d09f82..9393f2f93 100644 --- a/electrum/gui/qt/main_window.py +++ b/electrum/gui/qt/main_window.py @@ -1192,12 +1192,12 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, Logger, QtEventListener): else: self.show_error(_('Payment failed') + '\n\n' + reason) - def get_coins(self, *, nonlocal_only=False) -> Sequence[PartialTxInput]: + def get_coins(self, **kwargs) -> Sequence[PartialTxInput]: coins = self.get_manually_selected_coins() if coins is not None: return coins else: - return self.wallet.get_spendable_coins(None, nonlocal_only=nonlocal_only) + return self.wallet.get_spendable_coins(None, **kwargs) def get_manually_selected_coins(self) -> Optional[Sequence[PartialTxInput]]: """Return a list of selected coins or None. @@ -1242,9 +1242,8 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, Logger, QtEventListener): WaitingDialog(self, msg, task, on_success, on_failure) def mktx_for_open_channel(self, *, funding_sat, node_id): - coins = self.get_coins(nonlocal_only=True) - make_tx = lambda fee_est: self.wallet.lnworker.mktx_for_open_channel( - coins=coins, + make_tx = lambda fee_est, *, confirmed_only=False: self.wallet.lnworker.mktx_for_open_channel( + coins = self.get_coins(nonlocal_only=True, confirmed_only=confirmed_only), funding_sat=funding_sat, node_id=node_id, fee_est=fee_est) diff --git a/electrum/gui/qt/rbf_dialog.py b/electrum/gui/qt/rbf_dialog.py index d3ec31a89..f0de304a9 100644 --- a/electrum/gui/qt/rbf_dialog.py +++ b/electrum/gui/qt/rbf_dialog.py @@ -153,12 +153,12 @@ class BumpFeeDialog(_BaseRBFDialog): txid=txid, title=_('Bump Fee')) - def rbf_func(self, fee_rate): + def rbf_func(self, fee_rate, *, confirmed_only=False): return self.wallet.bump_fee( tx=self.old_tx, txid=self.old_txid, new_fee_rate=fee_rate, - coins=self.main_window.get_coins(), + coins=self.main_window.get_coins(nonlocal_only=True, confirmed_only=confirmed_only), decrease_payment=self.is_decrease_payment()) @@ -183,5 +183,5 @@ class DSCancelDialog(_BaseRBFDialog): self.method_label.setVisible(False) self.method_combo.setVisible(False) - def rbf_func(self, fee_rate): + def rbf_func(self, fee_rate, *, confirmed_only=False): return self.wallet.dscancel(tx=self.old_tx, new_fee_rate=fee_rate) diff --git a/electrum/gui/qt/send_tab.py b/electrum/gui/qt/send_tab.py index 4145295b1..99dcbc015 100644 --- a/electrum/gui/qt/send_tab.py +++ b/electrum/gui/qt/send_tab.py @@ -179,12 +179,11 @@ class SendTab(QWidget, MessageBoxMixin, Logger): outputs = self.payto_e.get_outputs(True) if not outputs: return - make_tx = lambda fee_est: self.wallet.make_unsigned_transaction( + make_tx = lambda fee_est, *, confirmed_only=False: self.wallet.make_unsigned_transaction( coins=self.window.get_coins(), outputs=outputs, fee=fee_est, is_sweep=False) - try: try: tx = make_tx(None) @@ -216,32 +215,30 @@ class SendTab(QWidget, MessageBoxMixin, Logger): QToolTip.showText(self.max_button.mapToGlobal(QPoint(0, 0)), msg) def pay_onchain_dialog( - self, inputs: Sequence[PartialTxInput], + self, outputs: List[PartialTxOutput], *, + nonlocal_only=False, external_keypairs=None) -> None: # trustedcoin requires this if run_hook('abort_send', self): return is_sweep = bool(external_keypairs) - make_tx = lambda fee_est: self.wallet.make_unsigned_transaction( - coins=inputs, + # we call get_coins inside make_tx, so that inputs can be changed dynamically + make_tx = lambda fee_est, *, confirmed_only=False: self.wallet.make_unsigned_transaction( + coins=self.window.get_coins(nonlocal_only=nonlocal_only, confirmed_only=confirmed_only), outputs=outputs, fee=fee_est, is_sweep=is_sweep) output_values = [x.value for x in outputs] - if any(parse_max_spend(outval) for outval in output_values): - output_value = '!' - else: - output_value = sum(output_values) + is_max = any(parse_max_spend(outval) for outval in output_values) + output_value = '!' if is_max else sum(output_values) conf_dlg = ConfirmTxDialog(window=self.window, make_tx=make_tx, output_value=output_value) if conf_dlg.not_enough_funds: - # Check if we had enough funds excluding fees, - # if so, still provide opportunity to set lower fees. - if not conf_dlg.have_enough_funds_assuming_zero_fees(): + confirmed_only = self.config.get('confirmed_only', False) + if not conf_dlg.can_pay_assuming_zero_fees(confirmed_only=False): text = self.get_text_not_enough_funds_mentioning_frozen() self.show_message(text) return - tx = conf_dlg.run() if tx is None: # user cancelled @@ -250,14 +247,12 @@ class SendTab(QWidget, MessageBoxMixin, Logger): if is_preview: self.window.show_transaction(tx) return - self.save_pending_invoice() def sign_done(success): if success: self.window.broadcast_or_show(tx) else: raise - self.window.sign_tx( tx, callback=sign_done, @@ -564,13 +559,13 @@ class SendTab(QWidget, MessageBoxMixin, Logger): outputs = [] for invoice in invoices: outputs += invoice.outputs - self.pay_onchain_dialog(self.window.get_coins(), outputs) + self.pay_onchain_dialog(outputs) def do_pay_invoice(self, invoice: 'Invoice'): if invoice.is_lightning(): self.pay_lightning_invoice(invoice) else: - self.pay_onchain_dialog(self.window.get_coins(), invoice.outputs) + self.pay_onchain_dialog(invoice.outputs) def read_outputs(self) -> List[PartialTxOutput]: if self.payment_request: @@ -695,7 +690,7 @@ class SendTab(QWidget, MessageBoxMixin, Logger): chan, swap_recv_amount_sat = can_pay_with_swap self.window.run_swap_dialog(is_reverse=False, recv_amount_sat=swap_recv_amount_sat, channels=[chan]) elif r == 3: - self.pay_onchain_dialog(coins, invoice.get_outputs()) + self.pay_onchain_dialog(invoice.get_outputs(), nonlocal_only=True) return assert lnworker is not None diff --git a/electrum/gui/qt/settings_dialog.py b/electrum/gui/qt/settings_dialog.py index e247e3bcb..1dad08386 100644 --- a/electrum/gui/qt/settings_dialog.py +++ b/electrum/gui/qt/settings_dialog.py @@ -119,15 +119,6 @@ class SettingsDialog(QDialog, QtEventListener): self.config.set_key('bip21_lightning', bool(x)) bip21_lightning_cb.stateChanged.connect(on_bip21_lightning) - batch_rbf_cb = QCheckBox(_('Batch unconfirmed transactions')) - batch_rbf_cb.setChecked(bool(self.config.get('batch_rbf', False))) - batch_rbf_cb.setToolTip( - _('If you check this box, your unconfirmed transactions will be consolidated into a single transaction.') + '\n' + \ - _('This will save fees.')) - def on_batch_rbf(x): - self.config.set_key('batch_rbf', bool(x)) - batch_rbf_cb.stateChanged.connect(on_batch_rbf) - # lightning help_recov = _(messages.MSG_RECOVERABLE_CHANNELS) recov_cb = QCheckBox(_("Create recoverable channels")) @@ -276,33 +267,6 @@ class SettingsDialog(QDialog, QtEventListener): filelogging_cb.stateChanged.connect(on_set_filelogging) filelogging_cb.setToolTip(_('Debug logs can be persisted to disk. These are useful for troubleshooting.')) - usechange_cb = QCheckBox(_('Use change addresses')) - usechange_cb.setChecked(self.wallet.use_change) - if not self.config.is_modifiable('use_change'): usechange_cb.setEnabled(False) - def on_usechange(x): - usechange_result = x == Qt.Checked - if self.wallet.use_change != usechange_result: - self.wallet.use_change = usechange_result - self.wallet.db.put('use_change', self.wallet.use_change) - multiple_cb.setEnabled(self.wallet.use_change) - usechange_cb.stateChanged.connect(on_usechange) - usechange_cb.setToolTip(_('Using change addresses makes it more difficult for other people to track your transactions.')) - - def on_multiple(x): - multiple = x == Qt.Checked - if self.wallet.multiple_change != multiple: - self.wallet.multiple_change = multiple - self.wallet.db.put('multiple_change', multiple) - multiple_change = self.wallet.multiple_change - multiple_cb = QCheckBox(_('Use multiple change addresses')) - multiple_cb.setEnabled(self.wallet.use_change) - multiple_cb.setToolTip('\n'.join([ - _('In some cases, use up to 3 change addresses in order to break ' - 'up large coin amounts and obfuscate the recipient address.'), - _('This may result in higher transactions fees.') - ])) - multiple_cb.setChecked(multiple_change) - multiple_cb.stateChanged.connect(on_multiple) def fmt_docs(key, klass): lines = [ln.lstrip(" ") for ln in klass.__doc__.split("\n")] @@ -323,25 +287,6 @@ class SettingsDialog(QDialog, QtEventListener): self.config.set_key('coin_chooser', chooser_name) chooser_combo.currentIndexChanged.connect(on_chooser) - def on_unconf(x): - self.config.set_key('confirmed_only', bool(x)) - conf_only = bool(self.config.get('confirmed_only', False)) - unconf_cb = QCheckBox(_('Spend only confirmed coins')) - unconf_cb.setToolTip(_('Spend only confirmed inputs.')) - unconf_cb.setChecked(conf_only) - unconf_cb.stateChanged.connect(on_unconf) - - def on_outrounding(x): - self.config.set_key('coin_chooser_output_rounding', bool(x)) - enable_outrounding = bool(self.config.get('coin_chooser_output_rounding', True)) - outrounding_cb = QCheckBox(_('Enable output value rounding')) - outrounding_cb.setToolTip( - _('Set the value of the change output so that it has similar precision to the other outputs.') + '\n' + - _('This might improve your privacy somewhat.') + '\n' + - _('If enabled, at most 100 satoshis might be lost due to this, per transaction.')) - outrounding_cb.setChecked(enable_outrounding) - outrounding_cb.stateChanged.connect(on_outrounding) - block_explorers = sorted(util.block_explorer_info().keys()) BLOCK_EX_CUSTOM_ITEM = _("Custom URL") if BLOCK_EX_CUSTOM_ITEM in block_explorers: # malicious translation? @@ -484,15 +429,7 @@ class SettingsDialog(QDialog, QtEventListener): invoices_widgets = [] invoices_widgets.append((bolt11_fallback_cb, None)) invoices_widgets.append((bip21_lightning_cb, None)) - tx_widgets = [] - tx_widgets.append((usechange_cb, None)) - tx_widgets.append((batch_rbf_cb, None)) - tx_widgets.append((unconf_cb, None)) - tx_widgets.append((multiple_cb, None)) - tx_widgets.append((outrounding_cb, None)) - if len(choosers) > 1: - tx_widgets.append((chooser_label, chooser_combo)) - tx_widgets.append((block_ex_label, block_ex_hbox_w)) + lightning_widgets = [] lightning_widgets.append((recov_cb, None)) lightning_widgets.append((trampoline_cb, None)) @@ -509,10 +446,12 @@ class SettingsDialog(QDialog, QtEventListener): misc_widgets.append((filelogging_cb, None)) misc_widgets.append((alias_label, self.alias_e)) misc_widgets.append((qr_label, qr_combo)) + misc_widgets.append((block_ex_label, block_ex_hbox_w)) + if len(choosers) > 1: + misc_widgets.append((chooser_label, chooser_combo)) tabs_info = [ (gui_widgets, _('Appearance')), - (tx_widgets, _('Transactions')), (invoices_widgets, _('Invoices')), (lightning_widgets, _('Lightning')), (fiat_widgets, _('Fiat')), diff --git a/electrum/gui/qt/transaction_dialog.py b/electrum/gui/qt/transaction_dialog.py index 0c6ee566b..e0a57fa33 100644 --- a/electrum/gui/qt/transaction_dialog.py +++ b/electrum/gui/qt/transaction_dialog.py @@ -132,12 +132,17 @@ class TxInOutWidget(QWidget): vbox.addWidget(self.outputs_textedit) self.setLayout(vbox) - def update(self, tx): + def update(self, tx: Optional[Transaction]): self.tx = tx - inputs_header_text = _("Inputs") + ' (%d)'%len(self.tx.inputs()) + if tx is None: + self.inputs_header.setText('') + self.inputs_textedit.setText('') + self.outputs_header.setText('') + self.outputs_textedit.setText('') + return + inputs_header_text = _("Inputs") + ' (%d)'%len(self.tx.inputs()) self.inputs_header.setText(inputs_header_text) - ext = QTextCharFormat() # "external" lnk = QTextCharFormat() lnk.setToolTip(_('Click to open, right-click for menu')) diff --git a/electrum/wallet.py b/electrum/wallet.py index 9b0681589..48b6f6e37 100644 --- a/electrum/wallet.py +++ b/electrum/wallet.py @@ -926,8 +926,8 @@ class Abstract_Wallet(ABC, Logger, EventListener): domain: Optional[Iterable[str]] = None, *, nonlocal_only: bool = False, + confirmed_only: bool = False, ) -> Sequence[PartialTxInput]: - confirmed_only = self.config.get('confirmed_only', False) with self._freeze_lock: frozen_addresses = self._frozen_addresses.copy() utxos = self.get_utxos(