tests: try to make "backup" regtest less flaky
What the developer wrote
Authored by SomberNight
tests: try to make "backup" regtest less flaky
Kind of shooting in the dark, but looking at logs, when the "backup" test fails,
Alice logs "Sending CHANNEL_REESTABLISH" and "Sending ERROR", but
Bob never receives them, instead he logs "Disconnecting: LightningPeerConnectionClosed()".
Maybe Alice's TCP socket should just be flushed, as done here.
What changed, and why it matters
This is a test-only reliability fix for a flaky automated test. The developer noticed that during a backup/restore test, one side (Alice) would send Lightning network messages and then close the connection, but the other side (Bob) sometimes never received those final messages. The change makes Alice explicitly wait for her outgoing messages to actually leave the network socket before closing it. There is no indication this is a security vulnerability or that it affects normal user operations.
No security action required. Treat as a normal test-stability / robustness improvement. If reviewing for broader reliability, consider whether the same flush-before-close pattern should be applied consistently elsewhere in lnpeer.py, as the comments suggest.
Security signals we found
No security framing by vendor: commit title and message describe test flakiness only
No mention of vulnerability, exploit, CVE, attacker, or security issue
Change is defensive/reliability-oriented: ensures messages are flushed before close
Only affects a regtest code path and the internal Lightning force-close request flow
Evidence from the diff
The commit modifies lnworker.py so that after requesting a force-close, the code awaits transport.writer.wait_closed() inside an async_timeout(1) to flush the TCP write buffer before the socket is torn down. Comments in lnpeer.py note that lower-level send/close paths are synchronous and cannot easily await drain()/wait_closed(). The regtest script gets explanatory comments and a FIXME about waiting for both channels. The change is framed as ‘shooting in the dark’ to reduce test flakiness.
Changed components
electrum/lnworker.py - LNWallet request_force_close pathelectrum/lnpeer.py - send_message and close path comments onlytests/regtest/regtest.sh - 'backup' regtest scenarioInspect captured patch +11 / −1
diff --git a/electrum/lnpeer.py b/electrum/lnpeer.py
index d4551cc..412241c 100644
--- a/electrum/lnpeer.py
+++ b/electrum/lnpeer.py
@@ -151,6 +151,7 @@ class Peer(Logger, EventListener):
raw_msg = encode_msg(message_name, **kwargs)
self._store_raw_msg_if_local_update(raw_msg, message_name=message_name, channel_id=kwargs.get("channel_id"))
self.transport.send_bytes(raw_msg)
+ # could `await self.transport.writer.drain()`, but not async
def _store_raw_msg_if_local_update(self, raw_msg: bytes, *, message_name: str, channel_id: Optional[bytes]):
is_commitment_signed = message_name == "commitment_signed"
@@ -907,6 +908,7 @@ class Peer(Logger, EventListener):
try:
if self.transport:
self.transport.close()
+ # could `await self.transport.writer.wait_closed()` with a timeout? but not async
except Exception:
pass
self.lnworker.lnpeermgr.peer_closed(self)
diff --git a/electrum/lnworker.py b/electrum/lnworker.py
index 80eb8f4..3dbb8ed 100644
--- a/electrum/lnworker.py
+++ b/electrum/lnworker.py
@@ -3818,6 +3818,9 @@ class LNWallet(Logger):
async with OldTaskGroup(wait=any) as group:
await group.spawn(peer._message_loop())
await group.spawn(peer.request_force_close(channel_id))
+ async with util.async_timeout(1):
+ peer.transport.close()
+ await peer.transport.writer.wait_closed() # flush write-buffer
return True
except Exception as e:
self.logger.info(f'failed to connect {host} {e}')
diff --git a/tests/regtest/regtest.sh b/tests/regtest/regtest.sh
index 0dcadfe..84df0e1 100755
--- a/tests/regtest/regtest.sh
+++ b/tests/regtest/regtest.sh
@@ -252,6 +252,11 @@ fi
if [[ $1 == "backup" ]]; then
+ # Alice has two channels with Bob.
+ # - chan1 has on-chain op_return backups,
+ # - chan2 has an imported backup.
+ # Alice restores from seed, and also imports backup for chan2.
+ # Test "request_force_close" works for both channels.
wait_for_balance alice 1
echo "alice opens channel"
bob_node=$($bob nodeid)
@@ -260,7 +265,7 @@ if [[ $1 == "backup" ]]; then
$alice setconfig use_recoverable_channels False
channel2=$($alice open_channel $bob_node 0.15 --password='')
new_blocks 3
- wait_until_channel_open alice
+ wait_until_channel_open alice # FIXME wait for *both* channels?
backup=$($alice export_channel_backup $channel2)
seed=$($alice getseed --password='')
$alice stop
Why this scored 12/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.