Conversation
WalkthroughIntroduces a new Solidity library package ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/price-utils/contracts/Helpers.sol`:
- Around line 17-22: The exponentiation 10 ** exp can overflow for exp > 77; add
a guard that requires exp <= 77 (or a safe max) before computing 10 ** exp and
revert with a clear error message; apply this check in both branches where exp
is computed (the branch comparing amountDecimals and targetDecimals, referencing
the local variable exp and the surrounding function in Helpers.sol) so the
function validates the bound before performing the multiplication/division.
In `@packages/price-utils/foundry.toml`:
- Line 24: The TOML contains a deprecated goerli network entry (goerli = { key =
"${API_KEY_ETHERSCAN}" }); remove or replace it with the current Holesky network
entry (e.g., holesky = { key = "${API_KEY_ETHERSCAN}" }) and update any other
identical occurrences mentioned (the second occurrence at the other location) so
references and env usage remain consistent; ensure you only change the network
name(s) and preserve the API key variable name unless you also plan to rename
the env var.
🧹 Nitpick comments (3)
packages/price-utils/test/PriceUtils.t.sol (1)
15-245: Consider adding fuzz tests.Good coverage of fixed cases. Add fuzz tests to catch edge cases (overflow, zero values, extreme decimals).
function testFuzz_quote(uint256 baseAmount, uint256 quoteAmount, uint256 input) public pure { vm.assume(baseAmount > 0 && quoteAmount > 0); vm.assume(input < type(uint256).max / baseAmount); // prevent overflow Price memory price = toPrice(baseAmount, quoteAmount); price.quote(input); // should not revert }packages/price-utils/contracts/Utils.sol (2)
97-102:div()allocates intermediatePriceon every call.As noted in PR for future optimization -
invert()creates new memory struct. Consider lazy inversion flag later.
53-61: ZeroquoteAmountcauses opaque revert.PRB Math will revert on division by zero but error message won't indicate price context. Consider explicit check if better UX needed.
| if (amountDecimals > targetDecimals) { | ||
| uint8 exp = amountDecimals - targetDecimals; | ||
| return amount / (10 ** exp); | ||
| } else if (amountDecimals < targetDecimals) { | ||
| uint8 exp = targetDecimals - amountDecimals; | ||
| return amount * (10 ** exp); |
There was a problem hiding this comment.
Guard exponent overflow in scaling.
10 ** exp overflows for exp > 77, causing revert. Add a bound check with a clear error.
Proposed fix
+error DecimalsOverflow(uint8 amountDecimals, uint8 targetDecimals);
+
function _normalizeAmount(
uint256 amount,
uint8 amountDecimals,
uint8 targetDecimals
) pure returns (uint256) {
if (amountDecimals > targetDecimals) {
uint8 exp = amountDecimals - targetDecimals;
+ if (exp > 77) revert DecimalsOverflow(amountDecimals, targetDecimals);
return amount / (10 ** exp);
} else if (amountDecimals < targetDecimals) {
uint8 exp = targetDecimals - amountDecimals;
+ if (exp > 77) revert DecimalsOverflow(amountDecimals, targetDecimals);
return amount * (10 ** exp);
} else {
return amount;
}
}🤖 Prompt for AI Agents
In `@packages/price-utils/contracts/Helpers.sol` around lines 17 - 22, The
exponentiation 10 ** exp can overflow for exp > 77; add a guard that requires
exp <= 77 (or a safe max) before computing 10 ** exp and revert with a clear
error message; apply this check in both branches where exp is computed (the
branch comparing amountDecimals and targetDecimals, referencing the local
variable exp and the surrounding function in Helpers.sol) so the function
validates the bound before performing the multiplication/division.
| avalanche = { key = "${API_KEY_SNOWTRACE}" } | ||
| base = { key = "${API_KEY_BASESCAN}" } | ||
| gnosis_chain = { key = "${API_KEY_GNOSISSCAN}" } | ||
| goerli = { key = "${API_KEY_ETHERSCAN}" } |
There was a problem hiding this comment.
Goerli testnet deprecated.
Goerli reached EOL in March 2023. Consider removing these references or replacing with Holesky.
Also applies to: 46-46
🤖 Prompt for AI Agents
In `@packages/price-utils/foundry.toml` at line 24, The TOML contains a deprecated
goerli network entry (goerli = { key = "${API_KEY_ETHERSCAN}" }); remove or
replace it with the current Holesky network entry (e.g., holesky = { key =
"${API_KEY_ETHERSCAN}" }) and update any other identical occurrences mentioned
(the second occurrence at the other location) so references and env usage remain
consistent; ensure you only change the network name(s) and preserve the API key
variable name unless you also plan to rename the env var.
1 participant
Description
Adapting PriceUtils.sol from monorepo to the new version of PRB. Also bumping up the interface to make it more efficient and easy to use.
Changes
PricetypePriceto invert it and quote an amountuint256to be able to multiply it or divide it by aPricetoPriceto createPriceinstancesBenefits
Testing
Added unit tests with several test cases to demonstrate that the library works properly. Particularle I've added tests cases to check for the decimal conversion
Next steps
invert()could be optimized further by including a flag in thePricestructure to indicate if the value is inverted or not, and use the flag to know which value is base and which one is quotePlease review and provide any feedback or suggestions for improvement.
Summary by CodeRabbit
Release Notes
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.