Merged
Conversation
Contributor
📝 WalkthroughWalkthroughA new Acre protocol adapter is introduced that calculates APY by querying Chainlink aggregators for historical rate data, fetching vault TVL and token prices, and computing annualized returns based on rate changes across approximately 30-day periods. Changes
Sequence DiagramsequenceDiagram
participant Adapter as Acre Adapter
participant Aggregator as Chainlink Aggregator
participant Vault as Acre BTC Vault
participant PriceOracle as Price Oracle/SDK
participant Output as Result
Adapter->>Aggregator: Query latest round data
Aggregator-->>Adapter: roundId, rate, updatedAt
Adapter->>Vault: Fetch total assets (TVL)
Vault-->>Adapter: TVL amount
Adapter->>PriceOracle: Fetch TBTC price
PriceOracle-->>Adapter: TBTC/USD price
Adapter->>Aggregator: Iterate backward through rounds
loop Find ~30 day old round
Aggregator-->>Adapter: Historical round data
end
Adapter->>Adapter: Compute TVL in USD<br/>(TVL × TBTC price)
Adapter->>Adapter: Calculate APY<br/>(latest rate / previous rate)<br/>annualized
Adapter->>Output: Return pool object<br/>(address, chain, APY, TVL, etc.)
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)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
|
The acre adapter exports pools: Test Suites: 1 passed, 1 total |
![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: 3
🧹 Nitpick comments (1)
src/adaptors/acre/index.js (1)
50-51: Comment does not match code logic.The comment says "Skip rounds with identical timestamps" but the code checks
rate > 0. Consider updating the comment to accurately describe the intent:- // Skip rounds with identical timestamps (no real update) + // Only use rounds with valid (positive) rate if (rate > 0) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/adaptors/acre/index.js` around lines 50 - 51, The comment above the conditional is misleading: it says "Skip rounds with identical timestamps" but the code checks the variable rate in the conditional if (rate > 0); update the comment to accurately describe the runtime check (e.g., "Process only positive rates" or "Skip non-positive rates") or change the condition to match the original intent; locate the conditional using the symbol rate and the if (rate > 0) line and make the comment and condition consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/adaptors/acre/index.js`:
- Around line 58-59: The code computes tvlUsd using tbtcPrice from
pricesByAddress[TBTC.toLowerCase()] but doesn't handle undefined; update the
tvlUsd calculation to guard against missing prices by checking tbtcPrice (from
pricesByAddress and TBTC) and using a safe fallback or throwing a clear
error—e.g., if tbtcPrice is falsy default to 0 (or handle upstream), then
compute tvlUsd using tvlRes.output / 1e18 multiplied by that safePrice; ensure
any logging or error path references tbtcPrice, pricesByAddress, TBTC, tvlUsd,
and tvlRes.output so the issue is detectable.
- Around line 38-56: The loop that computes historical round data can leave
prevRate and prevTs undefined (e.g., when latestId <= 1 or no rounds with rate >
0), causing NaN in the apyBase calculation; to fix it, initialize prevRate and
prevTs to safe fallbacks before the loop (for example set them to the current
round's rate and timestamp or to the same values as the latest round used to
compute targetTs) and/or guard the apyBase computation in the function that
computes APY (ensure daysBetween is non-zero and if prevRate or prevTs are
missing set apyBase = 0). Update the variables referenced in this diff
(prevRate, prevTs, latestId, targetTs, and the apyBase calculation) so the code
never divides by undefined and returns 0 APY when no valid historical round
exists.
- Around line 62-65: The calculation of apyBase can divide by prevRate which may
be zero; update the logic that computes apyBase (the expression using
latestRate, prevRate and daysBetween) to guard against prevRate === 0 (or
otherwise falsy) in addition to daysBetween > 0 — e.g., treat apyBase as 0 or
skip the computation when prevRate is 0 to avoid Infinity/NaN — and ensure the
check references the same symbols (latestRate, prevRate, daysBetween, apyBase)
so the ternary/conditional prevents division by zero before performing
(latestRate / prevRate) ** (365 / daysBetween).
---
Nitpick comments:
In `@src/adaptors/acre/index.js`:
- Around line 50-51: The comment above the conditional is misleading: it says
"Skip rounds with identical timestamps" but the code checks the variable rate in
the conditional if (rate > 0); update the comment to accurately describe the
runtime check (e.g., "Process only positive rates" or "Skip non-positive rates")
or change the condition to match the original intent; locate the conditional
using the symbol rate and the if (rate > 0) line and make the comment and
condition consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bec2bb8c-346d-4297-ba79-55a33c817e7c
📒 Files selected for processing (1)
src/adaptors/acre/index.js
Comment on lines
+38
to
+56
| let prevRate, prevTs; | ||
|
|
||
| for (let id = latestId - 1; id >= 1; id--) { | ||
| const round = await sdk.api.abi.call({ | ||
| target: AGGREGATOR, | ||
| abi: GET_ROUND_DATA_ABI, | ||
| params: [id], | ||
| chain: CHAIN, | ||
| }); | ||
| const ts = Number(round.output.updatedAt); | ||
| const rate = round.output.answer / 1e8; | ||
|
|
||
| // Skip rounds with identical timestamps (no real update) | ||
| if (rate > 0) { | ||
| prevRate = rate; | ||
| prevTs = ts; | ||
| } | ||
| if (ts <= targetTs) break; | ||
| } |
Contributor
There was a problem hiding this comment.
prevRate and prevTs may be undefined, causing NaN in APY calculation.
If latestId <= 1 (loop doesn't execute), or if no rounds have rate > 0, then prevRate and prevTs remain undefined. This will produce NaN for apyBase due to division by undefined.
🛡️ Proposed fix: Initialize fallback values
- let prevRate, prevTs;
+ let prevRate = latestRate;
+ let prevTs = latestTs;
for (let id = latestId - 1; id >= 1; id--) {This ensures if no valid historical round is found, apyBase will be 0 (since daysBetween would be 0).
📝 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
| let prevRate, prevTs; | |
| for (let id = latestId - 1; id >= 1; id--) { | |
| const round = await sdk.api.abi.call({ | |
| target: AGGREGATOR, | |
| abi: GET_ROUND_DATA_ABI, | |
| params: [id], | |
| chain: CHAIN, | |
| }); | |
| const ts = Number(round.output.updatedAt); | |
| const rate = round.output.answer / 1e8; | |
| // Skip rounds with identical timestamps (no real update) | |
| if (rate > 0) { | |
| prevRate = rate; | |
| prevTs = ts; | |
| } | |
| if (ts <= targetTs) break; | |
| } | |
| let prevRate = latestRate; | |
| let prevTs = latestTs; | |
| for (let id = latestId - 1; id >= 1; id--) { | |
| const round = await sdk.api.abi.call({ | |
| target: AGGREGATOR, | |
| abi: GET_ROUND_DATA_ABI, | |
| params: [id], | |
| chain: CHAIN, | |
| }); | |
| const ts = Number(round.output.updatedAt); | |
| const rate = round.output.answer / 1e8; | |
| // Skip rounds with identical timestamps (no real update) | |
| if (rate > 0) { | |
| prevRate = rate; | |
| prevTs = ts; | |
| } | |
| if (ts <= targetTs) break; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/adaptors/acre/index.js` around lines 38 - 56, The loop that computes
historical round data can leave prevRate and prevTs undefined (e.g., when
latestId <= 1 or no rounds with rate > 0), causing NaN in the apyBase
calculation; to fix it, initialize prevRate and prevTs to safe fallbacks before
the loop (for example set them to the current round's rate and timestamp or to
the same values as the latest round used to compute targetTs) and/or guard the
apyBase computation in the function that computes APY (ensure daysBetween is
non-zero and if prevRate or prevTs are missing set apyBase = 0). Update the
variables referenced in this diff (prevRate, prevTs, latestId, targetTs, and the
apyBase calculation) so the code never divides by undefined and returns 0 APY
when no valid historical round exists.
Comment on lines
+58
to
+59
| const tbtcPrice = pricesByAddress[TBTC.toLowerCase()]; | ||
| const tvlUsd = (tvlRes.output / 1e18) * tbtcPrice; |
Contributor
There was a problem hiding this comment.
Missing null check for tbtcPrice.
If the price API fails or doesn't return data for TBTC, pricesByAddress[TBTC.toLowerCase()] will be undefined, resulting in tvlUsd being NaN.
🛡️ Proposed fix: Add fallback
- const tbtcPrice = pricesByAddress[TBTC.toLowerCase()];
+ const tbtcPrice = pricesByAddress[TBTC.toLowerCase()] || 0;
const tvlUsd = (tvlRes.output / 1e18) * tbtcPrice;📝 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
| const tbtcPrice = pricesByAddress[TBTC.toLowerCase()]; | |
| const tvlUsd = (tvlRes.output / 1e18) * tbtcPrice; | |
| const tbtcPrice = pricesByAddress[TBTC.toLowerCase()] || 0; | |
| const tvlUsd = (tvlRes.output / 1e18) * tbtcPrice; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/adaptors/acre/index.js` around lines 58 - 59, The code computes tvlUsd
using tbtcPrice from pricesByAddress[TBTC.toLowerCase()] but doesn't handle
undefined; update the tvlUsd calculation to guard against missing prices by
checking tbtcPrice (from pricesByAddress and TBTC) and using a safe fallback or
throwing a clear error—e.g., if tbtcPrice is falsy default to 0 (or handle
upstream), then compute tvlUsd using tvlRes.output / 1e18 multiplied by that
safePrice; ensure any logging or error path references tbtcPrice,
pricesByAddress, TBTC, tvlUsd, and tvlRes.output so the issue is detectable.
Comment on lines
+62
to
+65
| const apyBase = | ||
| daysBetween > 0 | ||
| ? (latestRate / prevRate) ** (365 / daysBetween) * 100 - 100 | ||
| : 0; |
Contributor
There was a problem hiding this comment.
Potential division by zero if prevRate is 0.
If a historical round has answer = 0 and it's the only valid round found, latestRate / prevRate will be Infinity.
🛡️ Proposed fix: Add prevRate validation
const daysBetween = (latestTs - prevTs) / DAY;
const apyBase =
- daysBetween > 0
+ daysBetween > 0 && prevRate > 0
? (latestRate / prevRate) ** (365 / daysBetween) * 100 - 100
: 0;📝 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
| const apyBase = | |
| daysBetween > 0 | |
| ? (latestRate / prevRate) ** (365 / daysBetween) * 100 - 100 | |
| : 0; | |
| const apyBase = | |
| daysBetween > 0 && prevRate > 0 | |
| ? (latestRate / prevRate) ** (365 / daysBetween) * 100 - 100 | |
| : 0; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/adaptors/acre/index.js` around lines 62 - 65, The calculation of apyBase
can divide by prevRate which may be zero; update the logic that computes apyBase
(the expression using latestRate, prevRate and daysBetween) to guard against
prevRate === 0 (or otherwise falsy) in addition to daysBetween > 0 — e.g., treat
apyBase as 0 or skip the computation when prevRate is 0 to avoid Infinity/NaN —
and ensure the check references the same symbols (latestRate, prevRate,
daysBetween, apyBase) so the ternary/conditional prevents division by zero
before performing (latestRate / prevRate) ** (365 / daysBetween).
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
2 participants
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.
Summary by CodeRabbit