From c5cff4052b8340f2cff89ce1f19577b8838f2624 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Wed, 5 Feb 2025 08:17:42 +0200 Subject: [PATCH 1/5] lnwire_test: fix test doc string This test started out demonstrating a bug. But that bug has since been fixed. Fix the comment to reflect. --- lnwire/lnwire_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lnwire/lnwire_test.go b/lnwire/lnwire_test.go index f42426ac571..db3ffd39d69 100644 --- a/lnwire/lnwire_test.go +++ b/lnwire/lnwire_test.go @@ -356,8 +356,8 @@ func TestChanUpdateChanFlags(t *testing.T) { } } -// TestDecodeUnknownAddressType shows that an unknown address type is currently -// incorrectly dealt with. +// TestDecodeUnknownAddressType shows that an unknown address type is correctly +// decoded and encoded. func TestDecodeUnknownAddressType(t *testing.T) { // Add a normal, clearnet address. tcpAddr := &net.TCPAddr{ From b7509897d5aca573ee417e03b0997fed68dfa21c Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Wed, 5 Feb 2025 08:20:10 +0200 Subject: [PATCH 2/5] models: create a helper to convert wire NodeAnn to models.LNNode type And use it in the gossiper. This helps ensure that we do this conversion consistently. --- discovery/gossiper.go | 16 +--------------- graph/db/models/node.go | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/discovery/gossiper.go b/discovery/gossiper.go index 290e529bd9b..2afd38c28cf 100644 --- a/discovery/gossiper.go +++ b/discovery/gossiper.go @@ -1982,21 +1982,7 @@ func (d *AuthenticatedGossiper) addNode(msg *lnwire.NodeAnnouncement, err) } - timestamp := time.Unix(int64(msg.Timestamp), 0) - features := lnwire.NewFeatureVector(msg.Features, lnwire.Features) - node := &models.LightningNode{ - HaveNodeAnnouncement: true, - LastUpdate: timestamp, - Addresses: msg.Addresses, - PubKeyBytes: msg.NodeID, - Alias: msg.Alias.String(), - AuthSigBytes: msg.Signature.ToSignatureBytes(), - Features: features, - Color: msg.RGBColor, - ExtraOpaqueData: msg.ExtraOpaqueData, - } - - return d.cfg.Graph.AddNode(node, op...) + return d.cfg.Graph.AddNode(models.NodeFromWireAnnouncement(msg), op...) } // isPremature decides whether a given network message has a block height+delta diff --git a/graph/db/models/node.go b/graph/db/models/node.go index 96241543391..ee769ddb69a 100644 --- a/graph/db/models/node.go +++ b/graph/db/models/node.go @@ -131,3 +131,22 @@ func (l *LightningNode) NodeAnnouncement(signed bool) (*lnwire.NodeAnnouncement, return nodeAnn, nil } + +// NodeFromWireAnnouncement creates a LightningNode instance from an +// lnwire.NodeAnnouncement message. +func NodeFromWireAnnouncement(msg *lnwire.NodeAnnouncement) *LightningNode { + timestamp := time.Unix(int64(msg.Timestamp), 0) + features := lnwire.NewFeatureVector(msg.Features, lnwire.Features) + + return &LightningNode{ + HaveNodeAnnouncement: true, + LastUpdate: timestamp, + Addresses: msg.Addresses, + PubKeyBytes: msg.NodeID, + Alias: msg.Alias.String(), + AuthSigBytes: msg.Signature.ToSignatureBytes(), + Features: features, + Color: msg.RGBColor, + ExtraOpaqueData: msg.ExtraOpaqueData, + } +} From d68d24d97e2d11578d2c0a347e1cf0960fb8372c Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Wed, 5 Feb 2025 08:24:45 +0200 Subject: [PATCH 3/5] graph/db: demonstrate LightningNode serialisation bug --- graph/db/graph_test.go | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/graph/db/graph_test.go b/graph/db/graph_test.go index 8a02f24ff41..77463982984 100644 --- a/graph/db/graph_test.go +++ b/graph/db/graph_test.go @@ -4063,3 +4063,45 @@ func TestClosedScid(t *testing.T) { require.Nil(t, err) require.True(t, exists) } + +// testNodeAnn is a serialized node announcement message which contains an +// address type (6) that LND is not aware of. +var testNodeAnn = "01012674c2e7ef68c73a086b7de2603f4ef1567358df84bb4edaa06c" + + "f2132965b14e2434faab04170f0089216accbd79188fa3d40dbb0438bd89782cae" + + "27cc656bf60007800088082a69a2625e7a2a024b9a1fa8e006f1e3937f65f66c40" + + "8e6da8e1ca728ea43222a7381df1cc449605024b9a424c554549524f4e2d76302e" + + "31312e307263332d362d67663963613934650000001d0180c7caa8260702240061" + + "80000000d0000000005cd2a001260706204c" + +// TestLightningNodePersistence takes a raw serialized node announcement +// message, converts it to our internal models.LightningNode type and attempts +// to persist this to disk. +// +// NOTE: Currently, this tests demonstrates that we are _unable_ to do this if +// the node announcement has an address type unknown to LND. This will be fixed +// in an upcoming commit. +func TestLightningNodePersistence(t *testing.T) { + t.Parallel() + + // Create a new test graph instance. + graph, err := MakeTestGraph(t) + require.NoError(t, err) + + nodeAnnBytes, err := hex.DecodeString(testNodeAnn) + require.NoError(t, err) + + // Use the raw serialized node announcement message create an + // lnwire.NodeAnnouncement instance. + msg, err := lnwire.ReadMessage(bytes.NewBuffer(nodeAnnBytes), 0) + require.NoError(t, err) + na, ok := msg.(*lnwire.NodeAnnouncement) + require.True(t, ok) + + // Convert the wire message to our internal node representation. + node := models.NodeFromWireAnnouncement(na) + + // Attempt to persist the node to disk. This currently fails due to the + // unknown address type. + err = graph.AddLightningNode(node) + require.ErrorContains(t, err, "address type cannot be resolved") +} From 71b2338d531409f2baa66f8a1529ab04923862f7 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Wed, 5 Feb 2025 08:29:25 +0200 Subject: [PATCH 4/5] graph/db: de(ser)ialise opaque node addrs In this commit, we fix the bug demonstrated in the prior commit. We correctly handle the persistence of lnwire.OpaqueAddrs. --- graph/db/addr.go | 49 ++++++++++++++++++++++++++++++++++++++++++ graph/db/graph_test.go | 29 +++++++++++++++++-------- 2 files changed, 69 insertions(+), 9 deletions(-) diff --git a/graph/db/addr.go b/graph/db/addr.go index f9941315822..c68039a2624 100644 --- a/graph/db/addr.go +++ b/graph/db/addr.go @@ -7,6 +7,7 @@ import ( "io" "net" + "github.com/lightningnetwork/lnd/lnwire" "github.com/lightningnetwork/lnd/tor" ) @@ -26,6 +27,10 @@ const ( // v3OnionAddr denotes a version 3 Tor (prop224) onion service address. v3OnionAddr addressType = 3 + + // opaqueAddrs denotes an address (or a set of addresses) that LND was + // not able to parse since LND is not yet aware of the address type. + opaqueAddrs addressType = 4 ) // encodeTCPAddr serializes a TCP address into its compact raw bytes @@ -121,6 +126,27 @@ func encodeOnionAddr(w io.Writer, addr *tor.OnionAddr) error { return nil } +// encodeOpaqueAddrs serializes the lnwire.OpaqueAddrs type to a raw set of +// bytes that we will persist. +func encodeOpaqueAddrs(w io.Writer, addr *lnwire.OpaqueAddrs) error { + // Write the type byte. + if _, err := w.Write([]byte{byte(opaqueAddrs)}); err != nil { + return err + } + + // Write the length of the payload. + var l [2]byte + binary.BigEndian.PutUint16(l[:], uint16(len(addr.Payload))) + if _, err := w.Write(l[:]); err != nil { + return err + } + + // Write the payload. + _, err := w.Write(addr.Payload) + + return err +} + // DeserializeAddr reads the serialized raw representation of an address and // deserializes it into the actual address. This allows us to avoid address // resolution within the channeldb package. @@ -147,6 +173,7 @@ func DeserializeAddr(r io.Reader) (net.Addr, error) { IP: net.IP(ip[:]), Port: int(binary.BigEndian.Uint16(port[:])), } + case tcp6Addr: var ip [16]byte if _, err := r.Read(ip[:]); err != nil { @@ -162,6 +189,7 @@ func DeserializeAddr(r io.Reader) (net.Addr, error) { IP: net.IP(ip[:]), Port: int(binary.BigEndian.Uint16(port[:])), } + case v2OnionAddr: var h [tor.V2DecodedLen]byte if _, err := r.Read(h[:]); err != nil { @@ -181,6 +209,7 @@ func DeserializeAddr(r io.Reader) (net.Addr, error) { OnionService: onionService, Port: port, } + case v3OnionAddr: var h [tor.V3DecodedLen]byte if _, err := r.Read(h[:]); err != nil { @@ -200,6 +229,24 @@ func DeserializeAddr(r io.Reader) (net.Addr, error) { OnionService: onionService, Port: port, } + + case opaqueAddrs: + // Read the length of the payload. + var l [2]byte + if _, err := r.Read(l[:]); err != nil { + return nil, err + } + + // Read the payload. + payload := make([]byte, binary.BigEndian.Uint16(l[:])) + if _, err := r.Read(payload); err != nil { + return nil, err + } + + address = &lnwire.OpaqueAddrs{ + Payload: payload, + } + default: return nil, ErrUnknownAddressType } @@ -215,6 +262,8 @@ func SerializeAddr(w io.Writer, address net.Addr) error { return encodeTCPAddr(w, addr) case *tor.OnionAddr: return encodeOnionAddr(w, addr) + case *lnwire.OpaqueAddrs: + return encodeOpaqueAddrs(w, addr) default: return ErrUnknownAddressType } diff --git a/graph/db/graph_test.go b/graph/db/graph_test.go index 77463982984..d048cdafd12 100644 --- a/graph/db/graph_test.go +++ b/graph/db/graph_test.go @@ -4074,12 +4074,9 @@ var testNodeAnn = "01012674c2e7ef68c73a086b7de2603f4ef1567358df84bb4edaa06c" + "80000000d0000000005cd2a001260706204c" // TestLightningNodePersistence takes a raw serialized node announcement -// message, converts it to our internal models.LightningNode type and attempts -// to persist this to disk. -// -// NOTE: Currently, this tests demonstrates that we are _unable_ to do this if -// the node announcement has an address type unknown to LND. This will be fixed -// in an upcoming commit. +// message, converts it to our internal models.LightningNode type, persists it +// to disk, reads it again and converts it back to a wire message and asserts +// that the two messages are equal. func TestLightningNodePersistence(t *testing.T) { t.Parallel() @@ -4100,8 +4097,22 @@ func TestLightningNodePersistence(t *testing.T) { // Convert the wire message to our internal node representation. node := models.NodeFromWireAnnouncement(na) - // Attempt to persist the node to disk. This currently fails due to the - // unknown address type. + // Persist the node to disk. err = graph.AddLightningNode(node) - require.ErrorContains(t, err, "address type cannot be resolved") + require.NoError(t, err) + + // Read the node from disk. + diskNode, err := graph.FetchLightningNode(node.PubKeyBytes) + require.NoError(t, err) + + // Convert it back to a wire message. + wireMsg, err := diskNode.NodeAnnouncement(true) + require.NoError(t, err) + + // Encode it and compare against the original. + var b bytes.Buffer + _, err = lnwire.WriteMessage(&b, wireMsg, 0) + require.NoError(t, err) + + require.Equal(t, nodeAnnBytes, b.Bytes()) } From 16e2a48d0fb0bef3941e738f3f4994b94902f102 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Wed, 5 Feb 2025 08:32:03 +0200 Subject: [PATCH 5/5] docs: update release notes --- docs/release-notes/release-notes-0.19.0.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/release-notes/release-notes-0.19.0.md b/docs/release-notes/release-notes-0.19.0.md index 1a5992228e7..52e229004af 100644 --- a/docs/release-notes/release-notes-0.19.0.md +++ b/docs/release-notes/release-notes-0.19.0.md @@ -64,6 +64,10 @@ * [Fixed a bug](https://github.com/lightningnetwork/lnd/pull/9322) that caused estimateroutefee to ignore the default payment timeout. +* [Fix a bug](https://github.com/lightningnetwork/lnd/pull/9474) where LND would + fail to persist (and hence, propagate) node announcements containing address + types (such as a DNS hostname) unknown to LND. + # New Features * [Support](https://github.com/lightningnetwork/lnd/pull/8390) for