From da84c863059e1c04a0cf90aa487b2d57e6d21a08 Mon Sep 17 00:00:00 2001 From: niftynei Date: Tue, 9 Jun 2020 18:44:19 -0500 Subject: [PATCH 1/8] prepare-tx: pass back the feerate, as json_tx_prepare sometimes sets it Unused here, but we'll use it in the next commit so that we can always pass back the effective / used feerate to the caller of `reserveinputs` This makes opening a channel much easier if we've internally determined the feerate --- wallet/walletrpc.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/wallet/walletrpc.c b/wallet/walletrpc.c index 0df56f861a20..4afc14b0340e 100644 --- a/wallet/walletrpc.c +++ b/wallet/walletrpc.c @@ -140,8 +140,9 @@ static struct command_result *broadcast_and_wait(struct command *cmd, static struct command_result *json_prepare_tx(struct command *cmd, const char *buffer, const jsmntok_t *params, + bool for_withdraw, struct unreleased_tx **utx, - bool for_withdraw) + u32 *feerate) { u32 *feerate_per_kw = NULL; struct command_result *result; @@ -430,6 +431,8 @@ static struct command_result *json_prepare_tx(struct command *cmd, bitcoin_txid((*utx)->tx, &(*utx)->txid); + if (feerate) + *feerate = *feerate_per_kw; return NULL; } @@ -442,7 +445,7 @@ static struct command_result *json_txprepare(struct command *cmd, struct command_result *res; struct json_stream *response; - res = json_prepare_tx(cmd, buffer, params, &utx, false); + res = json_prepare_tx(cmd, buffer, params, false, &utx, NULL); if (res) return res; @@ -569,7 +572,7 @@ static struct command_result *json_withdraw(struct command *cmd, struct unreleased_tx *utx; struct command_result *res; - res = json_prepare_tx(cmd, buffer, params, &utx, true); + res = json_prepare_tx(cmd, buffer, params, true, &utx, NULL); if (res) return res; From b705d75f372efe05a49a3b41262ff12ce11099ed Mon Sep 17 00:00:00 2001 From: niftynei Date: Tue, 9 Jun 2020 18:41:51 -0500 Subject: [PATCH 2/8] reserve/unreserve input: new RPC commands for reserving inputs/outputs Reserve and unreserve wallet UTXOs using a PSBT which includes those inputs. Note that currently we unreserve inputs everytime the node restarts. This will be addressed in a future commit. Changelog-Added: JSON-RPC: Adds two new rpc methods, `reserveinputs` and `unreserveinputs`, which allow for reserving or unreserving wallet UTXOs --- contrib/pyln-client/pyln/client/lightning.py | 22 ++++ tests/test_wallet.py | 125 +++++++++++++++++++ wallet/wallet.c | 14 +++ wallet/wallet.h | 14 +++ wallet/walletrpc.c | 100 +++++++++++++++ 5 files changed, 275 insertions(+) diff --git a/contrib/pyln-client/pyln/client/lightning.py b/contrib/pyln-client/pyln/client/lightning.py index a56612c7ecf7..f374eafbcc37 100644 --- a/contrib/pyln-client/pyln/client/lightning.py +++ b/contrib/pyln-client/pyln/client/lightning.py @@ -1107,6 +1107,28 @@ def txsend(self, txid): } return self.call("txsend", payload) + def reserveinputs(self, outputs, feerate=None, minconf=None, utxos=None): + """ + Reserve UTXOs and return a psbt for a 'stub' transaction that + spends the reserved UTXOs. + """ + payload = { + "outputs": outputs, + "feerate": feerate, + "minconf": minconf, + "utxos": utxos, + } + return self.call("reserveinputs", payload) + + def unreserveinputs(self, psbt): + """ + Unreserve UTXOs that were previously reserved. + """ + payload = { + "psbt": psbt, + } + return self.call("unreserveinputs", payload) + def signmessage(self, message): """ Sign a message with this node's secret key. diff --git a/tests/test_wallet.py b/tests/test_wallet.py index e3a982b17402..a60677a64069 100644 --- a/tests/test_wallet.py +++ b/tests/test_wallet.py @@ -436,6 +436,131 @@ def test_txprepare(node_factory, bitcoind, chainparams): assert decode['vout'][changenum]['scriptPubKey']['type'] == 'witness_v0_keyhash' +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'] + + # 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) + + 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 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] + + # We should have two outputs + for vout in psbt['tx']['vout']: + 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 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 unreserved['all_unreserved'] + 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 unreserved['all_unreserved'] + 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'] + + # 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 + + # FIXME: restart the node, nothing will remain reserved + l1.restart() + assert len(l1.rpc.listfunds()['outputs']) == 12 + + def test_txsend(node_factory, bitcoind, chainparams): amount = 1000000 l1 = node_factory.get_node(random_hsm=True) diff --git a/wallet/wallet.c b/wallet/wallet.c index 1a5ce0677b46..a64f787e5af8 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -355,6 +355,15 @@ struct utxo *wallet_utxo_get(const tal_t *ctx, struct wallet *w, return utxo; } +bool wallet_unreserve_output(struct wallet *w, + const struct bitcoin_txid *txid, + const u32 outnum) +{ + return wallet_update_output_status(w, txid, outnum, + output_state_reserved, + output_state_available); +} + /** * unreserve_utxo - Mark a reserved UTXO as available again */ @@ -376,6 +385,11 @@ static void destroy_utxos(const struct utxo **utxos, struct wallet *w) unreserve_utxo(w, utxos[i]); } +void wallet_persist_utxo_reservation(struct wallet *w, const struct utxo **utxos) +{ + tal_del_destructor2(utxos, destroy_utxos, w); +} + void wallet_confirm_utxos(struct wallet *w, const struct utxo **utxos) { tal_del_destructor2(utxos, destroy_utxos, w); diff --git a/wallet/wallet.h b/wallet/wallet.h index 0f0d132e0552..8d326ddb272d 100644 --- a/wallet/wallet.h +++ b/wallet/wallet.h @@ -1246,6 +1246,20 @@ void add_unreleased_tx(struct wallet *w, struct unreleased_tx *utx); /* These will touch the db, so need to be explicitly freed. */ void free_unreleased_txs(struct wallet *w); +/* wallet_persist_utxo_reservation - Removes destructor + * + * Persists the reservation in the database (until a restart) + * instead of clearing the reservation when the utxo object + * is destroyed */ +void wallet_persist_utxo_reservation(struct wallet *w, const struct utxo **utxos); + +/* wallet_unreserve_output - Unreserve a utxo + * + * We unreserve utxos so that they can be spent elsewhere. + * */ +bool wallet_unreserve_output(struct wallet *w, + const struct bitcoin_txid *txid, + const u32 outnum); /** * Get a list of transactions that we track in the wallet. * diff --git a/wallet/walletrpc.c b/wallet/walletrpc.c index 4afc14b0340e..0612fd35881e 100644 --- a/wallet/walletrpc.c +++ b/wallet/walletrpc.c @@ -1158,3 +1158,103 @@ static const struct json_command listtransactions_command = { "it closes the channel and returns funds to the wallet." }; 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) +{ + /* Pull out the token into a string, then pass to + * the PSBT parser; PSBT parser can't handle streaming + * atm as it doesn't accept a len value */ + char *psbt_buff = json_strdup(cmd, buffer, tok); + if (psbt_from_b64(psbt_buff, psbt)) + return NULL; + + return command_fail(cmd, LIGHTNINGD, "'%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; + bool all_unreserved; + + /* 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); + all_unreserved = psbt->tx->num_inputs != 0; + 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); + all_unreserved &= unreserved; + } + json_array_end(response); + + json_add_bool(response, "all_unreserved", all_unreserved); + 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); From bec3fdc9b3fad85f217e4c3ceb96b1b962dce1aa Mon Sep 17 00:00:00 2001 From: niftynei Date: Tue, 9 Jun 2020 19:20:18 -0500 Subject: [PATCH 3/8] add manpages for reserve/unreserve --- doc/Makefile | 4 +- doc/index.rst | 2 + doc/lightning-reserveinputs.7 | 85 ++++++++++++++++++++++++++++++ doc/lightning-reserveinputs.7.md | 76 ++++++++++++++++++++++++++ doc/lightning-unreserveinputs.7 | 48 +++++++++++++++++ doc/lightning-unreserveinputs.7.md | 45 ++++++++++++++++ 6 files changed, 259 insertions(+), 1 deletion(-) create mode 100644 doc/lightning-reserveinputs.7 create mode 100644 doc/lightning-reserveinputs.7.md create mode 100644 doc/lightning-unreserveinputs.7 create mode 100644 doc/lightning-unreserveinputs.7.md diff --git a/doc/Makefile b/doc/Makefile index 2f2c4c59cef1..8672e74e4f65 100644 --- a/doc/Makefile +++ b/doc/Makefile @@ -35,6 +35,7 @@ MANPAGES := doc/lightning-cli.1 \ doc/lightning-newaddr.7 \ doc/lightning-pay.7 \ doc/lightning-plugin.7 \ + doc/lightning-reserveinputs.7 \ doc/lightning-sendonion.7 \ doc/lightning-sendpay.7 \ doc/lightning-setchannelfee.7 \ @@ -42,11 +43,12 @@ MANPAGES := doc/lightning-cli.1 \ doc/lightning-txprepare.7 \ doc/lightning-txdiscard.7 \ doc/lightning-txsend.7 \ + doc/lightning-unreserveinputs.7 \ doc/lightning-waitinvoice.7 \ doc/lightning-waitanyinvoice.7 \ doc/lightning-waitblockheight.7 \ doc/lightning-waitsendpay.7 \ - doc/lightning-withdraw.7 + doc/lightning-withdraw.7 doc-all: $(MANPAGES) doc/index.rst diff --git a/doc/index.rst b/doc/index.rst index 20533eab703b..27896f95538e 100644 --- a/doc/index.rst +++ b/doc/index.rst @@ -57,6 +57,7 @@ c-lightning Documentation lightning-newaddr lightning-pay lightning-plugin + lightning-reserveinputs lightning-sendonion lightning-sendpay lightning-setchannelfee @@ -64,6 +65,7 @@ c-lightning Documentation lightning-txdiscard lightning-txprepare lightning-txsend + lightning-unreserveinputs lightning-waitanyinvoice lightning-waitblockheight lightning-waitinvoice diff --git a/doc/lightning-reserveinputs.7 b/doc/lightning-reserveinputs.7 new file mode 100644 index 000000000000..06db8c23a33f --- /dev/null +++ b/doc/lightning-reserveinputs.7 @@ -0,0 +1,85 @@ +.TH "LIGHTNING-RESERVEINPUTS" "7" "" "" "lightning-reserveinputs" +.SH NAME +lightning-reserveinputs - Construct a transaction and reserve the UTXOs it spends +.SH SYNOPSIS + +\fBreserveinputs\fR \fIoutputs\fR [\fIfeerate\fR] [\fIminconf\fR] [\fIutxos\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\. + +.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 failure, an error is reported and no UTXOs are reserved\. + + +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\. + +.RE +.SH AUTHOR + +niftynei \fI is mainly responsible\. + +.SH SEE ALSO + +\fBlightning-unreserveinputs\fR(7), \fBlightning-signpsbt\fR(7), \fBlightning-sendpsbt\fR(7) + +.SH RESOURCES + +Main web site: \fIhttps://github.com/ElementsProject/lightning\fR + diff --git a/doc/lightning-reserveinputs.7.md b/doc/lightning-reserveinputs.7.md new file mode 100644 index 000000000000..0ee15a25a6b2 --- /dev/null +++ b/doc/lightning-reserveinputs.7.md @@ -0,0 +1,76 @@ +lightning-reserveinputs -- Construct a transaction and reserve the UTXOs it spends +================================================================================== + +SYNOPSIS +-------- + +**reserveinputs** *outputs* \[*feerate*\] \[*minconf*\] \[*utxos*\] + +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. + + +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 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. + +AUTHOR +------ + +niftynei <> is mainly responsible. + +SEE ALSO +-------- + +lightning-unreserveinputs(7), lightning-signpsbt(7), lightning-sendpsbt(7) + +RESOURCES +--------- + +Main web site: diff --git a/doc/lightning-unreserveinputs.7 b/doc/lightning-unreserveinputs.7 new file mode 100644 index 000000000000..037bf816bae2 --- /dev/null +++ b/doc/lightning-unreserveinputs.7 @@ -0,0 +1,48 @@ +.TH "LIGHTNING-UNRESERVEINPUTS" "7" "" "" "lightning-unreserveinputs" +.SH NAME +lightning-unreserveinputs - Release reserved UTXOs +.SH SYNOPSIS + +\fBunreserveinputs\fR \fIpsbt\fR + +.SH DESCRIPTION + +The \fBunreserveinputs\fR RPC command releases 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\. + + +\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\. + + +Note that restarting lightningd will unreserve all UTXOs by default\. + + +The following error codes may occur: + +.RS +.IP \[bu] +-1: An unparseable PSBT\. + +.RE +.SH AUTHOR + +niftynei \fI is mainly responsible\. + +.SH SEE ALSO + +\fBlightning-unreserveinputs\fR(7), \fBlightning-signpsbt\fR(7), \fBlightning-sendpsbt\fR(7) + +.SH RESOURCES + +Main web site: \fIhttps://github.com/ElementsProject/lightning\fR + diff --git a/doc/lightning-unreserveinputs.7.md b/doc/lightning-unreserveinputs.7.md new file mode 100644 index 000000000000..b431751d880c --- /dev/null +++ b/doc/lightning-unreserveinputs.7.md @@ -0,0 +1,45 @@ +lightning-unreserveinputs -- Release reserved UTXOs +=================================================== + +SYNOPSIS +-------- + +**unreserveinputs** *psbt* + +DESCRIPTION +----------- + +The **unreserveinputs** RPC command releases 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. + +*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. + +Note that restarting lightningd will unreserve all UTXOs by default. + +The following error codes may occur: +- -1: An unparseable PSBT. + +AUTHOR +------ + +niftynei <> is mainly responsible. + +SEE ALSO +-------- + +lightning-unreserveinputs(7), lightning-signpsbt(7), lightning-sendpsbt(7) + +RESOURCES +--------- + +Main web site: From 73041d4707696fee6d9e6cedb36e8ca5da4810b2 Mon Sep 17 00:00:00 2001 From: niftynei Date: Tue, 16 Jun 2020 13:40:13 -0500 Subject: [PATCH 4/8] tx: add helper for extracting script from a wally_tx the bitcoin_tx version is basically a wrapper for the wally_tx script extraction -- here we pull it apart so we can easily get a tal'd script for a wally_tx_output --- bitcoin/tx.c | 22 +++++++++++++--------- bitcoin/tx.h | 9 +++++++++ 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/bitcoin/tx.c b/bitcoin/tx.c index ca0a159734df..57cc593fd18b 100644 --- a/bitcoin/tx.c +++ b/bitcoin/tx.c @@ -265,22 +265,26 @@ void bitcoin_tx_output_set_amount(struct bitcoin_tx *tx, int outnum, } } -const u8 *bitcoin_tx_output_get_script(const tal_t *ctx, - const struct bitcoin_tx *tx, int outnum) +const u8 *wally_tx_output_get_script(const tal_t *ctx, + const struct wally_tx_output *output) { - const struct wally_tx_output *output; - u8 *res; - assert(outnum < tx->wtx->num_outputs); - output = &tx->wtx->outputs[outnum]; - if (output->script == NULL) { /* This can happen for coinbase transactions and pegin * transactions */ return NULL; } - res = tal_dup_arr(ctx, u8, output->script, output->script_len, 0); - return res; + return tal_dup_arr(ctx, u8, output->script, output->script_len, 0); +} + +const u8 *bitcoin_tx_output_get_script(const tal_t *ctx, + const struct bitcoin_tx *tx, int outnum) +{ + const struct wally_tx_output *output; + assert(outnum < tx->wtx->num_outputs); + output = &tx->wtx->outputs[outnum]; + + return wally_tx_output_get_script(ctx, output); } u8 *bitcoin_tx_output_get_witscript(const tal_t *ctx, const struct bitcoin_tx *tx, diff --git a/bitcoin/tx.h b/bitcoin/tx.h index 982f6ff22a92..bdc2928e0474 100644 --- a/bitcoin/tx.h +++ b/bitcoin/tx.h @@ -126,6 +126,15 @@ void bitcoin_tx_output_set_amount(struct bitcoin_tx *tx, int outnum, */ const u8 *bitcoin_tx_output_get_script(const tal_t *ctx, const struct bitcoin_tx *tx, int outnum); +/** + * Helper to get the script of a script's output as a tal_arr + * + * The script attached to a `wally_tx_output` is not a `tal_arr`, so in order to keep the + * comfort of being able to call `tal_bytelen` and similar on a script we just + * return a `tal_arr` clone of the original script. + */ +const u8 *wally_tx_output_get_script(const tal_t *ctx, + const struct wally_tx_output *output); /** * Helper to get a witness script for an output. */ From c57377588079327ec0484ca756bc7b3a6755ef80 Mon Sep 17 00:00:00 2001 From: niftynei Date: Tue, 16 Jun 2020 13:43:51 -0500 Subject: [PATCH 5/8] wallet: have wallet_extract_outputs take wally_tx, not bitcoin_tx With the incursion of PSBTs, we're moving away from bitcoin_tx --- lightningd/chaintopology.c | 2 +- wallet/wallet.c | 14 ++++++++------ wallet/wallet.h | 2 +- wallet/walletrpc.c | 2 +- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/lightningd/chaintopology.c b/lightningd/chaintopology.c index 0c874ee1b1e0..a2e60a8e6e72 100644 --- a/lightningd/chaintopology.c +++ b/lightningd/chaintopology.c @@ -92,7 +92,7 @@ static void filter_block_txs(struct chain_topology *topo, struct block *b) bitcoin_txid(tx, &txid); if (txfilter_match(topo->bitcoind->ld->owned_txfilter, tx)) { wallet_extract_owned_outputs(topo->bitcoind->ld->wallet, - tx, &b->height, &owned); + tx->wtx, &b->height, &owned); wallet_transaction_add(topo->ld->wallet, tx, b->height, i); } diff --git a/wallet/wallet.c b/wallet/wallet.c index a64f787e5af8..bba6704b18a9 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -1664,25 +1664,27 @@ void wallet_confirm_tx(struct wallet *w, db_exec_prepared_v2(take(stmt)); } -int wallet_extract_owned_outputs(struct wallet *w, const struct bitcoin_tx *tx, +int wallet_extract_owned_outputs(struct wallet *w, const struct wally_tx *wtx, const u32 *blockheight, struct amount_sat *total) { int num_utxos = 0; *total = AMOUNT_SAT(0); - for (size_t output = 0; output < tx->wtx->num_outputs; output++) { + for (size_t output = 0; output < wtx->num_outputs; output++) { struct utxo *utxo; u32 index; bool is_p2sh; const u8 *script; - struct amount_asset asset = bitcoin_tx_output_get_amount(tx, output); + struct amount_asset asset = + wally_tx_output_get_amount(&wtx->outputs[output]); struct chain_coin_mvt *mvt; if (!amount_asset_is_main(&asset)) continue; - script = bitcoin_tx_output_get_script(tmpctx, tx, output); + script = wally_tx_output_get_script(tmpctx, + &wtx->outputs[output]); if (!script) continue; @@ -1694,7 +1696,7 @@ int wallet_extract_owned_outputs(struct wallet *w, const struct bitcoin_tx *tx, utxo->is_p2sh = is_p2sh; utxo->amount = amount_asset_to_sat(&asset); utxo->status = output_state_available; - bitcoin_txid(tx, &utxo->txid); + wally_txid(wtx, &utxo->txid); utxo->outnum = output; utxo->close_info = NULL; @@ -1738,7 +1740,7 @@ int wallet_extract_owned_outputs(struct wallet *w, const struct bitcoin_tx *tx, if (!amount_sat_add(total, *total, utxo->amount)) fatal("Cannot add utxo output %zu/%zu %s + %s", - output, tx->wtx->num_outputs, + output, wtx->num_outputs, type_to_string(tmpctx, struct amount_sat, total), type_to_string(tmpctx, struct amount_sat, &utxo->amount)); diff --git a/wallet/wallet.h b/wallet/wallet.h index 8d326ddb272d..8213184ee820 100644 --- a/wallet/wallet.h +++ b/wallet/wallet.h @@ -560,7 +560,7 @@ void wallet_blocks_heights(struct wallet *w, u32 def, u32 *min, u32 *max); /** * wallet_extract_owned_outputs - given a tx, extract all of our outputs */ -int wallet_extract_owned_outputs(struct wallet *w, const struct bitcoin_tx *tx, +int wallet_extract_owned_outputs(struct wallet *w, const struct wally_tx *tx, const u32 *blockheight, struct amount_sat *total); diff --git a/wallet/walletrpc.c b/wallet/walletrpc.c index 0612fd35881e..a6fc4180c12b 100644 --- a/wallet/walletrpc.c +++ b/wallet/walletrpc.c @@ -59,7 +59,7 @@ static void wallet_withdrawal_broadcast(struct bitcoind *bitcoind UNUSED, wallet_confirm_utxos(ld->wallet, utx->wtx->utxos); /* Extract the change output and add it to the DB */ - wallet_extract_owned_outputs(ld->wallet, utx->tx, NULL, &change); + wallet_extract_owned_outputs(ld->wallet, utx->tx->wtx, NULL, &change); /* Note normally, change_satoshi == withdraw->wtx->change, but * not if we're actually making a payment to ourselves! */ From ea50e5de8750355849901363e856d93324abb7b4 Mon Sep 17 00:00:00 2001 From: niftynei Date: Tue, 16 Jun 2020 13:47:07 -0500 Subject: [PATCH 6/8] rpc: new signpsbt + sendpsbt rpcs Changelog-Added: JSON-RPC: new call `signpsbt` which will add the wallet's signatures to a provided psbt Changelog-Added: JSON-RPC: new call `sendpsbt` which will finalize and send a signed PSBT --- contrib/pyln-client/pyln/client/lightning.py | 18 ++ tests/test_wallet.py | 121 +++++++++++- wallet/walletrpc.c | 192 +++++++++++++++++-- 3 files changed, 316 insertions(+), 15 deletions(-) diff --git a/contrib/pyln-client/pyln/client/lightning.py b/contrib/pyln-client/pyln/client/lightning.py index f374eafbcc37..abba57f23fe5 100644 --- a/contrib/pyln-client/pyln/client/lightning.py +++ b/contrib/pyln-client/pyln/client/lightning.py @@ -1129,6 +1129,24 @@ def unreserveinputs(self, psbt): } return self.call("unreserveinputs", payload) + def signpsbt(self, psbt): + """ + Add internal wallet's signatures to PSBT + """ + payload = { + "psbt": psbt, + } + return self.call("signpsbt", payload) + + def sendpsbt(self, psbt): + """ + Finalize extract and broadcast a PSBT + """ + payload = { + "psbt": psbt, + } + return self.call("sendpsbt", payload) + def signmessage(self, message): """ Sign a message with this node's secret key. diff --git a/tests/test_wallet.py b/tests/test_wallet.py index a60677a64069..56e9bc1ac07c 100644 --- a/tests/test_wallet.py +++ b/tests/test_wallet.py @@ -1,3 +1,4 @@ +from bitcoin.rpc import JSONRPCError from decimal import Decimal from fixtures import * # noqa: F401,F403 from fixtures import TEST_NETWORK @@ -518,7 +519,7 @@ def test_reserveinputs(node_factory, bitcoind, chainparams): unreserve_psbt = bitcoind.rpc.createpsbt(unreserve_utxos, []) unreserved = l1.rpc.unreserveinputs(unreserve_psbt) - assert unreserved['all_unreserved'] + 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'])): @@ -531,7 +532,7 @@ def test_reserveinputs(node_factory, bitcoind, chainparams): 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 unreserved['all_unreserved'] + 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 @@ -561,6 +562,122 @@ def test_reserveinputs(node_factory, bitcoind, chainparams): assert len(l1.rpc.listfunds()['outputs']) == 12 +def test_sign_and_send_psbt(node_factory, bitcoind, chainparams): + """ + Tests for the sign + send psbt RPCs + """ + amount = 1000000 + total_outs = 12 + l1 = node_factory.get_node(feerates=(7500, 7500, 7500, 7500)) + l2 = node_factory.get_node() + addr = chainparams['example_addr'] + + # 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) + 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)}]) + assert len([x for x in l1.rpc.listfunds()['outputs'] if x['reserved']]) == 4 + psbt = bitcoind.rpc.decodepsbt(reserved['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']) + + # 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) + + # 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'] + + # Now we unreserve the singleton, so we can reserve it again + l1.rpc.unreserveinputs(second_reservation['psbt']) + + # 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) + # Sign + send the PSBT we've created + signed_psbt = l1.rpc.signpsbt(reserved['psbt'])['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 + 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']) + + # 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']) + + # Queue up another node, to make some PSBTs for us + for i in range(total_outs // 2): + bitcoind.rpc.sendtoaddress(l2.rpc.newaddr()['bech32'], + amount / 10**8) + bitcoind.rpc.sendtoaddress(l2.rpc.newaddr('p2sh-segwit')['p2sh-segwit'], + amount / 10**8) + # 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)}]) + + # Try to get L1 to sign it + with pytest.raises(RpcError, match=r"No wallet inputs to sign"): + l1.rpc.signpsbt(l2_reserved['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']]) + + half_signed_psbt = l1.rpc.signpsbt(joint_psbt)['signed_psbt'] + totally_signed = l2.rpc.signpsbt(half_signed_psbt)['signed_psbt'] + + broadcast_tx = l1.rpc.sendpsbt(totally_signed) + 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'] + l1.rpc.sendpsbt(l2_signed_psbt) + + # Re-try sending the same tx? + bitcoind.generate_block(1) + sync_blockheight(bitcoind, [l1]) + # Expect an error here + with pytest.raises(JSONRPCError, match=r"Transaction already in block chain"): + bitcoind.rpc.sendrawtransaction(broadcast_tx['tx']) + + # Try an empty PSBT + with pytest.raises(RpcError, match=r"should be a PSBT, not"): + l1.rpc.signpsbt('') + with pytest.raises(RpcError, match=r"should be a PSBT, not"): + l1.rpc.sendpsbt('') + + # Try a modified (invalid) PSBT string + modded_psbt = l2_reserved['psbt'][:-3] + 'A' + l2_reserved['psbt'][-3:] + with pytest.raises(RpcError, match=r"should be a PSBT, not"): + l1.rpc.signpsbt(modded_psbt) + + def test_txsend(node_factory, bitcoind, chainparams): amount = 1000000 l1 = node_factory.get_node(random_hsm=True) diff --git a/wallet/walletrpc.c b/wallet/walletrpc.c index a6fc4180c12b..ff40dd9c78c6 100644 --- a/wallet/walletrpc.c +++ b/wallet/walletrpc.c @@ -34,6 +34,28 @@ #include #include +struct tx_broadcast { + struct command *cmd; + const struct utxo **utxos; + const struct wally_tx *wtx; + struct amount_sat *expected_change; +}; + +static struct tx_broadcast *unreleased_tx_to_broadcast(const tal_t *ctx, + struct command *cmd, + struct unreleased_tx *utx) +{ + struct tx_broadcast *txb = tal(ctx, struct tx_broadcast); + struct amount_sat *change = tal(txb, struct amount_sat); + + txb->cmd = cmd; + txb->utxos = utx->wtx->utxos; + txb->wtx = utx->tx->wtx; + *change = utx->wtx->change; + txb->expected_change = change; + return txb; +} + /** * wallet_withdrawal_broadcast - The tx has been broadcast (or it failed) * @@ -45,29 +67,34 @@ */ static void wallet_withdrawal_broadcast(struct bitcoind *bitcoind UNUSED, bool success, const char *msg, - struct unreleased_tx *utx) + struct tx_broadcast *txb) { - struct command *cmd = utx->wtx->cmd; + struct command *cmd = txb->cmd; struct lightningd *ld = cmd->ld; - struct amount_sat change = AMOUNT_SAT(0); /* FIXME: This won't be necessary once we use ccan/json_out! */ /* Massage output into shape so it doesn't kill the JSON serialization */ char *output = tal_strjoin(cmd, tal_strsplit(cmd, msg, "\n", STR_NO_EMPTY), " ", STR_NO_TRAIL); if (success) { + struct bitcoin_txid txid; + struct amount_sat change = AMOUNT_SAT(0); + /* Mark used outputs as spent */ - wallet_confirm_utxos(ld->wallet, utx->wtx->utxos); + wallet_confirm_utxos(ld->wallet, txb->utxos); /* Extract the change output and add it to the DB */ - wallet_extract_owned_outputs(ld->wallet, utx->tx->wtx, NULL, &change); + wallet_extract_owned_outputs(ld->wallet, txb->wtx, NULL, &change); /* Note normally, change_satoshi == withdraw->wtx->change, but * not if we're actually making a payment to ourselves! */ - assert(amount_sat_greater_eq(change, utx->wtx->change)); + if (txb->expected_change) + assert(amount_sat_greater_eq(change, *txb->expected_change)); struct json_stream *response = json_stream_success(cmd); - json_add_tx(response, "tx", utx->tx); - json_add_txid(response, "txid", &utx->txid); + wally_txid(txb->wtx, &txid); + json_add_hex_talarr(response, "tx", + linearize_wtx(tmpctx, txb->wtx)); + json_add_txid(response, "txid", &txid); was_pending(command_success(cmd, response)); } else { was_pending(command_fail(cmd, LIGHTNINGD, @@ -127,7 +154,8 @@ static struct command_result *broadcast_and_wait(struct command *cmd, /* Now broadcast the transaction */ bitcoind_sendrawtx(cmd->ld->topology->bitcoind, tal_hex(tmpctx, linearize_tx(tmpctx, utx->tx)), - wallet_withdrawal_broadcast, utx); + wallet_withdrawal_broadcast, + unreleased_tx_to_broadcast(cmd, cmd, utx)); return command_still_pending(cmd); } @@ -1218,7 +1246,6 @@ static struct command_result *json_unreserveinputs(struct command *cmd, { struct json_stream *response; struct wally_psbt *psbt; - bool all_unreserved; /* for each input in the psbt, attempt to 'unreserve' it */ if (!param(cmd, buffer, params, @@ -1227,7 +1254,6 @@ static struct command_result *json_unreserveinputs(struct command *cmd, return command_param_failed(); response = json_stream_success(cmd); - all_unreserved = psbt->tx->num_inputs != 0; json_array_start(response, "outputs"); for (size_t i = 0; i < psbt->tx->num_inputs; i++) { struct wally_tx_input *in; @@ -1243,11 +1269,9 @@ static struct command_result *json_unreserveinputs(struct command *cmd, json_add_u64(response, "vout", in->index); json_add_bool(response, "unreserved", unreserved); json_object_end(response); - all_unreserved &= unreserved; } json_array_end(response); - json_add_bool(response, "all_unreserved", all_unreserved); return command_success(cmd, response); } static const struct json_command unreserveinputs_command = { @@ -1258,3 +1282,145 @@ static const struct json_command unreserveinputs_command = { 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) +{ + *utxos = tal_arr(cmd, struct utxo *, 0); + for (size_t i = 0; i < psbt->tx->num_inputs; i++) { + struct utxo *utxo; + struct bitcoin_txid txid; + + wally_tx_input_get_txid(&psbt->tx->inputs[i], &txid); + utxo = wallet_utxo_get(*utxos, cmd->ld->wallet, + &txid, psbt->tx->inputs[i].index); + 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) + return command_fail(cmd, LIGHTNINGD, + "Aborting PSBT signing. UTXO %s:%u is not reserved", + type_to_string(tmpctx, struct bitcoin_txid, + &utxo->txid), + utxo->outnum); + tal_arr_expand(utxos, utxo); + } + + return NULL; +} + +static struct command_result *json_signpsbt(struct command *cmd, + const char *buffer, + const jsmntok_t *obj UNNEEDED, + const jsmntok_t *params) +{ + struct command_result *res; + struct json_stream *response; + struct wally_psbt *psbt, *signed_psbt; + struct utxo **utxos; + + if (!param(cmd, buffer, params, + p_req("psbt", param_psbt, &psbt), + NULL)) + return command_param_failed(); + + /* We have to find/locate the utxos that are ours on this PSBT, + * so that the HSM knows how/what to sign for (it's possible some of + * our utxos require more complicated data to sign for e.g. + * closeinfo outputs */ + res = match_psbt_inputs_to_utxos(cmd, psbt, &utxos); + if (res) + return res; + + if (tal_count(utxos) == 0) + return command_fail(cmd, LIGHTNINGD, + "No wallet inputs to sign"); + + /* FIXME: hsm will sign almost anything, but it should really + * fail cleanly (not abort!) and let us report the error here. */ + u8 *msg = towire_hsm_sign_withdrawal(cmd, + cast_const2(const struct utxo **, utxos), + psbt); + + if (!wire_sync_write(cmd->ld->hsm_fd, take(msg))) + fatal("Could not write sign_withdrawal to HSM: %s", + strerror(errno)); + + msg = wire_sync_read(cmd, cmd->ld->hsm_fd); + + if (!fromwire_hsm_sign_withdrawal_reply(cmd, msg, &signed_psbt)) + fatal("HSM gave bad sign_withdrawal_reply %s", + tal_hex(tmpctx, msg)); + + response = json_stream_success(cmd); + json_add_psbt(response, "signed_psbt", signed_psbt); + return command_success(cmd, response); +} + +static const struct json_command signpsbt_command = { + "signpsbt", + "bitcoin", + json_signpsbt, + "Sign this wallet's inputs on a provided PSBT.", + false +}; + +AUTODATA(json_command, &signpsbt_command); + +static struct command_result *json_sendpsbt(struct command *cmd, + const char *buffer, + const jsmntok_t *obj UNNEEDED, + const jsmntok_t *params) +{ + struct command_result *res; + struct wally_psbt *psbt; + struct wally_tx *w_tx; + struct tx_broadcast *txb; + struct utxo **utxos; + + if (!param(cmd, buffer, params, + p_req("psbt", param_psbt, &psbt), + NULL)) + return command_param_failed(); + + w_tx = psbt_finalize(psbt, true); + if (!w_tx) + return command_fail(cmd, LIGHTNINGD, + "PSBT not finalizeable %s", + type_to_string(tmpctx, struct wally_psbt, + psbt)); + + /* We have to find/locate the utxos that are ours on this PSBT, + * so that we know who to mark as used. + */ + res = match_psbt_inputs_to_utxos(cmd, psbt, &utxos); + if (res) + return res; + + txb = tal(cmd, struct tx_broadcast); + txb->utxos = cast_const2(const struct utxo **, + tal_steal(txb, utxos)); + txb->wtx = tal_steal(txb, w_tx); + txb->cmd = cmd; + txb->expected_change = NULL; + + /* Now broadcast the transaction */ + bitcoind_sendrawtx(cmd->ld->topology->bitcoind, + tal_hex(tmpctx, linearize_wtx(tmpctx, w_tx)), + wallet_withdrawal_broadcast, txb); + + return command_still_pending(cmd); +} + +static const struct json_command sendpsbt_command = { + "sendpsbt", + "bitcoin", + json_sendpsbt, + "Finalize, extract and send a PSBT.", + false +}; + +AUTODATA(json_command, &sendpsbt_command); From 6b8b2d12647dd713716622af758e4f0efa953879 Mon Sep 17 00:00:00 2001 From: niftynei Date: Tue, 16 Jun 2020 17:07:28 -0500 Subject: [PATCH 7/8] tx: fix case where input amounts are less than total outputs when attempting to calculate the fees for a tx where we don't own all of the outputs, we can overshoot the feerate --- bitcoin/tx.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bitcoin/tx.c b/bitcoin/tx.c index 57cc593fd18b..2056bc195880 100644 --- a/bitcoin/tx.c +++ b/bitcoin/tx.c @@ -121,7 +121,9 @@ struct amount_sat bitcoin_tx_compute_fee_w_inputs(const struct bitcoin_tx *tx, ok = amount_sat_sub(&input_val, input_val, amount_asset_to_sat(&asset)); - assert(ok); + if (!ok) + return AMOUNT_SAT(0); + } return input_val; } From e101bc88f539bfcfc6fe2d428002bbe4a822f673 Mon Sep 17 00:00:00 2001 From: niftynei Date: Tue, 16 Jun 2020 17:09:21 -0500 Subject: [PATCH 8/8] coin_moves: update withdrawal logic to account for 'variable owner' txs Our existing coin_moves tracking logic assumed that any tx we had an input in belonged to *all* of our wallet (not a bad assumption as long as there was no way to update a tx that spends our wallets) Now that we've got `signpsbt` implemented, however, we need to be careful about how we account for withdrawals. For now we do a best guess at what the feerate is, and lump all of our spent outputs as a 'withdrawal' when it's impossible to disambiguate --- bitcoin/test/run-bitcoin_block_from_hex.c | 3 + bitcoin/test/run-tx-encode.c | 3 + cli/test/run-large-input.c | 3 + cli/test/run-remove-hint.c | 3 + common/test/run-bigsize.c | 3 + common/test/run-cryptomsg.c | 3 + common/test/run-derive_basepoints.c | 3 + common/test/run-features.c | 3 + common/test/run-gossip_rcvd_filter.c | 3 + common/test/run-ip_port_parsing.c | 3 + common/test/run-json_remove.c | 3 + common/test/run-key_derive.c | 3 + common/test/run-lock.c | 3 + common/test/run-softref.c | 3 + common/test/run-sphinx.c | 3 + connectd/test/run-initiator-success.c | 3 + connectd/test/run-responder-success.c | 3 + lightningd/chaintopology.c | 106 +++++++++++++--------- tests/test_wallet.py | 41 ++++++++- 19 files changed, 152 insertions(+), 46 deletions(-) diff --git a/bitcoin/test/run-bitcoin_block_from_hex.c b/bitcoin/test/run-bitcoin_block_from_hex.c index 6cf84bc2fcc5..52d13fa32c23 100644 --- a/bitcoin/test/run-bitcoin_block_from_hex.c +++ b/bitcoin/test/run-bitcoin_block_from_hex.c @@ -22,6 +22,9 @@ struct amount_sat amount_asset_to_sat(struct amount_asset *asset UNNEEDED) /* Generated stub for amount_sat_eq */ bool amount_sat_eq(struct amount_sat a UNNEEDED, struct amount_sat b UNNEEDED) { fprintf(stderr, "amount_sat_eq called!\n"); abort(); } +/* Generated stub for amount_sat_less */ +bool amount_sat_less(struct amount_sat a UNNEEDED, struct amount_sat b UNNEEDED) +{ fprintf(stderr, "amount_sat_less called!\n"); abort(); } /* Generated stub for amount_sat_sub */ bool amount_sat_sub(struct amount_sat *val UNNEEDED, struct amount_sat a UNNEEDED, diff --git a/bitcoin/test/run-tx-encode.c b/bitcoin/test/run-tx-encode.c index 6bc269a1d029..30238ef7e94f 100644 --- a/bitcoin/test/run-tx-encode.c +++ b/bitcoin/test/run-tx-encode.c @@ -23,6 +23,9 @@ struct amount_sat amount_asset_to_sat(struct amount_asset *asset UNNEEDED) /* Generated stub for amount_sat_eq */ bool amount_sat_eq(struct amount_sat a UNNEEDED, struct amount_sat b UNNEEDED) { fprintf(stderr, "amount_sat_eq called!\n"); abort(); } +/* Generated stub for amount_sat_less */ +bool amount_sat_less(struct amount_sat a UNNEEDED, struct amount_sat b UNNEEDED) +{ fprintf(stderr, "amount_sat_less called!\n"); abort(); } /* Generated stub for amount_sat_sub */ bool amount_sat_sub(struct amount_sat *val UNNEEDED, struct amount_sat a UNNEEDED, diff --git a/cli/test/run-large-input.c b/cli/test/run-large-input.c index cee090de43e7..2d4d01cf8689 100644 --- a/cli/test/run-large-input.c +++ b/cli/test/run-large-input.c @@ -47,6 +47,9 @@ struct amount_sat amount_asset_to_sat(struct amount_asset *asset UNNEEDED) /* Generated stub for amount_sat_eq */ bool amount_sat_eq(struct amount_sat a UNNEEDED, struct amount_sat b UNNEEDED) { fprintf(stderr, "amount_sat_eq called!\n"); abort(); } +/* Generated stub for amount_sat_less */ +bool amount_sat_less(struct amount_sat a UNNEEDED, struct amount_sat b UNNEEDED) +{ fprintf(stderr, "amount_sat_less called!\n"); abort(); } /* Generated stub for amount_sat_sub */ bool amount_sat_sub(struct amount_sat *val UNNEEDED, struct amount_sat a UNNEEDED, diff --git a/cli/test/run-remove-hint.c b/cli/test/run-remove-hint.c index f621fea31847..0207dc5f2f44 100644 --- a/cli/test/run-remove-hint.c +++ b/cli/test/run-remove-hint.c @@ -50,6 +50,9 @@ struct amount_sat amount_asset_to_sat(struct amount_asset *asset UNNEEDED) /* Generated stub for amount_sat_eq */ bool amount_sat_eq(struct amount_sat a UNNEEDED, struct amount_sat b UNNEEDED) { fprintf(stderr, "amount_sat_eq called!\n"); abort(); } +/* Generated stub for amount_sat_less */ +bool amount_sat_less(struct amount_sat a UNNEEDED, struct amount_sat b UNNEEDED) +{ fprintf(stderr, "amount_sat_less called!\n"); abort(); } /* Generated stub for amount_sat_sub */ bool amount_sat_sub(struct amount_sat *val UNNEEDED, struct amount_sat a UNNEEDED, diff --git a/common/test/run-bigsize.c b/common/test/run-bigsize.c index f9e44f8b3311..33b3640d2650 100644 --- a/common/test/run-bigsize.c +++ b/common/test/run-bigsize.c @@ -27,6 +27,9 @@ struct amount_sat amount_asset_to_sat(struct amount_asset *asset UNNEEDED) /* Generated stub for amount_sat_eq */ bool amount_sat_eq(struct amount_sat a UNNEEDED, struct amount_sat b UNNEEDED) { fprintf(stderr, "amount_sat_eq called!\n"); abort(); } +/* Generated stub for amount_sat_less */ +bool amount_sat_less(struct amount_sat a UNNEEDED, struct amount_sat b UNNEEDED) +{ fprintf(stderr, "amount_sat_less called!\n"); abort(); } /* Generated stub for amount_sat_sub */ bool amount_sat_sub(struct amount_sat *val UNNEEDED, struct amount_sat a UNNEEDED, diff --git a/common/test/run-cryptomsg.c b/common/test/run-cryptomsg.c index dced33197d97..9278076e56a5 100644 --- a/common/test/run-cryptomsg.c +++ b/common/test/run-cryptomsg.c @@ -22,6 +22,9 @@ struct amount_sat amount_asset_to_sat(struct amount_asset *asset UNNEEDED) /* Generated stub for amount_sat_eq */ bool amount_sat_eq(struct amount_sat a UNNEEDED, struct amount_sat b UNNEEDED) { fprintf(stderr, "amount_sat_eq called!\n"); abort(); } +/* Generated stub for amount_sat_less */ +bool amount_sat_less(struct amount_sat a UNNEEDED, struct amount_sat b UNNEEDED) +{ fprintf(stderr, "amount_sat_less called!\n"); abort(); } /* Generated stub for amount_sat_sub */ bool amount_sat_sub(struct amount_sat *val UNNEEDED, struct amount_sat a UNNEEDED, diff --git a/common/test/run-derive_basepoints.c b/common/test/run-derive_basepoints.c index 572bdc5c7294..733c284142e5 100644 --- a/common/test/run-derive_basepoints.c +++ b/common/test/run-derive_basepoints.c @@ -23,6 +23,9 @@ struct amount_sat amount_asset_to_sat(struct amount_asset *asset UNNEEDED) /* Generated stub for amount_sat_eq */ bool amount_sat_eq(struct amount_sat a UNNEEDED, struct amount_sat b UNNEEDED) { fprintf(stderr, "amount_sat_eq called!\n"); abort(); } +/* Generated stub for amount_sat_less */ +bool amount_sat_less(struct amount_sat a UNNEEDED, struct amount_sat b UNNEEDED) +{ fprintf(stderr, "amount_sat_less called!\n"); abort(); } /* Generated stub for amount_sat_sub */ bool amount_sat_sub(struct amount_sat *val UNNEEDED, struct amount_sat a UNNEEDED, diff --git a/common/test/run-features.c b/common/test/run-features.c index 5e7ea09cb11e..ef0f3f28cb5a 100644 --- a/common/test/run-features.c +++ b/common/test/run-features.c @@ -22,6 +22,9 @@ struct amount_sat amount_asset_to_sat(struct amount_asset *asset UNNEEDED) /* Generated stub for amount_sat_eq */ bool amount_sat_eq(struct amount_sat a UNNEEDED, struct amount_sat b UNNEEDED) { fprintf(stderr, "amount_sat_eq called!\n"); abort(); } +/* Generated stub for amount_sat_less */ +bool amount_sat_less(struct amount_sat a UNNEEDED, struct amount_sat b UNNEEDED) +{ fprintf(stderr, "amount_sat_less called!\n"); abort(); } /* Generated stub for amount_sat_sub */ bool amount_sat_sub(struct amount_sat *val UNNEEDED, struct amount_sat a UNNEEDED, diff --git a/common/test/run-gossip_rcvd_filter.c b/common/test/run-gossip_rcvd_filter.c index 7afc47b1dd8c..531ea29a0f27 100644 --- a/common/test/run-gossip_rcvd_filter.c +++ b/common/test/run-gossip_rcvd_filter.c @@ -19,6 +19,9 @@ struct amount_sat amount_asset_to_sat(struct amount_asset *asset UNNEEDED) /* Generated stub for amount_sat_eq */ bool amount_sat_eq(struct amount_sat a UNNEEDED, struct amount_sat b UNNEEDED) { fprintf(stderr, "amount_sat_eq called!\n"); abort(); } +/* Generated stub for amount_sat_less */ +bool amount_sat_less(struct amount_sat a UNNEEDED, struct amount_sat b UNNEEDED) +{ fprintf(stderr, "amount_sat_less called!\n"); abort(); } /* Generated stub for amount_sat_sub */ bool amount_sat_sub(struct amount_sat *val UNNEEDED, struct amount_sat a UNNEEDED, diff --git a/common/test/run-ip_port_parsing.c b/common/test/run-ip_port_parsing.c index 0a7df6bc6e1c..5439ddca1beb 100644 --- a/common/test/run-ip_port_parsing.c +++ b/common/test/run-ip_port_parsing.c @@ -21,6 +21,9 @@ struct amount_sat amount_asset_to_sat(struct amount_asset *asset UNNEEDED) /* Generated stub for amount_sat_eq */ bool amount_sat_eq(struct amount_sat a UNNEEDED, struct amount_sat b UNNEEDED) { fprintf(stderr, "amount_sat_eq called!\n"); abort(); } +/* Generated stub for amount_sat_less */ +bool amount_sat_less(struct amount_sat a UNNEEDED, struct amount_sat b UNNEEDED) +{ fprintf(stderr, "amount_sat_less called!\n"); abort(); } /* Generated stub for amount_sat_sub */ bool amount_sat_sub(struct amount_sat *val UNNEEDED, struct amount_sat a UNNEEDED, diff --git a/common/test/run-json_remove.c b/common/test/run-json_remove.c index 572274783953..4c318216aa2f 100644 --- a/common/test/run-json_remove.c +++ b/common/test/run-json_remove.c @@ -19,6 +19,9 @@ struct amount_sat amount_asset_to_sat(struct amount_asset *asset UNNEEDED) /* Generated stub for amount_sat_eq */ bool amount_sat_eq(struct amount_sat a UNNEEDED, struct amount_sat b UNNEEDED) { fprintf(stderr, "amount_sat_eq called!\n"); abort(); } +/* Generated stub for amount_sat_less */ +bool amount_sat_less(struct amount_sat a UNNEEDED, struct amount_sat b UNNEEDED) +{ fprintf(stderr, "amount_sat_less called!\n"); abort(); } /* Generated stub for amount_sat_sub */ bool amount_sat_sub(struct amount_sat *val UNNEEDED, struct amount_sat a UNNEEDED, diff --git a/common/test/run-key_derive.c b/common/test/run-key_derive.c index f48e58cbcb13..76a7e3068181 100644 --- a/common/test/run-key_derive.c +++ b/common/test/run-key_derive.c @@ -24,6 +24,9 @@ struct amount_sat amount_asset_to_sat(struct amount_asset *asset UNNEEDED) /* Generated stub for amount_sat_eq */ bool amount_sat_eq(struct amount_sat a UNNEEDED, struct amount_sat b UNNEEDED) { fprintf(stderr, "amount_sat_eq called!\n"); abort(); } +/* Generated stub for amount_sat_less */ +bool amount_sat_less(struct amount_sat a UNNEEDED, struct amount_sat b UNNEEDED) +{ fprintf(stderr, "amount_sat_less called!\n"); abort(); } /* Generated stub for amount_sat_sub */ bool amount_sat_sub(struct amount_sat *val UNNEEDED, struct amount_sat a UNNEEDED, diff --git a/common/test/run-lock.c b/common/test/run-lock.c index 6b32552967ad..d29f323e36b3 100644 --- a/common/test/run-lock.c +++ b/common/test/run-lock.c @@ -23,6 +23,9 @@ struct amount_sat amount_asset_to_sat(struct amount_asset *asset UNNEEDED) /* Generated stub for amount_sat_eq */ bool amount_sat_eq(struct amount_sat a UNNEEDED, struct amount_sat b UNNEEDED) { fprintf(stderr, "amount_sat_eq called!\n"); abort(); } +/* Generated stub for amount_sat_less */ +bool amount_sat_less(struct amount_sat a UNNEEDED, struct amount_sat b UNNEEDED) +{ fprintf(stderr, "amount_sat_less called!\n"); abort(); } /* Generated stub for amount_sat_sub */ bool amount_sat_sub(struct amount_sat *val UNNEEDED, struct amount_sat a UNNEEDED, diff --git a/common/test/run-softref.c b/common/test/run-softref.c index 731485a67db0..d9f7c381ee71 100644 --- a/common/test/run-softref.c +++ b/common/test/run-softref.c @@ -20,6 +20,9 @@ struct amount_sat amount_asset_to_sat(struct amount_asset *asset UNNEEDED) /* Generated stub for amount_sat_eq */ bool amount_sat_eq(struct amount_sat a UNNEEDED, struct amount_sat b UNNEEDED) { fprintf(stderr, "amount_sat_eq called!\n"); abort(); } +/* Generated stub for amount_sat_less */ +bool amount_sat_less(struct amount_sat a UNNEEDED, struct amount_sat b UNNEEDED) +{ fprintf(stderr, "amount_sat_less called!\n"); abort(); } /* Generated stub for amount_sat_sub */ bool amount_sat_sub(struct amount_sat *val UNNEEDED, struct amount_sat a UNNEEDED, diff --git a/common/test/run-sphinx.c b/common/test/run-sphinx.c index bf4ea50015d0..a31e55a31f12 100644 --- a/common/test/run-sphinx.c +++ b/common/test/run-sphinx.c @@ -36,6 +36,9 @@ void amount_msat_from_u64(struct amount_msat *msat UNNEEDED, u64 millisatoshis U /* Generated stub for amount_sat_eq */ bool amount_sat_eq(struct amount_sat a UNNEEDED, struct amount_sat b UNNEEDED) { fprintf(stderr, "amount_sat_eq called!\n"); abort(); } +/* Generated stub for amount_sat_less */ +bool amount_sat_less(struct amount_sat a UNNEEDED, struct amount_sat b UNNEEDED) +{ fprintf(stderr, "amount_sat_less called!\n"); abort(); } /* Generated stub for amount_sat_sub */ bool amount_sat_sub(struct amount_sat *val UNNEEDED, struct amount_sat a UNNEEDED, diff --git a/connectd/test/run-initiator-success.c b/connectd/test/run-initiator-success.c index 4f2d27c77e32..953a65b193a7 100644 --- a/connectd/test/run-initiator-success.c +++ b/connectd/test/run-initiator-success.c @@ -26,6 +26,9 @@ struct amount_sat amount_asset_to_sat(struct amount_asset *asset UNNEEDED) /* Generated stub for amount_sat_eq */ bool amount_sat_eq(struct amount_sat a UNNEEDED, struct amount_sat b UNNEEDED) { fprintf(stderr, "amount_sat_eq called!\n"); abort(); } +/* Generated stub for amount_sat_less */ +bool amount_sat_less(struct amount_sat a UNNEEDED, struct amount_sat b UNNEEDED) +{ fprintf(stderr, "amount_sat_less called!\n"); abort(); } /* Generated stub for amount_sat_sub */ bool amount_sat_sub(struct amount_sat *val UNNEEDED, struct amount_sat a UNNEEDED, diff --git a/connectd/test/run-responder-success.c b/connectd/test/run-responder-success.c index f93d6ecc80c9..29fdab58aa17 100644 --- a/connectd/test/run-responder-success.c +++ b/connectd/test/run-responder-success.c @@ -26,6 +26,9 @@ struct amount_sat amount_asset_to_sat(struct amount_asset *asset UNNEEDED) /* Generated stub for amount_sat_eq */ bool amount_sat_eq(struct amount_sat a UNNEEDED, struct amount_sat b UNNEEDED) { fprintf(stderr, "amount_sat_eq called!\n"); abort(); } +/* Generated stub for amount_sat_less */ +bool amount_sat_less(struct amount_sat a UNNEEDED, struct amount_sat b UNNEEDED) +{ fprintf(stderr, "amount_sat_less called!\n"); abort(); } /* Generated stub for amount_sat_sub */ bool amount_sat_sub(struct amount_sat *val UNNEEDED, struct amount_sat a UNNEEDED, diff --git a/lightningd/chaintopology.c b/lightningd/chaintopology.c index a2e60a8e6e72..fa32bc410ec7 100644 --- a/lightningd/chaintopology.c +++ b/lightningd/chaintopology.c @@ -662,22 +662,24 @@ static void updates_complete(struct chain_topology *topo) next_topology_timer(topo); } -static void record_output_spend(struct lightningd *ld, - const struct bitcoin_txid *txid, - const struct bitcoin_txid *utxo_txid, - u32 vout, u32 blockheight, - struct amount_sat *input_amt) +static void record_utxo_spent(struct lightningd *ld, + const struct bitcoin_txid *txid, + const struct bitcoin_txid *utxo_txid, + u32 vout, u32 blockheight, + struct amount_sat *input_amt) { struct utxo *utxo; struct chain_coin_mvt *mvt; u8 *ctx = tal(NULL, u8); utxo = wallet_utxo_get(ctx, ld->wallet, utxo_txid, vout); - if (!utxo) + if (!utxo) { log_broken(ld->log, "No record of utxo %s:%d", type_to_string(tmpctx, struct bitcoin_txid, utxo_txid), vout); + return; + } *input_amt = utxo->amount; mvt = new_coin_spend_track(ctx, txid, utxo_txid, vout, blockheight); @@ -685,23 +687,14 @@ static void record_output_spend(struct lightningd *ld, tal_free(ctx); } -static void record_tx_outs_and_fees(struct lightningd *ld, const struct bitcoin_tx *tx, - struct bitcoin_txid *txid, u32 blockheight, - struct amount_sat inputs_total) +static void record_outputs_as_withdraws(const tal_t *ctx, + struct lightningd *ld, + const struct bitcoin_tx *tx, + struct bitcoin_txid *txid, + u32 blockheight) { - struct amount_sat fee; struct chain_coin_mvt *mvt; - size_t i; - u8 *ctx = tal(NULL, u8); - - if (!tx) - log_broken(ld->log, "We have no record of transaction %s", - type_to_string(ctx, struct bitcoin_txid, txid)); - - /* We record every output on this transaction as a withdraw */ - /* FIXME: collaborative tx will need to keep track of which - * outputs are ours */ - for (i = 0; i < tx->wtx->num_outputs; i++) { + for (size_t i = 0; i < tx->wtx->num_outputs; i++) { struct amount_asset asset; struct amount_sat outval; if (elements_tx_output_is_fee(tx, i)) @@ -709,18 +702,55 @@ static void record_tx_outs_and_fees(struct lightningd *ld, const struct bitcoin_ asset = bitcoin_tx_output_get_amount(tx, i); assert(amount_asset_is_main(&asset)); outval = amount_asset_to_sat(&asset); - mvt = new_coin_withdrawal_sat(ctx, "wallet", txid, txid, - i, blockheight, outval); + mvt = new_coin_withdrawal_sat(ctx, "wallet", txid, + txid, i, blockheight, + outval); notify_chain_mvt(ld, mvt); } +} - fee = bitcoin_tx_compute_fee_w_inputs(tx, inputs_total); +static void record_tx_outs_and_fees(struct lightningd *ld, + const struct bitcoin_tx *tx, + struct bitcoin_txid *txid, + u32 blockheight, + struct amount_sat inputs_total, + bool our_tx) +{ + struct amount_sat fee, out_val; + struct chain_coin_mvt *mvt; + bool ok; + struct wally_psbt *psbt = NULL; + u8 *ctx = tal(NULL, u8); + + /* We own every input on this tx, so track withdrawals precisely */ + if (our_tx) { + record_outputs_as_withdraws(ctx, ld, tx, txid, blockheight); + fee = bitcoin_tx_compute_fee_w_inputs(tx, inputs_total); + goto log_fee; + } + + /* FIXME: look up stashed psbt! */ + if (!psbt) { + fee = bitcoin_tx_compute_fee_w_inputs(tx, inputs_total); + ok = amount_sat_sub(&out_val, inputs_total, fee); + assert(ok); + + /* We don't have detailed withdrawal info for this tx, + * so we log the wallet withdrawal as a single entry */ + mvt = new_coin_withdrawal_sat(ctx, "wallet", txid, NULL, + 0, blockheight, out_val); + notify_chain_mvt(ld, mvt); + goto log_fee; + } + + fee = AMOUNT_SAT(0); /* Note that to figure out the *total* 'onchain' * cost of a channel, you'll want to also include * fees logged here, to the 'wallet' account (for funding tx). * You can do this in post by accounting for any 'chain_fees' logged for * the funding txid when looking at a channel. */ +log_fee: notify_chain_mvt(ld, new_coin_chain_fees_sat(ctx, "wallet", txid, blockheight, fee)); @@ -736,7 +766,7 @@ static void topo_update_spends(struct chain_topology *topo, struct block *b) const struct short_channel_id *scid; for (size_t i = 0; i < tal_count(b->full_txs); i++) { const struct bitcoin_tx *tx = b->full_txs[i]; - bool our_tx = false; + bool our_tx = true, includes_our_spend = false; struct bitcoin_txid txid; struct amount_sat inputs_total = AMOUNT_SAT(0); @@ -758,34 +788,22 @@ static void topo_update_spends(struct chain_topology *topo, struct block *b) tal_free(scid); } - our_tx |= our_spend; + our_tx &= our_spend; + includes_our_spend |= our_spend; if (our_spend) { struct amount_sat input_amt; bool ok; - record_output_spend(topo->ld, &txid, &outpoint_txid, - input->index, b->height, &input_amt); + record_utxo_spent(topo->ld, &txid, &outpoint_txid, + input->index, b->height, &input_amt); ok = amount_sat_add(&inputs_total, inputs_total, input_amt); assert(ok); - } else if (our_tx) - log_broken(topo->ld->log, "Recording fee spend for tx %s but " - "our wallet did not contribute input %s:%d", - type_to_string(tmpctx, struct bitcoin_txid, - &txid), - type_to_string(tmpctx, struct bitcoin_txid, - &outpoint_txid), - input->index); - + } } - /* For now we assume that if one of the spent utxos - * in this tx is 'ours', that we own all of the - * utxos and therefore paid all of the fees - * FIXME: update once interactive tx construction - * is a reality */ - if (our_tx) + if (includes_our_spend) record_tx_outs_and_fees(topo->ld, tx, &txid, - b->height, inputs_total); + b->height, inputs_total, our_tx); } } diff --git a/tests/test_wallet.py b/tests/test_wallet.py index 56e9bc1ac07c..ab1640040f67 100644 --- a/tests/test_wallet.py +++ b/tests/test_wallet.py @@ -6,7 +6,7 @@ from pyln.client import RpcError, Millisatoshi from utils import ( only_one, wait_for, sync_blockheight, EXPERIMENTAL_FEATURES, COMPAT, - VALGRIND + VALGRIND, check_coin_moves ) import os @@ -568,7 +568,9 @@ def test_sign_and_send_psbt(node_factory, bitcoind, chainparams): """ amount = 1000000 total_outs = 12 - l1 = node_factory.get_node(feerates=(7500, 7500, 7500, 7500)) + coin_mvt_plugin = os.path.join(os.getcwd(), 'tests/plugins/coin_movements.py') + l1 = node_factory.get_node(options={'plugin': coin_mvt_plugin}, + feerates=(7500, 7500, 7500, 7500)) l2 = node_factory.get_node() addr = chainparams['example_addr'] @@ -677,6 +679,41 @@ def test_sign_and_send_psbt(node_factory, bitcoind, chainparams): with pytest.raises(RpcError, match=r"should be a PSBT, not"): l1.rpc.signpsbt(modded_psbt) + wallet_coin_mvts = [ + {'type': 'chain_mvt', 'credit': 1000000000, 'debit': 0, 'tag': 'deposit'}, + {'type': 'chain_mvt', 'credit': 1000000000, 'debit': 0, 'tag': 'deposit'}, + {'type': 'chain_mvt', 'credit': 1000000000, 'debit': 0, 'tag': 'deposit'}, + {'type': 'chain_mvt', 'credit': 1000000000, 'debit': 0, 'tag': 'deposit'}, + {'type': 'chain_mvt', 'credit': 1000000000, 'debit': 0, 'tag': 'deposit'}, + {'type': 'chain_mvt', 'credit': 1000000000, 'debit': 0, 'tag': 'deposit'}, + {'type': 'chain_mvt', 'credit': 1000000000, 'debit': 0, 'tag': 'deposit'}, + {'type': 'chain_mvt', 'credit': 1000000000, 'debit': 0, 'tag': 'deposit'}, + {'type': 'chain_mvt', 'credit': 1000000000, 'debit': 0, 'tag': 'deposit'}, + {'type': 'chain_mvt', 'credit': 1000000000, 'debit': 0, 'tag': 'deposit'}, + {'type': 'chain_mvt', 'credit': 1000000000, 'debit': 0, 'tag': 'deposit'}, + {'type': 'chain_mvt', 'credit': 1000000000, 'debit': 0, 'tag': 'deposit'}, + {'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'}, + # 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': 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': 988255000, 'debit': 0, 'tag': 'deposit'}, + ] + check_coin_moves(l1, 'wallet', wallet_coin_mvts) + def test_txsend(node_factory, bitcoind, chainparams): amount = 1000000