config: replace accept-htlc-tlv-types with accept-htlc-tlv-type

We use multi-specifiable options elsewhere, this is just another.
Otherwise you can't add, you can only set them all.

Changelog-Added: Config: `accept-htlc-tlv-type` (replaces awkward-to-use `accept-htlc-tlv-types`)
Changelog-Deprecated: Config: `accept-htlc-tlv-types` (use `accept-htlc-tlv-type` multiple times)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

common/json_parse.c

@@ -709,27 +709,6 @@ json_to_blinded_path(const tal_t *ctx, const char *buffer, const jsmntok_t *tok)
 	return rpath;
 }
 
-bool json_to_uintarr(const char *buffer, const jsmntok_t *tok, u64 **dest)
-{
-	char *str = json_strdup(NULL, buffer, tok);
-	char *endp, **elements = tal_strsplit(str, str, ",", STR_NO_EMPTY);
-	unsigned long long l;
-	u64 u;
-	for (int i = 0; elements[i] != NULL; i++) {
-		/* This is how the manpage says to do it.  Yech. */
-		errno = 0;
-		l = strtoull(elements[i], &endp, 0);
-		if (*endp || !str[0])
-			return tal_fmt(NULL, "'%s' is not a number", elements[i]);
-		u = l;
-		if (errno || u != l)
-			return tal_fmt(NULL, "'%s' is out of range", elements[i]);
-		tal_arr_expand(dest, u);
-	}
-	tal_free(str);
-	return NULL;
-}
-
 bool
 json_tok_channel_id(const char *buffer, const jsmntok_t *tok,
 		    struct channel_id *cid)

common/json_parse.h

@@ -114,9 +114,6 @@ bool json_to_channel_id(const char *buffer, const jsmntok_t *tok,
 bool json_to_coin_mvt_tag(const char *buffer, const jsmntok_t *tok,
 			  enum mvt_tag *tag);
 
-/* Read a JSON value into an array of u64 */
-bool json_to_uintarr(const char *buffer, const jsmntok_t *tok, u64 **dest);
-
 /* Extract reply path from this JSON */
 struct blinded_path *
 json_to_blinded_path(const tal_t *ctx, const char *buffer, const jsmntok_t *tok);

doc/lightning-listconfigs.7.md

@@ -323,7 +323,7 @@ On success, an object is returned, containing:
 - **force-feerates** (string, optional): force-feerate configuration setting, if any
 - **subdaemon** (string, optional): `subdaemon` fields from config or cmdline if any (can be more than one)
 - **fetchinvoice-noconnect** (boolean, optional): `fetchinvoice-noconnect` fields from config or cmdline, or default
-- **accept-htlc-tlv-types** (string, optional): `accept-htlc-tlv-types` fields from config or cmdline, or not present
+- **accept-htlc-tlv-types** (string, optional): `accept-htlc-tlv-types` field from config or cmdline, or not present **deprecated, removal in v24.05**
 - **tor-service-password** (string, optional): `tor-service-password` field from config or cmdline, if any
 - **dev-allowdustreserve** (boolean, optional): Whether we allow setting dust reserves
 - **announce-addr-dns** (boolean, optional): Whether we put DNS entries into node\_announcement **deprecated, removal in v24.05** *(added v22.11.1)*
@@ -446,4 +446,4 @@ RESOURCES
 
 Main web site: <https://github.com/ElementsProject/lightning>
 
-[comment]: # ( SHA256STAMP:c847cb106f78f616b3adfe506ef499c9228f3966a44e6164b9cf49e1cf67d417)
+[comment]: # ( SHA256STAMP:a3d32b74eb78b014e39ea85c19b3366e0468087ae44da633e9d5a194d2fe11b6)

doc/schemas/listconfigs.schema.json

@@ -1592,8 +1592,9 @@
       "description": "`fetchinvoice-noconnect` fields from config or cmdline, or default"
     },
     "accept-htlc-tlv-types": {
+      "deprecated": "v23.08",
       "type": "string",
-      "description": "`accept-htlc-tlv-types` fields from config or cmdline, or not present"
+      "description": "`accept-htlc-tlv-types` field from config or cmdline, or not present"
     },
     "tor-service-password": {
       "type": "string",

lightningd/options.c

@@ -190,25 +190,28 @@ static char *fmt_force_feerates(const tal_t *ctx, const u32 *force_feerates)
 	return ret;
 }
 
+static char *opt_add_accept_htlc_tlv(const char *arg,
+				     u64 **accept_extra_tlv_types)
+{
+	size_t n = tal_count(*accept_extra_tlv_types);
+
+	tal_resize(accept_extra_tlv_types, n+1);
+	return opt_set_u64(arg, &(*accept_extra_tlv_types)[n]);
+}
+
 static char *opt_set_accept_extra_tlv_types(const char *arg,
 					    struct lightningd *ld)
 {
-	char *endp, **elements = tal_strsplit(NULL, arg, ",", STR_NO_EMPTY);
-	unsigned long long l;
-	u64 u;
-	for (int i = 0; elements[i] != NULL; i++) {
-		/* This is how the manpage says to do it.  Yech. */
-		errno = 0;
-		l = strtoull(elements[i], &endp, 0);
-		if (*endp || !arg[0])
-			return tal_fmt(NULL, "'%s' is not a number", arg);
-		u = l;
-		if (errno || u != l)
-			return tal_fmt(NULL, "'%s' is out of range", arg);
-		tal_arr_expand(&ld->accept_extra_tlv_types, u);
-	}
+	char *ret, **elements = tal_strsplit(tmpctx, arg, ",", STR_NO_EMPTY);
 
-	tal_free(elements);
+	if (!deprecated_apis)
+		return "Please use --accept-htlc-tlv-type multiple times";
+	for (int i = 0; elements[i] != NULL; i++) {
+		ret = opt_add_accept_htlc_tlv(elements[i],
+					      &ld->accept_extra_tlv_types);
+		if (ret)
+			return ret;
+	}
 	return NULL;
 }
 
@@ -1385,6 +1388,10 @@ static void register_opts(struct lightningd *ld)
 	opt_register_arg("--accept-htlc-tlv-types",
 			 opt_set_accept_extra_tlv_types, NULL, ld,
 			 "Comma separated list of extra HTLC TLV types to accept.");
+	clnopt_witharg("--accept-htlc-tlv-type", OPT_MULTI|OPT_SHOWINT,
+		       opt_add_accept_htlc_tlv, NULL,
+		       &ld->accept_extra_tlv_types,
+		       "HTLC TLV type to accept (can be used multiple times)");
 
 	opt_register_early_noarg("--disable-dns", opt_set_invbool, &ld->config.use_dns,
 				 "Disable DNS lookups of peers");
@@ -1887,6 +1894,8 @@ static void add_config_deprecated(struct lightningd *ld,
 			   || opt->cb_arg == (void *)plugin_opt_flag_set) {
 			/* FIXME: We actually treat it as if they specified
 			 * --plugin for each one, so ignore these */
+		} else if (opt->cb_arg == (void *)opt_add_accept_htlc_tlv) {
+			/* We ignore this: it's printed below: */
 		} else if (opt->cb_arg == (void *)opt_set_accept_extra_tlv_types) {
 			for (size_t i = 0;
 			     i < tal_count(ld->accept_extra_tlv_types);
@@ -1989,6 +1998,7 @@ static const char *get_opt_val(const struct opt_table *ot,
 	    || ot->cb_arg == (void *)opt_subdaemon
 	    || ot->cb_arg == (void *)opt_set_db_upgrade
 	    || ot->cb_arg == (void *)arg_log_to_file
+	    || ot->cb_arg == (void *)opt_add_accept_htlc_tlv
 #if DEVELOPER
 	    || ot->cb_arg == (void *)opt_subd_dev_disconnect
 	    || ot->cb_arg == (void *)opt_force_featureset

plugins/keysend.c

@@ -8,6 +8,7 @@
 #include <common/json_stream.h>
 #include <common/memleak.h>
 #include <common/type_to_string.h>
+#include <errno.h>
 #include <plugins/libplugin-pay.h>
 #include <sodium.h>
 
@@ -161,29 +162,65 @@ REGISTER_PAYMENT_MODIFIER(check_preapprovekeysend, void *, NULL,
  * End of check_preapprovekeysend modifier
  *****************************************************************************/
 
+/* Deprecated: comma-separated string containing integers */
+static bool json_accumulate_uintarr(const char *buffer,
+				    const jsmntok_t *tok,
+				    u64 **dest)
+{
+	char *str = json_strdup(NULL, buffer, tok);
+	char *endp, **elements = tal_strsplit(str, str, ",", STR_NO_EMPTY);
+	unsigned long long l;
+	u64 u;
+	for (int i = 0; elements[i] != NULL; i++) {
+		/* This is how the manpage says to do it.  Yech. */
+		errno = 0;
+		l = strtoull(elements[i], &endp, 0);
+		if (*endp || !str[0])
+			return false;
+		u = l;
+		if (errno || u != l)
+			return false;
+		tal_arr_expand(dest, u);
+	}
+	tal_free(str);
+	return NULL;
+}
+
+/* values_int is a JSON array of u64s */
+static bool jsonarr_accumulate_u64(const char *buffer,
+				   const jsmntok_t *tok,
+				   u64 **dest)
+{
+	const jsmntok_t *t;
+	size_t i, n;
+
+	if (tok->type != JSMN_ARRAY)
+		return false;
+	n = tal_count(*dest);
+	tal_resize(dest, n + tok->size);
+	json_for_each_arr(i, t, tok) {
+		if (!json_to_u64(buffer, t, &(*dest)[n + i]))
+			return false;
+	}
+	return true;
+}
+
 static const char *init(struct plugin *p, const char *buf UNUSED,
 			const jsmntok_t *config UNUSED)
 {
-	const jsmntok_t *maxdelay, *extratlvs, *ctok;
-	const char *cbuf;
-
 	rpc_scan(p, "getinfo", take(json_out_obj(NULL, NULL, NULL)), "{id:%}",
 		 JSON_SCAN(json_to_node_id, &my_id));
 
-	ctok =
-	    jsonrpc_request_sync(tmpctx, p, "listconfigs",
-				 take(json_out_obj(NULL, NULL, NULL)), &cbuf);
-	/* `accept-htlc-tlv-types` may be missing if not set in the
-	 * config */
-	maxdelay = json_get_member(cbuf, ctok, "max-locktime-blocks");
-	extratlvs = json_get_member(cbuf, ctok, "accept-htlc-tlv-types");
 	accepted_extra_tlvs = notleak(tal_arr(NULL, u64, 0));
-
-	assert(maxdelay != NULL);
-	json_to_number(cbuf, maxdelay, &maxdelay_default);
-
-	if (extratlvs != NULL)
-		json_to_uintarr(cbuf, extratlvs, &accepted_extra_tlvs);
+	/* accept-htlc-tlv-types deprecated in v23.08, but still grab it! */
+	rpc_scan(p, "listconfigs", take(json_out_obj(NULL, NULL, NULL)),
+		 "{configs:{"
+		 "max-locktime-blocks:{value_int:%},"
+		 "accept-htlc-tlv-types?:{value_str:%},"
+		 "accept-htlc-tlv-type:{values_int:%}}}",
+		 JSON_SCAN(json_to_u32, &maxdelay_default),
+		 JSON_SCAN(json_accumulate_uintarr, &accepted_extra_tlvs),
+		 JSON_SCAN(jsonarr_accumulate_u64, &accepted_extra_tlvs));
 
 	return NULL;
 }

tests/test_pay.py

@@ -3546,7 +3546,7 @@ def test_keysend_strip_tlvs(node_factory):
         opts=[
             {
                 # Not needed, just for listconfigs test.
-                'accept-htlc-tlv-types': '133773310,99990',
+                'accept-htlc-tlv-type': [133773310, 99990],
                 "plugin": os.path.join(os.path.dirname(__file__), "plugins/sphinx-receiver.py"),
             },
             {
@@ -3556,7 +3556,7 @@ def test_keysend_strip_tlvs(node_factory):
     )
 
     # Make sure listconfigs works here
-    assert l1.rpc.listconfigs()['accept-htlc-tlv-types'] == '133773310,99990'
+    assert l1.rpc.listconfigs('accept-htlc-tlv-type')['configs']['accept-htlc-tlv-type']['values_int'] == [133773310, 99990]
 
     # l1 is configured to accept, so l2 should still filter them out
     l1.rpc.keysend(l2.info['id'], amt, extratlvs={133773310: 'FEEDC0DE'})