From 5e3690b3c55be806fe5a4be60c99e15560677f25 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 14 Jun 2019 13:11:39 +0930 Subject: [PATCH] gossipd: delete channel_amount from the store when we delete channel_announcement. Otherwise we slowly build up cruft: compaction simply moves them since they're not deleted. Signed-off-by: Rusty Russell --- gossipd/gossip_store.c | 53 ++++++++++++++++++++++++++---------------- tests/test_gossip.py | 10 ++------ 2 files changed, 35 insertions(+), 28 deletions(-) diff --git a/gossipd/gossip_store.c b/gossipd/gossip_store.c index 4b5534401..be824d47b 100644 --- a/gossipd/gossip_store.c +++ b/gossipd/gossip_store.c @@ -398,27 +398,24 @@ u64 gossip_store_add_private_update(struct gossip_store *gs, const u8 *update) return gossip_store_add(gs, pupdate, 0, NULL); } -void gossip_store_delete(struct gossip_store *gs, - struct broadcastable *bcast, - int type) +/* Returns index of following entry. */ +static u32 delete_by_index(struct gossip_store *gs, u32 index, int type) { beint32_t belen; int flags; - if (!bcast->index) - return; - /* Should never get here during loading! */ assert(gs->writable); #if DEVELOPER - const u8 *msg = gossip_store_get(tmpctx, gs, bcast->index); + const u8 *msg = gossip_store_get(tmpctx, gs, index); assert(fromwire_peektype(msg) == type); #endif - if (pread(gs->fd, &belen, sizeof(belen), bcast->index) != sizeof(belen)) + + if (pread(gs->fd, &belen, sizeof(belen), index) != sizeof(belen)) status_failed(STATUS_FAIL_INTERNAL_ERROR, "Failed reading len to delete @%u: %s", - bcast->index, strerror(errno)); + index, strerror(errno)); assert((be32_to_cpu(belen) & GOSSIP_STORE_LEN_DELETED_BIT) == 0); belen |= cpu_to_be32(GOSSIP_STORE_LEN_DELETED_BIT); @@ -433,16 +430,36 @@ void gossip_store_delete(struct gossip_store *gs, */ flags = fcntl(gs->fd, F_GETFL); fcntl(gs->fd, F_SETFL, flags & ~O_APPEND); - if (pwrite(gs->fd, &belen, sizeof(belen), bcast->index) != sizeof(belen)) + if (pwrite(gs->fd, &belen, sizeof(belen), index) != sizeof(belen)) status_failed(STATUS_FAIL_INTERNAL_ERROR, "Failed writing len to delete @%u: %s", - bcast->index, strerror(errno)); + index, strerror(errno)); fcntl(gs->fd, F_SETFL, flags); gs->deleted++; + return index + sizeof(struct gossip_hdr) + + (be32_to_cpu(belen) & ~GOSSIP_STORE_LEN_DELETED_BIT); +} + +void gossip_store_delete(struct gossip_store *gs, + struct broadcastable *bcast, + int type) +{ + u32 next_index; + + if (!bcast->index) + return; + + next_index = delete_by_index(gs, bcast->index, type); + /* Reset index. */ bcast->index = 0; + /* For a channel_announcement, we need to delete amount too */ + if (type == WIRE_CHANNEL_ANNOUNCEMENT) + delete_by_index(gs, next_index, + WIRE_GOSSIP_STORE_CHANNEL_AMOUNT); + gossip_store_maybe_compact(gs); } @@ -508,14 +525,6 @@ int gossip_store_readonly_fd(struct gossip_store *gs) return fd; } -static void delete_by_index(struct gossip_store *gs, u32 index, int type) -{ - /* We make a fake broadcastable for this corner case. */ - struct broadcastable bcast; - bcast.index = index; - gossip_store_delete(gs, &bcast, type); -} - /* If we ever truncated, we might have a dangling entries. */ static void cleanup_truncated_store(struct routing_state *rstate, struct gossip_store *gs, @@ -542,7 +551,11 @@ static void cleanup_truncated_store(struct routing_state *rstate, num = 0; while ((index = remove_unupdated_channel_announce(rstate)) != 0) { - delete_by_index(gs, index, WIRE_CHANNEL_ANNOUNCEMENT); + u32 next; + + /* Delete announcement and channel amount, too */ + next = delete_by_index(gs, index, WIRE_CHANNEL_ANNOUNCEMENT); + delete_by_index(gs, next, WIRE_GOSSIP_STORE_CHANNEL_AMOUNT); num++; } if (num) diff --git a/tests/test_gossip.py b/tests/test_gossip.py index 6966778da..c6739b2ca 100644 --- a/tests/test_gossip.py +++ b/tests/test_gossip.py @@ -945,7 +945,7 @@ def test_gossip_store_load_amount_truncated(node_factory): l1.start() # May preceed the Started msg waited for in 'start'. - wait_for(lambda: l1.daemon.is_in_log(r'Deleting un-updated channel_announcement @1')) + wait_for(lambda: l1.daemon.is_in_log(r'Deleting un-amounted channel_announcement @1')) wait_for(lambda: l1.daemon.is_in_log(r'gossip_store: Read 0/0/0/0 cannounce/cupdate/nannounce/cdelete from store \(1 deleted\) in 445 bytes')) assert not l1.daemon.is_in_log('gossip_store.*truncating') @@ -1263,11 +1263,5 @@ def test_gossip_store_load_no_channel_update(node_factory): # This should actually result in an empty store. l1.rpc.call('dev-compact-gossip-store') - # FIXME: We leave channel_amount in the store! with open(os.path.join(l1.daemon.lightning_dir, 'gossip_store'), "rb") as f: - assert bytearray(f.read()) == bytearray.fromhex("07" - "0000000a" # len - "99dc98b4" # csum - "00000000" # timestamp - "1005" # WIRE_GOSSIP_STORE_CHANNEL_AMOUNT - "0000000001000000") + assert bytearray(f.read()) == bytearray.fromhex("07")