wallet: update chain_moves and channel_moves tables from channel_id -> nonchannel in db when we close it.
What the developer wrote
Authored by Rusty Russell
wallet: update chain_moves and channel_moves tables from channel_id -> nonchannel in db when we close it.
This avoids us keeping references into closed channels.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
What changed, and why it matters
This change updates Core Lightning's internal accounting records when a payment channel closes. Previously, records in the `chain_moves` and `channel_moves` database tables kept pointing to the closed channel's internal database ID. After this patch, they are rewritten to reference a stable text identifier (the channel ID) instead. The commit message says this avoids dangling references to closed channels and makes future cleanup easier. There is no direct evidence in the commit of an exploitable security vulnerability; it looks like a data-integrity and maintainability fix.
Treat as a routine data-integrity/maintenance patch. Review whether any other tables still hold `channel_id` dbid references after close that could become dangling, and confirm the new `move_accounts_id` lookup cannot fail or leave a null reference. No urgent security action is indicated by the diff alone.
Security signals we found
Database referential-integrity cleanup on channel close
Avoids dangling foreign-key-like references to deleted channel rows
No input validation, cryptographic, or network changes visible
No explicit security framing by the author
Evidence from the diff
In wallet_channel_close(), three new SQL UPDATE statements now run before the channel row is deleted: they migrate rows in chain_moves and channel_moves from account_channel_id/originating_channel_id to account_nonchannel_id/originating_nonchannel_id, using move_accounts_id(w, fmt_channel_id(...)) as the new non-channel account key. The old channel dbid is bound as the WHERE clause. Test stubs for channel_id serialization functions were removed and common/channel_id.o was added to the wallet test Makefile so the real implementations are linked. The patch is partial in the sense that it only converts the accounting tables; other channel_id references may remain.
Changed components
wallet/wallet.cwallet/test/run-db.cwallet/test/run-wallet.cwallet/test/MakefileInspect captured patch +33 / −21
diff --git a/wallet/test/Makefile b/wallet/test/Makefile
index 61789189..c986342d 100644
--- a/wallet/test/Makefile
+++ b/wallet/test/Makefile
@@ -10,6 +10,7 @@ WALLET_TEST_COMMON_OBJS := \
common/autodata.o \
common/base32.o \
common/blockheight_states.o \
+ common/channel_id.o \
common/channel_type.o \
common/derive_basepoints.o \
common/features.o \
diff --git a/wallet/test/run-db.c b/wallet/test/run-db.c
index b0d5ee9a..43ea5340 100644
--- a/wallet/test/run-db.c
+++ b/wallet/test/run-db.c
@@ -59,10 +59,6 @@ struct onionreply *create_onionreply(const tal_t *ctx UNNEEDED,
const struct secret *shared_secret UNNEEDED,
const u8 *failure_msg UNNEEDED)
{ fprintf(stderr, "create_onionreply called!\n"); abort(); }
-/* Generated stub for derive_channel_id */
-void derive_channel_id(struct channel_id *channel_id UNNEEDED,
- const struct bitcoin_outpoint *outpoint UNNEEDED)
-{ fprintf(stderr, "derive_channel_id called!\n"); abort(); }
/* Generated stub for fatal */
void fatal(const char *fmt UNNEEDED, ...)
{ fprintf(stderr, "fatal called!\n"); abort(); }
@@ -72,9 +68,6 @@ void fatal_vfmt(const char *fmt UNNEEDED, va_list ap UNNEEDED)
/* Generated stub for find_peer_by_dbid */
struct peer *find_peer_by_dbid(struct lightningd *ld UNNEEDED, u64 dbid UNNEEDED)
{ fprintf(stderr, "find_peer_by_dbid called!\n"); abort(); }
-/* Generated stub for fmt_channel_id */
-char *fmt_channel_id(const tal_t *ctx UNNEEDED, const struct channel_id *channel_id UNNEEDED)
-{ fprintf(stderr, "fmt_channel_id called!\n"); abort(); }
/* Generated stub for forward_index_created */
u64 forward_index_created(struct lightningd *ld UNNEEDED,
enum forward_status status UNNEEDED,
diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c
index ae927e24..72deac48 100644
--- a/wallet/test/run-wallet.c
+++ b/wallet/test/run-wallet.c
@@ -233,10 +233,6 @@ bool depthcb_update_scid(struct channel *channel UNNEEDED,
const struct bitcoin_txid *txid UNNEEDED,
const struct bitcoin_outpoint *outpoint UNNEEDED)
{ fprintf(stderr, "depthcb_update_scid called!\n"); abort(); }
-/* Generated stub for derive_channel_id */
-void derive_channel_id(struct channel_id *channel_id UNNEEDED,
- const struct bitcoin_outpoint *outpoint UNNEEDED)
-{ fprintf(stderr, "derive_channel_id called!\n"); abort(); }
/* Generated stub for dev_disconnect_permanent */
bool dev_disconnect_permanent(struct lightningd *ld UNNEEDED)
{ fprintf(stderr, "dev_disconnect_permanent called!\n"); abort(); }
@@ -254,9 +250,6 @@ void fatal(const char *fmt UNNEEDED, ...)
/* Generated stub for fatal_vfmt */
void fatal_vfmt(const char *fmt UNNEEDED, va_list ap UNNEEDED)
{ fprintf(stderr, "fatal_vfmt called!\n"); abort(); }
-/* Generated stub for fmt_channel_id */
-char *fmt_channel_id(const tal_t *ctx UNNEEDED, const struct channel_id *channel_id UNNEEDED)
-{ fprintf(stderr, "fmt_channel_id called!\n"); abort(); }
/* Generated stub for force_peer_disconnect */
void force_peer_disconnect(struct lightningd *ld UNNEEDED,
const struct peer *peer UNNEEDED,
@@ -286,10 +279,6 @@ u64 forward_index_update_status(struct lightningd *ld UNNEEDED,
struct amount_msat in_amount UNNEEDED,
const struct short_channel_id *out_channel UNNEEDED)
{ fprintf(stderr, "forward_index_update_status called!\n"); abort(); }
-/* Generated stub for fromwire_channel_id */
-bool fromwire_channel_id(const u8 **cursor UNNEEDED, size_t *max UNNEEDED,
- struct channel_id *channel_id UNNEEDED)
-{ fprintf(stderr, "fromwire_channel_id called!\n"); abort(); }
/* Generated stub for fromwire_channel_type */
struct channel_type *fromwire_channel_type(const tal_t *ctx UNNEEDED, const u8 **cursor UNNEEDED, size_t *plen UNNEEDED)
{ fprintf(stderr, "fromwire_channel_type called!\n"); abort(); }
@@ -1056,9 +1045,6 @@ void topology_add_sync_waiter_(const tal_t *ctx UNNEEDED,
/* Generated stub for towire_announcement_signatures */
u8 *towire_announcement_signatures(const tal_t *ctx UNNEEDED, const struct channel_id *channel_id UNNEEDED, struct short_channel_id short_channel_id UNNEEDED, const secp256k1_ecdsa_signature *node_signature UNNEEDED, const secp256k1_ecdsa_signature *bitcoin_signature UNNEEDED)
{ fprintf(stderr, "towire_announcement_signatures called!\n"); abort(); }
-/* Generated stub for towire_channel_id */
-void towire_channel_id(u8 **pptr UNNEEDED, const struct channel_id *channel_id UNNEEDED)
-{ fprintf(stderr, "towire_channel_id called!\n"); abort(); }
/* Generated stub for towire_channel_reestablish */
u8 *towire_channel_reestablish(const tal_t *ctx UNNEEDED, const struct channel_id *channel_id UNNEEDED, u64 next_commitment_number UNNEEDED, u64 next_revocation_number UNNEEDED, const struct secret *your_last_per_commitment_secret UNNEEDED, const struct pubkey *my_current_per_commitment_point UNNEEDED, const struct tlv_channel_reestablish_tlvs *channel_reestablish UNNEEDED)
{ fprintf(stderr, "towire_channel_reestablish called!\n"); abort(); }
diff --git a/wallet/wallet.c b/wallet/wallet.c
index 451d35b8..f359c8a9 100644
--- a/wallet/wallet.c
+++ b/wallet/wallet.c
@@ -2961,6 +2961,7 @@ void wallet_channel_close(struct wallet *w,
* reestablish messages with enough information for nodes with lost
* dbs to recover. */
struct db_stmt *stmt;
+ u64 new_move_id;
/* Delete entries from `channel_htlcs` */
stmt = db_prepare_v2(w->db, SQL("DELETE FROM channel_htlcs "
@@ -3016,6 +3017,37 @@ void wallet_channel_close(struct wallet *w,
db_bind_u64(stmt, channel_state_in_db(CLOSED));
db_bind_u64(stmt, chan->dbid);
db_exec_prepared_v2(take(stmt));
+
+ /* Update all accouting records to use channel_id string, instead of
+ * referring to dbid. This is robust if we delete in future, and saves
+ * a lookup in the load path. */
+ new_move_id = move_accounts_id(w, fmt_channel_id(tmpctx, &chan->cid));
+ stmt = db_prepare_v2(w->db, SQL("UPDATE chain_moves "
+ "SET account_channel_id=?,"
+ " account_nonchannel_id=? "
+ "WHERE account_channel_id=?"));
+ db_bind_null(stmt);
+ db_bind_u64(stmt, new_move_id);
+ db_bind_u64(stmt, chan->dbid);
+ db_exec_prepared_v2(take(stmt));
+
+ stmt = db_prepare_v2(w->db, SQL("UPDATE channel_moves "
+ "SET account_channel_id=?,"
+ " account_nonchannel_id=? "
+ "WHERE account_channel_id=?"));
+ db_bind_null(stmt);
+ db_bind_u64(stmt, new_move_id);
+ db_bind_u64(stmt, chan->dbid);
+ db_exec_prepared_v2(take(stmt));
+
+ stmt = db_prepare_v2(w->db, SQL("UPDATE chain_moves "
+ "SET originating_channel_id=?,"
+ " originating_nonchannel_id=? "
+ "WHERE originating_channel_id=?"));
+ db_bind_null(stmt);
+ db_bind_u64(stmt, new_move_id);
+ db_bind_u64(stmt, chan->dbid);
+ db_exec_prepared_v2(take(stmt));
}
/* Completely unused channels get wiped entirely (we've already closed it above) */
Why this scored 20/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.