From 9feb02e593b93b87f739dde105e909cc694b79ca Mon Sep 17 00:00:00 2001 From: ziggie Date: Mon, 1 Dec 2025 19:29:36 +0100 Subject: [PATCH 1/2] routing: return error instead of bool from availableChanBandwidth Replace the (bandwidth, bool) return signature with (bandwidth, error) to provide more context about why bandwidth is unavailable. This allows callers to distinguish between: - Channel not found in local channels map (ErrLocalChannelNotFound) - Channel found but unusable (offline, HTLC limits, etc.) The new error-based approach improves error handling throughout the routing package: - pathfind: Use capacity fallback only for channels which are not found in the local graph map, which can happen when channels were opened and activated during the payment process started for example. - unified_edges: Skip unusable local channels instead of using max bandwidth - Tests updated to use checkErrIs/checkErrContains for precise validation This fixes TODO comments about unclear bandwidth hint failures and improves channel selection by avoiding attempts to route through channels that cannot carry payments. --- routing/bandwidth.go | 44 +++++++++++++-------- routing/bandwidth_test.go | 42 +++++++++++++++----- routing/integrated_routing_context_test.go | 10 +++-- routing/pathfind.go | 37 ++++++++++++------ routing/unified_edges.go | 45 +++++++++++----------- 5 files changed, 115 insertions(+), 63 deletions(-) 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, "+ From 745bcf5766ec236463a135fcdb5fdaa442daccb3 Mon Sep 17 00:00:00 2001 From: ziggie Date: Mon, 1 Dec 2025 19:42:49 +0100 Subject: [PATCH 2/2] docs: add release-notes for LND 20.1 --- docs/release-notes/release-notes-0.20.1.md | 6 ++++++ 1 file changed, 6 insertions(+) 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