From 104b7a09dba87aa57cf53bcf3c19b50650591352 Mon Sep 17 00:00:00 2001
From: yyforyongyu <yy2452@columbia.edu>
Date: Fri, 20 Aug 2021 17:33:42 +0800
Subject: [PATCH] itest: fix inheritance when creating timeout ctxt

This commit fixes the issue where a wrong context being inherited to
create a timeout context. When a parent context timed out, all its
children contexts also timed out, even the children contexts had a
larger timeout value. This means it only makes sense to inherite from a
parent when its children have smaller timeout value. Given the setup of
the itest, all the timeout contexts need to be created from a context
background(hence no timeout on the parent) unless there's an explicit
timeout bound we want to set.
---
 lntest/harness.go                             | 22 +++++++++----------
 lntest/itest/assertions.go                    |  6 ++---
 lntest/itest/lnd_forward_interceptor_test.go  |  9 ++++----
 lntest/itest/lnd_hold_persistence_test.go     |  4 ++--
 ...d_multi-hop_htlc_local_chain_claim_test.go |  5 +----
 ...ulti-hop_htlc_receiver_chain_claim_test.go |  5 +----
 ..._multi-hop_htlc_remote_chain_claim_test.go |  5 +----
 lntest/itest/lnd_multi-hop_test.go            |  8 +++++--
 lntest/itest/lnd_network_test.go              |  4 ----
 lntest/itest/lnd_payment_test.go              |  8 +++----
 lntest/itest/lnd_routing_test.go              |  6 ++---
 lntest/itest/lnd_single_hop_invoice_test.go   |  6 ++---
 lntest/itest/utils.go                         |  6 ++---
 13 files changed, 40 insertions(+), 54 deletions(-)

diff --git a/lntest/harness.go b/lntest/harness.go
index 9029c1309..061f77beb 100644
--- a/lntest/harness.go
+++ b/lntest/harness.go
@@ -550,7 +550,7 @@ tryconnect:
 // peers list, or until the 15s timeout expires.
 func (n *NetworkHarness) EnsureConnected(t *testing.T, a, b *HarnessNode) {
 	ctxb := context.Background()
-	ctx, cancel := context.WithTimeout(ctxb, DefaultTimeout)
+	ctx, cancel := context.WithTimeout(ctxb, DefaultTimeout*2)
 	defer cancel()
 
 	// errConnectionRequested is used to signal that a connection was
@@ -559,9 +559,7 @@ func (n *NetworkHarness) EnsureConnected(t *testing.T, a, b *HarnessNode) {
 	errConnectionRequested := errors.New("connection request in progress")
 
 	tryConnect := func(a, b *HarnessNode) error {
-		ctxt, cancel := context.WithTimeout(ctx, DefaultTimeout)
-		defer cancel()
-		bInfo, err := b.GetInfo(ctxt, &lnrpc.GetInfoRequest{})
+		bInfo, err := b.GetInfo(ctx, &lnrpc.GetInfoRequest{})
 		if err != nil {
 			return err
 		}
@@ -639,9 +637,7 @@ func (n *NetworkHarness) EnsureConnected(t *testing.T, a, b *HarnessNode) {
 		// If node B is seen in the ListPeers response from node A,
 		// then we can exit early as the connection has been fully
 		// established.
-		ctxt, cancel := context.WithTimeout(ctx, DefaultTimeout)
-		defer cancel()
-		resp, err := b.ListPeers(ctxt, &lnrpc.ListPeersRequest{})
+		resp, err := b.ListPeers(ctx, &lnrpc.ListPeersRequest{})
 		if err != nil {
 			return false
 		}
@@ -996,8 +992,10 @@ func (n *NetworkHarness) OpenChannel(srcNode, destNode *HarnessNode,
 	p OpenChannelParams) (lnrpc.Lightning_OpenChannelClient, error) {
 
 	ctxb := context.Background()
-	ctx, cancel := context.WithTimeout(ctxb, ChannelOpenTimeout)
-	defer cancel()
+	// The cancel is intentionally left out here because the returned
+	// item(open channel client) relies on the context being active. This
+	// will be fixed once we finish refactoring the NetworkHarness.
+	ctx, _ := context.WithTimeout(ctxb, ChannelOpenTimeout) // nolint: govet
 
 	// Wait until srcNode and destNode have the latest chain synced.
 	// Otherwise, we may run into a check within the funding manager that
@@ -1179,8 +1177,10 @@ func (n *NetworkHarness) CloseChannel(lnNode *HarnessNode,
 	force bool) (lnrpc.Lightning_CloseChannelClient, *chainhash.Hash, error) {
 
 	ctxb := context.Background()
-	ctx, cancel := context.WithTimeout(ctxb, ChannelCloseTimeout)
-	defer cancel()
+	// The cancel is intentionally left out here because the returned
+	// item(close channel client) relies on the context being active. This
+	// will be fixed once we finish refactoring the NetworkHarness.
+	ctx, _ := context.WithTimeout(ctxb, ChannelCloseTimeout) // nolint: govet
 
 	// Create a channel outpoint that we can use to compare to channels
 	// from the ListChannelsResponse.
diff --git a/lntest/itest/assertions.go b/lntest/itest/assertions.go
index dbfc02490..40c5bdc63 100644
--- a/lntest/itest/assertions.go
+++ b/lntest/itest/assertions.go
@@ -288,9 +288,8 @@ func assertChannelClosed(ctx context.Context, t *harnessTest,
 
 	// If the channel appears in list channels, ensure that its state
 	// contains ChanStatusCoopBroadcasted.
-	ctxt, _ := context.WithTimeout(ctx, defaultTimeout)
 	listChansRequest := &lnrpc.ListChannelsRequest{}
-	listChansResp, err := node.ListChannels(ctxt, listChansRequest)
+	listChansResp, err := node.ListChannels(ctx, listChansRequest)
 	require.NoError(t.t, err, "unable to query for list channels")
 
 	for _, channel := range listChansResp.Channels {
@@ -309,9 +308,8 @@ func assertChannelClosed(ctx context.Context, t *harnessTest,
 
 	// At this point, the channel should now be marked as being in the
 	// state of "waiting close".
-	ctxt, _ = context.WithTimeout(ctx, defaultTimeout)
 	pendingChansRequest := &lnrpc.PendingChannelsRequest{}
-	pendingChanResp, err := node.PendingChannels(ctxt, pendingChansRequest)
+	pendingChanResp, err := node.PendingChannels(ctx, pendingChansRequest)
 	require.NoError(t.t, err, "unable to query for pending channels")
 
 	var found bool
diff --git a/lntest/itest/lnd_forward_interceptor_test.go b/lntest/itest/lnd_forward_interceptor_test.go
index 2698cf4ca..26af2150b 100644
--- a/lntest/itest/lnd_forward_interceptor_test.go
+++ b/lntest/itest/lnd_forward_interceptor_test.go
@@ -67,8 +67,8 @@ func testForwardInterceptor(net *lntest.NetworkHarness, t *harnessTest) {
 	testContext.waitForChannels()
 
 	// Connect the interceptor.
-	ctx := context.Background()
-	ctxt, cancelInterceptor := context.WithTimeout(ctx, defaultTimeout)
+	ctxb := context.Background()
+	ctxt, cancelInterceptor := context.WithTimeout(ctxb, defaultTimeout)
 	interceptor, err := testContext.bob.RouterClient.HtlcInterceptor(ctxt)
 	require.NoError(t.t, err, "failed to create HtlcInterceptor")
 
@@ -230,8 +230,7 @@ func testForwardInterceptor(net *lntest.NetworkHarness, t *harnessTest) {
 	restartAlice, err := net.SuspendNode(alice)
 	require.NoError(t.t, err, "failed to suspend alice")
 
-	ctx = context.Background()
-	ctxt, cancelInterceptor = context.WithTimeout(ctx, defaultTimeout)
+	ctxt, cancelInterceptor = context.WithTimeout(ctxb, defaultTimeout)
 	defer cancelInterceptor()
 	interceptor, err = testContext.bob.RouterClient.HtlcInterceptor(ctxt)
 	require.NoError(t.t, err, "failed to create HtlcInterceptor")
@@ -259,7 +258,7 @@ func testForwardInterceptor(net *lntest.NetworkHarness, t *harnessTest) {
 	}()
 
 	err = wait.Predicate(func() bool {
-		channels, err := bob.ListChannels(ctx, &lnrpc.ListChannelsRequest{
+		channels, err := bob.ListChannels(ctxt, &lnrpc.ListChannelsRequest{
 			ActiveOnly: true, Peer: alice.PubKey[:],
 		})
 		return err == nil && len(channels.Channels) > 0
diff --git a/lntest/itest/lnd_hold_persistence_test.go b/lntest/itest/lnd_hold_persistence_test.go
index 4c065cc7e..a9b570a03 100644
--- a/lntest/itest/lnd_hold_persistence_test.go
+++ b/lntest/itest/lnd_hold_persistence_test.go
@@ -165,7 +165,7 @@ func testHoldInvoicePersistence(net *lntest.NetworkHarness, t *harnessTest) {
 		req := &lnrpc.ListPaymentsRequest{
 			IncludeIncomplete: true,
 		}
-		ctxt, _ = context.WithTimeout(ctxt, defaultTimeout)
+		ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
 		paymentsResp, err := net.Alice.ListPayments(ctxt, req)
 		if err != nil {
 			return fmt.Errorf("error when obtaining payments: %v",
@@ -368,7 +368,7 @@ func testHoldInvoicePersistence(net *lntest.NetworkHarness, t *harnessTest) {
 	req := &lnrpc.ListPaymentsRequest{
 		IncludeIncomplete: true,
 	}
-	ctxt, _ = context.WithTimeout(ctxt, defaultTimeout)
+	ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
 	paymentsResp, err := net.Alice.ListPayments(ctxt, req)
 	if err != nil {
 		t.Fatalf("error when obtaining Alice payments: %v", err)
diff --git a/lntest/itest/lnd_multi-hop_htlc_local_chain_claim_test.go b/lntest/itest/lnd_multi-hop_htlc_local_chain_claim_test.go
index 7264a1ccd..b21f2c2b7 100644
--- a/lntest/itest/lnd_multi-hop_htlc_local_chain_claim_test.go
+++ b/lntest/itest/lnd_multi-hop_htlc_local_chain_claim_test.go
@@ -298,9 +298,6 @@ func testMultiHopHtlcLocalChainClaim(net *lntest.NetworkHarness, t *harnessTest,
 
 	// Finally, check that the Alice's payment is correctly marked
 	// succeeded.
-	ctxt, _ = context.WithTimeout(ctxt, defaultTimeout)
-	err = checkPaymentStatus(
-		ctxt, alice, preimage, lnrpc.Payment_SUCCEEDED,
-	)
+	err = checkPaymentStatus(alice, preimage, lnrpc.Payment_SUCCEEDED)
 	require.NoError(t.t, err)
 }
diff --git a/lntest/itest/lnd_multi-hop_htlc_receiver_chain_claim_test.go b/lntest/itest/lnd_multi-hop_htlc_receiver_chain_claim_test.go
index 434a9df1d..2609d2f59 100644
--- a/lntest/itest/lnd_multi-hop_htlc_receiver_chain_claim_test.go
+++ b/lntest/itest/lnd_multi-hop_htlc_receiver_chain_claim_test.go
@@ -224,10 +224,7 @@ func testMultiHopReceiverChainClaim(net *lntest.NetworkHarness, t *harnessTest,
 
 	// Finally, check that the Alice's payment is correctly marked
 	// succeeded.
-	ctxt, _ = context.WithTimeout(ctxt, defaultTimeout)
-	err = checkPaymentStatus(
-		ctxt, alice, preimage, lnrpc.Payment_SUCCEEDED,
-	)
+	err = checkPaymentStatus(alice, preimage, lnrpc.Payment_SUCCEEDED)
 	require.NoError(t.t, err)
 
 	// We'll close out the channel between Alice and Bob, then shutdown
diff --git a/lntest/itest/lnd_multi-hop_htlc_remote_chain_claim_test.go b/lntest/itest/lnd_multi-hop_htlc_remote_chain_claim_test.go
index c18af58df..af401cd6d 100644
--- a/lntest/itest/lnd_multi-hop_htlc_remote_chain_claim_test.go
+++ b/lntest/itest/lnd_multi-hop_htlc_remote_chain_claim_test.go
@@ -262,9 +262,6 @@ func testMultiHopHtlcRemoteChainClaim(net *lntest.NetworkHarness, t *harnessTest
 
 	// Finally, check that the Alice's payment is correctly marked
 	// succeeded.
-	ctxt, _ = context.WithTimeout(ctxt, defaultTimeout)
-	err = checkPaymentStatus(
-		ctxt, alice, preimage, lnrpc.Payment_SUCCEEDED,
-	)
+	err = checkPaymentStatus(alice, preimage, lnrpc.Payment_SUCCEEDED)
 	require.NoError(t.t, err)
 }
diff --git a/lntest/itest/lnd_multi-hop_test.go b/lntest/itest/lnd_multi-hop_test.go
index bf245d7b1..8d0cdd1e3 100644
--- a/lntest/itest/lnd_multi-hop_test.go
+++ b/lntest/itest/lnd_multi-hop_test.go
@@ -148,8 +148,12 @@ func waitForInvoiceAccepted(t *harnessTest, node *lntest.HarnessNode,
 
 // checkPaymentStatus asserts that the given node list a payment with the given
 // preimage has the expected status.
-func checkPaymentStatus(ctxt context.Context, node *lntest.HarnessNode,
-	preimage lntypes.Preimage, status lnrpc.Payment_PaymentStatus) error {
+func checkPaymentStatus(node *lntest.HarnessNode, preimage lntypes.Preimage,
+	status lnrpc.Payment_PaymentStatus) error {
+
+	ctxb := context.Background()
+	ctxt, cancel := context.WithTimeout(ctxb, defaultTimeout)
+	defer cancel()
 
 	req := &lnrpc.ListPaymentsRequest{
 		IncludeIncomplete: true,
diff --git a/lntest/itest/lnd_network_test.go b/lntest/itest/lnd_network_test.go
index 49888040d..63f401859 100644
--- a/lntest/itest/lnd_network_test.go
+++ b/lntest/itest/lnd_network_test.go
@@ -69,10 +69,6 @@ func assertTimeoutError(ctxt context.Context, t *harnessTest,
 
 	t.t.Helper()
 
-	// Create a context with a timeout value.
-	ctxt, cancel := context.WithTimeout(ctxt, defaultTimeout)
-	defer cancel()
-
 	err := connect(ctxt, node, req)
 
 	// a DeadlineExceeded error will appear in the context if the above
diff --git a/lntest/itest/lnd_payment_test.go b/lntest/itest/lnd_payment_test.go
index 1c9b02858..5cf3330c6 100644
--- a/lntest/itest/lnd_payment_test.go
+++ b/lntest/itest/lnd_payment_test.go
@@ -31,7 +31,7 @@ func testListPayments(net *lntest.NetworkHarness, t *harnessTest) {
 
 	// Check that there are no payments before test.
 	reqInit := &lnrpc.ListPaymentsRequest{}
-	ctxt, _ = context.WithTimeout(ctxt, defaultTimeout)
+	ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
 	paymentsRespInit, err := net.Alice.ListPayments(ctxt, reqInit)
 	if err != nil {
 		t.Fatalf("error when obtaining Alice payments: %v", err)
@@ -92,7 +92,7 @@ func testListPayments(net *lntest.NetworkHarness, t *harnessTest) {
 	// Grab Alice's list of payments, she should show the existence of
 	// exactly one payment.
 	req := &lnrpc.ListPaymentsRequest{}
-	ctxt, _ = context.WithTimeout(ctxt, defaultTimeout)
+	ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
 	paymentsResp, err := net.Alice.ListPayments(ctxt, req)
 	if err != nil {
 		t.Fatalf("error when obtaining Alice payments: %v", err)
@@ -138,7 +138,7 @@ func testListPayments(net *lntest.NetworkHarness, t *harnessTest) {
 
 	// Delete all payments from Alice. DB should have no payments.
 	delReq := &lnrpc.DeleteAllPaymentsRequest{}
-	ctxt, _ = context.WithTimeout(ctxt, defaultTimeout)
+	ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
 	_, err = net.Alice.DeleteAllPayments(ctxt, delReq)
 	if err != nil {
 		t.Fatalf("Can't delete payments at the end: %v", err)
@@ -146,7 +146,7 @@ func testListPayments(net *lntest.NetworkHarness, t *harnessTest) {
 
 	// Check that there are no payments after test.
 	listReq := &lnrpc.ListPaymentsRequest{}
-	ctxt, _ = context.WithTimeout(ctxt, defaultTimeout)
+	ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
 	paymentsResp, err = net.Alice.ListPayments(ctxt, listReq)
 	if err != nil {
 		t.Fatalf("error when obtaining Alice payments: %v", err)
diff --git a/lntest/itest/lnd_routing_test.go b/lntest/itest/lnd_routing_test.go
index 130e107f6..e69354151 100644
--- a/lntest/itest/lnd_routing_test.go
+++ b/lntest/itest/lnd_routing_test.go
@@ -305,7 +305,7 @@ func testSingleHopSendToRouteCase(net *lntest.NetworkHarness, t *harnessTest,
 
 	// Verify that the payment's from Carol's PoV have the correct payment
 	// hash and amount.
-	ctxt, _ = context.WithTimeout(ctxt, defaultTimeout)
+	ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
 	paymentsResp, err := carol.ListPayments(
 		ctxt, &lnrpc.ListPaymentsRequest{},
 	)
@@ -385,7 +385,7 @@ func testSingleHopSendToRouteCase(net *lntest.NetworkHarness, t *harnessTest,
 
 	// Verify that the invoices's from Dave's PoV have the correct payment
 	// hash and amount.
-	ctxt, _ = context.WithTimeout(ctxt, defaultTimeout)
+	ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
 	invoicesResp, err := dave.ListInvoices(
 		ctxt, &lnrpc.ListInvoiceRequest{},
 	)
@@ -1118,7 +1118,7 @@ func testUpdateChannelPolicyForPrivateChannel(net *lntest.NetworkHarness,
 
 	// Check that Alice did make the payment with two HTLCs, one failed and
 	// one succeeded.
-	ctxt, _ = context.WithTimeout(ctxt, defaultTimeout)
+	ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
 	paymentsResp, err := net.Alice.ListPayments(
 		ctxt, &lnrpc.ListPaymentsRequest{},
 	)
diff --git a/lntest/itest/lnd_single_hop_invoice_test.go b/lntest/itest/lnd_single_hop_invoice_test.go
index 6bc2780b8..b8a347d7c 100644
--- a/lntest/itest/lnd_single_hop_invoice_test.go
+++ b/lntest/itest/lnd_single_hop_invoice_test.go
@@ -78,7 +78,7 @@ func testSingleHopInvoice(net *lntest.NetworkHarness, t *harnessTest) {
 	payHash := &lnrpc.PaymentHash{
 		RHash: invoiceResp.RHash,
 	}
-	ctxt, _ = context.WithTimeout(ctxt, defaultTimeout)
+	ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
 	dbInvoice, err := net.Bob.LookupInvoice(ctxt, payHash)
 	if err != nil {
 		t.Fatalf("unable to lookup invoice: %v", err)
@@ -105,7 +105,7 @@ func testSingleHopInvoice(net *lntest.NetworkHarness, t *harnessTest) {
 		Memo:  "test3",
 		Value: paymentAmt,
 	}
-	ctxt, _ = context.WithTimeout(ctxt, defaultTimeout)
+	ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
 	invoiceResp, err = net.Bob.AddInvoice(ctxt, invoice)
 	if err != nil {
 		t.Fatalf("unable to add invoice: %v", err)
@@ -198,7 +198,7 @@ func testSingleHopInvoice(net *lntest.NetworkHarness, t *harnessTest) {
 		RouteHints: hints,
 	}
 
-	ctxt, _ = context.WithTimeout(ctxt, defaultTimeout)
+	ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
 	invoiceResp, err = net.Bob.AddInvoice(ctxt, invoice)
 	if err != nil {
 		t.Fatalf("unable to add invoice: %v", err)
diff --git a/lntest/itest/utils.go b/lntest/itest/utils.go
index eb9efc6d1..5fbed3e07 100644
--- a/lntest/itest/utils.go
+++ b/lntest/itest/utils.go
@@ -98,8 +98,7 @@ func completePaymentRequests(client lnrpc.LightningClient,
 	// the send before cancelling the request. We wait for the number of
 	// updates to one of our channels has increased before we return.
 	err = wait.Predicate(func() bool {
-		ctxt, _ := context.WithTimeout(ctx, defaultTimeout)
-		newListResp, err := client.ListChannels(ctxt, req)
+		newListResp, err := client.ListChannels(ctx, req)
 		if err != nil {
 			return false
 		}
@@ -337,8 +336,7 @@ func waitForNodeBlockHeight(node *lntest.HarnessNode, height int32) error {
 
 	var predErr error
 	err := wait.Predicate(func() bool {
-		ctxt, _ := context.WithTimeout(ctx, defaultTimeout)
-		info, err := node.GetInfo(ctxt, &lnrpc.GetInfoRequest{})
+		info, err := node.GetInfo(ctx, &lnrpc.GetInfoRequest{})
 		if err != nil {
 			predErr = err
 			return false