Merge pull request #9603 from NishantBansal2003/validate-mpp

routerrpc: add validation to MPP params

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

@@ -168,6 +168,11 @@ close transaction.
   [changed](https://github.com/lightningnetwork/lnd/pull/9627) so the sweeper
   will always attempt the sweep as long as the budget can be partially covered.
 
+* [Add](https://github.com/lightningnetwork/lnd/pull/9603) validation to ensure
+  that MPP parameters are compatible with the payment amount before attempting
+  the payment. This prevents payments from entering a path finding loop that
+  would eventually timeout.
+
 ## RPC Additions
 
 * [Add a new rpc endpoint](https://github.com/lightningnetwork/lnd/pull/8843)

lnrpc/routerrpc/router_backend.go

@@ -36,6 +36,10 @@ const (
 	// TODO(roasbeef): make this value dynamic based on expected number of
 	// attempts for given amount.
 	DefaultMaxParts = 16
+
+	// MaxPartsUpperLimit defines the maximum allowable number of splits
+	// for MPP/AMP when the user is attempting to send a payment.
+	MaxPartsUpperLimit = 1000
 )
 
 // RouterBackend contains the backend implementation of the router rpc sub
@@ -868,6 +872,16 @@ func (r *RouterBackend) extractIntentFromSendRequest(
 	if rpcPayReq.MaxShardSizeMsat > 0 {
 		shardAmtMsat := lnwire.MilliSatoshi(rpcPayReq.MaxShardSizeMsat)
 		payIntent.MaxShardAmt = &shardAmtMsat
+
+		// If the requested max_parts exceeds the allowed limit, then we
+		// cannot send the payment amount.
+		if payIntent.MaxParts > MaxPartsUpperLimit {
+			return nil, fmt.Errorf("requested max_parts (%v) "+
+				"exceeds the allowed upper limit of %v; cannot"+
+				" send payment amount with max_shard_size_msat"+
+				"=%v", payIntent.MaxParts, MaxPartsUpperLimit,
+				*payIntent.MaxShardAmt)
+		}
 	}
 
 	// Take fee limit from request.
@@ -1203,6 +1217,23 @@ func (r *RouterBackend) extractIntentFromSendRequest(
 		payIntent.DestFeatures = features
 	}
 
+	// Validate that the MPP parameters are compatible with the
+	// payment amount. In other words, the parameters are invalid if
+	// they do not permit sending the full payment amount.
+	if payIntent.MaxShardAmt != nil {
+		maxPossibleAmount := (*payIntent.MaxShardAmt) *
+			lnwire.MilliSatoshi(payIntent.MaxParts)
+
+		if payIntent.Amount > maxPossibleAmount {
+			return nil, fmt.Errorf("payment amount %v exceeds "+
+				"maximum possible amount %v with max_parts=%v "+
+				"and max_shard_size_msat=%v", payIntent.Amount,
+				maxPossibleAmount, payIntent.MaxParts,
+				*payIntent.MaxShardAmt,
+			)
+		}
+	}
+
 	// Do bounds checking with the block padding so the router isn't
 	// left with a zombie payment in case the user messes up.
 	err = routing.ValidateCLTVLimit(

lnrpc/routerrpc/router_backend_test.go

@@ -7,8 +7,10 @@ import (
 	"testing"
 
 	"github.com/btcsuite/btcd/btcutil"
+	"github.com/btcsuite/btcd/chaincfg"
 	"github.com/lightningnetwork/lnd/lnrpc"
 	"github.com/lightningnetwork/lnd/lnwire"
+	"github.com/lightningnetwork/lnd/record"
 	"github.com/lightningnetwork/lnd/routing"
 	"github.com/lightningnetwork/lnd/routing/route"
 	"github.com/stretchr/testify/require"
@@ -475,3 +477,370 @@ func testUnmarshalAMP(t *testing.T, test unmarshalAMPTest) {
 		t.Fatalf("test case has non-standard outcome")
 	}
 }
+
+// extractIntentTestCase defines a test case for the
+// TestExtractIntentFromSendRequest function. It includes the test name, the
+// RouterBackend instance, the SendPaymentRequest to be tested, a boolean
+// indicating if the test case is valid, and the expected error message if
+// applicable.
+type extractIntentTestCase struct {
+	name             string
+	backend          *RouterBackend
+	sendReq          *SendPaymentRequest
+	valid            bool
+	expectedErrorMsg string
+}
+
+// TestExtractIntentFromSendRequest verifies that extractIntentFromSendRequest
+// correctly translates a SendPaymentRequest from an RPC client into a
+// LightningPayment intent.
+func TestExtractIntentFromSendRequest(t *testing.T) {
+	const paymentAmount = btcutil.Amount(300_000)
+
+	const paymentReq = "lnbcrt500u1pnh0xflpp56w08q26t896vg2e9mtdkrem320tp" +
+		"wws9z9sfr7dw86dx97d90u4sdqqcqzzsxqyz5vqsp5z9945kvfy5g9afmakz" +
+		"yrur2t4hhn2tr87un8j0r0e6l5m5zm0fus9qxpqysgqk98c6j7qefdpdmzt4" +
+		"g6aykds4ydvf2x9lpngqcfux3hv8qlraan9v3s9296r5w5eh959yzadgh5ck" +
+		"gjydgyfxdpumxtuk3p3caugmlqpz5necs"
+
+	destNodeBytes, err := hex.DecodeString(destKey)
+	require.NoError(t, err)
+
+	target, err := route.NewVertexFromBytes(destNodeBytes)
+	require.NoError(t, err)
+
+	testCases := []extractIntentTestCase{
+		{
+			name:    "Time preference out of range",
+			backend: &RouterBackend{},
+			sendReq: &SendPaymentRequest{
+				TimePref: 2,
+			},
+			valid:            false,
+			expectedErrorMsg: "time preference out of range",
+		},
+		{
+			name:    "Outgoing channel exclusivity violation",
+			backend: &RouterBackend{},
+			sendReq: &SendPaymentRequest{
+				OutgoingChanId:  38484,
+				OutgoingChanIds: []uint64{383322},
+			},
+			valid: false,
+			expectedErrorMsg: "outgoing_chan_id and " +
+				"outgoing_chan_ids are mutually exclusive",
+		},
+		{
+			name:    "Invalid last hop pubkey length",
+			backend: &RouterBackend{},
+			sendReq: &SendPaymentRequest{
+				LastHopPubkey: []byte{1},
+			},
+			valid:            false,
+			expectedErrorMsg: "invalid vertex length",
+		},
+		{
+			name: "total time lock exceeds max allowed",
+			backend: &RouterBackend{
+				MaxTotalTimelock: 1000,
+			},
+			sendReq: &SendPaymentRequest{
+				CltvLimit: 1001,
+			},
+			valid: false,
+			expectedErrorMsg: "total time lock of 1001 exceeds " +
+				"max allowed 1000",
+		},
+		{
+			name:    "Max parts exceed allowed limit",
+			backend: &RouterBackend{},
+			sendReq: &SendPaymentRequest{
+				MaxParts:         1001,
+				MaxShardSizeMsat: 300_000,
+			},
+			valid: false,
+			expectedErrorMsg: "requested max_parts (1001) exceeds" +
+				" the allowed upper limit",
+		},
+		{
+			name: "Fee limit conflict, both sat and msat " +
+				"specified",
+			backend: &RouterBackend{},
+			sendReq: &SendPaymentRequest{
+				FeeLimitSat:  1000000,
+				FeeLimitMsat: 1000000000,
+			},
+			valid: false,
+			expectedErrorMsg: "sat and msat arguments are " +
+				"mutually exclusive",
+		},
+		{
+			name:    "Fee limit cannot be negative",
+			backend: &RouterBackend{},
+			sendReq: &SendPaymentRequest{
+				FeeLimitSat: -1,
+			},
+			valid:            false,
+			expectedErrorMsg: "amount cannot be negative",
+		},
+		{
+			name: "Dest custom records with type below minimum" +
+				" range",
+			backend: &RouterBackend{},
+			sendReq: &SendPaymentRequest{
+				DestCustomRecords: map[uint64][]byte{
+					65530: {1, 2},
+				},
+			},
+			valid:            false,
+			expectedErrorMsg: "no custom records with types below",
+		},
+		{
+			name:    "MPP params with keysend payments",
+			backend: &RouterBackend{},
+			sendReq: &SendPaymentRequest{
+				DestCustomRecords: map[uint64][]byte{
+					record.KeySendType: {1, 2},
+				},
+				MaxShardSizeMsat: 300_000,
+			},
+			valid: false,
+			expectedErrorMsg: "MPP not supported with keysend " +
+				"payments",
+		},
+		{
+			name: "Custom record entry with TLV type below " +
+				"minimum range",
+			backend: &RouterBackend{},
+			sendReq: &SendPaymentRequest{
+				FirstHopCustomRecords: map[uint64][]byte{
+					65530: {1, 2},
+				},
+			},
+			valid:            false,
+			expectedErrorMsg: "custom records entry with TLV type",
+		},
+		{
+			name: "Amount conflict, both sat and msat specified",
+			backend: &RouterBackend{
+				ShouldSetExpEndorsement: func() bool {
+					return true
+				},
+			},
+			sendReq: &SendPaymentRequest{
+				Amt:     int64(paymentAmount),
+				AmtMsat: int64(paymentAmount) * 1000,
+			},
+			valid: false,
+			expectedErrorMsg: "sat and msat arguments are " +
+				"mutually exclusive",
+		},
+		{
+			name: "Both dest and payment_request provided",
+			backend: &RouterBackend{
+				ShouldSetExpEndorsement: func() bool {
+					return false
+				},
+			},
+			sendReq: &SendPaymentRequest{
+				Amt:            int64(paymentAmount),
+				PaymentRequest: "test",
+				Dest:           destNodeBytes,
+			},
+			valid: false,
+			expectedErrorMsg: "dest and payment_request " +
+				"cannot appear together",
+		},
+		{
+			name: "Both payment_hash and payment_request provided",
+			backend: &RouterBackend{
+				ShouldSetExpEndorsement: func() bool {
+					return false
+				},
+			},
+			sendReq: &SendPaymentRequest{
+				Amt:            int64(paymentAmount),
+				PaymentRequest: "test",
+				PaymentHash:    make([]byte, 32),
+			},
+			valid: false,
+			expectedErrorMsg: "payment_hash and payment_request " +
+				"cannot appear together",
+		},
+		{
+			name: "Both final_cltv_delta and payment_request " +
+				"provided",
+			backend: &RouterBackend{
+				ShouldSetExpEndorsement: func() bool {
+					return false
+				},
+			},
+			sendReq: &SendPaymentRequest{
+				Amt:            int64(paymentAmount),
+				PaymentRequest: "test",
+				FinalCltvDelta: 100,
+			},
+			valid: false,
+			expectedErrorMsg: "final_cltv_delta and " +
+				"payment_request cannot appear together",
+		},
+		{
+			name: "Invalid payment request length",
+			backend: &RouterBackend{
+				ShouldSetExpEndorsement: func() bool {
+					return false
+				},
+				ActiveNetParams: &chaincfg.RegressionNetParams,
+			},
+			sendReq: &SendPaymentRequest{
+				Amt:            int64(paymentAmount),
+				PaymentRequest: "test",
+			},
+			valid:            false,
+			expectedErrorMsg: "invalid bech32 string length",
+		},
+		{
+			name: "Expired invoice payment request",
+			backend: &RouterBackend{
+				ShouldSetExpEndorsement: func() bool {
+					return false
+				},
+				ActiveNetParams: &chaincfg.RegressionNetParams,
+			},
+			sendReq: &SendPaymentRequest{
+				Amt:            int64(paymentAmount),
+				PaymentRequest: paymentReq,
+			},
+			valid:            false,
+			expectedErrorMsg: "invoice expired.",
+		},
+		{
+			name: "Invalid dest vertex length",
+			backend: &RouterBackend{
+				ShouldSetExpEndorsement: func() bool {
+					return false
+				},
+			},
+			sendReq: &SendPaymentRequest{
+				Amt:  int64(paymentAmount),
+				Dest: []byte{1},
+			},
+			valid:            false,
+			expectedErrorMsg: "invalid vertex length",
+		},
+		{
+			name: "Payment request with missing amount",
+			backend: &RouterBackend{
+				ShouldSetExpEndorsement: func() bool {
+					return false
+				},
+			},
+			sendReq: &SendPaymentRequest{
+				Dest:           destNodeBytes,
+				FinalCltvDelta: 100,
+			},
+			valid:            false,
+			expectedErrorMsg: "amount must be specified",
+		},
+		{
+			name: "Destination lacks AMP support",
+			backend: &RouterBackend{
+				ShouldSetExpEndorsement: func() bool {
+					return false
+				},
+			},
+			sendReq: &SendPaymentRequest{
+				Dest:         destNodeBytes,
+				Amt:          int64(paymentAmount),
+				Amp:          true,
+				DestFeatures: []lnrpc.FeatureBit{},
+			},
+			valid: false,
+			expectedErrorMsg: "destination doesn't " +
+				"support AMP payments",
+		},
+		{
+			name: "Invalid payment hash length",
+			backend: &RouterBackend{
+				ShouldSetExpEndorsement: func() bool {
+					return false
+				},
+			},
+			sendReq: &SendPaymentRequest{
+				Dest:        destNodeBytes,
+				Amt:         int64(paymentAmount),
+				PaymentHash: make([]byte, 1),
+			},
+			valid:            false,
+			expectedErrorMsg: "invalid hash length",
+		},
+		{
+			name: "Payment amount exceeds maximum possible amount",
+			backend: &RouterBackend{
+				ShouldSetExpEndorsement: func() bool {
+					return false
+				},
+			},
+			sendReq: &SendPaymentRequest{
+				Dest:             destNodeBytes,
+				Amt:              int64(paymentAmount),
+				PaymentHash:      make([]byte, 32),
+				MaxParts:         10,
+				MaxShardSizeMsat: 300_000,
+			},
+			valid: false,
+			expectedErrorMsg: "payment amount 300000000 mSAT " +
+				"exceeds maximum possible amount",
+		},
+		{
+			name: "Reject self-payments if not permitted",
+			backend: &RouterBackend{
+				MaxTotalTimelock: 1000,
+				ShouldSetExpEndorsement: func() bool {
+					return false
+				},
+				SelfNode: target,
+			},
+			sendReq: &SendPaymentRequest{
+				Dest:        destNodeBytes,
+				Amt:         int64(paymentAmount),
+				PaymentHash: make([]byte, 32),
+			},
+			valid:            false,
+			expectedErrorMsg: "self-payments not allowed",
+		},
+		{
+			name: "Valid send req parameters, payment settled",
+			backend: &RouterBackend{
+				MaxTotalTimelock: 1000,
+				ShouldSetExpEndorsement: func() bool {
+					return false
+				},
+			},
+			sendReq: &SendPaymentRequest{
+				Dest:             destNodeBytes,
+				Amt:              int64(paymentAmount),
+				PaymentHash:      make([]byte, 32),
+				MaxParts:         10,
+				MaxShardSizeMsat: 30_000_000,
+			},
+			valid: true,
+		},
+	}
+
+	for _, test := range testCases {
+		t.Run(test.name, func(t *testing.T) {
+			t.Parallel()
+
+			_, err := test.backend.
+				extractIntentFromSendRequest(test.sendReq)
+
+			if test.valid {
+				require.NoError(t, err)
+			} else {
+				require.ErrorContains(t, err,
+					test.expectedErrorMsg)
+			}
+		})
+	}
+}