btcpayserver/lightning
btcpayserver/lightning
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

PSBT: Change bitcoin_tx routine to use TAKES

Browse Source 
`bitcoin_tx_with_psbt` would somewhat opaquely steal the passed `psbt` value.

This caused a bug where code made a `bitcoin_tx` using a psbt without realizing the value was stolen. Because the resulting `bitcoin_tx` was placed in tmpctx it was not immediately clear that using `psbt` afterwards was an error until the tmpctx was cleared — creating a valgrind backtrace far from the actual issue.

Switching to the routine to using TAKES and adding documentation in the header, makes it explicitly clear which operation the user is doing — helping prevent future regressions of this kind.

Changelog-None
This commit is contained in:
Dusty Daemon 2025-02-16 15:13:01 -05:00 committed by Rusty Russell
parent 58252a53a6
commit 23e80d9e81
5 changed files with 11 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
bitcoin/tx.c
View File

@ -548,9 +548,13 @@ void bitcoin_tx_finalize(struct bitcoin_tx *tx)
assert(bitcoin_tx_check(tx));
}
struct bitcoin_tx *bitcoin_tx_with_psbt(const tal_t *ctx, struct wally_psbt *psbt STEALS)
struct bitcoin_tx *bitcoin_tx_with_psbt(const tal_t *ctx, struct wally_psbt *psbt TAKES)
{
size_t locktime;
if (!taken(psbt))
psbt = clone_psbt(NULL, psbt);
wally_psbt_get_locktime(psbt, &locktime);
struct bitcoin_tx *tx = bitcoin_tx(ctx, chainparams,
psbt->num_inputs,

3
bitcoin/tx.h
View File

@ -111,7 +111,8 @@ bool bitcoin_txid_to_hex(const struct bitcoin_txid *txid,
char *hexstr, size_t hexstr_len);
/* Create a bitcoin_tx from a psbt */
struct bitcoin_tx *bitcoin_tx_with_psbt(const tal_t *ctx, struct wally_psbt *psbt);
struct bitcoin_tx *bitcoin_tx_with_psbt(const tal_t *ctx,
struct wally_psbt *psbt TAKES);
/* Internal de-linearization functions. */
/* Pull a bitcoin tx, and create a PSBT wrapper for it */

2
db/bindings.c
View File

@ -501,7 +501,7 @@ struct bitcoin_tx *db_col_psbt_to_tx(const tal_t *ctx, struct db_stmt *stmt, con
struct wally_psbt *psbt = db_col_psbt(ctx, stmt, colname);
if (!psbt)
return NULL;
return bitcoin_tx_with_psbt(ctx, psbt);
return bitcoin_tx_with_psbt(ctx, take(psbt));
}
struct channel_type *db_col_channel_type(const tal_t *ctx, struct db_stmt *stmt,

4
lightningd/channel.c
View File

@ -220,12 +220,10 @@ void inflight_set_last_tx(struct channel_inflight *inflight,
struct bitcoin_tx *last_tx,
const struct bitcoin_signature last_sig)
{
struct wally_psbt *last_tx_psbt_clone;
assert(inflight->last_tx == NULL);
assert(last_tx);
last_tx_psbt_clone = clone_psbt(inflight, last_tx->psbt);
inflight->last_tx = bitcoin_tx_with_psbt(inflight, last_tx_psbt_clone);
inflight->last_tx = bitcoin_tx_with_psbt(inflight, last_tx->psbt);
inflight->last_sig = last_sig;
}

7
lightningd/peer_control.c
View File

@ -2131,8 +2131,6 @@ void update_channel_from_inflight(struct lightningd *ld,
struct channel *channel,
const struct channel_inflight *inflight)
{
struct wally_psbt *psbt_copy;
channel->funding = inflight->funding->outpoint;
channel->funding_sats = inflight->funding->total_funds;
@ -2167,10 +2165,9 @@ void update_channel_from_inflight(struct lightningd *ld,
channel->opener,
&inflight->lease_blockheight_start);
/* Make a 'clone' of this tx */
psbt_copy = clone_psbt(channel, inflight->last_tx->psbt);
channel_set_last_tx(channel,
bitcoin_tx_with_psbt(channel, psbt_copy),
bitcoin_tx_with_psbt(channel,
inflight->last_tx->psbt),
&inflight->last_sig);
/* If the remote side rotated their pubkey during splice, update now */