Merge pull request #10091 from f321x/reserve_input_check_make_unsigned_transaction

wallet: don't spend reserve utxo to create new reserve utxo

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') + ':',

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

electrum/wallet.py

@@ -1882,8 +1882,8 @@ class Abstract_Wallet(ABC, Logger, EventListener):
 
     def should_keep_reserve_utxo(
             self,
-            tx_inputs: List[PartialTxInput],
+            tx_inputs: Sequence[PartialTxInput],
-            tx_outputs: List[PartialTxOutput],
+            tx_outputs: Sequence[PartialTxOutput],
             is_anchor_channel_opening: bool,
     ) -> bool:
         channels_need_reserve = self.lnworker and self.lnworker.has_anchor_channels()
@@ -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, *,
@@ -2049,18 +2062,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')
+                # check if any input of the tx is == LN_UTXO_RESERVE, then we can just remove the input
-                change_addr = self.get_change_addresses_for_new_transaction(change_addr)[0]
+                reserve_sized_input = None
-                change = PartialTxOutput.from_address_and_value(change_addr, self.config.LN_UTXO_RESERVE)
+                for tx_input in tx_inputs:
-                change.is_utxo_reserve = True  # for GUI
+                    if tx_input.value_sats() and tx_input.value_sats() == self.config.LN_UTXO_RESERVE:
-                outputs.append(change)
+                        reserve_sized_input = tx_input
-                to_distribute -= change.value
+                        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)

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).