From 7a276bbe09c4fd40f07433cc80174d61fe964da7 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 6 May 2025 05:21:46 +0930 Subject: [PATCH] common/utxo: use a real type for the UTXO, not a boolean is_p2sh. To actually evaluate spend cost, we need to know whether it's taproot or not. Using an enum (rather than making callers examine the script) means we can ensure all cases are handled. Signed-off-by: Rusty Russell --- common/utxo.c | 19 +++++++++++++++++-- common/utxo.h | 15 ++++++++++++++- devtools/mkfunding.c | 4 ++-- tests/test_closing.py | 11 +++++++---- tests/test_misc.py | 5 +++-- wallet/reservation.c | 2 +- wallet/test/run-wallet.c | 14 +++++++++----- wallet/wallet.c | 41 ++++++++++++++++++++++++++++++++++------ wallet/walletrpc.c | 4 ++-- 9 files changed, 90 insertions(+), 25 deletions(-) diff --git a/common/utxo.c b/common/utxo.c index 0b5bef357..d6f28b685 100644 --- a/common/utxo.c +++ b/common/utxo.c @@ -8,10 +8,10 @@ size_t utxo_spend_weight(const struct utxo *utxo, size_t min_witness_weight) /* If the min is less than what we'd use for a 'normal' tx, * we return the value with the greater added/calculated */ if (wit_weight < min_witness_weight) - return bitcoin_tx_input_weight(utxo->is_p2sh, + return bitcoin_tx_input_weight(utxo->utxotype == UTXO_P2SH_P2WPKH, min_witness_weight); - return bitcoin_tx_input_weight(utxo->is_p2sh, wit_weight); + return bitcoin_tx_input_weight(utxo->utxotype == UTXO_P2SH_P2WPKH, wit_weight); } u32 utxo_is_immature(const struct utxo *utxo, u32 blockheight) @@ -31,3 +31,18 @@ u32 utxo_is_immature(const struct utxo *utxo, u32 blockheight) return 0; } } + +const char *utxotype_to_str(enum utxotype utxotype) +{ + switch (utxotype) { + case UTXO_P2SH_P2WPKH: + return "p2sh_p2wpkh"; + case UTXO_P2WPKH: + return "p2wpkh"; + case UTXO_P2WSH_FROM_CLOSE: + return "p2wsh_from_close"; + case UTXO_P2TR: + return "p2tr"; + } + abort(); +} diff --git a/common/utxo.h b/common/utxo.h index f4dcf50df..799113f34 100644 --- a/common/utxo.h +++ b/common/utxo.h @@ -31,11 +31,24 @@ enum output_status { OUTPUT_STATE_ANY = 255 }; +enum utxotype { + /* Obsolete: we used to have P2SH-wrapped outputs (removed in 24.02, though can still have old UTXOs) */ + UTXO_P2SH_P2WPKH = 1, + /* "bech32" addresses */ + UTXO_P2WPKH = 2, + /* Used for closing addresses: implies ->close_info is non-NULL */ + UTXO_P2WSH_FROM_CLOSE = 3, + /* "p2tr" addresses. */ + UTXO_P2TR = 4, +}; + +const char *utxotype_to_str(enum utxotype utxotype); + struct utxo { struct bitcoin_outpoint outpoint; struct amount_sat amount; u32 keyindex; - bool is_p2sh; + enum utxotype utxotype; enum output_status status; /* Optional unilateral close information, NULL if this is just diff --git a/devtools/mkfunding.c b/devtools/mkfunding.c index 2bd7cb24f..319172131 100644 --- a/devtools/mkfunding.c +++ b/devtools/mkfunding.c @@ -42,7 +42,7 @@ static struct bitcoin_tx *tx_spending_utxo(const tal_t *ctx, struct bitcoin_tx *tx = bitcoin_tx(ctx, chainparams, 1, num_output, nlocktime); - assert(!utxo->is_p2sh); + assert(utxo->utxotype != UTXO_P2SH_P2WPKH); bitcoin_tx_add_input(tx, &utxo->outpoint, nsequence, NULL, utxo->amount, utxo->scriptPubkey, NULL); @@ -95,7 +95,7 @@ int main(int argc, char *argv[]) if (argc != 1 + 7) errx(1, "Usage: mkfunding "); - input.is_p2sh = false; + input.utxotype = UTXO_P2WPKH; input.close_info = NULL; argnum = 1; diff --git a/tests/test_closing.py b/tests/test_closing.py index b27f65abc..1681081a5 100644 --- a/tests/test_closing.py +++ b/tests/test_closing.py @@ -76,8 +76,9 @@ def test_closing_simple(node_factory, bitcoind, chainparams): ] bitcoind.generate_block(1) - l1.daemon.wait_for_log(r'Owning output.* \(SEGWIT\).* txid %s.* CONFIRMED' % closetxid) - l2.daemon.wait_for_log(r'Owning output.* \(SEGWIT\).* txid %s.* CONFIRMED' % closetxid) + outtype = 'p2tr' if not chainparams['elements'] else 'p2wpkh' + l1.daemon.wait_for_log(rf'Owning output.* \({outtype}\).* txid %s.* CONFIRMED' % closetxid) + l2.daemon.wait_for_log(rf'Owning output.* \({outtype}\).* txid %s.* CONFIRMED' % closetxid) # Make sure both nodes have grabbed their close tx funds assert closetxid in set([o['txid'] for o in l1.rpc.listfunds()['outputs']]) @@ -333,13 +334,15 @@ def test_closing_specified_destination(node_factory, bitcoind, chainparams): bitcoind.generate_block(1) sync_blockheight(bitcoind, [l1, l2, l3, l4]) + outtype = 'p2tr' if not chainparams['elements'] else 'p2wpkh' + # l1 can't spent the output to addr. for txid in closetxs.values(): - assert not l1.daemon.is_in_log(r'Owning output.* \(SEGWIT\).* txid {}.* CONFIRMED'.format(txid)) + assert not l1.daemon.is_in_log(rf'Owning output.* \({outtype}\).* txid {txid}.* CONFIRMED') # Check the txid has at least 1 confirmation for n, txid in closetxs.items(): - n.daemon.wait_for_log(r'Owning output.* \(SEGWIT\).* txid {}.* CONFIRMED'.format(txid)) + n.daemon.wait_for_log(rf'Owning output.* \({outtype}\).* txid {txid}.* CONFIRMED') for n in [l2, l3, l4]: # Make sure both nodes have grabbed their close tx funds diff --git a/tests/test_misc.py b/tests/test_misc.py index 0699c4096..b42f469bc 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -359,7 +359,7 @@ def test_ping(node_factory): ping_tests(l1, l2) -def test_htlc_sig_persistence(node_factory, bitcoind, executor): +def test_htlc_sig_persistence(node_factory, bitcoind, executor, chainparams): """Interrupt a payment between two peers, then fail and recover funds using the HTLC sig. """ # Feerates identical so we don't get gratuitous commit to update them @@ -399,8 +399,9 @@ def test_htlc_sig_persistence(node_factory, bitcoind, executor): bitcoind.generate_block(5) bitcoind.generate_block(1, wait_for_mempool=txid) + outtype = 'p2tr' if not chainparams['elements'] else 'p2wpkh' l1.daemon.wait_for_logs([ - r'Owning output . (\d+)sat .SEGWIT. txid', + rf'Owning output . (\d+)sat \({outtype}\) txid {txid} CONFIRMED', ]) # We should now have 1) the unilateral to us, and b) the HTLC respend to us diff --git a/wallet/reservation.c b/wallet/reservation.c index b68155420..9c100a0cc 100644 --- a/wallet/reservation.c +++ b/wallet/reservation.c @@ -277,7 +277,7 @@ struct wally_psbt *psbt_using_utxos(const tal_t *ctx, u32 this_nsequence; struct bitcoin_tx *tx; - if (utxos[i]->is_p2sh) { + if (utxos[i]->utxotype == UTXO_P2SH_P2WPKH) { bip32_pubkey(wallet->ld, &key, utxos[i]->keyindex); scriptSig = bitcoin_scriptsig_p2sh_p2wpkh(tmpctx, &key); redeemscript = bitcoin_redeem_p2sh_p2wpkh(tmpctx, &key); diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index 7e03d279e..b957639b4 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -1403,9 +1403,11 @@ static bool test_wallet_outputs(struct lightningd *ld, const tal_t *ctx) u.close_info->peer_id = id; u.close_info->commitment_point = &pk; u.close_info->option_anchors = false; - /* Arbitrarily set scriptpubkey len to 20 */ - u.scriptPubkey = tal_arr(w, u8, 20); - memset(u.scriptPubkey, 1, 20); + /* P2WSH */ + u.scriptPubkey = tal_arr(w, u8, BITCOIN_SCRIPTPUBKEY_P2WSH_LEN); + u.scriptPubkey[0] = OP_0; + u.scriptPubkey[1] = sizeof(struct sha256); + memset(u.scriptPubkey + 2, 1, sizeof(struct sha256)); CHECK_MSG(wallet_add_utxo(w, &u, our_change), "wallet_add_utxo with close_info"); @@ -1473,8 +1475,10 @@ static bool test_wallet_outputs(struct lightningd *ld, const tal_t *ctx) CHECK_MSG(!wallet_err, wallet_err); u.blockheight = blockheight; - u.scriptPubkey = tal_arr(w, u8, 20); - memset(u.scriptPubkey, 1, 20); + u.scriptPubkey = tal_arr(w, u8, BITCOIN_SCRIPTPUBKEY_P2WPKH_LEN); + u.scriptPubkey[0] = OP_0; + u.scriptPubkey[1] = sizeof(struct ripemd160); + memset(u.scriptPubkey + 2, 1, sizeof(struct ripemd160)); CHECK_MSG(wallet_add_utxo(w, &u, p2sh_wpkh), "wallet_add_utxo with close_info no commitment_point"); CHECK_MSG(!wallet_err, wallet_err); diff --git a/wallet/wallet.c b/wallet/wallet.c index df600a06f..9b27966ac 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -338,7 +338,6 @@ static struct utxo *wallet_stmt2output(const tal_t *ctx, struct db_stmt *stmt) db_col_txid(stmt, "prev_out_tx", &utxo->outpoint.txid); utxo->outpoint.n = db_col_int(stmt, "prev_out_index"); utxo->amount = db_col_amount_sat(stmt, "value"); - utxo->is_p2sh = db_col_int(stmt, "type") == p2sh_wpkh; utxo->status = db_col_int(stmt, "status"); utxo->keyindex = db_col_int(stmt, "keyindex"); @@ -364,6 +363,19 @@ static struct utxo *wallet_stmt2output(const tal_t *ctx, struct db_stmt *stmt) } utxo->scriptPubkey = db_col_arr(utxo, stmt, "scriptpubkey", u8); + /* FIXME: add p2tr to type? */ + if (db_col_int(stmt, "type") == p2sh_wpkh) + utxo->utxotype = UTXO_P2SH_P2WPKH; + else if (is_p2wpkh(utxo->scriptPubkey, tal_bytelen(utxo->scriptPubkey), NULL)) + utxo->utxotype = UTXO_P2WPKH; + else if (is_p2tr(utxo->scriptPubkey, tal_bytelen(utxo->scriptPubkey), NULL)) + utxo->utxotype = UTXO_P2TR; + else if (is_p2wsh(utxo->scriptPubkey, tal_bytelen(utxo->scriptPubkey), NULL)) { + if (!utxo->close_info) + fatal("Unspendable scriptPubkey without close_info %s", tal_hex(tmpctx, utxo->scriptPubkey)); + utxo->utxotype = UTXO_P2WSH_FROM_CLOSE; + } else + fatal("Unknown utxo type %s", tal_hex(tmpctx, utxo->scriptPubkey)); utxo->blockheight = NULL; utxo->spendheight = NULL; @@ -776,7 +788,7 @@ struct utxo *wallet_find_utxo(const tal_t *ctx, struct wallet *w, while (!utxo && db_step(stmt)) { utxo = wallet_stmt2output(ctx, stmt); if (excluded(excludes, utxo) - || (nonwrapped && utxo->is_p2sh) + || (nonwrapped && utxo->utxotype == UTXO_P2SH_P2WPKH) || !deep_enough(maxheight, utxo, current_blockheight)) utxo = tal_free(utxo); @@ -3028,7 +3040,24 @@ static void got_utxo(struct wallet *w, struct amount_asset asset = wally_tx_output_get_amount(txout); utxo->keyindex = keyindex; - utxo->is_p2sh = (addrtype == ADDR_P2SH_SEGWIT); + /* This switch() pattern catches anyone adding new cases, plus + * runtime errors */ + switch (addrtype) { + case ADDR_P2SH_SEGWIT: + utxo->utxotype = UTXO_P2SH_P2WPKH; + goto type_ok; + case ADDR_BECH32: + utxo->utxotype = UTXO_P2WPKH; + goto type_ok; + case ADDR_P2TR: + utxo->utxotype = UTXO_P2TR; + goto type_ok; + case ADDR_ALL: + break; + } + abort(); + +type_ok: utxo->amount = amount_asset_to_sat(&asset); utxo->status = OUTPUT_STATE_AVAILABLE; wally_txid(wtx, &utxo->outpoint.txid); @@ -3042,7 +3071,7 @@ static void got_utxo(struct wallet *w, log_debug(w->log, "Owning output %zu %s (%s) txid %s%s%s", outnum, fmt_amount_sat(tmpctx, utxo->amount), - utxo->is_p2sh ? "P2SH" : "SEGWIT", + utxotype_to_str(utxo->utxotype), fmt_bitcoin_txid(tmpctx, &utxo->outpoint.txid), blockheight ? " CONFIRMED" : "", is_coinbase ? " COINBASE" : ""); @@ -3058,7 +3087,7 @@ static void got_utxo(struct wallet *w, notify_chain_mvt(w->ld, mvt); } - if (!wallet_add_utxo(w, utxo, utxo->is_p2sh ? p2sh_wpkh : our_change)) { + if (!wallet_add_utxo(w, utxo, utxo->utxotype == UTXO_P2SH_P2WPKH ? p2sh_wpkh : our_change)) { /* In case we already know the output, make * sure we actually track its * blockheight. This can happen when we grab @@ -3070,7 +3099,7 @@ static void got_utxo(struct wallet *w, } /* This is an unconfirmed change output, we should track it */ - if (!utxo->is_p2sh && !blockheight) + if (utxo->utxotype != UTXO_P2SH_P2WPKH && !blockheight) txfilter_add_scriptpubkey(w->ld->owned_txfilter, txout->script); outpointfilter_add(w->owned_outpoints, &utxo->outpoint); diff --git a/wallet/walletrpc.c b/wallet/walletrpc.c index e1d255382..bc0a86f2d 100644 --- a/wallet/walletrpc.c +++ b/wallet/walletrpc.c @@ -349,7 +349,7 @@ static void json_add_utxo(struct json_stream *response, json_add_num(response, "output", utxo->outpoint.n); json_add_amount_sat_msat(response, "amount_msat", utxo->amount); - if (utxo->is_p2sh) { + if (utxo->utxotype == UTXO_P2SH_P2WPKH) { struct pubkey key; bip32_pubkey(wallet->ld, &key, utxo->keyindex); @@ -693,7 +693,7 @@ static struct command_result *match_psbt_inputs_to_utxos(struct command *cmd, if (!psbt->inputs[i].utxo && !psbt->inputs[i].witness_utxo) { u8 *scriptPubKey; - if (utxo->is_p2sh) { + if (utxo->utxotype == UTXO_P2SH_P2WPKH) { struct pubkey key; u8 *redeemscript; int wally_err;