Skip to content

offers: fix recurrence proportional amount #9413

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Lagrang3 wants to merge 1 commit into
base: master
Choose a base branch
from
+6 −3
Open

offers: fix recurrence proportional amount #9413

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions plugins/offers_invreq_hook.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -563,10 +563,13 @@ static struct command_result *check_period(struct command *cmd,
u64 end = offer_period_start(basetime, period_idx + 1,
invreq_recurrence(ir->invreq));

if (*ir->inv->invoice_created_at > start) {
assert(end > start);

@nGoline nGoline Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make this a fail_invreq() rather than an assert(). offer_period_start() gives no guarantee that end > start. param_recurrence() (plugins/offers_offer.c:96) has no non-zero check on period, and the period == 0 guard at common/bolt12.c:290 is in offer_decode(), not invrequest_decode(), so a recurrence=0seconds offer reaches here with start == end. period_idx is also invreq_recurrence_start + invreq_recurrence_counter, i.e. remote-chosen, and offer_period_start() truncates period*n into time_change()'s u32 number (common/bolt12.c:449), so the sequence is not monotone across that range either. offers is a builtin and so important, and NDEBUG is never defined, so an abort here
takes the node down.

Suggested change
assert(end > start);
if (end <= start)
return fail_invreq(cmd, ir,
"period_index %"PRIu64" bad period",
period_idx);

if (*ir->inv->invoice_created_at >= end) {
*ir->inv->invoice_amount = 1;
Comment on lines +567 to +568

@nGoline nGoline Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

paywindow_end is pstart + seconds_after (common/bolt12.c:513) and seconds_after is an unconstrained u32, so an offer whose paywindow outlives its period reaches this branch. Past the period end the time remaining is zero, so the request should be rejected as too late rather than priced
at 1 msat.

} else if (*ir->inv->invoice_created_at > start) {
*ir->inv->invoice_amount
*= (double)((*ir->inv->invoice_created_at - start)
/ (end - start));
*= ((double)end - *ir->inv->invoice_created_at)
/ (end - start);
/* Round up to make it non-zero if necessary. */
if (*ir->inv->invoice_amount == 0)
*ir->inv->invoice_amount = 1;
Expand Down
Loading