Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions src/controllers/accountPicker/accountPicker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
32 changes: 26 additions & 6 deletions src/controllers/accountPicker/accountPicker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down Expand Up @@ -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()
}

Expand Down Expand Up @@ -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 = []
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 = []
Expand All @@ -719,21 +738,22 @@ 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
}

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()
Expand All @@ -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()
Expand All @@ -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()
}
Expand Down
16 changes: 16 additions & 0 deletions src/libs/ledger/ledger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 ||
Expand All @@ -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() === '<none>'
) {
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'))
Expand Down