From 22f8f834126a9b8e319fdc0f9d5714ab5eb1b777 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Thu, 4 Aug 2022 03:38:09 +0800 Subject: [PATCH] multi: refactor `testUpdateChannelPolicy` --- lntemp/harness.go | 23 ++ lntemp/harness_assertion.go | 41 +++ lntemp/node/watcher.go | 6 +- lntemp/rpc/lnd.go | 50 ++++ lntemp/utils.go | 16 ++ lntest/itest/list_on_test.go | 4 + lntest/itest/lnd_channel_policy_test.go | 360 ++++++++++-------------- lntest/itest/lnd_test_list_on_test.go | 4 - 8 files changed, 290 insertions(+), 214 deletions(-) diff --git a/lntemp/harness.go b/lntemp/harness.go index 36d03a94c..f15fb443a 100644 --- a/lntemp/harness.go +++ b/lntemp/harness.go @@ -1321,3 +1321,26 @@ func (h *HarnessTest) QueryChannelByChanPoint(hn *node.HarnessNode, require.NoError(h, err, "failed to query channel") return channel } + +// SendPaymentAndAssertStatus sends a payment from the passed node and asserts +// the desired status is reached. +func (h *HarnessTest) SendPaymentAndAssertStatus(hn *node.HarnessNode, + req *routerrpc.SendPaymentRequest, + status lnrpc.Payment_PaymentStatus) *lnrpc.Payment { + + stream := hn.RPC.SendPayment(req) + return h.AssertPaymentStatusFromStream(stream, status) +} + +// SendPaymentAssertFail sends a payment from the passed node and asserts the +// payment is failed with the specified failure reason . +func (h *HarnessTest) SendPaymentAssertFail(hn *node.HarnessNode, + req *routerrpc.SendPaymentRequest, + reason lnrpc.PaymentFailureReason) *lnrpc.Payment { + + payment := h.SendPaymentAndAssertStatus(hn, req, lnrpc.Payment_FAILED) + require.Equal(h, reason, payment.FailureReason, + "payment failureReason not matched") + + return payment +} diff --git a/lntemp/harness_assertion.go b/lntemp/harness_assertion.go index 1b9c29a9c..87704dc05 100644 --- a/lntemp/harness_assertion.go +++ b/lntemp/harness_assertion.go @@ -1503,3 +1503,44 @@ func (h *HarnessTest) AssertTxAtHeight(hn *node.HarnessNode, height int32, return nil } + +// getChannelPolicies queries the channel graph and retrieves the current edge +// policies for the provided channel point. +func (h *HarnessTest) getChannelPolicies(hn *node.HarnessNode, + advertisingNode string, + cp *lnrpc.ChannelPoint) (*lnrpc.RoutingPolicy, error) { + + req := &lnrpc.ChannelGraphRequest{IncludeUnannounced: true} + chanGraph := hn.RPC.DescribeGraph(req) + + cpStr := channelPointStr(cp) + for _, e := range chanGraph.Edges { + if e.ChanPoint != cpStr { + continue + } + + if e.Node1Pub == advertisingNode { + return e.Node1Policy, nil + } + + return e.Node2Policy, nil + } + + // If we've iterated over all the known edges and we weren't + // able to find this specific one, then we'll fail. + return nil, fmt.Errorf("did not find edge with advertisingNode: %s"+ + ", channel point: %s", advertisingNode, cpStr) +} + +// AssertChannelPolicy asserts that the passed node's known channel policy for +// the passed chanPoint is consistent with the expected policy values. +func (h *HarnessTest) AssertChannelPolicy(hn *node.HarnessNode, + advertisingNode string, expectedPolicy *lnrpc.RoutingPolicy, + chanPoint *lnrpc.ChannelPoint) { + + policy, err := h.getChannelPolicies(hn, advertisingNode, chanPoint) + require.NoErrorf(h, err, "%s: failed to find policy", hn.Name()) + + err = node.CheckChannelPolicy(policy, expectedPolicy) + require.NoErrorf(h, err, "%s: check policy failed", hn.Name()) +} diff --git a/lntemp/node/watcher.go b/lntemp/node/watcher.go index eeb68534a..1425ef1dd 100644 --- a/lntemp/node/watcher.go +++ b/lntemp/node/watcher.go @@ -559,7 +559,7 @@ func (nw *nodeWatcher) handlePolicyUpdateWatchRequest(req *chanWatchRequest) { // Check if the latest policy is matched. policy := policies[len(policies)-1] - if checkChannelPolicy(policy.RoutingPolicy, req.policy) == nil { + if CheckChannelPolicy(policy.RoutingPolicy, req.policy) == nil { close(req.eventChan) return } @@ -653,8 +653,8 @@ func (nw *nodeWatcher) getChannelPolicies(include bool) policyUpdateMap { return policyUpdates } -// checkChannelPolicy checks that the policy matches the expected one. -func checkChannelPolicy(policy, expectedPolicy *lnrpc.RoutingPolicy) error { +// CheckChannelPolicy checks that the policy matches the expected one. +func CheckChannelPolicy(policy, expectedPolicy *lnrpc.RoutingPolicy) error { if policy.FeeBaseMsat != expectedPolicy.FeeBaseMsat { return fmt.Errorf("expected base fee %v, got %v", expectedPolicy.FeeBaseMsat, policy.FeeBaseMsat) diff --git a/lntemp/rpc/lnd.go b/lntemp/rpc/lnd.go index 360277369..941b4371e 100644 --- a/lntemp/rpc/lnd.go +++ b/lntemp/rpc/lnd.go @@ -432,3 +432,53 @@ func (h *HarnessRPC) BatchOpenChannelAssertErr( return err } + +// QueryRoutes makes a RPC call to QueryRoutes and asserts. +func (h *HarnessRPC) QueryRoutes( + req *lnrpc.QueryRoutesRequest) *lnrpc.QueryRoutesResponse { + + ctxt, cancel := context.WithTimeout(h.runCtx, DefaultTimeout) + defer cancel() + + routes, err := h.LN.QueryRoutes(ctxt, req) + require.NoErrorf(h, err, "failed to query routes") + + return routes +} + +// SendToRoute makes a RPC call to SendToRoute and asserts. +func (h *HarnessRPC) SendToRoute() lnrpc.Lightning_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. + client, err := h.LN.SendToRoute(h.runCtx) + h.NoError(err, "SendToRoute") + + return client +} + +// SendToRouteSync makes a RPC call to SendToRouteSync and asserts. +func (h *HarnessRPC) SendToRouteSync( + req *lnrpc.SendToRouteRequest) *lnrpc.SendResponse { + + ctxt, cancel := context.WithTimeout(h.runCtx, DefaultTimeout) + defer cancel() + + resp, err := h.LN.SendToRouteSync(ctxt, req) + require.NoErrorf(h, err, "unable to send to route for %s", h.Name) + + return resp +} + +// UpdateChannelPolicy makes a RPC call to UpdateChannelPolicy and asserts. +func (h *HarnessRPC) UpdateChannelPolicy( + req *lnrpc.PolicyUpdateRequest) *lnrpc.PolicyUpdateResponse { + + ctxt, cancel := context.WithTimeout(h.runCtx, DefaultTimeout) + defer cancel() + + resp, err := h.LN.UpdateChannelPolicy(ctxt, req) + require.NoErrorf(h, err, "failed to update policy") + + return resp +} diff --git a/lntemp/utils.go b/lntemp/utils.go index 590380423..d9946f365 100644 --- a/lntemp/utils.go +++ b/lntemp/utils.go @@ -8,6 +8,7 @@ import ( "strconv" "strings" + "github.com/btcsuite/btcd/wire" "github.com/lightningnetwork/lnd/lnrpc" "github.com/lightningnetwork/lnd/lntest" ) @@ -106,3 +107,18 @@ func ChanPointFromPendingUpdate(pu *lnrpc.PendingUpdate) *lnrpc.ChannelPoint { return chanPoint } + +// channelPointStr returns the string representation of the channel's +// funding transaction. +func channelPointStr(chanPoint *lnrpc.ChannelPoint) string { + fundingTxID, err := lnrpc.GetChanPointFundingTxid(chanPoint) + if err != nil { + return "" + } + cp := wire.OutPoint{ + Hash: *fundingTxID, + Index: chanPoint.OutputIndex, + } + + return cp.String() +} diff --git a/lntest/itest/list_on_test.go b/lntest/itest/list_on_test.go index c6bac981e..515f2ae99 100644 --- a/lntest/itest/list_on_test.go +++ b/lntest/itest/list_on_test.go @@ -135,4 +135,8 @@ var allTestCasesTemp = []*lntemp.TestCase{ Name: "batch channel funding", TestFunc: testBatchChanFunding, }, + { + Name: "update channel policy", + TestFunc: testUpdateChannelPolicy, + }, } diff --git a/lntest/itest/lnd_channel_policy_test.go b/lntest/itest/lnd_channel_policy_test.go index 638878933..2db663742 100644 --- a/lntest/itest/lnd_channel_policy_test.go +++ b/lntest/itest/lnd_channel_policy_test.go @@ -3,7 +3,6 @@ package itest import ( "context" "math" - "strings" "time" "github.com/btcsuite/btcd/btcutil" @@ -11,6 +10,9 @@ import ( "github.com/lightningnetwork/lnd/chainreg" "github.com/lightningnetwork/lnd/funding" "github.com/lightningnetwork/lnd/lnrpc" + "github.com/lightningnetwork/lnd/lnrpc/routerrpc" + "github.com/lightningnetwork/lnd/lntemp" + "github.com/lightningnetwork/lnd/lntemp/node" "github.com/lightningnetwork/lnd/lntest" "github.com/lightningnetwork/lnd/lnwire" "github.com/stretchr/testify/require" @@ -18,6 +20,7 @@ import ( // assertPolicyUpdate checks that a given policy update has been received by a // list of given nodes. +// TODO(yy): delete. func assertPolicyUpdate(t *harnessTest, nodes []*lntest.HarnessNode, advertisingNode string, policy *lnrpc.RoutingPolicy, chanPoint *lnrpc.ChannelPoint) { @@ -30,10 +33,8 @@ func assertPolicyUpdate(t *harnessTest, nodes []*lntest.HarnessNode, } // testUpdateChannelPolicy tests that policy updates made to a channel -// get propagated to other nodes in the network. -func testUpdateChannelPolicy(net *lntest.NetworkHarness, t *harnessTest) { - ctxb := context.Background() - +// gets propagated to other nodes in the network. +func testUpdateChannelPolicy(ht *lntemp.HarnessTest) { const ( defaultFeeBase = 1000 defaultFeeRate = 1 @@ -45,19 +46,19 @@ func testUpdateChannelPolicy(net *lntest.NetworkHarness, t *harnessTest) { chanAmt := funding.MaxBtcFundingAmount pushAmt := chanAmt / 2 + alice, bob := ht.Alice, ht.Bob + // Create a channel Alice->Bob. - chanPoint := openChannelAndAssert( - t, net, net.Alice, net.Bob, - lntest.OpenChannelParams{ + chanPoint := ht.OpenChannel( + alice, bob, lntemp.OpenChannelParams{ Amt: chanAmt, PushAmt: pushAmt, }, ) - defer closeChannelAndAssert(t, net, net.Alice, chanPoint, false) // We add all the nodes' update channels to a slice, such that we can // make sure they all receive the expected updates. - nodes := []*lntest.HarnessNode{net.Alice, net.Bob} + nodes := []*node.HarnessNode{alice, bob} // Alice and Bob should see each other's ChannelUpdates, advertising the // default routing policies. @@ -69,65 +70,45 @@ func testUpdateChannelPolicy(net *lntest.NetworkHarness, t *harnessTest) { MaxHtlcMsat: defaultMaxHtlc, } - assertPolicyUpdate( - t, nodes, net.Alice.PubKeyStr, expectedPolicy, chanPoint, - ) - assertPolicyUpdate( - t, nodes, net.Bob.PubKeyStr, expectedPolicy, chanPoint, - ) + assertNodesPolicyUpdate(ht, nodes, alice, expectedPolicy, chanPoint) + assertNodesPolicyUpdate(ht, nodes, bob, expectedPolicy, chanPoint) // They should now know about the default policies. for _, node := range nodes { - assertChannelPolicy( - t, node, net.Alice.PubKeyStr, expectedPolicy, chanPoint, + ht.AssertChannelPolicy( + node, alice.PubKeyStr, expectedPolicy, chanPoint, ) - assertChannelPolicy( - t, node, net.Bob.PubKeyStr, expectedPolicy, chanPoint, + ht.AssertChannelPolicy( + node, bob.PubKeyStr, expectedPolicy, chanPoint, ) } - err := net.Alice.WaitForNetworkChannelOpen(chanPoint) - if err != nil { - t.Fatalf("alice didn't report channel: %v", err) - } - err = net.Bob.WaitForNetworkChannelOpen(chanPoint) - if err != nil { - t.Fatalf("bob didn't report channel: %v", err) - } - // Create Carol with options to rate limit channel updates up to 2 per // day, and create a new channel Bob->Carol. - carol := net.NewNode( - t.t, "Carol", []string{ + carol := ht.NewNode( + "Carol", []string{ "--gossip.max-channel-update-burst=2", "--gossip.channel-update-interval=24h", }, ) - - // Clean up carol's node when the test finishes. - defer shutdownAndAssert(net, t, carol) - + ht.ConnectNodes(carol, bob) nodes = append(nodes, carol) // Send some coins to Carol that can be used for channel funding. - net.SendCoins(t.t, btcutil.SatoshiPerBitcoin, carol) - - net.ConnectNodes(t.t, carol, net.Bob) + ht.FundCoins(btcutil.SatoshiPerBitcoin, carol) // Open the channel Carol->Bob with a custom min_htlc value set. Since // Carol is opening the channel, she will require Bob to not forward // HTLCs smaller than this value, and hence he should advertise it as // part of his ChannelUpdate. const customMinHtlc = 5000 - chanPoint2 := openChannelAndAssert( - t, net, carol, net.Bob, - lntest.OpenChannelParams{ + chanPoint2 := ht.OpenChannel( + carol, bob, lntemp.OpenChannelParams{ Amt: chanAmt, PushAmt: pushAmt, MinHtlc: customMinHtlc, }, ) - defer closeChannelAndAssert(t, net, net.Bob, chanPoint2, false) expectedPolicyBob := &lnrpc.RoutingPolicy{ FeeBaseMsat: defaultFeeBase, @@ -144,37 +125,24 @@ func testUpdateChannelPolicy(net *lntest.NetworkHarness, t *harnessTest) { MaxHtlcMsat: defaultMaxHtlc, } - assertPolicyUpdate( - t, nodes, net.Bob.PubKeyStr, expectedPolicyBob, chanPoint2, - ) - assertPolicyUpdate( - t, nodes, carol.PubKeyStr, expectedPolicyCarol, chanPoint2, + assertNodesPolicyUpdate(ht, nodes, bob, expectedPolicyBob, chanPoint2) + assertNodesPolicyUpdate( + ht, nodes, carol, expectedPolicyCarol, chanPoint2, ) // Check that all nodes now know about the updated policies. for _, node := range nodes { - assertChannelPolicy( - t, node, net.Bob.PubKeyStr, expectedPolicyBob, - chanPoint2, + ht.AssertChannelPolicy( + node, bob.PubKeyStr, expectedPolicyBob, chanPoint2, ) - assertChannelPolicy( - t, node, carol.PubKeyStr, expectedPolicyCarol, - chanPoint2, + ht.AssertChannelPolicy( + node, carol.PubKeyStr, expectedPolicyCarol, chanPoint2, ) } - err = net.Alice.WaitForNetworkChannelOpen(chanPoint2) - if err != nil { - t.Fatalf("alice didn't report channel: %v", err) - } - err = net.Bob.WaitForNetworkChannelOpen(chanPoint2) - if err != nil { - t.Fatalf("bob didn't report channel: %v", err) - } - err = carol.WaitForNetworkChannelOpen(chanPoint2) - if err != nil { - t.Fatalf("carol didn't report channel: %v", err) - } + // Make sure Alice and Carol have seen each other's channels. + ht.AssertTopologyChannelOpen(alice, chanPoint2) + ht.AssertTopologyChannelOpen(carol, chanPoint) // First we'll try to send a payment from Alice to Carol with an amount // less than the min_htlc value required by Carol. This payment should @@ -184,23 +152,19 @@ func testUpdateChannelPolicy(net *lntest.NetworkHarness, t *harnessTest) { Memo: "testing", Value: int64(payAmt), } - ctxt, _ := context.WithTimeout(ctxb, defaultTimeout) - resp, err := carol.AddInvoice(ctxt, invoice) - if err != nil { - t.Fatalf("unable to add invoice: %v", err) - } - - err = completePaymentRequests( - net.Alice, net.Alice.RouterClient, - []string{resp.PaymentRequest}, true, - ) + resp := carol.RPC.AddInvoice(invoice) // Alice knows about the channel policy of Carol and should therefore // not be able to find a path during routing. - expErr := lnrpc.PaymentFailureReason_FAILURE_REASON_NO_ROUTE - if err.Error() != expErr.String() { - t.Fatalf("expected %v, instead got %v", expErr, err) + payReq := &routerrpc.SendPaymentRequest{ + PaymentRequest: resp.PaymentRequest, + TimeoutSeconds: 60, + FeeLimitMsat: noFeeLimitMsat, } + ht.SendPaymentAssertFail( + alice, payReq, + lnrpc.PaymentFailureReason_FAILURE_REASON_NO_ROUTE, + ) // Now we try to send a payment over the channel with a value too low // to be accepted. First we query for a route to route a payment of @@ -211,16 +175,8 @@ func testUpdateChannelPolicy(net *lntest.NetworkHarness, t *harnessTest) { Amt: int64(payAmt), FinalCltvDelta: defaultTimeLockDelta, } - - ctxt, _ = context.WithTimeout(ctxb, defaultTimeout) - routes, err := net.Alice.QueryRoutes(ctxt, routesReq) - if err != nil { - t.Fatalf("unable to get route: %v", err) - } - - if len(routes.Routes) != 1 { - t.Fatalf("expected to find 1 route, got %v", len(routes.Routes)) - } + routes := alice.RPC.QueryRoutes(routesReq) + require.Len(ht, routes.Routes, 1) // We change the route to carry a payment of 4000 mSAT instead of 5000 // mSAT. @@ -233,27 +189,19 @@ func testUpdateChannelPolicy(net *lntest.NetworkHarness, t *harnessTest) { routes.Routes[0].Hops[1].AmtToForwardMsat = amtMSat // Send the payment with the modified value. - 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) - } + alicePayStream := alice.RPC.SendToRoute() + sendReq := &lnrpc.SendToRouteRequest{ PaymentHash: resp.RHash, Route: routes.Routes[0], } - - err = alicePayStream.Send(sendReq) - if err != nil { - t.Fatalf("unable to send payment: %v", err) - } + err := alicePayStream.Send(sendReq) + require.NoError(ht, err, "unable to send payment") // We expect this payment to fail, and that the min_htlc value is // communicated back to us, since the attempted HTLC value was too low. sendResp, err := alicePayStream.Recv() - if err != nil { - t.Fatalf("unable to send payment: %v", err) - } + require.NoError(ht, err, "unable to receive payment stream") // Expected as part of the error message. substrs := []string{ @@ -261,10 +209,7 @@ func testUpdateChannelPolicy(net *lntest.NetworkHarness, t *harnessTest) { "HtlcMinimumMsat: (lnwire.MilliSatoshi) 5000 mSAT", } for _, s := range substrs { - if !strings.Contains(sendResp.PaymentError, s) { - t.Fatalf("expected error to contain \"%v\", instead "+ - "got %v", s, sendResp.PaymentError) - } + require.Contains(ht, sendResp.PaymentError, s) } // Make sure sending using the original value succeeds. @@ -292,22 +237,14 @@ func testUpdateChannelPolicy(net *lntest.NetworkHarness, t *harnessTest) { } err = alicePayStream.Send(sendReq) - if err != nil { - t.Fatalf("unable to send payment: %v", err) - } + require.NoError(ht, err, "unable to send payment") sendResp, err = alicePayStream.Recv() - if err != nil { - t.Fatalf("unable to send payment: %v", err) - } + require.NoError(ht, err, "unable to receive payment stream") + require.Empty(ht, sendResp.PaymentError, "expected payment to succeed") - if sendResp.PaymentError != "" { - t.Fatalf("expected payment to succeed, instead got %v", - sendResp.PaymentError) - } - - // With our little cluster set up, we'll update the fees and the max htlc - // size for the Bob side of the Alice->Bob channel, and make sure + // With our little cluster set up, we'll update the fees and the max + // htlc size for the Bob side of the Alice->Bob channel, and make sure // all nodes learn about it. baseFee := int64(1500) feeRate := int64(12) @@ -331,21 +268,15 @@ func testUpdateChannelPolicy(net *lntest.NetworkHarness, t *harnessTest) { ChanPoint: chanPoint, }, } - - ctxt, _ = context.WithTimeout(ctxb, defaultTimeout) - if _, err := net.Bob.UpdateChannelPolicy(ctxt, req); err != nil { - t.Fatalf("unable to get alice's balance: %v", err) - } + bob.RPC.UpdateChannelPolicy(req) // Wait for all nodes to have seen the policy update done by Bob. - assertPolicyUpdate( - t, nodes, net.Bob.PubKeyStr, expectedPolicy, chanPoint, - ) + assertNodesPolicyUpdate(ht, nodes, bob, expectedPolicy, chanPoint) // Check that all nodes now know about Bob's updated policy. for _, node := range nodes { - assertChannelPolicy( - t, node, net.Bob.PubKeyStr, expectedPolicy, chanPoint, + ht.AssertChannelPolicy( + node, bob.PubKeyStr, expectedPolicy, chanPoint, ) } @@ -361,39 +292,21 @@ func testUpdateChannelPolicy(net *lntest.NetworkHarness, t *harnessTest) { Memo: "testing", Value: int64(payAmt), } - ctxt, _ = context.WithTimeout(ctxb, defaultTimeout) - resp, err = carol.AddInvoice(ctxt, invoice) - if err != nil { - t.Fatalf("unable to add invoice: %v", err) - } + resp = carol.RPC.AddInvoice(invoice) - err = completePaymentRequests( - net.Alice, net.Alice.RouterClient, - []string{resp.PaymentRequest}, true, - ) - if err != nil { - t.Fatalf("unable to send payment: %v", err) - } + ht.CompletePaymentRequests(alice, []string{resp.PaymentRequest}) // We'll now open a channel from Alice directly to Carol. - net.ConnectNodes(t.t, net.Alice, carol) - chanPoint3 := openChannelAndAssert( - t, net, net.Alice, carol, - lntest.OpenChannelParams{ + ht.ConnectNodes(alice, carol) + chanPoint3 := ht.OpenChannel( + alice, carol, lntemp.OpenChannelParams{ Amt: chanAmt, PushAmt: pushAmt, }, ) - defer closeChannelAndAssert(t, net, net.Alice, chanPoint3, false) - err = net.Alice.WaitForNetworkChannelOpen(chanPoint3) - if err != nil { - t.Fatalf("alice didn't report channel: %v", err) - } - err = carol.WaitForNetworkChannelOpen(chanPoint3) - if err != nil { - t.Fatalf("bob didn't report channel: %v", err) - } + // Make sure Bob knows this channel. + ht.AssertTopologyChannelOpen(bob, chanPoint3) // Make a global update, and check that both channels' new policies get // propagated. @@ -414,28 +327,21 @@ func testUpdateChannelPolicy(net *lntest.NetworkHarness, t *harnessTest) { MaxHtlcMsat: maxHtlc, } req.Scope = &lnrpc.PolicyUpdateRequest_Global{} - - ctxt, _ = context.WithTimeout(ctxb, defaultTimeout) - _, err = net.Alice.UpdateChannelPolicy(ctxt, req) - if err != nil { - t.Fatalf("unable to update alice's channel policy: %v", err) - } + alice.RPC.UpdateChannelPolicy(req) // Wait for all nodes to have seen the policy updates for both of // Alice's channels. - assertPolicyUpdate( - t, nodes, net.Alice.PubKeyStr, expectedPolicy, chanPoint, - ) - assertPolicyUpdate( - t, nodes, net.Alice.PubKeyStr, expectedPolicy, chanPoint3, - ) + assertNodesPolicyUpdate(ht, nodes, alice, expectedPolicy, chanPoint) + assertNodesPolicyUpdate(ht, nodes, alice, expectedPolicy, chanPoint3) // And finally check that all nodes remembers the policy update they // received. for _, node := range nodes { - assertChannelPolicy( - t, node, net.Alice.PubKeyStr, expectedPolicy, - chanPoint, chanPoint3, + ht.AssertChannelPolicy( + node, alice.PubKeyStr, expectedPolicy, chanPoint, + ) + ht.AssertChannelPolicy( + node, alice.PubKeyStr, expectedPolicy, chanPoint3, ) } @@ -443,60 +349,87 @@ func testUpdateChannelPolicy(net *lntest.NetworkHarness, t *harnessTest) { // we'll send two more update from Alice. Carol should accept the first, // but not the second, as she only allows two updates per day and a day // has yet to elapse from the previous update. - const numUpdatesTilRateLimit = 2 - for i := 0; i < numUpdatesTilRateLimit; i++ { - prevAlicePolicy := *expectedPolicy - baseFee *= 2 - expectedPolicy.FeeBaseMsat = baseFee - req.BaseFeeMsat = baseFee - ctxt, cancel := context.WithTimeout(ctxb, defaultTimeout) - defer cancel() - _, err = net.Alice.UpdateChannelPolicy(ctxt, req) - require.NoError(t.t, err) + // assertAliceAndBob is a helper closure which updates Alice's policy + // and asserts that both Alice and Bob have heard and updated the + // policy in their graph. + assertAliceAndBob := func(req *lnrpc.PolicyUpdateRequest, + expectedPolicy *lnrpc.RoutingPolicy) { + + alice.RPC.UpdateChannelPolicy(req) // Wait for all nodes to have seen the policy updates for both // of Alice's channels. Carol will not see the last update as // the limit has been reached. - assertPolicyUpdate( - t, []*lntest.HarnessNode{net.Alice, net.Bob}, - net.Alice.PubKeyStr, expectedPolicy, chanPoint, + assertNodesPolicyUpdate( + ht, []*node.HarnessNode{alice, bob}, + alice, expectedPolicy, chanPoint, ) - assertPolicyUpdate( - t, []*lntest.HarnessNode{net.Alice, net.Bob}, - net.Alice.PubKeyStr, expectedPolicy, chanPoint3, + assertNodesPolicyUpdate( + ht, []*node.HarnessNode{alice, bob}, + alice, expectedPolicy, chanPoint3, ) // Check that all nodes remember the policy update // they received. - assertChannelPolicy( - t, net.Alice, net.Alice.PubKeyStr, - expectedPolicy, chanPoint, chanPoint3, + ht.AssertChannelPolicy( + alice, alice.PubKeyStr, expectedPolicy, chanPoint, ) - assertChannelPolicy( - t, net.Bob, net.Alice.PubKeyStr, - expectedPolicy, chanPoint, chanPoint3, + ht.AssertChannelPolicy( + alice, alice.PubKeyStr, expectedPolicy, chanPoint3, ) - - // Carol was added last, which is why we check the last index. - // Since Carol didn't receive the last update, she still has - // Alice's old policy. - if i == numUpdatesTilRateLimit-1 { - expectedPolicy = &prevAlicePolicy - } - assertPolicyUpdate( - t, []*lntest.HarnessNode{carol}, - net.Alice.PubKeyStr, expectedPolicy, chanPoint, + ht.AssertChannelPolicy( + bob, alice.PubKeyStr, expectedPolicy, chanPoint, ) - assertPolicyUpdate( - t, []*lntest.HarnessNode{carol}, - net.Alice.PubKeyStr, expectedPolicy, chanPoint3, - ) - assertChannelPolicy( - t, carol, net.Alice.PubKeyStr, - expectedPolicy, chanPoint, chanPoint3, + ht.AssertChannelPolicy( + bob, alice.PubKeyStr, expectedPolicy, chanPoint3, ) } + + // Double the base fee and attach to the policy. + baseFee1 := baseFee * 2 + expectedPolicy.FeeBaseMsat = baseFee1 + req.BaseFeeMsat = baseFee1 + assertAliceAndBob(req, expectedPolicy) + + // Check that Carol has both heard the policy and updated it in her + // graph. + assertNodesPolicyUpdate( + ht, []*node.HarnessNode{carol}, + alice, expectedPolicy, chanPoint, + ) + assertNodesPolicyUpdate( + ht, []*node.HarnessNode{carol}, + alice, expectedPolicy, chanPoint3, + ) + ht.AssertChannelPolicy( + carol, alice.PubKeyStr, expectedPolicy, chanPoint, + ) + ht.AssertChannelPolicy( + carol, alice.PubKeyStr, expectedPolicy, chanPoint3, + ) + + // Double the base fee and attach to the policy. + baseFee2 := baseFee1 * 2 + expectedPolicy.FeeBaseMsat = baseFee2 + req.BaseFeeMsat = baseFee2 + assertAliceAndBob(req, expectedPolicy) + + // Since Carol didn't receive the last update, she still has Alice's + // old policy. We validate this by checking the base fee is the older + // one. + expectedPolicy.FeeBaseMsat = baseFee1 + ht.AssertChannelPolicy( + carol, alice.PubKeyStr, expectedPolicy, chanPoint, + ) + ht.AssertChannelPolicy( + carol, alice.PubKeyStr, expectedPolicy, chanPoint3, + ) + + // Close all channels. + ht.CloseChannel(alice, chanPoint) + ht.CloseChannel(bob, chanPoint2) + ht.CloseChannel(alice, chanPoint3) } // testSendUpdateDisableChannel ensures that a channel update with the disable @@ -915,3 +848,16 @@ func testUpdateChannelPolicyFeeRateAccuracy(net *lntest.NetworkHarness, t, nodes, net.Alice.PubKeyStr, expectedPolicy, chanPoint, ) } + +// assertNodesPolicyUpdate checks that a given policy update has been received +// by a list of given nodes. +func assertNodesPolicyUpdate(ht *lntemp.HarnessTest, nodes []*node.HarnessNode, + advertisingNode *node.HarnessNode, policy *lnrpc.RoutingPolicy, + chanPoint *lnrpc.ChannelPoint) { + + for _, node := range nodes { + ht.AssertChannelPolicyUpdate( + node, advertisingNode, policy, chanPoint, false, + ) + } +} diff --git a/lntest/itest/lnd_test_list_on_test.go b/lntest/itest/lnd_test_list_on_test.go index ab0bd99cf..0341d7ac9 100644 --- a/lntest/itest/lnd_test_list_on_test.go +++ b/lntest/itest/lnd_test_list_on_test.go @@ -4,10 +4,6 @@ package itest var allTestCases = []*testCase{ - { - name: "update channel policy", - test: testUpdateChannelPolicy, - }, { name: "update channel policy fee rate accuracy", test: testUpdateChannelPolicyFeeRateAccuracy,