lightningd: check rune parameter names with and without punctuation.

Changelog-Changed: runes: named parameters (e.g. `pnameamountmsat`) no longer need to remove underscores (i.e. `pnameamount_msat` now works as expected).
Signed-off-by: Rusty Russell <[email protected]>

contrib/msggen/msggen/schema.json

@@ -4847,7 +4847,7 @@
                 "    * per: how often the rune can be used, with suffix \"sec\" (default), \"min\", \"hour\", \"day\" or \"msec\", \"usec\" or \"nsec\". e.g. \"per=5sec\".",
                 "    * rate: the rate limit, per minute, e.g. \"rate=60\" is equivalent to \"per=1sec\".",
                 "    * pnum: the number of parameters. e.g. \"pnum<2\".",
-                "    * pnameX: the parameter named X (with any punctuation like `_` removed). e.g. \"pnamedestination=1RustyRX2oai4EYYDpQGWvEL62BBGqN9T\".",
+                "    * pnameX: the parameter named X e.g. \"pnamedestination=1RustyRX2oai4EYYDpQGWvEL62BBGqN9T\".  NOTE: until v24.05, X had to remove underscores from the parameter name (e.g. `pnameamount_msat` had to be specified as `pnameamountmsat`) but that is now fixed.",
                 "    * parrN: the N'th parameter. e.g. \"parr0=1RustyRX2oai4EYYDpQGWvEL62BBGqN9T\"."
               ],
               "items": {

doc/schemas/lightning-createrune.json

@@ -35,7 +35,7 @@
               "    * per: how often the rune can be used, with suffix \"sec\" (default), \"min\", \"hour\", \"day\" or \"msec\", \"usec\" or \"nsec\". e.g. \"per=5sec\".",
               "    * rate: the rate limit, per minute, e.g. \"rate=60\" is equivalent to \"per=1sec\".",
               "    * pnum: the number of parameters. e.g. \"pnum<2\".",
-              "    * pnameX: the parameter named X (with any punctuation like `_` removed). e.g. \"pnamedestination=1RustyRX2oai4EYYDpQGWvEL62BBGqN9T\".",
+              "    * pnameX: the parameter named X e.g. \"pnamedestination=1RustyRX2oai4EYYDpQGWvEL62BBGqN9T\".  NOTE: until v24.05, X had to remove underscores from the parameter name (e.g. `pnameamount_msat` had to be specified as `pnameamountmsat`) but that is now fixed.",
               "    * parrN: the N'th parameter. e.g. \"parr0=1RustyRX2oai4EYYDpQGWvEL62BBGqN9T\"."
             ],
             "items": {

lightningd/runes.c

@@ -758,12 +758,17 @@ static const char *check_condition(const tal_t *ctx,
 
 		if (cinfo->params->type == JSMN_OBJECT) {
 			json_for_each_obj(i, t, cinfo->params) {
-				char *pmemname = tal_fmt(tmpctx,
+				char *pmemname = tal_fmt(ctx,
 							 "pname%.*s",
 							 t->end - t->start,
 							 cinfo->buf + t->start);
 				size_t off = strlen("pname");
-				/* Remove punctuation! */
+
+				/* First, add version with underscores intact. */
+				strmap_add(&cinfo->cached_params,
+					   tal_strdup(ctx, pmemname), t+1);
+
+				/* Now with punctuation removed: */
 				for (size_t n = off; pmemname[n]; n++) {
 					if (cispunct(pmemname[n]))
 						continue;

tests/test_runes.py

@@ -454,45 +454,45 @@ def test_rune_pay_amount(node_factory):
     # This doesn't really work, since amount_msat is illegal if invoice
     # includes an amount, and runes aren't smart enough to decode bolt11!
     rune = l1.rpc.createrune(restrictions=[['method=pay'],
-                                           ['pnameamountmsat<10000']])['rune']
+                                           ['pnameamount_msat<10000']])['rune']
 
     inv1 = l2.rpc.invoice(amount_msat=12300, label='inv1', description='description1')['bolt11']
     inv2 = l2.rpc.invoice(amount_msat='any', label='inv2', description='description2')['bolt11']
 
     # Rune requires amount_msat < 10,000!
-    with pytest.raises(RpcError, match='Not permitted: parameter amountmsat not present') as exc_info:
+    with pytest.raises(RpcError, match='Not permitted: parameter amount_msat not present') as exc_info:
         l1.rpc.checkrune(nodeid=l1.info['id'],
                          rune=rune,
                          method='pay',
                          params={'bolt11': inv1})
     assert exc_info.value.error['code'] == 0x5de
 
     # As a named parameter!
-    with pytest.raises(RpcError, match='Not permitted: parameter amountmsat not present') as exc_info:
+    with pytest.raises(RpcError, match='Not permitted: parameter amount_msat not present') as exc_info:
         l1.rpc.checkrune(nodeid=l1.info['id'],
                          rune=rune,
                          method='pay',
                          params=[inv1])
     assert exc_info.value.error['code'] == 0x5de
 
     # Can't get around it this way!
-    with pytest.raises(RpcError, match='Not permitted: parameter amountmsat not present') as exc_info:
+    with pytest.raises(RpcError, match='Not permitted: parameter amount_msat not present') as exc_info:
         l1.rpc.checkrune(nodeid=l1.info['id'],
                          rune=rune,
                          method='pay',
                          params=[inv2, 12000])
     assert exc_info.value.error['code'] == 0x5de
 
     # Nor this way, using a string!
-    with pytest.raises(RpcError, match='Not permitted: parameter amountmsat is not an integer') as exc_info:
+    with pytest.raises(RpcError, match='Not permitted: parameter amount_msat is not an integer') as exc_info:
         l1.rpc.checkrune(nodeid=l1.info['id'],
                          rune=rune,
                          method='pay',
                          params={'bolt11': inv2, 'amount_msat': '10000sat'})
     assert exc_info.value.error['code'] == 0x5de
 
     # Too much!
-    with pytest.raises(RpcError, match='Not permitted: parameter amountmsat is greater or equal to 10000') as exc_info:
+    with pytest.raises(RpcError, match='Not permitted: parameter amount_msat is greater or equal to 10000') as exc_info:
         l1.rpc.checkrune(nodeid=l1.info['id'],
                          rune=rune,
                          method='pay',
@@ -717,4 +717,3 @@ def test_rune_error_messages(node_factory):
                          rune=rune3,
                          method='pay',
                          params=['xxx', 12000])
-