-
Notifications
You must be signed in to change notification settings - Fork 1
wip on round #144
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
wip on round #144
Conversation
WalkthroughPow now handles zero and negative bases and negative exponents, uses reciprocal + exponentiation-by-squaring for integer parts, and composes log10/ pow10 for fractional parts. Log/antilog table access is centralized via a bounds-checked lookup helper; table dimensions reduced and many tests updated for new errors, casts, and lint rules. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Test
participant LibDF as LibDecimalFloat.pow
participant Impl as LibDecimalFloatImplementation
participant LogTable
Test->>LibDF: pow(a, b)
alt a == 0 or trivial
LibDF-->>Test: return packed result or revert (ZeroNegativePower)
else if negative base
LibDF-->>Test: revert PowNegativeBase(...)
else if b < 0
LibDF->>LibDF: compute a.inv()
LibDF->>LibDF: pow(a.inv(), -b)
else
LibDF->>Impl: log10(a) -- get characteristic & mantissa
Impl->>LogTable: lookupLogTableVal(tablesAddr, idx)
Note right of LogTable #f6fbff: bounds-check + assembly read
LogTable-->>Impl: tableEntry
Impl->>Impl: interpolate → fracLog
Impl-->>LibDF: fracLog
LibDF->>LibDF: exponentiation-by-squaring for integer part (uses Impl.mul)
LibDF->>LibDF: pow10(fracLog) and combine with squared-result
LibDF-->>Test: packed DecimalFloat result (packLossy)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/implementation/LibDecimalFloatImplementation.sol (1)
1128-1154: Avoid duplicated table-reading logic; reuse the new lookup helper.lookupAntilogTableY1Y2 reimplements extcodecopy with a different base. Prefer a shared internal to reduce drift/bugs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
src/generated/LogTables.pointers.solis excluded by!**/generated/**
📒 Files selected for processing (4)
src/lib/LibDecimalFloat.sol(3 hunks)src/lib/implementation/LibDecimalFloatImplementation.sol(4 hunks)src/lib/table/LibLogTable.sol(1 hunks)test/src/lib/LibDecimalFloat.pow.t.sol(1 hunks)
🧰 Additional context used
🧠 Learnings (11)
📚 Learning: 2025-09-09T15:11:31.003Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#130
File: src/lib/implementation/LibDecimalFloatImplementation.sol:320-330
Timestamp: 2025-09-09T15:11:31.003Z
Learning: In LibDecimalFloatImplementation.sol div function, returning zero when underflowExponentBy > 76 is intentional "non-error underflow" behavior rather than reverting. This graceful handling is the main goal of the changes.
Applied to files:
src/lib/LibDecimalFloat.solsrc/lib/implementation/LibDecimalFloatImplementation.sol
📚 Learning: 2025-09-02T09:33:32.513Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#119
File: test/src/lib/format/LibFormatDecimalFloat.toDecimalString.t.sol:174-175
Timestamp: 2025-09-02T09:33:32.513Z
Learning: The LibFormatDecimalFloat.toDecimalString function in src/lib/format/LibFormatDecimalFloat.sol does not include rounding logic. It formats decimal floats as-is without rounding values based on significant figures limits.
Applied to files:
src/lib/LibDecimalFloat.solsrc/lib/implementation/LibDecimalFloatImplementation.sol
📚 Learning: 2025-08-29T10:38:26.353Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: src/lib/implementation/LibDecimalFloatImplementation.sol:593-601
Timestamp: 2025-08-29T10:38:26.353Z
Learning: In Solidity, int256(1) when passed through the maximize function in LibDecimalFloatImplementation.sol produces exactly (1e76, -76), not an approximation. This means the special case for signedCoefficient == 1e76 in log10 correctly handles powers of 10 like log10(1).
Applied to files:
src/lib/LibDecimalFloat.solsrc/lib/implementation/LibDecimalFloatImplementation.sol
📚 Learning: 2025-08-29T10:38:26.353Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: src/lib/implementation/LibDecimalFloatImplementation.sol:593-601
Timestamp: 2025-08-29T10:38:26.353Z
Learning: The maximize function in LibDecimalFloatImplementation.sol produces exact results for simple integer values like 1. maximize(1, 0) yields exactly (1e76, -76) with no precision loss, and the log10 special case for signedCoefficient == 1e76 correctly handles this.
Applied to files:
src/lib/LibDecimalFloat.solsrc/lib/implementation/LibDecimalFloatImplementation.sol
📚 Learning: 2025-08-21T18:03:40.347Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#107
File: test/lib/LibDecimalFloatSlow.sol:37-45
Timestamp: 2025-08-21T18:03:40.347Z
Learning: In test/lib/LibDecimalFloatSlow.sol, the "slow" implementation is intentionally different from the production implementation to serve as an independent reference for fuzzing tests. The goal is to have two different approaches (expensive loops vs optimized jumps) that produce equivalent results, not identical implementations.
Applied to files:
src/lib/LibDecimalFloat.soltest/src/lib/LibDecimalFloat.pow.t.solsrc/lib/implementation/LibDecimalFloatImplementation.sol
📚 Learning: 2025-09-10T19:20:35.927Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#135
File: test/src/lib/LibDecimalFloat.pow.t.sol:160-161
Timestamp: 2025-09-10T19:20:35.927Z
Learning: In test/src/lib/LibDecimalFloat.pow.t.sol, the comment "we do not try" in the context of "The round trip should not error so we do not try" refers to not using try/catch constructs around the powExternal call, contrasting with previous versions that used nested try/catch blocks.
Applied to files:
src/lib/LibDecimalFloat.soltest/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-06-16T13:17:28.513Z
Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#58
File: src/concrete/DecimalFloat.sol:175-182
Timestamp: 2025-06-16T13:17:28.513Z
Learning: In the rainlanguage/rain.math.float codebase, there's an established naming convention where functions accepting a `Float` type parameter consistently use `float` as the parameter name, even though it shadows the type name. This pattern is used throughout `LibDecimalFloat.sol` and should be maintained for consistency in related contracts like `DecimalFloat.sol`.
Applied to files:
src/lib/LibDecimalFloat.sol
📚 Learning: 2025-08-29T14:54:24.240Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: test/src/lib/LibDecimalFloat.decimal.t.sol:122-126
Timestamp: 2025-08-29T14:54:24.240Z
Learning: In test/src/lib/LibDecimalFloat.decimal.t.sol, the fromFixedDecimalLossy function converts uint256(type(int256).max) losslessly because the boundary check is `value > uint256(type(int256).max)`, not `>=`. Values exactly equal to type(int256).max still take the lossless conversion path.
Applied to files:
src/lib/LibDecimalFloat.solsrc/lib/implementation/LibDecimalFloatImplementation.sol
📚 Learning: 2025-04-25T03:58:01.307Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#30
File: test/src/lib/LibDecimalFloat.gt.t.sol:33-36
Timestamp: 2025-04-25T03:58:01.307Z
Learning: In the rain.math.float library, all values of `Float` (which is a type alias for bytes32) are considered valid and can be safely used with methods like gt(), lt(), or eq() without causing reverts.
Applied to files:
src/lib/LibDecimalFloat.sol
📚 Learning: 2025-08-29T14:58:50.500Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: src/lib/implementation/LibDecimalFloatImplementation.sol:896-899
Timestamp: 2025-08-29T14:58:50.500Z
Learning: In unchecked Solidity blocks, arithmetic operations can overflow/underflow and wrap around, so bounds checks that seem "impossible" for normal arithmetic may actually be necessary to catch overflow edge cases. For example, in withTargetExponent function, the check `exponentDiff < 0` is needed because `targetExponent - exponent` could underflow in unchecked arithmetic.
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.solsrc/lib/implementation/LibDecimalFloatImplementation.sol
📚 Learning: 2025-07-24T04:32:14.171Z
Learnt from: rouzwelt
PR: rainlanguage/rain.math.float#83
File: src/concrete/DecimalFloat.sol:248-251
Timestamp: 2025-07-24T04:32:14.171Z
Learning: In the rainlanguage/rain.math.float project, functions in DecimalFloat.sol that return tuples from LibDecimalFloat calls must unpack the tuple into local variables before returning them (rather than returning directly) to maintain compatibility with Slither static analysis checks.
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.sol
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: rainix (ubuntu-latest, test-wasm-build)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-legal)
- GitHub Check: rainix (macos-latest, rainix-sol-legal)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-test)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-test)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-static)
- GitHub Check: rainix (macos-latest, rainix-rs-test)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-static)
- GitHub Check: git-clean
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/lib/table/LibLogTable.sol (2)
9-35: Fix the 16-bit encoder after shrinking the table.We cut
logTableDecdown to 90 rows, but this block still allocates 1820 bytes and starts fromi = table + 90 * 0x20. The loop therefore executes 910 iterations, walks one row past the real data, and publishes a bytes payload whose declared length (1820) no longer matches the 900 values we actually have. That leaves the tail of the lookup table corrupted, solookupLogTableValwill read garbage for the last chunk of indices (and will now revert once the new bounds check is wired in). Please update the loop to stop at row 89 and shrink the encoded length to 1800 bytes.- mstore(0x40, add(encoded, add(1820, 0x20))) + mstore(0x40, add(encoded, add(1800, 0x20))) ... - let i := add(table, mul(0x20, 90)) + let i := add(table, mul(0x20, 89)) ... - mstore(cursor, 1820) + mstore(cursor, 1800)
37-63: Apply the same correction to the 8-bit encoder.The small-table encoder still assumes 91 rows (length 910) and keeps iterating past the new 90-row payload, so we hit the same out-of-bounds read and bogus length issue. Mirror the adjustments from the uint16 variant so we only copy 900 bytes and start from row 89.
- mstore(0x40, add(encoded, add(910, 0x20))) + mstore(0x40, add(encoded, add(900, 0x20))) ... - let i := add(table, mul(0x20, 90)) + let i := add(table, mul(0x20, 89)) ... - mstore(cursor, 910) + mstore(cursor, 900)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
src/error/ErrDecimalFloat.sol(1 hunks)src/lib/LibDecimalFloat.sol(2 hunks)src/lib/implementation/LibDecimalFloatImplementation.sol(3 hunks)src/lib/table/LibLogTable.sol(5 hunks)test/src/lib/LibDecimalFloat.pow.t.sol(1 hunks)test/src/lib/implementation/LibDecimalFloatImplementation.lookupLogTableVal.t.sol(1 hunks)
👮 Files not reviewed due to content moderation or server errors (3)
- src/error/ErrDecimalFloat.sol
- test/src/lib/LibDecimalFloat.pow.t.sol
- test/src/lib/implementation/LibDecimalFloatImplementation.lookupLogTableVal.t.sol
🧰 Additional context used
🧠 Learnings (8)
📚 Learning: 2025-08-21T18:03:40.347Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#107
File: test/lib/LibDecimalFloatSlow.sol:37-45
Timestamp: 2025-08-21T18:03:40.347Z
Learning: In test/lib/LibDecimalFloatSlow.sol, the "slow" implementation is intentionally different from the production implementation to serve as an independent reference for fuzzing tests. The goal is to have two different approaches (expensive loops vs optimized jumps) that produce equivalent results, not identical implementations.
Applied to files:
test/src/lib/implementation/LibDecimalFloatImplementation.lookupLogTableVal.t.solsrc/lib/implementation/LibDecimalFloatImplementation.soltest/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-09-09T15:11:31.003Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#130
File: src/lib/implementation/LibDecimalFloatImplementation.sol:320-330
Timestamp: 2025-09-09T15:11:31.003Z
Learning: In LibDecimalFloatImplementation.sol div function, returning zero when underflowExponentBy > 76 is intentional "non-error underflow" behavior rather than reverting. This graceful handling is the main goal of the changes.
Applied to files:
src/error/ErrDecimalFloat.solsrc/lib/implementation/LibDecimalFloatImplementation.solsrc/lib/LibDecimalFloat.sol
📚 Learning: 2025-08-29T10:38:26.353Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: src/lib/implementation/LibDecimalFloatImplementation.sol:593-601
Timestamp: 2025-08-29T10:38:26.353Z
Learning: In Solidity, int256(1) when passed through the maximize function in LibDecimalFloatImplementation.sol produces exactly (1e76, -76), not an approximation. This means the special case for signedCoefficient == 1e76 in log10 correctly handles powers of 10 like log10(1).
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.soltest/src/lib/LibDecimalFloat.pow.t.solsrc/lib/LibDecimalFloat.sol
📚 Learning: 2025-08-29T10:38:26.353Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: src/lib/implementation/LibDecimalFloatImplementation.sol:593-601
Timestamp: 2025-08-29T10:38:26.353Z
Learning: The maximize function in LibDecimalFloatImplementation.sol produces exact results for simple integer values like 1. maximize(1, 0) yields exactly (1e76, -76) with no precision loss, and the log10 special case for signedCoefficient == 1e76 correctly handles this.
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.soltest/src/lib/LibDecimalFloat.pow.t.solsrc/lib/LibDecimalFloat.sol
📚 Learning: 2025-08-29T14:54:24.240Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: test/src/lib/LibDecimalFloat.decimal.t.sol:122-126
Timestamp: 2025-08-29T14:54:24.240Z
Learning: In test/src/lib/LibDecimalFloat.decimal.t.sol, the fromFixedDecimalLossy function converts uint256(type(int256).max) losslessly because the boundary check is `value > uint256(type(int256).max)`, not `>=`. Values exactly equal to type(int256).max still take the lossless conversion path.
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.sol
📚 Learning: 2025-08-11T14:30:48.562Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#96
File: test/src/lib/LibDecimalFloat.ceil.t.sol:43-50
Timestamp: 2025-08-11T14:30:48.562Z
Learning: When reviewing Solidity test files using Forge, verify the actual mutability of helper functions like `bound()` before suggesting changes to function mutability specifiers, as Forge implements many test utilities as pure functions.
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.soltest/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-09-10T19:20:35.927Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#135
File: test/src/lib/LibDecimalFloat.pow.t.sol:160-161
Timestamp: 2025-09-10T19:20:35.927Z
Learning: In test/src/lib/LibDecimalFloat.pow.t.sol, the comment "we do not try" in the context of "The round trip should not error so we do not try" refers to not using try/catch constructs around the powExternal call, contrasting with previous versions that used nested try/catch blocks.
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.solsrc/lib/LibDecimalFloat.sol
📚 Learning: 2025-08-29T14:58:50.500Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: src/lib/implementation/LibDecimalFloatImplementation.sol:896-899
Timestamp: 2025-08-29T14:58:50.500Z
Learning: In unchecked Solidity blocks, arithmetic operations can overflow/underflow and wrap around, so bounds checks that seem "impossible" for normal arithmetic may actually be necessary to catch overflow edge cases. For example, in withTargetExponent function, the check `exponentDiff < 0` is needed because `targetExponent - exponent` could underflow in unchecked arithmetic.
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.sol
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: rainix (ubuntu-latest, test-wasm-build)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-test)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-legal)
- GitHub Check: rainix (macos-latest, rainix-sol-legal)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-test)
- GitHub Check: rainix (macos-latest, rainix-rs-test)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-static)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-static)
- GitHub Check: git-clean
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/lib/LibDecimalFloat.sol (1)
670-685: Reinstate the negative-base guard before touchingb.Because we defer the negative-base check until after we unpack and process
b, the new code can throwWithTargetExponentOverflowinstead ofLog10Negative(see Rainix CI failure “Negative exponent error handling failed…”). We need to revert with the original error before we start any of the new exponentiation logic.if (b.isZero()) { return FLOAT_ONE; } else if (signedCoefficientA == 0) { if (b.lt(FLOAT_ZERO)) { // If b is negative, and a is 0, so we revert. revert ZeroNegativePower(b); } // If a is zero, then a^b is always zero, regardless of b. // This is a special case because log10(0) is undefined. return FLOAT_ZERO; - } - // Handle identity case for positive values of a, i.e. a^1. - else if (b.eq(FLOAT_ONE) && a.gt(FLOAT_ZERO)) { + } else if (signedCoefficientA < 0) { + revert Log10Negative(signedCoefficientA, exponentA); + } + // Handle identity case for positive values of a, i.e. a^1. + else if (b.eq(FLOAT_ONE) && a.gt(FLOAT_ZERO)) { return a; }src/lib/table/LibLogTable.sol (2)
13-33: Update log-table encoding to 1800 bytes.After shrinking
logTableDecto 90 rows, the payload is now 90×10×2 = 1800 bytes, yet we still reserve/write 1820. The loop therefore runs 910 iterations and reads tenuint16s past the end of the array, publishing an 1820-byte blob that includes uninitialized padding. Once the padding changes, lookups at the tail will start returning junk. Please adjust the free-memory bump and recorded length to 1800 so we only copy real table data.- mstore(0x40, add(encoded, add(1820, 0x20))) + mstore(0x40, add(encoded, add(1800, 0x20))) … - mstore(cursor, 1820) + mstore(cursor, 1800)
41-61: Correct small-table byte sizing to avoid trailing junk.The small table now stores 90×10×1 = 900 bytes, but we still allocate/emit 910. That again forces ten extra iterations reading beyond the array and misaligns the downstream offsets. Please bump the constants down to 900 so the serialized data matches the actual table.
- mstore(0x40, add(encoded, add(910, 0x20))) + mstore(0x40, add(encoded, add(900, 0x20))) … - mstore(cursor, 910) + mstore(cursor, 900)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
src/error/ErrDecimalFloat.sol(1 hunks)src/lib/LibDecimalFloat.sol(2 hunks)src/lib/implementation/LibDecimalFloatImplementation.sol(3 hunks)src/lib/table/LibLogTable.sol(5 hunks)test/src/lib/LibDecimalFloat.pow.t.sol(1 hunks)test/src/lib/implementation/LibDecimalFloatImplementation.lookupLogTableVal.t.sol(1 hunks)
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#135
File: test/src/lib/LibDecimalFloat.pow.t.sol:160-161
Timestamp: 2025-09-10T19:20:35.927Z
Learning: In test/src/lib/LibDecimalFloat.pow.t.sol, the comment "we do not try" in the context of "The round trip should not error so we do not try" refers to not using try/catch constructs around the powExternal call, contrasting with previous versions that used nested try/catch blocks.
📚 Learning: 2025-08-21T18:03:40.347Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#107
File: test/lib/LibDecimalFloatSlow.sol:37-45
Timestamp: 2025-08-21T18:03:40.347Z
Learning: In test/lib/LibDecimalFloatSlow.sol, the "slow" implementation is intentionally different from the production implementation to serve as an independent reference for fuzzing tests. The goal is to have two different approaches (expensive loops vs optimized jumps) that produce equivalent results, not identical implementations.
Applied to files:
test/src/lib/implementation/LibDecimalFloatImplementation.lookupLogTableVal.t.soltest/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-08-29T14:54:24.240Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: test/src/lib/LibDecimalFloat.decimal.t.sol:122-126
Timestamp: 2025-08-29T14:54:24.240Z
Learning: In test/src/lib/LibDecimalFloat.decimal.t.sol, the fromFixedDecimalLossy function converts uint256(type(int256).max) losslessly because the boundary check is `value > uint256(type(int256).max)`, not `>=`. Values exactly equal to type(int256).max still take the lossless conversion path.
Applied to files:
test/src/lib/implementation/LibDecimalFloatImplementation.lookupLogTableVal.t.solsrc/lib/implementation/LibDecimalFloatImplementation.sol
📚 Learning: 2025-09-10T19:20:35.927Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#135
File: test/src/lib/LibDecimalFloat.pow.t.sol:160-161
Timestamp: 2025-09-10T19:20:35.927Z
Learning: In test/src/lib/LibDecimalFloat.pow.t.sol, the comment "we do not try" in the context of "The round trip should not error so we do not try" refers to not using try/catch constructs around the powExternal call, contrasting with previous versions that used nested try/catch blocks.
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.solsrc/lib/LibDecimalFloat.sol
📚 Learning: 2025-08-11T14:30:48.562Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#96
File: test/src/lib/LibDecimalFloat.ceil.t.sol:43-50
Timestamp: 2025-08-11T14:30:48.562Z
Learning: When reviewing Solidity test files using Forge, verify the actual mutability of helper functions like `bound()` before suggesting changes to function mutability specifiers, as Forge implements many test utilities as pure functions.
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.solsrc/lib/implementation/LibDecimalFloatImplementation.sol
📚 Learning: 2025-08-29T14:58:50.500Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: src/lib/implementation/LibDecimalFloatImplementation.sol:896-899
Timestamp: 2025-08-29T14:58:50.500Z
Learning: In unchecked Solidity blocks, arithmetic operations can overflow/underflow and wrap around, so bounds checks that seem "impossible" for normal arithmetic may actually be necessary to catch overflow edge cases. For example, in withTargetExponent function, the check `exponentDiff < 0` is needed because `targetExponent - exponent` could underflow in unchecked arithmetic.
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-08-29T10:38:26.353Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: src/lib/implementation/LibDecimalFloatImplementation.sol:593-601
Timestamp: 2025-08-29T10:38:26.353Z
Learning: In Solidity, int256(1) when passed through the maximize function in LibDecimalFloatImplementation.sol produces exactly (1e76, -76), not an approximation. This means the special case for signedCoefficient == 1e76 in log10 correctly handles powers of 10 like log10(1).
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.solsrc/lib/implementation/LibDecimalFloatImplementation.solsrc/lib/LibDecimalFloat.sol
📚 Learning: 2025-08-29T10:38:26.353Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: src/lib/implementation/LibDecimalFloatImplementation.sol:593-601
Timestamp: 2025-08-29T10:38:26.353Z
Learning: The maximize function in LibDecimalFloatImplementation.sol produces exact results for simple integer values like 1. maximize(1, 0) yields exactly (1e76, -76) with no precision loss, and the log10 special case for signedCoefficient == 1e76 correctly handles this.
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.solsrc/lib/implementation/LibDecimalFloatImplementation.solsrc/lib/LibDecimalFloat.sol
📚 Learning: 2025-09-09T15:11:31.003Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#130
File: src/lib/implementation/LibDecimalFloatImplementation.sol:320-330
Timestamp: 2025-09-09T15:11:31.003Z
Learning: In LibDecimalFloatImplementation.sol div function, returning zero when underflowExponentBy > 76 is intentional "non-error underflow" behavior rather than reverting. This graceful handling is the main goal of the changes.
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.solsrc/error/ErrDecimalFloat.solsrc/lib/LibDecimalFloat.sol
🪛 GitHub Actions: Rainix CI
test/src/lib/LibDecimalFloat.pow.t.sol
[error] 1-1: Pow test failure: Negative exponent error handling failed. Expected 'Log10Negative' error but received 'WithTargetExponentOverflow'.
[error] 1-1: Pow test failure: Arithmetic underflow/overflow panic during round-trip fuzzed power computation in testRoundTripFuzzPow.
[error] 1-1: Division by zero error observed in testRoundTripSimple during power-related tests.
src/lib/implementation/LibDecimalFloatImplementation.sol
[warning] 689-714: Slither detector: divide-before-multiply issue in lookupLogTableVal(address,uint256). Consider reordering operations to avoid potential precision/overflow problems. Reference: Slither detector docs.
[warning] 727-832: Slither detector: divide-before-multiply issue in log10(address,int256,int256).y2Coefficient path. Potential precision/overflow risk due to multiplication after division.
[warning] 755-755: Slither detector: uninitialized local variable y2Coefficient referenced in LibDecimalFloatImplementation.log10(address,int256,int256).
🔇 Additional comments (1)
src/error/ErrDecimalFloat.sol (1)
45-46: Log-table bounds error looks goodThis pairs cleanly with the new guard in
lookupLogTableVal, so callers will see the dedicated revert instead of a generic panic.
test/src/lib/implementation/LibDecimalFloatImplementation.lookupLogTableVal.t.sol
Show resolved
Hide resolved
…float into 2025-09-25-round
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
src/error/ErrDecimalFloat.sol(1 hunks)src/lib/LibDecimalFloat.sol(2 hunks)test/src/lib/LibDecimalFloat.pow.t.sol(3 hunks)test/src/lib/LibDecimalFloat.sqrt.t.sol(2 hunks)
🧰 Additional context used
🧠 Learnings (8)
📚 Learning: 2025-09-09T15:11:31.003Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#130
File: src/lib/implementation/LibDecimalFloatImplementation.sol:320-330
Timestamp: 2025-09-09T15:11:31.003Z
Learning: In LibDecimalFloatImplementation.sol div function, returning zero when underflowExponentBy > 76 is intentional "non-error underflow" behavior rather than reverting. This graceful handling is the main goal of the changes.
Applied to files:
test/src/lib/LibDecimalFloat.sqrt.t.solsrc/lib/LibDecimalFloat.solsrc/error/ErrDecimalFloat.soltest/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-09-10T19:20:35.927Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#135
File: test/src/lib/LibDecimalFloat.pow.t.sol:160-161
Timestamp: 2025-09-10T19:20:35.927Z
Learning: In test/src/lib/LibDecimalFloat.pow.t.sol, the comment "we do not try" in the context of "The round trip should not error so we do not try" refers to not using try/catch constructs around the powExternal call, contrasting with previous versions that used nested try/catch blocks.
Applied to files:
test/src/lib/LibDecimalFloat.sqrt.t.solsrc/lib/LibDecimalFloat.soltest/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-08-21T18:03:40.347Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#107
File: test/lib/LibDecimalFloatSlow.sol:37-45
Timestamp: 2025-08-21T18:03:40.347Z
Learning: In test/lib/LibDecimalFloatSlow.sol, the "slow" implementation is intentionally different from the production implementation to serve as an independent reference for fuzzing tests. The goal is to have two different approaches (expensive loops vs optimized jumps) that produce equivalent results, not identical implementations.
Applied to files:
test/src/lib/LibDecimalFloat.sqrt.t.solsrc/lib/LibDecimalFloat.soltest/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-08-29T10:38:26.353Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: src/lib/implementation/LibDecimalFloatImplementation.sol:593-601
Timestamp: 2025-08-29T10:38:26.353Z
Learning: In Solidity, int256(1) when passed through the maximize function in LibDecimalFloatImplementation.sol produces exactly (1e76, -76), not an approximation. This means the special case for signedCoefficient == 1e76 in log10 correctly handles powers of 10 like log10(1).
Applied to files:
src/lib/LibDecimalFloat.soltest/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-08-29T10:38:26.353Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: src/lib/implementation/LibDecimalFloatImplementation.sol:593-601
Timestamp: 2025-08-29T10:38:26.353Z
Learning: The maximize function in LibDecimalFloatImplementation.sol produces exact results for simple integer values like 1. maximize(1, 0) yields exactly (1e76, -76) with no precision loss, and the log10 special case for signedCoefficient == 1e76 correctly handles this.
Applied to files:
src/lib/LibDecimalFloat.soltest/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-08-29T14:58:50.500Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: src/lib/implementation/LibDecimalFloatImplementation.sol:896-899
Timestamp: 2025-08-29T14:58:50.500Z
Learning: In unchecked Solidity blocks, arithmetic operations can overflow/underflow and wrap around, so bounds checks that seem "impossible" for normal arithmetic may actually be necessary to catch overflow edge cases. For example, in withTargetExponent function, the check `exponentDiff < 0` is needed because `targetExponent - exponent` could underflow in unchecked arithmetic.
Applied to files:
src/lib/LibDecimalFloat.soltest/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-08-29T14:54:24.240Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: test/src/lib/LibDecimalFloat.decimal.t.sol:122-126
Timestamp: 2025-08-29T14:54:24.240Z
Learning: In test/src/lib/LibDecimalFloat.decimal.t.sol, the fromFixedDecimalLossy function converts uint256(type(int256).max) losslessly because the boundary check is `value > uint256(type(int256).max)`, not `>=`. Values exactly equal to type(int256).max still take the lossless conversion path.
Applied to files:
src/lib/LibDecimalFloat.soltest/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-08-11T14:30:48.562Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#96
File: test/src/lib/LibDecimalFloat.ceil.t.sol:43-50
Timestamp: 2025-08-11T14:30:48.562Z
Learning: When reviewing Solidity test files using Forge, verify the actual mutability of helper functions like `bound()` before suggesting changes to function mutability specifiers, as Forge implements many test utilities as pure functions.
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.sol
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-static)
- GitHub Check: rainix (ubuntu-latest, test-wasm-build)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-static)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-test)
- GitHub Check: rainix (macos-latest, rainix-sol-legal)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-test)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-legal)
- GitHub Check: rainix (macos-latest, rainix-rs-test)
- GitHub Check: git-clean
🔇 Additional comments (8)
src/error/ErrDecimalFloat.sol (1)
45-49: New errors look good; public API clear.
LogTableIndexOutOfBounds(uint256)andPowNegativeBase(int256,int256)are appropriately specific and will aid downstream handling.test/src/lib/LibDecimalFloat.sqrt.t.sol (1)
71-71: Correct revert expectation for negative input.Expecting
PowNegativeBasefor sqrt on negative inputs is consistent with the updated pow semantics.src/lib/LibDecimalFloat.sol (2)
19-21: Imports updated appropriately.Bringing
ZeroNegativePowerandPowNegativeBaseinto the library scope is correct for the new error model.
694-714: Fix integer-exponent handling: avoid uint cast on possibly negative value; handle sign via inversion.Casting the integer part of
btouint256risks wraparound ifwithTargetExponent(...)returns a negative (e.g., via underflow in unchecked math). Also, negative integer exponents should invert the base before exponentiation. Apply this change to keep the code robust even if the earlyb < 0branch changes in future and to prevent CI-reported underflow regressions. Based on learnings.- uint256 exponentBInteger = - uint256(LibDecimalFloatImplementation.withTargetExponent(characteristicB, exponentB, 0)); + int256 exponentBIntegerSigned = + LibDecimalFloatImplementation.withTargetExponent(characteristicB, exponentB, 0); + bool exponentBIntegerNegative = exponentBIntegerSigned < 0; + uint256 exponentBInteger = + LibDecimalFloatImplementation.absUnsignedSignedCoefficient(exponentBIntegerSigned); // Exponentiation by squaring. - (int256 signedCoefficientResult, int256 exponentResult) = (1, 0); - (int256 signedCoefficientBase, int256 exponentBase) = a.unpack(); - while (exponentBInteger >= 1) { - if (exponentBInteger & 0x01 == 0x01) { + (int256 signedCoefficientResult, int256 exponentResult) = (1, 0); + int256 signedCoefficientBase = signedCoefficientA; + int256 exponentBase = exponentA; + if (exponentBIntegerNegative && exponentBInteger != 0) { + (signedCoefficientBase, exponentBase) = LibDecimalFloatImplementation.inv( + signedCoefficientBase, exponentBase + ); + } + while (exponentBInteger != 0) { + if ((exponentBInteger & 0x01) == 0x01) { (signedCoefficientResult, exponentResult) = LibDecimalFloatImplementation.mul( signedCoefficientResult, exponentResult, signedCoefficientBase, exponentBase ); } exponentBInteger >>= 1; (signedCoefficientBase, exponentBase) = LibDecimalFloatImplementation.mul( signedCoefficientBase, exponentBase, signedCoefficientBase, exponentBase ); }test/src/lib/LibDecimalFloat.pow.t.sol (4)
8-8: New error type imported correctlyThe import of
PowNegativeBaseis appropriate for the updated pow implementation.
40-67: Pow test cases updated with new expected valuesThe test values have been updated to reflect the new exponentiation-by-squaring implementation. The coefficients and exponents appear consistent with the mathematical expectations for the given inputs.
87-87: Error handling for negative bases confirmed VerifiedPowNegativeBaseis defined insrc/error/ErrDecimalFloat.sol:49and consistently used in both pow (src/lib/LibDecimalFloat.sol:684,test/src/lib/LibDecimalFloat.pow.t.sol:87) and sqrt (test/src/lib/LibDecimalFloat.sqrt.t.sol:71) scenarios.
150-160: Ensure pow round-trip tests still pass
AlltestRoundTripSimpleandtestRoundTripFuzzPowcases remain enabled—please run the full test suite (e.g.forge test) and confirm there are no precision regressions under the new exponentiation-by-squaring implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
.gas-snapshot(2 hunks)test/src/lib/LibDecimalFloat.pow.t.sol(3 hunks)test/src/lib/LibDecimalFloat.sqrt.t.sol(2 hunks)
🧰 Additional context used
🧠 Learnings (14)
📓 Common learnings
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#144
File: src/lib/LibDecimalFloat.sol:671-686
Timestamp: 2025-09-28T12:52:45.793Z
Learning: In LibDecimalFloat.sol, the pow function has always rejected negative bases and reverted with an error. Prior to recent changes, it used a different error type, but the fundamental policy of not allowing negative bases (except when b == 0) has been consistently maintained across the codebase's history.
📚 Learning: 2025-09-28T12:52:45.793Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#144
File: src/lib/LibDecimalFloat.sol:671-686
Timestamp: 2025-09-28T12:52:45.793Z
Learning: In LibDecimalFloat.sol, the pow function has always rejected negative bases and reverted with an error. Prior to recent changes, it used a different error type, but the fundamental policy of not allowing negative bases (except when b == 0) has been consistently maintained across the codebase's history.
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.soltest/src/lib/LibDecimalFloat.sqrt.t.sol
📚 Learning: 2025-09-10T19:20:35.927Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#135
File: test/src/lib/LibDecimalFloat.pow.t.sol:160-161
Timestamp: 2025-09-10T19:20:35.927Z
Learning: In test/src/lib/LibDecimalFloat.pow.t.sol, the comment "we do not try" in the context of "The round trip should not error so we do not try" refers to not using try/catch constructs around the powExternal call, contrasting with previous versions that used nested try/catch blocks.
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.soltest/src/lib/LibDecimalFloat.sqrt.t.sol
📚 Learning: 2025-08-21T18:03:40.347Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#107
File: test/lib/LibDecimalFloatSlow.sol:37-45
Timestamp: 2025-08-21T18:03:40.347Z
Learning: In test/lib/LibDecimalFloatSlow.sol, the "slow" implementation is intentionally different from the production implementation to serve as an independent reference for fuzzing tests. The goal is to have two different approaches (expensive loops vs optimized jumps) that produce equivalent results, not identical implementations.
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.soltest/src/lib/LibDecimalFloat.sqrt.t.sol.gas-snapshot
📚 Learning: 2025-09-09T15:11:31.003Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#130
File: src/lib/implementation/LibDecimalFloatImplementation.sol:320-330
Timestamp: 2025-09-09T15:11:31.003Z
Learning: In LibDecimalFloatImplementation.sol div function, returning zero when underflowExponentBy > 76 is intentional "non-error underflow" behavior rather than reverting. This graceful handling is the main goal of the changes.
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.soltest/src/lib/LibDecimalFloat.sqrt.t.sol
📚 Learning: 2025-08-29T14:54:24.240Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: test/src/lib/LibDecimalFloat.decimal.t.sol:122-126
Timestamp: 2025-08-29T14:54:24.240Z
Learning: In test/src/lib/LibDecimalFloat.decimal.t.sol, the fromFixedDecimalLossy function converts uint256(type(int256).max) losslessly because the boundary check is `value > uint256(type(int256).max)`, not `>=`. Values exactly equal to type(int256).max still take the lossless conversion path.
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.soltest/src/lib/LibDecimalFloat.sqrt.t.sol.gas-snapshot
📚 Learning: 2025-08-11T14:30:48.562Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#96
File: test/src/lib/LibDecimalFloat.ceil.t.sol:43-50
Timestamp: 2025-08-11T14:30:48.562Z
Learning: When reviewing Solidity test files using Forge, verify the actual mutability of helper functions like `bound()` before suggesting changes to function mutability specifiers, as Forge implements many test utilities as pure functions.
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-08-11T14:32:50.439Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#96
File: test/src/lib/implementation/LibDecimalFloatImplementation.maximize.t.sol:15-29
Timestamp: 2025-08-11T14:32:50.439Z
Learning: In test code for the rain.math.float repository, redundant checks may be intentionally kept for clarity and documentation purposes, even when they could be simplified. The maintainer (thedavidmeister) prefers explicit assertions in test code to make the test's intent clear to future readers, prioritizing readability over conciseness.
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-08-14T16:32:05.932Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#99
File: src/lib/implementation/LibDecimalFloatImplementation.sol:309-325
Timestamp: 2025-08-14T16:32:05.932Z
Learning: In the rainlanguage/rain.math.float repository, maintainer thedavidmeister prefers to keep assembly-based overflow checks inline for gas optimization rather than extracting them into helper functions, even when it results in code duplication.
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-08-14T16:56:28.978Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#99
File: src/lib/implementation/LibDecimalFloatImplementation.sol:574-581
Timestamp: 2025-08-14T16:56:28.978Z
Learning: In the rainlanguage/rain.math.float repository, maintainer thedavidmeister avoids using `&&` operators in gas-critical paths because they involve jumps in Solidity due to short-circuit evaluation, preferring approaches like mul-then-div overflow probes that avoid conditional jump overhead.
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-08-29T14:58:50.500Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: src/lib/implementation/LibDecimalFloatImplementation.sol:896-899
Timestamp: 2025-08-29T14:58:50.500Z
Learning: In unchecked Solidity blocks, arithmetic operations can overflow/underflow and wrap around, so bounds checks that seem "impossible" for normal arithmetic may actually be necessary to catch overflow edge cases. For example, in withTargetExponent function, the check `exponentDiff < 0` is needed because `targetExponent - exponent` could underflow in unchecked arithmetic.
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-08-29T10:38:26.353Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: src/lib/implementation/LibDecimalFloatImplementation.sol:593-601
Timestamp: 2025-08-29T10:38:26.353Z
Learning: In Solidity, int256(1) when passed through the maximize function in LibDecimalFloatImplementation.sol produces exactly (1e76, -76), not an approximation. This means the special case for signedCoefficient == 1e76 in log10 correctly handles powers of 10 like log10(1).
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.soltest/src/lib/LibDecimalFloat.sqrt.t.sol.gas-snapshot
📚 Learning: 2025-08-29T10:38:26.353Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: src/lib/implementation/LibDecimalFloatImplementation.sol:593-601
Timestamp: 2025-08-29T10:38:26.353Z
Learning: The maximize function in LibDecimalFloatImplementation.sol produces exact results for simple integer values like 1. maximize(1, 0) yields exactly (1e76, -76) with no precision loss, and the log10 special case for signedCoefficient == 1e76 correctly handles this.
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.soltest/src/lib/LibDecimalFloat.sqrt.t.sol
📚 Learning: 2025-09-02T09:33:32.513Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#119
File: test/src/lib/format/LibFormatDecimalFloat.toDecimalString.t.sol:174-175
Timestamp: 2025-09-02T09:33:32.513Z
Learning: The LibFormatDecimalFloat.toDecimalString function in src/lib/format/LibFormatDecimalFloat.sol does not include rounding logic. It formats decimal floats as-is without rounding values based on significant figures limits.
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.soltest/src/lib/LibDecimalFloat.sqrt.t.sol.gas-snapshot
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: git-clean
- GitHub Check: rainix (ubuntu-latest, rainix-sol-test)
- GitHub Check: rainix (ubuntu-latest, test-wasm-build)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-test)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-legal)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-static)
- GitHub Check: rainix (macos-latest, rainix-sol-legal)
- GitHub Check: rainix (macos-latest, rainix-rs-test)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-static)
🔇 Additional comments (6)
test/src/lib/LibDecimalFloat.sqrt.t.sol (3)
16-16: Tighter sqrt round‑trip tolerance LGTM.0.0017 (17e-4) is a solid, conservative bound for sqrt round trips.
71-73: Correct revert expectation for negative sqrt.Expecting PowNegativeBase with unpacked (signedCoefficient, exponent) matches the updated error contract.
Based on learnings
8-10: Prune unused imports; keep only PowNegativeBase.ZeroNegativePower, Log10Negative, and LibDecimalFloatImplementation aren’t referenced in this file; drop them to keep the test lean.
-import {ZeroNegativePower, Log10Negative, PowNegativeBase} from "src/error/ErrDecimalFloat.sol"; -import {LibDecimalFloatImplementation} from "src/lib/implementation/LibDecimalFloatImplementation.sol"; +import {PowNegativeBase} from "src/error/ErrDecimalFloat.sol";test/src/lib/LibDecimalFloat.pow.t.sol (3)
39-45: Deterministic baselines for simple cases look correct.5^13 and 0.5^30/60 expectations match known values and encoding approach.
87-89: Negative base now reverts with PowNegativeBase — correct.This matches the long‑standing semantics (negative bases reject for all b ≠ 0).
Based on learnings
111-116: 0^b with b<0 expecting ZeroNegativePower — correct.Error expectation aligns with current contract.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
test/src/lib/LibDecimalFloat.pow.t.sol(4 hunks)
🧰 Additional context used
🧠 Learnings (15)
📓 Common learnings
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#144
File: src/lib/LibDecimalFloat.sol:671-686
Timestamp: 2025-09-28T12:52:45.793Z
Learning: In LibDecimalFloat.sol, the pow function has always rejected negative bases and reverted with an error. Prior to recent changes, it used a different error type, but the fundamental policy of not allowing negative bases (except when b == 0) has been consistently maintained across the codebase's history.
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#144
File: test/src/lib/LibDecimalFloat.pow.t.sol:40-61
Timestamp: 2025-09-28T13:28:35.804Z
Learning: In test/src/lib/LibDecimalFloat.pow.t.sol, the diffLimit tolerance (currently 8.6%) represents a conservative worst-case bound to accommodate precision limitations of table lookup interpolation for fractional exponents in the pow function. The exponentiation-by-squaring approach used for integer parts achieves much tighter precision, but the tolerance must account for the less precise table-based calculations used for fractional exponents.
📚 Learning: 2025-09-28T12:52:45.793Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#144
File: src/lib/LibDecimalFloat.sol:671-686
Timestamp: 2025-09-28T12:52:45.793Z
Learning: In LibDecimalFloat.sol, the pow function has always rejected negative bases and reverted with an error. Prior to recent changes, it used a different error type, but the fundamental policy of not allowing negative bases (except when b == 0) has been consistently maintained across the codebase's history.
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-09-28T13:28:35.804Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#144
File: test/src/lib/LibDecimalFloat.pow.t.sol:40-61
Timestamp: 2025-09-28T13:28:35.804Z
Learning: In test/src/lib/LibDecimalFloat.pow.t.sol, the diffLimit tolerance (currently 8.6%) represents a conservative worst-case bound to accommodate precision limitations of table lookup interpolation for fractional exponents in the pow function. The exponentiation-by-squaring approach used for integer parts achieves much tighter precision, but the tolerance must account for the less precise table-based calculations used for fractional exponents.
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-09-10T19:20:35.927Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#135
File: test/src/lib/LibDecimalFloat.pow.t.sol:160-161
Timestamp: 2025-09-10T19:20:35.927Z
Learning: In test/src/lib/LibDecimalFloat.pow.t.sol, the comment "we do not try" in the context of "The round trip should not error so we do not try" refers to not using try/catch constructs around the powExternal call, contrasting with previous versions that used nested try/catch blocks.
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-08-21T18:03:40.347Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#107
File: test/lib/LibDecimalFloatSlow.sol:37-45
Timestamp: 2025-08-21T18:03:40.347Z
Learning: In test/lib/LibDecimalFloatSlow.sol, the "slow" implementation is intentionally different from the production implementation to serve as an independent reference for fuzzing tests. The goal is to have two different approaches (expensive loops vs optimized jumps) that produce equivalent results, not identical implementations.
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-09-09T15:11:31.003Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#130
File: src/lib/implementation/LibDecimalFloatImplementation.sol:320-330
Timestamp: 2025-09-09T15:11:31.003Z
Learning: In LibDecimalFloatImplementation.sol div function, returning zero when underflowExponentBy > 76 is intentional "non-error underflow" behavior rather than reverting. This graceful handling is the main goal of the changes.
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-08-29T14:54:24.240Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: test/src/lib/LibDecimalFloat.decimal.t.sol:122-126
Timestamp: 2025-08-29T14:54:24.240Z
Learning: In test/src/lib/LibDecimalFloat.decimal.t.sol, the fromFixedDecimalLossy function converts uint256(type(int256).max) losslessly because the boundary check is `value > uint256(type(int256).max)`, not `>=`. Values exactly equal to type(int256).max still take the lossless conversion path.
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-08-11T14:30:48.562Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#96
File: test/src/lib/LibDecimalFloat.ceil.t.sol:43-50
Timestamp: 2025-08-11T14:30:48.562Z
Learning: When reviewing Solidity test files using Forge, verify the actual mutability of helper functions like `bound()` before suggesting changes to function mutability specifiers, as Forge implements many test utilities as pure functions.
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-08-11T14:32:50.439Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#96
File: test/src/lib/implementation/LibDecimalFloatImplementation.maximize.t.sol:15-29
Timestamp: 2025-08-11T14:32:50.439Z
Learning: In test code for the rain.math.float repository, redundant checks may be intentionally kept for clarity and documentation purposes, even when they could be simplified. The maintainer (thedavidmeister) prefers explicit assertions in test code to make the test's intent clear to future readers, prioritizing readability over conciseness.
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-08-14T16:32:05.932Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#99
File: src/lib/implementation/LibDecimalFloatImplementation.sol:309-325
Timestamp: 2025-08-14T16:32:05.932Z
Learning: In the rainlanguage/rain.math.float repository, maintainer thedavidmeister prefers to keep assembly-based overflow checks inline for gas optimization rather than extracting them into helper functions, even when it results in code duplication.
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-08-14T16:56:28.978Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#99
File: src/lib/implementation/LibDecimalFloatImplementation.sol:574-581
Timestamp: 2025-08-14T16:56:28.978Z
Learning: In the rainlanguage/rain.math.float repository, maintainer thedavidmeister avoids using `&&` operators in gas-critical paths because they involve jumps in Solidity due to short-circuit evaluation, preferring approaches like mul-then-div overflow probes that avoid conditional jump overhead.
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-08-29T14:58:50.500Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: src/lib/implementation/LibDecimalFloatImplementation.sol:896-899
Timestamp: 2025-08-29T14:58:50.500Z
Learning: In unchecked Solidity blocks, arithmetic operations can overflow/underflow and wrap around, so bounds checks that seem "impossible" for normal arithmetic may actually be necessary to catch overflow edge cases. For example, in withTargetExponent function, the check `exponentDiff < 0` is needed because `targetExponent - exponent` could underflow in unchecked arithmetic.
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-08-29T10:38:26.353Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: src/lib/implementation/LibDecimalFloatImplementation.sol:593-601
Timestamp: 2025-08-29T10:38:26.353Z
Learning: In Solidity, int256(1) when passed through the maximize function in LibDecimalFloatImplementation.sol produces exactly (1e76, -76), not an approximation. This means the special case for signedCoefficient == 1e76 in log10 correctly handles powers of 10 like log10(1).
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-08-29T10:38:26.353Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: src/lib/implementation/LibDecimalFloatImplementation.sol:593-601
Timestamp: 2025-08-29T10:38:26.353Z
Learning: The maximize function in LibDecimalFloatImplementation.sol produces exact results for simple integer values like 1. maximize(1, 0) yields exactly (1e76, -76) with no precision loss, and the log10 special case for signedCoefficient == 1e76 correctly handles this.
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-09-02T09:33:32.513Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#119
File: test/src/lib/format/LibFormatDecimalFloat.toDecimalString.t.sol:174-175
Timestamp: 2025-09-02T09:33:32.513Z
Learning: The LibFormatDecimalFloat.toDecimalString function in src/lib/format/LibFormatDecimalFloat.sol does not include rounding logic. It formats decimal floats as-is without rounding values based on significant figures limits.
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.sol
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-test)
- GitHub Check: rainix (ubuntu-latest, test-wasm-build)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-static)
- GitHub Check: rainix (macos-latest, rainix-rs-test)
- GitHub Check: git-clean
- GitHub Check: rainix (macos-latest, rainix-sol-legal)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-test)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-static)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-legal)
🔇 Additional comments (6)
test/src/lib/LibDecimalFloat.pow.t.sol (6)
15-17: diffLimit update to 86e-3 looks good.Tightened worst‑case tolerance matches current approach.
Based on learnings
46-61: Large hard‑coded mantissas are fragile to rounding changes.Keep if you prefer explicitness, but consider switching to tolerance/independent reference if they start flapping.
Based on learnings
87-87: Expecting PowNegativeBase is correct.Matches current semantics for negative bases.
Based on learnings
149-150: Doc now references diffLimit — aligned.Thanks for removing the stale 0.25% statement.
8-10: Remove unused imports
DropLog10NegativeandLibDecimalFloatImplementation— they’re never referenced in this test.-import {ZeroNegativePower, Log10Negative, PowNegativeBase} from "src/error/ErrDecimalFloat.sol"; +import {ZeroNegativePower, PowNegativeBase} from "src/error/ErrDecimalFloat.sol"; -import {LibDecimalFloatImplementation} from "src/lib/implementation/LibDecimalFloatImplementation.sol";
40-45: Consider tolerance-based asserts for fractional‑base pow cases.These constants (0.5^30, 0.5^60) can flap with table tweaks; asserting within diffLimit would reduce brittleness.
Based on learnings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
flake.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
src/lib/LibDecimalFloat.sol(7 hunks)src/lib/format/LibFormatDecimalFloat.sol(2 hunks)src/lib/implementation/LibDecimalFloatImplementation.sol(8 hunks)test/src/lib/LibDecimalFloat.max.t.sol(1 hunks)test/src/lib/implementation/LibDecimalFloatImplementation.unabsUnsignedMulOrDivLossy.t.sol(1 hunks)test/src/lib/implementation/LibDecimalFloatImplementation.withTargetExponent.t.sol(1 hunks)
🧰 Additional context used
🧠 Learnings (20)
📓 Common learnings
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#144
File: src/lib/LibDecimalFloat.sol:671-686
Timestamp: 2025-09-28T12:52:45.793Z
Learning: In LibDecimalFloat.sol, the pow function has always rejected negative bases and reverted with an error. Prior to recent changes, it used a different error type, but the fundamental policy of not allowing negative bases (except when b == 0) has been consistently maintained across the codebase's history.
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#144
File: test/src/lib/LibDecimalFloat.pow.t.sol:40-61
Timestamp: 2025-09-28T13:28:35.804Z
Learning: In test/src/lib/LibDecimalFloat.pow.t.sol, the diffLimit tolerance (currently 8.6%) represents a conservative worst-case bound to accommodate precision limitations of table lookup interpolation for fractional exponents in the pow function. The exponentiation-by-squaring approach used for integer parts achieves much tighter precision, but the tolerance must account for the less precise table-based calculations used for fractional exponents.
📚 Learning: 2025-08-11T14:30:48.562Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#96
File: test/src/lib/LibDecimalFloat.ceil.t.sol:43-50
Timestamp: 2025-08-11T14:30:48.562Z
Learning: When reviewing Solidity test files using Forge, verify the actual mutability of helper functions like `bound()` before suggesting changes to function mutability specifiers, as Forge implements many test utilities as pure functions.
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.sol
📚 Learning: 2025-08-29T10:38:26.353Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: src/lib/implementation/LibDecimalFloatImplementation.sol:593-601
Timestamp: 2025-08-29T10:38:26.353Z
Learning: In Solidity, int256(1) when passed through the maximize function in LibDecimalFloatImplementation.sol produces exactly (1e76, -76), not an approximation. This means the special case for signedCoefficient == 1e76 in log10 correctly handles powers of 10 like log10(1).
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.soltest/src/lib/implementation/LibDecimalFloatImplementation.withTargetExponent.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.unabsUnsignedMulOrDivLossy.t.solsrc/lib/LibDecimalFloat.sol
📚 Learning: 2025-09-09T15:11:31.003Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#130
File: src/lib/implementation/LibDecimalFloatImplementation.sol:320-330
Timestamp: 2025-09-09T15:11:31.003Z
Learning: In LibDecimalFloatImplementation.sol div function, returning zero when underflowExponentBy > 76 is intentional "non-error underflow" behavior rather than reverting. This graceful handling is the main goal of the changes.
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.soltest/src/lib/implementation/LibDecimalFloatImplementation.withTargetExponent.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.unabsUnsignedMulOrDivLossy.t.solsrc/lib/format/LibFormatDecimalFloat.solsrc/lib/LibDecimalFloat.sol
📚 Learning: 2025-08-29T14:54:24.240Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: test/src/lib/LibDecimalFloat.decimal.t.sol:122-126
Timestamp: 2025-08-29T14:54:24.240Z
Learning: In test/src/lib/LibDecimalFloat.decimal.t.sol, the fromFixedDecimalLossy function converts uint256(type(int256).max) losslessly because the boundary check is `value > uint256(type(int256).max)`, not `>=`. Values exactly equal to type(int256).max still take the lossless conversion path.
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.soltest/src/lib/implementation/LibDecimalFloatImplementation.unabsUnsignedMulOrDivLossy.t.soltest/src/lib/LibDecimalFloat.max.t.solsrc/lib/LibDecimalFloat.sol
📚 Learning: 2025-09-28T13:28:35.804Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#144
File: test/src/lib/LibDecimalFloat.pow.t.sol:40-61
Timestamp: 2025-09-28T13:28:35.804Z
Learning: In test/src/lib/LibDecimalFloat.pow.t.sol, the diffLimit tolerance (currently 8.6%) represents a conservative worst-case bound to accommodate precision limitations of table lookup interpolation for fractional exponents in the pow function. The exponentiation-by-squaring approach used for integer parts achieves much tighter precision, but the tolerance must account for the less precise table-based calculations used for fractional exponents.
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.soltest/src/lib/implementation/LibDecimalFloatImplementation.withTargetExponent.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.unabsUnsignedMulOrDivLossy.t.soltest/src/lib/LibDecimalFloat.max.t.solsrc/lib/LibDecimalFloat.sol
📚 Learning: 2025-06-17T10:17:56.205Z
Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#59
File: crates/float/src/lib.rs:233-242
Timestamp: 2025-06-17T10:17:56.205Z
Learning: In the rainlanguage/rain.math.float repository, the maintainer 0xgleb prefers to handle documentation additions and improvements in separate issues rather than inline with feature PRs.
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.sol
📚 Learning: 2025-08-14T16:32:05.932Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#99
File: src/lib/implementation/LibDecimalFloatImplementation.sol:309-325
Timestamp: 2025-08-14T16:32:05.932Z
Learning: In the rainlanguage/rain.math.float repository, maintainer thedavidmeister prefers to keep assembly-based overflow checks inline for gas optimization rather than extracting them into helper functions, even when it results in code duplication.
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.solsrc/lib/LibDecimalFloat.sol
📚 Learning: 2025-08-27T13:37:22.601Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#111
File: test/src/lib/implementation/LibDecimalFloatImplementation.div.t.sol:126-133
Timestamp: 2025-08-27T13:37:22.601Z
Learning: In the rainlanguage/rain.math.float repository, maintainer thedavidmeister prefers to avoid inline `if` statements (even if they were supported in Solidity) because they create fragile code with meaningful whitespace and make debugging difficult when console logs need to be added, potentially causing subtle behavior changes if braces aren't reintroduced properly.
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.solsrc/lib/LibDecimalFloat.sol
📚 Learning: 2025-08-14T16:56:28.978Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#99
File: src/lib/implementation/LibDecimalFloatImplementation.sol:574-581
Timestamp: 2025-08-14T16:56:28.978Z
Learning: In the rainlanguage/rain.math.float repository, maintainer thedavidmeister avoids using `&&` operators in gas-critical paths because they involve jumps in Solidity due to short-circuit evaluation, preferring approaches like mul-then-div overflow probes that avoid conditional jump overhead.
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.solsrc/lib/LibDecimalFloat.sol
📚 Learning: 2025-08-21T18:03:40.347Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#107
File: test/lib/LibDecimalFloatSlow.sol:37-45
Timestamp: 2025-08-21T18:03:40.347Z
Learning: In test/lib/LibDecimalFloatSlow.sol, the "slow" implementation is intentionally different from the production implementation to serve as an independent reference for fuzzing tests. The goal is to have two different approaches (expensive loops vs optimized jumps) that produce equivalent results, not identical implementations.
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.soltest/src/lib/implementation/LibDecimalFloatImplementation.withTargetExponent.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.unabsUnsignedMulOrDivLossy.t.solsrc/lib/format/LibFormatDecimalFloat.solsrc/lib/LibDecimalFloat.sol
📚 Learning: 2025-08-29T10:38:26.353Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: src/lib/implementation/LibDecimalFloatImplementation.sol:593-601
Timestamp: 2025-08-29T10:38:26.353Z
Learning: The maximize function in LibDecimalFloatImplementation.sol produces exact results for simple integer values like 1. maximize(1, 0) yields exactly (1e76, -76) with no precision loss, and the log10 special case for signedCoefficient == 1e76 correctly handles this.
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.soltest/src/lib/LibDecimalFloat.max.t.solsrc/lib/LibDecimalFloat.sol
📚 Learning: 2025-08-29T14:58:50.500Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: src/lib/implementation/LibDecimalFloatImplementation.sol:896-899
Timestamp: 2025-08-29T14:58:50.500Z
Learning: In unchecked Solidity blocks, arithmetic operations can overflow/underflow and wrap around, so bounds checks that seem "impossible" for normal arithmetic may actually be necessary to catch overflow edge cases. For example, in withTargetExponent function, the check `exponentDiff < 0` is needed because `targetExponent - exponent` could underflow in unchecked arithmetic.
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.soltest/src/lib/implementation/LibDecimalFloatImplementation.withTargetExponent.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.unabsUnsignedMulOrDivLossy.t.solsrc/lib/LibDecimalFloat.sol
📚 Learning: 2025-09-10T19:20:35.927Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#135
File: test/src/lib/LibDecimalFloat.pow.t.sol:160-161
Timestamp: 2025-09-10T19:20:35.927Z
Learning: In test/src/lib/LibDecimalFloat.pow.t.sol, the comment "we do not try" in the context of "The round trip should not error so we do not try" refers to not using try/catch constructs around the powExternal call, contrasting with previous versions that used nested try/catch blocks.
Applied to files:
test/src/lib/implementation/LibDecimalFloatImplementation.withTargetExponent.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.unabsUnsignedMulOrDivLossy.t.solsrc/lib/LibDecimalFloat.sol
📚 Learning: 2025-08-29T15:26:25.984Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: test/src/lib/LibDecimalFloat.decimal.t.sol:56-56
Timestamp: 2025-08-29T15:26:25.984Z
Learning: In Forge tests, the bound() function is overloaded and requires explicit type casting for overload resolution. When using bound(value, min, max), if the max parameter involves type conversions, an outer cast on the entire bound() call may be necessary for the compiler to resolve which overload to use. Additionally, Solidity cannot change both signedness and bit width in a single cast, so complex casts like uint256(int256(type(int224).max)) require the staged approach rather than a direct cast.
Applied to files:
test/src/lib/implementation/LibDecimalFloatImplementation.unabsUnsignedMulOrDivLossy.t.sol
📚 Learning: 2025-09-28T12:52:45.793Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#144
File: src/lib/LibDecimalFloat.sol:671-686
Timestamp: 2025-09-28T12:52:45.793Z
Learning: In LibDecimalFloat.sol, the pow function has always rejected negative bases and reverted with an error. Prior to recent changes, it used a different error type, but the fundamental policy of not allowing negative bases (except when b == 0) has been consistently maintained across the codebase's history.
Applied to files:
test/src/lib/implementation/LibDecimalFloatImplementation.unabsUnsignedMulOrDivLossy.t.solsrc/lib/LibDecimalFloat.sol
📚 Learning: 2025-09-02T09:33:32.513Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#119
File: test/src/lib/format/LibFormatDecimalFloat.toDecimalString.t.sol:174-175
Timestamp: 2025-09-02T09:33:32.513Z
Learning: The LibFormatDecimalFloat.toDecimalString function in src/lib/format/LibFormatDecimalFloat.sol does not include rounding logic. It formats decimal floats as-is without rounding values based on significant figures limits.
Applied to files:
src/lib/format/LibFormatDecimalFloat.sol
📚 Learning: 2025-07-24T04:32:14.171Z
Learnt from: rouzwelt
PR: rainlanguage/rain.math.float#83
File: src/concrete/DecimalFloat.sol:248-251
Timestamp: 2025-07-24T04:32:14.171Z
Learning: In the rainlanguage/rain.math.float project, functions in DecimalFloat.sol that return tuples from LibDecimalFloat calls must unpack the tuple into local variables before returning them (rather than returning directly) to maintain compatibility with Slither static analysis checks.
Applied to files:
src/lib/format/LibFormatDecimalFloat.sol
📚 Learning: 2025-09-28T12:53:23.475Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#144
File: src/lib/LibDecimalFloat.sol:705-709
Timestamp: 2025-09-28T12:53:23.475Z
Learning: In Solidity, bitwise operations like `exponentBInteger & 0x01 == 0x01` are unambiguous due to the type system. Even though `==` has higher precedence than `&`, the expression cannot be misinterpreted as `exponentBInteger & (0x01 == 0x01)` because you cannot use the bitwise AND operator between a uint256 and a bool. The type system enforces the correct interpretation as `(exponentBInteger & 0x01) == 0x01`.
Applied to files:
src/lib/LibDecimalFloat.sol
📚 Learning: 2025-08-11T14:32:50.439Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#96
File: test/src/lib/implementation/LibDecimalFloatImplementation.maximize.t.sol:15-29
Timestamp: 2025-08-11T14:32:50.439Z
Learning: In test code for the rain.math.float repository, redundant checks may be intentionally kept for clarity and documentation purposes, even when they could be simplified. The maintainer (thedavidmeister) prefers explicit assertions in test code to make the test's intent clear to future readers, prioritizing readability over conciseness.
Applied to files:
src/lib/LibDecimalFloat.sol
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: rainix (ubuntu-latest, test-wasm-build)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-static)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-test)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-test)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-static)
- GitHub Check: rainix (macos-latest, rainix-sol-legal)
- GitHub Check: rainix (macos-latest, rainix-rs-test)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-legal)
- GitHub Check: git-clean
🔇 Additional comments (2)
src/lib/implementation/LibDecimalFloatImplementation.sol (2)
701-708: Fix small-table offsets to match new 90-row layout.
LOG_TABLES_SMALL/_ALTwere shrunk to 900‑byte blocks, but the offset math still skips 1820 bytes and strides by 910. That walks 20 bytes past the main table and keeps landing 10 bytes off for every ALT read, so lookups will pull the wrong rows once the generator stops padding to the legacy sizes. Update the base offset to1801(1 + 1800) and the stride to900so the helper copies from the intended table slots.- // Skip first byte of data contract then 1820 bytes - // of the log tables. - let smallTableOffset := 1821 + // Skip first byte of data contract then 1800 bytes + // of the log tables. + let smallTableOffset := 1801 if iszero(iszero(and(mainTableVal, 0x8000))) { // Small table is half the size of the main // table. - smallTableOffset := add(smallTableOffset, 910) + smallTableOffset := add(smallTableOffset, 900) }
807-809: Restore positive slope for the final log-table segment.When
idx == 8999, bothy1Coefficientandy2Coefficientresolve toscale * 10000, so(y2 - y1)becomes zero and the interpolation flattens: every mantissa in[9999·scale, 10000·scale)now returns the same log10. That breaks monotonicity and regresses accuracy at the top of the table. Give the terminal segment a non-zero slope again (e.g. reuse the old 10001 tail) so interpolation continues to increase smoothly.- y2Coefficient = idx == 8999 - ? int256(scale * 10000) + y2Coefficient = idx == 8999 + ? int256(scale * 10001) : int256(scale * lookupLogTableVal(tablesDataContract, idx + 1));
test/src/lib/implementation/LibDecimalFloatImplementation.withTargetExponent.t.sol
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/implementation/LibDecimalFloatImplementation.sol (1)
1148-1169: Replace hard-coded table offsets with computed values
This assembly block uses literal offsets (2831, 2020, 1820, 910, 100) tied to table sizes—these will break when the tables change.
- Derive each base offset from centralized byte-length constants or by querying the actual sizes of LOG_TABLES, LOG_TABLES_SMALL, LOG_TABLES_SMALL_ALT, ANTI_LOG_TABLES, and ANTI_LOG_TABLES_SMALL.
- Preserve the extcodecopy calls (dest 30/31) and lengths, but calculate
offsetprogrammatically.File: src/lib/implementation/LibDecimalFloatImplementation.sol lines 1150–1169
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (15)
src/lib/LibDecimalFloat.sol(9 hunks)src/lib/format/LibFormatDecimalFloat.sol(3 hunks)src/lib/implementation/LibDecimalFloatImplementation.sol(12 hunks)src/lib/parse/LibParseDecimalFloat.sol(1 hunks)test/src/lib/LibDecimalFloat.add.t.sol(1 hunks)test/src/lib/LibDecimalFloat.decimal.t.sol(2 hunks)test/src/lib/LibDecimalFloat.decimalLossless.t.sol(1 hunks)test/src/lib/LibDecimalFloat.floor.t.sol(1 hunks)test/src/lib/LibDecimalFloat.mul.t.sol(1 hunks)test/src/lib/LibDecimalFloat.pack.t.sol(1 hunks)test/src/lib/LibDecimalFloat.sqrt.t.sol(2 hunks)test/src/lib/LibDecimalFloat.sub.t.sol(1 hunks)test/src/lib/implementation/LibDecimalFloatImplementation.characteristicMantissa.t.sol(1 hunks)test/src/lib/implementation/LibDecimalFloatImplementation.unabsUnsignedMulOrDivLossy.t.sol(7 hunks)test/src/lib/implementation/LibDecimalFloatImplementation.withTargetExponent.t.sol(4 hunks)
🧰 Additional context used
🧠 Learnings (26)
📓 Common learnings
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#144
File: src/lib/LibDecimalFloat.sol:671-686
Timestamp: 2025-09-28T12:52:45.793Z
Learning: In LibDecimalFloat.sol, the pow function has always rejected negative bases and reverted with an error. Prior to recent changes, it used a different error type, but the fundamental policy of not allowing negative bases (except when b == 0) has been consistently maintained across the codebase's history.
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#144
File: test/src/lib/LibDecimalFloat.pow.t.sol:40-61
Timestamp: 2025-09-28T13:28:35.804Z
Learning: In test/src/lib/LibDecimalFloat.pow.t.sol, the diffLimit tolerance (currently 8.6%) represents a conservative worst-case bound to accommodate precision limitations of table lookup interpolation for fractional exponents in the pow function. The exponentiation-by-squaring approach used for integer parts achieves much tighter precision, but the tolerance must account for the less precise table-based calculations used for fractional exponents.
📚 Learning: 2025-09-09T15:11:31.003Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#130
File: src/lib/implementation/LibDecimalFloatImplementation.sol:320-330
Timestamp: 2025-09-09T15:11:31.003Z
Learning: In LibDecimalFloatImplementation.sol div function, returning zero when underflowExponentBy > 76 is intentional "non-error underflow" behavior rather than reverting. This graceful handling is the main goal of the changes.
Applied to files:
test/src/lib/implementation/LibDecimalFloatImplementation.characteristicMantissa.t.soltest/src/lib/LibDecimalFloat.sqrt.t.solsrc/lib/format/LibFormatDecimalFloat.soltest/src/lib/LibDecimalFloat.decimal.t.soltest/src/lib/LibDecimalFloat.floor.t.soltest/src/lib/LibDecimalFloat.add.t.solsrc/lib/LibDecimalFloat.soltest/src/lib/LibDecimalFloat.pack.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.withTargetExponent.t.soltest/src/lib/LibDecimalFloat.decimalLossless.t.solsrc/lib/parse/LibParseDecimalFloat.soltest/src/lib/LibDecimalFloat.mul.t.soltest/src/lib/LibDecimalFloat.sub.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.unabsUnsignedMulOrDivLossy.t.solsrc/lib/implementation/LibDecimalFloatImplementation.sol
📚 Learning: 2025-09-28T13:28:35.804Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#144
File: test/src/lib/LibDecimalFloat.pow.t.sol:40-61
Timestamp: 2025-09-28T13:28:35.804Z
Learning: In test/src/lib/LibDecimalFloat.pow.t.sol, the diffLimit tolerance (currently 8.6%) represents a conservative worst-case bound to accommodate precision limitations of table lookup interpolation for fractional exponents in the pow function. The exponentiation-by-squaring approach used for integer parts achieves much tighter precision, but the tolerance must account for the less precise table-based calculations used for fractional exponents.
Applied to files:
test/src/lib/implementation/LibDecimalFloatImplementation.characteristicMantissa.t.soltest/src/lib/LibDecimalFloat.sqrt.t.solsrc/lib/format/LibFormatDecimalFloat.soltest/src/lib/LibDecimalFloat.decimal.t.soltest/src/lib/LibDecimalFloat.floor.t.soltest/src/lib/LibDecimalFloat.add.t.solsrc/lib/LibDecimalFloat.soltest/src/lib/LibDecimalFloat.pack.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.withTargetExponent.t.soltest/src/lib/LibDecimalFloat.decimalLossless.t.solsrc/lib/parse/LibParseDecimalFloat.soltest/src/lib/LibDecimalFloat.mul.t.soltest/src/lib/LibDecimalFloat.sub.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.unabsUnsignedMulOrDivLossy.t.solsrc/lib/implementation/LibDecimalFloatImplementation.sol
📚 Learning: 2025-08-21T18:03:40.347Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#107
File: test/lib/LibDecimalFloatSlow.sol:37-45
Timestamp: 2025-08-21T18:03:40.347Z
Learning: In test/lib/LibDecimalFloatSlow.sol, the "slow" implementation is intentionally different from the production implementation to serve as an independent reference for fuzzing tests. The goal is to have two different approaches (expensive loops vs optimized jumps) that produce equivalent results, not identical implementations.
Applied to files:
test/src/lib/implementation/LibDecimalFloatImplementation.characteristicMantissa.t.soltest/src/lib/LibDecimalFloat.sqrt.t.solsrc/lib/format/LibFormatDecimalFloat.soltest/src/lib/LibDecimalFloat.decimal.t.soltest/src/lib/LibDecimalFloat.floor.t.soltest/src/lib/LibDecimalFloat.add.t.solsrc/lib/LibDecimalFloat.soltest/src/lib/LibDecimalFloat.pack.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.withTargetExponent.t.soltest/src/lib/LibDecimalFloat.decimalLossless.t.solsrc/lib/parse/LibParseDecimalFloat.soltest/src/lib/LibDecimalFloat.mul.t.soltest/src/lib/LibDecimalFloat.sub.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.unabsUnsignedMulOrDivLossy.t.solsrc/lib/implementation/LibDecimalFloatImplementation.sol
📚 Learning: 2025-09-10T19:20:35.927Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#135
File: test/src/lib/LibDecimalFloat.pow.t.sol:160-161
Timestamp: 2025-09-10T19:20:35.927Z
Learning: In test/src/lib/LibDecimalFloat.pow.t.sol, the comment "we do not try" in the context of "The round trip should not error so we do not try" refers to not using try/catch constructs around the powExternal call, contrasting with previous versions that used nested try/catch blocks.
Applied to files:
test/src/lib/implementation/LibDecimalFloatImplementation.characteristicMantissa.t.soltest/src/lib/LibDecimalFloat.sqrt.t.soltest/src/lib/LibDecimalFloat.decimal.t.soltest/src/lib/LibDecimalFloat.floor.t.solsrc/lib/LibDecimalFloat.soltest/src/lib/implementation/LibDecimalFloatImplementation.withTargetExponent.t.soltest/src/lib/LibDecimalFloat.decimalLossless.t.solsrc/lib/parse/LibParseDecimalFloat.soltest/src/lib/implementation/LibDecimalFloatImplementation.unabsUnsignedMulOrDivLossy.t.sol
📚 Learning: 2025-08-29T10:38:26.353Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: src/lib/implementation/LibDecimalFloatImplementation.sol:593-601
Timestamp: 2025-08-29T10:38:26.353Z
Learning: In Solidity, int256(1) when passed through the maximize function in LibDecimalFloatImplementation.sol produces exactly (1e76, -76), not an approximation. This means the special case for signedCoefficient == 1e76 in log10 correctly handles powers of 10 like log10(1).
Applied to files:
test/src/lib/implementation/LibDecimalFloatImplementation.characteristicMantissa.t.soltest/src/lib/LibDecimalFloat.sqrt.t.solsrc/lib/format/LibFormatDecimalFloat.soltest/src/lib/LibDecimalFloat.decimal.t.soltest/src/lib/LibDecimalFloat.floor.t.soltest/src/lib/LibDecimalFloat.add.t.solsrc/lib/LibDecimalFloat.soltest/src/lib/implementation/LibDecimalFloatImplementation.withTargetExponent.t.soltest/src/lib/LibDecimalFloat.decimalLossless.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.unabsUnsignedMulOrDivLossy.t.solsrc/lib/implementation/LibDecimalFloatImplementation.sol
📚 Learning: 2025-08-29T10:38:26.353Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: src/lib/implementation/LibDecimalFloatImplementation.sol:593-601
Timestamp: 2025-08-29T10:38:26.353Z
Learning: The maximize function in LibDecimalFloatImplementation.sol produces exact results for simple integer values like 1. maximize(1, 0) yields exactly (1e76, -76) with no precision loss, and the log10 special case for signedCoefficient == 1e76 correctly handles this.
Applied to files:
test/src/lib/implementation/LibDecimalFloatImplementation.characteristicMantissa.t.soltest/src/lib/LibDecimalFloat.sqrt.t.solsrc/lib/format/LibFormatDecimalFloat.soltest/src/lib/LibDecimalFloat.decimal.t.soltest/src/lib/LibDecimalFloat.add.t.solsrc/lib/LibDecimalFloat.soltest/src/lib/LibDecimalFloat.pack.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.withTargetExponent.t.soltest/src/lib/LibDecimalFloat.decimalLossless.t.soltest/src/lib/LibDecimalFloat.mul.t.soltest/src/lib/LibDecimalFloat.sub.t.solsrc/lib/implementation/LibDecimalFloatImplementation.sol
📚 Learning: 2025-09-28T12:52:45.793Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#144
File: src/lib/LibDecimalFloat.sol:671-686
Timestamp: 2025-09-28T12:52:45.793Z
Learning: In LibDecimalFloat.sol, the pow function has always rejected negative bases and reverted with an error. Prior to recent changes, it used a different error type, but the fundamental policy of not allowing negative bases (except when b == 0) has been consistently maintained across the codebase's history.
Applied to files:
test/src/lib/LibDecimalFloat.sqrt.t.solsrc/lib/LibDecimalFloat.soltest/src/lib/implementation/LibDecimalFloatImplementation.unabsUnsignedMulOrDivLossy.t.sol
📚 Learning: 2025-08-29T14:54:24.240Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: test/src/lib/LibDecimalFloat.decimal.t.sol:122-126
Timestamp: 2025-08-29T14:54:24.240Z
Learning: In test/src/lib/LibDecimalFloat.decimal.t.sol, the fromFixedDecimalLossy function converts uint256(type(int256).max) losslessly because the boundary check is `value > uint256(type(int256).max)`, not `>=`. Values exactly equal to type(int256).max still take the lossless conversion path.
Applied to files:
test/src/lib/LibDecimalFloat.sqrt.t.solsrc/lib/format/LibFormatDecimalFloat.soltest/src/lib/LibDecimalFloat.decimal.t.soltest/src/lib/LibDecimalFloat.floor.t.soltest/src/lib/LibDecimalFloat.add.t.solsrc/lib/LibDecimalFloat.soltest/src/lib/LibDecimalFloat.pack.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.withTargetExponent.t.soltest/src/lib/LibDecimalFloat.decimalLossless.t.solsrc/lib/parse/LibParseDecimalFloat.soltest/src/lib/LibDecimalFloat.mul.t.soltest/src/lib/LibDecimalFloat.sub.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.unabsUnsignedMulOrDivLossy.t.solsrc/lib/implementation/LibDecimalFloatImplementation.sol
📚 Learning: 2025-09-02T09:33:32.513Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#119
File: test/src/lib/format/LibFormatDecimalFloat.toDecimalString.t.sol:174-175
Timestamp: 2025-09-02T09:33:32.513Z
Learning: The LibFormatDecimalFloat.toDecimalString function in src/lib/format/LibFormatDecimalFloat.sol does not include rounding logic. It formats decimal floats as-is without rounding values based on significant figures limits.
Applied to files:
test/src/lib/LibDecimalFloat.sqrt.t.solsrc/lib/format/LibFormatDecimalFloat.soltest/src/lib/LibDecimalFloat.add.t.soltest/src/lib/LibDecimalFloat.pack.t.soltest/src/lib/LibDecimalFloat.decimalLossless.t.solsrc/lib/parse/LibParseDecimalFloat.soltest/src/lib/LibDecimalFloat.mul.t.soltest/src/lib/LibDecimalFloat.sub.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.unabsUnsignedMulOrDivLossy.t.sol
📚 Learning: 2025-08-29T14:58:50.500Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: src/lib/implementation/LibDecimalFloatImplementation.sol:896-899
Timestamp: 2025-08-29T14:58:50.500Z
Learning: In unchecked Solidity blocks, arithmetic operations can overflow/underflow and wrap around, so bounds checks that seem "impossible" for normal arithmetic may actually be necessary to catch overflow edge cases. For example, in withTargetExponent function, the check `exponentDiff < 0` is needed because `targetExponent - exponent` could underflow in unchecked arithmetic.
Applied to files:
test/src/lib/LibDecimalFloat.floor.t.solsrc/lib/LibDecimalFloat.soltest/src/lib/implementation/LibDecimalFloatImplementation.withTargetExponent.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.unabsUnsignedMulOrDivLossy.t.solsrc/lib/implementation/LibDecimalFloatImplementation.sol
📚 Learning: 2025-09-28T12:53:23.475Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#144
File: src/lib/LibDecimalFloat.sol:705-709
Timestamp: 2025-09-28T12:53:23.475Z
Learning: In Solidity, bitwise operations like `exponentBInteger & 0x01 == 0x01` are unambiguous due to the type system. Even though `==` has higher precedence than `&`, the expression cannot be misinterpreted as `exponentBInteger & (0x01 == 0x01)` because you cannot use the bitwise AND operator between a uint256 and a bool. The type system enforces the correct interpretation as `(exponentBInteger & 0x01) == 0x01`.
Applied to files:
src/lib/LibDecimalFloat.sol
📚 Learning: 2025-08-11T14:32:50.439Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#96
File: test/src/lib/implementation/LibDecimalFloatImplementation.maximize.t.sol:15-29
Timestamp: 2025-08-11T14:32:50.439Z
Learning: In test code for the rain.math.float repository, redundant checks may be intentionally kept for clarity and documentation purposes, even when they could be simplified. The maintainer (thedavidmeister) prefers explicit assertions in test code to make the test's intent clear to future readers, prioritizing readability over conciseness.
Applied to files:
src/lib/LibDecimalFloat.sol
📚 Learning: 2025-08-14T16:56:28.978Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#99
File: src/lib/implementation/LibDecimalFloatImplementation.sol:574-581
Timestamp: 2025-08-14T16:56:28.978Z
Learning: In the rainlanguage/rain.math.float repository, maintainer thedavidmeister avoids using `&&` operators in gas-critical paths because they involve jumps in Solidity due to short-circuit evaluation, preferring approaches like mul-then-div overflow probes that avoid conditional jump overhead.
Applied to files:
src/lib/LibDecimalFloat.solsrc/lib/implementation/LibDecimalFloatImplementation.sol
📚 Learning: 2025-08-27T13:37:22.601Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#111
File: test/src/lib/implementation/LibDecimalFloatImplementation.div.t.sol:126-133
Timestamp: 2025-08-27T13:37:22.601Z
Learning: In the rainlanguage/rain.math.float repository, maintainer thedavidmeister prefers to avoid inline `if` statements (even if they were supported in Solidity) because they create fragile code with meaningful whitespace and make debugging difficult when console logs need to be added, potentially causing subtle behavior changes if braces aren't reintroduced properly.
Applied to files:
src/lib/LibDecimalFloat.solsrc/lib/implementation/LibDecimalFloatImplementation.sol
📚 Learning: 2025-08-14T16:32:05.932Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#99
File: src/lib/implementation/LibDecimalFloatImplementation.sol:309-325
Timestamp: 2025-08-14T16:32:05.932Z
Learning: In the rainlanguage/rain.math.float repository, maintainer thedavidmeister prefers to keep assembly-based overflow checks inline for gas optimization rather than extracting them into helper functions, even when it results in code duplication.
Applied to files:
src/lib/LibDecimalFloat.solsrc/lib/implementation/LibDecimalFloatImplementation.sol
📚 Learning: 2025-06-16T13:17:28.513Z
Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#58
File: src/concrete/DecimalFloat.sol:175-182
Timestamp: 2025-06-16T13:17:28.513Z
Learning: In the rainlanguage/rain.math.float codebase, there's an established naming convention where functions accepting a `Float` type parameter consistently use `float` as the parameter name, even though it shadows the type name. This pattern is used throughout `LibDecimalFloat.sol` and should be maintained for consistency in related contracts like `DecimalFloat.sol`.
Applied to files:
src/lib/LibDecimalFloat.solsrc/lib/parse/LibParseDecimalFloat.soltest/src/lib/LibDecimalFloat.sub.t.sol
📚 Learning: 2025-04-25T03:58:01.307Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#30
File: test/src/lib/LibDecimalFloat.gt.t.sol:33-36
Timestamp: 2025-04-25T03:58:01.307Z
Learning: In the rain.math.float library, all values of `Float` (which is a type alias for bytes32) are considered valid and can be safely used with methods like gt(), lt(), or eq() without causing reverts.
Applied to files:
src/lib/LibDecimalFloat.sol
📚 Learning: 2025-07-24T04:32:14.171Z
Learnt from: rouzwelt
PR: rainlanguage/rain.math.float#83
File: src/concrete/DecimalFloat.sol:248-251
Timestamp: 2025-07-24T04:32:14.171Z
Learning: In the rainlanguage/rain.math.float project, functions in DecimalFloat.sol that return tuples from LibDecimalFloat calls must unpack the tuple into local variables before returning them (rather than returning directly) to maintain compatibility with Slither static analysis checks.
Applied to files:
test/src/lib/LibDecimalFloat.pack.t.solsrc/lib/parse/LibParseDecimalFloat.soltest/src/lib/LibDecimalFloat.mul.t.soltest/src/lib/LibDecimalFloat.sub.t.sol
📚 Learning: 2025-06-17T10:02:01.394Z
Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#64
File: src/concrete/DecimalFloat.sol:0-0
Timestamp: 2025-06-17T10:02:01.394Z
Learning: In the rain.math.float project, the Float type is designed with a specific bit layout: 224-bit signed coefficient and 32-bit signed exponent, represented as 32 bytes total. The LibDecimalFloat.unpack function returns values that are guaranteed to fit within int224 and int32 ranges due to this internal representation, making explicit range checks before casting unnecessary.
Applied to files:
test/src/lib/LibDecimalFloat.pack.t.sol
📚 Learning: 2025-08-26T16:50:11.113Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#111
File: test/src/lib/LibDecimalFloat.div.t.sol:48-51
Timestamp: 2025-08-26T16:50:11.113Z
Learning: In LibDecimalFloat, packLossless internally asserts that the packing operation is lossless, so there's no need to manually check the lossless flag when using packLossless instead of packLossy.
Applied to files:
test/src/lib/LibDecimalFloat.decimalLossless.t.sol
📚 Learning: 2025-08-29T15:26:25.984Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: test/src/lib/LibDecimalFloat.decimal.t.sol:56-56
Timestamp: 2025-08-29T15:26:25.984Z
Learning: In Forge tests, the bound() function is overloaded and requires explicit type casting for overload resolution. When using bound(value, min, max), if the max parameter involves type conversions, an outer cast on the entire bound() call may be necessary for the compiler to resolve which overload to use. Additionally, Solidity cannot change both signedness and bit width in a single cast, so complex casts like uint256(int256(type(int224).max)) require the staged approach rather than a direct cast.
Applied to files:
test/src/lib/LibDecimalFloat.decimalLossless.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.unabsUnsignedMulOrDivLossy.t.sol
📚 Learning: 2025-08-11T14:30:48.562Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#96
File: test/src/lib/LibDecimalFloat.ceil.t.sol:43-50
Timestamp: 2025-08-11T14:30:48.562Z
Learning: When reviewing Solidity test files using Forge, verify the actual mutability of helper functions like `bound()` before suggesting changes to function mutability specifiers, as Forge implements many test utilities as pure functions.
Applied to files:
src/lib/parse/LibParseDecimalFloat.soltest/src/lib/LibDecimalFloat.mul.t.soltest/src/lib/LibDecimalFloat.sub.t.solsrc/lib/implementation/LibDecimalFloatImplementation.sol
📚 Learning: 2025-08-18T13:52:43.369Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#105
File: src/lib/implementation/LibDecimalFloatImplementation.sol:248-335
Timestamp: 2025-08-18T13:52:43.369Z
Learning: The codebase has a policy of not modifying external code, such as mulDiv implementations adopted from standard libraries like OpenZeppelin, PRB Math, and Solady.
Applied to files:
test/src/lib/LibDecimalFloat.mul.t.sol
📚 Learning: 2025-09-08T12:32:24.760Z
Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#126
File: test_js/float.test.ts:26-29
Timestamp: 2025-09-08T12:32:24.760Z
Learning: In the rainlanguage/rain.math.float repository, the test_js/ directory contains tests specifically for TypeScript bindings functionality, not comprehensive feature testing. More extensive test coverage for the actual logic is maintained in the Rust and Solidity test suites.
Applied to files:
test/src/lib/LibDecimalFloat.mul.t.sol
📚 Learning: 2025-06-17T10:17:56.205Z
Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#59
File: crates/float/src/lib.rs:233-242
Timestamp: 2025-06-17T10:17:56.205Z
Learning: In the rainlanguage/rain.math.float repository, the maintainer 0xgleb prefers to handle documentation additions and improvements in separate issues rather than inline with feature PRs.
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.sol
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: git-clean
- GitHub Check: rainix (ubuntu-latest, rainix-rs-static)
- GitHub Check: rainix (ubuntu-latest, test-wasm-build)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-test)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-static)
- GitHub Check: rainix (macos-latest, rainix-sol-legal)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-legal)
- GitHub Check: rainix (macos-latest, rainix-rs-test)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-test)
🔇 Additional comments (14)
test/src/lib/implementation/LibDecimalFloatImplementation.characteristicMantissa.t.sol (1)
63-66: LGTM — helpful bounds/cast notes.The added comments clarify the safe range for the cast and match the implementation.
test/src/lib/implementation/LibDecimalFloatImplementation.unabsUnsignedMulOrDivLossy.t.sol (1)
36-41: LGTM — test annotations make the casts and bounds explicit.The lint suppressions and notes correctly justify the casts and prevent false positives; no behavior change.
Also applies to: 58-60, 83-85, 127-131, 153-156, 167-171
src/lib/LibDecimalFloat.sol (1)
681-694: Negative-base policy preserved; good.Reverting with PowNegativeBase for any a < 0 (except b == 0 case) matches long-standing behavior. Tests should continue asserting this policy.
Based on learnings
src/lib/implementation/LibDecimalFloatImplementation.sol (2)
702-721: Small-table offset constants are stale for 90-row layout; fix before encoder drops padding.The base (1821) and ALT stride (910) still target the old 91-row/ padded layout. With 90 rows the correct values are 1801 and 900; otherwise lookups will drift once padding disappears. Also update the comment to 1800 bytes.
Apply:
- // Skip first byte of data contract then 1820 bytes - // of the log tables. - let smallTableOffset := 1821 + // Skip first byte of data contract then 1800 bytes + // of the log tables. + let smallTableOffset := 1801 if iszero(iszero(and(mainTableVal, 0x8000))) { - // Small table is half the size of the main - // table. - smallTableOffset := add(smallTableOffset, 910) + // Small table is half the size of the main table. + smallTableOffset := add(smallTableOffset, 900) }
815-821: Terminal interpolation slope is zero; make tail strictly increasing.At idx == last row, y2Coefficient equals y1Coefficient (both 1e76), so (y2 - y1) = 0 and the final bucket flattens. Use 10001 to keep a positive slope.
- y2Coefficient = - idx == 8999 ? int256(1e76) : int256(1e72 * lookupLogTableVal(tablesDataContract, idx + 1)); + y2Coefficient = + idx == LOG_MANTISSA_LAST_INDEX + ? int256(1e72 * 10001) + : int256(1e72 * lookupLogTableVal(tablesDataContract, idx + 1));test/src/lib/implementation/LibDecimalFloatImplementation.withTargetExponent.t.sol (1)
200-202: Fix sign in comment (“targetExponent < exponent”).The relationship is reversed in the comment; it should be targetExponent < exponent here. Update or remove to avoid confusion.
- // targetExponent > exponent + // targetExponent < exponenttest/src/lib/LibDecimalFloat.decimalLossless.t.sol (1)
83-85: Non‑negativity invariant + targeted lint suppression look good.Asserting signedCoefficient >= 0 is consistent for uint→float lossless converts, and the one‑line lint suppression is scoped correctly. No further action.
test/src/lib/LibDecimalFloat.sqrt.t.sol (3)
16-16: Tightened sqrt round‑trip tolerance—please validate against fuzz range.Lowering diffLimit to 19e‑4 is reasonable since the pow(2) leg uses exponentiation‑by‑squaring (tighter precision). Given recent log‑table changes, please confirm fuzz runs still pass within 0.19% across the table domain.
Based on learnings.
71-71: Negative‑path revert aligned to PowNegativeBase.Selector update matches the consolidated negative‑base policy.
Based on learnings.
8-8: Remove unused error imports.Only PowNegativeBase is used. Drop ZeroNegativePower and Log10Negative.
-import {ZeroNegativePower, Log10Negative, PowNegativeBase} from "src/error/ErrDecimalFloat.sol"; +import {PowNegativeBase} from "src/error/ErrDecimalFloat.sol";src/lib/format/LibFormatDecimalFloat.sol (4)
62-68: Scientific scale selection looks good; verify maximizeFull invariant for boundary.The 1e76/1e75 split reads clean and matches maximizeFull outcomes (e.g., 1 → 1e76,-76). Please confirm maximizeFull guarantees signedCoefficient is either ≥1e76 (triggering 76) or <1e76 (→75) across ± values, and add a brief comment to document this invariant.
82-84: Explicit zero‑exponent path is clearer.LGTM; the explicit
scaleExponent = 0improves readability.
137-141: CastingscaleExponentback to int is safe only if-exponent∈ [0, int256.max]. Document the invariant.If
exponent == type(int256).min, earlier negation would revert; if larger magnitudes are possible,int256(scaleExponent)here would also be unsafe. Please assert/document the bound, or computedisplayExponentwithout the uint→int cast.Possible alternative:
- int256 displayExponent = exponent + int256(scaleExponent); + int256 displayExponent = scientific ? exponent + int256(scaleExponent) : exponent;
75-81: Verifyexponentdomain before negation: confirm that anyFloatpassed intotoDecimalStringcan never carryexponent == type(int256).min(for example, packing logic should clamp exponents to [-76,76]); if this invariant isn’t already enforced, either document/assert it or add an explicit guard before-exponent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/src/lib/implementation/LibDecimalFloatImplementation.unabsUnsignedMulOrDivLossy.t.sol (1)
99-107: Bound b to avoid sign-of-zero ambiguity in MixedAB test
Change the lower bound of b from 0 to 1 so the signed-zero case can’t occur:- b = bound(b, 0, uint256(type(int256).max)); + b = bound(b, 1, uint256(type(int256).max));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (31)
src/lib/format/LibFormatDecimalFloat.sol(4 hunks)src/lib/implementation/LibDecimalFloatImplementation.sol(13 hunks)src/lib/parse/LibParseDecimalFloat.sol(2 hunks)test/abstract/LogTest.sol(1 hunks)test/lib/LibDecimalFloatSlow.sol(1 hunks)test/src/concrete/DecimalFloat.log10.t.sol(0 hunks)test/src/concrete/DecimalFloat.pow10.t.sol(0 hunks)test/src/lib/LibDecimalFloat.ceil.t.sol(1 hunks)test/src/lib/LibDecimalFloat.decimal.t.sol(5 hunks)test/src/lib/LibDecimalFloat.floor.t.sol(1 hunks)test/src/lib/LibDecimalFloat.frac.t.sol(1 hunks)test/src/lib/LibDecimalFloat.max.t.sol(1 hunks)test/src/lib/LibDecimalFloat.min.t.sol(1 hunks)test/src/lib/LibDecimalFloat.mixed.t.sol(1 hunks)test/src/lib/LibDecimalFloat.mul.t.sol(1 hunks)test/src/lib/LibDecimalFloat.pack.t.sol(1 hunks)test/src/lib/LibDecimalFloat.pow.t.sol(4 hunks)test/src/lib/LibDecimalFloat.sqrt.t.sol(2 hunks)test/src/lib/LibDecimalFloat.sub.t.sol(1 hunks)test/src/lib/format/LibFormatDecimalFloat.countSigFigs.t.sol(1 hunks)test/src/lib/format/LibFormatDecimalFloat.toDecimalString.t.sol(1 hunks)test/src/lib/implementation/LibDecimalFloatImplementation.add.t.sol(1 hunks)test/src/lib/implementation/LibDecimalFloatImplementation.div.t.sol(1 hunks)test/src/lib/implementation/LibDecimalFloatImplementation.eq.t.sol(1 hunks)test/src/lib/implementation/LibDecimalFloatImplementation.log10.t.sol(1 hunks)test/src/lib/implementation/LibDecimalFloatImplementation.lookupLogTableVal.t.sol(1 hunks)test/src/lib/implementation/LibDecimalFloatImplementation.maximize.t.sol(1 hunks)test/src/lib/implementation/LibDecimalFloatImplementation.pow10.t.sol(0 hunks)test/src/lib/implementation/LibDecimalFloatImplementation.unabsUnsignedMulOrDivLossy.t.sol(8 hunks)test/src/lib/implementation/LibDecimalFloatImplementation.withTargetExponent.t.sol(5 hunks)test/src/lib/parse/LibParseDecimalFloat.t.sol(1 hunks)
💤 Files with no reviewable changes (3)
- test/src/concrete/DecimalFloat.log10.t.sol
- test/src/lib/implementation/LibDecimalFloatImplementation.pow10.t.sol
- test/src/concrete/DecimalFloat.pow10.t.sol
🧰 Additional context used
🧠 Learnings (27)
📓 Common learnings
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#144
File: src/lib/LibDecimalFloat.sol:671-686
Timestamp: 2025-09-28T12:52:45.815Z
Learning: In LibDecimalFloat.sol, the pow function has always rejected negative bases and reverted with an error. Prior to recent changes, it used a different error type, but the fundamental policy of not allowing negative bases (except when b == 0) has been consistently maintained across the codebase's history.
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#144
File: test/src/lib/LibDecimalFloat.pow.t.sol:40-61
Timestamp: 2025-09-28T13:28:35.814Z
Learning: In test/src/lib/LibDecimalFloat.pow.t.sol, the diffLimit tolerance (currently 8.6%) represents a conservative worst-case bound to accommodate precision limitations of table lookup interpolation for fractional exponents in the pow function. The exponentiation-by-squaring approach used for integer parts achieves much tighter precision, but the tolerance must account for the less precise table-based calculations used for fractional exponents.
📚 Learning: 2025-08-11T14:30:48.562Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#96
File: test/src/lib/LibDecimalFloat.ceil.t.sol:43-50
Timestamp: 2025-08-11T14:30:48.562Z
Learning: When reviewing Solidity test files using Forge, verify the actual mutability of helper functions like `bound()` before suggesting changes to function mutability specifiers, as Forge implements many test utilities as pure functions.
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.soltest/src/lib/implementation/LibDecimalFloatImplementation.div.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.lookupLogTableVal.t.soltest/src/lib/LibDecimalFloat.decimal.t.soltest/abstract/LogTest.soltest/src/lib/LibDecimalFloat.mul.t.soltest/src/lib/LibDecimalFloat.sub.t.soltest/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-09-28T13:28:35.814Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#144
File: test/src/lib/LibDecimalFloat.pow.t.sol:40-61
Timestamp: 2025-09-28T13:28:35.814Z
Learning: In test/src/lib/LibDecimalFloat.pow.t.sol, the diffLimit tolerance (currently 8.6%) represents a conservative worst-case bound to accommodate precision limitations of table lookup interpolation for fractional exponents in the pow function. The exponentiation-by-squaring approach used for integer parts achieves much tighter precision, but the tolerance must account for the less precise table-based calculations used for fractional exponents.
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.soltest/src/lib/LibDecimalFloat.pack.t.soltest/src/lib/parse/LibParseDecimalFloat.t.soltest/src/lib/LibDecimalFloat.sqrt.t.soltest/src/lib/format/LibFormatDecimalFloat.toDecimalString.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.div.t.soltest/src/lib/LibDecimalFloat.mixed.t.soltest/lib/LibDecimalFloatSlow.soltest/src/lib/implementation/LibDecimalFloatImplementation.withTargetExponent.t.soltest/src/lib/format/LibFormatDecimalFloat.countSigFigs.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.maximize.t.solsrc/lib/format/LibFormatDecimalFloat.soltest/src/lib/implementation/LibDecimalFloatImplementation.unabsUnsignedMulOrDivLossy.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.add.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.lookupLogTableVal.t.soltest/src/lib/LibDecimalFloat.decimal.t.soltest/src/lib/LibDecimalFloat.floor.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.eq.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.log10.t.soltest/src/lib/LibDecimalFloat.ceil.t.solsrc/lib/parse/LibParseDecimalFloat.soltest/src/lib/LibDecimalFloat.mul.t.soltest/src/lib/LibDecimalFloat.sub.t.soltest/src/lib/LibDecimalFloat.frac.t.soltest/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-08-29T10:38:26.353Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: src/lib/implementation/LibDecimalFloatImplementation.sol:593-601
Timestamp: 2025-08-29T10:38:26.353Z
Learning: In Solidity, int256(1) when passed through the maximize function in LibDecimalFloatImplementation.sol produces exactly (1e76, -76), not an approximation. This means the special case for signedCoefficient == 1e76 in log10 correctly handles powers of 10 like log10(1).
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.soltest/src/lib/LibDecimalFloat.sqrt.t.soltest/lib/LibDecimalFloatSlow.soltest/src/lib/implementation/LibDecimalFloatImplementation.withTargetExponent.t.soltest/src/lib/format/LibFormatDecimalFloat.countSigFigs.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.maximize.t.solsrc/lib/format/LibFormatDecimalFloat.soltest/src/lib/implementation/LibDecimalFloatImplementation.unabsUnsignedMulOrDivLossy.t.soltest/src/lib/LibDecimalFloat.decimal.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.log10.t.soltest/src/lib/LibDecimalFloat.ceil.t.soltest/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-09-09T15:11:31.003Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#130
File: src/lib/implementation/LibDecimalFloatImplementation.sol:320-330
Timestamp: 2025-09-09T15:11:31.003Z
Learning: In LibDecimalFloatImplementation.sol div function, returning zero when underflowExponentBy > 76 is intentional "non-error underflow" behavior rather than reverting. This graceful handling is the main goal of the changes.
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.soltest/src/lib/LibDecimalFloat.pack.t.soltest/src/lib/parse/LibParseDecimalFloat.t.soltest/src/lib/LibDecimalFloat.sqrt.t.soltest/src/lib/format/LibFormatDecimalFloat.toDecimalString.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.div.t.soltest/src/lib/LibDecimalFloat.mixed.t.soltest/lib/LibDecimalFloatSlow.soltest/src/lib/implementation/LibDecimalFloatImplementation.withTargetExponent.t.soltest/src/lib/format/LibFormatDecimalFloat.countSigFigs.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.maximize.t.solsrc/lib/format/LibFormatDecimalFloat.soltest/src/lib/implementation/LibDecimalFloatImplementation.unabsUnsignedMulOrDivLossy.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.add.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.lookupLogTableVal.t.soltest/src/lib/LibDecimalFloat.decimal.t.soltest/src/lib/LibDecimalFloat.floor.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.eq.t.solsrc/lib/parse/LibParseDecimalFloat.soltest/src/lib/LibDecimalFloat.mul.t.soltest/src/lib/LibDecimalFloat.sub.t.soltest/src/lib/LibDecimalFloat.frac.t.soltest/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-08-29T14:54:24.240Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: test/src/lib/LibDecimalFloat.decimal.t.sol:122-126
Timestamp: 2025-08-29T14:54:24.240Z
Learning: In test/src/lib/LibDecimalFloat.decimal.t.sol, the fromFixedDecimalLossy function converts uint256(type(int256).max) losslessly because the boundary check is `value > uint256(type(int256).max)`, not `>=`. Values exactly equal to type(int256).max still take the lossless conversion path.
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.soltest/src/lib/LibDecimalFloat.pack.t.soltest/src/lib/parse/LibParseDecimalFloat.t.soltest/src/lib/LibDecimalFloat.sqrt.t.soltest/src/lib/format/LibFormatDecimalFloat.toDecimalString.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.div.t.soltest/src/lib/LibDecimalFloat.mixed.t.soltest/lib/LibDecimalFloatSlow.soltest/src/lib/implementation/LibDecimalFloatImplementation.withTargetExponent.t.soltest/src/lib/format/LibFormatDecimalFloat.countSigFigs.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.maximize.t.soltest/src/lib/LibDecimalFloat.min.t.solsrc/lib/format/LibFormatDecimalFloat.soltest/src/lib/implementation/LibDecimalFloatImplementation.unabsUnsignedMulOrDivLossy.t.soltest/src/lib/LibDecimalFloat.max.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.add.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.lookupLogTableVal.t.soltest/src/lib/LibDecimalFloat.decimal.t.soltest/src/lib/LibDecimalFloat.floor.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.eq.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.log10.t.soltest/src/lib/LibDecimalFloat.ceil.t.solsrc/lib/parse/LibParseDecimalFloat.soltest/src/lib/LibDecimalFloat.mul.t.soltest/src/lib/LibDecimalFloat.sub.t.soltest/src/lib/LibDecimalFloat.frac.t.soltest/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-06-17T10:17:56.205Z
Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#59
File: crates/float/src/lib.rs:233-242
Timestamp: 2025-06-17T10:17:56.205Z
Learning: In the rainlanguage/rain.math.float repository, the maintainer 0xgleb prefers to handle documentation additions and improvements in separate issues rather than inline with feature PRs.
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.sol
📚 Learning: 2025-08-14T16:32:05.932Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#99
File: src/lib/implementation/LibDecimalFloatImplementation.sol:309-325
Timestamp: 2025-08-14T16:32:05.932Z
Learning: In the rainlanguage/rain.math.float repository, maintainer thedavidmeister prefers to keep assembly-based overflow checks inline for gas optimization rather than extracting them into helper functions, even when it results in code duplication.
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.soltest/src/lib/implementation/LibDecimalFloatImplementation.lookupLogTableVal.t.soltest/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-08-27T13:37:22.601Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#111
File: test/src/lib/implementation/LibDecimalFloatImplementation.div.t.sol:126-133
Timestamp: 2025-08-27T13:37:22.601Z
Learning: In the rainlanguage/rain.math.float repository, maintainer thedavidmeister prefers to avoid inline `if` statements (even if they were supported in Solidity) because they create fragile code with meaningful whitespace and make debugging difficult when console logs need to be added, potentially causing subtle behavior changes if braces aren't reintroduced properly.
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.soltest/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-08-14T16:56:28.978Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#99
File: src/lib/implementation/LibDecimalFloatImplementation.sol:574-581
Timestamp: 2025-08-14T16:56:28.978Z
Learning: In the rainlanguage/rain.math.float repository, maintainer thedavidmeister avoids using `&&` operators in gas-critical paths because they involve jumps in Solidity due to short-circuit evaluation, preferring approaches like mul-then-div overflow probes that avoid conditional jump overhead.
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.soltest/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-08-11T14:32:50.439Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#96
File: test/src/lib/implementation/LibDecimalFloatImplementation.maximize.t.sol:15-29
Timestamp: 2025-08-11T14:32:50.439Z
Learning: In test code for the rain.math.float repository, redundant checks may be intentionally kept for clarity and documentation purposes, even when they could be simplified. The maintainer (thedavidmeister) prefers explicit assertions in test code to make the test's intent clear to future readers, prioritizing readability over conciseness.
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.soltest/src/lib/implementation/LibDecimalFloatImplementation.lookupLogTableVal.t.soltest/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-08-21T18:03:40.347Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#107
File: test/lib/LibDecimalFloatSlow.sol:37-45
Timestamp: 2025-08-21T18:03:40.347Z
Learning: In test/lib/LibDecimalFloatSlow.sol, the "slow" implementation is intentionally different from the production implementation to serve as an independent reference for fuzzing tests. The goal is to have two different approaches (expensive loops vs optimized jumps) that produce equivalent results, not identical implementations.
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.soltest/src/lib/LibDecimalFloat.pack.t.soltest/src/lib/parse/LibParseDecimalFloat.t.soltest/src/lib/LibDecimalFloat.sqrt.t.soltest/src/lib/format/LibFormatDecimalFloat.toDecimalString.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.div.t.soltest/src/lib/LibDecimalFloat.mixed.t.soltest/lib/LibDecimalFloatSlow.soltest/src/lib/implementation/LibDecimalFloatImplementation.withTargetExponent.t.soltest/src/lib/format/LibFormatDecimalFloat.countSigFigs.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.maximize.t.solsrc/lib/format/LibFormatDecimalFloat.soltest/src/lib/implementation/LibDecimalFloatImplementation.unabsUnsignedMulOrDivLossy.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.add.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.lookupLogTableVal.t.soltest/src/lib/LibDecimalFloat.decimal.t.soltest/src/lib/LibDecimalFloat.floor.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.eq.t.soltest/src/lib/LibDecimalFloat.ceil.t.solsrc/lib/parse/LibParseDecimalFloat.soltest/src/lib/LibDecimalFloat.mul.t.soltest/src/lib/LibDecimalFloat.sub.t.soltest/src/lib/LibDecimalFloat.frac.t.soltest/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-08-29T10:38:26.353Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: src/lib/implementation/LibDecimalFloatImplementation.sol:593-601
Timestamp: 2025-08-29T10:38:26.353Z
Learning: The maximize function in LibDecimalFloatImplementation.sol produces exact results for simple integer values like 1. maximize(1, 0) yields exactly (1e76, -76) with no precision loss, and the log10 special case for signedCoefficient == 1e76 correctly handles this.
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.soltest/src/lib/LibDecimalFloat.pack.t.soltest/src/lib/LibDecimalFloat.sqrt.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.withTargetExponent.t.soltest/src/lib/format/LibFormatDecimalFloat.countSigFigs.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.maximize.t.solsrc/lib/format/LibFormatDecimalFloat.soltest/src/lib/LibDecimalFloat.max.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.add.t.soltest/src/lib/LibDecimalFloat.decimal.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.log10.t.solsrc/lib/parse/LibParseDecimalFloat.soltest/src/lib/LibDecimalFloat.mul.t.soltest/src/lib/LibDecimalFloat.sub.t.soltest/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-08-29T14:58:50.500Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: src/lib/implementation/LibDecimalFloatImplementation.sol:896-899
Timestamp: 2025-08-29T14:58:50.500Z
Learning: In unchecked Solidity blocks, arithmetic operations can overflow/underflow and wrap around, so bounds checks that seem "impossible" for normal arithmetic may actually be necessary to catch overflow edge cases. For example, in withTargetExponent function, the check `exponentDiff < 0` is needed because `targetExponent - exponent` could underflow in unchecked arithmetic.
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.soltest/lib/LibDecimalFloatSlow.soltest/src/lib/implementation/LibDecimalFloatImplementation.withTargetExponent.t.soltest/src/lib/format/LibFormatDecimalFloat.countSigFigs.t.solsrc/lib/format/LibFormatDecimalFloat.soltest/src/lib/implementation/LibDecimalFloatImplementation.unabsUnsignedMulOrDivLossy.t.soltest/src/lib/LibDecimalFloat.decimal.t.soltest/src/lib/LibDecimalFloat.floor.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.eq.t.soltest/src/lib/LibDecimalFloat.ceil.t.soltest/src/lib/LibDecimalFloat.frac.t.soltest/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-07-24T04:32:14.171Z
Learnt from: rouzwelt
PR: rainlanguage/rain.math.float#83
File: src/concrete/DecimalFloat.sol:248-251
Timestamp: 2025-07-24T04:32:14.171Z
Learning: In the rainlanguage/rain.math.float project, functions in DecimalFloat.sol that return tuples from LibDecimalFloat calls must unpack the tuple into local variables before returning them (rather than returning directly) to maintain compatibility with Slither static analysis checks.
Applied to files:
test/src/lib/LibDecimalFloat.pack.t.soltest/src/lib/format/LibFormatDecimalFloat.toDecimalString.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.div.t.soltest/src/lib/LibDecimalFloat.mixed.t.solsrc/lib/format/LibFormatDecimalFloat.soltest/src/lib/LibDecimalFloat.decimal.t.solsrc/lib/parse/LibParseDecimalFloat.soltest/src/lib/LibDecimalFloat.mul.t.soltest/src/lib/LibDecimalFloat.sub.t.sol
📚 Learning: 2025-09-02T09:33:32.513Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#119
File: test/src/lib/format/LibFormatDecimalFloat.toDecimalString.t.sol:174-175
Timestamp: 2025-09-02T09:33:32.513Z
Learning: The LibFormatDecimalFloat.toDecimalString function in src/lib/format/LibFormatDecimalFloat.sol does not include rounding logic. It formats decimal floats as-is without rounding values based on significant figures limits.
Applied to files:
test/src/lib/LibDecimalFloat.pack.t.soltest/src/lib/parse/LibParseDecimalFloat.t.soltest/src/lib/LibDecimalFloat.sqrt.t.soltest/src/lib/format/LibFormatDecimalFloat.toDecimalString.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.div.t.soltest/src/lib/LibDecimalFloat.mixed.t.soltest/src/lib/format/LibFormatDecimalFloat.countSigFigs.t.solsrc/lib/format/LibFormatDecimalFloat.soltest/src/lib/LibDecimalFloat.decimal.t.soltest/src/lib/LibDecimalFloat.floor.t.solsrc/lib/parse/LibParseDecimalFloat.soltest/src/lib/LibDecimalFloat.mul.t.soltest/src/lib/LibDecimalFloat.sub.t.soltest/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-06-17T10:02:01.394Z
Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#64
File: src/concrete/DecimalFloat.sol:0-0
Timestamp: 2025-06-17T10:02:01.394Z
Learning: In the rain.math.float project, the Float type is designed with a specific bit layout: 224-bit signed coefficient and 32-bit signed exponent, represented as 32 bytes total. The LibDecimalFloat.unpack function returns values that are guaranteed to fit within int224 and int32 ranges due to this internal representation, making explicit range checks before casting unnecessary.
Applied to files:
test/src/lib/LibDecimalFloat.pack.t.sol
📚 Learning: 2025-09-10T19:20:35.927Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#135
File: test/src/lib/LibDecimalFloat.pow.t.sol:160-161
Timestamp: 2025-09-10T19:20:35.927Z
Learning: In test/src/lib/LibDecimalFloat.pow.t.sol, the comment "we do not try" in the context of "The round trip should not error so we do not try" refers to not using try/catch constructs around the powExternal call, contrasting with previous versions that used nested try/catch blocks.
Applied to files:
test/src/lib/parse/LibParseDecimalFloat.t.soltest/src/lib/LibDecimalFloat.sqrt.t.soltest/src/lib/format/LibFormatDecimalFloat.toDecimalString.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.div.t.soltest/lib/LibDecimalFloatSlow.soltest/src/lib/implementation/LibDecimalFloatImplementation.withTargetExponent.t.soltest/src/lib/format/LibFormatDecimalFloat.countSigFigs.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.maximize.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.unabsUnsignedMulOrDivLossy.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.add.t.soltest/src/lib/LibDecimalFloat.decimal.t.soltest/src/lib/LibDecimalFloat.floor.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.eq.t.soltest/src/lib/LibDecimalFloat.ceil.t.soltest/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-09-28T12:52:45.815Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#144
File: src/lib/LibDecimalFloat.sol:671-686
Timestamp: 2025-09-28T12:52:45.815Z
Learning: In LibDecimalFloat.sol, the pow function has always rejected negative bases and reverted with an error. Prior to recent changes, it used a different error type, but the fundamental policy of not allowing negative bases (except when b == 0) has been consistently maintained across the codebase's history.
Applied to files:
test/src/lib/LibDecimalFloat.sqrt.t.soltest/src/lib/implementation/LibDecimalFloatImplementation.unabsUnsignedMulOrDivLossy.t.solsrc/lib/parse/LibParseDecimalFloat.soltest/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-08-18T13:52:43.369Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#105
File: src/lib/implementation/LibDecimalFloatImplementation.sol:248-335
Timestamp: 2025-08-18T13:52:43.369Z
Learning: The codebase has a policy of not modifying external code, such as mulDiv implementations adopted from standard libraries like OpenZeppelin, PRB Math, and Solady.
Applied to files:
test/src/lib/implementation/LibDecimalFloatImplementation.div.t.soltest/src/lib/LibDecimalFloat.mul.t.sol
📚 Learning: 2025-06-16T13:17:28.513Z
Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#58
File: src/concrete/DecimalFloat.sol:175-182
Timestamp: 2025-06-16T13:17:28.513Z
Learning: In the rainlanguage/rain.math.float codebase, there's an established naming convention where functions accepting a `Float` type parameter consistently use `float` as the parameter name, even though it shadows the type name. This pattern is used throughout `LibDecimalFloat.sol` and should be maintained for consistency in related contracts like `DecimalFloat.sol`.
Applied to files:
src/lib/format/LibFormatDecimalFloat.soltest/src/lib/LibDecimalFloat.decimal.t.solsrc/lib/parse/LibParseDecimalFloat.soltest/src/lib/LibDecimalFloat.mul.t.soltest/src/lib/LibDecimalFloat.sub.t.sol
📚 Learning: 2025-09-01T19:11:36.614Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#119
File: src/lib/format/LibFormatDecimalFloat.sol:0-0
Timestamp: 2025-09-01T19:11:36.614Z
Learning: In the rainlanguage/rain.math.float project, the codebase targets a specific OpenZeppelin version rather than maintaining compatibility across arbitrary OZ versions. Direct usage of Strings.toString on signed integers is acceptable within this versioning approach.
Applied to files:
src/lib/format/LibFormatDecimalFloat.sol
📚 Learning: 2025-08-29T15:26:25.984Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: test/src/lib/LibDecimalFloat.decimal.t.sol:56-56
Timestamp: 2025-08-29T15:26:25.984Z
Learning: In Forge tests, the bound() function is overloaded and requires explicit type casting for overload resolution. When using bound(value, min, max), if the max parameter involves type conversions, an outer cast on the entire bound() call may be necessary for the compiler to resolve which overload to use. Additionally, Solidity cannot change both signedness and bit width in a single cast, so complex casts like uint256(int256(type(int224).max)) require the staged approach rather than a direct cast.
Applied to files:
test/src/lib/implementation/LibDecimalFloatImplementation.unabsUnsignedMulOrDivLossy.t.soltest/src/lib/LibDecimalFloat.decimal.t.sol
📚 Learning: 2025-09-08T12:32:24.760Z
Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#126
File: test_js/float.test.ts:26-29
Timestamp: 2025-09-08T12:32:24.760Z
Learning: In the rainlanguage/rain.math.float repository, the test_js/ directory contains tests specifically for TypeScript bindings functionality, not comprehensive feature testing. More extensive test coverage for the actual logic is maintained in the Rust and Solidity test suites.
Applied to files:
test/src/lib/implementation/LibDecimalFloatImplementation.lookupLogTableVal.t.sol
📚 Learning: 2025-08-26T12:49:02.313Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#107
File: src/lib/implementation/LibDecimalFloatImplementation.sol:282-289
Timestamp: 2025-08-26T12:49:02.313Z
Learning: In Solidity 0.8+, arithmetic operations outside unchecked blocks automatically revert on overflow/underflow, so manual overflow checks are redundant and should be avoided for gas efficiency.
Applied to files:
test/src/lib/LibDecimalFloat.decimal.t.sol
📚 Learning: 2025-06-16T13:19:32.009Z
Learnt from: 0xgleb
PR: rainlanguage/rain.math.float#58
File: crates/float/src/lib.rs:382-401
Timestamp: 2025-06-16T13:19:32.009Z
Learning: In the rainlanguage/rain.math.float codebase, variable shadowing in property tests like test_lt_eq_gt_with_add is acceptable when it represents a logical progression of the same conceptual value being tested through different states.
Applied to files:
test/src/lib/implementation/LibDecimalFloatImplementation.eq.t.sol
📚 Learning: 2025-09-28T12:53:23.484Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#144
File: src/lib/LibDecimalFloat.sol:705-709
Timestamp: 2025-09-28T12:53:23.484Z
Learning: In Solidity, bitwise operations like `exponentBInteger & 0x01 == 0x01` are unambiguous due to the type system. Even though `==` has higher precedence than `&`, the expression cannot be misinterpreted as `exponentBInteger & (0x01 == 0x01)` because you cannot use the bitwise AND operator between a uint256 and a bool. The type system enforces the correct interpretation as `(exponentBInteger & 0x01) == 0x01`.
Applied to files:
test/src/lib/implementation/LibDecimalFloatImplementation.eq.t.sol
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: rainix (ubuntu-latest, test-wasm-build)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-test)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-legal)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-static)
- GitHub Check: rainix (macos-latest, rainix-sol-legal)
- GitHub Check: rainix (macos-latest, rainix-rs-test)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-test)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-static)
- GitHub Check: git-clean
🔇 Additional comments (45)
test/src/lib/LibDecimalFloat.sub.t.sol (1)
5-5: Import cleanup looks good.Thanks for trimming the unused constants; keeping the test imports narrow helps prevent accidental dependencies from creeping back in.
test/lib/LibDecimalFloatSlow.sol (1)
42-45: Appreciate the safety rationale before disabling the lint.The note that
adjustExponentoriginates from a tightly bounded loop explains why theint256cast cannot overflow, so the targeted suppression reads justified.src/lib/parse/LibParseDecimalFloat.sol (2)
15-15: Import path split LGTM.Using ParseEmptyDecimalString from rain.string while keeping other errors local is consistent with usage in tests.
104-107: Typo fixed; resolved.“truncation” spelling now correct per earlier feedback.
test/src/lib/format/LibFormatDecimalFloat.countSigFigs.t.sol (1)
119-121: LGTM.Clear range comment and appropriate lint suppression; safe within bound [-76,0].
test/src/lib/LibDecimalFloat.mixed.t.sol (1)
6-6: LGTM.Import surface narrowed to THREES_PACKED; test usage matches.
test/src/lib/format/LibFormatDecimalFloat.toDecimalString.t.sol (1)
89-91: LGTM.Bound ensures cast safety; lint suppression is appropriate.
test/abstract/LogTest.sol (1)
6-7: LGTM.console2 confined to tests; unused-import suppression avoids lint noise.
src/lib/implementation/LibDecimalFloatImplementation.sol (4)
703-729: Fix small-table offsets for 90-row layout (wrong extcodecopy offsets).Base and stride still use 1821/910; with 90 rows these should be 1801/900. Otherwise lookups will drift once the encoder stops padding.
Apply:
function lookupLogTableVal(address tables, uint256 index) internal view returns (uint256 result) { - if (index >= 9000) { + if (index >= 9000) { revert LogTableIndexOutOfBounds(index); } assembly ("memory-safe") { // First byte of the data contract must be skipped. let mainOffset := add(1, mul(div(index, 10), 2)) mstore(0, 0) extcodecopy(tables, 30, mainOffset, 2) let mainTableVal := mload(0) result := and(mainTableVal, 0x7FFF) - // Skip first byte of data contract then 1820 bytes - // of the log tables. - let smallTableOffset := 1821 + // Skip first byte then 1800 bytes of the main log table. + let smallTableOffset := 1801 if iszero(iszero(and(mainTableVal, 0x8000))) { // Small table is half the size of the main // table. - smallTableOffset := add(smallTableOffset, 910) + smallTableOffset := add(smallTableOffset, 900) } mstore(0, 0) extcodecopy(tables, 31, add(smallTableOffset, add(mul(div(index, 100), 10), mod(index, 10))), 1) result := add(result, mload(0)) } }Run to find any remaining stale offsets:
#!/bin/bash rg -n -C2 'smallTableOffset := 1821|add\(smallTableOffset, 910\)|\boffset := 2831\b|\b2020\b' src --type=sol
816-823: Restore positive terminal slope in log10 interpolation (avoid y2==y1 at idx=last).When idx==8999, y2Coefficient equals y1Coefficient, flattening the last interval and breaking monotonicity/accuracy. Use 10001 at the tail.
- if (x1Coefficient != signedCoefficient) { - y2Coefficient = - idx == 8999 ? int256(1e76) : int256(1e72 * lookupLogTableVal(tablesDataContract, idx + 1)); - } + if (x1Coefficient != signedCoefficient) { + y2Coefficient = + idx == 8999 ? int256(1e72 * 10001) : int256(1e72 * lookupLogTableVal(tablesDataContract, idx + 1)); + }
703-706: Avoid magic numbers (9000/8999); define and reuse cardinality constants.Reduces drift when table layout changes.
Add near other constants:
int256 constant LOG10_Y_EXPONENT = -76; +uint256 constant LOG_MANTISSA_IDX_CARDINALITY = 9000; +uint256 constant LOG_MANTISSA_LAST_INDEX = LOG_MANTISSA_IDX_CARDINALITY - 1;Then:
- if (index >= 9000) { + if (index >= LOG_MANTISSA_IDX_CARDINALITY) { revert LogTableIndexOutOfBounds(index); }And:
- idx == 8999 ? int256(1e72 * 10001) : ... + idx == LOG_MANTISSA_LAST_INDEX ? int256(1e72 * 10001) : ...Also applies to: 816-823
1144-1171: Consolidate table-offset math inlookupAntilogTableY1Y2: derive the hard-coded2831and2020values from shared constants/helpers and confirm they align with the updated encoder.test/src/lib/implementation/LibDecimalFloatImplementation.maximize.t.sol (1)
67-69: LGTM: comment + lint pragma are appropriate.Range annotation matches loop bounds [0,76]; cast suppression is scoped to the next line only.
test/src/lib/implementation/LibDecimalFloatImplementation.add.t.sol (1)
177-179: LGTM: clarifying constant + scoped lint suppression.Helpful doc for ADD_MAX_EXPONENT_DIFF and keeps assume readable.
test/src/lib/implementation/LibDecimalFloatImplementation.log10.t.sol (1)
20-23: LGTM: annotate log and suppress cast locally.Purely diagnostic; no behavioral impact.
test/src/lib/implementation/LibDecimalFloatImplementation.withTargetExponent.t.sol (2)
59-61: LGTM: bounds notes + cast suppressions are tight and accurate.All these spots document [1,76] diffs and keep casts localized.
Also applies to: 81-83, 89-91, 148-150, 180-181, 189-190
202-204: Fix reversed relation in comment (targetExponent vs exponent).Given targetExponent = exponent - targetExponentDiff (diff ∈ [1,76]), the correct relation here is exponent > targetExponent, not targetExponent > exponent.
Apply:
- // targetExponent > exponent + // exponent > targetExponentsrc/lib/format/LibFormatDecimalFloat.sol (7)
35-41: LGTM: sig-figs adjustment clarifications.Casts are justified post-negation; comments help future readers.
65-71: LGTM: explicit scale/scaleExponent split for scientific.Both 1e75 and 1e76 fit in int256; branch is clear.
74-76: Confirm exponent bounds to prevent 10exponent blowups.**Multiplying by 10**exponent on int256 can overflow if exponent unexpectedly exceeds ~76. Please confirm upstream invariants (e.g., exponent ∈ [-76,76]) or add a guard.
Based on learnings.
81-86: LGTM: non-scientific scaling paths are explicit.Negated exponent cast rationale is documented; zero-exponent case sets scaleExponent=0.
Also applies to: 87-89
120-123: Prefer uint math for leading‑zero detection; drop signed cast.Use an unsigned compare to avoid repeated signed division and the cast to int256(fracScale).
[ suggest_recommended_refactor ]
Apply:- // fracScale being 10x less than scale means it cannot overflow - // when cast to `int256`. - // forge-lint: disable-next-line(unsafe-typecast) - while (fractional / int256(fracScale) == 0) { - fracScale /= 10; - fracLeadingZeros++; - } + // Use unsigned compare; fractional is made non‑negative below. + uint256 f = uint256(fractional); + while (fracScale > 1 && f < fracScale) { + fracScale /= 10; + fracLeadingZeros++; + } + // Trim trailing zeros using uint ops, then write back. + while (f % 10 == 0 && f != 0) { + f /= 10; + } + // forge-lint: disable-next-line(unsafe-typecast) + fractional = int256(f);This also reduces divisions and avoids potential edge fragility. Based on learnings.
141-146: LGTM: displayExponent cast rationale is clear.Comment accurately documents provenance of scaleExponent and safe cast back.
92-101: Critical: avoid castingscaleto int256 for/and%(use uint math for q,r).In the non‑scientific path
scale = 10^(-exponent)can approach 1e76. Castingscaleto int256 and performing signed div/mod is fragile and was previously flagged. Compute quotient/remainder in uint space on |signedCoefficient|, then reapply sign. This removes truncation/overflow hazards and matches prior guidance.Apply:
- int256 integral = signedCoefficient; - int256 fractional = int256(0); - if (scale != 0) { - // scale is one of two possible values so won't truncate when cast. - // forge-lint: disable-next-line(unsafe-typecast) - integral = signedCoefficient / int256(scale); - // scale is one of two possible values so won't truncate when cast. - // forge-lint: disable-next-line(unsafe-typecast) - fractional = signedCoefficient % int256(scale); - } + int256 integral = signedCoefficient; + int256 fractional = 0; + if (scale != 0) { + bool neg = signedCoefficient < 0; + // Compute |signedCoefficient| as uint via two's-complement to handle int256.min. + uint256 u = uint256(signedCoefficient); + uint256 absCoeff = neg ? ((~u) + 1) : u; + uint256 q = absCoeff / scale; + uint256 r = absCoeff % scale; + // q,r ≤ absCoeff; with scale ≥ 10 in this branch, q < 2^255. Safe casts. + integral = neg ? -int256(q) : int256(q); + fractional = neg ? -int256(r) : int256(r); + }Based on learnings.
test/src/lib/LibDecimalFloat.min.t.sol (1)
19-22: LGTM on targeted lint suppression.Mixed-case variable lint is intentionally suppressed for readability of minXY/minYX; consistent with suite-wide pattern.
test/src/lib/LibDecimalFloat.max.t.sol (1)
19-22: LGTM; consistent lint suppression.Keeping maxXY/maxYX names and suppressing the mixed-case lint aligns with the broader test suite.
test/src/lib/LibDecimalFloat.ceil.t.sol (1)
61-63: Safe cast; bounds make this fine.exponent is bounded to [-76, -1], so
uint256(-exponent)∈ [1, 76] and10 ** ...fits in int256. Suppression is appropriate.test/src/lib/LibDecimalFloat.pack.t.sol (1)
5-5: Good import hygiene.Limiting to {LibDecimalFloat, ExponentOverflow, Float} removes unused imports and reduces coupling.
test/src/lib/implementation/LibDecimalFloatImplementation.div.t.sol (1)
5-5: Cleanup stdError imports in decimal tests
Remove thestdErrorimport attest/src/lib/LibDecimalFloat.decimal.t.sol:6and replacevm.expectRevert(stdError.arithmeticError)at line 311 with the specific revert selector.Likely an incorrect or invalid review comment.
test/src/lib/LibDecimalFloat.floor.t.sol (1)
38-43: Safe cast and intentional truncation: LGTMGiven exponent is bounded to [-76, -1], 10**(-exponent) ≤ 1e76 < int256.max, so the int256 cast is safe; truncation via (x/scale)*scale is intentional and correct.
test/src/lib/implementation/LibDecimalFloatImplementation.unabsUnsignedMulOrDivLossy.t.sol (1)
17-19: Casts and overflow guards look correct
- The int256 casts are preceded by tight bounds, making the unsafe-typecast suppressions justified.
- Cases that bump exponent use vm.assume(exponent != type(int256).max), preventing overflow.
If there are any remaining exponent+1 paths not covered by vm.assume, mirror the same guard.
Also applies to: 21-23, 41-43, 45-47, 60-62, 64-66, 83-87, 88-91, 104-108, 109-112, 126-131, 133-136, 153-156, 159-161, 173-177
test/src/lib/LibDecimalFloat.mul.t.sol (1)
5-5: Import surface simplification looks good.Only LibDecimalFloat and Float are needed here; tests still cover both implementation and facade paths via mulExternal methods.
test/src/lib/LibDecimalFloat.sqrt.t.sol (3)
8-8: Aligns sqrt negative-path to PowNegativeBase.Importing PowNegativeBase matches current revert contract for negative bases.
15-16: Tighter diffLimit acknowledged.0.0019 tolerance is fine for sqrt round-trips given table interpolation; flag if any flakes appear with future table updates.
69-72: Negative sqrt revert assertion updated correctly.Expecting PowNegativeBase with unpacked inputs is consistent and precise.
test/src/lib/LibDecimalFloat.decimal.t.sol (4)
6-6: Import cleanup is correct.Keeping stdError and removing unused console/implementation imports reduces noise.
206-210: Lint waiver comments are acceptable as-is.Given inputs are pre-bounded in this test, the cast is safe; comments document intent.
276-283: Assert finalExponent bounds before unsigned cast (optional).Add asserts for 1 <= finalExponent <= 77 to guard future edits and document why the cast is safe.
299-306: Same optional asserts for the overflow-variant block.Mirror the bounds/non-negativity asserts here to keep both paths consistent.
test/src/lib/LibDecimalFloat.pow.t.sol (5)
8-8: Error imports updated to current API.PowNegativeBase and ZeroNegativePower are the right selectors; stale imports removed.
15-16: diffLimit set to 8.6% matches worst‑case table interpolation.This aligns with the conservative bound discussed for fractional exponent paths.
61-67: Deterministic large‑mantissa baseline retained.If it’s stable across normalization/rounding, keeping exact equality is fine.
86-87: Negative‑base pow revert assertion is correct.Selector and payload match implementation contract.
147-150: Doc comment now defers to diffLimit — good.Removes drift risk between docs and tolerance.
test/src/lib/implementation/LibDecimalFloatImplementation.lookupLogTableVal.t.sol (1)
15-108: Add out‑of‑bounds revert coverage for index ≥ 9000.We should assert LogTableIndexOutOfBounds to protect the guard.
Apply this diff:
@@ -import {LibDecimalFloatImplementation} from "src/lib/implementation/LibDecimalFloatImplementation.sol"; +import {LibDecimalFloatImplementation} from "src/lib/implementation/LibDecimalFloatImplementation.sol"; +import {LogTableIndexOutOfBounds} from "src/error/ErrDecimalFloat.sol"; @@ function testLookupLogTableVal() external { @@ checkLookupLogTableVal(8999, 10000); + // Out-of-bounds indices revert. + address tables = logTables(); + vm.expectRevert(abi.encodeWithSelector(LogTableIndexOutOfBounds.selector, 9000)); + LibDecimalFloatImplementation.lookupLogTableVal(tables, 9000); }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
src/generated/LogTables.pointers.solis excluded by!**/generated/**
📒 Files selected for processing (1)
test/src/lib/LibDecimalFloat.frac.t.sol(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#144
File: src/lib/LibDecimalFloat.sol:671-686
Timestamp: 2025-09-28T12:52:45.815Z
Learning: In LibDecimalFloat.sol, the pow function has always rejected negative bases and reverted with an error. Prior to recent changes, it used a different error type, but the fundamental policy of not allowing negative bases (except when b == 0) has been consistently maintained across the codebase's history.
📚 Learning: 2025-08-29T14:58:50.500Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: src/lib/implementation/LibDecimalFloatImplementation.sol:896-899
Timestamp: 2025-08-29T14:58:50.500Z
Learning: In unchecked Solidity blocks, arithmetic operations can overflow/underflow and wrap around, so bounds checks that seem "impossible" for normal arithmetic may actually be necessary to catch overflow edge cases. For example, in withTargetExponent function, the check `exponentDiff < 0` is needed because `targetExponent - exponent` could underflow in unchecked arithmetic.
Applied to files:
test/src/lib/LibDecimalFloat.frac.t.sol
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: rainix (ubuntu-latest, test-wasm-build)
- GitHub Check: rainix (macos-latest, rainix-sol-legal)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-test)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-static)
- GitHub Check: rainix (macos-latest, rainix-rs-test)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-test)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-static)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-legal)
- GitHub Check: git-clean
🔇 Additional comments (1)
test/src/lib/LibDecimalFloat.frac.t.sol (1)
39-41: Thanks for scoping the lint suppression tightly.Calling out the exponent band and limiting the
forge-lintwaiver to the single cast keeps the intent clear and avoids muting future findings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
test/src/lib/LibDecimalFloat.frac.t.sol(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-28T13:28:35.814Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#144
File: test/src/lib/LibDecimalFloat.pow.t.sol:40-61
Timestamp: 2025-09-28T13:28:35.814Z
Learning: In test/src/lib/LibDecimalFloat.pow.t.sol, the diffLimit tolerance (currently 8.6%) represents a conservative worst-case bound to accommodate precision limitations of table lookup interpolation for fractional exponents in the pow function. The exponentiation-by-squaring approach used for integer parts achieves much tighter precision, but the tolerance must account for the less precise table-based calculations used for fractional exponents.
Applied to files:
test/src/lib/LibDecimalFloat.frac.t.sol
📚 Learning: 2025-08-29T14:58:50.500Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: src/lib/implementation/LibDecimalFloatImplementation.sol:896-899
Timestamp: 2025-08-29T14:58:50.500Z
Learning: In unchecked Solidity blocks, arithmetic operations can overflow/underflow and wrap around, so bounds checks that seem "impossible" for normal arithmetic may actually be necessary to catch overflow edge cases. For example, in withTargetExponent function, the check `exponentDiff < 0` is needed because `targetExponent - exponent` could underflow in unchecked arithmetic.
Applied to files:
test/src/lib/LibDecimalFloat.frac.t.sol
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: rainix (macos-latest, rainix-sol-legal)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-test)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-legal)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-static)
- GitHub Check: rainix (ubuntu-latest, test-wasm-build)
- GitHub Check: rainix (macos-latest, rainix-rs-test)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-static)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-test)
- GitHub Check: git-clean
🔇 Additional comments (1)
test/src/lib/LibDecimalFloat.frac.t.sol (1)
39-41: Bound + cast LGTM; verify CI lint support
Please confirm your CI pipeline’s linter recognizes theforge-lint: disable-next-line(unsafe-typecast)directive.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/lib/parse/LibParseDecimalFloat.sol (2)
83-86: Good: inline parser returns selector instead of reverting.This aligns with the no-throw contract of parseDecimalFloatInline and prior guidance.
94-107: Name the 67 bound; keep logic, improve clarity.Replace the magic number with a constant documenting packing/precision limits. No pow10 table needed (single execution).
- if (scale > 67) { + if (scale > MAX_FRACTIONAL_RESCALE_DIGITS) { return (ParseDecimalPrecisionLoss.selector, cursor, 0, 0); } - scale = 10 ** scale; + scale = 10 ** scale;Add near the top of the library:
// Max fractional digits that can be losslessly rescaled into the coefficient. uint256 constant MAX_FRACTIONAL_RESCALE_DIGITS = 67;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/lib/parse/LibParseDecimalFloat.sol(2 hunks)
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#144
File: src/lib/LibDecimalFloat.sol:671-686
Timestamp: 2025-09-28T12:52:45.815Z
Learning: In LibDecimalFloat.sol, the pow function has always rejected negative bases and reverted with an error. Prior to recent changes, it used a different error type, but the fundamental policy of not allowing negative bases (except when b == 0) has been consistently maintained across the codebase's history.
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#144
File: test/src/lib/LibDecimalFloat.pow.t.sol:40-61
Timestamp: 2025-09-28T13:28:35.814Z
Learning: In test/src/lib/LibDecimalFloat.pow.t.sol, the diffLimit tolerance (currently 8.6%) represents a conservative worst-case bound to accommodate precision limitations of table lookup interpolation for fractional exponents in the pow function. The exponentiation-by-squaring approach used for integer parts achieves much tighter precision, but the tolerance must account for the less precise table-based calculations used for fractional exponents.
📚 Learning: 2025-09-30T15:09:39.424Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#144
File: src/lib/parse/LibParseDecimalFloat.sol:88-111
Timestamp: 2025-09-30T15:09:39.424Z
Learning: In src/lib/parse/LibParseDecimalFloat.sol, the fractional rescaling path (around lines 88-111) executes at most once per parseDecimalFloatInline call within an if-else branch, not in a loop, so there are no repeated 10**scale operations to optimize with a lookup table.
Applied to files:
src/lib/parse/LibParseDecimalFloat.sol
📚 Learning: 2025-09-09T15:11:31.003Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#130
File: src/lib/implementation/LibDecimalFloatImplementation.sol:320-330
Timestamp: 2025-09-09T15:11:31.003Z
Learning: In LibDecimalFloatImplementation.sol div function, returning zero when underflowExponentBy > 76 is intentional "non-error underflow" behavior rather than reverting. This graceful handling is the main goal of the changes.
Applied to files:
src/lib/parse/LibParseDecimalFloat.sol
📚 Learning: 2025-08-29T14:54:24.240Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: test/src/lib/LibDecimalFloat.decimal.t.sol:122-126
Timestamp: 2025-08-29T14:54:24.240Z
Learning: In test/src/lib/LibDecimalFloat.decimal.t.sol, the fromFixedDecimalLossy function converts uint256(type(int256).max) losslessly because the boundary check is `value > uint256(type(int256).max)`, not `>=`. Values exactly equal to type(int256).max still take the lossless conversion path.
Applied to files:
src/lib/parse/LibParseDecimalFloat.sol
📚 Learning: 2025-07-24T04:32:14.171Z
Learnt from: rouzwelt
PR: rainlanguage/rain.math.float#83
File: src/concrete/DecimalFloat.sol:248-251
Timestamp: 2025-07-24T04:32:14.171Z
Learning: In the rainlanguage/rain.math.float project, functions in DecimalFloat.sol that return tuples from LibDecimalFloat calls must unpack the tuple into local variables before returning them (rather than returning directly) to maintain compatibility with Slither static analysis checks.
Applied to files:
src/lib/parse/LibParseDecimalFloat.sol
📚 Learning: 2025-09-28T13:28:35.814Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#144
File: test/src/lib/LibDecimalFloat.pow.t.sol:40-61
Timestamp: 2025-09-28T13:28:35.814Z
Learning: In test/src/lib/LibDecimalFloat.pow.t.sol, the diffLimit tolerance (currently 8.6%) represents a conservative worst-case bound to accommodate precision limitations of table lookup interpolation for fractional exponents in the pow function. The exponentiation-by-squaring approach used for integer parts achieves much tighter precision, but the tolerance must account for the less precise table-based calculations used for fractional exponents.
Applied to files:
src/lib/parse/LibParseDecimalFloat.sol
📚 Learning: 2025-08-21T18:03:40.347Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#107
File: test/lib/LibDecimalFloatSlow.sol:37-45
Timestamp: 2025-08-21T18:03:40.347Z
Learning: In test/lib/LibDecimalFloatSlow.sol, the "slow" implementation is intentionally different from the production implementation to serve as an independent reference for fuzzing tests. The goal is to have two different approaches (expensive loops vs optimized jumps) that produce equivalent results, not identical implementations.
Applied to files:
src/lib/parse/LibParseDecimalFloat.sol
📚 Learning: 2025-09-02T09:33:32.513Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#119
File: test/src/lib/format/LibFormatDecimalFloat.toDecimalString.t.sol:174-175
Timestamp: 2025-09-02T09:33:32.513Z
Learning: The LibFormatDecimalFloat.toDecimalString function in src/lib/format/LibFormatDecimalFloat.sol does not include rounding logic. It formats decimal floats as-is without rounding values based on significant figures limits.
Applied to files:
src/lib/parse/LibParseDecimalFloat.sol
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-test)
- GitHub Check: rainix (macos-latest, rainix-sol-legal)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-test)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-legal)
🔇 Additional comments (2)
src/lib/parse/LibParseDecimalFloat.sol (2)
88-90: LGTM: zero integer part fast-path.Assigning fracValue when signedCoefficient is 0 avoids unnecessary rescale.
15-15: Verify mixed error import origin
This library imports ParseEmptyDecimalString from rain.string/error/ErrParse.sol while other parsing errors are sourced from ../../error/ErrParse.sol; confirm this blend is intentional and that the error definitions and selectors remain version-consistent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/implementation/LibDecimalFloatImplementation.sol (1)
1158-1185: Verify antilog (pow10) table offsets; hoist constants
lookupAntilogTableY1Y2 hard-codes 2831 and 2020 (src/lib/implementation/LibDecimalFloatImplementation.sol:1158–1185); those values are derived from the log-table encoder (see src/lib/table/LibLogTable.sol — mstore lengths 910, 2020 and pointer entries including 2831). Replace these magic literals with named constants (e.g., DATA_HEADER_BYTES, MAIN_LOG_BYTES, SMALL_LOG_BYTES, ALT_SMALL_BYTES) or compute offsets from a single source of truth in the encoder, and confirm the encoder/table sizes (1820/910/100) before changing.
♻️ Duplicate comments (2)
src/lib/implementation/LibDecimalFloatImplementation.sol (2)
796-826: log10 index and interpolation flow LGTMIndex math, bounds via lookupLogTableVal, and tail-case handling are coherent. Terminal zero-slope at the last index is acceptable given table granularity, as discussed.
Also applies to: 828-836
723-731: Critical: log-table offsets still reflect old 91-row layout (1821/910)With 90 rows the correct values are base 1801 and stride 900. Leaving 1821/910 will misread rows once padding is removed. Fix the offsets and the comment.
Apply:
- // Skip first byte of data contract then 1820 bytes - // of the log tables. - let smallTableOffset := 1821 + // Skip first byte of data contract then 1800 bytes + // of the log tables. + let smallTableOffset := 1801 if iszero(iszero(and(mainTableVal, 0x8000))) { - // Small table is half the size of the main - // table. - smallTableOffset := add(smallTableOffset, 910) + // Small table is half the size of the main table (900 bytes). + smallTableOffset := add(smallTableOffset, 900) }Optionally, derive from named constants to prevent future drift:
- let smallTableOffset := 1801 + let smallTableOffset := add(1, LOG_MAIN_BYTES) if iszero(iszero(and(mainTableVal, 0x8000))) { - smallTableOffset := add(smallTableOffset, 900) + smallTableOffset := add(smallTableOffset, LOG_SMALL_BYTES) }Based on learnings.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/lib/implementation/LibDecimalFloatImplementation.sol(14 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#144
File: src/lib/LibDecimalFloat.sol:671-686
Timestamp: 2025-09-28T12:52:45.815Z
Learning: In LibDecimalFloat.sol, the pow function has always rejected negative bases and reverted with an error. Prior to recent changes, it used a different error type, but the fundamental policy of not allowing negative bases (except when b == 0) has been consistently maintained across the codebase's history.
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#144
File: test/src/lib/LibDecimalFloat.pow.t.sol:40-61
Timestamp: 2025-09-28T13:28:35.814Z
Learning: In test/src/lib/LibDecimalFloat.pow.t.sol, the diffLimit tolerance (currently 8.6%) represents a conservative worst-case bound to accommodate precision limitations of table lookup interpolation for fractional exponents in the pow function. The exponentiation-by-squaring approach used for integer parts achieves much tighter precision, but the tolerance must account for the less precise table-based calculations used for fractional exponents.
📚 Learning: 2025-09-30T15:09:39.424Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#144
File: src/lib/parse/LibParseDecimalFloat.sol:88-111
Timestamp: 2025-09-30T15:09:39.424Z
Learning: In src/lib/parse/LibParseDecimalFloat.sol, the fractional rescaling path (around lines 88-111) executes at most once per parseDecimalFloatInline call within an if-else branch, not in a loop, so there are no repeated 10**scale operations to optimize with a lookup table.
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.sol
📚 Learning: 2025-09-09T15:11:31.003Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#130
File: src/lib/implementation/LibDecimalFloatImplementation.sol:320-330
Timestamp: 2025-09-09T15:11:31.003Z
Learning: In LibDecimalFloatImplementation.sol div function, returning zero when underflowExponentBy > 76 is intentional "non-error underflow" behavior rather than reverting. This graceful handling is the main goal of the changes.
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.sol
📚 Learning: 2025-08-29T14:54:24.240Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: test/src/lib/LibDecimalFloat.decimal.t.sol:122-126
Timestamp: 2025-08-29T14:54:24.240Z
Learning: In test/src/lib/LibDecimalFloat.decimal.t.sol, the fromFixedDecimalLossy function converts uint256(type(int256).max) losslessly because the boundary check is `value > uint256(type(int256).max)`, not `>=`. Values exactly equal to type(int256).max still take the lossless conversion path.
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.sol
📚 Learning: 2025-08-29T10:38:26.353Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: src/lib/implementation/LibDecimalFloatImplementation.sol:593-601
Timestamp: 2025-08-29T10:38:26.353Z
Learning: In Solidity, int256(1) when passed through the maximize function in LibDecimalFloatImplementation.sol produces exactly (1e76, -76), not an approximation. This means the special case for signedCoefficient == 1e76 in log10 correctly handles powers of 10 like log10(1).
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.sol
📚 Learning: 2025-08-29T10:38:26.353Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: src/lib/implementation/LibDecimalFloatImplementation.sol:593-601
Timestamp: 2025-08-29T10:38:26.353Z
Learning: The maximize function in LibDecimalFloatImplementation.sol produces exact results for simple integer values like 1. maximize(1, 0) yields exactly (1e76, -76) with no precision loss, and the log10 special case for signedCoefficient == 1e76 correctly handles this.
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.sol
📚 Learning: 2025-08-29T14:58:50.500Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: src/lib/implementation/LibDecimalFloatImplementation.sol:896-899
Timestamp: 2025-08-29T14:58:50.500Z
Learning: In unchecked Solidity blocks, arithmetic operations can overflow/underflow and wrap around, so bounds checks that seem "impossible" for normal arithmetic may actually be necessary to catch overflow edge cases. For example, in withTargetExponent function, the check `exponentDiff < 0` is needed because `targetExponent - exponent` could underflow in unchecked arithmetic.
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.sol
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: rainix (ubuntu-latest, test-wasm-build)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-test)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-legal)
- GitHub Check: rainix (macos-latest, rainix-rs-test)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-static)
- GitHub Check: rainix (macos-latest, rainix-sol-legal)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-test)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-static)
- GitHub Check: git-clean
🔇 Additional comments (1)
src/lib/implementation/LibDecimalFloatImplementation.sol (1)
11-13: Error typing for lookup bounds is correctImporting LogTableIndexOutOfBounds and MaximizeOverflow here is appropriate and keeps errors precise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.gas-snapshot (1)
335-346: Parse rescaling: avoid needless 10scale micro‑opts**Ensure the fractional rescaling path remains a single execution per parse and doesn’t introduce a lookup table or repeated exponentiation; it brings no benefit here and adds code complexity.
Based on learnings.
♻️ Duplicate comments (3)
test/src/lib/LibDecimalFloat.pow.t.sol (3)
8-8: Imports align with current error surface.Good swap to PowNegativeBase and ZeroNegativePower; both are used below.
86-87: Negative base revert path validated.Expecting PowNegativeBase with unpacked a is correct and preserves longstanding semantics.
147-150: Round‑trip docs now source tolerance from diffLimit.Nice cleanup; avoids drift between comments and code.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
src/generated/LogTables.pointers.solis excluded by!**/generated/**
📒 Files selected for processing (4)
.gas-snapshot(2 hunks)src/lib/implementation/LibDecimalFloatImplementation.sol(13 hunks)src/lib/table/LibLogTable.sol(11 hunks)test/src/lib/LibDecimalFloat.pow.t.sol(4 hunks)
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#144
File: src/lib/LibDecimalFloat.sol:671-686
Timestamp: 2025-09-28T12:52:45.815Z
Learning: In LibDecimalFloat.sol, the pow function has always rejected negative bases and reverted with an error. Prior to recent changes, it used a different error type, but the fundamental policy of not allowing negative bases (except when b == 0) has been consistently maintained across the codebase's history.
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#144
File: test/src/lib/LibDecimalFloat.pow.t.sol:40-61
Timestamp: 2025-09-28T13:28:35.814Z
Learning: In test/src/lib/LibDecimalFloat.pow.t.sol, the diffLimit tolerance (currently 8.6%) represents a conservative worst-case bound to accommodate precision limitations of table lookup interpolation for fractional exponents in the pow function. The exponentiation-by-squaring approach used for integer parts achieves much tighter precision, but the tolerance must account for the less precise table-based calculations used for fractional exponents.
📚 Learning: 2025-09-28T13:28:35.814Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#144
File: test/src/lib/LibDecimalFloat.pow.t.sol:40-61
Timestamp: 2025-09-28T13:28:35.814Z
Learning: In test/src/lib/LibDecimalFloat.pow.t.sol, the diffLimit tolerance (currently 8.6%) represents a conservative worst-case bound to accommodate precision limitations of table lookup interpolation for fractional exponents in the pow function. The exponentiation-by-squaring approach used for integer parts achieves much tighter precision, but the tolerance must account for the less precise table-based calculations used for fractional exponents.
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-09-28T12:52:45.815Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#144
File: src/lib/LibDecimalFloat.sol:671-686
Timestamp: 2025-09-28T12:52:45.815Z
Learning: In LibDecimalFloat.sol, the pow function has always rejected negative bases and reverted with an error. Prior to recent changes, it used a different error type, but the fundamental policy of not allowing negative bases (except when b == 0) has been consistently maintained across the codebase's history.
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-09-10T19:20:35.927Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#135
File: test/src/lib/LibDecimalFloat.pow.t.sol:160-161
Timestamp: 2025-09-10T19:20:35.927Z
Learning: In test/src/lib/LibDecimalFloat.pow.t.sol, the comment "we do not try" in the context of "The round trip should not error so we do not try" refers to not using try/catch constructs around the powExternal call, contrasting with previous versions that used nested try/catch blocks.
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-08-21T18:03:40.347Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#107
File: test/lib/LibDecimalFloatSlow.sol:37-45
Timestamp: 2025-08-21T18:03:40.347Z
Learning: In test/lib/LibDecimalFloatSlow.sol, the "slow" implementation is intentionally different from the production implementation to serve as an independent reference for fuzzing tests. The goal is to have two different approaches (expensive loops vs optimized jumps) that produce equivalent results, not identical implementations.
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.sol.gas-snapshot
📚 Learning: 2025-08-29T14:54:24.240Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: test/src/lib/LibDecimalFloat.decimal.t.sol:122-126
Timestamp: 2025-08-29T14:54:24.240Z
Learning: In test/src/lib/LibDecimalFloat.decimal.t.sol, the fromFixedDecimalLossy function converts uint256(type(int256).max) losslessly because the boundary check is `value > uint256(type(int256).max)`, not `>=`. Values exactly equal to type(int256).max still take the lossless conversion path.
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.solsrc/lib/implementation/LibDecimalFloatImplementation.sol
📚 Learning: 2025-09-09T15:11:31.003Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#130
File: src/lib/implementation/LibDecimalFloatImplementation.sol:320-330
Timestamp: 2025-09-09T15:11:31.003Z
Learning: In LibDecimalFloatImplementation.sol div function, returning zero when underflowExponentBy > 76 is intentional "non-error underflow" behavior rather than reverting. This graceful handling is the main goal of the changes.
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-08-14T16:32:05.932Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#99
File: src/lib/implementation/LibDecimalFloatImplementation.sol:309-325
Timestamp: 2025-08-14T16:32:05.932Z
Learning: In the rainlanguage/rain.math.float repository, maintainer thedavidmeister prefers to keep assembly-based overflow checks inline for gas optimization rather than extracting them into helper functions, even when it results in code duplication.
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-08-11T14:32:50.439Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#96
File: test/src/lib/implementation/LibDecimalFloatImplementation.maximize.t.sol:15-29
Timestamp: 2025-08-11T14:32:50.439Z
Learning: In test code for the rain.math.float repository, redundant checks may be intentionally kept for clarity and documentation purposes, even when they could be simplified. The maintainer (thedavidmeister) prefers explicit assertions in test code to make the test's intent clear to future readers, prioritizing readability over conciseness.
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.sol
📚 Learning: 2025-08-29T14:58:50.500Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: src/lib/implementation/LibDecimalFloatImplementation.sol:896-899
Timestamp: 2025-08-29T14:58:50.500Z
Learning: In unchecked Solidity blocks, arithmetic operations can overflow/underflow and wrap around, so bounds checks that seem "impossible" for normal arithmetic may actually be necessary to catch overflow edge cases. For example, in withTargetExponent function, the check `exponentDiff < 0` is needed because `targetExponent - exponent` could underflow in unchecked arithmetic.
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.solsrc/lib/implementation/LibDecimalFloatImplementation.sol
📚 Learning: 2025-08-29T10:38:26.353Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: src/lib/implementation/LibDecimalFloatImplementation.sol:593-601
Timestamp: 2025-08-29T10:38:26.353Z
Learning: The maximize function in LibDecimalFloatImplementation.sol produces exact results for simple integer values like 1. maximize(1, 0) yields exactly (1e76, -76) with no precision loss, and the log10 special case for signedCoefficient == 1e76 correctly handles this.
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.solsrc/lib/implementation/LibDecimalFloatImplementation.sol
📚 Learning: 2025-08-29T10:38:26.353Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: src/lib/implementation/LibDecimalFloatImplementation.sol:593-601
Timestamp: 2025-08-29T10:38:26.353Z
Learning: In Solidity, int256(1) when passed through the maximize function in LibDecimalFloatImplementation.sol produces exactly (1e76, -76), not an approximation. This means the special case for signedCoefficient == 1e76 in log10 correctly handles powers of 10 like log10(1).
Applied to files:
test/src/lib/LibDecimalFloat.pow.t.solsrc/lib/implementation/LibDecimalFloatImplementation.sol
📚 Learning: 2025-09-30T15:09:39.424Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#144
File: src/lib/parse/LibParseDecimalFloat.sol:88-111
Timestamp: 2025-09-30T15:09:39.424Z
Learning: In src/lib/parse/LibParseDecimalFloat.sol, the fractional rescaling path (around lines 88-111) executes at most once per parseDecimalFloatInline call within an if-else branch, not in a loop, so there are no repeated 10**scale operations to optimize with a lookup table.
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.sol.gas-snapshot
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-test)
- GitHub Check: rainix (ubuntu-latest, test-wasm-build)
- GitHub Check: git-clean
🔇 Additional comments (18)
src/lib/implementation/LibDecimalFloatImplementation.sol (8)
5-43: LGTM: Constants centralized and imported from LibLogTable.The new imports (LOG_TABLE_SIZE_BYTES, LOG_TABLE_SIZE_BASE, LOG_MANTISSA_IDX_CARDINALITY, LOG_MANTISSA_LAST_INDEX, ANTILOG_IDX_LAST_INDEX) eliminate magic numbers and centralize table configuration, improving maintainability.
77-130: LGTM: Type casting suppressions are appropriate.The forge-lint disable directives for unsafe-typecast are properly placed and the comments clearly explain the bounds checking that ensures safe casting.
181-183: LGTM: Bounded casts properly documented.The unsafe-typecast suppressions at lines 181-183 and 421-423 are appropriate given the documented bounds on adjustExponent and underflowExponentBy respectively.
Also applies to: 421-423
624-636: LGTM: Type casting suppressions with documented bounds.The forge-lint disable directives for unsafe-typecast are appropriate. All bounds are documented (e.g., exponentDiff [0, 76]), ensuring safe casting from uint256 to int256.
Also applies to: 1053-1055, 1091-1093, 1099-1101
710-737: LGTM: Centralized log table lookup with proper bounds checking.The new
lookupLogTableValfunction centralizes table lookups and uses imported constants (LOG_MANTISSA_LAST_INDEX, LOG_TABLE_SIZE_BYTES, LOG_TABLE_SIZE_BASE) consistently. The bounds check on line 711 prevents out-of-range access. Based on learnings.
794-834: LGTM: log10 function updated to use centralized lookup and constants.The changes properly:
- Add forge-lint suppressions for bounded casts
- Use the centralized
lookupLogTableValfunction (line 826, 833)- Use LOG_MANTISSA_LAST_INDEX constant (line 831) instead of magic number 8999
- Handle terminal interpolation case appropriately
886-894: LGTM: Type casting suppressions in pow10 and characteristicMantissa.The forge-lint disable directives are appropriate:
- Line 892: idx is positive due to early negative check
- Line 1130: exponent is bounded [-76, -1]
- Line 1154: Exponent bounds ensure safe casting
Also applies to: 1129-1131, 1154-1155
1160-1191: LGTM: lookupAntilogTableY1Y2 updated with constants.The function now properly:
- Uses ANTILOG_IDX_LAST_INDEX for bounds checking (line 1166)
- Uses LOG_TABLE_SIZE_BYTES and LOG_TABLE_SIZE_BASE for offset calculations (line 1173)
- Documents table sizes in assembly comments (line 1181)
src/lib/table/LibLogTable.sol (5)
7-20: LGTM: Constants well-defined and consistent.The new constants properly define table dimensions:
- LOG_MANTISSA_IDX_CARDINALITY = 9000 with LAST_INDEX = 8999
- ANTILOG_IDX_CARDINALITY = 10000 with LAST_INDEX = 9999
- Derived constants (LOG_TABLE_SIZE_BASE = 900, LOG_TABLE_SIZE_BYTES = 1800) are correctly calculated
24-164: LGTM: toBytes functions updated to new dimensions.All
toBytesoverloads correctly:
- Updated array dimensions to [10][90] and [10][100]
- Use derived constants (LOG_TABLE_SIZE_BYTES, LOG_TABLE_SIZE_BASE) for sizes
- Adjusted loop bounds (e.g., line 34:
mul(0x20, 89)for 90-row tables)
166-369: LGTM: logTableDec reduced to 90 rows.The function correctly:
- Returns
uint16[10][90](line 166)- Ends at row with values up to 9996 (line 367)
- Removes the placeholder tail row (previously at 10000/10001)
This aligns with LOG_MANTISSA_IDX_CARDINALITY = 9000.
371-464: LGTM: logTableDecSmall reduced to 90 rows.The function correctly:
- Returns
uint8[10][90](line 371)- Ends with appropriate small decimal values (line 462)
- Matches the dimensional reduction in logTableDec
481-690: LGTM: Antilog tables reduced to 100 rows.Both antilog functions correctly:
- Updated return types to [10][100] (lines 481, 586)
- End with appropriate values (9977 at line 582, 20 at line 687)
- Match ANTILOG_IDX_CARDINALITY = 10000
test/src/lib/LibDecimalFloat.pow.t.sol (2)
14-16: Confirm 0.09 diffLimit is the intended worst‑case bound.If this was adjusted from prior 8.6% for table resolution changes, consider a brief note pointing to the table granularity that drives this bound so future tweaks have context.
Based on learnings
39-44: Sanity cases for small integer exponents look correct.Exact-equality on 5^13 and 0.5^(30/60) is appropriate here.
.gas-snapshot (3)
1-33: Rebaseline hygiene confirmed – solc v0.8.25, optimizer=true (optimizer_runs=1 000 000), fuzz runs=5096 are pinned in foundry.toml with no overrides.
287-287: Possible regression: MulPacked μ increased 9687 → 9793
LibDecimalFloatMulTest:testMulPacked mean gas (μ) rose from 9687 to 9793 (σ from 10464 to 10566). Is this expected from normalization/rounding changes? If not, consider:
- Fast-path when either operand is zero/one
- Early normalization to reduce shifts
Likely an incorrect or invalid review comment.
29-31: Pow invariants: negative base and zero-base/negative-exponent must still revert
- Negative base should revert with PowNegativeBase.
- 0^negative should revert with ZeroNegativePower.
Confirm tests in test/src/lib/LibDecimalFloat.pow.t.sol (lines 86, 113) and test/src/lib/LibDecimalFloat.sqrt.t.sol (line 70) use vm.expectRevert(abi.encodeWithSelector(...)) with these selectors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/lib/implementation/LibDecimalFloatImplementation.sol(13 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#144
File: src/lib/LibDecimalFloat.sol:671-686
Timestamp: 2025-09-28T12:52:45.815Z
Learning: In LibDecimalFloat.sol, the pow function has always rejected negative bases and reverted with an error. Prior to recent changes, it used a different error type, but the fundamental policy of not allowing negative bases (except when b == 0) has been consistently maintained across the codebase's history.
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#144
File: test/src/lib/LibDecimalFloat.pow.t.sol:40-61
Timestamp: 2025-09-28T13:28:35.814Z
Learning: In test/src/lib/LibDecimalFloat.pow.t.sol, the diffLimit tolerance (currently 8.6%) represents a conservative worst-case bound to accommodate precision limitations of table lookup interpolation for fractional exponents in the pow function. The exponentiation-by-squaring approach used for integer parts achieves much tighter precision, but the tolerance must account for the less precise table-based calculations used for fractional exponents.
📚 Learning: 2025-09-30T15:09:39.424Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#144
File: src/lib/parse/LibParseDecimalFloat.sol:88-111
Timestamp: 2025-09-30T15:09:39.424Z
Learning: In src/lib/parse/LibParseDecimalFloat.sol, the fractional rescaling path (around lines 88-111) executes at most once per parseDecimalFloatInline call within an if-else branch, not in a loop, so there are no repeated 10**scale operations to optimize with a lookup table.
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.sol
📚 Learning: 2025-09-09T15:11:31.003Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#130
File: src/lib/implementation/LibDecimalFloatImplementation.sol:320-330
Timestamp: 2025-09-09T15:11:31.003Z
Learning: In LibDecimalFloatImplementation.sol div function, returning zero when underflowExponentBy > 76 is intentional "non-error underflow" behavior rather than reverting. This graceful handling is the main goal of the changes.
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.sol
📚 Learning: 2025-08-29T14:54:24.240Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: test/src/lib/LibDecimalFloat.decimal.t.sol:122-126
Timestamp: 2025-08-29T14:54:24.240Z
Learning: In test/src/lib/LibDecimalFloat.decimal.t.sol, the fromFixedDecimalLossy function converts uint256(type(int256).max) losslessly because the boundary check is `value > uint256(type(int256).max)`, not `>=`. Values exactly equal to type(int256).max still take the lossless conversion path.
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.sol
📚 Learning: 2025-08-29T10:38:26.353Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: src/lib/implementation/LibDecimalFloatImplementation.sol:593-601
Timestamp: 2025-08-29T10:38:26.353Z
Learning: In Solidity, int256(1) when passed through the maximize function in LibDecimalFloatImplementation.sol produces exactly (1e76, -76), not an approximation. This means the special case for signedCoefficient == 1e76 in log10 correctly handles powers of 10 like log10(1).
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.sol
📚 Learning: 2025-08-29T10:38:26.353Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: src/lib/implementation/LibDecimalFloatImplementation.sol:593-601
Timestamp: 2025-08-29T10:38:26.353Z
Learning: The maximize function in LibDecimalFloatImplementation.sol produces exact results for simple integer values like 1. maximize(1, 0) yields exactly (1e76, -76) with no precision loss, and the log10 special case for signedCoefficient == 1e76 correctly handles this.
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.sol
📚 Learning: 2025-08-29T14:58:50.500Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: src/lib/implementation/LibDecimalFloatImplementation.sol:896-899
Timestamp: 2025-08-29T14:58:50.500Z
Learning: In unchecked Solidity blocks, arithmetic operations can overflow/underflow and wrap around, so bounds checks that seem "impossible" for normal arithmetic may actually be necessary to catch overflow edge cases. For example, in withTargetExponent function, the check `exponentDiff < 0` is needed because `targetExponent - exponent` could underflow in unchecked arithmetic.
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.sol
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-static)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-static)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-test)
- GitHub Check: rainix (ubuntu-latest, test-wasm-build)
- GitHub Check: rainix (macos-latest, rainix-sol-legal)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-test)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-legal)
- GitHub Check: rainix (macos-latest, rainix-rs-test)
- GitHub Check: git-clean
🔇 Additional comments (5)
src/lib/implementation/LibDecimalFloatImplementation.sol (5)
11-19: LGTM: Imports properly centralized.The new error types and table constants imported from their respective modules support the bounds-checking and table lookup refactoring. This follows the previous feedback to centralize magic numbers.
83-89: Good practice: Documented type cast suppressions.The forge-lint comments document why specific type casts are safe after bounds checks or when values are known to fit in the target type. This helps maintainers understand the safety guarantees without cluttering the logic.
Also applies to: 99-127, 180-182
790-831: LGTM: Clean refactoring of log table lookup.The table lookup logic now uses the centralized
lookupLogTableValhelper and the named constantLOG_MANTISSA_LAST_INDEXinstead of magic numbers. The special handling for the terminal index (where y2Coefficient = 1e76) is appropriate for logarithmic lookup tables with non-uniform progression. Based on learnings.
882-890: LGTM: Terminal case handling prevents out-of-bounds access.The special case for
idx == ANTILOG_IDX_LAST_INDEXproperly avoids calling the lookup function with an out-of-range index, using default interpolation values instead. This aligns with the bounds-checking approach.
1162-1166: Good: Bounds check prevents invalid table access.The function now validates that
idxis within the valid antilog table range before performing any lookups, properly using theANTILOG_IDX_LAST_INDEXconstant and raisingLogTableIndexOutOfBoundsfor invalid indices.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
.gas-snapshot(2 hunks)src/error/ErrDecimalFloat.sol(1 hunks)src/lib/implementation/LibDecimalFloatImplementation.sol(13 hunks)
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#144
File: src/lib/LibDecimalFloat.sol:671-686
Timestamp: 2025-09-28T12:52:45.815Z
Learning: In LibDecimalFloat.sol, the pow function has always rejected negative bases and reverted with an error. Prior to recent changes, it used a different error type, but the fundamental policy of not allowing negative bases (except when b == 0) has been consistently maintained across the codebase's history.
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#144
File: test/src/lib/LibDecimalFloat.pow.t.sol:40-61
Timestamp: 2025-09-28T13:28:35.814Z
Learning: In test/src/lib/LibDecimalFloat.pow.t.sol, the diffLimit tolerance (currently 8.6%) represents a conservative worst-case bound to accommodate precision limitations of table lookup interpolation for fractional exponents in the pow function. The exponentiation-by-squaring approach used for integer parts achieves much tighter precision, but the tolerance must account for the less precise table-based calculations used for fractional exponents.
📚 Learning: 2025-09-28T12:52:45.815Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#144
File: src/lib/LibDecimalFloat.sol:671-686
Timestamp: 2025-09-28T12:52:45.815Z
Learning: In LibDecimalFloat.sol, the pow function has always rejected negative bases and reverted with an error. Prior to recent changes, it used a different error type, but the fundamental policy of not allowing negative bases (except when b == 0) has been consistently maintained across the codebase's history.
Applied to files:
src/error/ErrDecimalFloat.sol
📚 Learning: 2025-09-09T15:11:31.003Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#130
File: src/lib/implementation/LibDecimalFloatImplementation.sol:320-330
Timestamp: 2025-09-09T15:11:31.003Z
Learning: In LibDecimalFloatImplementation.sol div function, returning zero when underflowExponentBy > 76 is intentional "non-error underflow" behavior rather than reverting. This graceful handling is the main goal of the changes.
Applied to files:
src/error/ErrDecimalFloat.sol
📚 Learning: 2025-08-29T14:54:24.240Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: test/src/lib/LibDecimalFloat.decimal.t.sol:122-126
Timestamp: 2025-08-29T14:54:24.240Z
Learning: In test/src/lib/LibDecimalFloat.decimal.t.sol, the fromFixedDecimalLossy function converts uint256(type(int256).max) losslessly because the boundary check is `value > uint256(type(int256).max)`, not `>=`. Values exactly equal to type(int256).max still take the lossless conversion path.
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.sol
📚 Learning: 2025-08-29T10:38:26.353Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: src/lib/implementation/LibDecimalFloatImplementation.sol:593-601
Timestamp: 2025-08-29T10:38:26.353Z
Learning: In Solidity, int256(1) when passed through the maximize function in LibDecimalFloatImplementation.sol produces exactly (1e76, -76), not an approximation. This means the special case for signedCoefficient == 1e76 in log10 correctly handles powers of 10 like log10(1).
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.sol
📚 Learning: 2025-08-29T10:38:26.353Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: src/lib/implementation/LibDecimalFloatImplementation.sol:593-601
Timestamp: 2025-08-29T10:38:26.353Z
Learning: The maximize function in LibDecimalFloatImplementation.sol produces exact results for simple integer values like 1. maximize(1, 0) yields exactly (1e76, -76) with no precision loss, and the log10 special case for signedCoefficient == 1e76 correctly handles this.
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.sol
📚 Learning: 2025-09-30T15:09:39.424Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#144
File: src/lib/parse/LibParseDecimalFloat.sol:88-111
Timestamp: 2025-09-30T15:09:39.424Z
Learning: In src/lib/parse/LibParseDecimalFloat.sol, the fractional rescaling path (around lines 88-111) executes at most once per parseDecimalFloatInline call within an if-else branch, not in a loop, so there are no repeated 10**scale operations to optimize with a lookup table.
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.sol.gas-snapshot
📚 Learning: 2025-08-29T14:58:50.500Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#115
File: src/lib/implementation/LibDecimalFloatImplementation.sol:896-899
Timestamp: 2025-08-29T14:58:50.500Z
Learning: In unchecked Solidity blocks, arithmetic operations can overflow/underflow and wrap around, so bounds checks that seem "impossible" for normal arithmetic may actually be necessary to catch overflow edge cases. For example, in withTargetExponent function, the check `exponentDiff < 0` is needed because `targetExponent - exponent` could underflow in unchecked arithmetic.
Applied to files:
src/lib/implementation/LibDecimalFloatImplementation.sol.gas-snapshot
📚 Learning: 2025-09-28T13:28:35.814Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#144
File: test/src/lib/LibDecimalFloat.pow.t.sol:40-61
Timestamp: 2025-09-28T13:28:35.814Z
Learning: In test/src/lib/LibDecimalFloat.pow.t.sol, the diffLimit tolerance (currently 8.6%) represents a conservative worst-case bound to accommodate precision limitations of table lookup interpolation for fractional exponents in the pow function. The exponentiation-by-squaring approach used for integer parts achieves much tighter precision, but the tolerance must account for the less precise table-based calculations used for fractional exponents.
Applied to files:
.gas-snapshot
📚 Learning: 2025-08-21T18:03:40.347Z
Learnt from: thedavidmeister
PR: rainlanguage/rain.math.float#107
File: test/lib/LibDecimalFloatSlow.sol:37-45
Timestamp: 2025-08-21T18:03:40.347Z
Learning: In test/lib/LibDecimalFloatSlow.sol, the "slow" implementation is intentionally different from the production implementation to serve as an independent reference for fuzzing tests. The goal is to have two different approaches (expensive loops vs optimized jumps) that produce equivalent results, not identical implementations.
Applied to files:
.gas-snapshot
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: rainix (ubuntu-latest, test-wasm-build)
- GitHub Check: rainix (macos-latest, rainix-rs-test)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-test)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-legal)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-static)
- GitHub Check: rainix (macos-latest, rainix-sol-legal)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-static)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-test)
- GitHub Check: git-clean
🔇 Additional comments (14)
src/lib/implementation/LibDecimalFloatImplementation.sol (12)
14-18: Constants imported from LibLogTable look good.The imports of
LOG_TABLE_SIZE_BYTES,LOG_TABLE_SIZE_BASE,LOG_MANTISSA_LAST_INDEX, andANTILOG_IDX_LAST_INDEXfrom LibLogTable centralize table dimensions and eliminate magic numbers throughout the implementation, improving maintainability.
789-830: Table lookup and interpolation logic refactored well.The log10 table lookup has been refactored to use the centralized
lookupLogTableValfunction. The code correctly:
- Calculates the index based on the maximized coefficient
- Handles the terminal case with
LOG_MANTISSA_LAST_INDEX- Performs interpolation when needed
The use of named constants (
LOG_MANTISSA_LAST_INDEX) instead of magic numbers (8999) improves readability and maintainability. Based on learnings, the zero-slope interpolation at the terminal case is expected behavior for logarithmic lookup tables.Based on learnings
880-889: Antilog table lookup integrated cleanly.The pow10 function correctly uses
lookupAntilogTableY1Y2with proper handling of the terminal case usingANTILOG_IDX_LAST_INDEX. The fallback values (9997, 10000) for the terminal case maintain appropriate interpolation behavior.
81-89: Unsafe typecast suppressions justified for absUnsignedSignedCoefficient.The
forge-lint: disable-next-line(unsafe-typecast)annotations on lines 82 and 87 are appropriate:
- Line 82: Casting
-signedCoefficientwheresignedCoefficient < 0(and nottype(int256).min) is safe because the result fits inuint256- Line 87: Casting
signedCoefficientwheresignedCoefficient >= 0is safe because non-negative int256 values fit in uint256
99-127: Unsafe typecast suppressions justified for unabsUnsignedMulOrDivLossy.The typecast suppressions are correctly placed:
- Lines 109, 114, 121, 125: All casts follow careful bounds checking and division/truncation to ensure the values fit in int256 range
- The logic properly handles the edge case of
uint256(type(int256).max) + 1fortype(int256).min- Division by 10 before casting ensures no truncation for overflow cases
179-181: Typecast suppression justified - adjustExponent bounded [0, 76].The cast on line 181 is safe because
adjustExponentis computed through a series of increments starting from 0, with each increment corresponding to a division by a power of 10. The maximum value is 76 (from multiple divisions totaling 76 orders of magnitude), which fits safely in int256.
419-421: Typecast suppression justified - underflowExponentBy bounded [1, 76].The cast is safe because the preceding check ensures
underflowExponentBy > 0(line 412) andunderflowExponentBy <= 76(line 413 returns early if > 76). The range [1, 76] fits safely in both uint256 and int256.
622-634: Typecast suppressions justified - alignment exponent diff bounded [0, 76].Both casts are safe:
- Line 625:
exponentA - exponentBis cast to uint256 after ensuringexponentA >= exponentB(swapped on lines 605-613), so the difference is non-negative and fits in uint256- Line 634:
alignmentExponentDiffis checked to be<= ADD_MAX_EXPONENT_DIFF(76) on line 628, so10 ** alignmentExponentDiffand the cast to int256 are both safe
800-818: Typecast suppressions justified in log10 table index calculation.All four suppressions (lines 801, 806, 810, 818) are safe:
- Line 801:
scaleis either1e73or1e72(both fit in int256), division is deliberate truncation- Line 806:
x1Coefficientis positive (from maximized coefficient in range [1e75, 1e76+]), subtraction of 1000 keeps it in valid int256 range- Line 810: Multiplication by scale where scale is bounded
- Line 818: Addition of scale to x1Coefficient maintains bounds
881-888: Typecast suppression justified - idx is non-negative.The cast on line 888 is safe because
signedCoefficient < 0is handled at the top of the pow10 function (lines 869-873), so by this point in the code, the mantissaCoefficient and resulting idx are non-negative. The comment on line 882 confirms this reasoning.
1048-1050: Typecast suppressions justified - exponentDiff calculations.All three suppressions are safe:
- Line 1050 (compareRescale):
exponentDiffis checked to be in [0, 76] via assembly checks on line 1039- Line 1088 (withTargetExponent):
exponentDiffis checked to be in (0, 76] on line 1083- Line 1096 (withTargetExponent):
exponentDiffis checked to be in (0, 76] on line 1091Based on learnings, these bounds checks are necessary even in unchecked blocks to catch overflow edge cases.
Based on learnings
Also applies to: 1086-1088, 1094-1096
1124-1126: Typecast suppressions justified - exponent range checks.Both suppressions are safe:
- Lines 1124-1126 (characteristicMantissa):
exponentis checked to be in [-76, -1] range (lines 1120-1122), so-exponentis in [1, 76] and the power of 10 fits- Line 1150 (mantissa4):
exponentis in [-3, -1] range (line 1148), so4 + exponentis in [1, 3] and the power of 10 fitsAlso applies to: 1149-1150
src/error/ErrDecimalFloat.sol (1)
44-46: New PowNegativeBase error defined correctly.The error definition properly captures the necessary context (signed coefficient and exponent) for debugging negative base exponentiation attempts. Based on learnings, the pow function has consistently rejected negative bases throughout the codebase's history, and this error formalizes that rejection with appropriate parameters for diagnostics.
Based on learnings
.gas-snapshot (1)
1-345: Gas snapshot rebased for lookup table optimization.The widespread gas baseline updates reflect the implementation changes, particularly the centralized lookup table access via
lookupLogTableValand the reduced table dimensions. Notable improvements include:
- Log10 tests (lines 184-188, 245): Significant gas reductions due to optimized table lookups
- Pow10 tests (lines 211-215, 292): Similar improvements from antilog table optimization
- Pow tests (lines 293-300): Updated baselines reflecting new error handling paths
The snapshot changes are consistent with the algorithmic improvements in the main implementation. Based on past review comments, ensure that:
- Property tests exist for log10/pow10 monotonicity and round-trip accuracy within the diffLimit tolerance (currently ~8.6%)
- Rounding rule boundary tests cover .5 ties across sign and digit boundaries
testParseFormatterRoundTripBug0remains protected with a comment explaining its purposeBased on learnings
|
@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:
|
|
Tip For best results, initiate chat on the files or code changes. SIZE=L |
Motivation
Solution
Checks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests
Chores