tests: fix flaky test_sql blockheight race#9304
Open
ksedgwic wants to merge 1 commit into
Open
Conversation
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 7f6a333 ("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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Observed on CI runs for #9299 and #9300 (Valgrind Test CLN 12/12):
https://github.com/ElementsProject/lightning/actions/runs/29053072126
https://github.com/ElementsProject/lightning/actions/runs/29053114743
The test mines ~106 blocks immediately before making three payments,
without waiting for the nodes to process them. Under valgrind the
paying node can lag far behind bitcoind, so it builds onions whose
cltv expiries the caught-up peers reject. The two runs showed both
shapes of the race:
l1 ~40 blocks behind (l2/l3 at 210): l2 failed the forward ("Expiry
cltv 181 too close to current 210", expiry_too_soon). The error
came from the invoice's route-hint channel, so xpay disabled it and
the payment failed hard with error 209.
l1 a few blocks behind: l3 failed the final hop ("Expiry cltv too
soon 211 < 210 + 5"). xpay's waitblockheight retry succeeded, but
the failed first attempt left status 'failed' in the forwards row
the test asserts is 'settled'.
The test paid with pay until 7f6a333 switched it to xpay; pay took
its start height from bitcoind's headercount and tracked the node's
chainlag, so lagging nodes still built onions valid at the true chain
tip, which is why this only started flaking recently.
The fix syncs the nodes' blockheights before paying, as other tests
do after mining bursts.
Changelog-None