hsmd/db: backfill pubkey information so that psbts signing works

the way we use PSBTs to sign things requires that we have the
scriptpubkey available on the utxo so we can populate the witness-utxo
field with it.

this causes problems if we don't already have the scriptpubkey cached in
the database, as in *some* cases we require a round trip to the HSM to
populate them

to get over this hump, we backfill any and all missing scriptpubkey
information for the utxo's that we hold in our wallet.

this will allow us to clean up the NULL handling of missing
scriptpubkeys.

Author: niftynei

hsmd/hsm_wire.csv

@@ -185,3 +185,13 @@ msgdata,hsm_sign_message,msg,u8,len
 
 msgtype,hsm_sign_message_reply,123
 msgdata,hsm_sign_message_reply,sig,secp256k1_ecdsa_recoverable_signature,
+
+# lightningd needs to get a scriptPubkey for a utxo with closeinfo
+msgtype,hsm_get_output_scriptpubkey,24
+msgdata,hsm_get_output_scriptpubkey,channel_id,u64,
+msgdata,hsm_get_output_scriptpubkey,peer_id,node_id,
+msgdata,hsm_get_output_scriptpubkey,commitment_point,?pubkey,
+
+msgtype,hsm_get_output_scriptpubkey_reply,124
+msgdata,hsm_get_output_scriptpubkey_reply,script_len,u16,
+msgdata,hsm_get_output_scriptpubkey_reply,script,u8,script_len

hsmd/hsmd.c

@@ -1571,6 +1571,31 @@ static struct io_plan *handle_sign_withdrawal_tx(struct io_conn *conn,
 			 take(towire_hsm_sign_withdrawal_reply(NULL, psbt)));
 }
 
+static struct io_plan *handle_get_output_scriptpubkey(struct io_conn *conn,
+						    struct client *c,
+						    const u8 *msg_in)
+{
+	struct pubkey pubkey;
+	struct privkey privkey;
+	struct unilateral_close_info info;
+	u8 *scriptPubkey;
+
+	info.commitment_point = NULL;
+	if (!fromwire_hsm_get_output_scriptpubkey(tmpctx, msg_in,
+						  &info.channel_id,
+						  &info.peer_id,
+						  &info.commitment_point))
+		return bad_req(conn, c, msg_in);
+
+	hsm_unilateral_close_privkey(&privkey, &info);
+	pubkey_from_privkey(&privkey, &pubkey);
+	scriptPubkey = scriptpubkey_p2wpkh(tmpctx, &pubkey);
+
+	return req_reply(conn, c,
+			 take(towire_hsm_get_output_scriptpubkey_reply(NULL,
+								       scriptPubkey)));
+}
+
 /*~ Lightning invoices, defined by BOLT 11, are signed.  This has been
  * surprisingly controversial; it means a node needs to be online to create
  * invoices.  However, it seems clear to me that in a world without
@@ -1799,6 +1824,7 @@ static bool check_client_capabilities(struct client *client,
 	case WIRE_HSM_GET_CHANNEL_BASEPOINTS:
 	case WIRE_HSM_DEV_MEMLEAK:
 	case WIRE_HSM_SIGN_MESSAGE:
+	case WIRE_HSM_GET_OUTPUT_SCRIPTPUBKEY:
 		return (client->capabilities & HSM_CAP_MASTER) != 0;
 
 	/*~ These are messages sent by the HSM so we should never receive them. */
@@ -1820,6 +1846,7 @@ static bool check_client_capabilities(struct client *client,
 	case WIRE_HSM_GET_CHANNEL_BASEPOINTS_REPLY:
 	case WIRE_HSM_DEV_MEMLEAK_REPLY:
 	case WIRE_HSM_SIGN_MESSAGE_REPLY:
+	case WIRE_HSM_GET_OUTPUT_SCRIPTPUBKEY_REPLY:
 		break;
 	}
 	return false;
@@ -1849,6 +1876,9 @@ static struct io_plan *handle_client(struct io_conn *conn, struct client *c)
 	case WIRE_HSM_GET_CHANNEL_BASEPOINTS:
 		return handle_get_channel_basepoints(conn, c, c->msg_in);
 
+	case WIRE_HSM_GET_OUTPUT_SCRIPTPUBKEY:
+		return handle_get_output_scriptpubkey(conn, c, c->msg_in);
+
 	case WIRE_HSM_ECDH_REQ:
 		return handle_ecdh(conn, c, c->msg_in);
 
@@ -1921,6 +1951,7 @@ static struct io_plan *handle_client(struct io_conn *conn, struct client *c)
 	case WIRE_HSM_GET_CHANNEL_BASEPOINTS_REPLY:
 	case WIRE_HSM_DEV_MEMLEAK_REPLY:
 	case WIRE_HSM_SIGN_MESSAGE_REPLY:
+	case WIRE_HSM_GET_OUTPUT_SCRIPTPUBKEY_REPLY:
 		break;
 	}
 

tests/data/pubkey_regen.sqlite.xz

@@ -180,6 +180,76 @@ def test_last_tx_psbt_upgrade(node_factory, bitcoind):
     bitcoind.rpc.decoderawtransaction(last_txs[1].hex())
 
 
+@unittest.skipIf(os.getenv('TEST_DB_PROVIDER', 'sqlite3') != 'sqlite3', "This test is based on a sqlite3 snapshot")
+@unittest.skipIf(TEST_NETWORK != 'regtest', "The network must match the DB snapshot")
+def test_backfill_scriptpubkeys(node_factory, bitcoind):
+    bitcoind.generate_block(214)
+
+    script_map = [
+        {
+            "txid": "2513F3340D493489811EAB440AC05650B5BC06290358972EB6A55533A9EED96A",
+            "scriptpubkey": "001438C10854C11E10CB3786460143C963C8530DF891",
+        }, {
+            "txid": "E380E18B6E810A464634B3A94B95AAA06B36A8982FD9D9D294982726EDC77DD3",
+            "scriptpubkey": "001407DB91DA65EF06B385F4EA20BA05FAF286165C0B",
+        }, {
+            "txid": "E9AE7C9A346F9B9E35868176F311F3F2EE5DB8B94A065963E26954E119C49A79",
+            "scriptpubkey": "00147E5B5C8F4FC1A9484E259F92CA4CBB7FA2814EA4",
+        }, {
+            "txid": "4C88F50BF00518E4FE3434ACA42351D5AC5FEEE17C35595DFBC3D1F4279F6EC1",
+            "scriptpubkey": "0014D0EAC62FDCEE2D1881259BE9CDA4C43DE9050DB8",
+        }, {
+            "txid": "55265C3CAFE98C355FE0A440DCC005CF5C3145280EAD44D6B903A45D2DF3619C",
+            "scriptpubkey": "0014D0EAC62FDCEE2D1881259BE9CDA4C43DE9050DB8",
+        }, {
+            "txid": "06F6D1D29B175146381EAB59924EC438572D18A3701F8E4FDF4EE17DE78D31E3",
+            "scriptpubkey": "A9149551336F1E360F5AFB977F24CE72C744A82463D187",
+        }, {
+            "txid": "91BCEC7867F3F97F4F575D1D9DEDF5CF22BDDE643B36C2D9E6097048334EE32A",
+            "scriptpubkey": "0014DFA9D65F06088E922A661C29797EE616F793C863",
+        },
+    ]
+
+    # Test the first time, all entries are with option_static_remotekey
+    l1 = node_factory.get_node(node_id=3, dbfile='pubkey_regen.sqlite.xz')
+    results = l1.db_query('SELECT hex(prev_out_tx) AS txid, hex(scriptpubkey) AS script FROM outputs')
+    scripts = [{'txid': x['txid'], 'scriptpubkey': x['script']} for x in results]
+    for exp, actual in zip(script_map, scripts):
+        assert exp == actual
+
+    # Test again, without option_static_remotekey
+    script_map_2 = [
+        {
+            "txid": "FF89677793AC6F39E4AEB9D393B45F1E3D902CBFA26B521C5C438345A6D36E54",
+            "scriptpubkey": "001438C10854C11E10CB3786460143C963C8530DF891",
+        }, {
+            "txid": "0F0685CCEE067638629B1CB27111EB0E15E19B75B1F5D368FC10D216D48FF4A5",
+            "scriptpubkey": "001407DB91DA65EF06B385F4EA20BA05FAF286165C0B",
+        }, {
+            "txid": "822466946527F940A53B823C507A319FDC91CCE55E455D916C9FE13B982058FA",
+            "scriptpubkey": "00144A94D23CD5A438531AADD86A0237FE11B9EA4E09",
+        }, {
+            "txid": "383145E40C8A9F45A0409E080DA5861C9E754B1EC8DD5EFA8A84DEB158E61C88",
+            "scriptpubkey": "0014D0EAC62FDCEE2D1881259BE9CDA4C43DE9050DB8",
+        }, {
+            "txid": "D221BE9B7CDB5FDB58B34D59B30304B7C4C2DF9C3BF73A4AE0E0265642FEC560",
+            "scriptpubkey": "0014D0EAC62FDCEE2D1881259BE9CDA4C43DE9050DB8",
+        }, {
+            "txid": "420F06E91CEE996D8E75E0565D776A96E8959ECA11E799FFE14522C2D43CCFA5",
+            "scriptpubkey": "A9149551336F1E360F5AFB977F24CE72C744A82463D187",
+        }, {
+            "txid": "9F6127316EBED57E7702A4DF19D6FC0EC23A8FAB9BC0D4AD82C29D3F93C525CD",
+            "scriptpubkey": "0014E445493A382C798AF195724DFF67DE4C9250AEC6",
+        }
+    ]
+
+    l2 = node_factory.get_node(node_id=3, dbfile='pubkey_regen_commitment_point.sqlite3.xz')
+    results = l2.db_query('SELECT hex(prev_out_tx) AS txid, hex(scriptpubkey) AS script FROM outputs')
+    scripts = [{'txid': x['txid'], 'scriptpubkey': x['script']} for x in results]
+    for exp, actual in zip(script_map_2, scripts):
+        assert exp == actual
+
+
 @unittest.skipIf(VALGRIND and not DEVELOPER, "Without developer valgrind will complain about debug symbols missing")
 def test_optimistic_locking(node_factory, bitcoind):
     """Have a node run against a DB, then change it under its feet, crashing it.

tests/data/pubkey_regen_commitment_point.sqlite3.xz

@@ -5,16 +5,20 @@
 #include <ccan/array_size/array_size.h>
 #include <ccan/tal/str/str.h>
 #include <common/derive_basepoints.h>
+#include <common/key_derive.h>
 #include <common/node_id.h>
 #include <common/onionreply.h>
 #include <common/version.h>
+#include <hsmd/gen_hsm_wire.h>
 #include <inttypes.h>
 #include <lightningd/channel.h>
 #include <lightningd/lightningd.h>
 #include <lightningd/log.h>
 #include <lightningd/plugin_hook.h>
 #include <wallet/db_common.h>
+#include <wallet/wallet.h>
 #include <wally_bip32.h>
+#include <wire/wire_sync.h>
 
 #define NSEC_IN_SEC 1000000000
 
@@ -33,6 +37,8 @@ static void migrate_our_funding(struct lightningd *ld, struct db *db,
 static void migrate_last_tx_to_psbt(struct lightningd *ld, struct db *db,
 				    const struct ext_key *bip32_base);
 
+static void fillin_missing_scriptpubkeys(struct lightningd *ld, struct db *db,
+					 const struct ext_key *bip32_base);
 
 /* Do not reorder or remove elements from this array, it is used to
  * migrate existing databases from a previous state, based on the
@@ -626,6 +632,7 @@ static struct migration dbmigrations[] = {
     {SQL("INSERT INTO vars (name, intval) VALUES ('coin_moves_count', 0);"), NULL},
     {NULL, migrate_last_tx_to_psbt},
     {SQL("ALTER TABLE outputs ADD reserved_til INTEGER DEFAULT NULL;"), NULL},
+    {NULL, fillin_missing_scriptpubkeys},
 };
 
 /* Leak tracking. */
@@ -1128,6 +1135,87 @@ static void migrate_our_funding(struct lightningd *ld, struct db *db,
 	tal_free(stmt);
 }
 
+void fillin_missing_scriptpubkeys(struct lightningd *ld, struct db *db,
+				  const struct ext_key *bip32_base)
+{
+	struct db_stmt *stmt;
+
+	stmt = db_prepare_v2(db, SQL("SELECT"
+				     " type"
+				     ", keyindex"
+				     ", prev_out_tx"
+				     ", prev_out_index"
+				     ", channel_id"
+				     ", peer_id"
+				     ", commitment_point"
+				     " FROM outputs"
+				     " WHERE scriptpubkey IS NULL;"));
+
+	db_query_prepared(stmt);
+	while (db_step(stmt)) {
+		int type;
+		u8 *scriptPubkey;
+		struct bitcoin_txid txid;
+		u32 outnum, keyindex;
+		struct pubkey key;
+		struct db_stmt *update_stmt;
+
+		type = db_column_int(stmt, 0);
+		keyindex = db_column_int(stmt, 1);
+		db_column_txid(stmt, 2, &txid);
+		outnum = db_column_int(stmt, 3);
+
+		/* This indiciates whether or not we have 'close_info' */
+		if (!db_column_is_null(stmt, 4)) {
+			struct pubkey *commitment_point;
+			struct node_id peer_id;
+			u64 channel_id;
+			u8 *msg;
+
+			channel_id = db_column_u64(stmt, 4);
+			db_column_node_id(stmt, 5, &peer_id);
+			if (!db_column_is_null(stmt, 6)) {
+				commitment_point = tal(stmt, struct pubkey);
+				db_column_pubkey(stmt, 6, commitment_point);
+			} else
+				commitment_point = NULL;
+
+			/* Have to go ask the HSM to derive the pubkey for us */
+			msg = towire_hsm_get_output_scriptpubkey(NULL,
+								 channel_id,
+								 &peer_id,
+								 commitment_point);
+			if (!wire_sync_write(ld->hsm_fd, take(msg)))
+				fatal("Could not write to HSM: %s", strerror(errno));
+			msg = wire_sync_read(stmt, ld->hsm_fd);
+			if (!fromwire_hsm_get_output_scriptpubkey_reply(stmt, msg,
+									&scriptPubkey))
+				fatal("HSM gave bad hsm_get_output_scriptpubkey_reply %s",
+				      tal_hex(msg, msg));
+		} else {
+			/* Build from bip32_base */
+			bip32_pubkey(bip32_base, &key, keyindex);
+			if (type == p2sh_wpkh) {
+				u8 *redeemscript = bitcoin_redeem_p2sh_p2wpkh(stmt, &key);
+				scriptPubkey = scriptpubkey_p2sh(tmpctx, redeemscript);
+			} else
+				scriptPubkey = scriptpubkey_p2wpkh(stmt, &key);
+		}
+
+		update_stmt = db_prepare_v2(db, SQL("UPDATE outputs"
+						    " SET scriptpubkey = ?"
+						    " WHERE prev_out_tx = ? "
+						    "   AND prev_out_index = ?"));
+		db_bind_blob(update_stmt, 0, scriptPubkey, tal_bytelen(scriptPubkey));
+		db_bind_txid(update_stmt, 1, &txid);
+		db_bind_int(update_stmt, 2, outnum);
+		db_exec_prepared_v2(update_stmt);
+		tal_free(update_stmt);
+	}
+
+	tal_free(stmt);
+}
+
 /* We're moving everything over to PSBTs from tx's, particularly our last_tx's
  * which are commitment transactions for channels.
  * This migration loads all of the last_tx's and 're-formats' them into psbts,

tests/test_db.py

@@ -21,6 +21,9 @@ static void db_log_(struct log *log UNUSED, enum log_level level UNUSED, const s
 /* Generated stub for fatal */
 void   fatal(const char *fmt UNNEEDED, ...)
 { fprintf(stderr, "fatal called!\n"); abort(); }
+/* Generated stub for fromwire_hsm_get_output_scriptpubkey_reply */
+bool fromwire_hsm_get_output_scriptpubkey_reply(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, u8 **script UNNEEDED)
+{ fprintf(stderr, "fromwire_hsm_get_output_scriptpubkey_reply called!\n"); abort(); }
 /* Generated stub for get_channel_basepoints */
 void get_channel_basepoints(struct lightningd *ld UNNEEDED,
 			    const struct node_id *peer_id UNNEEDED,
@@ -33,6 +36,15 @@ struct log *new_log(const tal_t *ctx UNNEEDED, struct log_book *record UNNEEDED,
 		    const struct node_id *default_node_id UNNEEDED,
 		    const char *fmt UNNEEDED, ...)
 { fprintf(stderr, "new_log called!\n"); abort(); }
+/* Generated stub for towire_hsm_get_output_scriptpubkey */
+u8 *towire_hsm_get_output_scriptpubkey(const tal_t *ctx UNNEEDED, u64 channel_id UNNEEDED, const struct node_id *peer_id UNNEEDED, const struct pubkey *commitment_point UNNEEDED)
+{ fprintf(stderr, "towire_hsm_get_output_scriptpubkey called!\n"); abort(); }
+/* Generated stub for wire_sync_read */
+u8 *wire_sync_read(const tal_t *ctx UNNEEDED, int fd UNNEEDED)
+{ fprintf(stderr, "wire_sync_read called!\n"); abort(); }
+/* Generated stub for wire_sync_write */
+bool wire_sync_write(int fd UNNEEDED, const void *msg TAKES UNNEEDED)
+{ fprintf(stderr, "wire_sync_write called!\n"); abort(); }
 /* AUTOGENERATED MOCKS END */
 
 static char *db_err;

wallet/db.c

@@ -135,6 +135,9 @@ bool fromwire_custommsg_in(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, u8
 /* Generated stub for fromwire_gossip_get_stripped_cupdate_reply */
 bool fromwire_gossip_get_stripped_cupdate_reply(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, u8 **stripped_update UNNEEDED)
 { fprintf(stderr, "fromwire_gossip_get_stripped_cupdate_reply called!\n"); abort(); }
+/* Generated stub for fromwire_hsm_get_output_scriptpubkey_reply */
+bool fromwire_hsm_get_output_scriptpubkey_reply(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, u8 **script UNNEEDED)
+{ fprintf(stderr, "fromwire_hsm_get_output_scriptpubkey_reply called!\n"); abort(); }
 /* Generated stub for fromwire_hsm_sign_commitment_tx_reply */
 bool fromwire_hsm_sign_commitment_tx_reply(const void *p UNNEEDED, struct bitcoin_signature *sig UNNEEDED)
 { fprintf(stderr, "fromwire_hsm_sign_commitment_tx_reply called!\n"); abort(); }
@@ -692,6 +695,9 @@ u8 *towire_final_incorrect_htlc_amount(const tal_t *ctx UNNEEDED, struct amount_
 /* Generated stub for towire_gossip_get_stripped_cupdate */
 u8 *towire_gossip_get_stripped_cupdate(const tal_t *ctx UNNEEDED, const struct short_channel_id *channel_id UNNEEDED)
 { fprintf(stderr, "towire_gossip_get_stripped_cupdate called!\n"); abort(); }
+/* Generated stub for towire_hsm_get_output_scriptpubkey */
+u8 *towire_hsm_get_output_scriptpubkey(const tal_t *ctx UNNEEDED, u64 channel_id UNNEEDED, const struct node_id *peer_id UNNEEDED, const struct pubkey *commitment_point UNNEEDED)
+{ fprintf(stderr, "towire_hsm_get_output_scriptpubkey called!\n"); abort(); }
 /* Generated stub for towire_hsm_sign_commitment_tx */
 u8 *towire_hsm_sign_commitment_tx(const tal_t *ctx UNNEEDED, const struct node_id *peer_id UNNEEDED, u64 channel_dbid UNNEEDED, const struct bitcoin_tx *tx UNNEEDED, const struct pubkey *remote_funding_key UNNEEDED)
 { fprintf(stderr, "towire_hsm_sign_commitment_tx called!\n"); abort(); }