Skip to content

accountable: floor NAV depositors leg at zero, fixes negative fees - #8790

Open
pawel-accountable wants to merge 1 commit into
DefiLlama:masterfrom
pawel-accountable:fix/accountable-nav-negative-fees
Open

accountable: floor NAV depositors leg at zero, fixes negative fees#8790
pawel-accountable wants to merge 1 commit into
DefiLlama:masterfrom
pawel-accountable:fix/accountable-nav-negative-fees

Conversation

@pawel-accountable

Copy link
Copy Markdown
Contributor

Summary

  • For NAV/looping/fixed-term vaults, dailyFees/dailyRevenue were computed from gross = (closeAssets - openAssets) + mintedAssets, with no floor. A NAV write-down (or any day the on-chain share value dips between the two read blocks) pushed gross negative, and the performance-fee split (fee = gross * performanceFee / DENOM) went negative with it, so a legitimate vault loss surfaced as negative revenue rather than being absorbed.
  • Also affected: a write-down landing on the same day as a real fee-share mint silently netted the two together, understating a genuine crystallization instead of reporting it in full.
  • Floored the depositors' share-price leg at zero (bigMax(0n, closeAssets - openAssets)) before adding the minted-shares leg, so gross is only ever the real crystallized/accrued value, never negative.

Test plan

  • Could not exercise this against live chain state end-to-end: the affected chain (Robinhood) is only reachable through a public RPC that prunes state after ~2h, so no day-boundary window is queryable locally.
  • Verified the changed arithmetic in isolation with a standalone script covering 4 scenarios (normal appreciating day, pure write-down, write-down + same-day mint, flat day): confirms dailyFees/dailyRevenue/the depositors leg are never negative after the fix, non-write-down days are byte-for-byte unchanged, and the write-down+mint case now reports the full minted amount instead of a netted-down figure.
  • tsc --project tsconfig.json --noEmit: clean.

@coderabbitai

coderabbitai Bot commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 7e6ba6cf-4fb0-42eb-957b-2788d8140b0f

📥 Commits

Reviewing files that changed from the base of the PR and between a09b9a5 and 01fecf3.

📒 Files selected for processing (1)
  • fees/accountable.ts

Summary by CodeRabbit

  • Bug Fixes
    • Prevented negative gross fees when priced-vault share prices decline.
    • Ensured depositor yield calculations do not offset minted-share yield during NAV losses.

Walkthrough

Changes

Priced-vault yield calculation

Layer / File(s) Summary
Floor depositor appreciation
fees/accountable.ts
Adds bigMax and floors priced-vault depositor appreciation at zero before calculating gross yield.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Merge Risk: ⚪ Minimal · up to 01fec

This localized change floors the depositor share-price leg at zero so losses cannot produce negative fees while preserving normal-day calculations; no actionable merge-blocking risk remains after normal checks and review.

Suggested labels: fees, methodology, bug-fix

Suggested reviewers: bheluga

🚥 Pre-merge checks | ✅ 14 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title describes the code change but does not follow the required [type] protocol-name - description format. Rewrite the title using the required format, for example [fix] accountable - floor NAV depositor leg at zero.
✅ Passed checks (14 passed)
Check name Status Explanation
Description check ✅ Passed The description clearly explains the change, scope, validation, and end-to-end testing limitation for this non-listing PR.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Breakdown Methodology Check ✅ Passed fees/accountable.ts maps all .add() labels to breakdownMethodology entries: two METRIC keys, DEPOSITORS, MANAGER, and PROTOCOL; adapter exports breakdownMethodology.
Income Statement Compliance ✅ Passed The change floors priced-vault gross at zero; book() assigns protocol revenue and supplier costs so dailyRevenue remains dailyFees minus dailySupplySideRevenue.
Version 2 Required ✅ Passed The PR updates existing fees/accountable.ts, and both parent and current exports declare version: 2; the diff changes only arithmetic, not adapter versioning.
Pullhourly Required For Version 2 ✅ Passed The changed adapter, fees/accountable.ts, declares version: 2 and pullHourly: true; the metadata is unchanged from the parent commit.
Dune Adapters Are Version 1 ✅ Passed The PR changes only fees/accountable.ts, which has no Dune query or helper import; its version: 2 and pullHourly: true are unchanged and unrelated.
Income Statement Balance ✅ Passed The diff only floors priced-vault gross at zero. book() records fees as residual supply-side plus protocol revenue, so dailyFees = dailyRevenue + dailySupplySideRevenue remains exact.
Fetchoptions Usage ✅ Passed The only changed adapter, fees/accountable.ts, uses fetch(options: FetchOptions), uses startTimestamp/endTimestamp, uses getLogs, and returns no timestamp field.
Adapter Shape ✅ Passed The diff changes only fee arithmetic; the existing adapter passes one per-chain config object with factories and starts, and fetch reads config[api.chain].
Efficiency And Error Handling ✅ Passed The diff only adds bigMax and floors the depositor leg; the change uses no getLogs, raw topics, single-call loops, swallowing catch, or allowNegativeValue, and its 0n floor has a source comment.
Methodology Keys ✅ Passed The methodology object uses Fees, Revenue, ProtocolRevenue, and SupplySideRevenue. The diff only changes fee arithmetic and does not alter methodology keys.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
✨ Simplify code
  • Create PR with simplified 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown

The accountable.ts adapter exports:

> adapters@1.0.0 test
> ts-node --transpile-only cli/testAdapter.ts fees accountable.ts

🦙 Running ACCOUNTABLE.TS adapter 🦙
---------------------------------------------------
Start Date:	Fri, 14 Aug 2026 19:00:00 GMT
End Date:	Sat, 15 Aug 2026 19:00:00 GMT
---------------------------------------------------

------ ERROR ------
Error: Accountable: every factory call failed on robinhood, refusing to report zero
    at listStrategies (/home/runner/work/dimension-adapters/dimension-adapters/fees/accountable.ts:121:11)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
Accountable: every factory call failed on robinhood, refusing to report zero

@bheluga bheluga left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pawel-accountable thanks for the PR
Ideally we shouldn't be capping losses, or else it inflates cumulative fees.

@bheluga bheluga self-assigned this Aug 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants