Skip to content

Commit cf7a663

Browse files
NishantBansal2003NishantBansal2003
committed
routerrpc: add validation to MPP params
Adds 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. Signed-off-by: Nishant Bansal <[email protected]>
1 parent c4a77d1 commit cf7a663

File tree

2 files changed

+359
-0
lines changed

2 files changed

+359
-0
lines changed

‎lnrpc/routerrpc/router_backend.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@ const (
3636
// TODO(roasbeef): make this value dynamic based on expected number of
3737
// attempts for given amount.
3838
DefaultMaxParts = 16
39+
40+
// MaxPartsUpperLimit defines the maximum allowable number of splits
41+
// for MPP/AMP when the user is attempting to send a payment.
42+
MaxPartsUpperLimit = 1000
3943
)
4044

4145
// RouterBackend contains the backend implementation of the router rpc sub
@@ -868,6 +872,16 @@ func (r *RouterBackend) extractIntentFromSendRequest(
868872
if rpcPayReq.MaxShardSizeMsat > 0 {
869873
shardAmtMsat := lnwire.MilliSatoshi(rpcPayReq.MaxShardSizeMsat)
870874
payIntent.MaxShardAmt = &shardAmtMsat
875+
876+
// If the requested max_parts exceeds the allowed limit, then we
877+
// cannot send the payment amount.
878+
if payIntent.MaxParts > MaxPartsUpperLimit {
879+
return nil, fmt.Errorf("requested max_parts (%v) "+
880+
"exceeds the allowed upper limit of %v; cannot"+
881+
" send payment amount with max_shard_size_msat"+
882+
"=%v", payIntent.MaxParts, MaxPartsUpperLimit,
883+
*payIntent.MaxShardAmt)
884+
}
871885
}
872886

873887
// Take fee limit from request.
@@ -1191,6 +1205,22 @@ func (r *RouterBackend) extractIntentFromSendRequest(
11911205
payIntent.DestFeatures = features
11921206
}
11931207

1208+
// Validate that the MPP parameters are compatible with the
1209+
// payment amount. In other words, the parameters are invalid if
1210+
// they do not permit sending the full payment amount.
1211+
if payIntent.MaxShardAmt != nil {
1212+
maxPossibleAmount := (*payIntent.MaxShardAmt) *
1213+
lnwire.MilliSatoshi(payIntent.MaxParts)
1214+
if payIntent.Amount > maxPossibleAmount {
1215+
return nil, fmt.Errorf("payment amount %v exceeds "+
1216+
"maximum possible amount %v with max_parts=%v "+
1217+
"and max_shard_size_msat=%v", payIntent.Amount,
1218+
maxPossibleAmount, payIntent.MaxParts,
1219+
*payIntent.MaxShardAmt,
1220+
)
1221+
}
1222+
}
1223+
11941224
// Do bounds checking with the block padding so the router isn't
11951225
// left with a zombie payment in case the user messes up.
11961226
err = routing.ValidateCLTVLimit(

‎lnrpc/routerrpc/router_backend_test.go

Lines changed: 329 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"testing"
88

99
"github.com/btcsuite/btcd/btcutil"
10+
"github.com/btcsuite/btcd/chaincfg"
1011
"github.com/lightningnetwork/lnd/lnrpc"
1112
"github.com/lightningnetwork/lnd/lnwire"
1213
"github.com/lightningnetwork/lnd/routing"
@@ -475,3 +476,331 @@ func testUnmarshalAMP(t *testing.T, test unmarshalAMPTest) {
475476
t.Fatalf("test case has non-standard outcome")
476477
}
477478
}
479+
480+
type extractIntentTestCase struct {
481+
name string
482+
backend *RouterBackend
483+
sendReq *SendPaymentRequest
484+
valid bool
485+
expectedErrorMsg string
486+
}
487+
488+
// TestExtractIntentFromSendRequest verifies that extractIntentFromSendRequest
489+
// correctly translates a SendPaymentRequest from an RPC client into a
490+
// LightningPayment intent.
491+
func TestExtractIntentFromSendRequest(t *testing.T) {
492+
const paymentAmount = btcutil.Amount(300_000)
493+
494+
const paymentReq = "lnbcrt500u1pnh0xflpp56w08q26t896vg2e9mtdkrem320tp" +
495+
"wws9z9sfr7dw86dx97d90u4sdqqcqzzsxqyz5vqsp5z9945kvfy5g9afmakz" +
496+
"yrur2t4hhn2tr87un8j0r0e6l5m5zm0fus9qxpqysgqk98c6j7qefdpdmzt4" +
497+
"g6aykds4ydvf2x9lpngqcfux3hv8qlraan9v3s9296r5w5eh959yzadgh5ck" +
498+
"gjydgyfxdpumxtuk3p3caugmlqpz5necs"
499+
500+
destNodeBytes, err := hex.DecodeString(destKey)
501+
require.NoError(t, err)
502+
503+
target, err := route.NewVertexFromBytes(destNodeBytes)
504+
require.NoError(t, err)
505+
506+
testCases := []extractIntentTestCase{
507+
{
508+
name: "Time preference out of range",
509+
backend: &RouterBackend{},
510+
sendReq: &SendPaymentRequest{
511+
TimePref: 2,
512+
},
513+
valid: false,
514+
expectedErrorMsg: "time preference out of range",
515+
},
516+
{
517+
name: "Outgoing channel exclusivity violation",
518+
backend: &RouterBackend{},
519+
sendReq: &SendPaymentRequest{
520+
OutgoingChanId: 38484,
521+
OutgoingChanIds: []uint64{383322},
522+
},
523+
valid: false,
524+
expectedErrorMsg: "outgoing_chan_id and " +
525+
"outgoing_chan_ids are mutually exclusive",
526+
},
527+
{
528+
name: "Invalid last hop pubkey",
529+
backend: &RouterBackend{},
530+
sendReq: &SendPaymentRequest{
531+
OutgoingChanId: 38484,
532+
LastHopPubkey: []byte{1},
533+
},
534+
valid: false,
535+
expectedErrorMsg: "invalid vertex length",
536+
},
537+
{
538+
name: "Max parts exceed allowed limit",
539+
backend: &RouterBackend{},
540+
sendReq: &SendPaymentRequest{
541+
MaxParts: 1001,
542+
MaxShardSizeMsat: 300_000,
543+
},
544+
valid: false,
545+
expectedErrorMsg: "requested max_parts (1001) exceeds" +
546+
" the allowed upper limit",
547+
},
548+
{
549+
name: "Fee limit conflict, both sat and msat " +
550+
"specified",
551+
backend: &RouterBackend{},
552+
sendReq: &SendPaymentRequest{
553+
FeeLimitSat: 1000000,
554+
FeeLimitMsat: 1000000000,
555+
},
556+
valid: false,
557+
expectedErrorMsg: "sat and msat arguments are " +
558+
"mutually exclusive",
559+
},
560+
{
561+
name: "Dest custom records with type below minimum",
562+
backend: &RouterBackend{},
563+
sendReq: &SendPaymentRequest{
564+
DestCustomRecords: map[uint64][]byte{
565+
65530: {1, 2},
566+
},
567+
},
568+
valid: false,
569+
expectedErrorMsg: "no custom records with types below",
570+
},
571+
{
572+
name: "Custom record entry with TLV type below " +
573+
"minimum",
574+
backend: &RouterBackend{},
575+
sendReq: &SendPaymentRequest{
576+
FirstHopCustomRecords: map[uint64][]byte{
577+
65530: {1, 2},
578+
},
579+
},
580+
valid: false,
581+
expectedErrorMsg: "custom records entry with TLV type",
582+
},
583+
{
584+
name: "Amount conflict, both sat and msat specified",
585+
backend: &RouterBackend{
586+
ShouldSetExpEndorsement: func() bool {
587+
return true
588+
},
589+
},
590+
sendReq: &SendPaymentRequest{
591+
Amt: int64(paymentAmount),
592+
AmtMsat: int64(paymentAmount) * 1000,
593+
},
594+
valid: false,
595+
expectedErrorMsg: "sat and msat arguments are " +
596+
"mutually exclusive",
597+
},
598+
{
599+
name: "Both dest and payment_request provided",
600+
backend: &RouterBackend{
601+
ShouldSetExpEndorsement: func() bool {
602+
return false
603+
},
604+
},
605+
sendReq: &SendPaymentRequest{
606+
Amt: int64(paymentAmount),
607+
PaymentRequest: "test",
608+
Dest: destNodeBytes,
609+
},
610+
valid: false,
611+
expectedErrorMsg: "dest and payment_request " +
612+
"cannot appear together",
613+
},
614+
{
615+
name: "Both payment_hash and payment_request provided",
616+
backend: &RouterBackend{
617+
ShouldSetExpEndorsement: func() bool {
618+
return false
619+
},
620+
},
621+
sendReq: &SendPaymentRequest{
622+
Amt: int64(paymentAmount),
623+
PaymentRequest: "test",
624+
PaymentHash: make([]byte, 32),
625+
},
626+
valid: false,
627+
expectedErrorMsg: "payment_hash and payment_request " +
628+
"cannot appear together",
629+
},
630+
{
631+
name: "Both final_cltv_delta and payment_request " +
632+
"provided",
633+
backend: &RouterBackend{
634+
ShouldSetExpEndorsement: func() bool {
635+
return false
636+
},
637+
},
638+
sendReq: &SendPaymentRequest{
639+
Amt: int64(paymentAmount),
640+
PaymentRequest: "test",
641+
FinalCltvDelta: 100,
642+
},
643+
valid: false,
644+
expectedErrorMsg: "final_cltv_delta and " +
645+
"payment_request cannot appear together",
646+
},
647+
{
648+
name: "Invalid payment request",
649+
backend: &RouterBackend{
650+
ShouldSetExpEndorsement: func() bool {
651+
return false
652+
},
653+
ActiveNetParams: &chaincfg.RegressionNetParams,
654+
},
655+
sendReq: &SendPaymentRequest{
656+
Amt: int64(paymentAmount),
657+
PaymentRequest: "test",
658+
},
659+
valid: false,
660+
expectedErrorMsg: "invalid bech32 string length",
661+
},
662+
{
663+
name: "Expired invoice payment request",
664+
backend: &RouterBackend{
665+
ShouldSetExpEndorsement: func() bool {
666+
return false
667+
},
668+
ActiveNetParams: &chaincfg.RegressionNetParams,
669+
},
670+
sendReq: &SendPaymentRequest{
671+
Amt: int64(paymentAmount),
672+
PaymentRequest: paymentReq,
673+
},
674+
valid: false,
675+
expectedErrorMsg: "invoice expired.",
676+
},
677+
{
678+
name: "Invalid dest vertex length",
679+
backend: &RouterBackend{
680+
ShouldSetExpEndorsement: func() bool {
681+
return false
682+
},
683+
},
684+
sendReq: &SendPaymentRequest{
685+
Amt: int64(paymentAmount),
686+
Dest: []byte{1},
687+
},
688+
valid: false,
689+
expectedErrorMsg: "invalid vertex length",
690+
},
691+
{
692+
name: "Payment request with missing amount",
693+
backend: &RouterBackend{
694+
ShouldSetExpEndorsement: func() bool {
695+
return false
696+
},
697+
},
698+
sendReq: &SendPaymentRequest{
699+
Dest: destNodeBytes,
700+
FinalCltvDelta: 100,
701+
},
702+
valid: false,
703+
expectedErrorMsg: "amount must be specified",
704+
},
705+
{
706+
name: "Destination lacks AMP support",
707+
backend: &RouterBackend{
708+
ShouldSetExpEndorsement: func() bool {
709+
return false
710+
},
711+
},
712+
sendReq: &SendPaymentRequest{
713+
Dest: destNodeBytes,
714+
Amt: int64(paymentAmount),
715+
Amp: true,
716+
DestFeatures: []lnrpc.FeatureBit{},
717+
},
718+
valid: false,
719+
expectedErrorMsg: "destination doesn't " +
720+
"support AMP payments",
721+
},
722+
{
723+
name: "Invalid payment hash",
724+
backend: &RouterBackend{
725+
ShouldSetExpEndorsement: func() bool {
726+
return false
727+
},
728+
},
729+
sendReq: &SendPaymentRequest{
730+
Dest: destNodeBytes,
731+
Amt: int64(paymentAmount),
732+
PaymentHash: make([]byte, 1),
733+
},
734+
valid: false,
735+
expectedErrorMsg: "invalid hash length",
736+
},
737+
{
738+
name: "Payment amount exceeds maximum possible amount",
739+
backend: &RouterBackend{
740+
ShouldSetExpEndorsement: func() bool {
741+
return false
742+
},
743+
},
744+
sendReq: &SendPaymentRequest{
745+
Dest: destNodeBytes,
746+
Amt: int64(paymentAmount),
747+
PaymentHash: make([]byte, 32),
748+
MaxParts: 10,
749+
MaxShardSizeMsat: 300_000,
750+
},
751+
valid: false,
752+
expectedErrorMsg: "payment amount 300000000 mSAT " +
753+
"exceeds maximum possible amount",
754+
},
755+
{
756+
name: "Reject self-payments if not permitted",
757+
backend: &RouterBackend{
758+
MaxTotalTimelock: 1000,
759+
ShouldSetExpEndorsement: func() bool {
760+
return false
761+
},
762+
SelfNode: target,
763+
},
764+
sendReq: &SendPaymentRequest{
765+
Dest: destNodeBytes,
766+
Amt: int64(paymentAmount),
767+
PaymentHash: make([]byte, 32),
768+
},
769+
valid: false,
770+
expectedErrorMsg: "self-payments not allowed",
771+
},
772+
{
773+
name: "Valid send req parameters, payment settled",
774+
backend: &RouterBackend{
775+
MaxTotalTimelock: 1000,
776+
ShouldSetExpEndorsement: func() bool {
777+
return false
778+
},
779+
},
780+
sendReq: &SendPaymentRequest{
781+
Dest: destNodeBytes,
782+
Amt: int64(paymentAmount),
783+
PaymentHash: make([]byte, 32),
784+
MaxParts: 10,
785+
MaxShardSizeMsat: 30_000_000,
786+
},
787+
valid: true,
788+
},
789+
}
790+
791+
for _, test := range testCases {
792+
t.Run(test.name, func(t *testing.T) {
793+
t.Parallel()
794+
795+
_, err := test.backend.
796+
extractIntentFromSendRequest(test.sendReq)
797+
798+
if test.valid {
799+
require.NoError(t, err)
800+
} else {
801+
require.ErrorContains(t, err,
802+
test.expectedErrorMsg)
803+
}
804+
})
805+
}
806+
}

0 commit comments

Comments
 (0)