lnwallet: fixing unit tests to properly handle new receive validation

This commit fixes the TestMaxAcceptedHTLCs, TestMaxPendingAmount,
TestMinHTLC, & TestChanReserve unit tests to pass with the new
ReceiveHTLC logic. Instead of asserting specific failures upon
receiving a new commitment signature, the various assertions were
moved to assert on the error returned from ReceiveHTLC.

lnwallet/channel_test.go

@@ -5462,23 +5462,26 @@ func TestMaxAcceptedHTLCs(t *testing.T) {
 	defer cleanUp()
 
 	// One over the maximum number of HTLCs that either can accept.
-	const numHTLCs = 20
+	const numHTLCs = 12
-	const numHTLCsReceived = 12
 
-	// Set the remote's required MaxAcceptedHtlcs. This means that alice
+	// Set the remote's required MaxAcceptedHtlcs. This means that Alice
 	// can only offer the remote up to numHTLCs HTLCs.
 	aliceChannel.channelState.LocalChanCfg.MaxAcceptedHtlcs = numHTLCs
 	bobChannel.channelState.RemoteChanCfg.MaxAcceptedHtlcs = numHTLCs
 
 	// Similarly, set the remote config's MaxAcceptedHtlcs. This means
-	// that the remote will be aware that Alice will only accept up to
+	// that the remote will be aware that Bob will only accept up to
-	// numHTLCsRecevied at a time.
+	// numHTLCs at a time.
-	aliceChannel.channelState.RemoteChanCfg.MaxAcceptedHtlcs = numHTLCsReceived
+	aliceChannel.channelState.RemoteChanCfg.MaxAcceptedHtlcs = numHTLCs
-	bobChannel.channelState.LocalChanCfg.MaxAcceptedHtlcs = numHTLCsReceived
+	bobChannel.channelState.LocalChanCfg.MaxAcceptedHtlcs = numHTLCs
 
 	// Each HTLC amount is 0.1 BTC.
 	htlcAmt := lnwire.NewMSatFromSatoshis(0.1 * btcutil.SatoshiPerBitcoin)
 
+	// htlcID is used to keep track of the HTLC that Bob will fail back to
+	// Alice.
+	var htlcID uint64
+
 	// Send the maximum allowed number of HTLCs.
 	for i := 0; i < numHTLCs; i++ {
 		htlc, _ := createHTLC(i, htlcAmt)
@@ -5488,6 +5491,13 @@ func TestMaxAcceptedHTLCs(t *testing.T) {
 		if _, err := bobChannel.ReceiveHTLC(htlc); err != nil {
 			t.Fatalf("unable to recv htlc: %v", err)
 		}
+
+		// Just assign htlcID to the last received HTLC.
+		htlcID = htlc.ID
+	}
+
+	if err := ForceStateTransition(aliceChannel, bobChannel); err != nil {
+		t.Fatalf("unable to transition state: %v", err)
 	}
 
 	// The next HTLC should fail with ErrMaxHTLCNumber.
@@ -5497,13 +5507,57 @@ func TestMaxAcceptedHTLCs(t *testing.T) {
 		t.Fatalf("expected ErrMaxHTLCNumber, instead received: %v", err)
 	}
 
-	// After receiving the next HTLC, next state transition should fail
+	// Receiving the next HTLC should fail.
-	// with ErrMaxHTLCNumber.
+	if _, err := bobChannel.ReceiveHTLC(htlc); err != ErrMaxHTLCNumber {
+		t.Fatalf("expected ErrMaxHTLCNumber, instead received: %v", err)
+	}
+
+	// Bob will fail the htlc specified by htlcID and then force a state
+	// transition.
+	err = bobChannel.FailHTLC(htlcID, []byte{}, nil, nil, nil)
+	if err != nil {
+		t.Fatalf("unable to fail htlc: %v", err)
+	}
+
+	if err := aliceChannel.ReceiveFailHTLC(htlcID, []byte{}); err != nil {
+		t.Fatalf("unable to receive fail htlc: %v", err)
+	}
+
+	if err := ForceStateTransition(bobChannel, aliceChannel); err != nil {
+		t.Fatalf("unable to transition state: %v", err)
+	}
+
+	// Bob should succeed in adding a new HTLC since a previous HTLC was just
+	// failed. We use numHTLCs here since the previous AddHTLC with this index
+	// failed.
+	htlc, _ = createHTLC(numHTLCs, htlcAmt)
+	if _, err := aliceChannel.AddHTLC(htlc, nil); err != nil {
+		t.Fatalf("unable to add htlc: %v", err)
+	}
 	if _, err := bobChannel.ReceiveHTLC(htlc); err != nil {
 		t.Fatalf("unable to recv htlc: %v", err)
 	}
-	err = ForceStateTransition(aliceChannel, bobChannel)
+
-	if err != ErrMaxHTLCNumber {
+	// Add a commitment to Bob's commitment chain.
+	aliceSig, aliceHtlcSigs, _, err := aliceChannel.SignNextCommitment()
+	if err != nil {
+		t.Fatalf("unable to sign next commitment: %v", err)
+	}
+	err = bobChannel.ReceiveNewCommitment(aliceSig, aliceHtlcSigs)
+	if err != nil {
+		t.Fatalf("unable to recv new commitment: %v", err)
+	}
+
+	// The next HTLC should fail with ErrMaxHTLCNumber. The index is incremented
+	// by one.
+	htlc, _ = createHTLC(numHTLCs+1, htlcAmt)
+	if _, err = aliceChannel.AddHTLC(htlc, nil); err != ErrMaxHTLCNumber {
+		t.Fatalf("expected ErrMaxHTLCNumber, instead received: %v", err)
+	}
+
+	// Likewise, Bob should not be able to receive this HTLC if Alice can't
+	// add it.
+	if _, err := bobChannel.ReceiveHTLC(htlc); err != ErrMaxHTLCNumber {
 		t.Fatalf("expected ErrMaxHTLCNumber, instead received: %v", err)
 	}
 }
@@ -5556,13 +5610,8 @@ func TestMaxPendingAmount(t *testing.T) {
 		t.Fatalf("expected ErrMaxPendingAmount, instead received: %v", err)
 	}
 
-	// And also Bob shouldn't be accepting this HTLC in the next state
+	// And also Bob shouldn't be accepting this HTLC upon calling ReceiveHTLC.
-	// transition.
+	if _, err := bobChannel.ReceiveHTLC(htlc); err != ErrMaxPendingAmount {
-	if _, err := bobChannel.ReceiveHTLC(htlc); err != nil {
-		t.Fatalf("unable to recv htlc: %v", err)
-	}
-	err = ForceStateTransition(aliceChannel, bobChannel)
-	if err != ErrMaxPendingAmount {
 		t.Fatalf("expected ErrMaxPendingAmount, instead received: %v", err)
 	}
 }
@@ -5684,12 +5733,8 @@ func TestChanReserve(t *testing.T) {
 		t.Fatalf("expected ErrBelowChanReserve, instead received: %v", err)
 	}
 
-	// Alice will reject this htlc when a state transition is attempted.
+	// Alice will reject this htlc upon receiving the htlc.
-	if _, err := aliceChannel.ReceiveHTLC(htlc); err != nil {
+	if _, err := aliceChannel.ReceiveHTLC(htlc); err != ErrBelowChanReserve {
-		t.Fatalf("unable to recv htlc: %v", err)
-	}
-	err = ForceStateTransition(aliceChannel, bobChannel)
-	if err != ErrBelowChanReserve {
 		t.Fatalf("expected ErrBelowChanReserve, instead received: %v", err)
 	}
 
@@ -5731,13 +5776,8 @@ func TestChanReserve(t *testing.T) {
 		t.Fatalf("expected ErrBelowChanReserve, instead received: %v", err)
 	}
 
-	// Likewise, Bob will reject a state transition after this htlc is
+	// Likewise, Bob will reject receiving the htlc because of the same reason.
-	// received, of the same reason.
+	if _, err := bobChannel.ReceiveHTLC(htlc); err != ErrBelowChanReserve {
-	if _, err := bobChannel.ReceiveHTLC(htlc); err != nil {
-		t.Fatalf("unable to recv htlc: %v", err)
-	}
-	err = ForceStateTransition(aliceChannel, bobChannel)
-	if err != ErrBelowChanReserve {
 		t.Fatalf("expected ErrBelowChanReserve, instead received: %v", err)
 	}
 
@@ -5857,13 +5897,9 @@ func TestMinHTLC(t *testing.T) {
 		t.Fatalf("expected ErrBelowMinHTLC, instead received: %v", err)
 	}
 
-	// Bob will receive this HTLC, but reject the next state update, since
+	// Bob will receive this HTLC, but reject the next received htlc, since
 	// the htlc is too small.
 	_, err = bobChannel.ReceiveHTLC(htlc)
-	if err != nil {
-		t.Fatalf("error receiving htlc: %v", err)
-	}
-	err = ForceStateTransition(aliceChannel, bobChannel)
 	if err != ErrBelowMinHTLC {
 		t.Fatalf("expected ErrBelowMinHTLC, instead received: %v", err)
 	}