De-duplicate and filter out invalid pnl ticks for megavault.

[vincentwschau]

Changelist

Add logic to de-duplicate pnl ticks for the same vault when combining PnL of multiple vaults to derive the megavault PnL.
Add logic to filter out megavault Pnl ticks that are aggregated from less Pnl ticks than the number of vaults existing at the time of the Pnl tick.
Both changes are to filter out inconsistencies caused by issues with the PnL task, where it either:

• creates 2 PnL ticks for the same vault subaccount within the same hour
• does not create a PnL tick for a vault subaccount for an hour

Test Plan

Updated unit tests.

Author/Reviewer Checklist

• If this PR has changes that result in a different app state given the same prior state and transaction list, manually add the state-breaking label.
• If the PR has breaking postgres changes to the indexer add the indexer-postgres-breaking label.
• If this PR isn't state-breaking but has changes that modify behavior in PrepareProposal or ProcessProposal, manually add the label proposal-breaking.
• If this PR is one of many that implement a specific feature, manually label them all feature:[feature-name].
• If you wish to for mergify-bot to automatically create a PR to backport your change to a release branch, manually add the label backport/[branch-name].
• Manually add any of the following labels: refactor, chore, bug.

Summary by CodeRabbit

Release Notes

• New Features

  • Introduced a new interface AggregatedPnlTick for improved handling of profit and loss (P&L) tick data.
  • Enhanced vault-related data handling and aggregation logic in the vault controller.
  • Added branch triggers for GitHub Actions workflows to include vincentc/dedup-filter-ticks.

• Bug Fixes

  • Updated test cases to ensure accurate responses from the API endpoints, particularly for the GET /megavault/historicalPnl endpoint.

• Chores

  • Added new dependencies in the project configuration to support recent changes.

[coderabbitai]

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (2)

indexer/services/comlink/src/controllers/api/v4/vault-controller.ts (2)

95-111: Consider handling undefined firstMainVaultTransferTimestamp

When firstMainVaultTransferTimestamp is undefined, it's passed directly to aggregateVaultPnlTicks. While the function handles this case, consider adding a log message or documentation to explain this scenario.

---

Line range hint 729-734: Improve error message formatting

The error message concatenation could be improved:

1. Use template literals consistently
2. Include more structured information in the log

-        message: `Vault clob pair id ${vaultMapping[subaccountId]} does not correspond to a ` +
-          'perpetual market.',
+        message: `Vault clob pair id ${vaultMapping[subaccountId].clobPairId} does not correspond to a perpetual market`,
+        vault: vaultMapping[subaccountId],

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 18f5b73 and 8ee7ab5.

📒 Files selected for processing (2)

• indexer/services/comlink/tests/controllers/api/v4/vault-controller.test.ts (4 hunks)
• indexer/services/comlink/src/controllers/api/v4/vault-controller.ts (9 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• indexer/services/comlink/tests/controllers/api/v4/vault-controller.test.ts

🔇 Additional comments (4)

indexer/services/comlink/src/controllers/api/v4/vault-controller.ts (4)

28-31: LGTM: Type safety improvements

The changes to the VaultMapping interface and additional type imports enhance type safety and provide better access to vault properties.

Also applies to: 64-64, 71-71

---

538-550: LGTM: Improved PnL tick validation

The updated signature and implementation properly handle PnL tick aggregation with vault validation.

---

652-669: LGTM: Well-implemented transfer timestamp retrieval

The function properly handles the case when no transfers exist and uses appropriate ordering.

---

681-709: Consider additional error handling in binary search

The binary search implementation using bounds.le should consider handling potential edge cases:

1. Empty array of vault creation times
2. Invalid DateTime comparisons