pack 0 standardized#136
Merged
thedavidmeister merged 4 commits intomainfrom Sep 16, 2025
Merged
Conversation
Contributor
WalkthroughAdds canonical-zero short-circuit and input-capture to LibDecimalFloat.packLossy, refines exponent overflow/underflow handling, and updates tests to expect exponent 0 when the resulting coefficient is zero; also adds tests for packing zero and exponent-overflow behavior. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller as Caller
participant Lib as LibDecimalFloat.packLossy
Caller->>Lib: packLossy(signedCoefficient, exponent)
alt signedCoefficient == 0
rect rgb(230,245,255)
Note over Lib: Capture inputs\nReturn canonical zero (lossless)
Lib-->>Caller: (FLOAT_ZERO, true)
end
else
rect rgb(245,245,245)
Lib->>Lib: normalize coefficient & adjust exponent
Lib->>Lib: check exponent bounds (int32)
alt exponent overflow/underflow
rect rgb(255,235,235)
Note over Lib: Use captured initial inputs\nRevert or return lossy zero as appropriate
Lib-->>Caller: revert(ExponentOverflow) or (FLOAT_ZERO,false)
end
else
Lib-->>Caller: (packedFloat, losslessFlag)
end
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (4)📓 Common learnings📚 Learning: 2025-09-09T15:11:31.003ZApplied to files:
📚 Learning: 2025-06-18T09:10:41.740ZApplied to files:
📚 Learning: 2025-06-11T09:03:20.771ZApplied to files:
⏰ 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)
🔇 Additional comments (1)
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 (7)
src/lib/LibDecimalFloat.sol(2 hunks)test/src/lib/LibDecimalFloat.abs.t.sol(1 hunks)test/src/lib/LibDecimalFloat.decimal.t.sol(1 hunks)test/src/lib/LibDecimalFloat.decimalLossless.t.sol(1 hunks)test/src/lib/LibDecimalFloat.frac.t.sol(1 hunks)test/src/lib/LibDecimalFloat.minus.t.sol(1 hunks)test/src/lib/LibDecimalFloat.pack.t.sol(1 hunks)
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
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.
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.
📚 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.minus.t.soltest/src/lib/LibDecimalFloat.decimalLossless.t.soltest/src/lib/LibDecimalFloat.decimal.t.solsrc/lib/LibDecimalFloat.soltest/src/lib/LibDecimalFloat.abs.t.soltest/src/lib/LibDecimalFloat.pack.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.minus.t.soltest/src/lib/LibDecimalFloat.decimalLossless.t.soltest/src/lib/LibDecimalFloat.frac.t.soltest/src/lib/LibDecimalFloat.decimal.t.solsrc/lib/LibDecimalFloat.soltest/src/lib/LibDecimalFloat.abs.t.soltest/src/lib/LibDecimalFloat.pack.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.minus.t.soltest/src/lib/LibDecimalFloat.decimalLossless.t.soltest/src/lib/LibDecimalFloat.frac.t.soltest/src/lib/LibDecimalFloat.decimal.t.soltest/src/lib/LibDecimalFloat.abs.t.soltest/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.soltest/src/lib/LibDecimalFloat.decimal.t.solsrc/lib/LibDecimalFloat.soltest/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/LibDecimalFloat.decimalLossless.t.soltest/src/lib/LibDecimalFloat.decimal.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.decimalLossless.t.soltest/src/lib/LibDecimalFloat.decimal.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.decimalLossless.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.decimalLossless.t.solsrc/lib/LibDecimalFloat.soltest/src/lib/LibDecimalFloat.abs.t.soltest/src/lib/LibDecimalFloat.pack.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.decimalLossless.t.soltest/src/lib/LibDecimalFloat.decimal.t.solsrc/lib/LibDecimalFloat.soltest/src/lib/LibDecimalFloat.pack.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.decimalLossless.t.soltest/src/lib/LibDecimalFloat.abs.t.soltest/src/lib/LibDecimalFloat.pack.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.soltest/src/lib/LibDecimalFloat.pack.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). (8)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-static)
- 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-rs-test)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-test)
- GitHub Check: rainix (ubuntu-latest, test-wasm-build)
- GitHub Check: git-clean
🔇 Additional comments (9)
src/lib/LibDecimalFloat.sol (2)
317-321: Canonical-zero handling LGTM.Zero coefficient packing to
FLOAT_ZEROirrespective of input exponent is consistent and simplifies downstream behavior.
323-330: Clarify underflow vs overflow classification using original exponent.Negative out‑of‑range exponents should only underflow-to-zero when the original input exponent was already negative; otherwise this path could misclassify. Use
initialExponentto decide.Apply this diff:
- if (int32(exponent) != exponent) { - // If the exponent is negative then this is a number too small - // to pack. We return zero but it is not a lossless conversion. - if (exponent < 0) { - return (FLOAT_ZERO, false); - } - revert ExponentOverflow(initialSignedCoefficient, initialExponent); - } + if (int32(exponent) != exponent) { + // Underflow to zero only if the original exponent was negative. + if (initialExponent < 0) { + return (FLOAT_ZERO, false); + } + revert ExponentOverflow(initialSignedCoefficient, initialExponent); + }⛔ Skipped due to learnings
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: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.test/src/lib/LibDecimalFloat.decimal.t.sol (1)
34-34: Expectation updated to canonical-zero exponent is correct.
exponentFloatshould be 0 whenever the packed coefficient is 0.test/src/lib/LibDecimalFloat.abs.t.sol (1)
19-19: Abs test aligns with canonical-zero.Clamping result exponent to 0 when coefficient is 0 is correct.
test/src/lib/LibDecimalFloat.minus.t.sol (1)
29-29: Minus test matches new packing semantics.Zero coefficient implies exponent 0 on unpack; assertion is correct.
test/src/lib/LibDecimalFloat.decimalLossless.t.sol (1)
48-48: Lossless packed round-trip assertion updated appropriately.Conditionally expecting exponent 0 for zero coefficient is correct.
test/src/lib/LibDecimalFloat.frac.t.sol (1)
26-26: Frac tests correctly adopt canonical-zero behavior and explicit modulo.
- Nonnegative exponent → zero fractional part with exponent 0.
- For very small numbers, exponent 0 when fractional part is 0.
- Using
y = x % 10**(-exponent)tightens expectations and readability.Also applies to: 33-33, 39-41
test/src/lib/LibDecimalFloat.pack.t.sol (2)
25-26: Round-trip exponent assertion updated correctly.Ensures packed zero normalizes exponent to 0.
28-33: Good: explicit zero-pack test.Validates both losslessness and canonical
FLOAT_ZERO.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
test/src/lib/LibDecimalFloat.pack.t.sol (1)
44-49: Resolved: underflow-to-zero path now covered.This addresses the earlier request to add coverage for negative exponent underflow that yields lossy zero.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
test/src/lib/LibDecimalFloat.pack.t.sol(1 hunks)
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
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.
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.
📚 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.pack.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.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.pack.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.pack.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.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.pack.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.pack.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.pack.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
⏰ 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). (8)
- 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 (ubuntu-latest, rainix-rs-test)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-static)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-legal)
- GitHub Check: rainix (macos-latest, rainix-rs-test)
- GitHub Check: git-clean
🔇 Additional comments (1)
test/src/lib/LibDecimalFloat.pack.t.sol (1)
25-26: LGTM: canonical-zero round‑trip assertion is correct.Asserting
exponentOut == 0whensignedCoefficient == 0aligns with the canonical-zero rule and removes ambiguity in expectations.
Comment on lines
+44
to
+49
| function testPackNegativeExponentLossyZero(int256 signedCoefficient, int256 exponent) external view { | ||
| exponent = bound(exponent, type(int256).min, int256(type(int32).min) - 77); | ||
| vm.assume(signedCoefficient != 0); | ||
| (Float float, bool lossless) = this.packLossyExternal(signedCoefficient, exponent); | ||
| assertFalse(lossless, "lossless"); | ||
| assertEq(Float.unwrap(float), Float.unwrap(LibDecimalFloat.FLOAT_ZERO), "float"); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick
Tighten the underflow test: make it pure and target the threshold precisely.
- Mark the test
pure(no state read/write). - Bound
signedCoefficientto ≥1 to avoid discards. - Pin
exponentto a value ≤ int32.min − 77 to deterministically exercise the lossy‑zero path.
Apply this diff:
- function testPackNegativeExponentLossyZero(int256 signedCoefficient, int256 exponent) external view {
- exponent = bound(exponent, type(int256).min, int256(type(int32).min) - 77);
- vm.assume(signedCoefficient != 0);
+ function testPackNegativeExponentLossyZero(int256 signedCoefficient, int256 exponent) external pure {
+ signedCoefficient = bound(signedCoefficient, 1, type(int256).max);
+ exponent = bound(exponent, int256(type(int32).min) - 77, int256(type(int32).min) - 77);
(Float float, bool lossless) = this.packLossyExternal(signedCoefficient, exponent);
assertFalse(lossless, "lossless");
assertEq(Float.unwrap(float), Float.unwrap(LibDecimalFloat.FLOAT_ZERO), "float");
}Consider adding a sibling test for the “within 1..76” underflow window to assert non‑zero, lossy pack clamped at int32.min.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function testPackNegativeExponentLossyZero(int256 signedCoefficient, int256 exponent) external view { | |
| exponent = bound(exponent, type(int256).min, int256(type(int32).min) - 77); | |
| vm.assume(signedCoefficient != 0); | |
| (Float float, bool lossless) = this.packLossyExternal(signedCoefficient, exponent); | |
| assertFalse(lossless, "lossless"); | |
| assertEq(Float.unwrap(float), Float.unwrap(LibDecimalFloat.FLOAT_ZERO), "float"); | |
| function testPackNegativeExponentLossyZero(int256 signedCoefficient, int256 exponent) external pure { | |
| signedCoefficient = bound(signedCoefficient, 1, type(int256).max); | |
| exponent = bound(exponent, int256(type(int32).min) - 77, int256(type(int32).min) - 77); | |
| (Float float, bool lossless) = this.packLossyExternal(signedCoefficient, exponent); | |
| assertFalse(lossless, "lossless"); | |
| assertEq(Float.unwrap(float), Float.unwrap(LibDecimalFloat.FLOAT_ZERO), "float"); | |
| } |
|
@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:
|
Contributor
|
Tip For best results, initiate chat on the files or code changes. |
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
when packing 0 coefficient always returns standard 0 float instead of sometimes erroring due to exponent
when packing if exponent does not fit in int32 and is negative, return lossy 0 instead of erroring
Solution
Checks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit
Bug Fixes
Performance
Tests