Fix test flake, improve close handling

lightningd/channel_control.c

@@ -305,8 +305,9 @@ static void handle_splice_abort(struct lightningd *ld,
 	struct bitcoin_outpoint *outpoint;
 	struct channel_inflight *inflight;
 	char *reason;
-	u8 *error;
-	int fds[2];
+	const u8 *error;
+	struct peer_fd *pfd;
+	int other_fd;
 
 	if (!fromwire_channeld_splice_abort(tmpctx, msg, &did_i_abort,
 					    &outpoint, &reason)) {
@@ -346,16 +347,8 @@ static void handle_splice_abort(struct lightningd *ld,
 		  "Restarting channeld after tx_abort on %s channel",
 		  channel_state_name(channel));
 
-	if (socketpair(AF_LOCAL, SOCK_STREAM, 0, fds) != 0) {
-		log_broken(channel->log,
-			   "Failed to create socketpair: %s",
-			   strerror(errno));
-
-		error = towire_warningfmt(tmpctx, &channel->cid,
-					  "Trouble in paradise?");
-		log_peer_debug(ld->log, &channel->peer->id,
-			       "Telling connectd to send error %s",
-			       tal_hex(tmpctx, error));
+	pfd = sockpair(tmpctx, channel, &other_fd, &error);
+	if (!pfd) {
 		/* Get connectd to send error and close. */
 		subd_send_msg(ld->connectd,
 			      take(towire_connectd_peer_send_msg(NULL,
@@ -368,21 +361,17 @@ static void handle_splice_abort(struct lightningd *ld,
 								peer->connectd_counter)));
 		return;
 	}
-	log_debug(channel->log, "made the socket pair");
 
-	if (peer_start_channeld(channel, new_peer_fd(tmpctx, fds[0]), NULL,
-						     false, false)) {
-		log_info(channel->log, "Sending the peer fd to connectd");
+	if (peer_start_channeld(channel, pfd, NULL, false, false)) {
 		subd_send_msg(ld->connectd,
 			      take(towire_connectd_peer_connect_subd(NULL,
 			      					     &peer->id,
 								     peer->connectd_counter,
 								     &channel->cid)));
-		subd_send_fd(ld->connectd, fds[1]);
-		log_info(channel->log, "Sent the peer fd to channeld");
+		subd_send_fd(ld->connectd, other_fd);
 	} else {
 		log_info(channel->log, "peer_start_channeld failed");
-		close(fds[1]);
+		close(other_fd);
 	}
 }
 

lightningd/dual_open_control.c

@@ -2446,27 +2446,26 @@ json_openchannel_abort(struct command *cmd,
 static char *restart_dualopend(const tal_t *ctx, const struct lightningd *ld,
 			       struct channel *channel, bool from_abort)
 {
-	int fds[2];
-	if (socketpair(AF_LOCAL, SOCK_STREAM, 0, fds) != 0) {
-		log_broken(channel->log,
-			   "Failed to create socketpair: %s",
-			   strerror(errno));
+	struct peer_fd *pfd;
+	int other_fd;
+
+	pfd = sockpair(tmpctx, channel, &other_fd, NULL);
+	if (!pfd)
 		return tal_fmt(ctx, "Unable to create socket: %s",
 			       strerror(errno));
-	}
 
 	if (!peer_restart_dualopend(channel->peer,
-				    new_peer_fd(tmpctx, fds[0]),
+				    pfd,
 				    channel, from_abort)) {
-		close(fds[1]);
+		close(other_fd);
 		return tal_fmt(ctx, "Peer not connected");
 	}
 	subd_send_msg(ld->connectd,
 		      take(towire_connectd_peer_connect_subd(NULL,
 							     &channel->peer->id,
 							     channel->peer->connectd_counter,
 							     &channel->cid)));
-	subd_send_fd(ld->connectd, fds[1]);
+	subd_send_fd(ld->connectd, other_fd);
 	return NULL;
 }
 

lightningd/peer_control.c

@@ -73,6 +73,26 @@
 #include <wire/onion_wire.h>
 #include <wire/wire_sync.h>
 
+/* Common pattern: create a sockpair for this channel, return one as a peer_fd */
+struct peer_fd *sockpair(const tal_t *ctx, struct channel *channel,
+			 int *otherfd, const u8 **warning)
+{
+	int fds[2];
+
+	if (socketpair(AF_LOCAL, SOCK_STREAM, 0, fds) != 0) {
+		/* Note: preserves errno! */
+		log_broken(channel->log,
+			   "Failed to create socketpair: %s",
+			   strerror(errno));
+		if (warning)
+			*warning = towire_warningfmt(ctx, &channel->cid,
+						     "Trouble in paradise?");
+		return NULL;
+	}
+	*otherfd = fds[1];
+	return new_peer_fd(ctx, fds[0]);
+}
+
 static void destroy_peer(struct peer *peer)
 {
 	peer_node_id_map_del(peer->ld->peers, peer);
@@ -1291,7 +1311,8 @@ peer_connected_serialize(struct peer_connected_hook_payload *payload,
 static void connect_activate_subd(struct lightningd *ld, struct channel *channel)
 {
 	const u8 *error;
-	int fds[2];
+	struct peer_fd *pfd;
+	int other_fd;
 
 	/* If we have a canned error for this channel, send it now */
 	if (channel->error) {
@@ -1318,19 +1339,15 @@ static void connect_activate_subd(struct lightningd *ld, struct channel *channel
 	case DUALOPEND_OPEN_COMMITTED:
 	case DUALOPEND_AWAITING_LOCKIN:
 		assert(!channel->owner);
-		if (socketpair(AF_LOCAL, SOCK_STREAM, 0, fds) != 0) {
-			log_broken(channel->log,
-				   "Failed to create socketpair: %s",
-				   strerror(errno));
-			error = towire_warningfmt(tmpctx, &channel->cid,
-						  "Trouble in paradise?");
+		pfd = sockpair(tmpctx, channel, &other_fd, &error);
+		if (!pfd)
 			goto send_error;
-		}
+
 		if (peer_restart_dualopend(channel->peer,
-					   new_peer_fd(tmpctx, fds[0]),
+					   pfd,
 					   channel, false))
 			goto tell_connectd;
-		close(fds[1]);
+		close(other_fd);
 		return;
 
 	case CHANNELD_AWAITING_LOCKIN:
@@ -1339,21 +1356,17 @@ static void connect_activate_subd(struct lightningd *ld, struct channel *channel
 	case CHANNELD_SHUTTING_DOWN:
 	case CLOSINGD_SIGEXCHANGE:
 		assert(!channel->owner);
-		if (socketpair(AF_LOCAL, SOCK_STREAM, 0, fds) != 0) {
-			log_broken(channel->log,
-				   "Failed to create socketpair: %s",
-				   strerror(errno));
-			error = towire_warningfmt(tmpctx, &channel->cid,
-						  "Trouble in paradise?");
+		pfd = sockpair(tmpctx, channel, &other_fd, &error);
+		if (!pfd)
 			goto send_error;
-		}
+
 		if (peer_start_channeld(channel,
-					new_peer_fd(tmpctx, fds[0]),
+					pfd,
 					NULL, true,
 					NULL)) {
 			goto tell_connectd;
 		}
-		close(fds[1]);
+		close(other_fd);
 		return;
 	}
 	abort();
@@ -1364,7 +1377,7 @@ static void connect_activate_subd(struct lightningd *ld, struct channel *channel
 							     &channel->peer->id,
 							     channel->peer->connectd_counter,
 							     &channel->cid)));
-	subd_send_fd(ld->connectd, fds[1]);
+	subd_send_fd(ld->connectd, other_fd);
 	return;
 
 send_error:
@@ -1838,8 +1851,9 @@ void peer_spoke(struct lightningd *ld, const u8 *msg)
 	struct channel_id channel_id;
 	struct peer *peer;
 	bool dual_fund;
-	u8 *error;
-	int fds[2];
+	const u8 *error;
+	int other_fd;
+	struct peer_fd *pfd;
 	char *errmsg;
 
 	if (!fromwire_connectd_peer_spoke(msg, msg, &id, &connectd_counter, &msgtype, &channel_id, &errmsg))
@@ -1855,8 +1869,26 @@ void peer_spoke(struct lightningd *ld, const u8 *msg)
 	if (channel) {
 		/* In this case, we'll send an error below, but send reestablish reply first
 		 * in case they lost their state and need it */
-		if (msgtype == WIRE_CHANNEL_REESTABLISH && channel_state_closed(channel->state))
+		if (msgtype == WIRE_CHANNEL_REESTABLISH && channel_state_closed(channel->state)) {
+			/* Maybe we know it's closed, but they don't?  Happy to negotiate again. */
+			if (channel->state == CLOSINGD_COMPLETE) {
+				pfd = sockpair(tmpctx, channel, &other_fd, &error);
+				if (!pfd)
+					goto send_error;
+
+				/* Tell channeld to handle reestablish, then it will call closingd */
+				if (peer_start_channeld(channel,
+							pfd,
+							NULL, true,
+							NULL)) {
+					goto tell_connectd;
+				}
+				error = towire_warningfmt(tmpctx, &channel_id,
+							  "Trouble in paradise?");
+				goto send_error;
+			}
 			send_reestablish(ld, channel);
+		}
 
 		/* If we have a canned error for this channel, send it now */
 		if (channel->error) {
@@ -1883,18 +1915,13 @@ void peer_spoke(struct lightningd *ld, const u8 *msg)
 
 			log_debug(channel->log, "channel already active");
 			if (channel->state == DUALOPEND_AWAITING_LOCKIN) {
-				if (socketpair(AF_LOCAL, SOCK_STREAM, 0, fds) != 0) {
-					log_broken(ld->log,
-						   "Failed to create socketpair: %s",
-						   strerror(errno));
-					error = towire_warningfmt(tmpctx, &channel_id,
-								  "Trouble in paradise?");
+				pfd = sockpair(tmpctx, channel, &other_fd, &error);
+				if (!pfd)
 					goto send_error;
-				}
-				if (peer_restart_dualopend(peer, new_peer_fd(tmpctx, fds[0]), channel, false))
+				if (peer_restart_dualopend(peer, pfd, channel, false))
 					goto tell_connectd;
 				/* FIXME: Send informative error? */
-				close(fds[1]);
+				close(other_fd);
 			}
 			return;
 		}
@@ -1925,19 +1952,13 @@ void peer_spoke(struct lightningd *ld, const u8 *msg)
 		}
 		peer->uncommitted_channel = new_uncommitted_channel(peer);
 		peer->uncommitted_channel->cid = channel_id;
-		if (socketpair(AF_LOCAL, SOCK_STREAM, 0, fds) != 0) {
-			log_broken(ld->log,
-				   "Failed to create socketpair: %s",
-				   strerror(errno));
-			error = towire_warningfmt(tmpctx, &channel_id,
-						  "Trouble in paradise?");
+		pfd = sockpair(tmpctx, channel, &other_fd, &error);
+		if (!pfd)
 			goto send_error;
-		}
-		if (peer_start_openingd(peer, new_peer_fd(tmpctx, fds[0]))) {
+		if (peer_start_openingd(peer, pfd))
 			goto tell_connectd;
-		}
 		/* FIXME: Send informative error? */
-		close(fds[1]);
+		close(other_fd);
 		return;
 
 	case WIRE_OPEN_CHANNEL2:
@@ -1950,18 +1971,14 @@ void peer_spoke(struct lightningd *ld, const u8 *msg)
 					      peer->ld->config.fee_base,
 					      peer->ld->config.fee_per_satoshi);
 		channel->cid = channel_id;
-		if (socketpair(AF_LOCAL, SOCK_STREAM, 0, fds) != 0) {
-			log_broken(ld->log,
-				   "Failed to create socketpair: %s",
-				   strerror(errno));
-			error = towire_warningfmt(tmpctx, &channel_id,
-						  "Trouble in paradise?");
+		pfd = sockpair(tmpctx, channel, &other_fd, &error);
+		if (!pfd)
 			goto send_error;
-		}
-		if (peer_start_dualopend(peer, new_peer_fd(tmpctx, fds[0]), channel))
+
+		if (peer_start_dualopend(peer, pfd, channel))
 			goto tell_connectd;
 		/* FIXME: Send informative error? */
-		close(fds[1]);
+		close(other_fd);
 		return;
 	}
 
@@ -1993,7 +2010,7 @@ void peer_spoke(struct lightningd *ld, const u8 *msg)
 		      take(towire_connectd_peer_connect_subd(NULL, &id,
 							     peer->connectd_counter,
 							     &channel_id)));
-	subd_send_fd(ld->connectd, fds[1]);
+	subd_send_fd(ld->connectd, other_fd);
 }
 
 struct disconnect_command {

lightningd/peer_control.h

@@ -101,6 +101,12 @@ void channel_errmsg(struct channel *channel,
 		    bool disconnect,
 		    bool warning);
 
+/* Helper to create a peer_fd and an other fd from socketpair.
+ * Logs error to channel if it fails, and if warning non-NULL, creates
+ * a warning message */
+struct peer_fd *sockpair(const tal_t *ctx, struct channel *channel,
+			 int *otherfd, const u8 **warning);
+
 u8 *p2wpkh_for_keyidx(const tal_t *ctx, struct lightningd *ld, u64 keyidx);
 u8 *p2tr_for_keyidx(const tal_t *ctx, struct lightningd *ld, u64 keyidx);
 

lightningd/peer_fd.h

@@ -1,6 +1,7 @@
 #ifndef LIGHTNING_LIGHTNINGD_PEER_FD_H
 #define LIGHTNING_LIGHTNINGD_PEER_FD_H
 #include "config.h"
+#include <ccan/compiler/compiler.h>
 #include <ccan/tal/tal.h>
 
 /* Tal wrapper for fd connecting subd to connectd */
@@ -10,9 +11,11 @@ struct peer_fd {
 };
 
 /* Allocate a new per-peer state and add destructor to close fd if set. */
+RETURNS_NONNULL
 struct peer_fd *new_peer_fd(const tal_t *ctx, int peer_fd);
 
 /* Array version of above: tal_count(fds) must be 1 */
+RETURNS_NONNULL
 struct peer_fd *new_peer_fd_arr(const tal_t *ctx, const int *fd);
 
 #endif /* LIGHTNING_LIGHTNINGD_PEER_FD_H */

lightningd/test/run-invoice-select-inchan.c

@@ -908,6 +908,10 @@ bool peer_start_channeld(struct channel *channel UNNEEDED,
 			 bool reconnected UNNEEDED,
 			 bool reestablish_only UNNEEDED)
 { fprintf(stderr, "peer_start_channeld called!\n"); abort(); }
+/* Generated stub for peer_start_closingd */
+void peer_start_closingd(struct channel *channel UNNEEDED,
+			 struct peer_fd *peer_fd UNNEEDED)
+{ fprintf(stderr, "peer_start_closingd called!\n"); abort(); }
 /* Generated stub for peer_start_dualopend */
 bool peer_start_dualopend(struct peer *peer UNNEEDED, struct peer_fd *peer_fd UNNEEDED,
 			  struct channel *channel UNNEEDED)

wallet/test/run-wallet.c

@@ -929,6 +929,10 @@ bool peer_start_channeld(struct channel *channel UNNEEDED,
 			 bool reconnected UNNEEDED,
 			 bool reestablish_only UNNEEDED)
 { fprintf(stderr, "peer_start_channeld called!\n"); abort(); }
+/* Generated stub for peer_start_closingd */
+void peer_start_closingd(struct channel *channel UNNEEDED,
+			 struct peer_fd *peer_fd UNNEEDED)
+{ fprintf(stderr, "peer_start_closingd called!\n"); abort(); }
 /* Generated stub for peer_start_dualopend */
 bool peer_start_dualopend(struct peer *peer UNNEEDED, struct peer_fd *peer_fd UNNEEDED,
 			  struct channel *channel UNNEEDED)