Repository navigation
feat(tron): stop syncing and emitting assets while the assets migration is active - #424
Open
ulissesferreira wants to merge 1 commit into
Open
ulissesferreira wants to merge 1 commit into
ulissesferreira wants to merge 1 commit into
Conversation
ulissesferreira
force-pushed
the
WPN-2054-skip-asset-sync-when-migration-active
branch
8 times, most recently
from
October 9, 2026 12:41
cbfbe94 to
08f7b77
Compare
ulissesferreira
marked this pull request as ready for review
October 9, 2026 13:29
Battambang
reviewed
Oct 9, 2026
Comment on lines
+16
to
+17
| - Stop fetching, persisting, and publishing assets from the snap while the assets migration feature flag is active: the periodic sync cronjob and the synchronization triggered by selected account changes no longer touch assets or emit `AccountAssetListUpdated` and `AccountBalancesUpdated` events, leaving asset syncing to the `AssetsController` ([#424](https://github.com/MetaMask/internal-snaps/pull/424)) | ||
| - Persist the live assets and balances fetched by `getAccountAssets` and `getAccountBalances` to local state without emitting `AccountAssetListUpdated` and `AccountBalancesUpdated` events ([#424](https://github.com/MetaMask/internal-snaps/pull/424)) |
Contributor
There was a problem hiding this comment.
should we regroup this one into one entry?
Battambang
reviewed
Oct 9, 2026
Comment on lines
+521
to
+527
| if (await this.#assetsService.isAssetsMigrationEnabled()) { | ||
| /** | ||
| * No-op when AssetsController is already handling assets | ||
| */ | ||
| return; | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
We are fine here that nothing is done at the moment?
Contributor
Author
There was a problem hiding this comment.
Yes indeed! If the AssetsMigration is on we are not supposed to do anything
ulissesferreira
force-pushed
the
WPN-2054-skip-asset-sync-when-migration-active
branch
from
October 9, 2026 17:00
08f7b77 to
7bbd68d
Compare
|
This branch was successfully deployed
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
2 participants
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
While the assets migration feature flag is off, the snap owns asset synchronization: it fetches assets and balances from the chain, persists them to snap state, and publishes updates to the extension by emitting
AccountAssetListUpdatedandAccountBalancesUpdated.Once the migration is active, the
AssetsControllerowns asset syncing, so the snap must stay out of the way. Until now, the snap's asset sync kept running while the flag was on: every sync trigger (the 60s cronjob, the background event scheduled bysetSelectedAccounts, and post-transaction refreshes) still fetched assets and emitted keyring events for snap-owned assets.This PR changes the last behaviours missing for the
AssetsControllermigration to happen:AccountsService.synchronizeAssetsnow consultsAssetsService.isAssetsMigrationEnabled(true only when the migration stage isOff) and returns early, so the snap neither fetches nor emits assets. Transaction synchronization is unaffected, and the cronjob itself keeps running so it still handles transaction syncing and resumes asset syncing as soon as the flag is turned off.getAccountAssetsandgetAccountBalances, they are persisted to the snap's local state without emittingAccountAssetListUpdatedandAccountBalancesUpdatedevents. This keeps snap state warm for theAssetsControllerto take over, while Core owns publishing updates once the migration is active.References
Ticket: WPN-2054
Checklist