Merged
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)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a new Zest v2 adapter for Stacks that enumerates static pools, fetches external prices, computes TVL in USD, reads on‑chain supply/borrow APYs and total assets from deployer/vault contracts, and returns standardized pool objects; errors are handled per‑pool and for the overall process. Changes
Sequence DiagramsequenceDiagram
participant Client as Client
participant Adapter as Zest v2 Adapter
participant PriceAPI as Price Data Service (CoinGecko/llama.fi)
participant Chain as Stacks Mainnet (Deployer & Vaults)
participant Result as Formatted Result
Client->>Adapter: getZestV2Pools()
Adapter->>PriceAPI: Fetch prices for asset keys
PriceAPI-->>Adapter: Return price data
Adapter->>Adapter: Select prices & compute TVL (per pool)
loop for each pool
Adapter->>Chain: Read deployer contract (supply/borrow APYs)
Chain-->>Adapter: APY responses
Adapter->>Chain: Read vault contract (total assets)
Chain-->>Adapter: Asset totals
Adapter->>Adapter: Build standardized pool object
end
Adapter->>Result: Aggregate pool objects
Result-->>Client: Return pools array
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 zest-v2 adapter exports pools: Test Suites: 1 passed, 1 total |
|
The zest-v2 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: 2
🧹 Nitpick comments (1)
src/adaptors/zest-v2/index.js (1)
76-88: Prefer the shared price utility over an inline llama client.This reimplements price fetching/selection logic that already exists in
src/adaptors/utils.js:334-363, so fallback and key-shaping behavior can drift independently over time.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/adaptors/zest-v2/index.js` around lines 76 - 88, Replace the local inline price client by removing the functions fetchPrices and getPrice and instead import and use the shared price utility exported from the adaptors utils module (the existing shared price-fetch/selection helpers) to obtain current prices and pick a matching key; update callers in this file to call the shared utility (use the util's price-fetch function to get the prices object and the util's selection function to choose the first available key/price), and remove the duplicate logic so fallback and key-shaping behavior is centralized.
🤖 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/zest-v2/index.js`:
- Around line 164-166: The catch block inside getZestV2Pools currently logs the
error and returns [], which masks adapter-wide failures as an empty dataset;
change it to propagate the failure instead of returning an empty array by
removing the "return []" and rethrowing (or throwing a new Error with contextual
message) after logging so upstream can handle/retry the error; ensure you
reference getZestV2Pools in src/adaptors/zest-v2/index.js when making the
change.
- Around line 76-80: fetchPrices uses axios.get without a timeout which can hang
the adapter; update fetchPrices to pass a timeout option to axios.get (e.g.,
axios.get(url, { timeout: <ms> })) so the request fails fast, and handle the
error path (catch) to avoid blocking pool emission. Make changes in the
fetchPrices function where keys, url, and axios.get are used so the request
includes a timeout value and errors are handled appropriately.
---
Nitpick comments:
In `@src/adaptors/zest-v2/index.js`:
- Around line 76-88: Replace the local inline price client by removing the
functions fetchPrices and getPrice and instead import and use the shared price
utility exported from the adaptors utils module (the existing shared
price-fetch/selection helpers) to obtain current prices and pick a matching key;
update callers in this file to call the shared utility (use the util's
price-fetch function to get the prices object and the util's selection function
to choose the first available key/price), and remove the duplicate logic so
fallback and key-shaping behavior is centralized.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b9e417d5-c851-4494-aff5-55e11e104273
📒 Files selected for processing (1)
src/adaptors/zest-v2/index.js
Comment on lines
+76
to
+80
| async function fetchPrices() { | ||
| const keys = [...new Set(POOLS.flatMap((p) => p.priceKeys))].join(','); | ||
| const url = `https://coins.llama.fi/prices/current/${keys}`; | ||
| const { data } = await axios.get(url); | ||
| return data.coins; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the actual code in the file
cat -n src/adaptors/zest-v2/index.js | head -100Repository: DefiLlama/yield-server
Length of output: 3469
🏁 Script executed:
# Check if there's any error handling around the axios call
rg -A 10 -B 5 "fetchPrices" src/adaptors/zest-v2/index.jsRepository: DefiLlama/yield-server
Length of output: 906
🏁 Script executed:
# Look for how fetchPrices is called and if there's any wrapper error handling
rg "fetchPrices\(\)" src/adaptors/zest-v2/index.jsRepository: DefiLlama/yield-server
Length of output: 136
🌐 Web query:
axios default timeout behavior javascript library
💡 Result:
- Axios’ default
timeoutis0, which means no timeout (it will wait indefinitely unless something else aborts the request). [1][2] timeoutis milliseconds before Axios aborts the request (when set to a positive number). [1]- You can set it per request (
axios.get(url, { timeout: 5000 })), globally (axios.defaults.timeout = 5000), or per instance (instance.defaults.timeout = 5000). [1][2] - For cases where a request may “hang” due to connection issues, Axios recommends combining
timeoutwith cancellation (e.g.,AbortController/AbortSignal.timeout(...)). [3]
Sources:
[1] Axios docs – Request Config (timeout: 1000, // default is 0 (no timeout)) (axios-http.com)
[2] Axios docs – Config defaults (notes default timeout is 0) (axios.rest)
[3] Axios docs – Cancellation (timeout vs cancellation, AbortSignal.timeout) (axios-http.com)
Citations:
- 1: https://axios-http.com/docs/req_config
- 2: https://axios.rest/pages/advanced/config-defaults
- 3: https://axios-http.com/docs/cancellation
Add a timeout to the price request.
axios.get() defaults to no timeout, so a hung llama.fi call will block the entire adapter indefinitely before any pools are emitted.
🛠️ Proposed fix
async function fetchPrices() {
const keys = [...new Set(POOLS.flatMap((p) => p.priceKeys))].join(',');
const url = `https://coins.llama.fi/prices/current/${keys}`;
- const { data } = await axios.get(url);
+ const { data } = await axios.get(url, { timeout: 10_000 });
return data.coins;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/adaptors/zest-v2/index.js` around lines 76 - 80, fetchPrices uses
axios.get without a timeout which can hang the adapter; update fetchPrices to
pass a timeout option to axios.get (e.g., axios.get(url, { timeout: <ms> })) so
the request fails fast, and handle the error path (catch) to avoid blocking pool
emission. Make changes in the fetchPrices function where keys, url, and
axios.get are used so the request includes a timeout value and errors are
handled appropriately.
Comment on lines
+164
to
+166
| } catch (error) { | ||
| console.log(`Error in getZestV2Pools: ${error.message}`); | ||
| return []; |
Contributor
There was a problem hiding this comment.
Don't mask adapter-wide failures as “zero pools”.
Returning [] here turns a transient upstream/RPC failure into a valid empty dataset, which can temporarily drop all Zest v2 markets from downstream consumers.
🛠️ Proposed fix
} catch (error) {
- console.log(`Error in getZestV2Pools: ${error.message}`);
- return [];
+ console.error(`Error in getZestV2Pools: ${error.message}`);
+ throw error;
}
}📝 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
| } catch (error) { | |
| console.log(`Error in getZestV2Pools: ${error.message}`); | |
| return []; | |
| } catch (error) { | |
| console.error(`Error in getZestV2Pools: ${error.message}`); | |
| throw error; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/adaptors/zest-v2/index.js` around lines 164 - 166, The catch block inside
getZestV2Pools currently logs the error and returns [], which masks adapter-wide
failures as an empty dataset; change it to propagate the failure instead of
returning an empty array by removing the "return []" and rethrowing (or throwing
a new Error with contextual message) after logging so upstream can handle/retry
the error; ensure you reference getZestV2Pools in src/adaptors/zest-v2/index.js
when making the change.
|
The zest-v2 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
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