From 10057c8335228dd1187bafc554b8312945a947c4 Mon Sep 17 00:00:00 2001 From: Simon Vrouwe Date: Wed, 16 Jan 2019 15:24:30 +0200 Subject: [PATCH] openingd/json_fund_channel: - result fundchannel command now depends on successful or failed broadcast of the funding tx - failure returns error code FUNDING_BROADCAST_FAIL - don't fail the channel when broadcast failed, but keep in CHANNELD_AWAITING_LOCKIN - after fixing the initial broadcast failure, the user could manually rebroadcast the tx and keep the channel openingd/opening_funder_finished: - broadcast_tx callback function now handles both success and failure jsonrpc: added error code FUNDING_BROADCAST_FAIL manpage: added error code returned by fundchannel command This makes the user more aware of broadcast failure, so it hopefully doesn't try to broadcast new tx's that depend on its change_outputs. Some users have reported (see issue #2171) a whole sequence of fundings failing, because each funding was using the change output of the previous one, which would not confirm. --- common/jsonrpc_errors.h | 1 + doc/lightning-fundchannel.7 | 15 +++++- doc/lightning-fundchannel.7.txt | 2 + lightningd/chaintopology.c | 8 ++-- lightningd/chaintopology.h | 2 +- lightningd/opening_control.c | 84 +++++++++++++++++++++++---------- tests/test_connection.py | 3 +- 7 files changed, 81 insertions(+), 34 deletions(-) diff --git a/common/jsonrpc_errors.h b/common/jsonrpc_errors.h index 5d91eaf66..473142e37 100644 --- a/common/jsonrpc_errors.h +++ b/common/jsonrpc_errors.h @@ -39,6 +39,7 @@ #define FUND_MAX_EXCEEDED 300 #define FUND_CANNOT_AFFORD 301 #define FUND_OUTPUT_IS_DUST 302 +#define FUNDING_BROADCAST_FAIL 303 /* Errors from `invoice` command */ #define INVOICE_LABEL_ALREADY_EXISTS 900 diff --git a/doc/lightning-fundchannel.7 b/doc/lightning-fundchannel.7 index a810f2655..76db867e5 100644 --- a/doc/lightning-fundchannel.7 +++ b/doc/lightning-fundchannel.7 @@ -2,12 +2,12 @@ .\" Title: lightning-fundchannel .\" Author: [FIXME: author] [see http://docbook.sf.net/el/author] .\" Generator: DocBook XSL Stylesheets v1.79.1 -.\" Date: 12/07/2018 +.\" Date: 01/22/2019 .\" Manual: \ \& .\" Source: \ \& .\" Language: English .\" -.TH "LIGHTNING\-FUNDCHANN" "7" "12/07/2018" "\ \&" "\ \&" +.TH "LIGHTNING\-FUNDCHANN" "7" "01/22/2019" "\ \&" "\ \&" .\" ----------------------------------------------------------------- .\" * Define some portability stuff .\" ----------------------------------------------------------------- @@ -95,6 +95,17 @@ The following error codes may occur: 302\&. The output amount is too small, and would be considered dust\&. .RE .sp +.RS 4 +.ie n \{\ +\h'-04'\(bu\h'+03'\c +.\} +.el \{\ +.sp -1 +.IP \(bu 2.3 +.\} +303\&. Broadcasting of the funding transaction failed, the internal call to bitcoin\-cli returned with an error\&. +.RE +.sp Failure may also occur if \fBlightningd\fR and the peer cannot agree on channel parameters (funding limits, channel reserves, fees, etc\&.)\&. .SH "SEE ALSO" .sp diff --git a/doc/lightning-fundchannel.7.txt b/doc/lightning-fundchannel.7.txt index c9c27bde9..85ac8db88 100644 --- a/doc/lightning-fundchannel.7.txt +++ b/doc/lightning-fundchannel.7.txt @@ -53,6 +53,8 @@ The following error codes may occur: * 301. There are not enough funds in the internal wallet (including fees) to create the transaction. * 302. The output amount is too small, and would be considered dust. +* 303. Broadcasting of the funding transaction failed, the internal call to + bitcoin-cli returned with an error. Failure may also occur if *lightningd* and the peer cannot agree on channel parameters (funding limits, channel reserves, fees, etc.). diff --git a/lightningd/chaintopology.c b/lightningd/chaintopology.c index c9bfa5e8d..9c5f94758 100644 --- a/lightningd/chaintopology.c +++ b/lightningd/chaintopology.c @@ -200,8 +200,8 @@ static void broadcast_done(struct bitcoind *bitcoind, /* No longer needs to be disconnected if channel dies. */ tal_del_destructor2(otx->channel, clear_otx_channel, otx); - if (otx->failed && exitstatus != 0) { - otx->failed(otx->channel, exitstatus, msg); + if (otx->failed_or_success) { + otx->failed_or_success(otx->channel, exitstatus, msg); tal_free(otx); } else { /* For continual rebroadcasting, until channel freed. */ @@ -213,7 +213,7 @@ static void broadcast_done(struct bitcoind *bitcoind, void broadcast_tx(struct chain_topology *topo, struct channel *channel, const struct bitcoin_tx *tx, - void (*failed)(struct channel *channel, + void (*failed_or_success)(struct channel *channel, int exitstatus, const char *err)) { /* Channel might vanish: topo owns it to start with. */ @@ -223,7 +223,7 @@ void broadcast_tx(struct chain_topology *topo, otx->channel = channel; bitcoin_txid(tx, &otx->txid); otx->hextx = tal_hex(otx, rawtx); - otx->failed = failed; + otx->failed_or_success = failed_or_success; tal_free(rawtx); tal_add_destructor2(channel, clear_otx_channel, otx); diff --git a/lightningd/chaintopology.h b/lightningd/chaintopology.h index 73d0be0fb..13fcbd43e 100644 --- a/lightningd/chaintopology.h +++ b/lightningd/chaintopology.h @@ -37,7 +37,7 @@ struct outgoing_tx { struct channel *channel; const char *hextx; struct bitcoin_txid txid; - void (*failed)(struct channel *channel, int exitstatus, const char *err); + void (*failed_or_success)(struct channel *channel, int exitstatus, const char *err); }; struct block { diff --git a/lightningd/opening_control.c b/lightningd/opening_control.c index 60dd261de..785760097 100644 --- a/lightningd/opening_control.c +++ b/lightningd/opening_control.c @@ -66,15 +66,20 @@ struct uncommitted_channel { struct funding_channel { - struct command *cmd; /* Which also owns us. */ + struct command *cmd; /* Which initially owns us until openingd request */ + struct wallet_tx wtx; u64 push_msat; u8 channel_flags; + /* Variables we need to compose fields in cmd's response */ + u8 *linear; + struct channel_id cid; + /* Peer we're trying to reach. */ struct pubkey peerid; - /* Channel. */ + /* Channel, subsequent owner of us */ struct uncommitted_channel *uc; }; @@ -216,18 +221,56 @@ wallet_commit_channel(struct lightningd *ld, } static void funding_broadcast_failed(struct channel *channel, - int exitstatus, const char *err) + int exitstatus, const char *msg) +{ + struct funding_channel *fc = channel->peer->uncommitted_channel->fc; + struct command *cmd = fc->cmd; + + /* 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); + was_pending(command_fail(cmd, FUNDING_BROADCAST_FAIL, + "Error broadcasting funding transaction: %s", output)); + + /* Frees fc too */ + tal_free(fc->uc); + + /* Keep in state CHANNELD_AWAITING_LOCKIN until (manual) broadcast */ +} + +static void funding_broadcast_success(struct channel *channel) +{ + struct json_stream *response; + struct funding_channel *fc = channel->peer->uncommitted_channel->fc; + struct command *cmd = fc->cmd; + + response = json_stream_success(cmd); + json_object_start(response, NULL); + json_add_hex_talarr(response, "tx", fc->linear); + json_add_txid(response, "txid", &channel->funding_txid); + json_add_string(response, "channel_id", + type_to_string(tmpctx, struct channel_id, &fc->cid)); + json_object_end(response); + was_pending(command_success(cmd, response)); + + /* Frees fc too */ + tal_free(fc->uc); +} + +static void funding_broadcast_failed_or_success(struct channel *channel, + int exitstatus, const char *msg) { - channel_internal_error(channel, - "Funding broadcast exited with %i: %s", - exitstatus, err); + if (exitstatus == 0) { + funding_broadcast_success(channel); + } else { + funding_broadcast_failed(channel, exitstatus, msg); + } } static void opening_funder_finished(struct subd *openingd, const u8 *resp, const int *fds, struct funding_channel *fc) { - u8 *msg, *linear; + u8 *msg; struct channel_info channel_info; struct bitcoin_tx *fundingtx; struct bitcoin_txid funding_txid, expected_txid; @@ -239,9 +282,7 @@ static void opening_funder_finished(struct subd *openingd, const u8 *resp, u32 feerate; u64 change_satoshi; struct channel *channel; - struct json_stream *response; struct lightningd *ld = openingd->ld; - struct channel_id cid; assert(tal_count(fds) == 2); @@ -372,32 +413,23 @@ static void opening_funder_finished(struct subd *openingd, const u8 *resp, if (tal_count(fundingtx->output) == 2) txfilter_add_scriptpubkey(ld->owned_txfilter, fundingtx->output[!funding_outnum].script); - /* Send it out and watch for confirms. */ - broadcast_tx(ld->topology, channel, fundingtx, funding_broadcast_failed); + /* We need these to compose cmd's response in funding_broadcast_success */ + fc->linear = linearize_tx(fc->cmd, fundingtx); + derive_channel_id(&fc->cid, &channel->funding_txid, funding_outnum); + /* Send it out and watch for confirms. */ + broadcast_tx(ld->topology, channel, fundingtx, funding_broadcast_failed_or_success); channel_watch_funding(ld, channel); - /* Start normal channel daemon. */ - peer_start_channeld(channel, &cs, fds[0], fds[1], NULL, false); - + /* Mark consumed outputs as spent */ wallet_confirm_utxos(ld->wallet, fc->wtx.utxos); - response = json_stream_success(fc->cmd); - json_object_start(response, NULL); - linear = linearize_tx(response, fundingtx); - json_add_hex_talarr(response, "tx", linear); - json_add_txid(response, "txid", &channel->funding_txid); - derive_channel_id(&cid, &channel->funding_txid, funding_outnum); - json_add_string(response, "channel_id", - type_to_string(tmpctx, struct channel_id, &cid)); - json_object_end(response); - was_pending(command_success(fc->cmd, response)); + /* Start normal channel daemon. */ + peer_start_channeld(channel, &cs, fds[0], fds[1], NULL, false); subd_release_channel(openingd, fc->uc); fc->uc->openingd = NULL; - /* Frees fc too, and tmpctx */ - tal_free(fc->uc); return; failed: diff --git a/tests/test_connection.py b/tests/test_connection.py index 8ef7028dd..13fca60df 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -1120,7 +1120,8 @@ def test_fundee_forget_funding_tx_unconfirmed(node_factory, bitcoind): # Fund the channel. # The process will complete, but funder will be unable # to broadcast and confirm funding tx. - l1.rpc.fundchannel(l2.info['id'], 10**6) + with pytest.raises(RpcError, match=r'sendrawtransaction disabled'): + l1.rpc.fundchannel(l2.info['id'], 10**6) # Generate blocks until unconfirmed. bitcoind.generate_block(blocks)