Add stakeTHENA erc4626 vault on BSC#2499
Open
ftm1337 wants to merge 3 commits intoDefiLlama:masterfrom
Open
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughNew adapter module Changes
Sequence DiagramsequenceDiagram
participant Client
participant Adapter as Guru\ Adapter
participant PriceAPI as coins.llama.fi
participant Utils as utils.getERC4626Info
participant Result as Results
Client->>Adapter: call main()/apy()
Adapter->>PriceAPI: fetch prices for underlying token list
PriceAPI-->>Adapter: return price and decimals
Adapter->>Utils: request ERC‑4626 info for each wrapper (concurrent)
Utils-->>Adapter: return vault info (tvl, decimals, apyBase, etc.)
Adapter->>Adapter: compute tvlUsd = ERC4626_TVL / 10^decimals * token_price
Adapter->>Result: assemble pool result objects
Result-->>Client: return array of pool results
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 guru-network-classic 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: 1
🤖 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/guru-network-classic/index.js`:
- Around line 17-21: The axios call that builds `prices` should include a
timeout (e.g., pass `{ timeout: 10_000 }`) to avoid hanging requests, and
downstream access to price data (where `prices[token].decimals` and
`prices[token].price` are used) must be guarded with optional chaining and
existence checks (e.g., `prices[token]?.price`) or skip/handle missing entries
from `wrappers` so the adapter doesn't throw; update the axios.get invocation
that uses `wrappers.map(...)` to include the timeout option and modify the code
paths that read `prices[token]` to check for a truthy entry (or provide a safe
default/skip) using the `prices` variable and the `token` identifier.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d1fb80cc-f29f-41a3-95bf-cc4e18d2b375
📒 Files selected for processing (1)
src/adaptors/guru-network-classic/index.js
|
The guru-network-classic adapter exports pools: Test Suites: 1 passed, 1 total |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/adaptors/guru-network-classic/index.js (1)
17-21:⚠️ Potential issue | 🟠 MajorPreviously reported hardening gap is still present (timeout + missing price-entry guard).
Line 18 still calls an external endpoint without timeout, and Line 32 still dereferences
prices[token]without existence checks. This can hang or throw when the pricing API is slow or omits a key.#!/bin/bash set -euo pipefail echo "Inspect current implementation:" nl -ba src/adaptors/guru-network-classic/index.js | sed -n '16,36p' echo echo "Compare common guard/timeout patterns in adaptors:" rg -nP --type=js -C2 'axios\.get\(|timeout:\s*[0-9_]+|prices\[[^\]]+\]\?\.|if\s*\(!prices\[' src/adaptors | head -n 200Also applies to: 32-32
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/adaptors/guru-network-classic/index.js` around lines 17 - 21, The axios.get call that builds the prices map (using wrappers.map -> `${w.underlyingChain}:${w.underlyingToken}`) must be hardened: call axios.get with a timeout option (e.g. { timeout: 5000 }) and wrap the request in try/catch to handle request errors; after receiving the response, validate that .data and .data.coins exist before assigning to prices and when later accessing prices[token] (i.e. guard prices[token] with an existence check or fallback value) so dereferencing a missing price does not throw—apply these changes around the axios.get invocation and any code that reads prices[token], and ensure errors are logged/handled gracefully.
🤖 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/guru-network-classic/index.js`:
- Around line 16-21: The main issue is that the exported async function main
does not accept the timetravel timestamp and always calls the current prices
endpoint; update the main signature to accept timestamp = null (async function
main(timestamp = null) or equivalent) and change the axios URL from
/prices/current/... to /prices/historical/${timestamp}/... (keeping the
wrappers.map(...) join logic unchanged); ensure the function still works when
timestamp is provided by the framework and that the unique symbols main and
wrappers are the places to edit.
---
Duplicate comments:
In `@src/adaptors/guru-network-classic/index.js`:
- Around line 17-21: The axios.get call that builds the prices map (using
wrappers.map -> `${w.underlyingChain}:${w.underlyingToken}`) must be hardened:
call axios.get with a timeout option (e.g. { timeout: 5000 }) and wrap the
request in try/catch to handle request errors; after receiving the response,
validate that .data and .data.coins exist before assigning to prices and when
later accessing prices[token] (i.e. guard prices[token] with an existence check
or fallback value) so dereferencing a missing price does not throw—apply these
changes around the axios.get invocation and any code that reads prices[token],
and ensure errors are logged/handled gracefully.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3b348728-fa9b-40f2-9032-f74c268b114c
📒 Files selected for processing (1)
src/adaptors/guru-network-classic/index.js
|
The guru-network-classic adapter exports pools: Test Suites: 1 passed, 1 total |
Contributor
Author
|
alright, good to go i guess. tvl & apy numbers tally up correctly 👍 |
0xkr3p
reviewed
Mar 25, 2026
| } | ||
| return { | ||
| pool: info.pool, | ||
| chain: wrappers[i].underlyingChain, |
Contributor
There was a problem hiding this comment.
please use the helper: utils.formatChain(wrappers[i].underlyingChain) so we can map correctly on our side e.g. bsc -> Binance
0xkr3p
reviewed
Mar 25, 2026
| module.exports = { | ||
| timetravel: true, | ||
| apy: main, | ||
| // url: 'https://example.com/pools', // Link to page with pools (Only required if you do not provide url's for each pool), |
Contributor
There was a problem hiding this comment.
add base url here as a fallback pls
0xkr3p
reviewed
Mar 25, 2026
| chain: wrappers[i].underlyingChain, | ||
| project: PROJECT, | ||
| symbol: wrappers[i].symbol, | ||
| tvlUsd: (Number(info.tvl) / 10 ** priceEntry.decimals) * priceEntry.price, |
0xkr3p
reviewed
Mar 25, 2026
| return infos | ||
| .map((info, i) => { | ||
| const token = `${wrappers[i].underlyingChain}:${wrappers[i].underlyingToken}`; | ||
| const priceEntry = prices[token]; |
Contributor
|
hey @ftm1337 - thanks for the PR! A couple of minor issues to resolve then we are good to go! Can you format the code with prettier? there are quite a few inconsistencies |
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.
Summary by CodeRabbit