Reconfigure policy#329
Closed
0xNeshi wants to merge 7 commits into
Closed
Conversation
Each `reconfigure_*` function now accepts a per-variant policy enum that selects in-flight state handling: - BucketReconfigurePolicy: ProjectAndReanchor (current), InstallOnly, Reset, PreservePhase, Proportional - FixedWindowReconfigurePolicy: ProjectAndReanchor (current), InstallOnly, Reset, Proportional - CooldownReconfigurePolicy: ProjectAndReanchor (current), InstallOnly, Reset, PreserveActiveGate, RebaseActiveGate, RebaseActiveGateNoExtend, Proportional Public constructor functions expose each variant to external callers.
This reverts commit 10a744c.
This reverts commit 5f1a1be.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the rate_limiter utility to support richer, policy-driven reconfiguration semantics and state reconstruction, while expanding test coverage to pin down the new behaviors.
Changes:
- Introduces explicit reconfiguration policy enums (Bucket/FixedWindow/Cooldown) and updates
reconfigure_*APIs to accept a per-call policy. - Expands constructors to accept/validate explicit anchor timestamps (
last_refill_ms,window_start_ms,cooldown_end_ms) and adds new invariants + error codes. - Adds getter APIs and significantly expands unit tests to cover new constructor invariants, policy behaviors, and “construct-fresh” reconfiguration patterns.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| contracts/utils/sources/rate_limiter.move | Adds policy enums + constructors, extends constructors with anchors, adds getters, and refactors reconfigure_* to be policy-driven. |
| contracts/utils/tests/rate_limiter_tests.move | Updates existing tests for new signatures and adds extensive new coverage for policies, anchors, getters, and reconstruct-by-construction flows. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+401
to
+403
| /// Two configurations are valid: | ||
| /// - greenfield: `initial_available > 0` with `cooldown_end_ms <= now` (typically `0`). The | ||
| /// gate is not armed; up to `initial_available` units can be consumed before the first arm. |
Comment on lines
+787
to
+795
| // intervals approach u64::MAX. The result is `<= refill_interval_ms` | ||
| // so the cast back to u64 is safe. | ||
| let elapsed_old = (now - new_last) as u128; | ||
| let elapsed_new = | ||
| elapsed_old * (refill_interval_ms as u128) / (old_interval as u128); | ||
| *cap_field = capacity; | ||
| *refill_amount_field = refill_amount; | ||
| *refill_interval_field = refill_interval_ms; | ||
| *last_refill_ms = now - (elapsed_new as u64); |
Comment on lines
+875
to
+879
| // SAFETY: u128 widening keeps the product in range. The result | ||
| // is `<= window_ms` so the cast back to u64 is safe. | ||
| let elapsed_old = (now - *window_start_ms) as u128; | ||
| let elapsed_new = elapsed_old * (window_ms as u128) / (old_window as u128); | ||
| *window_start_ms = now - (elapsed_new as u64); |
Comment on lines
+963
to
+967
| if (*available == 0) { | ||
| // SAFETY: see safety notes in `RebaseActiveGateNoExtend`. | ||
| let arm_time = *cooldown_end_ms - *cd_field; | ||
| let new_deadline = arm_time + cooldown_ms; | ||
| if (new_deadline <= now) { |
Both branches independently introduced rich `new_*` constructors and field getters on top of 07f7fb7. `reconfigure-policy` additionally kept the per-limiter policy-enum `reconfigure_*` path. The merge resolves to HEAD's superset (policy reconfigure + rich constructors + getters) and drops the rich-new-simpler module-doc paragraph claiming the module deliberately omits reconfigure functions, which no longer holds. EBucketAnchorInFuture stays at code 11 (HEAD), not 12. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
| const ECooldownArmedWithTokens: vector<u8> = | ||
| "Armed cooldown_end_ms is incompatible with initial_available > 0"; | ||
| /// `Bucket` refill anchor strictly in the future would underflow the next projection. | ||
| #[error(code = 11)] |
Comment on lines
+724
to
+735
| assert!(capacity > 0, EZeroCapacity); | ||
| assert!(refill_amount > 0, EZeroRefillAmount); | ||
| assert!(refill_interval_ms > 0, EZeroRefillInterval); | ||
|
|
||
| let now = clock.timestamp_ms(); | ||
| match (policy) { | ||
| BucketReconfigurePolicy::InstallOnly => { | ||
| *cap_field = capacity; | ||
| *refill_amount_field = refill_amount; | ||
| *refill_interval_field = refill_interval_ms; | ||
| *available = (*available).min(capacity); | ||
| }, |
Comment on lines
+833
to
+842
| assert!(capacity > 0, EZeroCapacity); | ||
| assert!(window_ms > 0, EZeroWindow); | ||
|
|
||
| let now = clock.timestamp_ms(); | ||
| match (policy) { | ||
| FixedWindowReconfigurePolicy::InstallOnly => { | ||
| *cap_field = capacity; | ||
| *window_field = window_ms; | ||
| *available = (*available).min(capacity); | ||
| }, |
Comment on lines
+918
to
+927
| assert!(capacity > 0, EZeroCapacity); | ||
| assert!(cooldown_ms > 0, EZeroCooldown); | ||
|
|
||
| let now = clock.timestamp_ms(); | ||
| match (policy) { | ||
| CooldownReconfigurePolicy::InstallOnly => { | ||
| *cap_field = capacity; | ||
| *cd_field = cooldown_ms; | ||
| *available = (*available).min(capacity); | ||
| }, |
Collaborator
Author
|
Closing in favor of #326 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #???
PR Checklist