Browse Source

qt: kind of fix bip70 notify_merchant logic by passing around PI

```
229.18 | E | gui.qt.main_window.[test_segwit_2] | on_error
Traceback (most recent call last):
  File "...\electrum\gui\qt\util.py", line 917, in run
    result = task.task()
  File "...\electrum\gui\qt\send_tab.py", line 681, in broadcast_thread
    if self.payto_e.payment_identifier.has_expired():
AttributeError: 'NoneType' object has no attribute 'has_expired'
```

In SendTab.broadcast_transaction.broadcast_thread, self.payto_e.payment_identifier was referenced -
but do_clear() has already cleared it by then.
E.g. consider SendTab.pay_onchain_dialog: it calls save_pending_invoice(), which calls do_clear(),
and later (in sign_done), it calls window.broadcast_or_show, which will call SendTab.broadcast_transaction().

As there might be multiple independent transaction dialogs open simultaneously, the single shared state
send_tab.payto_e.payment_identifier approach was problematic -- I think it is conceptually nicer to
pass around the payment_identifiers as needed, as done with this change.

However, this change is not a full proper fix, as it still somewhat relies on
send_tab.payto_e.payment_identifier (e.g. in pay_onchain_dialog). Hence, e.g. when using
the invoice_list context menu "Pay..." item, as payto_e.payment_identifier is not set,
payment_identifier will be None in broadcast_transaction.

but at least we handle PI being None gracefully -- before this change, broadcast_transaction
expected PI to be set, and it was never set to the correct thing (as do_clear() already ran by then):
depending on timing it was either None or a new empty PI. In the former case, producing the above
traceback and hard failing (not only for bip70 stuff!), and in the latter, silently ignoring the logic bug.
master
SomberNight 3 years ago
parent
commit
bb8c73cabd
No known key found for this signature in database
GPG Key ID: B33B5F232C6271E9
  1. 22
      electrum/gui/qt/main_window.py
  2. 4
      electrum/gui/qt/paytoedit.py
  3. 26
      electrum/gui/qt/send_tab.py
  4. 23
      electrum/gui/qt/transaction_dialog.py

22
electrum/gui/qt/main_window.py

@ -1062,8 +1062,14 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, Logger, QtEventListener):
from .channel_details import ChannelDetailsDialog
ChannelDetailsDialog(self, chan).show()
def show_transaction(self, tx: Transaction, *, external_keypairs=None):
show_transaction(tx, parent=self, external_keypairs=external_keypairs)
def show_transaction(
self,
tx: Transaction,
*,
external_keypairs=None,
payment_identifier: PaymentIdentifier = None,
):
show_transaction(tx, parent=self, external_keypairs=external_keypairs, payment_identifier=payment_identifier)
def show_lightning_transaction(self, tx_item):
from .lightning_tx_dialog import LightningTxDialog
@ -1208,18 +1214,18 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, Logger, QtEventListener):
"""
return self.utxo_list.get_spend_list()
def broadcast_or_show(self, tx: Transaction):
def broadcast_or_show(self, tx: Transaction, *, payment_identifier: PaymentIdentifier = None):
if not tx.is_complete():
self.show_transaction(tx)
self.show_transaction(tx, payment_identifier=payment_identifier)
return
if not self.network:
self.show_error(_("You can't broadcast a transaction without a live network connection."))
self.show_transaction(tx)
self.show_transaction(tx, payment_identifier=payment_identifier)
return
self.broadcast_transaction(tx)
self.broadcast_transaction(tx, payment_identifier=payment_identifier)
def broadcast_transaction(self, tx: Transaction):
self.send_tab.broadcast_transaction(tx)
def broadcast_transaction(self, tx: Transaction, *, payment_identifier: PaymentIdentifier = None):
self.send_tab.broadcast_transaction(tx, payment_identifier=payment_identifier)
@protected
def sign_tx(self, tx, *, callback, external_keypairs, password):

4
electrum/gui/qt/paytoedit.py

@ -173,7 +173,7 @@ class PayToEdit(QWidget, Logger, GenericInputHandler):
self.edit_timer.setInterval(1000)
self.edit_timer.timeout.connect(self._on_edit_timer)
self.payment_identifier = None
self.payment_identifier = None # type: Optional[PaymentIdentifier]
@property
def multiline(self):
@ -206,8 +206,8 @@ class PayToEdit(QWidget, Logger, GenericInputHandler):
self.line_edit.setToolTip(tt)
self.text_edit.setToolTip(tt)
'''set payment identifier only if valid, else exception'''
def try_payment_identifier(self, text) -> None:
'''set payment identifier only if valid, else exception'''
text = text.strip()
pi = PaymentIdentifier(self.send_tab.wallet, text)
if not pi.is_valid():

26
electrum/gui/qt/send_tab.py

@ -17,7 +17,7 @@ from electrum.util import NotEnoughFunds, NoDynamicFeeEstimates, parse_max_spend
from electrum.invoices import PR_PAID, Invoice, PR_BROADCASTING, PR_BROADCAST
from electrum.transaction import Transaction, PartialTxInput, PartialTxOutput
from electrum.network import TxBroadcastError, BestEffortRequestFailed
from electrum.payment_identifier import PaymentIdentifierState, PaymentIdentifierType
from electrum.payment_identifier import PaymentIdentifierState, PaymentIdentifierType, PaymentIdentifier
from .amountedit import AmountEdit, BTCAmountEdit, SizedFreezableLineEdit
from .paytoedit import InvalidPaymentIdentifier
@ -268,7 +268,8 @@ class SendTab(QWidget, MessageBoxMixin, Logger):
def pay_onchain_dialog(
self,
outputs: List[PartialTxOutput], *,
outputs: List[PartialTxOutput],
*,
nonlocal_only=False,
external_keypairs=None,
get_coins: Callable[..., Sequence[PartialTxInput]] = None,
@ -276,6 +277,9 @@ class SendTab(QWidget, MessageBoxMixin, Logger):
# trustedcoin requires this
if run_hook('abort_send', self):
return
# save current PI as local now. this is best-effort only...
# does not work e.g. when using InvoiceList context menu "pay"
payment_identifier = self.payto_e.payment_identifier
is_sweep = bool(external_keypairs)
# we call get_coins inside make_tx, so that inputs can be changed dynamically
if get_coins is None:
@ -302,12 +306,12 @@ class SendTab(QWidget, MessageBoxMixin, Logger):
return
is_preview = conf_dlg.is_preview
if is_preview:
self.window.show_transaction(tx, external_keypairs=external_keypairs)
self.window.show_transaction(tx, external_keypairs=external_keypairs, payment_identifier=payment_identifier)
return
self.save_pending_invoice()
def sign_done(success):
if success:
self.window.broadcast_or_show(tx)
self.window.broadcast_or_show(tx, payment_identifier=payment_identifier)
self.window.sign_tx(
tx,
callback=sign_done,
@ -527,7 +531,7 @@ class SendTab(QWidget, MessageBoxMixin, Logger):
outputs += invoice.outputs
self.pay_onchain_dialog(outputs)
def do_edit_invoice(self, invoice: 'Invoice'):
def do_edit_invoice(self, invoice: 'Invoice'): # FIXME broken
assert not bool(invoice.get_amount_sat())
text = invoice.lightning_invoice if invoice.is_lightning() else invoice.get_address()
self.payto_e._on_input_btn(text)
@ -674,11 +678,13 @@ class SendTab(QWidget, MessageBoxMixin, Logger):
coro = lnworker.pay_invoice(invoice.lightning_invoice, amount_msat=amount_msat)
self.window.run_coroutine_from_thread(coro, _('Sending payment'))
def broadcast_transaction(self, tx: Transaction):
def broadcast_transaction(self, tx: Transaction, *, payment_identifier: PaymentIdentifier = None):
# note: payment_identifier is explicitly passed as self.payto_e.payment_identifier might
# already be cleared or otherwise have changed.
def broadcast_thread():
# non-GUI thread
if self.payto_e.payment_identifier.has_expired():
if payment_identifier and payment_identifier.has_expired():
return False, _("Invoice has expired")
try:
self.network.run_from_another_thread(self.network.broadcast_transaction(tx))
@ -688,9 +694,9 @@ class SendTab(QWidget, MessageBoxMixin, Logger):
return False, repr(e)
# success
txid = tx.txid()
if self.payto_e.payment_identifier.need_merchant_notify():
if payment_identifier and payment_identifier.need_merchant_notify():
refund_address = self.wallet.get_receiving_address()
self.payto_e.payment_identifier.notify_merchant(
payment_identifier.notify_merchant(
tx=tx,
refund_address=refund_address,
on_finished=self.notify_merchant_done_signal.emit
@ -718,7 +724,7 @@ class SendTab(QWidget, MessageBoxMixin, Logger):
WaitingDialog(self, _('Broadcasting transaction...'),
broadcast_thread, broadcast_done, self.window.on_error)
def on_notify_merchant_done(self, pi):
def on_notify_merchant_done(self, pi: PaymentIdentifier):
if pi.is_error():
self.logger.debug(f'merchant notify error: {pi.get_error()}')
else:

23
electrum/gui/qt/transaction_dialog.py

@ -70,6 +70,7 @@ from .my_treeview import create_toolbar_with_menu
if TYPE_CHECKING:
from .main_window import ElectrumWindow
from electrum.wallet import Abstract_Wallet
from electrum.payment_identifier import PaymentIdentifier
_logger = get_logger(__name__)
@ -378,9 +379,16 @@ def show_transaction(
parent: 'ElectrumWindow',
prompt_if_unsaved: bool = False,
external_keypairs=None,
payment_identifier: 'PaymentIdentifier' = None,
):
try:
d = TxDialog(tx, parent=parent, prompt_if_unsaved=prompt_if_unsaved, external_keypairs=external_keypairs)
d = TxDialog(
tx,
parent=parent,
prompt_if_unsaved=prompt_if_unsaved,
external_keypairs=external_keypairs,
payment_identifier=payment_identifier,
)
except SerializationError as e:
_logger.exception('unable to deserialize the transaction')
parent.show_critical(_("Electrum was unable to deserialize the transaction:") + "\n" + str(e))
@ -392,7 +400,15 @@ class TxDialog(QDialog, MessageBoxMixin):
throttled_update_sig = pyqtSignal() # emit from thread to do update in main thread
def __init__(self, tx: Transaction, *, parent: 'ElectrumWindow', prompt_if_unsaved: bool, external_keypairs=None):
def __init__(
self,
tx: Transaction,
*,
parent: 'ElectrumWindow',
prompt_if_unsaved: bool,
external_keypairs=None,
payment_identifier: 'PaymentIdentifier' = None,
):
'''Transactions in the wallet will show their description.
Pass desc to give a description for txs not yet in the wallet.
'''
@ -403,6 +419,7 @@ class TxDialog(QDialog, MessageBoxMixin):
self.main_window = parent
self.config = parent.config
self.wallet = parent.wallet
self.payment_identifier = payment_identifier
self.prompt_if_unsaved = prompt_if_unsaved
self.saved = False
self.desc = None
@ -537,7 +554,7 @@ class TxDialog(QDialog, MessageBoxMixin):
self.main_window.push_top_level_window(self)
self.main_window.send_tab.save_pending_invoice()
try:
self.main_window.broadcast_transaction(self.tx)
self.main_window.broadcast_transaction(self.tx, payment_identifier=self.payment_identifier)
finally:
self.main_window.pop_top_level_window(self)
self.saved = True

Loading…
Cancel
Save