gossipd: put the last_writes array inside struct gossip_store.
What changed, and why it matters
This is a small internal code cleanup in the part of Core Lightning that saves network gossip messages to disk. It moves a bookkeeping array (used to recover from rare filesystem sync problems) from one internal structure into another, so the module that actually writes to disk also owns the recovery data. There is no user-facing change, no new feature, and no obvious security vulnerability introduced or fixed by this patch.
No security action required. Treat as normal code-quality refactor. If auditing, verify that `gossip_store_writes_confirmed` is always called after a successful `gossmap_manage_get_gossmap` refresh and that `last_writes` is freed on `gossip_store` destruction (tal parent ownership makes this likely).
Security signals we found
Refactor of existing reliability/recovery mechanism (last_writes rewrite on suspected fsync failure)
No change to wire protocol, RPC, or external interfaces
No new memory allocations or ownership patterns beyond moving existing tal array into another struct
No bounds-checking, parsing, or cryptographic changes
Evidence from the diff
The commit refactors last_writes (a tal array of recently written gossip messages kept to rewrite them if fsync fails to persist them, noted as observed on ZFS on Linux) from struct gossmap_manage into struct gossip_store. gossip_store_add no longer takes an out-parameter const u8 ***msgs; instead it appends to gs->last_writes. gossip_store_rewrite_end now reads from gs->last_writes, and a new gossip_store_writes_confirmed resets it. gossip_store_fsync is made static. Callers in gossmap_manage.c are updated accordingly. The change is architectural: it centralizes write-recovery state in the component responsible for file I/O.
Changed components
gossipd/gossip_store.cgossipd/gossip_store.hgossipd/gossmap_manage.cInspect captured patch +34 / −29
diff --git a/gossipd/gossip_store.c b/gossipd/gossip_store.c
index 38366533..93b8d36f 100644
--- a/gossipd/gossip_store.c
+++ b/gossipd/gossip_store.c
@@ -35,6 +35,10 @@ struct gossip_store {
/* Timestamp of store when we opened it (0 if we created it) */
u32 timestamp;
+
+ /* Last writes since previous sync, in case it messes up and
+ * we need to force it. */
+ const u8 **last_writes;
};
static void gossip_store_destroy(struct gossip_store *gs)
@@ -414,20 +418,22 @@ struct gossip_store *gossip_store_new(const tal_t *ctx,
gs->fd = gossip_store_upgrade(daemon, &gs->len, populated);
if (gs->fd < 0)
return tal_free(gs);
+ gs->last_writes = tal_arr(gs, const u8 *, 0);
tal_add_destructor(gs, gossip_store_destroy);
return gs;
}
-void gossip_store_fsync(const struct gossip_store *gs)
+static void gossip_store_fsync(const struct gossip_store *gs)
{
if (fsync(gs->fd) != 0)
status_failed(STATUS_FAIL_INTERNAL_ERROR,
"gossmap fsync failed: %s", strerror(errno));
}
-void gossip_store_rewrite_end(struct gossip_store *gs, const u8 **msgs)
+void gossip_store_rewrite_end(struct gossip_store *gs)
{
u64 offset = gs->len;
+ const u8 **msgs = gs->last_writes;
for (size_t i = 0; i < tal_count(msgs); i++) {
/* Don't overwrite version byte */
@@ -447,10 +453,15 @@ void gossip_store_rewrite_end(struct gossip_store *gs, const u8 **msgs)
gossip_store_fsync(gs);
}
+void gossip_store_writes_confirmed(struct gossip_store *gs)
+{
+ tal_free(gs->last_writes);
+ gs->last_writes = tal_arr(gs, const u8 *, 0);
+}
+
u64 gossip_store_add(struct gossip_store *gs,
const u8 *gossip_msg,
- u32 timestamp,
- const u8 ***msgs)
+ u32 timestamp)
{
u64 off = gs->len, filelen;
@@ -470,7 +481,7 @@ u64 gossip_store_add(struct gossip_store *gs,
filelen, off);
}
- if (!append_msg(gs->fd, gossip_msg, timestamp, &gs->len, msgs)) {
+ if (!append_msg(gs->fd, gossip_msg, timestamp, &gs->len, &gs->last_writes)) {
status_failed(STATUS_FAIL_INTERNAL_ERROR,
"Failed writing to gossip store: %s",
strerror(errno));
diff --git a/gossipd/gossip_store.h b/gossipd/gossip_store.h
index 5d839169..81e76d7d 100644
--- a/gossipd/gossip_store.h
+++ b/gossipd/gossip_store.h
@@ -44,14 +44,12 @@ void gossip_store_corrupt(void);
* @gs: gossip store
* @gossip_msg: the gossip message to insert.
* @timestamp: the timestamp for filtering of this messsage.
- * @msgs: the option pointer to a u8 *array to append the written msgs to.
*
* Returns the offset (after the gossip_hdr).
*/
u64 gossip_store_add(struct gossip_store *gs,
const u8 *gossip_msg,
- u32 timestamp,
- const u8 ***msgs);
+ u32 timestamp);
/**
* Delete the record at this offset (offset is that of
@@ -107,12 +105,16 @@ void gossip_store_set_timestamp(struct gossip_store *gs, u64 offset, u32 timesta
/**
* We've seen (ZFS on Linux) writes not show up in the gossip store.
* This lets us rewrite the last bytes. */
-void gossip_store_rewrite_end(struct gossip_store *gs, const u8 **msgs);
+void gossip_store_rewrite_end(struct gossip_store *gs);
+
+/**
+ * Once we've checked the contents are good, the last_writes storage can
+ * be reset. */
+void gossip_store_writes_confirmed(struct gossip_store *gs);
/**
* For debugging.
*/
u64 gossip_store_len_written(const struct gossip_store *gs);
-void gossip_store_fsync(const struct gossip_store *gs);
#endif /* LIGHTNING_GOSSIPD_GOSSIP_STORE_H */
diff --git a/gossipd/gossmap_manage.c b/gossipd/gossmap_manage.c
index c6cf4862..a9d7f2b8 100644
--- a/gossipd/gossmap_manage.c
+++ b/gossipd/gossmap_manage.c
@@ -65,10 +65,6 @@ struct gossmap_manage {
/* gossip map itself (access via gossmap_manage_get_gossmap, so it's fresh!) */
struct gossmap *raw_gossmap;
- /* Last writes to gossmap since previous sync, in case it
- * messes up and we need to force it. */
- const u8 **last_writes;
-
/* The gossip_store, which writes to the gossip_store file */
struct gossip_store *gs;
@@ -268,7 +264,7 @@ static void remove_channel(struct gossmap_manage *gm,
/* Put in tombstone marker. */
gossip_store_add(gm->gs,
towire_gossip_store_delete_chan(tmpctx, scid),
- 0, &gm->last_writes);
+ 0);
/* Delete from store */
gossip_store_del(gm->gs, chan->cann_off, WIRE_CHANNEL_ANNOUNCEMENT);
@@ -309,7 +305,7 @@ static void remove_channel(struct gossmap_manage *gm,
timestamp = gossip_store_get_timestamp(gm->gs, node->nann_off);
gossip_store_del(gm->gs, node->nann_off, WIRE_NODE_ANNOUNCEMENT);
- offset = gossip_store_add(gm->gs, nannounce, timestamp, &gm->last_writes);
+ offset = gossip_store_add(gm->gs, nannounce, timestamp);
} else {
/* Are all remaining channels dying but we weren't?
* Can happen if we removed this channel immediately
@@ -481,7 +477,6 @@ static bool setup_gossmap(struct gossmap_manage *gm,
gm->gs = tal_free(gm->gs);
return false;
}
- gm->last_writes = tal_arr(gm, const u8 *, 0);
gossmap_stats(gm->raw_gossmap, &num_live, &num_dead);
status_debug("gossip_store: %"PRIu64" live records, %"PRIu64" deleted",
@@ -636,10 +631,9 @@ const char *gossmap_manage_channel_announcement(const tal_t *ctx,
*/
if (known_amount) {
/* Set with timestamp 0 (we will update once we have a channel_update) */
- gossip_store_add(gm->gs, announce, 0, &gm->last_writes);
+ gossip_store_add(gm->gs, announce, 0);
gossip_store_add(gm->gs,
- towire_gossip_store_channel_amount(tmpctx, *known_amount), 0,
- &gm->last_writes);
+ towire_gossip_store_channel_amount(tmpctx, *known_amount), 0);
node_announcements_not_dying(gm, gossmap, pca);
tal_free(pca);
@@ -770,10 +764,9 @@ void gossmap_manage_handle_get_txout_reply(struct gossmap_manage *gm, const u8 *
}
/* Set with timestamp 0 (we will update once we have a channel_update) */
- gossip_store_add(gm->gs, pca->channel_announcement, 0, &gm->last_writes);
+ gossip_store_add(gm->gs, pca->channel_announcement, 0);
gossip_store_add(gm->gs,
- towire_gossip_store_channel_amount(tmpctx, sat), 0,
- &gm->last_writes);
+ towire_gossip_store_channel_amount(tmpctx, sat), 0);
/* If we looking specifically for this, we no longer are. */
remove_unknown_scid(gm->daemon->seeker, &scid, true);
@@ -875,7 +868,7 @@ static const char *process_channel_update(const tal_t *ctx,
}
/* OK, apply the new one */
- offset = gossip_store_add(gm->gs, update, timestamp, &gm->last_writes);
+ offset = gossip_store_add(gm->gs, update, timestamp);
/* If channel is dying, make sure update is also marked dying! */
if (gossmap_chan_is_dying(gossmap, chan)) {
@@ -1054,7 +1047,7 @@ static void process_node_announcement(struct gossmap_manage *gm,
}
/* OK, apply the new one */
- offset = gossip_store_add(gm->gs, nannounce, timestamp, &gm->last_writes);
+ offset = gossip_store_add(gm->gs, nannounce, timestamp);
/* If all channels are dying, make sure this is marked too. */
if (all_node_channels_dying(gossmap, node, NULL)) {
gossip_store_set_flag(gm->gs, offset,
@@ -1381,7 +1374,7 @@ void gossmap_manage_channel_spent(struct gossmap_manage *gm,
/* Save to gossip_store in case we restart */
msg = towire_gossip_store_chan_dying(tmpctx, cd.scid, cd.deadline);
- cd.gossmap_offset = gossip_store_add(gm->gs, msg, 0, &gm->last_writes);
+ cd.gossmap_offset = gossip_store_add(gm->gs, msg, 0);
tal_arr_expand(&gm->dying_channels, cd);
/* Mark it dying, so we don't gossip it */
@@ -1485,7 +1478,7 @@ struct gossmap *gossmap_manage_get_gossmap(struct gossmap_manage *gm)
gossmap_disable_mmap(gm->raw_gossmap);
/* Try rewriting the last few records, syncing. */
- gossip_store_rewrite_end(gm->gs, gm->last_writes);
+ gossip_store_rewrite_end(gm->gs);
gossmap_refresh(gm->raw_gossmap);
map_used = gossmap_lengths(gm->raw_gossmap, &map_size);
@@ -1497,8 +1490,7 @@ struct gossmap *gossmap_manage_get_gossmap(struct gossmap_manage *gm)
}
/* Free up last_writes, since we've seen it on disk */
- tal_free(gm->last_writes);
- gm->last_writes = tal_arr(gm, const u8 *, 0);
+ gossip_store_writes_confirmed(gm->gs);
return gm->raw_gossmap;
}
Why this scored 15/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.