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

@@ -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,
+	},
 }

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
+// testPaymentSucceededHTLCRemoteSwept checks that when an outgoing HTLC is
-// can send a direct payment to Bob. This test modifies the fee estimator to
+// timed out and is swept by the remote via the direct preimage spend path, the
-// return floor fee rate(1 sat/vb).
+// 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

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 {