Prepare 23.05.1

CHANGELOG.md

@@ -3,6 +3,24 @@ All notable changes to this project will be documented in this file.
 
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
+## [23.05.1] - 2023-06-05: "Austin Texas Agreement(ATXA) II"
+
+Bugfix release for bad issues found since 23.05 which can't wait for 23.08.
+
+### Fixed
+
+ - Fixed crash (memory corruption!) in `listtransactions` ([#6304])
+ - Don't crash on gossip store deletion fail ([#6297])
+ - Fix incompatibility with LND which prevented us opening private channels ([#6304])
+
+### EXPERIMENTAL
+
+ - Fixed crash in dual-funding. ([#6273])
+
+[#6273]: https://github.com/ElementsProject/lightning/pull/6273
+[#6304]: https://github.com/ElementsProject/lightning/pull/6304
+[#6297]: https://github.com/ElementsProject/lightning/pull/6297
+[#6304]: https://github.com/ElementsProject/lightning/pull/6304
 
 ## [23.05] - 2023-05-10: "Austin Texas Agreement(ATXA)"
 

gossipd/gossip_store.c

@@ -598,15 +598,27 @@ static u32 delete_by_index(struct gossip_store *gs, u32 index, int type)
 	/* Should never try to overwrite version */
 	assert(index);
 
-#if DEVELOPER
-	const u8 *msg = gossip_store_get(tmpctx, gs, index);
-	assert(fromwire_peektype(msg) == type);
-#endif
+	/* FIXME: debugging a gs->len overrun issue reported in #6270 */
+	if (pread(gs->fd, &hdr, sizeof(hdr), index) != sizeof(hdr)) {
+		status_broken("gossip_store overrun during delete @%u type: %i"
+			      " gs->len: %"PRIu64, index, type, gs->len);
+		return index;
+	}
+	if (index + sizeof(struct gossip_hdr) +
+	    be16_to_cpu(hdr.belen) > gs->len) {
+		status_broken("gossip_store overrun during delete @%u type: %i"
+			      " gs->len: %"PRIu64, index, type, gs->len);
+		return index;
+	}
 
-	if (pread(gs->fd, &hdr, sizeof(hdr), index) != sizeof(hdr))
-		status_failed(STATUS_FAIL_INTERNAL_ERROR,
-			      "Failed reading flags & len to delete @%u: %s",
-			      index, strerror(errno));
+	const u8 *msg = gossip_store_get(tmpctx, gs, index);
+	if(fromwire_peektype(msg) != type) {
+		status_broken("asked to delete type %i @%u but store contains "
+			      "%i (gs->len=%"PRIu64"): %s",
+			      type, index, fromwire_peektype(msg),
+			      gs->len, tal_hex(tmpctx, msg));
+		return index;
+	}
 
 	assert((be16_to_cpu(hdr.beflags) & GOSSIP_STORE_DELETED_BIT) == 0);
 	hdr.beflags |= cpu_to_be16(GOSSIP_STORE_DELETED_BIT);

lightningd/dual_open_control.c

@@ -2602,7 +2602,7 @@ static void validate_input_unspent(struct bitcoind *bitcoind,
 		struct bitcoin_outpoint outpoint;
 
 		assert(pv->next_index > 0);
-		wally_tx_input_get_outpoint(&pv->psbt->tx->inputs[pv->next_index - 1],
+		wally_psbt_input_get_outpoint(&pv->psbt->inputs[pv->next_index - 1],
 					    &outpoint);
 
 		err = tal_fmt(pv, "Requested only confirmed"
@@ -2629,8 +2629,8 @@ static void validate_input_unspent(struct bitcoind *bitcoind,
 		if (serial % 2 != pv->role_to_validate)
 			continue;
 
-		wally_tx_input_get_outpoint(&pv->psbt->tx->inputs[i],
-					    &outpoint);
+		wally_psbt_input_get_outpoint(&pv->psbt->inputs[i],
+					      &outpoint);
 		pv->next_index = i + 1;
 
 		/* Confirm input is in a block */

lightningd/opening_control.c

@@ -964,8 +964,7 @@ bool peer_start_openingd(struct peer *peer, struct peer_fd *peer_fd)
 				   feerate_min(peer->ld, NULL),
 				   feerate_max(peer->ld, NULL),
 				   IFDEV(peer->ld->dev_force_tmp_channel_id, NULL),
-				   peer->ld->config.allowdustreserve,
-				   !deprecated_apis);
+				   peer->ld->config.allowdustreserve);
 	subd_send_msg(uc->open_daemon, take(msg));
 	return true;
 }

openingd/openingd.c

@@ -102,9 +102,6 @@ struct state {
 	struct amount_sat *reserve;
 
 	bool allowdustreserve;
-
-	/* Are we allowed to set option_scid_alias is channel_type? */
-	bool can_set_scid_alias_channel_type;
 };
 
 /*~ If we can't agree on parameters, we fail to open the channel.
@@ -300,6 +297,33 @@ static void set_remote_upfront_shutdown(struct state *state,
 		peer_failed_err(state->pps, &state->channel_id, "%s", err);
 }
 
+/* Since we can't send OPT_SCID_ALIAS due to compat issues, intuit whether
+ * we really actually want it anyway, we just can't say that. */
+static bool intuit_scid_alias_type(struct state *state, u8 channel_flags,
+				   bool peer_sent_channel_type)
+{
+	/* Don't need to intuit if actually set */
+	if (channel_type_has(state->channel_type, OPT_SCID_ALIAS))
+		return false;
+
+	/* Old clients didn't send channel_type at all */
+	if (!peer_sent_channel_type)
+		return false;
+
+	/* Modern peer: no intuit hacks necessary. */
+	if (channel_type_has(state->channel_type, OPT_ANCHORS_ZERO_FEE_HTLC_TX))
+		return false;
+
+	/* Public channel: don't want OPT_SCID_ALIAS which means "only use
+	 * alias". */
+	if (channel_flags & CHANNEL_FLAGS_ANNOUNCE_CHANNEL)
+		return false;
+
+	/* If we both support it, presumably we want it? */
+	return feature_negotiated(state->our_features, state->their_features,
+				  OPT_SCID_ALIAS);
+}
+
 /* We start the 'open a channel' negotation with the supplied peer, but
  * stop when we get to the part where we need the funding txid */
 static u8 *funder_channel_start(struct state *state, u8 channel_flags)
@@ -336,9 +360,16 @@ static u8 *funder_channel_start(struct state *state, u8 channel_flags)
 						   state->their_features);
 
 	/* Spec says we should use the option_scid_alias variation if we
-	 * want them to *only* use the scid_alias.  But we didn't accept this
-	 * in CLN prior to v23.05, so we don't send that in deprecated mode! */
-	if (state->can_set_scid_alias_channel_type) {
+	 * want them to *only* use the scid_alias (which we do for unannounced
+	 * channels!).
+	 *
+	 * But:
+	 * 1. We didn't accept this in CLN prior to v23.05.
+	 * 2. LND won't accept that without OPT_ANCHORS_ZERO_FEE_HTLC_TX.
+	 *
+	 * So we keep it off for now, until anchors merge.
+	 */
+	if (channel_type_has(state->channel_type, OPT_ANCHORS_ZERO_FEE_HTLC_TX)) {
 		if (!(channel_flags & CHANNEL_FLAGS_ANNOUNCE_CHANNEL))
 			channel_type_set_scid_alias(state->channel_type);
 	}
@@ -433,14 +464,26 @@ static u8 *funder_channel_start(struct state *state, u8 channel_flags)
 	 *   `open_channel`, and they are not equal types:
 	 *    - MUST reject the channel.
 	 */
-	if (accept_tlvs->channel_type
-	    && !featurebits_eq(accept_tlvs->channel_type,
-			       state->channel_type->features)) {
-		negotiation_failed(state,
-				   "Return unoffered channel_type: %s",
-				   fmt_featurebits(tmpctx,
-						   accept_tlvs->channel_type));
-		return NULL;
+	if (accept_tlvs->channel_type) {
+		/* Except that v23.05 could set OPT_SCID_ALIAS in reply! */
+		struct channel_type *atype;
+
+		atype = channel_type_from(tmpctx, accept_tlvs->channel_type);
+		if (!channel_type_has(atype, OPT_ANCHORS_ZERO_FEE_HTLC_TX))
+			featurebits_unset(&atype->features, OPT_SCID_ALIAS);
+
+		if (!channel_type_eq(atype, state->channel_type)) {
+			negotiation_failed(state,
+					   "Return unoffered channel_type: %s",
+					   fmt_featurebits(tmpctx,
+							   accept_tlvs->channel_type));
+			return NULL;
+		}
+
+		/* If they "accepted" SCID_ALIAS, roll with it. */
+		tal_free(state->channel_type);
+		state->channel_type = channel_type_from(state,
+							accept_tlvs->channel_type);
 	}
 
 	/* BOLT #2:
@@ -547,6 +590,12 @@ static u8 *funder_channel_start(struct state *state, u8 channel_flags)
 		       "Funding channel start: awaiting funding_txid with output to %s",
 		       tal_hex(tmpctx, funding_output_script));
 
+	/* Backwards/cross compat hack */
+	if (intuit_scid_alias_type(state, channel_flags,
+				   accept_tlvs->channel_type != NULL)) {
+		channel_type_set_scid_alias(state->channel_type);
+	}
+
 	return towire_openingd_funder_start_reply(state,
 						  funding_output_script,
 						  feature_negotiated(
@@ -860,6 +909,7 @@ static u8 *fundee_channel(struct state *state, const u8 *open_channel_msg)
 	struct tlv_accept_channel_tlvs *accept_tlvs;
 	struct tlv_open_channel_tlvs *open_tlvs;
 	struct amount_sat *reserve;
+	bool open_channel_had_channel_type;
 
 	/* BOLT #2:
 	 *
@@ -901,6 +951,7 @@ static u8 *fundee_channel(struct state *state, const u8 *open_channel_msg)
 	 *     - if `type` includes `option_zeroconf` and it does not trust the sender to open an unconfirmed channel.
 	 */
 	if (open_tlvs->channel_type) {
+		open_channel_had_channel_type = true;
 		state->channel_type =
 			channel_type_accept(state,
 					    open_tlvs->channel_type,
@@ -914,21 +965,13 @@ static u8 *fundee_channel(struct state *state, const u8 *open_channel_msg)
 							   open_tlvs->channel_type));
 			return NULL;
 		}
-
-		/* If we're not using scid_alias in channel type, intuit it here.
-		 * We have to do this, because we used not to accept that bit, so older
-		 * clients won't send it! */
-		if (!state->can_set_scid_alias_channel_type
-		    && !(channel_flags & CHANNEL_FLAGS_ANNOUNCE_CHANNEL)
-		    && feature_negotiated(state->our_features, state->their_features,
-					  OPT_SCID_ALIAS)) {
-			channel_type_set_scid_alias(state->channel_type);
-		}
-	} else
+	} else {
+		open_channel_had_channel_type = false;
 		state->channel_type
 			= default_channel_type(state,
 					       state->our_features,
 					       state->their_features);
+	}
 
 	/* BOLT #2:
 	 *
@@ -1143,6 +1186,12 @@ static u8 *fundee_channel(struct state *state, const u8 *open_channel_msg)
 					       &state->channel_id),
 				type_to_string(msg, struct channel_id, &id_in));
 
+	/* Backwards/cross compat hack */
+	if (intuit_scid_alias_type(state, channel_flags,
+				   open_channel_had_channel_type)) {
+		channel_type_set_scid_alias(state->channel_type);
+	}
+
 	/*~ Channel is ready; Report the channel parameters to the signer. */
 	msg = towire_hsmd_ready_channel(NULL,
 				       false,	/* is_outbound */
@@ -1475,8 +1524,7 @@ int main(int argc, char *argv[])
 				    &state->minimum_depth,
 				    &state->min_feerate, &state->max_feerate,
 				    &force_tmp_channel_id,
-				    &state->allowdustreserve,
-				    &state->can_set_scid_alias_channel_type))
+				    &state->allowdustreserve))
 		master_badmsg(WIRE_OPENINGD_INIT, msg);
 
 #if DEVELOPER

openingd/openingd_wire.csv

@@ -28,8 +28,6 @@ msgdata,openingd_init,dev_temporary_channel_id,?byte,32
 # reserves? This is explicitly required by the spec for safety
 # reasons, but some implementations and users keep asking for it.
 msgdata,openingd_init,allowdustreserve,bool,
-# Core LN prior to 23.05 didn't like this bit set!
-msgdata,openingd_init,can_set_scid_alias_channel_type,bool,
 
 # Openingd->master: they offered channel, should we continue?
 msgtype,openingd_got_offer,6005

wallet/wallet.c

@@ -4809,11 +4809,7 @@ bool wallet_forward_delete(struct wallet *w,
 struct wallet_transaction *wallet_transactions_get(struct wallet *w, const tal_t *ctx)
 {
 	struct db_stmt *stmt;
-	struct wallet_transaction *cur = NULL, *txs = tal_arr(ctx, struct wallet_transaction, 0);
-	struct bitcoin_txid last;
-
-	/* Make sure we can check for changing txids */
-	memset(&last, 0, sizeof(last));
+	struct wallet_transaction *txs = tal_arr(ctx, struct wallet_transaction, 0);
 
 	stmt = db_prepare_v2(
 	    w->db,
@@ -4822,82 +4818,31 @@ struct wallet_transaction *wallet_transactions_get(struct wallet *w, const tal_t
 		", t.rawtx"
 		", t.blockheight"
 		", t.txindex"
-		", a.location"
-		", a.idx as ann_idx"
-		", a.type as annotation_type"
-		", c.scid"
 		" FROM"
 		"  transactions t LEFT JOIN"
-		"  transaction_annotations a ON (a.txid = t.id) LEFT JOIN"
-		"  channels c ON (a.channel = c.id) "
+		"  channels c ON (t.channel_id = c.id) "
 		"ORDER BY t.blockheight, t.txindex ASC"));
 	db_query_prepared(stmt);
 
 	while (db_step(stmt)) {
-		struct bitcoin_txid curtxid;
-		db_col_txid(stmt, "t.id", &curtxid);
-
-		/* If this is a new entry, allocate it in the array and set
-		 * the common fields (all fields from the transactions table. */
-		if (!bitcoin_txid_eq(&last, &curtxid)) {
-			last = curtxid;
-			tal_resize(&txs, tal_count(txs) + 1);
-			cur = &txs[tal_count(txs) - 1];
-			db_col_txid(stmt, "t.id", &cur->id);
-			cur->tx = db_col_tx(txs, stmt, "t.rawtx");
-			cur->rawtx = db_col_arr(txs, stmt, "t.rawtx", u8);
-			/* TX may be unconfirmed. */
-			if (!db_col_is_null(stmt, "t.blockheight")) {
-				cur->blockheight
-					= db_col_int(stmt, "t.blockheight");
-				if (!db_col_is_null(stmt, "t.txindex")) {
-					cur->txindex
-						= db_col_int(stmt, "t.txindex");
-				} else {
-					cur->txindex = 0;
-				}
+		struct wallet_transaction *cur;
+
+		tal_resize(&txs, tal_count(txs) + 1);
+		cur = &txs[tal_count(txs) - 1];
+		db_col_txid(stmt, "t.id", &cur->id);
+		cur->tx = db_col_tx(txs, stmt, "t.rawtx");
+		cur->rawtx = db_col_arr(txs, stmt, "t.rawtx", u8);
+		if (!db_col_is_null(stmt, "t.blockheight")) {
+			cur->blockheight = db_col_int(stmt, "t.blockheight");
+			if (!db_col_is_null(stmt, "t.txindex")) {
+				cur->txindex = db_col_int(stmt, "t.txindex");
 			} else {
-				db_col_ignore(stmt, "t.txindex");
-				cur->blockheight = 0;
 				cur->txindex = 0;
 			}
-			cur->output_annotations = tal_arrz(txs, struct tx_annotation, cur->tx->wtx->num_outputs);
-			cur->input_annotations = tal_arrz(txs, struct tx_annotation, cur->tx->wtx->num_inputs);
-		}
-
-		/* This should always be set by the above if-statement,
-		 * otherwise we have a txid of all 0x00 bytes... */
-		assert(cur != NULL);
-
-		/* Check if we have any annotations. If there are none the
-		 * fields are all set to null */
-		if (!db_col_is_null(stmt, "a.location")) {
-			enum wallet_tx_annotation_type loc
-				= db_col_int(stmt, "a.location");
-			int idx = db_col_int(stmt, "ann_idx");
-			struct tx_annotation *ann;
-
-			/* Select annotation from array to fill in. */
-			switch (loc) {
-			case OUTPUT_ANNOTATION:
-				ann = &cur->output_annotations[idx];
-				goto got_ann;
-			case INPUT_ANNOTATION:
-				ann = &cur->input_annotations[idx];
-				goto got_ann;
-			}
-			fatal("Transaction annotations are only available for inputs and outputs. Value %d", loc);
-
-		got_ann:
-			ann->type = db_col_int(stmt, "annotation_type");
-			if (!db_col_is_null(stmt, "c.scid"))
-				db_col_short_channel_id(stmt, "c.scid", &ann->channel);
-			else
-				ann->channel.u64 = 0;
 		} else {
-			db_col_ignore(stmt, "ann_idx");
-			db_col_ignore(stmt, "annotation_type");
-			db_col_ignore(stmt, "c.scid");
+			db_col_ignore(stmt, "t.txindex");
+			cur->blockheight = 0;
+			cur->txindex = 0;
 		}
 	}
 	tal_free(stmt);

wallet/wallet.h

@@ -406,12 +406,6 @@ struct wallet_transaction {
 
 	/* Fully parsed transaction */
 	const struct bitcoin_tx *tx;
-
-	/* tal_arr containing the annotation types, if any, for the respective
-	 * inputs and outputs. 0 if there are no annotations for the
-	 * element. */
-	struct tx_annotation *input_annotations;
-	struct tx_annotation *output_annotations;
 };
 
 /**