From 5536f16d4b683c8576a7be6ae98f47ac68f6e557 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Tue, 9 Aug 2022 01:36:00 +0800 Subject: [PATCH] itest: refactor `testInvoiceRoutingHints` --- lntest/itest/list_on_test.go | 4 + lntest/itest/lnd_routing_test.go | 172 ++++++++------------------ lntest/itest/lnd_test_list_on_test.go | 4 - 3 files changed, 56 insertions(+), 124 deletions(-) diff --git a/lntest/itest/list_on_test.go b/lntest/itest/list_on_test.go index 4ba74f732..feb1441e2 100644 --- a/lntest/itest/list_on_test.go +++ b/lntest/itest/list_on_test.go @@ -325,4 +325,8 @@ var allTestCasesTemp = []*lntemp.TestCase{ Name: "private channels", TestFunc: testPrivateChannels, }, + { + Name: "invoice routing hints", + TestFunc: testInvoiceRoutingHints, + }, } diff --git a/lntest/itest/lnd_routing_test.go b/lntest/itest/lnd_routing_test.go index 430b77ff1..2932cb2b9 100644 --- a/lntest/itest/lnd_routing_test.go +++ b/lntest/itest/lnd_routing_test.go @@ -604,9 +604,7 @@ func testPrivateChannels(ht *lntemp.HarnessTest) { // testInvoiceRoutingHints tests that the routing hints for an invoice are // created properly. -func testInvoiceRoutingHints(net *lntest.NetworkHarness, t *harnessTest) { - ctxb := context.Background() - +func testInvoiceRoutingHints(ht *lntemp.HarnessTest) { const chanAmt = btcutil.Amount(100000) // Throughout this test, we'll be opening a channel between Alice and @@ -615,169 +613,106 @@ func testInvoiceRoutingHints(net *lntest.NetworkHarness, t *harnessTest) { // First, we'll create a private channel between Alice and Bob. This // will be the only channel that will be considered as a routing hint // throughout this test. We'll include a push amount since we currently - // require channels to have enough remote balance to cover the invoice's - // payment. - chanPointBob := openChannelAndAssert( - t, net, net.Alice, net.Bob, - lntest.OpenChannelParams{ + // require channels to have enough remote balance to cover the + // invoice's payment. + alice, bob := ht.Alice, ht.Bob + chanPointBob := ht.OpenChannel( + alice, bob, lntemp.OpenChannelParams{ Amt: chanAmt, PushAmt: chanAmt / 2, Private: true, }, ) - // Then, we'll create Carol's node and open a public channel between her - // and Alice. This channel will not be considered as a routing hint due - // to it being public. - carol := net.NewNode(t.t, "Carol", nil) - defer shutdownAndAssert(net, t, carol) + // Then, we'll create Carol's node and open a public channel between + // her and Alice. This channel will not be considered as a routing hint + // due to it being public. + carol := ht.NewNode("Carol", nil) - net.ConnectNodes(t.t, net.Alice, carol) - chanPointCarol := openChannelAndAssert( - t, net, net.Alice, carol, - lntest.OpenChannelParams{ + ht.ConnectNodes(alice, carol) + chanPointCarol := ht.OpenChannel( + alice, carol, lntemp.OpenChannelParams{ Amt: chanAmt, PushAmt: chanAmt / 2, }, ) // We'll also create a public channel between Bob and Carol to ensure - // that Bob gets selected as the only routing hint. We do this as - // we should only include routing hints for nodes that are publicly + // that Bob gets selected as the only routing hint. We do this as we + // should only include routing hints for nodes that are publicly // advertised, otherwise we'd end up leaking information about nodes // that wish to stay unadvertised. - net.ConnectNodes(t.t, net.Bob, carol) - chanPointBobCarol := openChannelAndAssert( - t, net, net.Bob, carol, - lntest.OpenChannelParams{ + ht.ConnectNodes(bob, carol) + chanPointBobCarol := ht.OpenChannel( + bob, carol, lntemp.OpenChannelParams{ Amt: chanAmt, PushAmt: chanAmt / 2, }, ) - // Then, we'll create Dave's node and open a private channel between him - // and Alice. We will not include a push amount in order to not consider - // this channel as a routing hint as it will not have enough remote - // balance for the invoice's amount. - dave := net.NewNode(t.t, "Dave", nil) - defer shutdownAndAssert(net, t, dave) + // Then, we'll create Dave's node and open a private channel between + // him and Alice. We will not include a push amount in order to not + // consider this channel as a routing hint as it will not have enough + // remote balance for the invoice's amount. + dave := ht.NewNode("Dave", nil) - net.ConnectNodes(t.t, net.Alice, dave) - chanPointDave := openChannelAndAssert( - t, net, net.Alice, dave, - lntest.OpenChannelParams{ + ht.ConnectNodes(alice, dave) + chanPointDave := ht.OpenChannel( + alice, dave, lntemp.OpenChannelParams{ Amt: chanAmt, Private: true, }, ) // Finally, we'll create Eve's node and open a private channel between - // her and Alice. This time though, we'll take Eve's node down after the - // channel has been created to avoid populating routing hints for + // her and Alice. This time though, we'll take Eve's node down after + // the channel has been created to avoid populating routing hints for // inactive channels. - eve := net.NewNode(t.t, "Eve", nil) - net.ConnectNodes(t.t, net.Alice, eve) - chanPointEve := openChannelAndAssert( - t, net, net.Alice, eve, - lntest.OpenChannelParams{ + eve := ht.NewNode("Eve", nil) + ht.ConnectNodes(alice, eve) + chanPointEve := ht.OpenChannel( + alice, eve, lntemp.OpenChannelParams{ Amt: chanAmt, PushAmt: chanAmt / 2, Private: true, }, ) - // Make sure all the channels have been opened. - chanNames := []string{ - "alice-bob", "alice-carol", "bob-carol", "alice-dave", - "alice-eve", - } - aliceChans := []*lnrpc.ChannelPoint{ - chanPointBob, chanPointCarol, chanPointBobCarol, chanPointDave, - chanPointEve, - } - for i, chanPoint := range aliceChans { - err := net.Alice.WaitForNetworkChannelOpen(chanPoint) - if err != nil { - t.Fatalf("timed out waiting for channel open %s: %v", - chanNames[i], err) - } - } - - // Now that the channels are open, we'll take down Eve's node. - shutdownAndAssert(net, t, eve) + // Now that the channels are open, we'll disconnect the connection + // between Alice and Eve and then take down Eve's node. + ht.DisconnectNodes(alice, eve) + ht.Shutdown(eve) // We'll need the short channel ID of the channel between Alice and Bob // to make sure the routing hint is for this channel. - listReq := &lnrpc.ListChannelsRequest{} - ctxt, _ := context.WithTimeout(ctxb, defaultTimeout) - listResp, err := net.Alice.ListChannels(ctxt, listReq) - require.NoError(t.t, err, "unable to retrieve alice's channels") - - var aliceBobChanID uint64 - for _, channel := range listResp.Channels { - if channel.RemotePubkey == net.Bob.PubKeyStr { - aliceBobChanID = channel.ChanId - } - } - - require.NotZero(t.t, aliceBobChanID, - "channel between alice and bob not found") + aliceBobChanID := ht.QueryChannelByChanPoint(alice, chanPointBob).ChanId checkInvoiceHints := func(invoice *lnrpc.Invoice) { // Due to the way the channels were set up above, the channel // between Alice and Bob should be the only channel used as a // routing hint. - var predErr error var decoded *lnrpc.PayReq - err := wait.Predicate(func() bool { - ctxt, _ := context.WithTimeout(ctxb, defaultTimeout) - resp, err := net.Alice.AddInvoice(ctxt, invoice) - if err != nil { - predErr = fmt.Errorf( - "unable to add invoice: %w", err) - return false - } + err := wait.NoError(func() error { + resp := alice.RPC.AddInvoice(invoice) // We'll decode the invoice's payment request to // determine which channels were used as routing hints. - payReq := &lnrpc.PayReqString{ - PayReq: resp.PaymentRequest, - } - ctxt, _ = context.WithTimeout(ctxb, defaultTimeout) - decoded, err = net.Alice.DecodePayReq(ctxt, payReq) - if err != nil { - predErr = fmt.Errorf( - "unable to decode payment "+ - "request: %w", err) - - return false - } + decoded = alice.RPC.DecodePayReq(resp.PaymentRequest) if len(decoded.RouteHints) != 1 { - predErr = fmt.Errorf( - "expected one route hint, got %d", - len(decoded.RouteHints)) - - return false + return fmt.Errorf("expected one route hint, "+ + "got %d", len(decoded.RouteHints)) } - return true + return nil }, defaultTimeout) - if err != nil { - t.t.Fatalf(predErr.Error()) - } + require.NoError(ht, err, "timeout checking invoice hints") hops := decoded.RouteHints[0].HopHints - if len(hops) != 1 { - t.t.Fatalf("expected one hop in route hint, got %d", - len(hops)) - } - chanID := hops[0].ChanId + require.Len(ht, hops, 1, "expected one hop in route hint") - if chanID != aliceBobChanID { - t.t.Fatalf("expected channel ID %d, got %d", - aliceBobChanID, chanID) - } + chanID := hops[0].ChanId + require.Equal(ht, aliceBobChanID, chanID, "chanID mismatch") } // Create an invoice for Alice that will populate the routing hints. @@ -799,17 +734,14 @@ func testInvoiceRoutingHints(net *lntest.NetworkHarness, t *harnessTest) { // Now that we've confirmed the routing hints were added correctly, we // can close all the channels and shut down all the nodes created. - closeChannelAndAssert(t, net, net.Alice, chanPointBob, false) - closeChannelAndAssert(t, net, net.Alice, chanPointCarol, false) - closeChannelAndAssert(t, net, net.Bob, chanPointBobCarol, false) - closeChannelAndAssert(t, net, net.Alice, chanPointDave, false) + ht.CloseChannel(alice, chanPointBob) + ht.CloseChannel(alice, chanPointCarol) + ht.CloseChannel(bob, chanPointBobCarol) + ht.CloseChannel(alice, chanPointDave) // The channel between Alice and Eve should be force closed since Eve // is offline. - closeChannelAndAssert(t, net, net.Alice, chanPointEve, true) - - // Cleanup by mining the force close and sweep transaction. - cleanupForceClose(t, net, net.Alice, chanPointEve) + ht.ForceCloseChannel(alice, chanPointEve) } // testMultiHopOverPrivateChannels tests that private channels can be used as diff --git a/lntest/itest/lnd_test_list_on_test.go b/lntest/itest/lnd_test_list_on_test.go index 4b07cb3a5..26c668727 100644 --- a/lntest/itest/lnd_test_list_on_test.go +++ b/lntest/itest/lnd_test_list_on_test.go @@ -8,10 +8,6 @@ var allTestCases = []*testCase{ name: "single hop invoice", test: testSingleHopInvoice, }, - { - name: "invoice routing hints", - test: testInvoiceRoutingHints, - }, { name: "multi-hop payments over private channels", test: testMultiHopOverPrivateChannels,