lightningd: rename state_change_entry to channel_state_change, and us…

…e pointers. This name is clearer than the old one. And since the struct contains a string, it's more natural for the struct to be the tal parent of the string so it's a real object. This means we need an array of pointers, so each struct can be its own tal object. wallet_state_change_get is hoisted higher in the code and made static. Signed-off-by: Rusty Russell <[email protected]>
ElementsProject · Sep 20, 2024 · aba67a7 · aba67a7
1 parent 0440700
commit aba67a7
Show file tree

Hide file tree

Showing 10 changed files with 101 additions and 77 deletions.
diff --git a/lightningd/channel.c b/lightningd/channel.c
@@ -312,7 +312,7 @@ struct channel *new_unsaved_channel(struct peer *peer,
 	channel->stable_conn_timer = NULL;
 	/* Nothing happened yet */
 	memset(&channel->stats, 0, sizeof(channel->stats));
-	channel->state_changes = tal_arr(channel, struct state_change_entry, 0);
+	channel->state_changes = tal_arr(channel, struct channel_state_change *, 0);
 
 	/* No shachain yet */
 	channel->their_shachain.id = 0;
@@ -450,7 +450,7 @@ struct channel *new_channel(struct peer *peer, u64 dbid,
 			    struct peer_update *peer_update STEALS,
 			    u64 last_stable_connection,
 			    const struct channel_stats *stats,
-			    struct state_change_entry *state_changes STEALS)
+			    struct channel_state_change **state_changes STEALS)
 {
 	struct channel *channel = tal(peer->ld, struct channel);
 	struct amount_msat htlc_min, htlc_max;
@@ -834,6 +834,22 @@ void channel_set_last_tx(struct channel *channel,
 	channel->last_tx = tal_steal(channel, tx);
 }
 
+struct channel_state_change *new_channel_state_change(const tal_t *ctx,
+						      struct timeabs timestamp,
+						      enum channel_state old_state,
+						      enum channel_state new_state,
+						      enum state_change cause,
+						      const char *message TAKES)
+{
+	struct channel_state_change *c = tal(ctx, struct channel_state_change);
+	c->timestamp = timestamp;
+	c->old_state = old_state;
+	c->new_state = new_state;
+	c->cause = cause;
+	c->message = tal_strdup(c, message);
+	return c;
+}
+
 void channel_set_state(struct channel *channel,
 		       enum channel_state old_state,
 		       enum channel_state state,
@@ -867,17 +883,19 @@ void channel_set_state(struct channel *channel,
 
 	/* plugin notification channel_state_changed and DB entry */
 	if (state != old_state) {  /* see issue #4029 */
-		struct state_change_entry change;
-		change.timestamp = time_now();
-		change.old_state = old_state;
-		change.new_state = state;
-		change.cause = reason;
-		change.message = tal_strdup(channel->state_changes, why);
+		struct channel_state_change *change;
+
+		change = new_channel_state_change(channel->state_changes,
+						  time_now(),
+						  old_state,
+						  state,
+						  reason,
+						  why);
 		tal_arr_expand(&channel->state_changes, change);
 
 		wallet_state_change_add(channel->peer->ld->wallet,
 					channel->dbid,
-					change.timestamp,
+					change->timestamp,
 					old_state,
 					state,
 					reason,
@@ -886,7 +904,7 @@ void channel_set_state(struct channel *channel,
 					     &channel->peer->id,
 					     &channel->cid,
 					     channel->scid,
-					     change.timestamp,
+					     change->timestamp,
 					     old_state,
 					     state,
 					     reason,

diff --git a/lightningd/channel.h b/lightningd/channel.h
@@ -115,6 +115,15 @@ struct channel_stats {
 	struct amount_msat out_msatoshi_offered, out_msatoshi_fulfilled;
 };
 
+
+struct channel_state_change {
+	struct timeabs timestamp;
+	enum channel_state old_state;
+	enum channel_state new_state;
+	enum state_change cause;
+	const char *message;
+};
+
 struct channel {
 	/* Inside peer->channels. */
 	struct list_node list;
@@ -327,7 +336,7 @@ struct channel {
 	struct channel_stats stats;
 
 	/* Our change history. */
-	struct state_change_entry *state_changes;
+	struct channel_state_change **state_changes;
 };
 
 /* Is channel owned (and should be talking to peer) */
@@ -411,7 +420,7 @@ struct channel *new_channel(struct peer *peer, u64 dbid,
 			    struct peer_update *peer_update STEALS,
 			    u64 last_stable_connection,
 			    const struct channel_stats *stats,
-			    struct state_change_entry *state_changes STEALS);
+			    struct channel_state_change **state_changes STEALS);
 
 /* new_inflight - Create a new channel_inflight for a channel */
 struct channel_inflight *new_inflight(struct channel *channel,
@@ -431,6 +440,13 @@ struct channel_inflight *new_inflight(struct channel *channel,
 	     bool i_am_initiator,
 	     bool force_sign_first);
 
+struct channel_state_change *new_channel_state_change(const tal_t *ctx,
+						      struct timeabs timestamp,
+						      enum channel_state old_state,
+						      enum channel_state new_state,
+						      enum state_change cause,
+						      const char *message TAKES);
+
 /* Add a last_tx and sig to an inflight */
 void inflight_set_last_tx(struct channel_inflight *inflight,
 		          struct bitcoin_tx *last_tx STEALS,

diff --git a/lightningd/channel_state.h b/lightningd/channel_state.h
@@ -73,13 +73,4 @@ enum state_change {
 	/* Note: This is very likely a conscious remote decision. */
 	REASON_ONCHAIN
 };
-
-struct state_change_entry {
-	struct timeabs timestamp;
-	enum channel_state old_state;
-	enum channel_state new_state;
-	enum state_change cause;
-	char *message;
-};
-
 #endif /* LIGHTNING_LIGHTNINGD_CHANNEL_STATE_H */
diff --git a/lightningd/opening_control.c b/lightningd/opening_control.c
@@ -221,7 +221,7 @@ wallet_commit_channel(struct lightningd *ld,
 			      NULL,
 			      0,
 			      &zero_channel_stats,
-			      tal_arr(NULL, struct state_change_entry, 0));
+			      tal_arr(NULL, struct channel_state_change *, 0));
 
 	/* Now we finally put it in the database. */
 	wallet_channel_insert(ld->wallet, channel);
@@ -1600,7 +1600,7 @@ static struct channel *stub_chan(struct command *cmd,
 			      NULL,
 			      0,
 			      &zero_channel_stats,
-			      tal_arr(NULL, struct state_change_entry, 0));
+			      tal_arr(NULL, struct channel_state_change *, 0));
 
 	/* We don't want to gossip about this, ever. */
 	channel->channel_gossip = tal_free(channel->channel_gossip);

diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c
@@ -1162,16 +1162,17 @@ static void NON_NULL_ARGS(1, 2, 4, 5) json_add_channel(struct command *cmd,
 
 	json_array_start(response, "state_changes");
 	for (size_t i = 0; i < tal_count(channel->state_changes); i++) {
+		const struct channel_state_change *change
+			= channel->state_changes[i];
 		json_object_start(response, NULL);
-		json_add_timeiso(response, "timestamp",
-				 channel->state_changes[i].timestamp);
+		json_add_timeiso(response, "timestamp", change->timestamp);
 		json_add_string(response, "old_state",
-				channel_state_str(channel->state_changes[i].old_state));
+				channel_state_str(change->old_state));
 		json_add_string(response, "new_state",
-				channel_state_str(channel->state_changes[i].new_state));
+				channel_state_str(change->new_state));
 		json_add_string(response, "cause",
-				channel_change_state_reason_str(channel->state_changes[i].cause));
-		json_add_string(response, "message", channel->state_changes[i].message);
+				channel_change_state_reason_str(change->cause));
+		json_add_string(response, "message", change->message);
 		json_object_end(response);
 	}
 	json_array_end(response);

diff --git a/lightningd/test/run-invoice-select-inchan.c b/lightningd/test/run-invoice-select-inchan.c
@@ -1059,11 +1059,6 @@ char *wallet_offer_find(const tal_t *ctx UNNEEDED,
 			enum offer_status *status)
 
 { fprintf(stderr, "wallet_offer_find called!\n"); abort(); }
-/* Generated stub for wallet_state_change_get */
-struct state_change_entry *wallet_state_change_get(const tal_t *ctx UNNEEDED,
-						   struct wallet *w UNNEEDED,
-						   u64 channel_id UNNEEDED)
-{ fprintf(stderr, "wallet_state_change_get called!\n"); abort(); }
 /* Generated stub for wallet_total_forward_fees */
 struct amount_msat wallet_total_forward_fees(struct wallet *w UNNEEDED)
 { fprintf(stderr, "wallet_total_forward_fees called!\n"); abort(); }

diff --git a/wallet/test/run-db.c b/wallet/test/run-db.c
@@ -193,8 +193,16 @@ struct channel *new_channel(struct peer *peer UNNEEDED, u64 dbid UNNEEDED,
 			    struct peer_update *peer_update STEALS UNNEEDED,
 			    u64 last_stable_connection UNNEEDED,
 			    const struct channel_stats *stats UNNEEDED,
-			    struct state_change_entry *state_changes STEALS UNNEEDED)
+			    struct channel_state_change **state_changes STEALS UNNEEDED)
 { fprintf(stderr, "new_channel called!\n"); abort(); }
+/* Generated stub for new_channel_state_change */
+struct channel_state_change *new_channel_state_change(const tal_t *ctx UNNEEDED,
+						      struct timeabs timestamp UNNEEDED,
+						      enum channel_state old_state UNNEEDED,
+						      enum channel_state new_state UNNEEDED,
+						      enum state_change cause UNNEEDED,
+						      const char *message TAKES UNNEEDED)
+{ fprintf(stderr, "new_channel_state_change called!\n"); abort(); }
 /* Generated stub for new_coin_wallet_deposit */
 struct chain_coin_mvt *new_coin_wallet_deposit(const tal_t *ctx UNNEEDED,
 					       const struct bitcoin_outpoint *outpoint UNNEEDED,

diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c
@@ -1998,7 +1998,7 @@ static bool test_channel_inflight_crud(struct lightningd *ld, const tal_t *ctx)
 			   NULL,
 			   0,
 			   stats,
-			   tal_arr(NULL, struct state_change_entry, 0));
+			   tal_arr(NULL, struct channel_state_change *, 0));
 	db_begin_transaction(w->db);
 	CHECK(!wallet_err);
 	wallet_channel_insert(w, chan);

diff --git a/wallet/wallet.c b/wallet/wallet.c
@@ -1486,6 +1486,41 @@ static struct short_channel_id *db_col_optional_scid(const tal_t *ctx,
 	return scid;
 }
 
+static struct channel_state_change **wallet_state_change_get(const tal_t *ctx,
+							     struct wallet *w,
+							     u64 channel_id)
+{
+	struct db_stmt *stmt;
+	struct channel_state_change **res = tal_arr(ctx,
+						    struct channel_state_change *, 0);
+	stmt = db_prepare_v2(
+	    w->db, SQL("SELECT"
+		       " timestamp,"
+		       " old_state,"
+		       " new_state,"
+		       " cause,"
+		       " message "
+		       "FROM channel_state_changes "
+		       "WHERE channel_id = ? "
+		       "ORDER BY timestamp ASC;"));
+	db_bind_int(stmt, channel_id);
+	db_query_prepared(stmt);
+
+	while (db_step(stmt)) {
+		struct channel_state_change *c;
+
+		c = new_channel_state_change(res,
+					     db_col_timeabs(stmt, "timestamp"),
+					     db_col_int(stmt, "old_state"),
+					     db_col_int(stmt, "new_state"),
+					     state_change_in_db(db_col_int(stmt, "cause")),
+					     take(db_col_strdup(NULL, stmt, "message")));
+		tal_arr_expand(&res, c);
+	}
+	tal_free(stmt);
+	return res;
+}
+
 /**
  * wallet_stmt2channel - Helper to populate a wallet_channel from a `db_stmt`
  */
@@ -1522,7 +1557,7 @@ static struct channel *wallet_stmt2channel(struct wallet *w, struct db_stmt *stm
 	bool ignore_fee_limits;
 	struct peer_update *remote_update;
 	struct channel_stats stats;
-	struct state_change_entry *state_changes;
+	struct channel_state_change **state_changes;
 
 	peer_dbid = db_col_u64(stmt, "peer_id");
 	peer = find_peer_by_dbid(w->ld, peer_dbid);
@@ -2464,39 +2499,6 @@ void wallet_state_change_add(struct wallet *w,
 	db_exec_prepared_v2(take(stmt));
 }
 
-struct state_change_entry *wallet_state_change_get(const tal_t *ctx,
-						   struct wallet *w,
-						   u64 channel_id)
-{
-	struct db_stmt *stmt;
-	struct state_change_entry tmp;
-	struct state_change_entry *res = tal_arr(ctx,
-						 struct state_change_entry, 0);
-	stmt = db_prepare_v2(
-	    w->db, SQL("SELECT"
-		       " timestamp,"
-		       " old_state,"
-		       " new_state,"
-		       " cause,"
-		       " message "
-		       "FROM channel_state_changes "
-		       "WHERE channel_id = ? "
-		       "ORDER BY timestamp ASC;"));
-	db_bind_int(stmt, channel_id);
-	db_query_prepared(stmt);
-
-	while (db_step(stmt)) {
-		tmp.timestamp = db_col_timeabs(stmt, "timestamp");
-		tmp.old_state = db_col_int(stmt, "old_state");
-		tmp.new_state = db_col_int(stmt, "new_state");
-		tmp.cause = state_change_in_db(db_col_int(stmt, "cause"));
-		tmp.message = db_col_strdup(res, stmt, "message");
-		tal_arr_expand(&res, tmp);
-	}
-	tal_free(stmt);
-	return res;
-}
-
 static void wallet_peer_save(struct wallet *w, struct peer *peer)
 {
 	const char *addr =

diff --git a/wallet/wallet.h b/wallet/wallet.h
@@ -662,13 +662,6 @@ void wallet_state_change_add(struct wallet *w,
 			     enum state_change cause,
 			     const char *message);
 
-/**
- * Gets all state change history entries for a channel from the database
- */
-struct state_change_entry *wallet_state_change_get(const tal_t *ctx,
-						   struct wallet *w,
-						   u64 channel_id);
-
 /**
  * wallet_delete_peer_if_unused -- After no more channels in peer, forget about it
  */