Skip to content

Commit 9feb02e

Browse files
committed
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.
1 parent 0a2a5b2 commit 9feb02e

File tree

5 files changed

+115
-63
lines changed

5 files changed

+115
-63
lines changed

routing/bandwidth.go

Lines changed: 27 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package routing
22

33
import (
4+
"errors"
45
"fmt"
56

67
"github.com/lightningnetwork/lnd/fn/v2"
@@ -11,18 +12,29 @@ import (
1112
"github.com/lightningnetwork/lnd/tlv"
1213
)
1314

15+
var (
16+
// ErrLocalChannelNotFound is returned when querying bandwidth for a
17+
// channel that is not found in the local channels map.
18+
ErrLocalChannelNotFound = errors.New("local channel not found")
19+
)
20+
1421
// bandwidthHints provides hints about the currently available balance in our
1522
// channels.
1623
type bandwidthHints interface {
1724
// availableChanBandwidth returns the total available bandwidth for a
18-
// channel and a bool indicating whether the channel hint was found.
19-
// The amount parameter is used to validate the outgoing htlc amount
20-
// that we wish to add to the channel against its flow restrictions. If
21-
// a zero amount is provided, the minimum htlc value for the channel
22-
// will be used. If the channel is unavailable, a zero amount is
23-
// returned.
25+
// channel. The amount parameter is used to validate the outgoing htlc
26+
// amount that we wish to add to the channel against its flow
27+
// restrictions. If a zero amount is provided, the minimum htlc value
28+
// for the channel will be used.
29+
//
30+
// Returns:
31+
// - bandwidth: the available bandwidth if the channel is found and
32+
// usable, zero otherwise
33+
// - error: ErrLocalChannelNotFound if not in local channels map, or
34+
// another error if the channel is found but cannot be used due to
35+
// restrictions (offline, HTLC limits, etc.)
2436
availableChanBandwidth(channelID uint64,
25-
amount lnwire.MilliSatoshi) (lnwire.MilliSatoshi, bool)
37+
amount lnwire.MilliSatoshi) (lnwire.MilliSatoshi, error)
2638

2739
// isCustomHTLCPayment returns true if this payment is a custom payment.
2840
// For custom payments policy checks might not be needed.
@@ -184,27 +196,25 @@ func (b *bandwidthManager) getBandwidth(cid lnwire.ShortChannelID,
184196
return reportedBandwidth, nil
185197
}
186198

187-
// availableChanBandwidth returns the total available bandwidth for a channel
188-
// and a bool indicating whether the channel hint was found. If the channel is
189-
// unavailable, a zero amount is returned.
199+
// availableChanBandwidth returns the total available bandwidth for a channel.
200+
// If the channel is not found or unavailable, a zero amount and an error are
201+
// returned.
190202
func (b *bandwidthManager) availableChanBandwidth(channelID uint64,
191-
amount lnwire.MilliSatoshi) (lnwire.MilliSatoshi, bool) {
203+
amount lnwire.MilliSatoshi) (lnwire.MilliSatoshi, error) {
192204

193205
shortID := lnwire.NewShortChanIDFromInt(channelID)
194206
_, ok := b.localChans[shortID]
195207
if !ok {
196-
return 0, false
208+
return 0, ErrLocalChannelNotFound
197209
}
198210

199211
bandwidth, err := b.getBandwidth(shortID, amount)
200212
if err != nil {
201-
log.Warnf("failed to get bandwidth for channel %v: %v",
202-
shortID, err)
203-
204-
return 0, true
213+
return 0, fmt.Errorf("failed to get bandwidth for "+
214+
"channel %v: %w", shortID, err)
205215
}
206216

207-
return bandwidth, true
217+
return bandwidth, nil
208218
}
209219

210220
// isCustomHTLCPayment returns true if this payment is a custom payment.

routing/bandwidth_test.go

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,14 @@ func TestBandwidthManager(t *testing.T) {
2828
channelID uint64
2929
linkQuery getLinkQuery
3030
expectedBandwidth lnwire.MilliSatoshi
31-
expectFound bool
31+
// checkErrIs checks for specific error types using errors.Is.
32+
// This is preferred for typed/sentinel errors as it's
33+
// refactor-safe and works with wrapped errors.
34+
checkErrIs error
35+
// checkErrContains checks if the error message contains a
36+
// specific string. Only use this when the error doesn't have
37+
// a specific type (e.g., errors.New with dynamic messages).
38+
checkErrContains string
3239
}{
3340
{
3441
name: "channel not ours",
@@ -45,7 +52,7 @@ func TestBandwidthManager(t *testing.T) {
4552
return nil, nil
4653
},
4754
expectedBandwidth: 0,
48-
expectFound: false,
55+
checkErrIs: ErrLocalChannelNotFound,
4956
},
5057
{
5158
name: "channel ours, link not online",
@@ -56,7 +63,7 @@ func TestBandwidthManager(t *testing.T) {
5663
return nil, htlcswitch.ErrChannelLinkNotFound
5764
},
5865
expectedBandwidth: 0,
59-
expectFound: true,
66+
checkErrIs: htlcswitch.ErrChannelLinkNotFound,
6067
},
6168
{
6269
name: "channel ours, link not eligible",
@@ -69,7 +76,7 @@ func TestBandwidthManager(t *testing.T) {
6976
}, nil
7077
},
7178
expectedBandwidth: 0,
72-
expectFound: true,
79+
checkErrContains: "not eligible",
7380
},
7481
{
7582
name: "channel ours, link can't add htlc",
@@ -84,7 +91,7 @@ func TestBandwidthManager(t *testing.T) {
8491
}, nil
8592
},
8693
expectedBandwidth: 0,
87-
expectFound: true,
94+
checkErrContains: "can't add htlc",
8895
},
8996
{
9097
name: "channel ours, bandwidth available",
@@ -97,12 +104,10 @@ func TestBandwidthManager(t *testing.T) {
97104
}, nil
98105
},
99106
expectedBandwidth: 321,
100-
expectFound: true,
101107
},
102108
}
103109

104110
for _, testCase := range testCases {
105-
testCase := testCase
106111

107112
t.Run(testCase.name, func(t *testing.T) {
108113
g := newMockGraph(t)
@@ -126,11 +131,30 @@ func TestBandwidthManager(t *testing.T) {
126131
)
127132
require.NoError(t, err)
128133

129-
bandwidth, found := m.availableChanBandwidth(
134+
bandwidth, err := m.availableChanBandwidth(
130135
testCase.channelID, 10,
131136
)
132137
require.Equal(t, testCase.expectedBandwidth, bandwidth)
133-
require.Equal(t, testCase.expectFound, found)
138+
139+
// Check for specific error types.
140+
switch {
141+
case testCase.checkErrIs != nil:
142+
require.ErrorIs(t, err, testCase.checkErrIs)
143+
144+
case testCase.checkErrContains != "":
145+
// For errors without specific types, check the
146+
// error message contains expected string.
147+
require.Error(t, err)
148+
require.Contains(
149+
t, err.Error(),
150+
testCase.checkErrContains,
151+
)
152+
153+
default:
154+
// If no error checks specified, expect no
155+
// error.
156+
require.NoError(t, err)
157+
}
134158
})
135159
}
136160
}

routing/integrated_routing_context_test.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,18 @@ type mockBandwidthHints struct {
2525
}
2626

2727
func (m *mockBandwidthHints) availableChanBandwidth(channelID uint64,
28-
_ lnwire.MilliSatoshi) (lnwire.MilliSatoshi, bool) {
28+
_ lnwire.MilliSatoshi) (lnwire.MilliSatoshi, error) {
2929

3030
if m.hints == nil {
31-
return 0, false
31+
return 0, ErrLocalChannelNotFound
3232
}
3333

3434
balance, ok := m.hints[channelID]
35-
return balance, ok
35+
if !ok {
36+
return 0, ErrLocalChannelNotFound
37+
}
38+
39+
return balance, nil
3640
}
3741

3842
func (m *mockBandwidthHints) isCustomHTLCPayment() bool {

routing/pathfind.go

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -538,20 +538,33 @@ func getOutgoingBalance(node route.Vertex, outgoingChans map[uint64]struct{},
538538
}
539539
}
540540

541-
bandwidth, ok := bandwidthHints.availableChanBandwidth(
541+
bandwidth, err := bandwidthHints.availableChanBandwidth(
542542
chanID, 0,
543543
)
544-
545-
// If the bandwidth is not available, use the channel capacity.
546-
// This can happen when a channel is added to the graph after
547-
// we've already queried the bandwidth hints.
548-
if !ok {
549-
bandwidth = lnwire.NewMSatFromSatoshis(channel.Capacity)
550-
551-
log.Warnf("ShortChannelID=%v: not found in the local "+
552-
"channels map of the bandwidth manager, "+
553-
"using channel capacity=%v as bandwidth for "+
554-
"this channel", shortID, bandwidth)
544+
if err != nil {
545+
// If the channel is not in our local channels map, use
546+
// the channel capacity as a fallback. This can happen
547+
// when a channel is added to the graph after we've
548+
// already queried the bandwidth hints.
549+
if errors.Is(err, ErrLocalChannelNotFound) {
550+
log.Warnf("ShortChannelID=%v: not found in "+
551+
"the local channels map, using "+
552+
"channel capacity=%v as bandwidth",
553+
shortID, bandwidth)
554+
555+
bandwidth = lnwire.NewMSatFromSatoshis(
556+
channel.Capacity,
557+
)
558+
} else {
559+
// Channel is local but unusable (offline, HTLC
560+
// limits, etc.). Don't assume any bandwidth is
561+
// available.
562+
log.Debugf("ShortChannelID=%v: channel "+
563+
"unusable: %v, setting bandwidth to 0",
564+
shortID, err)
565+
566+
bandwidth = 0
567+
}
555568
}
556569

557570
if bandwidth > max {

routing/unified_edges.go

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package routing
22

33
import (
4+
"errors"
45
"math"
56

67
"github.com/btcsuite/btcd/btcutil"
@@ -285,32 +286,32 @@ func (u *edgeUnifier) getEdgeLocal(netAmtReceived lnwire.MilliSatoshi,
285286
// channel selection. The disabled flag is ignored for local
286287
// channels.
287288

288-
// Retrieve bandwidth for this local channel. If not
289-
// available, assume this channel has enough bandwidth.
290-
//
291-
// TODO(joostjager): Possibly change to skipping this
292-
// channel. The bandwidth hint is expected to be
293-
// available.
294-
bandwidth, ok := bandwidthHints.availableChanBandwidth(
289+
// Retrieve bandwidth for this local channel.
290+
bandwidth, err := bandwidthHints.availableChanBandwidth(
295291
edge.policy.ChannelID, amt,
296292
)
297-
if !ok {
298-
log.Warnf("Cannot get bandwidth for edge %v, use max "+
299-
"instead", edge.policy.ChannelID)
300-
301-
bandwidth = lnwire.MaxMilliSatoshi
293+
if err != nil {
294+
// If the channel is not in our local channels map, use
295+
// max bandwidth as a fallback.
296+
if errors.Is(err, ErrLocalChannelNotFound) {
297+
log.Warnf("Cannot get bandwidth for edge %v, "+
298+
"use max instead",
299+
edge.policy.ChannelID)
300+
301+
bandwidth = lnwire.MaxMilliSatoshi
302+
} else {
303+
// Channel is local but unusable (offline, HTLC
304+
// limits, etc.). Skip this edge entirely to
305+
// avoid attempting to route through a channel
306+
// that cannot carry the payment.
307+
log.Debugf("Skipped local edge %v: channel "+
308+
"unusable: %v", edge.policy.ChannelID,
309+
err)
310+
311+
continue
312+
}
302313
}
303314

304-
// TODO(yy): if the above `!ok` is chosen, we'd have
305-
// `bandwidth` to be the max value, which will end up having
306-
// the `maxBandwidth` to be have the largest value and this
307-
// edge will be the chosen one. This is wrong in two ways,
308-
// 1. we need to understand why `availableChanBandwidth` cannot
309-
// find bandwidth for this edge as something is wrong with this
310-
// channel, and,
311-
// 2. this edge is likely NOT the local channel with the
312-
// highest available bandwidth.
313-
//
314315
// Skip channels that can't carry the payment.
315316
if amt > bandwidth {
316317
log.Debugf("Skipped edge %v: not enough bandwidth, "+

0 commit comments

Comments
 (0)