From 49b18b4d353b74578aba08fef5b59c4cbc0e5c6e Mon Sep 17 00:00:00 2001
From: Olaoluwa Osuntokun <laolu32@gmail.com>
Date: Tue, 6 Jul 2021 16:35:04 -0700
Subject: [PATCH 1/2] lnwallet: add test case to exercise MayAddOutgoingHtlc
 bug

As is, if the remote party proposes a min HTLC of 0 `mSat` to us, then
we won't ever be able to _send outgoing_ in the channel as the
`MayAddOutgoingHtlc` will attempt to add a zero-value HTLC, which isn't
allowed within the protocol.

The default channels created actually already use a min HTLC value of
zero within the tests, so this test case fails as is.
---
 lnwallet/channel_test.go | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go
index 6de8b2ac3..6f190ade7 100644
--- a/lnwallet/channel_test.go
+++ b/lnwallet/channel_test.go
@@ -9648,3 +9648,24 @@ func TestChannelSignedAckRegression(t *testing.T) {
 	err = aliceChannel.ReceiveNewCommitment(bobSig, bobHtlcSigs)
 	require.NoError(t, err)
 }
+
+// TestMayAddOutgoingHtlcZeroValue tests that if the minHTLC value of the
+// channel is zero, then the MayAddOutgoingHtlc doesn't exit early due to
+// running into a zero valued HTLC.
+func TestMayAddOutgoingHtlcZeroValue(t *testing.T) {
+	t.Parallel()
+
+	// The default channel created as a part of the test fixture already
+	// has a MinHTLC value of zero, so we don't need to do anything here
+	// other than create it.
+	aliceChannel, bobChannel, cleanUp, err := CreateTestChannels(
+		channeldb.SingleFunderTweaklessBit,
+	)
+	require.NoError(t, err)
+	defer cleanUp()
+
+	// The channels start out with a 50/50 balance, so both sides should be
+	// able to add an outgoing HTLC.
+	require.NoError(t, aliceChannel.MayAddOutgoingHtlc())
+	require.NoError(t, bobChannel.MayAddOutgoingHtlc())
+}

From af43a863cbd44f9590de103431fc28a7c952ed02 Mon Sep 17 00:00:00 2001
From: Olaoluwa Osuntokun <laolu32@gmail.com>
Date: Tue, 6 Jul 2021 16:37:20 -0700
Subject: [PATCH 2/2] lnwallet: ensure MayAddOutgoingHtlc doesn't add
 zero-value HTLCs

In this commit, in order to allow the test added in the prior commit to
pass, we'll increment the mockHTLCAmt value by 1 to ensure we never
attempt to add a zero value HTLC.

Fixes #5468
---
 lnwallet/channel.go | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/lnwallet/channel.go b/lnwallet/channel.go
index 1301d83f3..56991bd3f 100644
--- a/lnwallet/channel.go
+++ b/lnwallet/channel.go
@@ -4940,13 +4940,22 @@ func (lc *LightningChannel) MayAddOutgoingHtlc() error {
 	lc.Lock()
 	defer lc.Unlock()
 
+	// As this is a mock HTLC, we'll attempt to add the smallest possible
+	// HTLC permitted in the channel. However certain implementations may
+	// set this value to zero, so we'll catch that and increment things so
+	// we always use a non-zero value.
+	mockHtlcAmt := lc.channelState.LocalChanCfg.MinHTLC
+	if mockHtlcAmt == 0 {
+		mockHtlcAmt++
+	}
+
 	// Create a "mock" outgoing htlc, using the smallest amount we can add
 	// to the commitment so that we validate commitment slots rather than
 	// available balance, since our actual htlc amount is unknown at this
 	// stage.
 	pd := lc.htlcAddDescriptor(
 		&lnwire.UpdateAddHTLC{
-			Amount: lc.channelState.LocalChanCfg.MinHTLC,
+			Amount: mockHtlcAmt,
 		},
 		&channeldb.CircuitKey{},
 	)