From 60036aca1da16c0d2d052ffd1dac8df638bd63a4 Mon Sep 17 00:00:00 2001
From: Wilmer Paulino <wilmer.paulino@gmail.com>
Date: Wed, 31 Oct 2018 10:38:27 -0800
Subject: [PATCH] contractcourt/channel_arbitrator: prevent force closing same
 channel twice

In this commit, we prevent the ChainArbitrator from sending a force
close request for a channel if it has previously already sent one. We do
this to prevent blocking the caller of ForceCloseContract.
---
 contractcourt/channel_arbitrator.go      | 18 ++++++++++
 contractcourt/channel_arbitrator_test.go | 44 ++++++++++++++++++++++++
 2 files changed, 62 insertions(+)

diff --git a/contractcourt/channel_arbitrator.go b/contractcourt/channel_arbitrator.go
index 615eecc1..5e02e722 100644
--- a/contractcourt/channel_arbitrator.go
+++ b/contractcourt/channel_arbitrator.go
@@ -1,6 +1,7 @@
 package contractcourt
 
 import (
+	"errors"
 	"sync"
 	"sync/atomic"
 
@@ -21,6 +22,13 @@ const (
 	broadcastRedeemMultiplier = 2
 )
 
+var (
+	// errAlreadyForceClosed is an error returned when we attempt to force
+	// close a channel that's already in the process of doing so.
+	errAlreadyForceClosed = errors.New("channel is already in the " +
+		"process of being force closed")
+)
+
 // WitnessSubscription represents an intent to be notified once new witnesses
 // are discovered by various active contract resolvers. A contract resolver may
 // use this to be notified of when it can satisfy an incoming contract after we
@@ -1652,6 +1660,16 @@ func (c *ChannelArbitrator) channelAttendant(bestHeight int32) {
 		// channel. We'll
 		case closeReq := <-c.forceCloseReqs:
 			if c.state != StateDefault {
+				select {
+				case closeReq.closeTx <- nil:
+				case <-c.quit:
+				}
+
+				select {
+				case closeReq.errResp <- errAlreadyForceClosed:
+				case <-c.quit:
+				}
+
 				continue
 			}
 
diff --git a/contractcourt/channel_arbitrator_test.go b/contractcourt/channel_arbitrator_test.go
index 8def9805..aae59f2b 100644
--- a/contractcourt/channel_arbitrator_test.go
+++ b/contractcourt/channel_arbitrator_test.go
@@ -1143,3 +1143,47 @@ func TestChannelArbitratorEmptyResolutions(t *testing.T) {
 	}
 	chanArb.Stop()
 }
+
+// TestChannelArbitratorAlreadyForceClosed ensures that we cannot force close a
+// channel that is already in the process of doing so.
+func TestChannelArbitratorAlreadyForceClosed(t *testing.T) {
+	t.Parallel()
+
+	// We'll create the arbitrator and its backing log to signal that it's
+	// already in the process of being force closed.
+	log := &mockArbitratorLog{
+		state: StateCommitmentBroadcasted,
+	}
+	chanArb, _, err := createTestChannelArbitrator(log)
+	if err != nil {
+		t.Fatalf("unable to create ChannelArbitrator: %v", err)
+	}
+	if err := chanArb.Start(); err != nil {
+		t.Fatalf("unable to start ChannelArbitrator: %v", err)
+	}
+	defer chanArb.Stop()
+
+	// Then, we'll create a request to signal a force close request to the
+	// channel arbitrator.
+	errChan := make(chan error, 1)
+	respChan := make(chan *wire.MsgTx, 1)
+
+	select {
+	case chanArb.forceCloseReqs <- &forceCloseReq{
+		closeTx: respChan,
+		errResp: errChan,
+	}:
+	case <-chanArb.quit:
+	}
+
+	// Finally, we should ensure that we are not able to do so by seeing the
+	// expected errAlreadyForceClosed error.
+	select {
+	case err = <-errChan:
+		if err != errAlreadyForceClosed {
+			t.Fatalf("expected errAlreadyForceClosed, got %v", err)
+		}
+	case <-time.After(time.Second):
+		t.Fatal("expected to receive error response")
+	}
+}