feat(funding): Add defaultFunding8hrPpm field to Perpetual Create/Update indexer events
#2674
Conversation
WalkthroughThis pull request introduces a comprehensive update to the system's handling of perpetual market funding rates. The changes span multiple packages and services, adding a new Changes
Sequence DiagramsequenceDiagram
participant Client
participant IndexerService
participant DatabaseLayer
participant EventProcessor
Client->>IndexerService: Request Perpetual Market Data
IndexerService->>DatabaseLayer: Fetch Market with defaultFundingRate1H
DatabaseLayer-->>IndexerService: Return Market Data
IndexerService->>EventProcessor: Process Market Creation Event
EventProcessor->>EventProcessor: Calculate Funding Rate
EventProcessor-->>IndexerService: Emit Updated Market Event
IndexerService-->>Client: Return Market Information
Possibly related PRs
Suggested Labels
Suggested Reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (10)
🔇 Additional comments (3)
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
teddyding
changed the title
defaultFunding8hrPpm field to perpetual indexer eventsdefaultFunding8hrPpm field to perpetual create and update indexer events
teddyding
changed the title
defaultFunding8hrPpm field to perpetual create and update indexer eventsdefaultFunding8hrPpm field to Perpetual Create/Update indexer events
teddyding
changed the title
defaultFunding8hrPpm field to Perpetual Create/Update indexer eventsdefaultFunding8hrPpm field to Perpetual Create/Update indexer events
teddyding
changed the title
defaultFunding8hrPpm field to Perpetual Create/Update indexer eventsdefaultFunding8hrPpm field to Perpetual Create/Update indexer events
teddyding
force-pushed
the
td/default-funding-indexer
branch
2 times, most recently
from
f2754f1 to
a5349cd
Compare
teddyding
force-pushed
the
td/default-funding-indexer
branch
from
a5349cd to
84dc866
Compare
teddyding
changed the title
defaultFunding8hrPpm field to Perpetual Create/Update indexer eventsdefaultFunding8hrPpm field to Perpetual Create/Update indexer events
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🔭 Outside diff range comments (1)
indexer/services/ender/src/validators/update-perpetual-validator.ts (1)
Line range hint
25-31: Fix type inconsistency in createHandlers method.The method's return type
Handler<UpdatePerpetualEventV1>[]doesn't match the class's type parameter which includes V2 and V3 events. This could lead to type errors when handling newer event versions.- ): Handler<UpdatePerpetualEventV1>[] { - const handler: Handler<UpdatePerpetualEventV1> = new UpdatePerpetualHandler( + ): Handler<UpdatePerpetualEventV1 | UpdatePerpetualEventV2 | UpdatePerpetualEventV3>[] { + const handler: Handler<UpdatePerpetualEventV1 | UpdatePerpetualEventV2 | UpdatePerpetualEventV3> = new UpdatePerpetualHandler(
🧹 Nitpick comments (10)
protocol/indexer/events/perpetual.go (1)
8-8: Update function documentation to include the new parameter.The documentation comment should describe the
defaultFundingPpmparameter and its purpose.-// NewUpdatePerpetualEvent creates a UpdatePerpetualEventV3 representing -// update of a perpetual. +// NewUpdatePerpetualEvent creates a UpdatePerpetualEventV3 representing +// update of a perpetual. +// +// Parameters: +// - defaultFundingPpm: The default 8-hour funding rate in parts per millionprotocol/indexer/events/perpetual_test.go (1)
20-20: Add test cases for edge values of defaultFundingPpm.Consider adding test cases for:
- Maximum allowed funding rate
- Minimum (negative) funding rate
- Zero funding rate
protocol/indexer/events/perpetual_market_create_test.go (2)
26-26: Document the significance of the default funding value.The test uses a value of 100 for
defaultFundingPpm. Consider adding a comment explaining why this specific value was chosen and what it represents in the context of funding rates.
Line range hint
28-41: Consider adding more test cases for the funding rate field.While the happy path is tested, consider adding test cases for:
- Edge cases (e.g., maximum/minimum allowed values)
- Negative values
- Zero value
indexer/services/ender/__tests__/handlers/update-perpetual-handler.test.ts (1)
17-17: LGTM! Handler tests properly extended for V3 events.The test suite is correctly updated to include the new V3 event type, maintaining consistency with existing test patterns.
Consider adding specific test assertions for the new
defaultFundingRate1Hfield in theperpetualMarketobject expectations, similar to how market type is conditionally checked:expect(perpetualMarket).toEqual(expect.objectContaining({ id: event.id.toString(), ticker: event.ticker, marketId: event.marketId, atomicResolution: event.atomicResolution, liquidityTierId: event.liquidityTier, // Add V2-specific field expectations when testing V2 events ...('marketType' in event && { marketType: eventPerpetualMarketTypeToIndexerPerpetualMarketType(event.marketType), }), + // Add V3-specific field expectations when testing V3 events + ...('defaultFunding8hrPpm' in event && { + defaultFundingRate1H: event.defaultFunding8hrPpm, + }), }));Also applies to: 25-25, 76-80, 84-85
indexer/services/comlink/src/types.ts (1)
355-355: LGTM: Optional defaultFundingRate1H field added to interfaceThe addition of the optional
defaultFundingRate1Hfield to thePerpetualMarketResponseObjectinterface is well-structured. Consider adding JSDoc comments to document the field's purpose and format.Add JSDoc comments to describe the field:
+ /** + * The default 1-hour funding rate for the perpetual market. + * Represented as a string decimal, e.g., "0.0001" for 0.01% per hour. + */ defaultFundingRate1H?: string,indexer/packages/v4-protos/src/codegen/dydxprotocol/indexer/events/events.ts (1)
1111-1186: New V3 interfaces look good, but could use more detailed documentation.The new interfaces properly extend their V2 counterparts with the
defaultFunding8hrPpmfield. Consider adding more detailed documentation for the new field:
- Valid range of values
- Impact on funding rate calculations
- Example values and their real-world interpretations
Also applies to: 1583-1627
indexer/packages/postgres/README.md (1)
10-11: Improve clarity of migration testing guidanceThe added note about test failures is helpful but could be clearer. Consider rewording to:
"Note: Test cases in__tests__/db/migrations.test.tsmay fail when a model is modified by a new migration. In such cases, comment out the affected test cases until the model changes are finalized."🧰 Tools
🪛 LanguageTool
[style] ~11-~11: Consider a shorter alternative to avoid wordiness.
Context: ... modified due to the latest migration. In order to migrate in dev and staging, you must re...(IN_ORDER_TO_PREMIUM)
indexer/services/ender/package.json (1)
10-10: Consider improving build:watch script robustnessThe build:watch script could be made more robust by:
- Using a cross-platform rimraf instead of rm -rf for better Windows compatibility
- Handling the cp command failure gracefully
- Consider using a package.json script for the cleanup steps
Example:
{ "scripts": { "clean": "rimraf build/", "copy-scripts": "cp -r src/scripts build/src/scripts", "build:watch": "pnpm run clean && (tsc --watch & pnpm run copy-scripts)" } }indexer/services/comlink/public/swagger.json (1)
1141-1143: Consider adding validation constraints for the rate field.The
defaultFundingRate1Hfield is defined as a string without any validation constraints. Consider adding:
- Pattern constraint to ensure valid numeric string format
- Description field to document the expected format and valid range
"defaultFundingRate1H": { - "type": "string" + "type": "string", + "pattern": "^-?\\d*\\.?\\d+$", + "description": "Default 1-hour funding rate as a decimal string (e.g., '0.0001' for 0.01%)" }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
protocol/indexer/events/events.pb.gois excluded by!**/*.pb.goprotocol/x/perpetuals/types/perpetual.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (61)
indexer/packages/postgres/README.md(1 hunks)indexer/packages/postgres/__tests__/db/migrations.test.ts(1 hunks)indexer/packages/postgres/__tests__/helpers/constants.ts(5 hunks)indexer/packages/postgres/src/db/migrations/migration_files/20250107145033_default_1hr_funding_for_perp.ts(1 hunks)indexer/packages/postgres/src/models/perpetual-market-model.ts(3 hunks)indexer/packages/postgres/src/types/db-model-types.ts(1 hunks)indexer/packages/postgres/src/types/perpetual-market-types.ts(2 hunks)indexer/packages/postgres/src/types/websocket-message-types.ts(1 hunks)indexer/packages/v4-protos/src/codegen/dydxprotocol/indexer/events/events.ts(8 hunks)indexer/packages/v4-protos/src/codegen/dydxprotocol/perpetuals/perpetual.ts(2 hunks)indexer/services/comlink/__tests__/lib/request-helpers/request-transformer.test.ts(1 hunks)indexer/services/comlink/public/api-documentation.md(6 hunks)indexer/services/comlink/public/swagger.json(1 hunks)indexer/services/comlink/src/request-helpers/request-transformer.ts(1 hunks)indexer/services/comlink/src/types.ts(1 hunks)indexer/services/ender/__tests__/handlers/perpetual-market-handler.test.ts(3 hunks)indexer/services/ender/__tests__/handlers/update-perpetual-handler.test.ts(2 hunks)indexer/services/ender/__tests__/helpers/constants.ts(4 hunks)indexer/services/ender/__tests__/helpers/indexer-proto-helpers.ts(2 hunks)indexer/services/ender/__tests__/helpers/notification-functions.test.ts(1 hunks)indexer/services/ender/__tests__/lib/on-message.test.ts(1 hunks)indexer/services/ender/__tests__/validators/perpetual-market-validator.test.ts(4 hunks)indexer/services/ender/__tests__/validators/update-perpetual-validator.test.ts(3 hunks)indexer/services/ender/package.json(1 hunks)indexer/services/ender/src/handlers/perpetual-market-handler.ts(3 hunks)indexer/services/ender/src/helpers/kafka-helper.ts(1 hunks)indexer/services/ender/src/helpers/postgres/postgres-functions.ts(2 hunks)indexer/services/ender/src/lib/block-processor.ts(1 hunks)indexer/services/ender/src/lib/helper.ts(3 hunks)indexer/services/ender/src/lib/types.ts(4 hunks)indexer/services/ender/src/scripts/handlers/dydx_block_processor_ordered_handlers.sql(2 hunks)indexer/services/ender/src/scripts/handlers/dydx_perpetual_market_v3_handler.sql(1 hunks)indexer/services/ender/src/scripts/handlers/dydx_update_perpetual_v3_handler.sql(1 hunks)indexer/services/ender/src/validators/update-perpetual-validator.ts(1 hunks)indexer/services/roundtable/__tests__/helpers/pnl-ticks-helper.test.ts(1 hunks)indexer/services/roundtable/src/lib/athena-ddl-tables/perpetual_markets.ts(2 hunks)indexer/services/roundtable/src/tasks/market-updater.ts(1 hunks)proto/dydxprotocol/indexer/events/events.proto(5 hunks)proto/dydxprotocol/perpetuals/perpetual.proto(1 hunks)protocol/indexer/events/constants.go(1 hunks)protocol/indexer/events/perpetual.go(2 hunks)protocol/indexer/events/perpetual_market_create.go(2 hunks)protocol/indexer/events/perpetual_market_create_test.go(2 hunks)protocol/indexer/events/perpetual_test.go(1 hunks)protocol/testutil/keeper/clob.go(1 hunks)protocol/x/clob/abci_test.go(2 hunks)protocol/x/clob/genesis_test.go(1 hunks)protocol/x/clob/keeper/clob_pair.go(1 hunks)protocol/x/clob/keeper/clob_pair_test.go(8 hunks)protocol/x/clob/keeper/deleveraging_test.go(2 hunks)protocol/x/clob/keeper/get_price_premium_test.go(1 hunks)protocol/x/clob/keeper/liquidations_test.go(8 hunks)protocol/x/clob/keeper/msg_server_create_clob_pair_test.go(1 hunks)protocol/x/clob/keeper/msg_server_place_order_test.go(2 hunks)protocol/x/clob/keeper/orders_test.go(4 hunks)protocol/x/clob/keeper/process_operations_test.go(1 hunks)protocol/x/clob/module_test.go(1 hunks)protocol/x/listing/keeper/listing.go(1 hunks)protocol/x/listing/keeper/msg_server_upgrade_isolated_perpetual_to_cross_test.go(2 hunks)protocol/x/perpetuals/keeper/perpetual.go(1 hunks)protocol/x/perpetuals/keeper/perpetual_test.go(3 hunks)
✅ Files skipped from review due to trivial changes (3)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/perpetuals/perpetual.ts
- proto/dydxprotocol/perpetuals/perpetual.proto
- protocol/x/clob/keeper/orders_test.go
🧰 Additional context used
📓 Learnings (4)
indexer/services/ender/src/helpers/postgres/postgres-functions.ts (2)
Learnt from: hwray
PR: dydxprotocol/v4-chain#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.
Learnt from: hwray
PR: dydxprotocol/v4-chain#2597
File: indexer/services/ender/src/helpers/postgres/postgres-functions.ts:47-48
Timestamp: 2024-11-22T18:12:38.778Z
Learning: Prefer to maintain old `v1` handler files, such as `dydx_update_perpetual_handler.sql`, instead of removing them, even when new versioned handlers are added.
indexer/services/ender/src/scripts/handlers/dydx_block_processor_ordered_handlers.sql (1)
Learnt from: hwray
PR: dydxprotocol/v4-chain#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.
indexer/services/ender/src/scripts/handlers/dydx_update_perpetual_v3_handler.sql (1)
Learnt from: hwray
PR: dydxprotocol/v4-chain#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.
indexer/services/ender/src/scripts/handlers/dydx_perpetual_market_v3_handler.sql (1)
Learnt from: hwray
PR: dydxprotocol/v4-chain#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.
🪛 LanguageTool
indexer/packages/postgres/README.md
[style] ~11-~11: Consider a shorter alternative to avoid wordiness.
Context: ... modified due to the latest migration. In order to migrate in dev and staging, you must re...
(IN_ORDER_TO_PREMIUM)
🪛 Biome (1.9.4)
indexer/services/ender/__tests__/handlers/perpetual-market-handler.test.ts
[error] 106-106: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
🪛 GitHub Actions: Protocol Lint
protocol/x/listing/keeper/msg_server_upgrade_isolated_perpetual_to_cross_test.go
[error] 236-236: Line length exceeds maximum limit: The line is 123 characters long, which exceeds the maximum of 120 characters
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: test-sim-multi-seed-short
- GitHub Check: test-sim-after-import
- GitHub Check: test-sim-import-export
- GitHub Check: unit-end-to-end-and-integration
- GitHub Check: test-race
- GitHub Check: test-coverage-upload
- GitHub Check: liveness-test
- GitHub Check: container-tests
- GitHub Check: Summary
🔇 Additional comments (62)
indexer/services/roundtable/src/lib/athena-ddl-tables/perpetual_markets.ts (1)
25-27: Verify naming convention for funding rate fields.There's an inconsistency in naming between the protocol layer (
DefaultFunding8HrPpm) and the database schema (defaultFundingRate1H). This suggests there might be a conversion happening between 8-hour and 1-hour rates. Please ensure:
- The conversion logic is properly documented
- The relationship between these rates is clear in the codebase
- Consider adding a comment explaining the conversion factor
Run this script to find related conversion logic:
Also applies to: 45-47
✅ Verification successful
Conversion between 8hr and 1hr funding rates is properly implemented.
The conversion from protocol's
DefaultFunding8HrPpmto database'sdefaultFundingRate1His implemented in the indexer:
- 8hr rate in PPM is converted to decimal:
rate_ppm / 1_000_000- 8hr rate is converted to 1hr rate:
8hr_rate / 8The naming difference reflects the different time periods and formats used in each layer. Consider adding a comment explaining this conversion in the database schema.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for funding rate conversion logic between 8hr and 1hr rates rg -A 5 -e "8[Hh]r.*1[Hh]|1[Hh]r.*8[Hh]r" --type ts --type goLength of output: 65600
Script:
#!/bin/bash # Search for DefaultFunding8HrPpm and defaultFundingRate1H definitions and usages rg -A 3 "DefaultFunding8HrPpm|defaultFundingRate1H" --type ts --type go # Also search for any comments or code mentioning funding rate conversion rg -A 3 "funding.*rate.*conversion|convert.*funding.*rate" --type ts --type goLength of output: 14144
protocol/indexer/events/constants.go (1)
35-35: LGTM! Version updates align with the new field addition.The version increments for both
PerpetualMarketEventVersionandUpdatePerpetualEventVersioncorrectly reflect the addition of thedefaultFunding8hrPpmfield to these events.Also applies to: 37-37
indexer/packages/postgres/__tests__/db/migrations.test.ts (2)
26-35: LGTM! Clear test description for the migration test.The test description clearly indicates that this is testing both UP and DOWN migrations without seed data.
37-50: Well-documented test skips with clear guidance.The skip messages are properly documented with:
- Clear warning about model modifications
- Reference to README for more information
- Descriptive test names indicating their purpose
This helps developers understand why tests are skipped and how to handle them in future migrations.
Also applies to: 53-67
indexer/services/ender/__tests__/validators/update-perpetual-validator.test.ts (1)
7-7: LGTM! Proper import and constant additions.The new
UpdatePerpetualEventV3is correctly imported and its default constant is added.Also applies to: 16-16
indexer/packages/postgres/src/models/perpetual-market-model.ts (1)
89-89: LGTM! Well-structured model changes for the new funding rate field.The
defaultFundingRate1Hfield is properly implemented with:
- Correct JSON schema definition including null handling and pattern validation
- Proper SQL to JSON conversion mapping
- Optional class property
Also applies to: 119-119, 157-157
indexer/services/ender/src/helpers/postgres/postgres-functions.ts (1)
41-41: LGTM! Handler scripts for V3 events added correctly.The new handler scripts for V3 perpetual market events are added consistently with the existing pattern.
Also applies to: 50-50
indexer/services/ender/__tests__/helpers/notification-functions.test.ts (1)
54-54: LGTM! Test mock updated with new funding rate field.The
defaultFundingRate1Hfield is added to the mockMarket object with a sensible default value.indexer/services/ender/__tests__/validators/perpetual-market-validator.test.ts (1)
5-5: LGTM! Test coverage properly extended for V3 events.The test suite is correctly updated to include validation for the new V3 event type, maintaining consistency with existing test patterns.
Also applies to: 16-16, 54-59, 64-65, 95-96, 104-105, 113-114, 119-120
indexer/services/roundtable/src/tasks/market-updater.ts (1)
73-73: Verify the TODO implementation plan for default funding rate.The TODO comment (CT-1340) indicates that default funding rate needs to be added to the next funding value. This aligns with the PR objectives of introducing the
defaultFunding8hrPpmfield.Would you like me to help implement the logic for adding the default funding rate to the next funding value? This would ensure consistency with the new
defaultFunding8hrPpmfield being introduced.protocol/x/clob/keeper/msg_server_create_clob_pair_test.go (1)
67-67: LGTM! DefaultFundingPpm correctly added to event creation test.The test has been properly updated to include the new
DefaultFundingPpmfield in theNewPerpetualMarketCreateEventcreation, ensuring the new field is properly tested.indexer/packages/postgres/src/types/websocket-message-types.ts (1)
220-220: LGTM! Type definition properly added for defaultFundingRate1H.The optional string field
defaultFundingRate1Hhas been correctly added to theTradingPerpetualMarketMessageinterface, maintaining consistency with other funding-related fields.protocol/x/clob/keeper/get_price_premium_test.go (1)
188-188: LGTM! DefaultFundingPpm correctly added to price premium test.The test has been properly updated to include the new
DefaultFundingPpmfield in theNewPerpetualMarketCreateEventcreation, maintaining consistency with other test files.indexer/services/ender/__tests__/handlers/perpetual-market-handler.test.ts (3)
4-4: LGTM! Import statements are correctly updated.The imports are properly organized to include the new V3 event type and its corresponding helper functions.
Also applies to: 33-33, 39-39
95-101: LGTM! Test case for V3 event is properly structured.The test case follows the existing pattern and correctly includes all necessary parameters for testing the V3 event.
107-108: LGTM! Type signature is properly updated.The function parameter type is correctly extended to include the V3 event type.
protocol/x/listing/keeper/listing.go (1)
206-206: LGTM! Default funding parameter is correctly included.The
DefaultFundingPpmis properly sourced from the perpetual's params and included in the event emission, aligning with the PR objectives.indexer/services/ender/src/lib/types.ts (2)
10-10: LGTM! Import statements are correctly updated.The imports properly include the new V3 event types.
Also applies to: 36-36
133-138: LGTM! Event type definitions are properly extended.The union type definitions correctly include the V3 events while maintaining the existing structure.
Also applies to: 163-168
indexer/packages/postgres/src/types/db-model-types.ts (1)
98-98: LGTM! Database model type is properly extended.The
defaultFundingRate1Hfield is correctly defined as an optional string field and follows the interface's naming convention.protocol/x/listing/keeper/msg_server_upgrade_isolated_perpetual_to_cross_test.go (1)
Line range hint
250-265: LGTM!The function has been correctly updated to handle the new
UpdatePerpetualEventV3type while maintaining the existing logic and error handling.protocol/testutil/keeper/clob.go (1)
351-351: LGTM!The mock indexer event has been correctly updated to include the new
DefaultFundingPpmfield.indexer/services/ender/src/helpers/kafka-helper.ts (1)
350-350: LGTM!The perpetual market message has been correctly updated to include the new
defaultFundingRate1Hfield.indexer/services/ender/src/lib/helper.ts (3)
21-21: LGTM!The new event types are correctly imported and properly organized within the import statements.
Also applies to: 26-26
176-183: LGTM!The version 3 case for perpetual market events is correctly implemented, following the existing pattern and maintaining consistent error handling.
228-235: LGTM!The version 3 case for update perpetual events is correctly implemented, following the existing pattern and maintaining consistent error handling.
indexer/services/comlink/__tests__/lib/request-helpers/request-transformer.test.ts (1)
84-84: LGTM! Test updated to include new funding rate field.The test correctly verifies that
defaultFundingRate1His included in the response object fromperpetualMarketToResponseObject.indexer/services/ender/src/lib/block-processor.ts (1)
52-52: LGTM! Added validators for version 3 perpetual market events.The block processor has been updated to handle version 3 of both
PERPETUAL_MARKETandUPDATE_PERPETUALevents, which include the new funding rate field.Also applies to: 56-56
indexer/services/ender/__tests__/helpers/constants.ts (2)
165-178: LGTM! Added test constant for PerpetualMarketCreateEventV3.The new constant correctly includes all required fields including the new
defaultFunding8hrPpmfield.
229-237: LGTM! Added test constant for UpdatePerpetualEventV3.The new constant correctly includes all required fields including the new
defaultFunding8hrPpmfield, maintaining consistency with the create event constant.protocol/x/clob/genesis_test.go (1)
456-456: LGTM! Updated genesis test to include funding parameter.The test has been properly updated to include
defaultFundingPpmwhen creating perpetual market events during genesis initialization.protocol/x/clob/module_test.go (1)
313-313: LGTM: DefaultFundingPpm parameter correctly added to event creationThe addition of
perpetual.Params.DefaultFundingPpmto theNewPerpetualMarketCreateEventcall is consistent with the PR's objective of including funding rate information in perpetual market events.indexer/services/roundtable/__tests__/helpers/pnl-ticks-helper.test.ts (1)
385-385: LGTM: Test data updated with defaultFundingRate1HThe addition of
defaultFundingRate1H: '0.0001'to the test data is appropriate and uses a realistic value (0.01% per hour) for testing purposes.protocol/x/clob/keeper/msg_server_place_order_test.go (2)
186-186: LGTM: DefaultFundingPpm parameter added to error test caseThe addition of
perpetual.Params.DefaultFundingPpmto the event creation in the error test case ensures complete test coverage of the funding rate information.
338-338: LGTM: DefaultFundingPpm parameter added to success test caseThe addition of
perpetual.Params.DefaultFundingPpmto the event creation in the success test case maintains consistency with the error test case and ensures proper testing of the funding rate information.indexer/services/comlink/src/request-helpers/request-transformer.ts (1)
392-392: LGTM!The addition of
defaultFundingRate1Hto the response object is straightforward and correctly maps the field from the database object.protocol/x/clob/keeper/clob_pair.go (1)
164-164: LGTM!The addition of
DefaultFundingPpmto the perpetual market create event is correct and aligns with the PR objectives.indexer/services/ender/__tests__/helpers/indexer-proto-helpers.ts (1)
975-997: Verify the funding rate calculation.The conversion of
defaultFunding8hrPpmtodefaultFundingRate1Hinvolves two steps:
- Converting PPM (parts per million) to decimal by dividing by 1,000,000
- Converting 8-hour rate to 1-hour rate by dividing by 8
Please verify that this calculation aligns with the business requirements and protocol specifications.
Run this script to find other instances of funding rate calculations in the codebase:
✅ Verification successful
Funding rate calculation verified ✓
The conversion of
defaultFunding8hrPpmtodefaultFundingRate1His correctly implemented and consistent with the protocol specifications, as evidenced by multiple implementations and test cases throughout the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find other instances of funding rate calculations to verify consistency # Search for files containing both "funding" and "ppm" or "rate" rg -l "funding.*(?:ppm|rate)" | xargs rg "(?:ppm|rate).*funding"Length of output: 5761
indexer/services/ender/__tests__/lib/on-message.test.ts (1)
255-256: LGTM! Test name is now more descriptive.The test case name has been updated to be more specific about the version being tested (
PerpetualMarketCreateV2), which improves test clarity.protocol/x/clob/keeper/clob_pair_test.go (1)
Line range hint
530-530: LGTM! Test coverage for default funding parameter.The test cases correctly include
DefaultFundingPpmin perpetual market creation events, maintaining test coverage for the new funding parameter.Also applies to: 563-564
protocol/x/perpetuals/keeper/perpetual.go (1)
176-176: LGTM! Funding parameter included in update events.The
DefaultFundingPpmis correctly included in the perpetual update event, ensuring consistency with the event schema.protocol/x/clob/abci_test.go (1)
530-530: LGTM! Test coverage for default funding parameter.The test cases correctly include
DefaultFundingPpmin event expectations, maintaining test coverage for the new funding parameter.Also applies to: 563-564
protocol/x/clob/keeper/deleveraging_test.go (1)
288-288: LGTM! Test setup correctly updated to include DefaultFundingPpm parameter.The changes correctly update the mock event creation to include the new
DefaultFundingPpmparameter from the perpetual's parameters, ensuring test coverage of the new event field.Also applies to: 686-686
protocol/x/perpetuals/keeper/perpetual_test.go (1)
Line range hint
45-71: LGTM! Test code correctly updated to use V3 event type with DefaultFunding8HrPpm field.The changes properly update the test code to:
- Use the new
UpdatePerpetualEventV3type instead of V2- Include expectations for the new
DefaultFunding8HrPpmfield- Update helper functions to handle the V3 event structure
Also applies to: 118-133
protocol/x/clob/keeper/process_operations_test.go (1)
2459-2459: LGTM! Test setup correctly updated to include DefaultFundingPpm parameter.The change properly updates the mock event creation to include the new
DefaultFundingPpmparameter from the perpetual's parameters, maintaining consistency with the new event structure.indexer/packages/v4-protos/src/codegen/dydxprotocol/indexer/events/events.ts (2)
962-966: Well-documented deprecation notices!The deprecation notices clearly indicate that users should use the V3 versions of these events instead.
Also applies to: 1496-1500
3579-3732: Clean and consistent implementations of the V3 events.The implementations:
- Properly handle all fields including the new
defaultFunding8hrPpm- Follow protobuf encoding/decoding patterns
- Include proper null checks and default values
- Use consistent type handling across both event types
Also applies to: 4084-4187
protocol/x/clob/keeper/liquidations_test.go (1)
1253-1253: LGTM: DefaultFundingPpm field added to test eventsThe changes correctly add the
DefaultFundingPpmfield toPerpetualMarketCreateEventin test cases, maintaining test coverage for the new field.Also applies to: 1284-1284
indexer/services/ender/src/handlers/perpetual-market-handler.ts (1)
6-9: LGTM: PerpetualMarketCreateEventV3 integrationThe addition of
PerpetualMarketCreateEventV3to imports and type union properly extends the handler to support the new event version.Also applies to: 19-19
proto/dydxprotocol/indexer/events/events.proto (3)
377-429: LGTM! Proper deprecation of V2 message.The deprecation of
PerpetualMarketCreateEventV2is well documented and follows protobuf best practices.
Line range hint
431-484: LGTM! Well-structured V3 message definition.The new
PerpetualMarketCreateEventV3message is properly documented and maintains consistent field numbering with the addition ofdefault_funding8hr_ppm.
Line range hint
579-641: LGTM! Consistent versioning pattern for UpdatePerpetualEvent.The changes follow the same versioning pattern, properly deprecating V2 and introducing V3 with the new funding rate field.
indexer/services/comlink/public/api-documentation.md (2)
2509-2510: LGTM! New fields added to PerpetualMarketResponseObjectThe addition of
baseOpenInterestanddefaultFundingRate1Hfields to the response schema is well-structured and follows the API's naming conventions.
5456-5457: LGTM! Consistent schema updates in PerpetualMarketResponseThe new fields are consistently added across all example properties in the markets object, maintaining the API documentation's structure and clarity.
Also applies to: 5480-5481
indexer/packages/postgres/__tests__/helpers/constants.ts (1)
288-288: LGTM! Consistent funding rate values.The
defaultFundingRate1Hvalues are consistently set across similar market types:
- '0' for default perpetual markets
- '0.0001' for isolated perpetual markets
Also applies to: 308-308, 328-328, 349-349, 370-370
indexer/services/ender/src/scripts/handlers/dydx_update_perpetual_v3_handler.sql (3)
12-13: LGTM! Well-defined constants for funding rate calculation.The constants are appropriately defined for converting 8-hour funding rate to 1-hour rate:
- PPM_EXPONENT = -6 for parts per million scaling
- FUNDING_RATE_FROM_PROTOCOL_IN_HOURS = 8 for converting from 8hr to 1hr rate
23-26: LGTM! Correct funding rate conversion formula.The calculation correctly:
- Scales the PPM value using
power(10, PPM_EXPONENT)- Converts from 8hr to 1hr rate by dividing by
FUNDING_RATE_FROM_PROTOCOL_IN_HOURS- Uses
dydx_trim_scalefor consistent decimal handling
38-40: LGTM! Proper error handling for non-existent markets.The function appropriately raises an exception when the perpetual market is not found.
indexer/services/ender/src/scripts/handlers/dydx_perpetual_market_v3_handler.sql (2)
21-25: LGTM! Proper initialization of market metrics.All market metrics are correctly initialized to zero:
- priceChange24H
- trades24H
- volume24H
- nextFundingRate
- openInterest
33-36: LGTM! Consistent funding rate calculation.The funding rate calculation is identical to the update handler, ensuring consistency in rate conversion across handlers.
indexer/services/ender/src/scripts/handlers/dydx_block_processor_ordered_handlers.sql (1)
59-60: LGTM! Proper integration of v3 handlers.The v3 handlers are correctly added to both event types:
- perpetual_market: Added case for version 3
- update_perpetual: Added case for version 3
The changes maintain the existing pattern and include proper fallback to NULL for unknown versions.
Also applies to: 72-73
indexer/services/comlink/public/swagger.json (1)
1141-1143: Verify field name consistency with PR objectives.The field name
defaultFundingRate1Hsuggests a 1-hour funding rate, while the PR objectives mentiondefaultFunding8hrPpm. This naming inconsistency could lead to confusion. Consider either:
- Renaming to match PR objectives if this field should represent 8-hour PPM rate
- Clarifying in PR objectives if this is an intentional conversion from 8-hour to 1-hour rate
✅ Verification successful
Field name is correct and consistent with codebase design
The field name
defaultFundingRate1Hin swagger.json is correct. The protocol internally uses 8-hour funding rates, but the indexer converts these to 1-hour rates for API responses, as evidenced by thefunding8HourValuePpmTo1HourRateconversion function in the codebase. This is a deliberate design pattern where 8hr protocol-level rates are converted to 1hr rates for client consumption.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for related funding rate fields and conversions # Look for other funding rate related fields and potential conversions between 1h and 8h rates rg -A 5 -B 5 'funding.*rate|ppm' --type ts --type goLength of output: 71463
|
|
||
| export async function up(knex: Knex): Promise<void> { | ||
| await knex.schema.alterTable('perpetual_markets', (table) => { | ||
| table.decimal('defaultFundingRate1H', null).defaultTo(0); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Specify decimal precision for funding rate column.
The decimal type without precision and scale could lead to data truncation or inconsistency. Consider specifying appropriate precision and scale parameters based on the expected range of funding rates.
- table.decimal('defaultFundingRate1H', null).defaultTo(0);
+ table.decimal('defaultFundingRate1H', 30, 18).defaultTo(0);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| table.decimal('defaultFundingRate1H', null).defaultTo(0); | |
| table.decimal('defaultFundingRate1H', 30, 18).defaultTo(0); |
| defaultFundingPpm int32, | ||
| ) *PerpetualMarketCreateEventV3 { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for defaultFundingPpm parameter.
Consider adding validation for the defaultFundingPpm parameter to ensure it falls within acceptable bounds for a funding rate. This could prevent issues with extreme or invalid values.
Example validation:
func NewPerpetualMarketCreateEvent(
// ... other parameters ...
defaultFundingPpm int32,
) *PerpetualMarketCreateEventV3 {
+ // Validate funding rate is within acceptable bounds
+ // For example: -10000 to 10000 for ±1% (10000 PPM = 1%)
+ if defaultFundingPpm < -10000 || defaultFundingPpm > 10000 {
+ panic(fmt.Sprintf("defaultFundingPpm %d is outside acceptable range [-10000, 10000]", defaultFundingPpm))
+ }
return &PerpetualMarketCreateEventV3{
// ... other fields ...
DefaultFunding8HrPpm: defaultFundingPpm,
}
}Committable suggestion skipped: line range outside the PR's diff.
| @@ -18,6 +18,7 @@ export interface PerpetualMarketCreateObject { | |||
| liquidityTierId: number, | |||
| marketType: PerpetualMarketType, | |||
| baseOpenInterest: string, | |||
| defaultFundingRate1H: string, | |||
There was a problem hiding this comment.
Add defaultFundingRate1H to PerpetualMarketUpdateObject.
The defaultFundingRate1H field is added to PerpetualMarketCreateObject but missing from PerpetualMarketUpdateObject. If this field should be updatable, add it to the update interface:
export interface PerpetualMarketUpdateObject {
// ... existing fields ...
liquidityTierId?: number,
+ defaultFundingRate1H?: string,
}Also applies to: 24-41
| 'UpdatePerpetualEventV3', | ||
| UpdatePerpetualEventV3.encode(defaultUpdatePerpetualEventV3).finish(), | ||
| defaultUpdatePerpetualEventV2, | ||
| ], |
There was a problem hiding this comment.
Fix incorrect event version in test data.
The test is using defaultUpdatePerpetualEventV2 for V3 event test case. This should be using defaultUpdatePerpetualEventV3 instead.
Apply this fix:
- defaultUpdatePerpetualEventV2,
+ defaultUpdatePerpetualEventV3,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 'UpdatePerpetualEventV3', | |
| UpdatePerpetualEventV3.encode(defaultUpdatePerpetualEventV3).finish(), | |
| defaultUpdatePerpetualEventV2, | |
| ], | |
| 'UpdatePerpetualEventV3', | |
| UpdatePerpetualEventV3.encode(defaultUpdatePerpetualEventV3).finish(), | |
| defaultUpdatePerpetualEventV3, | |
| ], |
| MarketId: perpetual.Params.MarketId, | ||
| AtomicResolution: perpetual.Params.AtomicResolution, | ||
| LiquidityTier: perpetual.Params.LiquidityTier, | ||
| MarketType: v1.ConvertToPerpetualMarketType(perpetualtypes.PerpetualMarketType_PERPETUAL_MARKET_TYPE_CROSS), |
There was a problem hiding this comment.
Fix line length to comply with linting rules.
The line exceeds the maximum length of 120 characters.
Apply this diff to fix the line length:
- MarketType: v1.ConvertToPerpetualMarketType(perpetualtypes.PerpetualMarketType_PERPETUAL_MARKET_TYPE_CROSS),
+ MarketType: v1.ConvertToPerpetualMarketType(
+ perpetualtypes.PerpetualMarketType_PERPETUAL_MARKET_TYPE_CROSS,
+ ),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| MarketType: v1.ConvertToPerpetualMarketType(perpetualtypes.PerpetualMarketType_PERPETUAL_MARKET_TYPE_CROSS), | |
| MarketType: v1.ConvertToPerpetualMarketType( | |
| perpetualtypes.PerpetualMarketType_PERPETUAL_MARKET_TYPE_CROSS, | |
| ), |
🧰 Tools
🪛 GitHub Actions: Protocol Lint
[error] 236-236: Line length exceeds maximum limit: The line is 123 characters long, which exceeds the maximum of 120 characters
| /** Default 8hr funding rate in parts-per-million. */ | ||
|
|
||
| defaultFunding8hrPpm: number; |
There was a problem hiding this comment.
💡 Codebase verification
Update required in market updater for the new funding rate field
The defaultFunding8hrPpm field is properly handled in database schemas, but the market updater needs to be updated to use this field:
indexer/services/roundtable/src/tasks/market-updater.ts: Unresolved TODO (CT-1340) to add default funding rate
🔗 Analysis chain
Verify handling of the new funding rate field in dependent systems.
The addition of defaultFunding8hrPpm may require updates to systems that:
- Parse and process these events
- Store event data in databases
- Display funding rate information in UIs
Please ensure all dependent systems are updated to handle this new field.
Also applies to: 1624-1626
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for systems that need to be updated to handle the new field
# Find event handlers
echo "Searching for event handlers..."
rg -l "PerpetualMarketCreateEventV[12]|UpdatePerpetualEventV[12]"
# Find database schemas/migrations
echo "Searching for database schemas..."
fd -e sql -e prisma -e graphql
# Find UI components
echo "Searching for UI components..."
rg -l "funding.*rate|funding.*ppm"
Length of output: 8789
Script:
#!/bin/bash
# Check funding handlers and database schema for the new field
# Check handlers and validators
echo "Checking handlers and validators..."
rg -A 5 "defaultFunding8hrPpm" indexer/services/ender/src/
# Check database schema
echo "Checking database schema..."
rg "funding.*ppm|funding.*rate" indexer/services/ender/src/scripts/handlers/dydx_funding_handler.sql
# Check if field is used in market updates
echo "Checking market updater..."
rg -A 5 "funding.*ppm|funding.*rate" indexer/services/roundtable/src/tasks/market-updater.ts
Length of output: 2262
Script:
#!/bin/bash
# Check for TODO ticket and recent changes
# Search for the ticket number
echo "Searching for ticket reference..."
rg "CT-1340"
# Check git history for recent changes to market updater
echo "Checking recent changes..."
git log -p -- indexer/services/roundtable/src/tasks/market-updater.ts | grep -A 5 "funding.*rate"
Length of output: 280
| @@ -22,6 +26,7 @@ export class PerpetualMarketCreationHandler extends Handler< | |||
|
|
|||
| // eslint-disable-next-line @typescript-eslint/require-await | |||
| public async internalHandle(resultRow: pg.QueryResultRow): Promise<ConsolidatedKafkaEvent[]> { | |||
| console.log(resultRow); | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove debugging console.log statement
This console.log statement appears to be debugging code and should be removed before merging.
- console.log(resultRow);
There was a problem hiding this comment.
nit: Remove console log.
There was a problem hiding this comment.
@vincentwschau Agreed, I'll remove the console.log statement.
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
CT-1340
Changelist
DefaultFundingRate1HPerpetualMarketCreateEventandUpdatePerpetualEvent, addDefaultFunding8hrPpm. In handlers, convert these ppm into 1hr funding rate and store in DB.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
Release Notes: Perpetual Market Funding Rate Enhancement
New Features
defaultFundingRate1Hproperty to perpetual market data structures.Changes
Compatibility
DefaultFunding8HrPpmparameter.Impact