overflow protections#139
Merged
thedavidmeister merged 2 commits intomainfrom Sep 20, 2025
Merged
Conversation
Contributor
WalkthroughRefactors internal control flow in LibDecimalFloatImplementation.sol for add, log10, and pow10, introduces an exponent overflow guard in add, and adds two test helpers. Also updates gas/metric snapshot numbers only. No public signatures or data structures changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Caller
participant L as LibDecimalFloatImplementation.add
Note over L: add overflow handling (new guard)
C->>L: add(a, b)
alt coefficient addition overflows
alt exponentA == int256.max
L-->>C: revert ExponentOverflow(signedCoefficientA, exponentA)
else exponentA < int256.max
L->>L: exponentA++ (adjust exponent)
L->>L: combine adjusted coefficients
L-->>C: return normalized result
end
else no overflow
L-->>C: return result
end
sequenceDiagram
autonumber
participant C as Caller
participant Lg as LibDecimalFloatImplementation.log10
participant Lt as LookupTable (inline asm)
Note over Lg: log10 positive path with inlined lookup
C->>Lg: log10(x)
alt x == 0 or x < 0
Lg-->>C: zero/negative handling (inv/log10 variants)
else positive
Lg->>Lg: normalize/maximize
alt exact power of 10
Lg-->>C: return exact exponent
else interpolate
Lg->>Lt: table lookup (inline asm)
Lt-->>Lg: table values/indices
Lg->>Lg: interpolate and finalize
Lg-->>C: result
end
end
sequenceDiagram
autonumber
participant C as Caller
participant P as LibDecimalFloatImplementation.pow10
participant Lt as LookupTable (inline)
Note over P: pow10 uses table or interpolation
C->>P: pow10(e)
alt e within precomputed table
P->>Lt: lookup
Lt-->>P: value
P-->>C: result
else interpolate
P->>Lt: bounds lookup
Lt-->>P: neighbors
P->>P: interpolate/compose value
P-->>C: result
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
test/src/lib/implementation/LibDecimalFloatImplementation.add.t.sol(3 hunks)
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
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.
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.
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.
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.
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).
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.
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.
📚 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.add.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.add.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/implementation/LibDecimalFloatImplementation.add.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/implementation/LibDecimalFloatImplementation.add.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.add.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/implementation/LibDecimalFloatImplementation.add.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/implementation/LibDecimalFloatImplementation.add.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/implementation/LibDecimalFloatImplementation.add.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-static)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-static)
- 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-sol-legal)
- GitHub Check: rainix (macos-latest, rainix-sol-legal)
- GitHub Check: rainix (macos-latest, rainix-rs-test)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-test)
🔇 Additional comments (1)
test/src/lib/implementation/LibDecimalFloatImplementation.add.t.sol (1)
5-11: LGTM — ExponentOverflow import and selector check verified.Defined as error ExponentOverflow(int256 signedCoefficient, int256 exponent) in src/error/ErrDecimalFloat.sol, so the test's selector/arity (two int256 args) is correct.
|
@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:
|
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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