2025 04 28 int - #322
2025 04 28 int#322
Conversation
WalkthroughThis change refactors the handling of floating-point values across the codebase by replacing the use of the Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test Suite
participant LibDecimalFloat as LibDecimalFloat
participant Float as Float Type
participant Implementation as Contract/Library
Test->>Implementation: Provide float input (as bytes32/int)
Implementation->>Float: Wrap input as Float
Implementation->>LibDecimalFloat: Call packLossless/add/eq on Float
LibDecimalFloat-->>Implementation: Return Float result
Implementation->>Float: Unwrap Float to bytes32/int
Implementation-->>Test: Return result for assertion
Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (4)
🔇 Additional comments (2)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (3)
src/generated/Rainterpreter.pointers.solis excluded by!**/generated/**src/generated/RainterpreterParser.pointers.solis excluded by!**/generated/**src/generated/RainterpreterReferenceExtern.pointers.solis excluded by!**/generated/**
📒 Files selected for processing (24)
.gas-snapshot(4 hunks)lib/rain.interpreter.interface(1 hunks)src/concrete/extern/RainterpreterReferenceExtern.sol(7 hunks)src/lib/extern/reference/op/LibExternOpContextCallingContract.sol(1 hunks)src/lib/extern/reference/op/LibExternOpIntInc.sol(1 hunks)src/lib/op/evm/LibOpBlockNumber.sol(2 hunks)src/lib/op/evm/LibOpChainId.sol(2 hunks)src/lib/op/evm/LibOpTimestamp.sol(2 hunks)src/lib/op/logic/LibOpEqualTo.sol(3 hunks)src/lib/op/math/LibOpMaxValue.sol(2 hunks)src/lib/parse/LibParseOperand.sol(7 hunks)src/lib/parse/literal/LibParseLiteralDecimal.sol(2 hunks)test/src/concrete/Rainterpreter.stateOverlay.t.sol(3 hunks)test/src/concrete/RainterpreterReferenceExtern.intInc.t.sol(6 hunks)test/src/lib/op/00/LibOpConstant.t.sol(2 hunks)test/src/lib/op/00/LibOpExternNP.t.sol(3 hunks)test/src/lib/op/00/LibOpStackNP.t.sol(3 hunks)test/src/lib/op/call/LibOpCall.t.sol(3 hunks)test/src/lib/op/crypto/LibOpHashNP.t.sol(2 hunks)test/src/lib/op/math/LibOpMaxValue.t.sol(2 hunks)test/src/lib/parse/LibParse.literalIntegerDecimal.t.sol(5 hunks)test/src/lib/parse/LibParse.namedLHS.t.sol(3 hunks)test/src/lib/parse/LibParseOperand.parseOperand.t.sol(5 hunks)test/src/lib/parse/literal/LibParseLiteralDecimal.parseDecimalFloat.t.sol(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-static)
- GitHub Check: rainix (ubuntu-latest, test-wasm-build)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-artifacts)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-static)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-test)
- GitHub Check: rainix (macos-latest, rainix-rs-artifacts)
- GitHub Check: rainix (macos-latest, rainix-rs-test)
🔇 Additional comments (64)
lib/rain.interpreter.interface (1)
1-1:Details
✅ Verification successful
Approve submodule pointer bump with verification
Updating the
lib/rain.interpreter.interfacesubproject reference to commitb27b40dd48e41247c644068cb330ce235a0236efaligns with the float-handling refactor. Ensure the commit exists in the submodule and that CI/tests pass against this interface version.
🏁 Script executed:
#!/bin/bash # Verify that the new submodule commit exists git submodule update --init lib/rain.interpreter.interface cd lib/rain.interpreter.interface git rev-parse --verify b27b40dd48e41247c644068cb330ce235a0236efLength of output: 594
✅ Submodule Commit Verified – Ready to Merge
The submodule atlib/rain.interpreter.interfacehas been updated to commit b27b40dd48e41247c644068cb330ce235a0236ef, and the commit exists in the remote repository. All that remains is to confirm that CI and existing tests pass against this interface version before merging.src/lib/op/evm/LibOpTimestamp.sol (2)
8-8: Import statement updated to use Float instead of PackedFloatThe import has been appropriately updated to import
Floatdirectly instead ofPackedFloat, aligning with the broader refactoring across the codebase.
33-33: Simplified floating-point conversion with direct packingThe implementation has been streamlined by using the more direct
Float.unwrap(LibDecimalFloat.fromFixedDecimalLosslessPacked())approach. This eliminates intermediate steps by directly converting the timestamp to a packed float representation.src/lib/op/math/LibOpMaxValue.sol (2)
8-8: Import statement updated to use Float instead of PackedFloatThe import has been appropriately updated to import
Floatdirectly instead ofPackedFloat, aligning with the broader refactoring across the codebase.
36-36: Simplified maximum float value constructionThe implementation has been improved by directly using
LibDecimalFloat.packLosslesswith maximum values forint224andint32. This replaces a likely multi-step packing and unwrapping process with a single direct packing call, resulting in cleaner code.src/lib/parse/literal/LibParseLiteralDecimal.sol (2)
7-7: Import statement updated to use Float instead of PackedFloatThe import has been appropriately updated to import
Floatdirectly instead ofPackedFloat, aligning with the broader refactoring across the codebase.
17-19: Updated parseDecimalFloatPacked to use Float instead of PackedFloatThe function now correctly uses the
Floattype instead ofPackedFloatfor the return value and local variable, maintaining consistency with the rest of the codebase.test/src/lib/parse/literal/LibParseLiteralDecimal.parseDecimalFloat.t.sol (1)
27-29: Updated test helper to use parseDecimalFloatPackedThe
checkParseDecimalReverthelper function has been correctly updated to callparseDecimalFloatPackedinstead of the now-commented-outparseDecimalFloatmethod, ensuring the tests match the updated implementation.src/lib/extern/reference/op/LibExternOpContextCallingContract.sol (1)
12-15: Library rename aligns with broader NPE2 suffix removal refactoring.The library has been renamed from
LibExternOpContextCallingContractNPE2toLibExternOpContextCallingContract, which aligns with the broader refactoring that removes theNPE2suffix from several external operation libraries as mentioned in the AI summary.src/lib/op/evm/LibOpBlockNumber.sol (2)
8-8: Updated import statement to use Float type.The import has been updated to use
Floatinstead ofPackedFloat, which aligns with the broader refactoring to useFloattype fromLibDecimalFloatacross the codebase.
33-33: Simplified float conversion with direct packed method.The implementation has been streamlined by replacing the previous two-step process (calling
fromFixedDecimalLosslessMemfollowed by.pack()) with a direct call tofromFixedDecimalLosslessPacked. This simplification might improve gas efficiency by reducing intermediate memory operations while maintaining the same functionality.test/src/lib/parse/LibParse.namedLHS.t.sol (3)
18-18: Updated import statement to use Float type.The import has been updated to use
Floatinstead ofPackedFloat, which aligns with the broader refactoring to useFloattype fromLibDecimalFloatacross the codebase.
163-165: Updated float assertion to use packLossless method.The constant assertions now use
Float.unwrap(LibDecimalFloat.packLossless(1, 0))instead of the previousPackedFloat.unwrap(LibDecimalFloat.pack(1e37, -37)). Both represent the same value of 1.0, but the new approach uses a more direct and lossless packing method, which is consistent with the refactoring pattern across the codebase.
217-220: Updated float assertions to use packLossless method.Similar to the previous float assertions, these constants now use the
packLosslessmethod instead of thepackmethod, while maintaining the same actual test values. This change is consistent with the broader refactoring pattern for float handling across the codebase.test/src/lib/op/call/LibOpCall.t.sol (2)
21-21: Updated import statement to use Float type.The import has been updated to use
Floatinstead ofPackedFloat, which aligns with the broader refactoring to useFloattype fromLibDecimalFloatacross the codebase.
166-166: Updated stack trace values to use packLossless method.The expected stack trace values now use
Float.unwrap(LibDecimalFloat.packLossless(1, 0))instead of the previousPackedFloatapproach. This maintains the same numeric values in the tests while using the new float type and packing method, consistent with the codebase-wide refactoring.Also applies to: 176-176
test/src/lib/op/00/LibOpExternNP.t.sol (4)
21-21: Import change from PackedFloat to FloatThe import statement has been updated to use
Floatinstead ofPackedFloatfrom theLibDecimalFloatlibrary, aligning with the codebase-wide refactoring of floating-point value handling.
228-232: Updated floating-point value handling in test casesThe test data preparation has been updated to use the
Float.unwrap(LibDecimalFloat.packLossless())pattern instead of the previousPackedFloat.unwrap(LibDecimalFloat.pack()). This change is consistent with the codebase's shift toward a more direct floating-point handling approach.
239-240: Updated expected stack item constructionThe expected stack item construction now uses the new
Float.unwrap(LibDecimalFloat.packLossless())pattern, maintaining consistency with the rest of the floating-point handling changes.
281-294: Simplified floating-point value handling in multiple inputs/outputs testThe multiple inputs/outputs test has been updated to use the new
Float.unwrap(LibDecimalFloat.packLossless())pattern consistently across all test data preparation. This change simplifies the floating-point representation by using direct lossless packing with zero exponents rather than the previous approach using explicit exponents.src/lib/op/evm/LibOpChainId.sol (2)
8-8: Updated import to use Float instead of PackedFloatThe import statement has been modified to import
Floatinstead ofPackedFloat, consistent with the codebase-wide refactoring of floating-point handling.
33-33: Simplified floating-point conversion in referenceFnThe conversion of
block.chainidto a floating-point value has been simplified by usingfromFixedDecimalLosslessPackeddirectly withFloat.unwrapinstead of the previous approach. This change streamlines the code by using a more direct conversion method.test/src/lib/op/00/LibOpStackNP.t.sol (3)
25-25: Updated import to use Float instead of PackedFloatThe import statement has been modified to use
Floatinstead ofPackedFloat, consistent with the codebase-wide refactoring of floating-point handling.
147-149: Updated assertion to use the new Float patternTest assertions have been updated to use
Float.unwrap(LibDecimalFloat.packLossless())for comparing stack values, maintaining consistency with the codebase's new approach to floating-point handling.
169-175: Updated multiple stack value assertions to use the new Float patternAll assertions in the multiple stack evaluation test have been updated to use the
Float.unwrap(LibDecimalFloat.packLossless())pattern consistently. This change simplifies floating-point representation by using lossless packing with zero exponents.test/src/lib/op/math/LibOpMaxValue.t.sol (2)
19-19: Updated import to use Float instead of PackedFloatThe import statement has been modified to import
Floatinstead ofPackedFloat, consistent with the codebase-wide refactoring of floating-point handling.
52-52: Simplified max value construction in testThe construction of the max value test case has been significantly simplified. Instead of manually creating a
Floatstruct withsignedCoefficientandexponentfields, the code now directly usesFloat.unwrap(LibDecimalFloat.packLossless())with the maximum values forint224andint32. This improves code readability and maintainability.test/src/lib/op/crypto/LibOpHashNP.t.sol (2)
27-27: LGTM! Import updated to use Float instead of PackedFloatThe import statement is correctly updated to use
Floatinstead ofPackedFloatfrom the LibDecimalFloat library, which aligns with the broader refactoring effort across the codebase.
107-107: LGTM! Float handling refactored correctlyThe assertion code has been properly updated to:
- Use
Float.unwrap()instead ofPackedFloat.unwrap()- Switch from
LibDecimalFloat.pack()with large exponent values toLibDecimalFloat.packLossless()with simpler argumentsThis change maintains the same test logic while using the new Float type implementation consistently.
Also applies to: 112-112
test/src/lib/op/00/LibOpConstant.t.sol (2)
22-22: LGTM! Import updated to use Float instead of PackedFloatThe import statement is correctly updated to use
Floatinstead ofPackedFloatfrom the LibDecimalFloat library, aligning with the broader refactoring effort across the codebase.
101-102: LGTM! Assertions properly updated to use FloatTest assertions have been correctly updated to use the new
Floattype withLibDecimalFloat.packLossless(), maintaining the same test logic while adapting to the refactored floating-point implementation.test/src/lib/parse/LibParse.literalIntegerDecimal.t.sol (5)
11-11: LGTM! Import updated to use Float instead of PackedFloatThe import statement is correctly updated to use
Floatinstead ofPackedFloatfrom the LibDecimalFloat library, aligning with the broader refactoring effort.
51-51: LGTM! Updated to use Float and packLosslessAssertion properly updated to use
Float.unwrap()withLibDecimalFloat.packLossless()instead of the previous PackedFloat approach.
88-89: LGTM! Assertions correctly updatedTest assertions are correctly updated to use the new Float type and packLossless method, maintaining test validity while using the refactored floating-point representation.
128-129: LGTM! Properly refactored assertionsThe assertions for the test case with duplicate values are correctly updated to use the new Float type implementation.
285-289: LGTM! E-notation test assertions properly updatedThe test for e-notation decimal literals has been correctly updated to use the new Float type with packLossless while maintaining the proper exponent values for scientific notation testing.
src/lib/parse/LibParseOperand.sol (8)
19-19: LGTM! Import updated to use Float instead of PackedFloatThe import statement is correctly updated to use
Floatinstead ofPackedFloatfrom the LibDecimalFloat library.
26-26: LGTM! Using directive added for FloatAdded the appropriate using directive for
LibDecimalFloat for Floatto support the refactored floating-point operations.
162-163: LGTM! Updated to use Float.wrap and unpackThe code has been correctly updated to use
Float.wrap(OperandV2.unwrap(operand)).unpack()for unpacking operands, which is consistent with the new floating-point representation approach.
182-183: LGTM! Float unpacking updatedSimilar to the previous update, the operand handling now correctly uses the Float type for unpacking values.
200-202: LGTM! Variable types updated to FloatVariable types are correctly changed from PackedFloat to Float, maintaining consistency with the library-wide refactoring.
230-232: LGTM! Variable types updated to FloatVariable types in handleOperand8M1M1 are correctly updated to use the Float type.
242-242: LGTM! Default initialization updated to use Float.wrapDefault initializations now correctly use
Float.wrap(0)instead of the previous PackedFloat approach.Also applies to: 250-250
278-279: LGTM! Variable types and initialization updatedIn the handleOperandM1M1 function, all variable declarations and initializations are correctly updated to use the Float type, maintaining consistency with the library-wide refactoring.
Also applies to: 286-286, 294-294
test/src/concrete/RainterpreterReferenceExtern.intInc.t.sol (4)
10-11: Migration to new Float type and updated library names.The code has been updated to use the
Floattype fromLibDecimalFloatand references the renamed libraryLibExternOpIntInc(previouslyLibExternOpIntIncNPE2). These changes are part of a larger refactoring effort in the codebase to standardize the floating-point representation.Also applies to: 23-23, 27-27
43-45: Improved floating-point handling in test expectations.The test now uses
Float.unwrap(LibDecimalFloat.packLossless())for test expectations instead of the previousPackedFloatapproach. This is consistent with the codebase-wide migration to theFloattype.Also applies to: 61-62
138-140: Cleaner float value manipulation using new Float APIs.The code now leverages the
Floattype's methods for float arithmetic, making the code more readable and consistent with the broader refactoring approach. UsingFloat.wrap,Float.unwrap, and theaddmethod provides a cleaner, more object-oriented approach to float manipulation.
143-143: Updated library reference to match renamed implementation.References to
LibExternOpIntIncNPE2have been updated toLibExternOpIntIncto match the renamed implementation library, ensuring consistency across the codebase.Also applies to: 157-157
test/src/concrete/Rainterpreter.stateOverlay.t.sol (2)
9-9: Updated import for Float type.This change correctly imports the
Floattype fromLibDecimalFloat, replacing the previousPackedFloatimport as part of the codebase-wide migration to the new floating-point representation.
16-16: Improved floating-point handling using Float type.The test now uses
Float.unwrap(LibDecimalFloat.packLossless())for state overlay keys and values, replacing the previousPackedFloatapproach. This improves consistency with the rest of the codebase and aligns with the broader refactoring efforts.Also applies to: 45-46, 63-63
src/lib/extern/reference/op/LibExternOpIntInc.sol (2)
7-7: Updated library name and Float type integration.The library has been renamed from
LibExternOpIntIncNPE2toLibExternOpIntInc(removing theNPE2suffix), and now imports and uses theFloattype with the appropriateusingdirective to enable method calls onFloatinstances. This change aligns with the codebase's standardization on theFloattype for floating-point operations.Also applies to: 14-17, 18-18
26-28: Cleaner floating-point arithmetic using Float methods.The implementation now leverages the
Floattype's methods for float arithmetic instead of manual unpacking and repacking. This shift to a more object-oriented approach withFloat.wrap,Float.unwrap, and theaddmethod improves code readability and maintainability.test/src/lib/parse/LibParseOperand.parseOperand.t.sol (2)
13-13: Updated import for Float type and implementation.This change correctly imports the
Floattype andLibDecimalFloatImplementationfromLibDecimalFloat, replacing the previousPackedFloatimport to align with the codebase-wide migration to the new floating-point representation.
84-84: Consistent Float packing approach for test expectations.The test now uses
Float.unwrap(LibDecimalFloat.packLossless())for expected operand values, replacing the previousPackedFloatapproach. This change is consistent across all test cases (single, two, three, and four decimal literals) and aligns with the broader refactoring of the codebase's float handling.Also applies to: 122-124, 183-187, 248-249
src/lib/op/logic/LibOpEqualTo.sol (5)
8-8: Import updated to useFloatinstead ofPackedFloatThis change aligns with the broader refactoring in the codebase to simplify float handling by using the
Floattype directly.
25-26: Updated variable types toFloatVariable types have been changed from
PackedFloattoFloatto use the new type system for floating-point values.
34-34: Simplified equality check usingFloat.eq()The equality comparison is now performed using the
eq()method on theFloattype, which is cleaner than the previous manual unpacking and component-wise equality check.
49-50: Updated reference function to useFloatThe reference implementation also uses
Float.wrap()to handle the stack items, maintaining consistency with the main implementation.
53-53: Simplified reference function equality checkThe reference implementation now uses the same
eq()method as the main implementation, ensuring consistency and simplifying the code.src/concrete/extern/RainterpreterReferenceExtern.sol (4)
18-18: Updated imports to use renamed libraries without theNPE2suffixUpdated the imports to use the new library names without the
NPE2suffix, which is part of a broader standardization effort across the codebase.Also applies to: 21-21
35-35: AddedFloattype import and usage directiveAdded the
LibDecimalFloatandFloatimports and the corresponding usage directive to support the updated float handling approach.Also applies to: 150-151
242-253: Updated float parsing to useFloattypeThe decimal float parsing logic now:
- Uses
parseDecimalFloatPackedto directly get a packed float- Wraps it with
Floatfor type safety- Uses
Float.gtfor comparison instead of manually comparing componentsThis aligns with the broader refactoring to simplify float handling.
316-316: Updated references to renamed librariesFunction references throughout the file have been updated to use the new library names without the
NPE2suffix, ensuring consistency with the import changes.Also applies to: 319-319, 351-351, 381-381
.gas-snapshot (1)
1-637: Updated gas measurements and replacedLibOpMaxUint256TestwithLibOpMaxValueTestThe gas snapshot has been updated to reflect the refactored code's performance metrics. Key changes include:
- Updated run counts across tests (mostly from 2048 to 2051)
- Minor gas usage fluctuations, which are expected from the float handling refactoring
- Added
LibOpMaxValueTest(lines 189-194) to replaceLibOpMaxUint256TestThese changes align with the codebase's shift from specific uint256 max handling to a more generic value approach, consistent with the float type system updates.
Motivation
Solution
Checks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit
PackedFloattype with the newFloattype for floating-point operations, simplifying float handling across parsing, opcode, and external operation logic.NPE2suffix for improved clarity.Floattype and related packing methods, ensuring consistency with refactored float handling.