From 3a1166ff4f53aa01402fe31ed2177233196b915d Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 19 Jul 2023 16:04:07 +0930 Subject: [PATCH 1/4] gossip_store: add "dying" flag to indicate not to gossip dying channels. Signed-off-by: Rusty Russell --- common/gossip_store.h | 5 +++++ devtools/dump-gossipstore.c | 8 +++++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/common/gossip_store.h b/common/gossip_store.h index d57af87fe1fe..ef6394dd7ce6 100644 --- a/common/gossip_store.h +++ b/common/gossip_store.h @@ -44,6 +44,11 @@ struct gossip_rcvd_filter; */ #define GOSSIP_STORE_ZOMBIE_BIT 0x1000U +/** + * Bit of flags used to mark a channel announcement closed (not deleted for 12 blocks) + */ +#define GOSSIP_STORE_DYING_BIT 0x0800U + /** * gossip_hdr -- On-disk format header. diff --git a/devtools/dump-gossipstore.c b/devtools/dump-gossipstore.c index 1f0df55a0720..3df77a92682e 100644 --- a/devtools/dump-gossipstore.c +++ b/devtools/dump-gossipstore.c @@ -68,13 +68,14 @@ int main(int argc, char *argv[]) u16 flags = be16_to_cpu(hdr.flags); u16 msglen = be16_to_cpu(hdr.len); u8 *msg, *inner; - bool deleted, push, ratelimit, zombie; + bool deleted, push, ratelimit, zombie, dying; u32 blockheight; deleted = (flags & GOSSIP_STORE_DELETED_BIT); push = (flags & GOSSIP_STORE_PUSH_BIT); ratelimit = (flags & GOSSIP_STORE_RATELIMIT_BIT); zombie = (flags & GOSSIP_STORE_ZOMBIE_BIT); + dying = (flags & GOSSIP_STORE_DYING_BIT); msg = tal_arr(NULL, u8, msglen); if (read(fd, msg, msglen) != msglen) @@ -84,11 +85,12 @@ int main(int argc, char *argv[]) != crc32c(be32_to_cpu(hdr.timestamp), msg, msglen)) warnx("Checksum verification failed"); - printf("%zu: %s%s%s%s", off, + printf("%zu: %s%s%s%s%s", off, deleted ? "DELETED " : "", push ? "PUSH " : "", ratelimit ? "RATE-LIMITED " : "", - zombie ? "ZOMBIE " : ""); + zombie ? "ZOMBIE " : "", + dying ? "DYING " : ""); if (print_timestamp) printf("T=%u ", be32_to_cpu(hdr.timestamp)); if (deleted && !print_deleted) { From 368f0e82c5489fde65d5518e5cc9f070f86c1da8 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 19 Jul 2023 16:05:31 +0930 Subject: [PATCH 2/4] gossipd: add dying marker to channel_announcement/channel_update. We don't actually delete them for 12 blocks, but we can't avoid propagating them. We don't mark node_announcements, which is a bit weird, but avoids us tracking logic to un-dying them if a channel is opened. Signed-off-by: Rusty Russell --- common/test/run-json_filter.c | 6 ++-- common/test/run-json_remove.c | 6 ++-- common/test/run-param.c | 6 ++-- gossipd/gossip_store.c | 34 +++++++++++++++++++ gossipd/gossip_store.h | 11 ++++++ gossipd/routing.c | 11 ++++++ gossipd/test/run-check_channel_announcement.c | 5 +++ gossipd/test/run-txout_failure.c | 5 +++ lightningd/test/run-invoice-select-inchan.c | 10 +++--- wallet/test/run-db.c | 6 ++-- wallet/test/run-wallet.c | 6 ++-- 11 files changed, 86 insertions(+), 20 deletions(-) diff --git a/common/test/run-json_filter.c b/common/test/run-json_filter.c index ff8978f6bbf5..58be0ca0c3e7 100644 --- a/common/test/run-json_filter.c +++ b/common/test/run-json_filter.c @@ -127,6 +127,9 @@ int segwit_addr_decode( const char* addr ) { fprintf(stderr, "segwit_addr_decode called!\n"); abort(); } +/* Generated stub for to_canonical_invstr */ +const char *to_canonical_invstr(const tal_t *ctx UNNEEDED, const char *invstring UNNEEDED) +{ fprintf(stderr, "to_canonical_invstr called!\n"); abort(); } /* Generated stub for towire */ void towire(u8 **pptr UNNEEDED, const void *data UNNEEDED, size_t len UNNEEDED) { fprintf(stderr, "towire called!\n"); abort(); } @@ -152,9 +155,6 @@ void towire_u8(u8 **pptr UNNEEDED, u8 v UNNEEDED) /* Generated stub for towire_u8_array */ void towire_u8_array(u8 **pptr UNNEEDED, const u8 *arr UNNEEDED, size_t num UNNEEDED) { fprintf(stderr, "towire_u8_array called!\n"); abort(); } -/* Generated stub for strip_lightning_prefix */ -const char *to_canonical_invstr(const tal_t *ctx, const char *invstring UNNEEDED) -{ fprintf(stderr, "strip_lightning_prefix called!\n"); abort(); } /* AUTOGENERATED MOCKS END */ bool deprecated_apis; diff --git a/common/test/run-json_remove.c b/common/test/run-json_remove.c index 35b0f98581c5..64e246e00085 100644 --- a/common/test/run-json_remove.c +++ b/common/test/run-json_remove.c @@ -162,6 +162,9 @@ int segwit_addr_decode( const char* addr ) { fprintf(stderr, "segwit_addr_decode called!\n"); abort(); } +/* Generated stub for to_canonical_invstr */ +const char *to_canonical_invstr(const tal_t *ctx UNNEEDED, const char *invstring UNNEEDED) +{ fprintf(stderr, "to_canonical_invstr called!\n"); abort(); } /* Generated stub for towire */ void towire(u8 **pptr UNNEEDED, const void *data UNNEEDED, size_t len UNNEEDED) { fprintf(stderr, "towire called!\n"); abort(); } @@ -187,9 +190,6 @@ void towire_u8(u8 **pptr UNNEEDED, u8 v UNNEEDED) /* Generated stub for towire_u8_array */ void towire_u8_array(u8 **pptr UNNEEDED, const u8 *arr UNNEEDED, size_t num UNNEEDED) { fprintf(stderr, "towire_u8_array called!\n"); abort(); } -/* Generated stub for strip_lightning_prefix */ -const char *to_canonical_invstr(const tal_t *ctx, const char *invstring UNNEEDED) -{ fprintf(stderr, "strip_lightning_prefix called!\n"); abort(); } /* AUTOGENERATED MOCKS END */ struct json { diff --git a/common/test/run-param.c b/common/test/run-param.c index f7fb5fc93dab..342e68f4e16f 100644 --- a/common/test/run-param.c +++ b/common/test/run-param.c @@ -40,15 +40,15 @@ struct command_result *command_fail(struct command *cmd, /* Generated stub for command_filter_ptr */ struct json_filter **command_filter_ptr(struct command *cmd UNNEEDED) { fprintf(stderr, "command_filter_ptr called!\n"); abort(); } -/* Generated stub for strip_lightning_prefix */ -const char *to_canonical_invstr(const tal_t *ctx, const char *invstring UNNEEDED) -{ fprintf(stderr, "strip_lightning_prefix called!\n"); abort(); } /* Generated stub for fromwire_tlv */ bool fromwire_tlv(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, const struct tlv_record_type *types UNNEEDED, size_t num_types UNNEEDED, void *record UNNEEDED, struct tlv_field **fields UNNEEDED, const u64 *extra_types UNNEEDED, size_t *err_off UNNEEDED, u64 *err_type UNNEEDED) { fprintf(stderr, "fromwire_tlv called!\n"); abort(); } +/* Generated stub for to_canonical_invstr */ +const char *to_canonical_invstr(const tal_t *ctx UNNEEDED, const char *invstring UNNEEDED) +{ fprintf(stderr, "to_canonical_invstr called!\n"); abort(); } /* Generated stub for towire_tlv */ void towire_tlv(u8 **pptr UNNEEDED, const struct tlv_record_type *types UNNEEDED, size_t num_types UNNEEDED, diff --git a/gossipd/gossip_store.c b/gossipd/gossip_store.c index 2c7c8ab52958..abc001e42139 100644 --- a/gossipd/gossip_store.c +++ b/gossipd/gossip_store.c @@ -579,6 +579,40 @@ u64 gossip_store_add_private_update(struct gossip_store *gs, const u8 *update) return gossip_store_add(gs, pupdate, 0, false, false, NULL); } +void gossip_store_mark_dying(struct gossip_store *gs, + const struct broadcastable *bcast, + int type) +{ + const u8 *msg; + be16 flags; + + /* Should never get here during loading! */ + assert(gs->writable); + + /* Should never try to overwrite version */ + assert(bcast->index); + + /* Sanity check, that this is a channel announcement */ + msg = gossip_store_get(tmpctx, gs, bcast->index); + if (fromwire_peektype(msg) != type) { + status_broken("gossip_store incorrect dying msg not %u @%u of %"PRIu64": %s", + type, bcast->index, gs->len, tal_hex(tmpctx, msg)); + return; + } + + if (pread(gs->fd, &flags, sizeof(flags), bcast->index) != sizeof(flags)) { + status_failed(STATUS_FAIL_INTERNAL_ERROR, + "Could not read to mark dying at %u/%"PRIu64": %s", + bcast->index, gs->len, strerror(errno)); + } + + flags |= cpu_to_be16(GOSSIP_STORE_DYING_BIT); + if (pwrite(gs->fd, &flags, sizeof(flags), bcast->index) != sizeof(flags)) + status_failed(STATUS_FAIL_INTERNAL_ERROR, + "Failed writing flags to dying @%u: %s", + bcast->index, strerror(errno)); +} + /* Returns index of following entry. */ static u32 delete_by_index(struct gossip_store *gs, u32 index, int type) { diff --git a/gossipd/gossip_store.h b/gossipd/gossip_store.h index 3d27df5ba28e..698eb64718db 100644 --- a/gossipd/gossip_store.h +++ b/gossipd/gossip_store.h @@ -76,6 +76,17 @@ void gossip_store_mark_channel_zombie(struct gossip_store *gs, void gossip_store_mark_cupdate_zombie(struct gossip_store *gs, struct broadcastable *bcast); +/** + * Mark this channel_announcement/channel_update as dying. + * + * We'll clean it up in 12 blocks, but this tells connectd not to gossip + * about it. + */ +void gossip_store_mark_dying(struct gossip_store *gs, + const struct broadcastable *bcast, + int type); + + /** * Direct store accessor: loads gossip msg back from store. * diff --git a/gossipd/routing.c b/gossipd/routing.c index e983db659289..ffc7daddd92b 100644 --- a/gossipd/routing.c +++ b/gossipd/routing.c @@ -2279,6 +2279,17 @@ void routing_channel_spent(struct routing_state *rstate, msg = towire_gossip_store_chan_dying(tmpctx, &chan->scid, deadline); index = gossip_store_add(rstate->gs, msg, 0, false, false, NULL); + /* Mark it dying, so we don't gossip it */ + gossip_store_mark_dying(rstate->gs, &chan->bcast, + WIRE_CHANNEL_ANNOUNCEMENT); + for (int dir = 0; dir < ARRAY_SIZE(chan->half); dir++) { + if (is_halfchan_defined(&chan->half[dir])) { + gossip_store_mark_dying(rstate->gs, + &chan->half[dir].bcast, + WIRE_CHANNEL_UPDATE); + } + } + /* Remember locally so we can kill it in 12 blocks */ status_debug("channel %s closing soon due" " to the funding outpoint being spent", diff --git a/gossipd/test/run-check_channel_announcement.c b/gossipd/test/run-check_channel_announcement.c index f2ded1cf68d2..bd2d70071cef 100644 --- a/gossipd/test/run-check_channel_announcement.c +++ b/gossipd/test/run-check_channel_announcement.c @@ -86,6 +86,11 @@ const u8 *gossip_store_get_private_update(const tal_t *ctx UNNEEDED, void gossip_store_mark_channel_deleted(struct gossip_store *gs UNNEEDED, const struct short_channel_id *scid UNNEEDED) { fprintf(stderr, "gossip_store_mark_channel_deleted called!\n"); abort(); } +/* Generated stub for gossip_store_mark_dying */ +void gossip_store_mark_dying(struct gossip_store *gs UNNEEDED, + const struct broadcastable *bcast UNNEEDED, + int type UNNEEDED) +{ fprintf(stderr, "gossip_store_mark_dying called!\n"); abort(); } /* Generated stub for gossip_store_new */ struct gossip_store *gossip_store_new(struct routing_state *rstate UNNEEDED) { fprintf(stderr, "gossip_store_new called!\n"); abort(); } diff --git a/gossipd/test/run-txout_failure.c b/gossipd/test/run-txout_failure.c index 1e924581ef68..2eb26db8a737 100644 --- a/gossipd/test/run-txout_failure.c +++ b/gossipd/test/run-txout_failure.c @@ -57,6 +57,11 @@ const u8 *gossip_store_get_private_update(const tal_t *ctx UNNEEDED, void gossip_store_mark_channel_deleted(struct gossip_store *gs UNNEEDED, const struct short_channel_id *scid UNNEEDED) { fprintf(stderr, "gossip_store_mark_channel_deleted called!\n"); abort(); } +/* Generated stub for gossip_store_mark_dying */ +void gossip_store_mark_dying(struct gossip_store *gs UNNEEDED, + const struct broadcastable *bcast UNNEEDED, + int type UNNEEDED) +{ fprintf(stderr, "gossip_store_mark_dying called!\n"); abort(); } /* Generated stub for memleak_add_helper_ */ void memleak_add_helper_(const tal_t *p UNNEEDED, void (*cb)(struct htable *memtable UNNEEDED, const tal_t *)){ } diff --git a/lightningd/test/run-invoice-select-inchan.c b/lightningd/test/run-invoice-select-inchan.c index 49e138ce3149..bc90a02d5b82 100644 --- a/lightningd/test/run-invoice-select-inchan.c +++ b/lightningd/test/run-invoice-select-inchan.c @@ -13,11 +13,6 @@ struct channel *any_channel_by_scid(struct lightningd *ld UNNEEDED, const struct short_channel_id *scid UNNEEDED, bool privacy_leak_ok UNNEEDED) { fprintf(stderr, "any_channel_by_scid called!\n"); abort(); } -/* Generated stub for param_invstring */ -struct command_result *param_invstring(struct command *cmd, const char *name, - const char * buffer, const jsmntok_t *tok, - const char **str) -{ fprintf(stderr, "param_invstring called!\n"); abort(); } /* Generated stub for bip32_pubkey */ void bip32_pubkey(struct lightningd *ld UNNEEDED, struct pubkey *pubkey UNNEEDED, u32 index UNNEEDED) { fprintf(stderr, "bip32_pubkey called!\n"); abort(); } @@ -715,6 +710,11 @@ struct command_result *param_escaped_string(struct command *cmd UNNEEDED, const jsmntok_t *tok UNNEEDED, const char **str UNNEEDED) { fprintf(stderr, "param_escaped_string called!\n"); abort(); } +/* Generated stub for param_invstring */ +struct command_result *param_invstring(struct command *cmd UNNEEDED, const char *name UNNEEDED, + const char * buffer UNNEEDED, const jsmntok_t *tok UNNEEDED, + const char **str UNNEEDED) +{ fprintf(stderr, "param_invstring called!\n"); abort(); } /* Generated stub for param_label */ struct command_result *param_label(struct command *cmd UNNEEDED, const char *name UNNEEDED, const char * buffer UNNEEDED, const jsmntok_t *tok UNNEEDED, diff --git a/wallet/test/run-db.c b/wallet/test/run-db.c index f8f9adcfad48..2efffb59d70d 100644 --- a/wallet/test/run-db.c +++ b/wallet/test/run-db.c @@ -53,6 +53,9 @@ void logv(struct log *log UNNEEDED, enum log_level level UNNEEDED, const struct /* Generated stub for psbt_fixup */ const u8 *psbt_fixup(const tal_t *ctx UNNEEDED, const u8 *psbtblob UNNEEDED) { fprintf(stderr, "psbt_fixup called!\n"); abort(); } +/* Generated stub for to_canonical_invstr */ +const char *to_canonical_invstr(const tal_t *ctx UNNEEDED, const char *invstring UNNEEDED) +{ fprintf(stderr, "to_canonical_invstr called!\n"); abort(); } /* Generated stub for towire_hsmd_get_channel_basepoints */ u8 *towire_hsmd_get_channel_basepoints(const tal_t *ctx UNNEEDED, const struct node_id *peerid UNNEEDED, u64 dbid UNNEEDED) { fprintf(stderr, "towire_hsmd_get_channel_basepoints called!\n"); abort(); } @@ -65,9 +68,6 @@ u8 *wire_sync_read(const tal_t *ctx UNNEEDED, int fd UNNEEDED) /* Generated stub for wire_sync_write */ bool wire_sync_write(int fd UNNEEDED, const void *msg TAKES UNNEEDED) { fprintf(stderr, "wire_sync_write called!\n"); abort(); } -/* Generated stub for strip_lightning_prefix */ -const char *to_canonical_invstr(const tal_t *ctx, const char *invstring UNNEEDED) -{ fprintf(stderr, "strip_lightning_prefix called!\n"); abort(); } /* AUTOGENERATED MOCKS END */ void plugin_hook_db_sync(struct db *db UNNEEDED) diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index 8f8043936b74..4ce775dbbbe8 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -38,9 +38,6 @@ static void test_error(struct lightningd *ld, bool fatal, const char *fmt, va_li #include /* AUTOGENERATED MOCKS START */ -/* Generated stub for strip_lightning_prefix */ -const char *to_canonical_invstr(const tal_t *ctx, const char *invstring UNNEEDED) -{ fprintf(stderr, "strip_lightning_prefix called!\n"); abort(); } /* Generated stub for bigsize_put */ size_t bigsize_put(u8 buf[BIGSIZE_MAX_LEN] UNNEEDED, bigsize_t v UNNEEDED) { fprintf(stderr, "bigsize_put called!\n"); abort(); } @@ -705,6 +702,9 @@ void subkey_from_hmac(const char *prefix UNNEEDED, const struct secret *base UNNEEDED, struct secret *key UNNEEDED) { fprintf(stderr, "subkey_from_hmac called!\n"); abort(); } +/* Generated stub for to_canonical_invstr */ +const char *to_canonical_invstr(const tal_t *ctx UNNEEDED, const char *invstring UNNEEDED) +{ fprintf(stderr, "to_canonical_invstr called!\n"); abort(); } /* Generated stub for topology_add_sync_waiter_ */ void topology_add_sync_waiter_(const tal_t *ctx UNNEEDED, struct chain_topology *topo UNNEEDED, From 649ae7998395f5c0f2bf1ad926ccae8cd4fc0315 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 19 Jul 2023 16:07:40 +0930 Subject: [PATCH 3/4] pytest: test for whether we gossip spent channels. Signed-off-by: Rusty Russell --- tests/test_gossip.py | 45 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/tests/test_gossip.py b/tests/test_gossip.py index 59a9514ba055..5780c3583a1f 100644 --- a/tests/test_gossip.py +++ b/tests/test_gossip.py @@ -2226,6 +2226,51 @@ def test_gossip_private_updates(node_factory, bitcoind): wait_for(lambda: l1.daemon.is_in_log(r'gossip_store_compact_offline: 5 deleted, 3 copied')) +@pytest.mark.xfail(strict=True) +def test_gossip_not_dying(node_factory, bitcoind): + l1 = node_factory.get_node() + l2, l3 = node_factory.line_graph(2, wait_for_announce=True) + + l1.rpc.connect(l2.info['id'], 'localhost', l2.port) + # Wait until it sees all the updates, node announcments. + wait_for(lambda: len([n for n in l1.rpc.listnodes()['nodes'] if 'alias' in n]) + + len(l1.rpc.listchannels()['channels']) == 4) + + def get_gossip(node): + out = subprocess.run(['devtools/gossipwith', + '--initial-sync', + '--timeout-after=2', + '{}@localhost:{}'.format(node.info['id'], node.port)], + check=True, + timeout=TIMEOUT, stdout=subprocess.PIPE).stdout + + msgs = [] + while len(out): + l, t = struct.unpack('>HH', out[0:4]) + msg = out[2:2 + l] + out = out[2 + l:] + + # Ignore pings, timestamp_filter + if t == 265 or t == 18: + continue + # channel_announcement node_announcement or channel_update + assert t == 256 or t == 257 or t == 258 + msgs.append(msg) + + return msgs + + assert len(get_gossip(l1)) == 5 + + # Close l2->l3, mine block. + l2.rpc.close(l3.info['id']) + bitcoind.generate_block(1, wait_for_mempool=1) + + l1.daemon.wait_for_log("closing soon due to the funding outpoint being spent") + + # We won't gossip the dead channel any more (but we still propagate node_announcement) + assert len(get_gossip(l1)) == 2 + + @pytest.mark.skip("Zombie research had unexpected side effects") @pytest.mark.developer("Needs --dev-fast-gossip, --dev-fast-gossip-prune") def test_channel_resurrection(node_factory, bitcoind): From 00b8bc9d23f9daf4b65d9d7f06d0abb40860d4d2 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 19 Jul 2023 16:07:49 +0930 Subject: [PATCH 4/4] connectd: don't gossip dying channels. Fixes: #6368 Changelog-Fixed: Protocol: we no longer gossip about recently-closed channels (Eclair gets upset with this). Signed-off-by: Rusty Russell --- connectd/gossip_store.c | 4 ++-- tests/test_gossip.py | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/connectd/gossip_store.c b/connectd/gossip_store.c index e387f55d97da..78af959c783f 100644 --- a/connectd/gossip_store.c +++ b/connectd/gossip_store.c @@ -130,8 +130,8 @@ u8 *gossip_store_next(const tal_t *ctx, flags = be16_to_cpu(hdr.flags); ratelimited = (flags & GOSSIP_STORE_RATELIMIT_BIT); - /* Skip any deleted entries. */ - if (flags & GOSSIP_STORE_DELETED_BIT) { + /* Skip any deleted/dying entries. */ + if (flags & (GOSSIP_STORE_DELETED_BIT|GOSSIP_STORE_DYING_BIT)) { *off += r + msglen; continue; } diff --git a/tests/test_gossip.py b/tests/test_gossip.py index 5780c3583a1f..7af97a090bcf 100644 --- a/tests/test_gossip.py +++ b/tests/test_gossip.py @@ -2226,7 +2226,6 @@ def test_gossip_private_updates(node_factory, bitcoind): wait_for(lambda: l1.daemon.is_in_log(r'gossip_store_compact_offline: 5 deleted, 3 copied')) -@pytest.mark.xfail(strict=True) def test_gossip_not_dying(node_factory, bitcoind): l1 = node_factory.get_node() l2, l3 = node_factory.line_graph(2, wait_for_announce=True)