Handle closed channels better.

channeld/channeld.c

@@ -4987,8 +4987,7 @@ static u8 *to_bytearr(const tal_t *ctx,
 }
 
 static void peer_reconnect(struct peer *peer,
-			   const struct secret *last_remote_per_commit_secret,
-			   bool reestablish_only)
+			   const struct secret *last_remote_per_commit_secret)
 {
 	struct channel_id channel_id;
 	/* Note: BOLT #2 uses these names! */
@@ -5146,14 +5145,6 @@ static void peer_reconnect(struct peer *peer,
 	do {
 		clean_tmpctx();
 		msg = peer_read(tmpctx, peer->pps);
-
-		/* connectd promised us the msg was reestablish? */
-		if (reestablish_only) {
-			if (fromwire_peektype(msg) != WIRE_CHANNEL_REESTABLISH)
-				status_failed(STATUS_FAIL_INTERNAL_ERROR,
-					      "Expected reestablish, got: %s",
-					      tal_hex(tmpctx, msg));
-		}
 	} while (handle_peer_error_or_warning(peer->pps, msg) ||
 		 capture_premature_msg(&premature_msgs, msg));
 
@@ -5501,23 +5492,6 @@ static void peer_reconnect(struct peer *peer,
 			set_channel_type(peer->channel, type);
 	}
 
-	/* Now stop, we've been polite long enough. */
-	if (reestablish_only) {
-		/* We've reestablished! */
-		wire_sync_write(MASTER_FD,
-				take(towire_channeld_reestablished(NULL)));
-
-		/* If we were successfully closing, we still go to closingd. */
-		if (shutdown_complete(peer)) {
-			send_shutdown_complete(peer);
-			daemon_shutdown();
-			exit(0);
-		}
-		peer_failed_err(peer->pps,
-				&peer->channel_id,
-				"Channel is already closed");
-	}
-
 	tal_free(send_tlvs);
 
 	/* We've reestablished! */
@@ -6103,7 +6077,6 @@ static void init_channel(struct peer *peer)
 	u32 minimum_depth, lease_expiry;
 	struct secret last_remote_per_commit_secret;
 	struct penalty_base *pbases;
-	bool reestablish_only;
 	struct channel_type *channel_type;
 
 	assert(!(fcntl(MASTER_FD, F_GETFL) & O_NONBLOCK));
@@ -6159,7 +6132,6 @@ static void init_channel(struct peer *peer)
 				    &channel_type,
 				    &peer->dev_disable_commit,
 				    &pbases,
-				    &reestablish_only,
 				    &peer->experimental_upgrade,
 				    &peer->splice_state->inflights,
 				    &peer->local_alias)) {
@@ -6249,10 +6221,7 @@ static void init_channel(struct peer *peer)
 
 	/* OK, now we can process peer messages. */
 	if (reconnected)
-		peer_reconnect(peer, &last_remote_per_commit_secret,
-			       reestablish_only);
-	else
-		assert(!reestablish_only);
+		peer_reconnect(peer, &last_remote_per_commit_secret);
 
 	/* If we have a messages to send, send them immediately */
 	if (fwd_msg)

channeld/channeld_wire.csv

@@ -72,7 +72,6 @@ msgdata,channeld_init,desired_type,channel_type,
 msgdata,channeld_init,dev_disable_commit,?u32,
 msgdata,channeld_init,num_penalty_bases,u32,
 msgdata,channeld_init,pbases,penalty_base,num_penalty_bases
-msgdata,channeld_init,reestablish_only,bool,
 msgdata,channeld_init,experimental_upgrade,bool,
 msgdata,channeld_init,num_inflights,u16,
 msgdata,channeld_init,inflights,inflight,num_inflights

lightningd/channel.c

@@ -88,19 +88,26 @@ static void destroy_channel(struct channel *channel)
 	list_del_from(&channel->peer->channels, &channel->list);
 }
 
-void delete_channel(struct channel *channel STEALS)
+void delete_channel(struct channel *channel STEALS, bool completely_eliminate)
 {
 	const u8 *msg;
-
 	struct peer *peer = channel->peer;
-	if (channel->dbid != 0)
-		wallet_channel_close(channel->peer->ld->wallet, channel);
+	struct lightningd *ld = peer->ld;
+
+	if (channel->dbid != 0) {
+		wallet_channel_close(ld->wallet, channel);
+		/* Never open at all, not ours. */
+		if (completely_eliminate)
+			wallet_channel_delete(ld->wallet, channel);
+		else
+			wallet_load_one_closed_channel(ld->wallet, ld->closed_channels, channel->dbid);
+	}
 
 	/* Tell the hsm to forget the channel, needs to be after it's
 	 * been forgotten here */
-	if (hsm_capable(channel->peer->ld, WIRE_HSMD_FORGET_CHANNEL)) {
-		msg = towire_hsmd_forget_channel(NULL, &channel->peer->id, channel->dbid);
-		msg = hsm_sync_req(tmpctx, channel->peer->ld, take(msg));
+	if (hsm_capable(ld, WIRE_HSMD_FORGET_CHANNEL)) {
+		msg = towire_hsmd_forget_channel(NULL, &peer->id, channel->dbid);
+		msg = hsm_sync_req(tmpctx, ld, take(msg));
 		if (!fromwire_hsmd_forget_channel_reply(msg))
 			fatal("HSM gave bad hsm_forget_channel_reply %s", tal_hex(msg, msg));
 	}
@@ -1035,7 +1042,7 @@ static void channel_fail_perm(struct channel *channel,
 	drop_to_chain(ld, channel, false, spent_by);
 
 	if (channel_state_open_uncommitted(channel->state))
-		delete_channel(channel);
+		delete_channel(channel, false);
 }
 
 void channel_fail_permanent(struct channel *channel,
@@ -1093,7 +1100,7 @@ void channel_fail_forget(struct channel *channel, const char *fmt, ...)
 		channel->error = towire_errorfmt(channel,
 						 &channel->cid, "%s", why);
 
-	delete_channel(channel);
+	delete_channel(channel, false);
 	tal_free(why);
 }
 
@@ -1162,7 +1169,7 @@ void channel_internal_error(struct channel *channel, const char *fmt, ...)
 	/* Nothing ventured, nothing lost! */
 	if (channel_state_uncommitted(channel->state)) {
 		channel_set_owner(channel, NULL);
-		delete_channel(channel);
+		delete_channel(channel, false);
 		return;
 	}
 

lightningd/channel.h

@@ -479,7 +479,8 @@ channel_current_inflight(const struct channel *channel);
 /* What's the last feerate used for a funding tx on this channel? */
 u32 channel_last_funding_feerate(const struct channel *channel);
 
-void delete_channel(struct channel *channel STEALS);
+/* Only set completely_eliminate for never-existed channels */
+void delete_channel(struct channel *channel STEALS, bool completely_eliminate);
 
 const char *channel_state_name(const struct channel *channel);
 const char *channel_state_str(enum channel_state state);

lightningd/channel_control.c

@@ -1,4 +1,5 @@
 #include "config.h"
+#include <ccan/asort/asort.h>
 #include <ccan/cast/cast.h>
 #include <ccan/mem/mem.h>
 #include <ccan/tal/str/str.h>
@@ -362,7 +363,7 @@ static void handle_splice_abort(struct lightningd *ld,
 		return;
 	}
 
-	if (peer_start_channeld(channel, pfd, NULL, false, false)) {
+	if (peer_start_channeld(channel, pfd, NULL, false)) {
 		subd_send_msg(ld->connectd,
 			      take(towire_connectd_peer_connect_subd(NULL,
 			      					     &peer->id,
@@ -1321,7 +1322,7 @@ static void forget(struct channel *channel)
 	channel->forgets = tal_arr(channel, struct command *, 0);
 
 	/* Forget the channel. */
-	delete_channel(channel);
+	delete_channel(channel, false);
 
 	for (size_t i = 0; i < tal_count(forgets); i++) {
 		assert(!forgets[i]->json_stream);
@@ -1640,8 +1641,7 @@ static void channel_control_errmsg(struct channel *channel,
 bool peer_start_channeld(struct channel *channel,
 			 struct peer_fd *peer_fd,
 			 const u8 *fwd_msg,
-			 bool reconnected,
-			 bool reestablish_only)
+			 bool reconnected)
 {
 	u8 *initmsg;
 	int hsmfd;
@@ -1864,7 +1864,6 @@ bool peer_start_channeld(struct channel *channel,
 					     ? NULL
 					     : (u32 *)&ld->dev_disable_commit,
 				       pbases,
-				       reestablish_only,
 				       ld->experimental_upgrade_protocol,
 				       cast_const2(const struct inflight **,
 						   inflights),
@@ -1927,18 +1926,8 @@ is_fundee_should_forget(struct lightningd *ld,
 			struct channel *channel,
 			u32 block_height)
 {
-	/* BOLT #2:
-	 *
-	 * A non-funding node (fundee):
-	 *   - SHOULD forget the channel if it does not see the
-	 * correct funding transaction after a timeout of 2016 blocks.
-	 */
-	u32 max_funding_unconfirmed;
-
-	if (ld->developer)
-		max_funding_unconfirmed = ld->dev_max_funding_unconfirmed;
-	else
-		max_funding_unconfirmed = 2016;
+	/* 2016 by default */
+	u32 max_funding_unconfirmed = ld->dev_max_funding_unconfirmed;
 
 	/* Only applies if we are fundee. */
 	if (channel->opener == LOCAL)
@@ -1969,30 +1958,55 @@ is_fundee_should_forget(struct lightningd *ld,
 	return true;
 }
 
+/* We want in ascending order, so oldest channels first */
+static int cmp_channel_start(struct channel *const *a, struct channel *const *b, void *unused)
+{
+	if ((*a)->first_blocknum < (*b)->first_blocknum)
+		return -1;
+	else if ((*a)->first_blocknum > (*b)->first_blocknum)
+		return 1;
+	return 0;
+}
+
 /* Notify all channels of new blocks. */
 void channel_notify_new_block(struct lightningd *ld,
 			      u32 block_height)
 {
 	struct peer *peer;
 	struct channel *channel;
-	struct channel **to_forget = tal_arr(NULL, struct channel *, 0);
+	struct channel **to_forget = tal_arr(tmpctx, struct channel *, 0);
 	size_t i;
 	struct peer_node_id_map_iter it;
 
+	/* BOLT #2:
+	 *
+	 * A non-funding node (fundee):
+	 *   - SHOULD forget the channel if it does not see the
+	 * correct funding transaction after a timeout of 2016 blocks.
+	 */
+
+	/* But we give some latitude!  Boltz reported that after a months-long
+	 * fee spike, they had some failed opens when the tx finally got mined.
+	 * They're individually cheap, keep the latest 100. */
+	size_t forgettable_channels_to_keep = 100;
+
+	/* For testing */
+	if (ld->dev_max_funding_unconfirmed != 2016)
+		forgettable_channels_to_keep = 1;
+
 	/* FIXME: keep separate block-aware channel structure instead? */
 	for (peer = peer_node_id_map_first(ld->peers, &it);
 	     peer;
 	     peer = peer_node_id_map_next(ld->peers, &it)) {
 		list_for_each(&peer->channels, channel, list) {
 			if (channel_state_uncommitted(channel->state))
 				continue;
-			if (is_fundee_should_forget(ld, channel, block_height)) {
+			if (is_fundee_should_forget(ld, channel, block_height))
 				tal_arr_expand(&to_forget, channel);
-			} else
-				/* Let channels know about new blocks,
-				 * required for lease updates */
-				try_update_blockheight(ld, channel,
-						       block_height);
+
+			/* Let channels know about new blocks,
+			 * required for lease updates */
+			try_update_blockheight(ld, channel, block_height);
 		}
 	}
 
@@ -2004,7 +2018,11 @@ void channel_notify_new_block(struct lightningd *ld,
 	 * just the freeing of the channel that occurs, but the
 	 * potential destruction of the peer that invalidates
 	 * memory the inner loop is accessing. */
-	for (i = 0; i < tal_count(to_forget); ++i) {
+	if (tal_count(to_forget) < forgettable_channels_to_keep)
+		return;
+
+	asort(to_forget, tal_count(to_forget), cmp_channel_start, NULL);
+	for (i = 0; i + forgettable_channels_to_keep < tal_count(to_forget); ++i) {
 		channel = to_forget[i];
 		/* Report it first. */
 		log_unusual(channel->log,
@@ -2017,11 +2035,9 @@ void channel_notify_new_block(struct lightningd *ld,
 			    block_height - channel->first_blocknum,
 			    fmt_bitcoin_txid(tmpctx, &channel->funding.txid));
 		/* FIXME: Send an error packet for this case! */
-		/* And forget it. */
-		delete_channel(channel);
+		/* And forget it. COMPLETELY. */
+		delete_channel(channel, true);
 	}
-
-	tal_free(to_forget);
 }
 
 /* Since this could vanish while we're checking with bitcoind, we need to save

lightningd/channel_control.h

@@ -13,8 +13,7 @@ struct peer;
 bool peer_start_channeld(struct channel *channel,
 			 struct peer_fd *peer_fd,
 			 const u8 *fwd_msg,
-			 bool reconnected,
-			 bool reestablish_only);
+			 bool reconnected);
 
 /* Send message to channeld (if connected) to tell it about depth
  * c.f. dualopen_tell_depth! */

lightningd/closed_channel.c

@@ -10,6 +10,11 @@
 #include <lightningd/peer_control.h>
 #include <wallet/wallet.h>
 
+size_t hash_cid(const struct channel_id *cid)
+{
+	return siphash24(siphash_seed(), cid->id, sizeof(cid->id));
+}
+
 static void json_add_closed_channel(struct json_stream *response,
 				    const char *fieldname,
 				    const struct closed_channel *channel)
@@ -90,7 +95,8 @@ static struct command_result *json_listclosedchannels(struct command *cmd,
 {
 	struct node_id *peer_id;
 	struct json_stream *response;
-	struct closed_channel **chans;
+	struct closed_channel *cc;
+	struct closed_channel_map_iter it;
 
 	if (!param(cmd, buffer, params,
 		   p_opt("id", param_node_id, &peer_id),
@@ -100,15 +106,16 @@ static struct command_result *json_listclosedchannels(struct command *cmd,
 	response = json_stream_success(cmd);
 	json_array_start(response, "closedchannels");
 
-	chans = wallet_load_closed_channels(cmd, cmd->ld->wallet);
-	for (size_t i = 0; i < tal_count(chans); i++) {
+	for (cc = closed_channel_map_first(cmd->ld->closed_channels, &it);
+	     cc;
+	     cc = closed_channel_map_next(cmd->ld->closed_channels, &it)) {
 		if (peer_id) {
-			if (!chans[i]->peer_id)
+			if (!cc->peer_id)
 				continue;
-			if (!node_id_eq(chans[i]->peer_id, peer_id))
+			if (!node_id_eq(cc->peer_id, peer_id))
 				continue;
 		}
-		json_add_closed_channel(response, NULL, chans[i]);
+		json_add_closed_channel(response, NULL, cc);
 	}
 	json_array_end(response);
 

lightningd/closed_channel.h

@@ -28,6 +28,23 @@ struct closed_channel {
 	enum state_change state_change_cause;
 	bool leased;
 	u64 last_stable_connection;
+	/* NULL for older closed channels */
+	const struct shachain *their_shachain;
 };
 
+static inline const struct channel_id *keyof_closed_channel(const struct closed_channel *cc)
+{
+	return &cc->cid;
+}
+
+size_t hash_cid(const struct channel_id *cid);
+
+static inline bool closed_channel_eq_cid(const struct closed_channel *cc, const struct channel_id *cid)
+{
+	return channel_id_eq(cid, &cc->cid);
+}
+
+HTABLE_DEFINE_NODUPS_TYPE(struct closed_channel, keyof_closed_channel, hash_cid, closed_channel_eq_cid,
+			  closed_channel_map);
+
 #endif /* LIGHTNING_LIGHTNINGD_CLOSED_CHANNEL_H */