df-tests: simultaneous openchannel_init (while in progress)

Reject a peer's request to open a channel while we're already in progress

Author: niftynei

lightningd/dual_open_control.c

@@ -361,6 +361,7 @@ static void
 openchannel2_hook_cb(struct openchannel2_payload *payload STEALS)
 {
 	struct subd *dualopend = payload->dualopend;
+	struct uncommitted_channel *uc;
 	u8 *msg;
 
 	/* Free payload regardless of what happens next */
@@ -370,6 +371,18 @@ openchannel2_hook_cb(struct openchannel2_payload *payload STEALS)
 	if (!dualopend)
 		return;
 
+	assert(dualopend->ctype == UNCOMMITTED);
+	uc = dualopend->channel;
+
+	/* Channel open is currently in progress elsewhere! */
+	if (uc->fc || uc->got_offer) {
+		msg = towire_dualopend_fail(NULL, "Already initiated channel"
+					    " open");
+		log_debug(dualopend->ld->log,
+			  "Our open in progress, denying their offer");
+		return subd_send_msg(dualopend, take(msg));
+	}
+
 	tal_del_destructor2(dualopend, openchannel2_remove_dualopend, payload);
 
 	if (payload->err_msg) {
@@ -407,6 +420,7 @@ openchannel2_hook_cb(struct openchannel2_payload *payload STEALS)
 		}
 	}
 
+	uc->got_offer = true;
 	msg = towire_dualopend_got_offer_reply(NULL, payload->accepter_funding,
 					       payload->funding_feerate_per_kw,
 					       payload->psbt,
@@ -1146,6 +1160,7 @@ static void open_failed(struct subd *dualopend, const u8 *msg)
 
 	assert(dualopend->ctype == UNCOMMITTED);
 	uc = dualopend->channel;
+	uc->got_offer = false;
 
 	if (!fromwire_dualopend_failed(msg, msg, &desc)) {
 		log_broken(uc->log,
@@ -1427,7 +1442,15 @@ static void accepter_got_offer(struct subd *dualopend,
 
 	if (peer_active_channel(uc->peer)) {
 		subd_send_msg(dualopend,
-				take(towire_dualopend_fail(NULL, "Already have active channel")));
+				take(towire_dualopend_fail(NULL,
+					"Already have active channel")));
+		return;
+	}
+
+	if (uc->fc) {
+		subd_send_msg(dualopend,
+				take(towire_dualopend_fail(NULL,
+					"Already initiated channel open")));
 		return;
 	}
 
@@ -1773,6 +1796,11 @@ static struct command_result *json_openchannel_init(struct command *cmd,
 		return command_fail(cmd, LIGHTNINGD, "Already funding channel");
 	}
 
+	if (peer->uncommitted_channel->got_offer) {
+		return command_fail(cmd, LIGHTNINGD,
+				    "Channel open in progress");
+	}
+
 #if EXPERIMENTAL_FEATURES
 	if (!feature_negotiated(cmd->ld->our_features,
 			        peer->their_features,

openingd/dualopend.c

@@ -277,16 +277,13 @@ static void negotiation_aborted(struct state *state, const char *why)
 	memset(&state->channel_id, 0, sizeof(state->channel_id));
 	state->channel = tal_free(state->channel);
 	state->changeset = tal_free(state->changeset);
-	if (state->psbt)
-		tal_free(state->psbt);
 
 	for (size_t i = 0; i < NUM_TX_MSGS; i++)
 		state->tx_msg_count[i] = 0;
 }
 
-/*~ For negotiation failures: we tell them the parameter we didn't like. */
-static void negotiation_failed(struct state *state,
-			       const char *fmt, ...)
+static void open_error(struct state *state,
+		       const char *fmt, ...)
 {
 	va_list ap;
 	const char *errmsg;
@@ -297,12 +294,27 @@ static void negotiation_failed(struct state *state,
 	va_end(ap);
 
 	msg = towire_errorfmt(NULL, &state->channel_id,
-			      "You gave bad parameters: %s", errmsg);
+			      "%s", errmsg);
 	sync_crypto_write(state->pps, take(msg));
 
 	negotiation_aborted(state, errmsg);
 }
 
+
+/*~ For negotiation failures: we tell them the parameter we didn't like. */
+static void negotiation_failed(struct state *state,
+			       const char *fmt, ...)
+{
+	va_list ap;
+	const char *errmsg;
+
+	va_start(ap, fmt);
+	errmsg = tal_vfmt(tmpctx, fmt, ap);
+	va_end(ap);
+
+	open_error(state, "You gave bad parameters: %s", errmsg);
+}
+
 static void billboard_update(struct state *state)
 {
 	const char *update = billboard_message(tmpctx, state->funding_locked,
@@ -885,9 +897,9 @@ fetch_psbt_changes(struct state *state, const struct wally_psbt *psbt)
 	wire_sync_write(REQ_FD, take(msg));
 	msg = wire_sync_read(tmpctx, REQ_FD);
 
-	if (fromwire_dualopend_fail(msg, msg, &err))
-		status_failed(STATUS_FAIL_MASTER_IO, "%s", err);
-	else if (fromwire_dualopend_psbt_updated(state, msg, &updated_psbt)) {
+	if (fromwire_dualopend_fail(msg, msg, &err)) {
+		open_error(state, "%s", err);
+	} else if (fromwire_dualopend_psbt_updated(state, msg, &updated_psbt)) {
 		return updated_psbt;
 #if DEVELOPER
 	} else if (fromwire_dualopend_dev_memleak(msg)) {
@@ -1507,7 +1519,7 @@ static void accepter_start(struct state *state, const u8 *oc2_msg)
 		if (!fromwire_dualopend_fail(msg, msg, &err_reason))
 			master_badmsg(msg_type, msg);
 
-		negotiation_failed(state, "%s", err_reason);
+		open_error(state, "%s", err_reason);
 		return;
 	}
 
@@ -1927,7 +1939,7 @@ static void opener_start(struct state *state, u8 *msg)
 	if (!msg)
 		return;
 
-	a_tlv = tlv_accept_tlvs_new(state);
+	a_tlv = notleak(tlv_accept_tlvs_new(state));
 	if (!fromwire_accept_channel2(msg, &cid,
 				      &state->accepter_funding,
 				      &state->feerate_per_kw_funding,
@@ -2017,9 +2029,11 @@ static void opener_start(struct state *state, u8 *msg)
 	 *   - MUST send at least one `tx_add_output`,  the channel
 	 *     funding output.
 	 */
-	wscript = bitcoin_redeem_2of2(state,
-				      &state->our_funding_pubkey,
-				      &state->their_funding_pubkey);
+	/* If fails before returning from `send_next`, this will
+	 * be marked as a memleak */
+	wscript = notleak(bitcoin_redeem_2of2(state,
+					      &state->our_funding_pubkey,
+					      &state->their_funding_pubkey));
 	funding_out = psbt_append_output(state->psbt,
 					 scriptpubkey_p2wsh(tmpctx,
 							    wscript),
@@ -2052,7 +2066,7 @@ static void opener_start(struct state *state, u8 *msg)
 
 	/* Send our first message, we're opener we initiate here */
 	if (!send_next(state, &state->psbt))
-		negotiation_failed(state, "Peer error, no updates to send");
+		open_error(state, "Peer error, no updates to send");
 
 	/* Figure out what the funding transaction looks like! */
 	if (!run_tx_interactive(state, &state->psbt, TX_INITIATOR))

tests/test_connection.py

@@ -3026,3 +3026,24 @@ def test_fundchannel_start_alternate(node_factory, executor):
     fut = executor.submit(l2.rpc.fundchannel_start, l1.info['id'], 100000)
     with pytest.raises(RpcError):
         fut.result(10)
+
+
+@unittest.skipIf(not EXPERIMENTAL_DUAL_FUND, "openchannel_init not available")
+def test_openchannel_init_alternate(node_factory, executor):
+    ''' Test to see what happens if two nodes start channeling to
+    each other alternately.
+    '''
+    l1, l2 = node_factory.get_nodes(2)
+
+    l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
+
+    l1.fundwallet(2000000)
+    l2.fundwallet(2000000)
+
+    psbt1 = l1.rpc.fundpsbt('1000000msat', '253perkw', 250)['psbt']
+    psbt2 = l2.rpc.fundpsbt('1000000msat', '253perkw', 250)['psbt']
+    l1.rpc.openchannel_init(l2.info['id'], 100000, psbt1)
+
+    fut = executor.submit(l2.rpc.openchannel_init, l1.info['id'], '1000000msat', psbt2)
+    with pytest.raises(RpcError):
+        fut.result(10)