fix transfers parent SA API for external addresses in transfers #2630
Conversation
Signed-off-by: Shrenuj Bansal <[email protected]>
WalkthroughThe pull request introduces new constants and modifications to the transfer functionality within the codebase. Specifically, it adds constants for creating transfer objects with alternate addresses and updates the filtering logic for transfers in the Changes
Possibly related PRs
Suggested labels
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: 0
🧹 Outside diff range and nitpick comments (2)
indexer/services/comlink/__tests__/controllers/api/v4/transfers-controller.test.ts (1)
462-464: Simplify the creation of a single subaccountUsing
Promise.allfor a single promise is unnecessary. Consider removingPromise.allto streamline the code.Apply this change:
- await Promise.all([ - SubaccountTable.create(defaultSubaccount2Num0), - ]); + await SubaccountTable.create(defaultSubaccount2Num0);indexer/services/comlink/src/request-helpers/request-transformer.ts (1)
273-273: Remove unnecessary inline commentThe comment
// if (recipientParentSubaccountNum === parentSubaccountNumber)is redundant and may cause confusion. Consider removing it for clarity.Apply this change:
- } else { // if (recipientParentSubaccountNum === parentSubaccountNumber) { + } else {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
indexer/packages/postgres/__tests__/helpers/constants.ts(2 hunks)indexer/services/comlink/__tests__/controllers/api/v4/transfers-controller.test.ts(4 hunks)indexer/services/comlink/src/controllers/api/v4/transfers-controller.ts(1 hunks)indexer/services/comlink/src/request-helpers/request-transformer.ts(3 hunks)
🔇 Additional comments (11)
indexer/services/comlink/__tests__/controllers/api/v4/transfers-controller.test.ts (4)
23-25: Updated imports are appropriate
The new constants defaultSubaccount2Num0 and defaultTendermintEventId4 are correctly imported to support the additional test cases.
470-470: Additional transfer included correctly for testing
The defaultTransferWithAlternateAddress is appropriately added to the test setup to validate transfers involving alternate addresses.
556-572: Expected response for alternate address transfer is correctly defined
The expectedTransferWithAlternateAddressResponse object accurately represents the expected transfer data for testing purposes.
588-590: New transfer assertion added to the test verification
Including expectedTransferWithAlternateAddressResponse in the assertions ensures that the test properly verifies transfers with alternate addresses.
indexer/services/comlink/src/controllers/api/v4/transfers-controller.ts (2)
221-221: Function call updated with additional parameters
Passing address and parentSubaccountNumber to transferToParentSubaccountResponseObject aligns with the updated function signature and logic.
229-230: Filter condition enhanced to exclude internal transfers
The updated filter correctly excludes transfers where both the sender and recipient have the same address and parent subaccount number, preventing internal transfers from being included.
indexer/services/comlink/src/request-helpers/request-transformer.ts (3)
246-246: Function signature updated appropriately
Adding address and parentSubaccountNumber as parameters to transferToParentSubaccountResponseObject accommodates the need for comparing addresses and parent subaccount numbers.
255-257: Correctly assign senderAddress with fallback
The assignment of senderAddress now properly defaults to the subaccount's address when senderWalletAddress is not provided.
267-267: Improved transfer type determination logic
The condition accurately identifies transfer types by comparing both senderAddress and senderParentSubaccountNum with the provided address and parentSubaccountNumber.
indexer/packages/postgres/__tests__/helpers/constants.ts (2)
648-653: New transfer constant defined correctly
The defaultTransferWithAlternateAddress constant is properly defined to test transfers involving alternate addresses.
663-671: Transfer ID generated correctly for alternate address transfer
The defaultTransferWithAlternateAddressId is correctly computed using the appropriate properties, ensuring the unique identification of the transfer.
Signed-off-by: Shrenuj Bansal <[email protected]>
|
@Mergifyio backport release/indexer/v7.x |
✅ Backports have been created
|
Signed-off-by: Shrenuj Bansal <[email protected]> (cherry picked from commit fc5ff36)
…port #2630) (#2631) Co-authored-by: shrenujb <[email protected]>
Changelist
The transfers parent subaccount API was filtering out transfers from different addresses if the parent subaccount number was the same. This fixes it
Test Plan
Added 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
Bug Fixes
Improvements