Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

runes: Reimplement rate in terms of per #6710

Merged
merged 2 commits into from
Oct 2, 2023
Merged

runes: Reimplement rate in terms of per #6710

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9a19d64
pytest: trim mostly-reundant commando-rune test.
rustyrussell Oct 2, 2023
12a583d
runes: Reimplement `rate` in terms of `per`
ShahanaFarooqui Sep 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion doc/lightning-createrune.7.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ being run:
* time: the current UNIX time, e.g. "time<1656759180".
* id: the node\_id of the peer, e.g. "id=024b9a1fa8e006f1e3937f65f66c408e6da8e1ca728ea43222a7381df1cc449605".
* method: the command being run, e.g. "method=withdraw".
* rate: the rate limit, per minute, e.g. "rate=60".
* 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".
* parrN: the N'th parameter. e.g. "parr0=1RustyRX2oai4EYYDpQGWvEL62BBGqN9T".
Expand Down
114 changes: 29 additions & 85 deletions lightningd/runes.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,27 +19,7 @@
#include <lightningd/runes.h>
#include <wallet/wallet.h>

struct usage {
/* If you really issue more than 2^32 runes, they'll share ratelimit buckets */
u32 id;
u32 counter;
};

static u64 usage_id(const struct usage *u)
{
return u->id;
}

static size_t id_hash(u64 id)
{
return siphash24(siphash_seed(), &id, sizeof(id));
}

static bool usage_eq_id(const struct usage *u, u64 id)
{
return u->id == id;
}
HTABLE_DEFINE_TYPE(struct usage, usage_id, id_hash, usage_eq_id, usage_table);
static const u64 sec_per_nsec = 1000000000;

struct cond_info {
const struct runes *runes;
Expand All @@ -49,7 +29,6 @@ struct cond_info {
struct timeabs now;
const jsmntok_t *params;
STRMAP(const jsmntok_t *) cached_params;
struct usage *usage;
};

/* This is lightningd->runes */
Expand All @@ -58,31 +37,13 @@ struct runes {
struct rune *master;
u64 next_unique_id;
struct rune_blacklist *blacklist;
struct usage_table *usage_table;
};

const char *rune_is_ours(struct lightningd *ld, const struct rune *rune)
{
return rune_is_derived(ld->runes->master, rune);
}

static void memleak_help_usage_table(struct htable *memtable,
struct usage_table *usage_table)
{
memleak_scan_htable(memtable, &usage_table->raw);
}

/* Every minute we forget entries. */
static void flush_usage_table(struct runes *runes)
{
tal_free(runes->usage_table);
runes->usage_table = tal(runes, struct usage_table);
usage_table_init(runes->usage_table);
memleak_add_helper(runes->usage_table, memleak_help_usage_table);

notleak(new_reltimer(runes->ld->timers, runes, time_from_sec(60), flush_usage_table, runes));
}

/* Convert unique_id string to u64. We only expect this to fail when we're
* dealing with runes from elsewhere (i.e. param_rune) */
static bool unique_id_num(const struct rune *rune, u64 *num)
Expand Down Expand Up @@ -114,16 +75,36 @@ u64 rune_unique_id(const struct rune *rune)
return num;
}

static const char *last_time_check(const tal_t *ctx,
static const char *last_time_check(const struct rune *rune,
struct cond_info *cinfo,
u64 n_sec)
{
u64 diff;
struct timeabs last_used;

if (!wallet_get_rune(tmpctx, cinfo->runes->ld->wallet, atol(rune->unique_id), &last_used)) {
/* FIXME: If we do not know the rune, per does not work */
return NULL;
}
if (time_before(cinfo->now, last_used)) {
last_used = cinfo->now;
}

diff = time_to_nsec(time_between(cinfo->now, last_used));
if (diff < n_sec) {
return "too soon";
}
return NULL;
}

static const char *per_time_check(const tal_t *ctx,
const struct runes *runes,
const struct rune *rune,
const struct rune_altern *alt,
struct cond_info *cinfo)
{
u64 r, multiplier, diff;
struct timeabs last_used;
u64 r, multiplier;
char *endp;
const u64 sec_per_nsec = 1000000000;

if (alt->condition != '=')
return "per operator must be =";
Expand Down Expand Up @@ -151,19 +132,7 @@ static const char *last_time_check(const tal_t *ctx,
if (mul_overflows_u64(r, multiplier)) {
return "per overflow";
}
if (!wallet_get_rune(tmpctx, cinfo->runes->ld->wallet, atol(rune->unique_id), &last_used)) {
/* FIXME: If we do not know the rune, per does not work */
return NULL;
}
if (time_before(cinfo->now, last_used)) {
last_used = cinfo->now;
}
diff = time_to_nsec(time_between(cinfo->now, last_used));

if (diff < (r * multiplier)) {
return "too soon";
}
return NULL;
return last_time_check(rune, cinfo, r * multiplier);
}

static const char *rate_limit_check(const tal_t *ctx,
Expand All @@ -174,31 +143,14 @@ static const char *rate_limit_check(const tal_t *ctx,
{
unsigned long r;
char *endp;
u64 unique_id = rune_unique_id(rune);

if (alt->condition != '=')
return "rate operator must be =";

r = strtoul(alt->value, &endp, 10);
if (endp == alt->value || *endp || r == 0 || r >= UINT32_MAX)
return "malformed rate";

/* We cache this: we only add usage counter if whole rune succeeds! */
if (!cinfo->usage) {
cinfo->usage = usage_table_get(runes->usage_table, unique_id);
if (!cinfo->usage) {
cinfo->usage = tal(runes->usage_table, struct usage);
cinfo->usage->id = unique_id;
cinfo->usage->counter = 0;
usage_table_add(runes->usage_table, cinfo->usage);
}
}

/* >= becuase if we allow this, counter will increment */
if (cinfo->usage->counter >= r)
return tal_fmt(ctx, "Rate of %lu per minute exceeded", r);

return NULL;
return last_time_check(rune, cinfo, 60 * sec_per_nsec / r);
}

/* We need to initialize master runes secret early, so db can use rune_is_ours */
Expand Down Expand Up @@ -227,10 +179,6 @@ void runes_finish_init(struct runes *runes)

runes->next_unique_id = db_get_intvar(ld->wallet->db, "runes_uniqueid", 0);
runes->blacklist = wallet_get_runes_blacklist(runes, ld->wallet);

/* Initialize usage table and start flush timer. */
runes->usage_table = NULL;
flush_usage_table(runes);
}

struct rune_and_string {
Expand Down Expand Up @@ -795,7 +743,7 @@ static const char *check_condition(const tal_t *ctx,
} else if (streq(alt->fieldname, "rate")) {
return rate_limit_check(ctx, cinfo->runes, rune, alt, cinfo);
} else if (streq(alt->fieldname, "per")) {
return last_time_check(ctx, cinfo->runes, rune, alt, cinfo);
return per_time_check(ctx, cinfo->runes, rune, alt, cinfo);
}

/* Rest are params looksup: generate this once! */
Expand Down Expand Up @@ -850,6 +798,7 @@ static const char *check_condition(const tal_t *ctx,
static void update_rune_usage_time(struct runes *runes,
struct rune *rune, struct timeabs now)
{
/* FIXME: we could batch DB access if this is too slow */
wallet_rune_update_last_used(runes->ld->wallet, rune, now);
}

Expand Down Expand Up @@ -882,8 +831,6 @@ static struct command_result *json_checkrune(struct command *cmd,
cinfo.method = method;
cinfo.params = methodparams;
cinfo.now = time_now();
/* We will populate it in rate_limit_check if required. */
cinfo.usage = NULL;
strmap_init(&cinfo.cached_params);

err = rune_is_ours(cmd->ld, ras->rune);
Expand All @@ -900,9 +847,6 @@ static struct command_result *json_checkrune(struct command *cmd,
return command_fail(cmd, RUNE_NOT_PERMITTED, "Not permitted: %s", err);
}

/* If it succeeded, *now* we increment any associated usage counter. */
if (cinfo.usage)
cinfo.usage->counter++;
update_rune_usage_time(cmd->ld->runes, ras->rune, cinfo.now);

js = json_stream_success(cmd);
Expand Down
82 changes: 2 additions & 80 deletions tests/test_plugin.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2796,17 +2796,7 @@ def test_commando_rune(node_factory):
{'alternatives': ['parr1!', 'parr1/io'],
'summary': "parr1 (array parameter #1) is missing OR parr1 (array parameter #1) unequal to 'io'"}]),
(rune7, [{'alternatives': ['pnum=0'],
'summary': "pnum (number of command parameters) equal to 0"}]),
(rune8, [{'alternatives': ['pnum=0'],
'summary': "pnum (number of command parameters) equal to 0"},
{'alternatives': ['rate=3'],
'summary': "rate (max per minute) equal to 3"}]),
(rune9, [{'alternatives': ['pnum=0'],
'summary': "pnum (number of command parameters) equal to 0"},
{'alternatives': ['rate=3'],
'summary': "rate (max per minute) equal to 3"},
{'alternatives': ['rate=1'],
'summary': "rate (max per minute) equal to 1"}]))
'summary': "pnum (number of command parameters) equal to 0"}]))
for decode in runedecodes:
rune = decode[0]
restrictions = decode[1]
Expand Down Expand Up @@ -2840,10 +2830,7 @@ def test_commando_rune(node_factory):
(rune6, "listpeers", [l2.info['id'], 'broken']),
(rune6, "listpeers", [l2.info['id']]),
(rune7, "listpeers", []),
(rune7, "getinfo", {}),
(rune9, "getinfo", {}),
(rune8, "getinfo", {}),
(rune8, "getinfo", {}))
(rune7, "getinfo", {}))

failures = ((rune2, "withdraw", {}),
(rune2, "plugin", {'subcommand': 'list'}),
Expand All @@ -2865,7 +2852,6 @@ def test_commando_rune(node_factory):
time.sleep(1)

for rune, cmd, params in failures:
print("{} {}".format(cmd, params))
with pytest.raises(RpcError, match='Invalid rune: Not permitted:') as exc_info:
l2.rpc.call(method='commando',
payload={'peer_id': l1.info['id'],
Expand All @@ -2874,70 +2860,6 @@ def test_commando_rune(node_factory):
'params': params})
assert exc_info.value.error['code'] == 0x4c51

# Now, this can flake if we cross a minute boundary! So wait until
# It succeeds again.
while True:
try:
l2.rpc.call(method='commando',
payload={'peer_id': l1.info['id'],
'rune': rune8['rune'],
'method': 'getinfo',
'params': {}})
break
except RpcError as e:
assert e.error['code'] == 0x4c51
time.sleep(1)

# This fails immediately, since we've done one.
with pytest.raises(RpcError, match='Invalid rune: Not permitted: Rate of 1 per minute exceeded') as exc_info:
l2.rpc.call(method='commando',
payload={'peer_id': l1.info['id'],
'rune': rune9['rune'],
'method': 'getinfo',
'params': {}})
assert exc_info.value.error['code'] == 0x4c51

# Two more succeed for rune8.
for _ in range(2):
l2.rpc.call(method='commando',
payload={'peer_id': l1.info['id'],
'rune': rune8['rune'],
'method': 'getinfo',
'params': {}})
assert exc_info.value.error['code'] == 0x4c51

# Now we've had 3 in one minute, this will fail.
with pytest.raises(RpcError, match='') as exc_info:
l2.rpc.call(method='commando',
payload={'peer_id': l1.info['id'],
'rune': rune8['rune'],
'method': 'getinfo',
'params': {}})
assert exc_info.value.error['code'] == 0x4c51

# rune5 can only be used by l2:
l3 = node_factory.get_node()
l3.connect(l1)
with pytest.raises(RpcError, match='Invalid rune: Not permitted: id does not start with 022d223620a359a47ff7') as exc_info:
l3.rpc.call(method='commando',
payload={'peer_id': l1.info['id'],
'rune': rune5['rune'],
'method': "listpeers",
'params': {}})
assert exc_info.value.error['code'] == 0x4c51

# Now wait for ratelimit expiry, ratelimits should reset.
time.sleep(61)

for rune, cmd, params in ((rune9, "getinfo", {}),
(rune8, "getinfo", {}),
(rune8, "getinfo", {})):
l2.rpc.call(method='commando',
payload={'peer_id': l1.info['id'],
'rune': rune['rune'],
'method': cmd,
'params': params})


def test_commando_listrunes(node_factory):
l1 = node_factory.get_node()
Expand Down
Loading