bkpr: add a 'consolidate-fees' flag to the income stmt

Defaults to true. This is actually what you want to see -- the fees for
an account's txid collapsed into a single entry.

plugins/bkpr/bookkeeper.c

@@ -44,14 +44,17 @@ static struct command_result *json_list_income(struct command *cmd,
 {
 	struct json_stream *res;
 	struct income_event **evs;
+	bool *consolidate_fees;
 
 	if (!param(cmd, buf, params,
+		   p_opt_def("consolidate_fees", param_bool,
+			     &consolidate_fees, true),
 		   NULL))
 		return command_param_failed();
 
 	/* Ok, go find me some income events! */
 	db_begin_transaction(db);
-	evs = list_income_events_all(cmd, db);
+	evs = list_income_events_all(cmd, db, *consolidate_fees);
 	db_commit_transaction(db);
 
 	res = jsonrpc_stream_success(cmd);

plugins/bkpr/incomestmt.c

@@ -217,10 +217,47 @@ static struct income_event *maybe_channel_income(const tal_t *ctx,
 	return channel_to_income(ctx, ev, ev->credit, ev->debit);
 }
 
+static struct onchain_fee **find_consolidated_fees(const tal_t *ctx,
+						   struct db *db,
+						   struct onchain_fee **fees TAKES)
+{
+	struct fee_sum **sums;
+	struct onchain_fee **updated_fees
+		= tal_arr(ctx, struct onchain_fee *, 0);
+
+	sums = calculate_onchain_fee_sums(ctx, db);
+
+	for (size_t i = 0; i < tal_count(sums); i++) {
+		/* Find the last matching feerate's data */
+		for (size_t j = tal_count(fees); j > 0; j--) {
+			struct onchain_fee *fee;
+			if (bitcoin_txid_eq(&fees[j - 1]->txid,
+					    sums[i]->txid)
+			    && fees[j - 1]->acct_db_id == sums[i]->acct_db_id) {
+				fee = tal_steal(updated_fees, fees[j - 1]);
+				fee->credit = sums[i]->fees_paid;
+				fee->debit = AMOUNT_MSAT(0);
+				tal_arr_expand(&updated_fees, fee);
+				fees[j - 1] = NULL;
+				break;
+			}
+		}
+
+		/* It's possible there weren't any fee events
+		 * for this txid in the time period we've selected */
+	}
+
+	if (taken(fees))
+		tal_free(fees);
+
+	return updated_fees;
+}
+
 struct income_event **list_income_events(const tal_t *ctx,
 					 struct db *db,
 					 u64 start_time,
-					 u64 end_time)
+					 u64 end_time,
+					 bool consolidate_fees)
 {
 	struct channel_event **channel_events;
 	struct chain_event **chain_events;
@@ -235,6 +272,10 @@ struct income_event **list_income_events(const tal_t *ctx,
 	onchain_fees = list_chain_fees_timebox(ctx, db, start_time, end_time);
 	accts = list_accounts(ctx, db);
 
+	if (consolidate_fees)
+		onchain_fees = find_consolidated_fees(ctx, db,
+						      take(onchain_fees));
+
 	evs = tal_arr(ctx, struct income_event *, 0);
 
 	for (size_t i = 0, j = 0, k = 0;
@@ -302,9 +343,11 @@ struct income_event **list_income_events(const tal_t *ctx,
 	return evs;
 }
 
-struct income_event **list_income_events_all(const tal_t *ctx, struct db *db)
+struct income_event **list_income_events_all(const tal_t *ctx, struct db *db,
+					     bool consolidate_fees)
 {
-	return list_income_events(ctx, db, 0, SQLITE_MAX_UINT);
+	return list_income_events(ctx, db, 0, SQLITE_MAX_UINT,
+				  consolidate_fees);
 }
 
 void json_add_income_event(struct json_stream *out, struct income_event *ev)

plugins/bkpr/incomestmt.h

@@ -18,14 +18,16 @@ struct income_event {
 };
 
 /* List all the events that are income related (gain/loss) */
-struct income_event **list_income_events_all(const tal_t *ctx, struct db *db);
+struct income_event **list_income_events_all(const tal_t *ctx, struct db *db,
+					     bool consolidate_fees);
 
 /* List all the events that are income related (gain/loss),
  * by a start and end date */
 struct income_event **list_income_events(const tal_t *ctx,
 					 struct db *db,
 					 u64 start_time,
-					 u64 end_time);
+					 u64 end_time,
+					 bool consolidate_fees);
 
 /* Given an event and a json_stream, add a new event object to the stream */
 void json_add_income_event(struct json_stream *str, struct income_event *ev);

plugins/bkpr/recorder.c

@@ -209,6 +209,43 @@ static struct chain_event **find_txos_for_tx(const tal_t *ctx,
 	return find_chain_events(ctx, take(stmt));
 }
 
+struct fee_sum **calculate_onchain_fee_sums(const tal_t *ctx, struct db *db)
+{
+	struct db_stmt *stmt;
+	struct fee_sum **sums;
+	stmt = db_prepare_v2(db, SQL("SELECT"
+				     "  txid"
+				     ", account_id"
+				     ", SUM(credit)"
+				     ", SUM(debit)"
+				     " FROM onchain_fees"
+				     " GROUP BY txid, account_id"
+				     " ORDER BY txid, account_id"));
+
+	db_query_prepared(stmt);
+
+	sums = tal_arr(ctx, struct fee_sum *, 0);
+	while (db_step(stmt)) {
+		struct fee_sum *sum;
+		struct amount_msat amt;
+		bool ok;
+
+		sum = tal(sums, struct fee_sum);
+		sum->txid = tal(sum, struct bitcoin_txid);
+		db_col_txid(stmt, "txid", sum->txid);
+		sum->acct_db_id = db_col_u64(stmt, "account_id");
+
+		db_col_amount_msat(stmt, "SUM(credit)", &sum->fees_paid);
+		db_col_amount_msat(stmt, "SUM(debit)", &amt);
+		ok = amount_msat_sub(&sum->fees_paid, sum->fees_paid, amt);
+		assert(ok);
+		tal_arr_expand(&sums, sum);
+	}
+
+	tal_free(stmt);
+	return sums;
+}
+
 struct fee_sum **find_account_onchain_fees(const tal_t *ctx,
 					   struct db *db,
 					   struct account *acct)
@@ -234,6 +271,7 @@ struct fee_sum **find_account_onchain_fees(const tal_t *ctx,
 		bool ok;
 
 		sum = tal(sums, struct fee_sum);
+		sum->acct_db_id = acct->db_id;
 		sum->txid = tal(sum, struct bitcoin_txid);
 		db_col_txid(stmt, "txid", sum->txid);
 
@@ -244,6 +282,7 @@ struct fee_sum **find_account_onchain_fees(const tal_t *ctx,
 		tal_arr_expand(&sums, sum);
 	}
 
+	tal_free(stmt);
 	return sums;
 }
 

plugins/bkpr/recorder.h

@@ -24,6 +24,7 @@ struct acct_balance {
 };
 
 struct fee_sum {
+	u64 acct_db_id;
 	struct bitcoin_txid *txid;
 	struct amount_msat fees_paid;
 };
@@ -134,6 +135,9 @@ struct fee_sum **find_account_onchain_fees(const tal_t *ctx,
 					   struct db *db,
 					   struct account *acct);
 
+/* Final all the onchain fees */
+struct fee_sum **calculate_onchain_fee_sums(const tal_t *ctx, struct db *db);
+
 /* Add the given account to the database */
 void account_add(struct db *db, struct account *acct);
 /* Given an account name, find that account record */

tests/test_bookkeeper.py

@@ -22,7 +22,7 @@ def find_first_tag(evs, tag):
 
 
 @pytest.mark.developer("dev-ignore-htlcs")
-def test_closing_trimmed_htlcs(node_factory, bitcoind, executor):
+def test_bookkeeping_closing_trimmed_htlcs(node_factory, bitcoind, executor):
     l1, l2 = node_factory.line_graph(2)
 
     # Send l2 funds via the channel
@@ -69,7 +69,7 @@ def test_closing_trimmed_htlcs(node_factory, bitcoind, executor):
     assert Millisatoshi(close_fee[0]['credit']) + Millisatoshi(deposit['credit']) == Millisatoshi(close['debit'])
 
 
-def test_closing_subsat_htlcs(node_factory, bitcoind, chainparams):
+def test_bookkeeping_closing_subsat_htlcs(node_factory, bitcoind, chainparams):
     """Test closing balances when HTLCs are: sub 1-satoshi"""
     l1, l2 = node_factory.line_graph(2)
 
@@ -172,23 +172,14 @@ def test_bookkeeping_external_withdraws(node_factory, bitcoind):
     # expect the withdrawal to appear in the incomes
     # and there should be an onchain fee
     incomes = l1.rpc.listincome()['income_events']
-    # 2 wallet deposits, 1 wallet withdrawal, 2 onchain_fees
+    # 2 wallet deposits, 1 wallet withdrawal, 1 onchain_fee
-    assert len(incomes) == 5
+    assert len(incomes) == 4
     withdraw_amt = Millisatoshi(find_tags(incomes, 'withdrawal')[0]['debit'])
     assert withdraw_amt == Millisatoshi(amount // 2 * 1000)
 
     fee_events = find_tags(incomes, 'onchain_fee')
-    fees = 0
+    assert len(fee_events) == 1
-    for e in fee_events:
+    fees = Millisatoshi(fee_events[0]['debit'])
-        # Millisatoshis cant be negative, so we reverse
-        # the credit/debit on the chain fee events and
-        # deal with them as ints instead of millisatoshis
-        fees -= int(e['credit'][:-4])
-        fees += int(e['debit'][:-4])
-
-    # In sum, the fees are a net loss
-    assert fees > 0
-    fees = Millisatoshi(fees)
 
     # wallet balance is decremented now
     btc_balance = only_one(only_one(l1.rpc.listbalances()['accounts'])['balances'])
@@ -253,17 +244,8 @@ def test_bookkeeping_external_withdraw_missing(node_factory, bitcoind):
     assert len(find_tags(incomes, 'withdrawal')) == 0
 
     fee_events = find_tags(incomes, 'onchain_fee')
-    fees = 0
+    assert len(fee_events) == 1
-    for e in fee_events:
+    fees = Millisatoshi(fee_events[0]['debit'])
-        # Millisatoshis cant be negative, so we reverse
-        # the credit/debit on the chain fee events and
-        # deal with them as ints instead of millisatoshis
-        fees -= int(e['credit'][:-4])
-        fees += int(e['debit'][:-4])
-
-    # In sum, the fees are a net loss
-    assert fees > 0
-    fees = Millisatoshi(fees)
     assert fees > Millisatoshi(amount // 2 * 1000)
 
     # wallet balance is decremented now
@@ -328,6 +310,11 @@ def test_bookkeeping_rbf_withdraw(node_factory, bitcoind):
 
     # make sure no onchain fees are counted for the replaced tx
     fees = find_tags(acct_evs, 'onchain_fee')
-    assert len(fees) > 0
+    assert len(fees) > 1
     for fee in fees:
         assert fee['txid'] == out2['txid']
+
+    fees = find_tags(l1.rpc.listincome(consolidate_fees=False)['income_events'], 'onchain_fee')
+    assert len(fees) == 2
+    fees = find_tags(l1.rpc.listincome(consolidate_fees=True)['income_events'], 'onchain_fee')
+    assert len(fees) == 1