New Vault Pnl tables #3228
New Vault Pnl tables #3228
Conversation
WalkthroughAdds two new materialized views (hourly/daily) with migrations, a VaultPnl view store (refresh + query functions), bulk PnL insert helper, PnlInterval enum, tests for new views, updates roundtable refresh task to refresh old and new views in parallel, adds Athena DDL helpers for pnl, and adds CI branch trigger patterns. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Test as Test Suite
participant PnlTable as PnL Table Store
participant DB as Postgres
participant VaultPnlView as VaultPnlView Module
participant OldView as VaultPnlTicksView
participant Roundtable as Refresh Task
Test->>PnlTable: createMany(pnlRecords)
PnlTable-->>DB: INSERT many rows
DB-->>Test: insert result
Test->>VaultPnlView: refreshHourlyView()
VaultPnlView->>DB: REFRESH MATERIALIZED VIEW vaults_hourly_pnl_v2 CONCURRENTLY
DB-->>VaultPnlView: refreshed
Test->>VaultPnlView: refreshDailyView()
VaultPnlView->>DB: REFRESH MATERIALIZED VIEW vaults_daily_pnl_v2 CONCURRENTLY
DB-->>VaultPnlView: refreshed
Note over Roundtable,VaultPnlView: Roundtable performs parallel refreshes for each view
Roundtable->>VaultPnlView: refresh hourly & daily (Promise.all)
Roundtable->>OldView: refresh hourly & daily (Promise.all)
VaultPnlView->>DB: REFRESH MATERIALIZED VIEW ...
OldView->>DB: REFRESH MATERIALIZED VIEW ...
DB-->>Roundtable: both refreshed
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (25)
🔇 Additional comments (2)
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: 2
🧹 Nitpick comments (10)
indexer/packages/postgres/src/db/migrations/migration_files/20251027172540_create_vaults_hourly_pnl_v2.ts (4)
1-1: Preferimport { Knex } from 'knex'for types-only import.Small consistency/readability nit for our migrations.
-import * as Knex from 'knex'; +import { Knex } from 'knex';
11-17: Hard-coded subaccount address — confirm intent and longevity.The special address inclusion is duplicated across hourly/daily migrations. Confirm this is stable and intended to be permanently included; otherwise, consider documenting the reason in a comment for future maintainers.
34-34: Improve interval readability.Use interval literals for clarity.
- AND "createdAt" >= NOW() - interval '604800 second' + AND "createdAt" >= NOW() - interval '7 day'
40-43: Initial MV build vs. refresh strategy.For very large pnl tables, consider:
- CREATE MATERIALIZED VIEW ... WITH NO DATA
- CREATE UNIQUE INDEX ...
- REFRESH MATERIALIZED VIEW CONCURRENTLY ...
This reduces lock impact during the first population.
indexer/packages/postgres/src/db/migrations/migration_files/20251027172547_create_vaults_daily_pnl_v2.ts (4)
1-1: Preferimport { Knex } from 'knex'for types-only import.-import * as Knex from 'knex'; +import { Knex } from 'knex';
11-17: Hard-coded subaccount address — confirm intent and longevity.Same note as hourly migration; please document why this address is included and whether it should be configurable for future networks.
34-34: Improve interval readability.Use a more readable interval literal.
- AND "createdAt" >= NOW() - interval '7776000 second' + AND "createdAt" >= NOW() - interval '90 day'
40-46: Consider initial MV build pattern to minimize locks.Same operational advice as hourly migration: WITH NO DATA + index + REFRESH CONCURRENTLY.
indexer/packages/postgres/src/stores/vault-pnl-view.ts (1)
12-19: Optional: add a statement timeout to refreshes.Avoid long blocking refreshes under load by setting a conservative statement timeout via sqlOptions.
export async function refreshHourlyView(): Promise<void> { await rawQuery( `REFRESH MATERIALIZED VIEW CONCURRENTLY ${VAULT_HOURLY_PNL_VIEW}`, { readReplica: false, + sqlOptions: { statement_timeout: 60000 }, // 60s; tune as needed }, ); }Apply similarly to refreshDailyView.
indexer/packages/postgres/__tests__/stores/vault-pnl-view.test.ts (1)
95-117: LGTM! Test logic is sound, with a minor style suggestion.The test correctly validates that
getLatestVaultPnlreturns the most recent PnL record for each subaccount. The expected equity values ('1100' for subaccount1, '200' for subaccount2) match the most recent records created bysetupIntervalPnl.Optional: Remove extra blank line for consistency.
Apply this diff to remove the extra blank line at line 104:
const latestPnl: PnlFromDatabase[] = await VaultPnlView.getLatestVaultPnl(); // Should return the most recent PNL for each subaccount expect(latestPnl).toHaveLength(2); - + const subaccount1Pnl = latestPnl.find(
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
indexer/packages/postgres/__tests__/stores/vault-pnl-view.test.ts(1 hunks)indexer/packages/postgres/src/db/migrations/migration_files/20251027172540_create_vaults_hourly_pnl_v2.ts(1 hunks)indexer/packages/postgres/src/db/migrations/migration_files/20251027172547_create_vaults_daily_pnl_v2.ts(1 hunks)indexer/packages/postgres/src/stores/pnl-table.ts(1 hunks)indexer/packages/postgres/src/stores/vault-pnl-view.ts(1 hunks)indexer/packages/postgres/src/types/pnl-types.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
indexer/packages/postgres/src/db/migrations/migration_files/20251027172547_create_vaults_daily_pnl_v2.ts (1)
indexer/packages/postgres/src/db/migrations/migration_files/20251027172540_create_vaults_hourly_pnl_v2.ts (2)
up(40-43)down(45-47)
indexer/packages/postgres/src/stores/vault-pnl-view.ts (2)
indexer/packages/postgres/src/helpers/stores-helpers.ts (1)
rawQuery(62-80)indexer/packages/postgres/src/types/db-model-types.ts (1)
PnlFromDatabase(358-365)
indexer/packages/postgres/__tests__/stores/vault-pnl-view.test.ts (4)
indexer/packages/postgres/src/helpers/db-helpers.ts (3)
migrate(176-178)clearData(143-159)teardown(180-186)indexer/packages/postgres/__tests__/helpers/mock-generators.ts (1)
seedData(58-107)indexer/packages/postgres/__tests__/helpers/constants.ts (5)
defaultWallet2(197-201)defaultSubaccountWithAlternateAddress(128-133)defaultVault(1083-1089)defaultSubaccount(85-90)defaultSubaccountIdWithAlternateAddress(172-175)indexer/packages/postgres/src/types/db-model-types.ts (1)
PnlFromDatabase(358-365)
indexer/packages/postgres/src/stores/pnl-table.ts (4)
indexer/packages/postgres/src/types/pnl-types.ts (1)
PnlCreateObject(10-17)indexer/packages/postgres/src/types/utility-types.ts (1)
Options(15-25)indexer/packages/postgres/src/types/db-model-types.ts (1)
PnlFromDatabase(358-365)indexer/packages/postgres/src/models/pnl-model.ts (1)
PnlModel(8-64)
indexer/packages/postgres/src/db/migrations/migration_files/20251027172540_create_vaults_hourly_pnl_v2.ts (1)
indexer/packages/postgres/src/db/migrations/migration_files/20251027172547_create_vaults_daily_pnl_v2.ts (2)
up(40-43)down(45-47)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (29)
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-bazooka-lambda / (bazooka) Build and Push Lambda
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-vulcan / (vulcan) Build and Push
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-ecs-service-roundtable / (roundtable) Build and Push
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-ecs-service-socks / (socks) Build and Push
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-auxo-lambda / (auxo) Build and Push Lambda
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-ecs-service-ender / (ender) Build and Push
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-ecs-service-comlink / (comlink) Build and Push
- GitHub Check: call-build-ecs-service-comlink / (comlink) Check docker image build
- GitHub Check: call-build-ecs-service-vulcan / (vulcan) Check docker image build
- GitHub Check: call-build-ecs-service-socks / (socks) Check docker image build
- GitHub Check: call-build-ecs-service-roundtable / (roundtable) Check docker image build
- GitHub Check: call-build-ecs-service-ender / (ender) Check docker image build
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-auxo-lambda / (auxo) Build and Push Lambda
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-ecs-service-roundtable / (roundtable) Build and Push
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-bazooka-lambda / (bazooka) Build and Push Lambda
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-ecs-service-socks / (socks) Build and Push
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-vulcan / (vulcan) Build and Push
- GitHub Check: check-build-bazooka
- GitHub Check: check-build-auxo
- GitHub Check: test / run_command
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-ecs-service-ender / (ender) Build and Push
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-ecs-service-comlink / (comlink) Build and Push
- GitHub Check: build-and-push-mainnet
- GitHub Check: build-and-push-testnet
- GitHub Check: lint
- GitHub Check: run_command
- GitHub Check: Summary
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
🔇 Additional comments (9)
indexer/packages/postgres/src/db/migrations/migration_files/20251027172540_create_vaults_hourly_pnl_v2.ts (1)
26-35: Row selection picks the earliest tick in each hour. Confirm requirement.ROW_NUMBER() ORDER BY "createdAt" ASC yields the first (earliest) record per hour. If “end‑of‑hour” snapshots are desired, ORDER BY DESC would be more appropriate.
- ORDER BY "createdAt" + -- Use DESC if latest-in-hour is desired: + ORDER BY "createdAt" DESCindexer/packages/postgres/src/db/migrations/migration_files/20251027172547_create_vaults_daily_pnl_v2.ts (1)
26-35: Row selection picks the earliest tick in each day. Confirm requirement.If “end‑of‑day” snapshots are desired, swap ordering to DESC.
- ORDER BY "createdAt" + -- Use DESC if latest-in-day is desired: + ORDER BY "createdAt" DESCindexer/packages/postgres/src/stores/vault-pnl-view.ts (1)
79-93: OK — DISTINCT ON pattern for latest per vault.Query shape and ordering look correct for “latest per subaccountId” given the hourly MV.
Confirm that restricting to the hourly view is intended; if a vault has no rows in the last 7 days (MV horizon), it won’t appear. If that’s undesirable, consider switching to the daily MV or underlying table with a fallback.
indexer/packages/postgres/src/types/pnl-types.ts (1)
18-22: The review comment is incorrect—no action needed.The types/index.ts barrel already re-exports PnlInterval via
export * from './pnl-types', which automatically includes the enum. The import in vault-pnl-view.ts will resolve correctly. No additional export statement is required.Likely an incorrect or invalid review comment.
indexer/packages/postgres/__tests__/stores/vault-pnl-view.test.ts (5)
1-17: LGTM! Clean imports.The imports are well-organized and include all necessary dependencies for testing the vault PnL views.
19-46: LGTM! Proper test lifecycle management.The test suite correctly sets up migrations, seeds fresh data before each test, and properly cleans up afterward.
119-126: LGTM! Time calculations are correct.The helper function properly sets up test timestamps relative to the start of the current UTC day. The luxon API usage is correct—both singular and plural duration keys are accepted.
127-203: LGTM! Comprehensive test data setup.The helper function creates a well-structured set of PnL records across different time periods for two subaccounts, providing good coverage for testing hourly/daily aggregation logic.
48-93: The review comment is based on an incorrect assumption about the parameter semantics.The third parameter of
getVaultsPnlis namedearliestDateand documented as "The earliest date to fetch data from" (not an end time). This means it defines the START of the query window, not the end. The query window is[earliestDate, earliestDate + timeWindowSeconds].The test correctly queries from
createdPnl[8].createdAt + 1 second(2 days ago + 1 second) with a 1-week window, which includes all records created between 2 days ago and 5 days in the future. The expected results [7, 5, 2, 0] for hourly and [7, 2] for daily filtering are chronologically within this window and are correct. No issues exist with the test logic.Likely an incorrect or invalid review comment.
Changelist
[Describe or list the changes made in this PR]
Test Plan
[Describe how this PR was tested (if applicable)]
Author/Reviewer Checklist
state-breakinglabel.indexer-postgres-breakinglabel.PrepareProposalorProcessProposal, manually add the labelproposal-breaking.feature:[feature-name].backport/[branch-name].refactor,chore,bug.Summary by CodeRabbit
New Features
Tests
Chores
CI