diff --git a/electrum/lnpeer.py b/electrum/lnpeer.py index c29c37526..f2020a2cb 100644 --- a/electrum/lnpeer.py +++ b/electrum/lnpeer.py @@ -2731,6 +2731,15 @@ class Peer(Logger): except OnionRoutingFailure as e: assert len(self.lnworker.active_forwardings[payment_key]) == 0 self.lnworker.save_forwarding_failure(payment_key, failure_message=e) + # TODO what about other errors? e.g. TxBroadcastError for a swap. + # - malicious electrum server could fake TxBroadcastError + # Could we "catch-all Exception" and fail back the htlcs with e.g. TEMPORARY_NODE_FAILURE? + # - we don't want to fail the inc-HTLC for a syntax error that happens in the callback + # If we don't call save_forwarding_failure(), the inc-HTLC gets stuck until expiry + # and then the inc-channel will get force-closed. + # => forwarding_callback() could have an API with two exceptions types: + # - type1, such as OnionRoutingFailure, that signals we need to fail back the inc-HTLC + # - type2, such as TxBroadcastError, that signals we want to retry the callback # add to list assert len(self.lnworker.active_forwardings.get(payment_key, [])) == 0 self.lnworker.active_forwardings[payment_key] = [] diff --git a/electrum/network.py b/electrum/network.py index e18dd463d..9a990a087 100644 --- a/electrum/network.py +++ b/electrum/network.py @@ -993,6 +993,7 @@ class Network(Logger, NetworkRetryManager[ServerAddr]): @best_effort_reliable async def broadcast_transaction(self, tx: 'Transaction', *, timeout=None) -> None: + """caller should handle TxBroadcastError""" if self.interface is None: # handled by best_effort_reliable raise RequestTimedOut() if timeout is None: diff --git a/electrum/submarine_swaps.py b/electrum/submarine_swaps.py index 448e8da54..0c4e2aeaa 100644 --- a/electrum/submarine_swaps.py +++ b/electrum/submarine_swaps.py @@ -30,7 +30,7 @@ from .i18n import _ from .bitcoin import construct_script from .crypto import ripemd from .invoices import Invoice -from .network import TxBroadcastServerReturnedError +from .network import TxBroadcastError from .lnonion import OnionRoutingFailure, OnionFailureCode @@ -312,7 +312,7 @@ class SwapManager(Logger): tx = self.lnwatcher.adb.get_transaction(txin.spent_txid) try: await self.network.broadcast_transaction(tx) - except TxBroadcastServerReturnedError: + except TxBroadcastError: self.logger.info(f'error broadcasting claim tx {txin.spent_txid}') elif funding_height.height == TX_HEIGHT_LOCAL: # the funding tx was double spent. @@ -385,7 +385,7 @@ class SwapManager(Logger): if funding_height.conf > 0 or (swap.is_reverse and self.wallet.config.LIGHTNING_ALLOW_INSTANT_SWAPS): try: await self.network.broadcast_transaction(tx) - except TxBroadcastServerReturnedError: + except TxBroadcastError: self.logger.info(f'error broadcasting claim tx {txin.spent_txid}') def get_claim_fee(self): @@ -423,7 +423,7 @@ class SwapManager(Logger): tx = self.create_funding_tx(swap, None, password=password, batch_rbf=batch_rbf) try: await self.broadcast_funding_tx(swap, tx) - except TxBroadcastServerReturnedError: + except TxBroadcastError: continue break @@ -704,6 +704,8 @@ class SwapManager(Logger): payment_hash = swap.payment_hash refund_pubkey = ECPrivkey(swap.privkey).get_public_key_bytes(compressed=True) async def callback(payment_hash): + # FIXME what if this raises, e.g. TxBroadcastError? + # We will never retry the hold-invoice-callback. await self.broadcast_funding_tx(swap, tx) self.lnworker.register_hold_invoice(payment_hash, callback)