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 <rusty@rustcorp.com.au>

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)
 {

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 *

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")

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");
+}

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;

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);