Open
Conversation
Contributor
Summary by CodeRabbitRelease Notes
WalkthroughThis pull request refactors the Fira fees adapter to implement a multi-stage market discovery and interest accounting pipeline across multiple contract components, while introducing a new Usual adapter integrating treasury balances and Dune query-based fee and revenue data. Changes
Sequence Diagram(s)sequenceDiagram
participant Discovery as Market Discovery
participant Params as Market Parameters
participant Tokens as Token Discovery
participant Interest as Interest Events
participant AMM as AMM Accounting
participant Liquidation as Liquidation Fees
participant Aggregation as Fee Aggregation
Discovery->>Discovery: Scan CreateMarket logs from fromBlock<br/>for each lendingMarket
Discovery->>Discovery: Deduplicate & append legacyMarkets
Discovery->>Params: Query idToMarketParams & market<br/>for loanToken, oracle, fee
Discovery->>Tokens: Query marketFactory CreateNewMarket<br/>logs for BTs
Tokens->>Tokens: Map BTs to CTs via CT()
Params->>Interest: For discovered markets,<br/>pull interest logs
Interest->>Interest: Variable: AccrueInterest logs<br/>Fixed-rate: RedeemInterest,<br/>CollectInterestFee,<br/>TreasuryFwInterestAccrued
Tokens->>AMM: Map fixed-rate lending BTs<br/>to AMM addresses
AMM->>AMM: Read AMM tokens via readTokens()
AMM->>AMM: Aggregate Swap event fees<br/>using netFwFee & netFwToReserve
Params->>Liquidation: Use oracle price()<br/>to convert seizedAssets to<br/>loan token value
Liquidation->>Liquidation: Calculate fee as<br/>seizedValueInLoanToken - repaidAssets
Interest->>Aggregation: Add interest revenues
AMM->>Aggregation: Add swap fees
Liquidation->>Aggregation: Add liquidation fees
Aggregation->>Aggregation: Populate dailyFees,<br/>dailyRevenue,<br/>dailySupplySideRevenue
sequenceDiagram
participant Treasury as Treasury Module
participant Dune as Dune Service
participant Balances as ERC-20 Query
participant Aggregation as Fee Aggregation
Treasury->>Balances: Query balanceOf() for<br/>fixed treasury addresses
Balances->>Aggregation: Return aggregated balances<br/>as totalRevenue
Treasury->>Dune: Query Dune (total_revenue ID)
Dune->>Aggregation: Return fee and revenue data
Treasury->>Dune: Query Dune (extra_revenue_usd ID)<br/>if present
Dune->>Aggregation: Return additional revenue data
Aggregation->>Aggregation: Construct dailyFees object
Aggregation->>Aggregation: Apply Dune mappings via<br/>addUSDValue(...)
Aggregation->>Aggregation: Return dailyFees &<br/>totalRevenue
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 4❌ Failed checks (1 warning, 3 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches✨ Simplify code
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 fira.ts adapter exports: |
|
The usual adapter exports: |
![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: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@fees/fira.ts`:
- Around line 263-265: The current calls to dailyFees.add, dailyRevenue.add, and
dailySupplySideRevenue.add are using source-only metric labels (e.g.,
METRIC.SWAP_FEES, METRIC.BORROW_INTEREST) which makes retained vs supplier flows
indistinguishable; update these calls so the metric label encodes both source
and destination (for example use distinct constants like
METRIC.SWAP_FEES_RETAINED and METRIC.SWAP_FEES_SUPPLIER or
METRIC.BORROW_INTEREST_RETAINED/SUPPLIER) and pass the appropriate
destination-specific label when adding values (affecting the lines using
dailyFees.add(feeToken, netFwFee,...), dailyRevenue.add(feeToken,
ammRevenue,...) and dailySupplySideRevenue.add(feeToken,
ammSupplySideRevenue,...) and the analogous calls at 283-285, 299-312, 356-369);
ensure naming follows existing METRIC enum/style and update any downstream
consumers if needed.
- Around line 316-334: The liquidation penalty is only being recorded into
dailyFees (METRIC.LIQUIDATION_FEES) but not routed to the actual recipient
buckets; update the logic around where liquidationFee is recorded so the same
computed liquidationFee is also allocated to dailySupplySideRevenue when lenders
receive it, split between dailySupplySideRevenue and dailyRevenue when the
protocol keeps a cut (use the marketData or fee-related fields on marketData to
determine the split), or omit it from revenue metrics if paid entirely to
external liquidators; ensure after changes the adapter preserves the invariant
dailyRevenue = dailyFees - dailySupplySideRevenue by adding corresponding
dailySupplySideRevenue.add(...) and/or dailyRevenue.add(...) calls alongside
dailyFees.add(...) wherever liquidationFee is processed (referencing variables
marketData, liquidationFee, dailyFees, dailySupplySideRevenue, dailyRevenue, and
METRIC.LIQUIDATION_FEES).
In `@fees/usual/index.ts`:
- Around line 39-55: The current code collapses Dune results into anonymous
dailyFees additions (using resRev/resFee -> dailyFees.addUSDValue) and thus
omits retained-side labels and breakdownMethodology; update the handlers for
queryDune("4957281") and queryDune("4957293") so that each mapped item calls
dailyFees.addUSDValue with an explicit label (e.g., "dailyRevenue" for resRev
and "dailyProtocolRevenue" or "dailyRevenue:extra" for resFee depending on
treasury allocation) and add corresponding entries to the exported
breakdownMethodology/meta for those labels; locate the mapping code around
resRev/resFee and the dailyFees object usage to add labeled adds and ensure
every emitted label has a descriptive breakdownMethodology entry.
- Around line 22-33: fetchTreasuryRevenue currently treats end-of-period token
balances as revenue—remove/replace that behavior: stop aggregating
YIELD_TREASURY balances into fee totals in fetchTreasuryRevenue (and the similar
block at lines 56-60) and either (a) implement proper revenue calculation by
summing actual fee-related events/transfers (e.g., use fee event logs or a
calculateFeesFromEvents function) or (b) convert this function into a treasury
snapshot/TVL helper (rename to fetchTreasurySnapshot or similar) that returns
plain balances without calling it "revenue"; update callers to use the correct
function (fee adapter should call the real revenue calculator, reporting adapter
should call the snapshot) and adjust function name/exports accordingly to avoid
mislabeling stock positions as revenue.
- Around line 39-42: The Dune queries are passing undefined timestamps because
they use options.startTimestamp and options.endTimestamp; change both queryDune
calls (e.g., the call assigned to resRev and any other queryDune usage in
fees/usual/index.ts) to use the correct FetchOptions fields
options.fromTimestamp and options.toTimestamp so the time window is forwarded
properly.
- Around line 63-81: Two modules resolve to the same slug "usual" (the
implementations in usual.ts and usual/index.ts) causing a discovery collision
and one adapter to overwrite the other; remove or rename one of these modules so
only a single "usual" adapter is exported. Keep the more comprehensive
implementation (the one that tracks broader revenue) and delete the narrower one
(or rename its module slug), ensuring only one export named adapter
(SimpleAdapter) remains and that fetchFeesAndRevenue / adapter.version /
adapter.adapter[CHAIN.ETHEREUM] are defined on the retained module. Also verify
buildModules' output no longer contains duplicate keys for "usual".
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e120441a-5097-4ef2-b669-a6955691e2a1
📒 Files selected for processing (2)
fees/fira.tsfees/usual/index.ts
Comment on lines
+263
to
+265
| dailyFees.add(feeToken, netFwFee, METRIC.SWAP_FEES); | ||
| dailyRevenue.add(feeToken, ammRevenue, METRIC.SWAP_FEES); | ||
| dailySupplySideRevenue.add(feeToken, ammSupplySideRevenue, METRIC.SWAP_FEES); |
Contributor
There was a problem hiding this comment.
Use destination-specific labels for retained vs supplier flows.
METRIC.SWAP_FEES and METRIC.BORROW_INTEREST are written into dailyFees, dailyRevenue, and dailySupplySideRevenue. That makes the retained side indistinguishable from the supplier-paid side in downstream breakdowns. The section-level descriptions in breakdownMethodology do not fix that, because the emitted label itself still stays source-only.
As per coding guidelines, dailyRevenue and dailySupplySideRevenue labels must include both the fee source and its destination.
Also applies to: 283-285, 299-312, 356-369
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@fees/fira.ts` around lines 263 - 265, The current calls to dailyFees.add,
dailyRevenue.add, and dailySupplySideRevenue.add are using source-only metric
labels (e.g., METRIC.SWAP_FEES, METRIC.BORROW_INTEREST) which makes retained vs
supplier flows indistinguishable; update these calls so the metric label encodes
both source and destination (for example use distinct constants like
METRIC.SWAP_FEES_RETAINED and METRIC.SWAP_FEES_SUPPLIER or
METRIC.BORROW_INTEREST_RETAINED/SUPPLIER) and pass the appropriate
destination-specific label when adding values (affecting the lines using
dailyFees.add(feeToken, netFwFee,...), dailyRevenue.add(feeToken,
ammRevenue,...) and dailySupplySideRevenue.add(feeToken,
ammSupplySideRevenue,...) and the analogous calls at 283-285, 299-312, 356-369);
ensure naming follows existing METRIC enum/style and update any downstream
consumers if needed.
Comment on lines
+316
to
334
| // Liquidation penalty (when seized collateral value exceeds repaid debt). | ||
| for (let i = 0; i < lendingMarkets.length; i++) { | ||
| const marketAddress = lendingMarkets[i]; | ||
| for (const log of liquidationLogs[i] ?? []) { | ||
| const marketId = (log.id ?? '').toLowerCase(); | ||
| const marketData = marketDataByKey.get(`${marketAddress}:${marketId}`); | ||
| if (!marketData) continue; | ||
|
|
||
| const oraclePrice = oraclePriceByAddress.get(marketData.oracle.toLowerCase()); | ||
| if (!oraclePrice || oraclePrice <= 0n) continue; | ||
|
|
||
| const repaidAssets = BigInt(log.repaidAssets ?? 0); | ||
| const seizedAssets = BigInt(log.seizedAssets ?? 0); | ||
| const seizedValueInLoanToken = (seizedAssets * oraclePrice) / ORACLE_PRICE_SCALE; | ||
| if (seizedValueInLoanToken <= repaidAssets) continue; | ||
|
|
||
| const liquidationFee = seizedValueInLoanToken - repaidAssets; | ||
| dailyFees.add(marketData.loanToken, liquidationFee, METRIC.LIQUIDATION_FEES); | ||
| } |
Contributor
There was a problem hiding this comment.
Route liquidation penalties into the correct destination bucket too.
Line 333 adds METRIC.LIQUIDATION_FEES only to dailyFees. On liquidation days, dailyRevenue + dailySupplySideRevenue will undercount gross fees because this source never gets assigned to whoever actually receives it. Book it into dailySupplySideRevenue if it goes to lenders, split it with dailyRevenue if the protocol keeps a cut, or drop it from fee metrics entirely if it is paid straight to external liquidators.
Based on learnings, fees adapters should keep dailyRevenue = dailyFees - dailySupplySideRevenue conceptually.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@fees/fira.ts` around lines 316 - 334, The liquidation penalty is only being
recorded into dailyFees (METRIC.LIQUIDATION_FEES) but not routed to the actual
recipient buckets; update the logic around where liquidationFee is recorded so
the same computed liquidationFee is also allocated to dailySupplySideRevenue
when lenders receive it, split between dailySupplySideRevenue and dailyRevenue
when the protocol keeps a cut (use the marketData or fee-related fields on
marketData to determine the split), or omit it from revenue metrics if paid
entirely to external liquidators; ensure after changes the adapter preserves the
invariant dailyRevenue = dailyFees - dailySupplySideRevenue by adding
corresponding dailySupplySideRevenue.add(...) and/or dailyRevenue.add(...) calls
alongside dailyFees.add(...) wherever liquidationFee is processed (referencing
variables marketData, liquidationFee, dailyFees, dailySupplySideRevenue,
dailyRevenue, and METRIC.LIQUIDATION_FEES).
Comment on lines
+22
to
+33
| const fetchTreasuryRevenue = async (options: FetchOptions) => { | ||
| const totalRevenue = options.createBalances(); | ||
| const usualXHolding = await getBalanceOf(options, USUALX, YIELD_TREASURY); | ||
| totalRevenue.add(USUALX, usualXHolding); | ||
|
|
||
| const usualHolding = await getBalanceOf(options, USUAL, YIELD_TREASURY); | ||
| totalRevenue.add(USUAL, usualHolding); | ||
|
|
||
| const usd0Holding = await getBalanceOf(options, USD0, YIELD_TREASURY); | ||
| totalRevenue.add(USD0, usd0Holding); | ||
| return totalRevenue; | ||
| }; |
Contributor
There was a problem hiding this comment.
Don't publish treasury balances as revenue.
fetchTreasuryRevenue() is reading end-of-period token balances, which are stock positions. Transfers, redemptions, emissions, and spending can change those balances without any fee event, so returning them as totalRevenue will misstate actual earnings. If you need this snapshot, keep it in a treasury/TVL-style adapter instead of a fee adapter.
Also applies to: 56-60
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@fees/usual/index.ts` around lines 22 - 33, fetchTreasuryRevenue currently
treats end-of-period token balances as revenue—remove/replace that behavior:
stop aggregating YIELD_TREASURY balances into fee totals in fetchTreasuryRevenue
(and the similar block at lines 56-60) and either (a) implement proper revenue
calculation by summing actual fee-related events/transfers (e.g., use fee event
logs or a calculateFeesFromEvents function) or (b) convert this function into a
treasury snapshot/TVL helper (rename to fetchTreasurySnapshot or similar) that
returns plain balances without calling it "revenue"; update callers to use the
correct function (fee adapter should call the real revenue calculator, reporting
adapter should call the snapshot) and adjust function name/exports accordingly
to avoid mislabeling stock positions as revenue.
Comment on lines
+39
to
+55
| const resRev = await queryDune("4957281", { | ||
| start: options.startTimestamp, | ||
| end: options.endTimestamp, | ||
| }); | ||
| resRev.map((item: any) => { | ||
| dailyFees.addUSDValue(item?.total_revenue); | ||
| }); | ||
| // USUAL Extra Collateral Revenue | ||
| const resFee = await queryDune("4957293", { | ||
| start: options.startTimestamp, | ||
| end: options.endTimestamp, | ||
| }); | ||
| resFee.map((item: any) => { | ||
| if (item?.extra_revenue_usd) { | ||
| dailyFees.addUSDValue(item?.extra_revenue_usd); | ||
| } | ||
| }); |
Contributor
There was a problem hiding this comment.
Return labeled retained-side dimensions for these Dune sources.
These two Dune sources are collapsed into anonymous dailyFees additions. That drops the retained-side dimension (dailyRevenue, and dailyProtocolRevenue if treasury-allocated) and leaves no emitted labels for a breakdownMethodology to document. The generic meta.methodology text is not a substitute.
As per coding guidelines, fees/**/*.ts must include dailyRevenue, always provide descriptive labels even for a single source, and include a breakdownMethodology entry for every emitted label.
Also applies to: 57-60, 70-74
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@fees/usual/index.ts` around lines 39 - 55, The current code collapses Dune
results into anonymous dailyFees additions (using resRev/resFee ->
dailyFees.addUSDValue) and thus omits retained-side labels and
breakdownMethodology; update the handlers for queryDune("4957281") and
queryDune("4957293") so that each mapped item calls dailyFees.addUSDValue with
an explicit label (e.g., "dailyRevenue" for resRev and "dailyProtocolRevenue" or
"dailyRevenue:extra" for resFee depending on treasury allocation) and add
corresponding entries to the exported breakdownMethodology/meta for those
labels; locate the mapping code around resRev/resFee and the dailyFees object
usage to add labeled adds and ensure every emitted label has a descriptive
breakdownMethodology entry.
Comment on lines
+39
to
+42
| const resRev = await queryDune("4957281", { | ||
| start: options.startTimestamp, | ||
| end: options.endTimestamp, | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== FetchOptions timestamp fields =="
rg -n -C2 --type=ts 'type FetchOptions|fromTimestamp|toTimestamp|startTimestamp|endTimestamp' adapters/types.ts fees/usual/index.ts
echo
echo "== Timestamp fields used by other v2 adapters =="
( rg -n -C1 --type=ts '\b(fromTimestamp|toTimestamp)\b' fees adapters | head -n 80 ) || trueRepository: DefiLlama/dimension-adapters
Length of output: 6986
Fix timestamp field names in Dune queries.
The code uses options.startTimestamp and options.endTimestamp, but FetchOptions defines fromTimestamp and toTimestamp. Both Dune queries will receive undefined values for the time window.
Required changes
const resRev = await queryDune("4957281", {
- start: options.startTimestamp,
- end: options.endTimestamp,
+ start: options.fromTimestamp,
+ end: options.toTimestamp,
}); const resFee = await queryDune("4957293", {
- start: options.startTimestamp,
- end: options.endTimestamp,
+ start: options.fromTimestamp,
+ end: options.toTimestamp,
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@fees/usual/index.ts` around lines 39 - 42, The Dune queries are passing
undefined timestamps because they use options.startTimestamp and
options.endTimestamp; change both queryDune calls (e.g., the call assigned to
resRev and any other queryDune usage in fees/usual/index.ts) to use the correct
FetchOptions fields options.fromTimestamp and options.toTimestamp so the time
window is forwarded properly.
Comment on lines
+63
to
+81
| const adapter: SimpleAdapter = { | ||
| version: 2, | ||
| adapter: { | ||
| [CHAIN.ETHEREUM]: { | ||
| fetch: fetchFeesAndRevenue, | ||
| start: 1716163200, | ||
| runAtCurrTime: false, | ||
| meta: { | ||
| methodology: { | ||
| Fees: "RWA on the treasury generates revenue. User pays fees when redeeming USD0, unlocking USD0++ or exiting vaults", | ||
| Revenue: "DAO holds Usual treasury and earns yield on it.", | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| isExpensiveAdapter: true, | ||
| }; | ||
|
|
||
| export default adapter; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Usual adapter entrypoints =="
for f in fees/usual.ts fees/usual/index.ts; do
if [ -f "$f" ]; then
echo "--- $f ---"
sed -n '1,120p' "$f"
fi
done
echo
echo "== Module discovery implementation =="
build_modules="$(fd -a 'buildModules\.ts$' . | head -n 1 || true)"
if [ -n "$build_modules" ]; then
echo "--- $build_modules ---"
sed -n '1,260p' "$build_modules"
else
echo "buildModules.ts not found; searching for adapter file enumeration:"
rg -n -C3 --type=ts 'readdirSync|glob|globby|walk|fees/' .
fiRepository: DefiLlama/dimension-adapters
Length of output: 15314
Both adapters resolve to the same module slug due to module discovery collision—delete or rename one before merging.
fees/usual.ts and fees/usual/index.ts both map to the slug usual in module discovery. The buildModules.ts code processes each directory entry and stores the result at dimensionsImports[adapterType][fileKey]. Since removeDotTs("usual.ts") and importing fees/usual (which resolves to fees/usual/index.ts) both result in fileKey="usual", whichever is processed second overwrites the first. The processing order from readdir() is not guaranteed, so either adapter could silently stop contributing data.
fees/usual.ts provides broader revenue tracking; fees/usual/index.ts focuses narrowly on Yield Treasury holdings. Keep the more comprehensive implementation and remove the other.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@fees/usual/index.ts` around lines 63 - 81, Two modules resolve to the same
slug "usual" (the implementations in usual.ts and usual/index.ts) causing a
discovery collision and one adapter to overwrite the other; remove or rename one
of these modules so only a single "usual" adapter is exported. Keep the more
comprehensive implementation (the one that tracks broader revenue) and delete
the narrower one (or rename its module slug), ensuring only one export named
adapter (SimpleAdapter) remains and that fetchFeesAndRevenue / adapter.version /
adapter.adapter[CHAIN.ETHEREUM] are defined on the retained module. Also verify
buildModules' output no longer contains duplicate keys for "usual".
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.
add fees and revenue for fira v1