2025 04 15 pack#28
Conversation
## Walkthrough
This set of changes expands the bit width for the signed coefficient in the decimal float packing/unpacking logic from 128 bits to 224 bits and reduces the exponent to 32 bits. The packing and unpacking functions in `LibDecimalFloat` are updated to reflect this new layout, including bitmask and shift adjustments. The exponent range constants are also updated to allow a much larger range. Test files are updated to match the new parameter types and behaviors, with some test signatures changed and a new normalization test added. Import statements are reordered or cleaned up in several test files, and minor test logic adjustments are made to align with the new internal representation.
## Changes
| File(s) | Change Summary |
|-----------------------------------------------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| src/lib/LibDecimalFloat.sol<br>src/lib/implementation/LibDecimalFloatImplementation.sol | Increased coefficient width from 128 to 224 bits and reduced exponent to 32 bits in packing/unpacking logic; updated exponent min/max constants to use a much larger range; adjusted bitmasks and shifts accordingly. |
| test/src/lib/LibDecimalFloat.pack.t.sol | Updated test function parameter types to match new coefficient and exponent bit widths; changed equality assertions to use a dedicated equality function; added normalization step; adjusted logic for zero coefficients. |
| .gas-snapshot | Updated test metadata to reflect new test signatures, added normalization test, and recorded updated gas/time metrics for all tests. |
| test/src/lib/LibDecimalFloat.divide.t.sol<br>test/src/lib/LibDecimalFloat.inv.t.sol<br>test/src/lib/LibDecimalFloat.minus.t.sol<br>test/src/lib/LibDecimalFloat.sub.t.sol | Reordered import statements for consistency; no logic changes. |
| test/src/lib/LibDecimalFloat.multiply.t.sol | Cleaned up and consolidated import statements; no functional changes. |
| test/src/lib/LibDecimalFloat.power10.t.sol | Removed unused imports and deleted gas measurement code from test helper function. |
| test/src/lib/parse/LibParseDecimalFloat.t.sol | Added a no-op reference to a returned variable to suppress unused variable warnings. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant Caller
participant LibDecimalFloat
Caller->>LibDecimalFloat: pack(signedCoefficient, exponent)
alt signedCoefficient == 0
LibDecimalFloat-->>Caller: return packed zero value
else signedCoefficient != 0
loop while signedCoefficient doesn't fit in 224 bits
LibDecimalFloat->>LibDecimalFloat: signedCoefficient /= 10, exponent += 1
end
LibDecimalFloat->>LibDecimalFloat: check exponent fits in 32 bits
LibDecimalFloat-->>Caller: return packed value (224-bit coeff, 32-bit exp)
end
Caller->>LibDecimalFloat: unpack(packedValue)
LibDecimalFloat-->>Caller: return (signedCoefficient: 224 bits, exponent: 32 bits)Possibly related PRs
Suggested reviewers
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (14)
.gas-snapshot(2 hunks)src/lib/LibDecimalFloat.sol(3 hunks)src/lib/implementation/LibDecimalFloatImplementation.sol(1 hunks)test/src/lib/LibDecimalFloat.add.t.sol(5 hunks)test/src/lib/LibDecimalFloat.divide.t.sol(1 hunks)test/src/lib/LibDecimalFloat.inv.t.sol(2 hunks)test/src/lib/LibDecimalFloat.minus.t.sol(2 hunks)test/src/lib/LibDecimalFloat.multiply.t.sol(2 hunks)test/src/lib/LibDecimalFloat.normalize.t.sol(1 hunks)test/src/lib/LibDecimalFloat.pack.t.sol(2 hunks)test/src/lib/LibDecimalFloat.power10.t.sol(1 hunks)test/src/lib/LibDecimalFloat.sub.t.sol(3 hunks)test/src/lib/implementation/LibDecimalFloatImplementation.normalize.t.sol(2 hunks)test/src/lib/parse/LibParseDecimalFloat.t.sol(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-static)
- GitHub Check: git-clean
- GitHub Check: rainix (ubuntu-latest, rainix-sol-test)
🔇 Additional comments (25)
src/lib/implementation/LibDecimalFloatImplementation.sol (1)
24-24: Expanded exponent range looks correct.Setting
EXPONENT_MAXto half oftype(int256).maxsignificantly broadens the exponent range, aligning with the design requirements for large-scale decimal arithmetic. This change appears logically consistent with the rest of the code.test/src/lib/LibDecimalFloat.divide.t.sol (1)
5-5: Import reorder is harmless.Reordering the import statement is a minor syntactical change that does not affect functionality. No concerns here.
test/src/lib/implementation/LibDecimalFloatImplementation.normalize.t.sol (2)
6-6: Import statement simplified to use only EXPONENT_MAXThe import statement has been modified to remove
EXPONENT_MINand only importEXPONENT_MAX, aligning with the library design change whereEXPONENT_MINis now defined as-EXPONENT_MAX.
19-19: Exponent bounding updated to use -EXPONENT_MAXThe exponent bounding has been updated to use
-EXPONENT_MAXinstead ofEXPONENT_MIN, consistent with the import change and the updated exponent range definition in the library.test/src/lib/LibDecimalFloat.inv.t.sol (2)
5-5: Import statement simplified to remove EXPONENT_MINThe import statement has been modified to remove
EXPONENT_MIN, keeping onlyLibDecimalFloat,Float, andEXPONENT_MAX, aligning with the library design change whereEXPONENT_MINis replaced by-EXPONENT_MAX.
36-36: Exponent bounding updated to use -EXPONENT_MAXThe exponent bounding has been updated to use
-EXPONENT_MAXinstead ofEXPONENT_MIN, maintaining consistency with the import changes and the underlying library modifications.test/src/lib/LibDecimalFloat.power10.t.sol (1)
4-4: Simplified import statementThe import statement has been modified to remove the unused
EXPONENT_MINsymbol and the import ofconsole2fromforge-std/Test.sol, which was likely only used for the removed gas measurement code. This change improves code cleanliness by removing unnecessary imports.test/src/lib/LibDecimalFloat.minus.t.sol (2)
4-4: Import updated to remove EXPONENT_MINThe import statement has been updated to remove
EXPONENT_MIN, which aligns with the architectural change to defineEXPONENT_MINas-EXPONENT_MAXelsewhere in the codebase.
36-37: Exponent bounds updated to use -EXPONENT_MAXExponent bounds have been updated from using
EXPONENT_MIN / 10to using-EXPONENT_MAX / 10, which maintains the same logical behavior while adapting to the new constant structure. This change ensures symmetry around zero for the exponent range.test/src/lib/LibDecimalFloat.sub.t.sol (3)
4-4: Import updated to remove EXPONENT_MINThe import statement has been simplified to remove
EXPONENT_MIN, which aligns with the architectural change to defineEXPONENT_MINas-EXPONENT_MAXelsewhere in the codebase.
43-44: Exponent bounds updated to use -EXPONENT_MAXExponent bounds have been updated from using
EXPONENT_MIN / 10to using-EXPONENT_MAX / 10, which maintains the same logical behavior while adapting to the new constant structure.
60-61: Exponent bounds updated to use -EXPONENT_MAXExponent bounds for the
testSubMinSignedValuefunction have been updated to use-EXPONENT_MAX / 10instead ofEXPONENT_MIN / 10, consistent with the architectural change throughout the codebase.test/src/lib/LibDecimalFloat.multiply.t.sol (2)
8-11: Imports reorganized for clarityThe imports have been reorganized to:
- Explicitly import
FloatandEXPONENT_MAXfromLibDecimalFloat.sol- Simplify the import of
LibDecimalFloatImplementationby removing redundant constantsThis makes the dependencies clearer and reduces import redundancy.
128-129: Exponent bounds updated to use -EXPONENT_MAXExponent bounds have been updated to use a symmetrical range from
-EXPONENT_MAXtoEXPONENT_MAX, which aligns with the architectural change to defineEXPONENT_MINas-EXPONENT_MAX.test/src/lib/LibDecimalFloat.add.t.sol (4)
4-4: Import updated to remove EXPONENT_MINThe import statement has been updated to remove
EXPONENT_MIN, consistent with the architectural changes throughout the codebase.
116-117: Exponent bounds updated to use -EXPONENT_MAXExponent bounds have been updated to use
-EXPONENT_MAX / 10instead ofEXPONENT_MIN / 10, which maintains the same logical behavior while adapting to the new constant structure.
130-131: Exponent bounds updated to use -EXPONENT_MAXExponent bounds in the
testAddingSmallToLargeReturnsLargeFuzzfunction have been updated consistently with other bounds changes throughout the codebase.
230-231: Exponent bounds updated to use -EXPONENT_MAXExponent bounds in the
testAddZeroToAnyNonZerofunction have been updated consistently with other bounds changes throughout the codebase..gas-snapshot (1)
1-188: Gas snapshot reflects updated parameter types and test runs.The gas snapshot shows run counts have been updated from 5099 to 5100 for many tests, with corresponding adjustments in gas metrics. Most importantly, the snapshot reflects the parameter type changes, particularly in
LibDecimalFloatPackTestfunctions which now useint224for coefficients andint32for exponents.src/lib/LibDecimalFloat.sol (2)
26-28: Import order change for exponent constants.The import order has been reorganized to place the exponent constants at the end of the import list.
334-339: Updated unpack function to match new bit width allocation.The changes correctly align with the pack function:
- Updated bit mask to type(uint224).max
- Updated signextend parameter from 15 to 27 (for 224 bits)
- Updated right shift from 0x80 to 0xe0 (128 bits to 224 bits)
The modifications ensure proper decoding of the new packed format.
test/src/lib/LibDecimalFloat.pack.t.sol (4)
4-4: Simplified import to include only necessary constants.The import has been streamlined to only include what's needed, improving code clarity.
26-26: Parameter type updated to match new bit allocation.The test function's parameters now match the updated bit width allocation in the main library:
int224 signedCoefficientfor the coefficientint32 exponentfor the exponentThis ensures thorough testing of the new implementation.
30-36: Improved handling of zero coefficient cases.This change adds special handling for the zero coefficient case:
- For non-zero coefficients, it verifies exact exponent matches
- For zero coefficients, it ensures the exponent is normalized to zero
This is a good addition that properly tests the zero-handling behavior added to the pack function.
39-54: Enhanced round-trip test with normalization and numeric equality.The normalized round-trip test was significantly improved:
- It now explicitly normalizes input values before packing
- Exponent range is validated before packing to avoid overflow
- Uses
LibDecimalFloat.eq()instead of direct equality comparisonThis approach ensures that round-trip conversions are tested for numeric equivalence rather than just byte-by-byte equality, which is more appropriate for floating-point values.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/lib/LibDecimalFloat.sol(5 hunks)src/lib/implementation/LibDecimalFloatImplementation.sol(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-test)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-static)
- GitHub Check: git-clean
🔇 Additional comments (8)
src/lib/implementation/LibDecimalFloatImplementation.sol (2)
22-26: Symmetry for minimum exponent.Defining
EXPONENT_MINas the negative ofEXPONENT_MAXis straightforward and ensures consistent range boundaries. The documentation is helpful, and the implementation is clear. No issues found.
19-19:Details
✅ Verification successful
Validate large exponent boundary.
Using
type(int256).max / 2sets a vastly large exponent boundary that dwarfs the 32-bit exponent limit enforced elsewhere. This is likely intentional for overflow checks, but please confirm the actual usage patterns to ensure no unintended consequences.Use the following script to search for all references to
EXPONENT_MAXthroughout the repository and verify any assumptions behind this large boundary:
🏁 Script executed:
#!/bin/bash rg -A 5 "EXPONENT_MAX"Length of output: 13833
Update on EXPONENT_MAX Boundary Validation
After reviewing all references to
EXPONENT_MAXacross the repository, it’s clear that the design consistently uses the value defined astype(int256).max / 2for overflow protection. Test cases in several modules (e.g., addition, subtraction, multiplication, division, and normalization) employ this constant—often scaled (like usingEXPONENT_MAX / 10)—to ensure that arithmetic operations remain within safe bounds. This pattern confirms that the intentionally large exponent boundary is used only for guarding against overflows, and there are no unintended consequences observed in usage.If the design decision to use half of the maximum
int256value as the upper bound was deliberate, then the current implementation appears sound. You may still want to double-check any external constraints (such as the referenced 32-bit limit elsewhere) to ensure that all parts of the codebase align with this approach.src/lib/LibDecimalFloat.sol (6)
26-28: Additional imports align with expanded exponent range.These imports for
SIGNED_NORMALIZED_MAX,EXPONENT_MAX, andEXPONENT_MINmatch the newly introduced constants in the implementation library. They appear consistent with the broader update to support wider exponent ranges.
286-288: Clarify packing documentation.The updated comments rightly note that two 32-byte values (coefficient + exponent) are compressed into one 32-byte word, leading to data loss for non-normalized floats. The doc updates are accurate and improve clarity.
302-304: Short-circuit for zero coefficient.Early returning a zero-packed float when the coefficient is zero offers a simple, efficient shortcut. This neatly sidesteps extraneous checks on the exponent.
310-313: Protecting against 32-bit exponent overflow.This explicit check reverts when the exponent cannot fit into
int32, aligning with the newly established 32-bit exponent range. It ensures correctness by guarding against out-of-bounds exponents.
315-317: Mask for storing the 224-bit coefficient.Zeroing out the upper bits helps cleanly store negative and positive coefficients in the lower 224 bits. This approach is consistent with the sign-extension logic used during unpack.
336-340: Correct sign extension for unpack.Using
signextend(27, ...)interprets bits [223..0] of the packed word as a 224-bit signed integer, whilesar(0xe0, ...)extracts the 32-bit exponent. This robustly reconstructs the original signed coefficient and exponent.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
.gas-snapshot(2 hunks)src/lib/LibDecimalFloat.sol(5 hunks)test/src/lib/LibDecimalFloat.divide.t.sol(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-test)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-static)
- GitHub Check: git-clean
🔇 Additional comments (10)
test/src/lib/LibDecimalFloat.divide.t.sol (2)
5-5: Use new exponent constants import.The updated import statement aligns with the revised exponent definitions and the new 224-bit coefficient logic.
123-124: Narrow exponent fuzzing range.Changing the fuzz bounds to
EXPONENT_MIN / 2andEXPONENT_MAX / 2may reduce test exploration of extreme exponents. Please confirm whether other tests still cover the full possible range or if additional test scenarios are needed..gas-snapshot (1)
1-4: Updated gas snapshot metrics.All changes here appear to be updated test run counts and gas usage statistics. There are no functional code modifications. This is expected behavior after adjusting the internal float representation and exponent ranges.
Also applies to: 7-8, 12-12, 14-14, 16-16, 18-18, 21-23, 25-27, 29-29, 35-36, 37-43, 45-46, 52-52, 57-57, 63-72, 77-81, 86-90, 96-105, 107-108, 110-114, 116-117, 122-122, 129-138, 139-144, 145-150, 151-152, 155-155, 159-160, 164-164, 166-170, 171-176, 179-181, 182-183, 205-205, 213-213
src/lib/LibDecimalFloat.sol (7)
26-28: Revised references to constants.Explicitly listing
SIGNED_NORMALIZED_MAX,EXPONENT_MAX, andEXPONENT_MINmatches the new 224-bit coefficient and 32-bit exponent design.
54-55: Doc comment alignment with new bit widths.The documentation now correctly reflects 224 signed bits for the coefficient and 32 bits for the exponent.
286-288: Clarified packing data-loss comment.The added explanations on fitting 64 bytes into 32 bytes provide clearer insight into potential truncation during packing.
303-304: Early return for zero coefficient.Short-circuiting when
signedCoefficient == 0is a neat optimization to avoid unnecessary processing.
318-320: Enforce exponent int32 constraint.Reverting on exponent overflow enforces the 32-bit limit, consistent with the new design.
326-326: Bit shifting exponent into upper 32 bits.Shifting left by 0xe0 (224 decimal) places the exponent in the top 32 bits of the packed value, aligning properly with the new layout.
343-343: Masking and sign-extending coefficient, extracting exponent.Using a
uint224mask for the coefficient and shifting out the top 32 bits for the exponent accurately unpacks the new 224+32 format.Also applies to: 345-346
1 participant
Motivation
Solution
Checks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit