From 738787021f048af823238dec97b1401985d319b7 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 6 Jun 2023 11:27:03 +0930 Subject: [PATCH 1/4] doc: document that we strip punctuation from parameter names. This lead to confusion about how to specify `amount_msat`. Signed-off-by: Rusty Russell --- doc/lightning-commando-rune.7.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/lightning-commando-rune.7.md b/doc/lightning-commando-rune.7.md index 8f0a2776c767..c06ee597d216 100644 --- a/doc/lightning-commando-rune.7.md +++ b/doc/lightning-commando-rune.7.md @@ -34,7 +34,7 @@ being run: * method: the command being run, e.g. "method=withdraw". * rate: the rate limit, per minute, e.g. "rate=60". * pnum: the number of parameters. e.g. "pnum<2". -* pnameX: the parameter named X. e.g. "pnamedestination=1RustyRX2oai4EYYDpQGWvEL62BBGqN9T". +* pnameX: the parameter named X (with any punctuation like `_` removed). e.g. "pnamedestination=1RustyRX2oai4EYYDpQGWvEL62BBGqN9T". * parrN: the N'th parameter. e.g. "parr0=1RustyRX2oai4EYYDpQGWvEL62BBGqN9T". RESTRICTION FORMAT From 1c821ec0966612ff20e4a35ae980d6910a879223 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 6 Jun 2023 11:28:03 +0930 Subject: [PATCH 2/4] pytest: test for amount comparison. Signed-off-by: Rusty Russell --- tests/test_plugin.py | 53 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/tests/test_plugin.py b/tests/test_plugin.py index a5c3a85d4d87..8cf3ca9bb8e2 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -2991,6 +2991,59 @@ def test_commando_listrunes(node_factory): assert not_our_rune['our_rune'] is False +@pytest.mark.xfail(strict=True) +def test_commando_rune_pay_amount(node_factory): + l1, l2 = node_factory.line_graph(2) + + # 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.commando_rune(restrictions=[['method=pay'], + ['pnameamountmsat<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! + with pytest.raises(RpcError, match='Not authorized:'): + l2.rpc.commando(peer_id=l1.info['id'], + rune=rune, + method='pay', + params={'bolt11': inv1}) + + # As a named parameter! + with pytest.raises(RpcError, match='Not authorized:'): + l2.rpc.commando(peer_id=l1.info['id'], + rune=rune, + method='pay', + params=[inv1]) + + # Can't get around it this way! + with pytest.raises(RpcError, match='Not authorized:'): + l2.rpc.commando(peer_id=l1.info['id'], + rune=rune, + method='pay', + params=[inv2, 12000]) + + # Nor this way, using a string! + with pytest.raises(RpcError, match='Not authorized:'): + l2.rpc.commando(peer_id=l1.info['id'], + rune=rune, + method='pay', + params={'bolt11': inv2, 'amount_msat': '10000sat'}) + + # Too much! + with pytest.raises(RpcError, match='Not authorized:'): + l2.rpc.commando(peer_id=l1.info['id'], + rune=rune, + method='pay', + params={'bolt11': inv2, 'amount_msat': 12000}) + + # This works + l2.rpc.commando(peer_id=l1.info['id'], + rune=rune, + method='pay', + params={'bolt11': inv2, 'amount_msat': 9999}) + + def test_commando_blacklist(node_factory): l1, l2 = node_factory.get_nodes(2) From 4c8e225ee56d5d6f9d2333c00f14050147e76d56 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 6 Jun 2023 11:29:03 +0930 Subject: [PATCH 3/4] commando: integer command parameters can be compared with < and >. Previously any attempt would result in "is not an integer field"; we now recognize valid JSON integers as integer fields. Changelog-Fixed: Plugins: `commando` runes can now compare integer parameters using '<' and '>' as expected. Signed-off-by: Rusty Russell --- plugins/commando.c | 12 ++++++++++++ tests/test_plugin.py | 1 - 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/plugins/commando.c b/plugins/commando.c index 5df9032f78e7..7871da531f44 100644 --- a/plugins/commando.c +++ b/plugins/commando.c @@ -427,6 +427,18 @@ static const char *check_condition(const tal_t *ctx, if (!ptok) return rune_alt_single_missing(ctx, alt); + /* Pass through valid integers as integers. */ + if (ptok->type == JSMN_PRIMITIVE) { + s64 val; + + if (json_to_s64(cinfo->buf, ptok, &val)) { + plugin_log(plugin, LOG_DBG, "It's an int %"PRId64, val); + return rune_alt_single_int(ctx, alt, val); + } + + /* Otherwise, treat it as a string (< and > will fail with + * "is not an integer field") */ + } return rune_alt_single_str(ctx, alt, cinfo->buf + ptok->start, ptok->end - ptok->start); diff --git a/tests/test_plugin.py b/tests/test_plugin.py index 8cf3ca9bb8e2..06879dd8314e 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -2991,7 +2991,6 @@ def test_commando_listrunes(node_factory): assert not_our_rune['our_rune'] is False -@pytest.mark.xfail(strict=True) def test_commando_rune_pay_amount(node_factory): l1, l2 = node_factory.line_graph(2) From b29732e43776303951cb94f29e0998c6867f6cf9 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 6 Jun 2023 14:32:56 +0930 Subject: [PATCH 4/4] pytest: mark test_commando_stress as slow. It sometimes takes > 1800 seconds under valgrind. Signed-off-by: Rusty Russell --- tests/test_plugin.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_plugin.py b/tests/test_plugin.py index 06879dd8314e..593a4e217391 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -3124,6 +3124,7 @@ def test_commando_blacklist(node_factory): assert blacklisted_rune is True +@pytest.mark.slow_test def test_commando_stress(node_factory, executor): """Stress test to slam commando with many large queries""" nodes = node_factory.get_nodes(5)