From 69928228656e515e625146ea7ce6961621b4fc8c Mon Sep 17 00:00:00 2001
From: Olaoluwa Osuntokun <laolu32@gmail.com>
Date: Thu, 21 Dec 2017 11:43:07 +0100
Subject: [PATCH] lntest: CloseChannel will wait for both nodes to show chan as
 active if not force
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

In this commit, we modify the CloseChannel to wait for both nodes to
detect that channel as being active before we attempt to close it. This
should serve to reduce many of the flakes that we’ve been seeing on
travis which were caused by node A detecting the channel as active, but
node B not, leading to a test flake under certain timing conditions.
The new function uses the recently added WaitPredicate method.
---
 lntest/harness.go | 69 ++++++++++++++++++++++++++++++++++-------------
 1 file changed, 50 insertions(+), 19 deletions(-)

diff --git a/lntest/harness.go b/lntest/harness.go
index b8d5d88a4..fc52f8930 100644
--- a/lntest/harness.go
+++ b/lntest/harness.go
@@ -642,32 +642,63 @@ func (n *NetworkHarness) CloseChannel(ctx context.Context,
 		Index: cp.OutputIndex,
 	}
 
-	// If we are not force closing the channel, wait for channel to become
-	// active before attempting to close it.
-	numTries := 10
-CheckActive:
-	for i := 0; !force && i < numTries; i++ {
+	// We'll wait for *both* nodes to read the channel as active if we're
+	// performing a cooperative channel closure.
+	if !force {
+		timeout := time.Second * 15
 		listReq := &lnrpc.ListChannelsRequest{}
-		listResp, err := lnNode.ListChannels(ctx, listReq)
-		if err != nil {
-			return nil, nil, fmt.Errorf("unable fetch node's "+
-				"channels: %v", err)
-		}
 
-		for _, c := range listResp.Channels {
-			if c.ChannelPoint == chanPoint.String() && c.Active {
-				break CheckActive
+		// We define two helper functions, one two locate a particular
+		// channel, and the other to check if a channel is active or
+		// not.
+		filterChannel := func(node *HarnessNode,
+			op wire.OutPoint) (*lnrpc.ActiveChannel, error) {
+			listResp, err := node.ListChannels(ctx, listReq)
+			if err != nil {
+				return nil, err
+			}
+
+			for _, c := range listResp.Channels {
+				if c.ChannelPoint == op.String() {
+					return c, nil
+				}
+			}
+
+			return nil, fmt.Errorf("unable to find channel")
+		}
+		activeChanPredicate := func(node *HarnessNode) func() bool {
+			return func() bool {
+				channel, err := filterChannel(node, chanPoint)
+				if err != nil {
+				}
+
+				return channel.Active
 			}
 		}
 
-		if i == numTries-1 {
-			// Last iteration, and channel is still not active.
-			return nil, nil, fmt.Errorf("channel did not become " +
-				"active")
+		// Next, we'll fetch the target channel in order to get the
+		// harness node that'll be receiving the channel close request.
+		targetChan, err := filterChannel(lnNode, chanPoint)
+		if err != nil {
+			return nil, nil, err
+		}
+		receivingNode, err := n.LookUpNodeByPub(targetChan.RemotePubkey)
+		if err != nil {
+			return nil, nil, err
 		}
 
-		// Sleep, and try again.
-		time.Sleep(300 * time.Millisecond)
+		// Before proceeding, we'll ensure that the channel is active
+		// for both nodes.
+		err = WaitPredicate(activeChanPredicate(lnNode), timeout)
+		if err != nil {
+			return nil, nil, fmt.Errorf("channel of closing " +
+				"node not active in time")
+		}
+		err = WaitPredicate(activeChanPredicate(receivingNode), timeout)
+		if err != nil {
+			return nil, nil, fmt.Errorf("channel of receiving " +
+				"node not active in time")
+		}
 	}
 
 	closeReq := &lnrpc.CloseChannelRequest{