Skip to content

Commit e46c676

Browse files
yyforyongyuyyforyongyu
authored
Merge pull request #10162 from ellemouton/graphMigUnwrapDNSAddrs
graph/db: unwrap dns addresses from opaque ones during migration
2 parents d9647f8 + a74f0b1 commit e46c676

File tree

4 files changed

+410
-147
lines changed

4 files changed

+410
-147
lines changed

‎graph/db/benchmark_test.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -435,6 +435,13 @@ func TestPopulateDBs(t *testing.T) {
435435
// NOTE: the testPostgres variable can be set to true to test with a
436436
// postgres backend instead of the kvdb-sqlite backend.
437437
//
438+
// NOTE: you will need to set the following build tags in order to run this
439+
// test:
440+
//
441+
// test_native_sql
442+
// kvdb_sqlite // If your source is kvdb-sqlite
443+
// kvdb_postgres // If your source is kvdb-postgres
444+
//
438445
// NOTE: this is a helper test and is not run by default.
439446
func TestPopulateViaMigration(t *testing.T) {
440447
// ======= STEP 0 ===========

‎graph/db/sql_migration.go‎

Lines changed: 79 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -260,18 +260,17 @@ func migrateNodes(ctx context.Context, cfg *sqldb.QueryConfig,
260260
"opaque data for node %x: %w", pub, err)
261261
}
262262

263+
if err = maybeOverrideNodeAddresses(node); err != nil {
264+
skipped++
265+
log.Warnf("Skipping migration of node %x with invalid "+
266+
"address (%v): %v", pub, node.Addresses, err)
267+
268+
return nil
269+
}
270+
263271
count++
264272
chunk++
265273

266-
// TODO(elle): At this point, we should check the loaded node
267-
// to see if we should extract any DNS addresses from its
268-
// opaque type addresses. This is expected to be done in:
269-
// https://github.com/lightningnetwork/lnd/pull/9455.
270-
// This TODO is being tracked in
271-
// https://github.com/lightningnetwork/lnd/issues/9795 as this
272-
// must be addressed before making this code path active in
273-
// production.
274-
275274
// Write the node to the SQL database.
276275
id, err := insertNodeSQLMig(ctx, sqlDB, node)
277276
if err != nil {
@@ -323,12 +322,82 @@ func migrateNodes(ctx context.Context, cfg *sqldb.QueryConfig,
323322
}
324323

325324
log.Infof("Migrated %d nodes from KV to SQL in %v (skipped %d nodes "+
326-
"due to invalid TLV streams)", count, time.Since(totalTime),
325+
"due to invalid TLV streams or invalid addresses)", count,
326+
time.Since(totalTime),
327+
327328
skipped)
328329

329330
return nil
330331
}
331332

333+
// maybeOverrideNodeAddresses checks if the node has any opaque addresses that
334+
// can be parsed. If so, it replaces the node's addresses with the parsed
335+
// addresses. If the address is unparseable, it returns an error.
336+
func maybeOverrideNodeAddresses(node *models.LightningNode) error {
337+
// In the majority of cases, the number of node addresses will remain
338+
// unchanged, so we pre-allocate a slice of the same length.
339+
addrs := make([]net.Addr, 0, len(node.Addresses))
340+
341+
// Iterate over each address in search of any opaque addresses that we
342+
// can inspect.
343+
for _, addr := range node.Addresses {
344+
opaque, ok := addr.(*lnwire.OpaqueAddrs)
345+
if !ok {
346+
// Any non-opaque address is left unchanged.
347+
addrs = append(addrs, addr)
348+
continue
349+
}
350+
351+
// For each opaque address, we'll now attempt to parse out any
352+
// known addresses. We'll do this in a loop, as it's possible
353+
// that there are several addresses encoded in a single opaque
354+
// address.
355+
payload := opaque.Payload
356+
for len(payload) > 0 {
357+
var (
358+
r = bytes.NewReader(payload)
359+
numAddrBytes = uint16(len(payload))
360+
)
361+
byteRead, readAddr, err := lnwire.ReadAddress(
362+
r, numAddrBytes,
363+
)
364+
if err != nil {
365+
return err
366+
}
367+
368+
// If we were able to read an address, we'll add it to
369+
// our list of addresses.
370+
if readAddr != nil {
371+
addrs = append(addrs, readAddr)
372+
}
373+
374+
// If the address we read was an opaque address, it
375+
// means we've hit an unknown address type, and it has
376+
// consumed the rest of the payload. We can break out
377+
// of the loop.
378+
if _, ok := readAddr.(*lnwire.OpaqueAddrs); ok {
379+
break
380+
}
381+
382+
// If we've read all the bytes, we can also break.
383+
if byteRead >= numAddrBytes {
384+
break
385+
}
386+
387+
// Otherwise, we'll advance our payload slice and
388+
// continue.
389+
payload = payload[byteRead:]
390+
}
391+
}
392+
393+
// Override the node addresses if we have any.
394+
if len(addrs) != 0 {
395+
node.Addresses = addrs
396+
}
397+
398+
return nil
399+
}
400+
332401
// migrateSourceNode migrates the source node from the KV backend to the
333402
// SQL database.
334403
func migrateSourceNode(ctx context.Context, kvdb kvdb.Backend,

‎graph/db/sql_migration_test.go‎

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,78 @@ var (
4949
BitcoinSig1Bytes: testSig.Serialize(),
5050
BitcoinSig2Bytes: testSig.Serialize(),
5151
}
52+
53+
// testOpaqueAddrWithEmbeddedDNSAddr is an opaque address that contains
54+
// a single DNS address within it.
55+
testOpaqueAddrWithEmbeddedDNSAddr = &lnwire.OpaqueAddrs{
56+
Payload: []byte{
57+
// The protocol level type for DNS addresses.
58+
0x05,
59+
// Hostname length: 11.
60+
0x0b,
61+
// The hostname itself.
62+
'e', 'x', 'a', 'm', 'p', 'l', 'e', '.', 'c', 'o', 'm',
63+
// Port 8080 in big-endian.
64+
0x1f, 0x90,
65+
},
66+
}
67+
68+
// testOpaqueAddrWithEmbeddedDNSAddrAndMore is an opaque address that
69+
// contains a DNS address within it, along with some extra bytes that
70+
// represent some other unknown address type.
71+
testOpaqueAddrWithEmbeddedDNSAddrAndMore = &lnwire.OpaqueAddrs{
72+
Payload: []byte{
73+
// The protocol level type for DNS addresses.
74+
0x05,
75+
// Hostname length: 11.
76+
0x0B,
77+
// The hostname itself.
78+
'e', 'x', 'a', 'm', 'p', 'l', 'e', '.', 'c', 'o', 'm',
79+
// port 8080 in big-endian.
80+
0x1F, 0x90,
81+
// Now we add more opaque bytes to represent more
82+
// addresses that we don't know about yet.
83+
// NOTE: the 0xff is an address type that we definitely
84+
// don't know about yet
85+
0xff, 0x02, 0x03, 0x04, 0x05, 0x06,
86+
},
87+
}
88+
89+
// testOpaqueAddrWithEmbeddedBadDNSAddr is an opaque address that
90+
// contains an invalid DNS address within it.
91+
testOpaqueAddrWithEmbeddedBadDNSAddr = &lnwire.OpaqueAddrs{
92+
Payload: []byte{
93+
// The protocol level type for DNS addresses.
94+
0x05,
95+
// Hostname length: We set this to a size that is
96+
// incorrect in order to simulate the bad DNS address.
97+
0xAA,
98+
// The hostname itself.
99+
'e', 'x', 'a', 'm', 'p', 'l', 'e', '.', 'c', 'o', 'm',
100+
// port 9735 in big-endian.
101+
0x26, 0x07,
102+
},
103+
}
104+
105+
// testOpaqueAddrWithTwoEmbeddedDNSAddrs is an opaque address that
106+
// contains two valid DNS addresses within it.
107+
testOpaqueAddrWithTwoEmbeddedDNSAddrs = &lnwire.OpaqueAddrs{
108+
Payload: []byte{
109+
// The protocol level type for DNS addresses.
110+
0x05,
111+
// Hostname length: 11.
112+
0x0B,
113+
// The hostname itself.
114+
'e', 'x', 'a', 'm', 'p', 'l', 'e', '.', 'c', 'o', 'm',
115+
// port 8080 in big-endian.
116+
0x1F, 0x90,
117+
// Another DNS address.
118+
0x05,
119+
0x0B,
120+
'e', 'x', 'a', 'm', 'p', 'l', 'e', '.', 'c', 'o', 'm',
121+
0x1F, 0x90,
122+
},
123+
}
52124
)
53125

54126
// TestMigrateGraphToSQL tests various deterministic cases that we want to test
@@ -1173,6 +1245,104 @@ func TestSQLMigrationEdgeCases(t *testing.T) {
11731245
zombies: []uint64{2},
11741246
})
11751247
})
1248+
1249+
// We have used this migration as a chance to also extract any DNS
1250+
// addresses that we previously may have wrapped in an opaque address.
1251+
// If we do encounter such a case, then the migrated node set will look
1252+
// slightly different from the original node set in the KV store, and so
1253+
// we test for that here.
1254+
t.Run("node with wrapped DNS address inside opaque addr",
1255+
func(t *testing.T) {
1256+
t.Parallel()
1257+
1258+
var expectedNodes []*models.LightningNode
1259+
1260+
// Let the first node have an opaque address that we
1261+
// still don't understand. This node will remain the
1262+
// same in the SQL store.
1263+
n1 := makeTestNode(t, func(n *models.LightningNode) {
1264+
n.Addresses = []net.Addr{
1265+
testOpaqueAddr,
1266+
}
1267+
})
1268+
expectedNodes = append(expectedNodes, n1)
1269+
1270+
// The second node will have a wrapped DNS address
1271+
// inside an opaque address. The opaque address will
1272+
// only contain a DNS address and so the migrated node
1273+
// will only contain a DNS address and no opaque
1274+
// address.
1275+
n2 := makeTestNode(t, func(n *models.LightningNode) {
1276+
n.Addresses = []net.Addr{
1277+
testOpaqueAddrWithEmbeddedDNSAddr,
1278+
}
1279+
})
1280+
n2Expected := *n2
1281+
n2Expected.Addresses = []net.Addr{
1282+
testDNSAddr,
1283+
}
1284+
expectedNodes = append(expectedNodes, &n2Expected)
1285+
1286+
// The third node will have an opaque address that
1287+
// wraps a DNS address along with some other data.
1288+
// So the resulting migrated node should have both
1289+
// the DNS address and remaining opaque address data.
1290+
n3 := makeTestNode(t, func(n *models.LightningNode) {
1291+
n.Addresses = []net.Addr{
1292+
//nolint:ll
1293+
testOpaqueAddrWithEmbeddedDNSAddrAndMore,
1294+
}
1295+
})
1296+
n3Expected := *n3
1297+
n3Expected.Addresses = []net.Addr{
1298+
testDNSAddr,
1299+
testOpaqueAddr,
1300+
}
1301+
expectedNodes = append(expectedNodes, &n3Expected)
1302+
1303+
// The fourth node will have an opaque address that
1304+
// wraps an invalid DNS address. Such a node will not be
1305+
// migrated since propagating an invalid DNS address
1306+
// is not allowed.
1307+
n4 := makeTestNode(t, func(n *models.LightningNode) {
1308+
n.Addresses = []net.Addr{
1309+
testOpaqueAddrWithEmbeddedBadDNSAddr,
1310+
}
1311+
})
1312+
// NOTE: we don't add this node to the expected nodes
1313+
// slice.
1314+
1315+
// The fifth node will have 2 DNS addresses embedded
1316+
// in the opaque address. The migration will result
1317+
// in _both_ dns addresses being extracted. This is
1318+
// invalid at a protocol level, and so we should not
1319+
// propagate such addresses, but this is left to higher
1320+
// level gossip logic.
1321+
n5 := makeTestNode(t, func(n *models.LightningNode) {
1322+
n.Addresses = []net.Addr{
1323+
testOpaqueAddrWithTwoEmbeddedDNSAddrs,
1324+
}
1325+
})
1326+
n5Expected := *n5
1327+
n5Expected.Addresses = []net.Addr{
1328+
testDNSAddr,
1329+
testDNSAddr,
1330+
}
1331+
expectedNodes = append(expectedNodes, &n5Expected)
1332+
1333+
populateKV := func(t *testing.T, db *KVStore) {
1334+
require.NoError(t, db.AddLightningNode(ctx, n1))
1335+
require.NoError(t, db.AddLightningNode(ctx, n2))
1336+
require.NoError(t, db.AddLightningNode(ctx, n3))
1337+
require.NoError(t, db.AddLightningNode(ctx, n4))
1338+
require.NoError(t, db.AddLightningNode(ctx, n5))
1339+
}
1340+
1341+
runTestMigration(t, populateKV, dbState{
1342+
nodes: expectedNodes,
1343+
})
1344+
},
1345+
)
11761346
}
11771347

11781348
// runTestMigration is a helper function that sets up the KVStore and SQLStore,

0 commit comments

Comments
 (0)