From 9e7c0d95491d300d7f0d94b2c19246be0826fb0d Mon Sep 17 00:00:00 2001 From: niftynei Date: Wed, 15 Jul 2020 04:59:26 +0930 Subject: [PATCH 01/12] utxos: add a 'reserved_til' marker for utxos Allow a utxo to be reserved until explicitly unreserved or until a timer runs out. Currently unused. We explicitly do not unreserve these at startup. --- common/utxo.h | 3 +++ wallet/db.c | 1 + wallet/wallet.c | 31 ++++++++++++++++++++++--------- wallet/walletrpc.c | 21 +++++++++++++++++---- wallet/walletrpc.h | 3 +++ 5 files changed, 46 insertions(+), 13 deletions(-) diff --git a/common/utxo.h b/common/utxo.h index 93a16bbf5b93..09ec3db8eff6 100644 --- a/common/utxo.h +++ b/common/utxo.h @@ -39,6 +39,9 @@ struct utxo { /* NULL if not spent yet, otherwise, the block the spending transaction is in */ const u32 *spendheight; + /* Block this utxo becomes unreserved, if applicable */ + u32 *reserved_til; + /* The scriptPubkey if it is known */ u8 *scriptPubkey; diff --git a/wallet/db.c b/wallet/db.c index 5ef666b0e2c0..b3cca1b9ea30 100644 --- a/wallet/db.c +++ b/wallet/db.c @@ -617,6 +617,7 @@ static struct migration dbmigrations[] = { /* We track the counter for coin_moves, as a convenience for notification consumers */ {SQL("INSERT INTO vars (name, intval) VALUES ('coin_moves_count', 0);"), NULL}, {NULL, migrate_last_tx_to_psbt}, + {SQL("ALTER TABLE outputs ADD reserved_til INTEGER DEFAULT NULL;"), NULL}, }; /* Leak tracking. */ diff --git a/wallet/wallet.c b/wallet/wallet.c index 3fdc8ee9d31d..1dfce94451e4 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -158,7 +158,7 @@ static bool wallet_add_utxo(struct wallet *w, struct utxo *utxo, static struct utxo *wallet_stmt2output(const tal_t *ctx, struct db_stmt *stmt) { struct utxo *utxo = tal(ctx, struct utxo); - u32 *blockheight, *spendheight; + u32 *blockheight, *spendheight, *reserved_til; db_column_txid(stmt, 0, &utxo->txid); utxo->outnum = db_column_int(stmt, 1); db_column_amount_sat(stmt, 2, &utxo->amount); @@ -184,6 +184,7 @@ static struct utxo *wallet_stmt2output(const tal_t *ctx, struct db_stmt *stmt) utxo->spendheight = NULL; utxo->scriptPubkey = NULL; utxo->scriptSig = NULL; + utxo->reserved_til = NULL; if (!db_column_is_null(stmt, 9)) { blockheight = tal(utxo, u32); @@ -202,6 +203,11 @@ static struct utxo *wallet_stmt2output(const tal_t *ctx, struct db_stmt *stmt) tal_dup_arr(utxo, u8, db_column_blob(stmt, 11), db_column_bytes(stmt, 11), 0); } + if (!db_column_is_null(stmt, 12)) { + reserved_til = tal(utxo, u32); + *reserved_til = db_column_int(stmt, 12); + utxo->reserved_til = reserved_til; + } return utxo; } @@ -255,6 +261,7 @@ struct utxo **wallet_get_utxos(const tal_t *ctx, struct wallet *w, const enum ou ", confirmation_height" ", spend_height" ", scriptpubkey " + ", reserved_til " "FROM outputs")); } else { stmt = db_prepare_v2(w->db, SQL("SELECT" @@ -270,6 +277,7 @@ struct utxo **wallet_get_utxos(const tal_t *ctx, struct wallet *w, const enum ou ", confirmation_height" ", spend_height" ", scriptpubkey " + ", reserved_til " "FROM outputs " "WHERE status= ? ")); db_bind_int(stmt, 0, output_status_in_db(state)); @@ -306,6 +314,7 @@ struct utxo **wallet_get_unconfirmed_closeinfo_utxos(const tal_t *ctx, ", confirmation_height" ", spend_height" ", scriptpubkey" + ", reserved_til" " FROM outputs" " WHERE channel_id IS NOT NULL AND " "confirmation_height IS NULL")); @@ -341,6 +350,7 @@ struct utxo *wallet_utxo_get(const tal_t *ctx, struct wallet *w, ", confirmation_height" ", spend_height" ", scriptpubkey" + ", reserved_til" " FROM outputs" " WHERE prev_out_tx = ?" " AND prev_out_index = ?")); @@ -3799,14 +3809,17 @@ static void process_utxo_result(struct bitcoind *bitcoind, enum output_status newstate = txout == NULL ? output_state_spent : output_state_available; - log_unusual(bitcoind->ld->wallet->log, - "wallet: reserved output %s/%u reset to %s", - type_to_string(tmpctx, struct bitcoin_txid, &utxos[0]->txid), - utxos[0]->outnum, - newstate == output_state_spent ? "spent" : "available"); - wallet_update_output_status(bitcoind->ld->wallet, - &utxos[0]->txid, utxos[0]->outnum, - utxos[0]->status, newstate); + /* Don't unreserve ones which are on timers */ + if (!utxos[0]->reserved_til || newstate == output_state_spent) { + log_unusual(bitcoind->ld->wallet->log, + "wallet: reserved output %s/%u reset to %s", + type_to_string(tmpctx, struct bitcoin_txid, &utxos[0]->txid), + utxos[0]->outnum, + newstate == output_state_spent ? "spent" : "available"); + wallet_update_output_status(bitcoind->ld->wallet, + &utxos[0]->txid, utxos[0]->outnum, + utxos[0]->status, newstate); + } /* If we have more, resolve them too. */ tal_arr_remove(&utxos, 0); diff --git a/wallet/walletrpc.c b/wallet/walletrpc.c index 0644c7826936..8fdaf84a740b 100644 --- a/wallet/walletrpc.c +++ b/wallet/walletrpc.c @@ -843,6 +843,19 @@ static const struct json_command listaddrs_command = { }; AUTODATA(json_command, &listaddrs_command); +bool is_reserved(const struct utxo *utxo, u32 current_height) +{ + if (utxo->status != output_state_reserved) + return false; + + /* FIXME: Eventually this will always be set! */ + if (!utxo->reserved_til) + return true; + + return *utxo->reserved_til > current_height; +} + + static void json_add_utxo(struct json_stream *response, const char *fieldname, struct wallet *wallet, @@ -899,7 +912,8 @@ static void json_add_utxo(struct json_stream *response, json_add_string(response, "status", "unconfirmed"); json_add_bool(response, "reserved", - utxo->status == output_state_reserved); + is_reserved(utxo, + get_block_height(wallet->ld->topology))); json_object_end(response); } @@ -1315,9 +1329,8 @@ static struct command_result *match_psbt_inputs_to_utxos(struct command *cmd, if (!utxo) continue; - /* Oops we haven't reserved this utxo yet. - * Let's just go ahead and reserve it now. */ - if (utxo->status != output_state_reserved) + /* Oops we haven't reserved this utxo yet! */ + if (!is_reserved(utxo, get_block_height(cmd->ld->topology))) return command_fail(cmd, LIGHTNINGD, "Aborting PSBT signing. UTXO %s:%u is not reserved", type_to_string(tmpctx, struct bitcoin_txid, diff --git a/wallet/walletrpc.h b/wallet/walletrpc.h index 3fc0b53f865c..d9717baec223 100644 --- a/wallet/walletrpc.h +++ b/wallet/walletrpc.h @@ -9,4 +9,7 @@ void json_add_utxos(struct json_stream *response, struct wallet *wallet, struct utxo **utxos); +/* We evaluate reserved timeouts lazily, so use this. */ +bool is_reserved(const struct utxo *utxo, u32 current_height); + #endif /* LIGHTNING_WALLET_WALLETRPC_H */ From 9c7a3744db6d5b5c5097497f4c23d67d8931cbba Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 15 Jul 2020 05:00:26 +0930 Subject: [PATCH 02/12] wallet: explicit routines to reserve/unreserve a UTXO. These keep the struct utxo in sync with the database, explicitly: these will be the only places where utxo->status is set. The old routines will be removed at the end. Signed-off-by: Rusty Russell --- wallet/wallet.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++ wallet/wallet.h | 17 ++++++++++ 2 files changed, 101 insertions(+) diff --git a/wallet/wallet.c b/wallet/wallet.c index 1dfce94451e4..3f096e7c0a94 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -28,6 +28,9 @@ * to prune? */ #define UTXO_PRUNE_DEPTH 144 +/* 12 hours is usually enough reservation time */ +#define RESERVATION_INC (6 * 12) + static void outpointfilters_init(struct wallet *w) { struct db_stmt *stmt; @@ -418,6 +421,87 @@ void wallet_confirm_utxos(struct wallet *w, const struct utxo **utxos) } } +static void db_set_utxo(struct db *db, const struct utxo *utxo) +{ + struct db_stmt *stmt; + + if (utxo->status == output_state_reserved) + assert(utxo->reserved_til); + else + assert(!utxo->reserved_til); + + stmt = db_prepare_v2( + db, SQL("UPDATE outputs SET status=?, reserved_til=?" + "WHERE prev_out_tx=? AND prev_out_index=?")); + db_bind_int(stmt, 0, output_status_in_db(utxo->status)); + if (utxo->reserved_til) + db_bind_int(stmt, 1, *utxo->reserved_til); + else + db_bind_null(stmt, 1); + db_bind_txid(stmt, 2, &utxo->txid); + db_bind_int(stmt, 3, utxo->outnum); + db_exec_prepared_v2(take(stmt)); +} + +bool wallet_reserve_utxo(struct wallet *w, struct utxo *utxo, u32 current_height) +{ + u32 reservation_height; + + if (utxo->status == output_state_reserved) + assert(utxo->reserved_til); + else + assert(!utxo->reserved_til); + + switch (utxo->status) { + case output_state_spent: + return false; + case output_state_available: + case output_state_reserved: + break; + case output_state_any: + abort(); + } + + /* We simple increase existing reservations, which DTRT if we unreserve */ + if (utxo->reserved_til + && *utxo->reserved_til >= current_height) + reservation_height = *utxo->reserved_til + RESERVATION_INC; + else + reservation_height = current_height + RESERVATION_INC; + + utxo->status = output_state_reserved; + tal_free(utxo->reserved_til); + utxo->reserved_til = tal_dup(utxo, u32, &reservation_height); + + db_set_utxo(w->db, utxo); + + return true; +} + +void wallet_unreserve_utxo(struct wallet *w, struct utxo *utxo, u32 current_height) +{ + if (utxo->status == output_state_reserved) { + /* FIXME: old code didn't set reserved_til, so fake it here */ + if (!utxo->reserved_til) + utxo->reserved_til = tal_dup(utxo, u32, ¤t_height); + assert(utxo->reserved_til); + } else + assert(!utxo->reserved_til); + + if (utxo->status != output_state_reserved) + fatal("UTXO %s:%u is not reserved", + type_to_string(tmpctx, struct bitcoin_txid, &utxo->txid), + utxo->outnum); + + if (*utxo->reserved_til <= current_height + RESERVATION_INC) { + utxo->status = output_state_available; + utxo->reserved_til = tal_free(utxo->reserved_til); + } else + *utxo->reserved_til -= RESERVATION_INC; + + db_set_utxo(w->db, utxo); +} + bool wallet_add_onchaind_utxo(struct wallet *w, const struct bitcoin_txid *txid, u32 outnum, diff --git a/wallet/wallet.h b/wallet/wallet.h index 19ef4fe5883f..34e76989ceaf 100644 --- a/wallet/wallet.h +++ b/wallet/wallet.h @@ -392,6 +392,23 @@ bool wallet_add_onchaind_utxo(struct wallet *w, /* NULL if option_static_remotekey */ const struct pubkey *commitment_point); +/** + * wallet_reserve_utxo - set a reservation on a UTXO. + * + * If the reservation is already reserved, refreshes the reservation, + * otherwise if it's not available, returns false. + */ +bool wallet_reserve_utxo(struct wallet *w, + struct utxo *utxo, + u32 reservation_blocknum); + +/* wallet_unreserve_utxo - make a reserved UTXO available again. + * + * Must be reserved. + */ +void wallet_unreserve_utxo(struct wallet *w, struct utxo *utxo, + u32 current_height); + /** wallet_utxo_get - Retrive a utxo. * * Returns a utxo, or NULL if not found. From 4f9fd4661624de7c701c2aeb7bca22e25b674f85 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 15 Jul 2020 15:00:49 +0930 Subject: [PATCH 03/12] wallet: add wallet_find_utxo(). This is a new fundamental routine to obtain UTXOs from the database. It's not the most efficient approach, as it returns a single UTXO at a time, but it can consolidate all our UTXO handling (becoming more complex by the reservation timeout logic). Signed-off-by: Rusty Russell --- wallet/wallet.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++++ wallet/wallet.h | 23 ++++++++++++++++ 2 files changed, 96 insertions(+) diff --git a/wallet/wallet.c b/wallet/wallet.c index 3f096e7c0a94..92ff7b8b6cbe 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -502,6 +502,79 @@ void wallet_unreserve_utxo(struct wallet *w, struct utxo *utxo, u32 current_heig db_set_utxo(w->db, utxo); } +static bool excluded(const struct utxo **excludes, + const struct utxo *utxo) +{ + for (size_t i = 0; i < tal_count(excludes); i++) { + if (bitcoin_txid_eq(&excludes[i]->txid, &utxo->txid) + && excludes[i]->outnum == utxo->outnum) + return true; + } + return false; +} + +static bool deep_enough(u32 maxheight, const struct utxo *utxo) +{ + /* If we require confirmations check that we have a + * confirmation height and that it is below the required + * maxheight (current_height - minconf) */ + if (maxheight == 0) + return true; + if (!utxo->blockheight) + return false; + return *utxo->blockheight <= maxheight; +} + +/* FIXME: Make this wallet_find_utxos, and branch and bound and I've + * left that to @niftynei to do, who actually read the paper! */ +struct utxo *wallet_find_utxo(const tal_t *ctx, struct wallet *w, + unsigned current_blockheight, + struct amount_sat *amount_hint, + unsigned feerate_per_kw, + u32 maxheight, + const struct utxo **excludes) +{ + struct db_stmt *stmt; + struct utxo *utxo; + + stmt = db_prepare_v2(w->db, SQL("SELECT" + " prev_out_tx" + ", prev_out_index" + ", value" + ", type" + ", status" + ", keyindex" + ", channel_id" + ", peer_id" + ", commitment_point" + ", confirmation_height" + ", spend_height" + ", scriptpubkey " + ", reserved_til" + " FROM outputs" + " WHERE status = ?" + " OR (status = ? AND reserved_til <= ?)" + "ORDER BY RANDOM();")); + db_bind_int(stmt, 0, output_status_in_db(output_state_available)); + db_bind_int(stmt, 1, output_status_in_db(output_state_reserved)); + db_bind_u64(stmt, 2, current_blockheight); + + /* FIXME: Use feerate + estimate of input cost to establish + * range for amount_hint */ + + db_query_prepared(stmt); + + utxo = NULL; + while (!utxo && db_step(stmt)) { + utxo = wallet_stmt2output(ctx, stmt); + if (excluded(excludes, utxo) || !deep_enough(maxheight, utxo)) + utxo = tal_free(utxo); + + } + tal_free(stmt); + return utxo; +} + bool wallet_add_onchaind_utxo(struct wallet *w, const struct bitcoin_txid *txid, u32 outnum, diff --git a/wallet/wallet.h b/wallet/wallet.h index 34e76989ceaf..690bd7272606 100644 --- a/wallet/wallet.h +++ b/wallet/wallet.h @@ -374,6 +374,29 @@ struct utxo **wallet_get_utxos(const tal_t *ctx, struct wallet *w, struct utxo **wallet_get_unconfirmed_closeinfo_utxos(const tal_t *ctx, struct wallet *w); +/** + * wallet_find_utxo - Select an available UTXO (does not reserve it!). + * @ctx: tal context + * @w: wallet + * @current_blockheight: current chain length. + * @amount_we_are_short: optional amount. + * @feerate_per_kw: feerate we are using. + * @maxheight: zero (if caller doesn't care) or maximum blockheight to accept. + * @excludes: UTXOs not to consider. + * + * If @amount_we_are_short is not NULL, we try to get something very close + * (i.e. when we add this input, we will add => @amount_we_are_short, but + * less than @amount_we_are_short + dustlimit). + * + * Otherwise we give a random UTXO. + */ +struct utxo *wallet_find_utxo(const tal_t *ctx, struct wallet *w, + unsigned current_blockheight, + struct amount_sat *amount_we_are_short, + unsigned feerate_per_kw, + u32 maxheight, + const struct utxo **excludes); + /** * wallet_add_onchaind_utxo - Add a UTXO with spending info from onchaind. * From 5611246d348bdbfabf09cb3bec54b02c97365f3c Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 15 Jul 2020 15:01:49 +0930 Subject: [PATCH 04/12] wallet: new JSON commands reserveinputs and unreserveinputs. reserveinputs marks UTXOs reserved for 12 hours, so we won't select them for spending: unreserveinputs marks them available again. Exposes param_psbt() for wider use. Disabled the test_sign_and_send_psbt since we're altering the API; the final patch restores it. Signed-off-by: Rusty Russell --- doc/lightning-reserveinputs.7 | 71 ++++--------- doc/lightning-reserveinputs.7.md | 55 +++-------- doc/lightning-unreserveinputs.7 | 29 ++++-- doc/lightning-unreserveinputs.7.md | 21 ++-- tests/test_wallet.py | 1 + wallet/Makefile | 5 +- wallet/reservation.c | 154 +++++++++++++++++++++++++++++ wallet/walletrpc.c | 91 ++--------------- wallet/walletrpc.h | 8 ++ 9 files changed, 238 insertions(+), 197 deletions(-) create mode 100644 wallet/reservation.c diff --git a/doc/lightning-reserveinputs.7 b/doc/lightning-reserveinputs.7 index 06db8c23a33f..861aa11c22a0 100644 --- a/doc/lightning-reserveinputs.7 +++ b/doc/lightning-reserveinputs.7 @@ -3,58 +3,32 @@ lightning-reserveinputs - Construct a transaction and reserve the UTXOs it spends .SH SYNOPSIS -\fBreserveinputs\fR \fIoutputs\fR [\fIfeerate\fR] [\fIminconf\fR] [\fIutxos\fR] +\fBreserveinputs\fR \fIpsbt\fR .SH DESCRIPTION -The \fBreserveinputs\fR RPC command creates an unsigned PSBT which -spends funds from c-lightning’s internal wallet to the outputs specified -in \fIoutputs\fR\. - - -The \fIoutputs\fR is the array of output that include \fIdestination\fR -and \fIamount\fR({\fIdestination\fR: \fIamount\fR})\. Its format is like: -[{address1: amount1}, {address2: amount2}] -or -[{address: \fIall\fR}]\. -It supports any number of outputs\. - - -The \fIdestination\fR of output is the address which can be of any Bitcoin accepted -type, including bech32\. - - -The \fIamount\fR of output is the amount to be sent from the internal wallet -(expressed, as name suggests, in amount)\. The string \fIall\fR can be used to specify -all available funds\. Otherwise, it is in amount precision; it can be a whole -number, a whole number ending in \fIsat\fR, a whole number ending in \fI000msat\fR, -or a number with 1 to 8 decimal places ending in \fIbtc\fR\. - - -\fIfeerate\fR is an optional feerate to use\. It can be one of the strings -\fIurgent\fR (aim for next block), \fInormal\fR (next 4 blocks or so) or \fIslow\fR -(next 100 blocks or so) to use lightningd’s internal estimates: \fInormal\fR -is the default\. - - -Otherwise, \fIfeerate\fR is a number, with an optional suffix: \fIperkw\fR means -the number is interpreted as satoshi-per-kilosipa (weight), and \fIperkb\fR -means it is interpreted bitcoind-style as satoshi-per-kilobyte\. Omitting -the suffix is equivalent to \fIperkb\fR\. - - -\fIminconf\fR specifies the minimum number of confirmations that reserved UTXOs -should have\. Default is 1\. - - -\fIutxos\fR specifies the utxos to be used to fund the transaction, as an array -of "txid:vout"\. These must be drawn from the node's available UTXO set\. +The \fBreserveinputs\fR RPC command places (or increases) reservations on any +inputs specified in \fIpsbt\fR which are known to lightningd\. It will fail +with an error it any of the inputs are known to be spent\. .SH RETURN VALUE -On success, an object with attributes \fIpsbt\fR and \fIfeerate_per_kw\fR will be -returned\. The inputs of the \fIpsbt\fR have been marked as reserved in the internal wallet\. +On success, an \fIreservations\fR array is returned, with an entry for each input +which was reserved: + +.RS +.IP \[bu] +\fItxid\fR is the input transaction id\. +.IP \[bu] +\fIvout\fR is the input index\. +.IP \[bu] +\fIwas_reserved\fR indicates whether the input was already reserved\. +.IP \[bu] +\fIreserved\fR indicates that the input is now reserved (i\.e\. true)\. +.IP \[bu] +\fIreserved_to_block\fR indicates what blockheight the reservation will expire\. +.RE On failure, an error is reported and no UTXOs are reserved\. @@ -63,12 +37,7 @@ The following error codes may occur: .RS .IP \[bu] --1: Catchall nonspecific error\. -.IP \[bu] -301: There are not enough funds in the internal wallet (including -fees) to create the transaction\. -.IP \[bu] -302: The dust limit is not met\. +-32602: Invalid parameter, such as specifying a spent input in \fIpsbt\fR\. .RE .SH AUTHOR diff --git a/doc/lightning-reserveinputs.7.md b/doc/lightning-reserveinputs.7.md index 0ee15a25a6b2..0ffb0e95dc43 100644 --- a/doc/lightning-reserveinputs.7.md +++ b/doc/lightning-reserveinputs.7.md @@ -4,61 +4,32 @@ lightning-reserveinputs -- Construct a transaction and reserve the UTXOs it spen SYNOPSIS -------- -**reserveinputs** *outputs* \[*feerate*\] \[*minconf*\] \[*utxos*\] +**reserveinputs** *psbt* DESCRIPTION ----------- -The **reserveinputs** RPC command creates an unsigned PSBT which -spends funds from c-lightning’s internal wallet to the outputs specified -in *outputs*. - -The *outputs* is the array of output that include *destination* -and *amount*(\{*destination*: *amount*\}). Its format is like: -\[\{address1: amount1\}, \{address2: amount2\}\] -or -\[\{address: *all*\}\]. -It supports any number of outputs. - -The *destination* of output is the address which can be of any Bitcoin accepted -type, including bech32. - -The *amount* of output is the amount to be sent from the internal wallet -(expressed, as name suggests, in amount). The string *all* can be used to specify -all available funds. Otherwise, it is in amount precision; it can be a whole -number, a whole number ending in *sat*, a whole number ending in *000msat*, -or a number with 1 to 8 decimal places ending in *btc*. - -*feerate* is an optional feerate to use. It can be one of the strings -*urgent* (aim for next block), *normal* (next 4 blocks or so) or *slow* -(next 100 blocks or so) to use lightningd’s internal estimates: *normal* -is the default. - -Otherwise, *feerate* is a number, with an optional suffix: *perkw* means -the number is interpreted as satoshi-per-kilosipa (weight), and *perkb* -means it is interpreted bitcoind-style as satoshi-per-kilobyte. Omitting -the suffix is equivalent to *perkb*. - -*minconf* specifies the minimum number of confirmations that reserved UTXOs -should have. Default is 1. - -*utxos* specifies the utxos to be used to fund the transaction, as an array -of "txid:vout". These must be drawn from the node's available UTXO set. +The **reserveinputs** RPC command places (or increases) reservations on any +inputs specified in *psbt* which are known to lightningd. It will fail +with an error it any of the inputs are known to be spent. RETURN VALUE ------------ -On success, an object with attributes *psbt* and *feerate_per_kw* will be -returned. The inputs of the *psbt* have been marked as reserved in the internal wallet. +On success, an *reservations* array is returned, with an entry for each input +which was reserved: + +- *txid* is the input transaction id. +- *vout* is the input index. +- *was_reserved* indicates whether the input was already reserved. +- *reserved* indicates that the input is now reserved (i.e. true). +- *reserved_to_block* indicates what blockheight the reservation will expire. On failure, an error is reported and no UTXOs are reserved. The following error codes may occur: -- -1: Catchall nonspecific error. -- 301: There are not enough funds in the internal wallet (including -fees) to create the transaction. -- 302: The dust limit is not met. +- -32602: Invalid parameter, such as specifying a spent input in *psbt*. AUTHOR ------ diff --git a/doc/lightning-unreserveinputs.7 b/doc/lightning-unreserveinputs.7 index 037bf816bae2..f5bfca0de8f8 100644 --- a/doc/lightning-unreserveinputs.7 +++ b/doc/lightning-unreserveinputs.7 @@ -7,31 +7,40 @@ lightning-unreserveinputs - Release reserved UTXOs .SH DESCRIPTION -The \fBunreserveinputs\fR RPC command releases UTXOs which were previously -marked as reserved, generally by \fBlightning-reserveinputs\fR(7)\. +The \fBunreserveinputs\fR RPC command releases (or reduces reservation) +on UTXOs which were previously marked as reserved, generally by +\fBlightning-reserveinputs\fR(7)\. The inputs to unreserve are the inputs specified in the passed-in \fIpsbt\fR\. .SH RETURN VALUE -On success, an object with \fIoutputs\fR will be returned\. +On success, an \fIreservations\fR array is returned, with an entry for each input +which was reserved: +.RS +.IP \[bu] +\fItxid\fR is the input transaction id\. +.IP \[bu] +\fIvout\fR is the input index\. +.IP \[bu] +\fIwas_reserved\fR indicates whether the input was already reserved (generally true) +.IP \[bu] +\fIreserved\fR indicates that the input is now reserved (may still be true, if it was previously reserved for a long time)\. +.IP \[bu] +\fIreserved_to_block\fR (if \fIreserved\fR is still true) indicates what blockheight the reservation will expire\. -\fIoutputs\fR will include an entry for each input specified in the \fIpsbt\fR, -indicating the \fItxid\fR and \fIvout\fR for that input plus a boolean result - \fIunreserved\fR, which will be true if that UTXO was successfully unreserved -by this call\. - +.RE -Note that restarting lightningd will unreserve all UTXOs by default\. +On failure, an error is reported and no UTXOs are unreserved\. The following error codes may occur: .RS .IP \[bu] --1: An unparseable PSBT\. +-32602: Invalid parameter, i\.e\. an unparseable PSBT\. .RE .SH AUTHOR diff --git a/doc/lightning-unreserveinputs.7.md b/doc/lightning-unreserveinputs.7.md index b431751d880c..5adc6c3188da 100644 --- a/doc/lightning-unreserveinputs.7.md +++ b/doc/lightning-unreserveinputs.7.md @@ -9,25 +9,28 @@ SYNOPSIS DESCRIPTION ----------- -The **unreserveinputs** RPC command releases UTXOs which were previously -marked as reserved, generally by lightning-reserveinputs(7). +The **unreserveinputs** RPC command releases (or reduces reservation) +on UTXOs which were previously marked as reserved, generally by +lightning-reserveinputs(7). The inputs to unreserve are the inputs specified in the passed-in *psbt*. RETURN VALUE ------------ -On success, an object with *outputs* will be returned. +On success, an *reservations* array is returned, with an entry for each input +which was reserved: -*outputs* will include an entry for each input specified in the *psbt*, -indicating the *txid* and *vout* for that input plus a boolean result - *unreserved*, which will be true if that UTXO was successfully unreserved -by this call. +- *txid* is the input transaction id. +- *vout* is the input index. +- *was_reserved* indicates whether the input was already reserved (generally true) +- *reserved* indicates that the input is now reserved (may still be true, if it was previously reserved for a long time). +- *reserved_to_block* (if *reserved* is still true) indicates what blockheight the reservation will expire. -Note that restarting lightningd will unreserve all UTXOs by default. +On failure, an error is reported and no UTXOs are unreserved. The following error codes may occur: -- -1: An unparseable PSBT. +- -32602: Invalid parameter, i.e. an unparseable PSBT. AUTHOR ------ diff --git a/tests/test_wallet.py b/tests/test_wallet.py index dc4f1e394428..a07ec8e066f4 100644 --- a/tests/test_wallet.py +++ b/tests/test_wallet.py @@ -564,6 +564,7 @@ def test_reserveinputs(node_factory, bitcoind, chainparams): assert len(l1.rpc.listfunds()['outputs']) == 12 +@pytest.mark.xfail(strict=True) def test_sign_and_send_psbt(node_factory, bitcoind, chainparams): """ Tests for the sign + send psbt RPCs diff --git a/wallet/Makefile b/wallet/Makefile index 864126263e42..0930596b63f7 100644 --- a/wallet/Makefile +++ b/wallet/Makefile @@ -11,11 +11,14 @@ WALLET_LIB_SRC := \ wallet/wallet.c \ wallet/walletrpc.c +WALLET_LIB_SRC_NOHDR := \ + wallet/reservation.c + WALLET_DB_DRIVERS := \ wallet/db_postgres.c \ wallet/db_sqlite3.c -WALLET_LIB_OBJS := $(WALLET_LIB_SRC:.c=.o) $(WALLET_DB_DRIVERS:.c=.o) +WALLET_LIB_OBJS := $(WALLET_LIB_SRC:.c=.o) $(WALLET_LIB_SRC_NOHDR:.c=.o) $(WALLET_DB_DRIVERS:.c=.o) WALLET_LIB_HEADERS := $(WALLET_LIB_SRC:.c=.h) # Make sure these depend on everything. diff --git a/wallet/reservation.c b/wallet/reservation.c new file mode 100644 index 000000000000..f0e96eec308d --- /dev/null +++ b/wallet/reservation.c @@ -0,0 +1,154 @@ +/* Dealing with reserving UTXOs */ +#include +#include +#include +#include +#include +#include +#include + +static bool was_reserved(enum output_status oldstatus, + const u32 *reserved_til, + u32 current_height) +{ + if (oldstatus != output_state_reserved) + return false; + + return *reserved_til > current_height; +} + +static void json_add_reservestatus(struct json_stream *response, + const struct utxo *utxo, + enum output_status oldstatus, + u32 old_res, + u32 current_height) +{ + json_object_start(response, NULL); + json_add_txid(response, "txid", &utxo->txid); + json_add_u32(response, "vout", utxo->outnum); + json_add_bool(response, "was_reserved", + was_reserved(oldstatus, &old_res, current_height)); + json_add_bool(response, "reserved", + is_reserved(utxo, current_height)); + if (utxo->reserved_til) + json_add_u32(response, "reserved_to_block", + *utxo->reserved_til); + json_object_end(response); +} + +static struct command_result *json_reserveinputs(struct command *cmd, + const char *buffer, + const jsmntok_t *obj UNNEEDED, + const jsmntok_t *params) +{ + struct json_stream *response; + struct wally_psbt *psbt; + struct utxo **utxos = tal_arr(cmd, struct utxo *, 0); + + if (!param(cmd, buffer, params, + p_req("psbt", param_psbt, &psbt), + NULL)) + return command_param_failed(); + + for (size_t i = 0; i < psbt->tx->num_inputs; i++) { + struct bitcoin_txid txid; + struct utxo *utxo; + + wally_tx_input_get_txid(&psbt->tx->inputs[i], &txid); + utxo = wallet_utxo_get(cmd, cmd->ld->wallet, + &txid, psbt->tx->inputs[i].index); + if (!utxo) + continue; + if (utxo->status == output_state_spent) + return command_fail(cmd, JSONRPC2_INVALID_PARAMS, + "%s:%u already spent", + type_to_string(tmpctx, + struct bitcoin_txid, + &utxo->txid), + utxo->outnum); + tal_arr_expand(&utxos, utxo); + } + + response = json_stream_success(cmd); + json_array_start(response, "reservations"); + for (size_t i = 0; i < tal_count(utxos); i++) { + enum output_status oldstatus; + u32 old_res; + + oldstatus = utxos[i]->status; + old_res = utxos[i]->reserved_til ? *utxos[i]->reserved_til : 0; + + if (!wallet_reserve_utxo(cmd->ld->wallet, + utxos[i], + get_block_height(cmd->ld->topology))) { + fatal("Unable to reserve %s:%u!", + type_to_string(tmpctx, + struct bitcoin_txid, + &utxos[i]->txid), + utxos[i]->outnum); + } + json_add_reservestatus(response, utxos[i], oldstatus, old_res, + get_block_height(cmd->ld->topology)); + } + json_array_end(response); + return command_success(cmd, response); +} + +static const struct json_command reserveinputs_command = { + "reserveinputs", + "bitcoin", + json_reserveinputs, + "Reserve utxos (or increase their reservation)", + false +}; +AUTODATA(json_command, &reserveinputs_command); + +static struct command_result *json_unreserveinputs(struct command *cmd, + const char *buffer, + const jsmntok_t *obj UNNEEDED, + const jsmntok_t *params) +{ + struct json_stream *response; + struct wally_psbt *psbt; + + if (!param(cmd, buffer, params, + p_req("psbt", param_psbt, &psbt), + NULL)) + return command_param_failed(); + + response = json_stream_success(cmd); + json_array_start(response, "reservations"); + for (size_t i = 0; i < psbt->tx->num_inputs; i++) { + struct bitcoin_txid txid; + struct utxo *utxo; + enum output_status oldstatus; + u32 old_res; + + wally_tx_input_get_txid(&psbt->tx->inputs[i], &txid); + utxo = wallet_utxo_get(cmd, cmd->ld->wallet, + &txid, psbt->tx->inputs[i].index); + if (!utxo || utxo->status != output_state_reserved) + continue; + + oldstatus = utxo->status; + old_res = *utxo->reserved_til; + + wallet_unreserve_utxo(cmd->ld->wallet, + utxo, + get_block_height(cmd->ld->topology)); + + json_add_reservestatus(response, utxo, oldstatus, old_res, + get_block_height(cmd->ld->topology)); + } + json_array_end(response); + return command_success(cmd, response); +} + +static const struct json_command unreserveinputs_command = { + "unreserveinputs", + "bitcoin", + json_unreserveinputs, + "Unreserve utxos (or at least, reduce their reservation)", + false +}; +AUTODATA(json_command, &unreserveinputs_command); diff --git a/wallet/walletrpc.c b/wallet/walletrpc.c index 8fdaf84a740b..f03de92115a9 100644 --- a/wallet/walletrpc.c +++ b/wallet/walletrpc.c @@ -1218,45 +1218,11 @@ static const struct json_command listtransactions_command = { }; AUTODATA(json_command, &listtransactions_command); -static struct command_result *json_reserveinputs(struct command *cmd, - const char *buffer, - const jsmntok_t *obj UNNEEDED, - const jsmntok_t *params) -{ - struct command_result *res; - struct json_stream *response; - struct unreleased_tx *utx; - - u32 feerate; - - res = json_prepare_tx(cmd, buffer, params, false, &utx, &feerate); - if (res) - return res; - - /* Unlike json_txprepare, we don't keep the utx object - * around, so we remove the auto-cleanup that happens - * when the utxo objects are free'd */ - wallet_persist_utxo_reservation(cmd->ld->wallet, utx->wtx->utxos); - - response = json_stream_success(cmd); - json_add_psbt(response, "psbt", utx->tx->psbt); - json_add_u32(response, "feerate_per_kw", feerate); - return command_success(cmd, response); -} -static const struct json_command reserveinputs_command = { - "reserveinputs", - "bitcoin", - json_reserveinputs, - "Reserve inputs and pass back the resulting psbt", - false -}; -AUTODATA(json_command, &reserveinputs_command); - -static struct command_result *param_psbt(struct command *cmd, - const char *name, - const char *buffer, - const jsmntok_t *tok, - struct wally_psbt **psbt) +struct command_result *param_psbt(struct command *cmd, + const char *name, + const char *buffer, + const jsmntok_t *tok, + struct wally_psbt **psbt) { /* Pull out the token into a string, then pass to * the PSBT parser; PSBT parser can't handle streaming @@ -1265,55 +1231,12 @@ static struct command_result *param_psbt(struct command *cmd, if (psbt_from_b64(psbt_buff, psbt)) return NULL; - return command_fail(cmd, LIGHTNINGD, "'%s' should be a PSBT, not '%.*s'", + return command_fail(cmd, JSONRPC2_INVALID_PARAMS, + "'%s' should be a PSBT, not '%.*s'", name, json_tok_full_len(tok), json_tok_full(buffer, tok)); } -static struct command_result *json_unreserveinputs(struct command *cmd, - const char *buffer, - const jsmntok_t *obj UNNEEDED, - const jsmntok_t *params) -{ - struct json_stream *response; - struct wally_psbt *psbt; - - /* for each input in the psbt, attempt to 'unreserve' it */ - if (!param(cmd, buffer, params, - p_req("psbt", param_psbt, &psbt), - NULL)) - return command_param_failed(); - - response = json_stream_success(cmd); - json_array_start(response, "outputs"); - for (size_t i = 0; i < psbt->tx->num_inputs; i++) { - struct wally_tx_input *in; - struct bitcoin_txid txid; - bool unreserved; - - in = &psbt->tx->inputs[i]; - wally_tx_input_get_txid(in, &txid); - unreserved = wallet_unreserve_output(cmd->ld->wallet, - &txid, in->index); - json_object_start(response, NULL); - json_add_txid(response, "txid", &txid); - json_add_u64(response, "vout", in->index); - json_add_bool(response, "unreserved", unreserved); - json_object_end(response); - } - json_array_end(response); - - return command_success(cmd, response); -} -static const struct json_command unreserveinputs_command = { - "unreserveinputs", - "bitcoin", - json_unreserveinputs, - "Unreserve inputs, freeing them up to be reused", - false -}; -AUTODATA(json_command, &unreserveinputs_command); - static struct command_result *match_psbt_inputs_to_utxos(struct command *cmd, struct wally_psbt *psbt, struct utxo ***utxos) diff --git a/wallet/walletrpc.h b/wallet/walletrpc.h index d9717baec223..c7f8e16afba6 100644 --- a/wallet/walletrpc.h +++ b/wallet/walletrpc.h @@ -1,9 +1,12 @@ #ifndef LIGHTNING_WALLET_WALLETRPC_H #define LIGHTNING_WALLET_WALLETRPC_H #include "config.h" +#include +struct command; struct json_stream; struct utxo; +struct wally_psbt; void json_add_utxos(struct json_stream *response, struct wallet *wallet, @@ -12,4 +15,9 @@ void json_add_utxos(struct json_stream *response, /* We evaluate reserved timeouts lazily, so use this. */ bool is_reserved(const struct utxo *utxo, u32 current_height); +struct command_result *param_psbt(struct command *cmd, + const char *name, + const char *buffer, + const jsmntok_t *tok, + struct wally_psbt **psbt); #endif /* LIGHTNING_WALLET_WALLETRPC_H */ From ba86cdc39cfb8ab79f06bcf740abfc057a5e500c Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 15 Jul 2020 15:02:49 +0930 Subject: [PATCH 05/12] reserveinputs: add exclusive flag. This is the normal case: you only want to reserve inputs which are not already reserved. This saves you iterating through the results and unreserving some if you weren't exclusive. Signed-off-by: Rusty Russell --- doc/lightning-reserveinputs.7 | 9 +++++++-- doc/lightning-reserveinputs.7.md | 8 ++++++-- wallet/reservation.c | 16 ++++++++++++++-- 3 files changed, 27 insertions(+), 6 deletions(-) diff --git a/doc/lightning-reserveinputs.7 b/doc/lightning-reserveinputs.7 index 861aa11c22a0..10ee87bf6c72 100644 --- a/doc/lightning-reserveinputs.7 +++ b/doc/lightning-reserveinputs.7 @@ -3,7 +3,7 @@ lightning-reserveinputs - Construct a transaction and reserve the UTXOs it spends .SH SYNOPSIS -\fBreserveinputs\fR \fIpsbt\fR +\fBreserveinputs\fR \fIpsbt\fR [\fIexclusive\fR] .SH DESCRIPTION @@ -11,6 +11,11 @@ The \fBreserveinputs\fR RPC command places (or increases) reservations on any inputs specified in \fIpsbt\fR which are known to lightningd\. It will fail with an error it any of the inputs are known to be spent\. + +Normally the command will fail (with no reservations made) if an input +is already reserved\. If \fIexclusive\fR is set to \fIFalse\fR, then existing +reservations are simply extended, rather than causing failure\. + .SH RETURN VALUE On success, an \fIreservations\fR array is returned, with an entry for each input @@ -37,7 +42,7 @@ The following error codes may occur: .RS .IP \[bu] --32602: Invalid parameter, such as specifying a spent input in \fIpsbt\fR\. +-32602: Invalid parameter, such as specifying a spent/reserved input in \fIpsbt\fR\. .RE .SH AUTHOR diff --git a/doc/lightning-reserveinputs.7.md b/doc/lightning-reserveinputs.7.md index 0ffb0e95dc43..44664bfd17d0 100644 --- a/doc/lightning-reserveinputs.7.md +++ b/doc/lightning-reserveinputs.7.md @@ -4,7 +4,7 @@ lightning-reserveinputs -- Construct a transaction and reserve the UTXOs it spen SYNOPSIS -------- -**reserveinputs** *psbt* +**reserveinputs** *psbt* [*exclusive*] DESCRIPTION ----------- @@ -13,6 +13,10 @@ The **reserveinputs** RPC command places (or increases) reservations on any inputs specified in *psbt* which are known to lightningd. It will fail with an error it any of the inputs are known to be spent. +Normally the command will fail (with no reservations made) if an input +is already reserved. If *exclusive* is set to *False*, then existing +reservations are simply extended, rather than causing failure. + RETURN VALUE ------------ @@ -29,7 +33,7 @@ which was reserved: On failure, an error is reported and no UTXOs are reserved. The following error codes may occur: -- -32602: Invalid parameter, such as specifying a spent input in *psbt*. +- -32602: Invalid parameter, such as specifying a spent/reserved input in *psbt*. AUTHOR ------ diff --git a/wallet/reservation.c b/wallet/reservation.c index f0e96eec308d..ecb355276372 100644 --- a/wallet/reservation.c +++ b/wallet/reservation.c @@ -44,12 +44,16 @@ static struct command_result *json_reserveinputs(struct command *cmd, struct json_stream *response; struct wally_psbt *psbt; struct utxo **utxos = tal_arr(cmd, struct utxo *, 0); + bool *exclusive; + u32 current_height; if (!param(cmd, buffer, params, p_req("psbt", param_psbt, &psbt), + p_opt_def("exclusive", param_bool, &exclusive, true), NULL)) return command_param_failed(); + current_height = get_block_height(cmd->ld->topology); for (size_t i = 0; i < psbt->tx->num_inputs; i++) { struct bitcoin_txid txid; struct utxo *utxo; @@ -59,6 +63,14 @@ static struct command_result *json_reserveinputs(struct command *cmd, &txid, psbt->tx->inputs[i].index); if (!utxo) continue; + if (*exclusive && is_reserved(utxo, current_height)) { + return command_fail(cmd, JSONRPC2_INVALID_PARAMS, + "%s:%u already reserved", + type_to_string(tmpctx, + struct bitcoin_txid, + &utxo->txid), + utxo->outnum); + } if (utxo->status == output_state_spent) return command_fail(cmd, JSONRPC2_INVALID_PARAMS, "%s:%u already spent", @@ -80,7 +92,7 @@ static struct command_result *json_reserveinputs(struct command *cmd, if (!wallet_reserve_utxo(cmd->ld->wallet, utxos[i], - get_block_height(cmd->ld->topology))) { + current_height)) { fatal("Unable to reserve %s:%u!", type_to_string(tmpctx, struct bitcoin_txid, @@ -88,7 +100,7 @@ static struct command_result *json_reserveinputs(struct command *cmd, utxos[i]->outnum); } json_add_reservestatus(response, utxos[i], oldstatus, old_res, - get_block_height(cmd->ld->topology)); + current_height); } json_array_end(response); return command_success(cmd, response); From 1056ebaf2986e78607fb8fffa1586c4b36003b87 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 15 Jul 2020 15:03:49 +0930 Subject: [PATCH 06/12] pytest: test reserve and unreserve. Signed-off-by: Rusty Russell --- contrib/pyln-client/pyln/client/lightning.py | 13 +- tests/test_wallet.py | 138 ++++--------------- 2 files changed, 35 insertions(+), 116 deletions(-) diff --git a/contrib/pyln-client/pyln/client/lightning.py b/contrib/pyln-client/pyln/client/lightning.py index abba57f23fe5..43692c5a5208 100644 --- a/contrib/pyln-client/pyln/client/lightning.py +++ b/contrib/pyln-client/pyln/client/lightning.py @@ -1107,22 +1107,19 @@ def txsend(self, txid): } return self.call("txsend", payload) - def reserveinputs(self, outputs, feerate=None, minconf=None, utxos=None): + def reserveinputs(self, psbt, exclusive=True): """ - Reserve UTXOs and return a psbt for a 'stub' transaction that - spends the reserved UTXOs. + Reserve any inputs in this psbt. """ payload = { - "outputs": outputs, - "feerate": feerate, - "minconf": minconf, - "utxos": utxos, + "psbt": psbt, + "exclusive": exclusive, } return self.call("reserveinputs", payload) def unreserveinputs(self, psbt): """ - Unreserve UTXOs that were previously reserved. + Unreserve (or reduce reservation) on any UTXOs in this psbt were previously reserved. """ payload = { "psbt": psbt, diff --git a/tests/test_wallet.py b/tests/test_wallet.py index a07ec8e066f4..f12c3abbd866 100644 --- a/tests/test_wallet.py +++ b/tests/test_wallet.py @@ -438,130 +438,52 @@ def test_txprepare(node_factory, bitcoind, chainparams): def test_reserveinputs(node_factory, bitcoind, chainparams): - """ - Reserve inputs is basically the same as txprepare, with the - slight exception that 'reserveinputs' doesn't keep the - unsent transaction around - """ amount = 1000000 total_outs = 12 l1 = node_factory.get_node(feerates=(7500, 7500, 7500, 7500)) - addr = chainparams['example_addr'] + outputs = [] # Add a medley of funds to withdraw later, bech32 + p2sh-p2wpkh for i in range(total_outs // 2): - bitcoind.rpc.sendtoaddress(l1.rpc.newaddr()['bech32'], - amount / 10**8) - bitcoind.rpc.sendtoaddress(l1.rpc.newaddr('p2sh-segwit')['p2sh-segwit'], - amount / 10**8) + txid = bitcoind.rpc.sendtoaddress(l1.rpc.newaddr()['bech32'], + amount / 10**8) + outputs.append((txid, bitcoind.rpc.gettransaction(txid)['details'][0]['vout'])) + txid = bitcoind.rpc.sendtoaddress(l1.rpc.newaddr('p2sh-segwit')['p2sh-segwit'], + amount / 10**8) + outputs.append((txid, bitcoind.rpc.gettransaction(txid)['details'][0]['vout'])) bitcoind.generate_block(1) wait_for(lambda: len(l1.rpc.listfunds()['outputs']) == total_outs) - utxo_count = 8 - sent = Decimal('0.01') * (utxo_count - 1) - reserved = l1.rpc.reserveinputs(outputs=[{addr: Millisatoshi(amount * (utxo_count - 1) * 1000)}]) - assert reserved['feerate_per_kw'] == 7500 - psbt = bitcoind.rpc.decodepsbt(reserved['psbt']) - out_found = False + assert not any(o['reserved'] for o in l1.rpc.listfunds()['outputs']) - assert len(psbt['inputs']) == utxo_count - outputs = l1.rpc.listfunds()['outputs'] - assert len([x for x in outputs if not x['reserved']]) == total_outs - utxo_count - assert len([x for x in outputs if x['reserved']]) == utxo_count - total_outs -= utxo_count - saved_input = psbt['tx']['vin'][0] + # Try reserving one at a time. + for out in outputs: + psbt = bitcoind.rpc.createpsbt([{'txid': out[0], 'vout': out[1]}], []) + l1.rpc.reserveinputs(psbt) - # We should have two outputs - for vout in psbt['tx']['vout']: - if chainparams['elements'] and vout['scriptPubKey']['type'] == 'fee': - continue - if vout['scriptPubKey']['addresses'][0] == addr: - assert vout['value'] == sent - out_found = True - assert out_found - - # Do it again, but for too many inputs - utxo_count = 12 - utxo_count + 1 - sent = Decimal('0.01') * (utxo_count - 1) - with pytest.raises(RpcError, match=r"Cannot afford transaction"): - reserved = l1.rpc.reserveinputs(outputs=[{addr: Millisatoshi(amount * (utxo_count - 1) * 1000)}]) - - utxo_count -= 1 - sent = Decimal('0.01') * (utxo_count - 1) - reserved = l1.rpc.reserveinputs(outputs=[{addr: Millisatoshi(amount * (utxo_count - 1) * 1000)}], feerate='10000perkw') - - assert reserved['feerate_per_kw'] == 10000 - psbt = bitcoind.rpc.decodepsbt(reserved['psbt']) + assert all(o['reserved'] for o in l1.rpc.listfunds()['outputs']) - assert len(psbt['inputs']) == utxo_count - outputs = l1.rpc.listfunds()['outputs'] - assert len([x for x in outputs if not x['reserved']]) == total_outs - utxo_count == 0 - assert len([x for x in outputs if x['reserved']]) == 12 - - # No more available - with pytest.raises(RpcError, match=r"Cannot afford transaction"): - reserved = l1.rpc.reserveinputs(outputs=[{addr: Millisatoshi(amount * 1)}], feerate='253perkw') - - # Unreserve three, from different psbts - unreserve_utxos = [ - { - 'txid': saved_input['txid'], - 'vout': saved_input['vout'], - 'sequence': saved_input['sequence'] - }, { - 'txid': psbt['tx']['vin'][0]['txid'], - 'vout': psbt['tx']['vin'][0]['vout'], - 'sequence': psbt['tx']['vin'][0]['sequence'] - }, { - 'txid': psbt['tx']['vin'][1]['txid'], - 'vout': psbt['tx']['vin'][1]['vout'], - 'sequence': psbt['tx']['vin'][1]['sequence'] - }] - unreserve_psbt = bitcoind.rpc.createpsbt(unreserve_utxos, []) - - unreserved = l1.rpc.unreserveinputs(unreserve_psbt) - assert all([x['unreserved'] for x in unreserved['outputs']]) - outputs = l1.rpc.listfunds()['outputs'] - assert len([x for x in outputs if not x['reserved']]) == len(unreserved['outputs']) - for i in range(len(unreserved['outputs'])): - un = unreserved['outputs'][i] - u_utxo = unreserve_utxos[i] - assert un['txid'] == u_utxo['txid'] and un['vout'] == u_utxo['vout'] and un['unreserved'] - - # Try unreserving the same utxos again, plus one that's not included - # We expect this to be a no-op. - unreserve_utxos.append({'txid': 'b' * 64, 'vout': 0, 'sequence': 0}) - unreserve_psbt = bitcoind.rpc.createpsbt(unreserve_utxos, []) - unreserved = l1.rpc.unreserveinputs(unreserve_psbt) - assert not any([x['unreserved'] for x in unreserved['outputs']]) - for un in unreserved['outputs']: - assert not un['unreserved'] - assert len([x for x in l1.rpc.listfunds()['outputs'] if not x['reserved']]) == 3 - - # passing in an empty string should fail - with pytest.raises(RpcError, match=r"should be a PSBT, not "): - l1.rpc.unreserveinputs('') - - # reserve one of the utxos that we just unreserved - utxos = [] - utxos.append(saved_input['txid'] + ":" + str(saved_input['vout'])) - reserved = l1.rpc.reserveinputs([{addr: Millisatoshi(amount * 0.5 * 1000)}], feerate='253perkw', utxos=utxos) - assert len([x for x in l1.rpc.listfunds()['outputs'] if not x['reserved']]) == 2 - psbt = bitcoind.rpc.decodepsbt(reserved['psbt']) - assert len(psbt['inputs']) == 1 - vin = psbt['tx']['vin'][0] - assert vin['txid'] == saved_input['txid'] and vin['vout'] == saved_input['vout'] + # Unreserve as a batch. + psbt = bitcoind.rpc.createpsbt([{'txid': out[0], 'vout': out[1]} for out in outputs], []) + l1.rpc.unreserveinputs(psbt) + assert not any(o['reserved'] for o in l1.rpc.listfunds()['outputs']) - # reserve them all! - reserved = l1.rpc.reserveinputs([{addr: 'all'}]) - outputs = l1.rpc.listfunds()['outputs'] - assert len([x for x in outputs if not x['reserved']]) == 0 - assert len([x for x in outputs if x['reserved']]) == 12 + # Reserve twice fails unless exclusive. + l1.rpc.reserveinputs(psbt) + with pytest.raises(RpcError, match=r"already reserved"): + l1.rpc.reserveinputs(psbt) + l1.rpc.reserveinputs(psbt, False) + l1.rpc.unreserveinputs(psbt) + assert all(o['reserved'] for o in l1.rpc.listfunds()['outputs']) - # FIXME: restart the node, nothing will remain reserved + # Stays reserved across restarts. l1.restart() - assert len(l1.rpc.listfunds()['outputs']) == 12 + assert all(o['reserved'] for o in l1.rpc.listfunds()['outputs']) + + # Final unreserve works. + l1.rpc.unreserveinputs(psbt) + assert not any(o['reserved'] for o in l1.rpc.listfunds()['outputs']) @pytest.mark.xfail(strict=True) From 78fcaacc56fac29ff670b9f30739739bd8fa6b66 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 15 Jul 2020 15:04:49 +0930 Subject: [PATCH 07/12] fundpsbt: new JSON API to gather UTXOs. Technically, they could do this themselves, but it's much nicer to have one place to do it (and it makes sure we get the required information into the PSBT, which is actually not entirely accessible through listfunds, as that doesn't want to consult with the HSM for close outputs). Signed-off-by: Rusty Russell Changelog-Added: JSON RPC: new low-level coin selection `fundpsbt` routine. --- bitcoin/tx.h | 4 ++ common/utxo.c | 1 - wallet/reservation.c | 135 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 139 insertions(+), 1 deletion(-) diff --git a/bitcoin/tx.h b/bitcoin/tx.h index f62c5c0f761d..cd63dbeb72b7 100644 --- a/bitcoin/tx.h +++ b/bitcoin/tx.h @@ -12,6 +12,10 @@ #include #define BITCOIN_TX_DEFAULT_SEQUENCE 0xFFFFFFFF + +/* BIP 125: Any nsequence < 0xFFFFFFFE is replacable. + * And bitcoind uses this value. */ +#define BITCOIN_TX_RBF_SEQUENCE 0xFFFFFFFD struct wally_psbt; struct bitcoin_txid { diff --git a/common/utxo.c b/common/utxo.c index 7ff9bcfa6140..af30e993c725 100644 --- a/common/utxo.c +++ b/common/utxo.c @@ -76,7 +76,6 @@ struct bitcoin_tx *tx_spending_utxos(const tal_t *ctx, struct pubkey key; u8 *scriptSig, *scriptPubkey, *redeemscript; - assert(num_output); size_t outcount = add_change_output ? 1 + num_output : num_output; struct bitcoin_tx *tx = bitcoin_tx(ctx, chainparams, tal_count(utxos), outcount, nlocktime); diff --git a/wallet/reservation.c b/wallet/reservation.c index ecb355276372..15bacae5a24f 100644 --- a/wallet/reservation.c +++ b/wallet/reservation.c @@ -2,6 +2,7 @@ #include #include #include +#include #include #include #include @@ -164,3 +165,137 @@ static const struct json_command unreserveinputs_command = { false }; AUTODATA(json_command, &unreserveinputs_command); + + +static struct command_result *json_fundpsbt(struct command *cmd, + const char *buffer, + const jsmntok_t *obj UNNEEDED, + const jsmntok_t *params) +{ + struct json_stream *response; + const struct utxo **utxos; + u32 *feerate_per_kw; + u32 *minconf; + struct amount_sat *amount, input, needed, excess, total_fee; + bool all; + u32 locktime, maxheight; + struct bitcoin_tx *tx; + + if (!param(cmd, buffer, params, + p_req("satoshi", param_sat_or_all, &amount), + p_req("feerate", param_feerate_val, &feerate_per_kw), + p_opt_def("minconf", param_number, &minconf, 1), + NULL)) + return command_param_failed(); + + all = amount_sat_eq(*amount, AMOUNT_SAT(-1ULL)); + maxheight = minconf_to_maxheight(*minconf, cmd->ld); + + /* We keep adding until we meet their output requirements. */ + input = AMOUNT_SAT(0); + utxos = tal_arr(cmd, const struct utxo *, 0); + total_fee = AMOUNT_SAT(0); + while (amount_sat_sub(&needed, *amount, input) + && !amount_sat_eq(needed, AMOUNT_SAT(0))) { + struct utxo *utxo; + + utxo = wallet_find_utxo(utxos, cmd->ld->wallet, + cmd->ld->topology->tip->height, + &needed, + *feerate_per_kw, + maxheight, + utxos); + if (utxo) { + struct amount_sat fee; + tal_arr_expand(&utxos, utxo); + + /* It supplies more input. */ + if (!amount_sat_add(&input, input, utxo->amount)) + return command_fail(cmd, LIGHTNINGD, + "impossible UTXO value"); + + /* But increase amount needed, to pay for new input */ + fee = amount_tx_fee(*feerate_per_kw, + utxo_spend_weight(utxo)); + if (!amount_sat_add(amount, *amount, fee)) + /* Either they specified "all", or we + * will fail anyway. */ + *amount = AMOUNT_SAT(-1ULL); + if (!amount_sat_add(&total_fee, total_fee, fee)) + return command_fail(cmd, LIGHTNINGD, + "impossible fee value"); + continue; + } + + /* If they said "all", we expect to run out of utxos. */ + if (all) { + /* If we have none at all though, fail */ + if (!tal_count(utxos)) + return command_fail(cmd, FUND_CANNOT_AFFORD, + "No available UTXOs"); + break; + } + + return command_fail(cmd, FUND_CANNOT_AFFORD, + "Could not afford %s using all %zu available UTXOs: %s short", + type_to_string(tmpctx, + struct amount_sat, + amount), + tal_count(utxos), + type_to_string(tmpctx, + struct amount_sat, + &needed)); + } + + /* Setting the locktime to the next block to be mined has multiple + * benefits: + * - anti fee-snipping (even if not yet likely) + * - less distinguishable transactions (with this we create + * general-purpose transactions which looks like bitcoind: + * native segwit, nlocktime set to tip, and sequence set to + * 0xFFFFFFFD by default. Other wallets are likely to implement + * this too). + */ + locktime = cmd->ld->topology->tip->height; + + /* Eventually fuzz it too. */ + if (locktime > 100 && pseudorand(10) == 0) + locktime -= pseudorand(100); + + /* FIXME: tx_spending_utxos does more than we need, but there + * are other users right now. */ + tx = tx_spending_utxos(cmd, chainparams, utxos, + cmd->ld->wallet->bip32_base, + false, 0, locktime, + BITCOIN_TX_RBF_SEQUENCE); + + if (all) { + /* Count everything not going towards fees as excess. */ + if (!amount_sat_sub(&excess, input, total_fee)) + return command_fail(cmd, FUND_CANNOT_AFFORD, + "All %zu inputs could not afford" + " %s fees", + tal_count(utxos), + type_to_string(tmpctx, + struct amount_sat, + &total_fee)); + } else { + /* This was the condition of exiting the loop above! */ + if (!amount_sat_sub(&excess, input, *amount)) + abort(); + } + + response = json_stream_success(cmd); + json_add_psbt(response, "psbt", tx->psbt); + json_add_amount_sat_only(response, "excess_msat", excess); + return command_success(cmd, response); +} + +static const struct json_command fundpsbt_command = { + "fundpsbt", + "bitcoin", + json_fundpsbt, + "Create PSBT using enough utxos to allow an output of {satoshi} at {feerate}", + false +}; +AUTODATA(json_command, &fundpsbt_command); From 32c7dbbfbb92a131795d3cf774f72d07847c9cea Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 15 Jul 2020 15:09:41 +0930 Subject: [PATCH 08/12] fundpsbt: add reserve arg. It's easier for us to call it atomically than have the user loop and retry! Signed-off-by: Rusty Russell --- wallet/reservation.c | 72 +++++++++++++++++++++++++++----------------- 1 file changed, 44 insertions(+), 28 deletions(-) diff --git a/wallet/reservation.c b/wallet/reservation.c index 15bacae5a24f..0b8e63b8557b 100644 --- a/wallet/reservation.c +++ b/wallet/reservation.c @@ -37,6 +37,35 @@ static void json_add_reservestatus(struct json_stream *response, json_object_end(response); } +/* Reserve these UTXOs and print to JSON */ +static void reserve_and_report(struct json_stream *response, + struct wallet *wallet, + u32 current_height, + struct utxo **utxos) +{ + json_array_start(response, "reservations"); + for (size_t i = 0; i < tal_count(utxos); i++) { + enum output_status oldstatus; + u32 old_res; + + oldstatus = utxos[i]->status; + old_res = utxos[i]->reserved_til ? *utxos[i]->reserved_til : 0; + + if (!wallet_reserve_utxo(wallet, + utxos[i], + current_height)) { + fatal("Unable to reserve %s:%u!", + type_to_string(tmpctx, + struct bitcoin_txid, + &utxos[i]->txid), + utxos[i]->outnum); + } + json_add_reservestatus(response, utxos[i], oldstatus, old_res, + current_height); + } + json_array_end(response); +} + static struct command_result *json_reserveinputs(struct command *cmd, const char *buffer, const jsmntok_t *obj UNNEEDED, @@ -83,27 +112,7 @@ static struct command_result *json_reserveinputs(struct command *cmd, } response = json_stream_success(cmd); - json_array_start(response, "reservations"); - for (size_t i = 0; i < tal_count(utxos); i++) { - enum output_status oldstatus; - u32 old_res; - - oldstatus = utxos[i]->status; - old_res = utxos[i]->reserved_til ? *utxos[i]->reserved_til : 0; - - if (!wallet_reserve_utxo(cmd->ld->wallet, - utxos[i], - current_height)) { - fatal("Unable to reserve %s:%u!", - type_to_string(tmpctx, - struct bitcoin_txid, - &utxos[i]->txid), - utxos[i]->outnum); - } - json_add_reservestatus(response, utxos[i], oldstatus, old_res, - current_height); - } - json_array_end(response); + reserve_and_report(response, cmd->ld->wallet, current_height, utxos); return command_success(cmd, response); } @@ -173,27 +182,30 @@ static struct command_result *json_fundpsbt(struct command *cmd, const jsmntok_t *params) { struct json_stream *response; - const struct utxo **utxos; + struct utxo **utxos; u32 *feerate_per_kw; u32 *minconf; struct amount_sat *amount, input, needed, excess, total_fee; - bool all; - u32 locktime, maxheight; + bool all, *reserve; + u32 locktime, maxheight, current_height; struct bitcoin_tx *tx; if (!param(cmd, buffer, params, p_req("satoshi", param_sat_or_all, &amount), p_req("feerate", param_feerate_val, &feerate_per_kw), p_opt_def("minconf", param_number, &minconf, 1), + p_opt_def("reserve", param_bool, &reserve, true), NULL)) return command_param_failed(); all = amount_sat_eq(*amount, AMOUNT_SAT(-1ULL)); maxheight = minconf_to_maxheight(*minconf, cmd->ld); + current_height = get_block_height(cmd->ld->topology); + /* We keep adding until we meet their output requirements. */ input = AMOUNT_SAT(0); - utxos = tal_arr(cmd, const struct utxo *, 0); + utxos = tal_arr(cmd, struct utxo *, 0); total_fee = AMOUNT_SAT(0); while (amount_sat_sub(&needed, *amount, input) && !amount_sat_eq(needed, AMOUNT_SAT(0))) { @@ -204,7 +216,7 @@ static struct command_result *json_fundpsbt(struct command *cmd, &needed, *feerate_per_kw, maxheight, - utxos); + cast_const2(const struct utxo **, utxos)); if (utxo) { struct amount_sat fee; tal_arr_expand(&utxos, utxo); @@ -256,7 +268,7 @@ static struct command_result *json_fundpsbt(struct command *cmd, * 0xFFFFFFFD by default. Other wallets are likely to implement * this too). */ - locktime = cmd->ld->topology->tip->height; + locktime = current_height; /* Eventually fuzz it too. */ if (locktime > 100 && pseudorand(10) == 0) @@ -264,7 +276,8 @@ static struct command_result *json_fundpsbt(struct command *cmd, /* FIXME: tx_spending_utxos does more than we need, but there * are other users right now. */ - tx = tx_spending_utxos(cmd, chainparams, utxos, + tx = tx_spending_utxos(cmd, chainparams, + cast_const2(const struct utxo **, utxos), cmd->ld->wallet->bip32_base, false, 0, locktime, BITCOIN_TX_RBF_SEQUENCE); @@ -288,6 +301,9 @@ static struct command_result *json_fundpsbt(struct command *cmd, response = json_stream_success(cmd); json_add_psbt(response, "psbt", tx->psbt); json_add_amount_sat_only(response, "excess_msat", excess); + if (*reserve) + reserve_and_report(response, cmd->ld->wallet, current_height, + utxos); return command_success(cmd, response); } From 42d6013e83959c0033db6cc4bf8393e947f462f3 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 15 Jul 2020 15:09:47 +0930 Subject: [PATCH 09/12] doc: document fundpsbt. Signed-off-by: Rusty Russell --- doc/Makefile | 1 + doc/index.rst | 1 + doc/lightning-fundpsbt.7 | 76 ++++++++++++++++++++++++++++++++ doc/lightning-fundpsbt.7.md | 69 +++++++++++++++++++++++++++++ doc/lightning-reserveinputs.7 | 5 ++- doc/lightning-reserveinputs.7.md | 5 ++- 6 files changed, 153 insertions(+), 4 deletions(-) create mode 100644 doc/lightning-fundpsbt.7 create mode 100644 doc/lightning-fundpsbt.7.md diff --git a/doc/Makefile b/doc/Makefile index 840953bda205..8c02044a5ed3 100644 --- a/doc/Makefile +++ b/doc/Makefile @@ -23,6 +23,7 @@ MANPAGES := doc/lightning-cli.1 \ doc/lightning-fundchannel_start.7 \ doc/lightning-fundchannel_complete.7 \ doc/lightning-fundchannel_cancel.7 \ + doc/lightning-fundpsbt.7 \ doc/lightning-getroute.7 \ doc/lightning-getsharedsecret.7 \ doc/lightning-hsmtool.8 \ diff --git a/doc/index.rst b/doc/index.rst index 0ac83fb1516e..95c286885a98 100644 --- a/doc/index.rst +++ b/doc/index.rst @@ -45,6 +45,7 @@ c-lightning Documentation lightning-fundchannel_cancel lightning-fundchannel_complete lightning-fundchannel_start + lightning-fundpsbt lightning-getroute lightning-getsharedsecret lightning-hsmtool diff --git a/doc/lightning-fundpsbt.7 b/doc/lightning-fundpsbt.7 new file mode 100644 index 000000000000..84b98f635797 --- /dev/null +++ b/doc/lightning-fundpsbt.7 @@ -0,0 +1,76 @@ +.TH "LIGHTNING-FUNDPSBT" "7" "" "" "lightning-fundpsbt" +.SH NAME +lightning-fundpsbt - Command to populate PSBT inputs from the wallet +.SH SYNOPSIS + +\fBfundpsbt\fR \fIsatoshi\fR \fIfeerate\fR [\fIminconf\fR] [\fIreserve\fR] + +.SH DESCRIPTION + +\fBfundpsbt\fR is a low-level RPC command which creates a PSBT using unreserved +inputs in the wallet, optionally reserving them as well\. + + +\fIsatoshi\fR is the minimum satoshi value of the output(s) needed (or the +string "all" meaning use all unreserved inputs)\. If a value, it can +be a whole number, a whole number ending in \fIsat\fR, a whole number +ending in \fI000msat\fR, or a number with 1 to 8 decimal places ending in +\fIbtc\fR\. + + +You calculate the value by starting with the amount you want to pay +and adding the fee which will be needed to pay for the base of the +transaction plus that output, and any other outputs and inputs you +will add to the final transaction\. + + +\fIfeerate\fR is a number, with an optional suffix: \fIperkw\fR means the +number is interpreted as satoshi-per-kilosipa (weight), and \fIperkb\fR +means it is interpreted bitcoind-style as +satoshi-per-kilobyte\. Omitting the suffix is equivalent to \fIperkb\fR\. + + +\fIminconf\fR specifies the minimum number of confirmations that used +outputs should have\. Default is 1\. + + +\fIreserve\fR is a boolean: if true (the default), then \fIreserveinputs\fR is +called (successfully, with \fIexclusive\fR true) on the returned PSBT\. + +.SH RETURN VALUE + +On success, returns the \fIpsbt\fR containing the inputs, and +\fIexcess_msat\fR containing the amount above \fIsatoshi\fR which is +available\. This could be zero, or dust\. If \fIsatoshi\fR was "all", +then \fIexcess_msat\fR is the entire amount once fees are subtracted +for the weights of the inputs\. + + +If \fIreserve\fR was true, then a \fIreservations\fR array is returned, +exactly like \fIreserveinputs\fR\. + + +On error the returned object will contain \fBcode\fR and \fBmessage\fR properties, +with \fBcode\fR being one of the following: + +.RS +.IP \[bu] +-32602: If the given parameters are wrong\. +.IP \[bu] +-1: Catchall nonspecific error\. +.IP \[bu] +301: Insufficient UTXOs to meet \fIsatoshi\fR value\. + +.RE +.SH AUTHOR + +Rusty Russell \fI is mainly responsible\. + +.SH SEE ALSO + +\fBlightning-reserveinputs\fR(7), \fBlightning-unreserveinputs\fR(7)\. + +.SH RESOURCES + +Main web site: \fIhttps://github.com/ElementsProject/lightning\fR + diff --git a/doc/lightning-fundpsbt.7.md b/doc/lightning-fundpsbt.7.md new file mode 100644 index 000000000000..87e6d43c6ce8 --- /dev/null +++ b/doc/lightning-fundpsbt.7.md @@ -0,0 +1,69 @@ +lightning-fundpsbt -- Command to populate PSBT inputs from the wallet +================================================================ + +SYNOPSIS +-------- + +**fundpsbt** *satoshi* *feerate* \[*minconf*\] \[*reserve*\] + +DESCRIPTION +----------- + +`fundpsbt` is a low-level RPC command which creates a PSBT using unreserved +inputs in the wallet, optionally reserving them as well. + +*satoshi* is the minimum satoshi value of the output(s) needed (or the +string "all" meaning use all unreserved inputs). If a value, it can +be a whole number, a whole number ending in *sat*, a whole number +ending in *000msat*, or a number with 1 to 8 decimal places ending in +*btc*. + +You calculate the value by starting with the amount you want to pay +and adding the fee which will be needed to pay for the base of the +transaction plus that output, and any other outputs and inputs you +will add to the final transaction. + +*feerate* is a number, with an optional suffix: *perkw* means the +number is interpreted as satoshi-per-kilosipa (weight), and *perkb* +means it is interpreted bitcoind-style as +satoshi-per-kilobyte. Omitting the suffix is equivalent to *perkb*. + +*minconf* specifies the minimum number of confirmations that used +outputs should have. Default is 1. + +*reserve* is a boolean: if true (the default), then *reserveinputs* is +called (successfully, with *exclusive* true) on the returned PSBT. + +RETURN VALUE +------------ + +On success, returns the *psbt* containing the inputs, and +*excess_msat* containing the amount above *satoshi* which is +available. This could be zero, or dust. If *satoshi* was "all", +then *excess_msat* is the entire amount once fees are subtracted +for the weights of the inputs. + +If *reserve* was true, then a *reservations* array is returned, +exactly like *reserveinputs*. + +On error the returned object will contain `code` and `message` properties, +with `code` being one of the following: + +- -32602: If the given parameters are wrong. +- -1: Catchall nonspecific error. +- 301: Insufficient UTXOs to meet *satoshi* value. + +AUTHOR +------ + +Rusty Russell <> is mainly responsible. + +SEE ALSO +-------- + +lightning-reserveinputs(7), lightning-unreserveinputs(7). + +RESOURCES +--------- + +Main web site: diff --git a/doc/lightning-reserveinputs.7 b/doc/lightning-reserveinputs.7 index 10ee87bf6c72..9abad87736db 100644 --- a/doc/lightning-reserveinputs.7 +++ b/doc/lightning-reserveinputs.7 @@ -9,7 +9,8 @@ lightning-reserveinputs - Construct a transaction and reserve the UTXOs it spend The \fBreserveinputs\fR RPC command places (or increases) reservations on any inputs specified in \fIpsbt\fR which are known to lightningd\. It will fail -with an error it any of the inputs are known to be spent\. +with an error if any of the inputs are known to be spent, and ignore inputs +which are unknown\. Normally the command will fail (with no reservations made) if an input @@ -18,7 +19,7 @@ reservations are simply extended, rather than causing failure\. .SH RETURN VALUE -On success, an \fIreservations\fR array is returned, with an entry for each input +On success, a \fIreservations\fR array is returned, with an entry for each input which was reserved: .RS diff --git a/doc/lightning-reserveinputs.7.md b/doc/lightning-reserveinputs.7.md index 44664bfd17d0..00691ab24873 100644 --- a/doc/lightning-reserveinputs.7.md +++ b/doc/lightning-reserveinputs.7.md @@ -11,7 +11,8 @@ DESCRIPTION The **reserveinputs** RPC command places (or increases) reservations on any inputs specified in *psbt* which are known to lightningd. It will fail -with an error it any of the inputs are known to be spent. +with an error if any of the inputs are known to be spent, and ignore inputs +which are unknown. Normally the command will fail (with no reservations made) if an input is already reserved. If *exclusive* is set to *False*, then existing @@ -21,7 +22,7 @@ reservations are simply extended, rather than causing failure. RETURN VALUE ------------ -On success, an *reservations* array is returned, with an entry for each input +On success, a *reservations* array is returned, with an entry for each input which was reserved: - *txid* is the input transaction id. From 1ce1209f5bdeeaddf95da2135f93f82b9ca48e6f Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 15 Jul 2020 15:09:47 +0930 Subject: [PATCH 10/12] pytest: test fundpsbt. Signed-off-by: Rusty Russell --- contrib/pyln-client/pyln/client/lightning.py | 12 ++++ tests/test_wallet.py | 58 ++++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/contrib/pyln-client/pyln/client/lightning.py b/contrib/pyln-client/pyln/client/lightning.py index 43692c5a5208..d531f33f391d 100644 --- a/contrib/pyln-client/pyln/client/lightning.py +++ b/contrib/pyln-client/pyln/client/lightning.py @@ -1126,6 +1126,18 @@ def unreserveinputs(self, psbt): } return self.call("unreserveinputs", payload) + def fundpsbt(self, satoshi, feerate, minconf=None, reserve=True): + """ + Create a PSBT with inputs sufficient to give an output of satoshi. + """ + payload = { + "satoshi": satoshi, + "feerate": feerate, + "minconf": minconf, + "reserve": reserve, + } + return self.call("fundpsbt", payload) + def signpsbt(self, psbt): """ Add internal wallet's signatures to PSBT diff --git a/tests/test_wallet.py b/tests/test_wallet.py index f12c3abbd866..611a05469b1c 100644 --- a/tests/test_wallet.py +++ b/tests/test_wallet.py @@ -486,6 +486,64 @@ def test_reserveinputs(node_factory, bitcoind, chainparams): assert not any(o['reserved'] for o in l1.rpc.listfunds()['outputs']) +def test_fundpsbt(node_factory, bitcoind, chainparams): + amount = 1000000 + total_outs = 4 + l1 = node_factory.get_node() + + outputs = [] + # Add a medley of funds to withdraw later, bech32 + p2sh-p2wpkh + for i in range(total_outs // 2): + txid = bitcoind.rpc.sendtoaddress(l1.rpc.newaddr()['bech32'], + amount / 10**8) + outputs.append((txid, bitcoind.rpc.gettransaction(txid)['details'][0]['vout'])) + txid = bitcoind.rpc.sendtoaddress(l1.rpc.newaddr('p2sh-segwit')['p2sh-segwit'], + amount / 10**8) + outputs.append((txid, bitcoind.rpc.gettransaction(txid)['details'][0]['vout'])) + + bitcoind.generate_block(1) + wait_for(lambda: len(l1.rpc.listfunds()['outputs']) == total_outs) + + feerate = '7500perkw' + + # Should get one input, plus some excess + funding = l1.rpc.fundpsbt(amount // 2, feerate, reserve=False) + psbt = bitcoind.rpc.decodepsbt(funding['psbt']) + assert len(psbt['tx']['vin']) == 1 + assert funding['excess_msat'] > Millisatoshi(0) + assert funding['excess_msat'] < Millisatoshi(amount // 2 * 1000) + + # Cannot afford this one (too much) + with pytest.raises(RpcError, match=r"not afford"): + l1.rpc.fundpsbt(amount * total_outs, feerate) + + # Nor this (depth insufficient) + with pytest.raises(RpcError, match=r"not afford"): + l1.rpc.fundpsbt(amount // 2, feerate, minconf=2) + + # Should get two inputs. + psbt = bitcoind.rpc.decodepsbt(l1.rpc.fundpsbt(amount, feerate, reserve=False)['psbt']) + assert len(psbt['tx']['vin']) == 2 + + # Should not use reserved outputs. + psbt = bitcoind.rpc.createpsbt([{'txid': out[0], 'vout': out[1]} for out in outputs], []) + l1.rpc.reserveinputs(psbt) + with pytest.raises(RpcError, match=r"not afford"): + l1.rpc.fundpsbt(amount // 2, feerate) + + # Will use first one if unreserved. + l1.rpc.unreserveinputs(bitcoind.rpc.createpsbt([{'txid': outputs[0][0], 'vout': outputs[0][1]}], [])) + psbt = l1.rpc.fundpsbt(amount // 2, feerate)['psbt'] + + # Should have passed to reserveinputs. + with pytest.raises(RpcError, match=r"already reserved"): + l1.rpc.reserveinputs(psbt) + + # And now we can't afford any more. + with pytest.raises(RpcError, match=r"not afford"): + l1.rpc.fundpsbt(amount // 2, feerate) + + @pytest.mark.xfail(strict=True) def test_sign_and_send_psbt(node_factory, bitcoind, chainparams): """ From 5a93794ef72e3921712f8cbc23c8f4b643b168f1 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 15 Jul 2020 15:09:47 +0930 Subject: [PATCH 11/12] pytest: restore test_sign_and_send_psbt. It uses reservations heavily, and assumed we generated change, etc. It's now a simpler test, in many ways. Signed-off-by: Rusty Russell --- tests/test_wallet.py | 85 +++++++++++++++++++++++--------------------- 1 file changed, 44 insertions(+), 41 deletions(-) diff --git a/tests/test_wallet.py b/tests/test_wallet.py index 611a05469b1c..8fabd83b025e 100644 --- a/tests/test_wallet.py +++ b/tests/test_wallet.py @@ -544,7 +544,6 @@ def test_fundpsbt(node_factory, bitcoind, chainparams): l1.rpc.fundpsbt(amount // 2, feerate) -@pytest.mark.xfail(strict=True) def test_sign_and_send_psbt(node_factory, bitcoind, chainparams): """ Tests for the sign + send psbt RPCs @@ -566,53 +565,58 @@ def test_sign_and_send_psbt(node_factory, bitcoind, chainparams): bitcoind.generate_block(1) wait_for(lambda: len(l1.rpc.listfunds()['outputs']) == total_outs) - # Make a PSBT out of our inputs - reserved = l1.rpc.reserveinputs(outputs=[{addr: Millisatoshi(3 * amount * 1000)}]) + # Make a PSBT out of our inputs (FIXME: satoshi amount should include fees!) + funding = l1.rpc.fundpsbt(satoshi=Millisatoshi(3 * amount * 1000), + feerate=7500, + reserve=True) assert len([x for x in l1.rpc.listfunds()['outputs'] if x['reserved']]) == 4 - psbt = bitcoind.rpc.decodepsbt(reserved['psbt']) + psbt = bitcoind.rpc.decodepsbt(funding['psbt']) saved_input = psbt['tx']['vin'][0] # Go ahead and unreserve the UTXOs, we'll use a smaller # set of them to create a second PSBT that we'll attempt to sign # and broadcast (to disastrous results) - l1.rpc.unreserveinputs(reserved['psbt']) + l1.rpc.unreserveinputs(funding['psbt']) # Re-reserve one of the utxos we just unreserved - utxos = [] - utxos.append(saved_input['txid'] + ":" + str(saved_input['vout'])) - second_reservation = l1.rpc.reserveinputs([{addr: Millisatoshi(amount * 0.5 * 1000)}], feerate='253perkw', utxos=utxos) + psbt = bitcoind.rpc.createpsbt([{'txid': saved_input['txid'], + 'vout': saved_input['vout']}], []) + l1.rpc.reserveinputs(psbt) # We require the utxos be reserved before signing them with pytest.raises(RpcError, match=r"Aborting PSBT signing. UTXO .* is not reserved"): - l1.rpc.signpsbt(reserved['psbt'])['signed_psbt'] + l1.rpc.signpsbt(funding['psbt'])['signed_psbt'] # Now we unreserve the singleton, so we can reserve it again - l1.rpc.unreserveinputs(second_reservation['psbt']) + l1.rpc.unreserveinputs(psbt) + + # Now add an output. + output_pbst = bitcoind.rpc.createpsbt([], + [{addr: 3 * amount / 10**8}]) + fullpsbt = bitcoind.rpc.joinpsbts([funding['psbt'], output_pbst]) # We re-reserve the first set... - utxos = [] - for vin in psbt['tx']['vin']: - utxos.append(vin['txid'] + ':' + str(vin['vout'])) - reserved = l1.rpc.reserveinputs(outputs=[{addr: Millisatoshi(3 * amount * 1000)}], utxos=utxos) + l1.rpc.reserveinputs(fullpsbt) + # Sign + send the PSBT we've created - signed_psbt = l1.rpc.signpsbt(reserved['psbt'])['signed_psbt'] + signed_psbt = l1.rpc.signpsbt(fullpsbt)['signed_psbt'] broadcast_tx = l1.rpc.sendpsbt(signed_psbt) # Check that it was broadcast successfully l1.daemon.wait_for_log(r'sendrawtx exit 0 .* sendrawtransaction {}'.format(broadcast_tx['tx'])) bitcoind.generate_block(1) - # We expect a change output to be added to the wallet - expected_outs = total_outs - 4 + 1 + # We didn't add a change output. + expected_outs = total_outs - 4 wait_for(lambda: len(l1.rpc.listfunds()['outputs']) == expected_outs) # Let's try *sending* a PSBT that can't be finalized (it's unsigned) with pytest.raises(RpcError, match=r"PSBT not finalizeable"): - l1.rpc.sendpsbt(second_reservation['psbt']) + l1.rpc.sendpsbt(fullpsbt) # Now we try signing a PSBT with an output that's already been spent - with pytest.raises(RpcError, match=r"Aborting PSBT signing. UTXO {} is not reserved".format(utxos[0])): - l1.rpc.signpsbt(second_reservation['psbt']) + with pytest.raises(RpcError, match=r"Aborting PSBT signing. UTXO .* is not reserved"): + l1.rpc.signpsbt(fullpsbt) # Queue up another node, to make some PSBTs for us for i in range(total_outs // 2): @@ -623,15 +627,22 @@ def test_sign_and_send_psbt(node_factory, bitcoind, chainparams): # Create a PSBT using L2 bitcoind.generate_block(1) wait_for(lambda: len(l2.rpc.listfunds()['outputs']) == total_outs) - l2_reserved = l2.rpc.reserveinputs(outputs=[{addr: Millisatoshi(3 * amount * 1000)}]) + l2_funding = l2.rpc.fundpsbt(satoshi=Millisatoshi(3 * amount * 1000), + feerate=7500, + reserve=True) # Try to get L1 to sign it with pytest.raises(RpcError, match=r"No wallet inputs to sign"): - l1.rpc.signpsbt(l2_reserved['psbt']) + l1.rpc.signpsbt(l2_funding['psbt']) # Add some of our own PSBT inputs to it - l1_reserved = l1.rpc.reserveinputs(outputs=[{addr: Millisatoshi(3 * amount * 1000)}]) - joint_psbt = bitcoind.rpc.joinpsbts([l1_reserved['psbt'], l2_reserved['psbt']]) + l1_funding = l1.rpc.fundpsbt(satoshi=Millisatoshi(3 * amount * 1000), + feerate=7500, + reserve=True) + + # Join and add an output + joint_psbt = bitcoind.rpc.joinpsbts([l1_funding['psbt'], l2_funding['psbt'], + output_pbst]) half_signed_psbt = l1.rpc.signpsbt(joint_psbt)['signed_psbt'] totally_signed = l2.rpc.signpsbt(half_signed_psbt)['signed_psbt'] @@ -640,8 +651,11 @@ def test_sign_and_send_psbt(node_factory, bitcoind, chainparams): l1.daemon.wait_for_log(r'sendrawtx exit 0 .* sendrawtransaction {}'.format(broadcast_tx['tx'])) # Send a PSBT that's not ours - l2_reserved = l2.rpc.reserveinputs(outputs=[{addr: Millisatoshi(3 * amount * 1000)}]) - l2_signed_psbt = l2.rpc.signpsbt(l2_reserved['psbt'])['signed_psbt'] + l2_funding = l2.rpc.fundpsbt(satoshi=Millisatoshi(3 * amount * 1000), + feerate=7500, + reserve=True) + psbt = bitcoind.rpc.joinpsbts([l2_funding['psbt'], output_pbst]) + l2_signed_psbt = l2.rpc.signpsbt(psbt)['signed_psbt'] l1.rpc.sendpsbt(l2_signed_psbt) # Re-try sending the same tx? @@ -658,7 +672,7 @@ def test_sign_and_send_psbt(node_factory, bitcoind, chainparams): l1.rpc.sendpsbt('') # Try a modified (invalid) PSBT string - modded_psbt = l2_reserved['psbt'][:-3] + 'A' + l2_reserved['psbt'][-3:] + modded_psbt = psbt[:-3] + 'A' + psbt[-3:] with pytest.raises(RpcError, match=r"should be a PSBT, not"): l1.rpc.signpsbt(modded_psbt) @@ -679,27 +693,16 @@ def test_sign_and_send_psbt(node_factory, bitcoind, chainparams): {'type': 'chain_mvt', 'credit': 0, 'debit': 0, 'tag': 'spend_track'}, {'type': 'chain_mvt', 'credit': 0, 'debit': 0, 'tag': 'spend_track'}, {'type': 'chain_mvt', 'credit': 0, 'debit': 0, 'tag': 'spend_track'}, - # Nicely splits out withdrawals and chain fees, because it's all our tx - {'type': 'chain_mvt', 'credit': 0, 'debit': 988255000, 'tag': 'withdrawal'}, {'type': 'chain_mvt', 'credit': 0, 'debit': 3000000000, 'tag': 'withdrawal'}, - {'type': 'chain_mvt', 'credit': 0, 'debit': 11745000, 'tag': 'chain_fees'}, - {'type': 'chain_mvt', 'credit': 988255000, 'debit': 0, 'tag': 'deposit'}, + {'type': 'chain_mvt', 'credit': 0, 'debit': 1000000000, 'tag': 'chain_fees'}, {'type': 'chain_mvt', 'credit': 0, 'debit': 0, 'tag': 'spend_track'}, {'type': 'chain_mvt', 'credit': 0, 'debit': 0, 'tag': 'spend_track'}, {'type': 'chain_mvt', 'credit': 0, 'debit': 0, 'tag': 'spend_track'}, {'type': 'chain_mvt', 'credit': 0, 'debit': 0, 'tag': 'spend_track'}, - # Note that this is technically wrong since we paid 11745sat in fees - # but since it includes inputs / outputs from a second node, we can't - # do proper acccounting for it. - {'type': 'chain_mvt', 'credit': 0, 'debit': 4000000000, 'tag': 'withdrawal'}, - {'type': 'chain_mvt', 'credit': 0, 'debit': 0, 'tag': 'chain_fees'}, + {'type': 'chain_mvt', 'credit': 0, 'debit': 3000000000, 'tag': 'withdrawal'}, + {'type': 'chain_mvt', 'credit': 0, 'debit': 1000000000, 'tag': 'chain_fees'}, ] - if chainparams['elements']: - wallet_coin_mvts.append({'type': 'chain_mvt', 'credit': 984625000, 'debit': 0, 'tag': 'deposit'}) - else: - wallet_coin_mvts.append({'type': 'chain_mvt', 'credit': 988285000, 'debit': 0, 'tag': 'deposit'}) - check_coin_moves(l1, 'wallet', wallet_coin_mvts, chainparams) From d1627391ceb99d9ec678cb33718420954cbdb3ee Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 15 Jul 2020 15:10:08 +0930 Subject: [PATCH 12/12] txprepare: revert 1fb9a078b620a65a2802bc8b1303970914cfb4cb (`psbt` field) We're actually going to deprecate this, so don't add new features! Signed-off-by: Rusty Russell Changelog-Added: ***REMOVE*** JSON-API: `txprepare` returns a psbt version of the created transaction --- tests/test_wallet.py | 1 - wallet/walletrpc.c | 1 - 2 files changed, 2 deletions(-) diff --git a/tests/test_wallet.py b/tests/test_wallet.py index 8fabd83b025e..36b61c70b3d8 100644 --- a/tests/test_wallet.py +++ b/tests/test_wallet.py @@ -307,7 +307,6 @@ def test_txprepare(node_factory, bitcoind, chainparams): wait_for(lambda: len(l1.rpc.listfunds()['outputs']) == 10) prep = l1.rpc.txprepare(outputs=[{addr: Millisatoshi(amount * 3 * 1000)}]) - assert prep['psbt'] decode = bitcoind.rpc.decoderawtransaction(prep['unsigned_tx']) assert decode['txid'] == prep['txid'] # 4 inputs, 2 outputs (3 if we have a fee output). diff --git a/wallet/walletrpc.c b/wallet/walletrpc.c index f03de92115a9..04a4da283b72 100644 --- a/wallet/walletrpc.c +++ b/wallet/walletrpc.c @@ -490,7 +490,6 @@ 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->psbt); return command_success(cmd, response); } static const struct json_command txprepare_command = {