Skip to content

Commit

Permalink
lightningd: name error messages a bit more readable.
Browse files Browse the repository at this point in the history 
Rather than speaking 'rune' we should speak english in error messages.

Signed-off-by: Rusty Russell <[email protected]>
Reported-by: @ShahanaFarooqui
  • Loading branch information
@rustyrussell
rustyrussell committed Mar 20, 2024
1 parent ba9daa4 commit a880146
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 10 deletions.
8 changes: 8 additions & 0 deletions lightningd/runes.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -849,6 +849,14 @@ static struct command_result *json_checkrune(struct command *cmd,
/* Just in case they manage to make us speak non-JSON, escape! */
if (err) {
err = json_escape(tmpctx, err)->s;

/* Turn runespeak into english! */
if (strstarts(err, "pname"))
err = tal_strcat(tmpctx, "parameter ", err + strlen("pname"));
else if (strstarts(err, "parr"))
err = tal_strcat(tmpctx, "parameter #", err + strlen("parr"));
else if (strstarts(err, "pnum"))
err = tal_strcat(tmpctx, "number of parameters", err + strlen("pnum"));
return command_fail(cmd, RUNE_NOT_PERMITTED, "Not permitted: %s", err);
}

Expand Down
10 changes: 5 additions & 5 deletions tests/test_plugin.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2939,35 +2939,35 @@ def test_commando_rune_pay_amount(node_factory):
inv2 = l2.rpc.invoice(amount_msat='any', label='inv2', description='description2')['bolt11']

# Rune requires amount_msat!
with pytest.raises(RpcError, match='Invalid rune: Not permitted: pnameamountmsat not present'):
with pytest.raises(RpcError, match='Invalid rune: Not permitted: parameter amountmsat not present'):
l2.rpc.commando(peer_id=l1.info['id'],
rune=rune,
method='pay',
params={'bolt11': inv1})

# As a named parameter!
with pytest.raises(RpcError, match='Invalid rune: Not permitted: pnameamountmsat not present'):
with pytest.raises(RpcError, match='Invalid rune: Not permitted: parameter amountmsat not present'):
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='Invalid rune: Not permitted: pnameamountmsat not present'):
with pytest.raises(RpcError, match='Invalid rune: Not permitted: parameter amountmsat not present'):
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='Invalid rune: Not permitted: pnameamountmsat is not an integer field'):
with pytest.raises(RpcError, match='Invalid rune: Not permitted: parameter amountmsat is not an integer field'):
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='Invalid rune: Not permitted: pnameamountmsat is greater or equal to 10000'):
with pytest.raises(RpcError, match='Invalid rune: Not permitted: parameter amountmsat is greater or equal to 10000'):
l2.rpc.commando(peer_id=l1.info['id'],
rune=rune,
method='pay',
Expand Down
57 changes: 52 additions & 5 deletions tests/test_runes.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -460,39 +460,39 @@ def test_rune_pay_amount(node_factory):
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: pnameamountmsat not present') as exc_info:
with pytest.raises(RpcError, match='Not permitted: parameter amountmsat 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: pnameamountmsat not present') as exc_info:
with pytest.raises(RpcError, match='Not permitted: parameter amountmsat 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: pnameamountmsat not present') as exc_info:
with pytest.raises(RpcError, match='Not permitted: parameter amountmsat 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: pnameamountmsat is not an integer') as exc_info:
with pytest.raises(RpcError, match='Not permitted: parameter amountmsat 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: pnameamountmsat is greater or equal to 10000') as exc_info:
with pytest.raises(RpcError, match='Not permitted: parameter amountmsat is greater or equal to 10000') as exc_info:
l1.rpc.checkrune(nodeid=l1.info['id'],
rune=rune,
method='pay',
Expand Down Expand Up @@ -671,3 +671,50 @@ def test_id_migration(node_factory):

# Our migration should have removed this row now
assert l1.db_query("SELECT * FROM vars WHERE name = 'runes_uniqueid';") == []


def test_rune_error_messages(node_factory):
l1 = node_factory.get_node()

rune1 = l1.rpc.createrune(restrictions=[['method=pay'],
['pnum=1']])['rune']
with pytest.raises(RpcError, match='Not permitted: number of parameters is not equal to 1'):
l1.rpc.checkrune(nodeid=l1.info['id'],
rune=rune1,
method='pay',
params=['xxx', 12000])
with pytest.raises(RpcError, match='Not permitted: number of parameters is not equal to 1'):
l1.rpc.checkrune(nodeid=l1.info['id'],
rune=rune1,
method='pay',
params={'bolt11': 'xxx', 'amount_msat': 17})

rune2 = l1.rpc.createrune(restrictions=[['method=pay'],
['parr1=17']])['rune']

with pytest.raises(RpcError, match='Not permitted: parameter #1 is not equal to 17'):
l1.rpc.checkrune(nodeid=l1.info['id'],
rune=rune2,
method='pay',
params=['xxx', 12000])

with pytest.raises(RpcError, match='Not permitted: parameter #1 not present'):
l1.rpc.checkrune(nodeid=l1.info['id'],
rune=rune2,
method='pay',
params={'bolt11': 'xxx', 'amount_msat': 12000})

rune3 = l1.rpc.createrune(restrictions=[['method=pay'],
['pnamebolt11=xxx']])['rune']

with pytest.raises(RpcError, match='Not permitted: parameter bolt11 is not equal to xxx'):
l1.rpc.checkrune(nodeid=l1.info['id'],
rune=rune3,
method='pay',
params={'bolt11': 'yyy', 'amount_msat': 12000})
with pytest.raises(RpcError, match='Not permitted: parameter bolt11 not present'):
l1.rpc.checkrune(nodeid=l1.info['id'],
rune=rune3,
method='pay',
params=['xxx', 12000])

0 comments on commit a880146

Please sign in to comment.