-
Notifications
You must be signed in to change notification settings - Fork 964
Fix gossipd node announcement ordering #8769
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
rustyrussell
wants to merge
3
commits into
ElementsProject:master
Choose a base branch
from
rustyrussell:fix-gossipd-node_announcement-ordering
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Fix gossipd node announcement ordering #8769
rustyrussell
wants to merge
3
commits into
ElementsProject:master
from
rustyrussell:fix-gossipd-node_announcement-ordering
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
```
lightningd: FATAL SIGNAL 11 (version v25.12rc3-1-g498c5b6)
0x5cc2f620ce4c send_backtrace
common/daemon.c:38
0x5cc2f620cee8 crashdump
common/daemon.c:83
0x7e3ac1e4532f ???
./signal/../sysdeps/unix/sysv/linux/x86_64/libc_sigaction.c:0
0x5cc2f615f186 fixup_scan_block
lightningd/chaintopology.c:1531
0x5cc2f615c22c getrawblockbyheight_callback
lightningd/bitcoind.c:484
0x5cc2f61aee87 plugin_response_handle
lightningd/plugin.c:701
0x5cc2f61b4043 plugin_read_json
lightningd/plugin.c:790
0x5cc2f6248d8b next_plan
ccan/ccan/io/io.c:60
0x5cc2f624925c do_plan
ccan/ccan/io/io.c:422
0x5cc2f6249319 io_ready
ccan/ccan/io/io.c:439
0x5cc2f624ad24 io_loop
ccan/ccan/io/poll.c:470
0x5cc2f618381a io_loop_with_timers
lightningd/io_loop_with_timers.c:22
0x5cc2f61892ff main
```
This happens intermittantly on in a few tests:
tests/test_invoices.py::test_invoice_botched_migration
tests/test_pay.py::test_pay_bolt11_metadata
tests/test_runes.py::test_id_migration
Signed-off-by: Rusty Russell <[email protected]>
…ements.
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 <[email protected]>
…nel 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 <[email protected]> Changelog-Fixed: gossipd: we would occasionally not show a node announcement in listnodes().
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Found a bug where we were not keeping our gossip_store in order, when a channel was deleted and that was the last channel_announcement preceding a node_announcement. This is actually a little unusual, but it's now fixed.