From 5b4aa82667a665106ca5add6a29c64d06a4ee948 Mon Sep 17 00:00:00 2001
From: Olaoluwa Osuntokun <laolu32@gmail.com>
Date: Sun, 10 Dec 2017 16:10:44 -0800
Subject: [PATCH] lnwallet: don't sign new commitment if next revocation point
 is unknown
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

In this commit, we extend the initial check within SignNextCommitment
to bail out early if we don’t yet know the commitment point of the
remote party. This prevents a class of nil pointer panics if we attempt
to create a new state without yet having received the FundingLocked
message.
---
 lnwallet/channel.go      | 11 +++++++----
 lnwallet/channel_test.go | 27 +++++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/lnwallet/channel.go b/lnwallet/channel.go
index 47c980ef..b7570156 100644
--- a/lnwallet/channel.go
+++ b/lnwallet/channel.go
@@ -2930,10 +2930,13 @@ func (lc *LightningChannel) SignNextCommitment() (*btcec.Signature, []*btcec.Sig
 	lc.Lock()
 	defer lc.Unlock()
 
-	// If we're awaiting for an ACK to a commitment signature, then we're
-	// unable to create new states. Basically we don't want to advance the
-	// commitment chain by more than one at a time.
-	if lc.remoteCommitChain.hasUnackedCommitment() {
+	// If we're awaiting for an ACK to a commitment signature, or if we
+	// don't yet have the initial next revocation point of the remote
+	// party, then we're unable to create new states. Each time we create a
+	// new state, we consume a prior revocation point.
+	if lc.remoteCommitChain.hasUnackedCommitment() ||
+		lc.channelState.RemoteCurrentRevocation == nil {
+
 		return nil, nil, ErrNoWindow
 	}
 
diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go
index a53a7c15..8c2c5fba 100644
--- a/lnwallet/channel_test.go
+++ b/lnwallet/channel_test.go
@@ -3832,4 +3832,31 @@ func TestChanAvailableBandwidth(t *testing.T) {
 	// TODO(roasbeef): additional tests from diff starting conditions
 }
 
+// TestSignCommitmentFailNotLockedIn tests that a channel will not attempt to
+// create a new state if it doesn't yet know of the next revocation point for
+// the remote party.
+func TestSignCommitmentFailNotLockedIn(t *testing.T) {
+	t.Parallel()
+
+	// Create a test channel which will be used for the duration of this
+	// unittest. The channel will be funded evenly with Alice having 5 BTC,
+	// and Bob having 5 BTC.
+	aliceChannel, _, cleanUp, err := createTestChannels(1)
+	if err != nil {
+		t.Fatalf("unable to create test channels: %v", err)
+	}
+	defer cleanUp()
+
+	// Next, we'll modify Alice's internal state to omit knowledge of Bob's
+	// next revocation point.
+	aliceChannel.channelState.RemoteCurrentRevocation = nil
+
+	// If we now try to initiate a state update, then it should fail as
+	// Alice is unable to actually create a new state.
+	_, _, err = aliceChannel.SignNextCommitment()
+	if err != ErrNoWindow {
+		t.Fatalf("expected ErrNoWindow, instead have: %v", err)
+	}
+}
+
 // TODO(roasbeef): testing.Quick test case for retrans!!!