multi: thread bandwidth check amount down to MayAddOutgoingHtlc

Pass htlc amount down to the channel so that we don't need to rely
on minHtlc (and pad it when the channel sets a 0 min htlc). Update
test to just check some sane values since we're no longer relying
on minHtlc amount at all.

htlcswitch/interfaces.go

@@ -102,8 +102,9 @@ type ChannelUpdateHandler interface {
 	EligibleToForward() bool
 
 	// MayAddOutgoingHtlc returns an error if we may not add an outgoing
-	// htlc to the channel.
-	MayAddOutgoingHtlc() error
+	// htlc to the channel, taking the amount of the htlc to add as a
+	// parameter.
+	MayAddOutgoingHtlc(lnwire.MilliSatoshi) error
 
 	// ShutdownIfChannelClean shuts the link down if the channel state is
 	// clean. This can be used with dynamic commitment negotiation or coop

htlcswitch/link.go

@@ -2203,10 +2203,12 @@ func (l *channelLink) Bandwidth() lnwire.MilliSatoshi {
 	return l.channel.AvailableBalance()
 }
 
-// MayAddOutgoingHtlc indicates whether we may add any more outgoing htlcs to
-// this channel.
-func (l *channelLink) MayAddOutgoingHtlc() error {
-	return l.channel.MayAddOutgoingHtlc()
+// MayAddOutgoingHtlc indicates whether we can add an outgoing htlc with the
+// amount provided to the link. This check does not reserve a space, since
+// forwards or other payments may use the available slot, so it should be
+// considered best-effort.
+func (l *channelLink) MayAddOutgoingHtlc(amt lnwire.MilliSatoshi) error {
+	return l.channel.MayAddOutgoingHtlc(amt)
 }
 
 // getDustSum is a wrapper method that calls the underlying channel's dust sum

htlcswitch/mock.go

@@ -784,7 +784,7 @@ func (f *mockChannelLink) Peer() lnpeer.Peer                            { return
 func (f *mockChannelLink) ChannelPoint() *wire.OutPoint                 { return &wire.OutPoint{} }
 func (f *mockChannelLink) Stop()                                        {}
 func (f *mockChannelLink) EligibleToForward() bool                      { return f.eligible }
-func (f *mockChannelLink) MayAddOutgoingHtlc() error                    { return nil }
+func (f *mockChannelLink) MayAddOutgoingHtlc(lnwire.MilliSatoshi) error { return nil }
 func (f *mockChannelLink) ShutdownIfChannelClean() error                { return nil }
 func (f *mockChannelLink) setLiveShortChanID(sid lnwire.ShortChannelID) { f.shortChanID = sid }
 func (f *mockChannelLink) UpdateShortChanID() (lnwire.ShortChannelID, error) {

lnwallet/channel.go

@@ -5038,19 +5038,29 @@ func (lc *LightningChannel) GetDustSum(remote bool) lnwire.MilliSatoshi {
 }
 
 // MayAddOutgoingHtlc validates whether we can add an outgoing htlc to this
-// channel. We don't have a value or circuit for this htlc, because we just
-// want to test that we have slots for a potential htlc so we use a "mock"
-// htlc to validate a potential commitment state with one more outgoing htlc.
-func (lc *LightningChannel) MayAddOutgoingHtlc() error {
+// channel. We don't have a circuit for this htlc, because we just want to test
+// that we have slots for a potential htlc so we use a "mock" htlc to validate
+// a potential commitment state with one more outgoing htlc. If a zero htlc
+// amount is provided, we'll attempt to add the smallest possible htlc to the
+// channel (either the minimum htlc, or 1 sat).
+func (lc *LightningChannel) MayAddOutgoingHtlc(amt lnwire.MilliSatoshi) 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 {
+	var mockHtlcAmt lnwire.MilliSatoshi
+	switch {
+	// If the caller specifically set an amount, we use it.
+	case amt != 0:
+		mockHtlcAmt = amt
+
+	// In absence of a specific amount, we want to use minimum htlc value
+	// for the channel. However certain implementations may set this value
+	// to zero, so we only use this value if it is non-zero.
+	case lc.channelState.LocalChanCfg.MinHTLC != 0:
+		mockHtlcAmt = lc.channelState.LocalChanCfg.MinHTLC
+
+	// As a last resort, we just add a non-zero amount.
+	default:
 		mockHtlcAmt++
 	}
 

lnwallet/channel_test.go

@@ -9841,10 +9841,9 @@ func TestChannelSignedAckRegression(t *testing.T) {
 	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) {
+// TestMayAddOutgoingHtlc tests MayAddOutgoingHtlc against zero and non-zero
+// htlc amounts.
+func TestMayAddOutgoingHtlc(t *testing.T) {
 	t.Parallel()
 
 	// The default channel created as a part of the test fixture already
@@ -9857,9 +9856,29 @@ func TestMayAddOutgoingHtlcZeroValue(t *testing.T) {
 	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())
+	// able to add an outgoing HTLC with no specific amount added.
+	require.NoError(t, aliceChannel.MayAddOutgoingHtlc(0))
+	require.NoError(t, bobChannel.MayAddOutgoingHtlc(0))
+
+	chanBal, err := btcutil.NewAmount(testChannelCapacity)
+	require.NoError(t, err)
+
+	// Each side should be able to add 1/4 of total channel balance since
+	// we're 50/50 split.
+	mayAdd := lnwire.MilliSatoshi(chanBal / 4 * 1000)
+	require.NoError(t, aliceChannel.MayAddOutgoingHtlc(mayAdd))
+	require.NoError(t, bobChannel.MayAddOutgoingHtlc(mayAdd))
+
+	// Both channels should fail if we try to add an amount more than
+	// their current balance.
+	mayNotAdd := lnwire.MilliSatoshi(chanBal * 1000)
+	require.Error(t, aliceChannel.MayAddOutgoingHtlc(mayNotAdd))
+	require.Error(t, bobChannel.MayAddOutgoingHtlc(mayNotAdd))
+
+	// Hard set alice's min htlc to zero and test the case where we just
+	// fall back to a non-zero value.
+	aliceChannel.channelState.LocalChanCfg.MinHTLC = 0
+	require.NoError(t, aliceChannel.MayAddOutgoingHtlc(0))
 }
 
 // TestIsChannelClean tests that IsChannelClean returns the expected values

lnwallet/test_utils.go

@@ -96,6 +96,8 @@ var (
 
 	aliceDustLimit = btcutil.Amount(200)
 	bobDustLimit   = btcutil.Amount(1300)
+
+	testChannelCapacity float64 = 10
 )
 
 // CreateTestChannels creates to fully populated channels to be used within
@@ -109,7 +111,7 @@ var (
 func CreateTestChannels(chanType channeldb.ChannelType) (
 	*LightningChannel, *LightningChannel, func(), error) {
 
-	channelCapacity, err := btcutil.NewAmount(10)
+	channelCapacity, err := btcutil.NewAmount(testChannelCapacity)
 	if err != nil {
 		return nil, nil, nil, err
 	}

peer/test_utils.go

@@ -445,7 +445,7 @@ func (m *mockUpdateHandler) Bandwidth() lnwire.MilliSatoshi { return 0 }
 func (m *mockUpdateHandler) EligibleToForward() bool { return false }
 
 // MayAddOutgoingHtlc currently returns nil.
-func (m *mockUpdateHandler) MayAddOutgoingHtlc() error { return nil }
+func (m *mockUpdateHandler) MayAddOutgoingHtlc(lnwire.MilliSatoshi) error { return nil }
 
 // ShutdownIfChannelClean currently returns nil.
 func (m *mockUpdateHandler) ShutdownIfChannelClean() error { return nil }

routing/bandwidth.go

@@ -12,8 +12,13 @@ import (
 type bandwidthHints interface {
 	// availableChanBandwidth returns the total available bandwidth for a
 	// channel and a bool indicating whether the channel hint was found.
-	// If the channel is unavailable, a zero amount is returned.
-	availableChanBandwidth(channelID uint64) (lnwire.MilliSatoshi, bool)
+	// The amount parameter is used to validate the outgoing htlc amount
+	// that we wish to add to the channel against its flow restrictions. If
+	// a zero amount is provided, the minimum htlc value for the channel
+	// will be used. If the channel is unavailable, a zero amount is
+	// returned.
+	availableChanBandwidth(channelID uint64,
+		amount lnwire.MilliSatoshi) (lnwire.MilliSatoshi, bool)
 }
 
 // getLinkQuery is the function signature used to lookup a link.
@@ -65,7 +70,8 @@ func newBandwidthManager(graph routingGraph, sourceNode route.Vertex,
 // available bandwidth. Note that this function assumes that the channel being
 // queried is one of our local channels, so any failure to retrieve the link
 // is interpreted as the link being offline.
-func (b *bandwidthManager) getBandwidth(cid lnwire.ShortChannelID) lnwire.MilliSatoshi {
+func (b *bandwidthManager) getBandwidth(cid lnwire.ShortChannelID,
+	amount lnwire.MilliSatoshi) lnwire.MilliSatoshi {
 	link, err := b.getLink(cid)
 	if err != nil {
 		// If the link isn't online, then we'll report that it has
@@ -82,7 +88,7 @@ func (b *bandwidthManager) getBandwidth(cid lnwire.ShortChannelID) lnwire.MilliS
 
 	// If our link isn't currently in a state where it can  add another
 	// outgoing htlc, treat the link as unusable.
-	if err := link.MayAddOutgoingHtlc(); err != nil {
+	if err := link.MayAddOutgoingHtlc(amount); err != nil {
 		return 0
 	}
 
@@ -94,8 +100,8 @@ func (b *bandwidthManager) getBandwidth(cid lnwire.ShortChannelID) lnwire.MilliS
 // availableChanBandwidth returns the total available bandwidth for a channel
 // and a bool indicating whether the channel hint was found. If the channel is
 // unavailable, a zero amount is returned.
-func (b *bandwidthManager) availableChanBandwidth(channelID uint64) (
-	lnwire.MilliSatoshi, bool) {
+func (b *bandwidthManager) availableChanBandwidth(channelID uint64,
+	amount lnwire.MilliSatoshi) (lnwire.MilliSatoshi, bool) {
 
 	shortID := lnwire.NewShortChanIDFromInt(channelID)
 	_, ok := b.localChans[shortID]
@@ -103,5 +109,5 @@ func (b *bandwidthManager) availableChanBandwidth(channelID uint64) (
 		return 0, false
 	}
 
-	return b.getBandwidth(shortID), true
+	return b.getBandwidth(shortID, amount), true
 }

routing/bandwidth_test.go

@@ -119,7 +119,7 @@ func TestBandwidthManager(t *testing.T) {
 			require.NoError(t, err)
 
 			bandwidth, found := m.availableChanBandwidth(
-				testCase.channelID,
+				testCase.channelID, 10,
 			)
 			require.Equal(t, testCase.expectedBandwidth, bandwidth)
 			require.Equal(t, testCase.expectFound, found)

routing/integrated_routing_context_test.go

@@ -22,8 +22,8 @@ type mockBandwidthHints struct {
 	hints map[uint64]lnwire.MilliSatoshi
 }
 
-func (m *mockBandwidthHints) availableChanBandwidth(channelID uint64) (
-	lnwire.MilliSatoshi, bool) {
+func (m *mockBandwidthHints) availableChanBandwidth(channelID uint64,
+	_ lnwire.MilliSatoshi) (lnwire.MilliSatoshi, bool) {
 
 	if m.hints == nil {
 		return 0, false

routing/mock_test.go

@@ -760,6 +760,6 @@ func (m *mockLink) EligibleToForward() bool {
 }
 
 // MayAddOutgoingHtlc returns the error configured in our mock.
-func (m *mockLink) MayAddOutgoingHtlc() error {
+func (m *mockLink) MayAddOutgoingHtlc(_ lnwire.MilliSatoshi) error {
 	return m.mayAddOutgoingErr
 }

routing/pathfind.go

@@ -373,7 +373,9 @@ func getOutgoingBalance(node route.Vertex, outgoingChans map[uint64]struct{},
 			}
 		}
 
-		bandwidth, ok := bandwidthHints.availableChanBandwidth(chanID)
+		bandwidth, ok := bandwidthHints.availableChanBandwidth(
+			chanID, 0,
+		)
 
 		// If the bandwidth is not available, use the channel capacity.
 		// This can happen when a channel is added to the graph after

routing/unified_policies.go

@@ -170,7 +170,7 @@ func (u *unifiedPolicy) getPolicyLocal(amt lnwire.MilliSatoshi,
 		// channel. The bandwidth hint is expected to be
 		// available.
 		bandwidth, ok := bandwidthHints.availableChanBandwidth(
-			edge.policy.ChannelID,
+			edge.policy.ChannelID, amt,
 		)
 		if !ok {
 			bandwidth = lnwire.MaxMilliSatoshi