From 33629a7600fb87caf42340a406ab9dbf49b4ec03 Mon Sep 17 00:00:00 2001 From: Lyubomir Nanev Date: Fri, 19 Jun 2026 14:06:42 +0300 Subject: [PATCH] Fix account picker infinite loading and improve Ledger error handling for benign SDK states --- .../accountPicker/accountPicker.test.ts | 18 +++++++++++ .../accountPicker/accountPicker.ts | 32 +++++++++++++++---- src/libs/ledger/ledger.ts | 16 ++++++++++ 3 files changed, 60 insertions(+), 6 deletions(-) diff --git a/src/controllers/accountPicker/accountPicker.test.ts b/src/controllers/accountPicker/accountPicker.test.ts index d5088f2859..6c1525178a 100644 --- a/src/controllers/accountPicker/accountPicker.test.ts +++ b/src/controllers/accountPicker/accountPicker.test.ts @@ -125,6 +125,24 @@ describe('AccountPicker', () => { }) }) + test('should clear accountsLoading when an invalid page is requested', async () => { + const { controller } = await prepareTest() + const keyIterator = new KeyIterator(process.env.SEED) + controller.setInitParams({ + keyIterator, + hdPathTemplate: BIP44_STANDARD_DERIVATION_TEMPLATE, + shouldGetAccountsUsedOnNetworks: false, + shouldSearchForLinkedAccounts: false, + shouldAddNextAccountAutomatically: false + }) + await controller.init() + await controller.setPage({ page: 0 }) + + expect(controller.accountsLoading).toBe(false) + expect(controller.pageError).toBeTruthy() + expect(controller.page).toEqual(DEFAULT_PAGE) + }) + test('should retrieve 5 basic and one smart account on each page', async () => { const { controller } = await prepareTest() const PAGE_SIZE = 5 diff --git a/src/controllers/accountPicker/accountPicker.ts b/src/controllers/accountPicker/accountPicker.ts index 5d72fc9395..1215fa7d12 100644 --- a/src/controllers/accountPicker/accountPicker.ts +++ b/src/controllers/accountPicker/accountPicker.ts @@ -161,6 +161,12 @@ export class AccountPickerController extends EventEmitter implements IAccountPic */ #findAndSetLinkedAccountsAbortController?: AbortController + /** + * Incremented on each setPage() call and on reset() to invalidate in-flight + * page loads. Prevents stale async work from leaving accountsLoading stuck. + */ + #setPageGeneration = 0 + #shouldDebounceFlags: { [key: string]: boolean } = {} #addAccountsOnKeystoreReady: { @@ -419,6 +425,7 @@ export class AccountPickerController extends EventEmitter implements IAccountPic shouldAddNextAccountAutomatically?: boolean }) { this.initParams = params + if (params.pageSize) this.pageSize = params.pageSize this.emitUpdate() } @@ -477,6 +484,8 @@ export class AccountPickerController extends EventEmitter implements IAccountPic this.#findAndSetLinkedAccountsAbortController.abort() this.#findAndSetLinkedAccountsAbortController = undefined } + this.#setPageGeneration += 1 + this.accountsLoading = false if (resetInitParams) this.initParams = null this.keyIterator = null this.selectedAccountsFromCurrentSession = [] @@ -680,6 +689,14 @@ export class AccountPickerController extends EventEmitter implements IAccountPic ) } + /** + * Guard to ensure we only proceed with data that matches the current page load + * request. Similar to #isFindAndSetLinkedAccountsCancelled. + */ + #isSetPageRequestStale(calledForPage: number, calledForGeneration: number): boolean { + return calledForGeneration !== this.#setPageGeneration || calledForPage !== this.page + } + async setPage({ page = this.page, pageSize, @@ -707,6 +724,8 @@ export class AccountPickerController extends EventEmitter implements IAccountPic this.page = page } else if (page === this.page && this.#derivedAccounts.length) return + const setPageGeneration = ++this.#setPageGeneration + this.page = page this.pageError = null this.#derivedAccounts = [] @@ -719,6 +738,7 @@ export class AccountPickerController extends EventEmitter implements IAccountPic if (page <= 0) { this.pageError = `Unexpected page was requested (page ${page}). Please try again or contact support for help.` this.page = DEFAULT_PAGE // fallback to the default (initial) page + this.accountsLoading = false this.emitUpdate() return } @@ -726,14 +746,14 @@ export class AccountPickerController extends EventEmitter implements IAccountPic try { const derivedAccounts = await this.#deriveAccounts() - if (this.page !== page) return + if (this.#isSetPageRequestStale(page, setPageGeneration)) return this.#derivedAccounts = derivedAccounts // The used on information is not critical. Allow the user to proceed after // 1 second. It will get popuplated in the background. const minWaitTimeout = setTimeout(() => { - if (this.page !== page) return + if (this.#isSetPageRequestStale(page, setPageGeneration)) return this.accountsLoading = false this.emitUpdate() @@ -744,11 +764,11 @@ export class AccountPickerController extends EventEmitter implements IAccountPic page }) - if (this.page !== page) return + if (this.#isSetPageRequestStale(page, setPageGeneration)) return this.#derivedAccounts = derivedAccountsWithUsedOn - if (minWaitTimeout) clearTimeout(minWaitTimeout) + clearTimeout(minWaitTimeout) this.accountsLoading = false this.emitUpdate() @@ -767,14 +787,14 @@ export class AccountPickerController extends EventEmitter implements IAccountPic } } } catch (e: any) { - if (this.page !== page) return + if (this.#isSetPageRequestStale(page, setPageGeneration)) return const fallbackMessage = `Failed to retrieve accounts on page ${this.page}. Please try again or contact support for assistance. Error details: ${e?.message}.` this.accountsLoading = false this.pageError = e instanceof ExternalSignerError ? e.message : fallbackMessage this.emitUpdate() } - if (this.page !== page) return + if (this.#isSetPageRequestStale(page, setPageGeneration)) return await this.findAndSetLinkedAccounts() } diff --git a/src/libs/ledger/ledger.ts b/src/libs/ledger/ledger.ts index 45464bf4a4..2d45459eba 100644 --- a/src/libs/ledger/ledger.ts +++ b/src/libs/ledger/ledger.ts @@ -4,6 +4,14 @@ * there is a message incoming from Ledger too, it's not self-explanatory and * can be difficult for the end users to understand. */ + +const BENIGN_LEDGER_USER_INTERACTIONS = new Set(['', 'none', 'None']) + +export const isBenignLedgerUserInteraction = (interaction?: string) => { + if (!interaction) return true + return BENIGN_LEDGER_USER_INTERACTIONS.has(interaction) +} + export const normalizeLedgerMessage = (error?: string): string => { if ( !error || @@ -12,6 +20,14 @@ export const normalizeLedgerMessage = (error?: string): string => { ) return 'Cannot connect to your Ledger device. Please make sure it is connected.' + if ( + error.toLowerCase().includes('no response from device') || + error.toLowerCase() === 'none' || + error.toLowerCase() === '' + ) { + return 'Cannot connect to your Ledger device. Please make sure it is unlocked and the Ethereum app is open.' + } + if (error.includes('unlock-device')) return 'Please unlock your Ledger device first.' if (error.includes('confirm-open-app'))