diff --git a/lntemp/harness_assertion.go b/lntemp/harness_assertion.go index edf1eea15..fc0351160 100644 --- a/lntemp/harness_assertion.go +++ b/lntemp/harness_assertion.go @@ -1329,3 +1329,117 @@ func (h *HarnessTest) AssertConnected(a, b *node.HarnessNode) { h.AssertPeerConnected(a, b) h.AssertPeerConnected(b, a) } + +// AssertAmountPaid checks that the ListChannels command of the provided +// node list the total amount sent and received as expected for the +// provided channel. +func (h *HarnessTest) AssertAmountPaid(channelName string, hn *node.HarnessNode, + chanPoint *lnrpc.ChannelPoint, amountSent, amountReceived int64) { + + checkAmountPaid := func() error { + // Find the targeted channel. + channel, err := h.findChannel(hn, chanPoint) + if err != nil { + return fmt.Errorf("assert amount failed: %w", err) + } + + if channel.TotalSatoshisSent != amountSent { + return fmt.Errorf("%v: incorrect amount"+ + " sent: %v != %v", channelName, + channel.TotalSatoshisSent, + amountSent) + } + if channel.TotalSatoshisReceived != + amountReceived { + + return fmt.Errorf("%v: incorrect amount"+ + " received: %v != %v", + channelName, + channel.TotalSatoshisReceived, + amountReceived) + } + + return nil + } + + // As far as HTLC inclusion in commitment transaction might be + // postponed we will try to check the balance couple of times, + // and then if after some period of time we receive wrong + // balance return the error. + err := wait.NoError(checkAmountPaid, DefaultTimeout) + require.NoError(h, err, "timeout while checking amount paid") +} + +// AssertLastHTLCError checks that the last sent HTLC of the last payment sent +// by the given node failed with the expected failure code. +func (h *HarnessTest) AssertLastHTLCError(hn *node.HarnessNode, + code lnrpc.Failure_FailureCode) { + + // Use -1 to specify the last HTLC. + h.assertHTLCError(hn, code, -1) +} + +// AssertFirstHTLCError checks that the first HTLC of the last payment sent +// by the given node failed with the expected failure code. +func (h *HarnessTest) AssertFirstHTLCError(hn *node.HarnessNode, + code lnrpc.Failure_FailureCode) { + + // Use 0 to specify the first HTLC. + h.assertHTLCError(hn, code, 0) +} + +// assertLastHTLCError checks that the HTLC at the specified index of the last +// payment sent by the given node failed with the expected failure code. +func (h *HarnessTest) assertHTLCError(hn *node.HarnessNode, + code lnrpc.Failure_FailureCode, index int) { + + req := &lnrpc.ListPaymentsRequest{ + IncludeIncomplete: true, + } + + err := wait.NoError(func() error { + paymentsResp := hn.RPC.ListPayments(req) + + payments := paymentsResp.Payments + if len(payments) == 0 { + return fmt.Errorf("no payments found") + } + + payment := payments[len(payments)-1] + htlcs := payment.Htlcs + if len(htlcs) == 0 { + return fmt.Errorf("no htlcs found") + } + + // If the index is greater than 0, check we have enough htlcs. + if index > 0 && len(htlcs) <= index { + return fmt.Errorf("not enough htlcs") + } + + // If index is less than or equal to 0, we will read the last + // htlc. + if index <= 0 { + index = len(htlcs) - 1 + } + + htlc := htlcs[index] + + // The htlc must have a status of failed. + if htlc.Status != lnrpc.HTLCAttempt_FAILED { + return fmt.Errorf("htlc should be failed") + } + // The failure field must not be empty. + if htlc.Failure == nil { + return fmt.Errorf("expected htlc failure") + } + + // Exit if the expected code is found. + if htlc.Failure.Code == code { + return nil + } + + return fmt.Errorf("unexpected failure code") + }, DefaultTimeout) + + require.NoError(h, err, "timeout checking HTLC error") +} diff --git a/lntest/itest/list_on_test.go b/lntest/itest/list_on_test.go index 8647c8d30..f60a70b8d 100644 --- a/lntest/itest/list_on_test.go +++ b/lntest/itest/list_on_test.go @@ -79,4 +79,8 @@ var allTestCasesTemp = []*lntemp.TestCase{ Name: "disconnecting target peer", TestFunc: testDisconnectingTargetPeer, }, + { + Name: "sphinx replay persistence", + TestFunc: testSphinxReplayPersistence, + }, } diff --git a/lntest/itest/lnd_misc_test.go b/lntest/itest/lnd_misc_test.go index b0558e757..dda61bc2f 100644 --- a/lntest/itest/lnd_misc_test.go +++ b/lntest/itest/lnd_misc_test.go @@ -1,7 +1,6 @@ package itest import ( - "bytes" "context" "crypto/rand" "encoding/hex" @@ -13,12 +12,12 @@ import ( "github.com/btcsuite/btcd/btcutil" "github.com/btcsuite/btcd/wire" "github.com/btcsuite/btcwallet/wallet" - "github.com/davecgh/go-spew/spew" "github.com/lightningnetwork/lnd/chainreg" "github.com/lightningnetwork/lnd/funding" "github.com/lightningnetwork/lnd/input" "github.com/lightningnetwork/lnd/lncfg" "github.com/lightningnetwork/lnd/lnrpc" + "github.com/lightningnetwork/lnd/lnrpc/invoicesrpc" "github.com/lightningnetwork/lnd/lnrpc/routerrpc" "github.com/lightningnetwork/lnd/lnrpc/walletrpc" "github.com/lightningnetwork/lnd/lntemp" @@ -120,38 +119,28 @@ func testDisconnectingTargetPeer(ht *lntemp.HarnessTest) { ht.AssertConnected(alice, bob) } -// testSphinxReplayPersistence verifies that replayed onion packets are rejected -// by a remote peer after a restart. We use a combination of unsafe -// configuration arguments to force Carol to replay the same sphinx packet after -// reconnecting to Dave, and compare the returned failure message with what we -// expect for replayed onion packets. -func testSphinxReplayPersistence(net *lntest.NetworkHarness, t *harnessTest) { - ctxb := context.Background() - - // Open a channel with 100k satoshis between Carol and Dave with Carol being - // the sole funder of the channel. +// testSphinxReplayPersistence verifies that replayed onion packets are +// rejected by a remote peer after a restart. We use a combination of unsafe +// configuration arguments to force Carol to replay the same sphinx packet +// after reconnecting to Dave, and compare the returned failure message with +// what we expect for replayed onion packets. +func testSphinxReplayPersistence(ht *lntemp.HarnessTest) { + // Open a channel with 100k satoshis between Carol and Dave with Carol + // being the sole funder of the channel. chanAmt := btcutil.Amount(100000) // First, we'll create Dave, the receiver, and start him in hodl mode. - dave := net.NewNode(t.t, "Dave", []string{"--hodl.exit-settle"}) - - // We must remember to shutdown the nodes we created for the duration - // of the tests, only leaving the two seed nodes (Alice and Bob) within - // our test network. - defer shutdownAndAssert(net, t, dave) + dave := ht.NewNode("Dave", []string{"--hodl.exit-settle"}) // Next, we'll create Carol and establish a channel to from her to // Dave. Carol is started in both unsafe-replay which will cause her to // replay any pending Adds held in memory upon reconnection. - carol := net.NewNode(t.t, "Carol", []string{"--unsafe-replay"}) - defer shutdownAndAssert(net, t, carol) + carol := ht.NewNode("Carol", []string{"--unsafe-replay"}) + ht.FundCoins(btcutil.SatoshiPerBitcoin, carol) - net.ConnectNodes(t.t, carol, dave) - net.SendCoins(t.t, btcutil.SatoshiPerBitcoin, carol) - - chanPoint := openChannelAndAssert( - t, net, carol, dave, - lntest.OpenChannelParams{ + ht.ConnectNodes(carol, dave) + chanPoint := ht.OpenChannel( + carol, dave, lntemp.OpenChannelParams{ Amt: chanAmt, }, ) @@ -161,136 +150,87 @@ func testSphinxReplayPersistence(net *lntest.NetworkHarness, t *harnessTest) { // by paying from Carol directly to Dave, because the '--unsafe-replay' // setup doesn't apply to locally added htlcs. In that case, the // mailbox, that is responsible for generating the replay, is bypassed. - fred := net.NewNode(t.t, "Fred", nil) - defer shutdownAndAssert(net, t, fred) + fred := ht.NewNode("Fred", nil) + ht.FundCoins(btcutil.SatoshiPerBitcoin, fred) - net.ConnectNodes(t.t, fred, carol) - net.SendCoins(t.t, btcutil.SatoshiPerBitcoin, fred) - - chanPointFC := openChannelAndAssert( - t, net, fred, carol, - lntest.OpenChannelParams{ + ht.ConnectNodes(fred, carol) + chanPointFC := ht.OpenChannel( + fred, carol, lntemp.OpenChannelParams{ Amt: chanAmt, }, ) + defer ht.CloseChannel(fred, chanPointFC) // Now that the channel is open, create an invoice for Dave which // expects a payment of 1000 satoshis from Carol paid via a particular // preimage. const paymentAmt = 1000 - preimage := bytes.Repeat([]byte("A"), 32) + preimage := ht.Random32Bytes() invoice := &lnrpc.Invoice{ Memo: "testing", RPreimage: preimage, Value: paymentAmt, } - ctxt, _ := context.WithTimeout(ctxb, defaultTimeout) - invoiceResp, err := dave.AddInvoice(ctxt, invoice) - if err != nil { - t.Fatalf("unable to add invoice: %v", err) - } + invoiceResp := dave.RPC.AddInvoice(invoice) // Wait for all channels to be recognized and advertized. - err = carol.WaitForNetworkChannelOpen(chanPoint) - if err != nil { - t.Fatalf("alice didn't advertise channel before "+ - "timeout: %v", err) - } - err = dave.WaitForNetworkChannelOpen(chanPoint) - if err != nil { - t.Fatalf("bob didn't advertise channel before "+ - "timeout: %v", err) - } - err = carol.WaitForNetworkChannelOpen(chanPointFC) - if err != nil { - t.Fatalf("alice didn't advertise channel before "+ - "timeout: %v", err) - } - err = fred.WaitForNetworkChannelOpen(chanPointFC) - if err != nil { - t.Fatalf("bob didn't advertise channel before "+ - "timeout: %v", err) - } + ht.AssertTopologyChannelOpen(carol, chanPoint) + ht.AssertTopologyChannelOpen(dave, chanPoint) + ht.AssertTopologyChannelOpen(carol, chanPointFC) + ht.AssertTopologyChannelOpen(fred, chanPointFC) // With the invoice for Dave added, send a payment from Fred paying // to the above generated invoice. - ctx, cancel := context.WithCancel(ctxb) - defer cancel() - - payStream, err := fred.RouterClient.SendPaymentV2( - ctx, - &routerrpc.SendPaymentRequest{ - PaymentRequest: invoiceResp.PaymentRequest, - TimeoutSeconds: 60, - FeeLimitMsat: noFeeLimitMsat, - }, - ) - if err != nil { - t.Fatalf("unable to open payment stream: %v", err) + req := &routerrpc.SendPaymentRequest{ + PaymentRequest: invoiceResp.PaymentRequest, + TimeoutSeconds: 60, + FeeLimitMsat: noFeeLimitMsat, } - - time.Sleep(200 * time.Millisecond) + payStream := fred.RPC.SendPayment(req) // Dave's invoice should not be marked as settled. - payHash := &lnrpc.PaymentHash{ - RHash: invoiceResp.RHash, - } - ctxt, _ = context.WithTimeout(ctxb, defaultTimeout) - dbInvoice, err := dave.LookupInvoice(ctxt, payHash) - if err != nil { - t.Fatalf("unable to lookup invoice: %v", err) - } - if dbInvoice.Settled { // nolint:staticcheck - t.Fatalf("dave's invoice should not be marked as settled: %v", - spew.Sdump(dbInvoice)) + msg := &invoicesrpc.LookupInvoiceMsg{ + InvoiceRef: &invoicesrpc.LookupInvoiceMsg_PaymentAddr{ + PaymentAddr: invoiceResp.PaymentAddr, + }, } + dbInvoice := dave.RPC.LookupInvoiceV2(msg) + require.NotEqual(ht, lnrpc.InvoiceHTLCState_SETTLED, dbInvoice.State, + "dave's invoice should not be marked as settled") // With the payment sent but hedl, all balance related stats should not // have changed. - err = wait.InvariantNoError( - assertAmountSent(0, carol, dave), 3*time.Second, - ) - if err != nil { - t.Fatalf(err.Error()) - } + ht.AssertAmountPaid("carol => dave", carol, chanPoint, 0, 0) + ht.AssertAmountPaid("dave <= carol", dave, chanPoint, 0, 0) + + // Before we restart Dave, make sure both Carol and Dave have added the + // HTLC. + ht.AssertNumActiveHtlcs(carol, 2) + ht.AssertNumActiveHtlcs(dave, 1) // With the first payment sent, restart dave to make sure he is // persisting the information required to detect replayed sphinx // packets. - if err := net.RestartNode(dave, nil); err != nil { - t.Fatalf("unable to restart dave: %v", err) - } + ht.RestartNode(dave) // Carol should retransmit the Add hedl in her mailbox on startup. Dave // should not accept the replayed Add, and actually fail back the // pending payment. Even though he still holds the original settle, if // he does fail, it is almost certainly caused by the sphinx replay // protection, as it is the only validation we do in hodl mode. - result, err := getPaymentResult(payStream) - if err != nil { - t.Fatalf("unable to receive payment response: %v", err) - } - + // // Assert that Fred receives the expected failure after Carol sent a // duplicate packet that fails due to sphinx replay detection. - if result.Status == lnrpc.Payment_SUCCEEDED { - t.Fatalf("expected payment error") - } - assertLastHTLCError(t, fred, lnrpc.Failure_INVALID_ONION_KEY) + ht.AssertPaymentStatusFromStream(payStream, lnrpc.Payment_FAILED) + ht.AssertLastHTLCError(fred, lnrpc.Failure_INVALID_ONION_KEY) // Since the payment failed, the balance should still be left // unaltered. - err = wait.InvariantNoError( - assertAmountSent(0, carol, dave), 3*time.Second, - ) - if err != nil { - t.Fatalf(err.Error()) - } - - closeChannelAndAssert(t, net, carol, chanPoint, true) + ht.AssertAmountPaid("carol => dave", carol, chanPoint, 0, 0) + ht.AssertAmountPaid("dave <= carol", dave, chanPoint, 0, 0) // Cleanup by mining the force close and sweep transaction. - cleanupForceClose(t, net, carol, chanPoint) + ht.ForceCloseChannel(carol, chanPoint) } // testListChannels checks that the response from ListChannels is correct. It diff --git a/lntest/itest/lnd_test_list_on_test.go b/lntest/itest/lnd_test_list_on_test.go index 879b48e5d..1c75310bc 100644 --- a/lntest/itest/lnd_test_list_on_test.go +++ b/lntest/itest/lnd_test_list_on_test.go @@ -64,10 +64,6 @@ var allTestCases = []*testCase{ name: "single hop invoice", test: testSingleHopInvoice, }, - { - name: "sphinx replay persistence", - test: testSphinxReplayPersistence, - }, { name: "list channels", test: testListChannels,