Guard displayExponent overflow in _toScientific (#185)#245
Guard displayExponent overflow in _toScientific (#185)#245thedavidmeister wants to merge 3 commits into
Conversation
After maximizeFull, scaleExponent is 75 or 76. For Floats with large positive exponents (within ~76 of int32.max), displayExponent = exponent + scaleExponent exceeds int32.max, producing a formatted string whose exponent the parser cannot re-pack into int32. Revert with UnformatableExponent rather than silently emitting an un-parseable string. The negative-overflow case is also guarded but unreachable in practice: the minimum post-maximizeFull exponent from a valid Float is int32.min - 76, giving displayExponent = int32.min exactly (still in range). Two new tests pin the fix: one asserting the revert for (10, int32.max) and one confirming (1, int32.max) still formats successfully. Bytecode changes - requires manual-sol-artifacts redeploy before merge. Co-Authored-By: Claude <noreply@anthropic.com>
Update DecimalFloat artifact hash after displayExponent overflow guard.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughTwo unit tests are added to ChangesScientific displayExponent overflow tests
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Accept main's originalExponent approach in UnformatableExponent revert; update test to expect originalExponent (user-visible) instead of post-maximizeFull exponent. Co-Authored-By: Claude <noreply@anthropic.com>
Summary
_toScientificcomputesdisplayExponent = exponent + scaleExponentwherescaleExponentis 75 or 76 (frommaximizeFull). For validFloatvalues whose exponent is within ~76 ofint32.max,displayExponentoverflowsint32and the formatter emits a string the parser cannot re-pack — a silent data-corruption bug.UnformatableExponent(exponent)before emitting.testFormatScientificDisplayExponentOverflowReverts(exact reproduction of the bug with(10, int32.max)) andtestFormatScientificExponentAtMaxBoundarySucceeds(boundary:(1, int32.max)formats cleanly to"1e2147483647").Closes #185
This changes
LibFormatDecimalFloat, which is part of the deployedDecimalFloatcontract. ThetestDeployAddressandtestExpectedCodeHashDecimalFloatCI tests will fail until theManual sol artifactsworkflow is triggered on this branch:Test plan
testFormatScientificDisplayExponentOverflowReverts— new, confirms revert for the overflow casetestFormatScientificExponentAtMaxBoundarySucceeds— new, confirms the boundary value still formatsLibFormatDecimalFloatToDecimalStringTesttests still passtestFormatScientificDisplayExponentOverflowRevertsusesvm.expectRevert— removing the guard causes the test to fail🤖 Generated with Claude Code
Summary by CodeRabbit