feat: (TokenBalancesController) add batching around update balance requests#8246
Open
Prithpal-Sooriya wants to merge 6 commits intomainfrom
Open
feat: (TokenBalancesController) add batching around update balance requests#8246Prithpal-Sooriya wants to merge 6 commits intomainfrom
Prithpal-Sooriya wants to merge 6 commits intomainfrom
Conversation
We have many things (events, interactions) invoking balance update requests. This adds some batching and aggregate logic to minimise the number of requests to process.
…g-around-update-balance-requests
Contributor
Author
There was a problem hiding this comment.
Similar to react-testing-library, a waitFor test util to allow us to wait for assertions to complete.
This is much cleaner compared to the "promise flushing" and random timeouts added in the TokenBalancesController.test.ts test spec.
Contributor
Author
There was a problem hiding this comment.
Generic batch handler creator.
Potential to reuse this on other controller methods that get called frequently and cannot use a debounce.
Contributor
Author
There was a problem hiding this comment.
Huuuuuuuge test spec update.
- Added tests around new changes
- Updated test specs to not use fake timers + promise flushing, most tests now use real timers and "waitFor", only some specific tests use fake timers (such as interval polling)
…calls Added functionality to the TokenBalancesController to batch rapid updateBalances requests, coalescing multiple calls into a single processed request. This enhancement reduces redundant balance fetches and improves performance. Updated changelog to reflect this new feature.
Contributor
Author
|
@metamaskbot publish-preview |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
Refined the batching logic in TokenBalancesController to handle empty buffers more effectively. The updated implementation now initializes the merge with the first buffer element when available, ensuring that the batching process remains robust and efficient. Additionally, enhanced error handling in the waitFor utility to provide more informative error messages upon timeout, improving debugging capabilities.
…ing to return Promises Modified the batching logic in TokenBalancesController to ensure that the #batchedUpdateBalances method returns a Promise, allowing for better error handling and synchronization. Updated the createBatchedHandler utility to return Promises for capture calls, enabling callers to await the completion of batch flushes and handle errors appropriately. Enhanced tests to verify the new asynchronous behavior and error propagation.
…cription logic and enhance error handling Reintroduced the subscription logic in the TokenBalancesController to ensure proper updates during balance changes. Additionally, improved the createBatchedHandler utility to handle errors more effectively by ensuring that all promises are settled correctly, enhancing the robustness of the batching mechanism. Updated tests to validate the new error handling behavior and ensure all scenarios are covered.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation
We have many things (events, interactions) invoking balance update requests. Since the balance update requests are pretty complicated, this potentially also opens up race conditions and causes flickering in the UI (see this conversation)
This adds some batching and aggregate logic to minimise the number of requests to process.
References
Checklist
Note
Medium Risk
Introduces debounced batching for
updateBalances, changing timing/ordering of balance refreshes and option-merging semantics, which could affect UI freshness and downstream callers that rely on immediate updates.Overview
TokenBalancesControllernow coalesces rapidupdateBalancescalls via a new debounced batching layer (UPDATE_BALANCES_BATCH_MS) so multiple requests within a short window flush as a single balance fetch.Adds
UpdateBalancesOptionsplusmergeUpdateBalancesOptionsto merge queued calls (unionchainIds/tokenAddresses, ORqueryAllAccounts), and routes polling through the non-batched execution path (#executeUpdateBalances) while publicupdateBalancesuses the batcher.Introduces a reusable
createBatchedHandlerutility (with unit tests) and updates controller tests to account for async batching using a newwaitForhelper; changelog updated to document the batching behavior.Written by Cursor Bugbot for commit 2cb9ef9. This will update automatically on new commits. Configure here.