From ead2c7c8de9dbf78969723fb48f3f3e4a0616421 Mon Sep 17 00:00:00 2001 From: f321x Date: Tue, 29 Jul 2025 11:46:25 +0200 Subject: [PATCH 1/3] wallet: don't add reserve input if reserve utxo exists Right now if a ln reserve is required and there is already a reserve sized utxo available, `make_unsigned_transaction()` will still add the reserve as input to a 'spend max' transaction and add a reserve change output. This seems wasteful, this patch instead just removes the input so it is not spent at all. --- electrum/wallet.py | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/electrum/wallet.py b/electrum/wallet.py index 13ab15d32..37c05e742 100644 --- a/electrum/wallet.py +++ b/electrum/wallet.py @@ -1882,8 +1882,8 @@ class Abstract_Wallet(ABC, Logger, EventListener): def should_keep_reserve_utxo( self, - tx_inputs: List[PartialTxInput], - tx_outputs: List[PartialTxOutput], + tx_inputs: Sequence[PartialTxInput], + tx_outputs: Sequence[PartialTxOutput], is_anchor_channel_opening: bool, ) -> bool: channels_need_reserve = self.lnworker and self.lnworker.has_anchor_channels() @@ -2049,18 +2049,32 @@ class Abstract_Wallet(ABC, Logger, EventListener): tx = PartialTransaction.from_io(list(tx_inputs), list(outputs)) fee = fee_estimator(tx.estimated_size()) - input_amount = sum(c.value_sats() for c in tx_inputs) + input_amount = sum(c.value_sats() for c in tx_inputs) # may change if reserve is needed allocated_amount = sum(o.value for o in outputs if not parse_max_spend(o.value)) to_distribute = input_amount - allocated_amount distribute_amount(to_distribute - fee) if self.should_keep_reserve_utxo(tx_inputs, outputs, is_anchor_channel_opening): - self.logger.info(f'Adding change output to meet utxo reserve requirements') - change_addr = self.get_change_addresses_for_new_transaction(change_addr)[0] - change = PartialTxOutput.from_address_and_value(change_addr, self.config.LN_UTXO_RESERVE) - change.is_utxo_reserve = True # for GUI - outputs.append(change) - to_distribute -= change.value + # check if any input of the tx is == LN_UTXO_RESERVE, then we can just remove the input + reserve_sized_input = None + for tx_input in tx_inputs: + if tx_input.value_sats() and tx_input.value_sats() == self.config.LN_UTXO_RESERVE: + reserve_sized_input = tx_input + break + + if reserve_sized_input: + self.logger.debug(f'Removing LN_UTXO_RESERVE sized input to keep utxo reserve') + tx_inputs.remove(reserve_sized_input) + to_distribute -= reserve_sized_input.value_sats() + else: + self.logger.info(f'Adding change output to meet utxo reserve requirements') + change_addr = self.get_change_addresses_for_new_transaction(change_addr)[0] + change = PartialTxOutput.from_address_and_value(change_addr, self.config.LN_UTXO_RESERVE) + change.is_utxo_reserve = True # for GUI + outputs.append(change) + to_distribute -= change.value + + assert not self.should_keep_reserve_utxo(tx_inputs, outputs, is_anchor_channel_opening) tx = PartialTransaction.from_io(list(tx_inputs), list(outputs)) fee = fee_estimator(tx.estimated_size()) distribute_amount(to_distribute - fee) From 30b146033d273223ca13ba653f040ce856052205 Mon Sep 17 00:00:00 2001 From: f321x Date: Tue, 29 Jul 2025 14:36:55 +0200 Subject: [PATCH 2/3] gui: detect if reserve inputs have been removed from tx Adapts the gui(s) to detect if an existing reserve input has not been used as input for the transaction even though the user tried to spend max. This allows to show the lightning reserve warning not only for reserve change outputs but also for existing reserve inputs that have been ignored for this max spend transaction. Still keeps the `is_utxo_reserve` flag on `PartialTxOutput` as it is used in the qml gui. --- electrum/gui/qml/qetxfinalizer.py | 5 ++++- electrum/gui/qt/confirm_tx_dialog.py | 2 +- electrum/wallet.py | 13 +++++++++++++ 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/electrum/gui/qml/qetxfinalizer.py b/electrum/gui/qml/qetxfinalizer.py index bafe4a6c4..1b378d608 100644 --- a/electrum/gui/qml/qetxfinalizer.py +++ b/electrum/gui/qml/qetxfinalizer.py @@ -434,7 +434,10 @@ class QETxFinalizer(TxFeeSlider): self.update_fee_warning_from_tx(tx=tx, invoice_amt=amount) if self._amount.isMax and not self.warning: - if reserve_sats := sum(txo.value for txo in tx.outputs() if txo.is_utxo_reserve): + if reserve_sats := self._wallet.wallet.tx_keeps_ln_utxo_reserve( + tx, + gui_spend_max=self._amount.isMax + ): reserve_str = self._config.format_amount_and_units(reserve_sats) self.warning = ' '.join([ _('Warning') + ':', diff --git a/electrum/gui/qt/confirm_tx_dialog.py b/electrum/gui/qt/confirm_tx_dialog.py index b18f8332b..6ab873331 100644 --- a/electrum/gui/qt/confirm_tx_dialog.py +++ b/electrum/gui/qt/confirm_tx_dialog.py @@ -544,7 +544,7 @@ class TxEditor(WindowModalDialog): if any((txin.block_height is not None and txin.block_height<=0) for txin in self.tx.inputs()): messages.append(_('This transaction will spend unconfirmed coins.')) # warn if a reserve utxo was added - if reserve_sats := sum(txo.value for txo in self.tx.outputs() if txo.is_utxo_reserve): + if reserve_sats := self.wallet.tx_keeps_ln_utxo_reserve(self.tx, gui_spend_max=bool(self.output_value == '!')): reserve_str = self.main_window.config.format_amount_and_units(reserve_sats) messages.append(_('Could not spend max: a security reserve of {} was kept for your Lightning channels.').format(reserve_str)) # warn if we merge from mempool diff --git a/electrum/wallet.py b/electrum/wallet.py index 37c05e742..efdcf03e9 100644 --- a/electrum/wallet.py +++ b/electrum/wallet.py @@ -1911,6 +1911,19 @@ class Abstract_Wallet(ABC, Logger, EventListener): def is_low_reserve(self) -> bool: return self.should_keep_reserve_utxo([], [], False) + def tx_keeps_ln_utxo_reserve(self, tx, *, gui_spend_max: bool) -> Optional[int]: + if reserve_output_amount := sum(txo.value for txo in tx.outputs() if txo.is_utxo_reserve): + # tx has a reserve change output + return reserve_output_amount + if gui_spend_max: # user tried to spend max amount + coins_in_wallet = self.get_spendable_coins(nonlocal_only=False, confirmed_only=False) + amount_in_wallet = sum(c.value_sats() for c in coins_in_wallet) + tx_spend_amount = tx.output_value() + tx.get_fee() + if amount_in_wallet - tx_spend_amount == self.config.LN_UTXO_RESERVE: + # tx keeps exactly LN_UTXO_RESERVE amount sats in the wallet + return self.config.LN_UTXO_RESERVE + return None + @profiler(min_threshold=0.1) def make_unsigned_transaction( self, *, From 23a48e7cfd5d742a33b21d086bcc59253c627283 Mon Sep 17 00:00:00 2001 From: f321x Date: Tue, 29 Jul 2025 15:31:57 +0200 Subject: [PATCH 3/3] test: add unittest for not spending ln reserve utxo adds unittest to `test_wallet_vertical.py` to verify it is not using the existing ln reserve utxo as tx input if a reserve is still required (in opposite to using it as input and creating a new reserve as output). --- tests/test_wallet_vertical.py | 44 +++++++++++++++++++++++++++++++++-- 1 file changed, 42 insertions(+), 2 deletions(-) diff --git a/tests/test_wallet_vertical.py b/tests/test_wallet_vertical.py index 15e7c04c0..aa53b0886 100644 --- a/tests/test_wallet_vertical.py +++ b/tests/test_wallet_vertical.py @@ -2060,9 +2060,10 @@ class TestWalletSending(ElectrumTestCase): fee_sats = 1000 outputs = [PartialTxOutput.from_address_and_value(outgoing_address, balance - fee_sats)] def make_tx(b): + wallet.lnworker = mock.Mock() + wallet.lnworker.has_anchor_channels.return_value = b return wallet.make_unsigned_transaction( outputs = outputs, - is_anchor_channel_opening = b, fee_policy = FixedFeePolicy(fee_sats), ) tx = make_tx(False) @@ -2075,9 +2076,10 @@ class TestWalletSending(ElectrumTestCase): wallet, outgoing_address, to_self_address = self._create_cause_carbon_wallet() def make_tx(address): outputs = [PartialTxOutput.from_address_and_value(address, '!')] + wallet.lnworker = mock.Mock() + wallet.lnworker.has_anchor_channels.return_value = True return wallet.make_unsigned_transaction( outputs = outputs, - is_anchor_channel_opening = True, fee_policy = FixedFeePolicy(100), ) tx = make_tx(outgoing_address) @@ -2085,6 +2087,44 @@ class TestWalletSending(ElectrumTestCase): tx = make_tx(to_self_address) self.assertEqual(1, len(tx.outputs())) + async def test_ln_reserve_keep_existing_reserve(self): + """ + tests if make_unsigned_transaction keeps the existing reserve utxo + instead of creating a new one + """ + wallet1, outgoing_address1, to_self_address1 = self._create_cause_carbon_wallet() + wallet2 = self.create_standard_wallet_from_seed('cycle rocket west magnet parrot shuffle foot correct salt library feed song') + wallet2_addr = wallet2.get_receiving_addresses()[0] + def make_tx(address, wallet): + outputs = [PartialTxOutput.from_address_and_value(address, '!')] + wallet.lnworker = mock.Mock() + wallet.lnworker.has_anchor_channels.return_value = True + return wallet.make_unsigned_transaction( + outputs = outputs, + fee_policy = FixedFeePolicy(100), + ) + # send ! from wallet1 to outgoing address so wallet1 has exactly one reserve utxo + tx = make_tx(wallet2_addr, wallet1) + self.assertEqual(2, len(tx.outputs())) + wallet1.sign_transaction(tx, password=None) + wallet1.adb.receive_tx_callback(tx, tx_height=1000000) + wallet2.adb.receive_tx_callback(tx, tx_height=1000000) + assert sum(utxo.value_sats() for utxo in wallet1.get_spendable_coins()) == self.config.LN_UTXO_RESERVE + + # send funds back to wallet1, so wallet1 is able to do a max spend again + tx = make_tx(to_self_address1, wallet2) + wallet2.sign_transaction(tx, password=None) + wallet1.adb.receive_tx_callback(tx, tx_height=1000100) + + # now there is a reserve UTXO of config.LN_UTXO_RESERVE sat in wallet1, so wallet1 should + # not add it as input to the tx + assert len(wallet1.get_spendable_coins()) > 1, f"{len(wallet1.get_spendable_coins())=}" + tx = make_tx(outgoing_address1, wallet1) + self.assertEqual(1, len(tx.outputs())) + wallet1.adb.receive_tx_callback(tx, tx_height=1000200) + assert len(wallet1.get_spendable_coins()) == 1, f"{len(wallet1.get_spendable_coins())=}" + assert wallet1.get_spendable_coins()[0].value_sats() == self.config.LN_UTXO_RESERVE + async def test_rbf_batching__cannot_batch_as_would_need_to_use_ismine_outputs_of_basetx(self): """Wallet history contains unconf tx1 that spends all its coins to two ismine outputs, one 'recv' address (20k sats) and one 'change' (80k sats).