From d7c1325e38dfa15ccb2021430d118ee6a14dd1ee Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 19 Sep 2022 10:19:53 +0930 Subject: [PATCH] wallet: use scid not string for failchannel (now failscid) in payments table. And remove the now-unused string-based helper functions. Signed-off-by: Rusty Russell --- db/bindings.c | 18 ----------------- db/bindings.h | 6 ------ tests/test_db.py | 2 +- wallet/db.c | 42 ++++++++++++++++++++++++++++++++++++++++ wallet/test/run-wallet.c | 2 +- wallet/wallet.c | 12 +++++------- 6 files changed, 49 insertions(+), 33 deletions(-) diff --git a/db/bindings.c b/db/bindings.c index 2f3546a2b..fc95ca52f 100644 --- a/db/bindings.c +++ b/db/bindings.c @@ -159,13 +159,6 @@ void db_bind_pubkey(struct db_stmt *stmt, int pos, const struct pubkey *pk) db_bind_blob(stmt, pos, der, PUBKEY_CMPR_LEN); } -void db_bind_short_channel_id_str(struct db_stmt *stmt, int col, - const struct short_channel_id *id) -{ - char *ser = short_channel_id_to_str(stmt, id); - db_bind_text(stmt, col, ser); -} - void db_bind_scid(struct db_stmt *stmt, int col, const struct short_channel_id *id) { @@ -368,17 +361,6 @@ void db_col_pubkey(struct db_stmt *stmt, assert(ok); } -/* Yes, we put this in as a string. Past mistakes; do not use! */ -bool db_col_short_channel_id_str(struct db_stmt *stmt, const char *colname, - struct short_channel_id *dest) -{ - size_t col = db_query_colnum(stmt, colname); - const char *source = db_column_blob(stmt, col); - size_t sourcelen = db_column_bytes(stmt, col); - db_column_null_warn(stmt, colname, col); - return short_channel_id_from_str(source, sourcelen, dest); -} - void db_col_scid(struct db_stmt *stmt, const char *colname, struct short_channel_id *dest) { diff --git a/db/bindings.h b/db/bindings.h index 74febcf83..cc7707cf5 100644 --- a/db/bindings.h +++ b/db/bindings.h @@ -37,9 +37,6 @@ void db_bind_node_id(struct db_stmt *stmt, int pos, const struct node_id *ni); void db_bind_node_id_arr(struct db_stmt *stmt, int col, const struct node_id *ids); void db_bind_pubkey(struct db_stmt *stmt, int pos, const struct pubkey *p); -/* DO NOT USE, deprecated! */ -void db_bind_short_channel_id_str(struct db_stmt *stmt, int col, - const struct short_channel_id *id); void db_bind_scid(struct db_stmt *stmt, int col, const struct short_channel_id *id); void db_bind_short_channel_id_arr(struct db_stmt *stmt, int col, @@ -86,9 +83,6 @@ struct node_id *db_col_node_id_arr(const tal_t *ctx, struct db_stmt *stmt, const char *colname); void db_col_pubkey(struct db_stmt *stmt, const char *colname, struct pubkey *p); -/* DO NOT USE: deprecated */ -bool db_col_short_channel_id_str(struct db_stmt *stmt, const char *colname, - struct short_channel_id *dest); void db_col_scid(struct db_stmt *stmt, const char *colname, struct short_channel_id *dest); struct short_channel_id * diff --git a/tests/test_db.py b/tests/test_db.py index de3e2ae3d..677a5d290 100644 --- a/tests/test_db.py +++ b/tests/test_db.py @@ -162,7 +162,7 @@ def test_scid_upgrade(node_factory, bitcoind): l1.daemon.opts['database-upgrade'] = True l1.daemon.start() assert l1.db_query('SELECT scid FROM channels;') == [{'scid': scid_to_int('103x1x1')}] - assert l1.db_query('SELECT failchannel from payments;') == [{'failchannel': '103x1x1'}] + assert l1.db_query('SELECT failscid FROM payments;') == [{'failscid': scid_to_int('103x1x1')}] @unittest.skipIf(not COMPAT, "needs COMPAT to convert obsolete db") diff --git a/wallet/db.c b/wallet/db.c index 0e206b255..a634bfea2 100644 --- a/wallet/db.c +++ b/wallet/db.c @@ -63,6 +63,10 @@ static void migrate_channels_scids_as_integers(struct lightningd *ld, struct db *db, const struct migration_context *mc); +static void migrate_payments_scids_as_integers(struct lightningd *ld, + struct db *db, + const struct migration_context *mc); + /* Do not reorder or remove elements from this array, it is used to * migrate existing databases from a previous state, based on the * string indices */ @@ -926,6 +930,7 @@ static struct migration dbmigrations[] = { {SQL("DROP TABLE forwarded_payments;"), NULL}, /* Adds scid column, then moves short_channel_id across to it */ {SQL("ALTER TABLE channels ADD scid BIGINT;"), migrate_channels_scids_as_integers}, + {SQL("ALTER TABLE payments ADD failscid BIGINT;"), migrate_payments_scids_as_integers}, }; /* Released versions are of form v{num}[.{num}]* */ @@ -1504,3 +1509,40 @@ static void migrate_channels_scids_as_integers(struct lightningd *ld, * When we can assume sqlite3 2021-04-19 (3.35.5), we can * simply use DROP COLUMN (yay!) */ } + +static void migrate_payments_scids_as_integers(struct lightningd *ld, + struct db *db, + const struct migration_context *mc) +{ + struct db_stmt *stmt; + const char *colnames[] = {"failchannel"}; + + stmt = db_prepare_v2(db, SQL("SELECT id, failchannel FROM payments")); + db_query_prepared(stmt); + while (db_step(stmt)) { + struct db_stmt *update_stmt; + struct short_channel_id scid; + const char *str; + + if (db_col_is_null(stmt, "failchannel")) { + db_col_ignore(stmt, "id"); + continue; + } + + str = db_col_strdup(tmpctx, stmt, "failchannel"); + if (!short_channel_id_from_str(str, strlen(str), &scid)) + db_fatal("Cannot convert invalid payments.failchannel '%s'", + str); + update_stmt = db_prepare_v2(db, SQL("UPDATE payments SET" + " failscid = ?" + " WHERE id = ?")); + db_bind_scid(update_stmt, 0, &scid); + db_bind_u64(update_stmt, 1, db_col_u64(stmt, "id")); + db_exec_prepared_v2(update_stmt); + tal_free(update_stmt); + } + tal_free(stmt); + + if (!db->config->delete_columns(db, "payments", colnames, ARRAY_SIZE(colnames))) + db_fatal("Could not delete payments.failchannel"); +} diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index 090e371ba..5c2a71ed4 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -977,9 +977,9 @@ static struct wallet *create_test_wallet(struct lightningd *ld, const tal_t *ctx w->bip32_base) == WALLY_OK); CHECK_MSG(w->db, "Failed opening the db"); + w->db->data_version = 0; db_begin_transaction(w->db); db_migrate(ld, w->db, bip32_base); - w->db->data_version = 0; db_commit_transaction(w->db); CHECK_MSG(!wallet_err, wallet_err); w->max_channel_dbid = 0; diff --git a/wallet/wallet.c b/wallet/wallet.c index b6bbd5af6..bca98eb4d 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -3403,7 +3403,7 @@ void wallet_payment_get_failinfo(const tal_t *ctx, stmt = db_prepare_v2(wallet->db, SQL("SELECT failonionreply, faildestperm" ", failindex, failcode" - ", failnode, failchannel" + ", failnode, failscid" ", failupdate, faildetail, faildirection" " FROM payments" " WHERE payment_hash=? AND partid=? AND groupid=?;")); @@ -3428,14 +3428,12 @@ void wallet_payment_get_failinfo(const tal_t *ctx, *failnode = tal(ctx, struct node_id); db_col_node_id(stmt, "failnode", *failnode); } - if (db_col_is_null(stmt, "failchannel")) { + if (db_col_is_null(stmt, "failscid")) { db_col_ignore(stmt, "faildirection"); *failchannel = NULL; } else { *failchannel = tal(ctx, struct short_channel_id); - resb = db_col_short_channel_id_str(stmt, "failchannel", - *failchannel); - assert(resb); + db_col_scid(stmt, "failscid", *failchannel); /* For pre-0.6.2 dbs, direction will be 0 */ *faildirection = db_col_int(stmt, "faildirection"); @@ -3474,7 +3472,7 @@ void wallet_payment_set_failinfo(struct wallet *wallet, " , failindex=?" " , failcode=?" " , failnode=?" - " , failchannel=?" + " , failscid=?" " , failupdate=?" " , faildetail=?" " , faildirection=?" @@ -3494,7 +3492,7 @@ void wallet_payment_set_failinfo(struct wallet *wallet, db_bind_null(stmt, 4); if (failchannel) { - db_bind_short_channel_id_str(stmt, 5, failchannel); + db_bind_scid(stmt, 5, failchannel); db_bind_int(stmt, 8, faildirection); } else { db_bind_null(stmt, 5);