diff --git a/channeldb/graph_test.go b/channeldb/graph_test.go index 256851238..00e30d5b2 100644 --- a/channeldb/graph_test.go +++ b/channeldb/graph_test.go @@ -3688,7 +3688,7 @@ func TestLightningNodeSigVerification(t *testing.T) { } } -// TestComputeFee tests fee calculation based on both in- and outgoing amt. +// TestComputeFee tests fee calculation based on the outgoing amt. func TestComputeFee(t *testing.T) { var ( policy = models.ChannelEdgePolicy{ @@ -3703,11 +3703,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/channeldb/models/cached_edge_policy.go b/channeldb/models/cached_edge_policy.go index 25bef567e..b770ec1fb 100644 --- a/channeldb/models/cached_edge_policy.go +++ b/channeldb/models/cached_edge_policy.go @@ -71,17 +71,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/models/channel_edge_policy.go b/channeldb/models/channel_edge_policy.go index 93902d31f..322ce3cd0 100644 --- a/channeldb/models/channel_edge_policy.go +++ b/channeldb/models/channel_edge_policy.go @@ -113,19 +113,3 @@ 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, - ) -} diff --git a/routing/router.go b/routing/router.go index d67fc2ebd..83cd3c734 100644 --- a/routing/router.go +++ b/routing/router.go @@ -1518,11 +1518,11 @@ func getEdgeUnifiers(source route.Vertex, hops []route.Vertex, } // Build unified policies for this hop based on the channels - // known in the graph. Don't use inbound fees. - // - // TODO: Add inbound fees support for BuildRoute. + // known in the graph. Inbound fees are only active if the edge + // is not the last hop. + isExitHop := i == len(hops)-1 u := newNodeEdgeUnifier( - source, toNode, false, outgoingChans, + source, toNode, !isExitHop, outgoingChans, ) err := u.addGraphPolicies(graph) @@ -1617,7 +1617,13 @@ func senderAmtBackwardPass(unifiers []*edgeUnifier, useMinAmt bool, return nil, 0, ErrNoChannel{position: i} } - fee := outboundFee + // The fee paid to B depends on the current hop's inbound fee + // policy and on the outbound fee for the next hop as any + // inbound fee discount is capped by the outbound fee such that + // the total fee for B can't become negative. + inboundFee := calcCappedInboundFee(edge, netAmount, outboundFee) + + fee := lnwire.MilliSatoshi(int64(outboundFee) + inboundFee) log.Tracef("Select channel %v at position %v", edge.policy.ChannelID, i) @@ -1655,12 +1661,11 @@ func receiverAmtForwardPass(runningAmt lnwire.MilliSatoshi, // been increased in the backward pass, fees need to be recalculated and // amount ranges re-checked. for i := 1; i < len(unifiedEdges); i++ { + inEdge := unifiedEdges[i-1] outEdge := unifiedEdges[i] // Decrease the amount to send while going forward. - runningAmt -= outEdge.policy.ComputeFeeFromIncoming( - runningAmt, - ) + runningAmt = outgoingFromIncoming(runningAmt, inEdge, outEdge) if !outEdge.amtInRange(runningAmt) { log.Errorf("Amount %v not in range for hop index %v", diff --git a/routing/router_test.go b/routing/router_test.go index ec095d787..a766325a5 100644 --- a/routing/router_test.go +++ b/routing/router_test.go @@ -1588,6 +1588,28 @@ func TestBuildRoute(t *testing.T) { MaxHTLC: lnwire.MilliSatoshi(20100), Features: paymentAddrFeatures, }, 8), + + // Create a route with inbound fees. + symmetricTestChannel("a", "d", chanCapSat, &testChannelPolicy{ + Expiry: 144, + FeeRate: 20000, + MinHTLC: lnwire.NewMSatFromSatoshis(5), + MaxHTLC: lnwire.NewMSatFromSatoshis( + chanCapSat, + ), + InboundFeeBaseMsat: -1000, + InboundFeeRate: -1000, + }, 9), + symmetricTestChannel("d", "f", chanCapSat, &testChannelPolicy{ + Expiry: 144, + FeeRate: 60000, + MinHTLC: lnwire.NewMSatFromSatoshis(20), + MaxHTLC: lnwire.NewMSatFromSatoshis(120), + Features: paymentAddrFeatures, + // The inbound fee will not be active for the last hop. + InboundFeeBaseMsat: 2000, + InboundFeeRate: 2000, + }, 10), } testGraph, err := createTestGraphFromChannels( @@ -1686,6 +1708,30 @@ func TestBuildRoute(t *testing.T) { require.Equal(t, lnwire.MilliSatoshi(21200), rt.TotalAmount) require.Equal(t, lnwire.MilliSatoshi(20000), rt.Hops[1].AmtToForward) + // Check that we compute a correct forwarding amount that involves + // inbound fees. We expect a similar amount as for the above case of + // b->c, but reduced by the inbound discount on the channel a->d. + // We get 106000 - 1000 (base in) - 0.001 * 106000 (rate in) = 104894. + hops = []route.Vertex{ctx.aliases["d"], ctx.aliases["f"]} + amt = lnwire.NewMSatFromSatoshis(100) + rt, err = ctx.router.BuildRoute(&amt, hops, nil, 40, &payAddr) + require.NoError(t, err) + checkHops(rt, []uint64{9, 10}, payAddr) + require.EqualValues(t, 104894, rt.TotalAmount) + + // Also check the min amount with inbound fees. The min amount bumps + // this to 20000 msat for the last hop. The outbound fee is 1200 msat, + // the inbound fee is -1021.2 msat (rounded down). This results in a + // total fee of 179 msat, giving a sender amount of 20179 msat. The + // determined receiver amount however reduces this to 20001 msat again + // due to rounding. This would not be compatible with the sender amount + // of 20179 msat, which results in underpayment of 1 msat in fee. There + // is a third pass through newRoute in which this gets corrected to end + hops = []route.Vertex{ctx.aliases["d"], ctx.aliases["f"]} + rt, err = ctx.router.BuildRoute(nil, hops, nil, 40, &payAddr) + require.NoError(t, err) + checkHops(rt, []uint64{9, 10}, payAddr) + require.EqualValues(t, 20180, rt.TotalAmount, "%v", rt.TotalAmount) } // TestReceiverAmtForwardPass tests that the forward pass returns the expected @@ -1831,6 +1877,9 @@ func TestSenderAmtBackwardPass(t *testing.T) { policy: &models.CachedEdgePolicy{ FeeBaseMSat: 112, }, + inboundFees: models.InboundFee{ + Base: 111, + }, capacity: capacity, }, }, @@ -1841,6 +1890,9 @@ func TestSenderAmtBackwardPass(t *testing.T) { policy: &models.CachedEdgePolicy{ FeeBaseMSat: 222, }, + inboundFees: models.InboundFee{ + Base: 222, + }, capacity: capacity, }, }, @@ -1852,6 +1904,12 @@ func TestSenderAmtBackwardPass(t *testing.T) { FeeBaseMSat: 333, MinHTLC: minHTLC, }, + // In pathfinding, inbound fees are not + // populated for exit hops because the + // newNodeEdgeUnifier enforces this. + // This is important as otherwise we + // would not fail the min HTLC check in + // getEdge. capacity: capacity, }, }, @@ -1870,20 +1928,51 @@ func TestSenderAmtBackwardPass(t *testing.T) { edgeUnifiers, true, 1, &bandwidthHints, ) require.NoError(t, err) - require.Equal(t, minHTLC+333+222, senderAmount) + require.Equal(t, minHTLC+333+222+222+111, senderAmount) // Do a search for a specific amount. unifiedEdges, senderAmount, err = senderAmtBackwardPass( edgeUnifiers, false, testReceiverAmt, &bandwidthHints, ) require.NoError(t, err) - require.Equal(t, testReceiverAmt+333+222, senderAmount) + require.Equal(t, testReceiverAmt+333+222+222+111, senderAmount) // Check that we arrive at the same receiver amount by doing a forward // pass. receiverAmt, err := receiverAmtForwardPass(senderAmount, unifiedEdges) require.NoError(t, err) require.Equal(t, testReceiverAmt, receiverAmt) + + // Insert a policy that leads to rounding. + edgeUnifiers[1] = &edgeUnifier{ + edges: []*unifiedEdge{ + { + policy: &models.CachedEdgePolicy{ + FeeBaseMSat: 20, + FeeProportionalMillionths: 100, + }, + inboundFees: models.InboundFee{ + Base: -10, + Rate: -50, + }, + capacity: capacity, + }, + }, + } + + unifiedEdges, senderAmount, err = senderAmtBackwardPass( + edgeUnifiers, false, testReceiverAmt, &bandwidthHints, + ) + require.NoError(t, err) + + // For this route, we have some rounding errors, so we can't expect the + // exact amount, but it should be higher than the exact amount, to not + // end up below a min HTLC constraint. + receiverAmt, err = receiverAmtForwardPass(senderAmount, unifiedEdges) + require.NoError(t, err) + require.NotEqual(t, testReceiverAmt, receiverAmt) + require.InDelta(t, int64(testReceiverAmt), int64(receiverAmt), 1) + require.GreaterOrEqual(t, int64(receiverAmt), int64(testReceiverAmt)) } // TestInboundOutbound tests the functions that computes the incoming and