From a6c2f183d6c71f2845ef117cbb69727294081001 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 15 Aug 2024 13:41:04 +0930 Subject: [PATCH] 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 --- plugins/bkpr/db.c | 25 +++++++++++++++++- ...r-accounts-pre-v24.08-migration.sqlite3.xz | Bin 0 -> 960 bytes tests/test_bookkeeper.py | 13 +++++++++ 3 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 tests/data/bookkeeper-accounts-pre-v24.08-migration.sqlite3.xz diff --git a/plugins/bkpr/db.c b/plugins/bkpr/db.c index 06943af91..aecfb36ec 100644 --- 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 new file mode 100644 index 0000000000000000000000000000000000000000..3ad3805a2a0dd48a2bba6e223f8c3e717d382fa8 GIT binary patch literal 960 zcmV;x13&!zH+ooF000E$*0e?f03iVu0001VFXf})kN*RHT>vSRMV(;C8Tck-h>j-y zF_rzthkyo`yX=3c34Oltq+khqIep+1g$El-0XS`Vt(B)k&^3HX1jLkOwDp3KQfIEY z+8tda&3>Y^DAV!bDE3&UU@_~x^$fFq*Sya z^?znAnvV<|!Xuhz)(u)}P*PTiVKVWWDXq3EhaxVR6d31o;P)8Sc6o`cVIjuG-?B!C z{Lrq(D!Od}Q%vEgIWk?ZfL*$g+RmtHu9K8hI_$y7k(i#vspS^#+Oyw(rhp8e2<^sS z!z;n5lPVnVy*9U;#Q*~llybkyz|`;rQFw_*mlL9^$w){L6t{x06_cQV;^Un}-J+%*?iTa;C>M#U?P?#-XsRbEaAd_s7)QG2$2d zH*zDlC7BPxQ}D~bkiOyEyO1xps5T7ely`Ap)*hIyRz{tbHyH3!w@ivn*+D{&9Vi@m z_PE?Py6BHya_f$Ot>#y8*b8pR;}BQ$RU_kpDcf}X+nMbkAUuew*(i2U1FGCHZ&gG5Vk!kJv?p&89CvX{cCUrjh0G zIz|uLp?$6Gsf`uz$i*mD0#4|fn!8yWKLTvIs@L2nEHOBZ2@{O^`#up#ocLMAPz4gf zROc<%#T4n0zdfQ^b^7G|d>?uOr;aTB>M^=89=PKyle($X-u=2fJy8((=`~ite+-Kv zhW#WQuHSf4eyU>^YZ$lD`?=C`?&N5-H<2@T3Ol3)YXQJ~G~`tt;aDxY>=<+1F$`D! zB|^?Er}6m1DK(ki3fx`ZT{G<{Z3#SV=u0_k&Lz%0n_Iyz!sTViJZZl&mHv#?*-v+% zKef7{_lk>=(XAN}fq^yPD_onEy9UeaR&j?|a!vc8$0lCvOv_7@7DPl3l(Ahf8xzhY z1jvEKp7a97rd=8U0h$MZpaK9fw4ckd#Ao{g000001X)^E2i+Y2 literal 0 HcmV?d00001 diff --git a/tests/test_bookkeeper.py b/tests/test_bookkeeper.py index c89783976..c32a59f60 100644 --- 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")