wallet: have db track what columns are accessed in DEVELOPER mode.

And add db_col_ignore helper for cases where it's deliberate.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This commit is contained in:
Rusty Russell
2021-11-15 04:30:46 +10:30
parent 7382616513
commit c462ccae1a
5 changed files with 138 additions and 11 deletions

View File

@@ -881,6 +881,23 @@ static void db_stmt_free(struct db_stmt *stmt)
if (!stmt->executed) if (!stmt->executed)
fatal("Freeing an un-executed statement from %s: %s", fatal("Freeing an un-executed statement from %s: %s",
stmt->location, stmt->query->query); stmt->location, stmt->query->query);
#if DEVELOPER
/* If they never got a db_step, we don't track */
if (stmt->cols_used) {
for (size_t i = 0; i < stmt->query->num_colnames; i++) {
if (!stmt->query->colnames[i].sqlname)
continue;
if (!strset_get(stmt->cols_used,
stmt->query->colnames[i].sqlname)) {
log_broken(stmt->db->log,
"Never accessed column %s in query %s",
stmt->query->colnames[i].sqlname,
stmt->query->query);
}
}
strset_clear(stmt->cols_used);
}
#endif
if (stmt->inner_stmt) if (stmt->inner_stmt)
stmt->db->config->stmt_free_fn(stmt); stmt->db->config->stmt_free_fn(stmt);
assert(stmt->inner_stmt == NULL); assert(stmt->inner_stmt == NULL);
@@ -929,6 +946,10 @@ struct db_stmt *db_prepare_v2_(const char *location, struct db *db,
list_add(&db->pending_statements, &stmt->list); list_add(&db->pending_statements, &stmt->list);
#if DEVELOPER
stmt->cols_used = NULL;
#endif /* DEVELOPER */
return stmt; return stmt;
} }
@@ -937,8 +958,19 @@ struct db_stmt *db_prepare_v2_(const char *location, struct db *db,
bool db_step(struct db_stmt *stmt) bool db_step(struct db_stmt *stmt)
{ {
bool ret;
assert(stmt->executed); assert(stmt->executed);
return stmt->db->config->step_fn(stmt); ret = stmt->db->config->step_fn(stmt);
#if DEVELOPER
/* We only track cols_used if we return a result! */
if (ret && !stmt->cols_used) {
stmt->cols_used = tal(stmt, struct strset);
strset_init(stmt->cols_used);
}
#endif
return ret;
} }
u64 db_column_u64(struct db_stmt *stmt, int col) u64 db_column_u64(struct db_stmt *stmt, int col)
@@ -1432,6 +1464,8 @@ void fillin_missing_scriptpubkeys(struct lightningd *ld, struct db *db,
fatal("HSM gave bad hsm_get_output_scriptpubkey_reply %s", fatal("HSM gave bad hsm_get_output_scriptpubkey_reply %s",
tal_hex(msg, msg)); tal_hex(msg, msg));
} else { } else {
db_col_ignore(stmt, "peer_id");
db_col_ignore(stmt, "commitment_point");
/* Build from bip32_base */ /* Build from bip32_base */
bip32_pubkey(mc->bip32_base, &key, keyindex); bip32_pubkey(mc->bip32_base, &key, keyindex);
if (type == p2sh_wpkh) { if (type == p2sh_wpkh) {
@@ -1623,11 +1657,18 @@ migrate_inflight_last_tx_to_psbt(struct lightningd *ld, struct db *db,
last_tx = db_col_tx(stmt, stmt, "inflight.last_tx"); last_tx = db_col_tx(stmt, stmt, "inflight.last_tx");
assert(last_tx != NULL); assert(last_tx != NULL);
/* FIXME: This is only needed inside the select? */
db_col_ignore(stmt, "inflight.last_tx");
/* If we've forgotten about the peer_id /* If we've forgotten about the peer_id
* because we closed / forgot the channel, * because we closed / forgot the channel,
* we can skip this. */ * we can skip this. */
if (db_col_is_null(stmt, "p.node_id")) if (db_col_is_null(stmt, "p.node_id")) {
db_col_ignore(stmt, "inflight.last_sig");
db_col_ignore(stmt, "inflight.funding_satoshi");
db_col_ignore(stmt, "inflight.funding_tx_id");
continue; continue;
}
db_col_node_id(stmt, "p.node_id", &peer_id); db_col_node_id(stmt, "p.node_id", &peer_id);
db_col_amount_sat(stmt, "inflight.funding_satoshi", &funding_sat); db_col_amount_sat(stmt, "inflight.funding_satoshi", &funding_sat);
db_col_pubkey(stmt, "c.fundingkey_remote", &remote_funding_pubkey); db_col_pubkey(stmt, "c.fundingkey_remote", &remote_funding_pubkey);
@@ -1711,8 +1752,13 @@ void migrate_last_tx_to_psbt(struct lightningd *ld, struct db *db,
/* If we've forgotten about the peer_id /* If we've forgotten about the peer_id
* because we closed / forgot the channel, * because we closed / forgot the channel,
* we can skip this. */ * we can skip this. */
if (db_col_is_null(stmt, "p.node_id")) if (db_col_is_null(stmt, "p.node_id")) {
db_col_ignore(stmt, "c.funding_satoshi");
db_col_ignore(stmt, "c.fundingkey_remote");
db_col_ignore(stmt, "c.last_sig");
continue; continue;
}
db_col_node_id(stmt, "p.node_id", &peer_id); db_col_node_id(stmt, "p.node_id", &peer_id);
db_col_amount_sat(stmt, "c.funding_satoshi", &funding_sat); db_col_amount_sat(stmt, "c.funding_satoshi", &funding_sat);
db_col_pubkey(stmt, "c.fundingkey_remote", &remote_funding_pubkey); db_col_pubkey(stmt, "c.fundingkey_remote", &remote_funding_pubkey);
@@ -2567,5 +2613,17 @@ size_t db_query_colnum(const struct db_stmt *stmt,
colname)) { colname)) {
col = (col + 1) % stmt->query->num_colnames; col = (col + 1) % stmt->query->num_colnames;
} }
#if DEVELOPER
strset_add(stmt->cols_used, colname);
#endif
return stmt->query->colnames[col].val; return stmt->query->colnames[col].val;
} }
void db_col_ignore(struct db_stmt *stmt, const char *colname)
{
#if DEVELOPER
db_query_colnum(stmt, colname);
#endif
}

View File

@@ -182,8 +182,7 @@ void db_column_amount_msat_or_default(struct db_stmt *stmt, int col,
/* Modern variants: get columns by name from SELECT */ /* Modern variants: get columns by name from SELECT */
/* Bridge function to get column number from SELECT /* Bridge function to get column number from SELECT
(must exist) */ (must exist) */
size_t db_query_colnum(const struct db_stmt *stmt, size_t db_query_colnum(const struct db_stmt *stmt, const char *colname);
const char *colname);
u64 db_col_u64(struct db_stmt *stmt, const char *colname); u64 db_col_u64(struct db_stmt *stmt, const char *colname);
int db_col_int(struct db_stmt *stmt, const char *colname); int db_col_int(struct db_stmt *stmt, const char *colname);
@@ -238,6 +237,9 @@ void db_col_amount_msat_or_default(struct db_stmt *stmt, const char *colname,
struct amount_msat def); struct amount_msat def);
/* Explicitly ignore a column (so we don't complain you didn't use it!) */
void db_col_ignore(struct db_stmt *stmt, const char *colname);
/** /**
* db_exec_prepared -- Execute a prepared statement * db_exec_prepared -- Execute a prepared statement
* *

View File

@@ -3,6 +3,7 @@
#include "config.h" #include "config.h"
#include <ccan/list/list.h> #include <ccan/list/list.h>
#include <ccan/short_types/short_types.h> #include <ccan/short_types/short_types.h>
#include <ccan/strset/strset.h>
#include <common/autodata.h> #include <common/autodata.h>
/* For testing, we want to catch fatal messages. */ /* For testing, we want to catch fatal messages. */
@@ -99,6 +100,11 @@ struct db_stmt {
bool executed; bool executed;
int row; int row;
#if DEVELOPER
/* Map as we reference into a SELECT statement in query. */
struct strset *cols_used;
#endif
}; };
struct db_config { struct db_config {

View File

@@ -96,6 +96,10 @@ static struct invoice_details *wallet_stmt2invoice_details(const tal_t *ctx,
dtl->pay_index = db_col_u64(stmt, "pay_index"); dtl->pay_index = db_col_u64(stmt, "pay_index");
db_col_amount_msat(stmt, "msatoshi_received", &dtl->received); db_col_amount_msat(stmt, "msatoshi_received", &dtl->received);
dtl->paid_timestamp = db_col_u64(stmt, "paid_timestamp"); dtl->paid_timestamp = db_col_u64(stmt, "paid_timestamp");
} else {
db_col_ignore(stmt, "pay_index");
db_col_ignore(stmt, "msatoshi_received");
db_col_ignore(stmt, "paid_timestamp");
} }
dtl->invstring = db_col_strdup(dtl, stmt, "bolt11"); dtl->invstring = db_col_strdup(dtl, stmt, "bolt11");

View File

@@ -109,6 +109,7 @@ static bool wallet_add_utxo(struct wallet *w, struct utxo *utxo,
/* If we get a result, that means a clash. */ /* If we get a result, that means a clash. */
if (db_step(stmt)) { if (db_step(stmt)) {
db_col_ignore(stmt, "*");
tal_free(stmt); tal_free(stmt);
return false; return false;
} }
@@ -197,6 +198,10 @@ static struct utxo *wallet_stmt2output(const tal_t *ctx, struct db_stmt *stmt)
utxo->close_info->csv = db_col_int(stmt, "csv_lock"); utxo->close_info->csv = db_col_int(stmt, "csv_lock");
} else { } else {
utxo->close_info = NULL; utxo->close_info = NULL;
db_col_ignore(stmt, "peer_id");
db_col_ignore(stmt, "commitment_point");
db_col_ignore(stmt, "option_anchor_outputs");
db_col_ignore(stmt, "csv_lock");
} }
utxo->scriptPubkey = db_col_arr(utxo, stmt, "scriptpubkey", u8); utxo->scriptPubkey = db_col_arr(utxo, stmt, "scriptpubkey", u8);
@@ -607,6 +612,7 @@ bool wallet_add_onchaind_utxo(struct wallet *w,
/* If we get a result, that means a clash. */ /* If we get a result, that means a clash. */
if (db_step(stmt)) { if (db_step(stmt)) {
db_col_ignore(stmt, "*");
tal_free(stmt); tal_free(stmt);
return false; return false;
} }
@@ -850,8 +856,11 @@ static struct peer *wallet_peer_load(struct wallet *w, const u64 dbid)
if (!db_step(stmt)) if (!db_step(stmt))
goto done; goto done;
if (db_col_is_null(stmt, "node_id")) if (db_col_is_null(stmt, "node_id")) {
db_col_ignore(stmt, "address");
db_col_ignore(stmt, "id");
goto done; goto done;
}
db_col_node_id(stmt, "node_id", &id); db_col_node_id(stmt, "node_id", &id);
@@ -923,6 +932,7 @@ bool wallet_remote_ann_sigs_load(const tal_t *ctx, struct wallet *w, u64 id,
/* if only one sig exists, forget the sig and hope peer send new ones*/ /* if only one sig exists, forget the sig and hope peer send new ones*/
if (db_col_is_null(stmt, "remote_ann_node_sig") if (db_col_is_null(stmt, "remote_ann_node_sig")
|| db_col_is_null(stmt, "remote_ann_bitcoin_sig")) { || db_col_is_null(stmt, "remote_ann_bitcoin_sig")) {
db_col_ignore(stmt, "remote_ann_bitcoin_sig");
*remote_ann_node_sig = *remote_ann_bitcoin_sig = NULL; *remote_ann_node_sig = *remote_ann_bitcoin_sig = NULL;
tal_free(stmt); tal_free(stmt);
return true; return true;
@@ -1147,6 +1157,9 @@ wallet_stmt2inflight(struct wallet *w, struct db_stmt *stmt,
lease_chan_max_msat = 0; lease_chan_max_msat = 0;
lease_chan_max_ppt = 0; lease_chan_max_ppt = 0;
lease_blockheight_start = 0; lease_blockheight_start = 0;
db_col_ignore(stmt, "lease_chan_max_msat");
db_col_ignore(stmt, "lease_chan_max_ppt");
db_col_ignore(stmt, "lease_blockheight_start");
} }
inflight = new_inflight(chan, &funding, inflight = new_inflight(chan, &funding,
@@ -1287,6 +1300,8 @@ static struct channel *wallet_stmt2channel(struct wallet *w, struct db_stmt *stm
last_sent_commit->id = db_col_u64(stmt, "last_sent_commit_id"); last_sent_commit->id = db_col_u64(stmt, "last_sent_commit_id");
} }
#endif #endif
db_col_ignore(stmt, "last_sent_commit_state");
db_col_ignore(stmt, "last_sent_commit_id");
if (!db_col_is_null(stmt, "future_per_commitment_point")) { if (!db_col_is_null(stmt, "future_per_commitment_point")) {
future_per_commitment_point = tal(tmpctx, struct pubkey); future_per_commitment_point = tal(tmpctx, struct pubkey);
@@ -1356,9 +1371,10 @@ static struct channel *wallet_stmt2channel(struct wallet *w, struct db_stmt *stm
db_col_pubkey(stmt, "delayed_payment_basepoint_local", db_col_pubkey(stmt, "delayed_payment_basepoint_local",
&local_basepoints.delayed_payment); &local_basepoints.delayed_payment);
db_col_pubkey(stmt, "funding_pubkey_local", &local_funding_pubkey); db_col_pubkey(stmt, "funding_pubkey_local", &local_funding_pubkey);
if (db_col_is_null(stmt, "shutdown_wrong_txid")) if (db_col_is_null(stmt, "shutdown_wrong_txid")) {
db_col_ignore(stmt, "shutdown_wrong_outnum");
shutdown_wrong_funding = NULL; shutdown_wrong_funding = NULL;
else { } else {
shutdown_wrong_funding = tal(tmpctx, struct bitcoin_outpoint); shutdown_wrong_funding = tal(tmpctx, struct bitcoin_outpoint);
db_col_txid(stmt, "shutdown_wrong_txid", db_col_txid(stmt, "shutdown_wrong_txid",
&shutdown_wrong_funding->txid); &shutdown_wrong_funding->txid);
@@ -1379,6 +1395,8 @@ static struct channel *wallet_stmt2channel(struct wallet *w, struct db_stmt *stm
lease_chan_max_msat = db_col_int(stmt, "lease_chan_max_msat"); lease_chan_max_msat = db_col_int(stmt, "lease_chan_max_msat");
lease_chan_max_ppt = db_col_int(stmt, "lease_chan_max_ppt"); lease_chan_max_ppt = db_col_int(stmt, "lease_chan_max_ppt");
} else { } else {
db_col_ignore(stmt, "lease_chan_max_msat");
db_col_ignore(stmt, "lease_chan_max_ppt");
lease_commit_sig = NULL; lease_commit_sig = NULL;
lease_chan_max_msat = 0; lease_chan_max_msat = 0;
lease_chan_max_ppt = 0; lease_chan_max_ppt = 0;
@@ -1552,6 +1570,9 @@ static bool wallet_channels_load_active(struct wallet *w)
ok = false; ok = false;
break; break;
} }
/* FIXME: Remove */
db_col_ignore(stmt, "local_feerate_per_kw");
db_col_ignore(stmt, "remote_feerate_per_kw");
count++; count++;
} }
log_debug(w->log, "Loaded %d channels from DB", count); log_debug(w->log, "Loaded %d channels from DB", count);
@@ -1702,6 +1723,8 @@ void wallet_blocks_heights(struct wallet *w, u32 def, u32 *min, u32 *max)
if (!db_col_is_null(stmt, "MIN(height)")) { if (!db_col_is_null(stmt, "MIN(height)")) {
*min = db_col_int(stmt, "MIN(height)"); *min = db_col_int(stmt, "MIN(height)");
*max = db_col_int(stmt, "MAX(height)"); *max = db_col_int(stmt, "MAX(height)");
} else {
db_col_ignore(stmt, "MAX(height)");
} }
} }
tal_free(stmt); tal_free(stmt);
@@ -1762,6 +1785,8 @@ bool wallet_channel_config_load(struct wallet *w, const u64 id,
if (!db_step(stmt)) if (!db_step(stmt))
return false; return false;
/* FIXME */
db_col_ignore(stmt, "id");
cc->id = id; cc->id = id;
db_col_amount_sat(stmt, "dust_limit_satoshis", &cc->dust_limit); db_col_amount_sat(stmt, "dust_limit_satoshis", &cc->dust_limit);
db_col_amount_msat(stmt, "max_htlc_value_in_flight_msat", db_col_amount_msat(stmt, "max_htlc_value_in_flight_msat",
@@ -2583,6 +2608,9 @@ static bool wallet_stmt2htlc_in(struct channel *channel,
in->fail_immediate = db_col_int(stmt, "fail_immediate"); in->fail_immediate = db_col_int(stmt, "fail_immediate");
/* FIXME: Don't fetch this at all! */
db_col_ignore(stmt, "origin_htlc");
return ok; return ok;
} }
@@ -2646,12 +2674,19 @@ static bool wallet_stmt2htlc_out(struct wallet *wallet,
in_id, out->dbid); in_id, out->dbid);
#endif #endif
} }
db_col_ignore(stmt, "partid");
db_col_ignore(stmt, "groupid");
} else { } else {
out->partid = db_col_u64(stmt, "partid"); out->partid = db_col_u64(stmt, "partid");
out->groupid = db_col_u64(stmt, "groupid"); out->groupid = db_col_u64(stmt, "groupid");
out->am_origin = true; out->am_origin = true;
} }
/* FIXME: don't SELECT these columns */
db_col_ignore(stmt, "received_time");
db_col_ignore(stmt, "shared_secret");
db_col_ignore(stmt, "malformed_onion");
return ok; return ok;
} }
@@ -2974,6 +3009,7 @@ void wallet_payment_store(struct wallet *wallet,
db_query_prepared(stmt); db_query_prepared(stmt);
res = db_step(stmt); res = db_step(stmt);
assert(res); assert(res);
db_col_ignore(stmt, "status");
tal_free(stmt); tal_free(stmt);
#endif #endif
return; return;
@@ -3345,9 +3381,10 @@ void wallet_payment_get_failinfo(const tal_t *ctx,
*failnode = tal(ctx, struct node_id); *failnode = tal(ctx, struct node_id);
db_col_node_id(stmt, "failnode", *failnode); db_col_node_id(stmt, "failnode", *failnode);
} }
if (db_col_is_null(stmt, "failchannel")) if (db_col_is_null(stmt, "failchannel")) {
db_col_ignore(stmt, "faildirection");
*failchannel = NULL; *failchannel = NULL;
else { } else {
*failchannel = tal(ctx, struct short_channel_id); *failchannel = tal(ctx, struct short_channel_id);
resb = db_col_short_channel_id_str(stmt, "failchannel", resb = db_col_short_channel_id_str(stmt, "failchannel",
*failchannel); *failchannel);
@@ -3793,6 +3830,8 @@ bool wallet_have_block(struct wallet *w, u32 blockheight)
db_bind_int(stmt, 0, blockheight); db_bind_int(stmt, 0, blockheight);
db_query_prepared(stmt); db_query_prepared(stmt);
result = db_step(stmt); result = db_step(stmt);
if (result)
db_col_ignore(stmt, "height");
tal_free(stmt); tal_free(stmt);
return result; return result;
} }
@@ -3925,6 +3964,7 @@ void wallet_transaction_add(struct wallet *w, const struct wally_tx *tx,
db_bind_tx(stmt, 3, tx); db_bind_tx(stmt, 3, tx);
db_exec_prepared_v2(take(stmt)); db_exec_prepared_v2(take(stmt));
} else { } else {
db_col_ignore(stmt, "blockheight");
tal_free(stmt); tal_free(stmt);
if (blockheight) { if (blockheight) {
@@ -3994,6 +4034,8 @@ void wallet_transaction_annotate(struct wallet *w,
if (channel_id == 0 && !db_col_is_null(stmt, "channel_id")) if (channel_id == 0 && !db_col_is_null(stmt, "channel_id"))
channel_id = db_col_u64(stmt, "channel_id"); channel_id = db_col_u64(stmt, "channel_id");
else
db_col_ignore(stmt, "channel_id");
tal_free(stmt); tal_free(stmt);
@@ -4555,6 +4597,7 @@ struct wallet_transaction *wallet_transactions_get(struct wallet *w, const tal_t
cur->txindex = 0; cur->txindex = 0;
} }
} else { } else {
db_col_ignore(stmt, "t.txindex");
cur->blockheight = 0; cur->blockheight = 0;
cur->txindex = 0; cur->txindex = 0;
} }
@@ -4601,6 +4644,10 @@ struct wallet_transaction *wallet_transactions_get(struct wallet *w, const tal_t
&ann->channel); &ann->channel);
else else
ann->channel.u64 = 0; ann->channel.u64 = 0;
} else {
db_col_ignore(stmt, "ann_idx");
db_col_ignore(stmt, "annotation_type");
db_col_ignore(stmt, "c.short_channel_id");
} }
} }
tal_free(stmt); tal_free(stmt);
@@ -4736,9 +4783,14 @@ char *wallet_offer_find(const tal_t *ctx,
*label = NULL; *label = NULL;
else else
*label = db_col_json_escape(ctx, stmt, "label"); *label = db_col_json_escape(ctx, stmt, "label");
} } else
db_col_ignore(stmt, "label");
if (status) if (status)
*status = offer_status_in_db(db_col_int(stmt, "status")); *status = offer_status_in_db(db_col_int(stmt, "status"));
else
db_col_ignore(stmt, "status");
tal_free(stmt); tal_free(stmt);
return bolt12; return bolt12;
} }
@@ -4965,8 +5017,13 @@ struct db_stmt *wallet_datastore_next(const tal_t *ctx,
*key = db_col_datastore_key(ctx, stmt, "key"); *key = db_col_datastore_key(ctx, stmt, "key");
if (data) if (data)
*data = db_col_arr(ctx, stmt, "data", u8); *data = db_col_arr(ctx, stmt, "data", u8);
else
db_col_ignore(stmt, "data");
if (generation) if (generation)
*generation = db_col_u64(stmt, "generation"); *generation = db_col_u64(stmt, "generation");
else
db_col_ignore(stmt, "generation");
return stmt; return stmt;
} }