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 13ab15d32..efdcf03e9 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() @@ -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') - 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) 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).