From f56b9e9b73f664b2b24b7f8b4b6fe29da9385036 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Sun, 4 Feb 2024 10:22:40 +1030 Subject: [PATCH] lightningd: deprecate @-prefix hack for offer recurrence_base. Christian points out that this makes the type harder, and it's just awkward. Changelog-EXPERIMENTAL: JSON-RPC: Deprecated `offer` parameter `recurrence_base` with `@` prefix: use `recurrence_start_any_period`. Changelog-EXPERIMENTAL: JSON-RPC: Added `offer` parameter `recurrence_start_any_period`. Signed-off-by: Rusty Russell --- doc/developers-guide/deprecations.md | 1 + doc/lightning-offer.7.md | 15 +++++++++------ plugins/offers_offer.c | 26 ++++++++++++++++++++++++-- tests/test_pay.py | 18 +++++++++++++++++- 4 files changed, 51 insertions(+), 9 deletions(-) diff --git a/doc/developers-guide/deprecations.md b/doc/developers-guide/deprecations.md index a0640c7bff10..944fac02d37d 100644 --- a/doc/developers-guide/deprecations.md +++ b/doc/developers-guide/deprecations.md @@ -48,6 +48,7 @@ hidden: false | estimatefees.min_acceptable | Field | v23.05 | v24.05 | `min_acceptable` feerate (implementation-specific, use modern feerates) | | estimatefees.max_acceptable | Field | v23.05 | v24.05 | `max_acceptable` feerate (implementation-specific, use modern feerates) | | commando.missing_id | Parameter | v23.02 | v24.02 | Incoming JSON commands without an `id` field | +| offer.recurrence_base.at_prefix | Parameter | v24.02 | v24.05 | `recurrence_base` with `@` prefix (use `recurrence_start_any_period`) | Inevitably there are features which need to change: either to be generalized, or removed when they can no longer be supported. diff --git a/doc/lightning-offer.7.md b/doc/lightning-offer.7.md index b7a2ce793cbf..df237c703fd3 100644 --- a/doc/lightning-offer.7.md +++ b/doc/lightning-offer.7.md @@ -6,7 +6,8 @@ SYNOPSIS **(WARNING: experimental-offers only)** -**offer** *amount* *description* [*issuer*] [*label*] [*quantity\_max*] [*absolute\_expiry*] [*recurrence*] [*recurrence\_base*] [*recurrence\_paywindow*] [*recurrence\_limit*] [*single\_use*] +**offer** *amount* *description* [*issuer*] [*label*] [*quantity\_max*] [*absolute\_expiry*] [*recurrence*] [*recurrence\_base*] [*recurrence\_paywindow*] [*recurrence\_limit*] [*single\_use*] [*recurrence\_start\_any\ +_period*] DESCRIPTION ----------- @@ -61,12 +62,9 @@ offer. The semantics of recurrence is fairly predictable, but fully documented in BOLT 12. e.g. "4weeks". *recurrence\_base* is an optional time in seconds since the first day -of 1970 UTC, optionally with a "@" prefix. This indicates when the +of 1970 UTC. This indicates when the first period begins; without this, the recurrence periods start from -the first invoice. The "@" prefix means that the invoice must start -by paying the first period; otherwise it is permitted to start at any -period. This is encoded in the offer. e.g. "@1609459200" indicates -you must start paying on the 1st January 2021. +the first invoice. *recurrence\_paywindow* is an optional argument of form '-time+time[%]'. The first time is the number of seconds before the @@ -87,6 +85,11 @@ period which exists. eg. "12" means there are 13 periods, from 0 to once; we may issue multiple invoices, but as soon as one is paid all other invoices will be expired (i.e. only one person can pay this offer). +*recurrence\_start\_any\_period* (default true) means that the invoice must +start by paying during any period; otherwise it must start by paying +at the first period. Setting this to false only makes sense if +*recurrence\_base* was provided. This is encoded in the offer. + RETURN VALUE ------------ diff --git a/plugins/offers_offer.c b/plugins/offers_offer.c index e3559b73f869..5c6953631d9a 100644 --- a/plugins/offers_offer.c +++ b/plugins/offers_offer.c @@ -166,7 +166,8 @@ static struct command_result *param_recurrence_base(struct command *cmd, jsmntok_t t = *tok; *base = tal(cmd, struct recurrence_base); - if (json_tok_startswith(buffer, &t, "@")) { + if (command_deprecated_in_ok(cmd, "recurrence_base.at_prefix", "v24.02", "v24.05") + && json_tok_startswith(buffer, &t, "@")) { t.start++; (*base)->start_any_period = false; } else @@ -174,7 +175,25 @@ static struct command_result *param_recurrence_base(struct command *cmd, if (!json_to_u64(buffer, &t, &(*base)->basetime)) return command_fail_badparam(cmd, name, buffer, tok, - "not a valid basetime or @basetime"); + "not a valid basetime"); + return NULL; +} + +static struct command_result *param_recurrence_start_any_period(struct command *cmd, + const char *name, + const char *buffer, + const jsmntok_t *tok, + struct recurrence_base **base) +{ + bool *val; + struct command_result *res = param_bool(cmd, name, buffer, tok, &val); + if (res) + return res; + + if (*val == false && !*base) + return command_fail_badparam(cmd, name, buffer, tok, + "Cannot set to false without specifying recurrence_base!"); + (*base)->start_any_period = false; return NULL; } @@ -303,6 +322,9 @@ struct command_result *json_offer(struct command *cmd, &offer->offer_recurrence_limit), p_opt_def("single_use", param_bool, &offinfo->single_use, false), + p_opt("recurrence_start_any_period", + param_recurrence_start_any_period, + &offer->offer_recurrence_base), /* FIXME: hints support! */ NULL)) return command_param_failed(); diff --git a/tests/test_pay.py b/tests/test_pay.py index ad157ce442b5..53d568aa1f6b 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -4380,10 +4380,26 @@ def test_offer(node_factory, bitcoind): # Test base # (1456740000 == 10:00:00 (am) UTC on 29 February, 2016) + + # This is deprecated, try modern alternative: + with pytest.raises(RpcError, match='invalid token'): + l1.rpc.call('offer', {'amount': '100000sat', + 'description': 'quantity_max test', + 'recurrence': '10minutes', + 'recurrence_base': '@1456740000'}) + + # Cannot use recurrence_start_any_period without recurrence_base + with pytest.raises(RpcError, match='Cannot set to false without specifying recurrence_base'): + l1.rpc.call('offer', {'amount': '100000sat', + 'description': 'quantity_max test', + 'recurrence': '10minutes', + 'recurrence_start_any_period': False}) + ret = l1.rpc.call('offer', {'amount': '100000sat', 'description': 'quantity_max test', 'recurrence': '10minutes', - 'recurrence_base': '@1456740000'}) + 'recurrence_base': 1456740000, + 'recurrence_start_any_period': False}) offer = only_one(l1.rpc.call('listoffers', [ret['offer_id']])['offers']) output = subprocess.check_output([bolt12tool, 'decode', offer['bolt12']]).decode('UTF-8')