qt: improve send_change_to_lightning feedback

Improves the GUI feedback given to the user when sending change to
lightning by showing different failure messages and the additional fee
charged for the swap.
Also considers the min/max swap amount announced
by the provider when deciding to add the swap dummy output in make tx
for improved reliability.

Author: f321x

electrum/gui/qt/confirm_tx_dialog.py

@@ -410,7 +410,7 @@ def cb():
             self.resize_to_fit_content()
         self.pref_menu.addConfig(self.config.cv.GUI_QT_TX_EDITOR_SHOW_LOCKTIME, callback=cb)
         self.pref_menu.addSeparator()
-        self.pref_menu.addConfig(self.config.cv.WALLET_SEND_CHANGE_TO_LIGHTNING, callback=self.trigger_update)
+        self.pref_menu.addConfig(self.config.cv.WALLET_SEND_CHANGE_TO_LIGHTNING, callback=self.toggle_send_change_to_ln)
         self.pref_menu.addToggle(
             _('Use change addresses'),
             self.toggle_use_change,
@@ -459,6 +459,15 @@ def toggle_multiple_change(self):
         self.wallet.db.put('multiple_change', self.wallet.multiple_change)
         self.trigger_update()
 
+    def toggle_send_change_to_ln(self):
+        """Toggling send_change_to_ln needs to restart the TxDialog as make_tx depends on the
+        NostrTransport being available"""
+        self.close()
+        state = _("Enabled") if self.config.WALLET_SEND_CHANGE_TO_LIGHTNING else _("Disabled")
+        self.show_message(
+            msg=_("{} sending change to lightning.").format(state)
+        )
+
     def set_io_visible(self):
         self.io_widget.setVisible(self.config.GUI_QT_TX_EDITOR_SHOW_IO)
 
@@ -543,6 +552,21 @@ def get_messages(self):
             self.error = _('Fee estimates not available. Please set a fixed fee or feerate.')
         if self.tx.get_dummy_output(DummyAddress.SWAP):
             messages.append(_('This transaction will send funds to a submarine swap.'))
+        elif self.config.WALLET_SEND_CHANGE_TO_LIGHTNING and self.tx.has_change():
+            swap_msg = _('Will not send change to lightning: ')
+            if not self.wallet.has_lightning():
+                swap_msg += _('Lightning is not enabled.')
+            elif sum(o.value for o in self.tx.get_change_outputs()) > self.wallet.lnworker.num_sats_can_receive():
+                swap_msg += _("Your channels cannot receive this amount.")
+            elif self.wallet.lnworker.swap_manager.is_initialized.is_set():
+                # we could display the limits here but then the dialog gets very convoluted
+                swap_msg += _('Change amount outside of your swap providers limits.')
+            else:
+                # If initialization of the swap manager failed they already got an error
+                # popup before, so they should know. It might also be a channel open.
+                swap_msg = None
+            if swap_msg:
+                messages.append(swap_msg)
         # warn if spending unconf
         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.'))
@@ -707,9 +731,22 @@ def create_grid(self):
         return grid
 
     def _update_extra_fees(self):
-        x_fee = run_hook('get_tx_extra_fee', self.wallet, self.tx)
-        if x_fee:
-            x_fee_address, x_fee_amount = x_fee
+        x_fee_total = 0
+
+        # fetch plugin extra fees
+        x_fee_plugin = run_hook('get_tx_extra_fee', self.wallet, self.tx)
+        if x_fee_plugin:
+            _x_fee_address_plugin, x_fee_amount_plugin = x_fee_plugin
+            x_fee_total += x_fee_amount_plugin
+
+        # this is a forward swap (send change to lightning)
+        if dummy_output := self.tx.get_dummy_output(DummyAddress.SWAP):
+            sm = self.wallet.lnworker.swap_manager
+            ln_amount_we_recv = sm.get_recv_amount(send_amount=dummy_output.value, is_reverse=False)
+            if ln_amount_we_recv is not None:
+                x_fee_total += dummy_output.value - ln_amount_we_recv
+
+        if x_fee_total > 0:
             self.extra_fee_label.setVisible(True)
             self.extra_fee_value.setVisible(True)
-            self.extra_fee_value.setText(self.main_window.format_amount_and_units(x_fee_amount))
+            self.extra_fee_value.setText(self.main_window.format_amount_and_units(x_fee_total))

electrum/gui/qt/main_window.py

@@ -1326,7 +1326,7 @@ async def wait_until_initialized():
                 except asyncio.TimeoutError:
                     return
             try:
-                self.run_coroutine_dialog(wait_until_initialized(), _('Please wait...'))
+                self.run_coroutine_dialog(wait_until_initialized(), _('Fetching swap providers...'))
             except UserCancelled:
                 return False
             except Exception as e:
@@ -1335,6 +1335,7 @@ async def wait_until_initialized():
 
         if not sm.is_initialized.is_set():
             if not self.config.SWAPSERVER_URL:
+                assert isinstance(transport, NostrTransport)
                 if not self.choose_swapserver_dialog(transport):
                     return False
             else:

electrum/gui/qt/send_tab.py

@@ -24,7 +24,7 @@
 from electrum.payment_identifier import (PaymentIdentifierType, PaymentIdentifier,
                                          invoice_from_payment_identifier,
                                          payment_identifier_from_invoice, PaymentIdentifierState)
-from electrum.submarine_swaps import SwapServerError
+from electrum.submarine_swaps import SwapServerError, SwapServerTransport
 from electrum.fee_policy import FeePolicy, FixedFeePolicy
 from electrum.lnurl import LNURL3Data, request_lnurl_withdraw_callback, LNURLError
 
@@ -295,17 +295,31 @@ def spend_max(self):
             msg += "\n" + _("Some coins are frozen: {} (can be unfrozen in the Addresses or in the Coins tab)").format(frozen_bal)
         QToolTip.showText(self.max_button.mapToGlobal(QPoint(0, 0)), msg)
 
+    @staticmethod
+    def maybe_pass_swap_transport(func):
+        def wrapper(self, *args, **kwargs):
+            assert isinstance(self, SendTab)
+            if self.config.WALLET_SEND_CHANGE_TO_LIGHTNING and not kwargs.get('swap_transport'):
+                with self.window.create_sm_transport() as transport:
+                    if self.window.initialize_swap_manager(transport):
+                        kwargs['swap_transport'] = transport
+                        return func(self, *args, **kwargs)
+            return func(self, *args, **kwargs)
+        return wrapper
+
     # TODO: instead of passing outputs, use an invoice instead (like pay_lightning_invoice)
     # so we have more context (we cannot rely on send_tab field contents or payment identifier
     # as this method is called from other places as well).
+    @maybe_pass_swap_transport
     def pay_onchain_dialog(
             self,
             outputs: List[PartialTxOutput],
             *,
             nonlocal_only=False,
             external_keypairs: Mapping[bytes, bytes] = None,
             get_coins: Callable[..., Sequence[PartialTxInput]] = None,
-            invoice: Optional[Invoice] = None
+            invoice: Optional[Invoice] = None,
+            swap_transport: Optional['SwapServerTransport'] = None,
     ) -> None:
         # trustedcoin requires this
         if run_hook('abort_send', self):
@@ -324,7 +338,7 @@ def make_tx(fee_policy, *, confirmed_only=False, base_tx=None):
                 outputs=outputs,
                 base_tx=base_tx,
                 is_sweep=is_sweep,
-                send_change_to_lightning=self.config.WALLET_SEND_CHANGE_TO_LIGHTNING,
+                send_change_to_lightning=bool(swap_transport),
                 merge_duplicate_outputs=self.config.WALLET_MERGE_DUPLICATE_OUTPUTS,
             )
         output_values = [x.value for x in outputs]
@@ -345,22 +359,20 @@ def make_tx(fee_policy, *, confirmed_only=False, base_tx=None):
             return
 
         if swap_dummy_output := tx.get_dummy_output(DummyAddress.SWAP):
-            sm = self.wallet.lnworker.swap_manager
-            with self.window.create_sm_transport() as transport:
-                if not self.window.initialize_swap_manager(transport):
-                    return
-                coro = sm.request_swap_for_amount(transport=transport, onchain_amount=swap_dummy_output.value)
-                try:
-                    swap, swap_invoice = self.window.run_coroutine_dialog(coro, _('Requesting swap invoice...'))
-                except (SwapServerError, UserFacingException) as e:
-                    self.show_error(str(e))
-                    return
-                except UserCancelled:
-                    return
-                tx.replace_output_address(DummyAddress.SWAP, swap.lockup_address)
-                assert tx.get_dummy_output(DummyAddress.SWAP) is None
-                tx.swap_invoice = swap_invoice
-                tx.swap_payment_hash = swap.payment_hash
+            assert swap_transport and swap_transport.sm.is_initialized.is_set()
+            sm = swap_transport.sm
+            coro = sm.request_forward_swap_for_amount(transport=swap_transport, onchain_amount=swap_dummy_output.value)
+            try:
+                swap, swap_invoice = self.window.run_coroutine_dialog(coro, _('Requesting swap invoice...'))
+            except (SwapServerError, UserFacingException) as e:
+                self.show_error(str(e))
+                return
+            except UserCancelled:
+                return
+            tx.replace_output_address(DummyAddress.SWAP, swap.lockup_address)
+            assert tx.get_dummy_output(DummyAddress.SWAP) is None
+            tx.swap_invoice = swap_invoice
+            tx.swap_payment_hash = swap.payment_hash
 
         if is_preview:
             self.window.show_transaction(tx, external_keypairs=external_keypairs, invoice=invoice)

electrum/submarine_swaps.py

@@ -982,7 +982,7 @@ def create_funding_tx(
         return tx
 
     @log_exceptions
-    async def request_swap_for_amount(
+    async def request_forward_swap_for_amount(
         self,
         *,
         transport: 'SwapServerTransport',

electrum/wallet.py

@@ -2059,10 +2059,14 @@ def fee_estimator(size: Union[int, float, Decimal]) -> int:
                 dust_threshold=self.dust_threshold(),
                 BIP69_sort=BIP69_sort)
             if self.lnworker and send_change_to_lightning:
+                sm = self.lnworker.swap_manager
+                assert sm and sm.is_initialized.is_set(), "swap manager should be initialized here"
+                min_swap_amnt, max_swap_amnt = sm.get_min_amount(), sm.get_provider_max_reverse_amount()
                 change = tx.get_change_outputs()
-                if len(change) == 1:
+                assert len(change) <= 1, len(change)
+                if len(change) == 1:  # tx creates change
                     amount = change[0].value
-                    if amount <= self.lnworker.num_sats_can_receive():
+                    if min_swap_amnt <= amount <= min(self.lnworker.num_sats_can_receive(), max_swap_amnt):
                         tx.replace_output_address(change[0].address, DummyAddress.SWAP)
             if self.should_keep_reserve_utxo(tx.inputs(), tx.outputs(), is_anchor_channel_opening):
                 raise NotEnoughFunds()