Skip to content

Commit 47e9f05

Browse files
RoasbeefRoasbeef
authored
Merge pull request #10027 from yyforyongyu/dyn-bigsize-msg
Fix `ExtraData` field and use `BigSize` encodine
2 parents 012fb79 + 18916da commit 47e9f05

File tree

10 files changed

+502
-155
lines changed

10 files changed

+502
-155
lines changed

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,10 @@
2929
- Fixed [shutdown deadlock](https://github.com/lightningnetwork/lnd/pull/10042)
3030
when we fail starting up LND before we startup the chanbackup sub-server.
3131

32+
- [Fixed](https://github.com/lightningnetwork/lnd/pull/10027) an issue where
33+
known TLV fields were incorrectly encoded into the `ExtraData` field of
34+
messages in the dynamic commitment set.
35+
3236
# New Features
3337

3438
## Functional Enhancements

‎lnwire/dyn_ack.go‎

Lines changed: 23 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,14 @@ import (
44
"bytes"
55
"io"
66

7-
"github.com/btcsuite/btcd/btcec/v2/schnorr/musig2"
8-
"github.com/lightningnetwork/lnd/fn/v2"
97
"github.com/lightningnetwork/lnd/tlv"
108
)
119

1210
const (
1311
// DALocalMusig2Pubnonce is the TLV type number that identifies the
1412
// musig2 public nonce that we need to verify the commitment transaction
1513
// signature.
16-
DALocalMusig2Pubnonce tlv.Type = 0
14+
DALocalMusig2Pubnonce tlv.Type = 14
1715
)
1816

1917
// DynAck is the message used to accept the parameters of a dynamic commitment
@@ -33,7 +31,7 @@ type DynAck struct {
3331
// used to verify the first commitment transaction signature. This will
3432
// only be populated if the DynPropose we are responding to specifies
3533
// taproot channels in the ChannelType field.
36-
LocalNonce fn.Option[Musig2Nonce]
34+
LocalNonce tlv.OptionalRecordT[tlv.TlvType14, Musig2Nonce]
3735

3836
// ExtraData is the set of data that was appended to this message to
3937
// fill out the full maximum transport message size. These fields can
@@ -62,31 +60,27 @@ func (da *DynAck) Encode(w *bytes.Buffer, _ uint32) error {
6260
return err
6361
}
6462

65-
var tlvRecords []tlv.Record
66-
da.LocalNonce.WhenSome(func(nonce Musig2Nonce) {
67-
tlvRecords = append(
68-
tlvRecords, tlv.MakeStaticRecord(
69-
DALocalMusig2Pubnonce, &nonce,
70-
musig2.PubNonceSize, nonceTypeEncoder,
71-
nonceTypeDecoder,
72-
),
73-
)
74-
})
75-
tlv.SortRecords(tlvRecords)
76-
77-
tlvStream, err := tlv.NewStream(tlvRecords...)
63+
// Create extra data records.
64+
producers, err := da.ExtraData.RecordProducers()
7865
if err != nil {
7966
return err
8067
}
8168

82-
var extraBytesWriter bytes.Buffer
83-
if err := tlvStream.Encode(&extraBytesWriter); err != nil {
69+
// Append the known records.
70+
da.LocalNonce.WhenSome(
71+
func(rec tlv.RecordT[tlv.TlvType14, Musig2Nonce]) {
72+
producers = append(producers, &rec)
73+
},
74+
)
75+
76+
// Encode all records.
77+
var tlvData ExtraOpaqueData
78+
err = tlvData.PackRecords(producers...)
79+
if err != nil {
8480
return err
8581
}
8682

87-
da.ExtraData = ExtraOpaqueData(extraBytesWriter.Bytes())
88-
89-
return WriteBytes(w, da.ExtraData)
83+
return WriteBytes(w, tlvData)
9084
}
9185

9286
// Decode deserializes the serialized DynAck stored in the passed io.Reader into
@@ -106,37 +100,22 @@ func (da *DynAck) Decode(r io.Reader, _ uint32) error {
106100
return err
107101
}
108102

109-
// Prepare receiving buffers to be filled by TLV extraction.
110-
var localNonceScratch Musig2Nonce
111-
localNonce := tlv.MakeStaticRecord(
112-
DALocalMusig2Pubnonce, &localNonceScratch, musig2.PubNonceSize,
113-
nonceTypeEncoder, nonceTypeDecoder,
103+
// Parse all known records and extra data.
104+
nonce := da.LocalNonce.Zero()
105+
knownRecords, extraData, err := ParseAndExtractExtraData(
106+
tlvRecords, &nonce,
114107
)
115-
116-
// Create set of Records to read TLV bytestream into.
117-
records := []tlv.Record{localNonce}
118-
tlv.SortRecords(records)
119-
120-
// Read TLV stream into record set.
121-
extraBytesReader := bytes.NewReader(tlvRecords)
122-
tlvStream, err := tlv.NewStream(records...)
123-
if err != nil {
124-
return err
125-
}
126-
typeMap, err := tlvStream.DecodeWithParsedTypesP2P(extraBytesReader)
127108
if err != nil {
128109
return err
129110
}
130111

131112
// Check the results of the TLV Stream decoding and appropriately set
132113
// message fields.
133-
if val, ok := typeMap[DALocalMusig2Pubnonce]; ok && val == nil {
134-
da.LocalNonce = fn.Some(localNonceScratch)
114+
if _, ok := knownRecords[da.LocalNonce.TlvType()]; ok {
115+
da.LocalNonce = tlv.SomeRecordT(nonce)
135116
}
136117

137-
if len(tlvRecords) != 0 {
138-
da.ExtraData = tlvRecords
139-
}
118+
da.ExtraData = extraData
140119

141120
return nil
142121
}

‎lnwire/dyn_ack_test.go‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
package lnwire
2+
3+
import (
4+
"bytes"
5+
"testing"
6+
7+
"github.com/lightningnetwork/lnd/lnutils"
8+
"github.com/stretchr/testify/require"
9+
)
10+
11+
// TestDynAckEncodeDecode checks that the Encode and Decode methods for DynAck
12+
// work as expected.
13+
func TestDynAckEncodeDecode(t *testing.T) {
14+
t.Parallel()
15+
16+
// Generate random channel ID.
17+
chanIDBytes, err := generateRandomBytes(32)
18+
require.NoError(t, err)
19+
20+
var chanID ChannelID
21+
copy(chanID[:], chanIDBytes)
22+
23+
// Generate random sig.
24+
sigBytes, err := generateRandomBytes(64)
25+
require.NoError(t, err)
26+
27+
var sig Sig
28+
copy(sig.bytes[:], sigBytes)
29+
30+
// Create test data for the TLVs. The actual value doesn't matter, as we
31+
// only care about that the raw bytes can be decoded into a msg, and the
32+
// msg can be encoded into the exact same raw bytes.
33+
testTlvData := []byte{
34+
// ExtraData - unknown tlv record.
35+
//
36+
// NOTE: This record is optional and occupies the type 1.
37+
0x1, // type.
38+
0x2, // length.
39+
0x79, 0x79, // value.
40+
41+
// LocalNonce tlv.
42+
0x14, // type.
43+
0x42, // length.
44+
0x2c, 0xd4, 0x53, 0x7d, 0xaa, 0x7b, // value.
45+
0x7e, 0xae, 0x18, 0x32, 0xa6, 0xc4, 0x29, 0xe9, 0xe0, 0x91,
46+
0x32, 0x7a, 0xaf, 0xd1, 0x1c, 0x2b, 0x04, 0xa0, 0x4d, 0xb5,
47+
0x6a, 0x6f, 0x8b, 0x6c, 0xdc, 0xd1, 0x80, 0x2d, 0xff, 0x72,
48+
0xd8, 0x3c, 0xfc, 0x01, 0x6e, 0x7c, 0x1a, 0xc8, 0x5e, 0x3a,
49+
0x16, 0x98, 0xbc, 0x9b, 0x6e, 0x22, 0x58, 0x96, 0x96, 0xad,
50+
0x88, 0xbf, 0xff, 0x59, 0x90, 0xbd, 0x36, 0x0b, 0x0b, 0x4d,
51+
52+
// ExtraData - unknown tlv.
53+
0x6f, // type.
54+
0x2, // length.
55+
0x79, 0x79, // value.
56+
}
57+
58+
msg := &DynAck{}
59+
60+
// Pre-allocate a new slice with enough capacity for all three parts for
61+
// efficiency.
62+
totalLen := len(chanIDBytes) + len(sigBytes) + len(testTlvData)
63+
rawBytes := make([]byte, 0, totalLen)
64+
65+
// Append each slice to the new rawBytes slice.
66+
rawBytes = append(rawBytes, chanIDBytes...)
67+
rawBytes = append(rawBytes, sigBytes...)
68+
rawBytes = append(rawBytes, testTlvData...)
69+
70+
// Decode the raw bytes.
71+
r := bytes.NewBuffer(rawBytes)
72+
err = msg.Decode(r, 0)
73+
require.NoError(t, err)
74+
75+
t.Logf("Encoded msg is %v", lnutils.SpewLogClosure(msg))
76+
77+
// Encode the msg into raw bytes and assert the encoded bytes equal to
78+
// the rawBytes.
79+
w := new(bytes.Buffer)
80+
err = msg.Encode(w, 0)
81+
require.NoError(t, err)
82+
83+
require.Equal(t, rawBytes, w.Bytes())
84+
}

‎lnwire/dyn_commit.go‎

Lines changed: 42 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -46,14 +46,28 @@ func (dc *DynCommit) Encode(w *bytes.Buffer, _ uint32) error {
4646
return err
4747
}
4848

49-
var extra ExtraOpaqueData
50-
err := extra.PackRecords(dynProposeRecords(&dc.DynPropose)...)
49+
// Create extra data records.
50+
producers, err := dc.ExtraData.RecordProducers()
5151
if err != nil {
5252
return err
5353
}
54-
dc.ExtraData = extra
5554

56-
return WriteBytes(w, dc.ExtraData)
55+
// Append the known records.
56+
producers = append(producers, dynProposeRecords(&dc.DynPropose)...)
57+
dc.LocalNonce.WhenSome(
58+
func(rec tlv.RecordT[tlv.TlvType14, Musig2Nonce]) {
59+
producers = append(producers, &rec)
60+
},
61+
)
62+
63+
// Encode all known records.
64+
var tlvData ExtraOpaqueData
65+
err = tlvData.PackRecords(producers...)
66+
if err != nil {
67+
return err
68+
}
69+
70+
return WriteBytes(w, tlvData)
5771
}
5872

5973
// Decode deserializes the serialized DynCommit stored in the passed io.Reader
@@ -75,58 +89,53 @@ func (dc *DynCommit) Decode(r io.Reader, _ uint32) error {
7589
}
7690

7791
// Prepare receiving buffers to be filled by TLV extraction.
78-
var dustLimit tlv.RecordT[tlv.TlvType0, uint64]
79-
var maxValue tlv.RecordT[tlv.TlvType2, uint64]
80-
var htlcMin tlv.RecordT[tlv.TlvType4, uint64]
81-
var reserve tlv.RecordT[tlv.TlvType6, uint64]
92+
var dustLimit tlv.RecordT[tlv.TlvType0, tlv.BigSizeT[btcutil.Amount]]
93+
var maxValue tlv.RecordT[tlv.TlvType2, MilliSatoshi]
94+
var htlcMin tlv.RecordT[tlv.TlvType4, MilliSatoshi]
95+
var reserve tlv.RecordT[tlv.TlvType6, tlv.BigSizeT[btcutil.Amount]]
8296
csvDelay := dc.CsvDelay.Zero()
8397
maxHtlcs := dc.MaxAcceptedHTLCs.Zero()
8498
chanType := dc.ChannelType.Zero()
99+
nonce := dc.LocalNonce.Zero()
85100

86-
typeMap, err := tlvRecords.ExtractRecords(
87-
&dustLimit, &maxValue, &htlcMin, &reserve, &csvDelay, &maxHtlcs,
88-
&chanType,
101+
// Parse all known records and extra data.
102+
knownRecords, extraData, err := ParseAndExtractExtraData(
103+
tlvRecords, &dustLimit, &maxValue, &htlcMin, &reserve,
104+
&csvDelay, &maxHtlcs, &chanType, &nonce,
89105
)
90106
if err != nil {
91107
return err
92108
}
93109

94110
// Check the results of the TLV Stream decoding and appropriately set
95111
// message fields.
96-
if val, ok := typeMap[dc.DustLimit.TlvType()]; ok && val == nil {
97-
var rec tlv.RecordT[tlv.TlvType0, btcutil.Amount]
98-
rec.Val = btcutil.Amount(dustLimit.Val)
99-
dc.DustLimit = tlv.SomeRecordT(rec)
112+
if _, ok := knownRecords[dc.DustLimit.TlvType()]; ok {
113+
dc.DustLimit = tlv.SomeRecordT(dustLimit)
100114
}
101-
if val, ok := typeMap[dc.MaxValueInFlight.TlvType()]; ok && val == nil {
102-
var rec tlv.RecordT[tlv.TlvType2, MilliSatoshi]
103-
rec.Val = MilliSatoshi(maxValue.Val)
104-
dc.MaxValueInFlight = tlv.SomeRecordT(rec)
115+
if _, ok := knownRecords[dc.MaxValueInFlight.TlvType()]; ok {
116+
dc.MaxValueInFlight = tlv.SomeRecordT(maxValue)
105117
}
106-
if val, ok := typeMap[dc.HtlcMinimum.TlvType()]; ok && val == nil {
107-
var rec tlv.RecordT[tlv.TlvType4, MilliSatoshi]
108-
rec.Val = MilliSatoshi(htlcMin.Val)
109-
dc.HtlcMinimum = tlv.SomeRecordT(rec)
118+
if _, ok := knownRecords[dc.HtlcMinimum.TlvType()]; ok {
119+
dc.HtlcMinimum = tlv.SomeRecordT(htlcMin)
110120
}
111-
if val, ok := typeMap[dc.ChannelReserve.TlvType()]; ok && val == nil {
112-
var rec tlv.RecordT[tlv.TlvType6, btcutil.Amount]
113-
rec.Val = btcutil.Amount(reserve.Val)
114-
dc.ChannelReserve = tlv.SomeRecordT(rec)
121+
if _, ok := knownRecords[dc.ChannelReserve.TlvType()]; ok {
122+
dc.ChannelReserve = tlv.SomeRecordT(reserve)
115123
}
116-
if val, ok := typeMap[dc.CsvDelay.TlvType()]; ok && val == nil {
124+
if _, ok := knownRecords[dc.CsvDelay.TlvType()]; ok {
117125
dc.CsvDelay = tlv.SomeRecordT(csvDelay)
118126
}
119-
if val, ok := typeMap[dc.MaxAcceptedHTLCs.TlvType()]; ok && val == nil {
127+
if _, ok := knownRecords[dc.MaxAcceptedHTLCs.TlvType()]; ok {
120128
dc.MaxAcceptedHTLCs = tlv.SomeRecordT(maxHtlcs)
121129
}
122-
if val, ok := typeMap[dc.ChannelType.TlvType()]; ok && val == nil {
130+
if _, ok := knownRecords[dc.ChannelType.TlvType()]; ok {
123131
dc.ChannelType = tlv.SomeRecordT(chanType)
124132
}
125-
126-
if len(tlvRecords) != 0 {
127-
dc.ExtraData = tlvRecords
133+
if _, ok := knownRecords[dc.LocalNonce.TlvType()]; ok {
134+
dc.LocalNonce = tlv.SomeRecordT(nonce)
128135
}
129136

137+
dc.ExtraData = extraData
138+
130139
return nil
131140
}
132141

0 commit comments

Comments
 (0)