bkpr-report: make ? work as expected on amount fields.
What changed, and why it matters
This commit fixes a small behavior bug in the bookkeeper plugin's report formatting. Previously, the '?' fallback feature in report templates checked whether an amount field was present, not whether it was zero. Now, for amount fields like credit, debit, fees, and creditdebit, '?' treats a zero value as if the field were missing, so fallback text is used as users would expect. This is a correctness/usability fix, not a security vulnerability.
No security action required. Treat as a normal bug fix and include in regular release testing.
Security signals we found
No memory safety issues evident
No input validation bypass
No privilege escalation or authentication change
No cryptographic or consensus code touched
Behavioral correctness fix in RPC output formatting
Evidence from the diff
The patch modifies plugins/bkpr/report.c so that amount formatting functions return a sentinel string ZERO_AMOUNT for zero amounts. In format_event(), if a field uses the ‘?’ alternative syntax and the value is ZERO_AMOUNT, it is treated as NULL/missing, triggering the fallback. Documentation and tests are updated accordingly. The change is localized to the bkpr-report RPC output formatting and does not alter transaction handling, accounting logic, or cryptographic operations.
Changed components
plugins/bkpr/report.cdoc/schemas/bkpr-report.jsoncontrib/msggen/msggen/schema.jsontests/test_bookkeeper.pyInspect captured patch +20 / −5
diff --git a/contrib/msggen/msggen/schema.json b/contrib/msggen/msggen/schema.json
index e1f02594..0dbe3289 100644
--- a/contrib/msggen/msggen/schema.json
+++ b/contrib/msggen/msggen/schema.json
@@ -5162,7 +5162,7 @@
"",
"If a field is unavailable, it expands to an empty string.",
"",
- "You can provide fallback with ?, including more variable:",
+ "You can provide fallback with ?, which is used when the tag is not present (or zero, for credit, debit, fees and creditdebit). This fallback can include more tags:",
" * {outpoint?NONE}",
" * {payment_id?txid: {txid?UNKNOWN}}",
"The first one the outpoint, or NONE if that is not available. ",
diff --git a/doc/schemas/bkpr-report.json b/doc/schemas/bkpr-report.json
index 2eed4048..27d69c1a 100644
--- a/doc/schemas/bkpr-report.json
+++ b/doc/schemas/bkpr-report.json
@@ -48,7 +48,7 @@
"",
"If a field is unavailable, it expands to an empty string.",
"",
- "You can provide fallback with ?, including more variable:",
+ "You can provide fallback with ?, which is used when the tag is not present (or zero, for credit, debit, fees and creditdebit). This fallback can include more tags:",
" * {outpoint?NONE}",
" * {payment_id?txid: {txid?UNKNOWN}}",
"The first one the outpoint, or NONE if that is not available. ",
diff --git a/plugins/bkpr/report.c b/plugins/bkpr/report.c
index 754f29a8..fa4d4fa6 100644
--- a/plugins/bkpr/report.c
+++ b/plugins/bkpr/report.c
@@ -15,6 +15,9 @@
#include <plugins/bkpr/report.h>
#include <plugins/libplugin.h>
+/* This is a zero, but treated specially if tested with ? */
+static const char ZERO_AMOUNT[] = "0";
+
static const char *report_fmt_acct_name(const tal_t *ctx UNNEEDED,
const struct bkpr *bkpr UNNEEDED,
const struct income_event *e)
@@ -40,6 +43,8 @@ static const char *report_fmt_credit(const tal_t *ctx,
const struct bkpr *bkpr UNNEEDED,
const struct income_event *e)
{
+ if (amount_msat_is_zero(e->credit))
+ return ZERO_AMOUNT;
return fmt_amount_msat_btc(ctx, e->credit, false);
}
@@ -47,6 +52,8 @@ static const char *report_fmt_debit(const tal_t *ctx,
const struct bkpr *bkpr UNNEEDED,
const struct income_event *e)
{
+ if (amount_msat_is_zero(e->debit))
+ return ZERO_AMOUNT;
return fmt_amount_msat_btc(ctx, e->debit, false);
}
@@ -54,6 +61,8 @@ static const char *report_fmt_fees(const tal_t *ctx,
const struct bkpr *bkpr UNNEEDED,
const struct income_event *e)
{
+ if (amount_msat_is_zero(e->fees))
+ return ZERO_AMOUNT;
return fmt_amount_msat_btc(ctx, e->fees, false);
}
@@ -134,7 +143,7 @@ static const char *report_fmt_credit_debit(const tal_t *ctx,
if (!amount_msat_is_zero(e->debit))
return tal_fmt(ctx, "-%s",
fmt_amount_msat_btc(tmpctx, e->debit, false));
- return "0";
+ return ZERO_AMOUNT;
}
static const char *report_fmt_currency_credit(const tal_t *ctx,
@@ -411,6 +420,10 @@ static char *format_event(const tal_t *ctx,
}
v = fmt->fmt[i](tmpctx, bkpr, e);
+ /* If there's an alternative, we treat ZERO_AMOUNT as missing. */
+ if (v == ZERO_AMOUNT && fmt->alt[i])
+ v = NULL;
+
if (v) {
v = escape_value(tmpctx, v, esc);
out = tal_strcat(ctx, take(out), v);
diff --git a/tests/test_bookkeeper.py b/tests/test_bookkeeper.py
index c62210e6..fb6b23b3 100644
--- a/tests/test_bookkeeper.py
+++ b/tests/test_bookkeeper.py
@@ -1275,22 +1275,24 @@ def test_bkpr_report_tags_and_fallback(node_factory):
assert any(r[2] == "NONE" or r[3] == "NONE" or r[4] == "NONE" for r in rows)
# Fancier fields should work, too
- res = l1.rpc.bkpr_report(format="{tag}|{account}|{credit}|{debit}|{creditdebit}|{currencycredit}|{currencydebit}|{currencycreditdebit}")
+ res = l1.rpc.bkpr_report(format="{tag}|{account}|{credit}|{debit}|{creditdebit}|{currencycredit}|{currencydebit}|{currencycreditdebit}|{credit?NONE}")
rows = [line.split("|") for line in res["report"]]
for r in rows:
- assert len(r) == 8
+ assert len(r) == 9
# Credit or debit?
if float(r[2]) > 0:
assert r[4] == '+' + r[2]
assert float(r[5]) > 0
assert r[6] == '0.00'
assert r[7] == '+' + r[5]
+ assert r[8] == r[2]
else:
assert r[4] == '-' + r[3]
assert float(r[6]) > 0
assert r[5] == '0.00'
assert r[7] == '-' + r[6]
+ assert r[8] == 'NONE'
def test_bkpr_report_invoice(node_factory):
Why this scored 17/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.