Skip to content

Move rate-limit-manager to messaging client layer#4021

Open
stubbsta wants to merge 3 commits into
masterfrom
chore/move-ratelimitmanager-to-client-layer
Open

Move rate-limit-manager to messaging client layer#4021
stubbsta wants to merge 3 commits into
masterfrom
chore/move-ratelimitmanager-to-client-layer

Conversation

@stubbsta

@stubbsta stubbsta commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Moves rate_limit_manager out of the reliable-channels layer into the
messaging layer, so it sits next to the send pipeline and (in follow-up
work) can interact directly with the RLN component. Three commits, each
reviewable on its own:

1. Drop the rate-limit stage from the reliable channel pipeline

The outgoing pipeline collapses to segmentation → sds → encryption → dispatch. Since the rate-limit hop was the only reason encrypt and
dispatch were decoupled through the event bus, the encrypt-and-dispatch
tail of onReadyToSend folds back into send(), and with it goes all
the machinery that existed only to bridge that hop:

  • the rateLimit field and rateConfig constructor param on
    ReliableChannel (and all call sites, incl. the channel manager),
  • the ReadyToSendEvent listener and the event type (no other producer),
  • the awaitingDispatch accounting,
  • enqueueToSend on the manager.

reliable_channel.nim is −191 lines net; the channels layer no longer
references rate limiting at all.

2. Relocate the module to logos_delivery/messaging/rate_limit_manager/

The move doubles as the layering proof: to compile in its new home the
manager loses channelId, the SdsChannelID type, the SDS import, and
brokerCtx — all channels-layer concerns. Rate limiting is a node-wide
property of the sender's RLN identity, not a per-channel one, so nothing
above needs the channel identity back.

The event-emission API is replaced with a direct call:

proc admit(self: RateLimitManager, msg: seq[byte]):
  Future[Result[void, RateLimitError]]

The epoch fields (epochPeriodSec, currentEpochStart,
sentInCurrentEpoch, queue, dequeueReady, resetEpoch) are kept
as-is: this PR changes the component's home and API surface, not the
epoch mechanism.

3. Meter SendService transmissions through the manager

MessagingClientConf gains a rateLimit: RateLimitConfig field
(defaults: DefaultEpochPeriodSec / DefaultMessagesPerEpoch, carried
as a field default so every construction site inherits it).
MessagingClient.new constructs the manager and hands it to
SendService, which consults admit() before the first
transmission
of a task — in send() and in the retry loop, guarded by
firstPropagatedTime.isNone() so re-publishes of an already-propagated
message (identical bytes) skip admission. An over-budget task parks in
the task cache as NextRoundRetry; the 1s service loop re-admits it as
the epoch rolls over.

Metering at the transmission stage rather than at MessagingClient.send
is deliberate:

  • SDS repair rebroadcasts (dispatchRepair, which documents "Pacing is
    done by SDS itself") dispatch through the same MessagingSend broker
    as user sends. An API-entry gate would let the messaging limiter
    reject repair traffic — exactly when the reliability layer is
    recovering from loss. At the transmission stage, SDS decides that a
    repair is needed and the scheduler decides when it fits the budget.
  • Admission is per WakuMessage (one DeliveryTask each), which matches
    RLN accounting: every SDS segment is a separate relay message needing
    its own proof.
  • Parking over-budget messages until the epoch rolls requires a retry
    loop; SendService already is one.

Behaviour

Unchanged. admit() is a pass-through skeleton (return ok()), and the
config default leaves enabled: false. This PR is the relocation and
the seam; enforcement lands separately (see below).

Not touched

  • The send processors (relay/lightpush) and the RLN proof provider
    registration.
  • The epoch/budget mechanism inside the manager.
  • ReliableChannelManagerConf still carries rateLimitEnabled /
    rateLimitEpochPeriodSec / rateLimitMessagesPerEpoch. Post-move
    nothing reads them, but tests/api/test_conf.nim exercises them via
    channelsOverrides JSON, so removing them is a follow-up API decision.

Tests

  • tests/messaging/test_rate_limit_manager.nim (new) pins the current
    contract: disabled admission is a pass-through, and — explicitly
    documented in the test — enabled admission is too, flipping red when
    real enforcement lands.
  • tests/channels/test_reliable_channel_send_receive.nim updated for
    the dropped constructor param; the e2e roundtrips are unchanged.

Follow-up

Per-epoch budgeting, RLN proof attach at the transmission stage, and
RLN-rejection handling in the retry loop are already staged on
feat/rate-limit-enforcement (stacked on this branch, targets #3855).
Quota initialization/persistence is #3838.

🤖 Generated with Claude Code

Issue

closes #3953

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown

You can find the image built from this PR at

quay.io/wakuorg/nwaku-pr:4021

Built from 07c79ef

@stubbsta
stubbsta force-pushed the chore/move-ratelimitmanager-to-client-layer branch 4 times, most recently from f4f21ba to c1d4040 Compare July 16, 2026 08:05
@stubbsta
stubbsta marked this pull request as ready for review July 16, 2026 12:26
@stubbsta
stubbsta force-pushed the chore/move-ratelimitmanager-to-client-layer branch from c1d4040 to 9bf7e57 Compare July 16, 2026 12:40
stubbsta and others added 3 commits July 17, 2026 13:19
Collapses the outgoing pipeline to `segmentation -> sds -> encryption ->
dispatch` by folding the encrypt-and-dispatch tail of `onReadyToSend`
directly into `send()`. Removes the `RateLimitManager` field, its
constructor param, the `ReadyToSendEvent` listener, and the
`awaitingDispatch` accounting that only existed to bridge the event-bus
hop between `send()` and `onReadyToSend`.

The `rate_limit_manager.nim` module itself is untouched — it will be
relocated to the messaging layer (co-located with RLN) in a follow-up
commit, where per-epoch admission actually belongs.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Moves rate_limit_manager.nim from `logos_delivery/channels/` to
`logos_delivery/messaging/rate_limit_manager/` — the correct owner
now that admission sits alongside RLN in the messaging layer instead
of being fanned out to per-channel event listeners.

The API surface changes:

- `enqueueToSend` + `ReadyToSendEvent` (broker-based fan-out to a
  ReliableChannel listener) is replaced by `admit(msg): Future[Result[
  void, RateLimitError]]`. Callers now branch directly on the result
  instead of subscribing to an event.
- `channelId`, `SdsChannelID` and the SDS import are dropped — the
  messaging layer has no notion of channels; SDS was a channels-layer
  concern that only survived on this type because of the old broker
  fan-out.
- `brokerCtx` is dropped for the same reason.

Epoch config (`epochPeriodSec`), the wall-clock `currentEpochStart`,
`queue`, `dequeueReady`, and `resetEpoch` are preserved exactly as the
original owner designed them — this refactor is intentionally scoped
to the API surface, not the epoch mechanism.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wires the relocated RateLimitManager into the messaging layer at the
transmission stage rather than the API entry point:

- MessagingClientConf gains a rateLimit: RateLimitConfig field
  (defaulting to DefaultEpochPeriodSec / DefaultMessagesPerEpoch), and
  MessagingClient.new hands the constructed manager to SendService.
- SendService consults admit() before the first transmission of a
  task, both in send() and in the retry loop. Re-publishes of an
  already-propagated message (firstPropagatedTime set) skip admission:
  they resend the same bytes, which reuse the same RLN proof/nullifier
  and consume no fresh epoch slot.
- An over-budget task parks in the task cache as NextRoundRetry; the
  service loop re-admits it as the epoch budget frees up. The skeleton
  admit() is a pass-through, so behaviour is unchanged today.

Gating transmissions instead of MessagingClient.send keeps SDS repair
rebroadcasts free of API-entry rejection (SDS decides that a repair is
needed; the transmission scheduler decides when it fits the budget)
while every wire transmission still draws from one node-wide budget --
which network-side RLN enforcement applies to repairs regardless of
any local bypass. It is also where RLN proof attachment must happen,
since proofs bind to the epoch current at transmission time.

Adds tests/messaging/test_rate_limit_manager.nim covering the current
disabled + enabled pass-through behaviour, wired into
all_tests_waku.nim.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@stubbsta
stubbsta force-pushed the chore/move-ratelimitmanager-to-client-layer branch from 9bf7e57 to c7f767d Compare July 17, 2026 11:27

@Ivansete-status Ivansete-status left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great movement! Thanks! 🙌

## Spec-level entry point. Looks the channel up by id and delegates
## to `ReliableChannel.send`, which exposes the visible pipeline
## segmentation -> sds -> rate_limit_manager -> encryption.
## segmentation -> sds -> encryption -> dispatch.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
## segmentation -> sds -> encryption -> dispatch.
## segmentation -> sds -> encryption -> rate_limit_manager.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

move rate_limit_manager to messaging layer

2 participants