diff --git a/itest/list_on_test.go b/itest/list_on_test.go index 3dc3ac91de..65cbe9fa33 100644 --- a/itest/list_on_test.go +++ b/itest/list_on_test.go @@ -383,6 +383,10 @@ var allTestCases = []*lntest.TestCase{ Name: "single hop invoice", TestFunc: testSingleHopInvoice, }, + { + Name: "send payment overpay", + TestFunc: testSendPaymentOverpay, + }, { Name: "wipe forwarding packages", TestFunc: testWipeForwardingPackages, diff --git a/itest/lnd_send_overpayment_test.go b/itest/lnd_send_overpayment_test.go new file mode 100644 index 0000000000..9ae920903e --- /dev/null +++ b/itest/lnd_send_overpayment_test.go @@ -0,0 +1,59 @@ +package itest + +import ( + "github.com/btcsuite/btcd/btcutil" + "github.com/lightningnetwork/lnd/lnrpc" + "github.com/lightningnetwork/lnd/lnrpc/routerrpc" + "github.com/lightningnetwork/lnd/lntest" + "github.com/stretchr/testify/require" +) + +// testSendPaymentOverpay verifies that a payer can intentionally overpay a +// fixed-amount BOLT11 invoice by specifying a larger amt_msat in the send +// request. +func testSendPaymentOverpay(ht *lntest.HarnessTest) { + // Open a channel with 100k satoshis between Alice and Bob. + chanAmt := btcutil.Amount(100_000) + _, nodes := ht.CreateSimpleNetwork( + [][]string{nil, nil}, + lntest.OpenChannelParams{Amt: chanAmt}, + ) + alice, bob := nodes[0], nodes[1] + + const invoiceAmtSat = 1_000 + const overpayAmtSat = 1_100 + const overpayAmtMsat = overpayAmtSat * 1000 + + // Create a fixed-amount invoice on Bob's side. + invoice := bob.RPC.AddInvoice(&lnrpc.Invoice{ + Memo: "overpay-test", + Value: invoiceAmtSat, + }) + + // Alice sends a payment with amt_msat larger than the invoice + // amount. This should succeed and deliver the overpaid amount. + payment := ht.SendPaymentAssertSettled( + alice, &routerrpc.SendPaymentRequest{ + PaymentRequest: invoice.PaymentRequest, + AmtMsat: overpayAmtMsat, + TimeoutSeconds: 60, + FeeLimitMsat: noFeeLimitMsat, + }, + ) + + // The payment value should reflect the overpaid amount. + require.Equal( + ht, int64(overpayAmtMsat), payment.ValueMsat, + "value_msat should equal the overpaid amount", + ) + + // The invoice on Bob's side should show the overpaid amount. + dbInvoice := bob.RPC.LookupInvoice(invoice.RHash) + require.Equal( + ht, lnrpc.Invoice_SETTLED, dbInvoice.State, + ) + require.Equal( + ht, int64(overpayAmtMsat), dbInvoice.AmtPaidMsat, + "Bob should receive the overpaid amount", + ) +} diff --git a/lnrpc/marshall_utils.go b/lnrpc/marshall_utils.go index 05a9e9a902..787376c5f6 100644 --- a/lnrpc/marshall_utils.go +++ b/lnrpc/marshall_utils.go @@ -71,6 +71,46 @@ func UnmarshallAmt(amtSat, amtMsat int64) (lnwire.MilliSatoshi, error) { return lnwire.MilliSatoshi(amtMsat), nil } +// ValidatePayReqAmt validates the amount for a payment that includes a BOLT11 +// payment request. If the invoice includes a fixed amount, the caller may +// optionally specify a larger amount to overpay, but underpayment is rejected. +// For zero-amount invoices, the caller must specify an amount. The returned +// value is the amount that should be used for the payment. +func ValidatePayReqAmt(invoiceMsat *lnwire.MilliSatoshi, + amtSat, amtMsat int64) (lnwire.MilliSatoshi, error) { + + reqAmt, err := UnmarshallAmt(amtSat, amtMsat) + if err != nil { + return 0, err + } + + if invoiceMsat == nil { + // For zero-amount invoices, the caller must specify an amount. + if reqAmt == 0 { + return 0, errors.New("amount must be specified " + + "when paying a zero amount invoice") + } + + return reqAmt, nil + } + + // The invoice has a fixed amount. If the caller also specified an + // amount, allow it as long as it is at least the invoice amount + // (overpayment is permitted by the spec). + if reqAmt != 0 { + if reqAmt < *invoiceMsat { + return 0, fmt.Errorf("payment amount (%v) must "+ + "not be less than invoice amount (%v)", + reqAmt, *invoiceMsat, + ) + } + + return reqAmt, nil + } + + return *invoiceMsat, nil +} + // ParseConfs validates the minimum and maximum confirmation arguments of a // ListUnspent request. func ParseConfs(min, max int32) (int32, int32, error) { diff --git a/lnrpc/marshall_utils_test.go b/lnrpc/marshall_utils_test.go new file mode 100644 index 0000000000..c92b332df4 --- /dev/null +++ b/lnrpc/marshall_utils_test.go @@ -0,0 +1,97 @@ +package lnrpc + +import ( + "testing" + + "github.com/lightningnetwork/lnd/lnwire" + "github.com/stretchr/testify/require" +) + +// TestValidatePayReqAmt tests the ValidatePayReqAmt helper for various +// combinations of invoice amount and caller-specified amount. +func TestValidatePayReqAmt(t *testing.T) { + t.Parallel() + + invoiceAmt := lnwire.MilliSatoshi(1_000_000) + + tests := []struct { + name string + invoiceMsat *lnwire.MilliSatoshi + amtSat int64 + amtMsat int64 + wantAmt lnwire.MilliSatoshi + wantErr string + }{ + { + name: "zero-amount invoice with amt_msat", + invoiceMsat: nil, + amtMsat: 500_000, + wantAmt: 500_000, + }, + { + name: "zero-amount invoice with amt_sat", + invoiceMsat: nil, + amtSat: 500, + wantAmt: 500_000, + }, + { + name: "zero-amount invoice with no amount", + invoiceMsat: nil, + wantErr: "amount must be specified", + }, + { + name: "fixed invoice, no caller amount", + invoiceMsat: &invoiceAmt, + wantAmt: 1_000_000, + }, + { + name: "fixed invoice, exact amount", + invoiceMsat: &invoiceAmt, + amtMsat: 1_000_000, + wantAmt: 1_000_000, + }, + { + name: "fixed invoice, overpayment via msat", + invoiceMsat: &invoiceAmt, + amtMsat: 1_100_000, + wantAmt: 1_100_000, + }, + { + name: "fixed invoice, overpayment via sat", + invoiceMsat: &invoiceAmt, + amtSat: 1100, + wantAmt: 1_100_000, + }, + { + name: "fixed invoice, underpayment rejected", + invoiceMsat: &invoiceAmt, + amtMsat: 999_999, + wantErr: "must not be less than invoice amount", + }, + { + name: "sat and msat mutually exclusive", + invoiceMsat: &invoiceAmt, + amtSat: 1, + amtMsat: 1000, + wantErr: "mutually exclusive", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + got, err := ValidatePayReqAmt( + tc.invoiceMsat, tc.amtSat, tc.amtMsat, + ) + + if tc.wantErr != "" { + require.ErrorContains(t, err, tc.wantErr) + return + } + + require.NoError(t, err) + require.Equal(t, tc.wantAmt, got) + }) + } +} diff --git a/lnrpc/routerrpc/router_backend.go b/lnrpc/routerrpc/router_backend.go index d8c8a17c41..e417f12a0b 100644 --- a/lnrpc/routerrpc/router_backend.go +++ b/lnrpc/routerrpc/router_backend.go @@ -1025,27 +1025,15 @@ func (r *RouterBackend) extractIntentFromSendRequest( "either a payment address or blinded paths") } - // If the amount was not included in the invoice, then we let - // the payer specify the amount of satoshis they wish to send. - // We override the amount to pay with the amount provided from - // the payment request. - if payReq.MilliSat == nil { - if reqAmt == 0 { - return nil, errors.New("amount must be " + - "specified when paying a zero amount " + - "invoice") - } - - payIntent.Amount = reqAmt - } else { - if reqAmt != 0 { - return nil, errors.New("amount must not be " + - "specified when paying a non-zero " + - "amount invoice") - } - - payIntent.Amount = *payReq.MilliSat + // Validate and resolve the payment amount against the + // invoice. Overpayment is allowed, underpayment is not. + payAmt, err := lnrpc.ValidatePayReqAmt( + payReq.MilliSat, rpcPayReq.Amt, rpcPayReq.AmtMsat, + ) + if err != nil { + return nil, err } + payIntent.Amount = payAmt if !payReq.Features.HasFeature(lnwire.MPPOptional) && !payReq.Features.HasFeature(lnwire.AMPOptional) { diff --git a/rpcserver.go b/rpcserver.go index e5d59a3171..d239865248 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -5768,27 +5768,15 @@ func (r *rpcServer) extractPaymentIntent(rpcPayReq *rpcPaymentRequest) (rpcPayme return payIntent, err } - // If the amount was not included in the invoice, then we let - // the payer specify the amount of satoshis they wish to send. - // We override the amount to pay with the amount provided from - // the payment request. - if payReq.MilliSat == nil { - amt, err := lnrpc.UnmarshallAmt( - rpcPayReq.Amt, rpcPayReq.AmtMsat, - ) - if err != nil { - return payIntent, err - } - if amt == 0 { - return payIntent, errors.New("amount must be " + - "specified when paying a zero amount " + - "invoice") - } - - payIntent.msat = amt - } else { - payIntent.msat = *payReq.MilliSat + // Validate and resolve the payment amount against the + // invoice. Overpayment is allowed, underpayment is not. + amt, err := lnrpc.ValidatePayReqAmt( + payReq.MilliSat, rpcPayReq.Amt, rpcPayReq.AmtMsat, + ) + if err != nil { + return payIntent, err } + payIntent.msat = amt // Calculate the fee limit that should be used for this payment. payIntent.feeLimit = lnrpc.CalculateFeeLimit(