[2/3] lnwire: fix encoding customized TLV records

docs/release-notes/release-notes-0.20.0.md

@@ -33,11 +33,15 @@
   https://github.com/lightningnetwork/lnd/pull/9993) are returned when receiving
   empty route hints or a non-UTF-8-encoded description.
 
-- [Fixed](https://github.com/lightningnetwork/lnd/pull/10027) an issue where
+- [Fixed](https://github.com/lightningnetwork/lnd/pull/10140) an issue where
   known TLV fields were incorrectly encoded into the `ExtraData` field of
   messages in the dynamic commitment set.
 
 
+- [Fixed](https://github.com/lightningnetwork/lnd/pull/10072) an issue where
+  known TLV fields were incorrectly encoded into the `ExtraData` field of
+  messages in the gossip set.
+
 # New Features
 
 - Added [NoOp HTLCs](https://github.com/lightningnetwork/lnd/pull/9871). This

lnwire/accept_channel.go

@@ -89,6 +89,8 @@ type AcceptChannel struct {
 	// within the commitment transaction of the sender.
 	FirstCommitmentPoint *btcec.PublicKey
 
+	// NOTE: The following fields are TLV records.
+	//
 	// UpfrontShutdownScript is the script to which the channel funds should
 	// be paid when mutually closing the channel. This field is optional, and
 	// and has a length prefix, so a zero will be written if it is not set
@@ -138,17 +140,27 @@ var _ SizeableMessage = (*AcceptChannel)(nil)
 //
 // This is part of the lnwire.Message interface.
 func (a *AcceptChannel) Encode(w *bytes.Buffer, pver uint32) error {
-	recordProducers := []tlv.RecordProducer{&a.UpfrontShutdownScript}
+	// Get producers from extra data.
+	producers, err := a.ExtraData.RecordProducers()
+	if err != nil {
+		return err
+	}
+
+	// Append known producers.
+	producers = append(producers, &a.UpfrontShutdownScript)
 	if a.ChannelType != nil {
-		recordProducers = append(recordProducers, a.ChannelType)
+		producers = append(producers, a.ChannelType)
 	}
 	if a.LeaseExpiry != nil {
-		recordProducers = append(recordProducers, a.LeaseExpiry)
+		producers = append(producers, a.LeaseExpiry)
 	}
 	a.LocalNonce.WhenSome(func(localNonce Musig2NonceTLV) {
-		recordProducers = append(recordProducers, &localNonce)
+		producers = append(producers, &localNonce)
 	})
-	err := EncodeMessageExtraData(&a.ExtraData, recordProducers...)
+
+	// Pack all records into a new TLV stream.
+	var tlvData ExtraOpaqueData
+	err = tlvData.PackRecords(producers...)
 	if err != nil {
 		return err
 	}
@@ -209,7 +221,7 @@ func (a *AcceptChannel) Encode(w *bytes.Buffer, pver uint32) error {
 		return err
 	}
 
-	return WriteBytes(w, a.ExtraData)
+	return WriteBytes(w, tlvData)
 }
 
 // Decode deserializes the serialized AcceptChannel stored in the passed
@@ -254,26 +266,26 @@ func (a *AcceptChannel) Decode(r io.Reader, pver uint32) error {
 		leaseExpiry LeaseExpiry
 		localNonce  = a.LocalNonce.Zero()
 	)
-	typeMap, err := tlvRecords.ExtractRecords(
-		&a.UpfrontShutdownScript, &chanType, &leaseExpiry,
+	knownRecords, extraData, err := ParseAndExtractExtraData(
+		tlvRecords, &a.UpfrontShutdownScript, &chanType, &leaseExpiry,
 		&localNonce,
 	)
 	if err != nil {
 		return err
 	}
 
 	// Set the corresponding TLV types if they were included in the stream.
-	if val, ok := typeMap[ChannelTypeRecordType]; ok && val == nil {
+	if _, ok := knownRecords[ChannelTypeRecordType]; ok {
 		a.ChannelType = &chanType
 	}
-	if val, ok := typeMap[LeaseExpiryRecordType]; ok && val == nil {
+	if _, ok := knownRecords[LeaseExpiryRecordType]; ok {
 		a.LeaseExpiry = &leaseExpiry
 	}
-	if val, ok := typeMap[a.LocalNonce.TlvType()]; ok && val == nil {
+	if _, ok := knownRecords[a.LocalNonce.TlvType()]; ok {
 		a.LocalNonce = tlv.SomeRecordT(localNonce)
 	}
 
-	a.ExtraData = tlvRecords
+	a.ExtraData = extraData
 
 	return nil
 }

lnwire/accept_channel_test.go

@@ -5,6 +5,7 @@ import (
 	"testing"
 
 	"github.com/btcsuite/btcd/btcec/v2"
+	"github.com/stretchr/testify/require"
 )
 
 // TestDecodeAcceptChannel tests decoding of an accept channel wire message with
@@ -70,3 +71,119 @@ func TestDecodeAcceptChannel(t *testing.T) {
 		})
 	}
 }
+
+// TestAcceptChannelEncodeDecode tests that a raw byte stream can be
+// decoded, then re-encoded to the same exact byte stream.
+func TestAcceptChannelEncodeDecode(t *testing.T) {
+	t.Parallel()
+
+	// Create a new private key and its corresponding public key.
+	priv, err := btcec.NewPrivateKey()
+	require.NoError(t, err)
+	pk := priv.PubKey()
+
+	// Create a sample AcceptChannel message with all fields populated.
+	// The exact values are not important, only that they are of the
+	// correct size.
+	var rawBytes []byte
+
+	// PendingChannelID
+	rawBytes = append(rawBytes, make([]byte, 32)...)
+
+	// DustLimit
+	rawBytes = append(rawBytes, []byte{0, 0, 0, 0, 0, 0, 0, 1}...)
+
+	// MaxValueInFlight
+	rawBytes = append(rawBytes, []byte{0, 0, 0, 0, 0, 0, 0, 2}...)
+
+	// ChannelReserve
+	rawBytes = append(rawBytes, []byte{0, 0, 0, 0, 0, 0, 0, 3}...)
+
+	// HtlcMinimum
+	rawBytes = append(rawBytes, []byte{0, 0, 0, 0, 0, 0, 0, 4}...)
+
+	// MinAcceptDepth
+	rawBytes = append(rawBytes, []byte{0, 0, 0, 5}...)
+
+	// CsvDelay
+	rawBytes = append(rawBytes, []byte{0, 6}...)
+
+	// MaxAcceptedHTLCs
+	rawBytes = append(rawBytes, []byte{0, 7}...)
+
+	// FundingKey
+	rawBytes = append(rawBytes, pk.SerializeCompressed()...)
+
+	// RevocationPoint
+	rawBytes = append(rawBytes, pk.SerializeCompressed()...)
+
+	// PaymentPoint
+	rawBytes = append(rawBytes, pk.SerializeCompressed()...)
+
+	// DelayedPaymentPoint
+	rawBytes = append(rawBytes, pk.SerializeCompressed()...)
+
+	// HtlcPoint
+	rawBytes = append(rawBytes, pk.SerializeCompressed()...)
+
+	// FirstCommitmentPoint
+	rawBytes = append(rawBytes, pk.SerializeCompressed()...)
+
+	// Add TLV data, including known and unknown records.
+	tlvData := []byte{
+		// UpfrontShutdownScript (known, type 0)
+		0,          // type
+		2,          // length
+		0xaa, 0xbb, // value
+
+		// ChannelType (known, type 1)
+		1,    // type
+		1,    // length
+		0x02, // value (feature bit 1 set)
+
+		// Unknown odd-type TLV record.
+		0x3,        // type
+		0x2,        // length
+		0xab, 0xcd, // value
+
+		// LocalNonce (known, type 4)
+		4,  // type
+		66, // length
+		// 66 bytes of dummy data
+		0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11,
+		0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11,
+		0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11,
+		0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11,
+		0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11,
+		0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11,
+		0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11,
+		0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11,
+		0x11, 0x11,
+
+		// Another unknown odd-type TLV record.
+		0x6f,       // type
+		0x2,        // length
+		0x79, 0x79, // value
+
+		// LeaseExpiry (known, type 65536)
+		0xfe, 0x00, 0x01, 0x00, 0x00, // type
+		4,                      // length
+		0x12, 0x34, 0x56, 0x78, // value
+	}
+	rawBytes = append(rawBytes, tlvData...)
+
+	// Now, create a new empty message and decode the raw bytes into it.
+	msg := &AcceptChannel{}
+	r := bytes.NewReader(rawBytes)
+	err = msg.Decode(r, 0)
+	require.NoError(t, err)
+
+	// Next, encode the message back into a new byte buffer.
+	var b bytes.Buffer
+	err = msg.Encode(&b, 0)
+	require.NoError(t, err)
+
+	// The re-encoded bytes should be exactly the same as the original raw
+	// bytes.
+	require.Equal(t, rawBytes, b.Bytes())
+}

lnwire/channel_ready.go

@@ -22,6 +22,8 @@ type ChannelReady struct {
 	// next commitment transaction for the channel.
 	NextPerCommitmentPoint *btcec.PublicKey
 
+	// NOTE: The following fields are TLV records.
+	//
 	// AliasScid is an alias ShortChannelID used to refer to the underlying
 	// channel. It can be used instead of the confirmed on-chain
 	// ShortChannelID for forwarding.
@@ -95,33 +97,29 @@ func (c *ChannelReady) Decode(r io.Reader, _ uint32) error {
 		nodeNonce  = tlv.ZeroRecordT[tlv.TlvType0, Musig2Nonce]()
 		btcNonce   = tlv.ZeroRecordT[tlv.TlvType2, Musig2Nonce]()
 	)
-	typeMap, err := tlvRecords.ExtractRecords(
-		&btcNonce, &aliasScid, &nodeNonce, &localNonce,
+	knownRecords, extraData, err := ParseAndExtractExtraData(
+		tlvRecords, &btcNonce, &aliasScid, &nodeNonce, &localNonce,
 	)
 	if err != nil {
 		return err
 	}
 
 	// We'll only set AliasScid if the corresponding TLV type was included
 	// in the stream.
-	if val, ok := typeMap[AliasScidRecordType]; ok && val == nil {
+	if _, ok := knownRecords[AliasScidRecordType]; ok {
 		c.AliasScid = &aliasScid
 	}
-	if val, ok := typeMap[c.NextLocalNonce.TlvType()]; ok && val == nil {
+	if _, ok := knownRecords[c.NextLocalNonce.TlvType()]; ok {
 		c.NextLocalNonce = tlv.SomeRecordT(localNonce)
 	}
-	val, ok := typeMap[c.AnnouncementBitcoinNonce.TlvType()]
-	if ok && val == nil {
+	if _, ok := knownRecords[c.AnnouncementBitcoinNonce.TlvType()]; ok {
 		c.AnnouncementBitcoinNonce = tlv.SomeRecordT(btcNonce)
 	}
-	val, ok = typeMap[c.AnnouncementNodeNonce.TlvType()]
-	if ok && val == nil {
+	if _, ok := knownRecords[c.AnnouncementNodeNonce.TlvType()]; ok {
 		c.AnnouncementNodeNonce = tlv.SomeRecordT(nodeNonce)
 	}
 
-	if len(tlvRecords) != 0 {
-		c.ExtraData = tlvRecords
-	}
+	c.ExtraData = extraData
 
 	return nil
 }
@@ -140,31 +138,38 @@ func (c *ChannelReady) Encode(w *bytes.Buffer, _ uint32) error {
 		return err
 	}
 
+	// Get producers from extra data.
+	producers, err := c.ExtraData.RecordProducers()
+	if err != nil {
+		return err
+	}
+
 	// We'll only encode the AliasScid in a TLV segment if it exists.
-	recordProducers := make([]tlv.RecordProducer, 0, 4)
 	if c.AliasScid != nil {
-		recordProducers = append(recordProducers, c.AliasScid)
+		producers = append(producers, c.AliasScid)
 	}
 	c.NextLocalNonce.WhenSome(func(localNonce Musig2NonceTLV) {
-		recordProducers = append(recordProducers, &localNonce)
+		producers = append(producers, &localNonce)
 	})
 	c.AnnouncementBitcoinNonce.WhenSome(
 		func(nonce tlv.RecordT[tlv.TlvType2, Musig2Nonce]) {
-			recordProducers = append(recordProducers, &nonce)
+			producers = append(producers, &nonce)
 		},
 	)
 	c.AnnouncementNodeNonce.WhenSome(
 		func(nonce tlv.RecordT[tlv.TlvType0, Musig2Nonce]) {
-			recordProducers = append(recordProducers, &nonce)
+			producers = append(producers, &nonce)
 		},
 	)
 
-	err := EncodeMessageExtraData(&c.ExtraData, recordProducers...)
+	// Pack all records into a new TLV stream.
+	var tlvData ExtraOpaqueData
+	err = tlvData.PackRecords(producers...)
 	if err != nil {
 		return err
 	}
 
-	return WriteBytes(w, c.ExtraData)
+	return WriteBytes(w, tlvData)
 }
 
 // MsgType returns the uint32 code which uniquely identifies this message as a