From d83863cc528e87c152c5b31f38cc86ab619bc72d Mon Sep 17 00:00:00 2001 From: SomberNight Date: Sun, 12 Mar 2023 00:19:39 +0000 Subject: [PATCH] qt tx dialog: add checkbox "Download input data" If checked, we download prev (parent) txs from the network, asynchronously. This allows calculating the fee and showing "input addresses". We could also SPV-verify the tx, to fill in missing tx_mined_status (block height, blockhash, timestamp, short ids), but this is not done currently. Note that there is no clean way to do this with electrum protocol 1.4: `blockchain.transaction.get_merkle(tx_hash, height)` requires knowledge of the block height. Loosely based on https://github.com/Electron-Cash/Electron-Cash/commit/6112fe0e51e48e9ceaaecf47a014e6f4a7b41703 --- electrum/gui/kivy/uix/dialogs/tx_dialog.py | 2 +- electrum/gui/qml/qetxdetails.py | 2 +- electrum/gui/qt/transaction_dialog.py | 99 ++++++++++++++++++---- electrum/transaction.py | 59 +++++++++++-- electrum/wallet.py | 2 + 5 files changed, 140 insertions(+), 24 deletions(-) diff --git a/electrum/gui/kivy/uix/dialogs/tx_dialog.py b/electrum/gui/kivy/uix/dialogs/tx_dialog.py index f61d2685b..0c8406c22 100644 --- a/electrum/gui/kivy/uix/dialogs/tx_dialog.py +++ b/electrum/gui/kivy/uix/dialogs/tx_dialog.py @@ -134,7 +134,7 @@ class TxDialog(Factory.Popup): tx.add_info_from_wallet(self.wallet) if not tx.is_complete() and tx.is_missing_info_from_network(): Network.run_from_another_thread( - tx.add_info_from_network(self.wallet.network)) # FIXME is this needed?... + tx.add_info_from_network(self.wallet.network, timeout=10)) # FIXME is this needed?... def on_open(self): self.update() diff --git a/electrum/gui/qml/qetxdetails.py b/electrum/gui/qml/qetxdetails.py index 6a3775598..54e3cab05 100644 --- a/electrum/gui/qml/qetxdetails.py +++ b/electrum/gui/qml/qetxdetails.py @@ -241,7 +241,7 @@ class QETxDetails(QObject, QtEventListener): self._tx.add_info_from_wallet(self._wallet.wallet) if not self._tx.is_complete() and self._tx.is_missing_info_from_network(): Network.run_from_another_thread( - self._tx.add_info_from_network(self._wallet.wallet.network)) # FIXME is this needed?... + self._tx.add_info_from_network(self._wallet.wallet.network, timeout=10)) # FIXME is this needed?... self._inputs = list(map(lambda x: x.to_json(), self._tx.inputs())) self._outputs = list(map(lambda x: { diff --git a/electrum/gui/qt/transaction_dialog.py b/electrum/gui/qt/transaction_dialog.py index d893165ff..1fb6ad5ce 100644 --- a/electrum/gui/qt/transaction_dialog.py +++ b/electrum/gui/qt/transaction_dialog.py @@ -23,7 +23,9 @@ # CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE # SOFTWARE. +import asyncio import sys +import concurrent.futures import copy import datetime import traceback @@ -32,7 +34,7 @@ from typing import TYPE_CHECKING, Callable, Optional, List, Union, Tuple from functools import partial from decimal import Decimal -from PyQt5.QtCore import QSize, Qt, QUrl, QPoint +from PyQt5.QtCore import QSize, Qt, QUrl, QPoint, pyqtSignal from PyQt5.QtGui import QTextCharFormat, QBrush, QFont, QPixmap, QCursor from PyQt5.QtWidgets import (QDialog, QLabel, QPushButton, QHBoxLayout, QVBoxLayout, QWidget, QGridLayout, QTextEdit, QFrame, QAction, QToolButton, QMenu, QCheckBox, QTextBrowser, QToolTip, @@ -49,8 +51,9 @@ from electrum.i18n import _ from electrum.plugin import run_hook from electrum import simple_config from electrum.transaction import SerializationError, Transaction, PartialTransaction, PartialTxInput, TxOutpoint +from electrum.transaction import TxinDataFetchProgress from electrum.logging import get_logger -from electrum.util import ShortID +from electrum.util import ShortID, get_asyncio_loop from electrum.network import Network from .util import (MessageBoxMixin, read_QIcon, Buttons, icon_path, @@ -60,6 +63,7 @@ from .util import (MessageBoxMixin, read_QIcon, Buttons, icon_path, TRANSACTION_FILE_EXTENSION_FILTER_ONLY_PARTIAL_TX, BlockingWaitingDialog, getSaveFileName, ColorSchemeItem, get_iconname_qrcode) +from .rate_limiter import rate_limited if TYPE_CHECKING: @@ -106,6 +110,11 @@ class TxInOutWidget(QWidget): self.inputs_textedit.textInteractionFlags() | Qt.LinksAccessibleByMouse | Qt.LinksAccessibleByKeyboard) self.inputs_textedit.setContextMenuPolicy(Qt.CustomContextMenu) self.inputs_textedit.customContextMenuRequested.connect(self.on_context_menu_for_inputs) + + self.inheader_hbox = QHBoxLayout() + self.inheader_hbox.setContentsMargins(0, 0, 0, 0) + self.inheader_hbox.addWidget(self.inputs_header) + self.txo_color_recv = TxOutputColoring( legend=_("Receiving Address"), color=ColorScheme.GREEN, tooltip=_("Wallet receive address")) self.txo_color_change = TxOutputColoring( @@ -130,7 +139,7 @@ class TxInOutWidget(QWidget): outheader_hbox.addWidget(self.txo_color_2fa.legend_label) vbox = QVBoxLayout() - vbox.addWidget(self.inputs_header) + vbox.addLayout(self.inheader_hbox) vbox.addWidget(self.inputs_textedit) vbox.addLayout(outheader_hbox) vbox.addWidget(self.outputs_textedit) @@ -374,6 +383,8 @@ def show_transaction(tx: Transaction, *, parent: 'ElectrumWindow', prompt_if_uns 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, external_keypairs=None): '''Transactions in the wallet will show their description. Pass desc to give a description for txs not yet in the wallet. @@ -408,6 +419,20 @@ class TxDialog(QDialog, MessageBoxMixin): self.io_widget = TxInOutWidget(self.main_window, self.wallet) vbox.addWidget(self.io_widget) + # add "fetch_txin_data" checkbox to io_widget + fetch_txin_data_cb = QCheckBox(_('Download input data')) + fetch_txin_data_cb.setChecked(bool(self.config.get('tx_dialog_fetch_txin_data', False))) + fetch_txin_data_cb.setToolTip(_('Download parent transactions from the network.\n' + 'Allows filling in missing fee and address details.')) + def on_fetch_txin_data_cb(x): + self.config.set_key('tx_dialog_fetch_txin_data', bool(x)) + if x: + self.initiate_fetch_txin_data() + fetch_txin_data_cb.stateChanged.connect(on_fetch_txin_data_cb) + self.io_widget.inheader_hbox.addStretch(1) + self.io_widget.inheader_hbox.addWidget(fetch_txin_data_cb) + self.io_widget.inheader_hbox.addStretch(10) + self.sign_button = b = QPushButton(_("Sign")) b.clicked.connect(self.sign) @@ -461,6 +486,10 @@ class TxDialog(QDialog, MessageBoxMixin): vbox.addLayout(hbox) dialogs.append(self) + self._fetch_txin_data_fut = None # type: Optional[concurrent.futures.Future] + self._fetch_txin_data_progress = None # type: Optional[TxinDataFetchProgress] + self.throttled_update_sig.connect(self._throttled_update, Qt.QueuedConnection) + self.set_tx(tx) self.update() self.set_title() @@ -479,13 +508,17 @@ class TxDialog(QDialog, MessageBoxMixin): # or that a beyond-gap-limit address is is_mine. # note: this might fetch prev txs over the network. tx.add_info_from_wallet(self.wallet) - # TODO fetch prev txs for any tx; guarded with a config key + # FIXME for PSBTs, we do a blocking fetch, as the missing data might be needed for e.g. signing + # - otherwise, the missing data is for display-completeness only, e.g. fee, input addresses (we do it async) if not tx.is_complete() and tx.is_missing_info_from_network(): BlockingWaitingDialog( self, _("Adding info to tx, from network..."), - lambda: Network.run_from_another_thread(tx.add_info_from_network(self.wallet.network)), + lambda: Network.run_from_another_thread( + tx.add_info_from_network(self.wallet.network, timeout=10)), ) + elif self.config.get('tx_dialog_fetch_txin_data', False): + self.initiate_fetch_txin_data() def do_broadcast(self): self.main_window.push_top_level_window(self) @@ -507,6 +540,9 @@ class TxDialog(QDialog, MessageBoxMixin): dialogs.remove(self) except ValueError: pass # was not in list already + if self._fetch_txin_data_fut: + self._fetch_txin_data_fut.cancel() + self._fetch_txin_data_fut = None def reject(self): # Override escape-key to close normally (and invoke closeEvent) @@ -660,6 +696,10 @@ class TxDialog(QDialog, MessageBoxMixin): return self.update() + @rate_limited(0.5, ts_after=True) + def _throttled_update(self): + self.update() + def update(self): if self.tx is None: return @@ -742,25 +782,30 @@ class TxDialog(QDialog, MessageBoxMixin): else: amount_str = _("Amount sent:") + ' %s' % format_amount(-amount) + ' ' + base_unit if fx.is_enabled(): - if tx_item_fiat: - amount_str += ' (%s)' % tx_item_fiat['fiat_value'].to_ui_string() - else: - amount_str += ' (%s)' % format_fiat_and_units(abs(amount)) + if tx_item_fiat: # historical tx -> using historical price + amount_str += ' ({})'.format(tx_item_fiat['fiat_value'].to_ui_string()) + elif tx_details.is_related_to_wallet: # probably "tx preview" -> using current price + amount_str += ' ({})'.format(format_fiat_and_units(abs(amount))) if amount_str: self.amount_label.setText(amount_str) else: self.amount_label.hide() size_str = _("Size:") + ' %d bytes'% size if fee is None: - fee_str = _("Fee") + ': ' + _("unknown") + if prog := self._fetch_txin_data_progress: + if not prog.has_errored: + fee_str = _("Downloading input data...") + f" ({prog.num_tasks_done}/{prog.num_tasks_total})" + else: + fee_str = _("Downloading input data...") + f" error." + else: + fee_str = _("Fee") + ': ' + _("unknown") else: fee_str = _("Fee") + f': {format_amount(fee)} {base_unit}' if fx.is_enabled(): - if tx_item_fiat: - fiat_fee_str = tx_item_fiat['fiat_fee'].to_ui_string() - else: - fiat_fee_str = format_fiat_and_units(fee) - fee_str += f' ({fiat_fee_str})' + if tx_item_fiat: # historical tx -> using historical price + fee_str += ' ({})'.format(tx_item_fiat['fiat_fee'].to_ui_string()) + elif tx_details.is_related_to_wallet: # probably "tx preview" -> using current price + fee_str += ' ({})'.format(format_fiat_and_units(fee)) if fee is not None: fee_rate = Decimal(fee) / size # sat/byte fee_str += ' ( %s ) ' % self.main_window.format_fee_rate(fee_rate * 1000) @@ -887,6 +932,30 @@ class TxDialog(QDialog, MessageBoxMixin): def update_fee_fields(self): pass # overridden in subclass + def initiate_fetch_txin_data(self): + """Download missing input data from the network, asynchronously. + Note: we fetch the prev txs, which allows calculating the fee and showing "input addresses". + We could also SPV-verify the tx, to fill in missing tx_mined_status (block height, blockhash, timestamp), + but this is not done currently. + """ + tx = self.tx + if not tx: + return + if self._fetch_txin_data_fut is not None: + return + network = self.wallet.network + def progress_cb(prog: TxinDataFetchProgress): + self._fetch_txin_data_progress = prog + self.throttled_update_sig.emit() + async def wrapper(): + try: + await tx.add_info_from_network(network, progress_cb=progress_cb) + finally: + self._fetch_txin_data_fut = None + + self._fetch_txin_data_progress = None + self._fetch_txin_data_fut = asyncio.run_coroutine_threadsafe(wrapper(), get_asyncio_loop()) + class TxDetailLabel(QLabel): def __init__(self, *, word_wrap=None): diff --git a/electrum/transaction.py b/electrum/transaction.py index 52f764ac6..5d6090e6b 100644 --- a/electrum/transaction.py +++ b/electrum/transaction.py @@ -91,6 +91,13 @@ class MissingTxInputAmount(Exception): pass +class TxinDataFetchProgress(NamedTuple): + num_tasks_done: int + num_tasks_total: int + has_errored: bool + has_finished: bool + + class Sighash(IntEnum): # note: this is not an IntFlag, as ALL|NONE != SINGLE @@ -361,13 +368,15 @@ class TxInput: network: Optional['Network'], *, ignore_network_issues: bool = True, - ) -> None: + timeout=None, + ) -> bool: + """Returns True iff successful.""" from .network import NetworkException async def fetch_from_network(txid) -> Optional[Transaction]: tx = None if network and network.has_internet_connection(): try: - raw_tx = await network.get_transaction(txid, timeout=10) + raw_tx = await network.get_transaction(txid, timeout=timeout) except NetworkException as e: _logger.info(f'got network error getting input txn. err: {repr(e)}. txid: {txid}. ' f'if you are intentionally offline, consider using the --offline flag') @@ -381,6 +390,7 @@ class TxInput: if self.utxo is None: self.utxo = await fetch_from_network(txid=self.prevout.txid.hex()) + return self.utxo is not None class BCDataStream(object): @@ -967,14 +977,49 @@ class Transaction: for txin in self.inputs(): wallet.add_input_info(txin) - async def add_info_from_network(self, network: Optional['Network'], *, ignore_network_issues: bool = True) -> None: + async def add_info_from_network( + self, + network: Optional['Network'], + *, + ignore_network_issues: bool = True, + progress_cb: Callable[[TxinDataFetchProgress], None] = None, + timeout=None, + ) -> None: """note: it is recommended to call add_info_from_wallet first, as this can save some network requests""" if not self.is_missing_info_from_network(): return - async with OldTaskGroup() as group: - for txin in self.inputs(): - if txin.utxo is None: - await group.spawn(txin.add_info_from_network(network=network, ignore_network_issues=ignore_network_issues)) + if progress_cb is None: + progress_cb = lambda *args, **kwargs: None + num_tasks_done = 0 + num_tasks_total = 0 + has_errored = False + has_finished = False + async def add_info_to_txin(txin: TxInput): + nonlocal num_tasks_done, has_errored + progress_cb(TxinDataFetchProgress(num_tasks_done, num_tasks_total, has_errored, has_finished)) + success = await txin.add_info_from_network( + network=network, + ignore_network_issues=ignore_network_issues, + timeout=timeout, + ) + if success: + num_tasks_done += 1 + else: + has_errored = True + progress_cb(TxinDataFetchProgress(num_tasks_done, num_tasks_total, has_errored, has_finished)) + # schedule a network task for each txin + try: + async with OldTaskGroup() as group: + for txin in self.inputs(): + if txin.utxo is None: + num_tasks_total += 1 + await group.spawn(add_info_to_txin(txin=txin)) + except Exception as e: + has_errored = True + _logger.error(f"tx.add_info_from_network() got exc: {e!r}") + finally: + has_finished = True + progress_cb(TxinDataFetchProgress(num_tasks_done, num_tasks_total, has_errored, has_finished)) def is_missing_info_from_network(self) -> bool: return any(txin.utxo is None for txin in self.inputs()) diff --git a/electrum/wallet.py b/electrum/wallet.py index 8b003a2a1..af42eba51 100644 --- a/electrum/wallet.py +++ b/electrum/wallet.py @@ -277,6 +277,7 @@ class TxWalletDetails(NamedTuple): mempool_depth_bytes: Optional[int] can_remove: bool # whether user should be allowed to delete tx is_lightning_funding_tx: bool + is_related_to_wallet: bool class Abstract_Wallet(ABC, Logger, EventListener): @@ -862,6 +863,7 @@ class Abstract_Wallet(ABC, Logger, EventListener): mempool_depth_bytes=exp_n, can_remove=can_remove, is_lightning_funding_tx=is_lightning_funding_tx, + is_related_to_wallet=is_relevant, ) def get_tx_parents(self, txid) -> Dict: