What the developer wrote
Authored by ziggie
itest: stop creating legacy channels
In this commit, we move the two tests that still opened legacy channels over
to static remote key. Both produce the same commitment shape and the same
fees, so no assertion moves.
The watchtower case opted into legacy deliberately. The remote signer one did
not: neither of its nodes runs with committweak, so both support static
remote key, and the channel only came out legacy because an empty
channel_type was accepted without any check. That is the bug this PR fixes,
sitting in our own suite.
The legacy node arguments and the unreferenced CfgLegacy go away with them,
since committweak no longer does anything.
What changed, and why it matters
This commit updates LND's integration tests to stop using an obsolete 'legacy' channel type and adds a test confirming that LND now rejects that legacy type. The commit message says the real fix is elsewhere in the same pull request: previously, an empty channel_type was accepted without checking whether the nodes actually supported the legacy format, which could cause a channel to open in an unintended, deprecated commitment shape. The diff itself only changes tests and removes unused legacy-node configuration flags; it does not contain the actual production-code fix.
Review the rest of the pull request to locate the production-code change that adds the ErrDeprecatedChanType validation, since this commit only contains tests and cleanup. Once the full fix is identified, consider whether any running nodes still have open legacy channels and plan migration or closure, because the legacy commitment type was removed from the BOLT spec in 2024.
Security signals we found
Deprecated commitment format (legacy) is now rejected by RPC
Test added to verify legacy channel type is refused
Unused legacy node configuration flag removed
Commit message describes a real bug where empty channel_type defaulted to legacy without validation
Evidence from the diff
The patch removes the last integration-test usages of lnrpc.CommitmentType_LEGACY, switching the watchtower and remote-signer tests to STATIC_REMOTE_KEY. It deletes the CfgLegacy node config and the legacy case in NodeArgsForCommitType. It also adds a new test, testLegacyChanTypeRejected, asserting that OpenChannel with CommitmentType_LEGACY returns funding.ErrDeprecatedChanType before the funding flow starts. The commit message explicitly states the production bug being fixed is that an empty channel_type was accepted with no check, causing a channel to come out legacy even when neither node requested it. The actual validation logic is not present in this diff, so this commit is a test-suite cleanup and regression test accompanying the fix.
Changed components
itest/lnd_funding_test.goitest/lnd_remote_signer_test.goitest/lnd_watchtower_test.golntest/node/config.golntest/utils.goInspect captured patch +24 / −8
### itest/lnd_funding_test.go
@@ -37,6 +37,10 @@ var basicFundingTestCases = []*lntest.TestCase{
Name: "basic flow simple taproot final",
TestFunc: testBasicChannelFundingSimpleTaprootFinal,
},
+ {
+ Name: "legacy chan type rejected",
+ TestFunc: testLegacyChanTypeRejected,
+ },
}
// allFundingTypes defines the channel types to test for the basic funding
@@ -48,6 +52,24 @@ var allFundingTypes = []lnrpc.CommitmentType{
lnrpc.CommitmentType_SIMPLE_TAPROOT_FINAL,
}
+// testLegacyChanTypeRejected asserts that lnd refuses to open a channel using
+// the legacy commitment type, which was removed from the spec in 2024.
+func testLegacyChanTypeRejected(ht *lntest.HarnessTest) {
+ carol := ht.NewNodeWithCoins("Carol", nil)
+ dave := ht.NewNodeWithCoins("Dave", nil)
+ ht.EnsureConnected(carol, dave)
+
+ // The RPC server turns the request down before the funding flow even
+ // starts, so no channel type ever reaches Dave.
+ ht.OpenChannelAssertErr(
+ carol, dave, lntest.OpenChannelParams{
+ Amt: funding.MaxBtcFundingAmount,
+ CommitmentType: lnrpc.CommitmentType_LEGACY,
+ },
+ funding.ErrDeprecatedChanType,
+ )
+}
+
// testBasicChannelFundingStaticRemote performs a test exercising expected
// behavior from a basic funding workflow. The test creates a new channel
// between Carol and Dave, with Carol using the static remote key commitment
### itest/lnd_remote_signer_test.go
@@ -614,7 +614,7 @@ func psbtTestCase(ht *lntest.HarnessTest,
fn: func(tt *lntest.HarnessTest, wo, carol *node.HarnessNode) {
runPsbtChanFundingWithNodes(
tt, carol, wo, false,
- lnrpc.CommitmentType_LEGACY,
+ lnrpc.CommitmentType_STATIC_REMOTE_KEY,
)
runSignPsbtSegWitV0P2WKH(tt, wo)
runSignPsbtSegWitV1KeySpendBip86(tt, wo)
### itest/lnd_watchtower_test.go
@@ -336,7 +336,7 @@ func testTowerClientSessionDeletion(ht *lntest.HarnessTest) {
// Carol's behalf sweeping her funds without a reward.
func testRevokedCloseRetributionAltruistWatchtower(ht *lntest.HarnessTest) {
for _, commitType := range []lnrpc.CommitmentType{
- lnrpc.CommitmentType_LEGACY,
+ lnrpc.CommitmentType_STATIC_REMOTE_KEY,
lnrpc.CommitmentType_ANCHORS,
lnrpc.CommitmentType_SIMPLE_TAPROOT,
lnrpc.CommitmentType_SIMPLE_TAPROOT_FINAL,
### lntest/node/config.go
@@ -47,10 +47,6 @@ var (
"btcdexec", "", "full path to btcd binary",
)
- // CfgLegacy specifies the config used to create a node that uses the
- // legacy channel format.
- CfgLegacy = []string{"--protocol.legacy.committweak"}
-
// CfgStaticRemoteKey specifies the config used to create a node that
// uses the static remote key feature.
CfgStaticRemoteKey = []string{}
### lntest/utils.go
@@ -158,8 +158,6 @@ func CommitTypeHasAnchors(commitType lnrpc.CommitmentType) bool {
// commitment type.
func NodeArgsForCommitType(commitType lnrpc.CommitmentType) []string {
switch commitType {
- case lnrpc.CommitmentType_LEGACY:
- return []string{"--protocol.legacy.committweak"}
case lnrpc.CommitmentType_STATIC_REMOTE_KEY:
return []string{}
case lnrpc.CommitmentType_ANCHORS:Why this scored 33/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.