unwrap dns addresses from opaque ones during migration

graph/db/addr_test.go

@@ -46,6 +46,71 @@ var (
 		Payload: []byte{0xff, 0x02, 0x03, 0x04, 0x05, 0x06},
 	}
 
+	testOpaqueAddrWithEmbeddedDNSAddr = &lnwire.OpaqueAddrs{
+		Payload: []byte{
+			/* Here we embed an actual DNS address */
+			// The protocol level type for DNS addresses.
+			0x05,
+			// Hostname length: 11.
+			0x0B,
+			// The hostname itself.
+			'e', 'x', 'a', 'm', 'p', 'l', 'e', '.', 'c', 'o', 'm',
+			// port 8080 in big-endian.
+			0x1F, 0x90,
+		},
+	}
+	testOpaqueAddrWithTwoEmbeddedDNSAddrs = &lnwire.OpaqueAddrs{
+		Payload: []byte{
+			/* Here we embed an actual DNS address */
+			// The protocol level type for DNS addresses.
+			0x05,
+			// Hostname length: 11.
+			0x0B,
+			// The hostname itself.
+			'e', 'x', 'a', 'm', 'p', 'l', 'e', '.', 'c', 'o', 'm',
+			// port 8080 in big-endian.
+			0x1F, 0x90,
+			// Another DNS address.
+			0x05,
+			0x0B,
+			'e', 'x', 'a', 'm', 'p', 'l', 'e', '.', 'c', 'o', 'm',
+			0x1F, 0x90,
+		},
+	}
+	testOpaqueAddrWithEmbeddedDNSAddrAndMore = &lnwire.OpaqueAddrs{
+		Payload: []byte{
+			/* Here we embed an actual DNS address */
+			// The protocol level type for DNS addresses.
+			0x05,
+			// Hostname length: 11.
+			0x0B,
+			// The hostname itself.
+			'e', 'x', 'a', 'm', 'p', 'l', 'e', '.', 'c', 'o', 'm',
+			// port 8080 in big-endian.
+			0x1F, 0x90,
+			// Now we add more opaque bytes to represent more
+			// addresses that we dont know about yet.
+			// NOTE: the 0xff is an address type that we definitely
+			// don't know about yet
+			0xff, 0x02, 0x03, 0x04, 0x05, 0x06,
+		},
+	}
+
+	testOpaqueAddrWithEmbeddedBadDNSAddr = &lnwire.OpaqueAddrs{
+		Payload: []byte{
+			/* Here we embed an actual invalid DNS address */
+			// The protocol level type for DNS addresses.
+			0x05,
+			// Hostname length: We set this to a size that is
+			// incorrect in order to simulate the bad DNS address.
+			0xAA,
+			// The hostname itself.
+			'e', 'x', 'a', 'm', 'p', 'l', 'e', '.', 'c', 'o', 'm',
+			// port 9735 in big-endian.
+			0x26, 0x07,
+		},
+	}
+
 	testDNSAddr = &lnwire.DNSAddress{
 		Hostname: "example.com",
 		Port:     8080,

graph/db/sql_migration.go

@@ -263,14 +263,41 @@ func migrateNodes(ctx context.Context, cfg *sqldb.QueryConfig,
 		count++
 		chunk++
 
-		// TODO(elle): At this point, we should check the loaded node
-		// to see if we should extract any DNS addresses from its
-		// opaque type addresses. This is expected to be done in:
-		// https://github.com/lightningnetwork/lnd/pull/9455.
-		// This TODO is being tracked in
-		//  https://github.com/lightningnetwork/lnd/issues/9795 as this
-		// must be addressed before making this code path active in
-		// production.
+		addrs := make([]net.Addr, 0, len(node.Addresses))
+		for _, addr := range node.Addresses {
+			opaque, ok := addr.(*lnwire.OpaqueAddrs)
+			if !ok {
+				addrs = append(addrs, addr)
+				continue
+			}
+
+			r := bytes.NewReader(opaque.Payload)
+			numAddrBytes := uint16(len(opaque.Payload))
+			byteRead, readAddr, err := lnwire.ReadAddress(
+				r, numAddrBytes,
+			)
+			if err != nil {
+				// We then just keep the address as opaque.
+				addrs = append(addrs, addr)
+
+				continue
+			}
+
+			if readAddr != nil {
+				addrs = append(addrs, readAddr)
+			}
+
+			if byteRead == numAddrBytes {
+				continue
+			}
+
+			addrs = append(addrs, &lnwire.OpaqueAddrs{
+				Payload: opaque.Payload[byteRead:],
+			})
+		}
+		if len(addrs) != 0 {
+			node.Addresses = addrs
+		}
 
 		// Write the node to the SQL database.
 		id, err := upsertNode(ctx, sqlDB, node)

graph/db/sql_migration_test.go

@@ -124,6 +124,7 @@ func TestMigrateGraphToSQL(t *testing.T) {
 						anotherAddr,
 						testOnionV2Addr,
 						testOnionV3Addr,
+						testDNSAddr,
 						testOpaqueAddr,
 					}
 				}),
@@ -884,6 +885,100 @@ func TestSQLMigrationEdgeCases(t *testing.T) {
 		})
 	})
 
+	t.Run("node with wrapped DNS address inside opaque addr",
+		func(t *testing.T) {
+			t.Parallel()
+
+			// Let the first node have an opaque address that we
+			// still don't understand. This node will remain the
+			// same in the SQL store.
+			n1 := makeTestNode(t, func(n *models.LightningNode) {
+				n.Addresses = []net.Addr{
+					testOpaqueAddr,
+				}
+			})
+
+			// The second node will have a wrapped DNS address
+			// inside an opaque address. The opaque address will
+			// only contain a DNS address and so the migrated node
+			// will only contain a DNS address and no opaque
+			// address.
+			n2 := makeTestNode(t, func(n *models.LightningNode) {
+				n.Addresses = []net.Addr{
+					testOpaqueAddrWithEmbeddedDNSAddr,
+				}
+			})
+			n2Expected := *n2
+			n2Expected.Addresses = []net.Addr{
+				testDNSAddr,
+			}
+
+			// The third node will have an opaque address that
+			// wraps a DNS address along with some other data.
+			// So the resulting migrated node should have both
+			// the DNS address and remaining opaque address data.
+			n3 := makeTestNode(t, func(n *models.LightningNode) {
+				n.Addresses = []net.Addr{
+					testOpaqueAddrWithEmbeddedDNSAddrAndMore,
+				}
+			})
+			n3Expected := *n3
+			n3Expected.Addresses = []net.Addr{
+				testDNSAddr,
+				testOpaqueAddr,
+			}
+
+			// The fourth node will have an opaque address that
+			// wraps an invalid DNS address. Such a node will be
+			// migrated, but we will keep the address as an opaque
+			// address in the SQL store.
+			n4 := makeTestNode(t, func(n *models.LightningNode) {
+				n.Addresses = []net.Addr{
+					testOpaqueAddrWithEmbeddedBadDNSAddr,
+				}
+			})
+
+			// The fifth node will have 2 DNS addresses embedded
+			// in the opaque address. The migration will result
+			// in the first dns address being extracted and the
+			// second one being left as an opaque address.
+			n5 := makeTestNode(t, func(n *models.LightningNode) {
+				n.Addresses = []net.Addr{
+					testOpaqueAddrWithTwoEmbeddedDNSAddrs,
+				}
+			})
+			n5Expected := *n5
+			n5Expected.Addresses = []net.Addr{
+				testDNSAddr,
+				testOpaqueAddrWithEmbeddedDNSAddr,
+			}
+
+			populateKV := func(t *testing.T, db *KVStore) {
+				require.NoError(t, db.AddLightningNode(ctx, n1))
+				require.NoError(t, db.AddLightningNode(ctx, n2))
+				require.NoError(t, db.AddLightningNode(ctx, n3))
+				require.NoError(t, db.AddLightningNode(ctx, n4))
+				require.NoError(t, db.AddLightningNode(ctx, n5))
+			}
+
+			runTestMigration(t, populateKV, dbState{
+				nodes: []*models.LightningNode{
+					n1,
+					&n2Expected,
+					&n3Expected,
+					n4,
+					&n5Expected,
+				},
+			})
+
+			/*
+
+
+
+			 */
+		},
+	)
+
 	// Here, we test that in the case where the KV store contains a channel
 	// with invalid TLV data, the migration will still succeed, but the
 	// channel and its policies will not end up in the SQL store.
@@ -1147,6 +1242,10 @@ type dbState struct {
 // assertResultState asserts that the SQLStore contains the expected
 // state after a migration.
 func assertResultState(t *testing.T, sql *SQLStore, expState dbState) {
+	for _, node := range expState.nodes {
+		sortAddrs(node.Addresses)
+	}
+
 	// Assert that the sql store contains the expected nodes.
 	require.ElementsMatch(t, expState.nodes, fetchAllNodes(t, sql))
 	require.ElementsMatch(

lnwire/lnwire.go

@@ -719,9 +719,42 @@ func ReadElement(r io.Reader, element interface{}) error {
 		}
 
 	case *[]net.Addr:
-		addresses, err := ReadAddresses(r)
-		if err != nil {
-			return fmt.Errorf("unable to read addresses: %w", err)
+		// First, we'll read the number of total bytes that have been
+		// used to encode the set of addresses.
+		var numAddrsBytes [2]byte
+		if _, err := io.ReadFull(r, numAddrsBytes[:]); err != nil {
+			return err
+		}
+		addrsLen := binary.BigEndian.Uint16(numAddrsBytes[:])
+
+		// With the number of addresses, read, we'll now pull in the
+		// buffer of the encoded addresses into memory.
+		addrs := make([]byte, addrsLen)
+		if _, err := io.ReadFull(r, addrs); err != nil {
+			return err
+		}
+		addrBuf := bytes.NewReader(addrs)
+
+		// Finally, we'll parse the remaining address payload in
+		// series, using the first byte to denote how to decode the
+		// address itself.
+		var (
+			addresses     []net.Addr
+			addrBytesRead uint16
+		)
+
+		for addrBytesRead < addrsLen {
+			bytesRead, address, err := ReadAddress(
+				addrBuf, addrsLen-addrBytesRead,
+			)
+			if err != nil {
+				return fmt.Errorf("unable to read address: %w",
+					err)
+			}
+			addrBytesRead += bytesRead
+			if address != nil {
+				addresses = append(addresses, address)
+			}
 		}
 
 		*e = addresses
@@ -785,179 +818,148 @@ func ReadElements(r io.Reader, elements ...interface{}) error {
 	return nil
 }
 
-// ReadAddresses reads encoded network addresses.
-//
-//nolint:funlen
-func ReadAddresses(r io.Reader) ([]net.Addr, error) {
-	// First, we'll read the number of total bytes that have been
-	// used to encode the set of addresses.
-	var numAddrsBytes [2]byte
-	if _, err := io.ReadFull(r, numAddrsBytes[:]); err != nil {
-		return nil, err
+func ReadAddress(addrBuf io.Reader, addrsLen uint16) (uint16, net.Addr, error) {
+	var descriptor [1]byte
+	if _, err := io.ReadFull(addrBuf, descriptor[:]); err != nil {
+		return 0, nil, err
 	}
-	addrsLen := binary.BigEndian.Uint16(numAddrsBytes[:])
 
-	// With the number of addresses, read, we'll now pull in the
-	// buffer of the encoded addresses into memory.
-	addrs := make([]byte, addrsLen)
-	if _, err := io.ReadFull(r, addrs); err != nil {
-		return nil, err
-	}
-	addrBuf := bytes.NewReader(addrs)
+	addrBytesRead := uint16(1)
 
-	// Finally, we'll parse the remaining address payload in
-	// series, using the first byte to denote how to decode the
-	// address itself.
-	var (
-		addresses     []net.Addr
-		addrBytesRead uint16
-	)
+	var address net.Addr
+	switch aType := addressType(descriptor[0]); aType {
+	case noAddr:
+		return addrBytesRead, nil, nil
 
-	for addrBytesRead < addrsLen {
-		var descriptor [1]byte
-		if _, err := io.ReadFull(addrBuf, descriptor[:]); err != nil {
-			return nil, err
+	case tcp4Addr:
+		var ip [4]byte
+		if _, err := io.ReadFull(addrBuf, ip[:]); err != nil {
+			return 0, nil, err
 		}
 
-		addrBytesRead++
-
-		var address net.Addr
-		switch aType := addressType(descriptor[0]); aType {
-		case noAddr:
-			continue
-
-		case tcp4Addr:
-			var ip [4]byte
-			if _, err := io.ReadFull(addrBuf, ip[:]); err != nil {
-				return nil, err
-			}
-
-			var port [2]byte
-			if _, err := io.ReadFull(addrBuf, port[:]); err != nil {
-				return nil, err
-			}
-
-			address = &net.TCPAddr{
-				IP:   net.IP(ip[:]),
-				Port: int(binary.BigEndian.Uint16(port[:])),
-			}
-			addrBytesRead += tcp4AddrLen
-
-		case tcp6Addr:
-			var ip [16]byte
-			if _, err := io.ReadFull(addrBuf, ip[:]); err != nil {
-				return nil, err
-			}
-
-			var port [2]byte
-			if _, err := io.ReadFull(addrBuf, port[:]); err != nil {
-				return nil, err
-			}
-
-			address = &net.TCPAddr{
-				IP:   net.IP(ip[:]),
-				Port: int(binary.BigEndian.Uint16(port[:])),
-			}
-			addrBytesRead += tcp6AddrLen
-
-		case v2OnionAddr:
-			var h [tor.V2DecodedLen]byte
-			if _, err := io.ReadFull(addrBuf, h[:]); err != nil {
-				return nil, err
-			}
-
-			var p [2]byte
-			if _, err := io.ReadFull(addrBuf, p[:]); err != nil {
-				return nil, err
-			}
-
-			onionService := tor.Base32Encoding.EncodeToString(h[:])
-			onionService += tor.OnionSuffix
-			port := int(binary.BigEndian.Uint16(p[:]))
-
-			address = &tor.OnionAddr{
-				OnionService: onionService,
-				Port:         port,
-			}
-			addrBytesRead += v2OnionAddrLen
-
-		case v3OnionAddr:
-			var h [tor.V3DecodedLen]byte
-			if _, err := io.ReadFull(addrBuf, h[:]); err != nil {
-				return nil, err
-			}
-
-			var p [2]byte
-			if _, err := io.ReadFull(addrBuf, p[:]); err != nil {
-				return nil, err
-			}
-
-			onionService := tor.Base32Encoding.EncodeToString(h[:])
-			onionService += tor.OnionSuffix
-			port := int(binary.BigEndian.Uint16(p[:]))
-
-			address = &tor.OnionAddr{
-				OnionService: onionService,
-				Port:         port,
-			}
-			addrBytesRead += v3OnionAddrLen
-
-		case dnsAddr:
-			var hostnameLen [1]byte
-			_, err := io.ReadFull(addrBuf, hostnameLen[:])
-			if err != nil {
-				return nil, err
-			}
-
-			hostname := make([]byte, hostnameLen[0])
-			_, err = io.ReadFull(addrBuf, hostname)
-			if err != nil {
-				return nil, err
-			}
-
-			var port [2]byte
-			_, err = io.ReadFull(addrBuf, port[:])
-			if err != nil {
-				return nil, err
-			}
-
-			address = &DNSAddress{
-				Hostname: string(hostname),
-				Port: binary.BigEndian.Uint16(
-					port[:],
-				),
-			}
-			addrBytesRead += dnsAddrOverhead +
-				uint16(len(hostname))
-
-		default:
-			// If we don't understand this address type,
-			// we just store it along with the remaining
-			// address bytes as type OpaqueAddrs. We need
-			// to hold onto the bytes so that we can still
-			// write them back to the wire when we
-			// propagate this message.
-			payloadLen := 1 + addrsLen - addrBytesRead
-			payload := make([]byte, payloadLen)
-
-			// First write a byte for the address type that
-			// we already read.
-			payload[0] = byte(aType)
-
-			// Now append the rest of the address bytes.
-			_, err := io.ReadFull(addrBuf, payload[1:])
-			if err != nil {
-				return nil, err
-			}
-
-			address = &OpaqueAddrs{
-				Payload: payload,
-			}
-			addrBytesRead = addrsLen
+		var port [2]byte
+		if _, err := io.ReadFull(addrBuf, port[:]); err != nil {
+			return 0, nil, err
 		}
 
-		addresses = append(addresses, address)
+		address = &net.TCPAddr{
+			IP:   net.IP(ip[:]),
+			Port: int(binary.BigEndian.Uint16(port[:])),
+		}
+		addrBytesRead += tcp4AddrLen
+
+	case tcp6Addr:
+		var ip [16]byte
+		if _, err := io.ReadFull(addrBuf, ip[:]); err != nil {
+			return 0, nil, err
+		}
+
+		var port [2]byte
+		if _, err := io.ReadFull(addrBuf, port[:]); err != nil {
+			return 0, nil, err
+		}
+
+		address = &net.TCPAddr{
+			IP:   net.IP(ip[:]),
+			Port: int(binary.BigEndian.Uint16(port[:])),
+		}
+		addrBytesRead += tcp6AddrLen
+
+	case v2OnionAddr:
+		var h [tor.V2DecodedLen]byte
+		if _, err := io.ReadFull(addrBuf, h[:]); err != nil {
+			return 0, nil, err
+		}
+
+		var p [2]byte
+		if _, err := io.ReadFull(addrBuf, p[:]); err != nil {
+			return 0, nil, err
+		}
+
+		onionService := tor.Base32Encoding.EncodeToString(h[:])
+		onionService += tor.OnionSuffix
+		port := int(binary.BigEndian.Uint16(p[:]))
+
+		address = &tor.OnionAddr{
+			OnionService: onionService,
+			Port:         port,
+		}
+		addrBytesRead += v2OnionAddrLen
+
+	case v3OnionAddr:
+		var h [tor.V3DecodedLen]byte
+		if _, err := io.ReadFull(addrBuf, h[:]); err != nil {
+			return 0, nil, err
+		}
+
+		var p [2]byte
+		if _, err := io.ReadFull(addrBuf, p[:]); err != nil {
+			return 0, nil, err
+		}
+
+		onionService := tor.Base32Encoding.EncodeToString(h[:])
+		onionService += tor.OnionSuffix
+		port := int(binary.BigEndian.Uint16(p[:]))
+
+		address = &tor.OnionAddr{
+			OnionService: onionService,
+			Port:         port,
+		}
+		addrBytesRead += v3OnionAddrLen
+
+	case dnsAddr:
+		var hostnameLen [1]byte
+		_, err := io.ReadFull(addrBuf, hostnameLen[:])
+		if err != nil {
+			return 0, nil, err
+		}
+
+		hostname := make([]byte, hostnameLen[0])
+		_, err = io.ReadFull(addrBuf, hostname)
+		if err != nil {
+			return 0, nil, err
+		}
+
+		var port [2]byte
+		_, err = io.ReadFull(addrBuf, port[:])
+		if err != nil {
+			return 0, nil, err
+		}
+
+		address = &DNSAddress{
+			Hostname: string(hostname),
+			Port: binary.BigEndian.Uint16(
+				port[:],
+			),
+		}
+		addrBytesRead += dnsAddrOverhead +
+			uint16(len(hostname))
+
+	default:
+		// If we don't understand this address type,
+		// we just store it along with the remaining
+		// address bytes as type OpaqueAddrs. We need
+		// to hold onto the bytes so that we can still
+		// write them back to the wire when we
+		// propagate this message.
+		payloadLen := 1 + addrsLen - addrBytesRead
+		payload := make([]byte, payloadLen)
+
+		// First write a byte for the address type that
+		// we already read.
+		payload[0] = byte(aType)
+
+		// Now append the rest of the address bytes.
+		_, err := io.ReadFull(addrBuf, payload[1:])
+		if err != nil {
+			return 0, nil, err
+		}
+
+		address = &OpaqueAddrs{
+			Payload: payload,
+		}
+		addrBytesRead = addrsLen
 	}
 
-	return addresses, nil
+	return addrBytesRead, address, nil
 }