feat: implement safe ingestion for treasury data#5
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
More reviews will be available in 33 minutes and 40 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThis PR implements a complete treasury balance snapshot system with database caching, live on-chain syncing via Gnosis RPC and CoinGecko pricing, gated API endpoints to fetch and refresh snapshots, and a new interactive dashboard component displayed on the member homepage with sync status indicators. ChangesTreasury Balance Caching & Syncing
Sequence Diagram(s)sequenceDiagram
participant MemberHome as Member Homepage
participant Dashboard as TreasuryDashboard
participant SyncAPI as POST /api/treasury/sync
participant SnapAPI as GET /api/treasury/snapshot
participant SyncService as syncTreasuryBalanceSnapshot()
participant RPC as Gnosis RPC
participant CoinGecko
participant Database
MemberHome->>Dashboard: render with initialSnapshot
Dashboard->>Dashboard: check isStale or missing syncedAt
alt should refresh
Dashboard->>SyncAPI: POST /api/treasury/sync
SyncAPI->>SyncService: call sync function
SyncService->>RPC: fetch live balances
SyncService->>CoinGecko: fetch wETH price
RPC-->>SyncService: raw amounts
CoinGecko-->>SyncService: USD price
SyncService->>Database: persist snapshot + assets
Database-->>SyncService: snapshot created
SyncService-->>SyncAPI: return synced snapshot
SyncAPI-->>Dashboard: success response
Dashboard->>Dashboard: update state, set justUpdated
else sync fails
Dashboard->>SnapAPI: GET /api/treasury/snapshot (fallback)
SnapAPI->>Database: fetch latest cached
Database-->>SnapAPI: cached snapshot
SnapAPI-->>Dashboard: fallback snapshot
Dashboard->>Dashboard: set error message
end
Dashboard->>Dashboard: render balance + assets + sync status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Pull request overview
Implements authenticated treasury balance snapshot ingestion and display, backed by new Drizzle/Postgres tables and refreshed from Gnosis RPC/CoinGecko.
Changes:
- Adds treasury snapshot/asset schema, migration metadata, and live balance sync/cache logic.
- Introduces authenticated treasury snapshot/sync API routes and a client
TreasuryDashboard. - Adds login analytics tracking and updates project/env documentation for treasury refresh behavior.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/lib/treasury/types.ts |
Expands snapshot/asset types for cached sync state and raw balances. |
src/lib/treasury/balances.ts |
Implements cache reads, live Gnosis balance fetching, CoinGecko pricing, and snapshot persistence. |
src/db/schema.ts |
Adds treasury snapshot status enum and snapshot/asset tables. |
src/components/treasury/treasury-dashboard.tsx |
Adds client dashboard with stale-data refresh behavior and asset breakdown UI. |
src/components/treasury/sync-status-badge.tsx |
Updates sync status badge for all treasury sync states. |
src/app/page.tsx |
Replaces inline treasury dashboard markup with the new component. |
src/app/api/treasury/sync/route.ts |
Adds authenticated POST endpoint for refreshing stale treasury snapshots. |
src/app/api/treasury/snapshot/route.ts |
Adds authenticated GET endpoint for reading the cached treasury snapshot. |
src/app/api/auth/verify/route.ts |
Tracks successful member login analytics with primary role. |
PROJECT_SPEC.md |
Documents treasury refresh and data source behavior. |
drizzle/0002_lovely_colossus.sql |
Adds database migration for treasury snapshot storage. |
drizzle/meta/0002_snapshot.json |
Adds Drizzle schema snapshot metadata for the new migration. |
drizzle/meta/_journal.json |
Registers the new Drizzle migration. |
.env.example |
Notes that the CoinGecko API key is optional. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/page.tsx (1)
122-124:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winServer snapshot fetch is unguarded.
Unlike
getSessionState()(Lines 11-21),getTreasuryBalanceSnapshot()here has no error handling. If it throws (DB/network), the whole authenticated page errors out — even thoughTreasuryDashboardis designed to handlefailed/stale states client-side. Consider catching and passing a degraded snapshot (or relying on an error boundary) so the dashboard can recover via its refresh flow.🤖 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 `@src/app/page.tsx` around lines 122 - 124, The server fetch of getTreasuryBalanceSnapshot is unguarded and can throw, so wrap the await in a try/catch in page.tsx: call getTreasuryBalanceSnapshot() inside try and on error log the error and return a degraded snapshot object (e.g., { status: 'failed', lastKnown: null } or whatever shape TreasuryDashboard expects) to MemberHome; keep getSessionState usage unchanged and ensure MemberHome/TreasuryDashboard receive the failure-indicating snapshot so the client-side refresh/recovery flow can run.
🧹 Nitpick comments (4)
src/lib/treasury/balances.ts (2)
284-284: 💤 Low valueSimplify ABI selection.
The conditional ABI selection between
WXDAI_ABIanderc20Abiis unnecessary complexity—both ABIs expose the samebalanceOf(address)signature. Consider usingerc20Abiuniformly for all ERC-20 tokens.♻️ Proposed simplification
return client.readContract({ - abi: asset.symbol === "wxDAI" ? WXDAI_ABI : erc20Abi, + abi: erc20Abi, address: asset.tokenAddress, functionName: "balanceOf", args: [address],You can also remove the
WXDAI_ABIconstant (line 30) if it's no longer needed.🤖 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 `@src/lib/treasury/balances.ts` at line 284, The conditional ABI choice is unnecessary: replace the ternary that picks WXDAI_ABI vs erc20Abi (uses asset.symbol === "wxDAI") to always use erc20Abi when constructing the contract (the code around abi: ... in the function that reads balances), and remove the now-unused WXDAI_ABI constant (referenced as WXDAI_ABI) to simplify the module; ensure calls relying on balanceOf(address) continue to use the same erc20Abi.
263-265: 💤 Low valueConsider validating that wETH price is positive.
The current validation checks whether the price is falsy or non-finite, but a price of
0or a negative value would pass. Consider adding a check forprice <= 0to catch invalid pricing data.🛡️ Proposed validation enhancement
- if (!price || !Number.isFinite(price)) { + if (!price || !Number.isFinite(price) || price <= 0) { throw new Error("CoinGecko wETH price unavailable"); }🤖 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 `@src/lib/treasury/balances.ts` around lines 263 - 265, The current check around the CoinGecko wETH price (the `price` variable that currently triggers `throw new Error("CoinGecko wETH price unavailable")`) should also guard against non-positive values; update the validation condition (where `if (!price || !Number.isFinite(price)) { ... }`) to include `price <= 0` so zero or negative prices are rejected, and adjust the thrown error message to reflect invalid/non-positive pricing data for easier debugging.src/components/treasury/treasury-dashboard.tsx (1)
102-118: 💤 Low valueRe-check
isMountedafter the fallback fetch resolves.The
isMountedguard is checked before the fallbackGET(Line 98) but not after it resolves, sosetSnapshoton Lines 105 and 114 can run after unmount. Also, a network error on this fallback fetch is uncaught and surfaces as an unhandled rejection. Both are low-impact but easy to tighten.🤖 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 `@src/components/treasury/treasury-dashboard.tsx` around lines 102 - 118, The fallback GET to fetch("/api/treasury/snapshot") can resolve after the component unmounts and its network errors are uncaught; re-check the isMounted guard after the await and wrap the fetch/json in a try/catch: after awaiting response and before calling setSnapshot (both the error branch and the success branch where nextSnapshot is set), verify isMounted() and only then call setSnapshot, and catch any thrown errors from fetch()/response.json() to setSnapshot an appropriate failure state (again only if isMounted()) or swallow/log the error to avoid unhandled rejections.drizzle/0002_lovely_colossus.sql (1)
31-32: Consider a composite index for the latest-snapshot lookup.If
getTreasuryBalanceSnapshot()fetches the most recent snapshot for an account (filter bychain_id/account_address, order bysynced_at desc), the two separate indexes can't both be used optimally. A composite index on(chain_id, account_address, synced_at desc)would serve that access pattern directly.🤖 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 `@drizzle/0002_lovely_colossus.sql` around lines 31 - 32, The current separate indexes on "treasury_balance_snapshots" ("treasury_balance_snapshots_account_idx" for (chain_id, account_address) and "treasury_balance_snapshots_synced_at_idx" for (synced_at)) are suboptimal for the pattern used by getTreasuryBalanceSnapshot() (WHERE chain_id, account_address ORDER BY synced_at DESC); add a composite index that matches that access pattern, e.g. create an index on (chain_id, account_address, synced_at DESC) (name it clearly, e.g. treasury_balance_snapshots_chain_account_synced_idx) so the query can use an index-only scan; optionally remove or keep the existing single-column indexes after verifying no other queries rely on them.
🤖 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 `@src/app/api/treasury/sync/route.ts`:
- Around line 52-65: The catch block must guard the fallback read so failures in
getTreasuryBalanceSnapshot() don't escape; inside the existing catch for the
main error, call getTreasuryBalanceSnapshot() inside its own try/catch (or
assign a safe default snapshot on failure) and then always return
NextResponse.json({ ...snapshot, errorMessage: getPublicSyncError(error),
status: "failed" }, { status: 502 }); ensure you still use
getPublicSyncError(error) for the original error and that any error thrown by
getTreasuryBalanceSnapshot() is swallowed/translated to the safe snapshot rather
than propagating.
---
Outside diff comments:
In `@src/app/page.tsx`:
- Around line 122-124: The server fetch of getTreasuryBalanceSnapshot is
unguarded and can throw, so wrap the await in a try/catch in page.tsx: call
getTreasuryBalanceSnapshot() inside try and on error log the error and return a
degraded snapshot object (e.g., { status: 'failed', lastKnown: null } or
whatever shape TreasuryDashboard expects) to MemberHome; keep getSessionState
usage unchanged and ensure MemberHome/TreasuryDashboard receive the
failure-indicating snapshot so the client-side refresh/recovery flow can run.
---
Nitpick comments:
In `@drizzle/0002_lovely_colossus.sql`:
- Around line 31-32: The current separate indexes on
"treasury_balance_snapshots" ("treasury_balance_snapshots_account_idx" for
(chain_id, account_address) and "treasury_balance_snapshots_synced_at_idx" for
(synced_at)) are suboptimal for the pattern used by getTreasuryBalanceSnapshot()
(WHERE chain_id, account_address ORDER BY synced_at DESC); add a composite index
that matches that access pattern, e.g. create an index on (chain_id,
account_address, synced_at DESC) (name it clearly, e.g.
treasury_balance_snapshots_chain_account_synced_idx) so the query can use an
index-only scan; optionally remove or keep the existing single-column indexes
after verifying no other queries rely on them.
In `@src/components/treasury/treasury-dashboard.tsx`:
- Around line 102-118: The fallback GET to fetch("/api/treasury/snapshot") can
resolve after the component unmounts and its network errors are uncaught;
re-check the isMounted guard after the await and wrap the fetch/json in a
try/catch: after awaiting response and before calling setSnapshot (both the
error branch and the success branch where nextSnapshot is set), verify
isMounted() and only then call setSnapshot, and catch any thrown errors from
fetch()/response.json() to setSnapshot an appropriate failure state (again only
if isMounted()) or swallow/log the error to avoid unhandled rejections.
In `@src/lib/treasury/balances.ts`:
- Line 284: The conditional ABI choice is unnecessary: replace the ternary that
picks WXDAI_ABI vs erc20Abi (uses asset.symbol === "wxDAI") to always use
erc20Abi when constructing the contract (the code around abi: ... in the
function that reads balances), and remove the now-unused WXDAI_ABI constant
(referenced as WXDAI_ABI) to simplify the module; ensure calls relying on
balanceOf(address) continue to use the same erc20Abi.
- Around line 263-265: The current check around the CoinGecko wETH price (the
`price` variable that currently triggers `throw new Error("CoinGecko wETH price
unavailable")`) should also guard against non-positive values; update the
validation condition (where `if (!price || !Number.isFinite(price)) { ... }`) to
include `price <= 0` so zero or negative prices are rejected, and adjust the
thrown error message to reflect invalid/non-positive pricing data for easier
debugging.
🪄 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: 2a358b49-413c-45ef-8a9d-040c583f26ef
📒 Files selected for processing (14)
.env.examplePROJECT_SPEC.mddrizzle/0002_lovely_colossus.sqldrizzle/meta/0002_snapshot.jsondrizzle/meta/_journal.jsonsrc/app/api/auth/verify/route.tssrc/app/api/treasury/snapshot/route.tssrc/app/api/treasury/sync/route.tssrc/app/page.tsxsrc/components/treasury/sync-status-badge.tsxsrc/components/treasury/treasury-dashboard.tsxsrc/db/schema.tssrc/lib/treasury/balances.tssrc/lib/treasury/types.ts
This pull request introduces a major update focused on implementing a robust treasury balance snapshot and sync system, including database schema changes, new API endpoints, and a refactored homepage dashboard. It also adds login analytics and improves documentation for treasury data handling.
Treasury snapshot and sync system:
treasury_balance_snapshots,treasury_balance_assets) and an enum type (treasury_snapshot_status) to track treasury balance snapshots, asset breakdowns, and sync status, along with relevant indexes and triggers. [1] [2]GET /api/treasury/snapshotfor fetching the latest treasury snapshot, andPOST /api/treasury/syncto trigger a background sync and handle errors gracefully. [1] [2]Homepage and UI improvements:
TreasuryDashboardcomponent, centralizing treasury balance display and removing inline formatting and logic frompage.tsx. [1] [2] [3] [4]SyncStatusBadgecomponent to reflect real-time sync status with improved visual cues and support for all sync states.Authentication and analytics:
Documentation and configuration:
.env.exampleindicating that the CoinGecko API key is optional.Summary by CodeRabbit
New Features
Documentation