tests: fix test_funding_v2_cancel_race flake - #9523
daywalker90 wants to merge 1 commit into
Conversation
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 replace the count check with the stronger invariant that no two distinct secured PSBTs (i.e. two commitments) were made. Changelog-None
Andezion
left a comment
There was a problem hiding this comment.
secured.add(result["psbt"]) compares the full serialized PSBT string for equality. But inflight->funding_psbt is mutated in place shortly after commitments are secured - once tx-signatures are exchanged, psbt_finalize(inflight->funding_psbt) is called (at file lightningd/dual_open_control.c at line 1809, reached from handle_peer_tx_sigs_sent, which fires immediately after the commit_send_ack round trip that follows handle_commit_ready). This adds final witness/scriptSig data to the same wally_psbt object that build_commit_response() serializes??
i mean we have call1 - gets its response from handle_commit_received right when commitments become secured -> serializes the unfinalized PSBT, and call2 arrives late enough (thread scheduling / socket jitter under 100 concurrent executor.submit calls) that by the time json_openchannel_update runs for it, psbt_finalize has already mutated the inflights PSBT -> serializes a different?, finalized? byte string for the same commitment? So both have commitments_secured == True, so both go into secured, and len(secured) <= 1 fails - even though there was only ever one commitment?
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 replace the count check with the stronger invariant that no two distinct secured PSBTs (i.e. two commitments) were made.
Changelog-None