psbt: have withdraw_tx use psbt's to create signed txs

this will allow us to add inputs that aren't ours to a tx that we sign
and finalize

hsmd/hsm_wire.csv

@@ -54,15 +54,14 @@ msgtype,hsm_node_announcement_sig_reply,106
 msgdata,hsm_node_announcement_sig_reply,signature,secp256k1_ecdsa_signature,
 
 # Sign a withdrawal request
+#include <bitcoin/psbt.h>
 msgtype,hsm_sign_withdrawal,7
-msgdata,hsm_sign_withdrawal,num_outputs,u16,
-msgdata,hsm_sign_withdrawal,outputs,bitcoin_tx_output,num_outputs
 msgdata,hsm_sign_withdrawal,num_inputs,u16,
 msgdata,hsm_sign_withdrawal,inputs,utxo,num_inputs
-msgdata,hsm_sign_withdrawal,nlocktime,u32,
+msgdata,hsm_sign_withdrawal,psbt,wally_psbt,
 
 msgtype,hsm_sign_withdrawal_reply,107
-msgdata,hsm_sign_withdrawal_reply,tx,bitcoin_tx,
+msgdata,hsm_sign_withdrawal_reply,psbt,wally_psbt,
 
 # Sign an invoice
 msgtype,hsm_sign_invoice,8

hsmd/hsmd.c

@@ -1512,87 +1512,63 @@ static void hsm_key_for_utxo(struct privkey *privkey, struct pubkey *pubkey,
 	}
 }
 
-static void sign_input(struct bitcoin_tx *tx, struct utxo *in,
+/* Find our inputs by the pubkey associated with the inputs, and
-		       struct pubkey *inkey,
+ * add a partial sig for each */
-		       struct bitcoin_signature *sig,
+static void sign_our_inputs(struct utxo **utxos, struct wally_psbt *psbt)
-		       int index)
 {
-	struct privkey inprivkey;
-	u8 *subscript, *wscript, *script;
-
-	/* Figure out keys to spend this. */
-	hsm_key_for_utxo(&inprivkey, inkey, in);
-
-	/* It's either a p2wpkh or p2sh (we support that so people from
-	 * the last bitcoin era can put funds into the wallet) */
-	wscript = p2wpkh_scriptcode(tmpctx, inkey);
-	if (in->is_p2sh) {
-		/* For P2SH-wrapped Segwit, the (implied) redeemScript
-		 * is defined in BIP141 */
-		subscript = bitcoin_redeem_p2sh_p2wpkh(tmpctx, inkey);
-		script = bitcoin_scriptsig_p2sh_p2wpkh(tx, inkey);
-		bitcoin_tx_input_set_script(tx, index, script);
-	} else {
-		/* Pure segwit uses an empty inputScript; NULL has
-		 * tal_count() == 0, so it works great here. */
-		subscript = NULL;
-		bitcoin_tx_input_set_script(tx, index, NULL);
-	}
-	/* This is the core crypto magic. */
-	sign_tx_input(tx, index, subscript, wscript, &inprivkey, inkey,
-		      SIGHASH_ALL, sig);
-
-	/* The witness is [sig] [key] */
-	bitcoin_tx_input_set_witness(
-		tx, index, take(bitcoin_witness_p2wpkh(tx, sig, inkey)));
-}
-
-/* This completes the tx by filling in the input scripts with signatures. */
-static void sign_all_inputs(struct bitcoin_tx *tx, struct utxo **utxos)
-{
-	/*~ Deep in my mind there's a continuous battle: should arrays be
-	 * named as singular or plural?  Is consistency the sign of a weak
-	 * mind?
-	 *
-	 * ZmnSCPxj answers thusly: One must make peace with the fact, that
-	 * the array itself is singular, yet its contents are plural. Do you
-	 * name the array, or do you name its contents? Is the array itself
-	 * the thing and the whole of the thing, or is it its contents that
-	 * define what it is?
-	 *
-	 *... I'm not sure that helps! */
-	assert(tx->wtx->num_inputs == tal_count(utxos));
 	for (size_t i = 0; i < tal_count(utxos); i++) {
-		struct pubkey inkey;
+		struct utxo *utxo = utxos[i];
-		struct bitcoin_signature sig;
+		for (size_t j = 0; j < psbt->num_inputs; j++) {
+			struct privkey privkey;
+			struct pubkey pubkey;
+
+			if (!wally_tx_input_spends(&psbt->tx->inputs[j],
+						   &utxo->txid, utxo->outnum))
+				continue;
+
+			hsm_key_for_utxo(&privkey, &pubkey, utxo);
+
+			/* This line is basically the entire reason we have
+			 * to iterate through to match the psbt input
+			 * to the UTXO -- otherwise we would just
+			 * call wally_sign_psbt for every utxo privkey
+			 * and be done with it. We can't do that though
+			 * because any UTXO that's derived from channel_info
+			 * requires the HSM to find the pubkey, and we
+			 * skip doing that until now as a bit of a reduction
+			 * of complexity in the calling code */
+			psbt_input_add_pubkey(psbt, j, &pubkey);
+
+			if (wally_sign_psbt(psbt, privkey.secret.data,
+					    sizeof(privkey.secret.data)) != WALLY_OK)
+				status_broken("Received wally_err attempting to "
+					      "sign utxo with key %s. PSBT: %s",
+					      type_to_string(tmpctx, struct pubkey,
+							     &pubkey),
+					      type_to_string(tmpctx, struct wally_psbt,
+							     psbt));
 
-		sign_input(tx, utxos[i], &inkey, &sig, i);
+		}
 	}
 }
 
-/*~ lightningd asks us to sign a withdrawal or funding as above but in theory
+/*~ lightningd asks us to sign a withdrawal; same as above but in theory
  * we can do more to check the previous case is valid. */
 static struct io_plan *handle_sign_withdrawal_tx(struct io_conn *conn,
 						 struct client *c,
 						 const u8 *msg_in)
 {
 	struct utxo **utxos;
-	struct bitcoin_tx *tx;
+	struct wally_psbt *psbt;
-	struct bitcoin_tx_output **outputs;
-	u32 nlocktime;
 
 	if (!fromwire_hsm_sign_withdrawal(tmpctx, msg_in,
-					  &outputs, &utxos, &nlocktime))
+					  &utxos, &psbt))
 		return bad_req(conn, c, msg_in);
 
-	tx = withdraw_tx(tmpctx, c->chainparams,
+	sign_our_inputs(utxos, psbt);
-			 cast_const2(const struct utxo **, utxos),
-			 outputs, NULL, nlocktime);
-
-	sign_all_inputs(tx, utxos);
 
 	return req_reply(conn, c,
-			 take(towire_hsm_sign_withdrawal_reply(NULL, tx)));
+			 take(towire_hsm_sign_withdrawal_reply(NULL, psbt)));
 }
 
 /*~ Lightning invoices, defined by BOLT 11, are signed.  This has been

wallet/walletrpc.c

@@ -80,16 +80,13 @@ static void wallet_withdrawal_broadcast(struct bitcoind *bitcoind UNUSED,
 static struct command_result *broadcast_and_wait(struct command *cmd,
 						 struct unreleased_tx *utx)
 {
-	struct bitcoin_tx *signed_tx;
+	struct wally_psbt *signed_psbt;
+	struct wally_tx *signed_wtx;
 	struct bitcoin_txid signed_txid;
 
 	/* FIXME: hsm will sign almost anything, but it should really
 	 * fail cleanly (not abort!) and let us report the error here. */
-	u8 *msg = towire_hsm_sign_withdrawal(cmd,
+	u8 *msg = towire_hsm_sign_withdrawal(cmd, utx->wtx->utxos, utx->tx->psbt);
-					     cast_const2(const struct bitcoin_tx_output **,
-							 utx->outputs),
-					     utx->wtx->utxos,
-					     utx->tx->wtx->locktime);
 
 	if (!wire_sync_write(cmd->ld->hsm_fd, take(msg)))
 		fatal("Could not write sign_withdrawal to HSM: %s",
@@ -97,25 +94,39 @@ static struct command_result *broadcast_and_wait(struct command *cmd,
 
 	msg = wire_sync_read(cmd, cmd->ld->hsm_fd);
 
-	if (!fromwire_hsm_sign_withdrawal_reply(utx, msg, &signed_tx))
+	if (!fromwire_hsm_sign_withdrawal_reply(utx, msg, &signed_psbt))
 		fatal("HSM gave bad sign_withdrawal_reply %s",
 		      tal_hex(tmpctx, msg));
-	signed_tx->chainparams = utx->tx->chainparams;
+
+	signed_wtx = psbt_finalize(signed_psbt, true);
+
+	if (!signed_wtx) {
+		/* Have the utx persist past this command */
+		tal_steal(cmd->ld->wallet, utx);
+		add_unreleased_tx(cmd->ld->wallet, utx);
+		return command_fail(cmd, LIGHTNINGD,
+				    "PSBT is not finalized %s",
+				    type_to_string(tmpctx,
+						   struct wally_psbt,
+						   signed_psbt));
+	}
 
 	/* Sanity check */
-	bitcoin_txid(signed_tx, &signed_txid);
+	wally_txid(signed_wtx, &signed_txid);
 	if (!bitcoin_txid_eq(&signed_txid, &utx->txid))
 		fatal("HSM changed txid: unsigned %s, signed %s",
 		      tal_hex(tmpctx, linearize_tx(tmpctx, utx->tx)),
-		      tal_hex(tmpctx, linearize_tx(tmpctx, signed_tx)));
+		      tal_hex(tmpctx, linearize_wtx(tmpctx, signed_wtx)));
 
 	/* Replace unsigned tx by signed tx. */
-	tal_free(utx->tx);
+	wally_tx_free(utx->tx->wtx);
-	utx->tx = signed_tx;
+	utx->tx->wtx = tal_steal(utx->tx, signed_wtx);
+	tal_free(utx->tx->psbt);
+	utx->tx->psbt = tal_steal(utx->tx, signed_psbt);
 
 	/* Now broadcast the transaction */
 	bitcoind_sendrawtx(cmd->ld->topology->bitcoind,
-			   tal_hex(tmpctx, linearize_tx(tmpctx, signed_tx)),
+			   tal_hex(tmpctx, linearize_tx(tmpctx, utx->tx)),
 			   wallet_withdrawal_broadcast, utx);
 
 	return command_still_pending(cmd);