Skip to content

Commit

Permalink
fix: App slower when changing account and switching network (#9721)
Browse files Browse the repository at this point in the history
## **Description**
This PR reduces the number of api calls that happen when switching
account and network, on cold and warm app starts.

* Transactions are being fetched for all the accounts instead of just
the selected one, that was fixed by adding a patch to the
TransactionController to only fetch per account selected (36 fetches to
eth_getTransactionReceipt and counted 40+ eth_getBlockByHash, to 0 since
when we are switching network on an account that doesn't have
transactions)
Screenshot of main branch with eth_getTransactionReceipt requests:
The selected account was not orangefox.eth

https://etherscan.io/tx/0x7ce730e2e87520e179d4e351e5cfe80565c346f9ab7688a3391306422e01f0a9
<img
src="https://github.com/MetaMask/metamask-mobile/assets/46944231/f4e2df99-20a2-4156-b16a-7fa26ed6af98"
width=650 />

* Fixed a bug on Transaction controller where the getBlockByHash request
was failing silently because it was missing one param.

* ~~ENS fetches now only once we switch network and we change the
selected account (This was looping by number of accounts and being
retriggered, one 1 second, counted 15+ fetches to [ens
contract](https://etherscan.io/address/0xa2c122be93b0074270ebee7f6b7292c7deb45047)
, now it's just fetching once it switches network and for the selected
account)~~
* Request parallelization of token balances. (This was not optimized on
number of requests but on the parallelization of the requests to have a
response quicker from all of them)
* Update pooling of token balances from 10000 to 180000 miliseconds
* Added a boolean to not let the updateBalances runs if there is an
update in progress. (Probably change this to use mutex when implementing
on core it's a good idea)

A snapshot was updated on this PR! It seems that this snapshot was
indeed wrong, and now it have the right mocked data.

Check this transaction controller core branch with the changes:
mobile-patch-performance-tx-controller-13-0-0
(Once this is approved, this changes need to be merged on the
patch/mobile-transaction-controller-13-0-0 branch)

Assets controller patch easy to ready, the core branch is this one:
patch/mobile-assets-controllers-26-performance.
(Once this is approved, this changes need to be merged on the
patch/mobile-assets-controllers-26 branch)


**Next steps**

* Postpone transactions requests to views that will show transactions
(exg: Asset View, Activity View)



**Product Changes**
* ~~ENS is now get by the selected account and not every account~~ This
was addressed in other PR already
* Transactions status is now updated only on the selected account (This
still decrease the number of requests if the user has multiple accounts
with multiple transactions)

## **Related issues**

Fixes: #9250 #9249 

## **Manual testing steps**

1. 
2.
3.

## **Screenshots/Recordings**


### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [x] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [x] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [x] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.
  • Loading branch information
tommasini committed May 29, 2024
1 parent d0b7cd9 commit 0cb5f6d
Show file tree
Hide file tree
Showing 4 changed files with 116 additions and 40 deletions.
2 changes: 1 addition & 1 deletion app/core/Engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1209,7 +1209,7 @@ class Engine {
getERC20BalanceOf: assetsContractController.getERC20BalanceOf.bind(
assetsContractController,
),
interval: 10000,
interval: 180000,
}),
new TokenRatesController({
onTokensStateChange: (listener) => tokensController.subscribe(listener),
Expand Down
66 changes: 58 additions & 8 deletions patches/@metamask+assets-controllers+26.0.0.patch
Original file line number Diff line number Diff line change
Expand Up @@ -1038,9 +1038,18 @@ index 796f2be..86d635f 100644
//# sourceMappingURL=TokenBalancesController.d.ts.map
\ No newline at end of file
diff --git a/node_modules/@metamask/assets-controllers/dist/TokenBalancesController.js b/node_modules/@metamask/assets-controllers/dist/TokenBalancesController.js
index 7d09db2..fa7866f 100644
index 7d09db2..92d57ce 100644
--- a/node_modules/@metamask/assets-controllers/dist/TokenBalancesController.js
+++ b/node_modules/@metamask/assets-controllers/dist/TokenBalancesController.js
@@ -19,7 +19,7 @@ var __classPrivateFieldGet = (this && this.__classPrivateFieldGet) || function (
if (typeof state === "function" ? receiver !== state || !f : !state.has(receiver)) throw new TypeError("Cannot read private member from an object whose class did not declare it");
return kind === "m" ? f : kind === "a" ? f.call(receiver) : f ? f.value : state.get(receiver);
};
-var _TokenBalancesController_handle, _TokenBalancesController_getERC20BalanceOf, _TokenBalancesController_interval, _TokenBalancesController_tokens, _TokenBalancesController_disabled;
+var _TokenBalancesController_handle, _TokenBalancesController_getERC20BalanceOf, _TokenBalancesController_interval, _TokenBalancesController_tokens, _TokenBalancesController_disabled, _TokenBalancesController_updateInProgress;
Object.defineProperty(exports, "__esModule", { value: true });
exports.TokenBalancesController = exports.getDefaultTokenBalancesState = void 0;
const base_controller_1 = require("@metamask/base-controller");
@@ -52,11 +52,12 @@ class TokenBalancesController extends base_controller_1.BaseController {
* @param options.interval - Polling interval used to fetch new token balances.
* @param options.tokens - List of tokens to track balances for.
Expand All @@ -1055,7 +1064,11 @@ index 7d09db2..fa7866f 100644
super({
name: controllerName,
metadata,
@@ -71,10 +72,10 @@ class TokenBalancesController extends base_controller_1.BaseController {
@@ -68,13 +69,14 @@ class TokenBalancesController extends base_controller_1.BaseController {
_TokenBalancesController_interval.set(this, void 0);
_TokenBalancesController_tokens.set(this, void 0);
_TokenBalancesController_disabled.set(this, void 0);
+ _TokenBalancesController_updateInProgress.set(this, false);
__classPrivateFieldSet(this, _TokenBalancesController_disabled, disabled, "f");
__classPrivateFieldSet(this, _TokenBalancesController_interval, interval, "f");
__classPrivateFieldSet(this, _TokenBalancesController_tokens, tokens, "f");
Expand All @@ -1069,22 +1082,59 @@ index 7d09db2..fa7866f 100644
__classPrivateFieldSet(this, _TokenBalancesController_getERC20BalanceOf, getERC20BalanceOf, "f");
this.poll();
}
@@ -135,6 +136,15 @@ class TokenBalancesController extends base_controller_1.BaseController {
@@ -114,29 +116,42 @@ class TokenBalancesController extends base_controller_1.BaseController {
*/
updateBalances() {
return __awaiter(this, void 0, void 0, function* () {
- if (__classPrivateFieldGet(this, _TokenBalancesController_disabled, "f")) {
+ if (__classPrivateFieldGet(this, _TokenBalancesController_disabled, "f") || __classPrivateFieldGet(this, _TokenBalancesController_updateInProgress, "f")) {
return;
}
+ __classPrivateFieldSet(this, _TokenBalancesController_updateInProgress, true, "f");
const newContractBalances = {};
- for (const token of __classPrivateFieldGet(this, _TokenBalancesController_tokens, "f")) {
+ const balancePromises = __classPrivateFieldGet(this, _TokenBalancesController_tokens, "f").map((token) => {
const { address } = token;
const { selectedAddress } = this.messagingSystem.call('PreferencesController:getState');
- try {
- newContractBalances[address] = (0, controller_utils_1.toHex)(yield __classPrivateFieldGet(this, _TokenBalancesController_getERC20BalanceOf, "f").call(this, address, selectedAddress));
+ return __classPrivateFieldGet(this, _TokenBalancesController_getERC20BalanceOf, "f").call(this, address, selectedAddress)
+ .then((balance) => {
+ newContractBalances[address] = (0, controller_utils_1.toHex)(balance);
token.balanceError = null;
- }
- catch (error) {
+ })
+ .catch((error) => {
newContractBalances[address] = (0, controller_utils_1.toHex)(0);
token.balanceError = error;
- }
- }
+ });
+ });
+ yield Promise.all(balancePromises);
this.update((state) => {
state.contractBalances = newContractBalances;
});
});
}
+ __classPrivateFieldSet(this, _TokenBalancesController_updateInProgress, false, "f");
+ });
+ }
+ /**
+ * THIS FUNCTIONS IS CURRENTLY PATCHED AND STILL NEEDS TO BE IMPLEMENTED ON THE CORE REPO
+ * Resets to the default state
+ */
+ reset() {
+ this.update((state) => {
+ state.contractBalances = {};
+ });
+ }
});
}
}
exports.TokenBalancesController = TokenBalancesController;
_TokenBalancesController_handle = new WeakMap(), _TokenBalancesController_getERC20BalanceOf = new WeakMap(), _TokenBalancesController_interval = new WeakMap(), _TokenBalancesController_tokens = new WeakMap(), _TokenBalancesController_disabled = new WeakMap();
-_TokenBalancesController_handle = new WeakMap(), _TokenBalancesController_getERC20BalanceOf = new WeakMap(), _TokenBalancesController_interval = new WeakMap(), _TokenBalancesController_tokens = new WeakMap(), _TokenBalancesController_disabled = new WeakMap();
+_TokenBalancesController_handle = new WeakMap(), _TokenBalancesController_getERC20BalanceOf = new WeakMap(), _TokenBalancesController_interval = new WeakMap(), _TokenBalancesController_tokens = new WeakMap(), _TokenBalancesController_disabled = new WeakMap(), _TokenBalancesController_updateInProgress = new WeakMap();
exports.default = TokenBalancesController;
//# sourceMappingURL=TokenBalancesController.js.map
\ No newline at end of file
diff --git a/node_modules/@metamask/assets-controllers/dist/TokenDetectionController.d.ts b/node_modules/@metamask/assets-controllers/dist/TokenDetectionController.d.ts
index 528f665..87859b7 100644
--- a/node_modules/@metamask/assets-controllers/dist/TokenDetectionController.d.ts
Expand Down
Loading

0 comments on commit 0cb5f6d

Please sign in to comment.