lnwire: pull gossip v2 messages in line with BOLT taproot-gossip review updates by ViktorT-11 · Pull Request #11164 · lightningnetwork/lnd

ViktorT-11 · 2026-09-03T08:44:09Z

Replaces #10837

This PR builds on #10837 and makes the PR up to date.

Summary

This branch pulls the gossip v2 wire messages in lnwire in line with the
review-driven updates on the BOLT taproot-gossip extension (lightning/bolts#1059).

The spec branch backing these changes is currently at
lightning/bolts#1059 (extension BOLT 7-style document
for gossip v2). The accompanying spec PR linked from each commit groups the
changes by reviewer rationale.

Each commit here is a single, independently-reviewable spec delta and is
written so that make unit pkg=lnwire and go build ./... both pass on its
own. make install builds clean at the tip.

Commits

lnwire: bump pure-TLV signed range to 0..=239; sig TLVs to type 240 —
align with BOLT 12's renumbering. Signed range expands from 0..=159 to
0..=239 and the signature TLVs in channel_announcement_2,
channel_update_2 and node_announcement_2 move from type 160 to 240.
lnwire: enforce compulsory-field presence on gossip v2 reader —
add an AssertRequiredPresent helper and use it in the three gossip v2
decoders so messages missing required TLVs are rejected up front.
lnwire: reject tor_v3_address with port 0 in node_announcement_2 —
extend the existing port-not-zero rule to cover tor v3 addresses too.
lnwire: collapse gossip_timestamp_range's two block-height TLVs into
one — replace the split first_block (type 2) / block_height_range
(type 4) pair with a single BlockHeightRange TLV at type 2 holding
{u32 first_block_height, tu32 num_blocks}.
lnwire: add funding_txid TLV to announcement_signatures_2 —
required funding_txid (type 6, sha256-shaped [32]byte) so each
message names its funding tx explicitly. NewAnnSigs2 and the existing
channeldb test callers are updated.
lnwire: assign inbound-fee TLVs proper types on channel_update_2 —
drop the experimental InboundFee at TlvType55555 in favour of two
defaulted-on-the-wire uint32 records: type 20 inbound_fee_base_msat
and type 22 inbound_fee_proportional_millionths. Positive-only.
ChanEdgePolicyFromWire folds the two fields into the existing
fn.Option[lnwire.Fee] contract (None when both are 0).
lnwire: carry two raw musig2 partial sigs in announcement_signatures_2
— introduce an AnnouncementSigPair value type (node || bitcoin, 64
bytes) and switch the PartialSignature field to it. The previously
pre-aggregated 32-byte form is gone; receivers can now verify each half
with the standard MuSig2 PartialSigVerify routine.
graph/db/models: clarify inbound-fee comment in ChanEdgePolicyFromWire
— tiny follow-up; the inbound-fee TLVs are defaulted on the wire, not
required.
lnwire: encode channel_update_2 direction in short_channel_id via
sciddir — switch the short_channel_id field to BOLT 1's
sciddir_or_pubkey type constrained to the 9-byte sciddir form
(<dirbyte><scid>). The separate second_peer TLV at type 8 becomes
redundant and is removed; IsNode1() now derives from the direction
byte.

Out of scope (deliberately)

Spec items Fixing some README typos #2 (channel_reestablish.announcement_nonces TLV at type 7
- bit 1 of my_current_funding_locked.retransmit_flags) and Make lnd go-gettable #3
  (splice_locked extensions carrying the announcement nonces) both
  rely on splicing infrastructure that lnd does not implement yet.
  They are tracked but left as follow-ups.
The spec PR's Can Not connect to any peer in segnet4, Could I use other block chain? #14 (test vectors) is intentionally not addressed here
— it is the spec-side change set's last item and is independent of
this implementation work.

Test plan

make unit pkg=lnwire passes
make unit pkg=channeldb passes (the only other unit suite that
builds AnnounceSignatures2 directly)
make unit pkg=graph/db passes (touches ChanEdgePolicyFromWire)
make unit pkg=discovery passes (gossip 2 consumers)
make install builds clean at HEAD
CI green

github-actions · 2026-09-03T08:49:38Z

🔴 PR Severity: CRITICAL

file classification | 17 files | 664 lines changed

🔴 Critical (9 files)

lnwire/announcement_signatures_2.go - Lightning wire protocol message (taproot channel announcement signatures)
lnwire/channel_announcement_2.go - Lightning wire protocol message
lnwire/channel_update_2.go - Lightning wire protocol message
lnwire/gossip_timestamp_range.go - Lightning wire protocol message
lnwire/node_announcement_2.go - Lightning wire protocol message
lnwire/partial_sig.go - Lightning wire protocol message (MuSig2 partial signature TLV record)
lnwire/pure_tlv.go - Wire protocol TLV encoding helpers
lnwire/sciddir.go - New wire protocol TLV type (short channel ID + direction)
lnwire/test_message.go - Wire protocol test-message support code (not a _test.go file, lives in the lnwire package)

🟠 High (1 file)

graph/db/models/channel_edge_policy.go - Network graph model changes

🟢 Low (7 files)

channeldb/waitingproof_test.go - test-only change
docs/release-notes/release-notes-0.22.0.md - release notes
lnwire/announcement_signatures_2_test.go - test-only change
lnwire/channel_announcement_2_test.go - test-only change
lnwire/channel_update_2_test.go - test-only change
lnwire/node_announcement_2_test.go - test-only change
lnwire/pure_tlv_test.go - test-only change

Analysis

The bulk of this PR modifies non-test files under lnwire/* (taproot/simple-taproot gossip messages: announcement signatures, channel announcement/update, node announcement, gossip timestamp range, partial sig, TLV helpers, and a new sciddir.go type), which is a CRITICAL package covering Lightning wire protocol messages. This alone sets the severity to CRITICAL.

Additionally, excluding test and auto-generated files, the PR changes ~605 lines across 11 files, which exceeds the 500-line bump threshold — though since CRITICAL is already the highest tier, this doesn't change the outcome.

The one graph/db/models/channel_edge_policy.go change is HIGH severity on its own but doesn't affect the overall verdict given the CRITICAL lnwire changes.

_{To override, add a severity-override-{critical,high,medium,low} label.}

bitromortac

Nice, I took a first look.

bitromortac · 2026-09-07T09:45:39Z

+
+// SciddirLen is the wire length of a Sciddir: one direction byte followed by
+// the 8-byte short_channel_id.
+const SciddirLen = 9


you could take a look at lnwire/intro_node.go, maybe we can reuse that or unify

I added a fixup commit which addresses this, as I'm not sure this change is actually cleaner than the previous version. Please let me know which version you prefer.

I think we may be able to delete sciddir.go and reuse the existing data type a bit more, see other comment.

See #11164 (comment)

bitromortac · 2026-09-07T09:48:25Z

+				BaseFee: int32(baseFee),
+				FeeRate: int32(propFee),


I think overflow is possible here

Good catch! I added a check which rejects such values which would overflow.

However, after adding that check, LND is now in theory not fully spec compliant as in theory someone could advertise an inbound fee between 2,147,483,648 and 4,294,967,295, which will be rejected by LND.

Since such high values for inbound fees are unrealistic to be used, we should consider if that's an ok trade-off or not.

The real fix for this would be to preserve the full unsigned range through LND’s inbound fee handling. That would be a big change though, so if we'd want ot go down that route I suggest that we do so through follow-ups.

Let me know your thoughts!

I think, rejecting values above math.MaxInt32 is a good solution for now and keep it for a follow-up.

Added this as a part of the detailed check list for Type Representation in #10293

litbot-9000 · 2026-09-14T11:11:36Z

@yyforyongyu: review reminder
@ViktorT-11, remember to re-request review from reviewers when ready

litbot-9000 · 2026-09-21T15:23:07Z

@yyforyongyu: review reminder
@bitromortac: review reminder

yyforyongyu

Left a few comments, think we are close!

yyforyongyu · 2026-09-23T15:58:55Z

+// DecodeWithParsedTypes helpers). It is used to enforce the spec's
+// reader-side requirement that compulsory TLVs are present in a received
+// pure-TLV message.
+func AssertRequiredPresent(typeMap tlv.TypeMap, required ...tlv.Type) error {


Q: is there a plan to use this method outside lnwire pkg? if not we should keep it private.

Edited this to keep it private. I think it's likely that this was planned by Elle to be used by some other package, but since I'm taking over and I don't have a clear answer to that at the moment, I changed for now and will update it in case we'd need it exported in a future PR.

yyforyongyu · 2026-09-23T15:59:54Z

+// type number is supplied by the wrapping RecordT.
+func (b *BlockHeightRange) Record() tlv.Record {
+	sizeFunc := func() uint64 {
+		return 4 + tlv.SizeTUint32(b.NumBlocks)


nit: add some inline docs explaining where the 4 is coming from?

Added a comment to explain that the 4 accounts for FirstBlockHeight, which is encoded as a fixed-width uint32.

yyforyongyu · 2026-09-23T16:02:19Z


+	// FundingTxID is the txid of the funding transaction that this
+	// announcement signature covers. For an initial channel announcement
+	// this is the original funding transaction; for a spliced channel it


Q: are we designing this for splicing too? I don't think it's in the roadmap right?

The spec PR does actually account for splicing after T-bast feedback, hence why I think this comment mentioned it. However, since it's currently not on our roadmap, I removed the comment for now, as it's probably just more confusing than helpful.

yyforyongyu · 2026-09-23T16:48:34Z

+		)
+	}
+
+	if c.InboundFeeBaseMsat.Val != defaultInboundFeeBaseMsat {


I think we need to preserve whether types 20/22 were present here. A peer may explicitly encode the default 0 (the spec only says SHOULD omit it), but AllRecords drops the record and reconstructs different signed bytes. Could we track presence separately and add an explicit-zero signature test?

This is wider than 20 and 22. It affects every field with a default: types 0, 6, 10, 12, 16, 18, 20 and 22 of channel_update_2, and chain_hash of channel_announcement_2. I checked type 10 by execution: 0a 02 00 50 is dropped on re-encode. Type 14 fails the other way, because lnd always emits it.

The bolt12 package already solves this: every defaulted field is an OptionalRecordT, the decoder records presence with lnwire.SetOptFromMap, and the encoder emits with lnwire.AddOpt, so a message re-encodes exactly as it was sent. The default applies only when a caller reads an absent field. The same approach fixes the optional features of channel_announcement_2.

Nice catch! I added @bitromortac suggestion, which uses the same approach as the bolt12 package.

yyforyongyu · 2026-09-23T16:48:36Z

+		return err
+	}
+
+	v.Node.SetBytes(&nodeBytes)


I think we need to check the return value from SetBytes here. Values greater than or equal to the curve order are reduced modulo n, so invalid partial sigs are accepted. Could we reuse PartialSig for both halves and reject overflow there?

Great catch! Addressed.

yyforyongyu · 2026-09-23T16:48:38Z


+// truncatedUint32Record preserves a typed record's value and type while using
+// BOLT's tu32 encoding, which omits leading zero bytes.
+func truncatedUint32Record[T tlv.TlvType](


Q: should the tu32 encoding live in the fee value type instead? These exported fields are still RecordT[..., uint32], so calling Record() directly emits fixed-width u32 and every caller has to remember this wrapper.

I'll address this specific feedback after the next review round, as I specifically missed addressing this one.

yyforyongyu · 2026-09-23T16:48:40Z

 func torV3AddrsEncoder(w io.Writer, val interface{}, _ *[8]byte) error {
 	if v, ok := val.(*TorV3Addrs); ok {
 		for _, addr := range *v {
+			if addr.Port == 0 {


I think this also needs to reject ports outside 1..65535 before the uint16 cast below. For example, 65536 passes this check and is encoded as port 0. Could we use a stable port error and add boundary tests?

yyforyongyu · 2026-09-23T16:48:42Z

+
+// SciddirLen is the wire length of a Sciddir: one direction byte followed by
+// the 8-byte short_channel_id.
+const SciddirLen = sciddirLen


nit: please squash f4bb4e91c into the preceding sciddir commit and fix the f - subject before merge

I'll do that as soon as we've decided if we'd like to use the fixup approach or not :)

bitromortac

Thanks for the fixes. I cross-checked every TLV of the five gossip v2 messages against the spec and there are some discrepancies still.

bitromortac · 2026-09-24T08:52:07Z

+// `0` (refers to node_id_1 of the corresponding channel_announcement_2) or
+// `1` (refers to node_id_2). The `pubkey` form of `sciddir_or_pubkey` is
+// rejected wherever a Sciddir is used.
+type Sciddir struct {


The spec types this field as BOLT 1's sciddir_or_pubkey, constrained to the sciddir form. It has no separate sciddir type. lnwire already implements sciddir_or_pubkey as IntroductionNode, so SciddirIntro could hold this field, with a small sciddir-only record that rejects the pubkey form with its own error. That removes the second implementation of the same 9 bytes.

Thanks for the suggested fix. I'm still not sure that the applied fixup is more simple than the original implementation, as it removes some abstraction that was nice to separate.

I've therefore kept the fixup as a fixup, so that we can discuss if you have a strong preference that it should be applied, or if we should just go with the original implemenation.

I'm not sure but would be nice to have a single implementation of the same type somewhere, so non-blocking.

bitromortac · 2026-09-24T08:52:33Z

+				BaseFee: int32(baseFee),
+				FeeRate: int32(propFee),


I think, rejecting values above math.MaxInt32 is a good solution for now and keep it for a follow-up.

bitromortac · 2026-09-24T08:52:34Z

+		)
+	}
+
+	if c.InboundFeeBaseMsat.Val != defaultInboundFeeBaseMsat {


This is wider than 20 and 22. It affects every field with a default: types 0, 6, 10, 12, 16, 18, 20 and 22 of channel_update_2, and chain_hash of channel_announcement_2. I checked type 10 by execution: 0a 02 00 50 is dropped on re-encode. Type 14 fails the other way, because lnd always emits it.

The bolt12 package already solves this: every defaulted field is an OptionalRecordT, the decoder records presence with lnwire.SetOptFromMap, and the encoder emits with lnwire.AddOpt, so a message re-encodes exactly as it was sent. The default applies only when a caller reads an absent field. The same approach fixes the optional features of channel_announcement_2.

bitromortac · 2026-09-24T08:55:44Z

+
+// SciddirLen is the wire length of a Sciddir: one direction byte followed by
+// the 8-byte short_channel_id.
+const SciddirLen = 9


I think we may be able to delete sciddir.go and reuse the existing data type a bit more, see other comment.

ViktorT-11 · 2026-09-25T18:39:00Z

Thanks a lot for the reviews, and suggested fixes @bitromortac & @yyforyongyu 🙏!

@bitromortac, I've looked through your suggested fixes and applied most of them, plus left a open question in #11164 (comment).

This is now ready for another review.

Also, as a reminder to my self, this comment needs to be addressed after the next review round: #11164 (comment)

litbot-9000 · 2026-10-05T15:03:13Z

@yyforyongyu: review reminder

litbot-9000 · 2026-10-12T15:35:38Z

@yyforyongyu: review reminder

yyforyongyu

I think we are very close!! My main comment/question is that a valid gossip v2 update can be rejected when htlc_maximum_msat is omitted.

yyforyongyu · 2026-10-15T14:42:19Z

+		MinHTLC: c.HTLCMinimumMsat.ValOpt().UnwrapOr(
+			defaultHtlcMinMsat,
+		),
+		HasMaxHTLC: maxHTLC.IsSome(),


I think we also need to handle the default for an absent htlc_maximum_msat here. We return HasMaxHTLC=false and MaxHTLC=0, but validateChannelUpdate2Fields rejects that zero, and ChanEdgePolicyFromWire drops the presence bit before the cached policy sets it to true. So a conformant update with type 14 omitted is rejected or gets an unusable limit. Could we apply the spec's half-capacity default where the channel capacity is known, while keeping the TLV absent for signature verification?

Good call. We may be dropping this though due to comment: lightning/bolts#1059 (comment)

But I addressed your comment and did as suggested and applied the "spec's half-capacity default" for now.

yyforyongyu · 2026-10-15T14:42:19Z

+		return tlv.NewTypeForEncodingErr(val, "lnwire.SciddirIntro")
+	}
+
+	if v.Direction > 1 {


Q: can we reuse SciddirIntro.validate() here, and NewSciddirIntro() in the decoder below? They already own the 0/1 direction rule. These checks duplicate it and return plain errors instead of ErrInvalidIntroNode, so the same invalid direction has different error identities depending on the path.

We can also rename SciddirIntro to anything that fits better if we rely more on the type.

Addressed with the latest push :)

yyforyongyu · 2026-10-15T14:42:19Z

+
+	tests := []struct {
+		name    string
+		msg     func() Message


Could we keep the message type and records as data here instead of storing a constructor? The fallback logic at lines 300-303 and the typ: 1 placeholders make it harder to see what each case exercises. An explicit message type and record set per case, with construction in the test body, would keep this table data-only.

yyforyongyu · 2026-10-15T14:42:19Z

+				var msg ChannelUpdate2
+				err = msg.Decode(bytes.NewReader(data), 0)
+				if test.wantErr {
+					require.Error(t, err)


I think we should check the specific error here. DTUint32 returns tlv.ErrTUintNotMinimal for the leading-zero cases and tlv.ErrTypeForDecoding for an oversized value, but require.Error would also pass for an unrelated decode failure. Could we use require.ErrorIs and require.ErrorAs for those cases?

Implemented with the latest push :)

yyforyongyu · 2026-10-15T14:42:19Z

+		{name: "update/htlc_minimum_msat", typ: 12, value: []byte{1}},
+		{name: "update/fee_base_msat", typ: 16, value: []byte{3, 0xe8}},
+		{name: "update/fee_proportional", typ: 18, value: []byte{1}},
+		{name: "update/inbound_fee_base", typ: 20, value: []byte{}},


Q: do we need these two inbound-fee cases here? The zero case in TestChanUpdate2FeeEncoding already covers types 20 and 22 and checks that AllRecords() preserves and re-encodes the explicit zero. We could drop these two rows and keep the distinct default-presence cases.

Thanks! I verified that they were indeed redundant. So I removed them with the latest push

yyforyongyu · 2026-10-15T14:42:19Z

+func assertRequiredPresent(typeMap tlv.TypeMap, required ...tlv.Type) error {
+	for _, t := range required {
+		if _, ok := typeMap[t]; !ok {
+			return fmt.Errorf("required TLV type %d missing", t)


Could we add negative decode tests for the new rejection paths? There isn't a case that removes each compulsory TLV from the four messages and checks that Decode rejects it. The scalar overflow checks in partial_sig.go:338-345, invalid sciddir forms in intro_node.go:244-262, and short block-height ranges in gossip_timestamp_range.go:105 also need malformed-input cases. These tests should exercise the public decoders and check stable error identities where available.

Done, this latest push added new tests for this, mainly through new files FYI.

yyforyongyu · 2026-10-15T14:42:19Z

+		// The graph's fee model uses signed values to support v1
+		// discounts. Reject v2 surcharges that would wrap into
+		// discounts.
+		if baseFee > math.MaxInt32 || propFee > math.MaxInt32 {


Could we add a table test for the v2 conversion here? The changed branch has no coverage for absent and explicitly zero inbound fees becoming None, non-zero fees becoming Some, or values above math.MaxInt32 being rejected. It should also check both sciddir directions so the SecondPeer conversion is covered.

Added a table test for this 👍

ViktorT-11 · 2026-10-19T18:43:49Z

Thanks for the review @yyforyongyu 🙏🎉! I've addressed your feedback. I'll do a rebase onto master later today.

The taproot-gossip BOLT extension was updated to align with BOLT 12, which long since moved its signature TLVs to type 240 and widened the signed TLV range from 0..=159 to 0..=239. Mirror that here: * pureTLVUnsignedRangeOneStart: 160 -> 240 * channel_announcement_2.Signature: TlvType160 -> TlvType240 * channel_update_2.Signature: TlvType160 -> TlvType240 * node_announcement_2.Signature: TlvType160 -> TlvType240 Update the pure-TLV test fixtures and the test message types correspondingly.

The BOLT taproot-gossip spec now requires each gossip v2 reader to reject messages that are missing any of their compulsory fields with a warning/close/ignore action. lnwire's three gossip v2 Decode implementations silently zero-valued the missing TLVs, which made later validation harder. Add a small AssertRequiredPresent helper to pure_tlv.go and call it from each Decode after DecodeWithParsedTypesP2P / ExtractRecords: * channel_announcement_2: short_channel_id, outpoint, capacity, node_id_1, node_id_2, signature. * channel_update_2: short_channel_id, block_height, signature. * node_announcement_2: features, block_height, node_id, signature.

ViktorT-11 · 2026-10-19T23:52:22Z

Rebased on master. This now made 59756c0 into a test change only, as the prod code included in that commit had already been added to master.

yyforyongyu

Cool i think this can be merged once the two CI failures are fixed!

yyforyongyu · 2026-10-20T08:34:40Z

  later in the reservation flow as a funder-balance-dust error; they now
  surface a clearer, spec-aligned error string up front.

+* The gossip v2 wire messages in `lnwire`


Could we move this entry to docs/release-notes/unreleased/11164.md? The rebase brings in the fragment workflow documented in docs/release-notes/README.md, and the release-note check is failing because the fragment is missing. The new file should include the applicable heading hierarchy, with this hunk removed from the version-wide document.

Ah yeah, thanks :)! Addressed.

yyforyongyu · 2026-10-20T08:34:40Z

+		)
+	}
+
+	for length := 0; length < 4; length++ {


The lint job flags this loop and the one at lnwire/test_message.go:1556 with intrange. Could we use for length := range 4 here and for range numAddrs there, since the latter doesn't use its index?

yyforyongyu · 2026-10-20T08:34:40Z

 	"time"

+	"github.com/btcsuite/btcd/btcutil/v2"
+


The format check and lint job both fail on formatting here and in lnwire/node_announcement_2_test.go. Could we run make fmt and include the formatting changes? The reported differences are this extra blank line between imports and the trailing blank line in the test file.

Thanks. I went through each commit and ran make fmt on each intermediate commit. If it still fails now, I'll push a new update.

yyforyongyu · 2026-10-20T08:34:40Z

+		// stays absent on the message, so its signature still
+		// verifies.
+		maxHTLC := policy.MaxHTLC
+		if !policy.HasMaxHTLC {


Q: can ChannelUpdate2 own choosing between an explicit max HTLC and the capacity default? This presence/default branch also lives in netann/channel_update.go:317-323. DefaultMaxHTLCMsat shares the arithmetic, but each caller still owns deciding which value to use. A capacity-aware method on the message would give validation and graph conversion one implementation of that rule, while keeping the signed TLV unchanged.

Good idea! I added a function for than in the message, and an explicit TestChanUpdate2MaxHTLCLeavesRecordAbsent test as the new function is a public exported function of the message. The added test was not really part of your feedback, so let me know if it should be dropped, but wasn't really covered in other tests.

The taproot-gossip extension defines TLV 11 of node_announcement_2 as dns_hostnames, a list of dns_hostname entries. Each entry is a u16 hostname length, the hostname and a u16 port. lnd modelled the field as a single DNSAddress with no length prefix. A conformant peer therefore read the first two bytes of lnd's hostname as a length, and lnd read a conformant entry's length bytes as part of the hostname. ValidateDNSAddr then rejected the hostname, so lnd dropped any conformant announcement that carried a DNS address. A node could also only announce one hostname. The new DNSAddrs type encodes the list as the spec defines it. The codec does not validate the hostname or the port. The list is in the signed range and the signature digest is rebuilt from the decoded records, so the codec has to round-trip every entry exactly. Deciding which entries are usable happens above the codec. DNSAddress and ValidateDNSAddr stay as they are, because the v1 announcement shares them.

BOLT 7 forbids a zero port on every announced address type, and tells a receiver to ignore such an address while it keeps the rest of the announcement. Add NodeAnnouncement2.Addresses, which returns the addresses a node can be reached at and leaves out any entry with a zero port and any DNS entry whose hostname is not valid. Consumers of a received announcement use it instead of the raw address fields. The filter cannot sit in the codec. The address fields are in the signed range, and the signature digest is rebuilt by re-encoding the decoded records, so the codec round-trips every entry. For the same reason the codec does not enforce the sender rule. That rule belongs where lnd builds its own node_announcement_2.

TestNodeAnn2AddressesIgnorePortZero gave both entries of each IP list the same address, so only the port told them apart. That made the test blind to the decoder sharing one buffer across entries, which the test admitted in a comment. Give each list a usable address followed by a distinct zero-port one, so decoding the second cannot overwrite the first's IP bytes without the test noticing. The decoder itself already allocates a buffer per address and reads through io.ReadFull, so this commit only adds the coverage for it.

The taproot-gossip BOLT extension swapped the two-TLV (first_block @ type 2, block_height_range @ type 4) layout in gossip_timestamp_range for a single TLV at type 2 holding both fields: u32 first_block_height and tu32 num_blocks. The two fields are always set together, and the truncated u32 num_blocks saves a few bytes on the wire. Replace the FirstBlockHeight (type 2, u32) and BlockRange (type 4, u32) optional records on GossipTimestampRange with a single BlockHeightRange (type 2) optional record holding a new BlockHeightRange struct. The struct's Record() method uses MakeDynamicRecord with EUint32T+ETUint32T for the encoder and DUint32+DTUint32 for the decoder. The property-test factory in test_message.go is updated to draw a single optional BlockHeightRange instead of two independent fields.

The taproot-gossip BOLT extension added a required funding_txid TLV (type 6, sha256) to announcement_signatures_2 so each message names its funding transaction explicitly. For an initial channel open this is the original funding tx; for a spliced channel it is the txid of the splice transaction that triggered the new round of announcement signing. Using funding_txid directly (rather than inferring it via short_channel_id) is what makes splice announcements unambiguous when multiple candidate funding transactions may exist at different points. Add the field on AnnounceSignatures2 (encoded as [32]byte, to follow the same primitive-encoding pattern channel_announcement_2 uses for its chain hash), thread it through NewAnnSigs2, AllRecords and the Decode presence assertion, and update the test fixtures + the rapid property-test factory to set it. The two channeldb waitingproof tests that build AnnounceSignatures2 via NewAnnSigs2 are updated to pass a placeholder funding_txid.

The taproot-gossip BOLT extension assigned the experimental inbound-fee field on channel_update_2 a real TLV layout: two separate uint32 records, type 20 (inbound_fee_base_msat) and type 22 (inbound_fee_proportional_millionths), both positive-only with a default of 0. Pull the implementation in line: - Replace the experimental InboundFee OptionalRecordT[TlvType55555, Fee] (which had a long-standing TODO to assign a real type) with two required uint32 RecordTs at types 20 and 22. - Suppress on encode when 0 and default-fill on decode, matching how the surrounding fee/htlc fields behave. - Update ChanEdgePolicyFromWire for ChannelUpdate2 to fold the two uint32 values into the existing fn.Option[lnwire.Fee] downstream contract: emit None when both are 0, Some otherwise. The Fee struct itself still uses int32 for the legacy ChannelUpdate1 case; ChannelUpdate2 inbound fees can only be non-negative so the uint32->int32 widening is safe in practice (and a v2 sender can't encode a negative fee anyway). - Update the channel_update_2 test fixture to carry valid type-20 and type-22 records, and move the previously-unknown extra TLV out of slot 20 to slot 24. - Update the rapid property factory to draw the two new uint32 fields from a non-zero range when including an inbound fee.

The taproot-gossip BOLT extension dropped the previously pre-aggregated 32-byte partial signature on announcement_signatures_2 in favour of emitting both raw musig2 partial sigs back-to-back -- one for the node_id key and one for the bitcoin key -- so the receiver can verify each half with the standard MuSig2 PartialSigVerify routine instead of the custom verifier the old layout required. Introduce an AnnouncementSigPair value type in partial_sig.go that encodes as `node || bitcoin` for a fixed 64 bytes, with a static- record builder via tlv.MakeStaticRecord. Swap announcement_signatures_2's PartialSignature (TlvType4, PartialSig) field for a new PartialSignatures (TlvType4, AnnouncementSigPair) field; update NewAnnSigs2 to take the pair; update the hardcoded test fixture (length 0x20 -> 0x40, 32 bytes -> 64 bytes of zero padding); update the rapid property factory to draw two independent partial sigs; and update the three channeldb waitingproof tests that build AnnounceSignatures2 directly. The existing 32-byte PartialSig type stays in place for the co-operative close flow and other call-sites that don't carry both sigs at once.

The channel_update_2 inbound-fee TLVs are not "required" -- they follow the same defaulted-on-the-wire pattern as fee_base_msat and friends, where the field is always present in the Go struct (a uint32 with the default-fill applied on decode) and suppressed from the wire when it equals the default of 0. Reword the comment to avoid implying that a sender MUST emit them. No code change.

…ddir The taproot-gossip BOLT extension switched channel_update_2.short_channel_id from a plain 8-byte SCID to BOLT 1's sciddir_or_pubkey type constrained to the sciddir form: a 9-byte <dirbyte><scid> encoding where the direction byte is 0 for node_id_1 and 1 for node_id_2. With the direction now part of the scid itself, the previously-separate type-8 second_peer flag TLV is fully redundant and is removed. Add a Sciddir value type in lnwire/sciddir.go with a 9-byte static record (custom encoder/decoder that rejects any direction byte other than 0 or 1, so we can never accidentally accept the pubkey form of sciddir_or_pubkey here). In ChannelUpdate2: * ShortChannelID now wraps a Sciddir instead of a ShortChannelID. * The SecondPeer OptionalRecordT and all its Decode/AllRecords wiring is removed. * IsNode1() now derives from the dir byte (`Direction == 0`). * SCID() projects the 8-byte scid portion so the ChannelUpdate interface stays unchanged for callers. * SetSCID() updates the scid portion while leaving the existing dir byte in place. ChanEdgePolicyFromWire now derives ChannelEdgePolicy.SecondPeer from !upd.IsNode1() instead of the old upd.SecondPeer.IsSome() lookup. The downstream SecondPeer field on ChannelEdgePolicy stays the same shape since it is also used by ChannelUpdate1. The channel_update_2 test fixture now carries a 9-byte sciddir at type 2 and no longer has a type-8 SecondPeer record. The rapid property factory draws a single bool for the direction byte instead of separately drawing an isSecondPeer flag for the (now removed) second_peer field.

The gossip v2 spec types the outbound fee fields of channel_update_2 as tu32, its htlc_minimum_msat and htlc_maximum_msat as tu64, and the capacity_satoshis of channel_announcement_2 as tu64. These fields are older than this series and were fixed-width or BigSize, so lnd could not read a conformant message, and a conformant peer misread lnd's. Use the truncated encoding for all of them. truncatedUint32Record covers the fee fields. A new truncatedUint64Record covers the rest, and takes any type whose underlying type is uint64, such as MilliSatoshi. MilliSatoshi.Record keeps its BigSize form, which other messages rely on. The tests cover canonical and malformed values for the fee fields, and the fixtures carry the truncated bytes.

Gossip v2 signatures cover the exact TLV records sent. Decoding currently replaces absent fields with their defaults, while encoding decides whether to include those fields from their values. As a result, an explicitly encoded default is omitted when the message is re-encoded, changing the signed byte stream. Represent optional fields using OptionalRecordT and retain their presence during decoding. Apply protocol defaults only through message accessors, and emit every field that was present even when it contains its default.

…proot-gossip The branch behind this PR pulls the lnwire gossip v2 messages in line with the review-driven updates on the BOLT taproot-gossip extension (lightning/bolts#1059). Record the change in the 0.22.0 release notes under "BOLT Spec Updates" so the PR check is satisfied and downstream implementors get a heads-up about the wire-format shifts.

yyforyongyu

LGTM!

ViktorT-11 closed this Sep 3, 2026

ViktorT-11 changed the title ~~version: bump to v0.2.19~~ lnwire: pull gossip v2 messages in line with BOLT taproot-gossip review updates Sep 3, 2026

github-actions Bot added the severity-critical Requires expert review - security/consensus critical label Sep 3, 2026

ViktorT-11 reopened this Sep 3, 2026

ViktorT-11 mentioned this pull request Sep 3, 2026

lnwire: pull gossip v2 messages in line with BOLT taproot-gossip review updates #10837

Closed

5 of 6 tasks

ViktorT-11 force-pushed the 2026-08-sync-gossip-with-spec branch from ac6a7e6 to fa3d115 Compare September 3, 2026 08:57

saubyk assigned ViktorT-11 Sep 3, 2026

saubyk added this to the v0.22.0 milestone Sep 3, 2026

saubyk added this to lnd v0.22 Sep 3, 2026

github-project-automation Bot moved this to Backlog in lnd v0.22 Sep 3, 2026

saubyk moved this from Backlog to In progress in lnd v0.22 Sep 3, 2026

ViktorT-11 requested review from bitromortac and yyforyongyu September 5, 2026 07:33

bitromortac reviewed Sep 7, 2026

View reviewed changes

ViktorT-11 force-pushed the 2026-08-sync-gossip-with-spec branch 2 times, most recently from 237eb33 to d366b60 Compare September 14, 2026 14:44

ViktorT-11 requested a review from bitromortac September 14, 2026 14:44

ViktorT-11 force-pushed the 2026-08-sync-gossip-with-spec branch from d366b60 to 059c3ea Compare September 22, 2026 13:02

yyforyongyu requested changes Sep 23, 2026

View reviewed changes

bitromortac requested changes Sep 24, 2026

View reviewed changes

ViktorT-11 force-pushed the 2026-08-sync-gossip-with-spec branch 3 times, most recently from a10815f to 5082b81 Compare September 25, 2026 14:35

ViktorT-11 requested review from bitromortac and yyforyongyu September 25, 2026 18:39

yyforyongyu requested changes Oct 16, 2026

View reviewed changes

ViktorT-11 force-pushed the 2026-08-sync-gossip-with-spec branch 2 times, most recently from ba9e6db to f14c0c7 Compare October 19, 2026 18:42

ellemouton added 2 commits October 20, 2026 01:16

ViktorT-11 force-pushed the 2026-08-sync-gossip-with-spec branch from f14c0c7 to e396e72 Compare October 19, 2026 23:51

ViktorT-11 requested a review from yyforyongyu October 19, 2026 23:52

yyforyongyu requested changes Oct 20, 2026

View reviewed changes

bitromortac and others added 10 commits October 20, 2026 13:05

ViktorT-11 force-pushed the 2026-08-sync-gossip-with-spec branch from e396e72 to 675f6bd Compare October 20, 2026 11:06

ViktorT-11 requested a review from yyforyongyu October 20, 2026 11:11

bitromortac and others added 2 commits October 20, 2026 17:06

ViktorT-11 force-pushed the 2026-08-sync-gossip-with-spec branch from 675f6bd to f2165c5 Compare October 20, 2026 15:06

yyforyongyu approved these changes Oct 20, 2026

View reviewed changes

yyforyongyu merged commit 26b0467 into lightningnetwork:master Oct 20, 2026
43 checks passed

github-project-automation Bot moved this from In progress to Done in lnd v0.22 Oct 20, 2026

Conversation

ViktorT-11 commented Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Commits

Out of scope (deliberately)

Test plan

Uh oh!

github-actions Bot commented Sep 3, 2026

🔴 PR Severity: CRITICAL

Analysis

Uh oh!

bitromortac left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

litbot-9000 commented Sep 14, 2026

Uh oh!

litbot-9000 commented Sep 21, 2026

Uh oh!

yyforyongyu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bitromortac left a comment

Choose a reason for hiding this comment

ViktorT-11 commented Sep 3, 2026 •

edited

Loading

ViktorT-11 Sep 25, 2026 •

edited

Loading

ViktorT-11 commented Sep 25, 2026 •

edited

Loading