diff --git a/contrib/pyln-testing/pyln/testing/utils.py b/contrib/pyln-testing/pyln/testing/utils.py index b70502b36..f9e8c639f 100644 --- a/contrib/pyln-testing/pyln/testing/utils.py +++ b/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 """ diff --git a/tests/test_db.py b/tests/test_db.py index 58aec2944..de3e2ae3d 100644 --- a/tests/test_db.py +++ b/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'}] diff --git a/tests/test_pay.py b/tests/test_pay.py index 382957eb5..dba8e6eb9 100644 --- a/tests/test_pay.py +++ b/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) diff --git a/tests/utils.py b/tests/utils.py index 5d3cfe515..c1380d02c 100644 --- a/tests/utils.py +++ b/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 diff --git a/wallet/db.c b/wallet/db.c index ee9e94025..0e206b255 100644 --- a/wallet/db.c +++ b/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!) */ +} diff --git a/wallet/wallet.c b/wallet/wallet.c index 6d6269678..b6bbd5af6 100644 --- a/wallet/wallet.c +++ b/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);