lightningd: require local_alias in new_channel().

We allowed NULL for stub channels, but just don't put the stub scid
into the hash tables.  This cleans up all the callers to make it
clear this is a non-optional parameter.

We opencode channel_set_random_local_alias, since there's only one caller now.

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

Author: rustyrussell

lightningd/channel.c

@@ -268,16 +268,6 @@ static void chanmap_add(struct lightningd *ld,
 	tal_add_destructor2(scc, destroy_scid_to_channel, ld);
 }
 
-static void channel_set_random_local_alias(struct channel *channel)
-{
-	assert(channel->alias[LOCAL] == NULL);
-	channel->alias[LOCAL] = tal(channel, struct short_channel_id);
-	*channel->alias[LOCAL] = random_scid();
-	/* We don't check for uniqueness.  We would crash on a clash, but your machine is
-	 * probably broken beyond repair if it gets two equal 64 bit numbers */
-	chanmap_add(channel->peer->ld, channel, *channel->alias[LOCAL]);
-}
-
 void channel_set_scid(struct channel *channel, const struct short_channel_id *new_scid)
 {
 	struct lightningd *ld = channel->peer->ld;
@@ -355,8 +345,12 @@ struct channel *new_unsaved_channel(struct peer *peer,
 		= CLOSING_FEE_NEGOTIATION_STEP_UNIT_PERCENTAGE;
 	channel->shutdown_wrong_funding = NULL;
 	channel->closing_feerate_range = NULL;
-	channel->alias[REMOTE] = channel->alias[LOCAL] = NULL;
-	channel_set_random_local_alias(channel);
+	channel->alias[REMOTE] = NULL;
+	channel->alias[LOCAL] = tal(channel, struct short_channel_id);
+	*channel->alias[LOCAL] = random_scid();
+	/* We don't check for uniqueness.  We would crash on a clash, but your machine is
+	 * probably broken beyond repair if it gets two equal 64 bit numbers */
+	chanmap_add(channel->peer->ld, channel, *channel->alias[LOCAL]);
 
 	channel->shutdown_scriptpubkey[REMOTE] = NULL;
 	channel->last_was_revoke = false;
@@ -477,9 +471,9 @@ struct channel *new_channel(struct peer *peer, u64 dbid,
 			    struct amount_sat our_funds,
 			    bool remote_channel_ready,
 			    /* NULL or stolen */
-			    struct short_channel_id *scid,
+			    struct short_channel_id *scid TAKES,
 			    struct short_channel_id *old_scids TAKES,
-			    struct short_channel_id *alias_local TAKES,
+			    struct short_channel_id alias_local,
 			    struct short_channel_id *alias_remote STEALS,
 			    struct channel_id *cid,
 			    struct amount_msat our_msat,
@@ -605,22 +599,18 @@ struct channel *new_channel(struct peer *peer, u64 dbid,
 	channel->push = push;
 	channel->our_funds = our_funds;
 	channel->remote_channel_ready = remote_channel_ready;
-	channel->scid = tal_steal(channel, scid);
+	channel->scid = tal_dup_or_null(channel, struct short_channel_id, scid);
 	channel->old_scids = tal_dup_talarr(channel, struct short_channel_id, old_scids);
-	channel->alias[LOCAL] = tal_dup_or_null(channel, struct short_channel_id, alias_local);
+	channel->alias[LOCAL] = tal_dup(channel, struct short_channel_id, &alias_local);
 	/* All these possible short_channel_id variants go in the lookup table! */
 	/* Stub channels all have the same scid though, *and* get loaded from db! */
 	if (channel->scid && !is_stub_scid(*channel->scid))
 		chanmap_add(peer->ld, channel, *channel->scid);
-	if (channel->alias[LOCAL])
+	if (!is_stub_scid(*channel->alias[LOCAL]))
 		chanmap_add(peer->ld, channel, *channel->alias[LOCAL]);
 	for (size_t i = 0; i < tal_count(channel->old_scids); i++)
 		chanmap_add(peer->ld, channel, channel->old_scids[i]);
 
-	/* We always make sure this is set (historical channels from db might not) */
-	if (!channel->alias[LOCAL])
-		channel_set_random_local_alias(channel);
-
 	channel->alias[REMOTE] = tal_steal(channel, alias_remote);  /* Haven't gotten one yet. */
 	channel->cid = *cid;
 	channel->our_msat = our_msat;
@@ -727,7 +717,7 @@ struct channel *new_channel(struct peer *peer, u64 dbid,
 	}
 	/* scid is NULL when opening a new channel so we don't
 	 * need to set error in that case as well */
-	if (scid && is_stub_scid(*scid))
+	if (channel->scid && is_stub_scid(*channel->scid))
 		channel->error = towire_errorfmt(peer->ld,
 						 &channel->cid,
 						 "We can't be together anymore.");

lightningd/channel.h

@@ -390,9 +390,9 @@ struct channel *new_channel(struct peer *peer, u64 dbid,
 			    struct amount_sat our_funds,
 			    bool remote_channel_ready,
 			    /* NULL or stolen */
-			    struct short_channel_id *scid STEALS,
+			    struct short_channel_id *scid TAKES,
 			    struct short_channel_id *old_scids TAKES,
-			    struct short_channel_id *alias_local STEALS,
+			    struct short_channel_id alias_local,
 			    struct short_channel_id *alias_remote STEALS,
 			    struct channel_id *cid,
 			    struct amount_msat our_msatoshi,

lightningd/opening_control.c

@@ -110,6 +110,7 @@ wallet_commit_channel(struct lightningd *ld,
 	struct timeabs timestamp;
 	struct channel_stats zero_channel_stats;
 	enum addrtype addrtype;
+	struct short_channel_id local_alias;
 
 	/* We can't have any payments yet */
 	memset(&zero_channel_stats, 0, sizeof(zero_channel_stats));
@@ -172,6 +173,8 @@ wallet_commit_channel(struct lightningd *ld,
 	else
 		static_remotekey_start = 0x7FFFFFFFFFFFFFFF;
 
+	local_alias = random_scid();
+
 	channel = new_channel(uc->peer, uc->dbid,
 			      NULL, /* No shachain yet */
 			      CHANNELD_AWAITING_LOCKIN,
@@ -190,7 +193,7 @@ wallet_commit_channel(struct lightningd *ld,
 			      false, /* !remote_channel_ready */
 			      NULL, /* no scid yet */
 			      NULL, /* no old scids */
-			      NULL, /* assign random local alias */
+			      local_alias, /* random local alias */
 			      NULL, /* They haven't told us an alias yet */
 			      cid,
 			      /* The three arguments below are msatoshi_to_us,
@@ -1477,7 +1480,7 @@ static struct channel *stub_chan(struct command *cmd,
 	struct peer *peer;
 	struct pubkey localFundingPubkey;
 	struct pubkey pk;
-	struct short_channel_id *scid;
+	struct short_channel_id scid;
 	u32 blockht;
 	u32 feerate;
 	struct channel_stats zero_channel_stats;
@@ -1555,10 +1558,9 @@ static struct channel *stub_chan(struct command *cmd,
 	channel_info->old_remote_per_commit = pk;
 
 	blockht = 100;
-	scid = tal(cmd, struct short_channel_id);
 
 	/*To indicate this is an stub channel we keep it's scid to 1x1x1.*/
-	if (!mk_short_channel_id(scid, 1, 1, 1))
+	if (!mk_short_channel_id(&scid, 1, 1, 1))
                 fatal("Failed to make short channel 1x1x1!");
 
 	memset(&zero_channel_stats, 0, sizeof(zero_channel_stats));
@@ -1579,9 +1581,9 @@ static struct channel *stub_chan(struct command *cmd,
 			      AMOUNT_MSAT(0),
 			      AMOUNT_SAT(0),
 			      true, /* remote_channel_ready */
-			      scid,
-			      NULL,
+			      &scid,
 			      NULL,
+			      scid,
 			      NULL,
 			      &cid,
 			      /* The three arguments below are msatoshi_to_us,

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

@@ -135,6 +135,9 @@ void channel_set_last_tx(struct channel *channel UNNEEDED,
 			 struct bitcoin_tx *tx UNNEEDED,
 			 const struct bitcoin_signature *sig UNNEEDED)
 { fprintf(stderr, "channel_set_last_tx called!\n"); abort(); }
+/* Generated stub for channel_set_scid */
+void channel_set_scid(struct channel *channel UNNEEDED, const struct short_channel_id *new_scid UNNEEDED)
+{ fprintf(stderr, "channel_set_scid called!\n"); abort(); }
 /* Generated stub for channel_state_name */
 const char *channel_state_name(const struct channel *channel UNNEEDED)
 { fprintf(stderr, "channel_state_name called!\n"); abort(); }

wallet/test/run-db.c

@@ -189,7 +189,7 @@ struct channel *new_channel(struct peer *peer UNNEEDED, u64 dbid UNNEEDED,
 			    /* NULL or stolen */
 			    struct short_channel_id *scid STEALS UNNEEDED,
 			    struct short_channel_id *old_scids TAKES UNNEEDED,
-			    struct short_channel_id *alias_local STEALS UNNEEDED,
+			    struct short_channel_id alias_local UNNEEDED,
 			    struct short_channel_id *alias_remote STEALS UNNEEDED,
 			    struct channel_id *cid UNNEEDED,
 			    struct amount_msat our_msatoshi UNNEEDED,

wallet/test/run-wallet.c

@@ -1983,6 +1983,7 @@ static bool test_channel_inflight_crud(struct lightningd *ld, const tal_t *ctx)
 	struct wally_psbt *funding_psbt;
 	struct channel_info *channel_info = tal(w, struct channel_info);
 	struct basepoints basepoints;
+	struct short_channel_id alias_local = random_scid();
 	secp256k1_ecdsa_signature *lease_commit_sig;
 	u32 feerate, lease_blockheight_start;
 	u64 dbid;
@@ -2034,7 +2035,7 @@ static bool test_channel_inflight_crud(struct lightningd *ld, const tal_t *ctx)
 			   our_sats,
 			   0, NULL,
 			   NULL, /* old scids */
-			   NULL, /* alias[LOCAL] */
+			   alias_local,
 			   NULL, /* alias[REMOTE] */
 			   &cid,
 			   AMOUNT_MSAT(3333333000),

wallet/wallet.c

@@ -1774,7 +1774,7 @@ static struct channel *wallet_stmt2channel(struct wallet *w, struct db_stmt *stm
 	struct channel_info channel_info;
 	struct fee_states *fee_states;
 	struct height_states *height_states;
-	struct short_channel_id *scid, *alias[NUM_SIDES], *old_scids;
+	struct short_channel_id *scid, alias_local, *alias_remote, *old_scids;
 	struct channel_id cid;
 	struct channel *chan;
 	u64 peer_dbid;
@@ -1814,9 +1814,8 @@ static struct channel *wallet_stmt2channel(struct wallet *w, struct db_stmt *stm
 
 	scid = db_col_optional_scid(tmpctx, stmt, "scid");
 	old_scids = db_col_short_channel_id_arr(tmpctx, stmt, "old_scids");
-	alias[LOCAL] = tal(tmpctx, struct short_channel_id);
-	*alias[LOCAL] = db_col_short_channel_id(stmt, "alias_local");
-	alias[REMOTE] = db_col_optional_scid(tmpctx, stmt, "alias_remote");
+	alias_local = db_col_short_channel_id(stmt, "alias_local");
+	alias_remote = db_col_optional_scid(tmpctx, stmt, "alias_remote");
 
  	ok &= wallet_shachain_load(w, db_col_u64(stmt, "shachain_remote_id"),
 				   &wshachain);
@@ -1971,7 +1970,7 @@ static struct channel *wallet_stmt2channel(struct wallet *w, struct db_stmt *stm
 		if (scid)
 			remote_update->scid = *scid;
 		else
-			remote_update->scid = *alias[LOCAL];
+			remote_update->scid = alias_local;
 		remote_update->fee_base = db_col_int(stmt, "remote_feerate_base");
 		remote_update->fee_ppm = db_col_int(stmt, "remote_feerate_ppm");
 		remote_update->cltv_delta = db_col_int(stmt, "remote_cltv_expiry_delta");
@@ -2028,10 +2027,10 @@ static struct channel *wallet_stmt2channel(struct wallet *w, struct db_stmt *stm
 			   push_msat,
 			   our_funding_sat,
 			   db_col_int(stmt, "funding_locked_remote") != 0,
-			   scid,
+			   take(scid),
 			   old_scids,
-			   alias[LOCAL],
-			   alias[REMOTE],
+			   alias_local,
+			   alias_remote,
 			   &cid,
 			   our_msat,
 			   msat_to_us_min, /* msatoshi_to_us_min */