diff --git a/channeld/watchtower.c b/channeld/watchtower.c index 285cdc460..9c2126d8f 100644 --- a/channeld/watchtower.c +++ b/channeld/watchtower.c @@ -79,8 +79,11 @@ penalty_tx_create(const tal_t *ctx, if (final_index) { size_t script_len = tal_bytelen(final_scriptpubkey); bool is_tr = is_p2tr(final_scriptpubkey, script_len, NULL); - psbt_add_keypath_to_last_output(tx, *final_index, - final_ext_key, is_tr); + if (!psbt_add_keypath_to_last_output(tx, *final_index, + final_ext_key, is_tr)) + status_failed(STATUS_FAIL_INTERNAL_ERROR, + "Cannot add final keypath?"); + } /* Worst-case sig is 73 bytes */ diff --git a/common/close_tx.c b/common/close_tx.c index ee549131e..8d6805b79 100644 --- a/common/close_tx.c +++ b/common/close_tx.c @@ -54,9 +54,11 @@ struct bitcoin_tx *create_close_tx(const tal_t *ctx, assert((local_wallet_index == NULL) == (local_wallet_ext_key == NULL)); if (local_wallet_index) { size_t script_len = tal_bytelen(script); - psbt_add_keypath_to_last_output( - tx, *local_wallet_index, local_wallet_ext_key, - is_p2tr(script, script_len, NULL)); + /* Should not happen! */ + if (!psbt_add_keypath_to_last_output( + tx, *local_wallet_index, local_wallet_ext_key, + is_p2tr(script, script_len, NULL))) + return tal_free(tx); } num_outputs++; } diff --git a/common/psbt_keypath.c b/common/psbt_keypath.c index dcd89a9ea..c137effbb 100644 --- a/common/psbt_keypath.c +++ b/common/psbt_keypath.c @@ -3,7 +3,11 @@ #include #include -void psbt_output_set_keypath(u32 index, const struct ext_key *ext, bool is_taproot, struct wally_psbt_output *output) { +bool psbt_output_set_keypath(u32 index, + const struct ext_key *ext, + bool is_taproot, + struct wally_psbt_output *output) +{ u8 fingerprint[BIP32_KEY_FINGERPRINT_LEN]; if (bip32_key_get_fingerprint( (struct ext_key *) ext, fingerprint, sizeof(fingerprint)) != WALLY_OK) @@ -18,24 +22,29 @@ void psbt_output_set_keypath(u32 index, const struct ext_key *ext, bool is_tapro NULL, 0, fingerprint, sizeof(fingerprint), path, 1) != WALLY_OK) - abort(); + return false; } else { if (wally_psbt_output_keypath_add(output, ext->pub_key, sizeof(ext->pub_key), fingerprint, sizeof(fingerprint), path, 1) != WALLY_OK) - abort(); + return false; } + return true; } -void psbt_add_keypath_to_last_output(struct bitcoin_tx *tx, +bool psbt_add_keypath_to_last_output(struct bitcoin_tx *tx, u32 key_index, - const struct ext_key *ext, - bool is_taproot) { + const struct ext_key *ext, + bool is_taproot) +{ size_t outndx = tx->psbt->num_outputs - 1; + bool ok; tal_wally_start(); - psbt_output_set_keypath(key_index, ext, is_taproot, &tx->psbt->outputs[outndx]); + ok = psbt_output_set_keypath(key_index, ext, is_taproot, &tx->psbt->outputs[outndx]); tal_wally_end(tx->psbt); + + return ok; } diff --git a/common/psbt_keypath.h b/common/psbt_keypath.h index ddafa0366..5af5a9493 100644 --- a/common/psbt_keypath.h +++ b/common/psbt_keypath.h @@ -2,6 +2,7 @@ #define LIGHTNING_COMMON_PSBT_KEYPATH_H #include "config.h" +#include #include #include @@ -15,11 +16,14 @@ struct wally_map; * @ext - extended public key of the immediate parent of the wallet key * @is_taproot - PSBT output has taproot script * @output - PSBT output to set + * + * This can fail, if it's adding the same thing twice (taproot only) */ -void psbt_output_set_keypath(u32 index, - const struct ext_key *ext, - bool is_taproot, - struct wally_psbt_output *output); +WARN_UNUSED_RESULT +bool psbt_output_set_keypath(u32 index, + const struct ext_key *ext, + bool is_taproot, + struct wally_psbt_output *output); /* psbt_add_keypath_to_last_output - augment the last output with the * given wallet keypath @@ -29,9 +33,10 @@ void psbt_output_set_keypath(u32 index, * @ext - extended public key of the immediate parent of the wallet key * @is_taproot - if the output is taproot */ -void psbt_add_keypath_to_last_output(struct bitcoin_tx *tx, +WARN_UNUSED_RESULT +bool psbt_add_keypath_to_last_output(struct bitcoin_tx *tx, u32 index, const struct ext_key *ext, - bool is_taproot); + bool is_taproot); #endif /* LIGHTNING_COMMON_PSBT_KEYPATH_H */ diff --git a/lightningd/onchain_control.c b/lightningd/onchain_control.c index 192450e39..f62a673a4 100644 --- a/lightningd/onchain_control.c +++ b/lightningd/onchain_control.c @@ -880,6 +880,7 @@ static struct bitcoin_tx *onchaind_tx_unsigned(const tal_t *ctx, struct ext_key final_wallet_ext_key; u64 block_target; struct lightningd *ld = channel->peer->ld; + bool keypath_ok; bip32_pubkey(ld, &final_key, channel->final_key_idx); if (bip32_key_from_parent(ld->bip32_base, @@ -899,13 +900,16 @@ static struct bitcoin_tx *onchaind_tx_unsigned(const tal_t *ctx, if (chainparams->is_elements) { bitcoin_tx_add_output( tx, scriptpubkey_p2wpkh(tmpctx, &final_key), NULL, info->out_sats); - psbt_add_keypath_to_last_output(tx, channel->final_key_idx, &final_wallet_ext_key, false /* is_taproot */); + keypath_ok = psbt_add_keypath_to_last_output(tx, channel->final_key_idx, &final_wallet_ext_key, false /* is_taproot */); } else { bitcoin_tx_add_output( tx, scriptpubkey_p2tr(tmpctx, &final_key), NULL, info->out_sats); - psbt_add_keypath_to_last_output(tx, channel->final_key_idx, &final_wallet_ext_key, true /* is_taproot */); + keypath_ok = psbt_add_keypath_to_last_output(tx, channel->final_key_idx, &final_wallet_ext_key, true /* is_taproot */); + } + if (!keypath_ok) { + channel_internal_error(channel, "Could not add keypath?"); + return tal_free(tx); } - /* Worst-case sig is 73 bytes */ weight = bitcoin_tx_weight(tx) + 1 + 3 + 73 + 0 + tal_count(info->wscript); weight += elements_tx_overhead(chainparams, 1, 1); diff --git a/tests/test_wallet.py b/tests/test_wallet.py index c34c8aedb..0dfa66cd1 100644 --- a/tests/test_wallet.py +++ b/tests/test_wallet.py @@ -948,7 +948,6 @@ def test_sign_external_psbt(node_factory, bitcoind, chainparams): l1.rpc.signpsbt(psbt) -@pytest.mark.xfail(strict=True) def test_sign_signed_psbt(node_factory, bitcoind, chainparams): l1 = node_factory.get_node() l1.fundwallet(10**6) diff --git a/wallet/walletrpc.c b/wallet/walletrpc.c index e3ecb56a6..6bb111eb2 100644 --- a/wallet/walletrpc.c +++ b/wallet/walletrpc.c @@ -738,9 +738,11 @@ static struct command_result *match_psbt_inputs_to_utxos(struct command *cmd, return NULL; } -static void match_psbt_outputs_to_wallet(struct wally_psbt *psbt, +static bool match_psbt_outputs_to_wallet(struct wally_psbt *psbt, struct wallet *w) { + bool ok = true; + tal_wally_start(); for (size_t outndx = 0; outndx < psbt->num_outputs; ++outndx) { struct ext_key ext; @@ -756,11 +758,16 @@ static void match_psbt_outputs_to_wallet(struct wally_psbt *psbt, abort(); } - psbt_output_set_keypath(index, &ext, - is_p2tr(script, script_len, NULL), - &psbt->outputs[outndx]); + if (!psbt_output_set_keypath(index, &ext, + is_p2tr(script, script_len, NULL), + &psbt->outputs[outndx])) { + ok = false; + break; + } } tal_wally_end(psbt); + + return ok; } static struct command_result *param_input_numbers(struct command *cmd, @@ -842,7 +849,9 @@ static struct command_result *json_signpsbt(struct command *cmd, return command_check_done(cmd); /* Update the keypaths on any outputs that are in our wallet (change addresses). */ - match_psbt_outputs_to_wallet(psbt, cmd->ld->wallet); + if (!match_psbt_outputs_to_wallet(psbt, cmd->ld->wallet)) + return command_fail(cmd, JSONRPC2_INVALID_PARAMS, + "Could not add keypaths to PSBT?"); /* FIXME: hsm will sign almost anything, but it should really * fail cleanly (not abort!) and let us report the error here. */