lightningd: don't do previous invoice checking in createinvoicerequest.
What changed, and why it matters
This commit removes a local sanity check in Core Lightning's experimental 'createinvoicerequest' command. Previously, when creating a recurring invoice request, the code looked up past payments by label to verify the previous recurrence was paid and to copy timing data. Now it skips that lookup and lets the remote node decide. The change also renames the 'recurrence_label' parameter to the simpler 'label'. It is described by the author as a cleanup of an undocumented interface, not as a security fix.
Treat as a behavior change rather than a vulnerability. Review whether removing local basetime/copy and prior-payment validation could allow clients to create malformed or premature recurring invoice requests, and confirm the remote node now enforces equivalent checks. If the interface is still experimental, consider documenting the new 'label' parameter and expected remote-side errors.
Security signals we found
Removal of local payment-state validation for recurring invoice requests
Parameter rename from recurrence_label to label in internal RPC calls
Test expectations changed from local 'previous invoice has not been paid' errors to remote failure messages
Undocumented interface changed without deprecation or migration note
No explicit security framing by commit author
Evidence from the diff
The patch deletes the prev_payment() helper and its invocation in json_createinvoicerequest(). That helper iterated wallet payments matching a label, decoded their invoice strings, compared offer IDs, enforced recurrence_start consistency, copied invoice_recurrence_basetime, and rejected requests if the prior recurrence_counter had not been paid. With this removed, createinvoicerequest no longer requires a label for recurring payments, no longer validates prior payment state locally, and no longer auto-fills prev_basetime from local history. Callers (fetchinvoice, cancelrecurringinvoice, offers_offer) are updated to send ‘label’ instead of ‘recurrence_label’. Tests are adjusted to expect remote-side error messages instead of local ones.
Changed components
lightningd/offer.cplugins/fetchinvoice.cplugins/offers_offer.ctests/test_pay.pyInspect captured patch +5 / −122
diff --git a/lightningd/offer.c b/lightningd/offer.c
index 1ba82d78..44e6bcde 100644
--- a/lightningd/offer.c
+++ b/lightningd/offer.c
@@ -313,101 +313,6 @@ static const struct json_command enableoffer_command = {
};
AUTODATA(json_command, &enableoffer_command);
-
-/* We do some sanity checks now, since we're looking up prev payment anyway,
- * but our main purpose is to fill in prev_basetime tweak. */
-static struct command_result *prev_payment(struct command *cmd,
- const struct json_escape *label,
- const struct tlv_invoice_request *invreq,
- u64 **prev_basetime)
-{
- struct sha256 invreq_oid;
- u64 last_recurrence = UINT64_MAX;
- bool prev_unpaid = false;
-
- invreq_offer_id(invreq, &invreq_oid);
-
- for (struct db_stmt *stmt = payments_by_label(cmd->ld->wallet, label);
- stmt;
- stmt = payments_next(cmd->ld->wallet, stmt)) {
- const struct wallet_payment *payment;
- const struct tlv_invoice *inv;
- const char *fail;
- struct sha256 inv_oid;
-
- payment = payment_get_details(tmpctx, stmt);
- if (!payment->invstring)
- continue;
-
- inv = invoice_decode(tmpctx, payment->invstring,
- strlen(payment->invstring),
- NULL, chainparams, &fail);
- if (!inv)
- continue;
-
- /* They can reuse labels across different offers. */
- invoice_offer_id(inv, &inv_oid);
- if (!sha256_eq(&inv_oid, &invreq_oid))
- continue;
-
- /* Be paranoid, in case someone inserts their own
- * clashing label! */
- if (!inv->invreq_recurrence_counter)
- continue;
-
- /* BOLT-recurrence #12:
- * - if `offer_recurrence_base` is present:
- * - MUST include `invreq_recurrence_start`
- * - MUST set `period_offset` to the period the sender wants for the
- * initial request
- * - MUST set `period_offset` to the same value on all following requests.
- */
- if (inv->invreq_recurrence_start
- && invreq->invreq_recurrence_start
- && *inv->invreq_recurrence_start != *invreq->invreq_recurrence_start) {
- tal_free(stmt);
- return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
- "recurrence_start was"
- " previously %u",
- *inv->invreq_recurrence_start);
- }
-
- /* They should all have the same basetime */
- if (!*prev_basetime)
- *prev_basetime = tal_dup(cmd, u64, inv->invoice_recurrence_basetime);
-
- /* Track highest one for better diagnostics */
- if (last_recurrence == UINT64_MAX
- || last_recurrence < *inv->invreq_recurrence_counter) {
- last_recurrence = *inv->invreq_recurrence_counter;
- }
-
- if (*inv->invreq_recurrence_counter == *invreq->invreq_recurrence_counter-1) {
- /* Got it! */
- if (payment->status == PAYMENT_COMPLETE) {
- tal_free(stmt);
- return NULL;
- } else
- prev_unpaid = true;
- }
- }
-
- /* We found one, but it didn't succeed */
- if (prev_unpaid)
- return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
- "previous invoice payment did not succeed");
-
- /* We found one, but it was not the previus one */
- if (last_recurrence != UINT64_MAX)
- return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
- "previous invoice has not been paid (last was %"PRIu64")",
- last_recurrence);
-
- return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
- "No previous payment attempted for this"
- " label and offer");
-}
-
/* FIXME(vincenzopalazzo): move this to comm/bolt12.h */
static struct command_result *param_b12_invreq(struct command *cmd,
const char *name,
@@ -483,7 +388,7 @@ static struct command_result *json_createinvoicerequest(struct command *cmd,
if (!param_check(cmd, buffer, params,
p_req("bolt12", param_b12_invreq, &invreq),
p_req("savetodb", param_bool, &save),
- p_opt("recurrence_label", param_label, &label),
+ p_opt("label", param_label, &label),
p_opt_def("single_use", param_bool, &single_use, true),
NULL))
return command_param_failed();
@@ -493,21 +398,6 @@ static struct command_result *json_createinvoicerequest(struct command *cmd,
else
status = OFFER_MULTIPLE_USE_UNUSED;
- /* If it's a recurring payment, we look for previous to copy basetime */
- if (invreq->invreq_recurrence_counter) {
- if (!label)
- return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
- "Need payment label for recurring payments");
-
- if (*invreq->invreq_recurrence_counter != 0) {
- struct command_result *err
- = prev_payment(cmd, label, invreq,
- &prev_basetime);
- if (err)
- return err;
- }
- }
-
/* If the payer_id is not our node id, we sanity check that it
* correctly maps from invreq_metadata */
if (!pubkey_eq(invreq->invreq_payer_id, &cmd->ld->our_pubkey)) {
diff --git a/plugins/fetchinvoice.c b/plugins/fetchinvoice.c
index 8fb0dea8..8d83328c 100644
--- a/plugins/fetchinvoice.c
+++ b/plugins/fetchinvoice.c
@@ -1096,7 +1096,7 @@ struct command_result *json_fetchinvoice(struct command *cmd,
json_add_string(req->js, "bolt12", invrequest_encode(tmpctx, invreq));
json_add_bool(req->js, "savetodb", false);
if (rec_label)
- json_add_string(req->js, "recurrence_label", rec_label);
+ json_add_string(req->js, "label", rec_label);
return send_outreq(req);
}
@@ -1250,7 +1250,7 @@ struct command_result *json_cancelrecurringinvoice(struct command *cmd,
/* We don't want this is the database: that's only for ones we publish */
json_add_string(req->js, "bolt12", invrequest_encode(tmpctx, invreq));
json_add_bool(req->js, "savetodb", false);
- json_add_string(req->js, "recurrence_label", rec_label);
+ json_add_string(req->js, "label", rec_label);
return send_outreq(req);
}
diff --git a/plugins/offers_offer.c b/plugins/offers_offer.c
index 77c3cf9b..0bd87ad7 100644
--- a/plugins/offers_offer.c
+++ b/plugins/offers_offer.c
@@ -607,7 +607,7 @@ static struct command_result *call_createinvoicerequest(struct command *cmd,
json_add_bool(req->js, "savetodb", true);
json_add_bool(req->js, "single_use", single_use);
if (label)
- json_add_string(req->js, "recurrence_label", label);
+ json_add_string(req->js, "label", label);
return send_outreq(req);
}
diff --git a/tests/test_pay.py b/tests/test_pay.py
index 419c70a0..df768bda 100644
--- a/tests/test_pay.py
+++ b/tests/test_pay.py
@@ -4822,7 +4822,7 @@ def test_fetchinvoice_recurrence(node_factory, bitcoind):
assert period2['paywindow_end'] == period2['endtime']
# Can't request 2 before paying 1.
- with pytest.raises(RpcError, match='previous invoice has not been paid'):
+ with pytest.raises(RpcError, match='Remote node sent failure message.*Previous invoice #1 status "unpaid"'):
l1.rpc.call('fetchinvoice', {'offer': offer3['bolt12'],
'recurrence_counter': 2,
'recurrence_label': 'test recurrence'})
@@ -7267,9 +7267,6 @@ def test_cancel_recurrence(node_factory):
with pytest.raises(RpcError, match="recurrence_counter: Must be non-zero"):
l1.rpc.cancelrecurringinvoice(offer['bolt12'], 0, 'test_cancel_recurrence')
- with pytest.raises(RpcError, match="No previous payment attempted for this label and offer"):
- l1.rpc.cancelrecurringinvoice(offer['bolt12'], 1, 'test_cancel_recurrence')
-
# Fetch and pay first one
ret = l1.rpc.fetchinvoice(offer=offer['bolt12'],
recurrence_counter=0,
@@ -7279,10 +7276,6 @@ def test_cancel_recurrence(node_factory):
decoded = l1.rpc.decode(m.group(1))
assert 'invreq_recurrence_cancel' not in decoded
- # Cancel counter must be correct!
- with pytest.raises(RpcError, match=r"previous invoice has not been paid \(last was 0\)"):
- l1.rpc.cancelrecurringinvoice(offer['bolt12'], 2, 'test_cancel_recurrence')
-
# Cancel second one.
l1.rpc.cancelrecurringinvoice(offer=offer['bolt12'],
recurrence_counter=1,
Why this scored 32/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.