[N-04] Core Math Audit

[ericnordelo]

Rationale: inlined median! macro + precompiled wrapper functions

Approach. We benchmarked eight median implementations with sui move test -s csv
(deterministic gas; inputs spanning 100–5000 elements; ascending, reverse-sorted, and
duplicate-heavy patterns), measuring both execution gas and compiled bytecode per call site.

What the benchmarks showed

• Pivot caching dominates: reading vec[pivot_index] on every comparison was the single
  largest cost; caching it in a local cut the u64 selection ~79%. An index-vector quickselect
  variant only looked competitive because it incidentally did this — compared fairly, plain
  value-based quickselect wins at every width.
• Widening is the macro's bottleneck: a generic median! that converts to u256 and
  delegates to one function pays the widening on every narrow-width call. Move macros cannot
  dispatch on $Int to width-specific functions (type errors at expansion), so the only way to
  run selection at native width through one generic entry is to inline the whole algorithm in
  the macro and let expansion monomorphize it.
• Cloning via dereference: *vec copies the whole vector in one instruction (~1 gas/KB) vs
  map_ref!'s per-element loop — worth ~20% end-to-end on its own.

Constraints that shaped the design

A public macro expands in the caller's module, so it may only reference public symbols — it
cannot use private helper macros or the module's private error constant. The empty-input abort
is therefore routed through median_u8, which validates before expanding the macro, preserving
the canonical EMedianOfEmptyVector everywhere. Inlining also removes the need for any engine
function, so every public entry takes an immutable reference — nothing is consumed or mutated.

The trade-off, and why wrappers exist

Each median! call site adds ~750–870 B of compiled bytecode (comparable to the existing
quick_sort! at ~750 B), which counts against Sui's package-size cap. The
median_u8…median_u256 wrappers precompile the same expansion once in the library, costing
call sites only a regular function call — measured at identical gas to within a few units.
Users pick per call site; the docs explain the trade-off.

Results

vs the previous implementation (n=1001, normalized gas):

Path	Before	After	Δ
median! u64	91.5k	6.9k	−92%
median! u128	95.5k	7.9k	−92%
median! u256	103.5k	46.3k	−55%
median! u64, sorted input (n=5001)	120.6k	8.0k	−93%

Validated by 775 tests including randomized cross-checks against a sorted reference, oracle-free
property tests, and an external consumer package proving cross-package macro expansion and
abort-code behavior.

One notable issue surfaced and fixed along the way: with the algorithm inlined, ~120 macro call
sites in our own test suite exceeded the Move VM's 10 MB package arena
(PACKAGE_ARENA_LIMIT_REACHED) — behavioral coverage now runs through the wrappers, with a
compact per-width macro section (the same guidance downstream test suites should follow).

Summary by CodeRabbit

• New Features

  • Introduced precompiled vector median wrapper functions for types u8, u16, u32, u64, u128, and u256 with optimized library-level bytecode compilation.
  • Updated median_u256 to accept non-mutating references for improved efficiency.

• Documentation

  • Enhanced vector documentation with median wrapper function details and performance trade-off information.

[coderabbitai]

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3012c095-6839-4bf6-84b9-2bf92dd1943f

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/core-math-N-04

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov]

Codecov Report

❌ Patch coverage is 0% with 29 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.90%. Comparing base (dc216cb) to head (6442ba3).
⚠️ Report is 1 commits behind head on release-v1.3.

Files with missing lines	Patch %	Lines
math/core/sources/vector.move	0.00%	29 Missing ⚠️

❌ Your patch status has failed because the patch coverage (0.00%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

@@               Coverage Diff                @@
##           release-v1.3     #363      +/-   ##
================================================
+ Coverage         78.42%   78.90%   +0.47%     
================================================
  Files                23       23              
  Lines              1789     1782       -7     
  Branches            656      640      -16     
================================================
+ Hits               1403     1406       +3     
+ Misses              351      341      -10     
  Partials             35       35              

Flag	Coverage Δ
contracts/access	65.40% <0.00%> (ø)
contracts/utils	44.09% <0.00%> (ø)
math/fixed_point	56.59% <0.00%> (+0.93%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Copilot]

Pull request overview

This PR implements the “[N-04] Core Math Audit” changes by inlining the median! selection algorithm (for gas efficiency) and introducing precompiled, per-width median_u8…median_u256 wrappers (to avoid repeated bytecode bloat at call sites). It also expands property-based test coverage around quicksort/median and updates documentation and CI to a newer Sui CLI version.

Changes:

• Reworked vector::median! to inline native-width quickselect (incl. pivot caching + vector copy via deref) and added median_u8…median_u256 wrapper functions.
• Expanded macro and wrapper test coverage with additional randomized/property tests.
• Updated documentation and CI configuration (Sui CLI version bump, API docs updates).

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
README.md	Bumps required Sui CLI version (needs link fix).
.github/workflows/test.yml	Updates CI SUI_VERSION to 1.72.5.
math/core/sources/vector.move	Inlines median! algorithm and adds median_u8…median_u256 wrappers; expands docs.
math/core/tests/macros/median.move	Refactors tests to exercise wrappers heavily; adds macro-vs-wrapper cross-checks and additional properties.
math/core/tests/macros/quick_sort.move	Adds randomized property test for sort correctness + permutation preservation.
math/core/README.md	Updates public API documentation for vector ops (needs macro ! notation fix).
CHANGELOG.md	Tweaks unreleased changelog entry for median (should mention new wrappers).
math/core/sources/internal/macros.move	Renames MAX_LOG_10 → MAX_LOG10 and updates log10 helper docs.
math/core/sources/u8.move	Adds BIT_WIDTH constant docstring.
math/core/sources/u16.move	Adds BIT_WIDTH constant docstring.
math/core/sources/u32.move	Adds BIT_WIDTH constant docstring.
math/core/sources/u64.move	Adds BIT_WIDTH constant docstring.
math/core/sources/u128.move	Adds BIT_WIDTH constant docstring.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

[bidzyyys]

LGTM

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.