lightningd: fix incorrect reuse of dualopend, leading to dev_queryfee…

…rates race

CI hit this issue where it would get a tx_abort in fundchannel.  This
happens when the dualopend we use to query the feerates has not exited
yet (it waits for the tx_abort reply), and we mistakenly reuse it.

With multi-channel support, this is wrong: just run another one and it
all Just Works.

This means we need to rework our dual_open_control.c logic, since it
would previously create an unsaved channel then not clean up if
something went wrong.

Most people will never try to negotiate opening multiple channels to
the same peer at the same time (vs. having an established channel and
opening a new one), so this case is a bit weird.

```
         rates = l1.rpc.dev_queryrates(l2.info['id'], amount, amount)
     
         # l1 leases a channel from l2
         l1.rpc.fundchannel(l2.info['id'], amount, request_amt=amount,
                            feerate='{}perkw'.format(feerate),
 >                          compact_lease=rates['compact_lease'])
 
 tests/test_opening.py:1611: 
 _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
 contrib/pyln-client/pyln/client/lightning.py:833: in fundchannel
     return self.call("fundchannel", payload)
 contrib/pyln-testing/pyln/testing/utils.py:721: in call
     res = LightningRpc.call(self, method, payload, cmdprefix, filter)

 _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
 
 self = <pyln.testing.utils.PrettyPrintingLightningRpc object at 0x7f6cbcd97950>
 method = 'fundchannel'
 payload = {'amount': 500000, 'announce': True, 'compact_lease': '029a00640064000000644c4b40', 'feerate': '2000perkw', ...}
 cmdprefix = None, filter = None
 
     def call(self, method, payload=None, cmdprefix=None, filter=None):
         """Generic call API: you can set cmdprefix here, or set self.cmdprefix
...
         if not isinstance(resp, dict):
             raise ValueError("Malformed response, response is not a dictionary %s." % resp)
         elif "error" in resp:
 >           raise RpcError(method, payload, resp['error'])
 E           pyln.client.lightning.RpcError: RPC call failed: method: fundchannel, payload: {'id': '022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59', 'amount': 500000, 'feerate': '2000perkw', 'announce': True, 'request_amt': 500000, 'compact_lease': '029a00640064000000644c4b40'}, error: {'code': -1, 'message': 'Abort requested', 'data': {'id': '022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59', 'method': 'openchannel_init'}}
```

Signed-off-by: Rusty Russell <[email protected]>

lightningd/dual_open_control.c

@@ -2842,24 +2842,6 @@ static struct command_result *json_openchannel_init(struct command *cmd,
 		return command_fail(cmd, FUNDING_UNKNOWN_PEER, "Unknown peer");
 	}
 
-	channel = peer_any_unsaved_channel(peer, NULL);
-	if (!channel) {
-		channel = new_unsaved_channel(peer,
-					      peer->ld->config.fee_base,
-					      peer->ld->config.fee_per_satoshi);
-
-		/* We derive initial channel_id *now*, so we can tell it to
-		 * connectd. */
-		derive_tmp_channel_id(&channel->cid,
-				      &channel->local_basepoints.revocation);
-	}
-
-	if (channel->open_attempt
-	     || !list_empty(&channel->inflights))
-		return command_fail(cmd, FUNDING_STATE_INVALID,
-				    "Channel funding in-progress. %s",
-				    channel_state_name(channel));
-
 	if (!feature_negotiated(cmd->ld->our_features,
 			        peer->their_features,
 				OPT_DUAL_FUND)) {
@@ -2898,6 +2880,20 @@ static struct command_result *json_openchannel_init(struct command *cmd,
 				    type_to_string(tmpctx, struct wally_psbt,
 						   psbt));
 
+	if (socketpair(AF_LOCAL, SOCK_STREAM, 0, fds) != 0) {
+		return command_fail(cmd, FUND_MAX_EXCEEDED,
+				    "Failed to create socketpair: %s",
+				    strerror(errno));
+	}
+
+	/* Now we can't fail, create channel */
+	channel = new_unsaved_channel(peer,
+				      peer->ld->config.fee_base,
+				      peer->ld->config.fee_per_satoshi);
+	/* We derive initial channel_id *now*, so we can tell it to connectd. */
+	derive_tmp_channel_id(&channel->cid,
+			      &channel->local_basepoints.revocation);
+
 	/* Get a new open_attempt going */
 	channel->opener = LOCAL;
 	channel->open_attempt = oa = new_channel_open_attempt(channel);
@@ -2943,12 +2939,6 @@ static struct command_result *json_openchannel_init(struct command *cmd,
 					   false,
 					   rates);
 
-	if (socketpair(AF_LOCAL, SOCK_STREAM, 0, fds) != 0) {
-		return command_fail(cmd, FUND_MAX_EXCEEDED,
-				    "Failed to create socketpair: %s",
-				    strerror(errno));
-	}
-
 	/* Start dualopend! */
 	if (!peer_start_dualopend(peer, new_peer_fd(cmd, fds[0]), channel)) {
 		close(fds[1]);
@@ -3424,24 +3414,14 @@ static struct command_result *json_queryrates(struct command *cmd,
 				    peer->connected == PEER_DISCONNECTED
 				    ? "not connected" : "still connecting");
 
-	/* FIXME: This is wrong: we should always create a new channel? */
-	channel = peer_any_unsaved_channel(peer, NULL);
-	if (!channel) {
-		channel = new_unsaved_channel(peer,
-					      peer->ld->config.fee_base,
-					      peer->ld->config.fee_per_satoshi);
+	channel = new_unsaved_channel(peer,
+				      peer->ld->config.fee_base,
+				      peer->ld->config.fee_per_satoshi);
 
-		/* We derive initial channel_id *now*, so we can tell it to
-		 * connectd. */
-		derive_tmp_channel_id(&channel->cid,
-				      &channel->local_basepoints.revocation);
-	}
-
-	if (channel->open_attempt
-	     || !list_empty(&channel->inflights))
-		return command_fail(cmd, FUNDING_STATE_INVALID,
-				    "Channel funding in-progress. %s",
-				    channel_state_name(channel));
+	/* We derive initial channel_id *now*, so we can tell it to
+	 * connectd. */
+	derive_tmp_channel_id(&channel->cid,
+			      &channel->local_basepoints.revocation);
 
 	if (!feature_negotiated(cmd->ld->our_features,
 			        peer->their_features,

tests/test_connection.py

@@ -1460,9 +1460,6 @@ def test_funding_v2_corners(node_factory, bitcoind):
         l1.rpc.openchannel_init(l2.info['id'], amount + 1, psbt)
 
     start = l1.rpc.openchannel_init(l2.info['id'], amount, psbt)
-    with pytest.raises(RpcError, match=r'Channel funding in-progress. DUALOPEND_OPEN_INIT'):
-        l1.rpc.fundchannel(l2.info['id'], amount)
-
     # We can abort a channel
     l1.rpc.openchannel_abort(start['channel_id'])