feat: add backend-driven top home widgets#2174
Open
stackingsaunter wants to merge 1 commit intomasterfrom
Open
feat: add backend-driven top home widgets#2174stackingsaunter wants to merge 1 commit intomasterfrom
stackingsaunter wants to merge 1 commit intomasterfrom
Conversation
Add top home cards for total balance, net flows, and bitcoin price with a backend chart endpoint to avoid transaction pagination in the browser. Note: Bitcoin Price 7d net change still needs a follow-up fix and is marked as n/a when reliable historical comparison data is unavailable. Made-with: Cursor
Contributor
📝 WalkthroughWalkthroughThis pull request introduces a complete home charts feature spanning backend and frontend. It adds a new API endpoint ( Changes
Sequence DiagramsequenceDiagram
participant Client
participant HTTPService
participant APIService
participant LNClient
participant Database
participant PriceService
Client->>HTTPService: GET /api/home/charts?from=X
HTTPService->>HTTPService: Validate from parameter (default to 7 days ago)
HTTPService->>APIService: GetHomeChartsData(ctx, from)
APIService->>LNClient: Verify LN client available
APIService->>Database: ListTransactions(from, cursor=0, page=0)
Database-->>APIService: Transaction records
APIService->>APIService: Compute timestamps & net sats per transaction
APIService->>APIService: Aggregate net flows & detect incoming deposits
APIService->>APIService: Sort chart points by timestamp
APIService-->>HTTPService: HomeChartsResponse (txPoints, hasIncomingDeposit, netFlowsSat)
HTTPService-->>Client: 200 OK with JSON response
Client->>PriceService: Fetch historical daily BTC prices (7 days)
PriceService-->>Client: Historical price data per day
Client->>Client: Compute rolling balance & net flows over 7 days
Client->>Client: Calculate price changes & render AreaChart sparklines
Client->>Client: Display HomeTopChartsRow with metrics
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![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: 5
🧹 Nitpick comments (1)
api/home_charts.go (1)
52-53: Wrap the storage error with call-site context.Returning
errdirectly here loses where the failure came from, which makes home chart failures harder to diagnose.♻️ Suggested change
import ( "context" "errors" + "fmt" "slices" "github.com/getAlby/hub/constants" "github.com/getAlby/hub/db" ) @@ if err != nil { - return nil, err + return nil, fmt.Errorf("list home chart transactions: %w", err) }As per coding guidelines:
**/*.go: Use error wrapping withfmt.Errorf("context: %w", err)for debugging.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/home_charts.go` around lines 52 - 53, Wrap the returned storage error at the call site instead of returning err directly: replace the bare "return nil, err" in the home chart retrieval function in api/home_charts.go with a wrapped error using fmt.Errorf("loading home charts from storage: %w", err) (or similar contextual text), and add the fmt import if missing so the wrapped error is returned to callers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@api/home_charts.go`:
- Around line 39-51: The endpoint is pulling the full transaction stream because
api.svc.GetTransactionsService().ListTransactions is called with 0 for the limit
(treated as "no limit"), causing O(n) work per poll; change the call in
home_charts.go to request a bounded number of points (e.g., a maxPoints
constant) or a server-side aggregated/downsampled window instead of raw
transactions, or add/enable an aggregation parameter to ListTransactions so the
service returns pre-aggregated/downsampled results for the sparkline; update the
parameters to pass that limit or aggregation flag and ensure the chart-building
code uses those aggregated points rather than replaying every transaction.
In `@frontend/src/components/home/widgets/HomeTopChartsRow.tsx`:
- Around line 307-330: The code currently rebuilds historicalPriceEndpoints and
re-fetches all of them every CHART_REFRESH_MS; split the endpoints into
immutable past endpoints and a single today endpoint (use the existing
daySnapshots/ts logic to identify which timestamp is today), then fetch past
endpoints once (use useSWRImmutable or useSWR with null refreshInterval) and
fetch only the today endpoint with the existing polling (useSWR with
refreshInterval: CHART_REFRESH_MS). Update the keys (e.g., keep
"home-bitcoin-price-daily" base key but add separate keys like
["home-bitcoin-price-past", ...pastEndpoints] and ["home-bitcoin-price-today",
todayEndpoint]) and combine their results into dailyPricePayloads so past data
isn’t polled repeatedly while today’s snapshot continues to refresh.
- Around line 419-430: The computed 7d change can be based on partial data
because firstPrice/lastPrice fall back to bitcoinRate and dayCloses is filtered;
require full day coverage before computing percent change. Change the logic so
firstPrice and lastPrice are taken only from dayCloses (no bitcoinRate fallback)
and set hasPriceChangeData to require dayCloses.length === daySnapshots.length
&& dayCloses.length >= 2 && firstPrice > 0 && lastPrice > 0 (using the existing
symbols dayCloses, daySnapshots, firstPrice, lastPrice, hasPriceChangeData,
priceChangePct), and ensure priceChangePct is not computed unless
hasPriceChangeData is true (otherwise fall back to n/a).
- Around line 331-349: nowTs is only set once on mount so fromSeconds is frozen;
update the fetch so the 7-day window is recomputed on every refresh by deriving
the window from Date.now() inside the SWR fetcher (or by removing nowTs state
and computing fromSeconds there) instead of using the outer fromSeconds value.
Modify the useSWR call/fetcher (referencing useSWR, fromSeconds, nowTs,
WINDOW_MS, CHART_REFRESH_MS) so the fetcher computes const windowStart =
Date.now() - WINDOW_MS and fromSeconds = Math.floor(windowStart / 1000) before
calling request(`/api/home/charts?from=${fromSeconds}`), ensuring the cache key
still triggers periodic refreshes via CHART_REFRESH_MS.
In `@http/home_charts_handlers.go`:
- Around line 14-23: The handler currently accepts any `from` (including 0) and
can produce an unbounded full-history read; constrain it by enforcing a maximum
lookback using the existing defaultHomeChartsDays constant (or reject values
older than now - defaultHomeChartsDays*24h). In the parsing branch that sets
`from`, validate parsed against a computed minTimestamp
(uint64(time.Now().Add(-defaultHomeChartsDays * 24 * time.Hour).Unix())), and
either clamp `from = minTimestamp` or return a BadRequest if parsed <
minTimestamp (choose consistent behavior with other handlers). Update the logic
around the `from` variable in this block so calls to the downstream service
never receive a timestamp older than the allowed lookback.
---
Nitpick comments:
In `@api/home_charts.go`:
- Around line 52-53: Wrap the returned storage error at the call site instead of
returning err directly: replace the bare "return nil, err" in the home chart
retrieval function in api/home_charts.go with a wrapped error using
fmt.Errorf("loading home charts from storage: %w", err) (or similar contextual
text), and add the fmt import if missing so the wrapped error is returned to
callers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a2d74984-f17b-47ad-a4e7-2470e8474ff2
⛔ Files ignored due to path filters (1)
frontend/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (7)
api/home_charts.goapi/models.gofrontend/package.jsonfrontend/src/components/home/widgets/HomeTopChartsRow.tsxfrontend/src/screens/Home.tsxhttp/home_charts_handlers.gohttp/http_service.go
Comment on lines
+39
to
+51
| transactions, _, err := api.svc.GetTransactionsService().ListTransactions( | ||
| ctx, | ||
| from, | ||
| 0, | ||
| 0, | ||
| 0, | ||
| false, | ||
| false, | ||
| nil, | ||
| lnClient, | ||
| nil, | ||
| false, | ||
| ) |
Contributor
There was a problem hiding this comment.
This chart endpoint still replays the raw transaction stream.
transactions/transactions_service.go:675-677 treats the 0 limit here as “no limit”, so every poll pulls the full matching set and later emits one point per transaction. For a 7-day sparkline refreshed every 5s, busy nodes will end up doing an O(n) query/sort/serialize cycle on each hit. Aggregate or downsample the window server-side, or cap the returned points.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/home_charts.go` around lines 39 - 51, The endpoint is pulling the full
transaction stream because api.svc.GetTransactionsService().ListTransactions is
called with 0 for the limit (treated as "no limit"), causing O(n) work per poll;
change the call in home_charts.go to request a bounded number of points (e.g., a
maxPoints constant) or a server-side aggregated/downsampled window instead of
raw transactions, or add/enable an aggregation parameter to ListTransactions so
the service returns pre-aggregated/downsampled results for the sparkline; update
the parameters to pass that limit or aggregation flag and ensure the
chart-building code uses those aggregated points rather than replaying every
transaction.
Comment on lines
+307
to
+330
| const historicalPriceEndpoints = daySnapshots.map((day) => { | ||
| const ts = Math.floor(endOfDay(day).getTime() / 1000); | ||
| const mempoolEndpoint = `/v1/historical-price?currency=${encodeURIComponent( | ||
| priceHistoryCurrency | ||
| )}×tamp=${ts}`; | ||
| return `/api/mempool?endpoint=${encodeURIComponent(mempoolEndpoint)}`; | ||
| }); | ||
| const { data: dailyPricePayloads } = useSWR<unknown[]>( | ||
| info ? ["home-bitcoin-price-daily", ...historicalPriceEndpoints] : null, | ||
| async () => { | ||
| const responses = await Promise.all( | ||
| historicalPriceEndpoints.map(async (endpoint) => { | ||
| try { | ||
| const response = await request<unknown>(endpoint); | ||
| return response ?? {}; | ||
| } catch { | ||
| return {}; | ||
| } | ||
| }) | ||
| ); | ||
| return responses; | ||
| }, | ||
| { refreshInterval: CHART_REFRESH_MS, refreshWhenHidden: true } | ||
| ); |
Contributor
There was a problem hiding this comment.
Most of these historical price calls never change, but they still refresh every 5s.
Six of the seven endpoints here are prior-day closes, yet the fetcher re-requests all of them on every interval. That is 84 mempool reads/minute per tab for mostly immutable data. Fetch past days once and only poll today's snapshot.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/home/widgets/HomeTopChartsRow.tsx` around lines 307 -
330, The code currently rebuilds historicalPriceEndpoints and re-fetches all of
them every CHART_REFRESH_MS; split the endpoints into immutable past endpoints
and a single today endpoint (use the existing daySnapshots/ts logic to identify
which timestamp is today), then fetch past endpoints once (use useSWRImmutable
or useSWR with null refreshInterval) and fetch only the today endpoint with the
existing polling (useSWR with refreshInterval: CHART_REFRESH_MS). Update the
keys (e.g., keep "home-bitcoin-price-daily" base key but add separate keys like
["home-bitcoin-price-past", ...pastEndpoints] and ["home-bitcoin-price-today",
todayEndpoint]) and combine their results into dailyPricePayloads so past data
isn’t polled repeatedly while today’s snapshot continues to refresh.
Comment on lines
+331
to
+349
| const [nowTs, setNowTs] = React.useState<number | null>(null); | ||
|
|
||
| React.useEffect(() => { | ||
| setNowTs(Date.now()); | ||
| }, []); | ||
| const windowStart = (nowTs ?? 0) - WINDOW_MS; | ||
| const fromSeconds = Math.floor(windowStart / 1000); | ||
| const { data: homeChartsData } = useSWR<HomeChartsResponse>( | ||
| info && nowTs !== null ? ["home-charts-data", fromSeconds] : null, | ||
| async () => { | ||
| const response = await request<HomeChartsResponse>( | ||
| `/api/home/charts?from=${fromSeconds}` | ||
| ); | ||
| if (!response) { | ||
| throw new Error("Missing home chart payload"); | ||
| } | ||
| return response; | ||
| }, | ||
| { refreshInterval: CHART_REFRESH_MS, refreshWhenHidden: true } |
Contributor
There was a problem hiding this comment.
The from window freezes at mount time.
nowTs is set once and then reused forever, so fromSeconds stays anchored to the first render. If Home stays open, the card gradually stops representing the last 7 days and starts representing “since 7 days before this tab was opened”. Recompute the window on each refresh or derive it from Date.now() inside the fetcher.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/home/widgets/HomeTopChartsRow.tsx` around lines 331 -
349, nowTs is only set once on mount so fromSeconds is frozen; update the fetch
so the 7-day window is recomputed on every refresh by deriving the window from
Date.now() inside the SWR fetcher (or by removing nowTs state and computing
fromSeconds there) instead of using the outer fromSeconds value. Modify the
useSWR call/fetcher (referencing useSWR, fromSeconds, nowTs, WINDOW_MS,
CHART_REFRESH_MS) so the fetcher computes const windowStart = Date.now() -
WINDOW_MS and fromSeconds = Math.floor(windowStart / 1000) before calling
request(`/api/home/charts?from=${fromSeconds}`), ensuring the cache key still
triggers periodic refreshes via CHART_REFRESH_MS.
Comment on lines
+419
to
+430
| const dayCloses = daySnapshots | ||
| .map((day, idx) => { | ||
| const points = parsePricePoints(dailyPricePayloads?.[idx], chartCurrency); | ||
| return getDayClosePrice(points, endOfDay(day).getTime()); | ||
| }) | ||
| .filter((value): value is number => typeof value === "number"); | ||
| const firstPrice = dayCloses[0] || bitcoinRate?.rate_float || 0; | ||
| const lastPrice = | ||
| dayCloses[dayCloses.length - 1] || bitcoinRate?.rate_float || 0; | ||
| const hasPriceChangeData = | ||
| dayCloses.length >= 2 && firstPrice > 0 && lastPrice > 0; | ||
| const priceChangePct = hasPriceChangeData |
Contributor
There was a problem hiding this comment.
Partial history still renders as a 7d change.
After the .filter(...), dayCloses[0] becomes the first available close, not necessarily the close from 7 days ago. If only 2-3 daily snapshots parsed successfully, this still prints a percentage labeled 7d instead of falling back to n/a. Require complete first/last-day coverage before computing priceChangePct.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/home/widgets/HomeTopChartsRow.tsx` around lines 419 -
430, The computed 7d change can be based on partial data because
firstPrice/lastPrice fall back to bitcoinRate and dayCloses is filtered; require
full day coverage before computing percent change. Change the logic so
firstPrice and lastPrice are taken only from dayCloses (no bitcoinRate fallback)
and set hasPriceChangeData to require dayCloses.length === daySnapshots.length
&& dayCloses.length >= 2 && firstPrice > 0 && lastPrice > 0 (using the existing
symbols dayCloses, daySnapshots, firstPrice, lastPrice, hasPriceChangeData,
priceChangePct), and ensure priceChangePct is not computed unless
hasPriceChangeData is true (otherwise fall back to n/a).
Comment on lines
+14
to
+23
| from := uint64(time.Now().Add(-defaultHomeChartsDays * 24 * time.Hour).Unix()) | ||
| if fromRaw := c.QueryParam("from"); fromRaw != "" { | ||
| parsed, err := strconv.ParseUint(fromRaw, 10, 64) | ||
| if err != nil { | ||
| return c.JSON(http.StatusBadRequest, ErrorResponse{ | ||
| Message: "invalid query parameter: from", | ||
| }) | ||
| } | ||
| from = parsed | ||
| } |
Contributor
There was a problem hiding this comment.
Clamp or reject overly broad from values at the boundary.
This accepts from=0 and passes it through unchanged, which lets an authenticated caller turn /api/home/charts into a full-history read instead of the intended widget-sized query. Please enforce a minimum allowed timestamp or maximum lookback here instead of trusting the raw query value.
As per coding guidelines: {api,http}/**/*.go: Validate all user input at system boundaries; trust internal service calls.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@http/home_charts_handlers.go` around lines 14 - 23, The handler currently
accepts any `from` (including 0) and can produce an unbounded full-history read;
constrain it by enforcing a maximum lookback using the existing
defaultHomeChartsDays constant (or reject values older than now -
defaultHomeChartsDays*24h). In the parsing branch that sets `from`, validate
parsed against a computed minTimestamp
(uint64(time.Now().Add(-defaultHomeChartsDays * 24 * time.Hour).Unix())), and
either clamp `from = minTimestamp` or return a BadRequest if parsed <
minTimestamp (choose consistent behavior with other handlers). Update the logic
around the `from` variable in this block so calls to the downstream service
never receive a timestamp older than the allowed lookback.
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
1 participant
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.
Summary
Total Balance,Net Flows, andBitcoin PricecardsGET /api/home/charts?from=...so frontend no longer paginates transactions for chart mathrechartsarea charts and tune edge clipping/top padding for visual parityKnown limitations
n/a; this still needs a follow-up fixResource usage estimate
Current implementation (one open Home tab) is approximately:
GET /api/home/charts: every 5s (~12 req/min)GET /api/mempoolhistorical price snapshots: 7 requests every 5s (~84 req/min)Estimated total for top widgets: ~96 req/min/tab (lightweight reads, but non-trivial at scale).
Test plan
go test ./api ./httpcd frontend && yarn tsc:compileMade with Cursor
Summary by CodeRabbit