submarine_swaps: wait for broadcast in wait_for_htlcs_and_broadcast
What the developer wrote
Authored by f321x
submarine_swaps: wait for broadcast in wait_for_htlcs_and_broadcast
Wait for broadcast of the funding transaction in
`SwapManager.wait_for_htlcs_and_broadcast` before returning.
Previously it might have returned the swap funding txid before the
funding tx was broadcast.
This caused the regtest `test_swapserver_success_forward` to fail
as it expected the funding tx to be broadcast once the CLI command
returned.
See:
```
.***** test_swapserver_success_forward ******
initializing alice
funding alice
ec354953f96e0de7be4354ab156611da9e31bd85c2d25664bdb995950530c12e
initializing bob
funding bob
1df19a419b5ae5b961bae6cfaf4e7c8e772c6632d30782fab8de6992a8901e4a
mining 1 blocks
starting daemon (PID 7471)
/tmp/alice/regtest/wallets/default_wallet
true
starting daemon (PID 7502)
/tmp/bob/regtest/wallets/default_wallet
true
alice opens channel
mining 3 blocks
wait until alice sees channel open.
wait until alice sees channel open..
wait until alice sees channel open...
alice initiates forward-swap
{
"lightning_amount": "0.01967661",
"onchain_amount": "0.02",
"txid": "b0f9ba48b670ee4a1f182655fe18f3f7d5c310923b7c85ad396b24e0a2f8e0cc"
}
utxo b0f9ba48b670ee4a1f182655fe18f3f7d5c310923b7c85ad396b24e0a2f8e0cc:0 does not exist
FDaemon stopped
Daemon stopped
======================================================================
FAIL: test_swapserver_success_forward (tests.regtest.TestLightningSwapserver.test_swapserver_success_forward)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/tmp/cirrus-ci-build/tests/regtest.py", line 105, in test_swapserver_success_forward
self.run_shell(['swapserver_success_forward'])
File "/tmp/cirrus-ci-build/tests/regtest.py", line 19, in run_shell
assert process.returncode == 0
^^^^^^^^^^^^^^^^^^^^^^^
AssertionError
```
Can be reproduced with:
```diff
diff --git a/electrum/submarine_swaps.py b/electrum/submarine_swaps.py
index f83d653f9..242aef070 100644
--- a/electrum/submarine_swaps.py
+++ b/electrum/submarine_swaps.py
@@ -1052,6 +1052,7 @@ class SwapManager(Logger):
@log_exceptions
async def broadcast_funding_tx(self, swap: SwapData, tx: Transaction) -> None:
swap.funding_txid = tx.txid()
+ await asyncio.sleep(5)
await self.network.broadcast_transaction(tx)
async def reverse_swap(
```
What changed, and why it matters
This commit fixes a timing bug in Electrum's submarine swap feature. Previously, when a user initiated a swap, the software could report the swap funding transaction ID back to the caller before the transaction was actually broadcast to the Bitcoin network. This meant a user or automated test might think the swap was funded and proceed, while the transaction had not yet been sent. The fix makes the code wait until the broadcast is complete before returning. There is also a small improvement in error handling when the broadcast fails.
Treat this as a low-severity reliability fix. It should be included in the next release because it prevents user-visible swap state inconsistency and improves error handling. No urgent security advisory is warranted based solely on the commit content, but downstream consumers relying on the swap API should be aware that prior versions could return a txid before network broadcast.
Security signals we found
Race condition between setting swap.funding_txid and broadcasting the funding transaction
Hold-invoice callback could raise unhandled TxBroadcastError, preventing retry
Caller could proceed on the assumption that funding tx was broadcast when it was not
Fix introduces explicit synchronization via asyncio.Event and timeout handling
Evidence from the diff
In electrum/submarine_swaps.py, the wait_for_htlcs_and_broadcast method previously polled swap.funding_txid is None to detect when the funding transaction had been broadcast. However, broadcast_funding_tx set swap.funding_txid = tx.txid() before calling network.broadcast_transaction(tx), so the polling loop could observe the txid and return before the broadcast completed. The patch removes broadcast_funding_tx and inlines the broadcast into the hold-invoice callback, using an asyncio.Event (funding_broadcast) that is only set after broadcast completes (successfully or with a TxBroadcastError). The caller now awaits this event with a timeout tied to invoice expiry. A failed broadcast is logged rather than silently raising and breaking retry logic.
Changed components
electrum/submarine_swaps.pySwapManager.wait_for_htlcs_and_broadcastSwapManager.broadcast_funding_tx (removed)Lightning submarine swap (normal/forward swap) flowInspect captured patch +17 / −12
diff --git a/electrum/submarine_swaps.py b/electrum/submarine_swaps.py
index 08f7bb1..863be00 100644
--- a/electrum/submarine_swaps.py
+++ b/electrum/submarine_swaps.py
@@ -22,6 +22,7 @@ from collections import defaultdict
from .i18n import _
+from .interface import TxBroadcastError
from .logging import Logger
from .crypto import sha256, ripemd
from .bitcoin import (script_to_p2wsh, opcodes, dust_threshold, DummyAddress, construct_witness,
@@ -976,12 +977,18 @@ class SwapManager(Logger):
await transport.is_connected.wait()
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)
+ funding_broadcast = asyncio.Event()
+ async def lightning_payment_callback(_payment_hash):
+ try:
+ await self.network.broadcast_transaction(tx)
+ except TxBroadcastError as e:
+ # FIXME: We will never retry the hold-invoice-callback.
+ self.logger.error(f"failed to broadcast swap funding transaction: {e}")
+ finally:
+ swap.funding_txid = tx.txid()
+ funding_broadcast.set()
- self.lnworker.register_hold_invoice(payment_hash, callback)
+ self.lnworker.register_hold_invoice(payment_hash, lightning_payment_callback)
# send invoice to server and wait for htlcs
# note: server will link this RPC to our previous 'createnormalswap' RPC
@@ -994,8 +1001,11 @@ class SwapManager(Logger):
await transport.send_request_to_server('addswapinvoice', request_data)
# wait for funding tx
lnaddr = decode_bolt11_invoice(invoice)
- while swap.funding_txid is None and not lnaddr.is_expired():
- await asyncio.sleep(0.1)
+ seconds_to_expiry = (lnaddr.date + lnaddr.get_expiry()) - now()
+ try:
+ await wait_for2(funding_broadcast.wait(), timeout=seconds_to_expiry)
+ except asyncio.TimeoutError:
+ self.logger.warning("timeout waiting for funding tx broadcast, invoice expired")
return swap.funding_txid
def create_funding_output(self, swap: SwapData) -> PartialTxOutput:
@@ -1045,11 +1055,6 @@ class SwapManager(Logger):
expected_onchain_amount_sat=onchain_amount)
return swap, invoice
- @log_exceptions
- async def broadcast_funding_tx(self, swap: SwapData, tx: Transaction) -> None:
- swap.funding_txid = tx.txid()
- await self.network.broadcast_transaction(tx)
-
async def reverse_swap(
self,
*,
Why this scored 32/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.