[TRA-507] Add Indexer changes for UpgradeIsolatedPerpetualToCross #2597
Conversation
WalkthroughThe pull request introduces significant updates across multiple files to accommodate a new version of the Changes
Possibly related PRs
Suggested reviewers
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (15)
protocol/indexer/events/perpetual.go (1)
8-16: Enhance function documentationThe current comment could be more descriptive about the V2 event type and the new marketType parameter.
Consider updating the comment to:
-// NewUpdatePerpetualEvent creates a UpdatePerpetualEventV2 representing -// update of a perpetual. +// NewUpdatePerpetualEvent creates a UpdatePerpetualEventV2 representing an update of a perpetual. +// It includes market type information in addition to the basic perpetual properties. +// This is an upgrade from V1 which adds support for different market types (isolated/cross).protocol/indexer/events/perpetual_test.go (1)
12-29: Consider adding more test coverageThe current test only covers the happy path with CROSS market type. Consider adding:
- Test cases for ISOLATED market type
- Negative test cases for invalid market types
- Tests to verify proper migration/handling of V1 events
Example test cases to add:
func TestNewUpdatePerpetualEvent_IsolatedType(t *testing.T) { // Test with ISOLATED market type } func TestNewUpdatePerpetualEvent_InvalidType(t *testing.T) { // Test with invalid market type } func TestUpdatePerpetualEventBackwardCompatibility(t *testing.T) { // Test handling of V1 events }indexer/services/ender/src/handlers/update-perpetual-handler.ts (1)
15-16: Document version handling strategy.Consider adding documentation to explain how V1 and V2 events are differentiated and processed, especially regarding the new
marketTypefield.Add a comment block like:
+/** + * Handles both V1 and V2 UpdatePerpetual events. + * V2 events include additional marketType field for cross/isolated market differentiation. + */ export class UpdatePerpetualHandler extends Handler< UpdatePerpetualEventV1 | UpdatePerpetualEventV2> {protocol/indexer/events/constants.go (1)
37-37: Add a comment explaining the version change.Consider adding a comment above the constant to document that V2 adds the
marketTypefield for supporting cross-perpetual markets.+// UpdatePerpetualEventVersion 2 adds marketType field to support cross-perpetual markets UpdatePerpetualEventVersion uint32 = 2indexer/services/ender/src/scripts/handlers/dydx_update_perpetual_v1_handler.sql (2)
1-11: Enhance deprecation notice documentation.While the documentation is thorough, consider adding:
- The reason for deprecation
- The key differences between V1 and V2 handlers
- Migration guidance for users
/** Note: This is a deprecated handler, see `dydx_update_perpetual_v2_handler` for the latest handler. + This handler has been deprecated in favor of V2 which adds support for market type. + To migrate, use dydx_update_perpetual_v2_handler which requires an additional marketType field.
31-33: Enhance error message with additional context.Consider including more details in the error message to aid in debugging:
- RAISE EXCEPTION 'Could not find perpetual market with corresponding id %', perpetual_market_id; + RAISE EXCEPTION 'Could not find perpetual market with id %. Ticker: %, MarketId: %', + perpetual_market_id, + perpetual_market_record."ticker", + perpetual_market_record."marketId";indexer/services/ender/src/scripts/handlers/dydx_update_perpetual_v2_handler.sql (3)
1-10: Enhance documentation for V2-specific changesThe documentation should explicitly mention:
- The new
marketTypefield and its significance- The difference between V1 and V2 handlers
- Valid values for
marketTypefield
22-30: Consider adding input validation and transaction controlSuggestions for improvement:
- Add input validation for critical fields before update
- Consider wrapping in an explicit transaction if this is part of a larger operation
- Add constraints or checks for valid market types
1-41: Consider backward compatibility and migration strategySince this introduces V2 event handling:
- Ensure a clear migration path from V1 to V2 events
- Consider adding a version field to the perpetual_markets table
- Document the upgrade process for existing markets
- Consider adding logging for tracking upgrade progress
indexer/services/ender/__tests__/validators/update-perpetual-validator.test.ts (1)
47-62: Fix typo in test case nameThe test structure looks good, efficiently testing both event versions. However, there's a typo in the V2 test case name.
- 'PerpetualMarketCreateEventV2', + 'UpdatePerpetualEventV2',indexer/services/ender/src/scripts/handlers/dydx_block_processor_ordered_handlers.sql (1)
70-72: Consider explicit error handling for unknown versionsCurrently, unknown versions silently return NULL. Consider adding explicit error handling to aid in debugging and monitoring.
WHEN 2 THEN rval[i] = dydx_update_perpetual_v2_handler(event_data); ELSE - NULL; + RAISE WARNING 'Unknown update_perpetual event version: %', (event_->'version')::int; + rval[i] = NULL; END CASE;indexer/services/ender/__tests__/handlers/update-perpetual-handler.test.ts (1)
Line range hint
137-171: Update helper function to support both event versionsThe
createKafkaMessageFromUpdatePerpetualEventfunction needs to be updated to handle both V1 and V2 events:function createKafkaMessageFromUpdatePerpetualEvent({ updatePerpetualEvent, transactionIndex, height, time, txHash, }: { - updatePerpetualEvent: UpdatePerpetualEventV1, + updatePerpetualEvent: UpdatePerpetualEventV1 | UpdatePerpetualEventV2, transactionIndex: number, height: number, time: Timestamp, txHash: string, }) { const events: IndexerTendermintEvent[] = []; + const eventBytes = updatePerpetualEvent instanceof UpdatePerpetualEventV2 + ? UpdatePerpetualEventV2.encode(updatePerpetualEvent).finish() + : UpdatePerpetualEventV1.encode(updatePerpetualEvent).finish(); events.push( createIndexerTendermintEvent( DydxIndexerSubtypes.UPDATE_PERPETUAL, - UpdatePerpetualEventV1.encode(updatePerpetualEvent).finish(), + eventBytes, transactionIndex, 0, ), ); // ... rest of the function }indexer/services/ender/src/lib/helper.ts (1)
202-225: LGTM with suggestions for improvementsThe version handling implementation is correct and follows the established pattern. Consider these improvements:
- Make error messages more descriptive (e.g., include event details)
- Add debug logging for successful version handling
- Extract version validation to a reusable function as this pattern is repeated across multiple event types
case (DydxIndexerSubtypes.UPDATE_PERPETUAL.toString()): { + const eventType = 'update perpetual'; + logger.debug({ + at: 'helpers#indexerTendermintEventToEventWithType', + message: `Processing ${eventType} event version ${version}`, + }); if (version === 1) { return { type: DydxIndexerSubtypes.UPDATE_PERPETUAL, eventProto: UpdatePerpetualEventV1.decode(eventDataBinary), indexerTendermintEvent: event, version, blockEventIndex, }; } else if (version === 2) { return { type: DydxIndexerSubtypes.UPDATE_PERPETUAL, eventProto: UpdatePerpetualEventV2.decode(eventDataBinary), indexerTendermintEvent: event, version, blockEventIndex, }; } else { - const message: string = `Invalid version for update perpetual event: ${version}`; + const message: string = `Invalid version ${version} for ${eventType} event (id: ${event.id})`; logger.error({ at: 'helpers#indexerTendermintEventToEventWithType', message, }); return undefined; } }indexer/services/ender/__tests__/helpers/constants.ts (1)
203-210: LGTM! Consider adding JSDoc.The new
defaultUpdatePerpetualEventV2constant correctly extends V1 with themarketTypefield. Consider adding JSDoc to document the purpose and usage of this test constant.Add JSDoc comment above the constant:
+/** + * Default test constant for UpdatePerpetualEventV2. + * Extends V1 with marketType field set to PERPETUAL_MARKET_TYPE_CROSS. + */ export const defaultUpdatePerpetualEventV2: UpdatePerpetualEventV2 = {proto/dydxprotocol/indexer/events/events.proto (1)
Line range hint
489-544: Consider documenting the upgrade path from V1 to V2.The changes support upgrading isolated perpetuals to cross perpetuals. Consider adding documentation or comments about:
- How existing V1 events should be handled during the upgrade
- Whether there's a default
market_typefor V1 events- Any necessary database migrations for the indexer
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
protocol/indexer/events/events.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (19)
indexer/packages/v4-protos/src/codegen/dydxprotocol/indexer/events/events.ts(4 hunks)indexer/services/ender/__tests__/handlers/update-perpetual-handler.test.ts(3 hunks)indexer/services/ender/__tests__/helpers/constants.ts(2 hunks)indexer/services/ender/__tests__/validators/update-perpetual-validator.test.ts(4 hunks)indexer/services/ender/src/handlers/update-perpetual-handler.ts(1 hunks)indexer/services/ender/src/helpers/postgres/postgres-functions.ts(1 hunks)indexer/services/ender/src/lib/block-processor.ts(1 hunks)indexer/services/ender/src/lib/helper.ts(2 hunks)indexer/services/ender/src/lib/types.ts(2 hunks)indexer/services/ender/src/scripts/handlers/dydx_block_processor_ordered_handlers.sql(1 hunks)indexer/services/ender/src/scripts/handlers/dydx_update_perpetual_v1_handler.sql(1 hunks)indexer/services/ender/src/scripts/handlers/dydx_update_perpetual_v2_handler.sql(1 hunks)indexer/services/ender/src/validators/update-perpetual-validator.ts(1 hunks)proto/dydxprotocol/indexer/events/events.proto(2 hunks)protocol/indexer/events/constants.go(1 hunks)protocol/indexer/events/perpetual.go(1 hunks)protocol/indexer/events/perpetual_test.go(1 hunks)protocol/x/perpetuals/keeper/perpetual.go(1 hunks)protocol/x/perpetuals/keeper/perpetual_test.go(4 hunks)
🔇 Additional comments (40)
protocol/x/perpetuals/keeper/perpetual_test.go (4)
20-20: Approved: Importing alias 'v1' for indexer protocol V1
The import statement with alias v1 is appropriate for referencing the correct version of the indexer protocol.
45-45: Approved: Updating expected indexer events to use UpdatePerpetualEventV2
Changing the expected indexer events to UpdatePerpetualEventV2 aligns with the updated event structure and is appropriate.
64-70: Approved: Initializing UpdatePerpetualEventV2 with new MarketType field
The inclusion of the MarketType field using v1.ConvertToPerpetualMarketType correctly reflects the transition to UpdatePerpetualEventV2.
117-125: Approved: Updating function to return and process UpdatePerpetualEventV2
Modifying getUpdatePerpetualEventsFromIndexerBlock to handle UpdatePerpetualEventV2 ensures consistency with the new event version.
indexer/packages/v4-protos/src/codegen/dydxprotocol/indexer/events/events.ts (3)
Line range hint 1252-1300: Deprecation of UpdatePerpetualEventV1 is properly handled
The UpdatePerpetualEventV1 interface is correctly marked as deprecated using the JSDoc @deprecated annotation. This maintains backward compatibility while clearly indicating to developers that they should transition to the new UpdatePerpetualEventV2 interface.
1335-1418: Definition of UpdatePerpetualEventV2 is accurate and comprehensive
The new UpdatePerpetualEventV2 interface is well-defined, including the addition of the marketType field of type PerpetualMarketType. All necessary fields are present, and the documentation provides clear guidance on each property's purpose and usage.
3580-3674: Serialization methods for UpdatePerpetualEventV2 are correctly implemented
The encode, decode, and fromPartial methods for UpdatePerpetualEventV2 correctly handle all the defined fields, including marketType. Field numbers in the encoding align with the protocol buffer specifications, ensuring accurate serialization and deserialization processes.
protocol/indexer/events/perpetual.go (3)
3-6: LGTM: Import statements are correctly structured
The new imports are properly organized and follow Go conventions with appropriate aliases.
1-24: Verify backward compatibility handling
Since this is an upgrade from V1 to V2 events, we should verify how existing V1 events are handled in the system.
✅ Verification successful
Based on the search results, I can generate the final response as I have sufficient information to verify the backward compatibility handling:
V1 to V2 event upgrade is properly handled
The codebase shows proper handling of both V1 and V2 perpetual events:
- V1 event is marked as deprecated in proto definition with clear documentation pointing to V2
- The indexer service handles both versions through version-aware type unions and validators
- The update perpetual handler supports both V1 and V2 event types
- Tests exist for both V1 and V2 event processing paths
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Look for V1 event handling code
rg -A 5 "UpdatePerpetualEventV1"
# Check for version compatibility handling
ast-grep --pattern 'switch $_ {
case $$$UpdatePerpetualEventV1$$$:
$$$
}'
Length of output: 31139
17-24: Verify market type conversion behavior
The implementation looks correct, but we should verify the behavior of v1.ConvertToPerpetualMarketType.
✅ Verification successful
Market type conversion implementation is correct and well-tested
The conversion function ConvertToPerpetualMarketType correctly handles all possible market types:
- Maps CROSS to CROSS (type 1)
- Maps ISOLATED to ISOLATED (type 2)
- Panics on UNSPECIFIED (type 0)
The implementation is thoroughly tested with automatically generated test cases covering all enum values, and the enum definitions in both packages match exactly.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Find and examine the ConvertToPerpetualMarketType implementation
ast-grep --pattern 'func ConvertToPerpetualMarketType($_) $_ {
$$$
}'
# Look for test cases
rg -A 5 "TestConvertToPerpetualMarketType"
Length of output: 1865
Script:
#!/bin/bash
# Let's examine the test cases to ensure all market types are covered
rg -A 20 "TestConvertToPerpetualMarketType" protocol/indexer/protocol/v1/v1_mappers_test.go
# Also check the enum definitions to ensure all possible values are handled
rg "type PerpetualMarketType" -A 10
Length of output: 3001
protocol/indexer/events/perpetual_test.go (2)
6-8: LGTM: Import changes are appropriate
The new imports are correctly aliased and necessary for supporting the MarketType field in the V2 event structure.
12-29: LGTM: Test updated correctly for V2 event structure
The test has been properly updated to verify the new V2 event structure with the MarketType field.
indexer/services/ender/src/validators/update-perpetual-validator.ts (3)
2-2: LGTM!
The import statement correctly includes both UpdatePerpetualEventV1 and UpdatePerpetualEventV2 types.
8-10: LGTM!
The class definition correctly extends the validator to handle both V1 and V2 event types using a union type.
Line range hint 20-31: Critical: Handler type mismatch with validator's event type
The createHandlers method's return type and implementation are still typed for UpdatePerpetualEventV1 only, which is inconsistent with the class's type parameter that now accepts both V1 and V2 events. This could lead to runtime errors when handling V2 events.
Apply this diff to fix the type mismatch:
public createHandlers(
indexerTendermintEvent: IndexerTendermintEvent,
txId: number,
_: string,
- ): Handler<UpdatePerpetualEventV1>[] {
- const handler: Handler<UpdatePerpetualEventV1> = new UpdatePerpetualHandler(
+ ): Handler<UpdatePerpetualEventV1 | UpdatePerpetualEventV2>[] {
+ const handler: Handler<UpdatePerpetualEventV1 | UpdatePerpetualEventV2> = new UpdatePerpetualHandler(
this.block,
this.blockEventIndex,
indexerTendermintEvent,
txId,
this.event,
);
return [handler];
}Let's verify if the UpdatePerpetualHandler class is prepared to handle both event types:
indexer/services/ender/src/handlers/update-perpetual-handler.ts (2)
Line range hint 23-35: Verify handling of marketType field from V2 events.
The internalHandle method remains unchanged despite the addition of V2 events with the new marketType field. Verify if this field needs to be persisted or handled differently.
✅ Verification successful
The marketType field is properly handled in the codebase
The verification shows that the marketType field from V2 events is properly handled:
- The database schema includes a
marketTypecolumn in theperpetual_marketstable with CROSS/ISOLATED enum values - The
PerpetualMarketModelincludes themarketTypefield in its required fields and type definitions - SQL functions exist to convert protocol market types to database market types (
dydx_protocol_market_type_to_perpetual_market_type) - The field is properly propagated through handlers and validators
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if marketType is handled in the database schema
rg -A 5 "CREATE TABLE.*perpetual_market" --type sql
# Check for any V2-specific handling in other handlers
ast-grep --pattern 'UpdatePerpetualEventV2 {
$$$
marketType
$$$
}'
Length of output: 128
Script:
#!/bin/bash
# Search for perpetual market table definition in schema files
fd "schema.*sql" | xargs rg -A 10 "CREATE TABLE.*perpetual"
# Search for V2 event type definitions and usage
rg -A 5 "UpdatePerpetualEventV2"
# Search for marketType field usage in the codebase
rg "marketType.*perpetual" -A 3
# Check the model and types for perpetual market
fd "perpetual.*model|perpetual.*type" | xargs cat
Length of output: 50852
17-17: Verify the impact of the event type string change.
The event type has been changed from version-specific to generic. Ensure all consumers of this event type string have been updated accordingly.
✅ Verification successful
Let me check the actual usage of these event types to ensure the change is safe.
Based on the search results, I can now generate the final response:
The event type string change is safe and correctly implemented.
The change from version-specific to generic event type is consistent with the codebase implementation:
- The handler class properly handles both V1 and V2 event types through union types
- The validator and tests have been updated to support both versions
- The event type string is only used internally within the handler class and doesn't affect external consumers
- The V1 event is marked as deprecated in the protos, and V2 is the recommended version
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any direct references to 'UpdatePerpetualEventV1' string
# that might need updating to 'UpdatePerpetualEvent'
rg -l "UpdatePerpetualEventV1" --type ts
# Search for event type string usage
rg -l "UpdatePerpetualEvent[^V]" --type ts
Length of output: 923
Script:
#!/bin/bash
# Check the actual usage of UpdatePerpetualEventV1 and UpdatePerpetualEvent in the files
rg "UpdatePerpetualEventV1" --type ts -A 2 -B 2
# Check the validator implementation specifically
ast-grep --pattern 'class UpdatePerpetualValidator {
$$$
validate($$$) {
$$$
}
$$$
}'
Length of output: 16503
protocol/indexer/events/constants.go (1)
37-37: LGTM! Verify documentation sync.
The version bump from 1 to 2 aligns with the addition of the marketType field in UpdatePerpetualEventV2.
Let's verify the sync with indexer types:
✅ Verification successful
Documentation is in sync with version change
The indexer types are properly synchronized with the version change:
UpdatePerpetualEventV1is marked as deprecated with a reference to V2UpdatePerpetualEventV2is fully defined with the new structure- The documentation correctly reflects the update and versioning
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the indexer types are in sync with this change
# Expected: Should find UpdatePerpetualEventV2 in types.ts
rg -A 5 "UpdatePerpetualEventV2" "indexer/packages/v4-protos/src/codegen/dydxprotocol/indexer/"
Length of output: 6762
indexer/services/ender/src/scripts/handlers/dydx_update_perpetual_v1_handler.sql (3)
22-29: Consider explicit transaction management.
While the update operation is correct, consider adding explicit transaction management to ensure atomicity, especially if this handler is part of a larger operation sequence.
Let's verify if this handler is used within a transaction:
1-40: Verify migration path and backwards compatibility.
As this is part of upgrading isolated perpetual to cross markets:
- Ensure V2 handler properly handles both isolated and cross market types
- Verify that existing events can still be processed during migration
✅ Verification successful
Let me search for the V2 handler implementation to verify the migration path.
Let me check the block processor handlers to understand how these handlers are registered and used.
Based on the gathered information, I can now provide the final response:
Migration path and backwards compatibility are properly handled
The codebase shows a robust migration path:
- V1 and V2 handlers coexist and are properly versioned in the block processor
- V2 handler adds
marketTypefield while maintaining all V1 fields - Events are routed to correct handler based on version:
CASE (event->'version')::int WHEN 1 THEN dydx_update_perpetual_v1_handler WHEN 2 THEN dydx_update_perpetual_v2_handler
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for V2 handler implementation
rg "CREATE.*FUNCTION.*dydx_update_perpetual_v2_handler"
# Search for handler usage in migration code
rg -A 10 "UpdatePerpetualEvent"
Length of output: 81404
Script:
#!/bin/bash
# Get the V2 handler implementation
cat indexer/services/ender/src/scripts/handlers/dydx_update_perpetual_v2_handler.sql
# Search for handler registration/usage to understand migration
rg -l "dydx_update_perpetual.*handler" --type sql
Length of output: 2544
Script:
#!/bin/bash
# Check block processor handlers registration
cat indexer/services/ender/src/scripts/handlers/dydx_block_processor_ordered_handlers.sql
# Check the base update perpetual handler
cat indexer/services/ender/src/scripts/handlers/dydx_update_perpetual_handler.sql
Length of output: 6607
35-38: Verify dydx_to_jsonb function implementation.
The function relies on dydx_to_jsonb for record serialization. Let's ensure this function handles all fields correctly.
✅ Verification successful
Let me gather more information about the implementation of dydx_to_jsonb function.
Let me try a different approach to get the implementation details.
Let me try one more search to find the model definition and schema.
Let me fix the regex and try one last search.
Based on the findings, I can now generate the final response. The code shows that dydx_to_jsonb is a dynamically generated function created for each model, including the perpetual_markets table, and is used consistently across multiple handlers for serializing perpetual market records. The implementation in db-helpers.ts shows it's a standard serialization function that converts database records to JSONB format.
dydx_to_jsonb function implementation verified
The function is correctly implemented as a PostgreSQL function that serializes database records to JSONB format:
- Created dynamically for each model including
perpetual_markets - Used consistently across multiple handlers
- Follows standard record serialization pattern
- Handles all fields from the model as shown by its usage in various handlers
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for dydx_to_jsonb implementation
rg "CREATE.*FUNCTION.*dydx_to_jsonb"
# Search for its usage patterns
rg "dydx_to_jsonb.*perpetual_market"
Length of output: 2069
Script:
#!/bin/bash
# Get the implementation details of dydx_to_jsonb function
cat indexer/packages/postgres/src/helpers/db-helpers.ts | grep -A 20 "CREATE.*FUNCTION.*dydx_to_jsonb"
# Get the perpetual market table schema to understand the fields being serialized
rg -A 10 "CREATE TABLE.*perpetual_market"
Length of output: 770
Script:
#!/bin/bash
# Search for the perpetual market model definition
rg -t ts "PerpetualMarket.*extends.*Model" -A 10
# Look for table creation SQL for perpetual markets
fd -e sql | xargs rg "CREATE TABLE.*perpetual_market"
# Check the db-helpers implementation differently
ast-grep --pattern 'createModelToJsonFunctions($$$)'
Length of output: 67561
Script:
#!/bin/bash
# Search for perpetual market table schema
fd -e sql | xargs rg "CREATE TABLE.*perpetual_markets"
# Search for PerpetualMarketModel properties
rg -t ts "PerpetualMarketModel.*{" -A 20
Length of output: 209
Script:
#!/bin/bash
# Search for perpetual market table schema
fd -e sql | xargs rg "CREATE TABLE perpetual_markets"
# Search for PerpetualMarketModel properties with fixed regex
rg -t ts "class PerpetualMarketModel" -A 20
# Look for model properties
rg -t ts "perpetual_market_record\." -A 1
Length of output: 2023
indexer/services/ender/src/scripts/handlers/dydx_update_perpetual_v2_handler.sql (4)
11-14: LGTM! Good use of %ROWTYPE
Using %ROWTYPE ensures type safety and automatic column updates when the table structure changes.
32-34: LGTM! Clear error message with relevant details
The error handling appropriately catches and reports missing market scenarios.
15-20: Verify market type conversion function implementation
The dydx_protocol_market_type_to_perpetual_market_type function needs verification for:
- Existence in the database
- Handling of all possible market types
- Error handling for invalid inputs
✅ Verification successful
Let me verify the market type definitions to ensure all cases are handled.
Market type conversion function is correctly implemented
The dydx_protocol_market_type_to_perpetual_market_type function is properly implemented with:
- Complete handling of all possible market types (CROSS=1, ISOLATED=2)
- Proper error handling for invalid inputs via RAISE EXCEPTION
- Consistent mapping with protocol definitions in both proto and TypeScript enums
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the market type conversion function
# Check if the function exists
echo "Checking function existence..."
rg -A 10 "CREATE.*FUNCTION.*dydx_protocol_market_type_to_perpetual_market_type"
# Look for market type definitions to ensure all cases are handled
echo "Checking market type definitions..."
rg "enum.*MarketType"
Length of output: 3612
Script:
#!/bin/bash
# Check the enum definitions in proto files to verify all possible market types
echo "Checking proto enum definitions..."
cat proto/dydxprotocol/perpetuals/perpetual.proto | grep -A 5 "enum PerpetualMarketType"
# Check TypeScript enum for consistency
echo "Checking TypeScript enum definitions..."
cat indexer/packages/postgres/src/types/perpetual-market-types.ts | grep -A 5 "enum PerpetualMarketType"
Length of output: 687
36-41: Verify dydx_to_jsonb function implementation
Need to verify the dydx_to_jsonb function:
- Exists in the database
- Correctly handles all fields of perpetual_markets table
✅ Verification successful
Let me gather more information about the function implementation and the model structure.
Let me check one more thing to understand the SQL to JSON conversions for the perpetual market model.
Based on the gathered information, I can now provide the final response:
dydx_to_jsonb function is properly implemented and safe to use
The function is automatically generated for the PerpetualMarketModel with proper type conversions. The implementation:
- Is created through a standardized process via
createModelToJsonFunctions - Handles all fields of the perpetual market model with appropriate SQL-to-JSON type conversions
- Is marked as IMMUTABLE and PARALLEL SAFE
- Is used consistently across the codebase for model conversions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the JSONB conversion function
# Check if the function exists and its implementation
echo "Checking function existence and implementation..."
rg -A 10 "CREATE.*FUNCTION.*dydx_to_jsonb"
Length of output: 1222
Script:
#!/bin/bash
# Check the model definition and table structure
echo "Checking perpetual market model and table structure..."
rg -A 20 "PerpetualMarket.*extends.*Model"
# Check the function creation call
echo "Checking function creation..."
rg -B 5 -A 10 "createModelToJsonFunctions"
Length of output: 6526
Script:
#!/bin/bash
# Check SQL to JSON conversion definitions for perpetual markets
echo "Checking SQL to JSON conversions..."
rg -B 5 -A 15 "SQL_TO_JSON_DEFINED_MODELS.*=.*\["
# Check the conversion types helper
echo "Checking conversion types..."
rg -A 10 "getSqlConversionForDydxModelTypes"
Length of output: 21844
indexer/services/ender/__tests__/validators/update-perpetual-validator.test.ts (3)
6-6: LGTM: Import changes for V2 support
The imports are correctly updated to support both V1 and V2 versions of UpdatePerpetualEvent, maintaining consistent naming conventions.
Also applies to: 13-14
65-66: LGTM: Test cases properly handle both versions
The test cases are well-structured to handle both V1 and V2 events while maintaining consistent validation logic and error messages.
Also applies to: 77-80
91-97: LGTM: Helper function properly adapted for version flexibility
The createBlock function is well-modified to handle encoded bytes from both event versions while maintaining type safety.
indexer/services/ender/src/helpers/postgres/postgres-functions.ts (1)
47-48: Consider handler execution order
The order of handlers in the array might be important for proper event processing. Since we now have V1 and V2 handlers, ensure that:
- V1 handler is listed before V2 for consistent processing
- Both handlers are properly registered in the block processor
✅ Verification successful
Let me gather more information about the handler registration and execution order.
Based on the gathered information, I can now provide the final response:
✓ Handler order and registration are correctly implemented
The code shows that both handlers are properly registered and executed in the correct order:
- In
dydx_block_processor_ordered_handlers, the version-based handler selection ensures V1 handler is executed for version 1 events and V2 for version 2 events:
WHEN '"update_perpetual"'::jsonb THEN
CASE (event_->'version')::int
WHEN 1 THEN
rval[i] = dydx_update_perpetual_v1_handler(event_data);
WHEN 2 THEN
rval[i] = dydx_update_perpetual_v2_handler(event_data);- The SQL files are loaded in alphabetical order, which naturally places V1 before V2 (
v1_handler.sqlbeforev2_handler.sql), matching the current array order in the code.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify handler registration and order in block processor
# Check block processor registration
ast-grep --pattern 'TXN_EVENT_SUBTYPE_VERSION_TO_VALIDATOR_MAPPING = {
$$$
[EventProtoEnum.UPDATE_PERPETUAL]: {
$$$
}
$$$
}'
# Check handler registration order in block processor SQL
fd "dydx_block_processor" --type f --extension sql --exec cat {}
Length of output: 11940
Script:
#!/bin/bash
# Check the event version mapping and handler registration in TypeScript code
ast-grep --pattern 'export const VERSION_TO_HANDLER_MAPPING = {
$$$
[EventProtoEnum.UPDATE_PERPETUAL]: {
$$$
}
$$$
}'
# Check the SQL handler files to verify their implementation order
fd "dydx_update_perpetual" --type f --extension sql --exec cat {}
Length of output: 8931
indexer/services/ender/src/scripts/handlers/dydx_block_processor_ordered_handlers.sql (1)
65-72: Verify the existence of handler functions
The implementation looks correct, following the same pattern as the perpetual_market event handling. Let's verify that both handler functions exist in the codebase.
✅ Verification successful
Both handler functions are present in the codebase
The verification confirms that both handler functions exist:
dydx_update_perpetual_v1_handlerinindexer/services/ender/src/scripts/handlers/dydx_update_perpetual_v1_handler.sqldydx_update_perpetual_v2_handlerinindexer/services/ender/src/scripts/handlers/dydx_update_perpetual_v2_handler.sql
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that both handler functions exist in SQL files
# Search for handler function definitions
echo "Searching for v1 handler..."
fd -e sql -x rg -l 'CREATE.*FUNCTION.*dydx_update_perpetual_v1_handler'
echo "Searching for v2 handler..."
fd -e sql -x rg -l 'CREATE.*FUNCTION.*dydx_update_perpetual_v2_handler'
Length of output: 443
indexer/services/ender/__tests__/handlers/update-perpetual-handler.test.ts (1)
15-16: LGTM: Import changes for V2 support
The imports correctly include both V1 and V2 event types and their corresponding default constants.
Also applies to: 23-23
indexer/services/ender/src/lib/types.ts (2)
34-34: LGTM: Import statement is correctly placed
The import of UpdatePerpetualEventV2 is properly added and alphabetically ordered among other event imports.
149-154: LGTM: Union type case follows the established pattern
The new case for UpdatePerpetualEventV2 is correctly structured and maintains type safety. Let's verify that all relevant handlers are updated to support this new event version.
✅ Verification successful
Let me check the handler implementation to verify version handling.
Let me check the handler implementation with a different approach to see the version handling.
Handler correctly supports both event versions without requiring version-specific logic
The handler implementation is correctly designed to handle both UpdatePerpetualEventV1 and UpdatePerpetualEventV2 through TypeScript union types. The handler's logic is version-agnostic as it processes the perpetual market data from the database row rather than directly from the event proto, making it naturally compatible with both versions without requiring explicit version switching.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that handlers are updated to support UpdatePerpetualEventV2
# Check for handler implementations
echo "Checking for UpdatePerpetualEventV2 handler implementations..."
rg -l "UpdatePerpetualEventV2" "indexer/services/ender/src/handlers/"
# Check for potential missing version handling
echo "Checking for potential missing version handling..."
ast-grep --pattern 'switch ($version) {
$$$
case $_ : {
$$$
}
$$$
}' "indexer/services/ender/src/handlers/"
Length of output: 497
Script:
#!/bin/bash
# Check the content of the update-perpetual handler
echo "Checking update-perpetual-handler.ts implementation..."
rg -A 10 "UpdatePerpetualEventV2" "indexer/services/ender/src/handlers/update-perpetual-handler.ts"
# Check version handling in the handler
echo "Checking version handling in update-perpetual-handler.ts..."
rg "switch.*version" -A 15 "indexer/services/ender/src/handlers/update-perpetual-handler.ts"
Length of output: 1294
Script:
#!/bin/bash
# Get the full content of the update-perpetual handler to see version handling
echo "Getting full content of update-perpetual-handler.ts..."
cat "indexer/services/ender/src/handlers/update-perpetual-handler.ts"
Length of output: 1675
indexer/services/ender/src/lib/helper.ts (2)
24-24: LGTM: Import addition is correct
The import of UpdatePerpetualEventV2 is properly placed and necessary for the new version handling.
202-225: Verify version handling in related code
Please ensure:
- All consumers properly handle the
undefinedreturn value when version is invalid - Version handling is consistent in related handlers
Let's verify the version handling in related code:
✅ Verification successful
Let me gather more specific information about how the undefined return value is handled by consumers.
Let me modify the search pattern to better find the usage and handling of this function.
Let me check how the undefined return value is handled in the message processing pipeline.
Let me check one more thing to see how the events are processed after being parsed.
Based on the gathered information, I can now provide a final response about the version handling verification.
Version handling is properly implemented and validated
The undefined return value from indexerTendermintEventToEventWithType is properly handled throughout the codebase:
- The function logs errors appropriately before returning undefined for invalid versions
- The event processing pipeline (
BlockProcessor) validates events before processing UPDATE_PERPETUALis registered as a synchronous subtype, ensuring ordered processing- Both V1 and V2 versions have proper validator mappings in
TXN_EVENT_SUBTYPE_VERSION_TO_VALIDATOR_MAPPING - Failed message parsing results in the message not being acknowledged, ensuring no data loss
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check version handling in related handlers and consumers
# Find all files that import or reference UpdatePerpetualEvent
echo "Files referencing UpdatePerpetualEvent:"
rg -l "UpdatePerpetualEvent[V]?" .
# Find all handlers that might process these events
echo -e "\nEvent handlers:"
rg -l "class.*Handler.*implements.*Handler" .
# Look for undefined checks
echo -e "\nUndefined handling:"
rg "(?:if|\\?|\\.|\\()\\s*.*undefined" .
Length of output: 157030
Script:
#!/bin/bash
# Find all files that import or use the helper function that returns undefined
echo "Files importing/using the helper function:"
rg -l "indexerTendermintEventToEventWithType" .
# Look for handling of undefined returns from this function
echo -e "\nHandling of undefined returns:"
rg "indexerTendermintEventToEventWithType.*\)(?:\s*;|\s*\n)" -A 5 .
Length of output: 582
Script:
#!/bin/bash
# Find usage of the helper function with surrounding context
echo "Usage and handling of the helper function:"
rg -U "indexerTendermintEventToEventWithType" -B 2 -A 5 .
# Also look for specific event handling in the test files
echo -e "\nTest file handling:"
rg -U "UpdatePerpetualEvent" ./indexer/services/ender/__tests__/ -B 2 -A 5
Length of output: 16938
Script:
#!/bin/bash
# Find where the helper function is called and how its result is handled
echo "Handler usage of the helper function:"
rg -U "const.*=.*indexerTendermintEventToEventWithType" -B 3 -A 10 .
# Look for message processing in the main handler
echo -e "\nMessage processing in handler:"
rg -U "onMessage" ./indexer/services/ender/src/ -B 3 -A 10
Length of output: 19387
Script:
#!/bin/bash
# Find the event processing code
echo "Event processing code:"
rg -U "processEvents|validateAndOrganizeEvents" ./indexer/services/ender/src/ -B 2 -A 10
# Look for event type handling
echo -e "\nEvent type handling:"
rg -U "DydxIndexerSubtypes.UPDATE_PERPETUAL" -B 2 -A 5 .
Length of output: 12692
indexer/services/ender/src/lib/block-processor.ts (1)
54-54: Verify UpdatePerpetualValidator compatibility with V2 events
The addition of version 2 support for UPDATE_PERPETUAL events is using the same validator as version 1. Let's verify that the validator can handle both versions correctly.
Consider documenting the version differences and validation strategy in the validator class to help maintainers understand the evolution of the event structure.
✅ Verification successful
Let me gather more information about the validator implementation to ensure it properly handles both versions.
Let me check the actual event types and handler implementation to ensure proper version handling.
Let me check the SQL handlers to verify version-specific handling.
Based on the gathered information, I can now provide the final response:
The UpdatePerpetualValidator correctly handles both V1 and V2 events
The validator is properly designed to handle both versions:
- It's explicitly typed to accept both
UpdatePerpetualEventV1 | UpdatePerpetualEventV2as seen in the validator class definition - The validation logic is version-agnostic, only checking the perpetual market ID which is common to both versions
- The handler implementation in
update-perpetual-handler.tsproperly processes both event versions and updates the market type field when present in V2
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that UpdatePerpetualValidator can handle both V1 and V2 events
# Check if UpdatePerpetualValidator has been updated to handle V2 events
rg -A 10 "class UpdatePerpetualValidator"
# Look for V2 event handling in the validator
rg -p "UpdatePerpetualEventV2" "indexer/services/ender/src/validators/"
# Check for version-specific validation logic
ast-grep --pattern 'validate() {
$$$
if ($event instanceof $_) {
$$$
}
$$$
}'
Length of output: 1623
Script:
#!/bin/bash
# Check the full implementation of UpdatePerpetualValidator
cat indexer/services/ender/src/validators/update-perpetual-validator.ts
# Look for any version-specific handling in the handler
rg -p "UpdatePerpetualHandler" -A 20
Length of output: 4969
Script:
#!/bin/bash
# Check the event type definitions
rg -p "UpdatePerpetualEventV2" -A 10 --type proto
# Check the handler implementation for version-specific logic
cat indexer/services/ender/src/handlers/update-perpetual-handler.ts
# Look for any version-specific SQL queries
rg -p "UPDATE.*perpetual.*market" --type sql
Length of output: 2132
Script:
#!/bin/bash
# Check the V1 and V2 SQL handlers
cat indexer/services/ender/src/scripts/handlers/dydx_update_perpetual_v1_handler.sql
cat indexer/services/ender/src/scripts/handlers/dydx_update_perpetual_v2_handler.sql
# Check if there are any version-specific models or types
rg -p "PerpetualMarketFromDatabase" -A 5
Length of output: 71055
indexer/services/ender/__tests__/helpers/constants.ts (1)
195-195: LGTM!
The renaming of the constant to include version suffix maintains consistency with other versioned constants in the file.
proto/dydxprotocol/indexer/events/events.proto (2)
489-492: LGTM! Proper deprecation implementation.
The deprecation is well-documented with both comments and the deprecated option, following Protocol Buffers best practices.
Line range hint 517-544: LGTM! Well-structured V2 message implementation.
The V2 message maintains backward compatibility while adding the new market_type field. The implementation follows Protocol Buffers versioning best practices.
Let's verify that the PerpetualMarketType enum is properly defined:
✅ Verification successful
PerpetualMarketType enum is properly defined and consistent across the codebase
The enum is well-defined in proto/dydxprotocol/indexer/protocol/v1/perpetual.proto with appropriate values and documentation:
- PERPETUAL_MARKET_TYPE_UNSPECIFIED (0)
- PERPETUAL_MARKET_TYPE_CROSS (1)
- PERPETUAL_MARKET_TYPE_ISOLATED (2)
The implementation is consistent across all generated code (Rust, TypeScript) and follows Protocol Buffers best practices with proper zero-value default and sequential numbering.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the PerpetualMarketType enum definition
# Expected: Find the enum definition with appropriate market types
rg -A 10 "enum PerpetualMarketType"
Length of output: 8971
protocol/x/perpetuals/keeper/perpetual.go (1)
169-175: LGTM! Verify event constructor usage.
The addition of MarketType to the NewUpdatePerpetualEvent call is correct and aligns with the changes to support market type information in events.
Let's verify that all callers of NewUpdatePerpetualEvent have been updated to include the market type parameter:
✅ Verification successful
Let me analyze the results and generate another verification script to find all usages of NewUpdatePerpetualEvent in Go files, since the previous search didn't find the relevant code.
Let me check the implementation and usage of NewUpdatePerpetualEvent to verify all callers are updated correctly.
Let me check the implementation of the event constructor to verify it properly handles the MarketType parameter.
All callers of NewUpdatePerpetualEvent are properly updated with MarketType parameter
The verification confirms:
- The
NewUpdatePerpetualEventconstructor properly accepts and handles themarketTypeparameter - The only caller in
protocol/x/perpetuals/keeper/perpetual.gocorrectly passesperpetual.Params.MarketType - The test in
perpetual_test.goalso includes the MarketType parameter
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all usages of NewUpdatePerpetualEvent to ensure they include MarketType
# Search for NewUpdatePerpetualEvent calls
ast-grep --pattern 'NewUpdatePerpetualEvent($$$)'
Length of output: 67041
Script:
#!/bin/bash
# Search for NewUpdatePerpetualEvent in Go files
rg -U "NewUpdatePerpetualEvent\(" --type go
Length of output: 288
Script:
#!/bin/bash
# Get the implementation and all usages of NewUpdatePerpetualEvent
echo "=== NewUpdatePerpetualEvent implementation ==="
rg -U "func NewUpdatePerpetualEvent.*?\}" --multiline --type go
echo -e "\n=== All usages of NewUpdatePerpetualEvent ==="
rg -B 5 -A 5 "NewUpdatePerpetualEvent\(" --type go
Length of output: 2512
Script:
#!/bin/bash
# Get the full implementation of NewUpdatePerpetualEvent
cat protocol/indexer/events/perpetual.go
Length of output: 764
indexer/services/ender/src/scripts/handlers/dydx_update_perpetual_v1_handler.sql
Show resolved
Hide resolved
indexer/services/ender/__tests__/validators/update-perpetual-validator.test.ts
Show resolved
Hide resolved
indexer/services/ender/__tests__/handlers/update-perpetual-handler.test.ts
Outdated
Show resolved
Hide resolved
indexer/services/ender/__tests__/handlers/update-perpetual-handler.test.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
indexer/services/ender/__tests__/handlers/update-perpetual-handler.test.ts (1)
106-138: LGTM: Comprehensive perpetual market update test with V2 supportThe test effectively verifies both V1 and V2 events, including proper handling of V2-specific fields. The verification covers both database state and Kafka message publication.
Consider enhancing the test by adding a specific assertion for the
marketTypevalue when testing V2 events: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: event.marketType, + marketType: event.marketType, // Should match defaultUpdatePerpetualEventV2.marketType }), })); +if ("marketType" in event) { + expect(perpetualMarket!.marketType).toBe(defaultUpdatePerpetualEventV2.marketType); +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
indexer/services/ender/__tests__/handlers/update-perpetual-handler.test.ts(4 hunks)
🔇 Additional comments (4)
indexer/services/ender/__tests__/handlers/update-perpetual-handler.test.ts (4)
15-16: LGTM: Import changes for V1/V2 support
The imports are correctly updated to support both V1 and V2 versions of the UpdatePerpetualEvent.
Also applies to: 23-23
62-77: LGTM: Well-structured test parameterization
The test suite is effectively parameterized to cover both V1 and V2 events. The structure ensures that all test cases are run against both event versions, promoting code reuse and comprehensive coverage.
84-84: LGTM: Proper event handling test updates
The event handling test correctly uses the parameterized values for both event bytes and event object, ensuring proper testing of both V1 and V2 versions.
Also applies to: 100-100
143-143: LGTM: Helper function properly updated
The helper function is correctly modified to handle both V1 and V2 events through the use of event bytes parameter.
Also applies to: 149-149, 159-159
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
protocol/x/listing/keeper/keeper.go (2)
Line range hint
34-52: Consider standardizing the Event/Events namingThe parameter name
indexerEventsManager(plural) differs from the field nameindexerEventManager(singular). Consider standardizing the naming for consistency.- indexerEventsManager indexer_manager.IndexerEventManager, + indexerEventManager indexer_manager.IndexerEventManager,
67-69: Add documentation for exported methodConsider adding a documentation comment for the exported method
GetIndexerEventManager()to describe its purpose and usage.+// GetIndexerEventManager returns the indexer event manager instance used for handling +// perpetual market events and their versioning. func (k Keeper) GetIndexerEventManager() indexer_manager.IndexerEventManager {protocol/testutil/keeper/listing.go (1)
Line range hint
198-207: Consider parameter grouping for better maintainabilityWhile the addition of
indexerEventManageris correct, consider grouping related parameters together. Event management and store-related parameters could be grouped separately from the keeper dependencies.Consider reordering parameters like this:
func createListingKeeper( // Store and codec parameters stateStore storetypes.CommitMultiStore, db *dbm.MemDB, cdc *codec.ProtoCodec, - indexerEventManager indexer_manager.IndexerEventManager, // Event management + indexerEventManager indexer_manager.IndexerEventManager, // Keeper dependencies pricesKeeper *priceskeeper.Keeper, perpetualsKeeper *perpetualskeeper.Keeper, clobKeeper *clobkeeper.Keeper, marketMapKeeper *marketmapkeeper.Keeper, subaccountsKeeper *subaccountskeeper.Keeper, vaultkeeper *vaultkeeper.Keeper, )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
protocol/app/app.go(1 hunks)protocol/testutil/keeper/listing.go(3 hunks)protocol/x/listing/keeper/keeper.go(3 hunks)protocol/x/listing/keeper/listing.go(2 hunks)
🔇 Additional comments (8)
protocol/x/listing/keeper/keeper.go (3)
10-10: LGTM: Import addition is appropriate
The new import for indexer_manager is correctly placed and necessary for the new functionality.
17-26: LGTM: Keeper struct field addition is well-organized
The new indexerEventManager field is appropriately placed and follows the codebase's naming conventions.
Line range hint 1-69: Verify integration with event handling system
Let's verify how this new indexer event manager is being used in the event handling system.
✅ Verification successful
Integration with event handling system is properly implemented
The verification shows that the indexer event manager is well-integrated across the codebase:
- The
GetIndexerEventManager()method is consistently implemented across multiple keepers (Assets, Subaccounts, Vault, Sending, Rewards, etc.) - Event manager is properly used for both transaction events (
AddTxnEvent) and block events (AddBlockEvent) - The listing module correctly uses the event manager to emit events in
listing.go - Test coverage exists for event manager integration across modules
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for usages of GetIndexerEventManager to ensure proper integration
rg -t go "GetIndexerEventManager|indexerEventManager" --no-heading
# Look for related event handling code
ast-grep --pattern 'func ($x *Keeper) $event($ctx sdk.Context, $$$) {
$$$
$x.indexerEventManager.$$$
$$$
}'
Length of output: 20056
protocol/testutil/keeper/listing.go (3)
Line range hint 177-183: LGTM: Proper dependency injection in keeper initialization
The addition of indexerEventManager to the createListingKeeper call maintains proper dependency injection patterns and aligns with the PR's objective of supporting the new event management functionality.
Line range hint 220-227: LGTM: Proper event manager integration
The indexerEventManager is correctly passed to NewKeeper, enabling proper event handling for the new UpdatePerpetualEventV2 functionality in tests.
Line range hint 177-227: Verify test coverage for both event versions
Since this test infrastructure supports the new UpdatePerpetualEventV2, we should verify that tests exist for both V1 and V2 event handling.
✅ Verification successful
Based on the search results, I can now generate a final response since I have enough information to verify the event handling:
V2 event is properly implemented and V1 is deprecated
The codebase shows proper implementation of UpdatePerpetualEventV2 with V1 being explicitly marked as deprecated. Key findings:
- UpdatePerpetualEventV1 is marked with "Deprecated. See UpdatePerpetualEventV2"
- All test files and implementations use V2 events exclusively
- The event creation helper NewUpdatePerpetualEvent generates V2 events
- No active usage of V1 events found in the codebase
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for test coverage of both V1 and V2 event handling
# Search for test files that handle UpdatePerpetualEvent
echo "Searching for test files handling UpdatePerpetualEvent..."
rg -l "UpdatePerpetualEvent" --type go --glob "*_test.go"
# Search for specific V1 and V2 event handling in tests
echo "Checking for V1 and V2 event handling..."
rg "UpdatePerpetualEventV1|UpdatePerpetualEventV2" --type go --glob "*_test.go" -C 2
Length of output: 3020
Script:
#!/bin/bash
# Let's verify the implementation details to ensure proper handling of both versions
# Check for V1 event definitions and their usage
echo "Checking for V1 event definitions and implementations..."
rg "UpdatePerpetualEvent[^V]|UpdatePerpetualEventV1" --type go -C 3
# Check for V2 event definitions and their implementations
echo "Checking for V2 event definitions and implementations..."
ast-grep --pattern 'type UpdatePerpetualEventV2 struct {
$$$
}'
# Check for any migration or conversion logic between V1 and V2
echo "Checking for migration/conversion logic..."
rg "V1.*V2|convert.*Perpetual" --type go -C 2
Length of output: 148093
protocol/x/listing/keeper/listing.go (1)
9-9: LGTM: Import changes are appropriate.
The new imports for indexer events are correctly added and properly organized.
Also applies to: 16-16
protocol/app/app.go (1)
1241-1241: LGTM! Verify event handling implementation.
The addition of IndexerEventManager to the ListingKeeper constructor is appropriate for supporting the upgrade from isolated to cross perpetual contracts. This change enables the keeper to emit events for indexing.
Let's verify the event handling implementation in the listing keeper:
✅ Verification successful
Based on the verification results, I can now provide a final response:
Event handling implementation verified and properly integrated
The addition of IndexerEventManager to the ListingKeeper is correctly implemented with proper event emission:
- The keeper properly emits
UpdatePerpetualEventV2events using version 2 of the event schema - Events are emitted through the standard indexer event manager interface
- The event contains all necessary fields including market type for cross/isolated perpetual distinction
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check the implementation of event handling in the listing keeper
# Check for event emission in the listing keeper
rg -A 5 "IndexerEventManager" "protocol/x/listing/keeper/"
# Check for any new event types related to perpetual upgrades
rg -A 5 "UpdatePerpetualEventV2" "protocol/x/listing/types/"
Length of output: 5616
Script:
#!/bin/bash
# Let's verify the event handling implementation more thoroughly
# Check for UpdatePerpetual event usage in the listing module
rg -A 5 "UpdatePerpetual" "protocol/x/listing/"
# Check for any new event types in indexer events
rg -A 5 "UpdatePerpetual" "protocol/indexer/events/"
# Check the implementation of UpgradeIsolatedPerpetualToCross
ast-grep --pattern 'func (k Keeper) UpgradeIsolatedPerpetualToCross($$$) {
$$$
}'
Length of output: 19933
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
protocol/x/listing/keeper/msg_server_upgrade_isolated_perpetual_to_cross_test.go (1)
192-201: Consider adding error handling for event verificationWhile the event verification is thorough, consider adding error handling for the case where no events are emitted. Currently, if
getUpdatePerpetualEventsFromIndexerBlockreturns an empty slice, the test might not provide clear feedback about why it failed.- require.Equal(t, emittedIndexerEvents, []*indexerevents.UpdatePerpetualEventV2{expectedIndexerEvent}) + require.Len(t, emittedIndexerEvents, 1, "expected one UpdatePerpetual event") + require.Equal(t, expectedIndexerEvent, emittedIndexerEvents[0], "unexpected event content")Also applies to: 242-244
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
protocol/x/listing/keeper/listing.go(2 hunks)protocol/x/listing/keeper/msg_server_upgrade_isolated_perpetual_to_cross_test.go(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- protocol/x/listing/keeper/listing.go
🔇 Additional comments (2)
protocol/x/listing/keeper/msg_server_upgrade_isolated_perpetual_to_cross_test.go (2)
9-12: LGTM: Import additions for indexer event handling
The new imports are necessary and properly organized for handling indexer events in the test.
Line range hint 85-107: LGTM: Improved test case naming convention
The test case naming changes from "Failure: ..." to "Failure - ..." improve readability and maintain consistency across test cases.
protocol/x/listing/keeper/msg_server_upgrade_isolated_perpetual_to_cross_test.go
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
protocol/x/listing/keeper/msg_server_upgrade_isolated_perpetual_to_cross_test.go (1)
226-239: Consider adding a comment explaining the event verification.The indexer event verification is thorough, but a brief comment explaining what we're verifying would improve test readability.
+ // Verify that the upgrade correctly emits an UpdatePerpetualEventV2 with CROSS market type // Verify that expected indexer event was emitted.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
protocol/testutil/keeper/listing.go(4 hunks)protocol/x/listing/keeper/listing_test.go(0 hunks)protocol/x/listing/keeper/msg_server_upgrade_isolated_perpetual_to_cross_test.go(5 hunks)
💤 Files with no reviewable changes (1)
- protocol/x/listing/keeper/listing_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- protocol/testutil/keeper/listing.go
🔇 Additional comments (3)
protocol/x/listing/keeper/msg_server_upgrade_isolated_perpetual_to_cross_test.go (3)
9-12: LGTM!
The new imports are necessary for handling indexer events in the test.
Line range hint 84-106: LGTM!
The test case naming convention change improves readability and consistency.
245-266: Past review comment about error handling is still applicable.
The existing review comment about improving error handling in this helper function remains valid.
hwray
changed the title
Changelist
UpgradePerpetualEventV2+ indexer changes forUpgradeIsolatedPerpetualToCrossTest Plan
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
New Features
UpdatePerpetualEventV2with enhanced structure includingmarketType.LiquidityTierUpsertEventV2for improved liquidity tier functionality.UpdatePerpetualEventV1andUpdatePerpetualEventV2.Improvements
Deprecations
UpdatePerpetualEventV1andLiquidityTierUpsertEventV1as deprecated, encouraging transition to their newer counterparts.