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 <[email protected]>
rustyrussell · Feb 3, 2024 · 9d7055d · 9d7055d
1 parent 04e140a
commit 9d7055d
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 9 deletions.
diff --git 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
@@ -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
@@ -166,15 +166,34 @@ 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
 		(*base)->start_any_period = true;
 
 	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
@@ -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')