From 30d3053cff3d2db3e55740ef33d5d328d6b90ffb Mon Sep 17 00:00:00 2001 From: niftynei Date: Fri, 29 May 2020 12:12:29 -0500 Subject: [PATCH 01/13] psbt: add type-to-string that prints b64 string Re-uses code from what was the bitcoin_tx_to_psbt_b64 --- bitcoin/psbt.c | 24 ++++++++++++++++++++++++ bitcoin/psbt.h | 2 ++ bitcoin/tx.c | 10 +--------- common/type_to_string.h | 2 ++ 4 files changed, 29 insertions(+), 9 deletions(-) diff --git a/bitcoin/psbt.c b/bitcoin/psbt.c index 56706fb54dc5..d14686916b90 100644 --- a/bitcoin/psbt.c +++ b/bitcoin/psbt.c @@ -5,7 +5,9 @@ #include #include #include +#include #include +#include #include #include #include @@ -310,6 +312,28 @@ struct amount_sat psbt_input_get_amount(struct wally_psbt *psbt, return val; } +bool psbt_from_b64(const char *b64str, struct wally_psbt **psbt) +{ + int wally_err; + wally_err = wally_psbt_from_base64(b64str, psbt); + return wally_err == WALLY_OK; +} + +char *psbt_to_b64(const tal_t *ctx, const struct wally_psbt *psbt) +{ + char *serialized_psbt, *ret_val; + int ret; + + ret = wally_psbt_to_base64(cast_const(struct wally_psbt *, psbt), + &serialized_psbt); + assert(ret == WALLY_OK); + + ret_val = tal_strdup(ctx, serialized_psbt); + wally_free_string(serialized_psbt); + return ret_val; +} +REGISTER_TYPE_TO_STRING(wally_psbt, psbt_to_b64); + const u8 *psbt_get_bytes(const tal_t *ctx, const struct wally_psbt *psbt, size_t *bytes_written) { diff --git a/bitcoin/psbt.h b/bitcoin/psbt.h index 218125c2453d..29b160672d28 100644 --- a/bitcoin/psbt.h +++ b/bitcoin/psbt.h @@ -60,6 +60,8 @@ void psbt_input_set_prev_utxo_wscript(struct wally_psbt *psbt, size_t in, struct amount_sat psbt_input_get_amount(struct wally_psbt *psbt, size_t in); +bool psbt_from_b64(const char *b64str, struct wally_psbt **psbt); +char *psbt_to_b64(const tal_t *ctx, const struct wally_psbt *psbt); const u8 *psbt_get_bytes(const tal_t *ctx, const struct wally_psbt *psbt, size_t *bytes_written); struct wally_psbt *psbt_from_bytes(const tal_t *ctx, const u8 *bytes, diff --git a/bitcoin/tx.c b/bitcoin/tx.c index 2dc66c6c7756..10f3f3a28788 100644 --- a/bitcoin/tx.c +++ b/bitcoin/tx.c @@ -486,15 +486,7 @@ void bitcoin_tx_finalize(struct bitcoin_tx *tx) char *bitcoin_tx_to_psbt_base64(const tal_t *ctx, struct bitcoin_tx *tx) { - char *serialized_psbt, *ret_val; - int ret; - - ret = wally_psbt_to_base64(tx->psbt, &serialized_psbt); - assert(ret == WALLY_OK); - - ret_val = tal_strdup(ctx, serialized_psbt); - wally_free_string(serialized_psbt); - return ret_val; + return psbt_to_b64(ctx, tx->psbt); } struct bitcoin_tx *bitcoin_tx_with_psbt(const tal_t *ctx, struct wally_psbt *psbt STEALS) diff --git a/common/type_to_string.h b/common/type_to_string.h index 209c26e6a5b1..ce134b946df2 100644 --- a/common/type_to_string.h +++ b/common/type_to_string.h @@ -4,6 +4,7 @@ #include "utils.h" #include #include +#include /* This must match the type_to_string_ cases. */ union printable_types { @@ -35,6 +36,7 @@ union printable_types { const struct amount_sat *amount_sat; const struct fee_states *fee_states; const char *charp_; + const struct wally_psbt *wally_psbt; }; #define type_to_string(ctx, type, ptr) \ From 3d02c6072c0e27ef2e306b1d0f08ce821db178a6 Mon Sep 17 00:00:00 2001 From: niftynei Date: Fri, 29 May 2020 12:13:47 -0500 Subject: [PATCH 02/13] psbt: return NULL instead of aborting on wally-lib problems This lets us parse invalid/bad psbt data from user input without crashing --- bitcoin/tx.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/bitcoin/tx.c b/bitcoin/tx.c index 10f3f3a28788..a9f67f6fead7 100644 --- a/bitcoin/tx.c +++ b/bitcoin/tx.c @@ -502,17 +502,22 @@ struct bitcoin_tx *bitcoin_tx_with_psbt(const tal_t *ctx, struct wally_psbt *psb * data, not the global tx. But 'finalizing' a tx destroys some fields * so we 'clone' it first and then finalize it */ if (wally_psbt_clone(psbt, &tmppsbt) != WALLY_OK) - abort(); + return NULL; - if (wally_finalize_psbt(tmppsbt) != WALLY_OK) - abort(); + if (wally_finalize_psbt(tmppsbt) != WALLY_OK) { + wally_psbt_free(tmppsbt); + return NULL; + } if (psbt_is_finalized(tmppsbt)) { - if (wally_extract_psbt(tmppsbt, &tx->wtx) != WALLY_OK) - abort(); - } else if (wally_tx_clone(psbt->tx, &tx->wtx) != WALLY_OK) - abort(); - + if (wally_extract_psbt(tmppsbt, &tx->wtx) != WALLY_OK) { + wally_psbt_free(tmppsbt); + return NULL; + } + } else if (wally_tx_clone(psbt->tx, &tx->wtx) != WALLY_OK) { + wally_psbt_free(tmppsbt); + return NULL; + } wally_psbt_free(tmppsbt); From 9542e9a52578000cc449f6b2c22fade3d3fd8ad0 Mon Sep 17 00:00:00 2001 From: lisa neigut Date: Mon, 16 Sep 2019 19:08:05 -0500 Subject: [PATCH 03/13] withdraw: refactor change output handling We're not using the change_outnum for withdraw tx's (and the way we were calculating it was broken as of the addition of 'multiple outputs'). This removes the change output knowhow from withdraw_tx entirely, and pushes the responsibility up to the caller to include the change output in the output set if desired. Consequently, we also remove the change output knowhow from hsmd. --- bitcoin/tx.c | 10 ++++++++++ bitcoin/tx.h | 4 ++++ common/withdraw_tx.c | 31 ++++--------------------------- common/withdraw_tx.h | 7 +------ hsmd/hsm_wire.csv | 3 --- hsmd/hsmd.c | 14 +++----------- wallet/wallet.h | 2 -- wallet/walletrpc.c | 35 +++++++++++++++++------------------ 8 files changed, 39 insertions(+), 67 deletions(-) diff --git a/bitcoin/tx.c b/bitcoin/tx.c index a9f67f6fead7..d7e8394179bf 100644 --- a/bitcoin/tx.c +++ b/bitcoin/tx.c @@ -32,6 +32,16 @@ int wally_tx_clone(struct wally_tx *tx, struct wally_tx **output) return ret; } +struct bitcoin_tx_output *new_tx_output(const tal_t *ctx, + struct amount_sat amount, + const u8 *script) +{ + struct bitcoin_tx_output *output = tal(ctx, struct bitcoin_tx_output); + output->amount = amount; + output->script = tal_dup_arr(output, u8, script, tal_count(script), 0); + return output; +} + int bitcoin_tx_add_output(struct bitcoin_tx *tx, const u8 *script, u8 *wscript, struct amount_sat amount) { diff --git a/bitcoin/tx.h b/bitcoin/tx.h index 31f81b02070f..4c7209081d26 100644 --- a/bitcoin/tx.h +++ b/bitcoin/tx.h @@ -35,6 +35,10 @@ struct bitcoin_tx_output { u8 *script; }; +struct bitcoin_tx_output *new_tx_output(const tal_t *ctx, + struct amount_sat amount, + const u8 *script); + /* SHA256^2 the tx in legacy format. */ void bitcoin_txid(const struct bitcoin_tx *tx, struct bitcoin_txid *txid); void wally_txid(const struct wally_tx *wtx, struct bitcoin_txid *txid); diff --git a/common/withdraw_tx.c b/common/withdraw_tx.c index 46fdddceeafb..b52aac517c84 100644 --- a/common/withdraw_tx.c +++ b/common/withdraw_tx.c @@ -3,6 +3,7 @@ #include #include #include +#include #include #include #include @@ -13,44 +14,20 @@ struct bitcoin_tx *withdraw_tx(const tal_t *ctx, const struct chainparams *chainparams, const struct utxo **utxos, struct bitcoin_tx_output **outputs, - const struct pubkey *changekey, - struct amount_sat change, const struct ext_key *bip32_base, - int *change_outnum, u32 nlocktime) + u32 nlocktime) { struct bitcoin_tx *tx; int output_count; tx = tx_spending_utxos(ctx, chainparams, utxos, bip32_base, - !amount_sat_eq(change, AMOUNT_SAT(0)), - tal_count(outputs), nlocktime, + false, tal_count(outputs), nlocktime, BITCOIN_TX_DEFAULT_SEQUENCE - 1); output_count = bitcoin_tx_add_multi_outputs(tx, outputs); assert(output_count == tal_count(outputs)); - if (!amount_sat_eq(change, AMOUNT_SAT(0))) { - /* Add one to the output_count, for the change */ - output_count++; - - const void *map[output_count]; - for (size_t i = 0; i < output_count; i++) - map[i] = int2ptr(i); - - bitcoin_tx_add_output(tx, scriptpubkey_p2wpkh(tmpctx, changekey), - NULL, change); - - assert(tx->wtx->num_outputs == output_count); - permute_outputs(tx, NULL, map); - - /* The change is the last output added, so the last position - * in the map */ - if (change_outnum) - *change_outnum = ptr2int(map[output_count - 1]); - - } else if (change_outnum) - *change_outnum = -1; - + permute_outputs(tx, NULL, (const void **)outputs); permute_inputs(tx, (const void **)utxos); bitcoin_tx_finalize(tx); diff --git a/common/withdraw_tx.h b/common/withdraw_tx.h index 163f1b4757ac..cf5d67af18c6 100644 --- a/common/withdraw_tx.h +++ b/common/withdraw_tx.h @@ -21,19 +21,14 @@ struct utxo; * @chainparams: (in) the params for the created transaction. * @utxos: (in/out) tal_arr of UTXO pointers to spend (permuted to match) * @outputs: (in) tal_arr of bitcoin_tx_output, scriptPubKeys with amount to send to. - * @changekey: (in) key to send change to (only used if change_satoshis != 0). - * @change: (in) amount to send as change. * @bip32_base: (in) bip32 base for key derivation, or NULL. - * @change_outnum: (out) set to output index of change output or -1 if none, unless NULL. * @nlocktime: (in) the value to set as the transaction's nLockTime. */ struct bitcoin_tx *withdraw_tx(const tal_t *ctx, const struct chainparams *chainparams, const struct utxo **utxos, struct bitcoin_tx_output **outputs, - const struct pubkey *changekey, - struct amount_sat change, const struct ext_key *bip32_base, - int *change_outnum, u32 nlocktime); + u32 nlocktime); #endif /* LIGHTNING_COMMON_WITHDRAW_TX_H */ diff --git a/hsmd/hsm_wire.csv b/hsmd/hsm_wire.csv index 32b78ac3d1a7..5f2a6056b2a1 100644 --- a/hsmd/hsm_wire.csv +++ b/hsmd/hsm_wire.csv @@ -55,9 +55,6 @@ msgdata,hsm_node_announcement_sig_reply,signature,secp256k1_ecdsa_signature, # Sign a withdrawal request msgtype,hsm_sign_withdrawal,7 -msgdata,hsm_sign_withdrawal,satoshi_out,amount_sat, -msgdata,hsm_sign_withdrawal,change_out,amount_sat, -msgdata,hsm_sign_withdrawal,change_keyindex,u32, msgdata,hsm_sign_withdrawal,num_outputs,u16, msgdata,hsm_sign_withdrawal,outputs,bitcoin_tx_output,num_outputs msgdata,hsm_sign_withdrawal,num_inputs,u16, diff --git a/hsmd/hsmd.c b/hsmd/hsmd.c index 264a6dd6afef..856a619f4502 100644 --- a/hsmd/hsmd.c +++ b/hsmd/hsmd.c @@ -1576,26 +1576,18 @@ static struct io_plan *handle_sign_withdrawal_tx(struct io_conn *conn, struct client *c, const u8 *msg_in) { - struct amount_sat satoshi_out, change_out; - u32 change_keyindex; struct utxo **utxos; struct bitcoin_tx *tx; - struct pubkey changekey; struct bitcoin_tx_output **outputs; u32 nlocktime; - if (!fromwire_hsm_sign_withdrawal(tmpctx, msg_in, &satoshi_out, - &change_out, &change_keyindex, + if (!fromwire_hsm_sign_withdrawal(tmpctx, msg_in, &outputs, &utxos, &nlocktime)) return bad_req(conn, c, msg_in); - if (!bip32_pubkey(&secretstuff.bip32, &changekey, change_keyindex)) - return bad_req_fmt(conn, c, msg_in, - "Failed to get key %u", change_keyindex); - tx = withdraw_tx(tmpctx, c->chainparams, - cast_const2(const struct utxo **, utxos), outputs, - &changekey, change_out, NULL, NULL, nlocktime); + cast_const2(const struct utxo **, utxos), + outputs, NULL, nlocktime); sign_all_inputs(tx, utxos); diff --git a/wallet/wallet.h b/wallet/wallet.h index 88dda612c813..0f0d132e0552 100644 --- a/wallet/wallet.h +++ b/wallet/wallet.h @@ -67,8 +67,6 @@ struct unreleased_tx { /* The tx itself (unsigned initially) */ struct bitcoin_tx *tx; struct bitcoin_txid txid; - /* Index of change output, or -1 if none. */ - int change_outnum; }; /* Possible states for tracked outputs in the database. Not sure yet diff --git a/wallet/walletrpc.c b/wallet/walletrpc.c index ca3650b77fa5..068c3d44baec 100644 --- a/wallet/walletrpc.c +++ b/wallet/walletrpc.c @@ -86,9 +86,6 @@ static struct command_result *broadcast_and_wait(struct command *cmd, /* 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, - utx->wtx->amount, - utx->wtx->change, - utx->wtx->change_key_index, cast_const2(const struct bitcoin_tx_output **, utx->outputs), utx->wtx->utxos, @@ -312,10 +309,8 @@ static struct command_result *json_prepare_tx(struct command *cmd, * Support only one output. */ if (destination) { outputs = tal_arr(tmpctx, struct bitcoin_tx_output *, 1); - outputs[0] = tal(outputs, struct bitcoin_tx_output); - outputs[0]->script = tal_steal(outputs[0], - cast_const(u8 *, destination)); - outputs[0]->amount = (*utx)->wtx->amount; + outputs[0] = new_tx_output(outputs, (*utx)->wtx->amount, + destination); out_len = tal_count(outputs[0]->script); goto create_tx; @@ -357,11 +352,9 @@ static struct command_result *json_prepare_tx(struct command *cmd, "'%.*s' is a invalid satoshi amount", t[2].end - t[2].start, buffer + t[2].start); + outputs[i] = new_tx_output(outputs, *amount, + cast_const(u8 *, destination)); out_len += tal_count(destination); - outputs[i] = tal(outputs, struct bitcoin_tx_output); - outputs[i]->amount = *amount; - outputs[i]->script = tal_steal(outputs[i], - cast_const(u8 *, destination)); /* In fact, the maximum amount of bitcoin satoshi is 2.1e15. * It can't be equal to/bigger than 2^64. @@ -387,8 +380,6 @@ static struct command_result *json_prepare_tx(struct command *cmd, } create_tx: - (*utx)->outputs = tal_steal(*utx, outputs); - if (chosen_utxos) result = wtx_from_utxos((*utx)->wtx, *feerate_per_kw, out_len, maxheight, @@ -405,19 +396,27 @@ static struct command_result *json_prepare_tx(struct command *cmd, if ((*utx)->wtx->all_funds) outputs[0]->amount = (*utx)->wtx->amount; + /* Add the change as the last output */ if (!amount_sat_eq((*utx)->wtx->change, AMOUNT_SAT(0))) { + struct bitcoin_tx_output *change_output; + changekey = tal(tmpctx, struct pubkey); if (!bip32_pubkey(cmd->ld->wallet->bip32_base, changekey, (*utx)->wtx->change_key_index)) return command_fail(cmd, LIGHTNINGD, "Keys generation failure"); - } else - changekey = NULL; + + change_output = new_tx_output(outputs, (*utx)->wtx->change, + scriptpubkey_p2wpkh(tmpctx, changekey)); + tal_arr_expand(&outputs, change_output); + } + + (*utx)->outputs = tal_steal(*utx, outputs); (*utx)->tx = withdraw_tx(*utx, chainparams, - (*utx)->wtx->utxos, (*utx)->outputs, - changekey, (*utx)->wtx->change, + (*utx)->wtx->utxos, + (*utx)->outputs, cmd->ld->wallet->bip32_base, - &(*utx)->change_outnum, locktime); + bitcoin_txid((*utx)->tx, &(*utx)->txid); return NULL; From c4dbab467500c301b8f075034308bdfdbe0ba688 Mon Sep 17 00:00:00 2001 From: niftynei Date: Sat, 6 Jun 2020 14:35:46 -0500 Subject: [PATCH 04/13] psbt: remove script sig info from inputs before adding them to global PSBT's dont' serialize / unserialize if there's any sig info set on the global transaction --- bitcoin/psbt.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/bitcoin/psbt.c b/bitcoin/psbt.c index d14686916b90..54517e2ba069 100644 --- a/bitcoin/psbt.c +++ b/bitcoin/psbt.c @@ -116,10 +116,32 @@ struct wally_psbt_input *psbt_add_input(struct wally_psbt *psbt, { struct wally_tx *tx; struct wally_tx_input tmp_in; + u8 *script; + size_t scriptlen = 0; + struct wally_tx_witness_stack *witness = NULL; tx = psbt->tx; assert(insert_at <= tx->num_inputs); + + /* Remove any script sig or witness info before adding it ! */ + if (input->script_len > 0) { + scriptlen = input->script_len; + input->script_len = 0; + script = (u8 *)input->script; + input->script = NULL; + } + if (input->witness) { + witness = input->witness; + input->witness = NULL; + } wally_tx_add_input(tx, input); + /* Put the script + witness info back */ + if (scriptlen > 0) { + input->script_len = scriptlen; + input->script = script; + } + if (witness) + input->witness = witness; tmp_in = tx->inputs[tx->num_inputs - 1]; MAKE_ROOM(tx->inputs, insert_at, tx->num_inputs); From 56604ac36d9f7c65a97a2bee1a637609de32ef91 Mon Sep 17 00:00:00 2001 From: niftynei Date: Sat, 6 Jun 2020 14:38:59 -0500 Subject: [PATCH 05/13] psbt: don't crash if we can't add a partial sig instead return a boolean indicating the success/failure of a sig set --- bitcoin/psbt.c | 19 +++++++------------ bitcoin/psbt.h | 6 +++--- channeld/channeld.c | 7 +++++-- openingd/openingd.c | 9 +++++---- wallet/db.c | 5 +++-- 5 files changed, 23 insertions(+), 23 deletions(-) diff --git a/bitcoin/psbt.c b/bitcoin/psbt.c index 54517e2ba069..c825e17c632d 100644 --- a/bitcoin/psbt.c +++ b/bitcoin/psbt.c @@ -243,29 +243,24 @@ void psbt_input_add_pubkey(struct wally_psbt *psbt, size_t in, assert(wally_err == WALLY_OK); } -void psbt_input_set_partial_sig(struct wally_psbt *psbt, size_t in, +bool psbt_input_set_partial_sig(struct wally_psbt *psbt, size_t in, const struct pubkey *pubkey, const struct bitcoin_signature *sig) { - int wally_err; u8 pk_der[PUBKEY_CMPR_LEN]; assert(in < psbt->num_inputs); if (!psbt->inputs[in].partial_sigs) if (wally_partial_sigs_map_init_alloc(1, &psbt->inputs[in].partial_sigs) != WALLY_OK) - abort(); + return false; /* we serialize the compressed version of the key, wally likes this */ pubkey_to_der(pk_der, pubkey); - wally_err = wally_add_new_partial_sig(psbt->inputs[in].partial_sigs, - pk_der, sizeof(pk_der), - cast_const(unsigned char *, sig->s.data), - sizeof(sig->s.data)); - assert(wally_err == WALLY_OK); - - wally_err = wally_psbt_input_set_sighash_type(&psbt->inputs[in], - sig->sighash_type); - assert(wally_err == WALLY_OK); + wally_psbt_input_set_sighash_type(&psbt->inputs[in], sig->sighash_type); + return wally_add_new_partial_sig(psbt->inputs[in].partial_sigs, + pk_der, sizeof(pk_der), + cast_const(unsigned char *, sig->s.data), + sizeof(sig->s.data)) == WALLY_OK; } void psbt_input_set_prev_utxo(struct wally_psbt *psbt, size_t in, diff --git a/bitcoin/psbt.h b/bitcoin/psbt.h index 29b160672d28..d43681da5aff 100644 --- a/bitcoin/psbt.h +++ b/bitcoin/psbt.h @@ -49,9 +49,9 @@ void psbt_rm_output(struct wally_psbt *psbt, void psbt_input_add_pubkey(struct wally_psbt *psbt, size_t in, const struct pubkey *pubkey); -void psbt_input_set_partial_sig(struct wally_psbt *psbt, size_t in, - const struct pubkey *pubkey, - const struct bitcoin_signature *sig); +WARN_UNUSED_RESULT bool psbt_input_set_partial_sig(struct wally_psbt *psbt, size_t in, + const struct pubkey *pubkey, + const struct bitcoin_signature *sig); void psbt_input_set_prev_utxo(struct wally_psbt *psbt, size_t in, const u8 *wscript, struct amount_sat amt); diff --git a/channeld/channeld.c b/channeld/channeld.c index 99aba4523e86..14ab5f2680b3 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -1291,8 +1291,11 @@ static void handle_peer_commit_sig(struct peer *peer, const u8 *msg) peer->next_index[LOCAL], LOCAL); /* Set the commit_sig on the commitment tx psbt */ - psbt_input_set_partial_sig(txs[0]->psbt, 0, - &peer->channel->funding_pubkey[REMOTE], &commit_sig); + if (!psbt_input_set_partial_sig(txs[0]->psbt, 0, + &peer->channel->funding_pubkey[REMOTE], + &commit_sig)) + status_failed(STATUS_FAIL_INTERNAL_ERROR, + "Unable to set signature internally"); if (!derive_simple_key(&peer->channel->basepoints[REMOTE].htlc, &peer->next_local_per_commit, &remote_htlckey)) diff --git a/openingd/openingd.c b/openingd/openingd.c index b72be7cdaebb..63a458b56f03 100644 --- a/openingd/openingd.c +++ b/openingd/openingd.c @@ -846,10 +846,11 @@ static bool funder_finalize_channel_setup(struct state *state, } /* We save their sig to our first commitment tx */ - psbt_input_set_partial_sig((*tx)->psbt, 0, - &state->their_funding_pubkey, - sig); - + if (!psbt_input_set_partial_sig((*tx)->psbt, 0, + &state->their_funding_pubkey, + sig)) + status_failed(STATUS_FAIL_INTERNAL_ERROR, + "Unable to set signature internally"); peer_billboard(false, "Funding channel: opening negotiation succeeded"); diff --git a/wallet/db.c b/wallet/db.c index 2a93167da80d..c97d2e08b28d 100644 --- a/wallet/db.c +++ b/wallet/db.c @@ -1173,8 +1173,9 @@ void migrate_last_tx_to_psbt(struct lightningd *ld, struct db *db) abort(); last_sig.sighash_type = SIGHASH_ALL; - psbt_input_set_partial_sig(last_tx->psbt, 0, - &remote_funding_pubkey, &last_sig); + if (!psbt_input_set_partial_sig(last_tx->psbt, 0, + &remote_funding_pubkey, &last_sig)) + abort(); psbt_input_add_pubkey(last_tx->psbt, 0, &local_funding_pubkey); psbt_input_add_pubkey(last_tx->psbt, 0, From c7d96ad537fb9f70e586d01c88dbb28c6cbffa7c Mon Sep 17 00:00:00 2001 From: niftynei Date: Sat, 6 Jun 2020 14:44:00 -0500 Subject: [PATCH 06/13] psbt: add helper method for setting a psbt input's redeemscript --- bitcoin/psbt.c | 11 +++++++++++ bitcoin/psbt.h | 2 ++ 2 files changed, 13 insertions(+) diff --git a/bitcoin/psbt.c b/bitcoin/psbt.c index c825e17c632d..383f51f4fcc8 100644 --- a/bitcoin/psbt.c +++ b/bitcoin/psbt.c @@ -312,6 +312,17 @@ void psbt_input_set_prev_utxo_wscript(struct wally_psbt *psbt, size_t in, psbt_input_set_prev_utxo(psbt, in, scriptPubkey, amt); } +bool psbt_input_set_redeemscript(struct wally_psbt *psbt, size_t in, + const u8 *redeemscript) +{ + int wally_err; + assert(psbt->num_inputs > in); + wally_err = wally_psbt_input_set_redeem_script(&psbt->inputs[in], + cast_const(u8 *, redeemscript), + tal_bytelen(redeemscript)); + return wally_err == WALLY_OK; +} + struct amount_sat psbt_input_get_amount(struct wally_psbt *psbt, size_t in) { diff --git a/bitcoin/psbt.h b/bitcoin/psbt.h index d43681da5aff..8a483d682faa 100644 --- a/bitcoin/psbt.h +++ b/bitcoin/psbt.h @@ -57,6 +57,8 @@ void psbt_input_set_prev_utxo(struct wally_psbt *psbt, size_t in, const u8 *wscript, struct amount_sat amt); void psbt_input_set_prev_utxo_wscript(struct wally_psbt *psbt, size_t in, const u8 *wscript, struct amount_sat amt); +bool psbt_input_set_redeemscript(struct wally_psbt *psbt, size_t in, + const u8 *redeemscript); struct amount_sat psbt_input_get_amount(struct wally_psbt *psbt, size_t in); From ef05605f988a5fb77e9e4c05c2b03380987debde Mon Sep 17 00:00:00 2001 From: niftynei Date: Sat, 6 Jun 2020 14:47:40 -0500 Subject: [PATCH 07/13] psbt: add method to finalize + extract a psbt will either use a temporary psbt (and not munge the passed in psbt) or will finalize in place -- finalization erases most of the signature metadata from the psbt struct --- bitcoin/psbt.c | 32 ++++++++++++++++++++++++++++++++ bitcoin/psbt.h | 2 ++ bitcoin/tx.c | 26 +++----------------------- 3 files changed, 37 insertions(+), 23 deletions(-) diff --git a/bitcoin/psbt.c b/bitcoin/psbt.c index 383f51f4fcc8..e17237ae5c6c 100644 --- a/bitcoin/psbt.c +++ b/bitcoin/psbt.c @@ -340,6 +340,38 @@ struct amount_sat psbt_input_get_amount(struct wally_psbt *psbt, return val; } +struct wally_tx *psbt_finalize(struct wally_psbt *psbt, bool finalize_in_place) +{ + struct wally_psbt *tmppsbt; + struct wally_tx *wtx; + + /* We want the 'finalized' tx since that includes any signature + * data, not the global tx. But 'finalizing' a tx destroys some fields + * so we 'clone' it first and then finalize it */ + if (!finalize_in_place) { + if (wally_psbt_clone(psbt, &tmppsbt) != WALLY_OK) + return NULL; + } else + tmppsbt = cast_const(struct wally_psbt *, psbt); + + if (wally_finalize_psbt(tmppsbt) != WALLY_OK) { + if (!finalize_in_place) + wally_psbt_free(tmppsbt); + return NULL; + } + + if (psbt_is_finalized(tmppsbt) + && wally_extract_psbt(tmppsbt, &wtx) == WALLY_OK) { + if (!finalize_in_place) + wally_psbt_free(tmppsbt); + return wtx; + } + + if (!finalize_in_place) + wally_psbt_free(tmppsbt); + return NULL; +} + bool psbt_from_b64(const char *b64str, struct wally_psbt **psbt) { int wally_err; diff --git a/bitcoin/psbt.h b/bitcoin/psbt.h index 8a483d682faa..68d718f9688c 100644 --- a/bitcoin/psbt.h +++ b/bitcoin/psbt.h @@ -32,6 +32,8 @@ struct wally_psbt *new_psbt(const tal_t *ctx, */ bool psbt_is_finalized(struct wally_psbt *psbt); +struct wally_tx *psbt_finalize(struct wally_psbt *psbt, bool finalize_in_place); + struct wally_psbt_input *psbt_add_input(struct wally_psbt *psbt, struct wally_tx_input *input, size_t insert_at); diff --git a/bitcoin/tx.c b/bitcoin/tx.c index d7e8394179bf..50f224a41397 100644 --- a/bitcoin/tx.c +++ b/bitcoin/tx.c @@ -501,38 +501,18 @@ char *bitcoin_tx_to_psbt_base64(const tal_t *ctx, struct bitcoin_tx *tx) struct bitcoin_tx *bitcoin_tx_with_psbt(const tal_t *ctx, struct wally_psbt *psbt STEALS) { - struct wally_psbt *tmppsbt; struct bitcoin_tx *tx = bitcoin_tx(ctx, chainparams, psbt->tx->num_inputs, psbt->tx->num_outputs, psbt->tx->locktime); wally_tx_free(tx->wtx); - - /* We want the 'finalized' tx since that includes any signature - * data, not the global tx. But 'finalizing' a tx destroys some fields - * so we 'clone' it first and then finalize it */ - if (wally_psbt_clone(psbt, &tmppsbt) != WALLY_OK) - return NULL; - - if (wally_finalize_psbt(tmppsbt) != WALLY_OK) { - wally_psbt_free(tmppsbt); - return NULL; - } - - if (psbt_is_finalized(tmppsbt)) { - if (wally_extract_psbt(tmppsbt, &tx->wtx) != WALLY_OK) { - wally_psbt_free(tmppsbt); - return NULL; - } - } else if (wally_tx_clone(psbt->tx, &tx->wtx) != WALLY_OK) { - wally_psbt_free(tmppsbt); + tx->wtx = psbt_finalize(psbt, false); + if (!tx->wtx && wally_tx_clone(psbt->tx, &tx->wtx) != WALLY_OK) return NULL; - } - - wally_psbt_free(tmppsbt); tal_free(tx->psbt); tx->psbt = tal_steal(tx, psbt); + return tx; } From 013bd7594607e4513c9e2ef97bb0c6955075f0db Mon Sep 17 00:00:00 2001 From: niftynei Date: Sat, 6 Jun 2020 14:50:23 -0500 Subject: [PATCH 08/13] psbt: have wally_tx serialization methods be legible for gen'd code our code generators expect the serialization name to match the struct type --- bitcoin/psbt.c | 6 +++--- bitcoin/psbt.h | 6 +++--- bitcoin/tx.c | 4 ++-- tools/generate-wire.py | 1 + 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/bitcoin/psbt.c b/bitcoin/psbt.c index e17237ae5c6c..a7cabe47f7bf 100644 --- a/bitcoin/psbt.c +++ b/bitcoin/psbt.c @@ -424,7 +424,7 @@ struct wally_psbt *psbt_from_bytes(const tal_t *ctx, const u8 *bytes, return psbt; } -void towire_psbt(u8 **pptr, const struct wally_psbt *psbt) +void towire_wally_psbt(u8 **pptr, const struct wally_psbt *psbt) { /* Let's include the PSBT bytes */ size_t bytes_written; @@ -434,8 +434,8 @@ void towire_psbt(u8 **pptr, const struct wally_psbt *psbt) tal_free(pbt_bytes); } -struct wally_psbt *fromwire_psbt(const tal_t *ctx, - const u8 **cursor, size_t *max) +struct wally_psbt *fromwire_wally_psbt(const tal_t *ctx, + const u8 **cursor, size_t *max) { struct wally_psbt *psbt; u32 psbt_byte_len; diff --git a/bitcoin/psbt.h b/bitcoin/psbt.h index 68d718f9688c..1282e41a21c8 100644 --- a/bitcoin/psbt.h +++ b/bitcoin/psbt.h @@ -70,7 +70,7 @@ const u8 *psbt_get_bytes(const tal_t *ctx, const struct wally_psbt *psbt, size_t *bytes_written); struct wally_psbt *psbt_from_bytes(const tal_t *ctx, const u8 *bytes, size_t byte_len); -void towire_psbt(u8 **pptr, const struct wally_psbt *psbt); -struct wally_psbt *fromwire_psbt(const tal_t *ctx, - const u8 **curosr, size_t *max); +void towire_wally_psbt(u8 **pptr, const struct wally_psbt *psbt); +struct wally_psbt *fromwire_wally_psbt(const tal_t *ctx, + const u8 **cursor, size_t *max); #endif /* LIGHTNING_BITCOIN_PSBT_H */ diff --git a/bitcoin/tx.c b/bitcoin/tx.c index 50f224a41397..b2f558413424 100644 --- a/bitcoin/tx.c +++ b/bitcoin/tx.c @@ -652,7 +652,7 @@ struct bitcoin_tx *fromwire_bitcoin_tx(const tal_t *ctx, /* pull_bitcoin_tx sets the psbt */ tal_free(tx->psbt); - tx->psbt = fromwire_psbt(tx, cursor, max); + tx->psbt = fromwire_wally_psbt(tx, cursor, max); return tx; } @@ -667,7 +667,7 @@ void towire_bitcoin_tx(u8 **pptr, const struct bitcoin_tx *tx) u8 *lin = linearize_tx(tmpctx, tx); towire_u8_array(pptr, lin, tal_count(lin)); - towire_psbt(pptr, tx->psbt); + towire_wally_psbt(pptr, tx->psbt); } struct bitcoin_tx_output *fromwire_bitcoin_tx_output(const tal_t *ctx, diff --git a/tools/generate-wire.py b/tools/generate-wire.py index f07a1800a8d0..9714c90dc751 100755 --- a/tools/generate-wire.py +++ b/tools/generate-wire.py @@ -235,6 +235,7 @@ class Type(FieldSet): 'onionmsg_path', 'route_hop', 'tx_parts', + 'wally_psbt', ] # Some BOLT types are re-typed based on their field name From 7886a260c0c86adf7002b77d96df653e2ba07b56 Mon Sep 17 00:00:00 2001 From: niftynei Date: Sat, 6 Jun 2020 14:55:54 -0500 Subject: [PATCH 09/13] psbt-json: remove reliance on bitcoin_tx, use straight wally_psbt struct --- bitcoin/tx.c | 5 ----- bitcoin/tx.h | 5 ----- common/json_helpers.c | 5 +++-- common/json_helpers.h | 2 +- wallet/walletrpc.c | 2 +- 5 files changed, 5 insertions(+), 14 deletions(-) diff --git a/bitcoin/tx.c b/bitcoin/tx.c index b2f558413424..ca0a159734df 100644 --- a/bitcoin/tx.c +++ b/bitcoin/tx.c @@ -494,11 +494,6 @@ void bitcoin_tx_finalize(struct bitcoin_tx *tx) assert(bitcoin_tx_check(tx)); } -char *bitcoin_tx_to_psbt_base64(const tal_t *ctx, struct bitcoin_tx *tx) -{ - return psbt_to_b64(ctx, tx->psbt); -} - struct bitcoin_tx *bitcoin_tx_with_psbt(const tal_t *ctx, struct wally_psbt *psbt STEALS) { struct bitcoin_tx *tx = bitcoin_tx(ctx, chainparams, diff --git a/bitcoin/tx.h b/bitcoin/tx.h index 4c7209081d26..982f6ff22a92 100644 --- a/bitcoin/tx.h +++ b/bitcoin/tx.h @@ -220,10 +220,5 @@ void towire_bitcoin_txid(u8 **pptr, const struct bitcoin_txid *txid); void towire_bitcoin_tx(u8 **pptr, const struct bitcoin_tx *tx); void towire_bitcoin_tx_output(u8 **pptr, const struct bitcoin_tx_output *output); -/* - * Get the base64 string encoded PSBT of a bitcoin transaction. - */ -char *bitcoin_tx_to_psbt_base64(const tal_t *ctx, struct bitcoin_tx *tx); - int wally_tx_clone(struct wally_tx *tx, struct wally_tx **output); #endif /* LIGHTNING_BITCOIN_TX_H */ diff --git a/common/json_helpers.c b/common/json_helpers.c index 42841b90ee8b..f82e4b8a0aaf 100644 --- a/common/json_helpers.c +++ b/common/json_helpers.c @@ -1,6 +1,7 @@ #include #include #include +#include #include #include #include @@ -242,10 +243,10 @@ void json_add_tx(struct json_stream *result, void json_add_psbt(struct json_stream *stream, const char *fieldname, - struct bitcoin_tx *tx) + struct wally_psbt *psbt) { const char *psbt_b64; - psbt_b64 = bitcoin_tx_to_psbt_base64(tx, tx); + psbt_b64 = psbt_to_b64(NULL, psbt); json_add_string(stream, fieldname, take(psbt_b64)); } diff --git a/common/json_helpers.h b/common/json_helpers.h index 230f1a5ba616..248344f209cd 100644 --- a/common/json_helpers.h +++ b/common/json_helpers.h @@ -141,6 +141,6 @@ void json_add_tx(struct json_stream *result, /* '"fieldname" : "cHNidP8BAJoCAAAAAljo..." or "cHNidP8BAJoCAAAAAljo..." if fieldname is NULL */ void json_add_psbt(struct json_stream *stream, const char *fieldname, - struct bitcoin_tx *tx); + struct wally_psbt *psbt); #endif /* LIGHTNING_COMMON_JSON_HELPERS_H */ diff --git a/wallet/walletrpc.c b/wallet/walletrpc.c index 068c3d44baec..9d7e4dcbbe24 100644 --- a/wallet/walletrpc.c +++ b/wallet/walletrpc.c @@ -442,7 +442,7 @@ static struct command_result *json_txprepare(struct command *cmd, response = json_stream_success(cmd); json_add_tx(response, "unsigned_tx", utx->tx); json_add_txid(response, "txid", &utx->txid); - json_add_psbt(response, "psbt", utx->tx); + json_add_psbt(response, "psbt", utx->tx->psbt); return command_success(cmd, response); } static const struct json_command txprepare_command = { From 54b5185100d75d680e426487aa870c782ac56e6b Mon Sep 17 00:00:00 2001 From: niftynei Date: Sat, 6 Jun 2020 15:02:22 -0500 Subject: [PATCH 10/13] psbt: add redeemscript info to psbt for utxos that have it --- common/utxo.c | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/common/utxo.c b/common/utxo.c index d2da66702bfb..f5a932e77e60 100644 --- a/common/utxo.c +++ b/common/utxo.c @@ -1,5 +1,8 @@ #include +#include +#include #include +#include #include #include #include @@ -71,7 +74,7 @@ struct bitcoin_tx *tx_spending_utxos(const tal_t *ctx, u32 nsequence) { struct pubkey key; - u8 *script; + u8 *scriptSig, *scriptPubkey, *redeemscript; assert(num_output); size_t outcount = add_change_output ? 1 + num_output : num_output; @@ -81,14 +84,31 @@ struct bitcoin_tx *tx_spending_utxos(const tal_t *ctx, for (size_t i = 0; i < tal_count(utxos); i++) { if (utxos[i]->is_p2sh && bip32_base) { bip32_pubkey(bip32_base, &key, utxos[i]->keyindex); - script = bitcoin_scriptsig_p2sh_p2wpkh(tmpctx, &key); + scriptSig = bitcoin_scriptsig_p2sh_p2wpkh(tmpctx, &key); + redeemscript = bitcoin_redeem_p2sh_p2wpkh(tmpctx, &key); + scriptPubkey = scriptpubkey_p2sh(tmpctx, redeemscript); + + /* Make sure we've got the right info! */ + if (utxos[i]->scriptPubkey) + assert(memeq(utxos[i]->scriptPubkey, + tal_bytelen(utxos[i]->scriptPubkey), + scriptPubkey, tal_bytelen(scriptPubkey))); } else { - script = NULL; + scriptSig = NULL; + redeemscript = NULL; + /* We can't definitively derive the pubkey without + * hitting the HSM, so we don't */ + scriptPubkey = utxos[i]->scriptPubkey; } bitcoin_tx_add_input(tx, &utxos[i]->txid, utxos[i]->outnum, - nsequence, script, utxos[i]->amount, - utxos[i]->scriptPubkey, NULL); + nsequence, scriptSig, utxos[i]->amount, + scriptPubkey, NULL); + + /* Add redeemscript to the PSBT input */ + if (redeemscript) + psbt_input_set_redeemscript(tx->psbt, i, redeemscript); + } return tx; From 584bad503d80a233f36ebf7f555f3d9cf720b221 Mon Sep 17 00:00:00 2001 From: niftynei Date: Sat, 6 Jun 2020 15:05:14 -0500 Subject: [PATCH 11/13] 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 | 7 ++-- hsmd/hsmd.c | 102 +++++++++++++++++---------------------------- wallet/walletrpc.c | 37 ++++++++++------ 3 files changed, 66 insertions(+), 80 deletions(-) diff --git a/hsmd/hsm_wire.csv b/hsmd/hsm_wire.csv index 5f2a6056b2a1..194d9a926d34 100644 --- a/hsmd/hsm_wire.csv +++ b/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 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 diff --git a/hsmd/hsmd.c b/hsmd/hsmd.c index 856a619f4502..b4e04705aae6 100644 --- a/hsmd/hsmd.c +++ b/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, - struct pubkey *inkey, - struct bitcoin_signature *sig, - int index) +/* Find our inputs by the pubkey associated with the inputs, and + * add a partial sig for each */ +static void sign_our_inputs(struct utxo **utxos, struct wally_psbt *psbt) { - 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 bitcoin_signature sig; + struct utxo *utxo = utxos[i]; + 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 bitcoin_tx_output **outputs; - u32 nlocktime; + struct wally_psbt *psbt; 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, - cast_const2(const struct utxo **, utxos), - outputs, NULL, nlocktime); - - sign_all_inputs(tx, utxos); + sign_our_inputs(utxos, psbt); 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 diff --git a/wallet/walletrpc.c b/wallet/walletrpc.c index 9d7e4dcbbe24..0df56f861a20 100644 --- a/wallet/walletrpc.c +++ b/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, - cast_const2(const struct bitcoin_tx_output **, - utx->outputs), - utx->wtx->utxos, - utx->tx->wtx->locktime); + u8 *msg = towire_hsm_sign_withdrawal(cmd, utx->wtx->utxos, utx->tx->psbt); 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); - utx->tx = signed_tx; + wally_tx_free(utx->tx->wtx); + 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); From 5a4e5d92faf302a00656a3ab7862e1baddec5d96 Mon Sep 17 00:00:00 2001 From: niftynei Date: Mon, 22 Jun 2020 13:28:25 -0500 Subject: [PATCH 12/13] libwally: update to latest commit on master, which contains psbt fixes fixes for p2sh-p2wpkh sigs in libwally etc --- external/libwally-core | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/external/libwally-core b/external/libwally-core index 5642664b6159..e0d0634aea71 160000 --- a/external/libwally-core +++ b/external/libwally-core @@ -1 +1 @@ -Subproject commit 5642664b6159b0fe3940a1276f23998dcfce5481 +Subproject commit e0d0634aea716d813744326ea6c7590eb9fc381c From 37811fd35a30c01cc31cabb0d76612e1c0b12246 Mon Sep 17 00:00:00 2001 From: niftynei Date: Mon, 22 Jun 2020 13:29:10 -0500 Subject: [PATCH 13/13] plugins-test: use pyln.client, not lightning to import Plugin --- tests/plugins/onionmessage-reply.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/plugins/onionmessage-reply.py b/tests/plugins/onionmessage-reply.py index 66ad0611c84a..2caa8b3b98be 100755 --- a/tests/plugins/onionmessage-reply.py +++ b/tests/plugins/onionmessage-reply.py @@ -2,7 +2,7 @@ """ This plugin is used to test the `onion_message` hook. """ -from lightning import Plugin +from pyln.client import Plugin plugin = Plugin()