Merged
Conversation
📝 WalkthroughWalkthroughThe adaptor now uses a single Vaults API to read APY (from data.data.vault.metrics.apy), converts it to percentage, adds error checks for missing APY, and integrates Prism (yPrism) into pool generation with chain-specific handling for Ethereum, Base, Arbitrum, and BSC. URL export updated to the vaults/yprism page. Changes
Sequence Diagram(s)sequenceDiagram
participant Adapter as "Adaptor (yieldfi)"
participant VaultAPI as "Vaults API (gw.yield.fi)"
participant ChainProc as "Chain Processors\n(Ethereum, Base, Arbitrum, BSC)"
participant Output as "Pools Output"
rect rgba(100,149,237,0.5)
Adapter->>VaultAPI: GET /vaults/{vaultId} (fetch vault data)
VaultAPI-->>Adapter: 200 OK with data.data.vault.metrics.apy
end
rect rgba(60,179,113,0.5)
Adapter->>Adapter: extract APY, convert decimal→percentage\nvalidate APY present
Adapter->>ChainProc: send vault + APY + contract mappings (YUSD, yPrism, vy*)
end
rect rgba(255,165,0,0.5)
ChainProc->>ChainProc: apply chain-specific logic\ninclude/exclude yPrism per chain
ChainProc-->>Output: emit generated pool entries
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 |
|
The yieldfi adapter exports pools: Test Suites: 1 passed, 1 total |
![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 `@src/adaptors/yieldfi/index.js`:
- Around line 92-100: The code currently assumes data.data.vault.metrics.apy
exists and is a scalar, causing TypeErrors on unexpected responses and
misdocumenting an array; replace the direct access with a null-safe lookup using
optional chaining (e.g., data?.data?.vault?.metrics?.apy) and then normalize
_apy: if it's an array use the first element, if it's a number use it directly;
if missing, log the tokenSymbol and return 0; finally compute latestAPY =
normalizedApy * 100. Make these changes around the variables data, _apy, and
latestAPY in the fetch/response handling block.
- Around line 205-216: The current conditional only processes yETH/vyETH for
'arbitrum' and 'base', but YETH_CONTRACTS contains a 'saga' entry (and saga is
used elsewhere like YUSD_CONTRACTS); update the logic in the block that calls
processToken so that 'saga' is handled: call processToken for
YETH_CONTRACTS['saga'] with token id 'yETH' and chain 'saga'; for vyETH only
call processToken(VYETH_CONTRACTS['saga'], 'vyETH', 'saga') if VYETH_CONTRACTS
has a saga entry (guard on VYETH_CONTRACTS[chain]) or add a clarifying comment
if vyETH support is intentionally missing. Ensure you reference YETH_CONTRACTS,
VYETH_CONTRACTS and processToken when making the change.
🧹 Nitpick comments (1)
src/adaptors/yieldfi/index.js (1)
235-235: Consider using a more general URL.The exported URL now points specifically to the yPrism vault, but this adaptor serves multiple tokens (yUSD, vyUSD, yETH, vyETH, yBTC, vyBTC, yPrism). A more general URL like
https://yield.fi/vaultsmight better represent the full scope of pools.
Comment on lines
92
to
100
| const data = await response.json(); | ||
| const apyHistory = data.apy_history; | ||
|
|
||
| if (!apyHistory || !Array.isArray(apyHistory) || apyHistory.length === 0) { | ||
| const _apy = data.data.vault.metrics.apy; | ||
| if (!_apy) { | ||
| console.error(`No APY history data found for ${tokenSymbol}`); | ||
| return 0; | ||
| } | ||
|
|
||
| // Get the latest APY (first entry in the array is the most recent) | ||
| const latestAPY = apyHistory[0].apy; | ||
|
|
||
| console.log(`${tokenSymbol} latest APY: ${latestAPY.toFixed(2)}%`); | ||
| const latestAPY = _apy * 100; |
There was a problem hiding this comment.
Add null-safe access to prevent runtime errors on unexpected API responses.
If the API returns an unexpected structure (e.g., missing vault or metrics), accessing data.data.vault.metrics.apy will throw a TypeError before the error handler catches it. Also, the comment on line 99 mentions "first entry in the array" but _apy is used as a scalar value.
🛡️ Proposed fix with optional chaining
const data = await response.json();
- const _apy = data.data.vault.metrics.apy;
+ const _apy = data?.data?.vault?.metrics?.apy;
if (!_apy) {
console.error(`No APY history data found for ${tokenSymbol}`);
return 0;
}
- // Get the latest APY (first entry in the array is the most recent)
+ // Convert APY from decimal to percentage
const latestAPY = _apy * 100;📝 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 data = await response.json(); | |
| const apyHistory = data.apy_history; | |
| if (!apyHistory || !Array.isArray(apyHistory) || apyHistory.length === 0) { | |
| const _apy = data.data.vault.metrics.apy; | |
| if (!_apy) { | |
| console.error(`No APY history data found for ${tokenSymbol}`); | |
| return 0; | |
| } | |
| // Get the latest APY (first entry in the array is the most recent) | |
| const latestAPY = apyHistory[0].apy; | |
| console.log(`${tokenSymbol} latest APY: ${latestAPY.toFixed(2)}%`); | |
| const latestAPY = _apy * 100; | |
| const data = await response.json(); | |
| const _apy = data?.data?.vault?.metrics?.apy; | |
| if (!_apy) { | |
| console.error(`No APY history data found for ${tokenSymbol}`); | |
| return 0; | |
| } | |
| // Convert APY from decimal to percentage | |
| const latestAPY = _apy * 100; |
🤖 Prompt for AI Agents
In `@src/adaptors/yieldfi/index.js` around lines 92 - 100, The code currently
assumes data.data.vault.metrics.apy exists and is a scalar, causing TypeErrors
on unexpected responses and misdocumenting an array; replace the direct access
with a null-safe lookup using optional chaining (e.g.,
data?.data?.vault?.metrics?.apy) and then normalize _apy: if it's an array use
the first element, if it's a number use it directly; if missing, log the
tokenSymbol and return 0; finally compute latestAPY = normalizedApy * 100. Make
these changes around the variables data, _apy, and latestAPY in the
fetch/response handling block.
|
The yieldfi adapter exports pools: Test Suites: 1 passed, 1 total |
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
3 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.
Added support for tracking Prism token across Ethereum mainnet and Layer 2 networks (bsc).
Updated api endpoint for fetching apy
NOTE: Please update the price feed for yPrism
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.