Merge pull request #8941 from bitromortac/fee-limit-inbound

routing: fix fee limit condition

docs/release-notes/release-notes-0.18.3.md

@@ -36,6 +36,10 @@
 * [Fixed a bug](https://github.com/lightningnetwork/lnd/pull/8896) that caused
   LND to use a default fee rate for the batch channel opening flow.
 
+* The fee limit for payments [was made
+  compatible](https://github.com/lightningnetwork/lnd/pull/8941) with inbound
+  fees.
+
 # New Features
 ## Functional Enhancements
 ## RPC Additions
@@ -150,6 +154,7 @@
 # Contributors (Alphabetical Order)
 
 * Andras Banki-Horvath
+* bitromortac
 * Bufo
 * Elle Mouton
 * Matheus Degiovani

itest/list_on_test.go

@@ -354,6 +354,10 @@ var allTestCases = []*lntest.TestCase{
 		Name:     "route fee cutoff",
 		TestFunc: testRouteFeeCutoff,
 	},
+	{
+		Name:     "route fee limit after queryroutes",
+		TestFunc: testFeeLimitAfterQueryRoutes,
+	},
 	{
 		Name:     "rpc middleware interceptor",
 		TestFunc: testRPCMiddlewareInterceptor,

itest/lnd_routing_test.go

@@ -1331,6 +1331,80 @@ func testRouteFeeCutoff(ht *lntest.HarnessTest) {
 	ht.CloseChannel(carol, chanPointCarolDave)
 }
 
+// testFeeLimitAfterQueryRoutes tests that a payment's fee limit is consistent
+// with the fee of a queried route.
+func testFeeLimitAfterQueryRoutes(ht *lntest.HarnessTest) {
+	// Create a three hop network: Alice -> Bob -> Carol.
+	chanAmt := btcutil.Amount(100000)
+	chanPoints, nodes := createSimpleNetwork(
+		ht, []string{}, 3, lntest.OpenChannelParams{Amt: chanAmt},
+	)
+	alice, bob, carol := nodes[0], nodes[1], nodes[2]
+	chanPointAliceBob, chanPointBobCarol := chanPoints[0], chanPoints[1]
+
+	// We set an inbound fee discount on Bob's channel to Alice to
+	// effectively set the outbound fees charged to Carol to zero.
+	expectedPolicy := &lnrpc.RoutingPolicy{
+		FeeBaseMsat:             1000,
+		FeeRateMilliMsat:        1,
+		InboundFeeBaseMsat:      -1000,
+		InboundFeeRateMilliMsat: -1,
+		TimeLockDelta: uint32(
+			chainreg.DefaultBitcoinTimeLockDelta,
+		),
+		MinHtlc:     1000,
+		MaxHtlcMsat: lntest.CalculateMaxHtlc(chanAmt),
+	}
+
+	updateFeeReq := &lnrpc.PolicyUpdateRequest{
+		Scope: &lnrpc.PolicyUpdateRequest_ChanPoint{
+			ChanPoint: chanPointAliceBob,
+		},
+		BaseFeeMsat:   expectedPolicy.FeeBaseMsat,
+		FeeRatePpm:    uint32(expectedPolicy.FeeRateMilliMsat),
+		TimeLockDelta: expectedPolicy.TimeLockDelta,
+		MaxHtlcMsat:   expectedPolicy.MaxHtlcMsat,
+		InboundFee: &lnrpc.InboundFee{
+			BaseFeeMsat: expectedPolicy.InboundFeeBaseMsat,
+			FeeRatePpm:  expectedPolicy.InboundFeeRateMilliMsat,
+		},
+	}
+	bob.RPC.UpdateChannelPolicy(updateFeeReq)
+
+	// Wait for Alice to receive the channel update from Bob.
+	ht.AssertChannelPolicyUpdate(
+		alice, bob, expectedPolicy, chanPointAliceBob, false,
+	)
+
+	// We query the only route available to Carol.
+	queryRoutesReq := &lnrpc.QueryRoutesRequest{
+		PubKey: carol.PubKeyStr,
+		Amt:    paymentAmt,
+	}
+	routesResp := alice.RPC.QueryRoutes(queryRoutesReq)
+
+	// Verify that the route has zero fees.
+	require.Len(ht, routesResp.Routes, 1)
+	require.Len(ht, routesResp.Routes[0].Hops, 2)
+	require.Zero(ht, routesResp.Routes[0].TotalFeesMsat)
+
+	// Attempt a payment with a fee limit of zero.
+	invoice := &lnrpc.Invoice{Value: paymentAmt}
+	invoiceResp := carol.RPC.AddInvoice(invoice)
+	sendReq := &routerrpc.SendPaymentRequest{
+		PaymentRequest: invoiceResp.PaymentRequest,
+		TimeoutSeconds: 60,
+		FeeLimitMsat:   0,
+	}
+
+	// We assert that a route compatible with the fee limit is available.
+	ht.SendPaymentAssertSettled(alice, sendReq)
+
+	// Once we're done, close the channels.
+	ht.CloseChannel(alice, chanPointAliceBob)
+	ht.CloseChannel(bob, chanPointBobCarol)
+}
+
 // computeFee calculates the payment fee as specified in BOLT07.
 func computeFee(baseFee, feeRate, amt lnwire.MilliSatoshi) lnwire.MilliSatoshi {
 	return baseFee + amt*feeRate/1000000

lntest/node/watcher.go

@@ -221,7 +221,7 @@ func (nw *nodeWatcher) WaitForChannelPolicyUpdate(
 		select {
 		// Send a watch request every second.
 		case <-ticker.C:
-			// Did the event can close in the meantime? We want to
+			// Did the event chan close in the meantime? We want to
 			// avoid a "close of closed channel" panic since we're
 			// re-using the same event chan for multiple requests.
 			select {

routing/pathfind.go

@@ -697,7 +697,6 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig,
 	// processEdge is a helper closure that will be used to make sure edges
 	// satisfy our specific requirements.
 	processEdge := func(fromVertex route.Vertex,
-		fromFeatures *lnwire.FeatureVector,
 		edge *unifiedEdge, toNodeDist *nodeWithDist) {
 
 		edgesExpanded++
@@ -724,6 +723,24 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig,
 		amountToSend := toNodeDist.netAmountReceived +
 			lnwire.MilliSatoshi(inboundFee)
 
+		// Check if accumulated fees would exceed fee limit when this
+		// node would be added to the path.
+		totalFee := int64(amountToSend) - int64(amt)
+
+		log.Trace(lnutils.NewLogClosure(func() string {
+			return fmt.Sprintf(
+				"Checking fromVertex (%v) with "+
+					"minInboundFee=%v, inboundFee=%v, "+
+					"amountToSend=%v, amt=%v, totalFee=%v",
+				fromVertex, minInboundFee, inboundFee,
+				amountToSend, amt, totalFee,
+			)
+		}))
+
+		if totalFee > 0 && lnwire.MilliSatoshi(totalFee) > r.FeeLimit {
+			return
+		}
+
 		// Request the success probability for this edge.
 		edgeProbability := r.ProbabilitySource(
 			fromVertex, toNodeDist.node, amountToSend,
@@ -780,13 +797,6 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig,
 		netAmountToReceive := amountToSend +
 			lnwire.MilliSatoshi(outboundFee)
 
-		// Check if accumulated fees would exceed fee limit when this
-		// node would be added to the path.
-		totalFee := int64(netAmountToReceive) - int64(amt)
-		if totalFee > 0 && lnwire.MilliSatoshi(totalFee) > r.FeeLimit {
-			return
-		}
-
 		// Calculate total probability of successfully reaching target
 		// by multiplying the probabilities. Both this edge and the rest
 		// of the route must succeed.
@@ -813,7 +823,7 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig,
 		// weight composed of the fee that this node will charge and
 		// the amount that will be locked for timeLockDelta blocks in
 		// the HTLC that is handed out to fromVertex.
-		weight := edgeWeight(netAmountToReceive, fee, timeLockDelta)
+		weight := edgeWeight(amountToSend, fee, timeLockDelta)
 
 		// Compute the tentative weight to this new channel/edge
 		// which is the weight from our toNode to the target node
@@ -1035,7 +1045,7 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig,
 
 			// Check if this candidate node is better than what we
 			// already have.
-			processEdge(fromNode, fromFeatures, edge, partialPath)
+			processEdge(fromNode, edge, partialPath)
 		}
 
 		if nodeHeap.Len() == 0 {