mirror of
https://github.com/logos-messaging/logos-messaging-nim.git
synced 2026-07-30 12:03:30 +00:00
c873f74b4b
* feat: enforce per-epoch budget in RateLimitManager.admit Replaces the pass-through skeleton with a lazily rolled fixed window: admit() charges one message against the current epoch, resets the counter once epochPeriodSec has elapsed, and rejects with OverBudget when messagesPerEpoch is exhausted. Disabled or non-positive configurations admit everything, so the default-constructed MessagingClientConf (enabled = false) keeps today's behaviour. Parking of over-budget messages stays with the SendService scheduler (NextRoundRetry); the manager only answers whether one more transmission fits. The queue / dequeueReady stubs that anticipated manager-side parking are removed accordingly. Extends tests/messaging/test_rate_limit_manager.nim with budget boundary, epoch rollover, resetEpoch, and degenerate-config cases, replacing the enabled-pass-through placeholder test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat: split rate limit manager into config, quota source, and enforcement modules Decomposes logos_delivery/messaging/rate_limit_manager/ into three modules with one responsibility each: - rate_limit_config: the configuration vocabulary (RateLimitConfig, RateLimitError, defaults, isEnforcing). The API conf layer now imports this alone instead of the enforcement engine. - quota_source: the RLN seam. A single QuotaProvider callback returns EpochQuota (epoch index + user message limit) so the two are read atomically and a read cannot straddle an epoch boundary. Returns none when RLN is unavailable, selecting the wall-clock fallback. The callback shape keeps the manager free of any dependency on the Waku kernel; the RLN-backed provider is built one layer up. - rate_limit_manager: enforcement only; re-exports the other two so existing single-import call sites are unchanged. Config field names now mirror RLN exactly, per the requirement that the rate limit share RLN Relay's format: epochSizeSec (was epochPeriodSec) and userMessageLimit (was messagesPerEpoch), both uint64 to match RlnConf. Renaming is contained to this branch: the config type moved into the new rate_limit_config module here. admit() now works in epoch-index terms: the epoch comes from the provider when set (RLN's calcEpoch value), else from an absolute wall-clock window (unixTime div epochSizeSec — absolute rather than anchored at first use, matching RLN's derivation). The effective limit is min(config.userMessageLimit, RLN's) — RLN can only tighten the configured limit, since exceeding it would fail at proof generation once the epoch's message ids are exhausted. With no provider wired (this commit), behaviour is the wall-clock window as before; RLN-backed provider wiring follows separately. Tests rewritten against injected fake providers: limit boundary, epoch rollover without sleeps, RLN-clamps-config, config-tightens-below-RLN, and the wall-clock fallback (7 tests). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: charge admission once per task and exempt budget-parked tasks from the reaper Two defects the send-service seam had once admission actually enforces, both fixed by a single DeliveryTask.firstAdmittedTime field: - The retry loop gated admission on firstPropagatedTime.isNone(), so a task that failed to propagate (e.g. no peers) was re-admitted on every 1s tick, re-charging the epoch budget and — with the shipped default of one message per epoch — starving all other traffic. Admission is now gated on firstAdmittedTime.isNone(): a task charges one slot / draws one nonce for its lifetime, and retries reuse it. - reportTaskResult reaped any never-propagated task older than MaxTimeInCache (60s) measured from message creation, so an over-budget task parked for a 600s epoch was hard-failed with a misleading "Unable to send within retry time window" long before the epoch could roll (issue #4049). The reaper now runs only for admitted tasks and measures from admission, so a task waiting for epoch budget is exempt and gets a full delivery window once it is finally admitted. The RLN-rejection branch in both processors resets firstAdmittedTime along with clearing the proof, so the regenerated proof's fresh nonce is re-admitted rather than sent uncharged. The reap decision is extracted to DeliveryTask.isDeliveryTimedOut and unit-tested (tests/messaging/test_delivery_task_reaping.nim); a scheduler-level integration test of park-and-release is a follow-up. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat: source the rate limit manager's epoch and budget from RLN Wires the quota seam to its producer, so enforcement tracks RLN rather than only the wall clock. - Waku.currentRlnEpochQuota (waku/api/publish) reads RLN's current epoch index and the epoch's user message limit together, returning none when RLN is not mounted (or its limit is unset). - MessagingClient.new builds a QuotaProvider closure over that accessor and hands it to the RateLimitManager. The closure is late-binding: it queries the kernel on each admission, so a node whose RLN mounts after construction upgrades from the wall-clock fallback to RLN's epoch and limit automatically, with no reconstruction. With this, admit() rolls its window on RLN's epoch and clamps the configured cap to RLN's user message limit; without RLN it still falls back to the absolute wall-clock window and the configured limit. Also switches the quota seam from std/options to results `Opt`, matching the kernel surface it now bridges (`groupManager.userMessageLimit` is `Opt`) and the rest of the messaging layer post-Opt migration. Tests: currentRlnEpochQuota is none unmounted and reports epoch + the configured userMessageLimit when mounted (anvil-backed). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: cover admit-once and park/release at the send service scheduler The rate limit manager's budget logic and the delivery-timeout reaper are unit-covered, but the send service's use of them across the service loop was not. Drive the scheduler a tick at a time against a scripted fake processor and a fixed-epoch quota provider — no network, no sleeps — asserting: - a task is charged against the budget exactly once however many rounds delivery takes (firstAdmittedTime guards re-admission); - an over-budget task parks as NextRoundRetry without reaching the processor, then is admitted and delivered on the first tick after the epoch rolls. Two testability seams keep this deterministic without a live relay: an optional sendProcessor override on SendService.new injects the fake, and trySendMessages is exported to drive one loop tick. The task is built directly (like test_delivery_task_reaping) since DeliveryTask.new resolves its shard through a broker provider only registered once the node starts. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs: note rateLimit config is settable only programmatically MessagingClientConf.rateLimit cannot be set through the JSON config or a CLI flag: it carries no name pragma, and RateLimitConfig is a nested object with no parseCmdArg, so applyJsonFieldsToConf rejects it with "cannot be set via JSON". Record the limitation and the fix path on the field. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor: consolidate the admission gate into admitOnce send() and the retry loop duplicated the admit-then-stamp-firstAdmittedTime sequence; both now call SendService.admitOnce, keeping the charge-once invariant in one place. No behavior change; messaging tests pass (14/14). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * use explicit return statement * Make comments mroe concise * refactor: give each rate-limit config field a single responsibility `isEnforcing` folded three fields into one "should I enforce" answer, so a zeroed `messagesPerEpoch` or `epochPeriodSec` silently disabled an enabled config. Split the responsibilities: - `enabled` alone gates enforcement; `admit` reads it directly and `isEnforcing` is removed. - a zero `messagesPerEpoch` now means what it says — admit nothing — which `admit` already yields (`0 >= 0` -> OverBudget), no special case. - `RateLimitManager.new` returns a Result and rejects an enabled config with `epochPeriodSec == 0`, the only value that could crash the wall-clock fallback (`unixTime div epochPeriodSec`). Callers thread the Result through; a zero-init `RateLimitConfig` stays valid (disabled), so default construction is unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
37 lines
1.6 KiB
Nim
37 lines
1.6 KiB
Nim
{.used.}
|
|
|
|
import results, chronos, testutils/unittests
|
|
|
|
import logos_delivery/messaging/delivery_service/send_service/delivery_task
|
|
|
|
const MaxTime = chronos.minutes(1)
|
|
|
|
proc taskWith(admitted, propagated: Opt[Moment]): DeliveryTask =
|
|
## Builds a DeliveryTask directly (bypassing `new`, which needs a broker).
|
|
return DeliveryTask(firstAdmittedTime: admitted, firstPropagatedTime: propagated)
|
|
|
|
suite "DeliveryTask - delivery-timeout reaping":
|
|
test "a task parked for budget (never admitted) is exempt, however old":
|
|
## Budget-parked tasks must survive to the epoch roll, not be aged out.
|
|
let task = taskWith(Opt.none(Moment), Opt.none(Moment))
|
|
check not task.isDeliveryTimedOut(MaxTime)
|
|
|
|
test "an admitted, never-propagated task past the window times out":
|
|
let task = taskWith(Opt.some(Moment.now() - chronos.minutes(2)), Opt.none(Moment))
|
|
check task.isDeliveryTimedOut(MaxTime)
|
|
|
|
test "an admitted task still within the window does not time out":
|
|
let task = taskWith(Opt.some(Moment.now()), Opt.none(Moment))
|
|
check not task.isDeliveryTimedOut(MaxTime)
|
|
|
|
test "a propagated task is never timed out here (store validation owns it)":
|
|
let task =
|
|
taskWith(Opt.some(Moment.now() - chronos.minutes(2)), Opt.some(Moment.now()))
|
|
check not task.isDeliveryTimedOut(MaxTime)
|
|
|
|
test "the timeout clock runs from admission, not message creation":
|
|
## A just-admitted task has a fresh clock, even after a long budget wait.
|
|
let task = taskWith(Opt.some(Moment.now()), Opt.none(Moment))
|
|
check task.admissionAge() < MaxTime
|
|
check not task.isDeliveryTimedOut(MaxTime)
|