add shapeswap v3 yields#2628
Open
DenSmolonski wants to merge 4 commits into
Open
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR introduces Shapeswap v3 adapter support for yield farming data. A subgraph domain whitelist is extended to recognize ChangesShapeswap v3 Protocol Support
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/adaptors/shapeswap-v3/index.jsParsing error: The keyword 'const' is reserved src/adaptors/utils.jsParsing error: The keyword 'const' is reserved 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 |
|
Error while running shapeswap-v3 adapter: Test Suites: 1 failed, 1 total |
|
Error while running shapeswap-v3 adapter: Test Suites: 1 failed, 1 total |
|
Error while running shapeswap-v3 adapter: Test Suites: 1 failed, 1 total |
Author
|
Hello @bheluga, Could you please take a look at this PR? The adapter has been merged: DefiLlama/DefiLlama-Adapters#18931 |
Author
|
Hello @bheluga, Do you have any updates regarding this PR? |
|
The shapeswap-v3 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
🧹 Nitpick comments (2)
src/adaptors/shapeswap-v3/index.js (2)
30-42: ⚡ Quick winAdd matching TVL filter for consistency.
The
priorVolumeQuerylacks thewhere: { totalValueLockedUSD_gt: "100" }filter present inpoolsQuery. This inconsistency means the two queries could return different pool sets, though the fallback logic on line 61 prevents errors. Adding the filter would improve consistency and slightly reduce query overhead.📋 Suggested fix to align query filters
const priorVolumeQuery = gql` query ($block: Int!) { pools( first: 1000 orderBy: totalValueLockedUSD orderDirection: desc block: { number: $block } + where: { totalValueLockedUSD_gt: "100" } ) { id volumeUSD } } `;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/adaptors/shapeswap-v3/index.js` around lines 30 - 42, The priorVolumeQuery (GraphQL template literal named priorVolumeQuery) is missing the same where filter used in poolsQuery; update priorVolumeQuery to include where: { totalValueLockedUSD_gt: "100" } inside the pools(...) args so both queries return the same filtered pool set (keep the existing block variable $block and other args intact) to improve consistency and reduce query load.
82-86: 💤 Low valueConsider clarifying time-travel support status.
The module exports
timetravel: false, but theapyfunction accepts atimestampparameter (line 44) and passes it toutils.getBlocks, which supports historical queries. This creates a potential inconsistency between the declared capability and the actual implementation.If historical queries are intentionally disabled due to subgraph reliability or testing concerns, this is fine. Otherwise, consider either setting
timetravel: trueor removing the timestamp parameter if historical queries aren't supported.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/adaptors/shapeswap-v3/index.js` around lines 82 - 86, The export declares timetravel: false while the apy(timestamp) function accepts a timestamp and calls utils.getBlocks (supporting historical lookups), causing inconsistency; either change the exported timetravel to true to reflect that apy supports historical queries, or remove the timestamp parameter and any utils.getBlocks calls from the apy function so it no longer performs historical queries—locate the apy function and the module.exports block and update timetravel or strip timestamp usage accordingly (ensure any callers and tests align with the chosen behavior).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/adaptors/shapeswap-v3/index.js`:
- Line 61: The calculation for 1-day USD volume uses a wrong fallback: replace
the fallback that uses volumeUsdAll with zero so missing prior data is treated
as priorVolume = 0. Update the expression that computes volumeUsd1d (which
references priorVolumeById, p.id and volumeUsdAll) to use priorVolumeById[p.id]
?? 0 instead of priorVolumeById[p.id] ?? volumeUsdAll so volumeUsd1d =
Math.max(volumeUsdAll - (priorVolumeById[p.id] ?? 0), 0); also review the
related log message about "falling back to zero prior volume" to ensure it
matches the new behavior.
---
Nitpick comments:
In `@src/adaptors/shapeswap-v3/index.js`:
- Around line 30-42: The priorVolumeQuery (GraphQL template literal named
priorVolumeQuery) is missing the same where filter used in poolsQuery; update
priorVolumeQuery to include where: { totalValueLockedUSD_gt: "100" } inside the
pools(...) args so both queries return the same filtered pool set (keep the
existing block variable $block and other args intact) to improve consistency and
reduce query load.
- Around line 82-86: The export declares timetravel: false while the
apy(timestamp) function accepts a timestamp and calls utils.getBlocks
(supporting historical lookups), causing inconsistency; either change the
exported timetravel to true to reflect that apy supports historical queries, or
remove the timestamp parameter and any utils.getBlocks calls from the apy
function so it no longer performs historical queries—locate the apy function and
the module.exports block and update timetravel or strip timestamp usage
accordingly (ensure any callers and tests align with the chosen behavior).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7fb8e7a9-5706-4621-a63e-504944e05bad
📒 Files selected for processing (2)
src/adaptors/shapeswap-v3/index.jssrc/adaptors/utils.js
| .map((p) => { | ||
| const tvlUsd = Number(p.totalValueLockedUSD); | ||
| const volumeUsdAll = Number(p.volumeUSD); | ||
| const volumeUsd1d = Math.max(volumeUsdAll - (priorVolumeById[p.id] ?? volumeUsdAll), 0); |
Contributor
There was a problem hiding this comment.
Incorrect fallback for missing prior volume data.
When priorVolumeById[p.id] is undefined (pool missing from prior data or prior query failed), the fallback volumeUsdAll causes the calculation to become volumeUsdAll - volumeUsdAll = 0, resulting in zero 1-day volume and zero APY for affected pools.
The error message on line 53 states "falling back to zero prior volume", which suggests the intent is to treat prior volume as 0 (not 1-day volume as 0). This would make 1-day volume equal to current cumulative volume for new pools or when prior data is unavailable.
🐛 Proposed fix to use zero prior volume as fallback
- const volumeUsd1d = Math.max(volumeUsdAll - (priorVolumeById[p.id] ?? volumeUsdAll), 0);
+ const volumeUsd1d = Math.max(volumeUsdAll - (priorVolumeById[p.id] ?? 0), 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 volumeUsd1d = Math.max(volumeUsdAll - (priorVolumeById[p.id] ?? volumeUsdAll), 0); | |
| const volumeUsd1d = Math.max(volumeUsdAll - (priorVolumeById[p.id] ?? 0), 0); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/adaptors/shapeswap-v3/index.js` at line 61, The calculation for 1-day USD
volume uses a wrong fallback: replace the fallback that uses volumeUsdAll with
zero so missing prior data is treated as priorVolume = 0. Update the expression
that computes volumeUsd1d (which references priorVolumeById, p.id and
volumeUsdAll) to use priorVolumeById[p.id] ?? 0 instead of priorVolumeById[p.id]
?? volumeUsdAll so volumeUsd1d = Math.max(volumeUsdAll - (priorVolumeById[p.id]
?? 0), 0); also review the related log message about "falling back to zero prior
volume" to ensure it matches the new behavior.
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
1 participant
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
New Features
Improvements