From a779004a18ca4f9ddc83a639d335c767ebdd0468 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Thu, 9 Aug 2018 15:36:28 +0200 Subject: [PATCH] lnrpc+routing: fix issues with missing data in unmarshallRoute In this commit the dependency of unmarshallRoute on edge policies being available is removed. Edge policies may be unknown and reported as nil. SendToRoute does not need the policies, but it does need pubkeys of the route hops. In this commit, unmarshallRoute is modified so that it takes the pubkeys from edgeInfo instead of channelEdgePolicy. In addition to this, the route structure is simplified. No more connection to the database at that point. Fees are determined based on incoming and outgoing amounts. --- routing/heap.go | 4 +- routing/pathfind.go | 255 +++++++++++++++++---------------------- routing/pathfind_test.go | 113 ++++++++--------- routing/router.go | 18 +-- routing/router_test.go | 62 +++++----- rpcserver.go | 93 ++++++++------ 6 files changed, 269 insertions(+), 276 deletions(-) diff --git a/routing/heap.go b/routing/heap.go index 6d405899..f155fecd 100644 --- a/routing/heap.go +++ b/routing/heap.go @@ -78,9 +78,9 @@ type path struct { // that the path requires. dist int - // hops is an ordered list of edge that comprises a potential payment + // hops is an ordered list of edges that comprises a potential payment // path. - hops []*ChannelHop + hops []*channeldb.ChannelEdgePolicy } // pathHeap is a min-heap that stores potential paths to be considered within diff --git a/routing/pathfind.go b/routing/pathfind.go index 3c3219d2..232da3d5 100644 --- a/routing/pathfind.go +++ b/routing/pathfind.go @@ -9,7 +9,6 @@ import ( "container/heap" "github.com/btcsuite/btcd/btcec" - "github.com/btcsuite/btcd/chaincfg/chainhash" "github.com/coreos/bbolt" "github.com/lightningnetwork/lightning-onion" "github.com/lightningnetwork/lnd/channeldb" @@ -62,30 +61,6 @@ type HopHint struct { CLTVExpiryDelta uint16 } -// 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 { - // Bandwidth is an estimate of the maximum amount that can be sent - // through the channel in the direction indicated by ChannelEdgePolicy. - // It is based on the on-chain capacity of the channel, bandwidth - // hints passed in via SendRoute RPC and/or running amounts that - // represent pending payments. These running amounts have msat as - // unit. Therefore this property is expressed in msat too. - Bandwidth lnwire.MilliSatoshi - - // Chain is a 32-byte has that denotes the base blockchain network of - // the channel. The 32-byte hash is the "genesis" block of the - // blockchain, or the very first block in the chain. - // - // TODO(roasbeef): store chain within edge info/policy in database. - Chain chainhash.Hash - - *channeldb.ChannelEdgePolicy -} - // 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 @@ -93,9 +68,13 @@ type ChannelHop struct { // 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 along which the packet - // travels to reach this hop. This is the _incoming_ channel to this hop. - Channel *ChannelHop + // PubKeyBytes is the raw bytes of the public key of the target node. + PubKeyBytes Vertex + + // ChannelID is the unique channel ID for the channel. The first 3 + // bytes are the block height, the next 3 the index within the block, + // and the last 2 bytes are the output index for the channel. + ChannelID uint64 // OutgoingTimeLock is the timelock value that should be used when // crafting the _outgoing_ HTLC from this hop. @@ -105,11 +84,6 @@ type Hop struct { // hop. This value is less than the value that the incoming HTLC // carries as a fee will be subtracted by the hop. AmtToForward lnwire.MilliSatoshi - - // Fee is the total fee that this hop will subtract from the incoming - // payment, this difference nets the hop fees for forwarding the - // payment. - Fee lnwire.MilliSatoshi } // edgePolicyWithSource is a helper struct to keep track of the source node @@ -131,7 +105,7 @@ func computeFee(amt lnwire.MilliSatoshi, // isSamePath returns true if path1 and path2 travel through the exact same // edges, and false otherwise. -func isSamePath(path1, path2 []*ChannelHop) bool { +func isSamePath(path1, path2 []*channeldb.ChannelEdgePolicy) bool { if len(path1) != len(path2) { return false } @@ -188,25 +162,38 @@ type Route struct { // nextHop maps a node, to the next channel that it will pass the HTLC // off to. With this map, we can easily look up the next outgoing // channel or node for pruning purposes. - nextHopMap map[Vertex]*ChannelHop + nextHopMap map[Vertex]*Hop // prevHop maps a node, to the channel that was directly before it // within the route. With this map, we can easily look up the previous // channel or node for pruning purposes. - prevHopMap map[Vertex]*ChannelHop + prevHopMap map[Vertex]*Hop +} + +// HopFee returns the fee charged by the route hop indicated by hopIndex. +func (r *Route) HopFee(hopIndex int) lnwire.MilliSatoshi { + var incomingAmt lnwire.MilliSatoshi + if hopIndex == 0 { + incomingAmt = r.TotalAmount + } else { + incomingAmt = r.Hops[hopIndex-1].AmtToForward + } + + // Fee is calculated as difference between incoming and outgoing amount. + return incomingAmt - r.Hops[hopIndex].AmtToForward } // nextHopVertex returns the next hop (by Vertex) after the target node. If the // target node is not found in the route, then false is returned. func (r *Route) nextHopVertex(n *btcec.PublicKey) (Vertex, bool) { hop, ok := r.nextHopMap[NewVertex(n)] - return Vertex(hop.Node.PubKeyBytes), ok + return Vertex(hop.PubKeyBytes), ok } // nextHopChannel returns the uint64 channel ID of the next hop after the // target node. If the target node is not found in the route, then false is // returned. -func (r *Route) nextHopChannel(n *btcec.PublicKey) (*ChannelHop, bool) { +func (r *Route) nextHopChannel(n *btcec.PublicKey) (*Hop, bool) { hop, ok := r.nextHopMap[NewVertex(n)] return hop, ok } @@ -214,7 +201,7 @@ func (r *Route) nextHopChannel(n *btcec.PublicKey) (*ChannelHop, bool) { // prevHopChannel returns the uint64 channel ID of the before hop after the // target node. If the target node is not found in the route, then false is // returned. -func (r *Route) prevHopChannel(n *btcec.PublicKey) (*ChannelHop, bool) { +func (r *Route) prevHopChannel(n *btcec.PublicKey) (*Hop, bool) { hop, ok := r.prevHopMap[NewVertex(n)] return hop, ok } @@ -258,7 +245,7 @@ func (r *Route) ToHopPayloads() []sphinx.HopData { // If we aren't on the last hop, then we set the "next address" // field to be the channel that directly follows it. if i != len(r.Hops)-1 { - nextHop = r.Hops[i+1].Channel.ChannelID + nextHop = r.Hops[i+1].ChannelID } binary.BigEndian.PutUint64(hopPayloads[i].NextAddress[:], @@ -276,44 +263,20 @@ func (r *Route) ToHopPayloads() []sphinx.HopData { // NOTE: The passed slice of ChannelHops MUST be sorted in forward order: from // the source to the target node of the path finding attempt. func newRoute(amtToSend, feeLimit lnwire.MilliSatoshi, sourceVertex Vertex, - pathEdges []*ChannelHop, currentHeight uint32, + pathEdges []*channeldb.ChannelEdgePolicy, currentHeight uint32, finalCLTVDelta uint16) (*Route, error) { - // First, we'll create a new empty route with enough hops to match the - // amount of path edges. We set the TotalTimeLock to the current block - // height, as this is the basis that all of the time locks will be - // calculated from. - route := &Route{ - Hops: make([]*Hop, len(pathEdges)), - TotalTimeLock: currentHeight, - nodeIndex: make(map[Vertex]struct{}), - chanIndex: make(map[uint64]struct{}), - nextHopMap: make(map[Vertex]*ChannelHop), - prevHopMap: make(map[Vertex]*ChannelHop), - } + var hops []*Hop - // We'll populate the next hop map for the _source_ node with the - // information for the first hop so the mapping is sound. - route.nextHopMap[sourceVertex] = pathEdges[0] + totalTimeLock := currentHeight pathLength := len(pathEdges) + + var nextIncomingAmount lnwire.MilliSatoshi + for i := pathLength - 1; i >= 0; i-- { edge := pathEdges[i] - // 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 := Vertex(edge.Node.PubKeyBytes) - route.nodeIndex[v] = struct{}{} - route.chanIndex[edge.ChannelID] = struct{}{} - - // 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 the hop this edge is leading to. This hop will // be called the 'current hop'. @@ -330,97 +293,112 @@ func newRoute(amtToSend, feeLimit lnwire.MilliSatoshi, sourceVertex Vertex, // 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 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 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 + // equal to the incoming amount of the next hop. + amtToForward = nextIncomingAmount // 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 the current hop. - currentHop := &Hop{ - Channel: edge, - AmtToForward: amtToForward, - Fee: fee, - } - - // Accumulate all fees. - route.TotalFees += currentHop.Fee - - // Invalidate this route if its total fees exceed our fee limit. - if route.TotalFees > feeLimit { - err := fmt.Sprintf("total route fees exceeded fee "+ - "limit of %v", feeLimit) - return nil, newErrf(ErrFeeLimitExceeded, err) - } - - // 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 > currentHop.Channel.Bandwidth { - - err := fmt.Sprintf("channel graph has insufficient "+ - "capacity for the payment: need %v, have %v", - currentHop.AmtToForward+fee, - currentHop.Channel.Bandwidth) - - return nil, newErrf(ErrInsufficientCapacity, err) + fee = computeFee(amtToForward, pathEdges[i+1]) } // 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. + var outgoingTimeLock uint32 if i == len(pathEdges)-1 { // As this is the last hop, we'll use the specified // final CLTV delta value instead of the value from the // last link in the route. - route.TotalTimeLock += uint32(finalCLTVDelta) + totalTimeLock += uint32(finalCLTVDelta) - currentHop.OutgoingTimeLock = currentHeight + uint32(finalCLTVDelta) + outgoingTimeLock = currentHeight + uint32(finalCLTVDelta) } else { // Next, increment the total timelock of the entire // route such that each hops time lock increases as we // walk backwards in the route, using the delta of the // previous hop. delta := uint32(pathEdges[i+1].TimeLockDelta) - route.TotalTimeLock += delta + totalTimeLock += delta // Otherwise, the value of the outgoing time-lock will // be the value of the time-lock for the _outgoing_ // HTLC, so we factor in their specified grace period // (time lock delta). - currentHop.OutgoingTimeLock = route.TotalTimeLock - delta + outgoingTimeLock = totalTimeLock - delta } - route.Hops[i] = currentHop + // Now we create the hop struct for the current hop. + currentHop := &Hop{ + PubKeyBytes: Vertex(edge.Node.PubKeyBytes), + ChannelID: edge.ChannelID, + AmtToForward: amtToForward, + OutgoingTimeLock: outgoingTimeLock, + } + + hops = append([]*Hop{currentHop}, hops...) + + nextIncomingAmount = amtToForward + fee } - // We'll then make a second run through our route in order to set up - // our prev hop mapping. - for _, hop := range route.Hops { - vertex := Vertex(hop.Channel.Node.PubKeyBytes) - route.prevHopMap[vertex] = hop.Channel + // With the base routing data expressed as hops, build the full route + // structure. + newRoute := NewRouteFromHops(nextIncomingAmount, totalTimeLock, + sourceVertex, hops) + + // Invalidate this route if its total fees exceed our fee limit. + if newRoute.TotalFees > feeLimit { + err := fmt.Sprintf("total route fees exceeded fee "+ + "limit of %v", feeLimit) + return nil, newErrf(ErrFeeLimitExceeded, err) } - // 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 newRoute, nil +} - return route, nil +// NewRouteFromHops creates a new Route structure from the minimally +// required information to perform the payment. It infers fee amounts and +// populates the node, chan and prev/next hop maps. +func NewRouteFromHops(amtToSend lnwire.MilliSatoshi, timeLock uint32, + sourceVertex Vertex, hops []*Hop) *Route { + + // First, we'll create a route struct and populate it with the fields + // for which the values are provided as arguments of this function. + // TotalFees is determined based on the difference between the amount + // that is send from the source and the final amount that is received by + // the destination. + route := &Route{ + Hops: hops, + TotalTimeLock: timeLock, + TotalAmount: amtToSend, + TotalFees: amtToSend - hops[len(hops)-1].AmtToForward, + nodeIndex: make(map[Vertex]struct{}), + chanIndex: make(map[uint64]struct{}), + nextHopMap: make(map[Vertex]*Hop), + prevHopMap: make(map[Vertex]*Hop), + } + + // Then we'll update the node and channel index, to indicate that this + // Vertex and incoming channel link are present within this route. Also, + // the prev and next hop maps will be populated. + prevNode := sourceVertex + for i := 0; i < len(hops); i++ { + hop := hops[i] + + v := Vertex(hop.PubKeyBytes) + + route.nodeIndex[v] = struct{}{} + route.chanIndex[hop.ChannelID] = struct{}{} + route.prevHopMap[v] = hop + route.nextHopMap[prevNode] = hop + + prevNode = v + } + + return route } // Vertex is a simple alias for the serialization of a compressed Bitcoin @@ -474,7 +452,7 @@ func findPath(tx *bolt.Tx, graph *channeldb.ChannelGraph, sourceNode *channeldb.LightningNode, target *btcec.PublicKey, ignoredNodes map[Vertex]struct{}, ignoredEdges map[uint64]struct{}, amt lnwire.MilliSatoshi, feeLimit lnwire.MilliSatoshi, - bandwidthHints map[uint64]lnwire.MilliSatoshi) ([]*ChannelHop, error) { + bandwidthHints map[uint64]lnwire.MilliSatoshi) ([]*channeldb.ChannelEdgePolicy, error) { var err error if tx == nil { @@ -552,7 +530,7 @@ func findPath(tx *bolt.Tx, graph *channeldb.ChannelGraph, // We'll use this map as a series of "next" hop pointers. So to get // from `Vertex` to the target node, we'll take the edge that it's // mapped to within `next`. - next := make(map[Vertex]*ChannelHop) + next := make(map[Vertex]*channeldb.ChannelEdgePolicy) // processEdge is a helper closure that will be used to make sure edges // satisfy our specific requirements. @@ -663,10 +641,7 @@ func findPath(tx *bolt.Tx, graph *channeldb.ChannelGraph, fee: fee, } - next[fromVertex] = &ChannelHop{ - ChannelEdgePolicy: edge, - Bandwidth: bandwidth, - } + next[fromVertex] = edge // Add this new node to our heap as we'd like to further // explore backwards through this edge. @@ -761,7 +736,7 @@ func findPath(tx *bolt.Tx, graph *channeldb.ChannelGraph, } // Use the nextHop map to unravel the forward path from source to target. - pathEdges := make([]*ChannelHop, 0, len(next)) + pathEdges := make([]*channeldb.ChannelEdgePolicy, 0, len(next)) currentNode := sourceVertex for currentNode != targetVertex { // TODO(roasbeef): assumes no cycles // Determine the next hop forward using the next map. @@ -801,7 +776,7 @@ func findPath(tx *bolt.Tx, graph *channeldb.ChannelGraph, func findPaths(tx *bolt.Tx, graph *channeldb.ChannelGraph, source *channeldb.LightningNode, target *btcec.PublicKey, amt lnwire.MilliSatoshi, feeLimit lnwire.MilliSatoshi, numPaths uint32, - bandwidthHints map[uint64]lnwire.MilliSatoshi) ([][]*ChannelHop, error) { + bandwidthHints map[uint64]lnwire.MilliSatoshi) ([][]*channeldb.ChannelEdgePolicy, error) { ignoredEdges := make(map[uint64]struct{}) ignoredVertexes := make(map[Vertex]struct{}) @@ -809,7 +784,7 @@ func findPaths(tx *bolt.Tx, graph *channeldb.ChannelGraph, // TODO(roasbeef): modifying ordering within heap to eliminate final // sorting step? var ( - shortestPaths [][]*ChannelHop + shortestPaths [][]*channeldb.ChannelEdgePolicy candidatePaths pathHeap ) @@ -828,11 +803,9 @@ func findPaths(tx *bolt.Tx, graph *channeldb.ChannelGraph, // Manually insert a "self" edge emanating from ourselves. This // self-edge is required in order for the path finding algorithm to // function properly. - firstPath := make([]*ChannelHop, 0, len(startingPath)+1) - firstPath = append(firstPath, &ChannelHop{ - ChannelEdgePolicy: &channeldb.ChannelEdgePolicy{ - Node: source, - }, + firstPath := make([]*channeldb.ChannelEdgePolicy, 0, len(startingPath)+1) + firstPath = append(firstPath, &channeldb.ChannelEdgePolicy{ + Node: source, }) firstPath = append(firstPath, startingPath...) @@ -908,7 +881,7 @@ func findPaths(tx *bolt.Tx, graph *channeldb.ChannelGraph, // rootPath to the spurPath. newPathLen := len(rootPath) + len(spurPath) newPath := path{ - hops: make([]*ChannelHop, 0, newPathLen), + hops: make([]*channeldb.ChannelEdgePolicy, 0, newPathLen), dist: newPathLen, } newPath.hops = append(newPath.hops, rootPath...) diff --git a/routing/pathfind_test.go b/routing/pathfind_test.go index fb0ab984..3b082a4f 100644 --- a/routing/pathfind_test.go +++ b/routing/pathfind_test.go @@ -592,14 +592,26 @@ func TestFindLowestFeePath(t *testing.T) { } // Assert that the lowest fee route is returned. - if !bytes.Equal(route.Hops[1].Channel.Node.PubKeyBytes[:], + if !bytes.Equal(route.Hops[1].PubKeyBytes[:], testGraphInstance.aliasMap["b"].SerializeCompressed()) { t.Fatalf("expected route to pass through b, "+ "but got a route through %v", - route.Hops[1].Channel.Node.Alias) + getAliasFromPubKey(route.Hops[1].PubKeyBytes[:], + testGraphInstance.aliasMap)) } } +func getAliasFromPubKey(pubKey []byte, + aliases map[string]*btcec.PublicKey) string { + + for alias, key := range aliases { + if bytes.Equal(key.SerializeCompressed(), pubKey) { + return alias + } + } + return "" +} + type expectedHop struct { alias string fee lnwire.MilliSatoshi @@ -733,11 +745,13 @@ func testBasicGraphPathFindingCase(t *testing.T, graphInstance *testGraphInstanc // Check hop nodes for i := 0; i < len(expectedHops); i++ { - if !bytes.Equal(route.Hops[i].Channel.Node.PubKeyBytes[:], + if !bytes.Equal(route.Hops[i].PubKeyBytes[:], aliases[expectedHops[i].alias].SerializeCompressed()) { t.Fatalf("%v-th hop should be %v, is instead: %v", - i, expectedHops[i], route.Hops[i].Channel.Node.Alias) + i, expectedHops[i], + getAliasFromPubKey(route.Hops[i].PubKeyBytes[:], + aliases)) } } @@ -753,7 +767,7 @@ func testBasicGraphPathFindingCase(t *testing.T, graphInstance *testGraphInstanc // Hops should point to the next hop for i := 0; i < len(expectedHops)-1; i++ { var expectedHop [8]byte - binary.BigEndian.PutUint64(expectedHop[:], route.Hops[i+1].Channel.ChannelID) + binary.BigEndian.PutUint64(expectedHop[:], route.Hops[i+1].ChannelID) if !bytes.Equal(hopPayloads[i].NextAddress[:], expectedHop[:]) { t.Fatalf("first hop has incorrect next hop: expected %x, got %x", expectedHop[:], hopPayloads[i].NextAddress) @@ -774,9 +788,10 @@ func testBasicGraphPathFindingCase(t *testing.T, graphInstance *testGraphInstanc // We'll ensure that the amount to forward, and fees // computed for each hop are correct. - if route.Hops[i].Fee != expectedHops[i].fee { + fee := route.HopFee(i) + if fee != expectedHops[i].fee { t.Fatalf("fee incorrect for hop %v: expected %v, got %v", - i, expectedHops[i].fee, route.Hops[i].Fee) + i, expectedHops[i].fee, fee) } if route.Hops[i].AmtToForward != expectedHops[i].fwdAmount { @@ -815,9 +830,9 @@ func testBasicGraphPathFindingCase(t *testing.T, graphInstance *testGraphInstanc if !ok { t.Fatalf("hop didn't have prev chan but should have") } - if prevChan.ChannelID != route.Hops[i].Channel.ChannelID { + if prevChan.ChannelID != route.Hops[i].ChannelID { t.Fatalf("incorrect prev chan: expected %v, got %v", - prevChan.ChannelID, route.Hops[i].Channel.ChannelID) + prevChan.ChannelID, route.Hops[i].ChannelID) } } @@ -826,9 +841,9 @@ func testBasicGraphPathFindingCase(t *testing.T, graphInstance *testGraphInstanc if !ok { t.Fatalf("hop didn't have prev chan but should have") } - if nextChan.ChannelID != route.Hops[i+1].Channel.ChannelID { + if nextChan.ChannelID != route.Hops[i+1].ChannelID { t.Fatalf("incorrect prev chan: expected %v, got %v", - nextChan.ChannelID, route.Hops[i+1].Channel.ChannelID) + nextChan.ChannelID, route.Hops[i+1].ChannelID) } } @@ -969,16 +984,13 @@ func TestNewRoute(t *testing.T) { createHop := func(baseFee lnwire.MilliSatoshi, feeRate lnwire.MilliSatoshi, bandwidth lnwire.MilliSatoshi, - timeLockDelta uint16) *ChannelHop { + timeLockDelta uint16) *channeldb.ChannelEdgePolicy { - return &ChannelHop{ - ChannelEdgePolicy: &channeldb.ChannelEdgePolicy{ - Node: &channeldb.LightningNode{}, - FeeProportionalMillionths: feeRate, - FeeBaseMSat: baseFee, - TimeLockDelta: timeLockDelta, - }, - Bandwidth: bandwidth, + return &channeldb.ChannelEdgePolicy{ + Node: &channeldb.LightningNode{}, + FeeProportionalMillionths: feeRate, + FeeBaseMSat: baseFee, + TimeLockDelta: timeLockDelta, } } @@ -988,7 +1000,7 @@ func TestNewRoute(t *testing.T) { // hops is the list of hops (the route) that gets passed into // the call to newRoute. - hops []*ChannelHop + hops []*channeldb.ChannelEdgePolicy // paymentAmount is the amount that is send into the route // indicated by hops. @@ -1029,7 +1041,7 @@ func TestNewRoute(t *testing.T) { // For a single hop payment, no fees are expected to be paid. name: "single hop", paymentAmount: 100000, - hops: []*ChannelHop{ + hops: []*channeldb.ChannelEdgePolicy{ createHop(100, 1000, 1000000, 10), }, expectedFees: []lnwire.MilliSatoshi{0}, @@ -1043,7 +1055,7 @@ func TestNewRoute(t *testing.T) { // a fee to receive the payment. name: "two hop", paymentAmount: 100000, - hops: []*ChannelHop{ + hops: []*channeldb.ChannelEdgePolicy{ createHop(0, 1000, 1000000, 10), createHop(30, 1000, 1000000, 5), }, @@ -1052,17 +1064,6 @@ func TestNewRoute(t *testing.T) { expectedTotalAmount: 100130, expectedTotalTimeLock: 6, feeLimit: noFeeLimit, - }, { - // Insufficient capacity in first channel when fees are added. - name: "two hop insufficient", - paymentAmount: 100000, - hops: []*ChannelHop{ - createHop(0, 1000, 100000, 10), - createHop(0, 1000, 1000000, 5), - }, - feeLimit: noFeeLimit, - 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 @@ -1071,7 +1072,7 @@ func TestNewRoute(t *testing.T) { // gets rounded down to 1. name: "three hop", paymentAmount: 100000, - hops: []*ChannelHop{ + hops: []*channeldb.ChannelEdgePolicy{ createHop(0, 10, 1000000, 10), createHop(0, 10, 1000000, 5), createHop(0, 10, 1000000, 3), @@ -1087,7 +1088,7 @@ func TestNewRoute(t *testing.T) { // because of the increase amount to forward. name: "three hop with fee carry over", paymentAmount: 100000, - hops: []*ChannelHop{ + hops: []*channeldb.ChannelEdgePolicy{ createHop(0, 10000, 1000000, 10), createHop(0, 10000, 1000000, 5), createHop(0, 10000, 1000000, 3), @@ -1103,7 +1104,7 @@ func TestNewRoute(t *testing.T) { // effect. name: "three hop with minimal fees for carry over", paymentAmount: 100000, - hops: []*ChannelHop{ + hops: []*channeldb.ChannelEdgePolicy{ createHop(0, 10000, 1000000, 10), // First hop charges 0.1% so the second hop fee @@ -1124,7 +1125,7 @@ func TestNewRoute(t *testing.T) { { name: "two hop success with fee limit (greater)", paymentAmount: 100000, - hops: []*ChannelHop{ + hops: []*channeldb.ChannelEdgePolicy{ createHop(0, 1000, 1000000, 144), createHop(0, 1000, 1000000, 144), }, @@ -1136,7 +1137,7 @@ func TestNewRoute(t *testing.T) { }, { name: "two hop success with fee limit (equal)", paymentAmount: 100000, - hops: []*ChannelHop{ + hops: []*channeldb.ChannelEdgePolicy{ createHop(0, 1000, 1000000, 144), createHop(0, 1000, 1000000, 144), }, @@ -1148,7 +1149,7 @@ func TestNewRoute(t *testing.T) { }, { name: "two hop failure with fee limit (smaller)", paymentAmount: 100000, - hops: []*ChannelHop{ + hops: []*channeldb.ChannelEdgePolicy{ createHop(0, 1000, 1000000, 144), createHop(0, 1000, 1000000, 144), }, @@ -1158,7 +1159,7 @@ func TestNewRoute(t *testing.T) { }, { name: "two hop failure with fee limit (zero)", paymentAmount: 100000, - hops: []*ChannelHop{ + hops: []*channeldb.ChannelEdgePolicy{ createHop(0, 1000, 1000000, 144), createHop(0, 1000, 1000000, 144), }, @@ -1177,13 +1178,13 @@ func TestNewRoute(t *testing.T) { } for i := 0; i < len(testCase.expectedFees); i++ { - if testCase.expectedFees[i] != - route.Hops[i].Fee { + fee := route.HopFee(i) + if testCase.expectedFees[i] != fee { t.Errorf("Expected fee for hop %v to "+ "be %v, but got %v instead", i, testCase.expectedFees[i], - route.Hops[i].Fee) + fee) } } @@ -1515,9 +1516,10 @@ func TestPathFindSpecExample(t *testing.T) { t.Fatalf("wrong forward amount: got %v, expected %v", firstRoute.Hops[0].AmtToForward, amt) } - if firstRoute.Hops[0].Fee != 0 { - t.Fatalf("wrong hop fee: got %v, expected %v", - firstRoute.Hops[0].Fee, 0) + + fee := firstRoute.HopFee(0) + if fee != 0 { + t.Fatalf("wrong hop fee: got %v, expected %v", fee, 0) } // The CLTV expiry should be the current height plus 9 (the expiry for @@ -1600,16 +1602,17 @@ func TestPathFindSpecExample(t *testing.T) { // hop, so we should get a fee of exactly: // // * 200 + 4999999 * 2000 / 1000000 = 10199 - if routes[0].Hops[0].Fee != 10199 { - t.Fatalf("wrong hop fee: got %v, expected %v", - routes[0].Hops[0].Fee, 10199) + + fee = routes[0].HopFee(0) + if fee != 10199 { + t.Fatalf("wrong hop fee: got %v, expected %v", fee, 10199) } // While for the final hop, as there's no additional hop afterwards, we // pay no fee. - if routes[0].Hops[1].Fee != 0 { - t.Fatalf("wrong hop fee: got %v, expected %v", - routes[0].Hops[0].Fee, 0) + fee = routes[0].HopFee(1) + if fee != 0 { + t.Fatalf("wrong hop fee: got %v, expected %v", fee, 0) } // The outgoing CLTV value itself should be the current height plus 30 @@ -1677,7 +1680,9 @@ func TestPathFindSpecExample(t *testing.T) { } } -func assertExpectedPath(t *testing.T, path []*ChannelHop, nodeAliases ...string) { +func assertExpectedPath(t *testing.T, path []*channeldb.ChannelEdgePolicy, + nodeAliases ...string) { + if len(path) != len(nodeAliases) { t.Fatal("number of hops and number of aliases do not match") } diff --git a/routing/router.go b/routing/router.go index 5186476f..adc41c26 100644 --- a/routing/router.go +++ b/routing/router.go @@ -1274,7 +1274,7 @@ func pruneChannelFromRoutes(routes []*Route, skipChan uint64) []*Route { // fee information attached. The set of routes returned may be less than the // initial set of paths as it's possible we drop a route if it can't handle the // total payment flow after fees are calculated. -func pathsToFeeSortedRoutes(source Vertex, paths [][]*ChannelHop, +func pathsToFeeSortedRoutes(source Vertex, paths [][]*channeldb.ChannelEdgePolicy, finalCLTVDelta uint16, amt, feeLimit lnwire.MilliSatoshi, currentHeight uint32) ([]*Route, error) { @@ -1461,19 +1461,13 @@ func generateSphinxPacket(route *Route, paymentHash []byte) ([]byte, // in each hop. nodes := make([]*btcec.PublicKey, len(route.Hops)) for i, hop := range route.Hops { - // We create a new instance of the public key to avoid possibly - // mutating the curve parameters, which are unset in a higher - // level in order to avoid spamming the logs. - nodePub, err := hop.Channel.Node.PubKey() + pub, err := btcec.ParsePubKey(hop.PubKeyBytes[:], + btcec.S256()) if err != nil { return nil, nil, err } - pub := btcec.PublicKey{ - Curve: btcec.S256(), - X: nodePub.X, - Y: nodePub.Y, - } - nodes[i] = &pub + + nodes[i] = pub } // Next we generate the per-hop payload which gives each node within @@ -1736,7 +1730,7 @@ func (r *ChannelRouter) sendPayment(payment *LightningPayment, // the payment. If this attempt fails, then we'll continue on // to the next available route. firstHop := lnwire.NewShortChanIDFromInt( - route.Hops[0].Channel.ChannelID, + route.Hops[0].ChannelID, ) preImage, sendError = r.cfg.SendToSwitch( firstHop, htlcAdd, circuit, diff --git a/routing/router_test.go b/routing/router_test.go index 2749c9ea..8bfebe82 100644 --- a/routing/router_test.go +++ b/routing/router_test.go @@ -262,9 +262,12 @@ func TestFindRoutesWithFeeLimit(t *testing.T) { t.Fatalf("expected 2 hops, got %d", len(hops)) } - if hops[0].Channel.Node.Alias != "songoku" { + if !bytes.Equal(hops[0].PubKeyBytes[:], + ctx.aliases["songoku"].SerializeCompressed()) { + t.Fatalf("expected first hop through songoku, got %s", - hops[0].Channel.Node.Alias) + getAliasFromPubKey(hops[0].PubKeyBytes[:], + ctx.aliases)) } } @@ -341,10 +344,13 @@ func TestSendPaymentRouteFailureFallback(t *testing.T) { } // The route should have satoshi as the first hop. - if route.Hops[0].Channel.Node.Alias != "satoshi" { + if !bytes.Equal(route.Hops[0].PubKeyBytes[:], + ctx.aliases["satoshi"].SerializeCompressed()) { + t.Fatalf("route should go through satoshi as first hop, "+ "instead passes through: %v", - route.Hops[0].Channel.Node.Alias) + getAliasFromPubKey(route.Hops[0].PubKeyBytes[:], + ctx.aliases)) } } @@ -406,24 +412,12 @@ func TestChannelUpdateValidation(t *testing.T) { hops := []*Hop{ { - Channel: &ChannelHop{ - ChannelEdgePolicy: &channeldb.ChannelEdgePolicy{ - ChannelID: 1, - Node: &channeldb.LightningNode{ - PubKeyBytes: hop1, - }, - }, - }, + ChannelID: 1, + PubKeyBytes: hop1, }, { - Channel: &ChannelHop{ - ChannelEdgePolicy: &channeldb.ChannelEdgePolicy{ - ChannelID: 2, - Node: &channeldb.LightningNode{ - PubKeyBytes: hop2, - }, - }, - }, + ChannelID: 2, + PubKeyBytes: hop2, }, } @@ -605,10 +599,13 @@ func TestSendPaymentErrorRepeatedFeeInsufficient(t *testing.T) { } // The route should have pham nuwen as the first hop. - if route.Hops[0].Channel.Node.Alias != "phamnuwen" { + if !bytes.Equal(route.Hops[0].PubKeyBytes[:], + ctx.aliases["phamnuwen"].SerializeCompressed()) { + t.Fatalf("route should go through satoshi as first hop, "+ "instead passes through: %v", - route.Hops[0].Channel.Node.Alias) + getAliasFromPubKey(route.Hops[0].PubKeyBytes[:], + ctx.aliases)) } } @@ -701,10 +698,13 @@ func TestSendPaymentErrorNonFinalTimeLockErrors(t *testing.T) { } // The route should have satoshi as the first hop. - if route.Hops[0].Channel.Node.Alias != "phamnuwen" { + if !bytes.Equal(route.Hops[0].PubKeyBytes[:], + ctx.aliases["phamnuwen"].SerializeCompressed()) { + t.Fatalf("route should go through phamnuwen as first hop, "+ "instead passes through: %v", - route.Hops[0].Channel.Node.Alias) + getAliasFromPubKey(route.Hops[0].PubKeyBytes[:], + ctx.aliases)) } } @@ -868,10 +868,13 @@ func TestSendPaymentErrorPathPruning(t *testing.T) { t.Fatalf("incorrect preimage used: expected %x got %x", preImage[:], paymentPreImage[:]) } - if route.Hops[0].Channel.Node.Alias != "satoshi" { + if !bytes.Equal(route.Hops[0].PubKeyBytes[:], + ctx.aliases["satoshi"].SerializeCompressed()) { + t.Fatalf("route should go through satoshi as first hop, "+ "instead passes through: %v", - route.Hops[0].Channel.Node.Alias) + getAliasFromPubKey(route.Hops[0].PubKeyBytes[:], + ctx.aliases)) } ctx.router.missionControl.ResetHistory() @@ -913,10 +916,13 @@ func TestSendPaymentErrorPathPruning(t *testing.T) { } // The route should have satoshi as the first hop. - if route.Hops[0].Channel.Node.Alias != "satoshi" { + if !bytes.Equal(route.Hops[0].PubKeyBytes[:], + ctx.aliases["satoshi"].SerializeCompressed()) { + t.Fatalf("route should go through satoshi as first hop, "+ "instead passes through: %v", - route.Hops[0].Channel.Node.Alias) + getAliasFromPubKey(route.Hops[0].PubKeyBytes[:], + ctx.aliases)) } } diff --git a/rpcserver.go b/rpcserver.go index f3e3abfb..c72c6355 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -1904,7 +1904,7 @@ func (r *rpcServer) savePayment(route *routing.Route, paymentPath := make([][33]byte, len(route.Hops)) for i, hop := range route.Hops { - hopPub := hop.Channel.Node.PubKeyBytes + hopPub := hop.PubKeyBytes copy(paymentPath[i][:], hopPub[:]) } @@ -2014,7 +2014,7 @@ func (r *rpcServer) SendToRoute(stream lnrpc.Lightning_SendToRouteServer) error routes := make([]*routing.Route, len(req.Routes)) for i, rpcroute := range req.Routes { - route, err := unmarshallRoute(rpcroute, graph) + route, err := r.unmarshallRoute(rpcroute, graph) if err != nil { return nil, err } @@ -2419,7 +2419,7 @@ func (r *rpcServer) sendPayment(stream *paymentStream) error { return } - marshalledRouted := marshallRoute(resp.Route) + marshalledRouted := r.marshallRoute(resp.Route) err := stream.send(&lnrpc.SendResponse{ PaymentPreimage: resp.Preimage[:], PaymentRoute: marshalledRouted, @@ -2460,7 +2460,7 @@ func (r *rpcServer) SendToRouteSync(ctx context.Context, routes := make([]*routing.Route, len(req.Routes)) for i, route := range req.Routes { - route, err := unmarshallRoute(route, graph) + route, err := r.unmarshallRoute(route, graph) if err != nil { return nil, err } @@ -2510,7 +2510,7 @@ func (r *rpcServer) sendPaymentSync(ctx context.Context, return &lnrpc.SendResponse{ PaymentPreimage: resp.Preimage[:], - PaymentRoute: marshallRoute(resp.Route), + PaymentRoute: r.marshallRoute(resp.Route), }, nil } @@ -3378,14 +3378,14 @@ func (r *rpcServer) QueryRoutes(ctx context.Context, } for i := int32(0); i < numRoutes; i++ { routeResp.Routes = append( - routeResp.Routes, marshallRoute(routes[i]), + routeResp.Routes, r.marshallRoute(routes[i]), ) } return routeResp, nil } -func marshallRoute(route *routing.Route) *lnrpc.Route { +func (r *rpcServer) marshallRoute(route *routing.Route) *lnrpc.Route { resp := &lnrpc.Route{ TotalTimeLock: route.TotalTimeLock, TotalFees: int64(route.TotalFees.ToSatoshis()), @@ -3394,72 +3394,87 @@ func marshallRoute(route *routing.Route) *lnrpc.Route { TotalAmtMsat: int64(route.TotalAmount), Hops: make([]*lnrpc.Hop, len(route.Hops)), } + graph := r.server.chanDB.ChannelGraph() + incomingAmt := route.TotalAmount for i, hop := range route.Hops { + fee := route.HopFee(i) + + // Channel capacity is not a defining property of a route. For + // backwards RPC compatibility, we retrieve it here from the + // graph. + var chanCapacity btcutil.Amount + info, _, _, err := graph.FetchChannelEdgesByID(hop.ChannelID) + if err == nil { + chanCapacity = info.Capacity + } else { + // If capacity cannot be retrieved, this may be a + // not-yet-received or private channel. Then report + // amount that is sent through the channel as capacity. + chanCapacity = incomingAmt.ToSatoshis() + } + resp.Hops[i] = &lnrpc.Hop{ - ChanId: hop.Channel.ChannelID, - ChanCapacity: int64(hop.Channel.Bandwidth.ToSatoshis()), + ChanId: hop.ChannelID, + ChanCapacity: int64(chanCapacity), AmtToForward: int64(hop.AmtToForward.ToSatoshis()), AmtToForwardMsat: int64(hop.AmtToForward), - Fee: int64(hop.Fee.ToSatoshis()), - FeeMsat: int64(hop.Fee), + Fee: int64(fee.ToSatoshis()), + FeeMsat: int64(fee), Expiry: uint32(hop.OutgoingTimeLock), } + incomingAmt = hop.AmtToForward } return resp } -func unmarshallRoute(rpcroute *lnrpc.Route, +func (r *rpcServer) unmarshallRoute(rpcroute *lnrpc.Route, graph *channeldb.ChannelGraph) (*routing.Route, error) { - route := &routing.Route{ - TotalTimeLock: rpcroute.TotalTimeLock, - TotalFees: lnwire.MilliSatoshi(rpcroute.TotalFeesMsat), - TotalAmount: lnwire.MilliSatoshi(rpcroute.TotalAmtMsat), - Hops: make([]*routing.Hop, len(rpcroute.Hops)), - } - node, err := graph.SourceNode() if err != nil { return nil, fmt.Errorf("unable to fetch source node from graph "+ "while unmarshaling route. %v", err) } + nodePubKeyBytes := node.PubKeyBytes[:] + + hops := make([]*routing.Hop, len(rpcroute.Hops)) for i, hop := range rpcroute.Hops { - edgeInfo, c1, c2, err := graph.FetchChannelEdgesByID(hop.ChanId) + // Discard edge policies, because they may be nil. + edgeInfo, _, _, err := graph.FetchChannelEdgesByID(hop.ChanId) if err != nil { return nil, fmt.Errorf("unable to fetch channel edges by "+ "channel ID for hop (%d): %v", i, err) } - var channelEdgePolicy *channeldb.ChannelEdgePolicy - + var pubKeyBytes [33]byte switch { - case bytes.Equal(node.PubKeyBytes[:], c1.Node.PubKeyBytes[:]): - channelEdgePolicy = c2 - node = c2.Node - case bytes.Equal(node.PubKeyBytes[:], c2.Node.PubKeyBytes[:]): - channelEdgePolicy = c1 - node = c1.Node + case bytes.Equal(nodePubKeyBytes[:], edgeInfo.NodeKey1Bytes[:]): + pubKeyBytes = edgeInfo.NodeKey2Bytes + case bytes.Equal(nodePubKeyBytes[:], edgeInfo.NodeKey2Bytes[:]): + pubKeyBytes = edgeInfo.NodeKey1Bytes default: - return nil, fmt.Errorf("could not find channel edge for hop=%d", i) + return nil, fmt.Errorf("channel edge does not match expected node") } - routingHop := &routing.ChannelHop{ - ChannelEdgePolicy: channelEdgePolicy, - Bandwidth: lnwire.NewMSatFromSatoshis( - btcutil.Amount(hop.ChanCapacity)), - Chain: edgeInfo.ChainHash, - } - - route.Hops[i] = &routing.Hop{ - Channel: routingHop, + hops[i] = &routing.Hop{ OutgoingTimeLock: hop.Expiry, AmtToForward: lnwire.MilliSatoshi(hop.AmtToForwardMsat), - Fee: lnwire.MilliSatoshi(hop.FeeMsat), + PubKeyBytes: pubKeyBytes, + ChannelID: edgeInfo.ChannelID, } + + nodePubKeyBytes = pubKeyBytes[:] } + route := routing.NewRouteFromHops( + lnwire.MilliSatoshi(rpcroute.TotalAmtMsat), + rpcroute.TotalTimeLock, + node.PubKeyBytes, + hops, + ) + return route, nil }