From cf0b64a5272739436ef3860c40756bc9fdbed36c Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 8 Dec 2025 16:10:15 +1030 Subject: [PATCH 1/2] pytest: add test to demonstrate gossip_store misordering node announcements. We usually lose the node announcement on restart, because the node_announcement message is ignored by gossmap, as it doesn't (yet!) know of the node, since the channel_announcement does not precede the node_announcement. This is supposed to be detected and fixed by gossipd, but this simple test shows that it is not! ``` FAILED tests/test_gossip.py::test_gossmap_lost_node - AssertionError: assert {'nodes': [{'nodeid': '022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59', 'alias': 'SILENTARTIST-v25.12-2-g703851b', 'color': '022d22', 'last_timestamp': 1765172273, 'features': '8898880a8a59a1', 'addresses': []}, {'nodeid': '0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518', 'alias': 'JUNIORBEAM-v25.12-2-g703851b', 'color': '0266e4', 'last_timestamp': 1765172273, 'features': '8898880a8a59a1', 'addresses': []}, {'nodeid': '035d2b1192dfba134e10e540875d366ebc8bc353d5aa766b80c090b39c3a5d885d'}, {'nodeid': '0382ce59ebf18be7d84677c2e35f23294b9992ceca95491fcf8a56c6cb2d9de199', 'alias': 'JUNIORFELONY-v25.12-2-g703851b', 'color': '0382ce', 'last_timestamp': 1765172273, 'features': '8898880a8a59a1', 'addresses': []}]} == {'nodes': [{'nodeid': '022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59', 'alias': 'SILENTARTIST-v25.12-2-g703851b', 'color': '022d22', 'last_timestamp': 1765172273, 'features': '8898880a8a59a1', 'addresses': []}, {'nodeid': '0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518', 'alias': 'JUNIORBEAM-v25.12-2-g703851b', 'color': '0266e4', 'last_timestamp': 1765172273, 'features': '8898880a8a59a1', 'addresses': []}, {'nodeid': '035d2b1192dfba134e10e540875d366ebc8bc353d5aa766b80c090b39c3a5d885d', 'alias': 'HOPPINGFIRE-v25.12-2-g703851b', 'color': '035d2b', 'last_timestamp': 1765172273, 'features': '8898880a8a59a1', 'addresses': []}, {'nodeid': '0382ce59ebf18be7d84677c2e35f23294b9992ceca95491fcf8a56c6cb2d9de199', 'alias': 'JUNIORFELONY-v25.12-2-g703851b', 'color': '0382ce', 'last_timestamp': 1765172273, 'features': '8898880a8a59a1', 'addresses': []}]} Differing items: {'nodes': [{'addresses': [], 'alias': 'SILENTARTIST-v25.12-2-g703851b', 'color': '022d22', 'features': '8898880a8a59a1...}, {'addresses': [], 'alias': 'JUNIORFELONY-v25.12-2-g703851b', 'color': '0382ce', 'features': '8898880a8a59a1', ...}]} != {'nodes': [{'addresses': [], 'alias': 'SILENTARTIST-v25.12-2-g703851b', 'color': '022d22', 'features': '8898880a8a59a1...}, {'addresses': [], 'alias': 'JUNIORFELONY-v25.12-2-g703851b', 'color': '0382ce', 'features': '8898880a8a59a1', ...}]} Full diff: { 'nodes': [ { 'addresses': [], 'alias': 'SILENTARTIST-v25.12-2-g703851b', 'color': '022d22', 'features': '8898880a8a59a1', 'last_timestamp': 1765172273, 'nodeid': '022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59', }, { 'addresses': [], 'alias': 'JUNIORBEAM-v25.12-2-g703851b', 'color': '0266e4', 'features': '8898880a8a59a1', 'last_timestamp': 1765172273, 'nodeid': '0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518', }, { - 'addresses': [], - 'alias': 'HOPPINGFIRE-v25.12-2-g703851b', - 'color': '035d2b', - 'features': '8898880a8a59a1', - 'last_timestamp': 1765172273, 'nodeid': '035d2b1192dfba134e10e540875d366ebc8bc353d5aa766b80c090b39c3a5d885d', }, { 'addresses': [], 'alias': 'JUNIORFELONY-v25.12-2-g703851b', 'color': '0382ce', 'features': '8898880a8a59a1', 'last_timestamp': 1765172273, 'nodeid': '0382ce59ebf18be7d84677c2e35f23294b9992ceca95491fcf8a56c6cb2d9de199', }, ], } ``` Signed-off-by: Rusty Russell --- tests/test_gossip.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/test_gossip.py b/tests/test_gossip.py index 44ceba5482d4..76c3a9059acb 100644 --- a/tests/test_gossip.py +++ b/tests/test_gossip.py @@ -2370,3 +2370,22 @@ def test_incoming_unreasonable(node_factory): wait_for(lambda: [c['updates']['remote']['fee_base_msat'] for c in l3.rpc.listpeerchannels()['channels']] == [100000000, 100000000]) l3.restart() l3.rpc.listincoming() + + +@pytest.mark.xfail(strict=True) +def test_gossmap_lost_node(node_factory, bitcoind): + l1, l2, l3, l4 = node_factory.line_graph(4, wait_for_announce=True) + + scid23 = only_one(l2.rpc.listpeerchannels(l3.info['id'])['channels'])['short_channel_id'] + l2.rpc.close(l3.info['id']) + bitcoind.generate_block(13, wait_for_mempool=1) + wait_for(lambda: l1.rpc.listchannels(scid23) == {'channels': []}) + + pre_channels = l1.rpc.listchannels() + pre_nodes = l1.rpc.listnodes() + l1.restart() + post_channels = l1.rpc.listchannels() + post_nodes = l1.rpc.listnodes() + + assert post_channels == pre_channels + assert post_nodes == pre_nodes From a8b8ea4c69adcb7003a9aab5a825da4ba48542db Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 8 Dec 2025 16:11:46 +1030 Subject: [PATCH 2/2] gossipd: make sure we correctly move node announcement when *no* channel preceeds it in the gossip store. We had the test backwards, so we moved it *all the time*. This bloats our gossip store, as well as not moving it in the case where we need to. Signed-off-by: Rusty Russell Changelog-Fixed: gossipd: we would occasionally not show a node announcement in listnodes(). --- gossipd/gossmap_manage.c | 2 +- tests/test_gossip.py | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/gossipd/gossmap_manage.c b/gossipd/gossmap_manage.c index 3227465da5ae..fb9a1d856765 100644 --- a/gossipd/gossmap_manage.c +++ b/gossipd/gossmap_manage.c @@ -300,7 +300,7 @@ static void remove_channel(struct gossmap_manage *gm, /* Maybe this was the last channel_announcement which preceeded node_announcement? */ if (chan->cann_off < node->nann_off - && any_cannounce_preceeds_offset(gossmap, node, chan, node->nann_off)) { + && !any_cannounce_preceeds_offset(gossmap, node, chan, node->nann_off)) { const u8 *nannounce; u32 timestamp; diff --git a/tests/test_gossip.py b/tests/test_gossip.py index 76c3a9059acb..df1717d756e6 100644 --- a/tests/test_gossip.py +++ b/tests/test_gossip.py @@ -2372,7 +2372,6 @@ def test_incoming_unreasonable(node_factory): l3.rpc.listincoming() -@pytest.mark.xfail(strict=True) def test_gossmap_lost_node(node_factory, bitcoind): l1, l2, l3, l4 = node_factory.line_graph(4, wait_for_announce=True)