Repository navigation
Check pow10 and log10 against the oracle, not the unrounded functions - #411
Conversation
testLog10RoundsUnrounded took its expected value from log10Unrounded. testLog10WithinOracle bounds log10 against LibTranscendentalOracle.log10 by the README's half a unit plus 2e-50, over the same inputs. The decade shift, monotone and no-revert tests of both functions, which compared the implementation only with itself, also check each result against the oracle. testPow10NearTiesWithinBound checks the testPow10RoundsHalfUp inputs against bc. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
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 45 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. Comment |
…bound Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…independent-rounding-oracles
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…independent-rounding-oracles
|
@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:
|
Closes #386
Test-only. The pow10/log10 rounding and relation tests no longer use the unrounded functions as their own oracle.
testLog10RoundsUnrounded(circular againstlog10Unrounded) is replaced bytestLog10WithinOracle. It covers the same domain and asserts thatlog10is within the README bound ofLibTranscendentalOracle.log10, compared exactly in U512 at 1e-70.testLog10DecadeShift,testLog10Monotone,testPow10DecadeShift,testPow10MonotoneandtestNoRevertkeep their relations and now also check each result against the oracle.testPow10NearTiesWithinBoundchecks the fourtestPow10RoundsHalfUpinputs against 70-digitbcvalues (scale 220), within the README pow10 bound.LibTranscendentalOracle.exp10's NatSpec domain is widened to "integer part fits int256"; the exponent ≥ 0 branch is exact.Open question, not changed here:
testPow10RoundsHalfUpis left as is. The true values of its inputs sit 9.6e-11 to 1.78e-9 of a unit from a 41st-digit tie, inside the README's allowed 3.28e-8. So the documented bound accepts either neighbour, and the tie behaviour can only be pinned throughpow10Unrounded. Relatedly, for x = 62876337819492131669053461917963730523385842721800e-50 the true 10^x is just above a tie, butpow10rounds down. That is within the bound, but conflicts with a literal reading of the NatSpec "rounded to nearest, half up".Overlap: PR #354 edits the same two files, including the decade-shift and monotone tests, and moves the remaining library-arithmetic call sites. Whichever merges second takes a textual rebase.
QA
log10Unrounded+ 1e-30) survived the old targeted tests and is killed bytestLog10WithinOraclealone. P3 and L4, previously killed only through the circular remainder checks, are now killed by the oracle checks.LibDecimalFloatImplementation.sol: L1 (roundSignificanttie>=→>, survives before and after), L2 (log10 truncates), L3, L4 (POW_FIXED_LN10 off at digit 30), P1 (pow10 guard tie rounds down, killed only bytestPow10RoundsHalfUp), P2 (pow10 truncates), P3 (exp10Fixed 10^(1/4) off at digit 30). Full two-file suite: 6/7 before and after.LibTranscendentalOracle(Taylor and atanh series over OpenZeppelinmulDiv,bcconstants) plus 70-digitbcreferences, with the README bounds.bccheck, but its tie assertions await the ruling above; bullet 4 is Bound the transcendental tests independently of the implementation #354's scope.forge test: 827 passed.🤖 Generated with Claude Code