chore(runway): cherry-pick fix: add nativeAsset key to staked ETH asset multichain #12930
+143
−68
Conversation
This file contains 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
![runway-github[bot]](https://avatars.githubusercontent.com/in/86225?s=60&v=4){kind=small}
…et multichain (#12851) ## **Description** 1. What is the reason for the change? There was a crash when clicking on the Staked Ethereum asset due to it missing a key `nativeAsset` which is required in some component. This was a regression from code refactoring. Separately, there was a currency switching bug that was introduced with the use of a different fiat conversion method. I have switched to a method that does not throw error for currency switching and this has been tested with Vince Separately, there were some issues with the Token list asset balances being slightly off. This was noticed when the balance we use for the Asset Detail staked Ethereum asset showed up differently than in the Token List. Some of this has been addressed, but there are still issues that I've noticed where we lose precision when getting the fiat total of a token asset or native asset. I left a comment in the code about this, and maybe it could be revisited it later. 2. What is the improvement/solution? We added the `nativeAsset` key back to the staked ethereum asset We added the weiToFiat method to replace the Intl.NumberFormat method We removed precision loss from multichain.ts asset fiatBalance calculations, however there is another place(s) where fiat balances are recalculated again i.e. [deriveBalanceFromAssetMarketDetails.ts](https://github.com/MetaMask/metamask-mobile/pull/12851/files#diff-283c109c8093bbb5d8e06be79bcbeac928b8353633e55dc7944594a0e6888b49) ## **Related issues** Fixes: #12680 Fixes: #12856 Closes: https://consensyssoftware.atlassian.net/browse/STAKE-912 ## **Manual testing steps** - Import the staking test wallet in MM - Click on the staked ethereum asset - The link should not crash the application more info here on testing this #12680 - Currency switching should not crash the application more info here on testing this #12856 ## **Screenshots/Recordings** ### **Before** ### **After** https://github.com/user-attachments/assets/c2525b70-8d04-4606-861b-3206ebd557f4 https://github.com/user-attachments/assets/a8c4010e-3589-4171-803a-4d07a962e6a6 ## **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 - [ ] 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.
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
nickewansmith
added
team-stake
team-assets
Run Smoke E2E
amitabh94
approved these changes
Jan 10, 2025
|
|
gambinish
approved these changes
Jan 10, 2025
sahar-fehri
added
Run Smoke E2E
nickewansmith
added
Run Smoke E2E
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
There was a crash when clicking on the Staked Ethereum asset due to it
missing a key
nativeAssetwhich is required in some component. Thiswas a regression from code refactoring.
Separately, there was a currency switching bug that was introduced with
the use of a different fiat conversion method. I have switched to a
method that does not throw error for currency switching and this has
been tested with Vince
Separately, there were some issues with the Token list asset balances
being slightly off. This was noticed when the balance we use for the
Asset Detail staked Ethereum asset showed up differently than in the
Token List. Some of this has been addressed, but there are still issues
that I've noticed where we lose precision when getting the fiat total of
a token asset or native asset. I left a comment in the code about this,
and maybe it could be revisited it later.
We added the
nativeAssetkey back to the staked ethereum assetWe added the weiToFiat method to replace the Intl.NumberFormat method
We removed precision loss from multichain.ts asset fiatBalance
calculations, however there is another place(s) where fiat balances are
recalculated again i.e.
deriveBalanceFromAssetMarketDetails.ts
Related issues
Fixes: #12680
Fixes: #12856
Closes: https://consensyssoftware.atlassian.net/browse/STAKE-912
Manual testing steps
this [Bug]: The app unexpectedly closes when users are navigating between tokens on various networks, Error: Missing chain id #12680
testing this RangeError: j6.l: Malformed currency code ! #12856
Screenshots/Recordings
Before
After
Simulator.Screen.Recording.-.iPhone.SE.3rd.generation.-.2025-01-09.at.08.42.08.mp4
Simulator.Screen.Recording.-.iPhone.SE.3rd.generation.-.2025-01-09.at.08.47.45.mp4
Pre-merge author checklist
Docs and MetaMask Mobile
Coding
Standards.
if applicable
guidelines).
Not required for external contributors.
Pre-merge reviewer checklist
app, test code being changed).
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots. 40a999c