routing: fix route fee calculation and channel capacity check
This commit fixes the logic inside the newRoute function to address the following problems: - Fee calculation for a hop does not include the fee that needs to be paid to the next hop. - The incoming channel capacity "sanity" check does not include the fee to be paid to the current hop.
This commit is contained in:
Joost Jager
parent
70ff4414d9
commit
d4471878df
@ -50,10 +50,11 @@ type HopHint struct {
|
||||
CLTVExpiryDelta uint16
|
||||
}
|
||||
|
||||
// ChannelHop is an intermediate hop within the network with a greater
|
||||
// multi-hop payment route. This struct contains the relevant routing policy of
|
||||
// the particular edge, as well as the total capacity, and origin chain of the
|
||||
// channel itself.
|
||||
// ChannelHop describes the channel through which an intermediate or final
|
||||
// hop can be reached. This struct contains the relevant routing policy of
|
||||
// the particular edge (which is a property of the source node of the channel
|
||||
// edge), as well as the total capacity. It also includes the origin chain of
|
||||
// the channel itself.
|
||||
type ChannelHop struct {
|
||||
// Capacity is the total capacity of the channel being traversed. This
|
||||
// value is expressed for stability in satoshis.
|
||||
@ -69,13 +70,15 @@ type ChannelHop struct {
|
||||
*channeldb.ChannelEdgePolicy
|
||||
}
|
||||
|
||||
// Hop represents the forwarding details at a particular position within the
|
||||
// final route. This struct houses the values necessary to create the HTLC
|
||||
// which will travel along this hop, and also encode the per-hop payload
|
||||
// included within the Sphinx packet.
|
||||
// Hop represents an intermediate or final node of the route. This naming
|
||||
// is in line with the definition given in BOLT #4: Onion Routing Protocol.
|
||||
// The struct houses the channel along which this hop can be reached and
|
||||
// the values necessary to create the HTLC that needs to be sent to the
|
||||
// next hop. It is also used to encode the per-hop payload included within
|
||||
// the Sphinx packet.
|
||||
type Hop struct {
|
||||
// Channel is the active payment channel edge that this hop will travel
|
||||
// along.
|
||||
// Channel is the active payment channel edge along which the packet
|
||||
// travels to reach this hop. This is the _incoming_ channel to this hop.
|
||||
Channel *ChannelHop
|
||||
|
||||
// OutgoingTimeLock is the timelock value that should be used when
|
||||
@ -269,11 +272,6 @@ func newRoute(amtToSend, feeLimit lnwire.MilliSatoshi, sourceVertex Vertex,
|
||||
// information for the first hop so the mapping is sound.
|
||||
route.nextHopMap[sourceVertex] = pathEdges[0]
|
||||
|
||||
// The running amount is the total amount of satoshis required at this
|
||||
// point in the route. We start this value at the amount we want to
|
||||
// send to the destination. This value will then get successively
|
||||
// larger as we compute the fees going backwards.
|
||||
runningAmt := amtToSend
|
||||
pathLength := len(pathEdges)
|
||||
for i := pathLength - 1; i >= 0; i-- {
|
||||
edge := pathEdges[i]
|
||||
@ -285,59 +283,56 @@ func newRoute(amtToSend, feeLimit lnwire.MilliSatoshi, sourceVertex Vertex,
|
||||
route.nodeIndex[v] = struct{}{}
|
||||
route.chanIndex[edge.ChannelID] = struct{}{}
|
||||
|
||||
// If this isn't a direct payment, and this isn't the last hop
|
||||
// in the route, then we'll also populate the nextHop map to
|
||||
// allow easy route traversal by callers.
|
||||
// If this isn't a direct payment, and this isn't the edge to
|
||||
// the last hop in the route, then we'll also populate the
|
||||
// nextHop map to allow easy route traversal by callers.
|
||||
if len(pathEdges) > 1 && i != len(pathEdges)-1 {
|
||||
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
|
||||
// payload for the hop this edge is leading to. This hop will
|
||||
// be called the 'current hop'.
|
||||
|
||||
// If it is the last hop, then the hop payload will contain
|
||||
// the exact amount. In BOLT #4: Onion Routing
|
||||
// Protocol / "Payload for the Last Node", this is detailed.
|
||||
amtToForward := amtToSend
|
||||
|
||||
// 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 isn't the last hop, to add enough funds to pay for
|
||||
// transit over the next link.
|
||||
// If the current hop isn't the last hop, then 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]
|
||||
// We'll grab the per-hop payload of the next hop (the
|
||||
// hop _after_ the hop this edge leads to) in the
|
||||
// route so we can calculate fees properly.
|
||||
nextHop := 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.ChannelEdgePolicy)
|
||||
// The amount that the current hop needs to forward is
|
||||
// based on how much the next hop forwards plus the fee
|
||||
// that needs to be paid to the next hop.
|
||||
amtToForward = nextHop.AmtToForward + nextHop.Fee
|
||||
|
||||
// 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
|
||||
// The fee that needs to be paid to the current hop is
|
||||
// based on the amount that this hop needs to forward
|
||||
// and its policy for the outgoing channel. This policy
|
||||
// is stored as part of the incoming channel of
|
||||
// the next hop.
|
||||
fee = computeFee(amtToForward, nextHop.Channel.ChannelEdgePolicy)
|
||||
}
|
||||
|
||||
// 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{
|
||||
// Now we create the hop struct for the current hop.
|
||||
currentHop := &Hop{
|
||||
Channel: edge,
|
||||
AmtToForward: amtToForward,
|
||||
Fee: fee,
|
||||
}
|
||||
|
||||
route.TotalFees += nextHop.Fee
|
||||
// Accumulate all fees.
|
||||
route.TotalFees += currentHop.Fee
|
||||
|
||||
// Invalidate this route if its total fees exceed our fee limit.
|
||||
if route.TotalFees > feeLimit {
|
||||
@ -346,14 +341,17 @@ func newRoute(amtToSend, feeLimit lnwire.MilliSatoshi, sourceVertex Vertex,
|
||||
return nil, newErrf(ErrFeeLimitExceeded, err)
|
||||
}
|
||||
|
||||
// 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.
|
||||
if nextHop.AmtToForward.ToSatoshis() > nextHop.Channel.Capacity {
|
||||
// As a sanity check, we ensure that the incoming channel has
|
||||
// enough capacity to carry the required amount which
|
||||
// includes the fee dictated at each hop. Make the comparison
|
||||
// in msat to prevent rounding errors.
|
||||
if currentHop.AmtToForward + fee > lnwire.NewMSatFromSatoshis(
|
||||
currentHop.Channel.Capacity) {
|
||||
|
||||
err := fmt.Sprintf("channel graph has insufficient "+
|
||||
"capacity for the payment: need %v, have %v",
|
||||
nextHop.AmtToForward.ToSatoshis(),
|
||||
nextHop.Channel.Capacity)
|
||||
currentHop.AmtToForward.ToSatoshis(),
|
||||
currentHop.Channel.Capacity)
|
||||
|
||||
return nil, newErrf(ErrInsufficientCapacity, err)
|
||||
}
|
||||
@ -367,7 +365,7 @@ func newRoute(amtToSend, feeLimit lnwire.MilliSatoshi, sourceVertex Vertex,
|
||||
// last link in the route.
|
||||
route.TotalTimeLock += uint32(finalCLTVDelta)
|
||||
|
||||
nextHop.OutgoingTimeLock = currentHeight + uint32(finalCLTVDelta)
|
||||
currentHop.OutgoingTimeLock = currentHeight + uint32(finalCLTVDelta)
|
||||
} else {
|
||||
// Next, increment the total timelock of the entire
|
||||
// route such that each hops time lock increases as we
|
||||
@ -380,10 +378,10 @@ func newRoute(amtToSend, feeLimit lnwire.MilliSatoshi, sourceVertex Vertex,
|
||||
// be the value of the time-lock for the _outgoing_
|
||||
// HTLC, so we factor in their specified grace period
|
||||
// (time lock delta).
|
||||
nextHop.OutgoingTimeLock = route.TotalTimeLock - delta
|
||||
currentHop.OutgoingTimeLock = route.TotalTimeLock - delta
|
||||
}
|
||||
|
||||
route.Hops[i] = nextHop
|
||||
route.Hops[i] = currentHop
|
||||
}
|
||||
|
||||
// We'll then make a second run through our route in order to set up
|
||||
@ -393,9 +391,11 @@ func newRoute(amtToSend, feeLimit lnwire.MilliSatoshi, sourceVertex Vertex,
|
||||
route.prevHopMap[vertex] = hop.Channel
|
||||
}
|
||||
|
||||
// The total amount required for this route will be the value the
|
||||
// source extends to the first hop in the route.
|
||||
route.TotalAmount = runningAmt
|
||||
// The total amount required for this route will be the value
|
||||
// that the first hop needs to forward plus the fee that
|
||||
// the first hop charges for this. Note that the sender of the
|
||||
// payment is not a hop in the route.
|
||||
route.TotalAmount = route.Hops[0].AmtToForward + route.Hops[0].Fee
|
||||
|
||||
return route, nil
|
||||
}
|
||||
|
||||
@ -616,6 +616,167 @@ func TestKShortestPathFinding(t *testing.T) {
|
||||
assertExpectedPath(t, paths[1], "roasbeef", "satoshi", "luoji")
|
||||
}
|
||||
|
||||
// TestNewRoute tests whether the construction of hop payloads by newRoute
|
||||
// is executed correctly.
|
||||
func TestNewRoute(t *testing.T) {
|
||||
|
||||
var sourceKey [33]byte
|
||||
sourceVertex := Vertex(sourceKey)
|
||||
|
||||
const (
|
||||
startingHeight = 100
|
||||
finalHopCLTV = 1
|
||||
)
|
||||
|
||||
createHop := func(baseFee lnwire.MilliSatoshi,
|
||||
feeRate lnwire.MilliSatoshi,
|
||||
capacity btcutil.Amount) (*ChannelHop) {
|
||||
|
||||
return &ChannelHop {
|
||||
ChannelEdgePolicy: &channeldb.ChannelEdgePolicy {
|
||||
Node: &channeldb.LightningNode{},
|
||||
FeeProportionalMillionths: feeRate,
|
||||
FeeBaseMSat: baseFee,
|
||||
},
|
||||
Capacity: capacity,
|
||||
}
|
||||
}
|
||||
|
||||
testCases := []struct {
|
||||
name string
|
||||
hops []*ChannelHop
|
||||
paymentAmount lnwire.MilliSatoshi
|
||||
expectedFees []lnwire.MilliSatoshi
|
||||
expectedTotalAmount lnwire.MilliSatoshi
|
||||
expectError bool
|
||||
expectedErrorCode errorCode
|
||||
} {
|
||||
{
|
||||
// For a single hop payment, no fees are expected to be paid.
|
||||
name: "single hop",
|
||||
paymentAmount: 100000,
|
||||
hops: []*ChannelHop {
|
||||
createHop(100, 1000, 1000),
|
||||
},
|
||||
expectedFees: []lnwire.MilliSatoshi {0},
|
||||
expectedTotalAmount: 100000,
|
||||
}, {
|
||||
// For a two hop payment, only the fee for the first hop
|
||||
// needs to be paid. The destination hop does not require
|
||||
// a fee to receive the payment.
|
||||
name: "two hop",
|
||||
paymentAmount: 100000,
|
||||
hops: []*ChannelHop {
|
||||
createHop(0, 1000, 1000),
|
||||
createHop(30, 1000, 1000),
|
||||
},
|
||||
expectedFees: []lnwire.MilliSatoshi {130, 0},
|
||||
expectedTotalAmount: 100130,
|
||||
}, {
|
||||
// Insufficient capacity in first channel when fees are added.
|
||||
name: "two hop insufficient",
|
||||
paymentAmount: 100000,
|
||||
hops: []*ChannelHop {
|
||||
createHop(0, 1000, 100),
|
||||
createHop(0, 1000, 1000),
|
||||
},
|
||||
expectError: true,
|
||||
expectedErrorCode: ErrInsufficientCapacity,
|
||||
}, {
|
||||
// A three hop payment where the first and second hop
|
||||
// will both charge 1 msat. The fee for the first hop
|
||||
// is actually slightly higher than 1, because the amount
|
||||
// to forward also includes the fee for the second hop. This
|
||||
// gets rounded down to 1.
|
||||
name: "three hop",
|
||||
paymentAmount: 100000,
|
||||
hops: []*ChannelHop {
|
||||
createHop(0, 10, 1000),
|
||||
createHop(0, 10, 1000),
|
||||
createHop(0, 10, 1000),
|
||||
},
|
||||
expectedFees: []lnwire.MilliSatoshi {1, 1, 0},
|
||||
expectedTotalAmount: 100002,
|
||||
}, {
|
||||
// A three hop payment where the fee of the first hop
|
||||
// is slightly higher (11) than the fee at the second hop,
|
||||
// because of the increase amount to forward.
|
||||
name: "three hop with fee carry over",
|
||||
paymentAmount: 100000,
|
||||
hops: []*ChannelHop {
|
||||
createHop(0, 10000, 1000),
|
||||
createHop(0, 10000, 1000),
|
||||
createHop(0, 10000, 1000),
|
||||
},
|
||||
expectedFees: []lnwire.MilliSatoshi {1010, 1000, 0},
|
||||
expectedTotalAmount: 102010,
|
||||
}, {
|
||||
// A three hop payment where the fee policies of the first and
|
||||
// second hop are just high enough to show the fee carry over
|
||||
// effect.
|
||||
name: "three hop with minimal fees for carry over",
|
||||
paymentAmount: 100000,
|
||||
hops: []*ChannelHop {
|
||||
createHop(0, 10000, 1000),
|
||||
|
||||
// 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, 1000),
|
||||
|
||||
// Second hop charges a fixed 1000 msat.
|
||||
createHop(1000, 0, 1000),
|
||||
},
|
||||
expectedFees: []lnwire.MilliSatoshi {101, 1000, 0},
|
||||
expectedTotalAmount: 101101,
|
||||
} }
|
||||
|
||||
for _, testCase := range testCases {
|
||||
assertRoute := func(t *testing.T, route *Route) {
|
||||
if route.TotalAmount != testCase.expectedTotalAmount {
|
||||
t.Errorf("Expected total amount is be %v" +
|
||||
", but got %v instead",
|
||||
testCase.expectedTotalAmount,
|
||||
route.TotalAmount)
|
||||
}
|
||||
|
||||
for i := 0; i < len(testCase.expectedFees); i++ {
|
||||
if testCase.expectedFees[i] !=
|
||||
route.Hops[i].Fee {
|
||||
|
||||
t.Errorf("Expected fee for hop %v to " +
|
||||
"be %v, but got %v instead",
|
||||
i, testCase.expectedFees[i],
|
||||
route.Hops[i].Fee)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
t.Run(testCase.name, func(t *testing.T) {
|
||||
route, err := newRoute(testCase.paymentAmount,
|
||||
noFeeLimit,
|
||||
sourceVertex, testCase.hops, startingHeight,
|
||||
finalHopCLTV)
|
||||
|
||||
if testCase.expectError {
|
||||
expectedCode := testCase.expectedErrorCode
|
||||
if err == nil || !IsError(err, expectedCode) {
|
||||
t.Errorf("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)
|
||||
}
|
||||
|
||||
assertRoute(t, route)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestNewRoutePathTooLong(t *testing.T) {
|
||||
t.Skip()
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user