diff --git a/docs/release-notes/release-notes-0.20.1.md b/docs/release-notes/release-notes-0.20.1.md index 4cc4556fb0..3f2db2c12a 100644 --- a/docs/release-notes/release-notes-0.20.1.md +++ b/docs/release-notes/release-notes-0.20.1.md @@ -60,6 +60,12 @@ safe single-writer behavior until the wallet subsystem is fully concurrent-safe. +* [Improved pathfinding + efficiency](https://github.com/lightningnetwork/lnd/pull/10406) by + identifying unusable local channels upfront and excluding them from route + construction, eliminating wasted retries and reducing pathfinding computation + overhead. + ## Deprecations # Technical and Architectural Updates diff --git a/routing/bandwidth.go b/routing/bandwidth.go index df68cea4f1..75f608cb7d 100644 --- a/routing/bandwidth.go +++ b/routing/bandwidth.go @@ -1,6 +1,7 @@ package routing import ( + "errors" "fmt" "github.com/lightningnetwork/lnd/fn/v2" @@ -11,18 +12,29 @@ import ( "github.com/lightningnetwork/lnd/tlv" ) +var ( + // ErrLocalChannelNotFound is returned when querying bandwidth for a + // channel that is not found in the local channels map. + ErrLocalChannelNotFound = errors.New("local channel not found") +) + // bandwidthHints provides hints about the currently available balance in our // channels. type bandwidthHints interface { // availableChanBandwidth returns the total available bandwidth for a - // channel and a bool indicating whether the channel hint was found. - // 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. + // channel. 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. + // + // Returns: + // - bandwidth: the available bandwidth if the channel is found and + // usable, zero otherwise + // - error: ErrLocalChannelNotFound if not in local channels map, or + // another error if the channel is found but cannot be used due to + // restrictions (offline, HTLC limits, etc.) availableChanBandwidth(channelID uint64, - amount lnwire.MilliSatoshi) (lnwire.MilliSatoshi, bool) + amount lnwire.MilliSatoshi) (lnwire.MilliSatoshi, error) // isCustomHTLCPayment returns true if this payment is a custom payment. // For custom payments policy checks might not be needed. @@ -184,27 +196,25 @@ func (b *bandwidthManager) getBandwidth(cid lnwire.ShortChannelID, return reportedBandwidth, nil } -// 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 returns the total available bandwidth for a channel. +// If the channel is not found or unavailable, a zero amount and an error are +// returned. func (b *bandwidthManager) availableChanBandwidth(channelID uint64, - amount lnwire.MilliSatoshi) (lnwire.MilliSatoshi, bool) { + amount lnwire.MilliSatoshi) (lnwire.MilliSatoshi, error) { shortID := lnwire.NewShortChanIDFromInt(channelID) _, ok := b.localChans[shortID] if !ok { - return 0, false + return 0, ErrLocalChannelNotFound } bandwidth, err := b.getBandwidth(shortID, amount) if err != nil { - log.Warnf("failed to get bandwidth for channel %v: %v", - shortID, err) - - return 0, true + return 0, fmt.Errorf("failed to get bandwidth for "+ + "channel %v: %w", shortID, err) } - return bandwidth, true + return bandwidth, nil } // isCustomHTLCPayment returns true if this payment is a custom payment. diff --git a/routing/bandwidth_test.go b/routing/bandwidth_test.go index b7f6e3f13b..c041a0307c 100644 --- a/routing/bandwidth_test.go +++ b/routing/bandwidth_test.go @@ -28,7 +28,14 @@ func TestBandwidthManager(t *testing.T) { channelID uint64 linkQuery getLinkQuery expectedBandwidth lnwire.MilliSatoshi - expectFound bool + // checkErrIs checks for specific error types using errors.Is. + // This is preferred for typed/sentinel errors as it's + // refactor-safe and works with wrapped errors. + checkErrIs error + // checkErrContains checks if the error message contains a + // specific string. Only use this when the error doesn't have + // a specific type (e.g., errors.New with dynamic messages). + checkErrContains string }{ { name: "channel not ours", @@ -45,7 +52,7 @@ func TestBandwidthManager(t *testing.T) { return nil, nil }, expectedBandwidth: 0, - expectFound: false, + checkErrIs: ErrLocalChannelNotFound, }, { name: "channel ours, link not online", @@ -56,7 +63,7 @@ func TestBandwidthManager(t *testing.T) { return nil, htlcswitch.ErrChannelLinkNotFound }, expectedBandwidth: 0, - expectFound: true, + checkErrIs: htlcswitch.ErrChannelLinkNotFound, }, { name: "channel ours, link not eligible", @@ -69,7 +76,7 @@ func TestBandwidthManager(t *testing.T) { }, nil }, expectedBandwidth: 0, - expectFound: true, + checkErrContains: "not eligible", }, { name: "channel ours, link can't add htlc", @@ -84,7 +91,7 @@ func TestBandwidthManager(t *testing.T) { }, nil }, expectedBandwidth: 0, - expectFound: true, + checkErrContains: "can't add htlc", }, { name: "channel ours, bandwidth available", @@ -97,12 +104,10 @@ func TestBandwidthManager(t *testing.T) { }, nil }, expectedBandwidth: 321, - expectFound: true, }, } for _, testCase := range testCases { - testCase := testCase t.Run(testCase.name, func(t *testing.T) { g := newMockGraph(t) @@ -126,11 +131,30 @@ func TestBandwidthManager(t *testing.T) { ) require.NoError(t, err) - bandwidth, found := m.availableChanBandwidth( + bandwidth, err := m.availableChanBandwidth( testCase.channelID, 10, ) require.Equal(t, testCase.expectedBandwidth, bandwidth) - require.Equal(t, testCase.expectFound, found) + + // Check for specific error types. + switch { + case testCase.checkErrIs != nil: + require.ErrorIs(t, err, testCase.checkErrIs) + + case testCase.checkErrContains != "": + // For errors without specific types, check the + // error message contains expected string. + require.Error(t, err) + require.Contains( + t, err.Error(), + testCase.checkErrContains, + ) + + default: + // If no error checks specified, expect no + // error. + require.NoError(t, err) + } }) } } diff --git a/routing/integrated_routing_context_test.go b/routing/integrated_routing_context_test.go index a98fa5602c..b2626a22b1 100644 --- a/routing/integrated_routing_context_test.go +++ b/routing/integrated_routing_context_test.go @@ -25,14 +25,18 @@ type mockBandwidthHints struct { } func (m *mockBandwidthHints) availableChanBandwidth(channelID uint64, - _ lnwire.MilliSatoshi) (lnwire.MilliSatoshi, bool) { + _ lnwire.MilliSatoshi) (lnwire.MilliSatoshi, error) { if m.hints == nil { - return 0, false + return 0, ErrLocalChannelNotFound } balance, ok := m.hints[channelID] - return balance, ok + if !ok { + return 0, ErrLocalChannelNotFound + } + + return balance, nil } func (m *mockBandwidthHints) isCustomHTLCPayment() bool { diff --git a/routing/pathfind.go b/routing/pathfind.go index fab8015dde..abc729ffa3 100644 --- a/routing/pathfind.go +++ b/routing/pathfind.go @@ -538,20 +538,33 @@ func getOutgoingBalance(node route.Vertex, outgoingChans map[uint64]struct{}, } } - bandwidth, ok := bandwidthHints.availableChanBandwidth( + bandwidth, err := 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 - // we've already queried the bandwidth hints. - if !ok { - bandwidth = lnwire.NewMSatFromSatoshis(channel.Capacity) - - log.Warnf("ShortChannelID=%v: not found in the local "+ - "channels map of the bandwidth manager, "+ - "using channel capacity=%v as bandwidth for "+ - "this channel", shortID, bandwidth) + if err != nil { + // If the channel is not in our local channels map, use + // the channel capacity as a fallback. This can happen + // when a channel is added to the graph after we've + // already queried the bandwidth hints. + if errors.Is(err, ErrLocalChannelNotFound) { + log.Warnf("ShortChannelID=%v: not found in "+ + "the local channels map, using "+ + "channel capacity=%v as bandwidth", + shortID, bandwidth) + + bandwidth = lnwire.NewMSatFromSatoshis( + channel.Capacity, + ) + } else { + // Channel is local but unusable (offline, HTLC + // limits, etc.). Don't assume any bandwidth is + // available. + log.Debugf("ShortChannelID=%v: channel "+ + "unusable: %v, setting bandwidth to 0", + shortID, err) + + bandwidth = 0 + } } if bandwidth > max { diff --git a/routing/unified_edges.go b/routing/unified_edges.go index 9b8f6c5c03..d7eed5a5ae 100644 --- a/routing/unified_edges.go +++ b/routing/unified_edges.go @@ -1,6 +1,7 @@ package routing import ( + "errors" "math" "github.com/btcsuite/btcd/btcutil" @@ -285,32 +286,32 @@ func (u *edgeUnifier) getEdgeLocal(netAmtReceived lnwire.MilliSatoshi, // channel selection. The disabled flag is ignored for local // channels. - // Retrieve bandwidth for this local channel. If not - // available, assume this channel has enough bandwidth. - // - // TODO(joostjager): Possibly change to skipping this - // channel. The bandwidth hint is expected to be - // available. - bandwidth, ok := bandwidthHints.availableChanBandwidth( + // Retrieve bandwidth for this local channel. + bandwidth, err := bandwidthHints.availableChanBandwidth( edge.policy.ChannelID, amt, ) - if !ok { - log.Warnf("Cannot get bandwidth for edge %v, use max "+ - "instead", edge.policy.ChannelID) - - bandwidth = lnwire.MaxMilliSatoshi + if err != nil { + // If the channel is not in our local channels map, use + // max bandwidth as a fallback. + if errors.Is(err, ErrLocalChannelNotFound) { + log.Warnf("Cannot get bandwidth for edge %v, "+ + "use max instead", + edge.policy.ChannelID) + + bandwidth = lnwire.MaxMilliSatoshi + } else { + // Channel is local but unusable (offline, HTLC + // limits, etc.). Skip this edge entirely to + // avoid attempting to route through a channel + // that cannot carry the payment. + log.Debugf("Skipped local edge %v: channel "+ + "unusable: %v", edge.policy.ChannelID, + err) + + continue + } } - // TODO(yy): if the above `!ok` is chosen, we'd have - // `bandwidth` to be the max value, which will end up having - // the `maxBandwidth` to be have the largest value and this - // edge will be the chosen one. This is wrong in two ways, - // 1. we need to understand why `availableChanBandwidth` cannot - // find bandwidth for this edge as something is wrong with this - // channel, and, - // 2. this edge is likely NOT the local channel with the - // highest available bandwidth. - // // Skip channels that can't carry the payment. if amt > bandwidth { log.Debugf("Skipped edge %v: not enough bandwidth, "+