tests: fix flaky test_sql blockheight race
What the developer wrote
Authored by Ken Sedgwick
tests: fix flaky test_sql blockheight race
The test mines ~106 blocks (channel close, replacement channel
funding, then generate_block(99)) immediately before making three
payments, without waiting for the nodes to process those blocks.
Under valgrind the paying node can lag far behind bitcoind when the
payments start, so it builds onions whose cltv expiries the caught-up
peers reject. Two CI failures showed both shapes of the race:
In one run l1 was ~40 blocks behind (l2/l3 at block 210, l1 still
catching up), so l2 failed the forward: "Expiry cltv 181 too close to
current 210" (expiry_too_soon). Since the error came from the
invoice's route-hint channel, xpay disabled it and the payment failed
hard: "All 1 channels to the destination are disabled."
In the other run l1 was only a few blocks behind, so l3 failed the
final hop: "Expiry cltv too soon 211 < 210 + 5". xpay's
waitblockheight retry then succeeded, but the failed first attempt
left status 'failed' in the forwards row the test later asserts is
'settled'.
The test paid with pay until 7f6a33394 ("pytest: use xpay, not pay in
misc tests."); pay took its start height from bitcoind's headercount
and tracked the node's chainlag, so a lagging node still built onions
valid at the true chain tip. xpay uses its own notification-fed
blockheight plus one block of slack, which is why this only started
flaking recently.
Sync the nodes' blockheights before paying, as other tests do after
mining bursts.
Changelog-None
What changed, and why it matters
This commit fixes a flaky test, not a security vulnerability. The test sometimes failed because one node was slower to process newly mined blocks than the others, causing payment route expiry values to be rejected. The fix simply waits for all test nodes to catch up to the same block height before making payments. There is no product bug being patched and no security relevance.
No security action required. Treat as a routine test reliability fix.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The change adds a sync_blockheight(bitcoind, [l1, l2, l3]) call in tests/test_plugin.py::test_sql after a burst of ~106 generated blocks and before three xpay payments. The race occurred under valgrind when the paying node lagged behind bitcoind, producing onion CLTV expiries that caught-up peers rejected as expiry_too_soon. This is a test-only synchronization fix; no production code is modified.
Changed components
tests/test_plugin.py::test_sqlInspect captured patch +5 / −0
diff --git a/tests/test_plugin.py b/tests/test_plugin.py
index 0025d4c3..8d2c7f43 100644
--- a/tests/test_plugin.py
+++ b/tests/test_plugin.py
@@ -4359,6 +4359,11 @@ def test_sql(node_factory, bitcoind):
# Make sure we have a node_announcement for l1
wait_for(lambda: l2.rpc.listnodes(l1.info['id'])['nodes'] != [])
+ # Under valgrind, the senders can still be digesting the blocks
+ # above and pick a stale blockheight for the payments below, which
+ # the caught-up peers reject as expiry_too_soon.
+ sync_blockheight(bitcoind, [l1, l2, l3])
+
# This should create a forward through l2
l1.rpc.xpay(l3.rpc.invoice(amount_msat=12300, label='inv1', description='description')['bolt11'])
Why this scored 15/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.