Repository navigation
fix(accounts): serialize account-mutating background messages - #3086
Draft
aristidesstaffieri wants to merge 6 commits into
Draft
aristidesstaffieri wants to merge 6 commits into
aristidesstaffieri wants to merge 6 commits into
Conversation
Messages that change accounts, sources or session state now run one at a time behind a single lock in the background message listener. Read-only messages skip the lock.
Contributor
|
PR Preview build is ready: https://github.com/stellar/freighter/releases/tag/untagged-76fe3b3cc6e3a2dda4a4 |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Critical queueing and persistence flaws can block sign-out or auto-lock, expose stale session state, and leave an unlocked session available for signing.
4 open issues
What changed in this PR
This PR serializes account and session mutations in Freighter’s background listener to prevent concurrent requests from overwriting each other, while allowing read-only requests to proceed independently.
Changes:
- Adds a shared promise-based queue for account-mutating messages.
- Places session auto-lock behind the same queue and awaits session persistence before releasing successful mutations.
- Adds concurrency regression tests and lock-ordering tests.
| File | Description |
|---|---|
extension/src/background/index.ts |
Integrates mutation locking, session flushing, and queued auto-lock handling. |
extension/src/background/helpers/account-mutation-lock.ts |
Defines queued message types and the shared lock helper. |
extension/src/background/helpers/__tests__/account-mutation-lock.test.ts |
Tests execution order and queue recovery after rejection. |
extension/src/background/__tests__/initAlarmListener.test.ts |
Adds the session-flush mock required by alarm handling. |
extension/src/background/__tests__/concurrentAccountMutations.test.ts |
Tests concurrent account changes, delayed persistence, sign-out, and read-only responsiveness. |
Review effort: Balanced
Sign-out, rename, switch, add and import skip the active-key mismatch check, so they still run when queued behind another surface's import or switch. The popup's sign-out now fails when the background refuses it.
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.


What
Background messages that change accounts, secret sources or session state now run one at a time behind a single promise-chain lock in the extension message listener. That covers create, recover, add, import, hardware import, migration, phrase generation and storage, password confirmation, rename, account switch, sign-out and reset, and the session-lock alarm takes the same lock. A message holds the lock from the moment it builds its session store until its final state has been flushed to
storage.session, so the next one starts from what the previous one wrote. Read-only messages skip the lock, so a slow Scrypt import doesn't hold up something like loading the account.Why
Each message builds its own session store from
storage.sessionand writes the whole state back, and the account lists instorage.localare read-modify-writes. When the popup, sidebar or fullscreen send account changes at the same time, the later write drops the earlier one, losing an imported account, a rename, a source label, a derivation index or a sign-out, and two adds on the same phrase can get the same index.Closes stellar/freighter-private#112
Screenshots / video
N/A, no UI change.
Checklist