diff --git a/db/bindings.c b/db/bindings.c index ac2fc4fa2..34941d7f2 100644 --- a/db/bindings.c +++ b/db/bindings.c @@ -29,11 +29,9 @@ static bool db_column_null_warn(struct db_stmt *stmt, const char *colname, if (!db_column_is_null(stmt, col)) return false; - /* FIXME: log broken? */ -#if DEVELOPER - db_fatal("Accessing a null column %s/%i in query %s", - colname, col, stmt->query->query); -#endif /* DEVELOPER */ + db_warn(stmt->db, "Accessing a null column %s/%i in query %s", + colname, col, stmt->query->query); + return true; } @@ -441,7 +439,8 @@ struct bitcoin_tx *db_col_tx(const tal_t *ctx, struct db_stmt *stmt, const char /* Column wasn't null, but we couldn't retrieve a valid wally_tx! */ u8 *tx_dup = tal_dup_arr(stmt, u8, src, len, 0); - db_fatal("db_col_tx: Invalid bitcoin transaction bytes retrieved: %s", + db_fatal(stmt->db, + "db_col_tx: Invalid bitcoin transaction bytes retrieved: %s", tal_hex(stmt, tx_dup)); return NULL; } @@ -486,7 +485,7 @@ void *db_col_arr_(const tal_t *ctx, struct db_stmt *stmt, const char *colname, sourcelen = db_column_bytes(stmt, col); if (sourcelen % bytes != 0) - db_fatal("%s: %s/%zu column size for %zu not a multiple of %s (%zu)", + db_fatal(stmt->db, "%s: %s/%zu column size for %zu not a multiple of %s (%zu)", caller, colname, col, sourcelen, label, bytes); p = tal_arr_label(ctx, char, sourcelen, label); diff --git a/db/common.h b/db/common.h index 4ced5132a..f1ef49c1f 100644 --- a/db/common.h +++ b/db/common.h @@ -6,6 +6,7 @@ #include #include #include +#include /** * Macro to annotate a named SQL query. @@ -35,6 +36,10 @@ struct db { /* DB-specific context */ void *conn; + /* function to log warnings, or fail (if fatal == true). vprintf-style */ + void (*errorfn)(void *arg, bool fatal, const char *fmt, va_list ap); + void *errorfn_arg; + /* The configuration for the current database driver */ const struct db_config *config; @@ -183,9 +188,6 @@ struct db_config { const char **colnames, size_t num_cols); }; -void db_fatal(const char *fmt, ...) - PRINTF_FMT(1, 2); - /* Provide a way for DB backends to register themselves */ AUTODATA_TYPE(db_backends, struct db_config); diff --git a/db/db_postgres.c b/db/db_postgres.c index 0d3eb7f0b..20323a122 100644 --- a/db/db_postgres.c +++ b/db/db_postgres.c @@ -92,7 +92,7 @@ static PGresult *db_postgres_do_exec(struct db_stmt *stmt) switch (b->type) { case DB_BINDING_UNINITIALIZED: - db_fatal("DB binding not initialized: position=%zu, " + db_fatal(stmt->db, "DB binding not initialized: position=%zu, " "query=\"%s\n", i, stmt->query->query); case DB_BINDING_UINT64: @@ -172,7 +172,7 @@ static u64 db_postgres_column_u64(struct db_stmt *stmt, int col) size_t expected = sizeof(bin), actual = PQgetlength(res, stmt->row, col); if (expected != actual) - db_fatal( + db_fatal(stmt->db, "u64 field doesn't match size: expected %zu, actual %zu\n", expected, actual); @@ -187,7 +187,7 @@ static s64 db_postgres_column_int(struct db_stmt *stmt, int col) size_t expected = sizeof(bin), actual = PQgetlength(res, stmt->row, col); if (expected != actual) - db_fatal( + db_fatal(stmt->db, "s32 field doesn't match size: expected %zu, actual %zu\n", expected, actual); diff --git a/db/db_sqlite3.c b/db/db_sqlite3.c index 26cbe2e6a..876049ed3 100644 --- a/db/db_sqlite3.c +++ b/db/db_sqlite3.c @@ -28,7 +28,8 @@ sqlite3 *conn2sql(void *conn) return wrapper->conn; } -static void replicate_statement(struct db_sqlite3 *wrapper, +static void replicate_statement(const struct db *db, + struct db_sqlite3 *wrapper, const char *qry) { sqlite3_stmt *stmt; @@ -43,7 +44,7 @@ static void replicate_statement(struct db_sqlite3 *wrapper, sqlite3_finalize(stmt); if (err != SQLITE_DONE) - db_fatal("Failed to replicate query: %s: %s: %s", + db_fatal(db, "Failed to replicate query: %s: %s: %s", sqlite3_errstr(err), sqlite3_errmsg(wrapper->backup_conn), qry); @@ -53,7 +54,7 @@ static void db_sqlite3_changes_add(struct db_sqlite3 *wrapper, struct db_stmt *stmt, const char *qry) { - replicate_statement(wrapper, qry); + replicate_statement(stmt->db, wrapper, qry); db_changes_add(stmt, qry); } @@ -125,7 +126,7 @@ static bool db_sqlite3_setup(struct db *db) struct db_sqlite3 *wrapper; if (!strstarts(db->filename, "sqlite3://") || strlen(db->filename) < 10) - db_fatal("Could not parse the wallet DSN: %s", db->filename); + db_fatal(db, "Could not parse the wallet DSN: %s", db->filename); /* Strip the scheme from the dsn. */ filename = db->filename + strlen("sqlite3://"); @@ -143,14 +144,14 @@ static bool db_sqlite3_setup(struct db *db) err = sqlite3_open_v2(filename, &sql, flags, NULL); if (err != SQLITE_OK) { - db_fatal("failed to open database %s: %s", filename, + db_fatal(db, "failed to open database %s: %s", filename, sqlite3_errstr(err)); } wrapper->conn = sql; err = sqlite3_extended_result_codes(wrapper->conn, 1); if (err != SQLITE_OK) { - db_fatal("failed to enable extended result codes: %s", + db_fatal(db, "failed to enable extended result codes: %s", sqlite3_errstr(err)); } @@ -161,7 +162,7 @@ static bool db_sqlite3_setup(struct db *db) &wrapper->backup_conn, flags, NULL); if (err != SQLITE_OK) { - db_fatal("failed to open backup database %s: %s", + db_fatal(db, "failed to open backup database %s: %s", backup_filename, sqlite3_errstr(err)); } @@ -173,7 +174,7 @@ static bool db_sqlite3_setup(struct db *db) sqlite3_finalize(stmt); if (err != SQLITE_DONE) { - db_fatal("failed to use backup database %s: %s", + db_fatal(db, "failed to use backup database %s: %s", backup_filename, sqlite3_errstr(err)); } @@ -189,13 +190,13 @@ static bool db_sqlite3_setup(struct db *db) wrapper->conn, "main"); if (!copier) { - db_fatal("failed to initiate copy to %s: %s", + db_fatal(db, "failed to initiate copy to %s: %s", backup_filename, sqlite3_errmsg(wrapper->backup_conn)); } err = sqlite3_backup_step(copier, -1); if (err != SQLITE_DONE) { - db_fatal("failed to copy database to %s: %s", + db_fatal(db, "failed to copy database to %s: %s", backup_filename, sqlite3_errstr(err)); } @@ -232,7 +233,7 @@ static bool db_sqlite3_query(struct db_stmt *stmt) int pos = i+1; switch (b->type) { case DB_BINDING_UNINITIALIZED: - db_fatal("DB binding not initialized: position=%zu, " + db_fatal(stmt->db, "DB binding not initialized: position=%zu, " "query=\"%s\n", i, stmt->query->query); case DB_BINDING_UINT64: @@ -332,7 +333,7 @@ static bool db_sqlite3_begin_tx(struct db *db) db->error = tal_fmt(db, "Failed to begin a transaction: %s", errmsg); return false; } - replicate_statement(wrapper, "BEGIN TRANSACTION;"); + replicate_statement(db, wrapper, "BEGIN TRANSACTION;"); return true; } @@ -349,7 +350,7 @@ static bool db_sqlite3_commit_tx(struct db *db) db->error = tal_fmt(db, "Failed to commit a transaction: %s", errmsg); return false; } - replicate_statement(wrapper, "COMMIT;"); + replicate_statement(db, wrapper, "COMMIT;"); return true; } @@ -432,7 +433,7 @@ static bool db_sqlite3_vacuum(struct db *db) db->error = tal_fmt(db, "%s", sqlite3_errmsg(conn2sql(db->conn))); sqlite3_finalize(stmt); - replicate_statement(wrapper, "VACUUM;"); + replicate_statement(db, wrapper, "VACUUM;"); return err == SQLITE_DONE; } @@ -495,7 +496,7 @@ static char **prepare_table_manip(const tal_t *ctx, * mirror changes to the db hook! */ bracket = strchr(sql, '('); if (!strstarts(sql, "CREATE TABLE") || !bracket) - db_fatal("Bad sql from prepare_table_manip %s: %s", + db_fatal(db, "Bad sql from prepare_table_manip %s: %s", tablename, sql); /* Split after ( by commas: any lower case is assumed to be a field */ diff --git a/db/exec.c b/db/exec.c index 21b9beef3..758c40110 100644 --- a/db/exec.c +++ b/db/exec.c @@ -113,7 +113,7 @@ static void db_data_version_incr(struct db *db) db_bind_int(stmt, 0, db->data_version); db_exec_prepared_v2(stmt); if (db_count_changes(stmt) != 1) - db_fatal("Optimistic lock on the database failed. There" + db_fatal(stmt->db, "Optimistic lock on the database failed. There" " may be a concurrent access to the database." " Aborting since concurrent access is unsafe."); tal_free(stmt); @@ -124,7 +124,7 @@ void db_begin_transaction_(struct db *db, const char *location) { bool ok; if (db->in_transaction) - db_fatal("Already in transaction from %s", db->in_transaction); + db_fatal(db, "Already in transaction from %s", db->in_transaction); /* No writes yet. */ db->dirty = false; @@ -132,7 +132,7 @@ void db_begin_transaction_(struct db *db, const char *location) db_prepare_for_changes(db); ok = db->config->begin_tx_fn(db); if (!ok) - db_fatal("Failed to start DB transaction: %s", db->error); + db_fatal(db, "Failed to start DB transaction: %s", db->error); db->in_transaction = location; } @@ -156,7 +156,7 @@ void db_commit_transaction(struct db *db) ok = db->config->commit_tx_fn(db); if (!ok) - db_fatal("Failed to commit DB transaction: %s", db->error); + db_fatal(db, "Failed to commit DB transaction: %s", db->error); db->in_transaction = NULL; db->dirty = false; diff --git a/db/utils.c b/db/utils.c index 9449e0160..48ae718a9 100644 --- a/db/utils.c +++ b/db/utils.c @@ -24,7 +24,7 @@ size_t db_query_colnum(const struct db_stmt *stmt, for (;;) { const char *n = stmt->query->colnames[col].sqlname; if (!n) - db_fatal("Unknown column name %s in query %s", + db_fatal(stmt->db, "Unknown column name %s in query %s", colname, stmt->query->query); if (streq(n, colname)) break; @@ -41,7 +41,7 @@ size_t db_query_colnum(const struct db_stmt *stmt, static void db_stmt_free(struct db_stmt *stmt) { if (!stmt->executed) - db_fatal("Freeing an un-executed statement from %s: %s", + db_fatal(stmt->db, "Freeing an un-executed statement from %s: %s", stmt->location, stmt->query->query); #if DEVELOPER /* If they never got a db_step, we don't track */ @@ -51,7 +51,7 @@ static void db_stmt_free(struct db_stmt *stmt) continue; if (!strset_get(stmt->cols_used, stmt->query->colnames[i].sqlname)) { - db_fatal("Never accessed column %s in query %s", + db_fatal(stmt->db, "Never accessed column %s in query %s", stmt->query->colnames[i].sqlname, stmt->query->query); } @@ -104,14 +104,14 @@ struct db_stmt *db_prepare_v2_(const char *location, struct db *db, query_id += 2; if (!db->in_transaction) - db_fatal("Attempting to prepare a db_stmt outside of a " + db_fatal(db, "Attempting to prepare a db_stmt outside of a " "transaction: %s", location); /* Look up the query by its ID */ pos = hash_djb2(query_id) % db->queries->query_table_size; for (;;) { if (!db->queries->query_table[pos].name) - db_fatal("Could not resolve query %s", query_id); + db_fatal(db, "Could not resolve query %s", query_id); if (streq(query_id, db->queries->query_table[pos].name)) break; pos = (pos + 1) % db->queries->query_table_size; @@ -154,7 +154,7 @@ bool db_query_prepared_canfail(struct db_stmt *stmt) void db_query_prepared(struct db_stmt *stmt) { if (!db_query_prepared_canfail(stmt)) - db_fatal("query failed: %s: %s", + db_fatal(stmt->db, "query failed: %s: %s", stmt->location, stmt->query->query); } @@ -190,7 +190,7 @@ void db_exec_prepared_v2(struct db_stmt *stmt TAKES) * we report an error. */ if (!ret) { assert(stmt->error); - db_fatal("Error executing statement: %s", stmt->error); + db_fatal(stmt->db, "Error executing statement: %s", stmt->error); } if (taken(stmt)) @@ -260,7 +260,7 @@ void db_assert_no_outstanding_statements(struct db *db) stmt = list_top(&db->pending_statements, struct db_stmt, list); if (stmt) - db_fatal("Unfinalized statement %s", stmt->location); + db_fatal(stmt->db, "Unfinalized statement %s", stmt->location); } #else void db_assert_no_outstanding_statements(struct db *db) @@ -277,7 +277,7 @@ static void destroy_db(struct db *db) db->config->teardown_fn(db); } -static struct db_config *db_config_find(const char *dsn) +static struct db_config *db_config_find(const struct db *db, const char *dsn) { size_t num_configs; struct db_config **configs = autodata_get(db_backends, &num_configs); @@ -285,7 +285,7 @@ static struct db_config *db_config_find(const char *dsn) sep = strstr(dsn, "://"); if (!sep) - db_fatal("%s doesn't look like a valid data-source name (missing \"://\" separator.", dsn); + db_fatal(db, "%s doesn't look like a valid data-source name (missing \"://\" separator.", dsn); driver_name = tal_strndup(tmpctx, dsn, sep - dsn); @@ -319,23 +319,43 @@ void db_prepare_for_changes(struct db *db) db->changes = tal_arr(db, const char *, 0); } -struct db *db_open(const tal_t *ctx, const char *filename) +void db_fatal(const struct db *db, const char *fmt, ...) +{ + va_list ap; + va_start(ap, fmt); + db->errorfn(db->errorfn_arg, true, fmt, ap); + va_end(ap); +} + +void db_warn(const struct db *db, const char *fmt, ...) +{ + va_list ap; + va_start(ap, fmt); + db->errorfn(db->errorfn_arg, false, fmt, ap); + va_end(ap); +} + +struct db *db_open_(const tal_t *ctx, const char *filename, + void (*errorfn)(void *arg, bool fatal, const char *fmt, va_list ap), + void *arg) { struct db *db; db = tal(ctx, struct db); db->filename = tal_strdup(db, filename); + db->errorfn = errorfn; + db->errorfn_arg = arg; list_head_init(&db->pending_statements); if (!strstr(db->filename, "://")) - db_fatal("Could not extract driver name from \"%s\"", db->filename); + db_fatal(db, "Could not extract driver name from \"%s\"", db->filename); - db->config = db_config_find(db->filename); + db->config = db_config_find(db, db->filename); if (!db->config) - db_fatal("Unable to find DB driver for %s", db->filename); + db_fatal(db, "Unable to find DB driver for %s", db->filename); db->queries = db_queries_find(db->config); if (!db->queries) - db_fatal("Unable to find DB queries for %s", db->config->name); + db_fatal(db, "Unable to find DB queries for %s", db->config->name); tal_add_destructor(db, destroy_db); db->in_transaction = NULL; @@ -346,7 +366,7 @@ struct db *db_open(const tal_t *ctx, const char *filename) db_prepare_for_changes(db); if (db->config->setup_fn && !db->config->setup_fn(db)) - db_fatal("Error calling DB setup: %s", db->error); + db_fatal(db, "Error calling DB setup: %s", db->error); db_report_changes(db, NULL, 0); return db; diff --git a/db/utils.h b/db/utils.h index 8793d5c09..f83ea87b5 100644 --- a/db/utils.h +++ b/db/utils.h @@ -85,7 +85,15 @@ struct db_stmt *db_prepare_v2_(const char *location, struct db *db, /** * db_open - Open or create a database */ -struct db *db_open(const tal_t *ctx, const char *filename); +#define db_open(ctx, filename, errfn, arg) \ + db_open_((ctx), (filename), \ + typesafe_cb_postargs(void, void *, (errfn), (arg), \ + bool, const char *, va_list), \ + (arg)) + +struct db *db_open_(const tal_t *ctx, const char *filename, + void (*errorfn)(void *arg, bool fatal, const char *fmt, va_list ap), + void *arg); /** * Report a statement that changes the wallet @@ -110,4 +118,11 @@ const char **db_changes(struct db *db); * to re-use the normal db hook and replication logic. */ struct db_stmt *db_prepare_untranslated(struct db *db, const char *query); + +/* Errors and warnings... */ +void db_fatal(const struct db *db, const char *fmt, ...) + PRINTF_FMT(2, 3); +void db_warn(const struct db *db, const char *fmt, ...) + PRINTF_FMT(2, 3); + #endif /* LIGHTNING_DB_UTILS_H */ diff --git a/plugins/bkpr/db.c b/plugins/bkpr/db.c index e0774f115..8f76d9439 100644 --- a/plugins/bkpr/db.c +++ b/plugins/bkpr/db.c @@ -13,8 +13,6 @@ struct migration { void (*func)(struct plugin *p, struct db *db); }; -static struct plugin *plugin; - static void migration_remove_dupe_lease_fees(struct plugin *p, struct db *db); /* Do not reorder or remove elements from this array. @@ -173,7 +171,7 @@ static void migration_remove_dupe_lease_fees(struct plugin *p, struct db *db) continue; } - plugin_log(plugin, LOG_INFORM, + plugin_log(p, LOG_INFORM, "Duplicate 'lease_fee' found for" " account %"PRIu64", deleting dupe", id); @@ -187,30 +185,22 @@ static void migration_remove_dupe_lease_fees(struct plugin *p, struct db *db) tal_free(stmt); } -/* Implement db_fatal, as a wrapper around fatal. - * We use a ifndef block so that it can get be - * implemented in a test file first, if necessary */ -#ifndef DB_FATAL -#define DB_FATAL -void db_fatal(const char *fmt, ...) +static void db_error(struct plugin *plugin, bool fatal, const char *fmt, va_list ap) { - va_list ap; - va_start(ap, fmt); - plugin_errv(plugin, fmt, ap); - /* Won't actually exit, but va_end() required to balance va_start in standard. */ - va_end(ap); + if (fatal) + plugin_errv(plugin, fmt, ap); + else + plugin_logv(plugin, LOG_BROKEN, fmt, ap); } -#endif /* DB_FATAL */ struct db *db_setup(const tal_t *ctx, struct plugin *p, - const char *db_dsn, bool *created) + const char *db_dsn, + bool *created) { bool migrated; struct db *db; - /* Set global for db_fatal */ - plugin = p; - db = db_open(ctx, db_dsn); + db = db_open(ctx, db_dsn, db_error, p); db->report_changes_fn = NULL; db_begin_transaction(db); @@ -222,7 +212,7 @@ struct db *db_setup(const tal_t *ctx, struct plugin *p, * It's a good idea to do this every so often, and on db * upgrade is a reasonable time. */ if (migrated && !db->config->vacuum_fn(db)) - db_fatal("Error vacuuming db: %s", db->error); + db_fatal(db, "Error vacuuming db: %s", db->error); return db; } diff --git a/plugins/bkpr/recorder.c b/plugins/bkpr/recorder.c index a3561b02b..3a600614c 100644 --- a/plugins/bkpr/recorder.c +++ b/plugins/bkpr/recorder.c @@ -73,7 +73,7 @@ static struct chain_event **find_chain_events(const tal_t *ctx, db_query_prepared(stmt); if (stmt->error) - db_fatal("find_chain_events err: %s", stmt->error); + db_fatal(stmt->db, "find_chain_events err: %s", stmt->error); results = tal_arr(ctx, struct chain_event *, 0); while (db_step(stmt)) { struct chain_event *e = stmt2chain_event(results, stmt); @@ -1497,10 +1497,10 @@ static void insert_chain_fees_diff(struct db *db, /* These should apply perfectly, as we sorted them by * insert order */ if (!amount_msat_add(¤t_amt, current_amt, credit)) - db_fatal("Overflow when adding onchain fees"); + db_fatal(db, "Overflow when adding onchain fees"); if (!amount_msat_sub(¤t_amt, current_amt, debit)) - db_fatal("Underflow when subtracting onchain fees"); + db_fatal(db, "Underflow when subtracting onchain fees"); } tal_free(stmt); @@ -1512,7 +1512,7 @@ static void insert_chain_fees_diff(struct db *db, if (!amount_msat_sub(&credit, amount, current_amt)) { credit = AMOUNT_MSAT(0); if (!amount_msat_sub(&debit, current_amt, amount)) - db_fatal("shouldn't happen, unable to subtract"); + db_fatal(db, "shouldn't happen, unable to subtract"); } else debit = AMOUNT_MSAT(0); diff --git a/plugins/bkpr/test/run-bkpr_db.c b/plugins/bkpr/test/run-bkpr_db.c index 731bf4853..43bb60291 100644 --- a/plugins/bkpr/test/run-bkpr_db.c +++ b/plugins/bkpr/test/run-bkpr_db.c @@ -3,22 +3,6 @@ #include #include -#ifndef DB_FATAL -#define DB_FATAL -static char *db_err; -void db_fatal(const char *fmt, ...) -{ - va_list ap; - - /* Fail hard if we're complaining about not being in transaction */ - assert(!strstarts(fmt, "No longer in transaction")); - - va_start(ap, fmt); - db_err = tal_vfmt(NULL, fmt, ap); - va_end(ap); -} -#endif /* DB_FATAL */ - #include "common/json_filter.c" #include "plugins/bkpr/db.c" #include "plugins/libplugin.c" @@ -256,7 +240,7 @@ static struct db *create_test_db(void) char *dsn; dsn = tmp_dsn(NULL); - db = db_open(NULL, dsn); + db = db_open(NULL, dsn, db_error, (struct plugin *)NULL); db->data_version = 0; db->report_changes_fn = NULL; diff --git a/plugins/bkpr/test/run-recorder.c b/plugins/bkpr/test/run-recorder.c index 382631ede..90be3e2c9 100644 --- a/plugins/bkpr/test/run-recorder.c +++ b/plugins/bkpr/test/run-recorder.c @@ -23,22 +23,6 @@ #include #include -static char *db_err; -#ifndef DB_FATAL -#define DB_FATAL -void db_fatal(const char *fmt, ...) -{ - va_list ap; - - /* Fail hard if we're complaining about not being in transaction */ - assert(!strstarts(fmt, "No longer in transaction")); - - va_start(ap, fmt); - db_err = tal_vfmt(NULL, fmt, ap); - va_end(ap); -} -#endif /* DB_FATAL */ - #include "plugins/bkpr/db.c" @@ -424,7 +408,7 @@ static bool test_onchain_fee_wallet_spend(const tal_t *ctx, struct plugin *p) account_add(db, wal_acct); account_add(db, ext_acct); db_commit_transaction(db); - CHECK_MSG(!db_err, db_err); + /* Send funds to an external address * tag utxo_id vout txid debits credits acct_id @@ -460,12 +444,10 @@ static bool test_onchain_fee_wallet_spend(const tal_t *ctx, struct plugin *p) '1', 0, '*')); maybe_update_onchain_fees(ctx, db, &txid); db_commit_transaction(db); - CHECK_MSG(!db_err, db_err); db_begin_transaction(db); ofs = list_chain_fees(ctx, db); db_commit_transaction(db); - CHECK_MSG(!db_err, db_err); CHECK(tal_count(ofs) == 2); @@ -509,7 +491,6 @@ static bool test_onchain_fee_chan_close(const tal_t *ctx, struct plugin *p) account_add(db, ext_acct); account_add(db, acct); db_commit_transaction(db); - CHECK_MSG(!db_err, db_err); /* Close a channel */ /* tag utxo_id vout txid debits credits acct_id @@ -579,7 +560,6 @@ static bool test_onchain_fee_chan_close(const tal_t *ctx, struct plugin *p) /* Should be no fees yet */ ofs = list_chain_fees(ctx, db); - CHECK_MSG(!db_err, db_err); CHECK(tal_count(ofs) == 0); log_chain_event(db, acct, @@ -601,7 +581,6 @@ static bool test_onchain_fee_chan_close(const tal_t *ctx, struct plugin *p) memset(&txid, '1', sizeof(struct bitcoin_txid)); maybe_update_onchain_fees(ctx, db, &txid); db_commit_transaction(db); - CHECK_MSG(!db_err, db_err); /* txid 2222 */ db_begin_transaction(db); @@ -627,14 +606,12 @@ static bool test_onchain_fee_chan_close(const tal_t *ctx, struct plugin *p) maybe_mark_account_onchain(db, acct); CHECK(acct->onchain_resolved_block == 0); db_commit_transaction(db); - CHECK_MSG(!db_err, db_err); /* Expect: 1 onchain fee records, all for chan-1 */ db_begin_transaction(db); ofs = list_chain_fees(ctx, db); ofs1 = account_onchain_fees(ctx, db, acct); db_commit_transaction(db); - CHECK_MSG(!db_err, db_err); CHECK(tal_count(ofs) == tal_count(ofs1)); CHECK(tal_count(ofs) == 1); @@ -687,14 +664,12 @@ static bool test_onchain_fee_chan_close(const tal_t *ctx, struct plugin *p) maybe_update_onchain_fees(ctx, db, &txid); db_commit_transaction(db); - CHECK_MSG(!db_err, db_err); /* Expect: onchain fee records for tx except channel close */ db_begin_transaction(db); ofs = list_chain_fees(ctx, db); ofs1 = account_onchain_fees(ctx, db, acct); db_commit_transaction(db); - CHECK_MSG(!db_err, db_err); CHECK(tal_count(ofs) == tal_count(ofs1)); CHECK(tal_count(ofs) == 3); @@ -703,7 +678,6 @@ static bool test_onchain_fee_chan_close(const tal_t *ctx, struct plugin *p) CHECK(acct->onchain_resolved_block == 0); db_begin_transaction(db); maybe_mark_account_onchain(db, acct); - CHECK_MSG(!db_err, db_err); CHECK(acct->onchain_resolved_block == blockheight + 2); err = update_channel_onchain_fees(ctx, db, acct); CHECK_MSG(!err, err); @@ -781,7 +755,6 @@ static bool test_onchain_fee_chan_open(const tal_t *ctx, struct plugin *p) account_add(db, acct); account_add(db, acct2); db_commit_transaction(db); - CHECK_MSG(!db_err, db_err); /* Assumption that we rely on later */ CHECK(acct->db_id < acct2->db_id); @@ -840,14 +813,12 @@ static bool test_onchain_fee_chan_open(const tal_t *ctx, struct plugin *p) maybe_update_onchain_fees(ctx, db, &txid); db_commit_transaction(db); - CHECK_MSG(!db_err, db_err); /* Expect: 5 onchain fee records, totaling to 151/150msat ea, * none for wallet */ db_begin_transaction(db); ofs = list_chain_fees(ctx, db); db_commit_transaction(db); - CHECK_MSG(!db_err, db_err); CHECK(tal_count(ofs) == 5); @@ -924,7 +895,6 @@ static bool test_channel_rebalances(const tal_t *ctx, struct plugin *p) 'A'); log_channel_event(db, acct3, ev3); db_commit_transaction(db); - CHECK_MSG(!db_err, db_err); db_begin_transaction(db); chan_evs = account_get_channel_events(ctx, db, acct1); @@ -963,7 +933,6 @@ static bool test_channel_rebalances(const tal_t *ctx, struct plugin *p) CHECK(amount_msat_eq(rebals[0]->fee_msat, AMOUNT_MSAT(12))); db_commit_transaction(db); - CHECK_MSG(!db_err, db_err); return true; } @@ -984,7 +953,6 @@ static bool test_channel_event_crud(const tal_t *ctx, struct plugin *p) account_add(db, acct); account_add(db, acct2); db_commit_transaction(db); - CHECK_MSG(!db_err, db_err); ev1 = tal(ctx, struct channel_event); ev1->payment_id = tal(ev1, struct sha256); @@ -1041,13 +1009,10 @@ static bool test_channel_event_crud(const tal_t *ctx, struct plugin *p) log_channel_event(db, acct2, ev3); db_commit_transaction(db); - CHECK_MSG(!db_err, db_err); db_begin_transaction(db); chan_evs = account_get_channel_events(ctx, db, acct); db_commit_transaction(db); - CHECK_MSG(!db_err, db_err); - CHECK_MSG(!db_err, db_err); CHECK(streq(acct->name, chan_evs[0]->acct_name)); CHECK(streq(acct->name, chan_evs[1]->acct_name)); @@ -1077,7 +1042,6 @@ static bool test_chain_event_crud(const tal_t *ctx, struct plugin *p) account_add(db, acct); account_add(db, acct2); db_commit_transaction(db); - CHECK_MSG(!db_err, db_err); /* This event spends the second inserted event */ ev1 = tal(ctx, struct chain_event); @@ -1101,7 +1065,6 @@ static bool test_chain_event_crud(const tal_t *ctx, struct plugin *p) db_begin_transaction(db); log_chain_event(db, acct, ev1); db_commit_transaction(db); - CHECK_MSG(!db_err, db_err); ev2->tag = tal_fmt(ctx, "deposit"); ev2->origin_acct = tal_fmt(ctx, "wallet"); @@ -1145,19 +1108,16 @@ static bool test_chain_event_crud(const tal_t *ctx, struct plugin *p) /* log new event to a different account.. */ log_chain_event(db, acct2, ev3); db_commit_transaction(db); - CHECK_MSG(!db_err, db_err); /* Try to add an already exiting event */ db_begin_transaction(db); log_chain_event(db, acct, ev2); db_commit_transaction(db); - CHECK_MSG(!db_err, db_err); /* Ok now we ge the list, there should only be two */ db_begin_transaction(db); chain_evs = account_get_chain_events(ctx, db, acct); db_commit_transaction(db); - CHECK_MSG(!db_err, db_err); CHECK(tal_count(chain_evs) == 2); CHECK(streq(acct->name, chain_evs[0]->acct_name)); @@ -1183,7 +1143,6 @@ static bool test_chain_event_crud(const tal_t *ctx, struct plugin *p) log_chain_event(db, acct, ev2); chain_evs = account_get_chain_events(ctx, db, acct); db_commit_transaction(db); - CHECK_MSG(!db_err, db_err); /* There should be four now */ CHECK(tal_count(chain_evs) == 4); @@ -1269,7 +1228,6 @@ static bool test_account_balances(const tal_t *ctx, struct plugin *p) &balances, NULL); CHECK_MSG(!err, err); db_commit_transaction(db); - CHECK_MSG(!db_err, db_err); /* Should have 2 balances */ CHECK(tal_count(balances) == 2); @@ -1328,12 +1286,10 @@ static bool test_account_crud(const tal_t *ctx, struct plugin *p) db_begin_transaction(db); account_add(db, acct); db_commit_transaction(db); - CHECK_MSG(!db_err, db_err); db_begin_transaction(db); acct_list = list_accounts(ctx, db); db_commit_transaction(db); - CHECK_MSG(!db_err, db_err); CHECK(tal_count(acct_list) == 1); accountseq(acct_list[0], acct); @@ -1343,19 +1299,16 @@ static bool test_account_crud(const tal_t *ctx, struct plugin *p) db_begin_transaction(db); account_add(db, acct); db_commit_transaction(db); - CHECK_MSG(!db_err, db_err); db_begin_transaction(db); acct_list = list_accounts(ctx, db); db_commit_transaction(db); - CHECK_MSG(!db_err, db_err); CHECK(tal_count(acct_list) == 2); /* Can we find an account ok? */ db_begin_transaction(db); acct2 = find_account(ctx, db, "wallet"); db_commit_transaction(db); - CHECK_MSG(!db_err, db_err); accountseq(acct, acct2); /* Will we update an account's properties @@ -1413,7 +1366,6 @@ static bool test_account_crud(const tal_t *ctx, struct plugin *p) CHECK(acct->we_opened); db_commit_transaction(db); - CHECK_MSG(!db_err, db_err); return true; } diff --git a/wallet/db.c b/wallet/db.c index 12623164b..0c523002e 100644 --- a/wallet/db.c +++ b/wallet/db.c @@ -971,19 +971,15 @@ static bool db_migrate(struct lightningd *ld, struct db *db, else if (available < current) { err_msg = tal_fmt(tmpctx, "Refusing to migrate down from version %u to %u", current, available); - log_info(ld->log, "%s", err_msg); - db_fatal("%s", err_msg); + db_fatal(db, "%s", err_msg); } else if (current != available) { if (ld->db_upgrade_ok && *ld->db_upgrade_ok == false) { - err_msg = tal_fmt(tmpctx, "Refusing to upgrade db from version %u to %u (database-upgrade=false)", + db_fatal(db, + "Refusing to upgrade db from version %u to %u (database-upgrade=false)", current, available); - log_info(ld->log, "%s", err_msg); - db_fatal("%s", err_msg); } else if (!ld->db_upgrade_ok && !is_released_version()) { - err_msg = tal_fmt(tmpctx, "Refusing to irreversibly upgrade db from version %u to %u in non-final version %s (use --database-upgrade=true to override)", - current, available, version()); - log_info(ld->log, "%s", err_msg); - db_fatal("%s", err_msg); + db_fatal(db, "Refusing to irreversibly upgrade db from version %u to %u in non-final version %s (use --database-upgrade=true to override)", + current, available, version()); } log_info(ld->log, "Updating database from version %u to %u", current, available); @@ -1019,10 +1015,22 @@ static bool db_migrate(struct lightningd *ld, struct db *db, return current != orig; } +static void db_error(struct lightningd *ld, bool fatal, const char *fmt, va_list ap) +{ + va_list ap2; + + va_copy(ap2, ap); + logv(ld->log, LOG_BROKEN, NULL, true, fmt, ap); + + if (fatal) + fatal_vfmt(fmt, ap2); + va_end(ap2); +} + struct db *db_setup(const tal_t *ctx, struct lightningd *ld, const struct ext_key *bip32_base) { - struct db *db = db_open(ctx, ld->wallet_dsn); + struct db *db = db_open(ctx, ld->wallet_dsn, db_error, ld); bool migrated; db->report_changes_fn = plugin_hook_db_sync; @@ -1038,7 +1046,7 @@ struct db *db_setup(const tal_t *ctx, struct lightningd *ld, * It's a good idea to do this every so often, and on db * upgrade is a reasonable time. */ if (migrated && !db->config->vacuum_fn(db)) - db_fatal("Error vacuuming db: %s", db->error); + db_fatal(db, "Error vacuuming db: %s", db->error); return db; } @@ -1072,7 +1080,8 @@ static void migrate_our_funding(struct lightningd *ld, struct db *db) " WHERE funder = 0;")); /* 0 == LOCAL */ db_exec_prepared_v2(stmt); if (stmt->error) - db_fatal("Error migrating funding satoshis to our_funding (%s)", + db_fatal(stmt->db, + "Error migrating funding satoshis to our_funding (%s)", stmt->error); tal_free(stmt); @@ -1490,7 +1499,7 @@ static void migrate_channels_scids_as_integers(struct lightningd *ld, for (size_t i = 0; i < tal_count(scids); i++) { struct short_channel_id scid; if (!short_channel_id_from_str(scids[i], strlen(scids[i]), &scid)) - db_fatal("Cannot convert invalid channels.short_channel_id '%s'", + db_fatal(db, "Cannot convert invalid channels.short_channel_id '%s'", scids[i]); stmt = db_prepare_v2(db, SQL("UPDATE channels" @@ -1546,7 +1555,7 @@ static void migrate_payments_scids_as_integers(struct lightningd *ld, str = db_col_strdup(tmpctx, stmt, "failchannel"); if (!short_channel_id_from_str(str, strlen(str), &scid)) - db_fatal("Cannot convert invalid payments.failchannel '%s'", + db_fatal(db, "Cannot convert invalid payments.failchannel '%s'", str); update_stmt = db_prepare_v2(db, SQL("UPDATE payments SET" " failscid = ?" @@ -1559,7 +1568,7 @@ static void migrate_payments_scids_as_integers(struct lightningd *ld, tal_free(stmt); if (!db->config->delete_columns(db, "payments", colnames, ARRAY_SIZE(colnames))) - db_fatal("Could not delete payments.failchannel"); + db_fatal(db, "Could not delete payments.failchannel"); } static void fillin_missing_lease_satoshi(struct lightningd *ld, diff --git a/wallet/test/run-db.c b/wallet/test/run-db.c index 66f6d5e72..80cf0f19a 100644 --- a/wallet/test/run-db.c +++ b/wallet/test/run-db.c @@ -30,6 +30,9 @@ void derive_channel_id(struct channel_id *channel_id UNNEEDED, /* Generated stub for fatal */ void fatal(const char *fmt UNNEEDED, ...) { fprintf(stderr, "fatal called!\n"); abort(); } +/* Generated stub for fatal_vfmt */ +void fatal_vfmt(const char *fmt UNNEEDED, va_list ap UNNEEDED) +{ fprintf(stderr, "fatal_vfmt called!\n"); abort(); } /* Generated stub for fromwire_hsmd_get_channel_basepoints_reply */ bool fromwire_hsmd_get_channel_basepoints_reply(const void *p UNNEEDED, struct basepoints *basepoints UNNEEDED, struct pubkey *funding_pubkey UNNEEDED) { fprintf(stderr, "fromwire_hsmd_get_channel_basepoints_reply called!\n"); abort(); } @@ -43,6 +46,10 @@ void get_channel_basepoints(struct lightningd *ld UNNEEDED, struct basepoints *local_basepoints UNNEEDED, struct pubkey *local_funding_pubkey UNNEEDED) { fprintf(stderr, "get_channel_basepoints called!\n"); abort(); } +/* Generated stub for logv */ +void logv(struct log *log UNNEEDED, enum log_level level UNNEEDED, const struct node_id *node_id UNNEEDED, + bool call_notifier UNNEEDED, const char *fmt UNNEEDED, va_list ap UNNEEDED) +{ fprintf(stderr, "logv called!\n"); abort(); } /* Generated stub for psbt_fixup */ const u8 *psbt_fixup(const tal_t *ctx UNNEEDED, const u8 *psbtblob UNNEEDED) { fprintf(stderr, "psbt_fixup called!\n"); abort(); } @@ -60,19 +67,6 @@ bool wire_sync_write(int fd UNNEEDED, const void *msg TAKES UNNEEDED) { fprintf(stderr, "wire_sync_write called!\n"); abort(); } /* AUTOGENERATED MOCKS END */ -static char *db_err; -void db_fatal(const char *fmt, ...) -{ - va_list ap; - - /* Fail hard if we're complaining about not being in transaction */ - assert(!strstarts(fmt, "No longer in transaction")); - - va_start(ap, fmt); - db_err = tal_vfmt(NULL, fmt, ap); - va_end(ap); -} - void plugin_hook_db_sync(struct db *db UNNEEDED) { } @@ -89,7 +83,7 @@ static struct db *create_test_db(void) dsn = tal_fmt(NULL, "sqlite3://%s", filename); tal_free(filename); - db = db_open(NULL, dsn); + db = db_open(NULL, dsn, db_error, (struct lightningd *)NULL); db->data_version = 0; db->report_changes_fn = NULL; @@ -119,7 +113,7 @@ static bool test_primitives(void) { struct db_stmt *stmt; struct db *db = create_test_db(); - db_err = NULL; + db_begin_transaction(db); CHECK(db->in_transaction); db_commit_transaction(db); @@ -130,7 +124,6 @@ static bool test_primitives(void) db_begin_transaction(db); stmt = db_prepare_v2(db, SQL("SELECT name FROM sqlite_master WHERE type='table';")); db_exec_prepared_v2(stmt); - CHECK_MSG(!db_err, "Simple correct SQL command"); tal_free(stmt); /* We didn't migrate the DB, so don't have the vars table. Pretend we @@ -181,12 +174,10 @@ static bool test_manip_columns(void) ", field1 INTEGER" ", PRIMARY KEY (id))")); db_exec_prepared_v2(stmt); - CHECK_MSG(!db_err, "Simple correct SQL command"); tal_free(stmt); stmt = db_prepare_v2(db, SQL("INSERT INTO tablea (id, field1) VALUES (0, 1);")); db_exec_prepared_v2(stmt); - CHECK_MSG(!db_err, "Simple correct SQL command"); tal_free(stmt); stmt = db_prepare_v2(db, SQL("CREATE TABLE tableb (" @@ -194,22 +185,18 @@ static bool test_manip_columns(void) ", field1 INTEGER" ", field2 INTEGER);")); db_exec_prepared_v2(stmt); - CHECK_MSG(!db_err, "Simple correct SQL command"); tal_free(stmt); stmt = db_prepare_v2(db, SQL("INSERT INTO tableb (id, field1, field2) VALUES (0, 1, 2);")); db_exec_prepared_v2(stmt); - CHECK_MSG(!db_err, "Simple correct SQL command"); tal_free(stmt); /* Needs vars table, since this changes db. */ stmt = db_prepare_v2(db, SQL("CREATE TABLE vars (name VARCHAR(32), intval);")); db_exec_prepared_v2(stmt); - CHECK_MSG(!db_err, "Simple correct SQL command"); tal_free(stmt); stmt = db_prepare_v2(db, SQL("INSERT INTO vars VALUES ('data_version', 0);")); db_exec_prepared_v2(stmt); - CHECK_MSG(!db_err, "Simple correct SQL command"); tal_free(stmt); /* Rename tablea.field1 -> table1.field1a. */ @@ -219,7 +206,6 @@ static bool test_manip_columns(void) stmt = db_prepare_v2(db, SQL("SELECT id, field1a FROM tablea;")); CHECK_MSG(db_query_prepared_canfail(stmt), "db_query_prepared must succeed"); - CHECK_MSG(!db_err, "Simple correct SQL command"); CHECK(db_step(stmt)); CHECK(db_col_u64(stmt, "id") == 0); CHECK(db_col_u64(stmt, "field1a") == 1); @@ -228,7 +214,6 @@ static bool test_manip_columns(void) stmt = db_prepare_v2(db, SQL("SELECT id, field2 FROM tableb;")); CHECK_MSG(db_query_prepared_canfail(stmt), "db_query_prepared must succeed"); - CHECK_MSG(!db_err, "Simple correct SQL command"); CHECK(db_step(stmt)); CHECK(db_col_u64(stmt, "id") == 0); CHECK(db_col_u64(stmt, "field2") == 2); diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index 17014a95a..82650d294 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -10,24 +10,15 @@ static void db_log_(struct log *log UNUSED, enum log_level level UNUSED, const s } #define log_ db_log_ -#ifndef DB_FATAL -#define DB_FATAL static char *wallet_err; -void db_fatal(const char *fmt, ...) + +static void test_error(struct lightningd *ld, bool fatal, const char *fmt, va_list ap) { - va_list ap; - /* Fail hard if we're complaining about not being in transaction */ assert(!strstarts(fmt, "No longer in transaction")); - /* Fail hard if we're complaining about not being in transaction */ - assert(!strstarts(fmt, "No longer in transaction")); - - va_start(ap, fmt); wallet_err = tal_vfmt(NULL, fmt, ap); - va_end(ap); } -#endif /* DB_FATAL */ #include "wallet/wallet.c" #include "lightningd/hsm_control.c" @@ -159,6 +150,9 @@ char *encode_scriptpubkey_to_addr(const tal_t *ctx UNNEEDED, /* Generated stub for fatal */ void fatal(const char *fmt UNNEEDED, ...) { fprintf(stderr, "fatal called!\n"); abort(); } +/* Generated stub for fatal_vfmt */ +void fatal_vfmt(const char *fmt UNNEEDED, va_list ap UNNEEDED) +{ fprintf(stderr, "fatal_vfmt called!\n"); abort(); } /* Generated stub for fromwire_channeld_dev_memleak_reply */ bool fromwire_channeld_dev_memleak_reply(const void *p UNNEEDED, bool *leak UNNEEDED) { fprintf(stderr, "fromwire_channeld_dev_memleak_reply called!\n"); abort(); } @@ -501,6 +495,10 @@ bool json_tok_streq(const char *buffer UNNEEDED, const jsmntok_t *tok UNNEEDED, void kill_uncommitted_channel(struct uncommitted_channel *uc UNNEEDED, const char *why UNNEEDED) { fprintf(stderr, "kill_uncommitted_channel called!\n"); abort(); } +/* Generated stub for logv */ +void logv(struct log *log UNNEEDED, enum log_level level UNNEEDED, const struct node_id *node_id UNNEEDED, + bool call_notifier UNNEEDED, const char *fmt UNNEEDED, va_list ap UNNEEDED) +{ fprintf(stderr, "logv called!\n"); abort(); } /* Generated stub for new_channel_mvt_invoice_hin */ struct channel_coin_mvt *new_channel_mvt_invoice_hin(const tal_t *ctx UNNEEDED, struct htlc_in *hin UNNEEDED, @@ -1045,7 +1043,7 @@ static struct wallet *create_test_wallet(struct lightningd *ld, const tal_t *ctx close(fd); dsn = tal_fmt(NULL, "sqlite3://%s", filename); - w->db = db_open(w, dsn); + w->db = db_open(w, dsn, test_error, ld); w->db->report_changes_fn = NULL; tal_free(dsn); tal_add_destructor2(w, cleanup_test_wallet, filename); diff --git a/wallet/wallet.c b/wallet/wallet.c index f4efe8477..4c3f22cd4 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -49,21 +49,6 @@ struct channel_state_param { const enum channel_state_bucket state; }; -/* Implement db_fatal, as a wrapper around fatal. - * We use a ifndef block so that it can get be - * implemented in a test file first, if necessary */ -#ifndef DB_FATAL -#define DB_FATAL -void db_fatal(const char *fmt, ...) -{ - va_list ap; - - va_start(ap, fmt); - fatal_vfmt(fmt, ap); - va_end(ap); -} -#endif /* DB_FATAL */ - /* These go in db, so values cannot change (we can't put this into * lightningd/channel_state.h since it confuses cdump!) */ static enum state_change state_change_in_db(enum state_change s) @@ -648,7 +633,7 @@ bool wallet_has_funds(struct wallet *w, /* Overflow Should Not Happen */ if (!amount_sat_add(&total, total, utxo->amount)) { - db_fatal("Invalid value for %s: %s", + db_fatal(w->db, "Invalid value for %s: %s", type_to_string(tmpctx, struct bitcoin_outpoint, &utxo->outpoint), @@ -1269,7 +1254,7 @@ wallet_stmt2inflight(struct wallet *w, struct db_stmt *stmt, if (!db_col_is_null(stmt, "last_tx")) { last_tx = db_col_psbt_to_tx(tmpctx, stmt, "last_tx"); if (!last_tx) - db_fatal("Failed to decode inflight psbt %s", + db_fatal(w->db, "Failed to decode inflight psbt %s", tal_hex(tmpctx, db_col_arr(tmpctx, stmt, "last_tx", u8))); } else @@ -1555,7 +1540,7 @@ static struct channel *wallet_stmt2channel(struct wallet *w, struct db_stmt *stm if (!db_col_is_null(stmt, "last_tx")) { last_tx = db_col_psbt_to_tx(tmpctx, stmt, "last_tx"); if (!last_tx) - db_fatal("Failed to decode channel %s psbt %s", + db_fatal(w->db, "Failed to decode channel %s psbt %s", type_to_string(tmpctx, struct channel_id, &cid), tal_hex(tmpctx, db_col_arr(tmpctx, stmt, "last_tx", u8))); @@ -4637,7 +4622,7 @@ struct amount_msat wallet_total_forward_fees(struct wallet *w) deleted = amount_msat(db_get_intvar(w->db, "deleted_forward_fees", 0)); if (!amount_msat_add(&total, total, deleted)) - db_fatal("Adding forward fees %s + %s overflowed", + db_fatal(w->db, "Adding forward fees %s + %s overflowed", type_to_string(tmpctx, struct amount_msat, &total), type_to_string(tmpctx, struct amount_msat, &deleted));