From aba67a757c4a3ad76b0d441e4047f8f0d15223a7 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 19 Sep 2024 16:34:33 +0930 Subject: [PATCH] lightningd: rename state_change_entry to channel_state_change, and use 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 --- lightningd/channel.c | 38 ++++++++--- lightningd/channel.h | 20 +++++- lightningd/channel_state.h | 9 --- lightningd/opening_control.c | 4 +- lightningd/peer_control.c | 13 ++-- lightningd/test/run-invoice-select-inchan.c | 5 -- wallet/test/run-db.c | 10 ++- wallet/test/run-wallet.c | 2 +- wallet/wallet.c | 70 +++++++++++---------- wallet/wallet.h | 7 --- 10 files changed, 101 insertions(+), 77 deletions(-) diff --git a/lightningd/channel.c b/lightningd/channel.c index 646c4f2ed..59c5be0a2 100644 --- 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 index 864854afa..4b4035699 100644 --- 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 index 2c655bc8b..8ef3da3be 100644 --- 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 index a52307323..d0a2c7e6e 100644 --- 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 index 4bace9f47..d514f5936 100644 --- 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 index cc17b7947..54fe654ac 100644 --- 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 index 445ef118c..755b6318e 100644 --- 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 index 0d069701d..31006b1f7 100644 --- 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 index c5b19ea42..811944c27 100644 --- 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 index dce2475a4..9329e5a37 100644 --- 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 */