Skip to content

Commit

Permalink
lightningd: check rune parameter names with and without punctuation.
Browse files Browse the repository at this point in the history
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 <rusty@rustcorp.com.au>
  • Loading branch information
rustyrussell committed Mar 19, 2024
1 parent 16de175 commit 6540d29
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 11 deletions.
2 changes: 1 addition & 1 deletion contrib/msggen/msggen/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
2 changes: 1 addition & 1 deletion doc/schemas/lightning-createrune.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
9 changes: 7 additions & 2 deletions lightningd/runes.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
13 changes: 6 additions & 7 deletions tests/test_runes.py
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -717,4 +717,3 @@ def test_rune_error_messages(node_factory):
rune=rune3,
method='pay',
params=['xxx', 12000])

0 comments on commit 6540d29

Please sign in to comment.