feat: Add APR safeguards to reward fetchers to prevent reporting of unrealistically high rates.#1326
feat: Add APR safeguards to reward fetchers to prevent reporting of unrealistically high rates.#1326halaprix wants to merge 3 commits into
Conversation
…reporting of unrealistically high rates.
… reward fetchers.
WalkthroughAdds MAX_APR_THRESHOLD clamping across four reward fetchers. Each returns "0" if APR exceeds 100%, otherwise returns APR * 100 as string. No control flow changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
background-jobs/update-summer-earn-rewards-apr/src/reward-fetchers/MorphoRewardFetcher.ts (1)
163-178:⚠️ Potential issue | 🟠 MajorClamp each market APR before weighting.
Line 178 clamps too late. A bad market APR can still leak into
totalWeightedApyif weighting pulls it below 100%, so the safeguard is bypassed.Fix
const weightedTokenRewardsApy = vaultData.state.allocation.reduce((acc, allocation) => { const marketRewards = allocation.market.state.rewards .filter((reward) => reward.asset.address === tokenAddress) .map( - (reward) => - (reward.supplyApr ?? 0) * ((allocation.supplyAssetsUsd ?? 0) / totalAssetsAllocated), + (reward) => { + const apr = reward.supplyApr ?? 0 + const safeApr = apr > this.MAX_APR_THRESHOLD ? 0 : apr + return safeApr * ((allocation.supplyAssetsUsd ?? 0) / totalAssetsAllocated) + }, ) return acc.concat(marketRewards) }, [] as number[])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@background-jobs/update-summer-earn-rewards-apr/src/reward-fetchers/MorphoRewardFetcher.ts` around lines 163 - 178, The code currently clamps after summing weighted APRs, allowing an outlier market APR to be diluted below the global cap; instead clamp each market APR before weighting: inside the allocation -> market.state.rewards mapping in MorphoRewardFetcher (the block producing weightedTokenRewardsApy), replace (reward.supplyApr ?? 0) with a clamped value like Math.min(reward.supplyApr ?? 0, this.MAX_APR_THRESHOLD) (then multiply that clamped APR by the allocation fraction) so every market APR is bounded prior to summation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@background-jobs/update-summer-earn-rewards-apr/src/reward-fetchers/FluidRewardFetcher.ts`:
- Around line 77-86: The code misinterprets the contract value: rateNum/100
yields percentage points (e.g., 263 -> 2.63%), so the current logic multiplies
small APRs by 100 and incorrectly clamps values; fix by normalizing to a decimal
APR before clamping — compute rateDecimal = rateNum / 10000 (or equivalently
divide ratePercent by 100), then compare rateDecimal to this.MAX_APR_THRESHOLD
and set rate = rateDecimal > this.MAX_APR_THRESHOLD ? 0 : rateDecimal; update
the value pushed into mapped (where rateNum, ratePercent, this.MAX_APR_THRESHOLD
and the mapped.push block in FluidRewardFetcher.ts are located).
---
Outside diff comments:
In
`@background-jobs/update-summer-earn-rewards-apr/src/reward-fetchers/MorphoRewardFetcher.ts`:
- Around line 163-178: The code currently clamps after summing weighted APRs,
allowing an outlier market APR to be diluted below the global cap; instead clamp
each market APR before weighting: inside the allocation -> market.state.rewards
mapping in MorphoRewardFetcher (the block producing weightedTokenRewardsApy),
replace (reward.supplyApr ?? 0) with a clamped value like
Math.min(reward.supplyApr ?? 0, this.MAX_APR_THRESHOLD) (then multiply that
clamped APR by the allocation fraction) so every market APR is bounded prior to
summation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4061418d-5d5e-4747-aa1f-664290809500
📒 Files selected for processing (4)
background-jobs/update-summer-earn-rewards-apr/src/reward-fetchers/CompoundRewardFetcher.tsbackground-jobs/update-summer-earn-rewards-apr/src/reward-fetchers/FluidRewardFetcher.tsbackground-jobs/update-summer-earn-rewards-apr/src/reward-fetchers/MorphoRewardFetcher.tsbackground-jobs/update-summer-earn-rewards-apr/src/reward-fetchers/MorphoV2RewardFetcher.ts
…sion factor from 100 to 10000.
Pull Request: Add APR safeguards to reward fetchers to prevent reporting of unrealistically high rates.
Description
This PR addresses the issue where the system would occasionally report extremely high reward rates (e.g., 100,000% APY) due to anomalous data from external APIs. We've introduced a consistent safeguard across all Morpho, Compound, and Fluid reward fetchers.
Changes
MAX_APR_THRESHOLDacross MorphoRewardFetcher, MorphoV2RewardFetcher, CompoundRewardFetcher, and FluidRewardFetcher.Benefits
Testing
rateof "0".Next steps
Additional Notes
1while keeping the comment1000%. I would recommend keeping these consistent (value10for 1000%, as1.0in the API usually represents 100%).Please review and provide any feedback or suggestions for improvement.
Summary by CodeRabbit
Bug Fixes