Skip to content

Commit e2f4269

Browse files
rustyrussellcdecker
authored andcommitted
gossipd: handle premature node_announcements in the store.
These happen after we compact the store; every log I've seen of a restart on a real node has a message about truncating the store, because node_announcements predate channel_announcements. I extracted one such case from testnet, and reduced it to test here. Signed-off-by: Rusty Russell <[email protected]>
1 parent 312209a commit e2f4269

File tree

4 files changed

+66
-11
lines changed

4 files changed

+66
-11
lines changed

gossipd/gossip_store.c

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -251,9 +251,12 @@ void gossip_store_load(struct routing_state *rstate, struct gossip_store *gs)
251251
/* We set/check version byte on creation */
252252
off_t known_good = 1;
253253
const char *bad;
254-
size_t stats[] = {0, 0, 0, 0};
254+
size_t stats[] = {0, 0, 0, 0, 0};
255255
int fd = gs->fd;
256256
gs->fd = -1;
257+
bool unknown_node;
258+
size_t num_delayed_na = 0;
259+
u8 **delayed_na = tal_arr(tmpctx, u8 *, num_delayed_na);
257260

258261
if (lseek(fd, known_good, SEEK_SET) < 0) {
259262
status_unusual("gossip_store: lseek failure");
@@ -294,11 +297,18 @@ void gossip_store_load(struct routing_state *rstate, struct gossip_store *gs)
294297
stats[1]++;
295298
} else if (fromwire_gossip_store_node_announcement(msg, msg,
296299
&gossip_msg)) {
297-
if (!routing_add_node_announcement(rstate, gossip_msg)) {
298-
bad = "Bad node_announcement";
299-
goto truncate;
300-
}
301-
stats[2]++;
300+
if (!routing_add_node_announcement(rstate, gossip_msg,
301+
&unknown_node)) {
302+
if (!unknown_node) {
303+
bad = "Bad node_announcement";
304+
goto truncate;
305+
}
306+
/* Defer until later. */
307+
tal_resize(&delayed_na, num_delayed_na+1);
308+
delayed_na[num_delayed_na++]
309+
= tal_steal(delayed_na, gossip_msg);
310+
} else
311+
stats[2]++;
302312
} else if (fromwire_gossip_store_channel_delete(msg, &scid)) {
303313
struct chan *c = get_channel(rstate, &scid);
304314
if (!c) {
@@ -318,6 +328,15 @@ void gossip_store_load(struct routing_state *rstate, struct gossip_store *gs)
318328
gs->count++;
319329
tal_free(msg);
320330
}
331+
332+
for (size_t i = 0; i < tal_count(delayed_na); i++) {
333+
if (routing_add_node_announcement(rstate, delayed_na[i], NULL)) {
334+
stats[2]++;
335+
stats[4]++;
336+
}
337+
}
338+
status_trace("Successfully processed %zu/%zu unknown node_announcement",
339+
stats[4], tal_count(delayed_na));
321340
goto out;
322341

323342
truncate:

gossipd/routing.c

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1276,7 +1276,9 @@ static struct wireaddr *read_addresses(const tal_t *ctx, const u8 *ser)
12761276
return wireaddrs;
12771277
}
12781278

1279-
bool routing_add_node_announcement(struct routing_state *rstate, const u8 *msg TAKES)
1279+
bool routing_add_node_announcement(struct routing_state *rstate,
1280+
const u8 *msg TAKES,
1281+
bool *unknown_node)
12801282
{
12811283
struct node *node;
12821284
secp256k1_ecdsa_signature signature;
@@ -1290,15 +1292,21 @@ bool routing_add_node_announcement(struct routing_state *rstate, const u8 *msg T
12901292
if (!fromwire_node_announcement(tmpctx, msg,
12911293
&signature, &features, &timestamp,
12921294
&node_id, rgb_color, alias,
1293-
&addresses))
1295+
&addresses)) {
1296+
if (unknown_node)
1297+
*unknown_node = false;
12941298
return false;
1299+
}
12951300

12961301
node = get_node(rstate, &node_id);
12971302

12981303
/* May happen if we accepted the node_announcement due to a local
12991304
* channel, for which we didn't have the announcement yet. */
1300-
if (node == NULL)
1305+
if (node == NULL) {
1306+
if (unknown_node)
1307+
*unknown_node = true;
13011308
return false;
1309+
}
13021310

13031311
wireaddrs = read_addresses(tmpctx, addresses);
13041312
tal_free(node->addresses);
@@ -1451,7 +1459,7 @@ u8 *handle_node_announcement(struct routing_state *rstate, const u8 *node_ann)
14511459
status_trace("Received node_announcement for node %s",
14521460
type_to_string(tmpctx, struct pubkey, &node_id));
14531461

1454-
applied = routing_add_node_announcement(rstate, serialized);
1462+
applied = routing_add_node_announcement(rstate, serialized, NULL);
14551463
assert(applied);
14561464
return NULL;
14571465
}

gossipd/routing.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -300,9 +300,13 @@ bool routing_add_channel_update(struct routing_state *rstate,
300300
* Directly add the node being announced to the network view, without verifying
301301
* it. This must be from a trusted source, e.g., gossip_store. For untrusted
302302
* sources (peers) please use @see{handle_node_announcement}.
303+
*
304+
* Populates *unknown_node if it isn't NULL and this returns false to indicate
305+
* if failure was due to an unknown node_id.
303306
*/
304307
bool routing_add_node_announcement(struct routing_state *rstate,
305-
const u8 *msg TAKES);
308+
const u8 *msg TAKES,
309+
bool *unknown_node);
306310

307311

308312
/**

tests/test_gossip.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -815,3 +815,27 @@ def test_gossip_addresses(node_factory, bitcoind):
815815
{'type': 'torv2', 'address': '3fyb44wdhnd2ghhl.onion', 'port': 1234},
816816
{'type': 'torv3', 'address': 'vww6ybal4bd7szmgncyruucpgfkqahzddi37ktceo3ah7ngmcopnpyyd.onion', 'port': 9735}
817817
]
818+
819+
820+
def test_gossip_store_load(node_factory):
821+
"""Make sure we can read canned gossip store"""
822+
l1 = node_factory.get_node(start=False)
823+
with open(os.path.join(l1.daemon.lightning_dir, 'gossip_store'), 'wb') as f:
824+
f.write(bytearray.fromhex("02" # GOSSIP_VERSION
825+
"00000099" # len
826+
"12abbbba" # csum
827+
"1002" # WIRE_GOSSIP_STORE_NODE_ANNOUNCEMENT
828+
"00950101cf5d870bc7ecabcb7cd16898ef66891e5f0c6c5851bd85b670f03d325bc44d7544d367cd852e18ec03f7f4ff369b06860a3b12b07b29f36fb318ca11348bf8ec00005aab817c03f113414ebdc6c1fb0f33c99cd5a1d09dd79e7fdf2468cf1fe1af6674361695d23974b250757a7a6c6549544300000000000000000000000000000000000000000000000007010566933e2607"
829+
"000001bc" # len
830+
"521ef598" # csum
831+
"1000" # WIRE_GOSSIP_STORE_CHANNEL_ANNOUNCEMENT
832+
"01b00100bb8d7b6998cca3c2b3ce12a6bd73a8872c808bb48de2a30c5ad9cdf835905d1e27505755087e675fb517bbac6beb227629b694ea68f49d357458327138978ebfd7adfde1c69d0d2f497154256f6d5567a5cf2317c589e0046c0cc2b3e986cf9b6d3b44742bd57bce32d72cd1180a7f657795976130b20508b239976d3d4cdc4d0d6e6fbb9ab6471f664a662972e406f519eab8bce87a8c0365646df5acbc04c91540b4c7c518cec680a4a6af14dae1aca0fd5525220f7f0e96fcd2adef3c803ac9427fe71034b55a50536638820ef21903d09ccddd38396675b598587fa886ca711415c813fc6d69f46552b9a0a539c18f265debd0e2e286980a118ba349c216000043497fd7f826957108f4a30fd9cec3aeba79972084e90ead01ea33090000000013a63c0000b50001021bf3de4e84e3d52f9a3e36fbdcd2c4e8dbf203b9ce4fc07c2f03be6c21d0c67503f113414ebdc6c1fb0f33c99cd5a1d09dd79e7fdf2468cf1fe1af6674361695d203801fd8ab98032f11cc9e4916dd940417082727077609d5c7f8cc6e9a3ad25dd102517164b97ab46cee3826160841a36c46a2b7b9c74da37bdc070ed41ba172033a0000000001000000"
833+
"00000086" # len
834+
"88c703c8" # csum
835+
"1001" # WIRE_GOSSIP_STORE_CHANNEL_UPDATE
836+
"008201021ea7c2eadf8a29eb8690511a519b5656e29aa0a853771c4e38e65c5abf43d907295a915e69e451f4c7a0c3dc13dd943cfbe3ae88c0b96667cd7d58955dbfedcf43497fd7f826957108f4a30fd9cec3aeba79972084e90ead01ea33090000000013a63c0000b500015b8d9b440000009000000000000003e8000003e800000001"))
837+
838+
l1.start()
839+
# May preceed the Started msg waited for in 'start'.
840+
wait_for(lambda: l1.daemon.is_in_log('gossip_store: Read 1/1/1/0 cannounce/cupdate/nannounce/cdelete from store in 744 bytes'))
841+
assert not l1.daemon.is_in_log('gossip_store.*truncating')

0 commit comments

Comments
 (0)