Conversation
📝 WalkthroughWalkthroughThe changes refactor Fira lending market integration across multiple modules: the main Fira TVL now discovers lending markets from logs and aggregates balances across fixed, variable, and legacy markets instead of a single vault; a new shared Changes
Sequence Diagram(s)sequenceDiagram
participant API
participant Logs as Blockchain Logs
participant Fixed as Fixed Market
participant Variable as Variable Market
participant Legacy as Legacy UZR Market
participant Tokens as Token Balances
API->>Logs: Fetch CreateMarket logs
Logs-->>API: Raw market events
API->>API: Normalize & deduplicate market params
API->>Fixed: Get totalBorrowAssets
API->>Variable: Get totalBorrowAssets
API->>Legacy: Get totalBorrowAssets
Fixed-->>API: Borrow amounts
Variable-->>API: Borrow amounts
Legacy-->>API: Borrow amounts
API->>Tokens: Fetch balances for loanToken & collateralToken
Tokens-->>API: Token balances
API->>API: Aggregate TVL from all markets
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ 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/fira exports TVL: |
|
The adapter at projects/treasury/usual.js exports TVL: |
|
The adapter at projects/treasury/fira.js exports TVL: |
![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
🧹 Nitpick comments (2)
projects/fira/treasuryHelper.js (1)
74-78: Inconsistency: UZR market discovery differs fromindex.js.This helper hardcodes
UZR_MARKET_PARAMSfor the legacy UZR market, whileprojects/fira/index.jsdiscovers UZR markets viagetLogs(LEGACY_UZR_LENDING_MARKET). If the hardcoded params differ from on-chain events (e.g., additional markets exist), treasury and TVL calculations would cover different market sets.Consider using
getMarketParams(api, LEGACY_UZR_LENDING_MARKET)here for consistency, or document why the hardcoded approach is intentional for treasury calculations.♻️ Suggested change for consistency
async function addFiraTreasuryPositions(api, owners) { - const [fixedParams, variableParams] = await Promise.all([ + const [fixedParams, variableParams, uzrParams] = await Promise.all([ getMarketParams(api, FIXED_LENDING_MARKET), getMarketParams(api, VARIABLE_LENDING_MARKET), + getMarketParams(api, LEGACY_UZR_LENDING_MARKET), ]); const markets = [ ...fixedParams.map((params) => ({ target: FIXED_LENDING_MARKET, params })), ...variableParams.map((params) => ({ target: VARIABLE_LENDING_MARKET, params })), - { target: LEGACY_UZR_LENDING_MARKET, params: UZR_MARKET_PARAMS }, + ...uzrParams.map((params) => ({ target: LEGACY_UZR_LENDING_MARKET, params })), ];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@projects/fira/treasuryHelper.js` around lines 74 - 78, The markets array in treasuryHelper.js currently hardcodes UZR_MARKET_PARAMS for LEGACY_UZR_LENDING_MARKET which may diverge from on-chain discovery in projects/fira/index.js; replace the hardcoded legacy entry by calling getMarketParams(api, LEGACY_UZR_LENDING_MARKET) (or otherwise invoke the same discovery helper used in index.js) and append its returned params to the markets list, ensuring you reference FIXED_LENDING_MARKET, VARIABLE_LENDING_MARKET, LEGACY_UZR_LENDING_MARKET, and UZR_MARKET_PARAMS locations when updating the markets construction or add an explicit comment if you intentionally keep the hardcoded UZR_MARKET_PARAMS.projects/fira/index.js (1)
101-121: Borrowed tracks only fixed-rate markets - methodology could be clearer.The comment mentions "For variable markets we use the same methodology as morpho" but doesn't explain what that means in practice. Since variable markets are excluded from the
borrowedcalculation, consider clarifying whether:
- Variable market debt is tracked elsewhere (e.g., by Morpho's adapter)
- Variable market debt isn't material for Fira's reporting
The implementation itself is correct.
📝 Suggested comment improvement
async function borrowed(api) { - // Borrowed is tracked separately from TVL. - // we compute borrowed debt from fixed-rate markets only. For variable markets we use the same methodology as morpho. + // Borrowed tracks outstanding debt from fixed-rate Fira markets only. + // Variable-rate markets use Morpho's underlying infrastructure, so their debt + // is already captured by Morpho's adapter and excluded here to avoid double-counting. const fixed = await getMarketsAndTokens(api, FIXED_LENDING_MARKET);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@projects/fira/index.js` around lines 101 - 121, The comment inside the borrowed function is vague about excluding variable markets; update the comment in the borrowed function (and nearby references like getMarketsAndTokens, FIXED_LENDING_MARKET, fixedMarkets, LEGACY_UZR_MARKET_ID/LEGACY_UZR_LENDING_MARKET) to explicitly state whether variable-market debt is tracked by another adapter (e.g., Morpho) or is immaterial for Fira reporting, and add one sentence clarifying that this function only sums totalBorrowAssets for fixed-rate markets (including the legacy UZR market) so readers know where to look for variable-rate debt.
🤖 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/treasury/fira.js`:
- Around line 3-9: The owners array in fira.js duplicates the same 5 Usual
treasury addresses and calls addFiraTreasuryPositions(api, owners), causing
double-counting with treasury/usual.js; either replace the owners array with
Fira-specific owner addresses and keep addFiraTreasuryPositions in fira.js, or
remove the addFiraTreasuryPositions call (and/or the duplicated owners array)
from fira.js so only treasury/usual.js tracks those wallets; also update or
remove the doublecounted flag in fira.js to accurately reflect which adapter
owns these positions (reference the owners constant,
addFiraTreasuryPositions(api, owners), and the doublecounted flag when making
the change).
---
Nitpick comments:
In `@projects/fira/index.js`:
- Around line 101-121: The comment inside the borrowed function is vague about
excluding variable markets; update the comment in the borrowed function (and
nearby references like getMarketsAndTokens, FIXED_LENDING_MARKET, fixedMarkets,
LEGACY_UZR_MARKET_ID/LEGACY_UZR_LENDING_MARKET) to explicitly state whether
variable-market debt is tracked by another adapter (e.g., Morpho) or is
immaterial for Fira reporting, and add one sentence clarifying that this
function only sums totalBorrowAssets for fixed-rate markets (including the
legacy UZR market) so readers know where to look for variable-rate debt.
In `@projects/fira/treasuryHelper.js`:
- Around line 74-78: The markets array in treasuryHelper.js currently hardcodes
UZR_MARKET_PARAMS for LEGACY_UZR_LENDING_MARKET which may diverge from on-chain
discovery in projects/fira/index.js; replace the hardcoded legacy entry by
calling getMarketParams(api, LEGACY_UZR_LENDING_MARKET) (or otherwise invoke the
same discovery helper used in index.js) and append its returned params to the
markets list, ensuring you reference FIXED_LENDING_MARKET,
VARIABLE_LENDING_MARKET, LEGACY_UZR_LENDING_MARKET, and UZR_MARKET_PARAMS
locations when updating the markets construction or add an explicit comment if
you intentionally keep the hardcoded UZR_MARKET_PARAMS.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 65a0225d-4947-4a96-a528-2e9cb7a59d75
📒 Files selected for processing (4)
projects/fira/index.jsprojects/fira/treasuryHelper.jsprojects/treasury/fira.jsprojects/treasury/usual.js
| const owners = [ | ||
| '0xF3D913De4B23ddB9CfdFAF955BAC5634CbAE95F4', | ||
| '0xc32e2a2F03d41768095e67b62C9c739f2C2Bc4aA', | ||
| '0x81ad394C0Fa87e99Ca46E1aca093BEe020f203f4', | ||
| '0xe3FD5A2cA538904A9e967CBd9e64518369e5a03f', | ||
| '0xcbf85D44178c01765Ab32Af72D5E291dcd39A06B', | ||
| ] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the owner overlap between fira.js and usual.js treasury adapters
echo "=== Fira treasury owners ==="
rg -A 10 "const owners" projects/treasury/fira.js | head -15
echo ""
echo "=== Usual treasury owners ==="
rg -A 10 "const owners" projects/treasury/usual.js | head -15Repository: DefiLlama/DefiLlama-Adapters
Length of output: 1017
🏁 Script executed:
# Check what addFiraTreasuryPositions does
cat projects/fira/treasuryHelper.jsRepository: DefiLlama/DefiLlama-Adapters
Length of output: 3902
🏁 Script executed:
# Check if there are any comments or context about ownership in the treasury files
rg -B 5 -A 15 "addFiraTreasuryPositions" projects/treasury/fira.js projects/treasury/usual.jsRepository: DefiLlama/DefiLlama-Adapters
Length of output: 4109
🏁 Script executed:
# Search for references to these specific addresses to understand their ownership
rg "0x81ad394C0Fa87e99Ca46E1aca093BEe020f203f4|0xF3D913De4B23ddB9CfdFAF955BAC5634CbAE95F4" --type js | head -20Repository: DefiLlama/DefiLlama-Adapters
Length of output: 502
🏁 Script executed:
# Check if fira.js is newly added and look for PR/commit context
git log --oneline --all -- projects/treasury/fira.js | head -5Repository: DefiLlama/DefiLlama-Adapters
Length of output: 92
🏁 Script executed:
# Check the Fira main adapter to see if it documents these treasury addresses
cat projects/fira/index.js 2>/dev/null || echo "File not found"Repository: DefiLlama/DefiLlama-Adapters
Length of output: 5007
🏁 Script executed:
# Search for documentation about Fira treasury in README or other files
rg -i "fira.*treasury|treasury.*fira" --type md --type js | grep -E "index|README|doc" | head -10Repository: DefiLlama/DefiLlama-Adapters
Length of output: 54
Fira treasury positions are double-counted in both treasury/fira.js and treasury/usual.js.
Both adapters use the identical 5 owner addresses (all labeled as Usual's treasuries in treasury/usual.js) and both call addFiraTreasuryPositions(api, owners) to track the same Fira lending market positions. This creates redundant counting of Fira positions held in Usual's treasury wallets.
The doublecounted: true flag in fira.js is misleading—it should clarify the inter-adapter duplication or be removed if the positions should only be tracked in treasury/usual.js.
Recommended actions:
- If
treasury/fira.jsis intended to track Fira-controlled treasuries, use different owner addresses specific to Fira. - If these Usual treasury addresses are the correct scope, remove
addFiraTreasuryPositionsfrom eitherfira.jsorusual.jsto prevent duplication. Given thatusual.jsalready includes these positions, consider removing them from the newfira.js.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@projects/treasury/fira.js` around lines 3 - 9, The owners array in fira.js
duplicates the same 5 Usual treasury addresses and calls
addFiraTreasuryPositions(api, owners), causing double-counting with
treasury/usual.js; either replace the owners array with Fira-specific owner
addresses and keep addFiraTreasuryPositions in fira.js, or remove the
addFiraTreasuryPositions call (and/or the duplicated owners array) from fira.js
so only treasury/usual.js tracks those wallets; also update or remove the
doublecounted flag in fira.js to accurately reflect which adapter owns these
positions (reference the owners constant, addFiraTreasuryPositions(api, owners),
and the doublecounted flag when making the change).
2 participants
add fira v1
Summary by CodeRabbit
New Features
Refactor