bolt12: split off review cleanups and test changes from #11191

bolt12/bech32.go

@@ -266,8 +266,8 @@ func stripContinuation(s string) (string, error) {
 		}
 		if j >= len(s) || !isContinuationNeighbour(s[j]) {
 			return "", fmt.Errorf(
-				"bolt12: %w: '+' must precede a "+
-					"non-whitespace character",
+				"bolt12: %w: '+' must join two "+
+					"characters",
 				ErrInvalidContinuation,
 			)
 		}

bolt12/doc.go

@@ -1,6 +1,7 @@
 // Package bolt12 implements encoding, decoding, and validation for BOLT 12
-// Offers, Invoice Requests, and Invoices. It provides a pure codec library
-// with no LND daemon dependencies.
+// Offers, Invoice Requests, and Invoices. It is a codec library: it does not
+// reach into the daemon, and it takes the chain, the clock and the known
+// feature bits from its caller.
 //
 // BOLT 12 messages use TLV streams encoded with a checksumless bech32 variant
 // and signed with BIP-340 Schnorr signatures over a Merkle tree of TLV fields.

bolt12/invoice.go

@@ -417,7 +417,7 @@ func DecodeInvoiceString(s string, now time.Time,
 		return nil, err
 	}
 
-	features := InvoiceFeatureCatalogues{
+	features := InvoiceKnownFeatures{
 		Invoice: Bolt12Features,
 		Blinded: Bolt12Features,
 	}

bolt12/invoice_error.go

@@ -46,6 +46,11 @@ func (ie *InvoiceError) allRecordProducers() []tlv.RecordProducer {
 // signature to invalidate by dropping unrecognized TLVs (unlike signed
 // messages such as invoices, where unknown TLVs must be preserved to keep
 // signatures valid).
+//
+// One writer rule stays unchecked: a suggested_value is not verified against
+// the type of the field erroneous_field names. Emitting a value the peer
+// cannot decode is therefore possible, see the TODO in
+// ValidateInvoiceErrorWrite.
 func (ie *InvoiceError) Encode() ([]byte, error) {
 	if err := ValidateInvoiceErrorWrite(ie); err != nil {
 		return nil, fmt.Errorf("validate invoice error: %w", err)

bolt12/invoice_error_test.go

@@ -49,15 +49,19 @@ func TestInvoiceErrorRoundTrip(t *testing.T) {
 			name: "all fields",
 			ie: &InvoiceError{
 				ErroneousField: someErrField(82),
+				// Field 82 is invreq_amount, a tu64, so the
+				// suggested value has to be minimal: a
+				// leading zero byte would make it undecodable
+				// for the peer.
 				SuggestedValue: someSuggested(
-					[]byte{0x00, 0x01, 0x86, 0xa0},
+					[]byte{0x01, 0x86, 0xa0},
 				),
 				Error: someError("amount too low"),
 			},
 			wantMsg:      "amount too low",
 			wantHasField: true,
 			wantFieldNum: 82,
-			wantSuggest:  []byte{0x00, 0x01, 0x86, 0xa0},
+			wantSuggest:  []byte{0x01, 0x86, 0xa0},
 		},
 		{
 			name: "minimal error only",

bolt12/invoice_request_test.go

@@ -5,56 +5,149 @@ import (
 	"testing"
 
 	"github.com/btcsuite/btcd/btcec/v2"
+	"github.com/lightningnetwork/lnd/lnwire"
 	"github.com/lightningnetwork/lnd/tlv"
 	"github.com/stretchr/testify/require"
 )
 
-// TestInvoiceRequestRoundTrip pins encode→decode→re-encode for an
-// InvoiceRequest with a representative subset of optional fields.
+// TestInvoiceRequestRoundTrip pins encode, decode and re-encode for an
+// InvoiceRequest with every field set, plus an unknown odd TLV. Comparing the
+// decoded struct against the fixture catches a field that is wired into the
+// encode path but not into the decode path, which byte identity alone cannot
+// see.
 func TestInvoiceRequestRoundTrip(t *testing.T) {
 	t.Parallel()
 
-	_, bobPub := bobKey()
+	priv, bobPub := bobKey()
+	_, intro := aliceKey()
+	_, blinding := bobKey()
+	_, hopPub := aliceKey()
 
-	metadata := tlv.Blob("payer-metadata")
+	introNode, err := lnwire.NewPubkeyIntro(intro)
+	require.NoError(t, err)
+
+	paths := lnwire.BlindedPaths{
+		Paths: []lnwire.BlindedPath{
+			{
+				IntroductionNode: introNode,
+				BlindingPoint:    blinding,
+				Hops: []lnwire.BlindedHop{
+					{
+						BlindedNodeID: hopPub,
+						EncryptedData: []byte{1, 2},
+					},
+				},
+			},
+		},
+	}
+
+	// name_len, name, domain_len, domain.
+	bip353 := append(
+		[]byte{3, 'b', 'o', 'b', 6}, []byte("ex.com")...,
+	)
 
 	ir := &InvoiceRequest{
+		OfferChains: tlv.SomeRecordT(
+			tlv.NewRecordT[tlv.TlvType2](ChainsRecord{
+				Chains: [][32]byte{bitcoinMainnetGenesisHash},
+			}),
+		),
+		OfferMetadata: tlv.SomeRecordT(
+			tlv.NewPrimitiveRecord[tlv.TlvType4](
+				tlv.Blob("opaque"),
+			),
+		),
+		OfferCurrency: tlv.SomeRecordT(
+			tlv.NewPrimitiveRecord[tlv.TlvType6](tlv.Blob("USD")),
+		),
+		OfferAmount: tlv.SomeRecordT(
+			tlv.NewRecordT[tlv.TlvType8](TUint64(1500)),
+		),
 		OfferDescription: tlv.SomeRecordT(
 			tlv.NewPrimitiveRecord[tlv.TlvType10](
-				tlv.Blob("description"),
+				tlv.Blob("coffee"),
 			),
 		),
-		InvreqPayerID: tlv.SomeRecordT(
-			tlv.NewPrimitiveRecord[tlv.TlvType88](bobPub),
+		OfferFeatures: tlv.SomeRecordT(
+			tlv.NewRecordT[tlv.TlvType12](
+				*lnwire.NewRawFeatureVector(lnwire.MPPOptional),
+			),
+		),
+		OfferAbsoluteExpiry: tlv.SomeRecordT(
+			tlv.NewRecordT[tlv.TlvType14](TUint64(1 << 32)),
+		),
+		OfferPaths: tlv.SomeRecordT(
+			tlv.NewRecordT[tlv.TlvType16](paths),
+		),
+		OfferIssuer: tlv.SomeRecordT(
+			tlv.NewPrimitiveRecord[tlv.TlvType18](
+				tlv.Blob("alice"),
+			),
+		),
+		OfferQuantityMax: tlv.SomeRecordT(
+			tlv.NewRecordT[tlv.TlvType20](TUint64(5)),
+		),
+		OfferIssuerID: tlv.SomeRecordT(
+			tlv.NewPrimitiveRecord[tlv.TlvType22](bobPub),
 		),
 		InvreqMetadata: tlv.SomeRecordT(
-			tlv.NewPrimitiveRecord[tlv.TlvType0](metadata),
+			tlv.NewPrimitiveRecord[tlv.TlvType0](
+				tlv.Blob("payer-metadata"),
+			),
+		),
+		InvreqChain: tlv.SomeRecordT(
+			tlv.NewPrimitiveRecord[tlv.TlvType80](
+				bitcoinMainnetGenesisHash,
+			),
 		),
 		InvreqAmount: tlv.SomeRecordT(
-			tlv.NewRecordT[tlv.TlvType82, TUint64](1000),
+			tlv.NewRecordT[tlv.TlvType82](TUint64(3000)),
 		),
-		Signature: tlv.SomeRecordT(
-			tlv.NewPrimitiveRecord[tlv.TlvType240](
-				[64]byte{0x01},
+		InvreqFeatures: tlv.SomeRecordT(
+			tlv.NewRecordT[tlv.TlvType84](
+				*lnwire.NewRawFeatureVector(lnwire.MPPOptional),
 			),
 		),
+		InvreqQuantity: tlv.SomeRecordT(
+			tlv.NewRecordT[tlv.TlvType86](TUint64(2)),
+		),
+		InvreqPayerID: tlv.SomeRecordT(
+			tlv.NewPrimitiveRecord[tlv.TlvType88](bobPub),
+		),
+		InvreqPayerNote: tlv.SomeRecordT(
+			tlv.NewPrimitiveRecord[tlv.TlvType89](tlv.Blob("tip")),
+		),
+		InvreqPaths: tlv.SomeRecordT(
+			tlv.NewRecordT[tlv.TlvType90](paths),
+		),
+		InvreqBip353Name: tlv.SomeRecordT(
+			tlv.NewPrimitiveRecord[tlv.TlvType91](bip353),
+		),
+
+		// An unknown odd type in the signed range must survive the
+		// round trip byte for byte.
+		decodedTLVs: tlv.TypeMap{93: []byte{0xde, 0xad}},
 	}
 
+	sig, err := SignInvoiceRequest(ir, priv)
+	require.NoError(t, err)
+	ir.Signature = tlv.SomeRecordT(
+		tlv.NewPrimitiveRecord[tlv.TlvType240](sig),
+	)
+
 	encoded, err := ir.Encode()
 	require.NoError(t, err)
 	require.NotEmpty(t, encoded)
 
 	decoded, err := DecodeInvoiceRequest(encoded)
 	require.NoError(t, err)
 
-	require.Equal(
-		t, bobPub.SerializeCompressed(),
-		decoded.InvreqPayerID.UnwrapOrFailV(t).SerializeCompressed(),
-	)
-	require.Equal(t, metadata, decoded.InvreqMetadata.UnwrapOrFailV(t))
-	require.Equal(
-		t, TUint64(1000), decoded.InvreqAmount.UnwrapOrFailV(t),
-	)
+	// The sidecar names every type seen on the wire, whereas the fixture
+	// carries only the unknown one. Adopt the decoded view so the
+	// comparison below covers the typed fields.
+	require.Equal(t, []byte{0xde, 0xad}, decoded.decodedTLVs[93])
+	ir.decodedTLVs = decoded.decodedTLVs
+	require.Equal(t, ir, decoded)
 
 	reencoded, err := decoded.Encode()
 	require.NoError(t, err)