Repository navigation
feat(id): configurable abuse budgets and per-counterparty overrides (#2247) - #4338
Conversation
…2247) P1 of #2032/#2033 on top of the merged P0-A integration (#3882, PR #3960): - abuse.rs: BudgetPolicy with rolling-window (sliding-window approximation) BudgetLimits at app-wide inbound/outbound, per-endpoint, and per-direct-IP scope, plus counterparty override rows that replace the per-endpoint defaults for one peer. Defaults are the #2247 values (app in 32 MiB/s + 20 GiB/h, out 64 MiB/s + 40 GiB/h, per peer 100 MiB/h burst 8 MiB/min, per IP 250 MiB/h). AbuseEvaluator::with_budgets / check_budget / check_frame_and_bytes_budgeted charge atomically with the token buckets; refusals are the shared BudgetExhausted Refusal with a bounded retry. budget_usage() is the observability surface and prune_budget_state() bounds retained state. Evaluators without a configured budget policy behave exactly as before. - frames.rs: charge_frame_budgeted seam; no IP is observed at the frame seam, so the per-IP scope is only charged by callers that directly observe an IP - a relayed peer IP is never inferred. - world_net.rs: WorldProtocol enforces BudgetPolicy::default() (or with_budgets(...)) at its inbound/outbound frame seams; budget refusals ride the existing backpressure/stream-shed paths, never a connection close. - server_config: [budgets] table in id-server-config.toml with per-knob overrides of the code defaults and [[budgets.counterparty]] rows (peer = declared user key or 64-hex public key), validated (zero values and duplicate peers refused). ServerConfig::budgets() returns the typed BudgetPolicy. Tests: budget defaults, per-peer exhaustion + bounded retry, override precedence both directions, message caps, direct-IP sharing vs relayed skip, app-wide scope/direction separation, window-roll recovery, atomic budgeted frame charge, unconfigured evaluator, and config parsing/defaults/errors.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 42 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f25156afae
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let retry_after_ms = match abuse.lock() { | ||
| Ok(mut abuse) => match charge_frame( | ||
| Ok(mut abuse) => match crate::frames::charge_frame_budgeted( | ||
| &mut abuse, | ||
| endpoint, | ||
| is_authorized, | ||
| payload_len, | ||
| Direction::Inbound, |
There was a problem hiding this comment.
On a fast Iroh module upload, the newly enforced default per-peer budget exhausts its 8 MiB/min allowance after roughly 4 MiB of hex-encoded Wasm. charge_inbound_frame then waits for the budget to recover, but install_module_session in pkgs/id/src/world_session/mod.rs:1909–1911 still times out each io.recv() after 30 seconds. The budget-induced pause can exceed that deadline, so otherwise valid uploads below the supported 16 MiB maximum fail with timed out receiving module. Coordinate the upload deadline with limiter-induced backpressure so enabling these defaults does not break supported uploads.
Useful? React with 👍 / 👎.
| BudgetUsage { | ||
| bytes: counter.estimate_bytes(now_ms), | ||
| messages: counter.estimate_messages(now_ms), |
There was a problem hiding this comment.
When a scope has received no further charges, budget_usage(now_ms) estimates its counters without first rolling them to now_ms. estimate_bytes and estimate_messages always include the stored current-window counts, so those charges remain visible even after both windows have expired. For example, charging 100 bytes at time zero and requesting usage two hours later still reports 100 bytes for every default window. This also affects WorldProtocol::budget_usage(), making idle scopes appear to retain consumed budget. Roll a copy of each counter before calculating the snapshot.
Useful? React with 👍 / 👎.
Implements #2247 (P1 of #2032/#2033), on top of the merged P0-A integration (#3882, PR #3960 — verified merged 2026-11-02T06:35:09Z before starting; issue #2244's integration is on main via that PR).
What
BudgetPolicy: rolling-window budgets (sliding-window approximation, O(1) state per scope) at app-wide inbound/outbound, per-endpoint, and per-direct-IP scope, plus per-counterparty override rows that replace the per-endpoint defaults for one specific peer (cap or raise one peer without touching the global budget). Defaults are the id(comms): budgets — app-wide, per-IP, per-ID, and per-counterparty overrides (P1 of #2032) #2247 values: app in 32 MiB/s + 20 GiB/h, out 64 MiB/s + 40 GiB/h; per peer 100 MiB/h, burst 8 MiB/min; per IP 250 MiB/h.AbuseEvaluator::with_budgets/check_budget/check_frame_and_bytes_budgeted— atomic with the existing token buckets (nothing is charged unless every check admits). Refusals are the sharedBudgetExhaustedRefusalwith a boundedretry_after_ms, never a disconnect.budget_usage()is the observability surface;prune_budget_state()bounds retained state. Evaluators with no budget policy configured behave exactly as before (world chat included).charge_frame_budgetedseam. The frame seam observes no IP, so the per-IP scope is charged only by callers that directly observe an IP — a relayed peer's IP is never inferred.WorldProtocolenforcesBudgetPolicy::default()(orwith_budgets(...)) at its inbound/outbound frame seams; budget refusals ride the existing backpressure / stream-shed paths — the peer connection stays up.WorldProtocol::budget_usage()exposes the counters.[budgets]inid-server-config.toml: per-knob overrides of the code defaults plus[[budgets.counterparty]]rows (peer= a declared user key or a 64-hex public key,bytes_per_window,window_seconds, optionalmessages_per_window). Zero values and duplicate peers are refused at parse time.ServerConfig::budgets()returns the typedBudgetPolicy.Known remainder (honest scope note)
ServerConfigas a whole is parsed-but-not-yet-loaded by theservecommand (pre-existing gap noted in the 2026-10-29 comms investigation, it affects every table, not just budgets). This PR wires budgets into the serving protocol object with code defaults and provides the parsed policy accessor; loading the config file incmd_serveis the remaining step for the whole config, best done once for all tables rather than budgets-only.Tests
New unit tests (CI runs them): budget defaults match #2247; per-peer exhaustion + bounded retry + refused charges not recorded; override precedence in both directions; message caps; direct-IP sharing across endpoints vs relayed traffic skipping the IP scope; app-wide scope spanning peers with direction separation; window-roll recovery + pruning; atomic budgeted frame charge; unconfigured evaluator unchanged; config defaults/partial override/counterparty resolution (user key + raw hex)/duplicate, zero, and unknown-peer refusals.
Queue: task 5936 (repo-health-code-pilot), lane C. PR #3834 was not touched.
Need help on this PR? Tag
@codesmith-botwith what you need.