From 25b6ff33366c920e61af987926f75d55d5d0a85a Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Tue, 9 Aug 2022 01:27:10 +0800 Subject: [PATCH] itest: refactor `testSendToRouteErrorPropagation` --- lntemp/harness_assertion.go | 32 ++++++++++ lntemp/rpc/lnd.go | 4 +- lntest/itest/list_on_test.go | 4 ++ lntest/itest/lnd_routing_test.go | 90 ++++++++------------------- lntest/itest/lnd_test_list_on_test.go | 4 -- 5 files changed, 64 insertions(+), 70 deletions(-) diff --git a/lntemp/harness_assertion.go b/lntemp/harness_assertion.go index a1c98be2d..e2d7c05e3 100644 --- a/lntemp/harness_assertion.go +++ b/lntemp/harness_assertion.go @@ -2042,3 +2042,35 @@ func (h *HarnessTest) AssertNumInvoices(hn *node.HarnessNode, return invoices } + +// ReceiveSendToRouteUpdate waits until a message is received on the +// SendToRoute client stream or the timeout is reached. +func (h *HarnessTest) ReceiveSendToRouteUpdate( + stream rpc.SendToRouteClient) (*lnrpc.SendResponse, error) { + + chanMsg := make(chan *lnrpc.SendResponse, 1) + errChan := make(chan error, 1) + go func() { + // Consume one message. This will block until the message is + // received. + resp, err := stream.Recv() + if err != nil { + errChan <- err + + return + } + chanMsg <- resp + }() + + select { + case <-time.After(DefaultTimeout): + require.Fail(h, "timeout", "timeout waiting for send resp") + return nil, nil + + case err := <-errChan: + return nil, err + + case updateMsg := <-chanMsg: + return updateMsg, nil + } +} diff --git a/lntemp/rpc/lnd.go b/lntemp/rpc/lnd.go index 7a451e3c2..53156dbec 100644 --- a/lntemp/rpc/lnd.go +++ b/lntemp/rpc/lnd.go @@ -481,8 +481,10 @@ func (h *HarnessRPC) QueryRoutes( return routes } +type SendToRouteClient lnrpc.Lightning_SendToRouteClient + // SendToRoute makes a RPC call to SendToRoute and asserts. -func (h *HarnessRPC) SendToRoute() lnrpc.Lightning_SendToRouteClient { +func (h *HarnessRPC) SendToRoute() SendToRouteClient { // SendToRoute needs to have the context alive for the entire test case // as the returned client will be used for send and receive payment // stream. Thus we use runCtx here instead of a timeout context. diff --git a/lntest/itest/list_on_test.go b/lntest/itest/list_on_test.go index 088897581..242d4b944 100644 --- a/lntest/itest/list_on_test.go +++ b/lntest/itest/list_on_test.go @@ -317,4 +317,8 @@ var allTestCasesTemp = []*lntemp.TestCase{ Name: "multi-hop send to route", TestFunc: testMultiHopSendToRoute, }, + { + Name: "send to route error propagation", + TestFunc: testSendToRouteErrorPropagation, + }, } diff --git a/lntest/itest/lnd_routing_test.go b/lntest/itest/lnd_routing_test.go index 4a08c6db6..da306fbe8 100644 --- a/lntest/itest/lnd_routing_test.go +++ b/lntest/itest/lnd_routing_test.go @@ -4,7 +4,6 @@ import ( "context" "encoding/hex" "fmt" - "strings" "testing" "time" @@ -410,54 +409,32 @@ func runMultiHopSendToRoute(ht *lntemp.HarnessTest, useGraphCache bool) { // testSendToRouteErrorPropagation tests propagation of errors that occur // while processing a multi-hop payment through an unknown route. -func testSendToRouteErrorPropagation(net *lntest.NetworkHarness, t *harnessTest) { - ctxb := context.Background() - +func testSendToRouteErrorPropagation(ht *lntemp.HarnessTest) { const chanAmt = btcutil.Amount(100000) // Open a channel with 100k satoshis between Alice and Bob with Alice // being the sole funder of the channel. - chanPointAlice := openChannelAndAssert( - t, net, net.Alice, net.Bob, - lntest.OpenChannelParams{ - Amt: chanAmt, - }, + alice, bob := ht.Alice, ht.Bob + chanPointAlice := ht.OpenChannel( + alice, bob, lntemp.OpenChannelParams{Amt: chanAmt}, ) - err := net.Alice.WaitForNetworkChannelOpen(chanPointAlice) - if err != nil { - t.Fatalf("alice didn't advertise her channel: %v", err) - } - // Create a new nodes (Carol and Charlie), load her with some funds, // then establish a connection between Carol and Charlie with a channel // that has identical capacity to the one created above.Then we will // get route via queryroutes call which will be fake route for Alice -> // Bob graph. // - // The network topology should now look like: Alice -> Bob; Carol -> Charlie. - carol := net.NewNode(t.t, "Carol", nil) - defer shutdownAndAssert(net, t, carol) + // The network topology should now look like: + // Alice -> Bob; Carol -> Charlie. + carol := ht.NewNode("Carol", nil) + ht.FundCoins(btcutil.SatoshiPerBitcoin, carol) - net.SendCoins(t.t, btcutil.SatoshiPerBitcoin, carol) + charlie := ht.NewNode("Charlie", nil) + ht.FundCoins(btcutil.SatoshiPerBitcoin, charlie) - charlie := net.NewNode(t.t, "Charlie", nil) - defer shutdownAndAssert(net, t, charlie) - - net.SendCoins(t.t, btcutil.SatoshiPerBitcoin, charlie) - - net.ConnectNodes(t.t, carol, charlie) - - chanPointCarol := openChannelAndAssert( - t, net, carol, charlie, - lntest.OpenChannelParams{ - Amt: chanAmt, - }, - ) - err = carol.WaitForNetworkChannelOpen(chanPointCarol) - if err != nil { - t.Fatalf("carol didn't advertise her channel: %v", err) - } + ht.ConnectNodes(carol, charlie) + ht.OpenChannel(carol, charlie, lntemp.OpenChannelParams{Amt: chanAmt}) // Query routes from Carol to Charlie which will be an invalid route // for Alice -> Bob. @@ -465,54 +442,37 @@ func testSendToRouteErrorPropagation(net *lntest.NetworkHarness, t *harnessTest) PubKey: charlie.PubKeyStr, Amt: int64(1), } - ctxt, _ := context.WithTimeout(ctxb, defaultTimeout) - fakeRoute, err := carol.QueryRoutes(ctxt, fakeReq) - if err != nil { - t.Fatalf("unable get fake route: %v", err) - } + fakeRoute := carol.RPC.QueryRoutes(fakeReq) - // Create 1 invoices for Bob, which expect a payment from Alice for 1k - // satoshis + // Create 1 invoice for Bob, which expect a payment from Alice for 1k + // satoshis. const paymentAmt = 1000 invoice := &lnrpc.Invoice{ Memo: "testing", Value: paymentAmt, } - ctxt, _ = context.WithTimeout(ctxb, defaultTimeout) - resp, err := net.Bob.AddInvoice(ctxt, invoice) - if err != nil { - t.Fatalf("unable to add invoice: %v", err) - } - + resp := bob.RPC.AddInvoice(invoice) rHash := resp.RHash - // Using Alice as the source, pay to the 5 invoices from Bob created above. - ctxt, _ = context.WithTimeout(ctxb, defaultTimeout) - alicePayStream, err := net.Alice.SendToRoute(ctxt) // nolint:staticcheck - if err != nil { - t.Fatalf("unable to create payment stream for alice: %v", err) - } + // Using Alice as the source, pay to the invoice from Bob. + alicePayStream := alice.RPC.SendToRoute() sendReq := &lnrpc.SendToRouteRequest{ PaymentHash: rHash, Route: fakeRoute.Routes[0], } - - if err := alicePayStream.Send(sendReq); err != nil { - t.Fatalf("unable to send payment: %v", err) - } + err := alicePayStream.Send(sendReq) + require.NoError(ht, err, "unable to send payment") // At this place we should get an rpc error with notification // that edge is not found on hop(0) - _, err = alicePayStream.Recv() - if err != nil && strings.Contains(err.Error(), "edge not found") { - } else if err != nil { - t.Fatalf("payment stream has been closed but fake route has consumed: %v", err) - } + event, err := ht.ReceiveSendToRouteUpdate(alicePayStream) + require.NoError(ht, err, "payment stream has been closed but fake "+ + "route has consumed") + require.Contains(ht, event.PaymentError, "UnknownNextPeer") - closeChannelAndAssert(t, net, net.Alice, chanPointAlice, false) - closeChannelAndAssert(t, net, carol, chanPointCarol, false) + ht.CloseChannel(alice, chanPointAlice) } // testPrivateChannels tests that a private channel can be used for diff --git a/lntest/itest/lnd_test_list_on_test.go b/lntest/itest/lnd_test_list_on_test.go index 6a76e255a..7732632a2 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: "send to route error propagation", - test: testSendToRouteErrorPropagation, - }, { name: "private channels", test: testPrivateChannels,