Merge pull request #8836 from hieblmi/payment-failure-reason-cancel

routing: add payment failure reason `FailureReasonCancel`

Author: Roasbeef

channeldb/payments.go

@@ -104,7 +104,7 @@ var (
 )
 
 var (
-	// ErrNoSequenceNumber is returned if we lookup a payment which does
+	// ErrNoSequenceNumber is returned if we look up a payment which does
 	// not have a sequence number.
 	ErrNoSequenceNumber = errors.New("sequence number not found")
 
@@ -147,18 +147,20 @@ const (
 	// balance to complete the payment.
 	FailureReasonInsufficientBalance FailureReason = 4
 
-	// TODO(halseth): cancel state.
+	// FailureReasonCanceled indicates that the payment was canceled by the
+	// user.
+	FailureReasonCanceled FailureReason = 5
 
 	// TODO(joostjager): Add failure reasons for:
 	// LocalLiquidityInsufficient, RemoteCapacityInsufficient.
 )
 
-// Error returns a human readable error string for the FailureReason.
+// Error returns a human-readable error string for the FailureReason.
 func (r FailureReason) Error() string {
 	return r.String()
 }
 
-// String returns a human readable FailureReason.
+// String returns a human-readable FailureReason.
 func (r FailureReason) String() string {
 	switch r {
 	case FailureReasonTimeout:
@@ -171,6 +173,8 @@ func (r FailureReason) String() string {
 		return "incorrect_payment_details"
 	case FailureReasonInsufficientBalance:
 		return "insufficient_balance"
+	case FailureReasonCanceled:
+		return "canceled"
 	}
 
 	return "unknown"

docs/release-notes/release-notes-0.18.3.md

@@ -114,6 +114,11 @@ commitment when the channel was force closed.
   `--amp` flag when sending a payment specifying the payment request.
 
 ## Code Health
+
+* [Added](https://github.com/lightningnetwork/lnd/pull/8836) a new failure
+  reason `FailureReasonCanceled` to the list of payment failure reasons. It
+  indicates that a payment was manually cancelled by the user.
+ 
 ## Breaking Changes
 ## Performance Improvements
 

itest/list_on_test.go

@@ -193,6 +193,10 @@ var allTestCases = []*lntest.TestCase{
 		Name:     "immediate payment after channel opened",
 		TestFunc: testPaymentFollowingChannelOpen,
 	},
+	{
+		Name:     "payment failure reason canceled",
+		TestFunc: testPaymentFailureReasonCanceled,
+	},
 	{
 		Name:     "invoice update subscription",
 		TestFunc: testInvoiceSubscriptions,

itest/lnd_payment_test.go

@@ -1,6 +1,7 @@
 package itest
 
 import (
+	"context"
 	"crypto/sha256"
 	"encoding/hex"
 	"fmt"
@@ -13,19 +14,21 @@ import (
 	"github.com/lightningnetwork/lnd/lnrpc/routerrpc"
 	"github.com/lightningnetwork/lnd/lntest"
 	"github.com/lightningnetwork/lnd/lntest/node"
+	"github.com/lightningnetwork/lnd/lntest/rpc"
 	"github.com/lightningnetwork/lnd/lntest/wait"
+	"github.com/lightningnetwork/lnd/lntypes"
 	"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).
+// 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
 
 	// Create a list of commitment types we want to test.
-	commitTyes := []lnrpc.CommitmentType{
+	commitmentTypes := []lnrpc.CommitmentType{
 		lnrpc.CommitmentType_ANCHORS,
 		lnrpc.CommitmentType_SIMPLE_TAPROOT,
 	}
@@ -109,7 +112,7 @@ func testSendDirectPayment(ht *lntest.HarnessTest) {
 	}
 
 	// Run the test cases.
-	for _, ct := range commitTyes {
+	for _, ct := range commitmentTypes {
 		ht.Run(ct.String(), func(t *testing.T) {
 			st := ht.Subtest(t)
 
@@ -132,8 +135,9 @@ func testSendDirectPayment(ht *lntest.HarnessTest) {
 			}
 
 			// Open private channel for taproot channels.
-			params.Private = ct ==
-				lnrpc.CommitmentType_SIMPLE_TAPROOT
+			if ct == lnrpc.CommitmentType_SIMPLE_TAPROOT {
+				params.Private = true
+			}
 
 			testSendPayment(st, params)
 		})
@@ -429,7 +433,7 @@ func runAsyncPayments(ht *lntest.HarnessTest, alice, bob *node.HarnessNode,
 	// likely be lower, but we can't guarantee that any more HTLCs will
 	// succeed due to the limited path diversity and inability of the router
 	// to retry via another path.
-	numInvoices := int(input.MaxHTLCNumber / 2)
+	numInvoices := input.MaxHTLCNumber / 2
 
 	bobAmt := int64(numInvoices * paymentAmt)
 	aliceAmt := info.LocalBalance - bobAmt
@@ -534,10 +538,10 @@ func testBidirectionalAsyncPayments(ht *lntest.HarnessTest) {
 
 	// We'll create a number of invoices equal the max number of HTLCs that
 	// can be carried in one direction. The number on the commitment will
-	// likely be lower, but we can't guarantee that any more HTLCs will
-	// succeed due to the limited path diversity and inability of the router
-	// to retry via another path.
-	numInvoices := int(input.MaxHTLCNumber / 2)
+	// likely be lower, but we can't guarantee that more HTLCs will succeed
+	// due to the limited path diversity and inability of the router to
+	// retry via another path.
+	numInvoices := input.MaxHTLCNumber / 2
 
 	// Nodes should exchange the same amount of money and because of this
 	// at the end balances should remain the same.
@@ -597,7 +601,7 @@ func testBidirectionalAsyncPayments(ht *lntest.HarnessTest) {
 	assertChannelState(ht, alice, chanPoint, aliceAmt, bobAmt)
 
 	// Next query for Bob's and Alice's channel states, in order to confirm
-	// that all payment have been successful transmitted.
+	// that all payment have been successfully transmitted.
 	assertChannelState(ht, bob, chanPoint, bobAmt, aliceAmt)
 
 	// Finally, immediately close the channel. This function will also
@@ -662,7 +666,7 @@ func testInvoiceSubscriptions(ht *lntest.HarnessTest) {
 
 	// Now that the set of invoices has been added, we'll re-register for
 	// streaming invoice notifications for Bob, this time specifying the
-	// add invoice of the last prior invoice.
+	// add index of the last prior invoice.
 	req = &lnrpc.InvoiceSubscription{AddIndex: lastAddIndex}
 	bobInvoiceSubscription = bob.RPC.SubscribeInvoices(req)
 
@@ -766,3 +770,133 @@ func assertChannelState(ht *lntest.HarnessTest, hn *node.HarnessNode,
 	}, lntest.DefaultTimeout)
 	require.NoError(ht, err, "timeout while chekcing for balance")
 }
+
+// testPaymentFailureReasonCanceled ensures that the cancellation of a
+// SendPayment request results in the payment failure reason
+// FAILURE_REASON_CANCELED. This failure reason indicates that the context was
+// cancelled manually by the user. It does not interrupt the current payment
+// attempt, but will prevent any further payment attempts. The test steps are:
+// 1.) Alice pays Carol's invoice through Bob.
+// 2.) Bob intercepts the htlc, keeping the payment pending.
+// 3.) Alice cancels the payment context, the payment is still pending.
+// 4.) Bob fails OR resumes the intercepted HTLC.
+// 5.) Alice observes a failed OR succeeded payment with failure reason
+// FAILURE_REASON_CANCELED which suppresses further payment attempts.
+func testPaymentFailureReasonCanceled(ht *lntest.HarnessTest) {
+	// Initialize the test context with 3 connected nodes.
+	ts := newInterceptorTestScenario(ht)
+
+	alice, bob, carol := ts.alice, ts.bob, ts.carol
+
+	// Open and wait for channels.
+	const chanAmt = btcutil.Amount(300000)
+	p := lntest.OpenChannelParams{Amt: chanAmt}
+	reqs := []*lntest.OpenChannelRequest{
+		{Local: alice, Remote: bob, Param: p},
+		{Local: bob, Remote: carol, Param: p},
+	}
+	resp := ht.OpenMultiChannelsAsync(reqs)
+	cpAB, cpBC := resp[0], resp[1]
+
+	// Make sure Alice is aware of channel Bob=>Carol.
+	ht.AssertTopologyChannelOpen(alice, cpBC)
+
+	// First we check that the payment is successful when bob resumes the
+	// htlc even though the payment context was canceled before invoice
+	// settlement.
+	sendPaymentInterceptAndCancel(
+		ht, ts, cpAB, routerrpc.ResolveHoldForwardAction_RESUME,
+		lnrpc.Payment_SUCCEEDED,
+	)
+
+	// Next we check that the context cancellation results in the expected
+	// failure reason while the htlc is being held and failed after
+	// cancellation.
+	// Note that we'd have to reset Alice's mission control if we tested the
+	// htlc fail case before the htlc resume case.
+	sendPaymentInterceptAndCancel(
+		ht, ts, cpAB, routerrpc.ResolveHoldForwardAction_FAIL,
+		lnrpc.Payment_FAILED,
+	)
+
+	// Finally, close channels.
+	ht.CloseChannel(alice, cpAB)
+	ht.CloseChannel(bob, cpBC)
+}
+
+func sendPaymentInterceptAndCancel(ht *lntest.HarnessTest,
+	ts *interceptorTestScenario, cpAB *lnrpc.ChannelPoint,
+	interceptorAction routerrpc.ResolveHoldForwardAction,
+	expectedPaymentStatus lnrpc.Payment_PaymentStatus) {
+
+	// Prepare the test cases.
+	alice, bob, carol := ts.alice, ts.bob, ts.carol
+
+	// Connect the interceptor.
+	interceptor, cancelInterceptor := bob.RPC.HtlcInterceptor()
+
+	// Prepare the test cases.
+	addResponse := carol.RPC.AddInvoice(&lnrpc.Invoice{
+		ValueMsat: 1000,
+	})
+	invoice := carol.RPC.LookupInvoice(addResponse.RHash)
+
+	// We initiate a payment from Alice and define the payment context
+	// cancellable.
+	ctx, cancelPaymentContext := context.WithCancel(context.Background())
+	var paymentStream rpc.PaymentClient
+	go func() {
+		req := &routerrpc.SendPaymentRequest{
+			PaymentRequest: invoice.PaymentRequest,
+			TimeoutSeconds: 60,
+			FeeLimitSat:    100000,
+			Cancelable:     true,
+		}
+
+		paymentStream = alice.RPC.SendPaymentWithContext(ctx, req)
+	}()
+
+	// We start the htlc interceptor with a simple implementation that
+	// saves all intercepted packets. These packets are held to simulate a
+	// pending payment.
+	packet := ht.ReceiveHtlcInterceptor(interceptor)
+
+	// Here we should wait for the channel to contain a pending htlc, and
+	// also be shown as being active.
+	ht.AssertIncomingHTLCActive(bob, cpAB, invoice.RHash)
+
+	// Ensure that Alice's payment is in-flight because Bob is holding the
+	// htlc.
+	ht.AssertPaymentStatusFromStream(paymentStream, lnrpc.Payment_IN_FLIGHT)
+
+	// Cancel the payment context. This should end the payment stream
+	// context, but the payment should still be in state in-flight without a
+	// failure reason.
+	cancelPaymentContext()
+
+	var preimage lntypes.Preimage
+	copy(preimage[:], invoice.RPreimage)
+	payment := ht.AssertPaymentStatus(
+		alice, preimage, lnrpc.Payment_IN_FLIGHT,
+	)
+	reasonNone := lnrpc.PaymentFailureReason_FAILURE_REASON_NONE
+	require.Equal(ht, reasonNone, payment.FailureReason)
+
+	// Bob sends the interceptor action to the intercepted htlc.
+	err := interceptor.Send(&routerrpc.ForwardHtlcInterceptResponse{
+		IncomingCircuitKey: packet.IncomingCircuitKey,
+		Action:             interceptorAction,
+	})
+	require.NoError(ht, err, "failed to send request")
+
+	// Assert that the payment status is as expected.
+	ht.AssertPaymentStatus(alice, preimage, expectedPaymentStatus)
+
+	// Since the payment context was cancelled, no further payment attempts
+	// should've been made, and we observe FAILURE_REASON_CANCELED.
+	expectedReason := lnrpc.PaymentFailureReason_FAILURE_REASON_CANCELED
+	ht.AssertPaymentFailureReason(alice, preimage, expectedReason)
+
+	// Cancel the context, which will disconnect the above interceptor.
+	cancelInterceptor()
+}