bkpr: don't exit when an account's debits exceed its credits

contrib/msggen/msggen/schema.json

@@ -5199,7 +5199,9 @@
       "description": [
         "The **bkpr-listbalances** RPC command is a list of all current and historical account balances. An account is either the on-chain *wallet* or a channel balance. Any funds sent to an *external* account will not be accounted for here.",
         "",
-        "Note that any channel that was recorded will be listed. Closed channel balances will be 0msat."
+        "Note that any channel that was recorded will be listed. Closed channel balances will be 0msat.",
+        "",
+        "If an account's recorded debits exceed its credits, the ledger for that account is inconsistent: its balance is reported as 0msat and a **BROKEN** line is logged."
       ],
       "request": {
         "required": [],

doc/schemas/bkpr-listbalances.json

@@ -6,7 +6,9 @@
   "description": [
     "The **bkpr-listbalances** RPC command is a list of all current and historical account balances. An account is either the on-chain *wallet* or a channel balance. Any funds sent to an *external* account will not be accounted for here.",
     "",
-    "Note that any channel that was recorded will be listed. Closed channel balances will be 0msat."
+    "Note that any channel that was recorded will be listed. Closed channel balances will be 0msat.",
+    "",
+    "If an account's recorded debits exceed its credits, the ledger for that account is inconsistent: its balance is reported as 0msat and a **BROKEN** line is logged."
   ],
   "request": {
     "required": [],

plugins/bkpr/bookkeeper.c

@@ -937,11 +937,17 @@ static struct command_result *do_list_balances(struct command *cmd,
 						      accts[i]->name,
 						      &credit, &debit);
 		if (!amount_msat_sub(&balance, credit, debit)) {
-			plugin_err(cmd->plugin,
-				   "Account balance underflow for account %s (credit %s, debit %s)",
+			/* The ledger for this account is inconsistent.
+			 * Log it and report zero: exiting here would
+			 * take lightningd down with us, since the
+			 * bookkeeper is an important plugin. */
+			plugin_log(cmd->plugin, LOG_BROKEN,
+				   "Account balance underflow for account %s"
+				   " (credit %s, debit %s): reporting 0msat",
 				   accts[i]->name,
 				   fmt_amount_msat(tmpctx, credit),
 				   fmt_amount_msat(tmpctx, debit));
+			balance = AMOUNT_MSAT(0);
 		}
 
 		/* Skip the external acct balance, it's effectively

tests/test_bookkeeper.py

@@ -1577,3 +1577,31 @@ def test_bkpr_report_lightning_cli_csv(node_factory):
     parsed = [next(csv.reader(io.StringIO(line))) for line in res.splitlines()]
     assert parsed
     assert all(len(row) == 3 for row in parsed)
+
+
+@unittest.skipIf(os.getenv('TEST_DB_PROVIDER', 'sqlite3') != 'sqlite3', "edits the sqlite db directly")
+def test_bookkeeping_listbalances_underflow(node_factory, bitcoind):
+    """An account whose debits exceed its credits must not take lightningd
+    down: bkpr-listbalances logs the inconsistency and reports 0msat."""
+    l1, l2 = node_factory.line_graph(2, opts=[{'broken_log': 'Account balance underflow'}, {}])
+
+    # A payment gives l1's channel account a debit entry to corrupt.
+    inv = l2.rpc.invoice(10000, 'inv', 'desc')
+    l1.rpc.pay(inv['bolt11'])
+
+    chan_id = first_channel_id(l1, l2)
+    accts = l1.rpc.bkpr_listbalances()['accounts']
+    wallet_bal = only_one([a for a in accts if a['account'] == 'wallet'])['balances'][0]['balance_msat']
+    assert only_one([a for a in accts if a['account'] == chan_id])['balances'][0]['balance_msat'] > 0
+
+    # Make that debit exceed everything the channel was ever credited.
+    l1.stop()
+    l1.db_manip("UPDATE channel_moves SET credit_or_debit = -10000000000000 WHERE credit_or_debit < 0")
+    l1.start()
+
+    accts = l1.rpc.bkpr_listbalances()['accounts']
+    assert l1.daemon.is_in_log('Account balance underflow for account {}'.format(chan_id))
+    assert only_one([a for a in accts if a['account'] == chan_id])['balances'][0]['balance_msat'] == 0
+    assert only_one([a for a in accts if a['account'] == 'wallet'])['balances'][0]['balance_msat'] == wallet_bal
+    # lightningd survived.
+    l1.rpc.getinfo()