tests: fix test_funding_v2_cancel_race flake
What the developer wrote
Authored by daywalker90
tests: fix test_funding_v2_cancel_race flake
openchannel_update is idempotent: once the commitments are secured a
repeat call returns the existing inflight, so two racing
openchannel_update calls can both legitimately succeed and the "only up
to one should succeed" assertion no longer holds.
Keep the original intent (catching commands that fight over the shared
command pointer and hang) by relying on result(TIMEOUT) raising
TimeoutError, and assert at most one inflight instead of counting
successes: that catches a genuine double commitment, while comparing
the returned PSBTs does not work as inflight->funding_psbt is mutated
in place (wally_psbt_combine/psbt_finalize) once tx-signatures arrive.
Changelog-None
What changed, and why it matters
This commit only changes a test file. It loosens an overly strict test assertion that was causing a flaky test failure. The test previously assumed only one of several racing openchannel_update calls could succeed, but the command is actually idempotent, so multiple calls can legitimately return the same existing commitment. The fix keeps the safety check by verifying the node ends up with at most one actual commitment 'inflight' rather than counting successful RPC responses. There is no change to production code and no security fix.
No security action needed. This is a test-only reliability fix. Reviewers may optionally confirm the new assertion adequately preserves the original test intent of detecting double commitments.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The patch modifies tests/test_connection.py in test_funding_v2_cancel_race. It removes the assertion that num_successes <= 1 among concurrent openchannel_update futures, because openchannel_update is idempotent after commitments are secured and may echo the existing inflight. It replaces that with an assertion that the resulting peer channel has at most one inflight record. The comment notes that comparing returned PSBTs is unreliable because the inflight’s funding_psbt is mutated in place by wally_psbt_combine/psbt_finalize after tx-signatures arrive. No production code is changed.
Changed components
tests/test_connection.pyInspect captured patch +21 / −4
### tests/test_connection.py
@@ -1608,22 +1608,39 @@ def test_funding_v2_cancel_race(node_factory, bitcoind, executor):
for i, c in enumerate(completes):
try:
+ # A command that fights over the shared command pointer hangs
+ # here until TIMEOUT raises TimeoutError (not an RpcError).
c[1].result(TIMEOUT)
completes[i] = (completes[i][0], True)
except RpcError:
completes[i] = (completes[i][0], False)
- # Only up to one should succeed.
- num_successes = sum(c[1] is True for c in completes)
- assert num_successes <= 1, f"Multiple successes in {completes}, cancels = {cancels}"
- num_complete += num_successes
+ # openchannel_update is idempotent: repeating it once the
+ # commitments are secured just echoes the inflight back, so several
+ # of these calls may legitimately succeed.
+ num_complete += sum(c[1] is True for c in completes)
for c in cancels:
try:
c[1].result(TIMEOUT)
num_cancel += 1
except RpcError:
pass
+
+ # What must not happen is more than one commitment for this open.
+ # Check the node's own inflight records rather than comparing the
+ # returned PSBTs: the inflight's PSBT is mutated in place
+ # (wally_psbt_combine/psbt_finalize) once tx-signatures arrive, so
+ # two responses for the same commitment can serialize differently.
+ inflights = [
+ inf
+ for chan in l1.rpc.listpeerchannels(n.info["id"])["channels"]
+ for inf in chan.get("inflight", [])
+ ]
+ assert len(inflights) <= 1, (
+ f"Multiple inflights {inflights}, "
+ f"completes = {completes}, cancels = {cancels}"
+ )
# Free up funds for next time
l1.rpc.unreserveinputs(psbt)
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.