From e52d8291684effab6f8fbd22772d338f297329a0 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Thu, 7 Jun 2018 11:00:58 +0200 Subject: [PATCH] routing: path finding test refactored --- routing/pathfind_test.go | 312 +++++++++++++++--------------- routing/testdata/basic_graph.json | 94 ++++++--- 2 files changed, 221 insertions(+), 185 deletions(-) diff --git a/routing/pathfind_test.go b/routing/pathfind_test.go index 76cc5c2e..15fba724 100644 --- a/routing/pathfind_test.go +++ b/routing/pathfind_test.go @@ -558,6 +558,53 @@ func TestFindLowestFeePath(t *testing.T) { } } +type expectedHop struct { + alias string + fee lnwire.MilliSatoshi + fwdAmount lnwire.MilliSatoshi + timeLock uint32 +} + +type basicGraphPathFindingTestCase struct { + target string + paymentAmt btcutil.Amount + totalAmt lnwire.MilliSatoshi + totalTimeLock uint32 + expectedHops []expectedHop +} + +var basicGraphPathFindingTests = []basicGraphPathFindingTestCase{ + // Basic route with one intermediate hop + {target: "sophon", paymentAmt: 100, totalTimeLock: 102, totalAmt: 100110, + expectedHops: []expectedHop{ + {alias: "songoku", fwdAmount: 100000, fee: 110, timeLock: 101}, + {alias: "sophon", fwdAmount: 100000, fee: 0, timeLock: 101}, + }}, + // Basic direct (one hop) route + {target: "luoji", paymentAmt: 100, totalTimeLock: 101, totalAmt: 100000, + expectedHops: []expectedHop{ + {alias: "luoji", fwdAmount: 100000, fee: 0, timeLock: 101}, + }}, + // Three hop route where fees need to be added in to the forwarding amount. + // The high fee hop phamnewun should be avoided + {target: "elst", paymentAmt: 50000, totalTimeLock: 103, totalAmt: 50050210, + expectedHops: []expectedHop{ + {alias: "songoku", fwdAmount: 50000200, fee: 50010, timeLock: 102}, + {alias: "sophon", fwdAmount: 50000000, fee: 200, timeLock: 101}, + {alias: "elst", fwdAmount: 50000000, fee: 0, timeLock: 101}, + }}, + // Three hop route where fees need to be added in to the forwarding amount. + // However this time the fwdAmount becomes too large for the roasbeef <-> + // songoku channel. Then there is no other option than to choose the + // expensive phamnuwen channel. This test case was failing before + // the route search was executed backwards. + {target: "elst", paymentAmt: 100000, totalTimeLock: 103, totalAmt: 110010220, + expectedHops: []expectedHop{ + {alias: "phamnuwen", fwdAmount: 100000200, fee: 10010020, timeLock: 102}, + {alias: "sophon", fwdAmount: 100000000, fee: 200, timeLock: 101}, + {alias: "elst", fwdAmount: 100000000, fee: 0, timeLock: 101}, + }}} + func TestBasicGraphPathFinding(t *testing.T) { t.Parallel() @@ -567,6 +614,24 @@ func TestBasicGraphPathFinding(t *testing.T) { t.Fatalf("unable to create graph: %v", err) } + // With the test graph loaded, we'll test some basic path finding using + // the pre-generated graph. Consult the testdata/basic_graph.json file + // to follow along with the assumptions we'll use to test the path + // finding. + + for _, testCase := range basicGraphPathFindingTests { + t.Run(testCase.target, func(subT *testing.T) { + testBasicGraphPathFindingCase(subT, graph, aliases, &testCase) + }) + } +} + +func testBasicGraphPathFindingCase(t *testing.T, graph *channeldb.ChannelGraph, + aliases aliasMap, test *basicGraphPathFindingTestCase) { + + expectedHops := test.expectedHops + expectedHopCount := len(expectedHops) + sourceNode, err := graph.SourceNode() if err != nil { t.Fatalf("unable to fetch source node: %v", err) @@ -576,17 +641,13 @@ func TestBasicGraphPathFinding(t *testing.T) { ignoredEdges := make(map[uint64]struct{}) ignoredVertexes := make(map[Vertex]struct{}) - // With the test graph loaded, we'll test some basic path finding using - // the pre-generated graph. Consult the testdata/basic_graph.json file - // to follow along with the assumptions we'll use to test the path - // finding. const ( startingHeight = 100 finalHopCLTV = 1 ) - paymentAmt := lnwire.NewMSatFromSatoshis(100) - target := aliases["sophon"] + paymentAmt := lnwire.NewMSatFromSatoshis(test.paymentAmt) + target := aliases[test.target] path, err := findPath( nil, graph, nil, sourceNode, target, ignoredVertexes, ignoredEdges, paymentAmt, nil, @@ -603,171 +664,116 @@ func TestBasicGraphPathFinding(t *testing.T) { t.Fatalf("unable to create path: %v", err) } - // The length of the route selected should be of exactly length two. - if len(route.Hops) != 2 { - t.Fatalf("route is of incorrect length, expected %v got %v", 2, - len(route.Hops)) + if len(route.Hops) != len(expectedHops) { + t.Fatalf("route is of incorrect length, expected %v got %v", + expectedHopCount, len(route.Hops)) } - // As each hop only decrements a single block from the time-lock, the - // total time lock value should two more than our starting block - // height. - if route.TotalTimeLock != 102 { - t.Fatalf("expected time lock of %v, instead have %v", 2, - route.TotalTimeLock) - } + // Check hop nodes + for i := 0; i < len(expectedHops); i++ { + if !bytes.Equal(route.Hops[i].Channel.Node.PubKeyBytes[:], + aliases[expectedHops[i].alias].SerializeCompressed()) { - // The first hop in the path should be an edge from roasbeef to goku. - if !bytes.Equal(route.Hops[0].Channel.Node.PubKeyBytes[:], - aliases["songoku"].SerializeCompressed()) { - - t.Fatalf("first hop should be goku, is instead: %v", - route.Hops[0].Channel.Node.Alias) - } - - // The second hop should be from goku to sophon. - if !bytes.Equal(route.Hops[1].Channel.Node.PubKeyBytes[:], - aliases["sophon"].SerializeCompressed()) { - - t.Fatalf("second hop should be sophon, is instead: %v", - route.Hops[0].Channel.Node.Alias) + t.Fatalf("%v-th hop should be %v, is instead: %v", + i, expectedHops[i], route.Hops[i].Channel.Node.Alias) + } } // Next, we'll assert that the "next hop" field in each route payload // properly points to the channel ID that the HTLC should be forwarded // along. hopPayloads := route.ToHopPayloads() - if len(hopPayloads) != 2 { + if len(hopPayloads) != expectedHopCount { t.Fatalf("incorrect number of hop payloads: expected %v, got %v", - 2, len(hopPayloads)) + expectedHopCount, len(hopPayloads)) } - // The first hop should point to the second hop. - var expectedHop [8]byte - binary.BigEndian.PutUint64(expectedHop[:], route.Hops[1].Channel.ChannelID) - if !bytes.Equal(hopPayloads[0].NextAddress[:], expectedHop[:]) { - t.Fatalf("first hop has incorrect next hop: expected %x, got %x", - expectedHop[:], hopPayloads[0].NextAddress) + // 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) + if !bytes.Equal(hopPayloads[i].NextAddress[:], expectedHop[:]) { + t.Fatalf("first hop has incorrect next hop: expected %x, got %x", + expectedHop[:], hopPayloads[i].NextAddress) + } } - // The second hop should have a next hop value of all zeroes in order + // The final hop should have a next hop value of all zeroes in order // to indicate it's the exit hop. var exitHop [8]byte - if !bytes.Equal(hopPayloads[1].NextAddress[:], exitHop[:]) { + lastHopIndex := len(expectedHops) - 1 + if !bytes.Equal(hopPayloads[lastHopIndex].NextAddress[:], exitHop[:]) { t.Fatalf("first hop has incorrect next hop: expected %x, got %x", - exitHop[:], hopPayloads[0].NextAddress) + exitHop[:], hopPayloads[lastHopIndex].NextAddress) } - // We'll also assert that the outgoing CLTV value for each hop was set - // accordingly. - if route.Hops[0].OutgoingTimeLock != 101 { - t.Fatalf("expected outgoing time-lock of %v, instead have %v", - 1, route.Hops[0].OutgoingTimeLock) - } - if route.Hops[1].OutgoingTimeLock != 101 { - t.Fatalf("outgoing time-lock for final hop is incorrect: "+ - "expected %v, got %v", 1, route.Hops[1].OutgoingTimeLock) + var expectedTotalFee lnwire.MilliSatoshi = 0 + for i := 0; i < expectedHopCount; i++ { + // We'll ensure that the amount to forward, and fees + // computed for each hop are correct. + + if route.Hops[i].Fee != expectedHops[i].fee { + t.Fatalf("fee incorrect for hop %v: expected %v, got %v", + i, expectedHops[i].fee, route.Hops[i].Fee) + } + + if route.Hops[i].AmtToForward != expectedHops[i].fwdAmount { + t.Fatalf("forwarding amount for hop %v incorrect: "+ + "expected %v, got %v", + i, expectedHops[i].fwdAmount, + route.Hops[i].AmtToForward) + } + + // We'll also assert that the outgoing CLTV value for each + // hop was set accordingly. + if route.Hops[i].OutgoingTimeLock != expectedHops[i].timeLock { + t.Fatalf("outgoing time-lock for hop %v is incorrect: "+ + "expected %v, got %v", i, + expectedHops[i].timeLock, + route.Hops[i].OutgoingTimeLock) + } + + expectedTotalFee += expectedHops[i].fee } - // Additionally, we'll ensure that the amount to forward, and fees - // computed for each hop are correct. - firstHopFee := computeFee( - paymentAmt, route.Hops[1].Channel.ChannelEdgePolicy, - ) - if route.Hops[0].Fee != firstHopFee { - t.Fatalf("first hop fee incorrect: expected %v, got %v", - firstHopFee, route.Hops[0].Fee) + if route.TotalAmount != test.totalAmt { + t.Fatalf("total amount incorrect: "+ + "expected %v, got %v", + test.totalAmt, route.TotalAmount) } - if route.TotalAmount != paymentAmt+firstHopFee { - t.Fatalf("first hop forwarding amount incorrect: expected %v, got %v", - paymentAmt+firstHopFee, route.TotalAmount) - } - if route.Hops[1].Fee != 0 { - t.Fatalf("first hop fee incorrect: expected %v, got %v", - firstHopFee, 0) - } - - if route.Hops[1].AmtToForward != paymentAmt { - t.Fatalf("second hop forwarding amount incorrect: expected %v, got %v", - paymentAmt+firstHopFee, route.Hops[1].AmtToForward) - } - - // Finally, the next and prev hop maps should be properly set. - // - // The previous hop from goku should be the channel from roasbeef, and - // the next hop should be the channel to sophon. - gokuPrevChan, ok := route.prevHopChannel(aliases["songoku"]) - if !ok { - t.Fatalf("goku didn't have next chan but should have") - } - if gokuPrevChan.ChannelID != route.Hops[0].Channel.ChannelID { - t.Fatalf("incorrect prev chan: expected %v, got %v", - gokuPrevChan.ChannelID, route.Hops[0].Channel.ChannelID) - } - gokuNextChan, ok := route.nextHopChannel(aliases["songoku"]) - if !ok { - t.Fatalf("goku didn't have prev chan but should have") - } - if gokuNextChan.ChannelID != route.Hops[1].Channel.ChannelID { - t.Fatalf("incorrect prev chan: expected %v, got %v", - gokuNextChan.ChannelID, route.Hops[1].Channel.ChannelID) - } - - // Sophon shouldn't have a next chan, but she should have a prev chan. - if _, ok := route.nextHopChannel(aliases["sophon"]); ok { - t.Fatalf("incorrect next hop map, no vertexes should " + - "be after sophon") - } - sophonPrevEdge, ok := route.prevHopChannel(aliases["sophon"]) - if !ok { - t.Fatalf("sophon didn't have prev chan but should have") - } - if sophonPrevEdge.ChannelID != route.Hops[1].Channel.ChannelID { - t.Fatalf("incorrect prev chan: expected %v, got %v", - sophonPrevEdge.ChannelID, route.Hops[1].Channel.ChannelID) - } - - // Next, attempt to query for a path to Luo Ji for 100 satoshis, there - // exist two possible paths in the graph, but the shorter (1 hop) path - // should be selected. - target = aliases["luoji"] - path, err = findPath( - nil, graph, nil, sourceNode, target, ignoredVertexes, - ignoredEdges, paymentAmt, nil, - ) - if err != nil { - t.Fatalf("unable to find route: %v", err) - } - - route, err = newRoute( - paymentAmt, noFeeLimit, sourceVertex, path, startingHeight, - finalHopCLTV, - ) - if err != nil { - t.Fatalf("unable to create path: %v", err) - } - - // The length of the path should be exactly one hop as it's the - // "shortest" known path in the graph. - if len(route.Hops) != 1 { - t.Fatalf("shortest path not selected, should be of length 1, "+ - "is instead: %v", len(route.Hops)) - } - - // As we have a direct path, the total time lock value should be - // exactly the current block height plus one. - if route.TotalTimeLock != 101 { - t.Fatalf("expected time lock of %v, instead have %v", 1, + if route.TotalTimeLock != test.totalTimeLock { + t.Fatalf("expected time lock of %v, instead have %v", 2, route.TotalTimeLock) } - // Additionally, since this is a single-hop payment, we shouldn't have - // to pay any fees in total, so the total amount should be the payment - // amount. - if route.TotalAmount != paymentAmt { - t.Fatalf("incorrect total amount, expected %v got %v", - paymentAmt, route.TotalAmount) + // The next and prev hop maps should be properly set. + for i := 0; i < expectedHopCount; i++ { + prevChan, ok := route.prevHopChannel(aliases[expectedHops[i].alias]) + if !ok { + t.Fatalf("hop didn't have prev chan but should have") + } + if prevChan.ChannelID != route.Hops[i].Channel.ChannelID { + t.Fatalf("incorrect prev chan: expected %v, got %v", + prevChan.ChannelID, route.Hops[i].Channel.ChannelID) + } + } + + for i := 0; i < expectedHopCount-1; i++ { + nextChan, ok := route.nextHopChannel(aliases[expectedHops[i].alias]) + if !ok { + t.Fatalf("hop didn't have prev chan but should have") + } + if nextChan.ChannelID != route.Hops[i+1].Channel.ChannelID { + t.Fatalf("incorrect prev chan: expected %v, got %v", + nextChan.ChannelID, route.Hops[i+1].Channel.ChannelID) + } + } + + // Final hop shouldn't have a next chan + if _, ok := route.nextHopChannel(aliases[expectedHops[lastHopIndex].alias]); ok { + t.Fatalf("incorrect next hop map, no vertexes should " + + "be after sophon") } } @@ -1289,10 +1295,10 @@ func TestRouteFailDisabledEdge(t *testing.T) { ignoredEdges := make(map[uint64]struct{}) ignoredVertexes := make(map[Vertex]struct{}) - // First, we'll try to route from roasbeef -> songoku. This should - // succeed without issue, and return a single path. - target := aliases["songoku"] - payAmt := lnwire.NewMSatFromSatoshis(10000) + // First, we'll try to route from roasbeef -> sophon. This should + // succeed without issue, and return a single path via phamnuwen + target := aliases["sophon"] + payAmt := lnwire.NewMSatFromSatoshis(120000) _, err = findPath( nil, graph, nil, sourceNode, target, ignoredVertexes, ignoredEdges, payAmt, nil, @@ -1301,14 +1307,14 @@ func TestRouteFailDisabledEdge(t *testing.T) { t.Fatalf("unable to find path: %v", err) } - // First, we'll modify the edge from roasbeef -> songoku, to read that + // First, we'll modify the edge from roasbeef -> phamnuwen, to read that // it's disabled. - _, gokuEdge, _, err := graph.FetchChannelEdgesByID(12345) + _, _, phamnuwenEdge, err := graph.FetchChannelEdgesByID(999991) if err != nil { t.Fatalf("unable to fetch goku's edge: %v", err) } - gokuEdge.Flags = lnwire.ChanUpdateDisabled - if err := graph.UpdateEdgePolicy(gokuEdge); err != nil { + phamnuwenEdge.Flags = lnwire.ChanUpdateDisabled | lnwire.ChanUpdateDirection + if err := graph.UpdateEdgePolicy(phamnuwenEdge); err != nil { t.Fatalf("unable to update edge: %v", err) } diff --git a/routing/testdata/basic_graph.json b/routing/testdata/basic_graph.json index 48958796..9ba0d87b 100644 --- a/routing/testdata/basic_graph.json +++ b/routing/testdata/basic_graph.json @@ -2,32 +2,33 @@ "info": [ "This file encodes a basic graph that resembles the following ascii graph:", "", -" 50k satoshis ┌──────┐ ", +" 50k satoshis ┌──────┐ ", " ┌───────────────────▶│luo ji│◀─┐ ", -" │ └──────┘ │ ", -" │ │ ", -" │ │ ", -" │ │ ", -" │ │ ", -" │ │ ", +" │ └──────┘ │ ┌──────┐ ", +" │ │ | elst | ", +" │ │ └──────┘ ", +" │ │ ▲ ", +" │ │ | 100k sat ", +" │ │ ▼ ", " ▼ │ ┌──────┐ ", " ┌────────┐ │ │sophon│◀┐ ", " │satoshi │ │ └──────┘ │ ", -" └────────┘ │ │ ", -" ▲ │ │ 500 satoshis ", -" │ ┌───────────────────┘ │ ", -" │ │ 100k satoshis │ ", -" │ │ │ ", -" │ │ │ ┌────────┐ ", -" └──────────┤ └─▶│son goku│ ", -" 10k satoshis │ └────────┘ ", -" │ ▲ ", -" │ │ ", -" │ │ ", -" ▼ │ ", -" ┌──────────┐ │ ", -" │ roasbeef │◀─────────────────────────────────────┘ ", -" └──────────┘ 100k satoshis ", +" └────────┘ │ ▲ │ ", +" ▲ │ | │ 110k satoshis ", +" │ ┌───────────────────┘ | │ ", +" │ │ 100k satoshis | │ ", +" │ │ | │ ", +" │ │ 120k sat | │ ┌────────┐ ", +" └──────────┤ (hi fee) ▼ └─▶│son goku│ ", +" 10k satoshis │ ┌────────────┐ └────────┘ ", +" │ | pham nuwen | ▲ ", +" │ └────────────┘ │ ", +" │ ▲ │ ", +" ▼ | 120k sat (hi fee) │ ", +" ┌──────────┐ | │ ", +" │ roasbeef │◀──────────────┴──────────────────────┘ ", +" └──────────┘ 100k satoshis ", + " the graph also includes a channel from roasbeef to sophon via pham nuwen" ], "nodes": [ @@ -60,9 +61,38 @@ "source": false, "pubkey": "02a1d2856be336a58af08989aea0d8c41e072ccc392c46f8ce0e6e069f002035f3", "alias": "phamnuwen" + }, + { + "source": false, + "pubkey": "02a4b236b69b09b8efe6ccf822fa95ee95a0196451f4d066a450b7489e2e354a64", + "alias": "elst" } ], "edges": [ + { + "node_1": "02a4b236b69b09b8efe6ccf822fa95ee95a0196451f4d066a450b7489e2e354a64", + "node_2": "036264734b40c9e91d3d990a8cdfbbe23b5b0b7ad3cd0e080a25dcd05d39eeb7eb", + "channel_id": 15433, + "channel_point": "33bd5d49a50e284221561b91e781f1fca0d60341c9f9dd785b5e379a6d88af3d:0", + "flags": 1, + "expiry": 1, + "min_htlc": 1000, + "fee_base_msat": 200, + "fee_rate": 0, + "capacity": 100000 + }, + { + "node_1": "02a4b236b69b09b8efe6ccf822fa95ee95a0196451f4d066a450b7489e2e354a64", + "node_2": "036264734b40c9e91d3d990a8cdfbbe23b5b0b7ad3cd0e080a25dcd05d39eeb7eb", + "channel_id": 15433, + "channel_point": "33bd5d49a50e284221561b91e781f1fca0d60341c9f9dd785b5e379a6d88af3d:0", + "flags": 0, + "expiry": 1, + "min_htlc": 1000, + "fee_base_msat": 200, + "fee_rate": 0, + "capacity": 100000 + }, { "node_1": "02a1d2856be336a58af08989aea0d8c41e072ccc392c46f8ce0e6e069f002035f3", "node_2": "0367cec75158a4129177bfb8b269cb586efe93d751b43800d456485e81c2620ca6", @@ -72,8 +102,8 @@ "expiry": 1, "min_htlc": 1000, "fee_base_msat": 10000, - "fee_rate": 1000000, - "capacity": 100000 + "fee_rate": 100000, + "capacity": 120000 }, { "node_1": "02a1d2856be336a58af08989aea0d8c41e072ccc392c46f8ce0e6e069f002035f3", @@ -84,8 +114,8 @@ "expiry": 1, "min_htlc": 1000, "fee_base_msat": 10000, - "fee_rate": 1000000, - "capacity": 100000 + "fee_rate": 100000, + "capacity": 120000 }, { "node_1": "02a1d2856be336a58af08989aea0d8c41e072ccc392c46f8ce0e6e069f002035f3", @@ -96,8 +126,8 @@ "expiry": 1, "min_htlc": 1000, "fee_base_msat": 10000, - "fee_rate": 1000000, - "capacity": 100000 + "fee_rate": 100000, + "capacity": 120000 }, { "node_1": "02a1d2856be336a58af08989aea0d8c41e072ccc392c46f8ce0e6e069f002035f3", @@ -108,8 +138,8 @@ "expiry": 1, "min_htlc": 1000, "fee_base_msat": 10000, - "fee_rate": 1000000, - "capacity": 100000 + "fee_rate": 100000, + "capacity": 120000 }, { "node_1": "0367cec75158a4129177bfb8b269cb586efe93d751b43800d456485e81c2620ca6", @@ -145,7 +175,7 @@ "min_htlc": 1, "fee_base_msat": 10, "fee_rate": 1000, - "capacity": 500 + "capacity": 110000 }, { "node_1": "032b480de5d002f1a8fd1fe1bbf0a0f1b07760f65f052e66d56f15d71097c01add", @@ -157,7 +187,7 @@ "min_htlc": 1, "fee_base_msat": 10, "fee_rate": 1000, - "capacity": 500 + "capacity": 110000 }, { "node_1": "0367cec75158a4129177bfb8b269cb586efe93d751b43800d456485e81c2620ca6",