lnwallet/chancloser: fix flaky test assertions
What the developer wrote
Authored by Olaoluwa Osuntokun
lnwallet/chancloser: fix flaky test assertions
Restore the defaultTimeout constant (500ms) that was lost during the
PR's commit squash, where the hardcoded 10ms replaced it. The 10ms
value was too tight and caused timeouts under -race -count=N.
Also fix assertSingleRemoteRbfIteration to consume both iteration
transitions in a single assertStateTransitions call. When
iteration=true, the state machine emits two ClosingNegotiation
transitions from a single event (via internal events). Making two
separate assertStateTransitions calls races with the "no more states"
check, which could drain the second transition before the second
assertion consumed it.
What changed, and why it matters
This commit fixes flaky test code in the Lightning Network Daemon (LND) project. It restores a longer timeout (500ms instead of 10ms) in a test helper and adjusts how test assertions consume state transitions to avoid race conditions during testing. There is no change to production code or user-facing behavior, and no security issue is present.
No security action needed. Treat as a normal test reliability fix and merge after standard review.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The change is confined to lnwallet/chancloser/rbf_coop_test.go. It reintroduces a defaultTimeout constant of 500ms that had been accidentally replaced by a hardcoded 10ms during a previous squash, which caused timeouts under -race and repeated test runs. It also updates assertSingleRemoteRbfIteration to call assertStateTransitions once with both expected ClosingNegotiation states, preventing a race where the ‘no more states’ cleanup drains the second transition before the second assertion can read it. No production wallet, channel-closer, or P2P logic is modified.
Changed components
lnwallet/chancloser/rbf_coop_test.goInspect captured patch +12 / −6
diff --git a/lnwallet/chancloser/rbf_coop_test.go b/lnwallet/chancloser/rbf_coop_test.go
index 0806d08..6a9389f 100644
--- a/lnwallet/chancloser/rbf_coop_test.go
+++ b/lnwallet/chancloser/rbf_coop_test.go
@@ -63,6 +63,8 @@ var (
localTx = wire.MsgTx{Version: 2}
closeTx = wire.NewMsgTx(2)
+
+ defaultTimeout = 500 * time.Millisecond
)
func sigMustParse(sigBytes []byte) ecdsa.Signature {
@@ -119,7 +121,8 @@ func assertStateTransitions[Event any, Env protofsm.Environment](
for _, expectedState := range expectedStates {
newState, err := fn.RecvOrTimeout(
- stateSub.NewItemCreated.ChanOut(), 10*time.Millisecond,
+ stateSub.NewItemCreated.ChanOut(),
+ defaultTimeout,
)
require.NoError(t, err, "expected state: %T", expectedState)
@@ -850,12 +853,15 @@ func (r *rbfCloserTestHarness) assertSingleRemoteRbfIteration(
}
// Our outer state should transition to ClosingNegotiation state.
- r.assertStateTransitions(&ClosingNegotiation{})
-
- // If this is an iteration, then we'll go from ClosePending ->
- // RemoteCloseStart -> ClosePending. So we'll assert an extra transition
- // here.
+ // If this is an iteration, we go ClosePending -> RemoteCloseStart ->
+ // ClosePending, producing two ClosingNegotiation transitions. We
+ // consume them in a single call to avoid the "no more states" check
+ // draining the second transition before we can assert it.
if iteration {
+ r.assertStateTransitions(
+ &ClosingNegotiation{}, &ClosingNegotiation{},
+ )
+ } else {
r.assertStateTransitions(&ClosingNegotiation{})
}
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.