mirrors/core-lightning
1
0
Fork 0
mirror of https://github.com/ElementsProject/lightning.git synced 2024-11-19 09:54:16 +01:00
Code Issues Projects Releases Wiki Activity

bkpr: dont log unknown accounts with zero balances

Browse Source 
We were putting out a lot of empty journal entries. Let's
stop doing that.

Now the wallet balance stays uninitialized until/unless you have
funds in it.

Fixes #5672
This commit is contained in:
niftynei 2023-11-21 14:53:31 -06:00 committed by Rusty Russell
parent 040af90e7d
commit 5484aaee33
3 changed files with 61 additions and 53 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

76
plugins/bkpr/bookkeeper.c
View File

@ -984,9 +984,10 @@ static struct command_result *json_balance_snapshot(struct command *cmd,
db_begin_transaction(db); db_begin_transaction(db);
json_for_each_arr(i, acct_tok, accounts_tok) { json_for_each_arr(i, acct_tok, accounts_tok) {
struct acct_balance **balances, *bal; struct acct_balance **balances, *bal;
struct account *acct;
struct amount_msat snap_balance, credit_diff, debit_diff; struct amount_msat snap_balance, credit_diff, debit_diff;
char *acct_name, *currency; char *acct_name, *currency;
bool exists; bool existed;
err = json_scan(cmd, buf, acct_tok, err = json_scan(cmd, buf, acct_tok,
"{account_id:%" "{account_id:%"
@ -1016,7 +1017,7 @@ static struct command_result *json_balance_snapshot(struct command *cmd,
* balances items */ * balances items */
true, true,
&balances, &balances,
&exists); NULL);
if (err) if (err)
plugin_err(cmd->plugin, plugin_err(cmd->plugin,
@ -1043,14 +1044,41 @@ static struct command_result *json_balance_snapshot(struct command *cmd,
"Unable to find_diff for amounts: %s", "Unable to find_diff for amounts: %s",
err); err);
if (!exists acct = find_account(cmd, db, acct_name);
|| !amount_msat_zero(credit_diff) if (!acct) {
|| !amount_msat_zero(debit_diff)) { plugin_log(cmd->plugin, LOG_INFORM,
struct account *acct; "account %s not found, adding",
struct channel_event *ev; acct_name);
/* FIXME: lookup peer id for channel? */
acct = new_account(cmd, acct_name, NULL);
account_add(db, acct);
existed = false;
} else
existed = true;
/* If we're entering a channel account,
* from a balance entry, we need to
* go find the channel open info*/
if (!existed && is_channel_account(acct)) {
struct new_account_info *info;
u64 timestamp_now; u64 timestamp_now;
/* This is *expected* on first run of bookkeeper! */ timestamp_now = time_now().ts.tv_sec;
info = tal(new_accts, struct new_account_info);
info->acct = tal_steal(info, acct);
info->curr_bal = snap_balance;
info->timestamp = timestamp_now;
info->currency =
tal_strdup(info, currency);
tal_arr_expand(&new_accts, info);
continue;
}
if (!amount_msat_zero(credit_diff) || !amount_msat_zero(debit_diff)) {
struct channel_event *ev;
plugin_log(cmd->plugin, plugin_log(cmd->plugin,
tom_jones ? LOG_DBG : LOG_UNUSUAL, tom_jones ? LOG_DBG : LOG_UNUSUAL,
"Snapshot balance does not equal ondisk" "Snapshot balance does not equal ondisk"
@ -1064,38 +1092,6 @@ static struct command_result *json_balance_snapshot(struct command *cmd,
&credit_diff), &credit_diff),
acct_name); acct_name);
timestamp_now = time_now().ts.tv_sec;
/* Log a channel "journal entry" to get
* the balances inline */
acct = find_account(cmd, db, acct_name);
if (!acct) {
struct new_account_info *info;
plugin_log(cmd->plugin, LOG_INFORM,
"account %s not found, adding"
" along with new balance",
acct_name);
/* FIXME: lookup peer id for channel? */
acct = new_account(cmd, acct_name, NULL);
account_add(db, acct);
/* If we're entering a channel account,
* from a balance entry, we need to
* go find the channel open info*/
if (is_channel_account(acct)) {
info = tal(new_accts, struct new_account_info);
info->acct = tal_steal(info, acct);
info->curr_bal = snap_balance;
info->timestamp = timestamp_now;
info->currency =
tal_strdup(info, currency);
tal_arr_expand(&new_accts, info);
continue;
}
}
ev = new_channel_event(cmd, ev = new_channel_event(cmd,
tal_fmt(tmpctx, "%s", tal_fmt(tmpctx, "%s",

35
tests/test_bookkeeper.py
View File

@ -152,7 +152,7 @@ def test_bookkeeping_external_withdraws(node_factory, bitcoind):
assert inc['credit_msat'] == amount_msat assert inc['credit_msat'] == amount_msat
# The event should show up in the 'bkpr_listaccountevents' however # The event should show up in the 'bkpr_listaccountevents' however
events = l1.rpc.bkpr_listaccountevents()['events'] events = l1.rpc.bkpr_listaccountevents()['events']
assert len(events) == 4 assert len(events) == 3
external = [e for e in events if e['account'] == 'external'][0] external = [e for e in events if e['account'] == 'external'][0]
assert external['credit_msat'] == Millisatoshi(amount // 2 * 1000) assert external['credit_msat'] == Millisatoshi(amount // 2 * 1000)
@ -169,8 +169,8 @@ def test_bookkeeping_external_withdraws(node_factory, bitcoind):
assert len(find_tags(incomes, 'journal_entry')) == 0 assert len(find_tags(incomes, 'journal_entry')) == 0
assert len(incomes) == 2 assert len(incomes) == 2
events = l1.rpc.bkpr_listaccountevents()['events'] events = l1.rpc.bkpr_listaccountevents()['events']
assert len(events) == 4 assert len(events) == 3
assert len(find_tags(events, 'journal_entry')) == 1 assert len(find_tags(events, 'journal_entry')) == 0
# the wallet balance should be unchanged # the wallet balance should be unchanged
btc_balance = only_one(only_one(l1.rpc.bkpr_listbalances()['accounts'])['balances']) btc_balance = only_one(only_one(l1.rpc.bkpr_listbalances()['accounts'])['balances'])
@ -223,8 +223,8 @@ def test_bookkeeping_external_withdraw_missing(node_factory, bitcoind):
# Only two income events: deposits # Only two income events: deposits
assert len(l1.rpc.bkpr_listincome()['income_events']) == 2 assert len(l1.rpc.bkpr_listincome()['income_events']) == 2
# 4 account events: empty wallet start, 2 wallet deposits, 1 external deposit # 4 account events: 2 wallet deposits, 1 external deposit
assert len(l1.rpc.bkpr_listaccountevents()['events']) == 4 assert len(l1.rpc.bkpr_listaccountevents()['events']) == 3
# Stop node and remove the accounts data # Stop node and remove the accounts data
l1.stop() l1.stop()
@ -274,22 +274,31 @@ def test_bookkeeping_rbf_withdraw(node_factory, bitcoind):
addr = l1.rpc.newaddr()['bech32'] addr = l1.rpc.newaddr()['bech32']
amount = 1111111 amount = 1111111
event_counter = 0
income_counter = 0
bitcoind.rpc.sendtoaddress(addr, amount / 10**8) bitcoind.rpc.sendtoaddress(addr, amount / 10**8)
event_counter += 1
income_counter += 1
bitcoind.generate_block(1) bitcoind.generate_block(1)
wait_for(lambda: len(l1.rpc.listfunds()['outputs']) == 1) wait_for(lambda: len(l1.rpc.listfunds()['outputs']) == 1)
assert len(l1.rpc.bkpr_listaccountevents()['events']) == 2 assert len(l1.rpc.bkpr_listaccountevents()['events']) == event_counter
assert len(l1.rpc.bkpr_listincome()['income_events']) == 1 assert len(l1.rpc.bkpr_listincome()['income_events']) == income_counter
# Ok, now we send some funds to an external address # Ok, now we send some funds to an external address
waddr = l1.bitcoin.rpc.getnewaddress() waddr = l1.bitcoin.rpc.getnewaddress()
out1 = l1.rpc.withdraw(waddr, amount // 2, feerate='253perkw') out1 = l1.rpc.withdraw(waddr, amount // 2, feerate='253perkw')
event_counter += 1
mempool = bitcoind.rpc.getrawmempool(True) mempool = bitcoind.rpc.getrawmempool(True)
assert len(list(mempool.keys())) == 1 assert len(list(mempool.keys())) == 1
assert out1['txid'] in list(mempool.keys()) assert out1['txid'] in list(mempool.keys())
# another account event, still one income event # another account event, still one income event
assert len(l1.rpc.bkpr_listaccountevents()['events']) == 3 assert len(l1.rpc.bkpr_listaccountevents()['events']) == event_counter
assert len(l1.rpc.bkpr_listincome()['income_events']) == 1 assert len(l1.rpc.bkpr_listincome()['income_events']) == income_counter
# unreserve the existing output # unreserve the existing output
l1.rpc.unreserveinputs(out1['psbt'], 200) l1.rpc.unreserveinputs(out1['psbt'], 200)
@ -297,12 +306,14 @@ def test_bookkeeping_rbf_withdraw(node_factory, bitcoind):
# resend the tx # resend the tx
out2 = l1.rpc.withdraw(waddr, amount // 2, feerate='1000perkw') out2 = l1.rpc.withdraw(waddr, amount // 2, feerate='1000perkw')
mempool = bitcoind.rpc.getrawmempool(True) mempool = bitcoind.rpc.getrawmempool(True)
event_counter += 1
assert len(list(mempool.keys())) == 1 assert len(list(mempool.keys())) == 1
assert out2['txid'] in list(mempool.keys()) assert out2['txid'] in list(mempool.keys())
# another account event, still one income event # another account event, still one income event
assert len(l1.rpc.bkpr_listaccountevents()['events']) == 4 assert len(l1.rpc.bkpr_listaccountevents()['events']) == event_counter
assert len(l1.rpc.bkpr_listincome()['income_events']) == 1 assert len(l1.rpc.bkpr_listincome()['income_events']) == income_counter
# ok now we mine a block # ok now we mine a block
bitcoind.generate_block(1) bitcoind.generate_block(1)
@ -583,7 +594,7 @@ def test_bookkeeping_onchaind_txs(node_factory, bitcoind):
# Wait for the balance snapshot to fire/finish # Wait for the balance snapshot to fire/finish
l1.daemon.wait_for_log('Snapshot balances updated') l1.daemon.wait_for_log('Snapshot balances updated')
# We should have the deposit and then the journal entry # We should have the deposit
events = l1.rpc.bkpr_listaccountevents()['events'] events = l1.rpc.bkpr_listaccountevents()['events']
assert len(events) == 2 assert len(events) == 2
assert events[0]['account'] == 'wallet' assert events[0]['account'] == 'wallet'

3
tests/test_pay.py
View File

@ -1384,7 +1384,8 @@ def test_forward_pad_fees_and_cltv(node_factory, bitcoind):
wallet = accts[0] wallet = accts[0]
chan_acct = accts[1] chan_acct = accts[1]
assert wallet['account'] == 'wallet' assert wallet['account'] == 'wallet'
assert only_one(wallet['balances'])['balance_msat'] == Millisatoshi(0) # We no longer make a zero balance entry for the wallet at start
assert wallet['balances'] == []
assert incomes[0]['tag'] == 'invoice' assert incomes[0]['tag'] == 'invoice'
assert only_one(chan_acct['balances'])['balance_msat'] == incomes[0]['debit_msat'] assert only_one(chan_acct['balances'])['balance_msat'] == incomes[0]['debit_msat']
inve = only_one([e for e in l1.rpc.bkpr_listaccountevents()['events'] if e['tag'] == 'invoice']) inve = only_one([e for e in l1.rpc.bkpr_listaccountevents()['events'] if e['tag'] == 'invoice'])