From 0d5428cce0be5a8bf725f0c48b176d8255df1abb Mon Sep 17 00:00:00 2001 From: ziggie Date: Fri, 7 Aug 2026 09:01:53 -0300 Subject: [PATCH 1/2] lnwallet/chancloser: use valid delivery scripts in RBF closer tests TestRbfChannelActiveTransitions shadowed the package-level localAddr and remoteAddr with twenty raw bytes each: localAddr := lnwire.DeliveryAddress(bytes.Repeat([]byte{0x01}, 20)) remoteAddr := lnwire.DeliveryAddress(bytes.Repeat([]byte{0x02}, 20)) Neither is a well-formed delivery script -- not a witness program, and not any other template a co-op close is willing to pay to -- so both are rejected by validateShutdownScript. That goes unnoticed today because validateShutdown is a chain of guards with early returns: the thaw height is checked first, then the taproot shutdown nonce, and only then the delivery script. Both subtests that feed the shadowed remoteAddr into a ShutdownReceived, namely remote_initiated_thaw_height_close_fail and remote_initiated_taproot_no_nonce_fail, trip an earlier guard on purpose, so the script check is never reached. The one subtest that does drive the whole chain, remote_initiated_close_ok, was extracted into testRemoteInitiatedCloseOkNonTap and testRemoteInitiatedCloseOkTaproot, which sit outside the shadow and so pick up the valid package-level scripts. Nothing currently passes for the wrong reason, since the harness asserts on specific sentinel errors, but that holds only by accident of where the failures happen to land. The cost is paid by whoever touches that guard chain next. Hoisting the delivery-script check above the thaw height check, a reasonable thing to want, does not fail readably: the unexpected ErrInvalidShutdownScript reaches the mock error reporter as an unmatched call and panics the package's test binary, with a stack pointing into protofsm and the mock plumbing rather than at the fixture that is actually wrong. Drop the shadowing locals so those subtests use the valid package-level P2TR scripts, and record the invariant where the scripts are declared. A negative test's fixture should be valid in every dimension except the one under test, or it is not isolating what it claims to. --- lnwallet/chancloser/rbf_coop_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lnwallet/chancloser/rbf_coop_test.go b/lnwallet/chancloser/rbf_coop_test.go index e3f0e88e8c..808c0494f4 100644 --- a/lnwallet/chancloser/rbf_coop_test.go +++ b/lnwallet/chancloser/rbf_coop_test.go @@ -32,6 +32,10 @@ import ( "github.com/stretchr/testify/require" ) +// Both of these must stay well-formed delivery scripts (they're P2TR here). +// Tests that drive a shutdown all the way through validateShutdown depend on +// them clearing the delivery-script check, and tests that target an earlier +// guard depend on the script not being what fails. var ( localAddr = lnwire.DeliveryAddress(append( []byte{txscript.OP_1, txscript.OP_DATA_32}, @@ -1369,8 +1373,6 @@ func testRemoteInitiatedCloseOkTaproot(t *testing.T, ctx context.Context) { // ChannelActive state. func TestRbfChannelActiveTransitions(t *testing.T) { ctx := t.Context() - localAddr := lnwire.DeliveryAddress(bytes.Repeat([]byte{0x01}, 20)) - remoteAddr := lnwire.DeliveryAddress(bytes.Repeat([]byte{0x02}, 20)) feeRate := chainfee.SatPerVByte(1000) From 7cad07012c2ad28a535f1edbd34b6cf369d904b1 Mon Sep 17 00:00:00 2001 From: ziggie Date: Fri, 7 Aug 2026 09:26:36 -0300 Subject: [PATCH 2/2] docs: add release notes entry for the RBF closer test fixture fix --- docs/release-notes/release-notes-0.22.0.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/docs/release-notes/release-notes-0.22.0.md b/docs/release-notes/release-notes-0.22.0.md index 750a20069c..10757e1601 100644 --- a/docs/release-notes/release-notes-0.22.0.md +++ b/docs/release-notes/release-notes-0.22.0.md @@ -136,6 +136,15 @@ ## Testing +* [Fixed an invalid delivery-script fixture in the RBF cooperative close + tests](https://github.com/lightningnetwork/lnd/pull/11042). + `TestRbfChannelActiveTransitions` shadowed the package-level delivery + addresses with byte blobs that are not well-formed shutdown scripts. The + subtests using them all tripped an earlier guard in `validateShutdown`, so the + bad fixture was never reached and nothing failed — but reordering those guards + would have panicked the package's test binary from inside the mock error + reporter, far from the fixture actually at fault. + ## Database ## Code Health