From a7fe7ae9418605710f91fec21572161f4718445e Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Tue, 24 Oct 2017 18:27:29 -0700 Subject: [PATCH] routing: correct miscalculation multi-hop onion payload fees In this commit we fix an existing miscalculation in the fees that we prescribe within the onion payloads for multi-hop routes. Before this commit, if a route had more than 3 hops, then we would erroneously give the second to last hop zero fees. In this commit we correct this behavior, and also re-write the fee calculation code fragment within newRoute for readability and clarity. There are now only two cases: this is the last hop, and this is any other hop. In the case of the last hop, simply send the exact amount with no additional fee. In the case of an intermediate hop, we use the _prior_ (closer to the destination) hop to calculate the amount of fees we need, which allows us to compute the incoming flow. Using that incoming flow, we then can compute the amount that the hop should forward out. Partially fixes #391. --- routing/pathfind.go | 98 ++++++++++++++++++++++----------------------- 1 file changed, 47 insertions(+), 51 deletions(-) diff --git a/routing/pathfind.go b/routing/pathfind.go index 3c2d63bb..54f70d5d 100644 --- a/routing/pathfind.go +++ b/routing/pathfind.go @@ -242,17 +242,7 @@ func newRoute(amtToSend lnwire.MilliSatoshi, sourceVertex vertex, for i := pathLength - 1; i >= 0; i-- { edge := pathEdges[i] - // Now we create the hop struct for this point in the route. - // The amount to forward is the running amount, and we compute - // the required fee based on this amount. - nextHop := &Hop{ - Channel: edge, - AmtToForward: runningAmt, - Fee: computeFee(runningAmt, edge), - } - edge.Node.PubKey.Curve = nil - - // Next, we'll update both the node and channel index, to + // First, we'll update both the node and channel index, to // indicate that this vertex, and outgoing channel link are // present within this route. v := newVertex(edge.Node.PubKey) @@ -266,6 +256,52 @@ func newRoute(amtToSend lnwire.MilliSatoshi, sourceVertex vertex, route.nextHopMap[v] = route.Hops[i+1].Channel } + // Now we'll start to calculate the items within the per-hop + // payload for this current hop. + // + // If this is the last hop, then we send the exact amount and + // pay no fee, as we're paying directly to the receiver, and + // there're no additional hops. + amtToForward := runningAmt + fee := lnwire.MilliSatoshi(0) + + // If this isn't the last hop, to add enough funds to pay for + // transit over the next link. + if i != len(pathEdges)-1 { + // We'll grab the edge policy and per-hop payload of + // the prior hop so we can calculate fees properly. + prevEdge := pathEdges[i+1] + prevHop := route.Hops[i+1] + + // The fee for this hop, will be based on how much the + // prior hop carried, as we'll need to increase the + // amount of satoshis incoming into this hop to + // properly pay the required fees. + prevAmount := prevHop.AmtToForward + fee = computeFee(prevAmount, prevEdge) + + // With the fee computed, we increment the total amount + // as we need to pay this fee. This value represents + // the amount of funds that will come _into_ this edge. + runningAmt += fee + + // Otherwise, for a node to forward an HTLC, then + // following inequality most hold true: + // + // * amt_in - fee >= amt_to_forward + amtToForward = runningAmt - fee + } + + // Now we create the hop struct for this point in the route. + // The amount to forward is the running amount, and we compute + // the required fee based on this amount. + nextHop := &Hop{ + Channel: edge, + AmtToForward: amtToForward, + Fee: fee, + } + edge.Node.PubKey.Curve = nil + // As a sanity check, we ensure that the selected channel has // enough capacity to forward the required amount which // includes the fee dictated at each hop. @@ -278,46 +314,6 @@ func newRoute(amtToSend lnwire.MilliSatoshi, sourceVertex vertex, return nil, newErrf(ErrInsufficientCapacity, err) } - // We don't pay any fees to ourselves on the first-hop channel, - // so we don't tally up the running fee and amount. - switch { - // If this is a single-hop payment, there's no fee required. - case i == 0 && len(pathEdges) == 1: - nextHop.Fee = 0 - - // If this is the "first" hop in a multi-hop payment, then we - // don't pay any fee to ourselves, but we craft the payload to - // prescribe the proper fee for the _next_ hop. - case i == 0 && len(pathEdges) > 1: - // Now, we'll compute the fee that we need to path this - // hop for its downstream transit. This is the value we - // want coming into the _next_ hop, using the fees from - // the _incoming_ node. - nextFee := computeFee(route.Hops[i+1].AmtToForward, - pathEdges[i+1]) - - nextHop.AmtToForward -= nextFee - nextHop.Fee = 0 - - // Otherwise, this is an intermediate hop, and we compute the - // fees as normal. - default: - // For a node to forward an HTLC, then following - // inequality most hold true: amt_in - fee >= - // amt_to_forward. Therefore we add the fee this node - // consumes in order to calculate the amount that it - // show be forwarded by the prior node which is the - // next hop in our loop. - runningAmt += nextHop.Fee - - // Next we tally the total fees (thus far) in the - // route, and also accumulate the total timelock in the - // route by adding the node's time lock delta which is - // the amount of blocks it'll subtract from the - // incoming time lock. - route.TotalFees += nextHop.Fee - } - // If this is the last hop, then for verification purposes, the // value of the outgoing time-lock should be _exactly_ the // absolute time out they'd expect in the HTLC.