wallet: find and remove any duplicates from the bug we just fixed.
What changed, and why it matters
This commit adds a database cleanup routine that removes accidentally duplicated accounting records created by a previous bug. It is a data-integrity fix, not a security patch that stops an active attack. The underlying bug allowed the same on-chain money movement to be recorded twice—once under a live channel reference and once under a non-channel account reference—so users' coin-movement reports could be wrong until the duplicates are removed.
Treat as a routine data-integrity follow-up. Operators should allow the migration to run at startup and verify that coin-movement/accounting reports look correct afterward. There is no immediate security action required, but the prior bug that caused duplicates should be reviewed separately if it has not already been assessed.
Security signals we found
Database migration to repair duplicate records caused by a prior bug
References a recently fixed bug in the same accounting/import code path
Corrects data integrity of on-chain coin-movement records
No input validation, authentication, cryptography, or network handling changes
Evidence from the diff
The patch introduces a new migration function, migrate_remove_chain_moves_duplicates(), registered in wallet/db.c’s migration table. It scans the chain_moves table, joins move_accounts and channels to normalize account identifiers, and uses an existing find_duplicate_chain_move() helper to detect rows that represent the same UTXO/spending transaction/tag set under logically equivalent account references. Detected duplicates are deleted. The commit comment explains the root cause: when importing from the legacy accounts.db, live channels used account_channel_id while replays used account_nonchannel_id, bypassing duplicate detection. The fix is purely corrective and runs at startup.
Changed components
wallet/db.cwallet/wallet.cwallet/wallet.hchain_moves tablemove_accounts tablechannels tableInspect captured patch +84 / −0
diff --git a/wallet/db.c b/wallet/db.c
index 6057952d..eb0ae7be 100644
--- a/wallet/db.c
+++ b/wallet/db.c
@@ -1093,6 +1093,8 @@ static struct migration dbmigrations[] = {
/* We do a lookup before each append, to avoid duplicates */
{SQL("CREATE INDEX chain_moves_utxo_idx ON chain_moves (utxo)"), NULL},
{NULL, migrate_from_account_db},
+ /* We accidentally allowed duplicate entries */
+ {NULL, migrate_remove_chain_moves_duplicates}
};
/**
diff --git a/wallet/wallet.c b/wallet/wallet.c
index 7537b11a..599e2f92 100644
--- a/wallet/wallet.c
+++ b/wallet/wallet.c
@@ -7723,3 +7723,84 @@ void migrate_setup_coinmoves(struct lightningd *ld, struct db *db)
tal_free(stmt);
}
+/* When we imported from accounts.db, we always used a reference into
+ * the move_accounts table (via account_nonchannel_id). But (on
+ * replay) if a channel was live, we used the reference into the
+ * channels table (via account_channel_id) and our duplicate detection
+ * didn't trigger. Now we need to get rid of such duplicates.
+ *
+ * Note that if the channel is now CLOSED, the references to account_channel_id
+ * will have been converted to references using account_nonchannel_id. */
+void migrate_remove_chain_moves_duplicates(struct lightningd *ld, struct db *db)
+{
+ /* This is O(n^2) but there just aren't that many! */
+ u64 *to_delete = tal_arr(tmpctx, u64, 0);
+ struct db_stmt *stmt;
+
+ /* Gather */
+ stmt = db_prepare_v2(db, SQL("SELECT"
+ " chain_moves.id,"
+ " utxo,"
+ " spending_txid,"
+ " tag_bitmap,"
+ " account_channel_id,"
+ " channels.full_channel_id,"
+ " move_accounts.name"
+ " FROM chain_moves "
+ " LEFT JOIN move_accounts "
+ " ON move_accounts.id = chain_moves.account_nonchannel_id "
+ " LEFT JOIN channels "
+ " ON channels.id = chain_moves.account_channel_id "
+ " ORDER BY move_accounts.id;"));
+ db_query_prepared(stmt);
+ while (db_step(stmt)) {
+ struct bitcoin_outpoint outpoint;
+ u64 id, channel_dbid;
+ struct bitcoin_txid *spending_txid;
+ struct mvt_tags tags;
+ const char *nonchannel_acctname;
+
+ id = db_col_u64(stmt, "chain_moves.id");
+ db_col_outpoint(stmt, "utxo", &outpoint);
+ if (db_col_is_null(stmt, "spending_txid"))
+ spending_txid = NULL;
+ else {
+ spending_txid = tal(tmpctx, struct bitcoin_txid);
+ db_col_txid(stmt, "spending_txid", spending_txid);
+ }
+ tags = db_col_mvt_tags(stmt, "tag_bitmap");
+ if (db_col_is_null(stmt, "account_channel_id")) {
+ channel_dbid = 0;
+ nonchannel_acctname = db_col_strdup(tmpctx, stmt, "move_accounts.name");
+ db_col_ignore(stmt, "channels.full_channel_id");
+ } else {
+ struct channel_id cid;
+ channel_dbid = db_col_u64(stmt, "account_channel_id");
+ db_col_channel_id(stmt, "channels.full_channel_id", &cid);
+ nonchannel_acctname = fmt_channel_id(tmpctx, &cid);
+ }
+
+ if (find_duplicate_chain_move(db, nonchannel_acctname,
+ channel_dbid,
+ &outpoint,
+ spending_txid,
+ tags,
+ id)) {
+ log_unusual(ld->log,
+ "Deleting redundant chain_moves %"PRIu64" for account %s",
+ id, nonchannel_acctname);
+ tal_arr_expand(&to_delete, id);
+ }
+ }
+ tal_free(stmt);
+
+ /* Do the delete. We do it separately to avoid any db issues
+ * while iterating */
+ for (size_t i = 0; i < tal_count(to_delete); i++) {
+ stmt = db_prepare_v2(db,
+ SQL("DELETE FROM chain_moves "
+ "WHERE id = ?;"));
+ db_bind_u64(stmt, to_delete[i]);
+ db_exec_prepared_v2(take(stmt));
+ }
+}
diff --git a/wallet/wallet.h b/wallet/wallet.h
index 08b8b7e1..f956196b 100644
--- a/wallet/wallet.h
+++ b/wallet/wallet.h
@@ -1926,6 +1926,7 @@ void wallet_datastore_save_payment_description(struct db *db,
const struct sha256 *payment_hash,
const char *desc);
void migrate_setup_coinmoves(struct lightningd *ld, struct db *db);
+void migrate_remove_chain_moves_duplicates(struct lightningd *ld, struct db *db);
/**
* wallet_memleak_scan - Check for memleaks in wallet.
Why this scored 27/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.