[IND-399] Fix line numbers in SQL function stack traces #821
Conversation
This makes so that the PG_EXCEPTION_CONTEXT contains the correct line numbers giving a stack like: PL/pgSQL function dydx_market_modify_handler(jsonb) line 17 at RAISE PL/pgSQL function dydx_block_processor(jsonb) line 85 at assignment
IND-399 Add logging and error handling to SQL statements
|
WalkthroughThe changes involve the addition of numerous SQL functions across various scripts in the Changes
TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 20
Configuration used: CodeRabbit UI
Files selected for processing (46)
- indexer/services/ender/src/scripts/dydx_asset_create_handler.sql (1 hunks)
- indexer/services/ender/src/scripts/dydx_clob_pair_status_to_market_status.sql (1 hunks)
- indexer/services/ender/src/scripts/dydx_create_initial_rows_for_tendermint_block.sql (1 hunks)
- indexer/services/ender/src/scripts/dydx_create_tendermint_event.sql (1 hunks)
- indexer/services/ender/src/scripts/dydx_create_transaction.sql (1 hunks)
- indexer/services/ender/src/scripts/dydx_deleveraging_handler.sql (2 hunks)
- indexer/services/ender/src/scripts/dydx_event_id_from_parts.sql (2 hunks)
- indexer/services/ender/src/scripts/dydx_from_jsonlib_long.sql (2 hunks)
- indexer/services/ender/src/scripts/dydx_from_protocol_order_side.sql (1 hunks)
- indexer/services/ender/src/scripts/dydx_from_protocol_time_in_force.sql (1 hunks)
- indexer/services/ender/src/scripts/dydx_from_serializable_int.sql (1 hunks)
- indexer/services/ender/src/scripts/dydx_funding_handler.sql (2 hunks)
- indexer/services/ender/src/scripts/dydx_get_fee_from_liquidity.sql (1 hunks)
- indexer/services/ender/src/scripts/dydx_get_order_status.sql (2 hunks)
- indexer/services/ender/src/scripts/dydx_get_perpetual_market_for_clob_pair.sql (1 hunks)
- indexer/services/ender/src/scripts/dydx_get_total_filled_from_liquidity.sql (1 hunks)
- indexer/services/ender/src/scripts/dydx_get_weighted_average.sql (2 hunks)
- indexer/services/ender/src/scripts/dydx_liquidation_fill_handler_per_order.sql (2 hunks)
- indexer/services/ender/src/scripts/dydx_liquidity_tier_handler.sql (1 hunks)
- indexer/services/ender/src/scripts/dydx_market_create_handler.sql (1 hunks)
- indexer/services/ender/src/scripts/dydx_market_modify_handler.sql (1 hunks)
- indexer/services/ender/src/scripts/dydx_market_price_update_handler.sql (2 hunks)
- indexer/services/ender/src/scripts/dydx_order_fill_handler_per_order.sql (2 hunks)
- indexer/services/ender/src/scripts/dydx_perpetual_market_handler.sql (1 hunks)
- indexer/services/ender/src/scripts/dydx_perpetual_position_and_order_side_matching.sql (1 hunks)
- indexer/services/ender/src/scripts/dydx_protocol_condition_type_to_order_type.sql (1 hunks)
- indexer/services/ender/src/scripts/dydx_stateful_order_handler.sql (2 hunks)
- indexer/services/ender/src/scripts/dydx_subaccount_update_handler.sql (2 hunks)
- indexer/services/ender/src/scripts/dydx_tendermint_event_to_transaction_index.sql (1 hunks)
- indexer/services/ender/src/scripts/dydx_transfer_handler.sql (2 hunks)
- indexer/services/ender/src/scripts/dydx_trim_scale.sql (1 hunks)
- indexer/services/ender/src/scripts/dydx_update_clob_pair_handler.sql (1 hunks)
- indexer/services/ender/src/scripts/dydx_update_perpetual_handler.sql (1 hunks)
- indexer/services/ender/src/scripts/dydx_update_perpetual_position_aggregate_fields.sql (2 hunks)
- indexer/services/ender/src/scripts/dydx_uuid.sql (1 hunks)
- indexer/services/ender/src/scripts/dydx_uuid_from_asset_position_parts.sql (1 hunks)
- indexer/services/ender/src/scripts/dydx_uuid_from_fill_event_parts.sql (1 hunks)
- indexer/services/ender/src/scripts/dydx_uuid_from_funding_index_update_parts.sql (1 hunks)
- indexer/services/ender/src/scripts/dydx_uuid_from_oracle_price_parts.sql (1 hunks)
- indexer/services/ender/src/scripts/dydx_uuid_from_order_id.sql (1 hunks)
- indexer/services/ender/src/scripts/dydx_uuid_from_order_id_parts.sql (1 hunks)
- indexer/services/ender/src/scripts/dydx_uuid_from_perpetual_position_parts.sql (1 hunks)
- indexer/services/ender/src/scripts/dydx_uuid_from_subaccount_id.sql (1 hunks)
- indexer/services/ender/src/scripts/dydx_uuid_from_subaccount_id_parts.sql (1 hunks)
- indexer/services/ender/src/scripts/dydx_uuid_from_transaction_parts.sql (1 hunks)
- indexer/services/ender/src/scripts/dydx_uuid_from_transfer_parts.sql (1 hunks)
Files skipped from review due to trivial changes (11)
- indexer/services/ender/src/scripts/dydx_from_serializable_int.sql
- indexer/services/ender/src/scripts/dydx_get_fee_from_liquidity.sql
- indexer/services/ender/src/scripts/dydx_get_total_filled_from_liquidity.sql
- indexer/services/ender/src/scripts/dydx_get_weighted_average.sql
- indexer/services/ender/src/scripts/dydx_liquidation_fill_handler_per_order.sql
- indexer/services/ender/src/scripts/dydx_liquidity_tier_handler.sql
- indexer/services/ender/src/scripts/dydx_market_modify_handler.sql
- indexer/services/ender/src/scripts/dydx_transfer_handler.sql
- indexer/services/ender/src/scripts/dydx_trim_scale.sql
- indexer/services/ender/src/scripts/dydx_uuid.sql
- indexer/services/ender/src/scripts/dydx_uuid_from_asset_position_parts.sql
Additional comments: 55
indexer/services/ender/src/scripts/dydx_clob_pair_status_to_market_status.sql (1)
- 1-15: The function
dydx_clob_pair_status_to_market_statushas been correctly implemented with a detailed comment block, appropriate error handling, and performance considerations such as being declared IMMUTABLE and PARALLEL SAFE. Ensure that all references to ClobPairStatus throughout the codebase are updated to use this new function for consistency.indexer/services/ender/src/scripts/dydx_create_initial_rows_for_tendermint_block.sql (1)
- 14-15: The summary states that the comment "Create block." has been removed from the function body, but it is still present in the code. This discrepancy should be addressed.
- -- Create block.indexer/services/ender/src/scripts/dydx_create_tendermint_event.sql (2)
2-14: The function
dydx_create_tendermint_eventusesblock_heightas text and casts it to an integer and bigint within the function. Ensure that theblock_heightpassed to this function is always a valid integer string to prevent runtime errors.14-14: The function casts
event->>'eventIndex'directly to an integer without validation. Consider adding a check to ensure thateventIndexis a valid integer string to avoid runtime errors.indexer/services/ender/src/scripts/dydx_create_transaction.sql (1)
- 1-15: The function
dydx_create_transactionis well-documented and follows the guidelines provided in the summary. The parameters and return type are correctly implemented, and the function includes a comment to ensure the accuracy of exception line numbers. The use ofdydx_uuid_from_transaction_partsfor generating a UUID and the insertion into thetransactionstable are correctly done. The function appears to be correct and well-structured.indexer/services/ender/src/scripts/dydx_deleveraging_handler.sql (2)
- 23-25: The summary indicates that the
DECLAREblock for constants has been removed, but the code still contains aDECLAREblock with constantsQUOTE_CURRENCY_ATOMIC_RESOLUTIONandFEE. Please verify if the summary is accurate or if the code needs to be updated to reflect the intended changes.- QUOTE_CURRENCY_ATOMIC_RESOLUTION constant numeric = -6; - FEE constant numeric = 0;
- 21-21: The comment about ensuring no text exists before the function declaration is a good practice for maintaining the accuracy of exception line numbers and aligns with the pull request's goal of improving maintainability and debuggability.
indexer/services/ender/src/scripts/dydx_event_id_from_parts.sql (3)
- 7-15: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [1-14]
The function
dydx_event_id_from_partsis well-documented and the logic for creating a bytea event ID from the given parts appears to be correct. The use ofint4sendto convert integers to network order before concatenation is appropriate for bytea type.
- 7-15: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [2-11]
The comment block provides clear documentation on the purpose of each parameter and the function itself. The note about ensuring correct exception line numbers is a good practice for maintainability and debugging.
- 14-14: The function is correctly marked as
IMMUTABLEandPARALLEL SAFE, which should help with performance in parallel query environments, assuming the function's logic truly does not modify any database state and gives the same result for the same inputs consistently.indexer/services/ender/src/scripts/dydx_from_jsonlib_long.sql (1)
- 8-16: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [1-16]
The function
dydx_from_jsonlib_longis well-documented with a clear comment block explaining its purpose and input format. The use of a constantPOWER_2_32for the conversion process is a good practice for readability and maintainability. The function is also correctly marked as IMMUTABLE and PARALLEL SAFE, which should aid in performance when used in parallel query execution. The comment regarding the placement of text before the function declaration is important for accurate exception line number reporting and is well-placed.indexer/services/ender/src/scripts/dydx_from_protocol_order_side.sql (1)
- 1-10: The function
dydx_from_protocol_order_sidecorrectly implements the conversion from the 'Side' enum to the 'OrderSide' enum, and the note about ensuring no text exists before the function declaration is present to maintain the accuracy of exception line numbers. The function is also marked as IMMUTABLE and PARALLEL SAFE, which is appropriate for its purpose.indexer/services/ender/src/scripts/dydx_from_protocol_time_in_force.sql (1)
- 1-12: The function
dydx_from_protocol_time_in_forcecorrectly implements the conversion of TimeInForce field from JSONB to text, including a default case and exception handling for unexpected values. The note about no text before the function declaration is also present to ensure accurate exception line numbers.indexer/services/ender/src/scripts/dydx_funding_handler.sql (5)
- 12-20: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [1-20]
The function
dydx_funding_handleris well-documented with parameter descriptions and return value expectations. The note about no text before the function declaration is important for maintaining accurate exception line numbers.
16-16: The note about ensuring no text before the function declaration is crucial for debugging and should be adhered to in all SQL function scripts.
18-20: Constants are defined at the beginning of the function, which is good for maintainability and readability.
18-20: The use of constants like
PPM_EXPONENTandFUNDING_RATE_FROM_PROTOCOL_IN_HOURSis good for clarity and maintainability, but it's important to ensure that these constants are accurate and reflect the domain logic correctly.18-20: The use of
jsonb_build_objectto construct the response is appropriate and ensures that the return type is consistent with the function's documentation.indexer/services/ender/src/scripts/dydx_get_order_status.sql (3)
1-5: The function signature and documentation have been updated to include the new parameter
order_canceled_status. Ensure that all calls to this function are updated to pass this new parameter.27-27: The note about not having text before the function declaration is important for maintaining accurate exception line numbers. This is a good practice for maintainability and debuggability.
30-31: The logic to return 'FILLED' when
total_filledis greater than or equal tosizeis correct and aligns with the expected behavior described in the documentation.indexer/services/ender/src/scripts/dydx_get_perpetual_market_for_clob_pair.sql (1)
- 1-15: The function
dydx_get_perpetual_market_for_clob_pairis well-defined with clear documentation and appropriate exception handling. The use ofSELECT INTO STRICTis correct for ensuring that exactly one row is returned or an exception is raised. The note about not having text before the function declaration is also present, which is important for maintaining the accuracy of exception line numbers.indexer/services/ender/src/scripts/dydx_market_create_handler.sql (1)
- 1-13: The function
dydx_market_create_handleris well-documented with clear parameter and return value descriptions. The note to ensure no text exists before the function declaration is also present, which is important for accurate exception line numbers.indexer/services/ender/src/scripts/dydx_market_price_update_handler.sql (1)
- 8-16: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [1-37]
The function implementation looks good and follows the requirements specified in the summary. The documentation is clear, and the code is well-structured.
indexer/services/ender/src/scripts/dydx_order_fill_handler_per_order.sql (5)
1-3: The summary indicates that the function
dydx_order_fill_handler_per_orderhas been modified to includefill,perpetual_market, andperpetual_positionas parameters, but these are not present in the function signature. Please verify if the summary is accurate or if the function signature needs to be updated.25-25: The addition of the note in the comment block to ensure no text exists before the function declaration is a good practice for accurate exception line numbers.
27-27: Using the
STRICToption in theSELECT INTOstatement is a good practice for ensuring that exactly one row is returned, which helps maintain data integrity.27-27: The use of exception handling for the case where no data is found for an asset is a robust error handling practice.
27-27: Ensure that all type casting and mathematical operations are correct and that there is proper validation of inputs to prevent potential errors.
indexer/services/ender/src/scripts/dydx_perpetual_market_handler.sql (2)
1-13: The function
dydx_perpetual_market_handleris well-documented, and the comment about not having text before the function declaration is in place to ensure correct exception line numbers. This aligns with the summary provided.1-13: > Note: This review was outside the patches, and no patch overlapping with it was found. Original lines [17-21]
Verify whether the hard-coded initialization of fields like
lastPrice,priceChange24H,trades24H,volume24H,nextFundingRate, andopenInterestto 0 is intentional. If these values are supposed to be derived fromevent_data, then the current implementation may need to be revised.indexer/services/ender/src/scripts/dydx_perpetual_position_and_order_side_matching.sql (1)
- 1-11: The function
dydx_perpetual_position_and_order_side_matchingis implemented correctly and follows the guidelines mentioned in the comment block. The logic is sound, and the function is appropriately marked as IMMUTABLE and PARALLEL SAFE.indexer/services/ender/src/scripts/dydx_protocol_condition_type_to_order_type.sql (1)
- 1-10: The removal of the comment block before the function declaration aligns with the goal of ensuring accurate exception line numbers in stack traces.
indexer/services/ender/src/scripts/dydx_stateful_order_handler.sql (2)
1-5: The comment block added provides clear documentation for the
dydx_stateful_order_handlerfunction, explaining the parameters and expected return value. This is a good practice for maintainability and clarity.12-12: The note added to ensure that no text exists before the function declaration is a good practice for maintaining the accuracy of exception line numbers, which is crucial for debugging.
indexer/services/ender/src/scripts/dydx_subaccount_update_handler.sql (1)
- 14-22: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [1-22]
The newly added function
dydx_subaccount_update_handleris well-documented and follows SQL best practices. The note about keeping the function declaration at the start of the file to ensure accurate exception line numbers is in line with the pull request summary and is a good practice for maintainability.indexer/services/ender/src/scripts/dydx_tendermint_event_to_transaction_index.sql (2)
1-13: The summary states that the function directly returns the transaction index without any conditional checks or additional logic, but the code includes conditional checks for 'transactionIndex' and 'blockEvent'. Please update the summary to accurately reflect the function's behavior.
12-13: The function correctly checks for the presence of 'transactionIndex' and returns it if available. This aligns with the function's intended purpose.
indexer/services/ender/src/scripts/dydx_update_clob_pair_handler.sql (4)
1-13: The function documentation is clear and provides a good explanation of the parameters and return values. The note about not having text before the function declaration is important for maintaining accurate exception line numbers.
12-13: Using
perpetual_markets%ROWTYPEis a good practice as it ensures that the record structure matches the table structure, reducing the risk of runtime errors due to schema changes.1-13: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [11-30]
The transaction logic is correct, and the use of
IF NOT FOUND THENto raise an exception if no rows are updated is a good practice for error handling.
- 1-13: > Note: This review was outside the patches, and no patch overlapping with it was found. Original lines [30-30]
The return statement correctly builds a JSON object from the updated record, which aligns with the function's documentation.
indexer/services/ender/src/scripts/dydx_update_perpetual_handler.sql (2)
1-13: The function
dydx_update_perpetual_handleris well-documented and follows the requirements specified in the summary. The comment block provides clear information about the parameters and expected return value, and the note about maintaining correct exception line numbers is present.1-13: > Note: This review was outside the patches, and no patch overlapping with it was found. Original lines [14-30]
The logic within the
BEGIN...ENDblock of thedydx_update_perpetual_handlerfunction appears to be correct. It properly updates theperpetual_marketstable based on theevent_dataand handles the case where no matching record is found. The function also correctly returns the updated market as a JSON object.indexer/services/ender/src/scripts/dydx_update_perpetual_position_aggregate_fields.sql (1)
- 1-10: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [1-21]
The function
dydx_update_perpetual_position_aggregate_fieldsis well-documented, and the parameters are consistent with the summary. The note to ensure no text exists before the function declaration is present, which is important for maintaining accurate exception line numbers.indexer/services/ender/src/scripts/dydx_uuid_from_fill_event_parts.sql (1)
- 1-9: The function
dydx_uuid_from_fill_event_partsis correctly defined and includes a comment to ensure proper line numbering for exceptions. However, ensure that thedydx_uuidfunction it calls is defined elsewhere in the codebase and is also IMMUTABLE and PARALLEL SAFE to maintain the properties of this function.indexer/services/ender/src/scripts/dydx_uuid_from_funding_index_update_parts.sql (1)
- 1-9: The function
dydx_uuid_from_funding_index_update_partsis implemented correctly according to the provided summary and pull request details. It is important to ensure that thedydx_uuidfunction it relies on is defined elsewhere in the codebase and that it behaves as expected when concatenating the input parameters to form a UUID.indexer/services/ender/src/scripts/dydx_uuid_from_oracle_price_parts.sql (1)
- 1-9: The new function
dydx_uuid_from_oracle_price_partsis added correctly and follows the best practices mentioned in the comment about ensuring accurate exception line numbers. However, ensure that thedydx_uuidfunction called within is defined and accessible in the codebase.indexer/services/ender/src/scripts/dydx_uuid_from_order_id_parts.sql (1)
- 1-9: The function
dydx_uuid_from_order_id_partsis well-defined and follows the guidelines for accurate exception line numbers. It is also marked as IMMUTABLE and PARALLEL SAFE, which is appropriate given its deterministic nature and the ability to run in parallel with other transactions. Ensure that thedydx_uuidfunction, which is called within this function, has proper error handling and is also IMMUTABLE and PARALLEL SAFE to maintain consistency.indexer/services/ender/src/scripts/dydx_uuid_from_perpetual_position_parts.sql (1)
- 1-9: The function
dydx_uuid_from_perpetual_position_partsis correctly defined to generate a UUID from the parts of a perpetual position. The comment regarding the placement of text to ensure correct exception line numbers is noted and adhered to. Ensure that thedydx_uuidfunction, which is called within this function, has proper error handling and is defined with the same immutability and parallel safety guarantees.indexer/services/ender/src/scripts/dydx_uuid_from_subaccount_id.sql (2)
1-9: The function
dydx_uuid_from_subaccount_idassumes that thesubaccount_idJSONB will always contain the keys 'owner' and 'number'. Ensure that the input JSONB is always well-formed with these keys present. If there's a possibility of these keys being absent or malformed, consider adding error handling to manage such cases.1-9: The function is declared as IMMUTABLE and PARALLEL SAFE. Verify that the underlying function
dydx_uuid_from_subaccount_id_partsalso adheres to these properties to ensure the correctness of these declarations.indexer/services/ender/src/scripts/dydx_uuid_from_subaccount_id_parts.sql (2)
1-9: The function
dydx_uuid_from_subaccount_id_partscorrectly implements the functionality as described. However, it is not clear from the provided hunk whether thedydx_uuidfunction is defined elsewhere in the codebase. Please ensure thatdydx_uuidis defined and accessible within the context wheredydx_uuid_from_subaccount_id_partsis used.1-9: The function is marked as IMMUTABLE and PARALLEL SAFE, which is appropriate for a UUID generation function assuming it has no side effects and always returns the same result for the same input. Ensure that these properties hold true for the
dydx_uuidfunction as well.indexer/services/ender/src/scripts/dydx_uuid_from_transaction_parts.sql (1)
- 1-9: The function
dydx_uuid_from_transaction_partsis implemented correctly and follows the guidelines to ensure accurate exception line numbers. It is also marked as IMMUTABLE and PARALLEL SAFE, which is appropriate for its deterministic behavior.
| to the 'OrderType' enum in postgres. | ||
|
|
||
| (Note that no text should exist before the function declaration to ensure that exception line numbers are correct.) | ||
| */ | ||
| BEGIN | ||
| CASE condition_type | ||
| WHEN '-1'::jsonb THEN RETURN 'LIMIT'; /** UNRECOGNIZED */ |
There was a problem hiding this comment.
The case for '-1'::jsonb is marked as /** UNRECOGNIZED */ but returns 'LIMIT', which seems incorrect. It should either return a value indicating an unrecognized condition or raise an exception.
- WHEN '-1'::jsonb THEN RETURN 'LIMIT'; /** UNRECOGNIZED */
+ WHEN '-1'::jsonb THEN RAISE EXCEPTION 'Unrecognized ConditionType: %', condition_type; /** UNRECOGNIZED */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.
| WHEN '-1'::jsonb THEN RETURN 'LIMIT'; /** UNRECOGNIZED */ | |
| WHEN '-1'::jsonb THEN RAISE EXCEPTION 'Unrecognized ConditionType: %', condition_type; /** UNRECOGNIZED */ |
| CREATE OR REPLACE FUNCTION dydx_uuid_from_transfer_parts(event_id bytea, asset_id text, sender_subaccount_id uuid, recipient_subaccount_id uuid, sender_wallet_address text, recipient_wallet_address text) RETURNS uuid AS $$ | ||
| /** | ||
| Returns a UUID using the parts of a transfer. | ||
|
|
||
| (Note that no text should exist before the function declaration to ensure that exception line numbers are correct.) | ||
| */ | ||
| CREATE OR REPLACE FUNCTION dydx_uuid_from_transfer_parts(event_id bytea, asset_id text, sender_subaccount_id uuid, recipient_subaccount_id uuid, sender_wallet_address text, recipient_wallet_address text) RETURNS uuid AS $$ | ||
| DECLARE | ||
| sender_subaccount_id_or_undefined text; | ||
| recipient_subaccount_id_or_undefined text; |
There was a problem hiding this comment.
The variables sender_wallet_address_or_undefined and recipient_wallet_address_or_undefined should be declared within the DECLARE block for clarity and maintainability. If they are declared outside of the hunk, please move them inside the DECLARE block.
| BEGIN | ||
| return dydx_uuid_from_order_id_parts( | ||
| dydx_uuid_from_subaccount_id(order_id->'subaccountId'), |
There was a problem hiding this comment.
The function body is missing the END keyword to properly close the BEGIN block. This will result in a syntax error when the function is executed. The END keyword should be added to ensure the function is correctly defined.
order_id->>'clientId',
order_id->>'clobPairId',
order_id->>'orderFlags');
+ END;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.
| BEGIN | |
| return dydx_uuid_from_order_id_parts( | |
| dydx_uuid_from_subaccount_id(order_id->'subaccountId'), | |
| BEGIN | |
| return dydx_uuid_from_order_id_parts( | |
| dydx_uuid_from_subaccount_id(order_id->'subaccountId'), | |
| order_id->>'clientId', | |
| order_id->>'clobPairId', | |
| order_id->>'orderFlags'); | |
| END; |
| CREATE OR REPLACE FUNCTION dydx_asset_create_handler(event_data jsonb) RETURNS jsonb AS $$ | ||
| /** | ||
| Parameters: | ||
| - event_data: The 'data' field of the IndexerTendermintEvent (https://github.com/dydxprotocol/v4-proto/blob/8d35c86/dydxprotocol/indexer/indexer_manager/event.proto#L25) | ||
| converted to JSON format. Conversion to JSON is expected to be done by JSON.stringify. | ||
| Returns: JSON object containing fields: | ||
| - asset: The created asset in asset-model format (https://github.com/dydxprotocol/indexer/blob/cc70982/packages/postgres/src/models/asset-model.ts). | ||
|
|
||
| (Note that no text should exist before the function declaration to ensure that exception line numbers are correct.) | ||
| */ | ||
| CREATE OR REPLACE FUNCTION dydx_asset_create_handler(event_data jsonb) RETURNS jsonb AS $$ | ||
| DECLARE | ||
| market_record_id integer; | ||
| asset_record assets%ROWTYPE; |
There was a problem hiding this comment.
The function dydx_asset_create_handler is well-documented and handles the creation of an asset with appropriate error handling for missing market records. However, consider adding error handling for the INSERT INTO assets statement to catch any potential exceptions that may occur during the insertion process.
The function does not perform any validation on the event_data parameter. It is assumed that the input is well-formed and matches the expected schema. If this function is exposed to external calls or if there's any chance that event_data might not be well-structured, it would be prudent to add validation logic to ensure robustness and prevent potential runtime errors.
| Returns: JSON object containing fields: | ||
| - market: The updated market in market-model format (https://github.com/dydxprotocol/v4-chain/blob/9ed26bd/indexer/packages/postgres/src/models/market-model.ts). | ||
| - oracle_price: The created oracle price in oracle-price-model format (https://github.com/dydxprotocol/v4-chain/blob/9ed26bd/indexer/packages/postgres/src/models/oracle-price-model.ts). | ||
|
|
||
| (Note that no text should exist before the function declaration to ensure that exception line numbers are correct.) | ||
| */ | ||
| CREATE OR REPLACE FUNCTION dydx_market_price_update_handler(block_height int, block_time timestamp, event_data jsonb) RETURNS jsonb AS $$ | ||
| DECLARE | ||
| market_record_id integer; | ||
| market_record markets%ROWTYPE; |
There was a problem hiding this comment.
Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [14-37]
Consider wrapping the database operations in a transaction block to ensure atomicity. If any operation fails, the transaction should be rolled back to maintain data integrity.
BEGIN;
-- Database operations here
COMMIT;| CREATE OR REPLACE FUNCTION dydx_market_create_handler(event_data jsonb) RETURNS jsonb AS $$ | ||
| /** | ||
| Parameters: | ||
| - event_data: The 'data' field of the IndexerTendermintEvent (https://github.com/dydxprotocol/v4-chain/blob/9ed26bd/proto/dydxprotocol/indexer/indexer_manager/event.proto#L25) | ||
| converted to JSON format. Conversion to JSON is expected to be done by JSON.stringify. | ||
| Returns: JSON object containing fields: | ||
| - market: The created market in market-model format (https://github.com/dydxprotocol/v4-chain/blob/9ed26bd/indexer/packages/postgres/src/models/market-model.ts). | ||
|
|
||
| (Note that no text should exist before the function declaration to ensure that exception line numbers are correct.) | ||
| */ | ||
| CREATE OR REPLACE FUNCTION dydx_market_create_handler(event_data jsonb) RETURNS jsonb AS $$ | ||
| DECLARE | ||
| market_record_id integer; | ||
| market_record markets%ROWTYPE; |
There was a problem hiding this comment.
The function does not contain explicit transaction control statements such as BEGIN, COMMIT, or ROLLBACK. If transaction control is not handled outside of this function, it would be beneficial to include it to ensure data integrity.
| CREATE OR REPLACE FUNCTION dydx_perpetual_market_handler(event_data jsonb) RETURNS jsonb AS $$ | ||
| /** | ||
| Parameters: | ||
| - event_data: The 'data' field of the IndexerTendermintEvent (https://github.com/dydxprotocol/v4-chain/blob/9ed26bd/proto/dydxprotocol/indexer/indexer_manager/event.proto#L25) | ||
| converted to JSON format. Conversion to JSON is expected to be done by JSON.stringify. | ||
| Returns: JSON object containing fields: | ||
| - perpetual_market: The updated perpetual market in perpetual-market-model format (https://github.com/dydxprotocol/v4-chain/blob/9ed26bd/indexer/packages/postgres/src/models/perpetual-market-model.ts). | ||
|
|
||
| (Note that no text should exist before the function declaration to ensure that exception line numbers are correct.) | ||
| */ | ||
| CREATE OR REPLACE FUNCTION dydx_perpetual_market_handler(event_data jsonb) RETURNS jsonb AS $$ | ||
| DECLARE | ||
| perpetual_market_record perpetual_markets%ROWTYPE; | ||
| BEGIN |
There was a problem hiding this comment.
Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [13-26]
Consider adding validation for event_data to ensure that all required fields are present and in the correct format before attempting to assign them to perpetual_market_record. This would prevent runtime errors and ensure data integrity.
Note: This review was outside the patches, and no patch overlapping with it was found. Original lines [27-27]
Ensure that there is proper error handling around the INSERT INTO perpetual_markets statement. In case of an insert failure, the function should handle the exception appropriately to avoid unhandled errors.
| */ | ||
| CREATE OR REPLACE FUNCTION dydx_order_fill_handler_per_order( | ||
| field text, block_height int, block_time timestamp, event_data jsonb, event_index int, transaction_index int, | ||
| transaction_hash text, fill_liquidity text, fill_type text, usdc_asset_id text, order_canceled_status text) RETURNS jsonb AS $$ | ||
| DECLARE |
There was a problem hiding this comment.
The function body is quite complex and handles multiple operations. Consider refactoring the code into smaller, more focused functions to improve maintainability and readability.
Consider defining constants for repeated string literals such as JSON keys and database column names to reduce the risk of typos and improve maintainability.
The TODO comment about extracting calculations into separate SQL functions should be tracked in a project management tool rather than in the code comments to ensure it is addressed in a timely manner.
| DECLARE | ||
| QUOTE_CURRENCY_ATOMIC_RESOLUTION constant numeric = -6; | ||
|
|
There was a problem hiding this comment.
The constant QUOTE_CURRENCY_ATOMIC_RESOLUTION is declared but not used within the provided code snippet. If it's not used elsewhere in the function, consider removing it to avoid confusion.
| DECLARE | ||
| PPM_EXPONENT constant numeric = -6; | ||
| FUNDING_RATE_FROM_PROTOCOL_IN_HOURS constant numeric = 8; |
There was a problem hiding this comment.
The constants TYPE_PREMIUM_SAMPLE and TYPE_FUNDING_RATE_AND_INDEX are defined as jsonb with numeric values, which might be a mistake. These should likely be integers or strings that represent the type of the event.
- TYPE_PREMIUM_SAMPLE constant jsonb = '1';
- TYPE_FUNDING_RATE_AND_INDEX constant jsonb = '2';
+ TYPE_PREMIUM_SAMPLE constant text = '1';
+ TYPE_FUNDING_RATE_AND_INDEX constant text = '2';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.
| DECLARE | |
| PPM_EXPONENT constant numeric = -6; | |
| FUNDING_RATE_FROM_PROTOCOL_IN_HOURS constant numeric = 8; | |
| DECLARE | |
| PPM_EXPONENT constant numeric = -6; | |
| FUNDING_RATE_FROM_PROTOCOL_IN_HOURS constant numeric = 8; | |
| TYPE_PREMIUM_SAMPLE constant text = '1'; | |
| TYPE_FUNDING_RATE_AND_INDEX constant text = '2'; |
The constants TYPE_PREMIUM_SAMPLE and TYPE_FUNDING_RATE_AND_INDEX are defined as jsonb but are used in a comparison with event_data->'type', which suggests they should be of a comparable type, likely text or int.
Appending NULL to the errors_response array after each iteration of the loop may lead to an array with many NULL values if no errors occur. Consider appending only when an error is present.
- errors_response = array_append(errors_response, NULL);
+ IF current_error IS NOT NULL THEN
+ errors_response = array_append(errors_response, current_error);
+ END IF;Where current_error is a variable that holds the error message if any error occurs during the loop iteration.
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.
| DECLARE | |
| PPM_EXPONENT constant numeric = -6; | |
| FUNDING_RATE_FROM_PROTOCOL_IN_HOURS constant numeric = 8; | |
| DECLARE | |
| PPM_EXPONENT constant numeric = -6; | |
| FUNDING_RATE_FROM_PROTOCOL_IN_HOURS constant numeric = 8; |
Changelist
[IND-399] Fix line numbers in SQL function stack traces
This makes so that the PG_EXCEPTION_CONTEXT contains the correct line numbers giving a stack like: PL/pgSQL function dydx_market_modify_handler(jsonb) line 17 at RAISE PL/pgSQL function dydx_block_processor(jsonb) line 85 at assignment
Test Plan
No functional changes.
Author/Reviewer Checklist
state-breakinglabel.PrepareProposalorProcessProposal, manually add the labelproposal-breaking.feature:[feature-name].backport/[branch-name].refactor,chore,bug.