diff --git a/channeldb/graph.go b/channeldb/graph.go index 957c0b828d4..084c2f77f89 100644 --- a/channeldb/graph.go +++ b/channeldb/graph.go @@ -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 diff --git a/channeldb/graph_cache.go b/channeldb/graph_cache.go index 1aae21d06d5..9ef9ef71d59 100644 --- a/channeldb/graph_cache.go +++ b/channeldb/graph_cache.go @@ -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{ diff --git a/channeldb/graph_test.go b/channeldb/graph_test.go index 35c7050eb3b..17b32c89466 100644 --- a/channeldb/graph_test.go +++ b/channeldb/graph_test.go @@ -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 diff --git a/routing/pathfind.go b/routing/pathfind.go index a92d0a7e52c..4f7bd32480f 100644 --- a/routing/pathfind.go +++ b/routing/pathfind.go @@ -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 @@ -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 @@ -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 @@ -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 diff --git a/routing/pathfind_test.go b/routing/pathfind_test.go index 7f576196a4e..0951920525a 100644 --- a/routing/pathfind_test.go +++ b/routing/pathfind_test.go @@ -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{} }, @@ -1318,6 +1327,12 @@ func TestNewRoute(t *testing.T) { FeeBaseMSat: baseFee, TimeLockDelta: timeLockDelta, } + + for _, o := range opts { + o(policy) + } + + return policy } testCases := []struct { @@ -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 @@ -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}, @@ -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}, @@ -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}, @@ -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}, @@ -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, @@ -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, @@ -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 @@ -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) } }) diff --git a/routing/route/route.go b/routing/route/route.go index 5992bd40468..8104d7bb0f4 100644 --- a/routing/route/route.go +++ b/routing/route/route.go @@ -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. diff --git a/routing/router.go b/routing/router.go index 348914db733..8171a21a7f5 100644 --- a/routing/router.go +++ b/routing/router.go @@ -2731,22 +2731,23 @@ func (r *ChannelRouter) BuildRoute(amt *lnwire.MilliSatoshi, // Allocate a list that will contain the unified policies for this // route. - edges := make([]*unifiedPolicy, len(hops)) + edges := make([]*channeldb.CachedEdgePolicy, len(hops)) - var runningAmt lnwire.MilliSatoshi + var paymentAmt lnwire.MilliSatoshi if useMinAmt { // For minimum amount routes, aim to deliver at least 1 msat to // the destination. There are nodes in the wild that have a // min_htlc channel policy of zero, which could lead to a zero // amount payment being made. - runningAmt = 1 + paymentAmt = 1 } else { // If an amount is specified, we need to build a route that // delivers exactly this amount to the final destination. - runningAmt = *amt + paymentAmt = *amt } // Traverse hops backwards to accumulate fees in the running amounts. + runningAmt := paymentAmt source := r.selfNode.PubKeyBytes for i := len(hops) - 1; i >= 0; i-- { toNode := hops[i] @@ -2778,12 +2779,10 @@ func (r *ChannelRouter) BuildRoute(amt *lnwire.MilliSatoshi, } } - // If using min amt, increase amt if needed. - if useMinAmt { - min := unifiedPolicy.minAmt() - if min > runningAmt { - runningAmt = min - } + // Increase amount if needed to satisfy minimum htlc size. + min := unifiedPolicy.minAmt() + if min > runningAmt { + runningAmt = min } // Get a forwarding policy for the specific amount that we want @@ -2803,40 +2802,15 @@ func (r *ChannelRouter) BuildRoute(amt *lnwire.MilliSatoshi, log.Tracef("Select channel %v at position %v", policy.ChannelID, i) - edges[i] = unifiedPolicy - } - - // Now that we arrived at the start of the route and found out the route - // total amount, we make a forward pass. Because the amount may have - // been increased in the backward pass, fees need to be recalculated and - // amount ranges re-checked. - var pathEdges []*channeldb.CachedEdgePolicy - receiverAmt := runningAmt - for i, edge := range edges { - policy := edge.getPolicy(receiverAmt, bandwidthHints) - if policy == nil { - return nil, ErrNoChannel{ - fromNode: hops[i-1], - position: i, - } - } - - if i > 0 { - // Decrease the amount to send while going forward. - receiverAmt -= policy.ComputeFeeFromIncoming( - receiverAmt, - ) - } - - pathEdges = append(pathEdges, policy) + edges[i] = policy } // Build and return the final route. return newRoute( - source, pathEdges, uint32(height), + source, edges, uint32(height), finalHopParams{ - amt: receiverAmt, - totalAmt: receiverAmt, + amt: paymentAmt, + totalAmt: paymentAmt, cltvDelta: uint16(finalCltvDelta), records: nil, paymentAddr: payAddr,