Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Multichain balances #13742

Open
wants to merge 26 commits into
base: main
Choose a base branch
from
Open

Fix: Multichain balances #13742

wants to merge 26 commits into from

Conversation

owencraston
Copy link
Contributor

@owencraston owencraston commented Feb 27, 2025

Description

This PR fixes the broken balances for Solana on mobile. There were a series of updates that broke the balances. This PR addresses this issue with the following..

  • updating the Solana snap to the latest.
  • updating the assets controller (which contains all of the multichain controllers) to the latest which has no breaking changes but some bug fixes
  • integrating the CronJobController which the snaps rely on to update the balances
  • Integrating the MultichainAssetRatesController which the MultichainBalancesController/MultichainAssetsController now relies on

This PR also migrates all of the Multichain controllers to use the latest "init" pattern. These changes were modelled off of the accounts controller.

NOTE: Bitcoin Balances are still broken and likely requires a the snap to be updated. BTC is not a priority for now.

Related issues

Fixes:

Manual testing steps

  1. Import a wallet that has funds with Solana (there are SRPs in 1Password that contain SOL)
  2. click on the account list at the top of the wallet
  3. click on add new account
  4. click on Add Solana account
  5. your account should be created and after a few seconds your wallet should be rendered in fiat.

Screenshots/Recordings

Before

Screen.Recording.2025-02-28.at.9.36.58.PM.mov

After

Screen.Recording.2025-02-28.at.9.07.51.PM.mov

Pre-merge author checklist

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.

Copy link
Contributor

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.

Copy link

socket-security bot commented Feb 27, 2025

Updated dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/@metamask/[email protected]24.1.0 None 0 307 kB metamaskbot
npm/@metamask/[email protected]51.0.2 None +5 1.14 MB
npm/@metamask/[email protected]17.2.1 None 0 553 kB metamaskbot
npm/@metamask/[email protected]1.9.0 None 0 781 kB danfinlay, gudahtt, kumavis, ...6 more

View full report↗︎

Have feedback? Participate in our User Experience Survey 📊

@owencraston owencraston force-pushed the fix/multichain-balances branch 2 times, most recently from 896d23e to 290f36d Compare February 28, 2025 04:36
@@ -454,6 +461,27 @@ export class Engine {
multichainRatesControllerMessenger,
multichainRatesController,
);

const cronjobControllerMessenger = this.controllerMessenger.getRestricted({
Copy link
Contributor

@tommasini tommasini Feb 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now we have a folders to have the messengers on, we can start to move this there, this is a new pattern introduced by this PR (We can address it later so we unblock stuff)

Solana = `${SolScope.Mainnet}/token:EPjFWdd5AufqSSqeM2qN1xzybapC8G4wEGGkZwyTDt1v`,
SolanaDevnet = `${SolScope.Devnet}/token:EPjFWdd5AufqSSqeM2qN1xzybapC8G4wEGGkZwyTDt1v`,
SolanaTestnet = `${SolScope.Testnet}/token:EPjFWdd5AufqSSqeM2qN1xzybapC8G4wEGGkZwyTDt1v`,
Solana = `${SolScope.Mainnet}/slip44:501`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch!

@tommasini
Copy link
Contributor

Where is this event "ExecutionService:unhandledError" coming from? Weird error!

@owencraston owencraston force-pushed the fix/multichain-balances branch from 290f36d to 58a55e1 Compare February 28, 2025 19:22
@owencraston owencraston force-pushed the fix/multichain-balances branch from 28d1be3 to 6067fc0 Compare March 1, 2025 02:56
@owencraston owencraston marked this pull request as ready for review March 1, 2025 04:17
@owencraston owencraston requested review from a team as code owners March 1, 2025 04:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Needs dev review
Development

Successfully merging this pull request may close these issues.

2 participants