Skip to content

[Bug] Rate limiter records the per-delegate-key entry before a later layer can deny, and the "roll back" the comment promises does not exist #1002

Open

Description

@takaolovely

Surface

Relayer / hosted API

Network

Mainnet (relayer.memory.walrus.xyz)

Package version

Relayer at dev f248bd37 (services/server, crate version 0.1.0)

What happened?

rate_limit_middleware runs three layers in sequence. Layer 1 atomically checks and records the per-delegate-key window entry. Layer 2 (per-account burst) can then return Denied — and the comment at that branch states the layer-1 entry is rolled back. No rollback exists: there is no ZREM or any other removal call between the two Lua invocations. Every request denied at the account layer therefore permanently consumes per-delegate-key budget for the rest of the 60 s window, and the 429 the caller eventually receives is labelled layer: "delegate_key" even though the delegate-key budget was only consumed because of the account-layer denial.

Steps to reproduce

// services/server/src/rate_limit.rs:505-537 — layer 2 denial
Ok(WindowCheckResult::Denied) => {
    // Roll back the delegate-key window entry just recorded above
    // (best-effort; a Lua multi-key script would be fully atomic across keys)
    return rate_limit_response("account_burst", config.max_requests_per_minute, "min", 60);
}
grep -n "ZREM\|rollback\|roll_back" services/server/src/rate_limit.rs
# -> one hit only: ZREMRANGEBYSCORE inside the sliding-window Lua (normal pruning)
#    plus a test asserting that Lua string contains it. No rollback call exists.

Runtime demonstration (unit test in the file's existing mod tests, next to concurrent_same_millisecond_requests_respect_limit):

  1. Set a delegate-key budget of N and an account budget of 1.
  2. Send two requests in the same window: the first is allowed by both layers; the second is denied at layer 2.
  3. Assert ZCARD rate:dk:<public_key> — it still counts both, i.e. the denied request spent delegate-key budget.

Novelty

SUGGESTED FIX

Move all three layers into a single Lua script keyed on the three windows so a denial rolls back the entries the same invocation already recorded, or (cheaper) explicitly ZREM the layer-1 member when a later layer denies — and while there, make the returned layer field name the layer that actually denied.

Expected

Either the layer-1 entry is removed when a later layer denies (as the comment claims), or the budgets are evaluated in one atomic multi-key script so a denied request consumes nothing in a sibling layer.

Actual

The denial consumes delegate-key budget, so a client that momentarily trips the account cap finds its tighter, fail-closed per-key budget pre-drained: an unrelated retry later in the same minute is rejected with layer: "delegate_key", pointing the user at the wrong remediation.

Logs or error text

429 {"error":"Rate limit exceeded","layer":"delegate_key",
     "limit":"60 weighted-requests/min","retry_after_seconds":60}

(the layer label is what misattributes the denial; the delegated budget was drained by an account-layer denial)

Checks

  • I searched existing issues and this is not a duplicate.
  • This report contains no private keys, mnemonics, or other secrets.

Activity

  1. github-actions commented on Sep 23, 2026

    @github-actions

    Thank you for opening this issue, a team member will review it shortly.

  2. takaolovely commented on Oct 14, 2026

    @takaolovely
    Author

    Still present on dev. services/server/src/rate_limit.rs:516-537 still
    returns on a layer-2 denial with the comment at :522-523 promising the
    rollback, and the crate still contains no removal call for it: the only
    ZREMRANGEBYSCORE is the ordinary pruning inside SLIDING_WINDOW_LUA
    (:216), and there is no ZREM anywhere.

    So a request denied by the account-burst layer permanently spends
    per-delegate-key budget for the rest of the window, and a later unrelated
    retry in the same minute is answered as a delegate-key denial that names
    the wrong remediation. The WALM-415 work and #1028 added limits to other
    surfaces; neither touches inter-layer accounting.

    Cheapest test that pins it: set a delegate-key budget of N and an account
    budget of 1, send two requests in one window, then assert ZCARD
    rate:dk:<public_key> counts one rather than two.

  3. harrymove-ctrl commented on Oct 28, 2026

    @harrymove-ctrl
    Collaborator

    Thanks for the detailed report. We reproduced this on dev and confirmed the root cause is that RateLimiter::check recorded the Layer-1 per-delegate-key request before evaluating Layer-2 per-account and Layer-3 global limits without rolling back earlier layers on denial. A fix is in progress and tracked in #1331; we will update this thread when it ships.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions