Uh oh!
There was an error while loading. Please reload this page.
sub - #97
Conversation
WalkthroughAdds an internal test helper and a new external pure test (testSubOneFromMax) for LibDecimalFloatImplementation subtraction edge-case, and updates the gas snapshot with a new entry and minor metric shifts. No production code or public API changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes ✨ Finishing Touches🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File ( |
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 (2)
.gas-snapshot(1 hunks)test/src/lib/implementation/LibDecimalFloatImplementation.sub.t.sol(1 hunks)
⏰ 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). (7)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-static)
- GitHub Check: rainix (ubuntu-latest, test-wasm-build)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-static)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-test)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-test)
- GitHub Check: rainix (macos-latest, rainix-rs-test)
- GitHub Check: git-clean
🔇 Additional comments (2)
.gas-snapshot (1)
198-200: Gas snapshot updates look consistent with new test coverageNew entry for testSubOneFromMax and minor μ shifts for adjacent tests are expected. No action needed.
test/src/lib/implementation/LibDecimalFloatImplementation.sub.t.sol (1)
64-66: Solid edge-case coverage: subtracting 1 from max exponent/value pathExpectation aligns with normalization (shift exponent by 8, scale coefficient by 1e8). LGTM.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…t.sol Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
test/src/lib/implementation/LibDecimalFloatImplementation.sub.t.sol (2)
60-61: Assertion messages added — thanksThis addresses the earlier nit about adding messages to assertEq for quicker diagnosis.
64-66: Clarify the “8-digit” normalization to avoid magic numbersAdd a short comment explaining why normalization is 8 digits here. Optionally, factor 1e8 and 8 into local constants to keep the test resilient if the normalization granularity changes.
- function testSubOneFromMax() external pure {- checkSub(type(int224).max, type(int32).max, 1, 0, int256(type(int224).max) * 1e8, type(int32).max - 8);- }+ function testSubOneFromMax() external pure {+ // Sub normalization shifts exponent by 8 and scales coefficient by 1e8 for this boundary case.+ // This documents the expected normalization granularity to avoid confusion over the magic number "8".+ checkSub(type(int224).max, type(int32).max, 1, 0, int256(type(int224).max) * 1e8, type(int32).max - 8);+ }Optionally define local constants at the top of the test contract and use them here:
// Optional: make normalization explicit in testsint256constant NORMALIZATION_DIGITS =8; int256constant NORMALIZATION_SCALE =1e8;Then:
- checkSub(type(int224).max, type(int32).max, 1, 0, int256(type(int224).max) * 1e8, type(int32).max - 8);+ checkSub(+ type(int224).max,+ type(int32).max,+ 1,+ 0,+ int256(type(int224).max) * NORMALIZATION_SCALE,+ type(int32).max - NORMALIZATION_DIGITS+ );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
test/src/lib/implementation/LibDecimalFloatImplementation.sub.t.sol(1 hunks)
⏰ 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). (7)
- GitHub Check: rainix (ubuntu-latest, test-wasm-build)
- 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 (macos-latest, rainix-rs-test)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-test)
- GitHub Check: git-clean
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Motivation
Solution
Checks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit
Tests
Chores