fix: add assets-controller temp state migration healing#9393
Open
Prithpal-Sooriya wants to merge 5 commits into
Open
fix: add assets-controller temp state migration healing#9393Prithpal-Sooriya wants to merge 5 commits into
Prithpal-Sooriya wants to merge 5 commits into
Conversation
- Updated the `tempHealAssetsInfoMetadata` function to return the updated controller state directly instead of a callback for the `update` method. - Introduced a new `selectedCurrency` field in `AssetsControllerStateInternal` to track the currently active ISO 4217 currency code. - Improved test coverage for `tempHealAssetsInfoMetadata`, including scenarios for healing state and error handling. This change aims to streamline the healing process for asset metadata and ensure better state management during migrations.
…ate healing - Added a test to verify that the `tempHealAssetsInfoMetadata` function does not mutate existing `customAssets` arrays in the input state. - Refactored the `tempHealAssetsInfoMetadata` function to use `cloneDeep` for state management, ensuring that the original state remains unchanged during the healing process. This change enhances the reliability of state migrations by preventing unintended side effects on the input state.
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
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
Background: After a prior defect in AssetsController, metadata (
AssetsController.assetsInfo) for custom tokens were wiped. Most popular chains support auto-detection and can self-heal, however not all chains support auto-detection.Fix: This migration restores metadata for custom tokens on niche EVM chains that are unable to auto-detect/self-heal.
References
Checklist
Note
Medium Risk
Mutates persisted wallet asset state at construction from untrusted legacy data; behavior is heavily guarded and idempotent, but incorrect host wiring could still affect token display and custom-asset lists.
Overview
Adds a temporary constructor hook
tempMigrateAssetsInfoMetadataAssets3346so hosts can supply legacyTokensController/AccountsControllerpersisted state. On startup,AssetsControllerrunstempHealAssetsInfoMetadatato restore wipedassetsInfoERC-20 metadata andcustomAssetstracking for niche EVM chains that do not self-heal via the Accounts API.Healing reads
allTokens, respectsallIgnoredTokensand current hiddenassetPreferences, skips Accounts-API-supported chains and ERC-721s, never overwrites existing metadata, and treats legacy input as untrusted with defensive validation. Failures are reported through optionalcaptureExceptionwithout blocking controller construction.Also documents the fix in the package changelog and aligns
AssetsControllerStateInternalintypes.tswithselectedCurrency(typed asSupportedCurrency).Reviewed by Cursor Bugbot for commit 5d54234. Bugbot is set up for automated code reviews on this repo. Configure here.