From 780f2e84a324ab9b0fff5f46af02d0b2e51a102f Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Thu, 11 Aug 2022 04:10:40 +0800 Subject: [PATCH] itest: refactor `testSendPaymentAMPInvoice` --- lntest/itest/list_on_test.go | 4 + lntest/itest/lnd_amp_test.go | 157 ++++++++++++-------------- lntest/itest/lnd_mpp_test.go | 3 +- lntest/itest/lnd_test_list_on_test.go | 4 - 4 files changed, 79 insertions(+), 89 deletions(-) diff --git a/lntest/itest/list_on_test.go b/lntest/itest/list_on_test.go index 6dcf9b163..eed5ddaee 100644 --- a/lntest/itest/list_on_test.go +++ b/lntest/itest/list_on_test.go @@ -393,4 +393,8 @@ var allTestCasesTemp = []*lntemp.TestCase{ Name: "send multi path payment", TestFunc: testSendMultiPathPayment, }, + { + Name: "sendpayment amp invoice", + TestFunc: testSendPaymentAMPInvoice, + }, } diff --git a/lntest/itest/lnd_amp_test.go b/lntest/itest/lnd_amp_test.go index 2ed33951a..b6c3a5abc 100644 --- a/lntest/itest/lnd_amp_test.go +++ b/lntest/itest/lnd_amp_test.go @@ -14,6 +14,7 @@ import ( "github.com/lightningnetwork/lnd/lnrpc" "github.com/lightningnetwork/lnd/lnrpc/invoicesrpc" "github.com/lightningnetwork/lnd/lnrpc/routerrpc" + "github.com/lightningnetwork/lnd/lntemp" "github.com/lightningnetwork/lnd/lntest" "github.com/lightningnetwork/lnd/lntypes" "github.com/stretchr/testify/require" @@ -21,33 +22,33 @@ import ( // testSendPaymentAMPInvoice tests that we can send an AMP payment to a // specified AMP invoice using SendPaymentV2. -func testSendPaymentAMPInvoice(net *lntest.NetworkHarness, t *harnessTest) { - t.t.Run("native payaddr", func(t *testing.T) { - tt := newHarnessTest(t, net) - testSendPaymentAMPInvoiceCase(net, tt, false) +func testSendPaymentAMPInvoice(ht *lntemp.HarnessTest) { + succeed := ht.Run("native payaddr", func(t *testing.T) { + tt := ht.Subtest(t) + testSendPaymentAMPInvoiceCase(tt, false) }) - t.t.Run("external payaddr", func(t *testing.T) { - tt := newHarnessTest(t, net) - testSendPaymentAMPInvoiceCase(net, tt, true) + + // Abort the test if failed. + if !succeed { + return + } + + ht.Run("external payaddr", func(t *testing.T) { + tt := ht.Subtest(t) + testSendPaymentAMPInvoiceCase(tt, true) }) } -func testSendPaymentAMPInvoiceCase(net *lntest.NetworkHarness, t *harnessTest, +func testSendPaymentAMPInvoiceCase(ht *lntemp.HarnessTest, useExternalPayAddr bool) { - ctxb := context.Background() - - ctx := newMppTestContext(t, net) - defer ctx.shutdownNodes() + mts := newMppTestScenario(ht) // Subscribe to bob's invoices. Do this early in the test to make sure // that the subscription has actually been completed when we add an // invoice. Otherwise the notification will be missed. req := &lnrpc.InvoiceSubscription{} - ctxc, cancelSubscription := context.WithCancel(ctxb) - bobInvoiceSubscription, err := ctx.bob.SubscribeInvoices(ctxc, req) - require.NoError(t.t, err) - defer cancelSubscription() + bobInvoiceSubscription := mts.bob.RPC.SubscribeInvoices(req) const paymentAmt = btcutil.Amount(300000) @@ -61,49 +62,45 @@ func testSendPaymentAMPInvoiceCase(net *lntest.NetworkHarness, t *harnessTest, // \ / // \__ Dave ____/ // - ctx.openChannel(ctx.carol, ctx.bob, 135000) - ctx.openChannel(ctx.alice, ctx.carol, 235000) - ctx.openChannel(ctx.dave, ctx.bob, 135000) - ctx.openChannel(ctx.alice, ctx.dave, 135000) - ctx.openChannel(ctx.eve, ctx.bob, 135000) - ctx.openChannel(ctx.carol, ctx.eve, 135000) + mppReq := &mppOpenChannelRequest{ + amtAliceCarol: 235000, + amtAliceDave: 135000, + amtCarolBob: 135000, + amtCarolEve: 135000, + amtDaveBob: 135000, + amtEveBob: 135000, + } + mts.openChannels(mppReq) + chanPointAliceDave := mts.channelPoints[1] + chanPointDaveBob := mts.channelPoints[4] - defer ctx.closeChannels() - - ctx.waitForChannels() - - addInvoiceResp, err := ctx.bob.AddInvoice(context.Background(), &lnrpc.Invoice{ + invoice := &lnrpc.Invoice{ Value: int64(paymentAmt), IsAmp: true, - }) - require.NoError(t.t, err) + } + addInvoiceResp := mts.bob.RPC.AddInvoice(invoice) // Ensure we get a notification of the invoice being added by Bob. - rpcInvoice, err := bobInvoiceSubscription.Recv() - require.NoError(t.t, err) + rpcInvoice := ht.ReceiveInvoiceUpdate(bobInvoiceSubscription) - require.False(t.t, rpcInvoice.Settled) // nolint:staticcheck - require.Equal(t.t, lnrpc.Invoice_OPEN, rpcInvoice.State) - require.Equal(t.t, int64(0), rpcInvoice.AmtPaidSat) - require.Equal(t.t, int64(0), rpcInvoice.AmtPaidMsat) - - require.Equal(t.t, 0, len(rpcInvoice.Htlcs)) + require.False(ht, rpcInvoice.Settled) // nolint:staticcheck + require.Equal(ht, lnrpc.Invoice_OPEN, rpcInvoice.State) + require.Equal(ht, int64(0), rpcInvoice.AmtPaidSat) + require.Equal(ht, int64(0), rpcInvoice.AmtPaidMsat) + require.Equal(ht, 0, len(rpcInvoice.Htlcs)) // Increase Dave's fee to make the test deterministic. Otherwise it // would be unpredictable whether pathfinding would go through Charlie // or Dave for the first shard. - _, err = ctx.dave.UpdateChannelPolicy( - context.Background(), - &lnrpc.PolicyUpdateRequest{ - Scope: &lnrpc.PolicyUpdateRequest_Global{Global: true}, - BaseFeeMsat: 500000, - FeeRate: 0.001, - TimeLockDelta: 40, - }, + expectedPolicy := mts.updateDaveGlobalPolicy() + + // Make sure Alice has heard it for both Dave's channels. + ht.AssertChannelPolicyUpdate( + mts.alice, mts.dave, expectedPolicy, chanPointAliceDave, false, + ) + ht.AssertChannelPolicyUpdate( + mts.alice, mts.dave, expectedPolicy, chanPointDaveBob, false, ) - if err != nil { - t.Fatalf("dave policy update: %v", err) - } // Generate an external payment address when attempting to pseudo-reuse // an AMP invoice. When using an external payment address, we'll also @@ -116,19 +113,16 @@ func testSendPaymentAMPInvoiceCase(net *lntest.NetworkHarness, t *harnessTest, ) if useExternalPayAddr { expNumInvoices = 2 - externalPayAddr = make([]byte, 32) - _, err = rand.Read(externalPayAddr) - require.NoError(t.t, err) + externalPayAddr = ht.Random32Bytes() } - payment := sendAndAssertSuccess( - t, ctx.alice, &routerrpc.SendPaymentRequest{ - PaymentRequest: addInvoiceResp.PaymentRequest, - PaymentAddr: externalPayAddr, - TimeoutSeconds: 60, - FeeLimitMsat: noFeeLimitMsat, - }, - ) + sendReq := &routerrpc.SendPaymentRequest{ + PaymentRequest: addInvoiceResp.PaymentRequest, + PaymentAddr: externalPayAddr, + TimeoutSeconds: 60, + FeeLimitMsat: noFeeLimitMsat, + } + payment := ht.SendPaymentAssertSettled(mts.alice, sendReq) // Check that Alice split the payment in at least three shards. Because // the hand-off of the htlc to the link is asynchronous (via a mailbox), @@ -145,67 +139,62 @@ func testSendPaymentAMPInvoiceCase(net *lntest.NetworkHarness, t *harnessTest, } const minExpectedShards = 3 - if succeeded < minExpectedShards { - t.Fatalf("expected at least %v shards, but got %v", - minExpectedShards, succeeded) - } + require.GreaterOrEqual(ht, succeeded, minExpectedShards, + "expected num of shards not reached") // When an external payment address is supplied, we'll get an extra // notification for the JIT inserted invoice, since it differs from the // original. if useExternalPayAddr { - _, err = bobInvoiceSubscription.Recv() - require.NoError(t.t, err) + ht.ReceiveInvoiceUpdate(bobInvoiceSubscription) } // There should now be a settle event for the invoice. - rpcInvoice, err = bobInvoiceSubscription.Recv() - require.NoError(t.t, err) + rpcInvoice = ht.ReceiveInvoiceUpdate(bobInvoiceSubscription) // Also fetch Bob's invoice from ListInvoices and assert it is equal to // the one received via the subscription. - invoiceResp, err := ctx.bob.ListInvoices( - ctxb, &lnrpc.ListInvoiceRequest{}, - ) - require.NoError(t.t, err) - require.Equal(t.t, expNumInvoices, len(invoiceResp.Invoices)) - assertInvoiceEqual(t.t, rpcInvoice, invoiceResp.Invoices[expNumInvoices-1]) + invoices := ht.AssertNumInvoices(mts.bob, expNumInvoices) + assertInvoiceEqual(ht.T, rpcInvoice, invoices[expNumInvoices-1]) // Assert that the invoice is settled for the total payment amount and // has the correct payment address. - require.True(t.t, rpcInvoice.Settled) // nolint:staticcheck - require.Equal(t.t, lnrpc.Invoice_SETTLED, rpcInvoice.State) - require.Equal(t.t, int64(paymentAmt), rpcInvoice.AmtPaidSat) - require.Equal(t.t, int64(paymentAmt*1000), rpcInvoice.AmtPaidMsat) + require.True(ht, rpcInvoice.Settled) // nolint:staticcheck + require.Equal(ht, lnrpc.Invoice_SETTLED, rpcInvoice.State) + require.Equal(ht, int64(paymentAmt), rpcInvoice.AmtPaidSat) + require.Equal(ht, int64(paymentAmt*1000), rpcInvoice.AmtPaidMsat) // Finally, assert that the same set id is recorded for each htlc, and // that the preimage hash pair is valid. var setID []byte - require.Equal(t.t, succeeded, len(rpcInvoice.Htlcs)) + require.Equal(ht, succeeded, len(rpcInvoice.Htlcs)) for _, htlc := range rpcInvoice.Htlcs { - require.NotNil(t.t, htlc.Amp) + require.NotNil(ht, htlc.Amp) if setID == nil { setID = make([]byte, 32) copy(setID, htlc.Amp.SetId) } - require.Equal(t.t, setID, htlc.Amp.SetId) + require.Equal(ht, setID, htlc.Amp.SetId) // Parse the child hash and child preimage, and assert they are // well-formed. childHash, err := lntypes.MakeHash(htlc.Amp.Hash) - require.NoError(t.t, err) + require.NoError(ht, err) childPreimage, err := lntypes.MakePreimage(htlc.Amp.Preimage) - require.NoError(t.t, err) + require.NoError(ht, err) // Assert that the preimage actually matches the hashes. validPreimage := childPreimage.Matches(childHash) - require.True(t.t, validPreimage) + require.True(ht, validPreimage) } // The set ID we extract above should be shown in the final settled // state. ampState := rpcInvoice.AmpInvoiceState[hex.EncodeToString(setID)] - require.Equal(t.t, lnrpc.InvoiceHTLCState_SETTLED, ampState.State) + require.Equal(ht, lnrpc.InvoiceHTLCState_SETTLED, ampState.State) + + // Finally, close all channels. + mts.closeChannels() } // testSendPaymentAMPInvoiceRepeat tests that it's possible to pay an AMP diff --git a/lntest/itest/lnd_mpp_test.go b/lntest/itest/lnd_mpp_test.go index e06db6f48..480d3d300 100644 --- a/lntest/itest/lnd_mpp_test.go +++ b/lntest/itest/lnd_mpp_test.go @@ -488,7 +488,8 @@ func (m *mppTestScenario) buildRoute(amt btcutil.Amount, } // updatePolicy updates a Dave's global channel policy and returns the expected -// policy for further check. +// policy for further check. It changes Dave's `FeeBaseMsat` from 1000 msat to +// 500,000 msat, and `FeeProportionalMillonths` from 1 msat to 1000 msat. func (m *mppTestScenario) updateDaveGlobalPolicy() *lnrpc.RoutingPolicy { const ( baseFeeMsat = 500_000 diff --git a/lntest/itest/lnd_test_list_on_test.go b/lntest/itest/lnd_test_list_on_test.go index e0a3daa32..9517689f6 100644 --- a/lntest/itest/lnd_test_list_on_test.go +++ b/lntest/itest/lnd_test_list_on_test.go @@ -48,10 +48,6 @@ var allTestCases = []*testCase{ name: "sendpayment amp", test: testSendPaymentAMP, }, - { - name: "sendpayment amp invoice", - test: testSendPaymentAMPInvoice, - }, { name: "sendpayment amp invoice repeat", test: testSendPaymentAMPInvoiceRepeat,