Skip to content

[ENG-1369] Fix daily PnL aggregation to exclude child subaccounts created mid-day (backport #3248)#3266

Closed
mergify[bot] wants to merge 1 commit intorelease/indexer/v9.5.xfrom
mergify/bp/release/indexer/v9.5.x/pr-3248
Closed

[ENG-1369] Fix daily PnL aggregation to exclude child subaccounts created mid-day (backport #3248)#3266
mergify[bot] wants to merge 1 commit intorelease/indexer/v9.5.xfrom
mergify/bp/release/indexer/v9.5.x/pr-3248

Conversation

@mergify
Copy link
Copy Markdown
Contributor

@mergify mergify Bot commented Dec 1, 2025

Changelist

Fixed a bug in findAllDailyAggregate where child subaccounts created mid-day were incorrectly included in daily PnL aggregations, causing mismatched values between the daily and hourly endpoints.

Root Cause:
The DISTINCT ON query was selecting the first record of each day for each individual subaccount, which meant newly created child subaccounts would contribute their first record (e.g., from 05:00) while existing subaccounts contributed their 00:00 records. This mixed timestamps from different hours within the same daily aggregate.

Changes:
Modified findAllDailyAggregate to first identify the earliest timestamp for each day across ALL subaccounts
Only aggregate records that match that earliest timestamp (typically 00:00:00)
Child subaccounts without records at the earliest timestamp are now correctly excluded from that day's aggregate
This ensures daily aggregated values match hourly aggregated values at 00:00 timestamps

Impact:
Daily PnL endpoint now returns consistent values that match the hourly endpoint at 00:00:00
Fixes incorrect equity, totalPnl, and netTransfers calculations when child subaccounts are created mid-day
Example: For Nov 11, the bug caused equity of 8320.45 (mixing 00:00 and 05:00 data) instead of correct value of 7823.86 (only 00:00 data)

    {
      "equity": "8320.4565512370",
      "netTransfers": "-41963.867455",
      "totalPnl": "50284.3240062370",
      "createdAt": "2025-11-11T00:00:00.000Z",
      "createdAtHeight": "62876520"
    },
    {
      "equity": "7823.8584687190",
      "netTransfers": "-42467.553285",
      "totalPnl": "50291.4117537190",
      "createdAt": "2025-11-11T00:00:00.000Z",
      "createdAtHeight": "62847739"
    },

Test Plan

Unit Tests:

Added test Successfully handles child subaccounts created mid-day by excluding them from daily aggregation which verifies:
Day 1: Only parent exists → aggregates only parent
Day 2: Child created at 05:00 → aggregates only parent (at 00:00), excludes child
Day 3: Both have 00:00 records → aggregates both correctly

Added test Daily aggregated values should match hourly aggregated values at 00:00 timestamps which verifies consistency between daily and hourly endpoints across multiple days

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

  • Bug Fixes

    • Fixed daily PNL aggregation to correctly exclude child subaccounts created mid-day unless they have a 00:00 record, ensuring day-level summaries reflect the proper earliest-timestamp snapshots.
  • Tests

    • Added tests validating daily PNL aggregation for mid-day child subaccount creation scenarios.
    • Added tests confirming daily PNL values match hourly values at 00:00 timestamps.

This is an automatic backport of pull request #3248 done by [Mergify](https://mergify.com).

…ated mid-day (#3248)

(cherry picked from commit 3f8d74c)

# Conflicts:
#	indexer/packages/postgres/__tests__/stores/pnl-table.test.ts
#	indexer/packages/postgres/src/stores/pnl-table.ts
@mergify mergify Bot added the conflicts label Dec 1, 2025
@mergify mergify Bot requested a review from a team as a code owner December 1, 2025 18:43
@mergify
Copy link
Copy Markdown
Contributor Author

mergify Bot commented Dec 1, 2025

Cherry-pick of 3f8d74c has failed:

On branch mergify/bp/release/indexer/v9.5.x/pr-3248
Your branch is up to date with 'origin/release/indexer/v9.5.x'.

You are currently cherry-picking commit 3f8d74c1.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   indexer/services/comlink/__tests__/controllers/api/v4/pnl-controller.test.ts

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   indexer/packages/postgres/__tests__/stores/pnl-table.test.ts
	both modified:   indexer/packages/postgres/src/stores/pnl-table.ts

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@linear
Copy link
Copy Markdown

linear Bot commented Dec 1, 2025

ENG-1369 Debug PnL issue

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Dec 1, 2025

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.


Comment @coderabbitai help to get the list of available commands and usage tips.

@shrenujb shrenujb deleted the branch release/indexer/v9.5.x January 8, 2026 16:59
@shrenujb shrenujb closed this Jan 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants