diff --git a/channeldb/graph_test.go b/channeldb/graph_test.go index 2568512385b..00e30d5b241 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 25bef567ec2..b770ec1fbe4 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 93902d31f2f..322ce3cd090 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/docs/release-notes/release-notes-0.18.3.md b/docs/release-notes/release-notes-0.18.3.md index 4cb59b76d9c..de34239b694 100644 --- a/docs/release-notes/release-notes-0.18.3.md +++ b/docs/release-notes/release-notes-0.18.3.md @@ -84,6 +84,9 @@ * [`ChanInfoRequest`](https://github.com/lightningnetwork/lnd/pull/8813) adds support for channel points. +* [BuildRoute](https://github.com/lightningnetwork/lnd/pull/8886) now supports + inbound fees. + ## lncli Updates * [`importmc`](https://github.com/lightningnetwork/lnd/pull/8779) now accepts diff --git a/lnrpc/routerrpc/router_server.go b/lnrpc/routerrpc/router_server.go index 85b6b2b2c49..a88bb4d0cfe 100644 --- a/lnrpc/routerrpc/router_server.go +++ b/lnrpc/routerrpc/router_server.go @@ -15,6 +15,7 @@ import ( "github.com/btcsuite/btcd/wire" "github.com/grpc-ecosystem/grpc-gateway/v2/runtime" "github.com/lightningnetwork/lnd/channeldb" + "github.com/lightningnetwork/lnd/fn" "github.com/lightningnetwork/lnd/lnrpc" "github.com/lightningnetwork/lnd/lnrpc/invoicesrpc" "github.com/lightningnetwork/lnd/lntypes" @@ -1400,6 +1401,10 @@ func (s *Server) trackPaymentStream(context context.Context, func (s *Server) BuildRoute(_ context.Context, req *BuildRouteRequest) (*BuildRouteResponse, error) { + if len(req.HopPubkeys) == 0 { + return nil, errors.New("no hops specified") + } + // Unmarshall hop list. hops := make([]route.Vertex, len(req.HopPubkeys)) for i, pubkeyBytes := range req.HopPubkeys { @@ -1411,10 +1416,10 @@ func (s *Server) BuildRoute(_ context.Context, } // Prepare BuildRoute call parameters from rpc request. - var amt *lnwire.MilliSatoshi + var amt fn.Option[lnwire.MilliSatoshi] if req.AmtMsat != 0 { rpcAmt := lnwire.MilliSatoshi(req.AmtMsat) - amt = &rpcAmt + amt = fn.Some(rpcAmt) } var outgoingChan *uint64 diff --git a/routing/router.go b/routing/router.go index 5551c53451a..dbbec74c4a6 100644 --- a/routing/router.go +++ b/routing/router.go @@ -5,6 +5,7 @@ import ( "context" "fmt" "math" + "math/big" "sort" "sync" "sync/atomic" @@ -836,6 +837,7 @@ func generateSphinxPacket(rt *route.Route, paymentHash []byte, hopCopy := sphinxPath[i] path[i] = hopCopy } + return spew.Sdump(path) }), ) @@ -1393,24 +1395,22 @@ func (r *ChannelRouter) extractChannelUpdate( // channels that satisfy all requirements. type ErrNoChannel struct { position int - fromNode route.Vertex } // Error returns a human-readable string describing the error. func (e ErrNoChannel) Error() string { return fmt.Sprintf("no matching outgoing channel available for "+ - "node %v (%v)", e.position, e.fromNode) + "node index %v", e.position) } // BuildRoute returns a fully specified route based on a list of pubkeys. If // amount is nil, the minimum routable amount is used. To force a specific // outgoing channel, use the outgoingChan parameter. -func (r *ChannelRouter) BuildRoute(amt *lnwire.MilliSatoshi, +func (r *ChannelRouter) BuildRoute(amt fn.Option[lnwire.MilliSatoshi], hops []route.Vertex, outgoingChan *uint64, finalCltvDelta int32, payAddr *[32]byte) (*route.Route, error) { - log.Tracef("BuildRoute called: hopsCount=%v, amt=%v", - len(hops), amt) + log.Tracef("BuildRoute called: hopsCount=%v, amt=%v", len(hops), amt) var outgoingChans map[uint64]struct{} if outgoingChan != nil { @@ -1419,23 +1419,6 @@ func (r *ChannelRouter) BuildRoute(amt *lnwire.MilliSatoshi, } } - // If no amount is specified, we need to build a route for the minimum - // amount that this route can carry. - useMinAmt := amt == nil - - var runningAmt 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 - } else { - // If an amount is specified, we need to build a route that - // delivers exactly this amount to the final destination. - runningAmt = *amt - } - // We'll attempt to obtain a set of bandwidth hints that helps us select // the best outgoing channel to use in case no outgoing channel is set. bandwidthHints, err := newBandwidthManager( @@ -1445,25 +1428,52 @@ func (r *ChannelRouter) BuildRoute(amt *lnwire.MilliSatoshi, return nil, err } - // Fetch the current block height outside the routing transaction, to - // prevent the rpc call blocking the database. - _, height, err := r.cfg.Chain.GetBestBlock() + sourceNode := r.cfg.SelfNode + + // We check that each node in the route has a connection to others that + // can forward in principle. + unifiers, err := getEdgeUnifiers( + r.cfg.SelfNode, hops, outgoingChans, r.cfg.RoutingGraph, + ) if err != nil { return nil, err } - sourceNode := r.cfg.SelfNode - unifiers, senderAmt, err := getRouteUnifiers( - sourceNode, hops, useMinAmt, runningAmt, outgoingChans, - r.cfg.RoutingGraph, bandwidthHints, + var ( + receiverAmt lnwire.MilliSatoshi + senderAmt lnwire.MilliSatoshi + pathEdges []*unifiedEdge + ) + + // We determine the edges compatible with the requested amount, as well + // as the amount to send, which can be used to determine the final + // receiver amount, if a minimal amount was requested. + pathEdges, senderAmt, err = senderAmtBackwardPass( + unifiers, amt, bandwidthHints, ) if err != nil { return nil, err } - pathEdges, receiverAmt, err := getPathEdges( - sourceNode, senderAmt, unifiers, bandwidthHints, hops, - ) + // For the minimal amount search, we need to do a forward pass to find a + // larger receiver amount due to possible min HTLC bumps, otherwise we + // just use the requested amount. + receiverAmt, err = fn.ElimOption( + amt, + func() fn.Result[lnwire.MilliSatoshi] { + return fn.NewResult( + receiverAmtForwardPass(senderAmt, pathEdges), + ) + }, + fn.Ok[lnwire.MilliSatoshi], + ).Unpack() + if err != nil { + return nil, err + } + + // Fetch the current block height outside the routing transaction, to + // prevent the rpc call blocking the database. + _, height, err := r.cfg.Chain.GetBestBlock() if err != nil { return nil, err } @@ -1481,12 +1491,10 @@ func (r *ChannelRouter) BuildRoute(amt *lnwire.MilliSatoshi, ) } -// getRouteUnifiers returns a list of edge unifiers for the given route. -func getRouteUnifiers(source route.Vertex, hops []route.Vertex, - useMinAmt bool, runningAmt lnwire.MilliSatoshi, - outgoingChans map[uint64]struct{}, graph Graph, - bandwidthHints *bandwidthManager) ([]*edgeUnifier, lnwire.MilliSatoshi, - error) { +// getEdgeUnifiers returns a list of edge unifiers for the given route. +func getEdgeUnifiers(source route.Vertex, hops []route.Vertex, + outgoingChans map[uint64]struct{}, + graph Graph) ([]*edgeUnifier, error) { // Allocate a list that will contain the edge unifiers for this route. unifiers := make([]*edgeUnifier, len(hops)) @@ -1502,100 +1510,388 @@ func getRouteUnifiers(source route.Vertex, hops []route.Vertex, fromNode = hops[i-1] } - localChan := i == 0 - // 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) if err != nil { - return nil, 0, err + return nil, err } // Exit if there are no channels. edgeUnifier, ok := u.edgeUnifiers[fromNode] if !ok { log.Errorf("Cannot find policy for node %v", fromNode) - return nil, 0, ErrNoChannel{ - fromNode: fromNode, - position: i, - } + return nil, ErrNoChannel{position: i} } - // If using min amt, increase amt if needed. - if useMinAmt { - min := edgeUnifier.minAmt() - if min > runningAmt { - runningAmt = min - } + unifiers[i] = edgeUnifier + } + + return unifiers, nil +} + +// senderAmtBackwardPass returns a list of unified edges for the given route and +// determines the amount that should be sent to fulfill min HTLC requirements. +// The minimal sender amount can be searched for by using amt=None. +func senderAmtBackwardPass(unifiers []*edgeUnifier, + amt fn.Option[lnwire.MilliSatoshi], + bandwidthHints bandwidthHints) ([]*unifiedEdge, lnwire.MilliSatoshi, + error) { + + if len(unifiers) == 0 { + return nil, 0, fmt.Errorf("no unifiers provided") + } + + var unifiedEdges = make([]*unifiedEdge, len(unifiers)) + + // We traverse the route backwards and handle the last hop separately. + edgeUnifier := unifiers[len(unifiers)-1] + + // incomingAmt tracks the amount that is forwarded on the edges of a + // route. The last hop only forwards the amount that the receiver should + // receive, as there are no fees paid to the last node. + // 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. + incomingAmt := amt.UnwrapOr(1) + + // If using min amt, increase the amount if needed to fulfill min HTLC + // requirements. + if amt.IsNone() { + min := edgeUnifier.minAmt() + if min > incomingAmt { + incomingAmt = min } + } - // Get an edge for the specific amount that we want to forward. - edge := edgeUnifier.getEdge(runningAmt, bandwidthHints, 0) - if edge == nil { - log.Errorf("Cannot find policy with amt=%v for node %v", - runningAmt, fromNode) + // Get an edge for the specific amount that we want to forward. + edge := edgeUnifier.getEdge(incomingAmt, bandwidthHints, 0) + if edge == nil { + log.Errorf("Cannot find policy with amt=%v "+ + "for hop %v", incomingAmt, len(unifiers)-1) + + return nil, 0, ErrNoChannel{position: len(unifiers) - 1} + } - return nil, 0, ErrNoChannel{ - fromNode: fromNode, - position: i, + unifiedEdges[len(unifiers)-1] = edge + + // Handle the rest of the route except the last hop. + for i := len(unifiers) - 2; i >= 0; i-- { + edgeUnifier = unifiers[i] + + // If using min amt, increase the amount if needed to fulfill + // min HTLC requirements. + if amt.IsNone() { + min := edgeUnifier.minAmt() + if min > incomingAmt { + incomingAmt = min } } - // Add fee for this hop. - if !localChan { - runningAmt += edge.policy.ComputeFee(runningAmt) + // A --current hop-- B --next hop: incomingAmt-- C + // The outbound fee paid to the current end node B is based on + // the amount that the next hop forwards and B's policy for that + // hop. + outboundFee := unifiedEdges[i+1].policy.ComputeFee( + incomingAmt, + ) + + netAmount := incomingAmt + outboundFee + + // We need to select an edge that can forward the requested + // amount. + edge = edgeUnifier.getEdge( + netAmount, bandwidthHints, outboundFee, + ) + if edge == nil { + return nil, 0, ErrNoChannel{position: i} } + // 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) - unifiers[i] = edgeUnifier + // Finally, we update the amount that needs to flow into node B + // from A, which is the next hop's forwarding amount plus the + // fee for B: A --current hop: incomingAmt-- B --next hop-- C + incomingAmt += fee + + unifiedEdges[i] = edge } - return unifiers, runningAmt, nil + return unifiedEdges, incomingAmt, nil } -// getPathEdges returns the edges that make up the path and the total amount, -// including fees, to send the payment. -func getPathEdges(source route.Vertex, receiverAmt lnwire.MilliSatoshi, - unifiers []*edgeUnifier, bandwidthHints *bandwidthManager, - hops []route.Vertex) ([]*unifiedEdge, - lnwire.MilliSatoshi, error) { +// receiverAmtForwardPass returns the amount that a receiver will receive after +// deducting all fees from the sender amount. +func receiverAmtForwardPass(runningAmt lnwire.MilliSatoshi, + unifiedEdges []*unifiedEdge) (lnwire.MilliSatoshi, error) { + + if len(unifiedEdges) == 0 { + return 0, fmt.Errorf("no edges to forward through") + } + + inEdge := unifiedEdges[0] + if !inEdge.amtInRange(runningAmt) { + log.Errorf("Amount %v not in range for hop index %v", + runningAmt, 0) + + return 0, ErrNoChannel{position: 0} + } // 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 []*unifiedEdge - for i, unifier := range unifiers { - edge := unifier.getEdge(receiverAmt, bandwidthHints, 0) - if edge == nil { - fromNode := source - if i > 0 { - fromNode = hops[i-1] - } + for i := 1; i < len(unifiedEdges); i++ { + inEdge := unifiedEdges[i-1] + outEdge := unifiedEdges[i] - return nil, 0, ErrNoChannel{ - fromNode: fromNode, - position: i, - } - } + // Decrease the amount to send while going forward. + runningAmt = outgoingFromIncoming(runningAmt, inEdge, outEdge) - if i > 0 { - // Decrease the amount to send while going forward. - receiverAmt -= edge.policy.ComputeFeeFromIncoming( - receiverAmt, - ) + if !outEdge.amtInRange(runningAmt) { + log.Errorf("Amount %v not in range for hop index %v", + runningAmt, i) + + return 0, ErrNoChannel{position: i} } + } + + return runningAmt, nil +} + +// incomingFromOutgoing computes the incoming amount based on the outgoing +// amount by adding fees to the outgoing amount, replicating the path finding +// and routing process, see also CheckHtlcForward. +func incomingFromOutgoing(outgoingAmt lnwire.MilliSatoshi, + incoming, outgoing *unifiedEdge) lnwire.MilliSatoshi { + + outgoingFee := outgoing.policy.ComputeFee(outgoingAmt) + + // Net amount is the amount the inbound fees are calculated with. + netAmount := outgoingAmt + outgoingFee + + inboundFee := incoming.inboundFees.CalcFee(netAmount) + + // The inbound fee is not allowed to reduce the incoming amount below + // the outgoing amount. + if int64(outgoingFee)+inboundFee < 0 { + return outgoingAmt + } + + return netAmount + lnwire.MilliSatoshi(inboundFee) +} + +// outgoingFromIncoming computes the outgoing amount based on the incoming +// amount by subtracting fees from the incoming amount. Note that this is not +// exactly the inverse of incomingFromOutgoing, because of some rounding. +func outgoingFromIncoming(incomingAmt lnwire.MilliSatoshi, + incoming, outgoing *unifiedEdge) lnwire.MilliSatoshi { + + // Convert all quantities to big.Int to be able to hande negative + // values. The formulas to compute the outgoing amount involve terms + // with PPM*PPM*A, which can easily overflow an int64. + A := big.NewInt(int64(incomingAmt)) + Ro := big.NewInt(int64(outgoing.policy.FeeProportionalMillionths)) + Bo := big.NewInt(int64(outgoing.policy.FeeBaseMSat)) + Ri := big.NewInt(int64(incoming.inboundFees.Rate)) + Bi := big.NewInt(int64(incoming.inboundFees.Base)) + PPM := big.NewInt(1_000_000) + + // The following discussion was contributed by user feelancer21, see + //nolint:lll + // https://github.com/feelancer21/lnd/commit/f6f05fa930985aac0d27c3f6681aada1b599162a. + + // The incoming amount Ai based on the outgoing amount Ao is computed by + // Ai = max(Ai(Ao), Ao), which caps the incoming amount such that the + // total node fee (Ai - Ao) is non-negative. This is commonly enforced + // by routing nodes. + + // The function Ai(Ao) is given by: + // Ai(Ao) = (Ao + Bo + Ro/PPM) + (Bi + (Ao + Ro/PPM + Bo)*Ri/PPM), where + // the first term is the net amount (the outgoing amount plus the + // outbound fee), and the second is the inbound fee computed based on + // the net amount. + + // Ai(Ao) can potentially become more negative in absolute value than + // Ao, which is why the above mentioned capping is needed. We can + // abbreviate Ai(Ao) with Ai(Ao) = m*Ao + n, where m and n are: + // m := (1 + Ro/PPM) * (1 + Ri/PPM) + // n := Bi + Bo*(1 + Ri/PPM) + + // If we know that m > 0, this is equivalent of Ri/PPM > -1, because Ri + // is the only factor that can become negative. A value or Ri/PPM = -1, + // means that the routing node is willing to give up on 100% of the + // net amount (based on the fee rate), which is likely to not happen in + // practice. This condition will be important for a later trick. + + // If we want to compute the incoming amount based on the outgoing + // amount, which is the reverse problem, we need to solve Ai = + // max(Ai(Ao), Ao) for Ao(Ai). Given an incoming amount A, + // we look for an Ao such that A = max(Ai(Ao), Ao). + + // The max function separates this into two cases. The case to take is + // not clear yet, because we don't know Ao, but later we see a trick + // how to determine which case is the one to take. + + // first case: Ai(Ao) <= Ao: + // Therefore, A = max(Ai(Ao), Ao) = Ao, we find Ao = A. + // This also leads to Ai(A) <= A by substitution into the condition. + + // second case: Ai(Ao) > Ao: + // Therefore, A = max(Ai(Ao), Ao) = Ai(Ao) = m*Ao + n. Solving for Ao + // gives Ao = (A - n)/m. + // + // We know + // Ai(Ao) > Ao <=> A = Ai(Ao) > Ao = (A - n)/m, + // so A > (A - n)/m. + // + // **Assuming m > 0**, by multiplying with m, we can transform this to + // A * m + n > A. + // + // We know Ai(A) = A*m + n, therefore Ai(A) > A. + // + // This means that if we apply the incoming amount calculation to the + // **incoming** amount, and this condition holds, then we know that we + // deal with the second case, being able to compute the outgoing amount + // based off the formula Ao = (A - n)/m, otherwise we will just return + // the incoming amount. + + // In case the inbound fee rate is less than -1 (-100%), we fail to + // compute the outbound amount and return the incoming amount. This also + // protects against zero division later. + + // We compute m in terms of big.Int to be safe from overflows and to be + // consistent with later calculations. + // m := (PPM*PPM + Ri*PPM + Ro*PPM + Ro*Ri)/(PPM*PPM) + + // Compute terms in (PPM*PPM + Ri*PPM + Ro*PPM + Ro*Ri). + m1 := new(big.Int).Mul(PPM, PPM) + m2 := new(big.Int).Mul(Ri, PPM) + m3 := new(big.Int).Mul(Ro, PPM) + m4 := new(big.Int).Mul(Ro, Ri) + + // Add up terms m1..m4. + m := big.NewInt(0) + m.Add(m, m1) + m.Add(m, m2) + m.Add(m, m3) + m.Add(m, m4) + + // Since we compare to 0, we can multiply by PPM*PPM to avoid the + // division. + if m.Int64() <= 0 { + return incomingAmt + } + + // In order to decide if the total fee is negative, we apply the fee + // to the *incoming* amount as mentioned before. + + // We compute the test amount in terms of big.Int to be safe from + // overflows and to be consistent later calculations. + // testAmtF := A*m + n = + // = A + Bo + Bi + (PPM*(A*Ri + A*Ro + Ro*Ri) + A*Ri*Ro)/(PPM*PPM) + + // Compute terms in (A*Ri + A*Ro + Ro*Ri). + t1 := new(big.Int).Mul(A, Ri) + t2 := new(big.Int).Mul(A, Ro) + t3 := new(big.Int).Mul(Ro, Ri) + + // Sum up terms t1-t3. + t4 := big.NewInt(0) + t4.Add(t4, t1) + t4.Add(t4, t2) + t4.Add(t4, t3) + + // Compute PPM*(A*Ri + A*Ro + Ro*Ri). + t6 := new(big.Int).Mul(PPM, t4) + + // Compute A*Ri*Ro. + t7 := new(big.Int).Mul(A, Ri) + t7.Mul(t7, Ro) + + // Compute (PPM*(A*Ri + A*Ro + Ro*Ri) + A*Ri*Ro)/(PPM*PPM). + num := new(big.Int).Add(t6, t7) + denom := new(big.Int).Mul(PPM, PPM) + fraction := new(big.Int).Div(num, denom) + + // Sum up all terms. + testAmt := big.NewInt(0) + testAmt.Add(testAmt, A) + testAmt.Add(testAmt, Bo) + testAmt.Add(testAmt, Bi) + testAmt.Add(testAmt, fraction) + + // Protect against negative values for the integer cast to Msat. + if testAmt.Int64() < 0 { + return incomingAmt + } - pathEdges = append(pathEdges, edge) + // If the second case holds, we have to compute the outgoing amount. + if lnwire.MilliSatoshi(testAmt.Int64()) > incomingAmt { + // Compute the outgoing amount by integer ceiling division. This + // precision is needed because PPM*PPM*A and other terms can + // easily overflow with int64, which happens with about + // A = 10_000 sat. + + // out := (A - n) / m = numerator / denominator + // numerator := PPM*(PPM*(A - Bo - Bi) - Bo*Ri) + // denominator := PPM*(PPM + Ri + Ro) + Ri*Ro + + var numerator big.Int + + // Compute (A - Bo - Bi). + temp1 := new(big.Int).Sub(A, Bo) + temp2 := new(big.Int).Sub(temp1, Bi) + + // Compute terms in (PPM*(A - Bo - Bi) - Bo*Ri). + temp3 := new(big.Int).Mul(PPM, temp2) + temp4 := new(big.Int).Mul(Bo, Ri) + + // Compute PPM*(PPM*(A - Bo - Bi) - Bo*Ri) + temp5 := new(big.Int).Sub(temp3, temp4) + numerator.Mul(PPM, temp5) + + var denominator big.Int + + // Compute (PPM + Ri + Ro). + temp1 = new(big.Int).Add(PPM, Ri) + temp2 = new(big.Int).Add(temp1, Ro) + + // Compute PPM*(PPM + Ri + Ro) + Ri*Ro. + temp3 = new(big.Int).Mul(PPM, temp2) + temp4 = new(big.Int).Mul(Ri, Ro) + denominator.Add(temp3, temp4) + + // We overestimate the outgoing amount by taking the ceiling of + // the division. This means that we may round slightly up by a + // MilliSatoshi, but this helps to ensure that we don't hit min + // HTLC constrains in the context of finding the minimum amount + // of a route. + // ceil = floor((numerator + denominator - 1) / denominator) + ceil := new(big.Int).Add(&numerator, &denominator) + ceil.Sub(ceil, big.NewInt(1)) + ceil.Div(ceil, &denominator) + + return lnwire.MilliSatoshi(ceil.Int64()) } - return pathEdges, receiverAmt, nil + // Otherwise the inbound fee made up for the outbound fee, which is why + // we just return the incoming amount. + return incomingAmt } diff --git a/routing/router_test.go b/routing/router_test.go index f631669a9db..9a394a9fd7a 100644 --- a/routing/router_test.go +++ b/routing/router_test.go @@ -24,6 +24,7 @@ import ( "github.com/lightningnetwork/lnd/channeldb" "github.com/lightningnetwork/lnd/channeldb/models" "github.com/lightningnetwork/lnd/clock" + "github.com/lightningnetwork/lnd/fn" "github.com/lightningnetwork/lnd/graph" "github.com/lightningnetwork/lnd/htlcswitch" "github.com/lightningnetwork/lnd/input" @@ -1536,10 +1537,10 @@ func TestBuildRoute(t *testing.T) { }, 6), // Create two channels from b to c. For building routes, we - // expect the lowest cost channel to be selected. Note that this - // isn't a situation that we are expecting in reality. Routing - // nodes are recommended to keep their channel policies towards - // the same peer identical. + // expect the highest cost channel to be selected. Note that + // this isn't a situation that we are expecting in reality. + // Routing nodes are recommended to keep their channel policies + // towards the same peer identical. symmetricTestChannel("b", "c", chanCapSat, &testChannelPolicy{ Expiry: 144, FeeRate: 50000, @@ -1555,6 +1556,8 @@ func TestBuildRoute(t *testing.T) { Features: paymentAddrFeatures, }, 7), + // Create some channels that have conflicting min/max + // constraints. symmetricTestChannel("a", "e", chanCapSat, &testChannelPolicy{ Expiry: 144, FeeRate: 80000, @@ -1569,9 +1572,50 @@ func TestBuildRoute(t *testing.T) { MaxHTLC: lnwire.NewMSatFromSatoshis(chanCapSat), Features: paymentAddrFeatures, }, 4), + + // Create some channels that have a conflicting max HTLC + // constraint for one node pair, similar to the b->c channels. + symmetricTestChannel("b", "z", chanCapSat, &testChannelPolicy{ + Expiry: 144, + FeeRate: 50000, + MinHTLC: lnwire.NewMSatFromSatoshis(20), + MaxHTLC: lnwire.NewMSatFromSatoshis(25), + Features: paymentAddrFeatures, + }, 3), + symmetricTestChannel("b", "z", chanCapSat, &testChannelPolicy{ + Expiry: 144, + FeeRate: 60000, + MinHTLC: lnwire.NewMSatFromSatoshis(20), + 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(t, true, testChannels, "a") + testGraph, err := createTestGraphFromChannels( + t, true, testChannels, "a", + ) require.NoError(t, err, "unable to create graph") const startingBlockHeight = 101 @@ -1583,15 +1627,10 @@ func TestBuildRoute(t *testing.T) { t.Helper() - if len(rt.Hops) != len(expected) { - t.Fatal("hop count mismatch") - } + require.Len(t, rt.Hops, len(expected)) + for i, hop := range rt.Hops { - if hop.ChannelID != expected[i] { - t.Fatalf("expected channel %v at pos %v, but "+ - "got channel %v", - expected[i], i, hop.ChannelID) - } + require.Equal(t, expected[i], hop.ChannelID) } lastHop := rt.Hops[len(rt.Hops)-1] @@ -1603,101 +1642,204 @@ func TestBuildRoute(t *testing.T) { _, err = rand.Read(payAddr[:]) require.NoError(t, err) + noAmt := fn.None[lnwire.MilliSatoshi]() + + // Test that we can't build a route when no hops are given. + hops = []route.Vertex{} + _, err = ctx.router.BuildRoute(noAmt, hops, nil, 40, nil) + require.Error(t, err) + + // Create hop list for an unknown destination. + hops := []route.Vertex{ctx.aliases["b"], ctx.aliases["y"]} + _, err = ctx.router.BuildRoute(noAmt, hops, nil, 40, &payAddr) + noChanErr := ErrNoChannel{} + require.ErrorAs(t, err, &noChanErr) + require.Equal(t, 1, noChanErr.position) + // Create hop list from the route node pubkeys. - hops := []route.Vertex{ - ctx.aliases["b"], ctx.aliases["c"], - } + hops = []route.Vertex{ctx.aliases["b"], ctx.aliases["c"]} amt := lnwire.NewMSatFromSatoshis(100) // Build the route for the given amount. - rt, err := ctx.router.BuildRoute( - &amt, hops, nil, 40, &payAddr, - ) - if err != nil { - t.Fatal(err) - } + rt, err := ctx.router.BuildRoute(fn.Some(amt), hops, nil, 40, &payAddr) + require.NoError(t, err) // Check that we get the expected route back. The total amount should be // the amount to deliver to hop c (100 sats) plus the max fee for the // connection b->c (6 sats). checkHops(rt, []uint64{1, 7}, payAddr) - if rt.TotalAmount != 106000 { - t.Fatalf("unexpected total amount %v", rt.TotalAmount) - } + require.Equal(t, lnwire.MilliSatoshi(106000), rt.TotalAmount) // Build the route for the minimum amount. - rt, err = ctx.router.BuildRoute( - nil, hops, nil, 40, &payAddr, - ) - if err != nil { - t.Fatal(err) - } + rt, err = ctx.router.BuildRoute(noAmt, hops, nil, 40, &payAddr) + require.NoError(t, err) // Check that we get the expected route back. The minimum that we can // send from b to c is 20 sats. Hop b charges 1200 msat for the // forwarding. The channel between hop a and b can carry amounts in the // range [5, 100], so 21200 msats is the minimum amount for this route. checkHops(rt, []uint64{1, 7}, payAddr) - if rt.TotalAmount != 21200 { - t.Fatalf("unexpected total amount %v", rt.TotalAmount) - } + require.Equal(t, lnwire.MilliSatoshi(21200), rt.TotalAmount) + + // The receiver gets sent the minimal HTLC amount. + require.Equal(t, lnwire.MilliSatoshi(20000), rt.Hops[1].AmtToForward) // Test a route that contains incompatible channel htlc constraints. // There is no amount that can pass through both channel 5 and 4. - hops = []route.Vertex{ - ctx.aliases["e"], ctx.aliases["c"], - } - _, err = ctx.router.BuildRoute( - nil, hops, nil, 40, nil, - ) - errNoChannel, ok := err.(ErrNoChannel) - if !ok { - t.Fatalf("expected incompatible policies error, but got %v", - err) - } - if errNoChannel.position != 0 { - t.Fatalf("unexpected no channel error position") - } - if errNoChannel.fromNode != ctx.aliases["a"] { - t.Fatalf("unexpected no channel error node") - } + hops = []route.Vertex{ctx.aliases["e"], ctx.aliases["c"]} + _, err = ctx.router.BuildRoute(noAmt, hops, nil, 40, nil) + require.Error(t, err) + noChanErr = ErrNoChannel{} + require.ErrorAs(t, err, &noChanErr) + require.Equal(t, 0, noChanErr.position) + + // Test a route that contains channel constraints that lead to a + // different selection of a unified edge, when the amount is rescaled + // for the final edge. From a backward pass we expect the policy of + // channel 8 to be used, because its policy has the highest fee rate, + // bumping the amount to 20000 msat leading to a sender amount of 21200 + // msat including the fees for hop over channel 8. In the forward pass + // however, we subtract that fee again, resulting in the min HTLC + // amount. The forward pass doesn't check for a different policy that + // could me more applicable, which is why we don't get back the highest + // amount that could be delivered to the receiver of 21819 msat, using + // policy of channel 3. + hops = []route.Vertex{ctx.aliases["b"], ctx.aliases["z"]} + rt, err = ctx.router.BuildRoute(noAmt, hops, nil, 40, &payAddr) + require.NoError(t, err) + checkHops(rt, []uint64{1, 8}, payAddr) + 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(fn.Some(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(noAmt, hops, nil, 40, &payAddr) + require.NoError(t, err) + checkHops(rt, []uint64{9, 10}, payAddr) + require.EqualValues(t, 20180, rt.TotalAmount, "%v", rt.TotalAmount) } -// TestGetPathEdges tests that the getPathEdges function returns the expected -// edges and amount when given a set of unifiers and does not panic. -func TestGetPathEdges(t *testing.T) { +// TestReceiverAmtForwardPass tests that the forward pass returns the expected +// receiver amount when given a set of edges and does not panic. +func TestReceiverAmtForwardPass(t *testing.T) { t.Parallel() - const startingBlockHeight = 101 - ctx := createTestCtxFromFile(t, startingBlockHeight, basicGraphFilePath) - testCases := []struct { - sourceNode route.Vertex - amt lnwire.MilliSatoshi - unifiers []*edgeUnifier - bandwidthHints *bandwidthManager - hops []route.Vertex - - expectedEdges []*models.CachedEdgePolicy - expectedAmt lnwire.MilliSatoshi - expectedErr string - }{{ - sourceNode: ctx.aliases["roasbeef"], - unifiers: []*edgeUnifier{ - { - edges: []*unifiedEdge{}, - localChan: true, + name string + amt lnwire.MilliSatoshi + unifiedEdges []*unifiedEdge + hops []route.Vertex + + expectedAmt lnwire.MilliSatoshi + expectedErr string + }{ + { + name: "empty", + expectedErr: "no edges to forward through", + }, + { + name: "single edge, no valid policy", + amt: 1000, + unifiedEdges: []*unifiedEdge{ + { + policy: &models.CachedEdgePolicy{ + MinHTLC: 1001, + }, + }, }, + expectedErr: fmt.Sprintf("no matching outgoing " + + "channel available for node index 0"), }, - expectedErr: fmt.Sprintf("no matching outgoing channel "+ - "available for node 0 (%v)", ctx.aliases["roasbeef"]), - }} + { + name: "single edge", + amt: 1000, + unifiedEdges: []*unifiedEdge{ + { + policy: &models.CachedEdgePolicy{ + MinHTLC: 1000, + }, + }, + }, + expectedAmt: 1000, + }, + { + name: "outbound fee, no rounding", + amt: 1e9, + unifiedEdges: []*unifiedEdge{ + { + // The first hop's outbound fee is + // irrelevant in fee calculation. + policy: &models.CachedEdgePolicy{ + FeeBaseMSat: 1234, + FeeProportionalMillionths: 1234, + }, + }, + { + // No rounding is done here. + policy: &models.CachedEdgePolicy{ + FeeBaseMSat: 1000, + FeeProportionalMillionths: 1000, + }, + }, + }, + // From an outgoing amount of 999000000 msat, we get + // in = out + base + out * rate = 1000000000.0 + // + // The inverse outgoing amount for this is + // out = (in - base) / (1 + rate) = + // (1e9 - 1000) / (1 + 1e-3) = 999000000.0000001, + // which is rounded down. + expectedAmt: 999000000, + }, + { + name: "outbound fee, rounding", + amt: 1e9, + unifiedEdges: []*unifiedEdge{ + { + // The first hop's outbound fee is + // irrelevant in fee calculation. + policy: &models.CachedEdgePolicy{ + FeeBaseMSat: 1234, + FeeProportionalMillionths: 1234, + }, + }, + { + // This policy is chosen such that we + // round down. + policy: &models.CachedEdgePolicy{ + FeeBaseMSat: 1000, + FeeProportionalMillionths: 999, + }, + }, + }, + // The float amount for this is + // out = (in - base) / (1 + rate) = + // (1e9 - 1000) / (1 + 999e-6) = 999000998.002995, + // which is rounded up. + expectedAmt: 999000999, + }, + } for _, tc := range testCases { - pathEdges, amt, err := getPathEdges( - tc.sourceNode, tc.amt, tc.unifiers, tc.bandwidthHints, - tc.hops, - ) + amt, err := receiverAmtForwardPass(tc.amt, tc.unifiedEdges) if tc.expectedErr != "" { require.Error(t, err) @@ -1707,11 +1849,285 @@ func TestGetPathEdges(t *testing.T) { } require.NoError(t, err) - require.Equal(t, pathEdges, tc.expectedEdges) require.Equal(t, amt, tc.expectedAmt) } } +// TestSenderAmtBackwardPass tests that the computation of the sender amount is +// done correctly for route building. +func TestSenderAmtBackwardPass(t *testing.T) { + bandwidthHints := bandwidthManager{ + getLink: func(chanId lnwire.ShortChannelID) ( + htlcswitch.ChannelLink, error) { + + return nil, nil + }, + localChans: make(map[lnwire.ShortChannelID]struct{}), + } + + var ( + capacity btcutil.Amount = 1_000_000 + testReceiverAmt lnwire.MilliSatoshi = 1_000_000 + minHTLC lnwire.MilliSatoshi = 1_000 + ) + + edgeUnifiers := []*edgeUnifier{ + { + edges: []*unifiedEdge{ + { + // This outbound fee doesn't have an + // effect (sender doesn't pay outbound). + policy: &models.CachedEdgePolicy{ + FeeBaseMSat: 112, + }, + inboundFees: models.InboundFee{ + Base: 111, + }, + capacity: capacity, + }, + }, + }, + { + edges: []*unifiedEdge{ + { + policy: &models.CachedEdgePolicy{ + FeeBaseMSat: 222, + }, + inboundFees: models.InboundFee{ + Base: 222, + }, + capacity: capacity, + }, + }, + }, + { + edges: []*unifiedEdge{ + { + policy: &models.CachedEdgePolicy{ + 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, + }, + }, + }, + } + + // A search for an amount that is below the minimum HTLC amount should + // fail. + _, _, err := senderAmtBackwardPass( + edgeUnifiers, fn.Some(minHTLC-1), &bandwidthHints, + ) + require.Error(t, err) + + // Do a min amount search. + _, senderAmount, err := senderAmtBackwardPass( + edgeUnifiers, fn.None[lnwire.MilliSatoshi](), &bandwidthHints, + ) + require.NoError(t, err) + require.Equal(t, minHTLC+333+222+222+111, senderAmount) + + // Do a search for a specific amount. + unifiedEdges, senderAmount, err := senderAmtBackwardPass( + edgeUnifiers, fn.Some(testReceiverAmt), &bandwidthHints, + ) + require.NoError(t, err) + 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, fn.Some(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 +// outgoing amounts based on the fees of the incoming and outgoing channels. +func TestInboundOutbound(t *testing.T) { + var outgoingAmt uint64 = 10_000_000 + + tests := []struct { + name string + incomingBase int32 + incomingRate int32 + outgoingBase uint64 + outgoingRate uint64 + }{ + { + name: "only outbound fee", + incomingBase: 0, + incomingRate: 0, + outgoingBase: 20, + outgoingRate: 100, + }, + { + name: "positive inbound and outbound fee", + incomingBase: 20, + incomingRate: 100, + outgoingBase: 20, + outgoingRate: 100, + }, + { + name: "small negative inbound and outbound fee", + incomingBase: -10, + incomingRate: -50, + outgoingBase: 20, + outgoingRate: 100, + }, + { + name: "equal negative inbound and outbound fee", + incomingBase: -20, + incomingRate: -100, + outgoingBase: 20, + outgoingRate: 100, + }, + { + name: "large negative inbound and outbound fee", + incomingBase: -30, + incomingRate: -200, + outgoingBase: 20, + outgoingRate: 100, + }, + { + name: "order of PPM negative inbound and " + + "outbound fee (m=0)", + incomingBase: -30, + incomingRate: -1_000_000, + outgoingBase: 20, + outgoingRate: 100, + }, + { + name: "huge negative inbound and " + + "outbound fee (m<0)", + incomingBase: -30, + incomingRate: -2_000_000, + outgoingBase: 20, + outgoingRate: 100, + }, + } + + for _, tc := range tests { + tc := tc + + t.Run(tc.name, func(tt *testing.T) { + testInboundOutboundFee( + tt, outgoingAmt, tc.incomingBase, + tc.incomingRate, tc.outgoingBase, + tc.outgoingRate, + ) + }) + } +} + +// testInboundOutboundFee is a helper function that tests the outgoing and +// incoming amount relationship. +func testInboundOutboundFee(t *testing.T, outgoingAmt uint64, inBase, + inRate int32, outBase, outRate uint64) { + + debugStr := fmt.Sprintf( + "outAmt=%d, inBase=%d, inRate=%d, outBase=%d, outRate=%d", + outgoingAmt, inBase, inRate, outBase, outRate, + ) + + incomingEdge := &unifiedEdge{ + policy: &models.CachedEdgePolicy{}, + inboundFees: models.InboundFee{ + Base: inBase, + Rate: inRate, + }, + } + + outgoingEdge := &unifiedEdge{ + policy: &models.CachedEdgePolicy{ + FeeBaseMSat: lnwire.MilliSatoshi( + outBase, + ), + FeeProportionalMillionths: lnwire.MilliSatoshi( + outRate, + ), + }, + } + + // We compute the incoming amount based on the outgoing amount, which + // mimicks the path finding process. + incomingAmt := incomingFromOutgoing( + lnwire.MilliSatoshi(outgoingAmt), incomingEdge, + outgoingEdge, + ) + + // We do the reverse and compute the outgoing amount based on the + // incoming amount. + outgoingAmtNew := outgoingFromIncoming( + incomingAmt, incomingEdge, outgoingEdge, + ) + + // We require that the incoming amount is always larger than or equal to + // the outgoing amount, because total fees (=incoming-outgoing) should + // not become negative. + require.GreaterOrEqual( + t, int64(incomingAmt), int64(outgoingAmtNew), debugStr, + "expected incomingAmt >= outgoingAmtNew", + ) + + // We check that up to rounding the amounts are equal. + require.InDelta( + t, int64(outgoingAmt), int64(outgoingAmtNew), 1.0, debugStr, + "expected |outgoingAmt - outgoingAmtNew | <= 1", + ) + + // If we round, the computed outgoing amount should be larger than the + // exact outgoing amount, to not hit any min HTLC limits. + require.GreaterOrEqual( + t, int64(outgoingAmtNew), int64(outgoingAmt), debugStr, + "expected outgoingAmtNew >= outgoingAmt", + ) +} + +// FuzzInboundOutbound tests the incoming and outgoing amount calculation +// functions with fuzzing. +func FuzzInboundOutboundFee(f *testing.F) { + f.Add(uint64(0), int32(0), int32(0), uint64(0), uint64(0)) + + f.Fuzz(testInboundOutboundFee) +} + // TestSendToRouteSkipTempErrSuccess validates a successful payment send. func TestSendToRouteSkipTempErrSuccess(t *testing.T) { t.Parallel()