Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

routing: improve BuildRoute robustness #6912

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 0 additions & 16 deletions channeldb/graph.go
Original file line number Diff line number Diff line change
Expand Up @@ -3489,22 +3489,6 @@ func (c *ChannelEdgePolicy) ComputeFee(
return c.FeeBaseMSat + (amt*c.FeeProportionalMillionths)/feeRateParts
}

// divideCeil divides dividend by factor and rounds the result up.
func divideCeil(dividend, factor lnwire.MilliSatoshi) lnwire.MilliSatoshi {
return (dividend + factor - 1) / factor
}

// ComputeFeeFromIncoming computes the fee to forward an HTLC given the incoming
// amount.
func (c *ChannelEdgePolicy) ComputeFeeFromIncoming(
incomingAmt lnwire.MilliSatoshi) lnwire.MilliSatoshi {

return incomingAmt - divideCeil(
feeRateParts*(incomingAmt-c.FeeBaseMSat),
feeRateParts+c.FeeProportionalMillionths,
)
}

// FetchChannelEdgesByOutpoint attempts to lookup the two directed edges for
// the channel identified by the funding outpoint. If the channel can't be
// found, then ErrEdgeNotFound is returned. A struct which houses the general
Expand Down
11 changes: 0 additions & 11 deletions channeldb/graph_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,17 +92,6 @@ func (c *CachedEdgePolicy) ComputeFee(
return c.FeeBaseMSat + (amt*c.FeeProportionalMillionths)/feeRateParts
}

// ComputeFeeFromIncoming computes the fee to forward an HTLC given the incoming
// amount.
func (c *CachedEdgePolicy) ComputeFeeFromIncoming(
incomingAmt lnwire.MilliSatoshi) lnwire.MilliSatoshi {

return incomingAmt - divideCeil(
feeRateParts*(incomingAmt-c.FeeBaseMSat),
feeRateParts+c.FeeProportionalMillionths,
)
}

// NewCachedPolicy turns a full policy into a minimal one that can be cached.
func NewCachedPolicy(policy *ChannelEdgePolicy) *CachedEdgePolicy {
return &CachedEdgePolicy{
Expand Down
5 changes: 0 additions & 5 deletions channeldb/graph_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3264,11 +3264,6 @@ func TestComputeFee(t *testing.T) {
if fee != expectedFee {
t.Fatalf("expected fee %v, got %v", expectedFee, fee)
}

fwdFee := policy.ComputeFeeFromIncoming(outgoingAmt + fee)
if fwdFee != expectedFee {
t.Fatalf("expected fee %v, but got %v", fee, fwdFee)
}
}

// TestBatchedAddChannelEdge asserts that BatchedAddChannelEdge properly
Expand Down
36 changes: 27 additions & 9 deletions routing/pathfind.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,6 @@ func newRoute(sourceVertex route.Vertex,
// we compute the route in reverse.
var (
amtToForward lnwire.MilliSatoshi
fee lnwire.MilliSatoshi
outgoingTimeLock uint32
tlvPayload bool
customRecords record.CustomSet
Expand Down Expand Up @@ -172,9 +171,15 @@ func newRoute(sourceVertex route.Vertex,
// detailed.
amtToForward = finalHop.amt

// Fee is not part of the hop payload, but only used for
// reporting through RPC. Set to zero for the final hop.
fee = lnwire.MilliSatoshi(0)
// If this amount is however below the edge's min htlc
// constraint, then raise the amount to that minimum. A
// difference between the amount in the onion payload
// and the actual htlc amount is not tolerated.
if amtToForward < edge.MinHTLC {
amtToForward = edge.MinHTLC
}

nextIncomingAmount = amtToForward

// As this is the last hop, we'll use the specified
// final CLTV delta value instead of the value from the
Expand Down Expand Up @@ -219,7 +224,18 @@ func newRoute(sourceVertex route.Vertex,
// and its policy for the outgoing channel. This policy
// is stored as part of the incoming channel of
// the next hop.
fee = pathEdges[i+1].ComputeFee(amtToForward)
fee := pathEdges[i+1].ComputeFee(amtToForward)

// We update the amount that needs to flow into the
// *next* hop, which is the amount this hop needs to
// forward, accounting for the fee that it takes.
nextIncomingAmount = amtToForward + fee

// If this amount is below the edge's min htlc
// constraint, then raise the amount to that minimum.
if nextIncomingAmount < edge.MinHTLC {
nextIncomingAmount = edge.MinHTLC
}

// We'll take the total timelock of the preceding hop as
// the outgoing timelock or this hop. Then we'll
Expand All @@ -244,10 +260,12 @@ func newRoute(sourceVertex route.Vertex,

hops = append([]*route.Hop{currentHop}, hops...)

// Finally, we update the amount that needs to flow into the
// *next* hop, which is the amount this hop needs to forward,
// accounting for the fee that it takes.
nextIncomingAmount = amtToForward + fee
// Fail route building if max htlc is exceeded.
if edge.MessageFlags.HasMaxHtlc() &&
nextIncomingAmount > edge.MaxHTLC {

return nil, route.ErrMaxHtlcExceeded
}
}

// With the base routing data expressed as hops, build the full route
Expand Down
126 changes: 87 additions & 39 deletions routing/pathfind_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1304,12 +1304,21 @@ func TestNewRoute(t *testing.T) {
finalHopCLTV = 1
)

type createOpt func(*channeldb.CachedEdgePolicy)

withHtlcAmtRange := func(min, max lnwire.MilliSatoshi) createOpt {
return func(o *channeldb.CachedEdgePolicy) {
o.MinHTLC = min
o.MaxHTLC = max
o.MessageFlags |= lnwire.ChanUpdateOptionMaxHtlc
}
}

createHop := func(baseFee lnwire.MilliSatoshi,
feeRate lnwire.MilliSatoshi,
bandwidth lnwire.MilliSatoshi,
timeLockDelta uint16) *channeldb.CachedEdgePolicy {
timeLockDelta uint16, opts ...createOpt) *channeldb.CachedEdgePolicy {

return &channeldb.CachedEdgePolicy{
policy := &channeldb.CachedEdgePolicy{
ToNodePubKey: func() route.Vertex {
return route.Vertex{}
},
Expand All @@ -1318,6 +1327,12 @@ func TestNewRoute(t *testing.T) {
FeeBaseMSat: baseFee,
TimeLockDelta: timeLockDelta,
}

for _, o := range opts {
o(policy)
}

return policy
}

testCases := []struct {
Expand Down Expand Up @@ -1362,13 +1377,9 @@ func TestNewRoute(t *testing.T) {
// expectedTotalTimeLock is relative to the current block height.
expectedTotalTimeLock uint32

// expectError indicates whether the newRoute call is expected
// expectedError indicates whether the newRoute call is expected
// to fail or succeed.
expectError bool

// expectedErrorCode indicates the expected error code when
// expectError is true.
expectedErrorCode errorCode
expectedError error

expectedTLVPayload bool

Expand All @@ -1379,7 +1390,7 @@ func TestNewRoute(t *testing.T) {
name: "single hop",
paymentAmount: 100000,
hops: []*channeldb.CachedEdgePolicy{
createHop(100, 1000, 1000000, 10),
createHop(100, 1000, 10),
},
metadata: []byte{1, 2, 3},
expectedFees: []lnwire.MilliSatoshi{0},
Expand All @@ -1393,8 +1404,8 @@ func TestNewRoute(t *testing.T) {
name: "two hop",
paymentAmount: 100000,
hops: []*channeldb.CachedEdgePolicy{
createHop(0, 1000, 1000000, 10),
createHop(30, 1000, 1000000, 5),
createHop(0, 1000, 10),
createHop(30, 1000, 5),
},
expectedFees: []lnwire.MilliSatoshi{130, 0},
expectedTimeLocks: []uint32{1, 1},
Expand All @@ -1408,8 +1419,8 @@ func TestNewRoute(t *testing.T) {
destFeatures: tlvFeatures,
paymentAmount: 100000,
hops: []*channeldb.CachedEdgePolicy{
createHop(0, 1000, 1000000, 10),
createHop(30, 1000, 1000000, 5),
createHop(0, 1000, 10),
createHop(30, 1000, 5),
},
expectedFees: []lnwire.MilliSatoshi{130, 0},
expectedTimeLocks: []uint32{1, 1},
Expand All @@ -1425,8 +1436,8 @@ func TestNewRoute(t *testing.T) {
paymentAddr: &testPaymentAddr,
paymentAmount: 100000,
hops: []*channeldb.CachedEdgePolicy{
createHop(0, 1000, 1000000, 10),
createHop(30, 1000, 1000000, 5),
createHop(0, 1000, 10),
createHop(30, 1000, 5),
},
expectedFees: []lnwire.MilliSatoshi{130, 0},
expectedTimeLocks: []uint32{1, 1},
Expand All @@ -1445,9 +1456,9 @@ func TestNewRoute(t *testing.T) {
name: "three hop",
paymentAmount: 100000,
hops: []*channeldb.CachedEdgePolicy{
createHop(0, 10, 1000000, 10),
createHop(0, 10, 1000000, 5),
createHop(0, 10, 1000000, 3),
createHop(0, 10, 10),
createHop(0, 10, 5),
createHop(0, 10, 3),
},
expectedFees: []lnwire.MilliSatoshi{1, 1, 0},
expectedTotalAmount: 100002,
Expand All @@ -1460,9 +1471,9 @@ func TestNewRoute(t *testing.T) {
name: "three hop with fee carry over",
paymentAmount: 100000,
hops: []*channeldb.CachedEdgePolicy{
createHop(0, 10000, 1000000, 10),
createHop(0, 10000, 1000000, 5),
createHop(0, 10000, 1000000, 3),
createHop(0, 10000, 10),
createHop(0, 10000, 5),
createHop(0, 10000, 3),
},
expectedFees: []lnwire.MilliSatoshi{1010, 1000, 0},
expectedTotalAmount: 102010,
Expand All @@ -1475,21 +1486,69 @@ func TestNewRoute(t *testing.T) {
name: "three hop with minimal fees for carry over",
paymentAmount: 100000,
hops: []*channeldb.CachedEdgePolicy{
createHop(0, 10000, 1000000, 10),
createHop(0, 10000, 10),

// First hop charges 0.1% so the second hop fee
// should show up in the first hop fee as 1 msat
// extra.
createHop(0, 1000, 1000000, 5),
createHop(0, 1000, 5),

// Second hop charges a fixed 1000 msat.
createHop(1000, 0, 1000000, 3),
createHop(1000, 0, 3),
},
expectedFees: []lnwire.MilliSatoshi{101, 1000, 0},
expectedTotalAmount: 101101,
expectedTimeLocks: []uint32{4, 1, 1},
expectedTotalTimeLock: 9,
}}
},
{
name: "min htlc",
paymentAmount: 4000,
hops: []*channeldb.CachedEdgePolicy{
createHop(
0, 0, 10,
),
createHop(
4000, 20000, 10,
withHtlcAmtRange(10000, 50000),
),
createHop(
3000, 10000, 10,
withHtlcAmtRange(5000, 50000),
),
},
expectedTotalAmount: 14200,
expectedTotalTimeLock: 21,
expectedFees: []lnwire.MilliSatoshi{
// First hop fee is based on min htlc of
// outgoing channel.
4200,

// Second hop gets overpaid fee because of min
// htlc.
5000,

// Final hop gets paid no fee, but is overpaid
// the payment.
0,
},
},
{
name: "max htlc",
paymentAmount: 4000,
hops: []*channeldb.CachedEdgePolicy{
createHop(
0, 0, 10,
withHtlcAmtRange(0, 8000),
),
createHop(
3000, 10000, 10,
withHtlcAmtRange(5000, 10000),
),
},
expectedError: route.ErrMaxHtlcExceeded,
},
}

for _, testCase := range testCases {
testCase := testCase
Expand Down Expand Up @@ -1584,20 +1643,9 @@ func TestNewRoute(t *testing.T) {
},
)

if testCase.expectError {
expectedCode := testCase.expectedErrorCode
if err == nil || !IsError(err, expectedCode) {
t.Fatalf("expected newRoute to fail "+
"with error code %v but got "+
"%v instead",
expectedCode, err)
}
} else {
if err != nil {
t.Errorf("unable to create path: %v", err)
return
}
require.ErrorIs(t, err, testCase.expectedError)

if testCase.expectedError == nil {
assertRoute(t, route)
}
})
Expand Down
4 changes: 4 additions & 0 deletions routing/route/route.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ var (
// construct a new sphinx packet, but provides too many hops.
ErrMaxRouteHopsExceeded = fmt.Errorf("route has too many hops")

// ErrMaxHtlcExceeded is returned when a channel policy's max htlc is
// exceeded.
ErrMaxHtlcExceeded = fmt.Errorf("route exceeds max htlc policy")

// ErrIntermediateMPPHop is returned when a hop tries to deliver an MPP
// record to an intermediate hop, only final hops can receive MPP
// records.
Expand Down
Loading