Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new MaxShot adaptor at Changes
Sequence Diagram(s)sequenceDiagram
participant Adaptor as MaxShot Adaptor
participant API as MaxShot API
participant SDK as DefiLlama SDK
participant Contract as Vault Contract
Adaptor->>API: GET /vaults (vaults with netApy24h, totalValue, multi-chain info)
API-->>Adaptor: vaults payload
loop For each vault and chain
Adaptor->>SDK: getContract(vaultAddress, chain) / call asset()
SDK->>Contract: asset()
Contract-->>SDK: underlying token address
SDK-->>Adaptor: underlying token
alt multi-chain
Adaptor->>SDK: call totalSupply()
SDK-->>Adaptor: totalSupply
Adaptor->>Adaptor: compute TVL using totalSupply, exchangeRate, assetDecimals
else single-chain
Adaptor->>Adaptor: use totalValue from API as TVL
end
Adaptor->>Adaptor: compute APY from netApy24h (scale 1e18 → %)
Adaptor-->>Adaptor: build pool object
end
Adaptor-->>Adaptor: return array of pool objects
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 maxshot adapter exports pools: Test Suites: 1 passed, 1 total |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/adaptors/maxshot/index.js`:
- Around line 27-67: The TVL for multi-chain vaults is being duplicated because
tvlUsd is pushed for every chain; update the logic in the vaults loop to split
TVL across chains by computing a per-chain value (e.g., const perChainTvl =
isMultiChain ? tvlUsd / chainIds.length : tvlUsd) and use perChainTvl instead of
tvlUsd in the object pushed to pools (inside the for (const chainId of chainIds)
loop where pools.push is called). Ensure the rest of the fields (poolAddress
resolution, underlyingToken lookup via sdk.api.abi.call, url) remain unchanged.
- Around line 34-35: The code assumes vault.chainIds exists before calling
.split(), causing a crash if it's null/undefined; update the logic around the
chainIds constant in src/adaptors/maxshot/index.js so you validate
vault.chainIds first (e.g., guard and treat missing value as an empty
string/array or skip the vault) before doing
vault.chainIds.split(',').map(Number), and ensure downstream code that uses the
constant chainIds handles the empty-array case appropriately.
- Around line 49-55: The sequential awaits for sdk.api.abi.call when resolving
asset() per pool (the assetResult and underlyingToken assignment using
poolAddress) create a performance bottleneck; refactor the loop that calls
sdk.api.abi.call to collect all poolAddress/chainName inputs and run the calls
in parallel with Promise.all (or batch the RPC requests) then map results back
to set underlyingToken for each pool, ensuring you preserve chainName and
poolAddress association and handle errors per-call (e.g., default/skip on
failure).
🧹 Nitpick comments (1)
src/adaptors/maxshot/index.js (1)
55-55: Useconstinstead ofletforunderlyingToken.The variable is never reassigned.
♻️ Proposed fix
- let underlyingToken = assetResult.output; + const underlyingToken = assetResult.output;
| for (const vault of vaults) { | ||
| // Convert netApy24h from 1e18 to percentage (e.g., 30480776637436565 -> 3.0480776637436565%) | ||
| const apyValue = (Number(vault.netApy24h) / 1e18) * 100; | ||
|
|
||
| // Convert totalValue from 1e18 to USD (e.g., 1860856671194000000000000 -> 1860856.671194) | ||
| const tvlUsd = Number(vault.totalValue) / 1e18; | ||
|
|
||
| // Get all chain IDs from chainIds string (e.g., "1,10,8453,42161") | ||
| const chainIds = vault.chainIds.split(',').map(Number); | ||
| const isMultiChain = chainIds.length > 1; | ||
|
|
||
| // Create a pool for each chain | ||
| for (const chainId of chainIds) { | ||
| const chainName = CHAIN_ID_TO_NAME[chainId]; | ||
| if (!chainName) continue; // Skip unknown chain IDs | ||
|
|
||
| // Determine pool address: use multi-chain address if chainIds > 1 | ||
| let poolAddress = vault.address; | ||
| if (isMultiChain && MULTI_CHAIN_VAULT_ADDRESSES[vault.symbol]) { | ||
| poolAddress = MULTI_CHAIN_VAULT_ADDRESSES[vault.symbol]; | ||
| } | ||
|
|
||
| // Get underlying asset address by calling asset() function | ||
| const assetResult = await sdk.api.abi.call({ | ||
| target: poolAddress, | ||
| abi: "function asset() public view returns (address)", | ||
| chain: chainName, | ||
| }); | ||
| let underlyingToken = assetResult.output; | ||
|
|
||
| pools.push({ | ||
| pool: `${poolAddress.toLowerCase()}-${chainName}`, | ||
| chain: utils.formatChain(chainName), | ||
| project: 'maxshot', | ||
| symbol: utils.formatSymbol(vault.symbol), | ||
| tvlUsd, | ||
| apy: apyValue, | ||
| underlyingTokens: [underlyingToken], | ||
| url: `https://app.maxshot.ai/#/earn/${vault.address}`, | ||
| }); | ||
| } |
There was a problem hiding this comment.
TVL is duplicated across chains for multi-chain vaults.
When a vault has multiple chain IDs, the same tvlUsd value is pushed for each chain. This inflates the total TVL by a factor of chainIds.length. The TVL should either be divided among chains or only reported once for the primary chain.
🐛 Proposed fix to split TVL across chains
// Get all chain IDs from chainIds string (e.g., "1,10,8453,42161")
const chainIds = vault.chainIds.split(',').map(Number);
const isMultiChain = chainIds.length > 1;
+
+ // Split TVL evenly across chains for multi-chain vaults
+ const chainTvlUsd = tvlUsd / chainIds.length;
// Create a pool for each chain
for (const chainId of chainIds) {
// ...
pools.push({
pool: `${poolAddress.toLowerCase()}-${chainName}`,
chain: utils.formatChain(chainName),
project: 'maxshot',
symbol: utils.formatSymbol(vault.symbol),
- tvlUsd,
+ tvlUsd: chainTvlUsd,
apy: apyValue,
underlyingTokens: [underlyingToken],
url: `https://app.maxshot.ai/#/earn/${vault.address}`,
});📝 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.
| for (const vault of vaults) { | |
| // Convert netApy24h from 1e18 to percentage (e.g., 30480776637436565 -> 3.0480776637436565%) | |
| const apyValue = (Number(vault.netApy24h) / 1e18) * 100; | |
| // Convert totalValue from 1e18 to USD (e.g., 1860856671194000000000000 -> 1860856.671194) | |
| const tvlUsd = Number(vault.totalValue) / 1e18; | |
| // Get all chain IDs from chainIds string (e.g., "1,10,8453,42161") | |
| const chainIds = vault.chainIds.split(',').map(Number); | |
| const isMultiChain = chainIds.length > 1; | |
| // Create a pool for each chain | |
| for (const chainId of chainIds) { | |
| const chainName = CHAIN_ID_TO_NAME[chainId]; | |
| if (!chainName) continue; // Skip unknown chain IDs | |
| // Determine pool address: use multi-chain address if chainIds > 1 | |
| let poolAddress = vault.address; | |
| if (isMultiChain && MULTI_CHAIN_VAULT_ADDRESSES[vault.symbol]) { | |
| poolAddress = MULTI_CHAIN_VAULT_ADDRESSES[vault.symbol]; | |
| } | |
| // Get underlying asset address by calling asset() function | |
| const assetResult = await sdk.api.abi.call({ | |
| target: poolAddress, | |
| abi: "function asset() public view returns (address)", | |
| chain: chainName, | |
| }); | |
| let underlyingToken = assetResult.output; | |
| pools.push({ | |
| pool: `${poolAddress.toLowerCase()}-${chainName}`, | |
| chain: utils.formatChain(chainName), | |
| project: 'maxshot', | |
| symbol: utils.formatSymbol(vault.symbol), | |
| tvlUsd, | |
| apy: apyValue, | |
| underlyingTokens: [underlyingToken], | |
| url: `https://app.maxshot.ai/#/earn/${vault.address}`, | |
| }); | |
| } | |
| for (const vault of vaults) { | |
| // Convert netApy24h from 1e18 to percentage (e.g., 30480776637436565 -> 3.0480776637436565%) | |
| const apyValue = (Number(vault.netApy24h) / 1e18) * 100; | |
| // Convert totalValue from 1e18 to USD (e.g., 1860856671194000000000000 -> 1860856.671194) | |
| const tvlUsd = Number(vault.totalValue) / 1e18; | |
| // Get all chain IDs from chainIds string (e.g., "1,10,8453,42161") | |
| const chainIds = vault.chainIds.split(',').map(Number); | |
| const isMultiChain = chainIds.length > 1; | |
| // Split TVL evenly across chains for multi-chain vaults | |
| const chainTvlUsd = tvlUsd / chainIds.length; | |
| // Create a pool for each chain | |
| for (const chainId of chainIds) { | |
| const chainName = CHAIN_ID_TO_NAME[chainId]; | |
| if (!chainName) continue; // Skip unknown chain IDs | |
| // Determine pool address: use multi-chain address if chainIds > 1 | |
| let poolAddress = vault.address; | |
| if (isMultiChain && MULTI_CHAIN_VAULT_ADDRESSES[vault.symbol]) { | |
| poolAddress = MULTI_CHAIN_VAULT_ADDRESSES[vault.symbol]; | |
| } | |
| // Get underlying asset address by calling asset() function | |
| const assetResult = await sdk.api.abi.call({ | |
| target: poolAddress, | |
| abi: "function asset() public view returns (address)", | |
| chain: chainName, | |
| }); | |
| let underlyingToken = assetResult.output; | |
| pools.push({ | |
| pool: `${poolAddress.toLowerCase()}-${chainName}`, | |
| chain: utils.formatChain(chainName), | |
| project: 'maxshot', | |
| symbol: utils.formatSymbol(vault.symbol), | |
| tvlUsd: chainTvlUsd, | |
| apy: apyValue, | |
| underlyingTokens: [underlyingToken], | |
| url: `https://app.maxshot.ai/#/earn/${vault.address}`, | |
| }); | |
| } |
🤖 Prompt for AI Agents
In `@src/adaptors/maxshot/index.js` around lines 27 - 67, The TVL for multi-chain
vaults is being duplicated because tvlUsd is pushed for every chain; update the
logic in the vaults loop to split TVL across chains by computing a per-chain
value (e.g., const perChainTvl = isMultiChain ? tvlUsd / chainIds.length :
tvlUsd) and use perChainTvl instead of tvlUsd in the object pushed to pools
(inside the for (const chainId of chainIds) loop where pools.push is called).
Ensure the rest of the fields (poolAddress resolution, underlyingToken lookup
via sdk.api.abi.call, url) remain unchanged.
There was a problem hiding this comment.
can you double check this to ensure we aren't double counting tvl across chains @Skyge
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| // Get all chain IDs from chainIds string (e.g., "1,10,8453,42161") | ||
| const chainIds = vault.chainIds.split(',').map(Number); |
There was a problem hiding this comment.
Missing null check on vault.chainIds before calling .split().
If a vault has a missing or undefined chainIds property, calling .split() will throw a TypeError and crash the adaptor.
🛡️ Proposed fix to add validation
// Get all chain IDs from chainIds string (e.g., "1,10,8453,42161")
+ if (!vault.chainIds) continue;
const chainIds = vault.chainIds.split(',').map(Number);🤖 Prompt for AI Agents
In `@src/adaptors/maxshot/index.js` around lines 34 - 35, The code assumes
vault.chainIds exists before calling .split(), causing a crash if it's
null/undefined; update the logic around the chainIds constant in
src/adaptors/maxshot/index.js so you validate vault.chainIds first (e.g., guard
and treat missing value as an empty string/array or skip the vault) before doing
vault.chainIds.split(',').map(Number), and ensure downstream code that uses the
constant chainIds handles the empty-array case appropriately.
|
The maxshot adapter exports pools: Test Suites: 1 passed, 1 total |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/adaptors/maxshot/index.js`:
- Around line 95-100: The TVL calculation currently casts large uint256 values
to Number (totalSupply, vault.exchangeRate) which can overflow; update the
isMultiChain branch to perform the arithmetic using BigInt for totalSupply and
exchangeRate, account for scaling factors (1e18 and 10**vault.assetDecimals) as
BigInt, compute the division using BigInt arithmetic, then convert the final
result to Number only when assigning tvlUsd; leave the else branch using
Number(vault.totalValue)/1e18 unchanged and reference the symbols isMultiChain,
totalSupply, vault.exchangeRate, vault.assetDecimals, vault.totalValue, and
tvlUsd when making the change.
|
The maxshot adapter exports pools: Test Suites: 1 passed, 1 total |
|
@0xkr3p I've finished implementing the changes from the AI code review. My updates are ready for the next round of review. What's the best way to proceed—should I ping a specific reviewer or just mark the PR as ready? |
|
The maxshot adapter exports pools: Test Suites: 1 passed, 1 total |
|
The maxshot adapter exports pools: Test Suites: 1 passed, 1 total |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/adaptors/maxshot/index.js`:
- Around line 52-83: The per-vault RPC work inside rpcPromises (mapping
poolInfos and calling sdk.api.abi.call) lacks error handling so one failed call
rejects the whole Promise.all; wrap the async callback for poolInfos (the
function creating assetPromise/totalSupplyPromise and awaiting Promise.all) in a
try/catch that returns null on error, and ensure rpcResults (and subsequent
mapping that builds pools) filters out null entries (e.g., skip entries where
rpcResults[index] is falsy) so a single bad vault is skipped instead of crashing
the adaptor.
- Line 91: The computation of apyValue can produce NaN when vault.netApy24h is
undefined (Number(undefined) === NaN); update the expression that sets apyValue
in src/adaptors/maxshot/index.js to defensively default vault.netApy24h to 0
(e.g., use vault.netApy24h || 0 or vault.netApy24h ?? 0) so apyValue calculation
always yields a numeric value; adjust the single place where apyValue is
computed to reference the corrected expression.
| const { underlyingToken, totalSupply } = rpcResults[index]; | ||
|
|
||
| // Convert netApy24h from 1e18 to percentage | ||
| const apyValue = (Number(vault.netApy24h) / 1e18) * 100; |
There was a problem hiding this comment.
Potential NaN if vault.netApy24h is null or undefined.
Number(null) is 0 and Number(undefined) is NaN. If a vault has no APY data, this will propagate NaN into the pool object. A defensive default would help:
const apyValue = (Number(vault.netApy24h || 0) / 1e18) * 100;🤖 Prompt for AI Agents
In `@src/adaptors/maxshot/index.js` at line 91, The computation of apyValue can
produce NaN when vault.netApy24h is undefined (Number(undefined) === NaN);
update the expression that sets apyValue in src/adaptors/maxshot/index.js to
defensively default vault.netApy24h to 0 (e.g., use vault.netApy24h || 0 or
vault.netApy24h ?? 0) so apyValue calculation always yields a numeric value;
adjust the single place where apyValue is computed to reference the corrected
expression.
|
The maxshot adapter exports pools: Test Suites: 1 passed, 1 total |
|
The maxshot adapter exports pools: Test Suites: 1 passed, 1 total |
Summary by CodeRabbit