diff --git a/plugins/bkpr/bookkeeper.c b/plugins/bkpr/bookkeeper.c index 964dc70fc..95c55b0f4 100644 --- a/plugins/bkpr/bookkeeper.c +++ b/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); diff --git a/plugins/bkpr/incomestmt.c b/plugins/bkpr/incomestmt.c index ffc074a94..18866a89f 100644 --- a/plugins/bkpr/incomestmt.c +++ b/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) diff --git a/plugins/bkpr/incomestmt.h b/plugins/bkpr/incomestmt.h index e24f3ecb9..50b42b4ef 100644 --- a/plugins/bkpr/incomestmt.h +++ b/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); diff --git a/plugins/bkpr/recorder.c b/plugins/bkpr/recorder.c index 68969baf4..bd5712f43 100644 --- a/plugins/bkpr/recorder.c +++ b/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; } diff --git a/plugins/bkpr/recorder.h b/plugins/bkpr/recorder.h index 228ce07b7..c71daab61 100644 --- a/plugins/bkpr/recorder.h +++ b/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 */ diff --git a/tests/test_bookkeeper.py b/tests/test_bookkeeper.py index cb0a12920..620cde37e 100644 --- a/tests/test_bookkeeper.py +++ b/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 - assert len(incomes) == 5 + # 2 wallet deposits, 1 wallet withdrawal, 1 onchain_fee + 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 - for e in fee_events: - # 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 len(fee_events) == 1 + fees = Millisatoshi(fee_events[0]['debit']) # 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 - for e in fee_events: - # 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 len(fee_events) == 1 + fees = Millisatoshi(fee_events[0]['debit']) 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