lnwallet: perform sanity check on cooperative closure transacitons

This commit fixes a class of bug that currently exists within the
cooperative closure methods for the channel state machine. As an
example, due to the current hard coded fees, if one of the outputs
generated within the generated closure transaction has a negative
output, then the initiating node would gladly forward this to the
remote node. The remote node would then reject the closure as the
transaction is invalid. However, the act of completing the closure
would cause the remote node’s state machine to shift into a “closed”
state. As a result, any further closure attempts by the first node
(force or regular) would go unnoticed by the remote node.

We fix this issue by ensuring the transaction is “sane” before
initiating of completing a cooperative channel closure.

At test case has been added exercising the particular erroneous case
reported by “moli” on IRC.

lnwallet/channel.go

@@ -12,6 +12,7 @@ import (
 	"github.com/lightningnetwork/lnd/chainntnfs"
 	"github.com/lightningnetwork/lnd/channeldb"
 	"github.com/lightningnetwork/lnd/lnwire"
+	"github.com/roasbeef/btcd/blockchain"
 	"github.com/roasbeef/btcd/chaincfg/chainhash"
 
 	"github.com/roasbeef/btcd/btcec"
@@ -2238,7 +2239,14 @@ func (lc *LightningChannel) InitCooperativeClose() ([]byte, *chainhash.Hash, err
 		lc.channelState.OurBalance, lc.channelState.TheirBalance,
 		lc.channelState.OurDeliveryScript, lc.channelState.TheirDeliveryScript,
 		lc.channelState.IsInitiator)
-	closeTxSha := closeTx.TxHash()
+
+	// Ensure that the transaction doesn't explicitly validate any
+	// consensus rules such as being too big, or having any value with a
+	// negative output.
+	tx := btcutil.NewTx(closeTx)
+	if err := blockchain.CheckTransactionSanity(tx); err != nil {
+		return nil, nil, err
+	}
 
 	// Finally, sign the completed cooperative closure transaction. As the
 	// initiator we'll simply send our signature over to the remote party,
@@ -2254,6 +2262,7 @@ func (lc *LightningChannel) InitCooperativeClose() ([]byte, *chainhash.Hash, err
 	// channel closure has been initiated.
 	lc.status = channelClosing
 
+	closeTxSha := closeTx.TxHash()
 	return closeSig, &closeTxSha, nil
 }
 
@@ -2283,6 +2292,14 @@ func (lc *LightningChannel) CompleteCooperativeClose(remoteSig []byte) (*wire.Ms
 		lc.channelState.OurDeliveryScript, lc.channelState.TheirDeliveryScript,
 		lc.channelState.IsInitiator)
 
+	// Ensure that the transaction doesn't explicitly validate any
+	// consensus rules such as being too big, or having any value with a
+	// negative output.
+	tx := btcutil.NewTx(closeTx)
+	if err := blockchain.CheckTransactionSanity(tx); err != nil {
+		return nil, err
+	}
+
 	// With the transaction created, we can finally generate our half of
 	// the 2-of-2 multi-sig needed to redeem the funding output.
 	hashCache := txscript.NewTxSigHashes(closeTx)

lnwallet/channel_test.go

@@ -1287,3 +1287,39 @@ func TestCancelHTLC(t *testing.T) {
 			bobChannel.channelState.TheirBalance, expectedBalance)
 	}
 }
+
+func TestCloseTransactionSanityChecks(t *testing.T) {
+	// We'd like to ensure that transactions which aren't "sane" aren't
+	// accepted as valid coopertive channel closure transactions.
+
+	// We'll first create two test channels for our test case below.
+	aliceChannel, bobChannel, cleanUp, err := createTestChannels(5)
+	if err != nil {
+		t.Fatalf("unable to create test channels: %v", err)
+	}
+	defer cleanUp()
+
+	// In order to force the transaction to be "insane", we'll modify
+	// Alice's balance to be zero. With the current fee logic, this'll
+	// cause a negative output due to the hard coded fees. A transaction
+	// with a negative output is not "sane".
+	//
+	// TODO(roasbeef): modify test-case after dynamic fees
+	aliceChannel.channelState.OurBalance = 0
+	bobChannel.channelState.TheirBalance = 0
+
+	// Both Alice and Bob should reject a close attempt at this point since
+	// it will lead to Alice having a negative output within the commitment
+	// transaction.
+	_, _, err = aliceChannel.InitCooperativeClose()
+	if err == nil {
+		t.Fatalf("alice's closure transaction should have been rejected, " +
+			"but wasn't!")
+	}
+	var fakeSig []byte
+	_, err = bobChannel.CompleteCooperativeClose(fakeSig)
+	if err == nil {
+		t.Fatalf("bob's closure transaction should have been rejected, but " +
+			"wasn't!")
+	}
+}