openingd: return to master for more gossip when negotiation fails.

We can open other channels, if we want.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

lightningd/peer_control.c

@@ -193,7 +193,11 @@ void peer_fail_permanent(struct peer *peer, const u8 *msg TAKES)
 	 * zero (ie. all bytes zero), in which case it refers to all
 	 * channels. */
 	static const struct channel_id all_channels;
-	const char *why = tal_strndup(NULL, (char *)msg, tal_len(msg));
+	char *why;
+
+	/* Subtle: we don't want tal_strndup here, it will take() msg! */
+	why = tal_arrz(NULL, char, tal_len(msg) + 1);
+	memcpy(why, msg, tal_len(msg));
 
 	log_unusual(peer->log, "Peer permanent failure in %s: %s",
 		    peer_state_name(peer->state), why);
@@ -2379,6 +2383,42 @@ static void opening_fundee_finished(struct subd *opening,
 	peer_set_condition(peer, OPENINGD, CHANNELD_AWAITING_LOCKIN);
 }
 
+/* Negotiation failed, but we can keep gossipping */
+static unsigned int opening_negotiation_failed(struct subd *openingd,
+					       const u8 *msg,
+					       const int *fds)
+{
+	struct crypto_state cs;
+	struct peer *peer = openingd->peer;
+	u8 *err;
+	const char *why;
+
+	/* We need the peer fd. */
+	if (tal_count(fds) == 0)
+		return 1;
+	
+	if (!fromwire_opening_negotiation_failed(msg, msg, NULL, &cs, &err)) {
+		peer_internal_error(peer,
+				    "bad OPENING_NEGOTIATION_FAILED %s",
+				    tal_hex(msg, msg));
+		return 0;
+	}
+
+	/* FIXME: Should we save addr in peer, or should gossipd remember it? */
+	msg = towire_gossipctl_handle_peer(msg, &peer->id, NULL, &cs,
+					   peer->gfeatures, peer->lfeatures,
+					   NULL);
+	subd_send_msg(openingd->ld->gossip, take(msg));
+	subd_send_fd(openingd->ld->gossip, fds[0]);
+
+	why = tal_strndup(peer, (const char *)err, tal_len(err));
+	log_unusual(peer->log, "Opening negotiation failed: %s", why);
+
+	/* This will free openingd, since that's peer->owner */
+	free_peer(peer, why);
+	return 0;
+}
+
 /* Peer has spontaneously exited from gossip due to open msg */
 static void peer_accept_channel(struct lightningd *ld,
 				const struct pubkey *peer_id,
@@ -2400,7 +2440,8 @@ static void peer_accept_channel(struct lightningd *ld,
 	peer_set_condition(peer, UNINITIALIZED, OPENINGD);
 	peer_set_owner(peer,
 		       new_peer_subd(ld, "lightning_openingd", peer,
-				     opening_wire_type_name, NULL,
+				     opening_wire_type_name,
+				     opening_negotiation_failed,
 				     take(&peer_fd), take(&gossip_fd), NULL));
 	if (!peer->owner) {
 		peer_fail_transient(peer, "Failed to subdaemon opening: %s",
@@ -2475,7 +2516,8 @@ static void peer_offer_channel(struct lightningd *ld,
 	peer_set_owner(fc->peer,
 		       new_peer_subd(ld,
 				     "lightning_openingd", fc->peer,
-				     opening_wire_type_name, NULL,
+				     opening_wire_type_name,
+				     opening_negotiation_failed,
 				     take(&peer_fd), take(&gossip_fd), NULL));
 	if (!fc->peer->owner) {
 		fc->peer = tal_free(fc->peer);

lightningd/peer_control.h

@@ -215,6 +215,9 @@ void peer_fail_permanent_str(struct peer *peer, const char *str TAKES);
 /* Permanent error, but due to internal problems, not peer. */
 void peer_internal_error(struct peer *peer, const char *fmt, ...);
 
+/* Peer has failed to open; return to gossipd. */
+void opening_failed(struct peer *peer, const u8 *msg TAKES);
+
 const char *peer_state_name(enum peer_state state);
 void peer_set_condition(struct peer *peer, enum peer_state oldstate,
 			enum peer_state state);

lightningd/subd.c

@@ -377,7 +377,6 @@ static bool log_status_fail(struct subd *sd,
 	case STATUS_FAIL_INTERNAL_ERROR:
 		name = "STATUS_FAIL_INTERNAL_ERROR";
 		goto log_str_broken;
-
 	/*
 	 * These errors happen when the other peer misbehaves:
 	 */

openingd/Makefile

@@ -59,7 +59,8 @@ OPENINGD_COMMON_OBJS :=				\
 	common/type_to_string.o			\
 	common/utils.o				\
 	common/utxo.o				\
-	common/version.o
+	common/version.o			\
+	common/wire_error.o
 
 $(LIGHTNINGD_OPENING_OBJS): $(LIGHTNINGD_HEADERS)
 

openingd/opening.c

@@ -5,6 +5,7 @@
 #include <ccan/breakpoint/breakpoint.h>
 #include <ccan/fdpass/fdpass.h>
 #include <ccan/structeq/structeq.h>
+#include <ccan/tal/str/str.h>
 #include <common/crypto_sync.h>
 #include <common/debug.h>
 #include <common/derive_basepoints.h>
@@ -17,6 +18,7 @@
 #include <common/status.h>
 #include <common/type_to_string.h>
 #include <common/version.h>
+#include <common/wire_error.h>
 #include <errno.h>
 #include <inttypes.h>
 #include <openingd/gen_opening_wire.h>
@@ -63,6 +65,34 @@ struct state {
 	const struct chainparams *chainparams;
 };
 
+/* For negotiation failures: we can still gossip with client. */
+static void negotiation_failed(struct state *state, const char *fmt, ...)
+{
+	va_list ap;
+	const char *errmsg;
+	u8 *msg;
+
+	va_start(ap, fmt);
+	errmsg = tal_vfmt(state, fmt, ap);
+	va_end(ap);
+
+	/* Make sure it's correct length for towire_. */
+	tal_resize(&errmsg, strlen(errmsg)+1);
+
+	/* Tell peer we're bailing on this channel. */
+	msg = towire_errorfmt(errmsg, &state->channel_id, "%s", errmsg);
+	sync_crypto_write(&state->cs, PEER_FD, take(msg));
+
+	/* Tell master we should return to gossiping. */
+	msg = towire_opening_negotiation_failed(state, &state->cs,
+						(const u8 *)errmsg);
+	wire_sync_write(REQ_FD, msg);
+	fdpass_send(REQ_FD, PEER_FD);
+
+	tal_free(state);
+	exit(0);
+}
+
 static void check_config_bounds(struct state *state,
 				const struct channel_config *remoteconf)
 {
@@ -75,9 +105,10 @@ static void check_config_bounds(struct state *state,
 	 * unreasonably large.
 	 */
 	if (remoteconf->to_self_delay > state->max_to_self_delay)
-		peer_failed(PEER_FD, &state->cs, &state->channel_id,
-			    "to_self_delay %u larger than %u",
-			    remoteconf->to_self_delay, state->max_to_self_delay);
+		negotiation_failed(state,
+				   "to_self_delay %u larger than %u",
+				   remoteconf->to_self_delay,
+				   state->max_to_self_delay);
 
 	/* BOLT #2:
 	 *
@@ -92,11 +123,11 @@ static void check_config_bounds(struct state *state,
 
 	/* Overflow check before capacity calc. */
 	if (remoteconf->channel_reserve_satoshis > state->funding_satoshis)
-		peer_failed(PEER_FD, &state->cs, &state->channel_id,
-			    "Invalid channel_reserve_satoshis %"PRIu64
-			    " for funding_satoshis %"PRIu64,
-			    remoteconf->channel_reserve_satoshis,
-			    state->funding_satoshis);
+		negotiation_failed(state,
+				   "Invalid channel_reserve_satoshis %"PRIu64
+				   " for funding_satoshis %"PRIu64,
+				   remoteconf->channel_reserve_satoshis,
+				   state->funding_satoshis);
 
 	/* Consider highest reserve. */
 	reserve_msat = remoteconf->channel_reserve_satoshis * 1000;
@@ -109,32 +140,32 @@ static void check_config_bounds(struct state *state,
 		capacity_msat = remoteconf->max_htlc_value_in_flight_msat;
 
 	if (remoteconf->htlc_minimum_msat * (u64)1000 > capacity_msat)
-		peer_failed(PEER_FD, &state->cs, &state->channel_id,
-			    "Invalid htlc_minimum_msat %"PRIu64
-			    " for funding_satoshis %"PRIu64
-			    " capacity_msat %"PRIu64,
-			    remoteconf->htlc_minimum_msat,
-			    state->funding_satoshis,
-			    capacity_msat);
+		negotiation_failed(state,
+				   "Invalid htlc_minimum_msat %"PRIu64
+				   " for funding_satoshis %"PRIu64
+				   " capacity_msat %"PRIu64,
+				   remoteconf->htlc_minimum_msat,
+				   state->funding_satoshis,
+				   capacity_msat);
 
 	if (capacity_msat < state->min_effective_htlc_capacity_msat)
-		peer_failed(PEER_FD, &state->cs, &state->channel_id,
-			    "Channel capacity with funding %"PRIu64" msat,"
-			    " reserves %"PRIu64"/%"PRIu64" msat,"
-			    " max_htlc_value_in_flight_msat %"PRIu64
-			    " is %"PRIu64" msat, which is below %"PRIu64" msat",
-			    state->funding_satoshis * 1000,
-			    remoteconf->channel_reserve_satoshis * 1000,
-			    state->localconf.channel_reserve_satoshis * 1000,
-			    remoteconf->max_htlc_value_in_flight_msat,
-			    capacity_msat,
-			    state->min_effective_htlc_capacity_msat);
+		negotiation_failed(state,
+				   "Channel capacity with funding %"PRIu64" msat,"
+				   " reserves %"PRIu64"/%"PRIu64" msat,"
+				   " max_htlc_value_in_flight_msat %"PRIu64
+				   " is %"PRIu64" msat, which is below %"PRIu64" msat",
+				   state->funding_satoshis * 1000,
+				   remoteconf->channel_reserve_satoshis * 1000,
+				   state->localconf.channel_reserve_satoshis * 1000,
+				   remoteconf->max_htlc_value_in_flight_msat,
+				   capacity_msat,
+				   state->min_effective_htlc_capacity_msat);
 
 	/* We don't worry about how many HTLCs they accept, as long as > 0! */
 	if (remoteconf->max_accepted_htlcs == 0)
-		peer_failed(PEER_FD, &state->cs, &state->channel_id,
-			    "max_accepted_htlcs %u invalid",
-			    remoteconf->max_accepted_htlcs);
+		negotiation_failed(state,
+				   "max_accepted_htlcs %u invalid",
+				   remoteconf->max_accepted_htlcs);
 
 	/* BOLT #2:
 	 *
@@ -298,7 +329,7 @@ static u8 *funder_channel(struct state *state,
 	 * The `temporary_channel_id` MUST be the same as the
 	 * `temporary_channel_id` in the `open_channel` message. */
 	if (!structeq(&id_in, &state->channel_id))
-		peer_failed(PEER_FD, &state->cs, &id_in,
+		peer_failed(PEER_FD, &state->cs, &state->channel_id,
 			    "accept_channel ids don't match: sent %s got %s",
 			    type_to_string(msg, struct channel_id, &id_in),
 			    type_to_string(msg, struct channel_id,
@@ -313,9 +344,9 @@ static u8 *funder_channel(struct state *state,
 	 * `open_channel`.
 	 */
 	if (minimum_depth > max_minimum_depth)
-		peer_failed(PEER_FD, &state->cs, &state->channel_id,
-			    "minimum_depth %u larger than %u",
-			    minimum_depth, max_minimum_depth);
+		negotiation_failed(state,
+				   "minimum_depth %u larger than %u",
+				   minimum_depth, max_minimum_depth);
 	check_config_bounds(state, state->remoteconf);
 
 	/* Now, ask create funding transaction to pay those two addresses. */
@@ -506,10 +537,11 @@ static u8 *fundee_channel(struct state *state,
 	 * unknown to the receiver.
 	 */
 	if (!structeq(&chain_hash, &state->chainparams->genesis_blockhash)) {
-		peer_failed(PEER_FD, &state->cs, &state->channel_id,
-			    "Unknown chain-hash %s",
-			    type_to_string(peer_msg, struct sha256_double,
-					   &chain_hash));
+		negotiation_failed(state,
+				   "Unknown chain-hash %s",
+				   type_to_string(peer_msg,
+						  struct sha256_double,
+						  &chain_hash));
 	}
 
 	/* BOLT #2 FIXME:
@@ -538,14 +570,14 @@ static u8 *fundee_channel(struct state *state,
 	 * too small for timely processing, or unreasonably large.
 	 */
 	if (state->feerate_per_kw < min_feerate)
-		peer_failed(PEER_FD, &state->cs, &state->channel_id,
-			    "feerate_per_kw %u below minimum %u",
-			    state->feerate_per_kw, min_feerate);
+		negotiation_failed(state,
+				   "feerate_per_kw %u below minimum %u",
+				   state->feerate_per_kw, min_feerate);
 
 	if (state->feerate_per_kw > max_feerate)
-		peer_failed(PEER_FD, &state->cs, &state->channel_id,
-			    "feerate_per_kw %u above maximum %u",
-			    state->feerate_per_kw, max_feerate);
+		negotiation_failed(state,
+				   "feerate_per_kw %u above maximum %u",
+				   state->feerate_per_kw, max_feerate);
 
 	set_reserve(&state->localconf.channel_reserve_satoshis,
 		    state->funding_satoshis);

openingd/opening_wire.csv

@@ -73,3 +73,11 @@ opening_fundee_reply,,feerate_per_kw,u32
 # The (encrypted) funding signed message: send this and we're committed.
 opening_fundee_reply,,msglen,u16
 opening_fundee_reply,,funding_signed_msg,msglen*u8
+
+# We disagreed with opening parameters, but peer is ok for gossip (+ peerfd)
+opening_negotiation_failed,6010
+opening_negotiation_failed,,crypto_state,struct crypto_state
+opening_negotiation_failed,,len,u16
+# FIXME: string support!
+opening_negotiation_failed,,msg,len*u8
+