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

@@ -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

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 <http://docbook.sf.net/>
-.\"      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

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.).

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);
 

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 {

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)
 {
-	channel_internal_error(channel,
-			       "Funding broadcast exited with %i: %s",
-			       exitstatus, err);
+	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)
+{
+	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);
 
+	/* Mark consumed outputs as spent */
+	wallet_confirm_utxos(ld->wallet, fc->wtx.utxos);
+
 	/* Start normal channel daemon. */
 	peer_start_channeld(channel, &cs, fds[0], fds[1], NULL, false);
 
-	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));
-
 	subd_release_channel(openingd, fc->uc);
 	fc->uc->openingd = NULL;
 
-	/* Frees fc too, and tmpctx */
-	tal_free(fc->uc);
 	return;
 
 failed:

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)