From 941716d60e8e4109f8e921dd50d464b1e155e338 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Mon, 13 Nov 2023 15:25:12 +0800 Subject: [PATCH] lntest+itest: add new test `testPaymentHTLCTimeout` This commit adds a new test case to validate that when an HTLC has timed out, the corresponding payment is marked as failed. --- itest/list_on_test.go | 8 + itest/lnd_payment_test.go | 317 +++++++++++++++++++++++++++++++++++++- lntest/harness.go | 10 ++ 3 files changed, 332 insertions(+), 3 deletions(-) diff --git a/itest/list_on_test.go b/itest/list_on_test.go index 7b3c579d6..336cec1b0 100644 --- a/itest/list_on_test.go +++ b/itest/list_on_test.go @@ -654,4 +654,12 @@ var allTestCases = []*lntest.TestCase{ Name: "coop close with external delivery", TestFunc: testCoopCloseWithExternalDelivery, }, + { + Name: "payment failed htlc local swept", + TestFunc: testPaymentFailedHTLCLocalSwept, + }, + { + Name: "payment succeeded htlc remote swept", + TestFunc: testPaymentSucceededHTLCRemoteSwept, + }, } diff --git a/itest/lnd_payment_test.go b/itest/lnd_payment_test.go index cdd163605..4a93fabfe 100644 --- a/itest/lnd_payment_test.go +++ b/itest/lnd_payment_test.go @@ -10,7 +10,9 @@ import ( "github.com/btcsuite/btcd/btcutil" "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/lntest" "github.com/lightningnetwork/lnd/lntest/node" @@ -20,9 +22,318 @@ import ( "github.com/stretchr/testify/require" ) -// testSendDirectPayment creates a topology Alice->Bob and then tests that Alice -// can send a direct payment to Bob. This test modifies the fee estimator to -// return floor fee rate(1 sat/vb). +// testPaymentSucceededHTLCRemoteSwept checks that when an outgoing HTLC is +// timed out and is swept by the remote via the direct preimage spend path, the +// payment will be marked as succeeded. This test creates a topology from Alice +// -> Bob, and let Alice send payments to Bob. Bob then goes offline, such that +// Alice's outgoing HTLC will time out. Once the force close transaction is +// broadcast by Alice, she then goes offline and Bob comes back online to take +// her outgoing HTLC. And Alice should mark this payment as succeeded after she +// comes back online again. +func testPaymentSucceededHTLCRemoteSwept(ht *lntest.HarnessTest) { + // Set the feerate to be 10 sat/vb. + ht.SetFeeEstimate(2500) + + // Open a channel with 100k satoshis between Alice and Bob with Alice + // being the sole funder of the channel. + chanAmt := btcutil.Amount(100_000) + openChannelParams := lntest.OpenChannelParams{ + Amt: chanAmt, + } + + // Create a two hop network: Alice -> Bob. + chanPoints, nodes := createSimpleNetwork(ht, nil, 2, openChannelParams) + chanPoint := chanPoints[0] + alice, bob := nodes[0], nodes[1] + + // We now create two payments, one above dust and the other below dust, + // and we should see different behavior in terms of when the payment + // will be marked as failed due to the HTLC timeout. + // + // First, create random preimages. + preimage := ht.RandomPreimage() + dustPreimage := ht.RandomPreimage() + + // Get the preimage hashes. + payHash := preimage.Hash() + dustPayHash := dustPreimage.Hash() + + // Create an hold invoice for Bob which expects a payment of 10k + // satoshis from Alice. + const paymentAmt = 10_000 + req := &invoicesrpc.AddHoldInvoiceRequest{ + Value: paymentAmt, + Hash: payHash[:], + // Use a small CLTV value so we can mine fewer blocks. + CltvExpiry: finalCltvDelta, + } + invoice := bob.RPC.AddHoldInvoice(req) + + // Create another hold invoice for Bob which expects a payment of 1k + // satoshis from Alice. + const dustAmt = 1000 + req = &invoicesrpc.AddHoldInvoiceRequest{ + Value: dustAmt, + Hash: dustPayHash[:], + // Use a small CLTV value so we can mine fewer blocks. + CltvExpiry: finalCltvDelta, + } + dustInvoice := bob.RPC.AddHoldInvoice(req) + + // Alice now sends both payments to Bob. + payReq := &routerrpc.SendPaymentRequest{ + PaymentRequest: invoice.PaymentRequest, + TimeoutSeconds: 3600, + } + dustPayReq := &routerrpc.SendPaymentRequest{ + PaymentRequest: dustInvoice.PaymentRequest, + TimeoutSeconds: 3600, + } + + // We expect the payment to stay in-flight from both streams. + ht.SendPaymentAssertInflight(alice, payReq) + ht.SendPaymentAssertInflight(alice, dustPayReq) + + // We also check the payments are marked as IN_FLIGHT in Alice's + // database. + ht.AssertPaymentStatus(alice, preimage, lnrpc.Payment_IN_FLIGHT) + ht.AssertPaymentStatus(alice, dustPreimage, lnrpc.Payment_IN_FLIGHT) + + // Bob should have two incoming HTLC. + ht.AssertIncomingHTLCActive(bob, chanPoint, payHash[:]) + ht.AssertIncomingHTLCActive(bob, chanPoint, dustPayHash[:]) + + // Alice should have two outgoing HTLCs. + ht.AssertOutgoingHTLCActive(alice, chanPoint, payHash[:]) + ht.AssertOutgoingHTLCActive(alice, chanPoint, dustPayHash[:]) + + // Let Bob go offline. + restartBob := ht.SuspendNode(bob) + + // Alice force closes the channel, which puts her commitment tx into + // the mempool. + ht.CloseChannelAssertPending(alice, chanPoint, true) + + // We now let Alice go offline to avoid her sweeping her outgoing htlc. + restartAlice := ht.SuspendNode(alice) + + // Mine one block to confirm Alice's force closing tx. + ht.MineBlocksAndAssertNumTxes(1, 1) + + // Restart Bob to settle the invoice and sweep the htlc output. + require.NoError(ht, restartBob()) + + // Bob now settles the invoices, since his link with Alice is broken, + // Alice won't know the preimages. + bob.RPC.SettleInvoice(preimage[:]) + bob.RPC.SettleInvoice(dustPreimage[:]) + + // Once Bob comes back up, he should find the force closing transaction + // from Alice and try to sweep the non-dust outgoing htlc via the + // direct preimage spend. + ht.AssertNumPendingSweeps(bob, 1) + + // Mine a block to trigger the sweep. + // + // TODO(yy): remove it once `blockbeat` is implemented. + ht.MineEmptyBlocks(1) + + // Mine Bob's sweeping tx. + ht.MineBlocksAndAssertNumTxes(1, 1) + + // Let Alice come back up. Since the channel is now closed, we expect + // different behaviors based on whether the HTLC is a dust. + // - For dust payment, it should be failed now as the HTLC won't go + // onchain. + // - For non-dust payment, it should be marked as succeeded since her + // outgoing htlc is swept by Bob. + require.NoError(ht, restartAlice()) + + // Since Alice is restarted, we need to track the payments again. + payStream := alice.RPC.TrackPaymentV2(payHash[:]) + dustPayStream := alice.RPC.TrackPaymentV2(dustPayHash[:]) + + // Check that the dust payment is failed in both the stream and DB. + ht.AssertPaymentStatus(alice, dustPreimage, lnrpc.Payment_FAILED) + ht.AssertPaymentStatusFromStream(dustPayStream, lnrpc.Payment_FAILED) + + // We expect the non-dust payment to marked as succeeded in Alice's + // database and also from her stream. + ht.AssertPaymentStatus(alice, preimage, lnrpc.Payment_SUCCEEDED) + ht.AssertPaymentStatusFromStream(payStream, lnrpc.Payment_SUCCEEDED) +} + +// testPaymentFailedHTLCLocalSwept checks that when an outgoing HTLC is timed +// out and claimed onchain via the timeout path, the payment will be marked as +// failed. This test creates a topology from Alice -> Bob, and let Alice send +// payments to Bob. Bob then goes offline, such that Alice's outgoing HTLC will +// time out. Alice will also be restarted to make sure resumed payments are +// also marked as failed. +func testPaymentFailedHTLCLocalSwept(ht *lntest.HarnessTest) { + success := ht.Run("fail payment", func(t *testing.T) { + st := ht.Subtest(t) + runTestPaymentHTLCTimeout(st, false) + }) + if !success { + return + } + + ht.Run("fail resumed payment", func(t *testing.T) { + st := ht.Subtest(t) + runTestPaymentHTLCTimeout(st, true) + }) +} + +// runTestPaymentHTLCTimeout is the helper function that actually runs the +// testPaymentFailedHTLCLocalSwept. +func runTestPaymentHTLCTimeout(ht *lntest.HarnessTest, restartAlice bool) { + // Set the feerate to be 10 sat/vb. + ht.SetFeeEstimate(2500) + + // Open a channel with 100k satoshis between Alice and Bob with Alice + // being the sole funder of the channel. + chanAmt := btcutil.Amount(100_000) + openChannelParams := lntest.OpenChannelParams{ + Amt: chanAmt, + } + + // Create a two hop network: Alice -> Bob. + chanPoints, nodes := createSimpleNetwork(ht, nil, 2, openChannelParams) + chanPoint := chanPoints[0] + alice, bob := nodes[0], nodes[1] + + // We now create two payments, one above dust and the other below dust, + // and we should see different behavior in terms of when the payment + // will be marked as failed due to the HTLC timeout. + // + // First, create random preimages. + preimage := ht.RandomPreimage() + dustPreimage := ht.RandomPreimage() + + // Get the preimage hashes. + payHash := preimage.Hash() + dustPayHash := dustPreimage.Hash() + + // Create an hold invoice for Bob which expects a payment of 10k + // satoshis from Alice. + const paymentAmt = 20_000 + req := &invoicesrpc.AddHoldInvoiceRequest{ + Value: paymentAmt, + Hash: payHash[:], + // Use a small CLTV value so we can mine fewer blocks. + CltvExpiry: finalCltvDelta, + } + invoice := bob.RPC.AddHoldInvoice(req) + + // Create another hold invoice for Bob which expects a payment of 1k + // satoshis from Alice. + const dustAmt = 1000 + req = &invoicesrpc.AddHoldInvoiceRequest{ + Value: dustAmt, + Hash: dustPayHash[:], + // Use a small CLTV value so we can mine fewer blocks. + CltvExpiry: finalCltvDelta, + } + dustInvoice := bob.RPC.AddHoldInvoice(req) + + // Alice now sends both the payments to Bob. + payReq := &routerrpc.SendPaymentRequest{ + PaymentRequest: invoice.PaymentRequest, + TimeoutSeconds: 3600, + } + dustPayReq := &routerrpc.SendPaymentRequest{ + PaymentRequest: dustInvoice.PaymentRequest, + TimeoutSeconds: 3600, + } + + // We expect the payment to stay in-flight from both streams. + ht.SendPaymentAssertInflight(alice, payReq) + ht.SendPaymentAssertInflight(alice, dustPayReq) + + // We also check the payments are marked as IN_FLIGHT in Alice's + // database. + ht.AssertPaymentStatus(alice, preimage, lnrpc.Payment_IN_FLIGHT) + ht.AssertPaymentStatus(alice, dustPreimage, lnrpc.Payment_IN_FLIGHT) + + // Bob should have two incoming HTLC. + ht.AssertIncomingHTLCActive(bob, chanPoint, payHash[:]) + ht.AssertIncomingHTLCActive(bob, chanPoint, dustPayHash[:]) + + // Alice should have two outgoing HTLCs. + ht.AssertOutgoingHTLCActive(alice, chanPoint, payHash[:]) + ht.AssertOutgoingHTLCActive(alice, chanPoint, dustPayHash[:]) + + // Let Bob go offline. + ht.Shutdown(bob) + + // We'll now mine enough blocks to trigger Alice to broadcast her + // commitment transaction due to the fact that the HTLC is about to + // timeout. With the default outgoing broadcast delta of zero, this + // will be the same height as the htlc expiry height. + numBlocks := padCLTV( + uint32(req.CltvExpiry - lncfg.DefaultOutgoingBroadcastDelta), + ) + ht.MineBlocks(int(numBlocks)) + + // Restart Alice if requested. + if restartAlice { + // Restart Alice to test the resumed payment is canceled. + ht.RestartNode(alice) + } + + // We now subscribe to the payment status. + payStream := alice.RPC.TrackPaymentV2(payHash[:]) + dustPayStream := alice.RPC.TrackPaymentV2(dustPayHash[:]) + + // Mine a block to confirm Alice's closing transaction. + ht.MineBlocksAndAssertNumTxes(1, 1) + + // Now the channel is closed, we expect different behaviors based on + // whether the HTLC is a dust. For dust payment, it should be failed + // now as the HTLC won't go onchain. For non-dust payment, it should + // still be inflight. It won't be marked as failed unless the outgoing + // HTLC is resolved onchain. + // + // NOTE: it's possible for Bob to race against Alice using the + // preimage path. If Bob successfully claims the HTLC, Alice should + // mark the non-dust payment as succeeded. + // + // Check that the dust payment is failed in both the stream and DB. + ht.AssertPaymentStatus(alice, dustPreimage, lnrpc.Payment_FAILED) + ht.AssertPaymentStatusFromStream(dustPayStream, lnrpc.Payment_FAILED) + + // Check that the non-dust payment is still in-flight. + // + // NOTE: we don't check the payment status from the stream here as + // there's no new status being sent. + ht.AssertPaymentStatus(alice, preimage, lnrpc.Payment_IN_FLIGHT) + + // We now have two possible cases for the non-dust payment: + // - Bob stays offline, and Alice will sweep her outgoing HTLC, which + // makes the payment failed. + // - Bob comes back online, and claims the HTLC on Alice's commitment + // via direct preimage spend, hence racing against Alice onchain. If + // he succeeds, Alice should mark the payment as succeeded. + // + // TODO(yy): test the second case once we have the RPC to clean + // mempool. + + // Since Alice's force close transaction has been confirmed, she should + // sweep her outgoing HTLC in next block. + ht.MineBlocksAndAssertNumTxes(1, 1) + + // Cleanup the channel. + ht.CleanupForceClose(alice) + + // We expect the non-dust payment to marked as failed in Alice's + // database and also from her stream. + ht.AssertPaymentStatus(alice, preimage, lnrpc.Payment_FAILED) + ht.AssertPaymentStatusFromStream(payStream, lnrpc.Payment_FAILED) +} + +// testSendDirectPayment creates a topology Alice->Bob and then tests that +// Alice can send a direct payment to Bob. This test modifies the fee estimator +// to return floor fee rate(1 sat/vb). func testSendDirectPayment(ht *lntest.HarnessTest) { // Grab Alice and Bob's nodes for convenience. alice, bob := ht.Alice, ht.Bob diff --git a/lntest/harness.go b/lntest/harness.go index 9a11eaa88..3e7ae3fae 100644 --- a/lntest/harness.go +++ b/lntest/harness.go @@ -399,6 +399,8 @@ func (h *HarnessTest) resetStandbyNodes(t *testing.T) { // config for the coming test. This will also inherit the // test's running context. h.RestartNodeWithExtraArgs(hn, hn.Cfg.OriginalExtraArgs) + + hn.AddToLogf("Finished test case %v", h.manager.currentTestCase) } } @@ -1771,6 +1773,14 @@ func (h *HarnessTest) SendPaymentAssertSettled(hn *node.HarnessNode, return h.SendPaymentAndAssertStatus(hn, req, lnrpc.Payment_SUCCEEDED) } +// SendPaymentAssertInflight sends a payment from the passed node and asserts +// the payment is inflight. +func (h *HarnessTest) SendPaymentAssertInflight(hn *node.HarnessNode, + req *routerrpc.SendPaymentRequest) *lnrpc.Payment { + + return h.SendPaymentAndAssertStatus(hn, req, lnrpc.Payment_IN_FLIGHT) +} + // OpenChannelRequest is used to open a channel using the method // OpenMultiChannelsAsync. type OpenChannelRequest struct {