Conversation
![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: 0
🧹 Nitpick comments (1)
indexer/services/comlink/__tests__/lib/helpers.test.ts (1)
737-810: Consider explicitly settingtotalRealizedPnlin the test setup for clarity.The test now expects
realizedPnlto be'100', reflecting the new behavior where realized P&L is sourced fromposition.totalRealizedPnlrather than computed. However, the test setup doesn't explicitly set this field, making it unclear where the expected value originates.For improved test maintainability and clarity, consider:
- Explicitly setting
totalRealizedPnl: '100'in theperpetualPositionsarray at line 737-743- Adding a comment at line 810 explaining that this value comes from the stored
totalRealizedPnlfieldApply this diff to make the test data setup more explicit:
const perpetualPositions: PerpetualPositionFromDatabase[] = [{ ...testConstants.defaultPerpetualPosition, id, entryPrice: '20000', sumOpen: '10', sumClose: '0', + totalRealizedPnl: '100', }];And optionally add a comment for clarity:
netFunding: getFixedRepresentation('199500'), + // realizedPnl now sourced from position.totalRealizedPnl realizedPnl: getFixedRepresentation('100'),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
indexer/services/comlink/__tests__/lib/helpers.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
indexer/services/comlink/__tests__/lib/helpers.test.ts (1)
indexer/services/comlink/__tests__/helpers/helpers.ts (1)
getFixedRepresentation(123-125)
⏰ 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-auxo-lambda / (auxo) Build and Push Lambda
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-vulcan / (vulcan) Build and Push
- GitHub Check: (Public Testnet) 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-bazooka-lambda / (bazooka) Build and Push Lambda
- GitHub Check: (Public Testnet) 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-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: (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-socks / (socks) Build and Push
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-ecs-service-comlink / (comlink) Build and Push
- 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-vulcan / (vulcan) 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: (Public Testnet) Build and Push ECS Services / call-build-and-push-ecs-service-ender / (ender) Build and Push
- GitHub Check: call-build-ecs-service-ender / (ender) Check docker image build
- GitHub Check: call-build-ecs-service-comlink / (comlink) Check docker image build
- GitHub Check: call-build-ecs-service-roundtable / (roundtable) Check docker image build
- GitHub Check: call-build-ecs-service-vulcan / (vulcan) Check docker image build
- GitHub Check: check-build-auxo
- GitHub Check: call-build-ecs-service-socks / (socks) Check docker image build
- GitHub Check: test / run_command
- GitHub Check: check-build-bazooka
- GitHub Check: build-and-push-testnet
- GitHub Check: run_command
- GitHub Check: lint
- GitHub Check: build-and-push-mainnet
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Summary
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
indexer/services/ender/__tests__/handlers/order-fills/order-handler.test.ts (3)
460-472: Consider adding comments explaining the totalRealizedPnl calculations.The expected values ('-1' for maker, '-2.5' for taker) are correct, but adding brief comments would improve readability:
- Maker: -1 (maker fee, no position closed since BUY adds to LONG)
- Taker: (10000 - 15000) × 0.0001 - 2 = -2.5 (realized loss on position close plus taker fee)
2475-2589: Solid test coverage for before-position fields.This test explicitly validates that maker and taker fills both capture the pre-fill position snapshot. Consider adding a test variant for the edge case where no prior position exists (first fill for a perpetual/subaccount pair) to ensure the before-fields handle null/absent position state correctly.
2591-2666: Test correctly validates totalRealizedPnl on position close.The expected value '-2.5' is correct: (exitPrice - entryPrice) × fillSize - takerFee = (10000 - 15000) × 0.0001 - 2. Consider adding a brief comment explaining this calculation for future maintainers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
indexer/services/ender/__tests__/handlers/order-fills/order-handler.test.ts(17 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
indexer/services/ender/__tests__/handlers/order-fills/order-handler.test.ts (5)
indexer/services/ender/__tests__/helpers/indexer-proto-helpers.ts (4)
createOrder(399-493)createKafkaMessageFromOrderFillEvent(495-528)expectFillInDatabase(571-653)expectPerpetualPosition(983-1015)indexer/packages/postgres/__tests__/helpers/constants.ts (3)
defaultSubaccountId(156-159)defaultSubaccountId2(160-163)defaultPerpetualPosition(530-546)indexer/services/ender/__tests__/helpers/constants.ts (6)
defaultSubaccountId(279-282)defaultSubaccountId2(283-286)defaultHeight(271-271)defaultTime(273-276)defaultTxHash(277-277)defaultDateTime(272-272)indexer/packages/v4-proto-parser/src/constants.ts (2)
ORDER_FLAG_LONG_TERM(4-4)ORDER_FLAG_SHORT_TERM(3-3)indexer/services/ender/src/lib/on-message.ts (1)
onMessage(44-161)
⏰ 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: (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-auxo-lambda / (auxo) Build and Push Lambda
- GitHub Check: test / run_command
- 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-vulcan / (vulcan) Build and Push
- GitHub Check: call-build-ecs-service-vulcan / (vulcan) Check docker image build
- 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-ecs-service-ender / (ender) Build and Push
- GitHub Check: (Mainnet) 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-comlink / (comlink) Build and Push
- GitHub Check: call-build-ecs-service-ender / (ender) Check docker image build
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-bazooka-lambda / (bazooka) Build and Push Lambda
- GitHub Check: call-build-ecs-service-comlink / (comlink) Check docker image build
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-vulcan / (vulcan) Build and Push
- GitHub Check: call-build-ecs-service-roundtable / (roundtable) Check docker image build
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-ecs-service-socks / (socks) Build and Push
- GitHub Check: check-build-bazooka
- GitHub Check: check-build-auxo
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-ecs-service-ender / (ender) Build and Push
- GitHub Check: call-build-ecs-service-socks / (socks) Check docker image build
- 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-comlink / (comlink) Build and Push
- GitHub Check: build-and-push-testnet
- GitHub Check: lint
- GitHub Check: run_command
- GitHub Check: build-and-push-mainnet
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
- GitHub Check: Summary
🔇 Additional comments (2)
indexer/services/ender/__tests__/handlers/order-fills/order-handler.test.ts (2)
374-399: LGTM!The before-position field assertions are correctly added for both maker and taker fills, matching the initial position state from
defaultPerpetualPosition(size='10', entryPrice='15000', side='LONG').
1992-1994: Correctly tests dynamic entry price after first fill.The precise
entryPriceBeforevalue ('0.001499999850010015') differs from other tests because this is the second TWAP suborder fill, capturing the weighted average entry price after the first fill. This validates that the before-fields snapshot the position state at execution time.
|
|
||
| closing_amount := LEAST(fill_size, pos_size_before); -- cap to existing | ||
|
|
||
| IF pos_side_before = 'LONG' THEN |
Contributor
There was a problem hiding this comment.
in general for operations on trades, using a side multiplier avoids problems introduced by branching
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
indexer/services/ender/src/scripts/helpers/dydx_order_fill_handler_per_order.sql(5 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-11-22T18:12:04.606Z
Learnt from: hwray
Repo: dydxprotocol/v4-chain PR: 2597
File: indexer/services/ender/src/scripts/handlers/dydx_update_perpetual_v1_handler.sql:16-20
Timestamp: 2024-11-22T18:12:04.606Z
Learning: Avoid suggesting changes to deprecated functions such as `dydx_update_perpetual_v1_handler` in `indexer/services/ender/src/scripts/handlers/dydx_update_perpetual_v1_handler.sql` if they are unchanged in the PR.
Applied to files:
indexer/services/ender/src/scripts/helpers/dydx_order_fill_handler_per_order.sql
📚 Learning: 2025-05-30T17:01:07.514Z
Learnt from: Kefancao
Repo: dydxprotocol/v4-chain PR: 2869
File: indexer/services/roundtable/src/scripts/update_funding_payments.sql:55-61
Timestamp: 2025-05-30T17:01:07.514Z
Learning: In the dydx v4-chain funding payment system, if a clob_pair_id from the fills table doesn't exist in the perpetual_markets table, it represents a data integrity issue and those records should not be processed for funding payments. The INNER JOIN between net fills and perpetual_markets is intentional and serves as a data validation filter to ensure only valid perpetual market records are included in funding payment calculations.
Applied to files:
indexer/services/ender/src/scripts/helpers/dydx_order_fill_handler_per_order.sql
⏰ 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). (57)
- 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-bazooka-lambda / (bazooka) Build and Push Lambda
- GitHub Check: call-build-ecs-service-vulcan / (vulcan) Check docker image build
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-vulcan / (vulcan) Build and Push
- GitHub Check: call-build-ecs-service-roundtable / (roundtable) Check docker image build
- 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-ecs-service-roundtable / (roundtable) Build and Push
- GitHub Check: call-build-ecs-service-socks / (socks) Check docker image build
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-ecs-service-ender / (ender) Build and Push
- GitHub Check: call-build-ecs-service-comlink / (comlink) 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-comlink / (comlink) 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-auxo-lambda / (auxo) Build and Push Lambda
- 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-socks / (socks) Build and Push
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-ecs-service-comlink / (comlink) Build and Push
- GitHub Check: (Mainnet) Build and Push ECS Services / call-build-and-push-ecs-service-ender / (ender) Build and Push
- GitHub Check: check-build-auxo
- GitHub Check: check-build-bazooka
- GitHub Check: test / run_command
- GitHub Check: build-and-push-testnet
- GitHub Check: build-and-push-mainnet
- GitHub Check: lint
- GitHub Check: run_command
- GitHub Check: (Dev4) Build and Push ECS Services / call-build-and-push-auxo-lambda / (auxo) Build and Push Lambda
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
- GitHub Check: (Staging) Build and Push ECS Services / call-build-and-push-ecs-service-ender / (ender) Build and Push
- GitHub Check: (Staging) Build and Push ECS Services / call-build-and-push-ecs-service-comlink / (comlink) Build and Push
- GitHub Check: (Staging) Build and Push ECS Services / call-build-and-push-auxo-lambda / (auxo) Build and Push Lambda
- GitHub Check: (Staging) Build and Push ECS Services / call-build-and-push-bazooka-lambda / (bazooka) Build and Push Lambda
- GitHub Check: (Staging) Build and Push ECS Services / call-build-and-push-ecs-service-roundtable / (roundtable) Build and Push
- GitHub Check: (Staging) Build and Push ECS Services / call-build-and-push-vulcan / (vulcan) Build and Push
- GitHub Check: (Staging) Build and Push ECS Services / call-build-and-push-ecs-service-socks / (socks) Build and Push
- GitHub Check: (Dev4) Build and Push ECS Services / call-build-and-push-ecs-service-roundtable / (roundtable) Build and Push
- GitHub Check: (Dev4) Build and Push ECS Services / call-build-and-push-ecs-service-socks / (socks) Build and Push
- GitHub Check: (Dev) Build and Push ECS Services / call-build-and-push-bazooka-lambda / (bazooka) Build and Push Lambda
- GitHub Check: (Dev4) Build and Push ECS Services / call-build-and-push-vulcan / (vulcan) Build and Push
- GitHub Check: (Dev4) Build and Push ECS Services / call-build-and-push-ecs-service-comlink / (comlink) Build and Push
- GitHub Check: (Dev4) Build and Push ECS Services / call-build-and-push-ecs-service-ender / (ender) Build and Push
- GitHub Check: (Dev4) Build and Push ECS Services / call-build-and-push-bazooka-lambda / (bazooka) Build and Push Lambda
- GitHub Check: (Dev) Build and Push ECS Services / call-build-and-push-ecs-service-comlink / (comlink) Build and Push
- GitHub Check: (Dev) Build and Push ECS Services / call-build-and-push-vulcan / (vulcan) Build and Push
- GitHub Check: (Dev) Build and Push ECS Services / call-build-and-push-auxo-lambda / (auxo) Build and Push Lambda
- GitHub Check: (Dev) Build and Push ECS Services / call-build-and-push-ecs-service-ender / (ender) Build and Push
- GitHub Check: (Dev) Build and Push ECS Services / call-build-and-push-ecs-service-roundtable / (roundtable) Build and Push
- GitHub Check: (Dev) Build and Push ECS Services / call-build-and-push-ecs-service-socks / (socks) Build and Push
- GitHub Check: (Dev2) Build and Push ECS Services / call-build-and-push-ecs-service-ender / (ender) Build and Push
- GitHub Check: (Dev2) Build and Push ECS Services / call-build-and-push-ecs-service-comlink / (comlink) Build and Push
- GitHub Check: (Dev2) Build and Push ECS Services / call-build-and-push-auxo-lambda / (auxo) Build and Push Lambda
- GitHub Check: (Dev2) Build and Push ECS Services / call-build-and-push-bazooka-lambda / (bazooka) Build and Push Lambda
- GitHub Check: (Dev2) Build and Push ECS Services / call-build-and-push-ecs-service-roundtable / (roundtable) Build and Push
- GitHub Check: (Dev2) Build and Push ECS Services / call-build-and-push-vulcan / (vulcan) Build and Push
- GitHub Check: (Dev2) Build and Push ECS Services / call-build-and-push-ecs-service-socks / (socks) Build and Push
- GitHub Check: Summary
🔇 Additional comments (2)
indexer/services/ender/src/scripts/helpers/dydx_order_fill_handler_per_order.sql (2)
246-248: Position snapshot values now included in fill record.The three new columns (
positionSizeBefore,entryPriceBefore,positionSideBefore) are populated from snapshot values derived prior to position update. This aligns with the PR objective to capture pre-fill position metrics.Ensure the corresponding columns exist in the fills table schema and that API/model layers have been updated to surface these fields.
221-249: Function exists with correct signature and matching invocation.Verification confirms that
dydx_apply_fill_realized_effectsis properly defined atindexer/services/ender/src/scripts/helpers/dydx_apply_fill_realized_effects.sqlwith 8 parameters. The call site at lines 262–268 indydx_order_fill_handler_per_order.sqlpasses all 8 arguments in the correct order and types:
- position_id (uuid)
- fill_side (text)
- fill_size, fill_price (numeric)
- fill_fee (numeric)
- pos_side_before (text)
- pos_size_before, entry_price_before (numeric)
No issues detected.
Comment on lines
+206
to
+216
| -- Retrieve the latest perpetual position record. | ||
| SELECT * INTO read_perpetual_position_record | ||
| FROM perpetual_positions | ||
| WHERE "subaccountId" = subaccount_uuid | ||
| AND "perpetualId" = perpetual_market_record."id" | ||
| ORDER BY "openEventId" DESC | ||
| LIMIT 1; | ||
|
|
||
| snap_size_before = COALESCE(ABS(read_perpetual_position_record."sumOpen"), 0) - COALESCE(ABS(read_perpetual_position_record."sumClose"), 0); | ||
| snap_entry_before = NULLIF(read_perpetual_position_record."entryPrice", 0); | ||
| snap_side_before = read_perpetual_position_record."side"; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
Verify snap_size_before calculation logic for all position states.
The calculation at line 214 uses ABS(sumOpen) - ABS(sumClose), which may not correctly represent net position size. If sumOpen and sumClose have opposite signs (indicating mixed long/short history), taking absolute values independently distorts the result.
For example: sumOpen = 10, sumClose = -5 yields ABS(10) - ABS(-5) = 10 - 5 = 5, but the net position should be 10 - (-5) = 15.
Additionally, the SELECT at line 207 has no explicit handling if no perpetual_position record exists for the subaccount/perpetual pair—snap_* variables would be NULL. Confirm this is the intended behavior (e.g., for the first fill).
Please verify:
- The position size calculation should use
sumOpen - sumClose(without absolutes) or follow the actual position accounting model. - Confirm whether NULL snapshots for a non-existent perpetual_position record is intentional (valid for initial fills).
I'll now verify the concerns raised in the review comment by examining the codebase.
---
🏁 Script executed:
```shell
#!/bin/bash
# Find the dydx_apply_fill_realized_effects function to understand what parameters it expects
rg -n "CREATE.*FUNCTION.*dydx_apply_fill_realized_effects" indexer/
Length of output: 142
🏁 Script executed:
#!/bin/bash
# Search for other snap_size or position size calculations to compare
rg -n "snap_size|sumOpen.*sumClose" indexer/services/ender/src/scripts/ -A2 -B2Length of output: 4422
🏁 Script executed:
#!/bin/bash
# Look for perpetual_positions table definition and comments
rg -n "perpetual_positions" indexer/services/ender/src/scripts/ -C3 | head -100Length of output: 12177
🏁 Script executed:
#!/bin/bash
# Find other position size or fill calculations to understand the pattern
rg -n "positionSizeBefore|entryPriceBefore" indexer/services/ender/src/scripts/ -C2Length of output: 1841
🏁 Script executed:
#!/bin/bash
# Check if there are tests or migrations that show the expected behavior
fd -e "sql" indexer/services/ender/src/db/migrations/ | xargs rg -l "snap_size|perpetual_position" 2>/dev/null | head -5Length of output: 307
---
🏁 Script executed:
```shell
#!/bin/bash
# Read the full dydx_apply_fill_realized_effects function
cat indexer/services/ender/src/scripts/helpers/dydx_apply_fill_realized_effects.sql
Length of output: 1575
🏁 Script executed:
#!/bin/bash
# Look for fills table definition to understand positionSizeBefore column
rg -n "CREATE TABLE.*fills" indexer/ -A50 | grep -E "(CREATE TABLE|positionSizeBefore|COMMENT)" | head -30Length of output: 0
🏁 Script executed:
#!/bin/bash
# Search for comments or schema documentation about perpetual_positions columns
rg -n "sumOpen|sumClose" indexer/ -B3 -A3 | grep -E "(sumOpen|sumClose|COMMENT|--)" | head -40Length of output: 2930
🏁 Script executed:
#!/bin/bash
# Look for how closing_amount is calculated in dydx_apply_fill_realized_effects
rg -n "closing_amount" indexer/services/ender/src/scripts/helpers/ -B5 -A5Length of output: 3109
---
🏁 Script executed:
```shell
#!/bin/bash
# Find perpetual_positions table definition in migrations or schema
fd -e "sql" indexer/services/ender/src/db/ | xargs grep -l "perpetual_positions" 2>/dev/null | head -5
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Look for perpetual_positions model or schema in TypeScript
fd -e "ts" indexer/ | xargs grep -l "perpetual.*position.*model\|PerpetualPosition" 2>/dev/null | grep -E "(model|schema|type)" | head -5Length of output: 337
🏁 Script executed:
#!/bin/bash
# Search for documentation or comments about position size calculation
rg -n "net.*position|position.*size|sumOpen.*sumClose" indexer/ -B2 -A2 | head -80Length of output: 10505
🏁 Script executed:
#!/bin/bash
# Check if there are any position reversals (where sign changes) in test data
rg -n "sumOpen.*sumClose" indexer/ --type ts -A2 -B2 | grep -E "(sumOpen|sumClose|Long|Short)" | head -50Length of output: 137
🏁 Script executed:
#!/bin/bash
# Look for how perpetual_position records are created/updated to understand these fields
rg -n "INSERT INTO perpetual_positions|UPDATE perpetual_positions" indexer/services/ender/src/scripts/ -B3 -A8 | head -80Length of output: 9961
Remove redundant ABS() function wrapping in position size calculation.
The snap_size_before calculation at line 214 uses ABS(sumOpen) - ABS(sumClose), but based on the position update logic (see dydx_update_perpetual_position_aggregate_fields.sql), both sumOpen and sumClose are cumulative counters that only increase via addition. The ABS() wrapping is unnecessary and obscures the intent.
The correct formula is simply: sumOpen - sumClose (net position size).
Rewrite as:
snap_size_before = COALESCE(read_perpetual_position_record."sumOpen", 0) - COALESCE(read_perpetual_position_record."sumClose", 0);Apply the same fix to dydx_liquidation_fill_handler_per_order.sql line 225, which duplicates this pattern.
🤖 Prompt for AI Agents
In
indexer/services/ender/src/scripts/helpers/dydx_order_fill_handler_per_order.sql
around lines 206 to 216, the calculation for snap_size_before uses ABS() around
sumOpen and sumClose which is redundant because both fields are cumulative
non-negative counters; replace the expression with
COALESCE(read_perpetual_position_record."sumOpen", 0) -
COALESCE(read_perpetual_position_record."sumClose", 0) to compute the net
position size without ABS and similarly update the duplicate occurrence in
indexer/services/ender/src/scripts/helpers/dydx_liquidation_fill_handler_per_order.sql
at line 225 to use the same COALESCE(sumOpen,0) - COALESCE(sumClose,0)
expression.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
indexer/services/ender/__tests__/handlers/order-fills/order-handler.test.ts(17 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
indexer/services/ender/__tests__/handlers/order-fills/order-handler.test.ts (4)
indexer/services/ender/__tests__/helpers/indexer-proto-helpers.ts (4)
createOrder(399-493)createKafkaMessageFromOrderFillEvent(495-528)expectFillInDatabase(571-653)expectPerpetualPosition(983-1015)indexer/packages/postgres/__tests__/helpers/constants.ts (3)
defaultSubaccountId(156-159)defaultSubaccountId2(160-163)defaultPerpetualPosition(530-546)indexer/services/ender/__tests__/helpers/constants.ts (6)
defaultSubaccountId(279-282)defaultSubaccountId2(283-286)defaultHeight(271-271)defaultTime(273-276)defaultTxHash(277-277)defaultDateTime(272-272)indexer/packages/v4-proto-parser/src/constants.ts (2)
ORDER_FLAG_LONG_TERM(4-4)ORDER_FLAG_SHORT_TERM(3-3)
⏰ 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). (57)
- 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-auxo-lambda / (auxo) Build and Push Lambda
- 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-socks / (socks) 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-comlink / (comlink) Build and Push
- GitHub Check: build-and-push-mainnet
- 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-comlink / (comlink) 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: call-build-ecs-service-ender / (ender) Check docker image build
- 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-ecs-service-ender / (ender) Build and Push
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-vulcan / (vulcan) 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: check-build-auxo
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-ecs-service-roundtable / (roundtable) Build and Push
- GitHub Check: check-build-bazooka
- GitHub Check: test / run_command
- GitHub Check: run_command
- GitHub Check: build-and-push-testnet
- GitHub Check: lint
- GitHub Check: (Staging) Build and Push ECS Services / call-build-and-push-ecs-service-comlink / (comlink) Build and Push
- GitHub Check: (Staging) Build and Push ECS Services / call-build-and-push-ecs-service-ender / (ender) Build and Push
- GitHub Check: (Staging) Build and Push ECS Services / call-build-and-push-auxo-lambda / (auxo) Build and Push Lambda
- GitHub Check: (Staging) Build and Push ECS Services / call-build-and-push-bazooka-lambda / (bazooka) Build and Push Lambda
- GitHub Check: (Staging) Build and Push ECS Services / call-build-and-push-vulcan / (vulcan) Build and Push
- GitHub Check: (Staging) Build and Push ECS Services / call-build-and-push-ecs-service-roundtable / (roundtable) Build and Push
- GitHub Check: (Staging) Build and Push ECS Services / call-build-and-push-ecs-service-socks / (socks) Build and Push
- GitHub Check: (Dev4) Build and Push ECS Services / call-build-and-push-auxo-lambda / (auxo) Build and Push Lambda
- GitHub Check: (Dev4) Build and Push ECS Services / call-build-and-push-ecs-service-roundtable / (roundtable) Build and Push
- GitHub Check: (Dev4) Build and Push ECS Services / call-build-and-push-bazooka-lambda / (bazooka) Build and Push Lambda
- GitHub Check: (Dev4) Build and Push ECS Services / call-build-and-push-vulcan / (vulcan) Build and Push
- GitHub Check: (Dev4) Build and Push ECS Services / call-build-and-push-ecs-service-socks / (socks) Build and Push
- GitHub Check: (Dev4) Build and Push ECS Services / call-build-and-push-ecs-service-comlink / (comlink) Build and Push
- GitHub Check: (Dev4) Build and Push ECS Services / call-build-and-push-ecs-service-ender / (ender) Build and Push
- GitHub Check: (Dev2) Build and Push ECS Services / call-build-and-push-auxo-lambda / (auxo) Build and Push Lambda
- GitHub Check: (Dev2) Build and Push ECS Services / call-build-and-push-bazooka-lambda / (bazooka) Build and Push Lambda
- GitHub Check: (Dev2) Build and Push ECS Services / call-build-and-push-ecs-service-ender / (ender) Build and Push
- GitHub Check: (Dev2) Build and Push ECS Services / call-build-and-push-vulcan / (vulcan) Build and Push
- GitHub Check: (Dev2) Build and Push ECS Services / call-build-and-push-ecs-service-roundtable / (roundtable) Build and Push
- GitHub Check: (Dev2) Build and Push ECS Services / call-build-and-push-ecs-service-socks / (socks) Build and Push
- GitHub Check: (Dev2) Build and Push ECS Services / call-build-and-push-ecs-service-comlink / (comlink) Build and Push
- GitHub Check: (Dev) Build and Push ECS Services / call-build-and-push-ecs-service-comlink / (comlink) Build and Push
- GitHub Check: (Dev) Build and Push ECS Services / call-build-and-push-ecs-service-ender / (ender) Build and Push
- GitHub Check: (Dev) Build and Push ECS Services / call-build-and-push-bazooka-lambda / (bazooka) Build and Push Lambda
- GitHub Check: (Dev) Build and Push ECS Services / call-build-and-push-vulcan / (vulcan) Build and Push
- GitHub Check: (Dev) Build and Push ECS Services / call-build-and-push-auxo-lambda / (auxo) Build and Push Lambda
- GitHub Check: (Dev) Build and Push ECS Services / call-build-and-push-ecs-service-socks / (socks) Build and Push
- GitHub Check: (Dev) Build and Push ECS Services / call-build-and-push-ecs-service-roundtable / (roundtable) Build and Push
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Summary
🔇 Additional comments (3)
indexer/services/ender/__tests__/handlers/order-fills/order-handler.test.ts (3)
374-376: LGTM: Before-position fields and totalRealizedPnl correctly added.The assertions properly validate the new snapshot fields using values from
defaultPerpetualPosition. The totalRealizedPnl calculations are correct: maker pays only the fee (-1) when adding to position, while taker realizes a loss of -0.5 from price movement plus -2 in fees, totaling -2.5.Also applies to: 396-398, 460-460, 471-471
2475-2589: Excellent test coverage for before-position snapshot fields.This new test comprehensively validates that position state is correctly captured at fill execution time for both maker and taker sides. The test properly:
- Sets up initial positions with known state
- Executes a fill that affects both subaccounts
- Verifies all three snapshot fields (size, entry price, side) are persisted correctly
2591-2666: Well-designed test for totalRealizedPnl tracking.This test properly validates that realized P&L is accumulated when a reduce-only order partially closes a position. The expected value of
-2.5000correctly accounts for both the price difference loss (-0.5) and taker fee (-2).
indexer/services/ender/__tests__/handlers/order-fills/order-handler.test.ts
Show resolved
Hide resolved
UnbornAztecKing
approved these changes
Nov 6, 2025
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
indexer/services/comlink/public/swagger.json (2)
640-648: Consider using PositionSide enum reference for type safety.Line 647 defines
positionSideBeforeas a plain string type, but elsewhere in the schema (e.g., line 35-36),sideproperties reference thePositionSideenum to enforce valid values ("LONG" or "SHORT"). For consistency and better API contract clarity, consider using$ref:"positionSideBefore": { - "type": "string" + "$ref": "#/components/schemas/PositionSide" }
640-648: Clarify nullability of optional "before" snapshot fields.The three new snapshot fields (
positionSizeBefore,entryPriceBefore,positionSideBefore) are optional and not marked as required, which is appropriate. However, they are also not marked asnullable: true. For opening fills (where no prior position exists), these fields may need to represent an absent or null state. Consider adding"nullable": trueto clarify the API contract:"positionSizeBefore": { - "type": "string" + "type": "string", + "nullable": true }, "entryPriceBefore": { - "type": "string" + "type": "string", + "nullable": true },Alternatively, if these fields are always present (even if zero/empty for opening fills), no change is needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
indexer/services/comlink/public/api-documentation.md(5 hunks)indexer/services/comlink/public/swagger.json(1 hunks)indexer/services/comlink/src/types.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- indexer/services/comlink/src/types.ts
- indexer/services/comlink/public/api-documentation.md
⏰ 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-auxo-lambda / (auxo) 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-bazooka-lambda / (bazooka) Build and Push Lambda
- 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-ecs-service-roundtable / (roundtable) Build and Push
- GitHub Check: (Public Testnet) 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-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: (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-bazooka-lambda / (bazooka) Build and Push Lambda
- GitHub Check: (Public Testnet) Build and Push ECS Services / call-build-and-push-vulcan / (vulcan) Build and Push
- 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-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: call-build-ecs-service-comlink / (comlink) Check docker image build
- GitHub Check: call-build-ecs-service-roundtable / (roundtable) 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-ender / (ender) Check docker image build
- GitHub Check: check-build-bazooka
- GitHub Check: test / run_command
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: check-build-auxo
- GitHub Check: Analyze (go)
- GitHub Check: run_command
- GitHub Check: lint
- GitHub Check: build-and-push-testnet
- GitHub Check: build-and-push-mainnet
- GitHub Check: Summary
Contributor
Author
|
@mergify backport release/indexer/v9.x |
Contributor
✅ Backports have been createdDetails
|
6 tasks
6 tasks
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
2 participants
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.
Changelist
Test Plan
Added new tests to check for the inclusion of the before field entries and updated existing ones to support the new fill schema.
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
Documentation