bookkeeper: fix up out-of-order migrations in rc1

This was introduced in rc1, resulting in: ``` 2024-08-15T01:33:48.343Z **BROKEN** plugin-bookkeeper: query failed: plugins/bkpr/recorder.c:738: SELECT e.id, e.account_id, a.name, e.origin, e.tag, e.credit, e.debit, e.output_value, e.currency, e.timestamp, e.blockheight, e.utxo_txid, e.outnum, e.spending_txid, e.payment_id, e.ignored, e.stealable, e.ev_desc, e.spliced FROM chain_events e LEFT OUTER JOIN accounts a ON e.account_id = a.id WHERE e.spending_txid = ? AND e.account_id = ? AND e.utxo_txid = ? AND e.outnum = ? ``` Signed-off-by: Rusty Russell <[email protected]>
ElementsProject · Aug 15, 2024 · a6c2f18 · a6c2f18
1 parent be5ad3d
commit a6c2f18
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 1 deletion.
diff --git a/plugins/bkpr/db.c b/plugins/bkpr/db.c
@@ -14,6 +14,7 @@ struct migration {
 };
 
 static void migration_remove_dupe_lease_fees(struct plugin *p, struct db *db);
+static void migration_maybe_add_chainevents_spliced(struct plugin *p, struct db *db);
 
 /* Do not reorder or remove elements from this array.
  * It is used to migrate existing databases from a prevoius state, based on
@@ -100,7 +101,8 @@ static struct migration db_migrations[] = {
 	{SQL("ALTER TABLE channel_events ADD ev_desc TEXT DEFAULT NULL;"), NULL},
 	{SQL("ALTER TABLE channel_events ADD rebalance_id BIGINT DEFAULT NULL;"), NULL},
 	{SQL("ALTER TABLE chain_events ADD spliced INTEGER DEFAULT 0;"), NULL},
-	{NULL, migration_remove_dupe_lease_fees}
+	{NULL, migration_remove_dupe_lease_fees},
+	{NULL, migration_maybe_add_chainevents_spliced}
 };
 
 static bool db_migrate(struct plugin *p, struct db *db)
@@ -184,6 +186,27 @@ static void migration_remove_dupe_lease_fees(struct plugin *p, struct db *db)
 	tal_free(stmt);
 }
 
+/* OK, funny story.  We added the "ALTER TABLE chain_events ADD spliced INTEGER DEFAULT 0;"
+ * migration in the wrong place, NOT at the end.  So if you are migrating from an old version,
+ * "migration_remove_dupe_lease_fees" ran (again), which is harmless, but this migration
+ * never got added. */
+static void migration_maybe_add_chainevents_spliced(struct plugin *p, struct db *db)
+{
+	struct db_stmt *stmt;
+	bool col_exists;
+
+	stmt = db_prepare_v2(db, SQL("SELECT spliced FROM chain_events"));
+	col_exists = db_query_prepared_canfail(stmt);
+	tal_free(stmt);
+	if (col_exists)
+		return;
+
+	plugin_log(p, LOG_INFORM,
+		   "Database fixup: adding spliced column to chain_events table");
+	stmt = db_prepare_v2(db, SQL("ALTER TABLE chain_events ADD spliced INTEGER DEFAULT 0;"));
+	db_exec_prepared_v2(take(stmt));
+}
+
 static void db_error(struct plugin *plugin, bool fatal, const char *fmt, va_list ap)
 {
 	if (fatal)

diff --git a/tests/data/bookkeeper-accounts-pre-v24.08-migration.sqlite3.xz b/tests/data/bookkeeper-accounts-pre-v24.08-migration.sqlite3.xz
diff --git a/tests/test_bookkeeper.py b/tests/test_bookkeeper.py
@@ -947,3 +947,16 @@ def test_bookkeeper_custom_notifs(node_factory):
     incomes = l1.rpc.bkpr_listincome()['income_events']
     acct_fee = only_one([inc['debit_msat'] for inc in incomes if inc['account'] == acct and inc['tag'] == 'onchain_fee'])
     assert acct_fee == Millisatoshi(fee)
+
+
+@unittest.skipIf(os.getenv('TEST_DB_PROVIDER', 'sqlite3') != 'sqlite3', "This test is based on a sqlite3 snapshot")
+def test_bookkeeper_bad_migration(node_factory):
+    l1 = node_factory.get_node(bkpr_dbfile='bookkeeper-accounts-pre-v24.08-migration.sqlite3.xz')
+    l2 = node_factory.get_node()
+
+    # Make sure l1 does fixup, use query to force an access (which would fail if column not present)
+    assert l1.daemon.is_in_log("plugin-bookkeeper: Database fixup: adding spliced column to chain_events table")
+    l1.rpc.bkpr_listaccountevents('wallet')
+
+    # l2 will *not* do this
+    assert not l2.daemon.is_in_log("adding spliced column")