From 317360e5c119ddf3399653e29f48b87656ba34e4 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 20 Apr 2022 07:16:35 +0930 Subject: [PATCH 1/7] gossipd: revert useless bf8cb640b71e15f935ced19934150c921c025981. This attempted to make us re-xmit our own node_announcement at restart, by moving the node_announcement to the end of the gossip store. But, as nothing is connected, yet, this had no effect! We will rexmit it anyway, since it's marked PUSH. Signed-off-by: Rusty Russell --- gossipd/gossip_generation.c | 20 +++----------------- gossipd/routing.c | 4 ++-- gossipd/routing.h | 4 ---- gossipd/test/run-check_node_announcement.c | 3 --- gossipd/test/run-crc32_of_update.c | 3 --- 5 files changed, 5 insertions(+), 29 deletions(-) diff --git a/gossipd/gossip_generation.c b/gossipd/gossip_generation.c index 1a561ac3d418..a07a5f5da4ff 100644 --- a/gossipd/gossip_generation.c +++ b/gossipd/gossip_generation.c @@ -335,19 +335,9 @@ static bool update_own_node_announcement(struct daemon *daemon, return true; } -/* This retransmits the existing node announcement */ -static void force_self_nannounce_rexmit(struct daemon *daemon) -{ - struct node *self = get_node(daemon->rstate, &daemon->id); - - force_node_announce_rexmit(daemon->rstate, self); -} - static void update_own_node_announcement_after_startup(struct daemon *daemon) { - /* If that doesn't send one, arrange rexmit anyway */ - if (!update_own_node_announcement(daemon, false, false)) - force_self_nannounce_rexmit(daemon); + update_own_node_announcement(daemon, false, false); } /* This creates and transmits a *new* node announcement */ @@ -362,7 +352,7 @@ static void force_self_nannounce_regen(struct daemon *daemon) update_own_node_announcement(daemon, false, true); } -/* Because node_announcement propagation is spotty, we rexmit this every +/* Because node_announcement propagation is spotty, we regenerate this every * 24 hours. */ static void setup_force_nannounce_regen_timer(struct daemon *daemon) { @@ -386,11 +376,7 @@ void maybe_send_own_node_announce(struct daemon *daemon, bool startup) if (!daemon->rstate->local_channel_announced) return; - /* If we didn't send one, arrange rexmit of existing at startup */ - if (!update_own_node_announcement(daemon, startup, false)) { - if (startup) - force_self_nannounce_rexmit(daemon); - } + update_own_node_announcement(daemon, startup, false); } /* Fast accessors for channel_update fields */ diff --git a/gossipd/routing.c b/gossipd/routing.c index abe1d7a49486..a81e400f969c 100644 --- a/gossipd/routing.c +++ b/gossipd/routing.c @@ -423,8 +423,8 @@ static bool node_announce_predates_channels(const struct node *node) /* Move this node's announcement to the tail of the gossip_store, to * make everyone send it again. */ -void force_node_announce_rexmit(struct routing_state *rstate, - struct node *node) +static void force_node_announce_rexmit(struct routing_state *rstate, + struct node *node) { const u8 *announce; bool is_local = node_id_eq(&node->id, &rstate->local_id); diff --git a/gossipd/routing.h b/gossipd/routing.h index 35abe5eb35dd..1641284bb667 100644 --- a/gossipd/routing.h +++ b/gossipd/routing.h @@ -406,8 +406,4 @@ void remove_all_gossip(struct routing_state *rstate); /* This scid is dead to us. */ void add_to_txout_failures(struct routing_state *rstate, const struct short_channel_id *scid); - -/* Move this node's announcement to the tail of the gossip_store, to - * make everyone send it again. */ -void force_node_announce_rexmit(struct routing_state *rstate, struct node *node); #endif /* LIGHTNING_GOSSIPD_ROUTING_H */ diff --git a/gossipd/test/run-check_node_announcement.c b/gossipd/test/run-check_node_announcement.c index e2d36b59dcbe..7d20a8720211 100644 --- a/gossipd/test/run-check_node_announcement.c +++ b/gossipd/test/run-check_node_announcement.c @@ -33,9 +33,6 @@ void ecdh(const struct pubkey *point UNNEEDED, struct secret *ss UNNEEDED) /* Generated stub for find_peer */ struct peer *find_peer(struct daemon *daemon UNNEEDED, const struct node_id *id UNNEEDED) { fprintf(stderr, "find_peer called!\n"); abort(); } -/* Generated stub for force_node_announce_rexmit */ -void force_node_announce_rexmit(struct routing_state *rstate UNNEEDED, struct node *node UNNEEDED) -{ fprintf(stderr, "force_node_announce_rexmit called!\n"); abort(); } /* Generated stub for fromwire_gossipd_local_channel_update */ bool fromwire_gossipd_local_channel_update(const void *p UNNEEDED, struct node_id *id UNNEEDED, struct short_channel_id *short_channel_id UNNEEDED, bool *disable UNNEEDED, u16 *cltv_expiry_delta UNNEEDED, struct amount_msat *htlc_minimum_msat UNNEEDED, u32 *fee_base_msat UNNEEDED, u32 *fee_proportional_millionths UNNEEDED, struct amount_msat *htlc_maximum_msat UNNEEDED) { fprintf(stderr, "fromwire_gossipd_local_channel_update called!\n"); abort(); } diff --git a/gossipd/test/run-crc32_of_update.c b/gossipd/test/run-crc32_of_update.c index ea1104c7bc69..3b46a0f54c15 100644 --- a/gossipd/test/run-crc32_of_update.c +++ b/gossipd/test/run-crc32_of_update.c @@ -51,9 +51,6 @@ void ecdh(const struct pubkey *point UNNEEDED, struct secret *ss UNNEEDED) /* Generated stub for find_peer */ struct peer *find_peer(struct daemon *daemon UNNEEDED, const struct node_id *id UNNEEDED) { fprintf(stderr, "find_peer called!\n"); abort(); } -/* Generated stub for force_node_announce_rexmit */ -void force_node_announce_rexmit(struct routing_state *rstate UNNEEDED, struct node *node UNNEEDED) -{ fprintf(stderr, "force_node_announce_rexmit called!\n"); abort(); } /* Generated stub for fromwire_gossipd_dev_set_max_scids_encode_size */ bool fromwire_gossipd_dev_set_max_scids_encode_size(const void *p UNNEEDED, u32 *max UNNEEDED) { fprintf(stderr, "fromwire_gossipd_dev_set_max_scids_encode_size called!\n"); abort(); } From 14ec43f6ac8619d81e06f267252dafdc197977ba Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 20 Apr 2022 07:16:35 +0930 Subject: [PATCH 2/7] common/gossip_store: add flag to *only* fetch "push"-marked messages. These are the ones which are for our own channels (and our own node_announcement). Signed-off-by: Rusty Russell --- common/gossip_store.c | 3 +++ common/gossip_store.h | 1 + connectd/multiplex.c | 1 + 3 files changed, 5 insertions(+) diff --git a/common/gossip_store.c b/common/gossip_store.c index 686b1f1773a7..eba8a5ab03c4 100644 --- a/common/gossip_store.c +++ b/common/gossip_store.c @@ -53,6 +53,7 @@ static size_t reopen_gossip_store(int *gossip_store_fd, const u8 *msg) u8 *gossip_store_next(const tal_t *ctx, int *gossip_store_fd, u32 timestamp_min, u32 timestamp_max, + bool push_only, size_t *off, size_t *end) { u8 *msg = NULL; @@ -109,6 +110,8 @@ u8 *gossip_store_next(const tal_t *ctx, !timestamp_filter(timestamp_min, timestamp_max, timestamp)) { msg = tal_free(msg); + } else if (!push && push_only) { + msg = tal_free(msg); } } diff --git a/common/gossip_store.h b/common/gossip_store.h index b5d8fad1905d..8fa6ee145c78 100644 --- a/common/gossip_store.h +++ b/common/gossip_store.h @@ -46,6 +46,7 @@ struct gossip_hdr { u8 *gossip_store_next(const tal_t *ctx, int *gossip_store_fd, u32 timestamp_min, u32 timestamp_max, + bool push_only, size_t *off, size_t *end); /** diff --git a/connectd/multiplex.c b/connectd/multiplex.c index fafc326a9fb0..8a94a33ee3d1 100644 --- a/connectd/multiplex.c +++ b/connectd/multiplex.c @@ -367,6 +367,7 @@ static u8 *maybe_from_gossip_store(const tal_t *ctx, struct peer *peer) msg = gossip_store_next(ctx, &peer->daemon->gossip_store_fd, peer->gs.timestamp_min, peer->gs.timestamp_max, + false, &peer->gs.off, &peer->daemon->gossip_store_end); /* Don't send back gossip they sent to us! */ From 4770adce6b5df6f3de33f863f2ce29f4a3dfc64e Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 20 Apr 2022 07:23:02 +0930 Subject: [PATCH 3/7] connectd: send our own gossip, even if peer hasn't sent timestamp_filter. We seem to have made node_announcement propagation *worse*, not better. Explorers don't see my nodes updates. At least some LND nodes never send us timestamp_filter, so we are never actually stream *any* gossip. We should send gossip about ourselves, even if they haven't set a filter (yet). Signed-off-by: Rusty Russell Changelog-Added: Protocol: we more aggressively send our own gossip, to improve propagation chances. --- connectd/multiplex.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/connectd/multiplex.c b/connectd/multiplex.c index 8a94a33ee3d1..118f3189745c 100644 --- a/connectd/multiplex.c +++ b/connectd/multiplex.c @@ -356,7 +356,27 @@ static u8 *maybe_from_gossip_store(const tal_t *ctx, struct peer *peer) { u8 *msg; - /* Not streaming yet? */ + /* dev-mode can suppress all gossip */ + if (IFDEV(peer->daemon->dev_suppress_gossip, false)) + return NULL; + + /* BOLT #7: + * - if the `gossip_queries` feature is negotiated: + * - MUST NOT relay any gossip messages it did not generate itself, + * unless explicitly requested. + */ + + /* So, even if they didn't send us a timestamp_filter message, + * we *still* send our own gossip. */ + if (!peer->gs.gossip_timer) { + return gossip_store_next(ctx, &peer->daemon->gossip_store_fd, + 0, 0xFFFFFFFF, + true, + &peer->gs.off, + &peer->daemon->gossip_store_end); + } + + /* Not streaming right now? */ if (!peer->gs.active) return NULL; From 780a61e4b5a1c03ce9620a5859108bde4ab2d1fc Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 20 Apr 2022 07:24:02 +0930 Subject: [PATCH 4/7] gossipd: fix daily node_announcement regeneration. We have an explicit filter against redundant node_announcement updates; we only allow 1 a week. This means that our change to force a reannouncement every 24 hours did not work! Allow once a day, instead. Signed-off-by: Rusty Russell --- gossipd/routing.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/gossipd/routing.c b/gossipd/routing.c index a81e400f969c..cfed0e0eb74c 100644 --- a/gossipd/routing.c +++ b/gossipd/routing.c @@ -1614,6 +1614,7 @@ bool routing_add_node_announcement(struct routing_state *rstate, if (node->bcast.index) { bool only_tlv_diff; + u32 redundant_time; if (index != 0) { status_broken("gossip_store node_announcement %u replaces %u!", @@ -1627,8 +1628,9 @@ bool routing_add_node_announcement(struct routing_state *rstate, return index == 0; } - /* Allow redundant updates once every 7 days */ - if (timestamp < node->bcast.timestamp + GOSSIP_PRUNE_INTERVAL(rstate->dev_fast_gossip_prune) / 2 + /* Allow redundant updates once a day (faster in dev-fast-gossip-prune mode) */ + redundant_time = GOSSIP_PRUNE_INTERVAL(rstate->dev_fast_gossip_prune) / 14; + if (timestamp < node->bcast.timestamp + redundant_time && !nannounce_different(rstate->gs, node, msg, &only_tlv_diff)) { SUPERVERBOSE( From 4b1a94d702a43e63fba670f3334e1e2afab62740 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 20 Apr 2022 07:24:04 +0930 Subject: [PATCH 5/7] pytest: test that node_announcement regeneration works as expected. We shorten 24 hours to 24 seconds using --dev--fast-gossip-prune. Signed-off-by: Rusty Russell --- gossipd/gossip_generation.c | 10 +++++++++- tests/test_gossip.py | 5 +++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/gossipd/gossip_generation.c b/gossipd/gossip_generation.c index a07a5f5da4ff..04a920777017 100644 --- a/gossipd/gossip_generation.c +++ b/gossipd/gossip_generation.c @@ -356,11 +356,19 @@ static void force_self_nannounce_regen(struct daemon *daemon) * 24 hours. */ static void setup_force_nannounce_regen_timer(struct daemon *daemon) { + struct timerel regen_time; + + /* For developers we can force a regen every 24 seconds to test */ + if (IFDEV(daemon->rstate->dev_fast_gossip_prune, false)) + regen_time = time_from_sec(24); + else + regen_time = time_from_sec(24 * 3600); + tal_free(daemon->node_announce_regen_timer); daemon->node_announce_regen_timer = new_reltimer(&daemon->timers, daemon, - time_from_sec(24 * 3600), + regen_time, force_self_nannounce_regen, daemon); } diff --git a/tests/test_gossip.py b/tests/test_gossip.py index 9e6b463ce3f3..e5d76d6affac 100644 --- a/tests/test_gossip.py +++ b/tests/test_gossip.py @@ -45,6 +45,11 @@ def test_gossip_pruning(node_factory, bitcoind): wait_for(lambda: [c['active'] for c in l2.rpc.listchannels()['channels']] == [True] * 4) wait_for(lambda: [c['active'] for c in l3.rpc.listchannels()['channels']] == [True] * 4) + # Also check that it sends a redundant node_announcement. + ts1 = only_one(l2.rpc.listnodes(l1.info['id'])['nodes'])['last_timestamp'] + wait_for(lambda: only_one(l2.rpc.listnodes(l1.info['id'])['nodes'])['last_timestamp'] != ts1) + assert only_one(l2.rpc.listnodes(l1.info['id'])['nodes'])['last_timestamp'] >= ts1 + 24 + # All of them should send a keepalive message (after 30 seconds) l1.daemon.wait_for_logs([ 'Sending keepalive channel_update for {}'.format(scid1), From b29a367d116ec0eaf95393713b6520f2c9230ec4 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 20 Apr 2022 07:24:04 +0930 Subject: [PATCH 6/7] connectd: remove a noisy debug msg, fix name typo. Signed-off-by: Rusty Russell --- connectd/multiplex.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/connectd/multiplex.c b/connectd/multiplex.c index 118f3189745c..54740a7b3c4f 100644 --- a/connectd/multiplex.c +++ b/connectd/multiplex.c @@ -392,9 +392,6 @@ static u8 *maybe_from_gossip_store(const tal_t *ctx, struct peer *peer) &peer->daemon->gossip_store_end); /* Don't send back gossip they sent to us! */ if (msg) { - status_peer_debug(&peer->id, - "Sending gossip %s", - peer_wire_name(fromwire_peektype(msg))); if (gossip_rcvd_filter_del(peer->gs.grf, msg)) { msg = tal_free(msg); goto again; @@ -582,7 +579,7 @@ static void handle_gossip_in(struct peer *peer, const u8 *msg) daemon_conn_send(peer->daemon->gossipd, take(gmsg)); } -static void handle_gossip_timetamp_filter_in(struct peer *peer, const u8 *msg) +static void handle_gossip_timestamp_filter_in(struct peer *peer, const u8 *msg) { struct bitcoin_blkid chain_hash; u32 first_timestamp, timestamp_range; @@ -650,7 +647,7 @@ static bool handle_message_locally(struct peer *peer, const u8 *msg) gossip_rcvd_filter_add(peer->gs.grf, msg); if (type == WIRE_GOSSIP_TIMESTAMP_FILTER) { - handle_gossip_timetamp_filter_in(peer, msg); + handle_gossip_timestamp_filter_in(peer, msg); return true; } else if (type == WIRE_PING) { handle_ping_in(peer, msg); From 5db0d25101384ca1f5587b32adaa9a4ed3fc3481 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 20 Apr 2022 20:32:04 +0930 Subject: [PATCH 7/7] connectd: disable advertizement of WEBSOCKET addresses. This seems to prevent broad propagation, due to LND not allowing it. See https://github.com/lightningnetwork/lnd/issues/6432 We still announce it if you disable deprecated-apis, so tests still work, and hopefully we can enable it in future. Fixes: #5196 Signed-off-by: Rusty Russell Changelog-EXPERIMENTAL: Protocol: disabled websocket announcement due to LND propagation issues --- CHANGELOG.md | 4 +++- connectd/connectd.c | 14 ++++++++++---- connectd/connectd.h | 3 +++ connectd/connectd_wire.csv | 1 + lightningd/connect_control.c | 1 + 5 files changed, 18 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e52fbbab8a4..fc300ef59744 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ This release named by Simon Vrouwe; this marks the name change to core-lightning - Protocol: we now support opening multiple channels with the same peer. ([#5078]) - Protocol: we send/receive IP addresses in `init`, and send updated node_announcement when two peers report the same remote_addr (`disable-ip-discovery` suppresses this announcement). ([#5052]) + - Protocol: we more aggressively send our own gossip, to improve propagation chances. ([#5200]) - Plugins: `cln-grpc` first class GRPC interface for remotely controlling nodes over mTLS authentication; set `grpc-port` to activate ([#5013]) - Database: With the `sqlite3://` scheme for `--wallet` option, you can now specify a second file path for real-time database backup by separating it from the main file path with a `:` character. ([#4890]) - Protocol: `pay` (and decode, etc) supports bolt11 payment_metadata a-la https://github.com/lightning/bolts/pull/912 ([#5086]) @@ -96,7 +97,7 @@ Note: You should always set `allow-deprecated-apis=false` to test for changes. - Fixed `experimental-websocket-port` to work with default addresses. ([#4945]) - Protocol: removed support for v0.10.1 onion messages. ([#4921]) - Protocol: Ability to announce DNS addresses ([#4829]) - + - Protocol: disabled websocket announcement due to LND propagation issues ([#5200]) [#4829]: https://github.com/ElementsProject/lightning/pull/4829 @@ -141,6 +142,7 @@ Note: You should always set `allow-deprecated-apis=false` to test for changes. [#5130]: https://github.com/ElementsProject/lightning/pull/5130 [#5136]: https://github.com/ElementsProject/lightning/pull/5136 [#5146]: https://github.com/ElementsProject/lightning/pull/5146 +[#5200]: https://github.com/ElementsProject/lightning/pull/5200 [0.11.0]: https://github.com/ElementsProject/lightning/releases/tag/v0.11.0 ## [0.10.2] - 2021-11-03: Bitcoin Dust Consensus Rule diff --git a/connectd/connectd.c b/connectd/connectd.c index daa159f660ba..10e9a4c7d625 100644 --- a/connectd/connectd.c +++ b/connectd/connectd.c @@ -1397,10 +1397,15 @@ setup_listeners(const tal_t *ctx, * different type. */ if (tal_count(*announceable) != 0) { - wireaddr_from_websocket(&addr.u.wireaddr, - daemon->websocket_port); - add_announceable(announceable, - &addr.u.wireaddr); + /* See https://github.com/lightningnetwork/lnd/issues/6432: + * if we add websocket to the node_announcement, it doesn't propagate. + * So we do not do this for now in general! */ + if (daemon->announce_websocket) { + wireaddr_from_websocket(&addr.u.wireaddr, + daemon->websocket_port); + add_announceable(announceable, + &addr.u.wireaddr); + } } else { status_unusual("Bound to websocket %s," " but we cannot announce" @@ -1535,6 +1540,7 @@ static void connect_init(struct daemon *daemon, const u8 *msg) &daemon->timeout_secs, &daemon->websocket_helper, &daemon->websocket_port, + &daemon->announce_websocket, &dev_fast_gossip, &dev_disconnect, &dev_no_ping_timer)) { diff --git a/connectd/connectd.h b/connectd/connectd.h index df039f02ef34..ab5829b48145 100644 --- a/connectd/connectd.h +++ b/connectd/connectd.h @@ -188,6 +188,9 @@ struct daemon { int gossip_store_fd; size_t gossip_store_end; + /* We only announce websocket addresses if !deprecated_apis */ + bool announce_websocket; + #if DEVELOPER /* Hack to speed up gossip timer */ bool dev_fast_gossip; diff --git a/connectd/connectd_wire.csv b/connectd/connectd_wire.csv index 07751e2e51bf..67af1dca055f 100644 --- a/connectd/connectd_wire.csv +++ b/connectd/connectd_wire.csv @@ -22,6 +22,7 @@ msgdata,connectd_init,use_v3_autotor,bool, msgdata,connectd_init,timeout_secs,u32, msgdata,connectd_init,websocket_helper,wirestring, msgdata,connectd_init,websocket_port,u16, +msgdata,connectd_init,announce_websocket,bool, msgdata,connectd_init,dev_fast_gossip,bool, # If this is set, then fd 5 is dev_disconnect_fd. msgdata,connectd_init,dev_disconnect,bool, diff --git a/lightningd/connect_control.c b/lightningd/connect_control.c index 38fb2c342d31..144f777100b3 100644 --- a/lightningd/connect_control.c +++ b/lightningd/connect_control.c @@ -558,6 +558,7 @@ int connectd_init(struct lightningd *ld) ld->config.connection_timeout_secs, websocket_helper_path, ld->websocket_port, + !deprecated_apis, IFDEV(ld->dev_fast_gossip, false), IFDEV(ld->dev_disconnect_fd >= 0, false), IFDEV(ld->dev_no_ping_timer, false));