From b8e5b122d23290934f070c009a4eac8c88f7d2c6 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Sat, 30 Nov 2024 19:14:45 +1030 Subject: [PATCH] decode: don't fail to decode just because a bolt12 invoice has expired. In fact, there are several places where we try to decode old invoices, and they should all work. The only place we should enforce expiration is when we're going to pay. This also revealed that xpay wasn't checking bolt11 expiries! Reported-by: hMsats Fixes: https://github.com/ElementsProject/lightning/issues/7869 Signed-off-by: Rusty Russell Changelog-Fixed: JSON-RPC: `decode` refused to decode expired bolt12 invoices. --- common/bolt12.c | 46 ++++++++++++++++--------------- common/bolt12.h | 4 +++ contrib/msggen/msggen/schema.json | 1 + doc/schemas/lightning-xpay.json | 1 + plugins/xpay/xpay.c | 10 +++++++ tests/test_pay.py | 1 - 6 files changed, 40 insertions(+), 23 deletions(-) diff --git a/common/bolt12.c b/common/bolt12.c index f82c790856db..a0710dbc8f05 100644 --- a/common/bolt12.c +++ b/common/bolt12.c @@ -489,7 +489,6 @@ struct tlv_invoice *invoice_decode(const tal_t *ctx, char **fail) { struct tlv_invoice *invoice; - u64 expiry, now; invoice = invoice_decode_minimal(ctx, b12, b12len, our_features, must_be_chain, fail); @@ -527,27 +526,6 @@ struct tlv_invoice *invoice_decode(const tal_t *ctx, return tal_free(invoice); } - /* BOLT-offers #12: - * - if `invoice_relative_expiry` is present: - * - MUST reject the invoice if the current time since 1970-01-01 UTC - * is greater than `invoice_created_at` plus `seconds_from_creation`. - * - otherwise: - * - MUST reject the invoice if the current time since 1970-01-01 UTC - * is greater than `invoice_created_at` plus 7200. - */ - if (invoice->invoice_relative_expiry) - expiry = *invoice->invoice_relative_expiry; - else - expiry = 7200; - now = time_now().ts.tv_sec; - /* If it overflows, it's forever */ - if (!add_overflows_u64(*invoice->invoice_created_at, expiry) - && now > *invoice->invoice_created_at + expiry) { - *fail = tal_fmt(ctx, "expired %"PRIu64" seconds ago", - now - (*invoice->invoice_created_at + expiry)); - return tal_free(invoice); - } - /* BOLT-offers #12: * - MUST reject the invoice if `invoice_paths` is not present or is * empty. */ @@ -583,6 +561,30 @@ struct tlv_invoice *invoice_decode(const tal_t *ctx, return invoice; } +u64 invoice_expiry(const struct tlv_invoice *invoice) +{ + u64 expiry; + + /* BOLT-offers #12: + * - if `invoice_relative_expiry` is present: + * - MUST reject the invoice if the current time since 1970-01-01 UTC + * is greater than `invoice_created_at` plus `seconds_from_creation`. + * - otherwise: + * - MUST reject the invoice if the current time since 1970-01-01 UTC + * is greater than `invoice_created_at` plus 7200. + */ + if (invoice->invoice_relative_expiry) + expiry = *invoice->invoice_relative_expiry; + else + expiry = 7200; + + /* If it overflows, it's forever */ + if (add_overflows_u64(*invoice->invoice_created_at, expiry)) + return UINT64_MAX; + + return *invoice->invoice_created_at + expiry; +} + static bool bolt12_has_invoice_prefix(const char *str) { return strstarts(str, "lni1") || strstarts(str, "LNI1"); diff --git a/common/bolt12.h b/common/bolt12.h index ae8b2286c9cb..cb7450961d49 100644 --- a/common/bolt12.h +++ b/common/bolt12.h @@ -75,6 +75,7 @@ char *invoice_encode(const tal_t *ctx, const struct tlv_invoice *bolt12_tlv); * is not expired). It also checks signature. * * Note: blinded path features need to be checked by the caller before use! + * Note: expiration must be check by caller before use! */ struct tlv_invoice *invoice_decode(const tal_t *ctx, const char *b12, size_t b12len, @@ -82,6 +83,9 @@ struct tlv_invoice *invoice_decode(const tal_t *ctx, const struct chainparams *must_be_chain, char **fail); +/* UINT64_MAX if no expiry. */ +u64 invoice_expiry(const struct tlv_invoice *invoice); + /* This one only checks it decides, and optionally is correct chain/features */ struct tlv_invoice *invoice_decode_minimal(const tal_t *ctx, const char *b12, size_t b12len, diff --git a/contrib/msggen/msggen/schema.json b/contrib/msggen/msggen/schema.json index 155e401a1dc2..8f369998d5ca 100644 --- a/contrib/msggen/msggen/schema.json +++ b/contrib/msggen/msggen/schema.json @@ -36330,6 +36330,7 @@ "- -1: Catchall nonspecific error.", "- 203: Permanent failure from destination (e.g. it said it didn't recognize invoice)", "- 205: Couldn't find, or find a way to, the destination.", + "- 207: Invoice has expired.", "- 219: Invoice has already been paid.", "- 209: Other payment error." ], diff --git a/doc/schemas/lightning-xpay.json b/doc/schemas/lightning-xpay.json index c46c6e373aa6..8cdfcc037198 100644 --- a/doc/schemas/lightning-xpay.json +++ b/doc/schemas/lightning-xpay.json @@ -107,6 +107,7 @@ "- -1: Catchall nonspecific error.", "- 203: Permanent failure from destination (e.g. it said it didn't recognize invoice)", "- 205: Couldn't find, or find a way to, the destination.", + "- 207: Invoice has expired.", "- 219: Invoice has already been paid.", "- 209: Other payment error." ], diff --git a/plugins/xpay/xpay.c b/plugins/xpay/xpay.c index 27166cda016d..0798cc4fe85f 100644 --- a/plugins/xpay/xpay.c +++ b/plugins/xpay/xpay.c @@ -1414,6 +1414,7 @@ static struct command_result *json_xpay(struct command *cmd, struct payment *payment = tal(cmd, struct payment); unsigned int *retryfor; struct out_req *req; + u64 now, invexpiry; char *err; if (!param_check(cmd, buffer, params, @@ -1448,6 +1449,7 @@ static struct command_result *json_xpay(struct command *cmd, return command_fail(cmd, JSONRPC2_INVALID_PARAMS, "Invalid bolt12 invoice: %s", err); + invexpiry = invoice_expiry(b12inv); payment->full_amount = amount_msat(*b12inv->invoice_amount); if (msat) return command_fail(cmd, JSONRPC2_INVALID_PARAMS, @@ -1517,8 +1519,16 @@ static struct command_result *json_xpay(struct command *cmd, payment->full_amount = *b11->msat; else payment->full_amount = *msat; + + invexpiry = b11->timestamp + b11->expiry; } + now = time_now().ts.tv_sec; + if (now > invexpiry) + return command_fail(cmd, PAY_INVOICE_EXPIRED, + "Invoice expired %"PRIu64" seconds ago", + now - invexpiry); + if (partial) { payment->amount = *partial; if (amount_msat_greater(payment->amount, payment->full_amount)) diff --git a/tests/test_pay.py b/tests/test_pay.py index 68880fb429e8..bddb2ab721f7 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -6770,7 +6770,6 @@ def test_pay_unannounced_routehint(node_factory, bitcoind): assert result["status"] == "complete", f"pay result is {result}" -@pytest.mark.xfail(strict=True) def test_decode_expired_bolt12(node_factory): l1 = node_factory.get_node()