[IND-498]: Add TradingRewardsHandler #902
Conversation
WalkthroughThe recent updates involve the implementation of a trading rewards feature within a blockchain indexer system. The changes include database schema updates to handle new precision and scale for trading reward amounts, the addition of new constants and types to handle trading rewards data, and the creation of new SQL scripts and functions for processing trading rewards events. Moreover, a new Changes
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 X ? 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: 1
Configuration used: CodeRabbit UI
Files selected for processing (19)
- indexer/packages/postgres/tests/helpers/constants.ts (1 hunks)
- indexer/packages/postgres/src/constants.ts (2 hunks)
- indexer/packages/postgres/src/db/migrations/migration_files/20231221153944_update_trading_rewards_amount_scale.ts (1 hunks)
- indexer/packages/postgres/src/db/migrations/migration_files/20231221165820_update_wallet_total_trading_rewards_scale.ts (1 hunks)
- indexer/packages/postgres/src/index.ts (1 hunks)
- indexer/packages/postgres/src/stores/trading-reward-table.ts (1 hunks)
- indexer/packages/postgres/src/types/websocket-message-types.ts (2 hunks)
- indexer/services/ender/tests/handlers/trading-rewards-handler.test.ts (1 hunks)
- indexer/services/ender/tests/helpers/constants.ts (2 hunks)
- indexer/services/ender/src/handlers/trading-rewards-handler.ts (1 hunks)
- indexer/services/ender/src/helpers/postgres/postgres-functions.ts (3 hunks)
- indexer/services/ender/src/lib/block-processor.ts (3 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_unordered_handlers.sql (1 hunks)
- indexer/services/ender/src/scripts/handlers/dydx_trading_rewards_handler.sql (1 hunks)
- indexer/services/ender/src/scripts/helpers/dydx_process_trading_reward_event.sql (1 hunks)
- indexer/services/ender/src/scripts/helpers/dydx_uuid_from_trading_rewards_parts.sql (1 hunks)
- indexer/services/ender/src/validators/trading-rewards-validator.ts (1 hunks)
Files skipped from review due to trivial changes (1)
- indexer/packages/postgres/src/index.ts
Additional comments: 26
indexer/services/ender/src/scripts/helpers/dydx_uuid_from_trading_rewards_parts.sql (1)
- 1-10: The function
dydx_uuid_from_trading_rewards_partscorrectly concatenates the address and block height to generate a UUID. However, it's important to ensure that thedydx_uuidfunction it calls is designed to handle this specific concatenation format and that it generates a UUID that is unique across different addresses and block heights.indexer/packages/postgres/src/db/migrations/migration_files/20231221153944_update_trading_rewards_amount_scale.ts (1)
- 7-7: The migration alters the 'amount' column to have a precision and scale of 18, which means it can store values up to 18 digits long, all of which can be decimal. This is unusual as typically the scale (number of digits to the right of the decimal point) is less than the precision (total number of significant digits). Please verify that this is the intended behavior, as it would mean the 'amount' can only store values less than 1.
indexer/packages/postgres/src/db/migrations/migration_files/20231221165820_update_wallet_total_trading_rewards_scale.ts (1)
- 7-7: Similar to the previous migration file, this migration also sets both precision and scale to 18 for the 'totalTradingRewards' column. This would allow only for values less than 1 to be stored, which might not be the intended behavior. Please confirm that this precision and scale setting is correct.
indexer/services/ender/src/validators/trading-rewards-validator.ts (1)
- 20-26: The validation logic checks if
denomsis zero, which is good for avoiding processing rewards with no value. However, it's important to ensure thatbytesToBigIntcorrectly interprets the byte array fromreward.denomAmount. If the byte array is not in the expected format, this could lead to incorrect validation.indexer/services/ender/src/handlers/trading-rewards-handler.ts (2)
23-33: The
internalHandlefunction maps theresultRow.trading_rewardstoTradingRewardFromDatabaseobjects usingTradingRewardModel.fromJson. It's important to ensure thatfromJsonis robust and can handle any discrepancies in thetrading_rewardsdata format to prevent runtime errors.45-65: The
generateKafkaEventsfunction creates a Kafka event for each trading reward. It's good to see that the event includes thecreatedAtHeightandcreatedAtfields, which are important for event ordering and traceability. Ensure that thegenerateConsolidatedSubaccountKafkaEventfunction properly handles the serialization ofsubaccountMessageContents.indexer/services/ender/src/scripts/helpers/dydx_process_trading_reward_event.sql (1)
- 22-44: The function
dydx_process_trading_reward_eventperforms several operations, including updating wallets and inserting into thetrading_rewardstable. It's important to ensure that thedydx_trim_scaleanddydx_from_serializable_intfunctions are well-tested and handle edge cases, as they are critical to calculating the correctamount_in_human_readable.indexer/packages/postgres/src/stores/trading-reward-table.ts (1)
- 98-107: The
findByIdfunction has been updated to use anidinstead of anaddress. This change aligns with the new UUID generation strategy and should be reflected in all places wherefindByIdis used. Ensure that all references to this function have been updated accordingly.indexer/services/ender/src/scripts/handlers/dydx_trading_rewards_handler.sql (1)
- 1-67: The
dydx_trading_rewards_handlerfunction processes trading reward events and returns a JSON object with the processed rewards. It's important to ensure that thedydx_process_trading_reward_eventfunction is called with the correct parameters and that the array indexing is handled correctly, considering PostgreSQL arrays are 1-indexed.indexer/services/ender/src/helpers/postgres/postgres-functions.ts (3)
42-45: The addition of
dydx_trading_rewards_handler.sqlto theHANDLER_SCRIPTSarray is correct. Ensure that the script is placed in the correct directory and that the path resolution innewScriptfunction correctly locates the script file.71-74: Similarly, the addition of
dydx_process_trading_reward_event.sqlto theHELPER_SCRIPTSarray is correct. Verify that the script is properly managed in the source directory and that it's accessible during runtime.86-89: The inclusion of
dydx_uuid_from_trading_rewards_parts.sqlin theHELPER_SCRIPTSarray is appropriate. Confirm that the function is correctly implemented and available for use by other scripts or functions that require UUID generation for trading rewards.indexer/services/ender/src/scripts/handlers/dydx_block_processor_unordered_handlers.sql (1)
- 63-64: The addition of the
dydx_trading_rewards_handlercall in theCASEstatement is correct. Ensure that thedydx_trading_rewards_handlerfunction is thoroughly tested to handle all possible event data structures it may receive.indexer/packages/postgres/src/constants.ts (1)
- 103-103: The inclusion of
TradingRewardModelin theSQL_TO_JSON_DEFINED_MODELSarray is correct. This ensures that the model is recognized for SQL to JSON conversion processes. Confirm that theTradingRewardModelhas the necessary SQL to JSON conversion logic implemented.indexer/services/ender/src/lib/types.ts (2)
53-53: The addition of
TRADING_REWARDto theDydxIndexerSubtypesenum is correct. This will allow the system to recognize and handle trading reward events appropriately.139-143: The extension of the
EventProtoWithTypeAndVersionunion type to includeTradingRewardsEventV1is correct. This ensures that the event handling system can process trading reward events with the new structure.indexer/packages/postgres/src/types/websocket-message-types.ts (2)
39-39: The addition of the
tradingRewardfield to theSubaccountMessageContentsinterface is correct. This will allow the system to include trading reward information in the subaccount messages sent over websockets.167-171: The new interface
TradingRewardSubaccountMessageContentsis well-defined with appropriate fields for the trading reward, creation height, and creation time. Ensure that all systems consuming this message type are updated to handle the new structure.indexer/services/ender/__tests__/handlers/trading-rewards-handler.test.ts (1)
- 1-238: The unit tests for the
TradingRewardsHandlerare well-structured and cover the creation and update of trading rewards and wallets. The use ofBigfor precision,toISOfor date formatting, andbigIntToBytesfor handling binary data is appropriate. The asynchronous calls are correctly awaited, and the assertions usingtoEqualare suitable for the test cases.indexer/services/ender/src/lib/helper.ts (2)
26-26: The import of
TradingRewardsEventV1is correctly added to support the new trading rewards functionality.211-219: The addition of the
DydxIndexerSubtypes.TRADING_REWARDcase in theindexerTendermintEventToEventProtoWithTypefunction is correctly implemented to handle the new event type.indexer/services/ender/src/lib/block-processor.ts (2)
25-25: The import of
TradingRewardsValidatoris correctly added to validate the new trading rewards events.50-50: The update to
TXN_EVENT_SUBTYPE_VERSION_TO_VALIDATOR_MAPPINGto includeTradingRewardsValidatorforDydxIndexerSubtypes.TRADING_REWARDis correctly implemented.indexer/services/ender/__tests__/helpers/constants.ts (2)
31-31: The import of
TradingRewardsEventV1is correctly added to support the new trading rewards functionality in tests.392-399: The
defaultTradingRewardsEventconstant is correctly defined to provide a default value for theTradingRewardsEventV1type in tests.indexer/packages/postgres/__tests__/helpers/constants.ts (1)
- 106-109: The addition of
defaultWallet2with atotalTradingRewardsvalue of '1' is consistent with the existing pattern of using strings for numeric fields in financial applications. Ensure that the new constant is integrated properly wherever it's needed in the codebase.
| if (reward.owner === undefined) { | ||
| return this.logAndThrowParseMessageError( | ||
| `TradingReward in TradingRewardEvent at index ${index} is missing an owner.`, | ||
| { event: this.event, address: reward.owner }, | ||
| ); | ||
| } |
There was a problem hiding this comment.
The check for reward.owner being undefined is crucial for data integrity. However, the error message uses reward.owner in the logging, which will not be informative if it is indeed undefined. Consider logging additional context, such as the index or other identifiable information from the reward object.
- { event: this.event, address: reward.owner },
+ { event: this.event, index: index },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.
| if (reward.owner === undefined) { | |
| return this.logAndThrowParseMessageError( | |
| `TradingReward in TradingRewardEvent at index ${index} is missing an owner.`, | |
| { event: this.event, address: reward.owner }, | |
| ); | |
| } | |
| if (reward.owner === undefined) { | |
| return this.logAndThrowParseMessageError( | |
| `TradingReward in TradingRewardEvent at index ${index} is missing an owner.`, | |
| { event: this.event, index: index }, | |
| ); | |
| } |
|
|
||
| (Note that no text should exist before the function declaration to ensure that exception line numbers are correct.) | ||
| DECLARE | ||
| trading_rewards_array trading_rewards%ROWTYPE; |
There was a problem hiding this comment.
oops my bad, this was when I was testing
indexer/services/ender/src/validators/trading-rewards-validator.ts
Outdated
Show resolved
Hide resolved
indexer/services/ender/src/scripts/helpers/dydx_process_trading_reward_event.sql
Outdated
Show resolved
Hide resolved
indexer/services/ender/__tests__/handlers/trading-rewards-handler.test.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (13)
- indexer/packages/postgres/tests/helpers/constants.ts (4 hunks)
- indexer/packages/postgres/tests/helpers/conversion-helpers.ts (1 hunks)
- indexer/packages/postgres/tests/stores/trading-reward-aggregation-table.test.ts (2 hunks)
- indexer/packages/postgres/src/db/migrations/migration_files/20231221153944_update_trading_rewards_amount_scale.ts (1 hunks)
- indexer/packages/postgres/src/db/migrations/migration_files/20231221165820_update_wallet_total_trading_rewards_scale.ts (1 hunks)
- indexer/packages/postgres/src/db/migrations/migration_files/20231221233356_update_trading_reward_aggregations_amount_scale.ts (1 hunks)
- indexer/packages/postgres/src/index.ts (2 hunks)
- indexer/services/ender/tests/handlers/trading-rewards-handler.test.ts (1 hunks)
- indexer/services/ender/tests/handlers/transfer-handler.test.ts (4 hunks)
- indexer/services/ender/tests/helpers/conversion-helpers.ts (1 hunks)
- indexer/services/ender/src/scripts/handlers/dydx_trading_rewards_handler.sql (1 hunks)
- indexer/services/ender/src/scripts/helpers/dydx_process_trading_reward_event.sql (1 hunks)
- indexer/services/ender/src/validators/trading-rewards-validator.ts (1 hunks)
Files skipped from review as they are similar to previous changes (6)
- indexer/packages/postgres/src/db/migrations/migration_files/20231221153944_update_trading_rewards_amount_scale.ts
- indexer/packages/postgres/src/index.ts
- indexer/services/ender/tests/handlers/trading-rewards-handler.test.ts
- indexer/services/ender/src/scripts/handlers/dydx_trading_rewards_handler.sql
- indexer/services/ender/src/scripts/helpers/dydx_process_trading_reward_event.sql
- indexer/services/ender/src/validators/trading-rewards-validator.ts
Additional comments: 14
indexer/services/ender/__tests__/helpers/conversion-helpers.ts (1)
- 3-5: The function
intToSerializedIntcorrectly converts a number to aUint8Array. Ensure that the input number can safely be represented as aBigInt.indexer/packages/postgres/__tests__/helpers/conversion-helpers.ts (1)
- 6-8: The function
denomToHumanReadableConversioncorrectly usesBigfor conversion and formatting. Verify that thedenominput is always a number that can be represented accurately in JavaScript.indexer/packages/postgres/src/db/migrations/migration_files/20231221165820_update_wallet_total_trading_rewards_scale.ts (2)
3-10: The
upmigration correctly alters thetotalTradingRewardscolumn with a precision of 27 and a scale of 18. Ensure this precision and scale are consistent with the application's requirements for representing trading rewards.12-17: The
downmigration reverts thetotalTradingRewardscolumn without specifying precision and scale. Confirm if this is the intended behavior or if the previous precision and scale should be restored.indexer/packages/postgres/src/db/migrations/migration_files/20231221233356_update_trading_reward_aggregations_amount_scale.ts (2)
3-10: The
upmigration correctly alters theamountcolumn with a precision of 27 and a scale of 18. Ensure this precision and scale are consistent with the application's requirements for representing trading reward amounts.12-17: The
downmigration reverts theamountcolumn without specifying precision and scale. Confirm if this is the intended behavior or if the previous precision and scale should be restored.indexer/packages/postgres/__tests__/stores/trading-reward-aggregation-table.test.ts (2)
12-12: The import of
denomToHumanReadableConversionis correctly added to the test file.86-86: The use of
denomToHumanReadableConversion(100000)replaces a hardcoded string, which is a good practice for maintainability and accuracy in tests.indexer/packages/postgres/__tests__/helpers/constants.ts (2)
104-104: The change to
defaultWallet'stotalTradingRewardsfield to usedenomToHumanReadableConversion(0)ensures consistency and accuracy in representing trading rewards.107-109: The addition of
defaultWallet2withtotalTradingRewardsinitialized usingdenomToHumanReadableConversion(1)is consistent and allows for varied test scenarios.indexer/services/ender/__tests__/handlers/transfer-handler.test.ts (4)
28-28: The import of
testConversionHelpersis correctly added to the test file.308-308: Replacing hardcoded '0' with
testConversionHelpers.denomToHumanReadableConversion(0)for thetotalTradingRewardsfield in the test case is a good practice for maintainability and accuracy.360-360: Using
testConversionHelpers.denomToHumanReadableConversion(0)for initializingtotalTradingRewardsin the test case ensures consistency and accuracy.413-413: The replacement of the hardcoded '0' with
testConversionHelpers.denomToHumanReadableConversion(0)for thetotalTradingRewardsfield in the test case is appropriate and improves test reliability.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (3)
- indexer/packages/postgres/src/db/migrations/migration_files/20231221153944_update_trading_rewards_amount_scale.ts (1 hunks)
- indexer/packages/postgres/src/db/migrations/migration_files/20231221165820_update_wallet_total_trading_rewards_scale.ts (1 hunks)
- indexer/packages/postgres/src/db/migrations/migration_files/20231221233356_update_trading_reward_aggregations_amount_scale.ts (1 hunks)
Files skipped from review due to trivial changes (1)
- indexer/packages/postgres/src/db/migrations/migration_files/20231221233356_update_trading_reward_aggregations_amount_scale.ts
Files skipped from review as they are similar to previous changes (2)
- indexer/packages/postgres/src/db/migrations/migration_files/20231221153944_update_trading_rewards_amount_scale.ts
- indexer/packages/postgres/src/db/migrations/migration_files/20231221165820_update_wallet_total_trading_rewards_scale.ts
Changelist
Add support for Trading Rewards Handler
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.