db: add scid field to channels table.

Normally, we'd use the delete_columns function to remove the old
`short_channel_id` string field, *but* we can't do that for sqlite, as
there are other tables with references to it.  So add a FIXME to do
it once everyone has upgraded to an sqlite3 which has native support
for column deletion.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

contrib/pyln-testing/pyln/testing/utils.py

@@ -107,6 +107,22 @@ def write_config(filename, opts, regtest_opts=None, section_name='regtest'):
                 f.write("{}={}\n".format(k, v))
 
 
+def scid_to_int(scid):
+    """Convert a 1x2x3 scid to a raw integer"""
+    blocknum, txnum, outnum = scid.split("x")
+
+    # BOLT #7:
+    # ## Definition of `short_channel_id`
+    #
+    # The `short_channel_id` is the unique description of the funding transaction.
+    # It is constructed as follows:
+    # 1. the most significant 3 bytes: indicating the block height
+    # 2. the next 3 bytes: indicating the transaction index within the block
+    # 3. the least significant 2 bytes: indicating the output index that pays to the
+    #    channel.
+    return (int(blocknum) << 40) | (int(txnum) << 16) | int(outnum)
+
+
 def only_one(arr):
     """Many JSON RPC calls return an array; often we only expect a single entry
     """

tests/test_db.py

@@ -2,7 +2,7 @@ from decimal import Decimal
 from fixtures import *  # noqa: F401,F403
 from fixtures import TEST_NETWORK
 from pyln.client import RpcError
-from utils import wait_for, sync_blockheight, COMPAT, VALGRIND, DEVELOPER, TIMEOUT, only_one
+from utils import wait_for, sync_blockheight, COMPAT, VALGRIND, DEVELOPER, TIMEOUT, only_one, scid_to_int
 
 import base64
 import os
@@ -161,7 +161,7 @@ def test_scid_upgrade(node_factory, bitcoind):
 
     l1.daemon.opts['database-upgrade'] = True
     l1.daemon.start()
-    assert l1.db_query('SELECT short_channel_id from channels;') == [{'short_channel_id': '103x1x1'}]
+    assert l1.db_query('SELECT scid FROM channels;') == [{'scid': scid_to_int('103x1x1')}]
     assert l1.db_query('SELECT failchannel from payments;') == [{'failchannel': '103x1x1'}]
 
 

tests/test_pay.py

@@ -3,7 +3,7 @@ from fixtures import TEST_NETWORK
 from io import BytesIO
 from pyln.client import RpcError, Millisatoshi
 from pyln.proto.onion import TlvPayload
-from pyln.testing.utils import EXPERIMENTAL_DUAL_FUND, FUNDAMOUNT
+from pyln.testing.utils import EXPERIMENTAL_DUAL_FUND, FUNDAMOUNT, scid_to_int
 from utils import (
     DEVELOPER, wait_for, only_one, sync_blockheight, TIMEOUT,
     EXPERIMENTAL_FEATURES, VALGRIND, mine_funding_to_announce, first_scid
@@ -1957,7 +1957,7 @@ def test_setchannel_usage(node_factory, bitcoind):
     def channel_get_config(scid):
         return l1.db.query(
             'SELECT feerate_base, feerate_ppm, htlc_minimum_msat, htlc_maximum_msat FROM channels '
-            'WHERE short_channel_id=\'{}\';'.format(scid))
+            'WHERE scid={};'.format(scid_to_int(scid)))
 
     # get short channel id
     scid = l1.get_channel_scid(l2)

tests/utils.py

@@ -1,5 +1,5 @@
 from pyln.testing.utils import TEST_NETWORK, TIMEOUT, VALGRIND, DEVELOPER, DEPRECATED_APIS  # noqa: F401
-from pyln.testing.utils import env, only_one, wait_for, write_config, TailableProc, sync_blockheight, wait_channel_quiescent, get_tx_p2wsh_outnum, mine_funding_to_announce  # noqa: F401
+from pyln.testing.utils import env, only_one, wait_for, write_config, TailableProc, sync_blockheight, wait_channel_quiescent, get_tx_p2wsh_outnum, mine_funding_to_announce, scid_to_int  # noqa: F401
 import bitstring
 from pyln.client import Millisatoshi
 from pyln.testing.utils import EXPERIMENTAL_DUAL_FUND

wallet/db.c

@@ -59,6 +59,10 @@ static void fillin_missing_channel_blockheights(struct lightningd *ld,
 						struct db *db,
 						const struct migration_context *mc);
 
+static void migrate_channels_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 */
@@ -920,6 +924,8 @@ static struct migration dbmigrations[] = {
     {SQL("DROP INDEX forwarded_payments_state;"), NULL},
     {SQL("DROP INDEX forwarded_payments_out_htlc_id;"), NULL},
     {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},
 };
 
 /* Released versions are of form v{num}[.{num}]* */
@@ -1455,3 +1461,46 @@ void migrate_last_tx_to_psbt(struct lightningd *ld, struct db *db,
 
 	tal_free(stmt);
 }
+
+/* We used to store scids as strings... */
+static void migrate_channels_scids_as_integers(struct lightningd *ld,
+					       struct db *db,
+					       const struct migration_context *mc)
+{
+	struct db_stmt *stmt;
+	char **scids = tal_arr(tmpctx, char *, 0);
+
+	stmt = db_prepare_v2(db, SQL("SELECT short_channel_id FROM channels"));
+	db_query_prepared(stmt);
+	while (db_step(stmt)) {
+		if (db_col_is_null(stmt, "short_channel_id"))
+			continue;
+		tal_arr_expand(&scids,
+			       db_col_strdup(scids, stmt, "short_channel_id"));
+	}
+	tal_free(stmt);
+
+	for (size_t i = 0; i < tal_count(scids); i++) {
+		struct short_channel_id scid;
+		if (!short_channel_id_from_str(scids[i], strlen(scids[i]), &scid))
+			db_fatal("Cannot convert invalid channels.short_channel_id '%s'",
+				 scids[i]);
+
+		stmt = db_prepare_v2(db, SQL("UPDATE channels"
+					     " SET scid = ?"
+					     " WHERE short_channel_id = ?"));
+		db_bind_scid(stmt, 0, &scid);
+		db_bind_text(stmt, 1, scids[i]);
+		db_exec_prepared_v2(stmt);
+		if (db_count_changes(stmt) != 1)
+			db_fatal("Converting channels.short_channel_id '%s' gave %zu changes != 1?",
+				 scids[i], db_count_changes(stmt));
+		tal_free(stmt);
+	}
+
+	/* FIXME: We cannot use ->delete_columns to remove
+	 * short_channel_id, as other tables reference the channels
+	 * (and sqlite3 has them referencing a now-deleted table!).
+	 * When we can assume sqlite3 2021-04-19 (3.35.5), we can
+	 * simply use DROP COLUMN (yay!) */
+}

wallet/wallet.c

@@ -1293,10 +1293,9 @@ static struct channel *wallet_stmt2channel(struct wallet *w, struct db_stmt *stm
 		}
 	}
 
-	if (!db_col_is_null(stmt, "short_channel_id")) {
+	if (!db_col_is_null(stmt, "scid")) {
 		scid = tal(tmpctx, struct short_channel_id);
-		if (!db_col_short_channel_id_str(stmt, "short_channel_id", scid))
-			return NULL;
+		db_col_scid(stmt, "scid", scid);
 	} else {
 		scid = NULL;
 	}
@@ -1552,7 +1551,7 @@ static bool wallet_channels_load_active(struct wallet *w)
 	stmt = db_prepare_v2(w->db, SQL("SELECT"
 					"  id"
 					", peer_id"
-					", short_channel_id"
+					", scid"
 					", full_channel_id"
 					", channel_config_local"
 					", channel_config_remote"
@@ -1856,7 +1855,7 @@ void wallet_channel_save(struct wallet *w, struct channel *chan)
 
 	stmt = db_prepare_v2(w->db, SQL("UPDATE channels SET"
 					"  shachain_remote_id=?," // 0
-					"  short_channel_id=?," // 1
+					"  scid=?," // 1
 					"  full_channel_id=?," // 2
 					"  state=?," // 3
 					"  funder=?," // 4
@@ -1903,7 +1902,7 @@ void wallet_channel_save(struct wallet *w, struct channel *chan)
 					" WHERE id=?")); // 46
 	db_bind_u64(stmt, 0, chan->their_shachain.id);
 	if (chan->scid)
-		db_bind_short_channel_id_str(stmt, 1, chan->scid);
+		db_bind_scid(stmt, 1, chan->scid);
 	else
 		db_bind_null(stmt, 1);
 
@@ -4678,11 +4677,11 @@ struct wallet_transaction *wallet_transactions_get(struct wallet *w, const tal_t
 		", t.blockheight"
 		", t.txindex"
 		", t.type as txtype"
-		", c2.short_channel_id as txchan"
+		", c2.scid as txchan"
 		", a.location"
 		", a.idx as ann_idx"
 		", a.type as annotation_type"
-		", c.short_channel_id"
+		", c.scid"
 		" FROM"
 		"  transactions t LEFT JOIN"
 		"  transaction_annotations a ON (a.txid = t.id) LEFT JOIN"
@@ -4725,8 +4724,7 @@ struct wallet_transaction *wallet_transactions_get(struct wallet *w, const tal_t
 			else
 				cur->annotation.type = 0;
 			if (!db_col_is_null(stmt, "txchan"))
-				db_col_short_channel_id_str(stmt, "txchan",
-							    &cur->annotation.channel);
+				db_col_scid(stmt, "txchan", &cur->annotation.channel);
 			else
 				cur->annotation.channel.u64 = 0;
 
@@ -4756,16 +4754,14 @@ struct wallet_transaction *wallet_transactions_get(struct wallet *w, const tal_t
 
 			/* cppcheck-suppress uninitvar - false positive on fatal() above */
 			ann->type = db_col_int(stmt, "annotation_type");
-			if (!db_col_is_null(stmt, "c.short_channel_id"))
-				db_col_short_channel_id_str(stmt,
-							    "c.short_channel_id",
-							    &ann->channel);
+			if (!db_col_is_null(stmt, "c.scid"))
+				db_col_scid(stmt, "c.scid", &ann->channel);
 			else
 				ann->channel.u64 = 0;
 		} else {
 			db_col_ignore(stmt, "ann_idx");
 			db_col_ignore(stmt, "annotation_type");
-			db_col_ignore(stmt, "c.short_channel_id");
+			db_col_ignore(stmt, "c.scid");
 		}
 	}
 	tal_free(stmt);