Skip to content
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

Don't gossip about recently-closed channels #6413

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions common/gossip_store.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
6 changes: 3 additions & 3 deletions common/test/run-json_filter.c
Original file line number Diff line number Diff line change
Expand Up @@ -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(); }
Expand All @@ -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;
Expand Down
6 changes: 3 additions & 3 deletions common/test/run-json_remove.c
Original file line number Diff line number Diff line change
Expand Up @@ -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(); }
Expand All @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions common/test/run-param.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions connectd/gossip_store.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
8 changes: 5 additions & 3 deletions devtools/dump-gossipstore.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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) {
Expand Down
34 changes: 34 additions & 0 deletions gossipd/gossip_store.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...or update, as appropriate

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)
{
Expand Down
11 changes: 11 additions & 0 deletions gossipd/gossip_store.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
11 changes: 11 additions & 0 deletions gossipd/routing.c
Original file line number Diff line number Diff line change
Expand Up @@ -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++) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is ARRAY_SIZE(chan->half) generally preferred over 2? (Just so I can stay consistent)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we're explicitly iterating over the chan->half array, ARRAY_SIZE makes sense here?

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",
Expand Down
5 changes: 5 additions & 0 deletions gossipd/test/run-check_channel_announcement.c
Original file line number Diff line number Diff line change
Expand Up @@ -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(); }
Expand Down
5 changes: 5 additions & 0 deletions gossipd/test/run-txout_failure.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 *)){ }
Expand Down
10 changes: 5 additions & 5 deletions lightningd/test/run-invoice-select-inchan.c
Original file line number Diff line number Diff line change
Expand Up @@ -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(); }
Expand Down Expand Up @@ -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,
Expand Down
44 changes: 44 additions & 0 deletions tests/test_gossip.py
Original file line number Diff line number Diff line change
Expand Up @@ -2226,6 +2226,50 @@ 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'))


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):
Expand Down
6 changes: 3 additions & 3 deletions wallet/test/run-db.c
Original file line number Diff line number Diff line change
Expand Up @@ -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(); }
Expand All @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions wallet/test/run-wallet.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,6 @@ static void test_error(struct lightningd *ld, bool fatal, const char *fmt, va_li
#include <stdio.h>

/* 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(); }
Expand Down Expand Up @@ -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,
Expand Down