[TRA-109] Filter out transfers between child subaccounts of same parent. #1544
Conversation
WalkthroughThe recent updates focus on improving the handling of subaccount transfers within the indexer service. These changes include introducing new type declarations for transfer messages and enhancing test coverage for different transfer scenarios. A significant aspect is the refinement of handling transfers between child subaccounts for parent subaccount channels, ensuring accurate tracking of transactions. These modifications enhance the clarity and reliability of the transfer logic in the 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 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 as PR comments)
Additionally, you can add CodeRabbit Configration 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: 1
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- indexer/services/socks/tests/constants.ts (2 hunks)
- indexer/services/socks/tests/helpers/from-kafka-helpers.test.ts (3 hunks)
- indexer/services/socks/src/helpers/from-kafka-helpers.ts (3 hunks)
Additional Context Used
Path-based Instructions (3)
indexer/services/socks/__tests__/constants.ts (1)
Pattern
**/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.indexer/services/socks/src/helpers/from-kafka-helpers.ts (1)
Pattern
**/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.indexer/services/socks/__tests__/helpers/from-kafka-helpers.test.ts (1)
Pattern
**/**: Do not include a walkthrough. For all comments on line changes that are longer than 15 lines, add a prefix to the comment of what lines you are commenting on (for example: "The changes from lines 100 to 200...". Add your comment to be on the last 15 lines of the chunk rather than adding your comment to the entire chunk.
Additional comments not posted (3)
indexer/services/socks/__tests__/constants.ts (2)
9-10: The addition ofTransferSubaccountMessageContentsandTransferTypealigns with the PR's objectives to handle subaccount transfers more effectively.
108-123: ThedefaultTransferContentsconstant is well-defined and crucial for testing the new functionality of filtering subaccount transfers. Ensure its usage is consistent across all relevant tests.Verification successful
The
defaultTransferContentsconstant is well-defined and crucial for testing the new functionality of filtering subaccount transfers. It is used consistently in thefrom-kafka-helpers.test.tsfile, ensuring its relevance in the test scenarios.
- Locations:
indexer/services/socks/__tests__/helpers/from-kafka-helpers.test.tsScripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `defaultTransferContents` in tests. # Test: Search for the constant usage in test files. Expect: Usage in relevant test scenarios. rg --type typescript $'defaultTransferContents' -- 'indexer/services/socks/__tests__/*.ts'Length of output: 124
Script:
#!/bin/bash # Description: Verify the usage of `defaultTransferContents` in tests. # Test: Search for the constant usage in test files. Expect: Usage in relevant test scenarios. rg 'defaultTransferContents' --glob 'indexer/services/socks/__tests__/*.ts'Length of output: 197
Script:
#!/bin/bash # Description: Verify the usage of `defaultTransferContents` in all TypeScript files within the test directory. # Test: Search for the constant usage in all TypeScript files. Expect: Usage in relevant test scenarios. rg 'defaultTransferContents' --glob 'indexer/services/socks/**/*.ts'Length of output: 709
indexer/services/socks/__tests__/helpers/from-kafka-helpers.test.ts (1)
159-192: The new test effectively verifies the filtering of transfers between child subaccounts for the parent subaccount channel. Consider adding more edge cases to ensure robustness, such as scenarios with malformed transfer data.Verification successful
The
getMessageToForwardfunction decodes the Kafka message and processes it based on the channel. However, the snippet provided does not show the handling for theV4_PARENT_ACCOUNTSchannel. To ensure the filtering logic is correctly implemented, we need to see the complete implementation of thegetMessageToForwardfunction, especially the part handling theV4_PARENT_ACCOUNTSchannel.
The
getMessageToForwardfunction includes handling for theV4_PARENT_ACCOUNTSchannel, where it decodes theSubaccountMessageand uses thegetParentSubaccountContentsfunction to filter the contents. This aligns with the test case that verifies the filtering of transfers between child subaccounts.The test case and the implementation appear to be correctly aligned, ensuring that transfers between child subaccounts are filtered out for the parent subaccount channel.
- : The test case effectively verifies the filtering of transfers between child subaccounts for the parent subaccount channel. The implementation of
getMessageToForwardcorrectly handles this scenario.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the effectiveness of the new test. # Test: Run the test suite and check for failures in this specific test. Expect: All tests pass. # Command to run the test suite (assuming a typical Node.js environment): npm test -- indexer/services/socks/__tests__/helpers/from-kafka-helpers.test.tsLength of output: 152
Script:
#!/bin/bash # Description: Verify the effectiveness of the new test. # Check if yarn is available if command -v yarn &> /dev/null then # Run the test suite using yarn yarn test indexer/services/socks/__tests__/helpers/from-kafka-helpers.test.ts else echo "Neither npm nor yarn is available to run the tests." fiLength of output: 130
Script:
#!/bin/bash # Description: Locate and review the implementation of `getMessageToForward` and related helper functions. # Find the implementation of `getMessageToForward` ast-grep --lang typescript --pattern 'function getMessageToForward($_, $_) { $$$ }' # Find the implementation of `createKafkaMessage` ast-grep --lang typescript --pattern 'function createKafkaMessage($_) { $$$ }'Length of output: 166
Script:
#!/bin/bash # Description: Locate the implementation of `getMessageToForward` and `createKafkaMessage` using `rg`. # Search for the implementation of `getMessageToForward` rg 'function getMessageToForward' # Search for the implementation of `createKafkaMessage` rg 'function createKafkaMessage'Length of output: 1993
Script:
#!/bin/bash # Description: Extract and display the code for `getMessageToForward` and `createKafkaMessage`. # Display the implementation of `getMessageToForward` rg -A 20 'export function getMessageToForward' indexer/services/socks/src/helpers/from-kafka-helpers.ts # Display the implementation of `createKafkaMessage` rg -A 20 'export function createKafkaMessage' indexer/services/socks/__tests__/helpers/kafka.tsLength of output: 1152
Script:
#!/bin/bash # Description: Extract and display the complete implementation of `getMessageToForward`. # Display the complete implementation of `getMessageToForward` rg -A 50 'export function getMessageToForward' indexer/services/socks/src/helpers/from-kafka-helpers.tsLength of output: 1945
Script:
#!/bin/bash # Description: Extract and display the complete implementation of `getMessageToForward` with an increased line limit. # Display the complete implementation of `getMessageToForward` rg -A 100 'export function getMessageToForward' indexer/services/socks/src/helpers/from-kafka-helpers.tsLength of output: 3890
|
|
||
| function getParentSubaccountContents(msg: SubaccountMessage): SubaccountMessageContents { | ||
| // Filter out transfers between child subaccounts of the same parent subaccount. | ||
| const contents: SubaccountMessageContents = JSON.parse(msg.contents) as SubaccountMessageContents; |
There was a problem hiding this comment.
Sorta unrelated to this change but why does SubaccountMessageContents have multiples of positions, orders, fills but only a singular transfer?
There was a problem hiding this comment.
That's tech debt that was never fixed.
indexer/services/socks/__tests__/helpers/from-kafka-helpers.test.ts
Outdated
Show resolved
Hide resolved
…st.ts Co-authored-by: shrenujb <98204323+shrenujb@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- indexer/services/socks/tests/helpers/from-kafka-helpers.test.ts (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- indexer/services/socks/tests/helpers/from-kafka-helpers.test.ts
|
@Mergifyio backport release/indexer/v5.x |
✅ Backports have been created
|
Changelist
Update
socksto filter out transfers between child subaccounts for the same parent subaccount for the parent subaccount websocket channel.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
New Features
Tests