Open
Conversation
Contributor
📝 WalkthroughWalkthroughAdds a new DexFi adapter module implementing per-chain TVL logic and Base-specific staking adjustments; extends helper unwrapping utilities to support Algebra and Shadow (RamsesV3) NFT position variants and exports a new NFT unwrap helper. Changes
Sequence Diagram(s)sequenceDiagram
participant API as API Instance
participant Factory as Vault Factory
participant Vault as Vault Contract
participant Farm as Farm Connector
participant NFTDetect as NFT/Token Detector
participant AMMUnwrap as AMM Unwrapper (Slipstream/V3/Algebra)
participant Aggregator as Token Aggregator
API->>Factory: fetchVaults()
Factory-->>API: vault list
loop per vault
API->>Vault: getFarms()
Vault-->>API: farm addresses
loop per farm
API->>Farm: stakingToken(), tokenId?()
Farm-->>API: token / tokenId
alt NFT position
API->>NFTDetect: probeNFT(contract)
NFTDetect-->>API: flavor (Ramses/Slipstream/Algebra/UNI)
API->>AMMUnwrap: unwrap(flavor, tokenIds)
AMMUnwrap-->>API: underlying tokens & amounts
else ERC20/LP
API->>Farm: liquidity()
Farm-->>API: balances
end
API->>Aggregator: addBalances(...)
end
end
Aggregator->>Aggregator: resolveLPs & aggregate
Aggregator-->>API: total TVL
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 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 adapter at projects/dexfinance-vault-v2 exports TVL: |
![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)
projects/dexfinance-vault-v2/index.js (2)
89-123: Well-structured flavor detection for NFT positions.The detection logic correctly identifies four NFT position flavors:
- Shadow: Detected via
deployer→RamsesV3Factorychain- Slipstream: Detected via
poolImplementationon factory- Algebra: Detected via
poolByPairfunction availability- Standard UniV3: Fallback via
sumTokens2withuniV3ExtraConfigMinor simplification possible:
♻️ Simplify null check
- else if (poolByPairResults[j] !== null && poolByPairResults[j] !== undefined) algebraSet.add(f.idx); + else if (poolByPairResults[j] != null) algebraSet.add(f.idx);The
!= nullcheck covers bothnullandundefined.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@projects/dexfinance-vault-v2/index.js` around lines 89 - 123, The null/undefined check in the factory detection loop is verbose; in factoriesWithIndex.forEach where you currently do "if (poolImplResults[j]) slipstreamSet.add(f.idx); else if (poolByPairResults[j] !== null && poolByPairResults[j] !== undefined) algebraSet.add(f.idx);", replace the latter condition with the simpler "poolByPairResults[j] != null" to cover both null and undefined; update the condition referencing poolByPairResults[j] accordingly to keep the same behavior.
48-48: Avoid mutating input objects.
delete item.farm.datamutates the original farm object. While this likely works sincevaultFarmsis freshly created, it's safer to avoid mutation:♻️ Suggested change to avoid mutation
- delete item.farm.data; - return { ...item, connector }; + const { data, ...farmWithoutData } = item.farm; + return { ...item, farm: farmWithoutData, connector };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@projects/dexfinance-vault-v2/index.js` at line 48, The code currently mutates the farm object with "delete item.farm.data"; instead, create a non-mutating copy and assign that to item.farm (for example by shallow-cloning item or item.farm and omitting the data property) so vaultFarms remains derived without altering the original objects; locate the transformation where vaultFarms is built (the loop/map that references item and item.farm) and replace the delete with creation of a new farm object that excludes data, then set item.farm to that new object or build a new item object for the returned array.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@projects/helper/unwrapLPs.js`:
- Around line 342-343: The ABI string stored in algebraPositionsABI incorrectly
types the nonce as uint96 causing decoding errors; update the ABI declaration
for algebraPositionsABI so the positions(uint256) return tuple uses uint88 nonce
(matching the contract) instead of uint96, ensuring the rest of the return types
remain unchanged and references to algebraPositionsABI or the positions()
decoding logic will now match the on-chain contract.
---
Nitpick comments:
In `@projects/dexfinance-vault-v2/index.js`:
- Around line 89-123: The null/undefined check in the factory detection loop is
verbose; in factoriesWithIndex.forEach where you currently do "if
(poolImplResults[j]) slipstreamSet.add(f.idx); else if (poolByPairResults[j] !==
null && poolByPairResults[j] !== undefined) algebraSet.add(f.idx);", replace the
latter condition with the simpler "poolByPairResults[j] != null" to cover both
null and undefined; update the condition referencing poolByPairResults[j]
accordingly to keep the same behavior.
- Line 48: The code currently mutates the farm object with "delete
item.farm.data"; instead, create a non-mutating copy and assign that to
item.farm (for example by shallow-cloning item or item.farm and omitting the
data property) so vaultFarms remains derived without altering the original
objects; locate the transformation where vaultFarms is built (the loop/map that
references item and item.farm) and replace the delete with creation of a new
farm object that excludes data, then set item.farm to that new object or build a
new item object for the returned array.
🪄 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: 2dacaa8a-5667-43f3-9dc7-710bbf3eacf1
📒 Files selected for processing (2)
projects/dexfinance-vault-v2/index.jsprojects/helper/unwrapLPs.js
|
The adapter at projects/dexfinance-vault-v2 exports TVL: |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
projects/dexfinance-vault-v2/index.js (1)
44-51: Avoid mutating input object.
delete item.farm.datamutates the original array elements passed to this function. While harmless in current usage, this is a side effect that could cause unexpected behavior ifvaultFarmsis reused.♻️ Suggested fix: Create new object instead of mutating
return vaultFarms .map((item, i) => { const connector = connectors[i]; if (!connector) return null; - delete item.farm.data; - return { ...item, connector }; + const { data, ...farmWithoutData } = item.farm; + return { ...item, farm: farmWithoutData, connector }; }).filter(item => item !== null);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@projects/dexfinance-vault-v2/index.js` around lines 44 - 51, The code mutates input objects by calling delete on item.farm.data; instead, return a new object with connector and a copied farm that omits data. Locate the map that iterates vaultFarms and uses connectors (variables: vaultFarms, connectors, item, connector) and replace the mutation with creation of a newItem that clones item and its farm (or destructures farm to exclude data) and then return { ...newItem, connector } so the original vaultFarms elements are not modified.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@projects/dexfinance-vault-v2/index.js`:
- Around line 111-114: The current classification loop using factoriesWithIndex
incorrectly treats a zero address from poolByPairResults[j] as a valid Algebra
pool; update the condition inside the forEach (which references
factoriesWithIndex, poolImplResults, poolByPairResults, slipstreamSet,
algebraSet, and f.idx) to also ensure poolByPairResults[j] is not the zero
address (e.g. compare against '0x0000000000000000000000000000000000000000' or a
shared ADDRESS_ZERO constant) before adding f.idx to algebraSet, keeping the
existing slipstreamSet check intact.
- Around line 131-137: The liquidity() multicall is currently using
abi.vault.liquidity with the connector address as target (see liquidityCalls and
abi.vault.liquidity), which may be wrong: either connectors must implement
liquidity(address) or the call should target the vault (as in v1 where
calculationConnector was used). Update the multicall to target the correct
contract (vault or calculationConnector) and/or move the liquidity ABI to the
correct namespace to reflect its real target, and add a short code comment
documenting which contract (connector vs vault) is expected to expose
liquidity(address); keep permitFailure: true only if silent failures are
intentional.
---
Nitpick comments:
In `@projects/dexfinance-vault-v2/index.js`:
- Around line 44-51: The code mutates input objects by calling delete on
item.farm.data; instead, return a new object with connector and a copied farm
that omits data. Locate the map that iterates vaultFarms and uses connectors
(variables: vaultFarms, connectors, item, connector) and replace the mutation
with creation of a newItem that clones item and its farm (or destructures farm
to exclude data) and then return { ...newItem, connector } so the original
vaultFarms elements are not modified.
🪄 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: f4a39aa8-6b3c-4c69-a29a-6a026e09e97b
📒 Files selected for processing (1)
projects/dexfinance-vault-v2/index.js
Comment on lines
+111
to
+114
| factoriesWithIndex.forEach((f, j) => { | ||
| if (poolImplResults[j]) slipstreamSet.add(f.idx); | ||
| else if (poolByPairResults[j] !== null && poolByPairResults[j] !== undefined) algebraSet.add(f.idx); | ||
| }); |
Contributor
There was a problem hiding this comment.
Algebra detection may have false positives with zero address.
The check poolByPairResults[j] !== null && poolByPairResults[j] !== undefined would pass if poolByPair returns the zero address for non-existent pairs. This could incorrectly classify non-Algebra factories as Algebra.
🐛 Suggested fix: Also check for zero address
factoriesWithIndex.forEach((f, j) => {
if (poolImplResults[j]) slipstreamSet.add(f.idx);
- else if (poolByPairResults[j] !== null && poolByPairResults[j] !== undefined) algebraSet.add(f.idx);
+ else if (poolByPairResults[j] && poolByPairResults[j] !== ADDRESSES.null) algebraSet.add(f.idx);
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@projects/dexfinance-vault-v2/index.js` around lines 111 - 114, The current
classification loop using factoriesWithIndex incorrectly treats a zero address
from poolByPairResults[j] as a valid Algebra pool; update the condition inside
the forEach (which references factoriesWithIndex, poolImplResults,
poolByPairResults, slipstreamSet, algebraSet, and f.idx) to also ensure
poolByPairResults[j] is not the zero address (e.g. compare against
'0x0000000000000000000000000000000000000000' or a shared ADDRESS_ZERO constant)
before adding f.idx to algebraSet, keeping the existing slipstreamSet check
intact.
Comment on lines
+131
to
+137
| // ERC20 tokens and LPs | ||
| const liquidityCalls = tokenFarms.map(({ connector }) => ({ target: connector, params: [connector] })); | ||
| const liquidities = await api.multiCall({ calls: liquidityCalls, abi: abi.vault.liquidity, permitFailure: true }); | ||
| tokenFarms.forEach(({ stakingToken }, i) => { | ||
| if (liquidities[i]) api.add(stakingToken, liquidities[i]); | ||
| }); | ||
| await sumTokens2({ api, resolveLP: true }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify if liquidity() is called on vault or connector in existing dexfinance adapters
echo "=== Checking existing dexfinance adapter patterns ==="
rg -n "abi\.vault\.liquidity" projects/dexfinance*/
echo ""
echo "=== Checking how liquidity calls are structured ==="
rg -nB2 -A2 "liquidityCalls" projects/dexfinance*/Repository: DefiLlama/DefiLlama-Adapters
Length of output: 2996
🏁 Script executed:
#!/bin/bash
# Check what properties are available in tokenFarms
echo "=== Checking tokenFarms structure in getChainFarms ==="
rg -nB5 -A10 "function getChainFarms" projects/dexfinance-vault-v2/
echo ""
echo "=== Checking farm/vault structure ==="
rg -n "connector\|vault\|farm" projects/dexfinance-vault-v2/index.js | head -30
echo ""
echo "=== Looking for ABI definitions ==="
rg -n "abi" projects/dexfinance-vault-v2/index.js | head -20Repository: DefiLlama/DefiLlama-Adapters
Length of output: 1860
🏁 Script executed:
#!/bin/bash
# Check the beginning of dexfinance-vault-v2/index.js to find getChainFarms
echo "=== First 150 lines of dexfinance-vault-v2/index.js ==="
head -150 projects/dexfinance-vault-v2/index.js | cat -n
echo ""
echo "=== Check the ABI definitions ==="
cat projects/dexfinance-vault/abi.jsRepository: DefiLlama/DefiLlama-Adapters
Length of output: 9338
Clarify ABI usage for liquidity() calls.
The code calls abi.vault.liquidity on the connector address (line 132), but the ABI is defined under vault: in the ABI file. This differs from v1 dexfinance adapter which calls the same method on calculationConnector instead. If v2 connectors expose their own liquidity(address) method, this architectural change should be documented. If not, the method should be called on the vault address with the connector as a parameter (as in v1), and the ABI should be repositioned accordingly to reflect its actual target.
The same pattern appears at line 163-164. The permitFailure: true flag masks call failures, which is acceptable if intentional, but the target mismatch should be clarified.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@projects/dexfinance-vault-v2/index.js` around lines 131 - 137, The
liquidity() multicall is currently using abi.vault.liquidity with the connector
address as target (see liquidityCalls and abi.vault.liquidity), which may be
wrong: either connectors must implement liquidity(address) or the call should
target the vault (as in v1 where calculationConnector was used). Update the
multicall to target the correct contract (vault or calculationConnector) and/or
move the liquidity ABI to the correct namespace to reflect its real target, and
add a short code comment documenting which contract (connector vs vault) is
expected to expose liquidity(address); keep permitFailure: true only if silent
failures are intentional.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
projects/helper/unwrapLPs.js (1)
431-451:⚠️ Potential issue | 🟠 MajorAlgebra path bypasses
blacklistedPoolsfiltering.Line 448 currently skips pool blacklist enforcement when
isAlgebrais true. That creates a behavior gap and can include pools callers explicitly intended to exclude.Proposed fix
- if (!isAlgebra) { - const poolKey = `${token0.toLowerCase()}-${token1.toLowerCase()}-${position.fee}`.toLowerCase() - if (blacklistedPools.includes(poolKey)) return - } + const poolKey = isAlgebra + ? `${token0.toLowerCase()}-${token1.toLowerCase()}`.toLowerCase() + : `${token0.toLowerCase()}-${token1.toLowerCase()}-${position.fee}`.toLowerCase() + if (blacklistedPools.includes(poolKey)) return🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@projects/helper/unwrapLPs.js` around lines 431 - 451, The Algebra branch in addV3PositionBalances bypasses the pool blacklist check; ensure the same blacklistedPools filtering runs for Algebra positions by constructing the pool key from token0, token1 and position.fee (use the same normalization as the non-Algebra path, e.g., token0.toLowerCase()-token1.toLowerCase()-position.fee) and returning early if blacklistedPools.includes(poolKey) before proceeding; keep the check centralized in addV3PositionBalances and still preserve the existing positionId/blacklistedPositionIds logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@projects/helper/unwrapLPs.js`:
- Around line 431-451: The Algebra branch in addV3PositionBalances bypasses the
pool blacklist check; ensure the same blacklistedPools filtering runs for
Algebra positions by constructing the pool key from token0, token1 and
position.fee (use the same normalization as the non-Algebra path, e.g.,
token0.toLowerCase()-token1.toLowerCase()-position.fee) and returning early if
blacklistedPools.includes(poolKey) before proceeding; keep the check centralized
in addV3PositionBalances and still preserve the existing
positionId/blacklistedPositionIds logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cffbd546-1382-4748-9011-68e29563d77e
📒 Files selected for processing (1)
projects/helper/unwrapLPs.js
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.
resolves #18625
Summary by CodeRabbit
New Features
Chores