Extract active protocols dynamically rather than statically for Nolus Protocol#2321
Open
metodi96 wants to merge 3 commits intoDefiLlama:masterfrom
Open
Extract active protocols dynamically rather than statically for Nolus Protocol#2321metodi96 wants to merge 3 commits intoDefiLlama:masterfrom
metodi96 wants to merge 3 commits intoDefiLlama:masterfrom
Conversation
📝 WalkthroughWalkthroughThe Nolus Protocol adaptor now sources active protocols and currency decimals from the ETL API instead of hard-coded definitions; Changes
Sequence DiagramsequenceDiagram
participant getApy as getApy()
participant ETL as ETL API
participant Pool as Pool Data Source
participant Calc as Calc Logic
getApy->>ETL: fetchActiveProtocols()
ETL-->>getApy: active protocol descriptors (lpp, oracle, symbol, protocolName)
getApy->>ETL: fetchCurrencyDecimals()
ETL-->>getApy: ticker → decimals map
loop for each protocol
getApy->>Pool: request pool data (using lpp/oracle)
Pool-->>getApy: pool balances, rates
getApy->>Calc: compute price & TVL (use decimals map & getDecimalAdjustment)
Calc-->>getApy: adjusted price & APY
end
getApy->>getApy: aggregate results and return APY list
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
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 nolus-protocol adapter exports pools: Test Suites: 1 passed, 1 total |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/adaptors/nolus-protocol/index.js`:
- Around line 34-40: The protocols.map projection directly accesses nested
properties p.contracts.lpp and p.contracts.oracle which will throw if contracts
is null/undefined; update the mapping in the protocols.map callback (the code
creating objects with lpp/oracle/symbol/protocolName/meta) to defensively read
contracts (e.g., const c = p.contracts || {}; or use optional chaining) and
assign lpp and oracle from that safe reference so missing contracts produce
undefined (or a sensible default string) instead of throwing.
🧹 Nitpick comments (2)
src/adaptors/nolus-protocol/index.js (2)
71-75: Consider parallelizing independent API calls.
fetchActiveProtocols()andfetchCurrencyDecimals()are independent and could be fetched concurrently to reduce latency.⚡ Proposed optimization
- // Fetch active protocols dynamically from ETL - const contracts = await fetchActiveProtocols(); - - // Fetch currency decimals for dynamic adjustment - const currencyDecimals = await fetchCurrencyDecimals(); + // Fetch active protocols and currency decimals concurrently + const [contracts, currencyDecimals] = await Promise.all([ + fetchActiveProtocols(), + fetchCurrencyDecimals(), + ]);
94-97: Prefer strict equality operator.Use
===instead of==for type-safe comparison.♻️ Proposed fix
let currencyData = _.find( oracleCurrenciesData.data, - (n) => n.ticker == lppTickerData.data, + (n) => n.ticker === lppTickerData.data, );
|
The nolus-protocol 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/nolus-protocol/index.js`:
- Around line 62-70: The code currently falls back to stable decimals using the
logical OR operator which treats 0 as falsy; update the decimal fallback to use
nullish coalescing (??) so only null/undefined trigger the default. In practice,
modify getDecimalAdjustment usage and any other places (e.g., the block around
lines referenced 132-134) to compute lpnDecimals = (currency.decimal_digits ??
STABLE_QUOTE_DECIMALS) and then call getDecimalAdjustment(lpnDecimals), and
replace any other patterns like x || STABLE_QUOTE_DECIMALS with x ??
STABLE_QUOTE_DECIMALS to preserve a possible zero value.
|
The nolus-protocol 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/nolus-protocol/index.js`:
- Around line 62-70: getDecimalAdjustment currently returns 1 whenever
lpnDecimals < STABLE_QUOTE_DECIMALS which skips the required downscaling and
inflates price/TVL; update the function (and the duplicate logic at the other
occurrence around lines 131-134) to always return Math.pow(10, decimalDiff)
(where decimalDiff = lpnDecimals - STABLE_QUOTE_DECIMALS) so negative
differences correctly produce fractional adjustment factors, and ensure callers
use that factor for price/TVL scaling.
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
✏️ Tip: You can customize this high-level summary in your review settings.