db: add generic warn/error function.

This avoids the mess where we override db_fatal for teqsts, and keeps it
generic.

Also allows us to get rid of one #if DEVELOPER, and an ugly global for
bookkeeper.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

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;
 }

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(&current_amt, current_amt, credit))
-			db_fatal("Overflow when adding onchain fees");
+			db_fatal(db, "Overflow when adding onchain fees");
 
 		if (!amount_msat_sub(&current_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);
 

plugins/bkpr/test/run-bkpr_db.c

@@ -3,22 +3,6 @@
 #include <ccan/tal/str/str.h>
 #include <db/common.h>
 
-#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;
 

plugins/bkpr/test/run-recorder.c

@@ -23,22 +23,6 @@
 #include <unistd.h>
 #include <wire/wire.h>
 
-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;
 }