[N-02] Core Math Audit (#359)

ericnordelo · 0xNeshi · claude · web-flow · commit 230d0c2b0867 · 2026-06-10T14:40:28.000+02:00
* feat: Implement Rate Limiter module (#315) * feat: add rate limiter implementation * docs: document invariants * fix: enforce missing invariants * test: add more tests * feat: cooldown supports capacity/used * ref: assume monotonic Clock * fix: new_bucket accepts initial tokens (remove new_bucket_with_tokens) * ref: track cooldown_end instead of last_used * fix: cooldown used increases by 1 on consume * ref: use 'available' design instead of 'used' * ref: inline roll_window * test: add cooldown_reconfigure_rearms_when_drained_and_deadline_elapsed * docs: mention operator's responsibility for cooldown_ms * ref: rename q to steps_to_full in bucket_accrue * docs: update invariants * fix: remove capacity + refill_amount assertion in assert_bucket_config * docs: add missing / update docs * feat: add variety of errors * docs: add missing / update docs * test: remove invariant class mentions * ref: apply Code Quality Checklist (openzeppelin_utils) * test: replace cleanup with 'abort EUnreachable' in expected-failure tests * test: use abort 0 instead of error * feat: remove 'Ms' suffix from Error names * docs: error docs don't reference internal variables * docs: rephrase rate_limiter error messages without internal field names Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs: update README * docs: update README * docs: align invariants.md to be sharing-ready * docs: improve wording & clarity for invariants.md * feat: cooldown now decrements available by amount * test: change abort 0 to 100 (avoids confusing with ERateLimited) * test: change all destructor patterns into abortions * docs: invariant rewording * docs: update format for invariants.md * docs: update format for invariants.md and add missing invariants * docs: align invariants format * docs: update outdated invariant references * docs: remove rate limiter temp. artifacts * chore: simplify changelog * ref: convert assert macros into funs * docs: document private funs * test(fix): add proper teardown in happy tests * ci: add contracts/utils to CI test job dir matrix * test: add regression tests for bucket_accrue * fix: bucket_accrue double spend bug * docs: add empty line before enum variant docs Co-authored-by: Daniel Bigos <daniel.bigos96@gmail.com> * Revert "docs: add empty line before enum variant docs" This reverts commit 21a9076. * feat: inline config validation assertions * docs: add more info on rate limiter types in README * docs: add clarifying comments for apparent overflow suspects * docs: mention library being access control-agnostic * docs: reword library to module * feat: add initial_available to all limiter types * ref: put capacity as first cooldown type field * ref: use spread op instead of ignoring cooldown_ms field * docs: show identical API for all types in README * docs: for fixed window update wording that it resets to full capacity * fix: reconfigure sets last_refill to now + forbid 0 initial available for cooldown * docs: remove post-core artifact * test: add bucket_reconfigure_to_faster_rate_discards_old_subinterval * docs: add SAFETY before overflow-related comments * test: verify available always returns up-to-date accrual state * test: add missing zero-config reconfigure tests * test: add try_consume_with_zero_amount for all limiter types * test: add try_consume_of_available_aborts_when_drained * docs: note that try_consume(0) aborts after available() == 0 * docs: note RateLimiter enum is not upgrade-compatible Adding variants or fields to a deployed public enum would break BCS deserialization for integrator objects that already stored the prior shape. Document this constraint at the module level. * test: add edge case test cases * docs: remove llm artifacts * ref: reorder fileds in new_cooldown * test: remove Pub.local.toml * ref: in reconfigure_fixed_window move let now into match * ref: in reconfigure_bucket move let now into match * docs: remove mention of reconfigure_cooldown clamping available to 0 * fix: align cooldown clamp behavior on reconfigure * fix: try_consume updates state only on success * docs: align * ref: fixedwindow now shares bucket's internal logic * ref: update capacity prior to accrual on reconfigure_bucket * docs: add comment * Revert "docs: add comment" This reverts commit ecb0145. * Revert "ref: update capacity prior to accrual on reconfigure_bucket" This reverts commit b545b6d. * ref: Rate Limiter: Reconfiguration via Rich `new_*` Functions (#326) * impl * simplify further * fix: expose last_refill_ms for bucket * tests: add missing * docs: update utils README * docs: fix new_cooldown accepted combos * feat: project bucket-shaped anchor getters `last_refill_ms` and `window_start_ms` now take `&Clock` and return the anchor advanced by every whole interval elapsed since the last commit, so the snapshot `(anchor, available(clock))` is internally coherent. `cooldown_end_ms` stays a plain getter — a cooldown deadline does not evolve with time. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: error numbering * fix: error messages no longer reference private fields * docs: move EBucketAnchorInFuture comment next to the error * add spec * update spec * tests: add missing invariant tests * feat: add is_type discriminator getters --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Eric Nordelo <eric.nordelo39@gmail.com> Co-authored-by: Daniel Bigos <daniel.bigos96@gmail.com> * tests: fix formatting in rate limiter tests (#334) * feat: add median implementation (#135) * add separate quick_sort implementations for each type * remove duplicated qsort implementations * add in-place qsort implementation * add partition function * rewrite qsort without recursion * convert quick_sort to macro * add comprehensive test cases for qsort * update qsort documentation * add review fixes * remove --trace flag from testing pipeline * Revert "remove --trace flag from testing pipeline" This reverts commit 9bf7065. * split macro module * inline partition macro * add median implementation * add more edge case test cases * fix fmt * remove quick_sort duplicate * feat: add median * docs: update PR * Route median through u256 workhorse fun (#325) * fix: fmt * fix: apply CR comments * test: add missing test * fix: macro * feat: use quickselect * feat: improve CHANGELOG * feat: apply review updates * feat: update comment --------- Co-authored-by: Daniel Bigos <daniel.bigos@openzeppelin.com> Co-authored-by: Daniel Bigos <daniel.bigos@icloud.com> Co-authored-by: immrsd <103599616+immrsd@users.noreply.github.com> Co-authored-by: Eric Nordelo <eric.nordelo39@gmail.com> * feat: update check * feat: update version * feat: bump version * feat: apply improvements --------- Co-authored-by: Nenad <nenad.misic@openzeppelin.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Daniel Bigos <daniel.bigos96@gmail.com> Co-authored-by: Alisander Qoshqosh <37006439+qalisander@users.noreply.github.com> Co-authored-by: Daniel Bigos <daniel.bigos@openzeppelin.com> Co-authored-by: Daniel Bigos <daniel.bigos@icloud.com> Co-authored-by: immrsd <103599616+immrsd@users.noreply.github.com>
diff --git a/math/core/README.md b/math/core/README.md
@@ -21,7 +21,7 @@ Operations for `u8`, `u16`, `u32`, `u64`, `u128`, and `u256`, including:
 
 ### Vector operations
 
-Generic over `u8`..`u256`:
+Generic over `u8`..`u256`, with comparator-based sorting available for other types:
 
 - `vector::quick_sort!` / `vector::quick_sort_by!`: In-place iterative quicksort with three-way partitioning
 - `vector::median!`: Median of a borrowed unsigned integer vector with configurable rounding for even-length input; uses quickselect instead of sorting the full vector and aborts on empty input
diff --git a/math/core/sources/vector.move b/math/core/sources/vector.move
@@ -16,12 +16,13 @@ const EMedianOfEmptyVector: vector<u8> = "Median of empty vector is undefined";
 ///
 /// This macro implements the iterative quicksort algorithm with three-way partitioning
 /// (Dutch National Flag scheme), which efficiently sorts vectors in-place with `O(n log n)`
-/// average-case time complexity. The theoretical worst case is `O(n²)`, but median-of-three
-/// pivot selection makes it much less likely for common inputs, while three-way partitioning
-/// improves practical performance when duplicate elements are present.
+/// average-case time complexity. The theoretical worst case is `O(n²)` and remains reachable
+/// for adversarially ordered inputs despite median-of-three pivot selection. Three-way
+/// partitioning improves practical performance when duplicate elements are present.
 ///
-/// The macro uses an explicit stack to avoid recursion limitations for `Move` macros, making
-/// it suitable for arbitrarily large vectors.
+/// The macro uses an explicit stack to avoid recursion limitations for `Move` macros. Input
+/// size is still bounded by transaction gas and memory limits; avoid sorting unbounded vectors,
+/// especially when the caller can control the vector contents.
 ///
 /// #### Generics
 /// - `$Int`: Any unsigned integer type (`u8`, `u16`, `u32`, `u64`, `u128`, or `u256`).
@@ -31,6 +32,8 @@ const EMedianOfEmptyVector: vector<u8> = "Median of empty vector is undefined";
 ///
 /// #### Example
 /// ```move
+/// use openzeppelin_math::vector;
+///
 /// let mut vec = vector[3u64, 1, 4, 1, 5, 9, 2, 6];
 /// vector::quick_sort!(&mut vec);
 /// // vec is now [1, 1, 2, 3, 4, 5, 6, 9]
@@ -45,28 +48,35 @@ public macro fun quick_sort<$Int>($vec: &mut vector<$Int>) {
 ///
 /// This macro implements the iterative quicksort algorithm with three-way partitioning
 /// (Dutch National Flag scheme), which efficiently sorts vectors in-place with `O(n log n)`
-/// average-case time complexity. The theoretical worst case is `O(n²)`, but median-of-three
-/// pivot selection makes it much less likely for common inputs, while three-way partitioning
-/// improves practical performance when duplicate elements are present. Using an incorrect
-/// comparator (see `$le` below) can also degrade performance.
+/// average-case time complexity. The theoretical worst case is `O(n²)` and remains reachable
+/// for adversarially ordered inputs despite median-of-three pivot selection. Three-way
+/// partitioning improves practical performance when duplicate elements are present. Using an
+/// incorrect comparator (see `$le` below) can also degrade performance or produce unsorted
+/// output.
 ///
-/// The macro uses an explicit stack to avoid recursion limitations for `Move` macros, making
-/// it suitable for arbitrarily large vectors.
+/// The macro uses an explicit stack to avoid recursion limitations for `Move` macros. Input
+/// size is still bounded by transaction gas and memory limits; avoid sorting unbounded vectors,
+/// especially when the caller can control the vector contents.
 ///
 /// #### Generics
 /// - `$T`: Any type that can be compared using the provided comparison function.
 ///
 /// #### Parameters
 /// - `$vec`: A mutable reference to the vector to be sorted in-place.
 /// - `$le`: A comparison function that takes two references and returns `true` if the first
-///   element should be ordered before or equal to the second element. **Must implement
-///   non-strict ordering** (i.e., `<=` for ascending, `>=` for descending). Using a strict
-///   comparator (e.g., `<` instead of `<=`) defeats three-way partitioning, which can degrade
-///   performance to `O(n²)` when duplicate elements are present. Always use non-strict
-///   operators to ensure optimal behavior.
+///   element should be ordered before or equal to the second element. **Must implement a
+///   consistent total preorder** (i.e., non-strict `<=` for ascending, `>=` for descending).
+///   Using a strict comparator (e.g., `<` instead of `<=`) defeats three-way partitioning,
+///   which can degrade performance to `O(n²)` when duplicate elements are present. An
+///   inconsistent, non-transitive, or non-total comparator is not detected and can silently
+///   return a vector that is not sorted according to the intended order. The macro may invoke
+///   `$le` twice for a single element probe to distinguish equality from before/after ordering,
+///   so account for that when the comparator is expensive.
 ///
 /// #### Example
 /// ```move
+/// use openzeppelin_math::vector;
+///
 /// // Sort in ascending order
 /// let mut vec = vector[3u64, 1, 4, 1, 5, 9, 2, 6];
 /// vector::quick_sort_by!(&mut vec, |x: &u64, y: &u64| *x <= *y);
@@ -214,8 +224,10 @@ public macro fun quick_sort_by<$T>($vec: &mut vector<$T>, $le: |&$T, &$T| -> boo
 /// The input vector is not mutated.
 ///
 /// Median selection uses quickselect over a `u256` working vector instead of
-/// sorting the full input. Benchmarks show similar cost for tiny vectors and
-/// substantially lower cost than sorting as input size grows.
+/// sorting the full input. Average-case time complexity is `O(n)`, with `O(n)` extra storage
+/// for the working vector. The theoretical worst case is `O(n²)` and remains reachable for
+/// adversarially ordered inputs despite median-of-three pivot selection. Avoid computing the
+/// median of unbounded vectors, especially when the caller can control the vector contents.
 ///
 /// #### Generics
 /// - `$Int`: Any unsigned integer type (`u8`, `u16`, `u32`, `u64`, `u128`, or `u256`).
@@ -252,8 +264,10 @@ public macro fun median<$Int>($vec: &vector<$Int>, $rounding_mode: RoundingMode)
 /// This function consumes and partially reorders `vec`.
 ///
 /// Median selection uses quickselect instead of sorting the full input.
-/// Benchmarks show similar cost for tiny vectors and substantially lower cost
-/// than sorting as input size grows.
+/// Average-case time complexity is `O(n)`. The theoretical worst case is `O(n²)` and remains
+/// reachable for adversarially ordered inputs despite median-of-three pivot selection. Avoid
+/// computing the median of unbounded vectors, especially when the caller can control the vector
+/// contents.
 ///
 /// #### Parameters
 /// - `vec`: Vector whose median is desired (consumed).
