Add treasury composition, DAO NFT holdings, and revenue analytics to the Treasury tab#981
Add treasury composition, DAO NFT holdings, and revenue analytics to the Treasury tab#981sktbrd wants to merge 1 commit into
Conversation
…the Treasury tab Milestone 3 (Gnars Prop 61) — Treasury Analytics, Batch 1. - TreasuryComposition: allocation donut + curated asset rows (ETH plus a global per-chain token registry and the DAO's clanker token) valued in USD via on-chain multicall. Scales to every DAO with no per-DAO config and no Alchemy enumerate-all (which 500s on large treasuries). - TreasuryNfts: the DAO's own NFTs held in the treasury, read natively from the Builder subgraph (tokensQuery by owner) — no Alchemy, no spam heuristics. - TreasuryAnalytics: cumulative auction-revenue chart + aggregates (revenue, auctions, average winning bid excluding no-bid burns, highest sale), paged so high-volume DAOs aren't undercounted. Custom SVG, no chart library. - Real token brand logos embedded as base64 (Trust Wallet assets). - Replaces the Alchemy-dependent TokenBalance/NFTBalance sections with key-free, subgraph/RPC-backed equivalents. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@sktbrd is attempting to deploy a commit to the Nouns Builder Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe DAO Treasury tab now renders curated asset composition, subgraph-backed NFT holdings, and selectable auction-revenue analytics. New helpers, responsive styles, embedded token logos, tests, and public exports support the replacement Treasury experience. ChangesTreasury asset composition
Auction revenue analytics
NFT holdings and integration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant TreasuryAnalytics
participant SWR
participant AuctionAPI
TreasuryAnalytics->>SWR: request auction data for selected window
SWR->>AuctionAPI: paginate using startTime cursor
AuctionAPI-->>SWR: return auction batches
SWR-->>TreasuryAnalytics: provide deduplicated history
TreasuryAnalytics->>TreasuryAnalytics: calculate metrics and render chart
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/dao-ui/src/components/Treasury/Treasury.tsxESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. packages/dao-ui/src/components/Treasury/TreasuryAnalytics.css.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. packages/dao-ui/src/components/Treasury/TreasuryAnalytics.tsxESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox.
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
packages/dao-ui/src/components/Treasury/TreasuryComposition.tsx (1)
88-101: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winEnsure unique keys and prevent redundant on-chain calls if the subgraph returns duplicates.
If the subgraph query (
clankerTokens) returns multiple entries for the same token address, the current filter will allow duplicates becauseseen.has()is not updated within the loop. This can lead to redundantbalanceOfmulticall queries and trigger React's "two children with the same key" warning in the UI.Update the
seenset during filtering to guarantee a strictly unique token list.♻️ Proposed refactor
const tokenList = useMemo<RegistryToken[]>(() => { const common = COMMON_TREASURY_TOKENS[chain.id] ?? [] const seen = new Set(common.map((t) => t.address.toLowerCase())) const clankers: RegistryToken[] = (clankerTokens ?? []) - .filter((c) => c.tokenAddress && !seen.has(c.tokenAddress.toLowerCase())) + .filter((c) => { + if (!c.tokenAddress) return false + const lower = c.tokenAddress.toLowerCase() + if (seen.has(lower)) return false + seen.add(lower) + return true + }) .map((c) => ({ symbol: c.tokenSymbol || 'TOKEN', address: c.tokenAddress.toLowerCase() as `0x${string}`, decimals: 18, kind: 'other' as const, logo: c.tokenImage || undefined, })) return [...common, ...clankers] }, [chain.id, clankerTokens])🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dao-ui/src/components/Treasury/TreasuryComposition.tsx` around lines 88 - 101, Update the tokenList useMemo’s clankerTokens filtering to add each accepted token address to the seen set as it is processed, not only check membership. Preserve the existing common-token deduplication and mapping behavior so the returned list contains one entry per address and avoids duplicate balance calls and React keys.packages/dao-ui/src/components/Treasury/TreasuryAnalytics.tsx (2)
152-164: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winWindow-tab buttons don't expose selected state to assistive tech.
Selection is conveyed only via
windowTab.selected/unselectedborder styling; there's noaria-pressed(or similar) so screen-reader users can't tell which window is active.♻️ Suggested fix
<Button key={w} variant={'ghost'} size={'sm'} px={'x2'} + aria-pressed={w === window} className={w === window ? windowTab.selected : windowTab.unselected} onClick={() => setWindow(w)} >🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dao-ui/src/components/Treasury/TreasuryAnalytics.tsx` around lines 152 - 164, Update the RevenueWindow buttons rendered in TreasuryAnalytics to expose their active selection state through an appropriate ARIA attribute such as aria-pressed, setting it true only when w equals window and false otherwise. Preserve the existing styling and window-switching behavior.
53-53: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueHardcoded SVG gradient
idcan collide across multiple instances.
GRADIENT_ID = 'treasuryRevGradient'is a fixed string used as the<linearGradient id>. If this component is ever rendered more than once on the same page (e.g. a multi-DAO dashboard), the duplicate SVG ids would collide, and browsers resolveurl(#id)references to the first matching element in the DOM.♻️ Suggested fix (React 19 `useId`)
+import { useId } from 'react' ... -const GRADIENT_ID = 'treasuryRevGradient' ... + const gradientId = useId()Then reference
gradientIdinstead of the module-level constant.Also applies to: 184-189
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dao-ui/src/components/Treasury/TreasuryAnalytics.tsx` at line 53, Replace the module-level GRADIENT_ID constant in TreasuryAnalytics with a per-instance React useId value, and use that gradientId for both the linearGradient id and all corresponding url(#...) references. Ensure multiple TreasuryAnalytics instances produce distinct SVG gradient identifiers.packages/dao-ui/src/components/Treasury/treasuryAnalytics.helper.ts (1)
57-57: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valuePrefer
reduceover spreading intoMath.max/Math.minfor large arrays.
Math.max(...amounts)andMath.min/max(...xs)spread the full auction array into function arguments. WithMAX_PAGES=25andPAGE=1000in the caller, this can spread up to ~25,000 elements — currently within engine limits, but fragile if the caller's pagination bounds ever grow.♻️ Suggested refactor
- highestSale: Math.max(...amounts), + highestSale: amounts.reduce((max, v) => (v > max ? v : max), 0),Also applies to: 90-92
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/dao-ui/src/components/Treasury/treasuryAnalytics.helper.ts` at line 57, Replace the spread-based Math.max/Math.min calculations in the treasury analytics helper, including highestSale and the related calculations around lines 90–92, with reduce-based aggregation over amounts. Preserve the existing results and behavior for the full auction array without expanding its elements into function arguments.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/dao-ui/src/components/Treasury/TreasuryAnalytics.tsx`:
- Line 84: Rename the state tuple in TreasuryAnalytics from window/setWindow to
selectedWindow/setSelectedWindow, and update every subsequent reference in the
component, including the usages around the analytics rendering logic, while
preserving the existing RevenueWindow state behavior.
- Around line 86-96: Update the useSWR handling in
packages/dao-ui/src/components/Treasury/TreasuryAnalytics.tsx#L86-L96 to
destructure error and render a distinct error state before the empty-data
message. Apply the same change in
packages/dao-ui/src/components/Treasury/TreasuryNfts.tsx#L24-L28, ensuring
subgraph or network failures are not presented as legitimate empty states.
- Around line 106-109: Update the paged axios.get call inside the MAX_PAGES loop
to include a finite request timeout in its configuration, ensuring stalled
auction-history requests fail and allow the existing flow to recover instead of
blocking indefinitely.
---
Nitpick comments:
In `@packages/dao-ui/src/components/Treasury/treasuryAnalytics.helper.ts`:
- Line 57: Replace the spread-based Math.max/Math.min calculations in the
treasury analytics helper, including highestSale and the related calculations
around lines 90–92, with reduce-based aggregation over amounts. Preserve the
existing results and behavior for the full auction array without expanding its
elements into function arguments.
In `@packages/dao-ui/src/components/Treasury/TreasuryAnalytics.tsx`:
- Around line 152-164: Update the RevenueWindow buttons rendered in
TreasuryAnalytics to expose their active selection state through an appropriate
ARIA attribute such as aria-pressed, setting it true only when w equals window
and false otherwise. Preserve the existing styling and window-switching
behavior.
- Line 53: Replace the module-level GRADIENT_ID constant in TreasuryAnalytics
with a per-instance React useId value, and use that gradientId for both the
linearGradient id and all corresponding url(#...) references. Ensure multiple
TreasuryAnalytics instances produce distinct SVG gradient identifiers.
In `@packages/dao-ui/src/components/Treasury/TreasuryComposition.tsx`:
- Around line 88-101: Update the tokenList useMemo’s clankerTokens filtering to
add each accepted token address to the seen set as it is processed, not only
check membership. Preserve the existing common-token deduplication and mapping
behavior so the returned list contains one entry per address and avoids
duplicate balance calls and React keys.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 234ede7f-1402-44dc-ab59-07c860045a1c
📒 Files selected for processing (17)
.changeset/treasury-composition.mdpackages/dao-ui/src/components/Treasury/NFTBalance.tsxpackages/dao-ui/src/components/Treasury/TokenBalance.tsxpackages/dao-ui/src/components/Treasury/Treasury.tsxpackages/dao-ui/src/components/Treasury/TreasuryAnalytics.css.tspackages/dao-ui/src/components/Treasury/TreasuryAnalytics.tsxpackages/dao-ui/src/components/Treasury/TreasuryComposition.css.tspackages/dao-ui/src/components/Treasury/TreasuryComposition.tsxpackages/dao-ui/src/components/Treasury/TreasuryNfts.css.tspackages/dao-ui/src/components/Treasury/TreasuryNfts.tsxpackages/dao-ui/src/components/Treasury/index.tsxpackages/dao-ui/src/components/Treasury/tokenLogos.tspackages/dao-ui/src/components/Treasury/treasuryAnalytics.helper.test.tspackages/dao-ui/src/components/Treasury/treasuryAnalytics.helper.tspackages/dao-ui/src/components/Treasury/treasuryComposition.helper.test.tspackages/dao-ui/src/components/Treasury/treasuryComposition.helper.tspackages/dao-ui/src/components/Treasury/treasuryTokens.ts
💤 Files with no reviewable changes (2)
- packages/dao-ui/src/components/Treasury/NFTBalance.tsx
- packages/dao-ui/src/components/Treasury/TokenBalance.tsx
| addresses: { token }, | ||
| } = useDaoStore() | ||
|
|
||
| const [window, setWindow] = useState(RevenueWindow['All']) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Rename window state — it shadows the global window object.
const [window, setWindow] = useState(...) shadows globalThis.window for the rest of the component's scope. Not exploited today, but any future code in this component that expects lexical access to the browser window (e.g. window.matchMedia, window.innerWidth) will silently resolve to this state variable instead.
♻️ Suggested fix
- const [window, setWindow] = useState(RevenueWindow['All'])
+ const [selectedWindow, setSelectedWindow] = useState(RevenueWindow['All'])And update all subsequent usages (window → selectedWindow, setWindow → setSelectedWindow).
Also applies to: 152-164
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/dao-ui/src/components/Treasury/TreasuryAnalytics.tsx` at line 84,
Rename the state tuple in TreasuryAnalytics from window/setWindow to
selectedWindow/setSelectedWindow, and update every subsequent reference in the
component, including the usages around the analytics rendering logic, while
preserving the existing RevenueWindow state behavior.
| const { data, isValidating } = useSWR( | ||
| token && chain.id | ||
| ? ([ | ||
| SWR_KEYS.AUCTION_HISTORY, | ||
| 'treasury-analytics', | ||
| token, | ||
| chain.id, | ||
| window, | ||
| ] as const) | ||
| : null, | ||
| async ([, , _token, _chainId, _window]) => { |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Both new SWR-backed components swallow fetch errors, making failures indistinguishable from legitimate empty states. Neither useSWR call destructures error, so a network/subgraph failure renders the same UI as "no data" — silently misleading users.
packages/dao-ui/src/components/Treasury/TreasuryAnalytics.tsx#L86-L96: destructureerrorfromuseSWRand render a distinct error state instead of falling through to "Not enough settled auctions in this window to chart revenue." when a fetch actually failed.packages/dao-ui/src/components/Treasury/TreasuryNfts.tsx#L24-L28: destructureerrorfromuseSWRand render a distinct error state instead of falling through to "No DAO NFTs held in the treasury." when the subgraph query actually failed.
📍 Affects 2 files
packages/dao-ui/src/components/Treasury/TreasuryAnalytics.tsx#L86-L96(this comment)packages/dao-ui/src/components/Treasury/TreasuryNfts.tsx#L24-L28
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/dao-ui/src/components/Treasury/TreasuryAnalytics.tsx` around lines
86 - 96, Update the useSWR handling in
packages/dao-ui/src/components/Treasury/TreasuryAnalytics.tsx#L86-L96 to
destructure error and render a distinct error state before the empty-data
message. Apply the same change in
packages/dao-ui/src/components/Treasury/TreasuryNfts.tsx#L24-L28, ensuring
subgraph or network failures are not presented as legitimate empty states.
| for (let page = 0; page < MAX_PAGES; page++) { | ||
| const { data } = await axios.get<{ auctionHistory: AuctionHistoryQuery }>( | ||
| `${BASE_URL}/api/auctionHistory/${_token}?chainId=${_chainId}&startTime=${cursor}` | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Add a request timeout to the paged axios.get call.
Each page of the pagination loop awaits axios.get with no timeout. A single stalled request blocks the whole loop indefinitely, leaving the chart on its skeleton state with no recovery path.
🔧 Suggested fix
- const { data } = await axios.get<{ auctionHistory: AuctionHistoryQuery }>(
- `${BASE_URL}/api/auctionHistory/${_token}?chainId=${_chainId}&startTime=${cursor}`
- )
+ const { data } = await axios.get<{ auctionHistory: AuctionHistoryQuery }>(
+ `${BASE_URL}/api/auctionHistory/${_token}?chainId=${_chainId}&startTime=${cursor}`,
+ { timeout: 15_000 }
+ )📝 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 (let page = 0; page < MAX_PAGES; page++) { | |
| const { data } = await axios.get<{ auctionHistory: AuctionHistoryQuery }>( | |
| `${BASE_URL}/api/auctionHistory/${_token}?chainId=${_chainId}&startTime=${cursor}` | |
| ) | |
| for (let page = 0; page < MAX_PAGES; page++) { | |
| const { data } = await axios.get<{ auctionHistory: AuctionHistoryQuery }>( | |
| `${BASE_URL}/api/auctionHistory/${_token}?chainId=${_chainId}&startTime=${cursor}`, | |
| { timeout: 15_000 } | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/dao-ui/src/components/Treasury/TreasuryAnalytics.tsx` around lines
106 - 109, Update the paged axios.get call inside the MAX_PAGES loop to include
a finite request timeout in its configuration, ensuring stalled auction-history
requests fail and allow the existing flow to recover instead of blocking
indefinitely.
1 participant
Description
Adds a Treasury Analytics view to the DAO Treasury tab, replacing the Alchemy-dependent token/NFT balance sections with subgraph/RPC-backed equivalents that scale to every DAO with no API key:
useReadContractsmulticallbalanceOf(treasury).tokensQueryby owner).Motivation & context
The existing
TokenBalance/NFTBalancesections use Alchemy's enumerate-all endpoints, which (a) require an API key, (b) return HTTP 500 on treasuries holding many tokens/NFTs (per-item metadata/price enrichment fails as one batch), and (c) spam-flag a DAO's own NFTs. This replaces them with:Part of Gnars DAO Proposal 61, Milestone 3 (Treasury Analytics).
Code review
useEthUsdPrice), other tokens balance-only.keepPreviousDataon the multicall keeps rows stable across the app's 5s refetch interval (imported the same waypackages/hooksalready imports@tanstack/react-query, via wagmi's peer — no new dependency).treasuryComposition.helper,treasuryAnalytics.helper).TokenBalance/NFTBalancecomponents (not exported anywhere).Type of change
Checklist
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation