BOLT11 invoice follows the new spec changes

docs/release-notes/release-notes-0.22.0.md

@@ -119,6 +119,23 @@
   later in the reservation flow as a funder-balance-dust error; they now
   surface a clearer, spec-aligned error string up front.
 
+* [Require an explicit `channel_type` during channel
+  funding](https://github.com/lightningnetwork/lnd/pull/11064). It is now always
+  set in `open_channel` and echoed back in `accept_channel`, and an
+  `open_channel` that omits it is rejected. Implicit commitment type negotiation
+  is removed; if the RPC caller doesn't request a type, a default is derived
+  from both peers' features and signaled explicitly.
+
+* BOLT 11 invoice decoding [now
+  fails](https://github.com/lightningnetwork/lnd/pull/10326) if any field with
+  a fixed length (`p`, `h`, `s`, `n`) does not have the correct length (52, 52,
+  52, 53), instead of skipping the field, and if the invoice has no payment
+  secret (`s`) and no blinded paths, as the BOLT 11 reader requirements
+  demand. The length rule was clarified in
+  [lightning/bolts#1243](https://github.com/lightning/bolts/pull/1243).
+  Stored invoices created by lnd versions before v0.9.0, which have no payment
+  secret, are still returned by the invoice RPCs.
+
 ## BOLT 12 (Offers)
 
 * [Initial BOLT 12 Offer codec](https://github.com/lightningnetwork/lnd/pull/10789):
@@ -201,3 +218,5 @@
 * Jared Tobin
 * Kevin Cai
 * Vandit Singh
+* Nishant Bansal
+* Pins

lnrpc/invoicesrpc/utils.go

@@ -33,8 +33,13 @@ func decodePayReq(invoice *invoices.Invoice,
 		}, nil
 	}
 
-	var err error
-	decoded, err := zpay32.Decode(paymentRequest, activeNetParams)
+	// Invoices created by lnd versions before v0.9.0 have no payment
+	// secret. We skip that check so that they can still be returned by the
+	// RPCs, since the invoice is only converted for display here.
+	decoded, err := zpay32.Decode(
+		paymentRequest, activeNetParams,
+		zpay32.WithSkipPaymentSecretCheck(),
+	)
 	if err != nil {
 		return nil, fmt.Errorf("unable to decode payment "+
 			"request: %v", err)

lnrpc/invoicesrpc/utils_test.go

@@ -0,0 +1,31 @@
+package invoicesrpc
+
+import (
+	"testing"
+
+	"github.com/btcsuite/btcd/chaincfg/v2"
+	"github.com/lightningnetwork/lnd/invoices"
+	"github.com/stretchr/testify/require"
+)
+
+// TestCreateRPCInvoiceWithoutPaymentSecret checks that a stored invoice
+// without a payment secret, as created by lnd versions before v0.9.0, can
+// still be converted to its RPC representation.
+func TestCreateRPCInvoiceWithoutPaymentSecret(t *testing.T) {
+	t.Parallel()
+
+	payReq := "lnbc20m1pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5" +
+		"rqwzqfqqqsyqcyq5rqwzqfqypqdq5xysxxatsyp3k7en" +
+		"xv4jszjlrkes0mgx4ghum45ha5gkzlac8xmrr4skgwey" +
+		"p6xqxucu4wz4j8uvpg0jznsesezax6hdt0gtyn3tuqpf" +
+		"y2curryn83zygkydmpxcqdfu7k0"
+
+	invoice := &invoices.Invoice{
+		PaymentRequest: []byte(payReq),
+		State:          invoices.ContractOpen,
+	}
+
+	rpcInvoice, err := CreateRPCInvoice(invoice, &chaincfg.MainNetParams)
+	require.NoError(t, err)
+	require.Equal(t, payReq, rpcInvoice.PaymentRequest)
+}

lnrpc/routerrpc/router_backend.go

@@ -1038,7 +1038,9 @@ func (r *RouterBackend) extractIntentFromSendRequest(
 		}
 
 		// An invoice must include either a payment address or
-		// blinded paths.
+		// blinded paths. zpay32.Decode already enforces this, but we
+		// keep the check here as a defense in case the decoder is
+		// relaxed.
 		if payReq.PaymentAddr.IsNone() &&
 			len(payReq.BlindedPaymentPaths) == 0 {
 

lnrpc/routerrpc/router_backend_test.go

@@ -831,9 +831,8 @@ func TestExtractIntentFromSendRequest(t *testing.T) {
 			sendReq: &SendPaymentRequest{
 				PaymentRequest: paymentReqMissingAddr,
 			},
-			valid: false,
-			expectedErrorMsg: "payment request must contain " +
-				"either a payment address or blinded paths",
+			valid:            false,
+			expectedErrorMsg: "payment secret not found",
 		},
 		{
 			name: "Invalid dest vertex length",

lnrpc/routerrpc/router_server_test.go

@@ -838,6 +838,7 @@ func TestProbePaymentRequestUsesUniqueHashPerLSP(t *testing.T) {
 		zpay32.RouteHint([]zpay32.HopHint{bobHint}),
 		zpay32.RouteHint([]zpay32.HopHint{eveHint}),
 		zpay32.RouteHint([]zpay32.HopHint{daveHint}),
+		zpay32.PaymentAddr([32]byte{4, 5}),
 	)
 	require.NoError(t, err)
 

zpay32/decode.go

@@ -64,10 +64,23 @@ func WithErrorOnUnknownFeatureBit() DecodeOption {
 	}
 }
 
+// WithSkipPaymentSecretCheck is a functional option that allows the Decode
+// function to accept an invoice that has neither a payment secret nor blinded
+// paths. BOLT 11 requires a reader to fail the payment for such an invoice, so
+// this option must only be used when the invoice is not going to be paid, for
+// example to display invoices that were stored before payment secrets were
+// mandatory.
+func WithSkipPaymentSecretCheck() DecodeOption {
+	return func(options *decodeOptions) {
+		options.skipPaymentSecretCheck = true
+	}
+}
+
 // decodeOptions holds the set of Decode options.
 type decodeOptions struct {
-	knownFeatureBits      map[lnwire.FeatureBit]string
-	errorOnUnknownFeature bool
+	knownFeatureBits       map[lnwire.FeatureBit]string
+	errorOnUnknownFeature  bool
+	skipPaymentSecretCheck bool
 }
 
 // newDecodeOptions constructs the default decodeOptions struct.
@@ -212,6 +225,16 @@ func Decode(invoice string, net *chaincfg.Params, opts ...DecodeOption) (
 		return nil, err
 	}
 
+	// The invoice must include a payment secret, unless it uses blinded
+	// paths, in which case the PathID in the final hop serves the same
+	// purpose.
+	if !options.skipPaymentSecretCheck &&
+		len(decodedInvoice.BlindedPaymentPaths) == 0 &&
+		decodedInvoice.PaymentAddr.IsNone() {
+
+		return nil, ErrPaymentSecretNotFound
+	}
+
 	if options.errorOnUnknownFeature {
 		// Make sure that we understand all the required feature bits
 		// in the invoice.
@@ -265,11 +288,7 @@ func parseTimestamp(data []byte) (uint64, error) {
 // parseTaggedFields takes the base32 encoded tagged fields of the invoice, and
 // fills the Invoice struct accordingly.
 func parseTaggedFields(invoice *Invoice, fields []byte, net *chaincfg.Params) error {
-	var (
-		index           int
-		paymentHashSeen bool
-	)
-
+	index := 0
 	for len(fields)-index > 0 {
 		// If there are less than 3 groups to read, there cannot be more
 		// interesting information, as we need the type (1 group) and
@@ -298,27 +317,30 @@ func parseTaggedFields(invoice *Invoice, fields []byte, net *chaincfg.Params) er
 
 		switch typ {
 		case fieldTypeP:
-			if paymentHashSeen {
+			paymentHash, err := parse32Bytes(base32Data)
+			if err != nil {
+				return fmt.Errorf("payment hash: %w", err)
+			}
+
+			if invoice.PaymentHash != nil {
 				return ErrDuplicatePaymentHash
 			}
-			paymentHashSeen = true
 
-			invoice.PaymentHash, err = parse32Bytes(base32Data)
+			invoice.PaymentHash = paymentHash
 
 		case fieldTypeS:
+			addr, err := parse32Bytes(base32Data)
+			if err != nil {
+				return fmt.Errorf("payment secret: %w", err)
+			}
+
 			if invoice.PaymentAddr.IsSome() {
 				// We skip the field if we have already seen a
 				// supported one.
 				continue
 			}
 
-			addr, err := parse32Bytes(base32Data)
-			if err != nil {
-				return err
-			}
-			if addr != nil {
-				invoice.PaymentAddr = fn.Some(*addr)
-			}
+			invoice.PaymentAddr = fn.Some(*addr)
 
 		case fieldTypeD:
 			if invoice.Description != nil {
@@ -339,22 +361,32 @@ func parseTaggedFields(invoice *Invoice, fields []byte, net *chaincfg.Params) er
 			invoice.Metadata, err = parseMetadata(base32Data)
 
 		case fieldTypeN:
+			destination, err := parseDestination(base32Data)
+			if err != nil {
+				return fmt.Errorf("destination id: %w", err)
+			}
+
 			if invoice.Destination != nil {
 				// We skip the field if we have already seen a
 				// supported one.
 				continue
 			}
 
-			invoice.Destination, err = parseDestination(base32Data)
+			invoice.Destination = destination
 
 		case fieldTypeH:
+			descriptionHash, err := parse32Bytes(base32Data)
+			if err != nil {
+				return fmt.Errorf("description hash: %w", err)
+			}
+
 			if invoice.DescriptionHash != nil {
 				// We skip the field if we have already seen a
 				// supported one.
 				continue
 			}
 
-			invoice.DescriptionHash, err = parse32Bytes(base32Data)
+			invoice.DescriptionHash = descriptionHash
 
 		case fieldTypeX:
 			if invoice.expiry != nil {
@@ -449,16 +481,10 @@ func parseFieldDataLength(data []byte) (uint16, error) {
 func parse32Bytes(data []byte) (*[32]byte, error) {
 	var paymentHash [32]byte
 
-	// A field with an unexpected length is reported as absent rather
-	// than as an error, leaving it to the caller to decide whether a
-	// missing field is fatal. Note that BOLT 11 is stricter, and
-	// requires a reader to fail on a fixed-length field (p, h, s, n)
-	// with the wrong length. For the payment hash the end result is the
-	// same: a lone wrong-length field leaves PaymentHash nil and
-	// validateInvoice rejects the invoice, and a wrong-length field
-	// paired with a valid one is rejected as a duplicate.
+	// As BOLT-11 states, a reader must fail the payment if a 32-byte
+	// field (p, h, s) does not have a length of 52.
 	if len(data) != hashBase32Len {
-		return nil, nil
+		return nil, ErrInvalidFieldLength
 	}
 
 	hash, err := bech32.ConvertBits(data, 5, 8, false)
@@ -497,10 +523,10 @@ func parseMetadata(data []byte) ([]byte, error) {
 // parseDestination converts the data (encoded in base32) into a 33-byte public
 // key of the payee node.
 func parseDestination(data []byte) (*btcec.PublicKey, error) {
-	// As BOLT-11 states, a reader must skip over the destination field
-	// if it does not have a length of 53, so avoid returning an error.
+	// As BOLT-11 states, a reader must fail the payment if the destination
+	// field does not have a length of 53.
 	if len(data) != pubKeyBase32Len {
-		return nil, nil
+		return nil, ErrInvalidFieldLength
 	}
 
 	base256Data, err := bech32.ConvertBits(data, 5, 8, false)

zpay32/invoice.go

@@ -109,6 +109,10 @@ var (
 	ErrDuplicatePaymentHash = errors.New(
 		"invoice contains multiple payment hashes",
 	)
+
+	// ErrPaymentSecretNotFound is returned when a decoded invoice has
+	// neither a payment secret nor blinded paths.
+	ErrPaymentSecretNotFound = errors.New("payment secret not found")
 )
 
 // MessageSigner is passed to the Encode method to provide a signature