openingd/: Fail fundchannel_start if we already are, or will become, the fundee.

lightningd/opening_common.c

@@ -55,6 +55,8 @@ new_uncommitted_channel(struct peer *peer)
 	uc->peer->uncommitted_channel = uc;
 	tal_add_destructor(uc, destroy_uncommitted_channel);
 
+	uc->got_offer = false;
+
 	return uc;
 }
 

lightningd/opening_common.h

@@ -46,6 +46,11 @@ struct uncommitted_channel {
 	/* Public key for funding tx. */
 	struct pubkey local_funding_pubkey;
 
+	/* If true, we are already in fundee-mode and any future
+	 * `fundchannel_start` on our end should fail.
+	 */
+	bool got_offer;
+
 	/* These are *not* filled in by new_uncommitted_channel: */
 
 	/* Minimum funding depth (if opener == REMOTE). */

lightningd/opening_control.c

@@ -104,6 +104,11 @@ wallet_commit_channel(struct lightningd *ld,
 	bool option_static_remotekey;
 	bool option_anchor_outputs;
 
+	/* We cannot both be the fundee *and* have a `fundchannel_start`
+	 * command running!
+	 */
+	assert(!(uc->got_offer && uc->fc));
+
 	/* Get a key to use for closing outputs from this tx */
 	final_key_idx = wallet_get_newindex(ld);
 	if (final_key_idx == -1) {
@@ -538,21 +543,13 @@ static void opening_fundee_finished(struct subd *openingd,
 	tal_free(uc);
 }
 
-static void opening_funder_failed(struct subd *openingd, const u8 *msg,
-				  struct uncommitted_channel *uc)
+static void
+opening_funder_failed_cancel_commands(struct uncommitted_channel *uc,
+				      const char *desc)
 {
-	char *desc;
-
-	if (!fromwire_openingd_funder_failed(msg, msg, &desc)) {
-		log_broken(uc->log,
-			   "bad OPENING_FUNDER_FAILED %s",
-			   tal_hex(tmpctx, msg));
-		was_pending(command_fail(uc->fc->cmd, LIGHTNINGD,
-					 "bad OPENING_FUNDER_FAILED %s",
-					 tal_hex(uc->fc->cmd, msg)));
-		tal_free(uc);
+	/* If no funding command(s) pending, do nothing.  */
+	if (!uc->fc)
 		return;
-	}
 
 	/* Tell anyone who was trying to cancel */
 	for (size_t i = 0; i < tal_count(uc->fc->cancels); i++) {
@@ -568,12 +565,35 @@ static void opening_funder_failed(struct subd *openingd, const u8 *msg,
 		was_pending(command_fail(uc->fc->cmd, LIGHTNINGD, "%s", desc));
 
 	/* Clear uc->fc, so we can try again, and so we don't fail twice
-	 * if they close. */
+	 * if they close.
+	 * This code is also used in the case where we turn out to already
+	 * be the fundee, in which case we should not have any `fc` at all,
+	 * so we definitely should clear this.
+	 */
 	uc->fc = tal_free(uc->fc);
 }
+static void opening_funder_failed(struct subd *openingd, const u8 *msg,
+				  struct uncommitted_channel *uc)
+{
+	char *desc;
+
+	if (!fromwire_openingd_funder_failed(msg, msg, &desc)) {
+		log_broken(uc->log,
+			   "bad OPENING_FUNDER_FAILED %s",
+			   tal_hex(tmpctx, msg));
+		was_pending(command_fail(uc->fc->cmd, LIGHTNINGD,
+					 "bad OPENING_FUNDER_FAILED %s",
+					 tal_hex(uc->fc->cmd, msg)));
+		tal_free(uc);
+		return;
+	}
+
+	opening_funder_failed_cancel_commands(uc, desc);
+}
 
 struct openchannel_hook_payload {
 	struct subd *openingd;
+	struct uncommitted_channel* uc;
 	struct amount_sat funding_satoshis;
 	struct amount_msat push_msat;
 	struct amount_sat dust_limit_satoshis;
@@ -631,6 +651,7 @@ openchannel_hook_final(struct openchannel_hook_payload *payload STEALS)
 	struct subd *openingd = payload->openingd;
 	const u8 *our_upfront_shutdown_script = payload->our_upfront_shutdown_script;
 	const char *errmsg = payload->errmsg;
+	struct uncommitted_channel* uc = payload->uc;
 
 	/* We want to free this, whatever happens. */
 	tal_steal(tmpctx, payload);
@@ -641,6 +662,16 @@ openchannel_hook_final(struct openchannel_hook_payload *payload STEALS)
 
 	tal_del_destructor2(openingd, openchannel_payload_remove_openingd, payload);
 
+	if (!errmsg) {
+		/* Plugins accepted the offer, cancel any of our
+		 * funder-side commands.  */
+		opening_funder_failed_cancel_commands(uc,
+						      "Have in-progress "
+						      "`open_channel` from "
+						      "peer");
+		uc->got_offer = true;
+	}
+
 	subd_send_msg(openingd,
 		      take(towire_openingd_got_offer_reply(NULL, errmsg,
 							  our_upfront_shutdown_script)));
@@ -739,6 +770,7 @@ static void opening_got_offer(struct subd *openingd,
 
 	payload = tal(openingd, struct openchannel_hook_payload);
 	payload->openingd = openingd;
+	payload->uc = uc;
 	payload->our_upfront_shutdown_script = NULL;
 	payload->errmsg = NULL;
 	if (!fromwire_openingd_got_offer(payload, msg,
@@ -1089,6 +1121,13 @@ static struct command_result *json_fund_channel_start(struct command *cmd,
 		return command_fail(cmd, LIGHTNINGD, "Already funding channel");
 	}
 
+	if (peer->uncommitted_channel->got_offer) {
+		return command_fail(cmd, LIGHTNINGD,
+				    "Have in-progress "
+				    "`open_channel` from "
+				    "peer");
+	}
+
 	/* BOLT #2:
 	 *  - if both nodes advertised `option_support_large_channel`:
 	 *    - MAY set `funding_satoshis` greater than or equal to 2^24 satoshi.

tests/test_connection.py

@@ -2759,3 +2759,19 @@ def test_channel_opener(node_factory):
     l1.rpc.close(l2.rpc.getinfo()["id"])
     assert(l1.rpc.listpeers()['peers'][0]['channels'][0]['closer'] == 'local')
     assert(l2.rpc.listpeers()['peers'][0]['channels'][0]['closer'] == 'remote')
+
+
+def test_fundchannel_start_alternate(node_factory, executor):
+    ''' Test to see what happens if two nodes start channeling to
+    each other alternately.
+    Issue #4108
+    '''
+    l1, l2 = node_factory.get_nodes(2)
+
+    l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
+
+    l1.rpc.fundchannel_start(l2.info['id'], 100000)
+
+    fut = executor.submit(l2.rpc.fundchannel_start, l1.info['id'], 100000)
+    with pytest.raises(RpcError):
+        fut.result(10)