-
-
Notifications
You must be signed in to change notification settings - Fork 266
chore: Simplify TokenListController initialization #7740
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
base: main
Are you sure you want to change the base?
Conversation
8bed328 to
ca3acb1
Compare
ca3acb1 to
07a04f4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
| * data loaded from storage would be immediately written back. | ||
| * Chains are removed from this set after being skipped once. | ||
| */ | ||
| readonly #chainsLoadedFromStorage: Set<Hex> = new Set(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for removing this! That was a workaround to avoid doing redundant writes 🙏
There was some complexity in handling the order of operations between controller initialization and the event to persist state changes. To simplify this, the controller has been updated such that it no longer persists state changes until _after_ initialization. This makes the logic easier to follow, and lets us delete an instance variable and a few blocks of code. The controller will be initialized as part of wallet initialization, so it will not be "constructed but uninitialized" for any significant length of time.
ad6a0f8 to
11d2a4f
Compare
Explanation
There was some complexity in handling the order of operations between controller initialization and the event to persist state changes.
To simplify this, the controller has been updated such that it no longer persists state changes until after initialization. This makes the logic easier to follow, and lets us delete an instance variable and a few blocks of code.
The controller will be initialized as part of wallet initialization, so it will not be "constructed but uninitialized" for any significant length of time.
References
Related to changes made in this PR: #7413
Checklist
Note
Streamlines persistence and storage sync for
TokenListController.initialize(); noStorageServicewrites occur before init, while pre-init state updates are persisted after init#chainsLoadedFromStorageand related skip logic;#onCacheChangednow consistently marks changed chains for debounced save#synchronizeCacheWithStorage: loads per-chain cache, merges without overwriting fresher in-memory data, and schedules persistence for chains already in stateinitialize()where required and adds coverage for pre-init/non-init persistence behaviors and storage error handlingWritten by Cursor Bugbot for commit 11d2a4f. This will update automatically on new commits. Configure here.