bookkeeper: use iso4217 check, scale and print correct minor units.
What the developer wrote
Authored by Rusty Russell
bookkeeper: use iso4217 check, scale and print correct minor units.
e.g. USD = 2, CLF = 4, XAU = 0.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
What changed, and why it matters
This commit fixes how the Core Lightning bookkeeper plugin handles fiat currency codes and their decimal places. Previously it assumed every currency used 4 decimal places when storing and displaying exchange rates. Now it uses a proper ISO 4217 table so currencies like USD use 2 decimals, Chilean Unidad de Fomento (CLF) uses 4, and gold (XAU) uses 0. It also validates that the configured currency is a known ISO 4217 code. The main risk is that old stored rates could be misinterpreted after the change, and users might see wrong converted values if they relied on the previous fixed scaling.
Treat as a functional correctness patch, not a security emergency. Operators using bkpr-currency should verify that historical currency rate data is still interpreted correctly after upgrade, because the scaling factor for stored rates may have changed. Review whether existing datastore entries need migration or whether the plugin already handles the transition.
Security signals we found
Input validation added: currency option is now checked against ISO 4217 list
Data-format change: stored raw_rate scaling changes from fixed 4 decimals to currency-specific decimals
Potential downgrade in data fidelity for currencies with fewer than 4 minor units (e.g. USD now uses 2 decimals instead of 4)
No cryptographic, network, or memory-safety changes observed
Evidence from the diff
The patch replaces the hard-coded RATE_MUL_FACTOR (10000) with a per-currency minor_unit divisor derived from a new common/iso4217.h table. The bkpr->currency pointer is changed from char to const struct iso4217_name_and_divisor, and option_currency now calls find_iso4217() to validate the configured currency. currencyrate_str formats the rate with the correct number of fractional digits. The test is updated to expect 100 instead of 10000 for a USD-like currency. This is a correctness fix for currency conversion display and storage, not a memory-safety or cryptographic bug.
Changed components
plugins/bkpr/bookkeeper.cplugins/bkpr/bookkeeper.htests/test_currencyrate.pycommon/iso4217.h (referenced, not in diff)Inspect captured patch +57 / −26
diff --git a/plugins/bkpr/bookkeeper.c b/plugins/bkpr/bookkeeper.c
index 55a79cf3..ea0ec8ca 100644
--- a/plugins/bkpr/bookkeeper.c
+++ b/plugins/bkpr/bookkeeper.c
@@ -12,6 +12,7 @@
#include <common/bolt12.h>
#include <common/clock_time.h>
#include <common/coin_mvt.h>
+#include <common/iso4217.h>
#include <common/json_param.h>
#include <common/json_stream.h>
#include <common/memleak.h>
@@ -59,19 +60,36 @@ static const struct currencyrate *covering_currencyrate(const struct bkpr *bkpr,
return NULL;
}
+static u64 ratefactor(const struct iso4217_name_and_divisor *currency)
+{
+ u64 mul = 1;
+ for (u32 i = 0; i < currency->minor_unit; i++)
+ mul *= 10;
+ return mul;
+}
+
const char *currencyrate_str(const tal_t *ctx,
const struct bkpr *bkpr,
u64 timestamp)
{
const struct currencyrate *crate;
+ u64 mul, intpart, fracpart;
crate = covering_currencyrate(bkpr, timestamp);
if (!crate)
return NULL;
+ mul = ratefactor(bkpr->currency);
+
+ intpart = crate->raw_rate / mul;
+ fracpart = crate->raw_rate % mul;
- return tal_fmt(ctx, "%"PRIu64".%04"PRIu64,
- crate->raw_rate / RATE_MUL_FACTOR,
- crate->raw_rate % RATE_MUL_FACTOR);
+ if (bkpr->currency->minor_unit == 0)
+ return tal_fmt(ctx, "%"PRIu64, intpart);
+
+ return tal_fmt(ctx, "%"PRIu64".%0*"PRIu64,
+ intpart,
+ (int)bkpr->currency->minor_unit,
+ fracpart);
}
void json_add_currencyrate(struct json_stream *result,
@@ -1165,7 +1183,7 @@ static struct command_result *lookup_invoice_desc(struct command *cmd,
struct currency_time {
struct refresh_info *rinfo;
- const char *currency;
+ const struct iso4217_name_and_divisor *currency;
u64 timestamp;
};
@@ -1196,10 +1214,10 @@ static struct command_result *currency_done(struct command *cmd,
json_tok_full_len(result),
json_tok_full(buf, result), err);
/* Make sure they didn't change currency while we were out! */
- } else if (bkpr->currency && streq(ctime->currency, bkpr->currency)) {
+ } else if (bkpr->currency == ctime->currency) {
char *val;
const char **key;
- u64 raw_rate = (u64)(rate * RATE_MUL_FACTOR);
+ u64 raw_rate = (u64)(rate * ratefactor(bkpr->currency));
/* Can we extend previous entry */
u64 ts = ctime->timestamp + 1;
struct currencyrate *crate
@@ -1235,7 +1253,7 @@ static struct command_result *currency_done(struct command *cmd,
key = mkdatastorekey(tmpctx,
"bookkeeper",
"currencyrate",
- ctime->currency,
+ ctime->currency->name,
take(tal_fmt(NULL, "%"PRIu64, ts)));
jsonrpc_set_datastore_string(cmd, key, val,
"create-or-replace",
@@ -1291,7 +1309,7 @@ static void lookup_currency(struct command *cmd,
"Event %s timestamp %"PRIu64" is %"PRIu64" seconds old: too old for current %s currencyrate (only logging first such event: there may be others)",
mvt_tag_str(primary_tag),
timestamp, now - timestamp,
- bkpr->currency);
+ bkpr->currency->name);
bkpr->warned_currency_fail = true;
}
return;
@@ -1299,14 +1317,14 @@ static void lookup_currency(struct command *cmd,
ctime = tal(cmd, struct currency_time);
ctime->timestamp = timestamp;
- ctime->currency = tal_strdup(ctime, bkpr->currency);
+ ctime->currency = bkpr->currency;
ctime->rinfo = use_rinfo(rinfo);
req = jsonrpc_request_start(cmd,
"currencyrate",
currency_done,
currency_error,
ctime);
- json_add_string(req->js, "currency", bkpr->currency);
+ json_add_string(req->js, "currency", bkpr->currency->name);
send_outreq(req);
}
@@ -1886,7 +1904,7 @@ static void load_currencyrates(struct command *cmd,
json_out_start(params, "key", '[');
json_out_addstr(params, NULL, "bookkeeper");
json_out_addstr(params, NULL, "currencyrate");
- json_out_addstr(params, NULL, bkpr->currency);
+ json_out_addstr(params, NULL, bkpr->currency->name);
json_out_end(params, ']');
json_out_end(params, '}');
json_out_finished(params);
@@ -1931,16 +1949,32 @@ static void load_currencyrates(struct command *cmd,
}
}
+static bool currency_jsonfmt(struct command *cmd,
+ struct json_stream *js,
+ const char *fieldname,
+ struct bkpr *bkpr)
+{
+ if (!bkpr->currency)
+ return false;
+ json_add_string(js, fieldname, bkpr->currency->name);
+ return true;
+}
+
static char *option_currency(struct command *cmd,
const char *arg,
bool check_only,
- char **p)
+ struct bkpr *bkpr)
{
- struct bkpr *bkpr = bkpr_of(cmd->plugin);
+ const struct iso4217_name_and_divisor *newcur;
- assert(p == &bkpr->currency);
+ /* Explicit empty string means unset. */
+ if (!streq(arg, "")) {
+ newcur = find_iso4217(arg, strlen(arg));
+ if (!newcur)
+ return "unknown ISO4217 code";
+ } else
+ newcur = NULL;
- /* FIXME: Check for valid currency? */
if (check_only)
return NULL;
@@ -1953,14 +1987,13 @@ static char *option_currency(struct command *cmd,
bkpr->currency_rates = tal(bkpr, currencymap_t);
uintmap_init(bkpr->currency_rates);
memleak_add_helper(bkpr->currency_rates, memleak_scan_currencyrates);
- bkpr->currency = tal_free(bkpr->currency);
+ bkpr->currency = NULL;
}
- /* Explicit empty string means unset. */
- if (streq(arg, ""))
+ if (!newcur)
return NULL;
- bkpr->currency = tal_strdup(bkpr, arg);
+ bkpr->currency = newcur;
/* Reset this so we get a new message for new currency */
bkpr->warned_currency_fail = false;
@@ -1994,7 +2027,7 @@ int main(int argc, char *argv[])
plugin_option_dynamic("bkpr-currency",
"string",
"Look up and record this currency on each event",
- option_currency, charp_jsonfmt, &bkpr->currency),
+ option_currency, currency_jsonfmt, bkpr),
NULL);
return 0;
diff --git a/plugins/bkpr/bookkeeper.h b/plugins/bkpr/bookkeeper.h
index 921f5730..95cf9356 100644
--- a/plugins/bkpr/bookkeeper.h
+++ b/plugins/bkpr/bookkeeper.h
@@ -7,9 +7,7 @@
struct command;
struct plugin;
-
-/* Most currencies have 2 decimal places, but 4 is the current maximum. */
-#define RATE_MUL_FACTOR 10000
+struct iso4217_name_and_divisor;
struct currencyrate {
u32 duration;
@@ -34,7 +32,7 @@ struct bkpr {
u64 chainmoves_index, channelmoves_index;
/* Optional currency if we're doing currencyconvert lookups */
- char *currency;
+ const struct iso4217_name_and_divisor *currency;
/* Map of UNIX time -> currency rate */
currencymap_t *currency_rates;
/* True if we've warned about currency failures */
diff --git a/tests/test_currencyrate.py b/tests/test_currencyrate.py
index a25efbd4..8431e529 100644
--- a/tests/test_currencyrate.py
+++ b/tests/test_currencyrate.py
@@ -616,11 +616,11 @@ def test_bkpr_currencyrate_ranges(node_factory, fake_rateserver):
assert int(rates[0]['key'][3]) < int(rates[1]['key'][3])
raw_rate, duration = rates[0]['string'].split(':')
- assert int(raw_rate) == int(old_median * 10000)
+ assert int(raw_rate) == int(old_median * 100)
assert int(duration) >= 2
raw_rate, duration = rates[1]['string'].split(':')
- assert int(raw_rate) == int(new_median * 10000)
+ assert int(raw_rate) == int(new_median * 100)
assert int(duration) >= 1
# We will load them fine on restart, too.
Why this scored 18/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.