lnwire+netann: update structure of g175 messages to be pure TLV #9175

ellemouton · 2024-10-11T14:42:39Z

Update the structure of all gossip 1.75 messages to be purely TLV.
With this update, the signature is also a TLV record. There is a un-signed
and signed range of TLVs. The signature along with things like SPV proofs
will fall in the un-signed range.
I've used the same un-signed range used by Bolt12 messages.

This PR introduces a new PureTLVMessage interface for messages like this
and adds a comprehensive test to ensure that old clients (who are aware of
fewer fields of a message than new clients) and new clients still generate the
same correct digest of a message regardless of if they are aware of all the
records or not.

Bonus Commit

Ok i've also now updated this PR to include validating the signature of a
ChannelAnnouncement2 message for the case where it is announcing a P2WSH channel.

ellemouton · 2024-10-16T11:26:17Z

Ok i've also now updated this PR to include validating the signature of a
ChannelAnnouncement2 message for the case where it is announcing a P2WSH channel.

PureTLVMessage describes an LN message that is a pure TLV stream. If the message includes a signature, it will sign all the TLV records in the inclusive ranges: 0 to 159 and 1000000000 to 2999999999. A comprehensive test is added that shows how two versions of the same message remain forward compatible.

So that we can use it as a TLV record type.

Such that all fields are now TLV (including the signature).

such that all fields are now TLV including the signature.

This commit expands the ChannelAnnouncement2 validation for the case where it is announcing a P2WSH channel.

Roasbeef · 2024-10-16T22:43:44Z

go.mod

@@ -210,3 +210,5 @@ replace google.golang.org/protobuf => github.com/lightninglabs/protobuf-go-hex-d
 go 1.22.6

 retract v0.0.2
+
+replace github.com/lightningnetwork/lnd/tlv => github.com/ellemouton/lnd/tlv v0.0.0-20241012094556-298ff9eaed58


I think we can remove this now?

Roasbeef · 2024-10-16T22:44:26Z

lnwire/pure_tlv.go

+)
+
+const (
+	pureTLVUnsignedRangeOneStart  = 160


Should have godoc comments.

Roasbeef · 2024-10-16T22:44:52Z

lnwire/pure_tlv.go

+const (
+	pureTLVUnsignedRangeOneStart  = 160
+	pureTLVSignedSecondRangeStart = 1000000000
+	pureTLVUnsignedRangeTwoStart  = 3000000000


How did we arrive at these values?

Also I think we should try to capture this in the spec, independent of G 1.75.

Roasbeef · 2024-10-16T22:46:52Z

lnwire/pure_tlv.go

+
+// SerialiseFieldsToSign serialises all the records from the given
+// PureTLVMessage that fall within the signed TLV range.
+func SerialiseFieldsToSign(msg PureTLVMessage) ([]byte, error) {


We might want to adopt a similar structure for the signed portion of the dyn commit messages @ProofOfKeags. Basically we copied over from the way BOLT 12 works, and we can use the TLV type values, then let this handle how serialization works (does require a message to be pure TLV).

Roasbeef · 2024-10-16T22:50:36Z

lnwire/pure_tlv_test.go

+	// Any extra fields in the signed range that we do not yet know about,
+	// but we need to keep them for signature validation and to produce a
+	// valid message.
+	ExtraFieldsInSignedRange map[uint64][]byte


Should something like this be split out into an embeddable struct?

Roasbeef · 2024-10-16T22:54:27Z

lnwire/announcement_signatures_2.go

+	// Any extra fields in the signed range that we do not yet know about,
+	// but we need to keep them for signature validation and to produce a
+	// valid message.
+	ExtraFieldsInSignedRange map[uint64][]byte


Also why can't we just continue to store it in the serialized form?

Roasbeef · 2024-10-16T22:54:56Z

discovery/gossiper.go

+	// AuthenticatedGossiper. Any announcements that don't match this
+	// chain hash will be ignored. This is an internal config value obtained
+	// from ChainParams.
+	chainHash *chainhash.Hash


Why do we need it here if we have ChainParams?

Roasbeef · 2024-10-16T22:55:02Z

discovery/gossiper.go

-	ChainHash chainhash.Hash
+	// ChainParams holds the chain parameters for the active network this
+	// node is participating on.
+	ChainParams *chaincfg.Params


I think this can be a normal value rather than a pointer.

Roasbeef · 2024-10-16T22:55:38Z

discovery/gossiper.go

-	[]byte, error) {
+	txscript.ScriptClass, btcutil.Address, error) {
+
+	pkScript, err := lnwallet.FetchPKScriptWithQuit(


What about the merkle proof case?

Roasbeef · 2024-10-16T22:55:56Z

netann/channel_announcement.go

@@ -108,7 +110,8 @@ func CreateChanAnnouncement(chanProof *models.ChannelAuthProof,

 // FetchPkScript defines a function that can be used to fetch the output script
 // for the transaction with the given SCID.
-type FetchPkScript func(*lnwire.ShortChannelID) ([]byte, error)
+type FetchPkScript func(*lnwire.ShortChannelID) (txscript.ScriptClass,


Does the arg here need to be a pointer?

lightninglabs-deploy · 2024-11-28T01:39:09Z

@ellemouton, remember to re-request review from reviewers when ready

ellemouton self-assigned this Oct 11, 2024

ellemouton added the gossip label Oct 11, 2024

ellemouton force-pushed the g175UpdateMessageStructure branch 2 times, most recently from ee6ff8c to 4160fd5 Compare October 12, 2024 10:01

go.mod: update TLV package dep

20db079

ellemouton force-pushed the g175UpdateMessageStructure branch from 4160fd5 to 5d3c965 Compare October 16, 2024 11:24

ellemouton added 6 commits October 16, 2024 13:39

lnwire: let ChannelID implement RecordProducer

321083c

So that we can use it as a TLV record type.

lnwire+netann: update ChannelAnnouncement2 structure

df034bc

Such that all fields are now TLV (including the signature).

lnwire+netann: update ChannelUpdate2

7483ed7

such that all fields are now TLV including the signature.

lnwire: update AnnounceSigs2 to use pure TLV

22293d5

netann: update ChanAnn2 validation to work for P2WSH channels

6f0ce3b

This commit expands the ChannelAnnouncement2 validation for the case where it is announcing a P2WSH channel.

ellemouton force-pushed the g175UpdateMessageStructure branch from 5d3c965 to 6f0ce3b Compare October 16, 2024 11:39

Roasbeef requested changes Oct 16, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

lnwire+netann: update structure of g175 messages to be pure TLV #9175

lnwire+netann: update structure of g175 messages to be pure TLV #9175

ellemouton commented Oct 11, 2024 •

edited

Loading

ellemouton commented Oct 16, 2024

Roasbeef Oct 16, 2024

Roasbeef Oct 16, 2024

Roasbeef Oct 16, 2024

Roasbeef Oct 16, 2024

Roasbeef Oct 16, 2024

Roasbeef Oct 16, 2024

Roasbeef Oct 16, 2024

Roasbeef Oct 16, 2024

Roasbeef Oct 16, 2024

Roasbeef Oct 16, 2024

Roasbeef Oct 16, 2024

lightninglabs-deploy commented Nov 28, 2024

lnwire+netann: update structure of g175 messages to be pure TLV #9175

Are you sure you want to change the base?

lnwire+netann: update structure of g175 messages to be pure TLV #9175

Conversation

ellemouton commented Oct 11, 2024 • edited Loading

Bonus Commit

ellemouton commented Oct 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lightninglabs-deploy commented Nov 28, 2024

ellemouton commented Oct 11, 2024 •

edited

Loading