From 07ba0e632933d638bf45729151e860aec03eb68c Mon Sep 17 00:00:00 2001 From: ThomasV Date: Fri, 18 Apr 2025 12:37:19 +0200 Subject: [PATCH 1/2] Qt: do not require password in memory - do not require full encryption - do not store password on startup - add lock/unlock functions to qt GUI --- electrum/daemon.py | 2 -- electrum/gui/qt/__init__.py | 1 - electrum/gui/qt/main_window.py | 64 +++++++++++++++++++++++----------- electrum/wallet.py | 11 +++--- 4 files changed, 49 insertions(+), 29 deletions(-) diff --git a/electrum/daemon.py b/electrum/daemon.py index caf2dc4bf..434dd7092 100644 --- a/electrum/daemon.py +++ b/electrum/daemon.py @@ -470,8 +470,6 @@ class Daemon(Logger): if wallet := self._wallets.get(wallet_key): return wallet wallet = self._load_wallet(path, password, upgrade=upgrade, config=self.config) - if wallet.requires_unlock() and password is not None: - wallet.unlock(password) wallet.start_network(self.network) self.add_wallet(wallet) self.update_recently_opened_wallets(path) diff --git a/electrum/gui/qt/__init__.py b/electrum/gui/qt/__init__.py index 4d102d0ab..891a5ba31 100644 --- a/electrum/gui/qt/__init__.py +++ b/electrum/gui/qt/__init__.py @@ -308,7 +308,6 @@ class ElectrumGui(BaseElectrumGui, Logger): self.build_tray_menu() w.warn_if_testnet() w.warn_if_watching_only() - w.require_full_encryption() return w def count_wizards_in_progress(func): diff --git a/electrum/gui/qt/main_window.py b/electrum/gui/qt/main_window.py index 1fc988190..3f87c03c7 100644 --- a/electrum/gui/qt/main_window.py +++ b/electrum/gui/qt/main_window.py @@ -141,7 +141,7 @@ def protected(func): password = None msg = kwargs.get('message') while self.wallet.has_keystore_encryption(): - password = self.password_dialog(parent=parent, msg=msg) + password = self.wallet.get_unlocked_password() or self.password_dialog(parent=parent, msg=msg) if password is None: # User cancelled password input return @@ -330,6 +330,45 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, Logger, QtEventListener): self._coroutines_scheduled[fut] = name self.need_update.set() + def toggle_lock(self): + if self.wallet.get_unlocked_password(): + self.lock_wallet() + else: + msg = ' '.join([ + _('Your wallet is locked.'), + _('If you unlock it, its password will not be required to sign transactions.'), + _('Enter your password to unlock your wallet:') + ]) + self.unlock_wallet(message=msg) + + def update_lock_menu(self): + self.lock_menu.setEnabled(self.wallet.has_password()) + text = _('Lock') if self.wallet.get_unlocked_password() else _('Unlock') + self.lock_menu.setText(text) + + @protected + def unlock_wallet(self, password, message=None): + self.wallet.unlock(password) + self.update_lock_icon() + self.update_lock_menu() + icon = read_QIcon("unlock.png") + msg = ' '.join([ + _('Your wallet is unlocked.'), + _('Its password will not be required to sign transactions.'), + ]) + self.show_message(msg, icon=icon.pixmap(30)) + + def lock_wallet(self): + self.wallet.lock_wallet() + self.update_lock_icon() + self.update_lock_menu() + icon = read_QIcon("lock.png") + msg = ' '.join([ + _('Your wallet is locked.'), + _('Its password will be required to sign transactions.'), + ]) + self.show_message(msg, icon=icon.pixmap(30)) + def on_fx_history(self): self.history_model.refresh('fx_history') self.address_list.refresh_all() @@ -580,24 +619,6 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, Logger, QtEventListener): ]) self.show_warning(msg, title=_('Watch-only wallet')) - def require_full_encryption(self): - if self.wallet.has_keystore_encryption() and not self.wallet.has_storage_encryption(): - msg = ' '.join([ - _("Your wallet is password-protected, but the wallet file is not encrypted."), - _("This is no longer supported."), - _("Please enter your password in order to encrypt your wallet file."), - ]) - while True: - password = self.password_dialog(msg) - if not password: - self.close() - raise UserCancelled() - try: - self.wallet.update_password(password, password, encrypt_storage=True) - break - except InvalidPassword as e: - self.show_error(str(e)) - def warn_if_testnet(self): if not constants.net.TESTNET: return @@ -724,6 +745,8 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, Logger, QtEventListener): self.wallet_menu.addSeparator() self.password_menu = self.wallet_menu.addAction(_("&Password"), self.change_password_dialog) + self.lock_menu = self.wallet_menu.addAction(_("&Unlock"), self.toggle_lock) + self.update_lock_menu() self.seed_menu = self.wallet_menu.addAction(_("&Seed"), self.show_seed_dialog) self.private_keys_menu = self.wallet_menu.addMenu(_("&Private keys")) self.private_keys_menu.addAction(_("&Sweep"), self.sweep_key_dialog) @@ -1864,7 +1887,7 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, Logger, QtEventListener): "Payments are more likely to succeed with a more complete graph.")) def update_lock_icon(self): - icon = read_QIcon("lock.png") if self.wallet.has_password() else read_QIcon("unlock.png") + icon = read_QIcon("lock.png") if self.wallet.has_password() and (self.wallet.get_unlocked_password() is None) else read_QIcon("unlock.png") self.password_button.setIcon(icon) def update_buttons_on_seed(self): @@ -1897,6 +1920,7 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, Logger, QtEventListener): return self._update_wallet_password( old_password=old_password, new_password=new_password, encrypt_storage=encrypt_file) + self.update_lock_menu() def _update_wallet_password(self, *, old_password, new_password, encrypt_storage: bool): try: diff --git a/electrum/wallet.py b/electrum/wallet.py index 12e2df1b5..d77430ede 100644 --- a/electrum/wallet.py +++ b/electrum/wallet.py @@ -502,9 +502,6 @@ class Abstract_Wallet(ABC, Logger, EventListener): def has_channels(self): return self.lnworker is not None and len(self.lnworker._channels) > 0 - def requires_unlock(self): - return self.config.ENABLE_ANCHOR_CHANNELS and self.has_channels() - def can_have_lightning(self) -> bool: """ whether this wallet can create new channels """ # we want static_remotekey to be a wallet address @@ -3098,9 +3095,8 @@ class Abstract_Wallet(ABC, Logger, EventListener): # save changes. force full rewrite to rm remnants of old password if self.storage and self.storage.file_exists(): self.db.write_and_force_consolidation() - # if wallet was previously unlocked, update password in memory - if self.requires_unlock(): - self.unlock(new_pw) + # if wallet was previously unlocked, reset password_in_memory + self.lock_wallet() @abstractmethod def _update_password_for_keystore(self, old_pw: Optional[str], new_pw: Optional[str]) -> None: @@ -3418,6 +3414,9 @@ class Abstract_Wallet(ABC, Logger, EventListener): self.check_password(password) self._password_in_memory = password + def lock_wallet(self): + self._password_in_memory = None + def get_unlocked_password(self): return self._password_in_memory From 7bcc7fb0c4700803a59af985be6788f670f0ca62 Mon Sep 17 00:00:00 2001 From: ThomasV Date: Wed, 23 Apr 2025 08:51:57 +0200 Subject: [PATCH 2/2] txbatcher: do not require password in memory - if password is needed, await future and request it from GUI - run asyncio task for each TxBatch, so that batches can request the password concurrently --- electrum/gui/qt/main_window.py | 31 ++++++++ electrum/lnchannel.py | 2 + electrum/transaction.py | 7 ++ electrum/txbatcher.py | 130 +++++++++++++++++++++++++-------- 4 files changed, 138 insertions(+), 32 deletions(-) diff --git a/electrum/gui/qt/main_window.py b/electrum/gui/qt/main_window.py index 3f87c03c7..315181398 100644 --- a/electrum/gui/qt/main_window.py +++ b/electrum/gui/qt/main_window.py @@ -351,6 +351,7 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, Logger, QtEventListener): self.wallet.unlock(password) self.update_lock_icon() self.update_lock_menu() + self.wallet.txbatcher.set_password_future(password) icon = read_QIcon("unlock.png") msg = ' '.join([ _('Your wallet is unlocked.'), @@ -463,6 +464,26 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, Logger, QtEventListener): self.tx_notification_queue.put(tx) self.need_update.set() + @qt_event_listener + def on_event_password_required(self, wallet): + if wallet == self.wallet: + self.password_required_button.show() + + @qt_event_listener + def on_event_password_not_required(self, wallet): + if wallet == self.wallet: + self.password_required_button.hide() + + def on_password_required_button_clicked(self): + if self.wallet.txbatcher.password_future is None: + return + txids = self.wallet.txbatcher.password_future.txids + labels = [ ' - %s ' % (self.wallet.get_label_for_txid(txid) or (txid[0:15] + '...')) for txid in txids ] + message = _('Your password is needed to sign the following transactions:') + '\n' + '\n'.join(labels) + password = self.get_password(message=message) + if password: + self.wallet.txbatcher.set_password_future(password) + @qt_event_listener def on_event_status(self): self.update_status() @@ -1800,6 +1821,15 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, Logger, QtEventListener): self.update_check_button.hide() sb.addPermanentWidget(self.update_check_button) + self.password_required_button = QPushButton(_('Password required')) + self.password_required_button.setFlat(True) + self.password_required_button.setCursor(QCursor(Qt.CursorShape.PointingHandCursor)) + self.password_required_button.setIcon(read_QIcon("warning.png")) + self.password_required_button.setIconSize(self.password_required_button.iconSize() * 1.3) + self.password_required_button.clicked.connect(self.on_password_required_button_clicked) + self.password_required_button.hide() + sb.addPermanentWidget(self.password_required_button) + self.tasks_label = QLabel('') sb.addPermanentWidget(self.tasks_label) @@ -2670,6 +2700,7 @@ class ElectrumWindow(QMainWindow, MessageBoxMixin, Logger, QtEventListener): coro_keys = list(self._coroutines_scheduled.keys()) for fut in coro_keys: fut.cancel() + self.wallet.txbatcher.set_password_future(None) self.unregister_callbacks() self.config.GUI_QT_WINDOW_IS_MAXIMIZED = self.isMaximized() self.save_notes_text() diff --git a/electrum/lnchannel.py b/electrum/lnchannel.py index b84918491..5e39e36f4 100644 --- a/electrum/lnchannel.py +++ b/electrum/lnchannel.py @@ -425,6 +425,8 @@ class AbstractChannel(Logger, ABC): conf = closing_height.conf if conf > 0: self.set_state(ChannelState.CLOSED) + if self.lnworker: + self.lnworker.wallet.txbatcher.set_password_future(None) else: # we must not trust the server with unconfirmed transactions, # because the state transition is irreversible. if the remote diff --git a/electrum/transaction.py b/electrum/transaction.py index 454d59ad8..7f3da030e 100644 --- a/electrum/transaction.py +++ b/electrum/transaction.py @@ -2120,6 +2120,13 @@ class PartialTransaction(Transaction): return tx + def requires_keystore(self): + """ + Returns True if signing will require private keys from the keystore + Called by txbatcher in order to know if a password is needed + """ + return not all(hasattr(txin, 'make_witness') for txin in self.inputs()) + @classmethod def from_io( cls, diff --git a/electrum/txbatcher.py b/electrum/txbatcher.py index e048cbadf..d556d1d56 100644 --- a/electrum/txbatcher.py +++ b/electrum/txbatcher.py @@ -6,7 +6,7 @@ from typing import Dict, Sequence from . import util from .bitcoin import dust_threshold from .logging import Logger -from .util import log_exceptions, NotEnoughFunds, BelowDustLimit, NoDynamicFeeEstimates +from .util import log_exceptions, NotEnoughFunds, BelowDustLimit, NoDynamicFeeEstimates, OldTaskGroup from .transaction import PartialTransaction, PartialTxOutput, Transaction from .address_synchronizer import TX_HEIGHT_LOCAL, TX_HEIGHT_FUTURE from .lnsweep import SweepInfo @@ -92,6 +92,8 @@ class TxBatcher(Logger): for key, item_storage in self.storage.items(): self.tx_batches[key] = TxBatch(self.wallet, item_storage) self._legacy_htlcs = {} + self.taskgroup = None + self.password_future = None @locked def add_payment_output(self, key: str, output: 'PartialTxOutput', fee_policy_descriptor: str): @@ -114,15 +116,20 @@ class TxBatcher(Logger): batch.add_sweep_input(sweep_info) def _maybe_create_new_batch(self, key, fee_policy_descriptor: str): + assert util.get_running_loop() == util.get_asyncio_loop(), f"this must be run on the asyncio thread!" if key not in self.storage: + self.logger.info(f'creating new batch: {key}') self.storage[key] = { 'fee_policy': fee_policy_descriptor, 'txids': [], 'prevout': None } - self.tx_batches[key] = TxBatch(self.wallet, self.storage[key]) + self.tx_batches[key] = batch = TxBatch(self.wallet, self.storage[key]) + if self.taskgroup: + asyncio.ensure_future(self.taskgroup.spawn(self.run_batch(key, batch))) elif self.storage[key]['fee_policy'] != fee_policy_descriptor: # maybe update policy? self.logger.warning('fee policy passed to txbatcher inconsistent with existing batch') return self.tx_batches[key] - def _delete_batch(self, key): + @locked + def delete_batch(self, key): self.logger.info(f'deleting TxBatch {key}') self.storage.pop(key) self.tx_batches.pop(key) @@ -141,24 +148,21 @@ class TxBatcher(Logger): # used to prevent GUI from interfering return bool(self.find_batch_of_txid(txid)) + async def run_batch(self, key, batch): + await batch.run() + self.delete_batch(key) + @log_exceptions async def run(self): + self.taskgroup = OldTaskGroup() + for key, batch in self.tx_batches.items(): + await self.taskgroup.spawn(self.run_batch(key, batch)) + async with self.taskgroup as group: + await group.spawn(self.redeem_legacy_htlcs()) + + async def redeem_legacy_htlcs(self): while True: await asyncio.sleep(self.SLEEP_INTERVAL) - password = self.wallet.get_unlocked_password() - if self.wallet.has_keystore_encryption() and not password: - continue - if not (self.wallet.network and self.wallet.network.is_connected()): - continue - for key, txbatch in list(self.tx_batches.items()): - try: - await txbatch.run_iteration(password) - if txbatch.is_done(): - self._delete_batch(key) - except Exception as e: - self.logger.exception(f'TxBatch error: {repr(e)}') - self._delete_batch(key) - continue for sweep_info in self._legacy_htlcs.values(): await self._maybe_redeem_legacy_htlcs(sweep_info) @@ -182,6 +186,39 @@ class TxBatcher(Logger): if await self.wallet.network.try_broadcasting(tx, sweep_info.name): self.wallet.adb.add_transaction(tx) + async def get_password(self, txid:str): + # daemon, android have password in memory + password = self.wallet.get_unlocked_password() + if password: + return password + future = self.get_password_future(txid) + try: + + await future + except asyncio.CancelledError as e: + return + password = future.result() + return password + + @locked + def set_password_future(self, password: Optional[str]): + if self.password_future is not None: + if password is not None: + self.password_future.set_result(password) + else: + self.password_future.cancel() + self.password_future = None + util.trigger_callback('password_not_required', self.wallet) + + @locked + def get_password_future(self, txid: str): + if self.password_future is None: + self.password_future = asyncio.Future() + self.password_future.txids = [] + self.logger.info(f'password required: {txid}') + self.password_future.txids.append(txid) + util.trigger_callback('password_required', self.wallet) + return self.password_future class TxBatch(Logger): @@ -201,6 +238,18 @@ class TxBatch(Logger): self._parent_tx = None self._unconfirmed_sweeps = set() # list of inputs we are sweeping (until spending tx is confirmed) + @log_exceptions + async def run(self): + while not self.is_done(): + await asyncio.sleep(self.wallet.txbatcher.SLEEP_INTERVAL) + if not (self.wallet.network and self.wallet.network.is_connected()): + continue + try: + await self.run_iteration() + except Exception as e: + self.logger.exception(f'TxBatch error: {repr(e)}') + break + def is_mine(self, txid): return txid in self._batch_txids @@ -310,8 +359,6 @@ class TxBatch(Logger): spender_txid = self.wallet.adb.db.get_spent_outpoint(prev_txid, int(index)) tx = self.wallet.adb.get_transaction(spender_txid) if tx: - tx = PartialTransaction.from_tx(tx) - tx.add_info_from_wallet(self.wallet) # this adds input amounts if spender_txid == last_txid: if self._base_tx is None: # log initialization @@ -322,7 +369,7 @@ class TxBatch(Logger): self._new_base_tx(tx) return self._base_tx - async def run_iteration(self, password): + async def run_iteration(self): conf_tx = self._find_confirmed_base_tx() if conf_tx: self.logger.info(f'base tx confirmed {conf_tx.txid()}') @@ -330,8 +377,11 @@ class TxBatch(Logger): self._start_new_batch(conf_tx) base_tx = self.find_base_tx() + if base_tx: + base_tx = PartialTransaction.from_tx(base_tx) + base_tx.add_info_from_wallet(self.wallet) # this sets is_change try: - tx = self.create_next_transaction(base_tx, password) + tx = self.create_next_transaction(base_tx) except NoDynamicFeeEstimates: self.logger.debug('no dynamic fee estimates available') return @@ -348,19 +398,38 @@ class TxBatch(Logger): # nothing to do return + # add tx to wallet, in order to reserve utxos + self.wallet.adb.add_transaction(tx) + # await password + if not await self.sign_transaction(tx): + self.wallet.adb.remove_transaction(tx.txid()) + return + if await self.wallet.network.try_broadcasting(tx, 'batch'): - self.wallet.adb.add_transaction(tx) self._new_base_tx(tx) else: # most likely reason is that base_tx is not replaceable # this may be the case if it has children (because we don't pay enough fees to replace them) # or if we are trying to sweep unconfirmed inputs (replacement-adds-unconfirmed error) - self.logger.info(f'cannot broadcast tx {tx}') + self.logger.info(f'cannot broadcast tx {tx.txid()}') + self.wallet.adb.remove_transaction(tx.txid()) if base_tx: self.logger.info(f'starting new batch because could not broadcast') self._start_new_batch(base_tx) - def create_next_transaction(self, base_tx, password): + + async def sign_transaction(self, tx): + tx.add_info_from_wallet(self.wallet) # this adds input amounts + self.add_sweep_info_to_tx(tx) + pw_required = self.wallet.has_keystore_encryption() and tx.requires_keystore() + password = await self.wallet.txbatcher.get_password(tx.txid()) if pw_required else None + if password is None and pw_required: + return + self.wallet.sign_transaction(tx, password) + assert tx.is_complete() + return tx + + def create_next_transaction(self, base_tx): to_pay = self._to_pay_after(base_tx) to_sweep = self._to_sweep_after(base_tx) to_sweep_now = {} @@ -373,15 +442,14 @@ class TxBatch(Logger): if not to_pay and not to_sweep_now and not self._should_bump_fee(base_tx): return while True: - tx = self._create_batch_tx(base_tx, to_sweep_now, to_pay, password) + tx = self._create_batch_tx(base_tx, to_sweep_now, to_pay) # 100 kb max standardness rule if tx.estimated_size() < 100_000: break to_sweep_now = to_sweep_now[0:len(to_sweep_now)//2] to_pay = to_pay[0:len(to_pay)//2] - self.logger.info(f'created tx with {len(tx.inputs())} inputs and {len(tx.outputs())} outputs') - self.logger.info(f'{str(tx)}') + self.logger.info(f'created tx {tx.txid()} with {len(tx.inputs())} inputs and {len(tx.outputs())} outputs') return tx def add_sweep_info_to_tx(self, base_tx): @@ -393,7 +461,7 @@ class TxBatch(Logger): txin.witness_script = sweep_info.txin.witness_script txin.script_sig = sweep_info.txin.script_sig - def _create_batch_tx(self, base_tx, to_sweep, to_pay, password): + def _create_batch_tx(self, base_tx, to_sweep, to_pay): self.logger.info(f'to_sweep: {list(to_sweep.keys())}') self.logger.info(f'to_pay: {to_pay}') inputs = [] @@ -411,10 +479,10 @@ class TxBatch(Logger): self.logger.info(f'locktime: {locktime}') outputs += to_pay inputs += self._create_inputs_from_tx_change(self._parent_tx) if self._parent_tx else [] - if base_tx: - self.add_sweep_info_to_tx(base_tx) # create tx + coins = self.wallet.get_spendable_coins(nonlocal_only=True) tx = self.wallet.make_unsigned_transaction( + coins=coins, fee_policy=self.fee_policy, base_tx=base_tx, inputs=inputs, @@ -423,10 +491,8 @@ class TxBatch(Logger): BIP69_sort=False, merge_duplicate_outputs=False, ) - self.wallet.sign_transaction(tx, password) # this assert will fail if we merge duplicate outputs for o in outputs: assert o in tx.outputs() - assert tx.is_complete() return tx def _clear_unconfirmed_sweeps(self, tx):