pytest: restore and fix disabled test test_excluded_adjacent_routehint.
What the developer wrote
Authored by Rusty Russell
pytest: restore and fix disabled test test_excluded_adjacent_routehint.
1. It was flaky, probably because it didn't wait for the remote update_channel.
2. Rusty applied a fix in 5f664dac77d, not clear if it worked.
3. Christian disabled it altogether in 23ce9a947df.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
What changed, and why it matters
This commit re-enables a flaky test and makes a small code change so that all pending 'connect' commands are failed when a peer disconnects during connection setup. The test itself checks that the payment system handles an overly expensive route hint without crashing. There is no direct evidence this fixes a security vulnerability, but it removes a potential hang/crash path in connection handling.
Treat as a routine robustness fix. Review whether CONNECT_DISCONNECTED_DURING failure path could leave connect commands pending in production, but no immediate security response is indicated.
Security signals we found
Code change in connection failure path could prevent a stuck or leaked connect command
Test re-enabled after prior disabling due to flakiness
No explicit security claim in commit message or diff
Evidence from the diff
The diff modifies connect_control.c so that when a connection attempt fails with CONNECT_DISCONNECTED_DURING, any queued connect commands for that peer are also failed, not just when the failure reason starts with ‘connect command’. In tests/test_pay.py it removes the @unittest.skip decorator from test_excluded_adjacent_routehint and adds waits for remote channel updates before proceeding, addressing flakiness.
Changed components
lightningd/connect_control.ctests/test_pay.pyInspect captured patch +8 / −3
diff --git a/lightningd/connect_control.c b/lightningd/connect_control.c
index f2710db5..611b5ba2 100644
--- a/lightningd/connect_control.c
+++ b/lightningd/connect_control.c
@@ -286,7 +286,8 @@ static void connect_failed(struct lightningd *ld,
* does simply combine those, so we don't get a response per request, and it's a
* very rare corner case (which, unlike the above, doesn't happen in CI!).
*/
- if (strstarts(connect_reason, "connect command")) {
+ if (strstarts(connect_reason, "connect command")
+ || errcode == CONNECT_DISCONNECTED_DURING) {
/* We can have multiple connect commands: fail them all */
while ((c = find_connect(ld, id)) != NULL) {
/* They delete themselves from list */
diff --git a/tests/test_pay.py b/tests/test_pay.py
index 94ee2c0a..1b35d509 100644
--- a/tests/test_pay.py
+++ b/tests/test_pay.py
@@ -3480,7 +3480,6 @@ def test_reject_invalid_payload(node_factory):
l2.daemon.wait_for_log(r'Failing HTLC because of an invalid payload')
-@unittest.skip("Test is flaky causing CI to be unusable.")
def test_excluded_adjacent_routehint(node_factory, bitcoind):
"""Test case where we try have a routehint which leads to an adjacent
node, but the result exceeds our maxfee; we crashed trying to find
@@ -3489,10 +3488,15 @@ def test_excluded_adjacent_routehint(node_factory, bitcoind):
"""
l1, l2, l3 = node_factory.line_graph(3)
+ # Make sure l2->l3 is usable.
+ wait_for(lambda: 'remote' in only_one(l3.rpc.listpeerchannels()['channels'])['updates'])
+
# We'll be forced to use routehint, since we don't know about l3.
inv = l3.rpc.invoice(10**3, "lbl", "desc", exposeprivatechannels=l2.get_channel_scid(l3))
- l1.wait_channel_active(l1.get_channel_scid(l2))
+ # Make sure l1->l2 is usable.
+ wait_for(lambda: 'remote' in only_one(l1.rpc.listpeerchannels()['channels'])['updates'])
+
# This will make it reject the routehint.
err = r'Fee exceeds our fee budget: 1msat > 0msat, discarding route'
with pytest.raises(RpcError, match=err):
Why this scored 23/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.