test: fuzz partial maximization in maximize/maximizeFull#238
Conversation
The existing maximize tests only ever exercise the fully-maximized case: they all bound the exponent to [EXPONENT_MIN, EXPONENT_MAX] and call `maximizeFull`, which reverts whenever the result is only partially maximized. The `maximize` function's `full == false` branch (returned when the exponent is so close to `type(int256).min` that the coefficient cannot be pushed to magnitude >= 1e75 without underflowing the exponent) was never covered. Add tests that call `maximize` directly in the partial region: - testMaximizePartialExamples: explicit floor-region inputs return the expected best-effort coefficient/exponent with full == false, and `maximizeFull` reverts with MaximizeOverflow on exactly those inputs. - testMaximizePartialBoundary: 1e74 is partial, 1e75 (exponent min + 75) is the first full point, with `maximize`/`maximizeFull` agreeing once full. - testMaximizePartialFuzz: invariants over the partial region — full is false, result is below the 1e75 threshold, stays non-zero, exponent is pinned to the floor, and the represented value is preserved. - testMaximizePartialIdempotent: re-maximizing a partial result is a no-op and stays partial. Verified the new tests kill a mutant that forces `full` to always be true, whereas the pre-existing tests do not. Closes #134 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 56 minutes and 33 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
…emes Adds adversarial, mutation-validated tests for `maximize` / `maximizeFull` that target the type boundaries the existing suite avoids (it bounded the exponent to [EXPONENT_MIN, EXPONENT_MAX] and only ever exercised the full case of `maximizeFull`): - An independent oracle (`maximizeOracle`) that greedily applies one OOM at a time, overflow-safely, and respects the `type(int256).min` exponent floor. Unlike `maximizeSlow` it is correct right at the floor, so it can be checked across the *entire* int256 coefficient/exponent domain, including the type(int256).min / type(int256).max corners and exponents far below EXPONENT_MIN. - `testMaximizeOracleFullDomain` / `testMaximizeOracleBoundaryCorners`: exact (coefficient, exponent, full) agreement, with corners hammered onto int224/int32/int256 boundaries. - `testMaximizeStaircaseBoundarySweep`: a dense, deterministic sweep over every exponent headroom 0..80 above the floor, killing per-step off-by-one mutants in the staircase exponent bounds without relying on fuzz luck. - `testMaximizeValuePreservedFullDomain` + `testMaximizeZeroCanonicalizes`: value preservation (coefficient_out == coefficient_in * 10^shift with shift in [0,76]) over the full domain, plus the zero-canonicalization special case. - `testMaximizeFullRevertsIffPartial`: `maximizeFull` reverts with MaximizeOverflow exactly on the partial inputs and returns the maximized pair otherwise, across boundary corners that actually produce partial results. - `testMaximizeOneMoreOomSignAsymmetry`: pins the positive/negative tail one-more-OOM step (the int256.min vs int256.max magnitude asymmetry) and the no-headroom floor case. - `testMaximizePartialBoundaryCoefficients`: partial-region fuzz reaching the int224 coefficient boundary instead of capping at 1e10. Each new test was mutation-validated: it fails (kills the mutant) when the relevant production line is flipped/dropped (tail-step guard, full predicate 1e75->1e76, staircase exponent bounds +N->+(N-1), step threshold 1e57->1e58, and the maximizeFull revert), and passes on the unmutated code. No production change: `maximize`/`maximizeFull` are correct at all probed boundaries. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Reviewed 2065ad4: fuzz the partial (full==false) maximize/maximizeFull branch near int256.min; mutation-validated discriminating test (not enshrining). All checks green. LGTM. |
|
@coderabbitai assess this PR size classification for the totality of the PR with the following criterias and report it in your comment: S/M/L PR Classification Guidelines:This guide helps classify merged pull requests by effort and complexity rather than just line count. The goal is to assess the difficulty and scope of changes after they have been completed. Small (S)Characteristics:
Review Effort: Would have taken 5-10 minutes Examples:
Medium (M)Characteristics:
Review Effort: Would have taken 15-30 minutes Examples:
Large (L)Characteristics:
Review Effort: Would have taken 45+ minutes Examples:
Additional Factors to ConsiderWhen deciding between sizes, also consider:
Notes:
|
1 participant
Fixes #134.
The issue asks to cover cases where maximization is partial — the existing maximize tests only ever cover the full case.
Why the partial case was uncovered
LibDecimalFloatImplementation.maximizereturns a thirdfullbool that isfalsewhen the exponent is so close totype(int256).minthat the coefficient cannot be pushed up to magnitude>= 1e75without underflowing the exponent. Every pre-existing test bounds the exponent to[EXPONENT_MIN, EXPONENT_MAX]and callsmaximizeFull(which reverts on a partial result), so thefull == falsepath was never executed.What this adds (test-only)
testMaximizePartialExamples— explicit floor-region inputs (e.g.(1, type(int256).min),(1, min+74),(99e72, min)) return the expected best-effort coefficient/exponent withfull == false, andmaximizeFullreverts withMaximizeOverflowon exactly those inputs (via an external wrapper sovm.expectReverthas a call boundary).testMaximizePartialBoundary—1e74is partial;1e75at exponentmin+75is the first full point;maximize/maximizeFullagree once full.testMaximizePartialFuzz— invariants wheneverfull == false: result below the 1e75 threshold, stays non-zero, exponent pinned to the floor, value preserved (coefficient * 10^exponentunchanged).testMaximizePartialIdempotent— re-maximizing a partial result is a no-op and stays partial.Verification
LibDecimalFloatDeployProdfork tests that need*_RPC_URLenv vars (missing in this environment); they fail identically on cleanorigin/main.maximizeto always returnfull = truemakes all 4 new tests fail while the pre-existing maximize tests stay green — confirming the new tests exercise the previously-uncovered partial path.forge fmtapplied; no source/ABI/bytecode changes (test-only).🤖 Generated with Claude Code