bkpr: remove currency support from balances.

We're going to get rid of this concept, but the main change is that the
account_get_balance API can be drastically simplified:

account_get_credit_debit() accesses the raw fields, never fails, but
returns the a flag which tells us if the account doesn't actually have
any events.

The one place we care about the balance, calculate by hand.  Then
account_get_balance() (and struct account_balance) can simply be
moved to th test.

Subtly, without the "GROUP BY" clause, you always get one row, even if
there are no rows (but the SUM are null).

Signed-off-by: Rusty Russell <[email protected]>

Author: rustyrussell

plugins/bkpr/bookkeeper.c

@@ -1,4 +1,5 @@
 #include "config.h"
+#include <bitcoin/chainparams.h>
 #include <ccan/array_size/array_size.h>
 #include <ccan/cast/cast.h>
 #include <ccan/json_escape/json_escape.h>
@@ -556,7 +557,6 @@ static struct command_result *json_list_balances(struct command *cmd,
 {
 	struct json_stream *res;
 	struct account **accts;
-	char *err;
 
 	if (!param(cmd, buf, params, NULL))
 		return command_param_failed();
@@ -568,18 +568,19 @@ static struct command_result *json_list_balances(struct command *cmd,
 
 	json_array_start(res, "accounts");
 	for (size_t i = 0; i < tal_count(accts); i++) {
-		struct acct_balance **balances;
-
-		err = account_get_balance(cmd, db,
-					  accts[i]->name,
-					  true,
-					  &balances);
+		struct amount_msat credit, debit, balance;
+		bool has_events;
 
-		if (err)
+		has_events = account_get_credit_debit(cmd->plugin, db,
+						      accts[i]->name,
+						      &credit, &debit);
+		if (!amount_msat_sub(&balance, credit, debit)) {
 			plugin_err(cmd->plugin,
-				   "Get account balance returned err"
-				   " for account %s: %s",
-				   accts[i]->name, err);
+				   "Account balance underflow for account %s (credit %s, debit %s)",
+				   accts[i]->name,
+				   fmt_amount_msat(tmpctx, credit),
+				   fmt_amount_msat(tmpctx, debit));
+		}
 
 		/* Skip the external acct balance, it's effectively
 		 * meaningless */
@@ -602,13 +603,15 @@ static struct command_result *json_list_balances(struct command *cmd,
 					     accts[i]->onchain_resolved_block);
 		}
 
+		/* FIXME: This API is now overkill! */
 		json_array_start(res, "balances");
-		for (size_t j = 0; j < tal_count(balances); j++) {
+		/* We expect no entry if account is not used. */
+		for (size_t j = 0; j < has_events; j++) {
 			json_object_start(res, NULL);
 			json_add_amount_msat(res, "balance_msat",
-					     balances[j]->balance);
+					     balance);
 			json_add_string(res, "coin_type",
-					balances[j]->currency);
+					chainparams->lightning_hrp);
 			json_object_end(res);
 		}
 		json_array_end(res);
@@ -951,8 +954,7 @@ static struct command_result *listpeerchannels_multi_done(struct command *cmd,
 	/* Let's register all these accounts! */
 	for (size_t i = 0; i < tal_count(new_accts); i++) {
 		struct new_account_info *info = new_accts[i];
-		struct acct_balance **balances, *bal;
-		struct amount_msat credit_diff, debit_diff;
+		struct amount_msat credit, debit, credit_diff, debit_diff;
 		char *err;
 
 		if (!new_missed_channel_account(cmd, buf, result,
@@ -966,25 +968,14 @@ static struct command_result *listpeerchannels_multi_done(struct command *cmd,
 		}
 
 		db_begin_transaction(db);
-		err = account_get_balance(tmpctx, db, info->acct->name,
-					  false, &balances);
+		account_get_credit_debit(cmd->plugin, db,
+					 info->acct->name,
+					 &credit, &debit);
 		db_commit_transaction(db);
 
-		if (err)
-			plugin_err(cmd->plugin, "%s", err);
-
-		/* FIXME: multiple currencies */
-		if (tal_count(balances) > 0)
-			bal = balances[0];
-		else {
-			bal = tal(tmpctx, struct acct_balance);
-			bal->credit = AMOUNT_MSAT(0);
-			bal->debit= AMOUNT_MSAT(0);
-		}
-
 		err = msat_find_diff(info->curr_bal,
-				     bal->credit,
-				     bal->debit,
+				     credit,
+				     debit,
 				     &credit_diff, &debit_diff);
 		if (err)
 			plugin_err(cmd->plugin, "%s", err);
@@ -1075,9 +1066,8 @@ static struct command_result *json_balance_snapshot(struct command *cmd,
 
 	db_begin_transaction(db);
 	json_for_each_arr(i, acct_tok, accounts_tok) {
-		struct acct_balance **balances, *bal;
 		struct account *acct;
-		struct amount_msat snap_balance, credit_diff, debit_diff;
+		struct amount_msat snap_balance, credit, debit, credit_diff, debit_diff;
 		char *acct_name, *currency;
 		bool existed;
 
@@ -1101,34 +1091,13 @@ static struct command_result *json_balance_snapshot(struct command *cmd,
 			   fmt_amount_msat(tmpctx, snap_balance));
 
 		/* Find the account balances */
-		err = account_get_balance(cmd, db, acct_name,
-					  /* Don't error if negative */
-					  false,
-					  &balances);
-
-		if (err)
-			plugin_err(cmd->plugin,
-				   "Get account balance returned err"
-				   " for account %s: %s",
-				   acct_name, err);
-
-		/* multiple currency balances! */
-		bal = NULL;
-		for (size_t j = 0; j < tal_count(balances); j++) {
-			if (streq(balances[j]->currency, currency))
-				bal = balances[j];
-		}
-
-		if (!bal) {
-			bal = tal(balances, struct acct_balance);
-			bal->credit = AMOUNT_MSAT(0);
-			bal->debit = AMOUNT_MSAT(0);
-		}
+		account_get_credit_debit(cmd->plugin, db, acct_name,
+					 &credit, &debit);
 
 		/* Figure out what the net diff is btw reported & actual */
 		err = msat_find_diff(snap_balance,
-				     bal->credit,
-				     bal->debit,
+				     credit,
+				     debit,
 				     &credit_diff, &debit_diff);
 		if (err)
 			plugin_err(cmd->plugin,
@@ -1398,37 +1367,23 @@ listpeerchannels_done(struct command *cmd,
 		      const jsmntok_t *result,
 		      struct event_info *info)
 {
-	struct acct_balance **balances, *bal;
-	struct amount_msat credit_diff, debit_diff;
+	struct amount_msat credit, debit, credit_diff, debit_diff;
 	const char *err;
 
 	if (new_missed_channel_account(cmd, buf, result,
 					info->acct,
 					info->ev->currency,
 					info->ev->timestamp)) {
 		db_begin_transaction(db);
-		err = account_get_balance(tmpctx, db, info->acct->name,
-					  false, &balances);
+		account_get_credit_debit(cmd->plugin, db, info->acct->name,
+					 &credit, &debit);
 		db_commit_transaction(db);
 
-		if (err)
-			plugin_err(cmd->plugin, "%s", err);
-
-		/* FIXME: multiple currencies per account? */
-		if (tal_count(balances) > 0)
-			bal = balances[0];
-		else {
-			bal = tal(balances, struct acct_balance);
-			bal->credit = AMOUNT_MSAT(0);
-			bal->debit = AMOUNT_MSAT(0);
-		}
-		assert(tal_count(balances) == 1);
-
 		/* The expected current balance is zero, since
 		 * we just got the channel close event */
 		err = msat_find_diff(AMOUNT_MSAT(0),
-				     bal->credit,
-				     bal->debit,
+				     credit,
+				     debit,
 				     &credit_diff, &debit_diff);
 		if (err)
 			plugin_err(cmd->plugin, "%s", err);

plugins/bkpr/recorder.c

@@ -15,6 +15,7 @@
 #include <plugins/bkpr/channel_event.h>
 #include <plugins/bkpr/onchain_fee.h>
 #include <plugins/bkpr/recorder.h>
+#include <plugins/libplugin.h>
 
 
 static struct chain_event *stmt2chain_event(const tal_t *ctx, struct db_stmt *stmt)
@@ -917,103 +918,72 @@ static struct chain_event *find_chain_event(const tal_t *ctx,
 	return e;
 }
 
-char *account_get_balance(const tal_t *ctx,
-			  struct db *db,
-			  const char *acct_name,
-			  bool calc_sum,
-			  struct acct_balance ***balances)
+bool account_get_credit_debit(struct plugin *plugin,
+			      struct db *db,
+			      const char *acct_name,
+			      struct amount_msat *credit,
+			      struct amount_msat *debit)
 {
 	struct db_stmt *stmt;
+	bool exists;
 
+	/* Get sum from chain_events */
 	stmt = db_prepare_v2(db, SQL("SELECT"
 				     "  CAST(SUM(ce.credit) AS BIGINT) as credit"
 				     ", CAST(SUM(ce.debit) AS BIGINT) as debit"
-				     ", ce.currency"
 				     " FROM chain_events ce"
 				     " LEFT OUTER JOIN accounts a"
 				     " ON a.id = ce.account_id"
-				     " WHERE a.name = ?"
-				     " GROUP BY ce.currency"));
-
+				     " WHERE a.name = ?"));
 	db_bind_text(stmt, acct_name);
 	db_query_prepared(stmt);
-	*balances = tal_arr(ctx, struct acct_balance *, 0);
-
-	while (db_step(stmt)) {
-		struct acct_balance *bal;
 
-		bal = tal(*balances, struct acct_balance);
-
-		bal->currency = db_col_strdup(bal, stmt, "ce.currency");
-		bal->credit = db_col_amount_msat(stmt, "credit");
-		bal->debit = db_col_amount_msat(stmt, "debit");
-		tal_arr_expand(balances, bal);
+	db_step(stmt);
+	if (db_col_is_null(stmt, "credit")) {
+		db_col_ignore(stmt, "debit");
+		*credit = *debit = AMOUNT_MSAT(0);
+		exists = false;
+	} else {
+		*credit = db_col_amount_msat(stmt, "credit");
+		*debit = db_col_amount_msat(stmt, "debit");
+		exists = true;
 	}
 	tal_free(stmt);
 
+	/* Get sum from channel_events */
 	stmt = db_prepare_v2(db, SQL("SELECT"
 				     "  CAST(SUM(ce.credit) AS BIGINT) as credit"
 				     ", CAST(SUM(ce.debit) AS BIGINT) as debit"
-				     ", ce.currency"
 				     " FROM channel_events ce"
 				     " LEFT OUTER JOIN accounts a"
 				     " ON a.id = ce.account_id"
-				     " WHERE a.name = ?"
-				     " GROUP BY ce.currency"));
+				     " WHERE a.name = ?"));
 	db_bind_text(stmt, acct_name);
 	db_query_prepared(stmt);
+	db_step(stmt);
 
-	while (db_step(stmt)) {
-		struct amount_msat amt;
-		struct acct_balance *bal = NULL;
-		char *currency;
-
-		currency = db_col_strdup(ctx, stmt, "ce.currency");
-
-		/* Find the currency entry from above */
-		for (size_t i = 0; i < tal_count(*balances); i++) {
-			if (streq((*balances)[i]->currency, currency)) {
-				bal = (*balances)[i];
-				break;
-			}
-		}
-
-		if (!bal) {
-			bal = tal(*balances, struct acct_balance);
-			bal->credit = AMOUNT_MSAT(0);
-			bal->debit = AMOUNT_MSAT(0);
-			bal->currency = tal_steal(bal, currency);
-			tal_arr_expand(balances, bal);
-		}
-
-		amt = db_col_amount_msat(stmt, "credit");
-		if (!amount_msat_accumulate(&bal->credit, amt)) {
-			tal_free(stmt);
-			return "overflow adding channel_event credits";
+	if (db_col_is_null(stmt, "credit")) {
+		db_col_ignore(stmt, "debit");
+	} else {
+		if (!amount_msat_accumulate(credit,
+					    db_col_amount_msat(stmt, "credit"))) {
+			plugin_err(plugin, "db overflow: chain credit %s, adding channel credit %s",
+				   fmt_amount_msat(tmpctx, *credit),
+				   fmt_amount_msat(tmpctx,
+						   db_col_amount_msat(stmt, "credit")));
 		}
 
-		amt = db_col_amount_msat(stmt, "debit");
-		if (!amount_msat_accumulate(&bal->debit, amt)) {
-			tal_free(stmt);
-			return "overflow adding channel_event debits";
+		if (!amount_msat_accumulate(debit,
+					    db_col_amount_msat(stmt, "debit"))) {
+			plugin_err(plugin, "db overflow: chain debit %s, adding channel debit %s",
+				   fmt_amount_msat(tmpctx, *debit),
+				   fmt_amount_msat(tmpctx,
+						   db_col_amount_msat(stmt, "debit")));
 		}
+		exists = true;
 	}
 	tal_free(stmt);
-
-	if (!calc_sum)
-		return NULL;
-
-	for (size_t i = 0; i < tal_count(*balances); i++) {
-		struct acct_balance *bal = (*balances)[i];
-		if (!amount_msat_sub(&bal->balance, bal->credit, bal->debit))
-			return tal_fmt(ctx,
-				"%s channel balance is negative? %s - %s",
-				bal->currency,
-				fmt_amount_msat(ctx, bal->credit),
-				fmt_amount_msat(ctx, bal->debit));
-	}
-
-	return NULL;
+	return exists;
 }
 
 struct channel_event **list_channel_events_timebox(const tal_t *ctx,

plugins/bkpr/recorder.h

@@ -9,18 +9,12 @@ struct bitcoin_txid;
 struct chain_event;
 struct channel_event;
 struct db;
+struct plugin;
 enum mvt_tag;
 struct onchain_fee;
 
 #define SQLITE_MAX_UINT 0x7FFFFFFFFFFFFFFF
 
-struct acct_balance {
-	char *currency;
-	struct amount_msat credit;
-	struct amount_msat debit;
-	struct amount_msat balance;
-};
-
 struct fee_sum {
 	u64 acct_db_id;
 	char *acct_name;
@@ -116,15 +110,14 @@ struct chain_event **get_chain_events_by_outpoint(const tal_t *ctx,
 						  const struct bitcoin_outpoint *outpoint,
 						  bool credits_only);
 
-/* Calculate the balances for an account
- *
- * @calc_sum     - compute the total balance. error if negative
- * */
-char *account_get_balance(const tal_t *ctx,
-			  struct db *db,
-			  const char *acct_name,
-			  bool calc_sum,
-			  struct acct_balance ***balances);
+/* Get total credits and debits for this account: returns false if no entries at all
+ * (in which case, credit and debit will both be AMOUNT_MSAT(0)). */
+bool account_get_credit_debit(struct plugin *plugin,
+			      struct db *db,
+			      const char *acct_name,
+			      struct amount_msat *credit,
+			      struct amount_msat *debit);
+
 
 /* Get chain fees for account */
 struct onchain_fee **account_get_chain_fees(const tal_t *ctx, struct db *db,