df: check mempool/block for funding output on broadcast fail

If we can't broadcast the tx, confirm that it didn't end up in the
mempool or the utxo set before throwing an error.

Note that this doesn't protect us in the case where the funding
output has already been *spent*... but that's extremely rare, right?

Fixes ElementsProject#5296

Reported-By: @rustyrussell
Collab-With: @vincenzopalazzo

Author: niftynei

lightningd/dual_open_control.c

@@ -1413,50 +1413,19 @@ static void handle_local_private_channel(struct subd *dualopend,
 struct channel_send {
 	const struct wally_tx *wtx;
 	struct channel *channel;
+	const char *err_msg;
 };
 
-static void sendfunding_done(struct bitcoind *bitcoind UNUSED,
-			     bool success, const char *msg,
-			     struct channel_send *cs)
+static void handle_tx_broadcast(struct channel_send *cs)
 {
 	struct lightningd *ld = cs->channel->peer->ld;
-	struct channel *channel = cs->channel;
 	const struct wally_tx *wtx = cs->wtx;
+	struct channel *channel = cs->channel;
+	struct command *cmd = channel->openchannel_signed_cmd;
 	struct json_stream *response;
 	struct bitcoin_txid txid;
 	struct amount_sat unused;
 	int num_utxos;
-	struct command *cmd = channel->openchannel_signed_cmd;
-	channel->openchannel_signed_cmd = NULL;
-
-	if (!cmd && channel->opener == LOCAL)
-		log_unusual(channel->log,
-			    "No outstanding command for channel %s,"
-			    " funding sent was success? %d",
-			    type_to_string(tmpctx, struct channel_id,
-					   &channel->cid),
-			    success);
-
-	if (!success) {
-		if (cmd)
-			was_pending(command_fail(cmd,
-						 FUNDING_BROADCAST_FAIL,
-						 "Error broadcasting funding "
-						 "tx: %s. Unsent tx discarded "
-						 "%s.",
-						 msg,
-						 type_to_string(tmpctx,
-								struct wally_tx,
-								wtx)));
-		log_unusual(channel->log,
-			    "Error broadcasting funding "
-			    "tx: %s. Unsent tx discarded "
-			    "%s.",
-			    msg,
-			    type_to_string(tmpctx, struct wally_tx, wtx));
-		tal_free(cs);
-		return;
-	}
 
 	/* This might have spent UTXOs from our wallet */
 	num_utxos = wallet_extract_owned_outputs(ld->wallet,
@@ -1472,11 +1441,78 @@ static void sendfunding_done(struct bitcoind *bitcoind UNUSED,
 		json_add_txid(response, "txid", &txid);
 		json_add_channel_id(response, "channel_id", &channel->cid);
 		was_pending(command_success(cmd, response));
+
+		cs->channel->openchannel_signed_cmd = NULL;
 	}
+}
+
+static void check_utxo_block(struct bitcoind *bitcoind UNUSED,
+			     const struct bitcoin_tx_output *txout,
+			     void *arg)
+{
+	struct channel_send *cs = arg;
+	struct command *cmd = cs->channel->openchannel_signed_cmd;
+	const struct wally_tx *wtx = cs->wtx;
+
+	/* note: if this tx has been included in a block *and spent*
+	 * then this will also fail... */
+	if (!txout) {
+		if (cmd) {
+			was_pending(command_fail(cmd,
+						 FUNDING_BROADCAST_FAIL,
+						 "Error broadcasting funding "
+						 "tx: %s. Unsent tx discarded "
+						 "%s.",
+						 cs->err_msg,
+						 type_to_string(tmpctx,
+								struct wally_tx,
+								wtx)));
+			cs->channel->openchannel_signed_cmd = NULL;
+		}
+
+		log_unusual(cs->channel->log,
+			    "Error broadcasting funding "
+			    "tx: %s. Unsent tx discarded "
+			    "%s.",
+			    cs->err_msg,
+			    type_to_string(tmpctx, struct wally_tx, wtx));
+	} else
+		handle_tx_broadcast(cs);
 
 	tal_free(cs);
 }
 
+static void sendfunding_done(struct bitcoind *bitcoind UNUSED,
+			     bool success, const char *msg,
+			     struct channel_send *cs)
+{
+	struct lightningd *ld = cs->channel->peer->ld;
+	struct channel *channel = cs->channel;
+	struct command *cmd = channel->openchannel_signed_cmd;
+
+	if (!cmd && channel->opener == LOCAL)
+		log_unusual(channel->log,
+			    "No outstanding command for channel %s,"
+			    " funding sent was success? %d",
+			    type_to_string(tmpctx, struct channel_id,
+					   &channel->cid),
+			    success);
+
+	if (success) {
+		handle_tx_broadcast(cs);
+		tal_free(cs);
+	} else {
+		/* If the tx was mined into a block, it's possible
+		 * that the broadcast would fail. Verify that's not
+		 * the case here. */
+		cs->err_msg = tal_strdup(cs, msg);
+		bitcoind_getutxout(ld->topology->bitcoind,
+				   &channel->funding,
+				   check_utxo_block,
+				   cs);
+	}
+}
+
 
 static void send_funding_tx(struct channel *channel,
 			    const struct wally_tx *wtx TAKES)

tests/test_connection.py

@@ -2680,7 +2680,7 @@ def test_fundee_forget_funding_tx_unconfirmed(node_factory, bitcoind):
     # is much slower in VALGRIND mode and wait_for_log
     # could time out before lightningd processes all the
     # blocks.
-    blocks = 200
+    blocks = 50
     # opener
     l1 = node_factory.get_node()
     # peer
@@ -2689,14 +2689,16 @@ def test_fundee_forget_funding_tx_unconfirmed(node_factory, bitcoind):
 
     # Give opener some funds.
     l1.fundwallet(10**7)
-    # Let blocks settle.
-    time.sleep(1)
 
     def mock_sendrawtransaction(r):
         return {'id': r['id'], 'error': {'code': 100, 'message': 'sendrawtransaction disabled'}}
 
+    def mock_donothing(r):
+        return {'id': r['id'], 'result': {'success': True}}
+
     # Prevent opener from broadcasting funding tx (any tx really).
     l1.daemon.rpcproxy.mock_rpc('sendrawtransaction', mock_sendrawtransaction)
+    l2.daemon.rpcproxy.mock_rpc('sendrawtransaction', mock_donothing)
 
     # Fund the channel.
     # The process will complete, but opener will be unable
@@ -2708,7 +2710,8 @@ def mock_sendrawtransaction(r):
     bitcoind.generate_block(blocks)
 
     # fundee will forget channel!
-    l2.daemon.wait_for_log('Forgetting channel: It has been {} blocks'.format(blocks))
+    # (Note that we let the last number be anything (hence the {}\d)
+    l2.daemon.wait_for_log(r'Forgetting channel: It has been {}\d blocks'.format(str(blocks)[:-1]))
 
     # fundee will also forget and disconnect from peer.
     assert len(l2.rpc.listpeers(l1.info['id'])['peers']) == 0