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
- failure still fails the channel into -> AWAITING_UNILATERAL

This gives the user a more explicit warning of broadcast failure, so it (hopefully) doesn't
try to broadcast new tx's that depend on its change_outputs, see issue ElementsProject#2171

openingd/opening_funder_finished: broadcast_tx callback function now handles both
success and failure

pytest: mock_sendrawtransaction now returns error _number_, the jsonrpc's command_fail
needs a _number_ instead of 'error'

jsonrpc: added error code FUNDING_BROADCAST_FAIL

improved some comments

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

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,60 @@ 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);
+
+	/* TODO: keep in state CHANNELD_AWAITING_LOCKIN until (manual) broadcast ? */
 	channel_internal_error(channel,
 			       "Funding broadcast exited with %i: %s",
-			       exitstatus, err);
+			       exitstatus, msg);
+}
+
+static void funding_broadcast_success(struct channel *channel,
+				     int exitstatus, const char *msg)
+{
+	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(struct channel *channel,
+				     int exitstatus, const char *msg)
+{
+	if (exitstatus == 0) {
+		funding_broadcast_success(channel, exitstatus, msg);
+	} 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 +286,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);
 
@@ -373,7 +418,7 @@ static void opening_funder_finished(struct subd *openingd, const u8 *resp,
 		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);
+	broadcast_tx(ld->topology, channel, fundingtx, funding_broadcast);
 
 	channel_watch_funding(ld, channel);
 
@@ -382,22 +427,13 @@ static void opening_funder_finished(struct subd *openingd, const u8 *resp,
 
 	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));
+	/* We need these to compose cmd's response, handled by funding_broadcast_success */
+	fc->linear = linearize_tx(fc->cmd, fundingtx);
+	derive_channel_id(&fc->cid, &channel->funding_txid, funding_outnum);
 
 	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

@@ -1115,7 +1115,7 @@ def test_fundee_forget_funding_tx_unconfirmed(node_factory, bitcoind):
     time.sleep(1)
 
     def mock_sendrawtransaction(r):
-        return {'error': 'sendrawtransaction disabled'}
+        return {'42': 'sendrawtransaction disabled'}
 
     # Prevent funder from broadcasting funding tx (any tx really).
     l1.daemon.rpcproxy.mock_rpc('sendrawtransaction', mock_sendrawtransaction)