itest: refactor testOptionScidUpgrade

lntest/itest/list_on_test.go

@@ -429,4 +429,8 @@ var allTestCasesTemp = []*lntemp.TestCase{
 		Name:     "scid alias channel update",
 		TestFunc: testUpdateChannelPolicyScidAlias,
 	},
+	{
+		Name:     "scid alias upgrade",
+		TestFunc: testOptionScidUpgrade,
+	},
 }

lntest/itest/lnd_test_list_on_test.go

@@ -56,10 +56,6 @@ var allTestCases = []*testCase{
 		name: "taproot",
 		test: testTaproot,
 	},
-	{
-		name: "scid alias upgrade",
-		test: testOptionScidUpgrade,
-	},
 	{
 		name: "nonstd sweep",
 		test: testNonstdSweep,

lntest/itest/lnd_zero_conf_test.go

@@ -1,7 +1,6 @@
 package itest
 
 import (
-	"context"
 	"testing"
 	"time"
 
@@ -13,7 +12,6 @@ import (
 	"github.com/lightningnetwork/lnd/lntemp"
 	"github.com/lightningnetwork/lnd/lntemp/node"
 	"github.com/lightningnetwork/lnd/lntemp/rpc"
-	"github.com/lightningnetwork/lnd/lntest"
 	"github.com/lightningnetwork/lnd/lntest/wait"
 	"github.com/lightningnetwork/lnd/lnwire"
 	"github.com/stretchr/testify/require"
@@ -761,68 +759,51 @@ func testPrivateUpdateAlias(ht *lntemp.HarnessTest,
 
 // testOptionScidUpgrade tests that toggling the option-scid-alias feature bit
 // correctly upgrades existing channels.
-func testOptionScidUpgrade(net *lntest.NetworkHarness, t *harnessTest) {
+func testOptionScidUpgrade(ht *lntemp.HarnessTest) {
-	ctxb := context.Background()
+	bob := ht.Bob
 
 	// Start carol with anchors only.
 	carolArgs := []string{
 		"--protocol.anchors",
 	}
-	carol := net.NewNode(t.t, "carol", carolArgs)
+	carol := ht.NewNode("carol", carolArgs)
 
 	// Start dave with anchors + scid-alias.
 	daveArgs := []string{
 		"--protocol.anchors",
 		"--protocol.option-scid-alias",
 	}
-	dave := net.NewNode(t.t, "dave", daveArgs)
+	dave := ht.NewNode("dave", daveArgs)
-	defer shutdownAndAssert(net, t, dave)
 
 	// Give carol some coins.
-	net.SendCoins(t.t, btcutil.SatoshiPerBitcoin, carol)
+	ht.FundCoins(btcutil.SatoshiPerBitcoin, carol)
 
 	// Ensure carol and are connected.
-	net.EnsureConnected(t.t, carol, dave)
+	ht.EnsureConnected(carol, dave)
 
 	chanAmt := btcutil.Amount(1_000_000)
 
-	fundingPoint := openChannelAndAssert(
+	p := lntemp.OpenChannelParams{
-		t, net, carol, dave,
+		Amt:     chanAmt,
-		lntest.OpenChannelParams{
+		PushAmt: chanAmt / 2,
-			Amt:     chanAmt,
+		Private: true,
-			PushAmt: chanAmt / 2,
+	}
-			Private: true,
+	ht.OpenChannel(carol, dave, p)
-		},
-	)
-	defer closeChannelAndAssert(t, net, carol, fundingPoint, false)
-
-	err := carol.WaitForNetworkChannelOpen(fundingPoint)
-	require.NoError(t.t, err)
-	err = dave.WaitForNetworkChannelOpen(fundingPoint)
-	require.NoError(t.t, err)
 
 	// Bob will open a channel to Carol now.
-	net.EnsureConnected(t.t, net.Bob, carol)
+	ht.EnsureConnected(bob, carol)
 
-	fundingPoint2 := openChannelAndAssert(
+	p = lntemp.OpenChannelParams{
-		t, net, net.Bob, carol,
+		Amt: chanAmt,
-		lntest.OpenChannelParams{
+	}
-			Amt: chanAmt,
+	fundingPoint2 := ht.OpenChannel(bob, carol, p)
-		},
-	)
 
-	err = net.Bob.WaitForNetworkChannelOpen(fundingPoint2)
+	// Make sure Dave knows this channel.
-	require.NoError(t.t, err)
+	ht.AssertTopologyChannelOpen(dave, fundingPoint2)
-	err = carol.WaitForNetworkChannelOpen(fundingPoint2)
-	require.NoError(t.t, err)
-	err = dave.WaitForNetworkChannelOpen(fundingPoint2)
-	require.NoError(t.t, err)
 
 	// Carol will now set the option-scid-alias feature bit and restart.
 	carolArgs = append(carolArgs, "--protocol.option-scid-alias")
-	carol.SetExtraArgs(carolArgs)
+	ht.RestartNodeWithExtraArgs(carol, carolArgs)
-	err = net.RestartNode(carol, nil)
-	require.NoError(t.t, err)
 
 	// Dave will create an invoice for Carol to pay, it should contain an
 	// alias in the hop hints.
@@ -836,21 +817,9 @@ func testOptionScidUpgrade(net *lntest.NetworkHarness, t *harnessTest) {
 	var startingAlias lnwire.ShortChannelID
 	startingAlias.BlockHeight = 16_000_000
 
-	err = wait.Predicate(func() bool {
+	err := wait.Predicate(func() bool {
-		ctxt, _ := context.WithTimeout(ctxb, defaultTimeout)
+		invoiceResp := dave.RPC.AddInvoice(daveParams)
-		invoiceResp, err := dave.AddInvoice(ctxt, daveParams)
+		decodedReq := dave.RPC.DecodePayReq(invoiceResp.PaymentRequest)
-		if err != nil {
-			return false
-		}
-
-		payReq := &lnrpc.PayReqString{
-			PayReq: invoiceResp.PaymentRequest,
-		}
-
-		decodedReq, err := dave.DecodePayReq(ctxb, payReq)
-		if err != nil {
-			return false
-		}
 
 		if len(decodedReq.RouteHints) != 1 {
 			return false
@@ -868,27 +837,33 @@ func testOptionScidUpgrade(net *lntest.NetworkHarness, t *harnessTest) {
 
 		return false
 	}, defaultTimeout)
-	require.NoError(t.t, err)
+	require.NoError(ht, err)
 
 	// Carol should be able to pay it.
-	_ = sendAndAssertSuccess(
+	ht.CompletePaymentRequests(carol, []string{daveInvoice.PaymentRequest})
-		t, carol, &routerrpc.SendPaymentRequest{
-			PaymentRequest: daveInvoice.PaymentRequest,
-			TimeoutSeconds: 60,
-			FeeLimitMsat:   noFeeLimitMsat,
-		},
-	)
 
-	daveInvoice2, err := dave.AddInvoice(ctxb, daveParams)
+	// TODO(yy): remove this connection once the following bug is fixed.
-	require.NoError(t.t, err)
+	// When Carol restarts, she will try to make a persistent connection to
+	// Bob. Meanwhile, Bob will also make a conn request as he notices the
+	// connection is broken. If they make these conn requests at the same
+	// time, they both have an outbound conn request, and will close the
+	// inbound conn they receives, which ends up in no conn.
+	ht.EnsureConnected(bob, carol)
 
-	_ = sendAndAssertSuccess(
+	daveInvoice2 := dave.RPC.AddInvoice(daveParams)
-		t, net.Bob, &routerrpc.SendPaymentRequest{
+	ht.CompletePaymentRequests(bob, []string{daveInvoice2.PaymentRequest})
-			PaymentRequest: daveInvoice2.PaymentRequest,
+
-			TimeoutSeconds: 60,
+	// TODO(yy): remove the sleep once the following bug is fixed.  When
-			FeeLimitMsat:   noFeeLimitMsat,
+	// the payment is reported as settled by Bob, it's expected the
-		},
+	// commitment dance is finished and all subsequent states have been
-	)
+	// updated. Yet we'd receive the error `cannot co-op close channel with
+	// active htlcs` or `link failed to shutdown` if we close the channel.
+	// We need to investigate the order of settling the payments and
+	// updating commitments to understand and fix.
+	time.Sleep(2 * time.Second)
+
+	// Close standby node's channels.
+	ht.CloseChannel(bob, fundingPoint2)
 }
 
 // acceptChannel is used to accept a single channel that comes across. This