fixed apy bug #779
fixed apy bug #779
Conversation
WalkthroughThe pull request introduces changes to the Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
packages/web/src/hooks/vaults/useVaultApy.ts (1)
90-92: Consider making edge case handling more explicitWhile the division by zero protection is correct, the logic could be more explicit about handling edge cases.
Consider this more explicit approach:
- const apy = vaultTokenStaked > 0 ? (annualRewardValue / vaultTokenStaked) * 100 : 0; + if (vaultTokenStaked <= 0) return 0; + if (annualRewardValue <= 0) return 0; + + const apy = (annualRewardValue / vaultTokenStaked) * 100;This makes the edge cases more obvious and maintainable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
packages/web/src/hooks/vaults/useVaultApy.ts(2 hunks)packages/web/src/pages/Honeypots/VaultDetailsPage/Sections/VaultDepositsSection/VaultAssetsSection/components/VaultAsset.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/web/src/pages/Honeypots/VaultDetailsPage/Sections/VaultDepositsSection/VaultAssetsSection/components/VaultAsset.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
packages/web/src/hooks/vaults/useVaultApy.ts
[error] 59-59: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.
(lint/suspicious/noGlobalIsNan)
🔇 Additional comments (1)
packages/web/src/hooks/vaults/useVaultApy.ts (1)
90-92: Add validation for unrealistic APY values
While the code handles basic edge cases, it might be worth adding validation for unrealistically high APY values that could indicate calculation errors.
Let's verify if there have been any instances of extreme APY values in the codebase:
Consider adding a maximum threshold check:
const apy = vaultTokenStaked > 0 ? (annualRewardValue / vaultTokenStaked) * 100 : 0;
+ // Add a maximum threshold (e.g., 1000000%) to catch potential calculation errors
+ const MAX_REASONABLE_APY = 1000000;
+ if (apy > MAX_REASONABLE_APY) {
+ console.warn(`Unusually high APY detected: ${apy}%. This might indicate a calculation error.`);
+ }| @@ -56,7 +56,7 @@ export const useVaultApy = (vault?: IVault): IVaultApy[] => { | |||
| vault.amountsInfo?.depositedAmount ? +vault.amountsInfo.depositedAmount.tokens : 0 | |||
| ); | |||
|
|
|||
| apys.push({ rewardController: controller, apy }); | |||
| if (!isNaN(apy)) apys.push({ rewardController: controller, apy }); | |||
There was a problem hiding this comment.
Replace unsafe isNaN with Number.isNaN
The global isNaN() function attempts type coercion which can lead to unexpected results. Use Number.isNaN() instead for strict NaN checking.
Apply this change:
- if (!isNaN(apy)) apys.push({ rewardController: controller, apy });
+ if (!Number.isNaN(apy)) apys.push({ rewardController: controller, apy });📝 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.
| if (!isNaN(apy)) apys.push({ rewardController: controller, apy }); | |
| if (!Number.isNaN(apy)) apys.push({ rewardController: controller, apy }); |
🧰 Tools
🪛 Biome (1.9.4)
[error] 59-59: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.
(lint/suspicious/noGlobalIsNan)
Deploying dapp with
|
| Latest commit: |
7ac6e6a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://57a56a06.dapp-a9y.pages.dev |
| Branch Preview URL: | https://fix-apy.dapp-a9y.pages.dev |
Summary by CodeRabbit
New Features
Bug Fixes
Chores
vaultApyin the VaultAsset component for debugging purposes.