[TRA-572] Add handler for upsert vault events to ender. #2274
Conversation
|
Warning Rate limit exceeded@vincentwschau has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 17 minutes and 44 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThis pull request introduces enhancements to the vault management system, focusing on the integration of the Changes
Possibly related PRs
Suggested labels
Poem
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: 0
Outside diff range and nitpick comments (3)
indexer/services/ender/src/handlers/upsert-vault-handler.ts (1)
7-18: LGTM! The class structure is set up correctly.The
UpsertVaultHandlerclass extends theHandlerbase class with the appropriate generic type parameterUpsertVaultEventV1. TheeventTypeproperty is correctly set to identify the type of event this handler processes.The
getParallelizationIdsmethod returns an empty array, indicating that no parallelization identifiers are used for event processing. This is acceptable if parallelization is not required for this specific handler.The
internalHandlemethod is prepared to handle incoming data of typepg.QueryResultRowand returns a promise that resolves to an empty array of typeConsolidatedKafkaEvent[]. While the method is set up correctly, the actual event processing logic has yet to be implemented.As a next step, consider implementing the necessary logic within the
internalHandlemethod to process the incomingUpsertVaultEventV1events and perform any required data transformations or operations. This may involve interacting with the database, validating data, or emitting new events based on the business requirements.indexer/services/ender/src/scripts/helpers/dydx_protocol_vault_status_to_vault_status.sql (1)
1-2: Add a comment to explain the function's purpose.Consider adding a brief comment above the function definition to explain its purpose and the meaning of the mapped statuses. This will help future maintainers understand the function's role in the codebase.
+/** + * Converts a JSONB representation of vault status to a human-readable text format. + * The function maps the following JSONB values to their corresponding statuses: + * - '1': DEACTIVATED + * - '2': STAND_BY + * - '3': QUOTING + * - '4': CLOSE_ONLY + * @param vaultStatus The JSONB representation of the vault status. + * @return The corresponding human-readable text status. + * @throws If the provided vault status is invalid. + */ CREATE OR REPLACE FUNCTION dydx_protocol_vault_status_to_vault_status(vaultStatus jsonb) RETURNS text AS $$indexer/packages/postgres/src/models/vault-model.ts (1)
35-49: LGTM! ThesqlToJsonConversionsmethod is a valuable addition to theVaultModelclass.The new static getter method
sqlToJsonConversionsprovides a clear mapping from column names to their expected JSON conversion types. This mapping can improve data handling and validation processes by ensuring that the fields are converted to the expected data types.The comment block and TODO note are also helpful for developers working with this code, promoting code maintainability and consistency.
A few suggestions:
- Address the TODO note to ensure consistency between the JSON schema, SQL conversions, and model fields. This will help maintain a cohesive and reliable data model.
- Consider adding unit tests to verify the functionality of the
sqlToJsonConversionsmethod. This will ensure that the mapping behaves as expected and catches any potential issues early in the development process.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (16)
- indexer/packages/postgres/src/constants.ts (2 hunks)
- indexer/packages/postgres/src/db/migrations/migration_files/20240912180829_create_vaults_table.ts (1 hunks)
- indexer/packages/postgres/src/index.ts (1 hunks)
- indexer/packages/postgres/src/models/vault-model.ts (1 hunks)
- indexer/packages/v4-protos/src/index.ts (1 hunks)
- indexer/services/ender/tests/handlers/upsert-vault-handler.test.ts (1 hunks)
- indexer/services/ender/tests/validators/upsert-vault-validator.test.ts (1 hunks)
- indexer/services/ender/src/handlers/upsert-vault-handler.ts (1 hunks)
- indexer/services/ender/src/helpers/postgres/postgres-functions.ts (2 hunks)
- indexer/services/ender/src/lib/block-processor.ts (2 hunks)
- indexer/services/ender/src/lib/helper.ts (2 hunks)
- indexer/services/ender/src/lib/types.ts (3 hunks)
- indexer/services/ender/src/scripts/handlers/dydx_block_processor_ordered_handlers.sql (1 hunks)
- indexer/services/ender/src/scripts/handlers/dydx_vault_upsert_handler.sql (1 hunks)
- indexer/services/ender/src/scripts/helpers/dydx_protocol_vault_status_to_vault_status.sql (1 hunks)
- indexer/services/ender/src/validators/upsert-vault-validator.ts (1 hunks)
Additional context used
Biome
indexer/services/ender/src/validators/upsert-vault-validator.ts
[error] 10-12: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
Additional comments not posted (23)
indexer/services/ender/src/scripts/helpers/dydx_protocol_vault_status_to_vault_status.sql (1)
6-9: LGTM!The CASE statement correctly maps the expected JSONB values to their corresponding text statuses. The logic is clear and concise.
indexer/packages/postgres/src/db/migrations/migration_files/20240912180829_create_vaults_table.ts (1)
9-9: Verify the impact of the enum value change across the codebase.The
statusenum value has been changed from'STANDBY'to'STAND_BY'. This change could have the following impact:
- Existing records with the old enum value would become invalid, leading to data integrity issues.
- Any code that relies on the old enum value would break, leading to runtime errors.
- Inconsistent naming conventions could lead to confusion and maintainability issues.
Please run the following script to verify the enum value usage across the codebase:
If the above tests fail, please update the codebase and database records to use the new enum value consistently.
Verification successful
Enum value change has been consistently applied across the codebase
The comprehensive search for 'STANDBY' and 'STAND_BY' usage in the codebase has revealed that the enum value change has been consistently applied. Here are the key findings:
- No occurrences of the old 'STANDBY' value were found in any files.
- The new 'STAND_BY' value is used consistently across TypeScript, Go, SQL, and protocol definition files.
- The migration file (20240912180829_create_vaults_table.ts) correctly uses the new 'STAND_BY' value.
Based on these results, it appears that the enum value change has been thoroughly implemented throughout the codebase. There is no evidence of any remaining usage of the old 'STANDBY' value, which suggests that there are likely no data integrity issues or broken code related to this change.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the old enum value 'STANDBY' across the codebase. # Test 1: Search for the old enum value usage in TypeScript files. # Expect: No occurrences of the old enum value. rg --type ts 'STANDBY' # Test 2: Search for the old enum value usage in the database. # Expect: No occurrences of the old enum value. echo "SELECT COUNT(*) FROM vaults WHERE status = 'STANDBY';" | psql <database_name>Length of output: 251
Script:
#!/bin/bash # Description: Comprehensive search for 'STANDBY' and 'STAND_BY' usage in the codebase # Search for 'STANDBY' in all files echo "Searching for 'STANDBY' in all files:" rg 'STANDBY' echo "\nSearching for 'STAND_BY' in all files:" rg 'STAND_BY' # Search for enum declarations containing 'STANDBY' or 'STAND_BY' echo "\nSearching for enum declarations with 'STANDBY' or 'STAND_BY':" ast-grep --lang typescript --pattern 'enum $_ { $$$ STANDBY $$$ }' ast-grep --lang typescript --pattern 'enum $_ { $$$ STAND_BY $$$ }' # Search for string literals 'STANDBY' or 'STAND_BY' in TypeScript files echo "\nSearching for string literals 'STANDBY' or 'STAND_BY' in TypeScript files:" ast-grep --lang typescript --pattern '"STANDBY"' ast-grep --lang typescript --pattern "'STANDBY'" ast-grep --lang typescript --pattern '"STAND_BY"' ast-grep --lang typescript --pattern "'STAND_BY'"Length of output: 4889
indexer/services/ender/src/validators/upsert-vault-validator.ts (3)
7-31: LGTM!The
UpsertVaultValidatorclass is well-structured and aligns with the PR objectives of introducing a validator specifically designed for handlingUpsertVaultEventV1events. The class extends theValidatorclass and provides a clear validation mechanism and a method to create handlers for the events.Tools
Biome
[error] 10-12: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
8-14: LGTM!The
validatemethod correctly checks if theaddressfield is an empty string and throws an error if it is. This aligns with the purpose of the validator class to ensure that theaddressfield is populated.Regarding the static analysis hint:
The
logAndThrowParseMessageErrormethod likely throws an error, which would prevent the function from returning a value. Therefore, the static analysis hint can be safely ignored in this case.Tools
Biome
[error] 10-12: The function should not return a value because its return type is void.
The function is here:
'void' signals the absence of value. The returned value is likely to be ignored by the caller.
(lint/correctness/noVoidTypeReturn)
16-30: LGTM!The
createHandlersmethod correctly creates a newUpsertVaultHandlerwith the necessary parameters and returns an array ofHandlerinstances. This structure allows for the processing of upsert vault events in a modular and organized manner, facilitating the handling of these events in the broader indexing system. The method aligns with the PR objectives and the AI-generated summary.indexer/packages/v4-protos/src/index.ts (1)
19-19: LGTM!The change to export everything from the
./codegen/dydxprotocol/vault/vaultmodule is straightforward and enhances the module's interface by including additional functionality related to the vault. It does not alter any existing exports or the overall structure of the file.indexer/services/ender/src/scripts/handlers/dydx_vault_upsert_handler.sql (1)
1-37: Excellent implementation of thedydx_vault_upsert_handlerfunction!The function effectively handles the upsert operation for vault records in the database. Here are some key highlights:
- The function signature and parameters are well-defined and documented, making it clear what the function expects and returns.
- The use of the
DECLAREblock to define variables is a good practice for readability and maintainability.- The function correctly extracts relevant data from
event_datato populate thevault_record, ensuring that the latest information is stored in the database.- The
INSERTstatement with theON CONFLICTclause efficiently handles both new records and updates to existing records, maintaining data integrity and preventing duplicates.- Returning the upserted vault as a JSON object using
jsonb_build_objectanddydx_to_jsonbfunctions provides flexibility for further processing or integration with other systems.Overall, this implementation ensures that vault records are accurately maintained in the database, reflecting the latest status and timestamps based on incoming event data.
indexer/services/ender/__tests__/validators/upsert-vault-validator.test.ts (2)
19-57: LGTM!The test suite follows the standard structure of setup, teardown, and test cases. It covers the happy path scenario of validating a valid
UpsertVaultEventV1event using theUpsertVaultValidatorclass.The test suite ensures that the
UpsertVaultValidatorbehaves as expected when processing valid events.
59-75: LGTM!The
createBlockfunction is a helper that simplifies the creation of anIndexerTendermintBlockfor testing purposes. It ensures that the providedUpsertVaultEventV1event is properly encoded and associated with default parameters such as height, time, and transaction hash.The function is used in the test case to create a block corresponding to the valid event, making the test setup more concise and readable.
indexer/services/ender/src/scripts/handlers/dydx_block_processor_ordered_handlers.sql (1)
67-68: LGTM! The new event handler enhances vault management capabilities.The addition of the
"upsert_vault"event handling in theCASEstatement allows thedydx_block_processor_ordered_handlersfunction to process vault upsert events. When this event type is encountered, thedydx_vault_upsert_handlerfunction is called with theblock_timeandevent_dataas arguments, which suggests that it is responsible for updating or inserting vault-related data based on the provided event data.This enhancement expands the script's capabilities to accommodate additional event types related to vault management, improving the overall functionality of the system.
Please ensure that the implementation of the
dydx_vault_upsert_handlerfunction is thoroughly tested to confirm that it correctly processes vault upsert events and updates the relevant data structures as expected. Consider running the following script to verify the function's usage and behavior:Verification successful
LGTM with a note on testing. The
dydx_vault_upsert_handlerfunction is correctly implemented and used.The
dydx_vault_upsert_handlerfunction indydx_vault_upsert_handler.sqlis correctly defined with the expected signature(block_time timestamp, event_data jsonb). It properly handles both insertion of new vault records and updating existing ones, which aligns with its usage in thedydx_block_processor_ordered_handlers.sqlfile.The function implementation looks sound, processing the input data and performing the upsert operation on the
vaultstable as expected. It also returns the upserted vault data in the required format.However, it's worth noting that we couldn't find any specific unit tests for this function. While this doesn't necessarily indicate a problem with the implementation, it might be beneficial to add targeted tests to ensure the function behaves correctly under various scenarios.
To further improve the codebase:
- Consider adding unit tests specifically for the
dydx_vault_upsert_handlerfunction to ensure its behavior is verified under different input conditions.- If there's a general testing strategy for similar handler functions, it might be helpful to document it for consistency across the project.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation and usage of the `dydx_vault_upsert_handler` function. # Test: Search for the function definition. # Expect: The function should be defined with the correct signature. ast-grep --lang sql --pattern $'def dydx_vault_upsert_handler(block_time, event_data)' # Test: Search for the function usage within the `dydx_block_processor_ordered_handlers` function. # Expect: The function should be called with the correct arguments when the `"upsert_vault"` event type is encountered. rg --type sql -A 5 $'dydx_vault_upsert_handler\(block_time, event_data\)' # Test: Search for unit tests related to the `dydx_vault_upsert_handler` function. # Expect: There should be unit tests covering various scenarios and edge cases. fd 'test_dydx_vault_upsert_handler' --exec rg --type sql $'def test_dydx_vault_upsert_handler'Length of output: 27777
Script:
#!/bin/bash # Search for the function definition with a more flexible pattern ast-grep --lang sql --pattern 'def dydx_vault_upsert_handler' # Search for any file containing the function name (might catch the definition) rg --type sql 'dydx_vault_upsert_handler' # Search for test files that might include tests for this function fd test --type sql --exec rg --type sql 'dydx_vault_upsert_handler' # Search for any documentation or comments about testing strategy rg --type sql 'test.*strategy|testing.*approach'Length of output: 1031
Script:
#!/bin/bash # Examine the content of dydx_vault_upsert_handler.sql cat indexer/services/ender/src/scripts/handlers/dydx_vault_upsert_handler.sql # Search for test files that might include tests for this function (corrected command) fd -e sql | xargs grep -i 'test.*dydx_vault_upsert_handler' # Broader search for testing-related comments or documentation rg --type sql -i 'test|spec|assert|verify|validate'Length of output: 2718
indexer/services/ender/src/helpers/postgres/postgres-functions.ts (2)
49-49: LGTM!The addition of the
'dydx_vault_upsert_handler.sql'script to theHANDLER_SCRIPTSarray is a valid enhancement to handle vault upsert events. The naming convention is consistent with the existing scripts.
95-95: Looks good!The addition of the
'dydx_protocol_vault_status_to_vault_status.sql'script to theHELPER_SCRIPTSarray is a valid enhancement to facilitate the conversion or mapping of protocol vault statuses to a standardized format. The naming convention is consistent with the existing scripts.indexer/packages/postgres/src/index.ts (1)
51-51: LGTM!The export statement is consistent with the existing export statements in the file and allows other parts of the application to access the
VaultTablefunctionality.indexer/services/ender/__tests__/handlers/upsert-vault-handler.test.ts (3)
50-91: LGTM!The test case for upserting new vaults in a single block looks good. It correctly sets up the test data, sends the Kafka message, and verifies the expected behavior.
93-125: LGTM!The test case for upserting an existing vault looks good. It correctly sets up the test data, sends the Kafka message, and verifies the expected behavior.
128-150: LGTM!The
createBlockFromEventshelper function looks good. It correctly creates anIndexerTendermintBlockobject from the provided vault events.indexer/packages/postgres/src/constants.ts (2)
25-25: LGTM!The import statement for
VaultModelis syntactically correct and the import path is valid.
107-107: Looks good!The addition of
VaultModelto theSQL_TO_JSON_DEFINED_MODELSarray is correct and consistent with the existing pattern. This enables the conversion ofVaultModelfrom SQL to JSON format, extending the module's data handling capabilities.indexer/services/ender/src/lib/types.ts (2)
63-63: LGTM!The addition of the
UPSERT_VAULTenum value toDydxIndexerSubtypesis consistent with the existing structure and naming convention. It will enable the system to handle events related to vault operations.
178-183: Looks good!The new case added to the
EventProtoWithTypeAndVersiontype for theUPSERT_VAULTevent subtype is consistent with the existing structure and correctly associates it with theUpsertVaultEventV1protocol buffer message. This will enable type-safe handling of vault-related events.indexer/services/ender/src/lib/helper.ts (1)
255-263: LGTM!The new case for handling the
UPSERT_VAULTevent subtype is implemented correctly and consistently with the existing cases in the function. It enhances the functionality of the event processing logic by enabling it to decode and processUpsertVaultEventV1events.indexer/services/ender/src/lib/block-processor.ts (2)
33-33: LGTM!The import statement for
UpsertVaultValidatorfollows the existing naming convention and import pattern.
59-59: Looks good!The addition of
UpsertVaultValidatorto theTXN_EVENT_SUBTYPE_VERSION_TO_VALIDATOR_MAPPINGandBLOCK_EVENT_SUBTYPE_VERSION_TO_VALIDATOR_MAPPINGmappings enables the validation and processing of vault upsert events for both transaction and block events.This change expands the functionality of the block processor to handle the
UPSERT_VAULTsubtype, with the validator being invoked when a corresponding event is encountered.Also applies to: 67-67
Changelist
Add sql handler for upsert vault events.
Test Plan
Unit tests.
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
VaultModelfor enhanced SQL to JSON data handling.UPSERT_VAULTevent type, expanding event processing capabilities.Bug Fixes
Tests
Documentation