From 387b9e81cd49129b42651564e10515d36830eafd Mon Sep 17 00:00:00 2001 From: Nick Gambino <35090461+gambinish@users.noreply.github.com> Date: Fri, 6 Sep 2024 14:56:09 -0700 Subject: [PATCH] fix: Intermittent Display Issue of Fiat Currency on Main Wallet View (#11087) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** This bug is difficult to reproduce organically, but we suspect that the root cause is that `fetchExchangeRate` function to crypto-compare could sometimes be down, causing the function to throw an error prematurely. We were able to simulate this locally, and it caused the zero balance to occur. In `CurrencyController` we `updateExchangeRate` in a finally block (so, even when there is an error in the fetchExchangeRates call). In this case, the Controller will returns nullish values, causing the balance to zero out. This patch only updates state in the `try` block, after a successful response. We are maintaining existing error handling in the `catch` statement, and keeping the `releaseLock` in the finally to handle concurrent requests gracefully. ## **Related issues** Fixes: https://github.com/MetaMask/metamask-mobile/issues/11035 ## **Manual testing steps** 1. Run app normally, to get initial balance 2. Throw error in `fetchExchangeRate` function 3. Balance should not zero out like it did before this patch. ## **Screenshots/Recordings** ### **Before** https://github.com/user-attachments/assets/ac0fa1c2-9f4d-4e42-85f4-9b5edb8fc753 ### **After** https://github.com/user-attachments/assets/62c3fc40-9dfc-4573-97e5-6c2a1fabcec8 ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile 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** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] 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. --- .../@metamask+assets-controllers+30.0.0.patch | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/patches/@metamask+assets-controllers+30.0.0.patch b/patches/@metamask+assets-controllers+30.0.0.patch index bc3dcfd0a44..31f21c3eaed 100644 --- a/patches/@metamask+assets-controllers+30.0.0.patch +++ b/patches/@metamask+assets-controllers+30.0.0.patch @@ -23,6 +23,59 @@ index 0dc70ec..461a210 100644 } }; return acc; +diff --git a/node_modules/@metamask/assets-controllers/dist/chunk-ELSMS5S7.js b/node_modules/@metamask/assets-controllers/dist/chunk-ELSMS5S7.js +index 45254ad..83634ab 100644 +--- a/node_modules/@metamask/assets-controllers/dist/chunk-ELSMS5S7.js ++++ b/node_modules/@metamask/assets-controllers/dist/chunk-ELSMS5S7.js +@@ -87,6 +87,7 @@ var CurrencyRateController = class extends _pollingcontroller.StaticIntervalPoll + const nativeCurrencyForExchangeRate = Object.values( + _controllerutils.TESTNET_TICKER_SYMBOLS + ).includes(nativeCurrency) ? _controllerutils.FALL_BACK_VS_CURRENCY : nativeCurrency; ++ let shouldUpdateState = true; + try { + if (currentCurrency && nativeCurrency && // if either currency is an empty string we can skip the comparison + // because it will result in an error from the api and ultimately +@@ -103,23 +104,27 @@ var CurrencyRateController = class extends _pollingcontroller.StaticIntervalPoll + } + } catch (error) { + if (!(error instanceof Error && error.message.includes("market does not exist for this coin pair"))) { ++ // Don't update state on transient / unexpected errors ++ shouldUpdateState = false; + throw error; + } + } finally { + try { +- this.update(() => { +- return { +- currencyRates: { +- ...currencyRates, +- [nativeCurrency]: { +- conversionDate, +- conversionRate, +- usdConversionRate +- } +- }, +- currentCurrency +- }; +- }); ++ if (shouldUpdateState) { ++ this.update(() => { ++ return { ++ currencyRates: { ++ ...currencyRates, ++ [nativeCurrency]: { ++ conversionDate, ++ conversionRate, ++ usdConversionRate ++ } ++ }, ++ currentCurrency ++ }; ++ }); ++ } + } finally { + releaseLock(); + } diff --git a/node_modules/@metamask/assets-controllers/dist/chunk-FMZML3V5.js b/node_modules/@metamask/assets-controllers/dist/chunk-FMZML3V5.js index ee6155c..06a3a04 100644 --- a/node_modules/@metamask/assets-controllers/dist/chunk-FMZML3V5.js