Merge pull request #9626 from ziggie1984/payment-lifecycle-small-fix

payment lifecycle small fix

docs/release-notes/release-notes-0.19.0.md

@@ -103,6 +103,10 @@
 * [Fixed](https://github.com/lightningnetwork/lnd/pull/9609) a bug that may
   cause `listunspent` to give inaccurate wallet UTXOs.
 
+* [Fixed](https://github.com/lightningnetwork/lnd/pull/9626) a bug where a
+keysend payment would not fail properly and only resolve after restart. Now
+keysend payment validation is stricter.
+
 # New Features
 
 * Add support for [archiving channel backup](https://github.com/lightningnetwork/lnd/pull/9232)

itest/list_on_test.go

@@ -402,6 +402,10 @@ var allTestCases = []*lntest.TestCase{
 		Name:     "sendtoroute amp",
 		TestFunc: testSendToRouteAMP,
 	},
+	{
+		Name:     "send payment keysend mpp fail",
+		TestFunc: testSendPaymentKeysendMPPFail,
+	},
 	{
 		Name:     "forward interceptor dedup htlcs",
 		TestFunc: testForwardInterceptorDedupHtlc,

itest/lnd_payment_test.go

@@ -1,6 +1,7 @@
 package itest
 
 import (
+	"bytes"
 	"context"
 	"crypto/sha256"
 	"encoding/hex"
@@ -20,6 +21,7 @@ import (
 	"github.com/lightningnetwork/lnd/lntest/wait"
 	"github.com/lightningnetwork/lnd/lntypes"
 	"github.com/lightningnetwork/lnd/lnwire"
+	"github.com/lightningnetwork/lnd/record"
 	"github.com/stretchr/testify/require"
 )
 
@@ -1351,3 +1353,46 @@ func runSendToRouteFailHTLCTimeout(ht *lntest.HarnessTest, restartAlice bool) {
 	ht.AssertPaymentStatus(alice, preimage.Hash(), lnrpc.Payment_FAILED)
 	ht.AssertPaymentStatusFromStream(payStream, lnrpc.Payment_FAILED)
 }
+
+// testSendPaymentKeysendMPPFail tests sending a keysend payment and trying to
+// split it will fail because keysend payment in combination with MPP is not
+// supported.
+func testSendPaymentKeysendMPPFail(ht *lntest.HarnessTest) {
+	const chanAmt = btcutil.Amount(100_000)
+	p := lntest.OpenChannelParams{Amt: chanAmt}
+
+	// Initialize the test context with 2 connected nodes.
+	cfgs := [][]string{nil, nil}
+
+	// Open and wait for channels.
+	_, nodes := ht.CreateSimpleNetwork(cfgs, p)
+	alice, bob := nodes[0], nodes[1]
+
+	// Send a keysend payment which has a total amount which is smaller
+	// than the maxShardAmt. The payment will still be recorded in the db
+	// but it will fail immediately because keysend and MPP cannot be
+	// combined.
+	dest, err := hex.DecodeString(bob.PubKeyStr)
+	require.NoError(ht, err)
+
+	// Create a preimage and corresponding payment hash for the keysend
+	// payment.
+	preimage := bytes.Repeat([]byte{0x10}, 32)
+	hash := sha256.Sum256(preimage)
+
+	client := alice.RPC.SendPayment(&routerrpc.SendPaymentRequest{
+		DestCustomRecords: map[uint64][]byte{
+			record.KeySendType: preimage,
+		},
+		MaxShardSizeMsat: 10_000,
+		Amt:              100_000,
+		MaxParts:         10,
+		Dest:             dest,
+		PaymentHash:      hash[:],
+	})
+
+	// We expect the payment to fail immediately because keysend and MPP
+	// cannot be combined.
+	_, err = ht.ReceivePaymentUpdate(client)
+	require.Error(ht, err)
+}

lnrpc/routerrpc/router_backend.go

@@ -884,6 +884,18 @@ func (r *RouterBackend) extractIntentFromSendRequest(
 	}
 	payIntent.DestCustomRecords = customRecords
 
+	// Keysend payments do not support MPP payments.
+	//
+	// NOTE: There is no need to validate the `MaxParts` value here because
+	// it is set to 1 somewhere else in case it's a keysend payment.
+	if customRecords.IsKeysend() {
+		if payIntent.MaxShardAmt != nil {
+			return nil, errors.New("keysend payments cannot " +
+				"specify a max shard amount - MPP not " +
+				"supported with keysend payments")
+		}
+	}
+
 	firstHopRecords := lnwire.CustomRecords(rpcPayReq.FirstHopCustomRecords)
 	if err := firstHopRecords.Validate(); err != nil {
 		return nil, err

record/custom_records.go

@@ -1,6 +1,8 @@
 package record
 
-import "fmt"
+import (
+	"fmt"
+)
 
 const (
 	// CustomTypeStart is the start of the custom tlv type range as defined
@@ -22,3 +24,8 @@ func (c CustomSet) Validate() error {
 
 	return nil
 }
+
+// IsKeysend checks if the custom records contain the key send type.
+func (c CustomSet) IsKeysend() bool {
+	return c[KeySendType] != nil
+}

routing/router.go

@@ -1255,6 +1255,8 @@ func (r *ChannelRouter) sendPayment(ctx context.Context,
 	}
 
 	// Validate the custom records before we attempt to send the payment.
+	// TODO(ziggie): Move this check before registering the payment in the
+	// db (InitPayment).
 	if err := firstHopCustomRecords.Validate(); err != nil {
 		return [32]byte{}, nil, err
 	}