Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
6 changes: 5 additions & 1 deletion packages/controllers/src/utils/ConnectUtil.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,11 @@ export const ConnectUtil = {
*/
getWalletConnectWallets(wcAllWallets: WcWallet[], wcSearchWallets: WcWallet[]) {
if (wcSearchWallets.length > 0) {
return wcSearchWallets.map(w => this.mapWalletToWalletItem(w))
const withInstalled = WalletUtil.markWalletsAsInstalled(wcSearchWallets)
const filtered = WalletUtil.filterWalletsByWcSupport(withInstalled)
const withIndex = WalletUtil.markWalletsWithDisplayIndex(filtered)

return withIndex.map(w => this.mapWalletToWalletItem(w))
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

markWalletsWithDisplayIndex() adds display_index onto the WcWallets, but mapWalletToWalletItem()/WalletItem never reads that field, so this extra pass is currently dead work in this code path. Consider dropping the markWalletsWithDisplayIndex call here (or explicitly mapping the index into the returned shape if callers actually need it) to avoid unnecessary allocations/iteration.

Suggested change
const withIndex = WalletUtil.markWalletsWithDisplayIndex(filtered)
return withIndex.map(w => this.mapWalletToWalletItem(w))
return filtered.map(w => this.mapWalletToWalletItem(w))

Copilot uses AI. Check for mistakes.
}

return WalletUtil.getWalletConnectWallets(wcAllWallets).map(w => this.mapWalletToWalletItem(w))
Expand Down
91 changes: 91 additions & 0 deletions packages/controllers/tests/utils/ConnectUtil.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'

import {
ConnectionController,
ConnectorController,
OptionsController
} from '../../exports/index.js'
import { AssetUtil } from '../../src/utils/AssetUtil.js'
import { ConnectUtil } from '../../src/utils/ConnectUtil.js'
import { CoreHelperUtil } from '../../src/utils/CoreHelperUtil.js'
import type { WcWallet } from '../../src/utils/TypeUtil.js'

// -- Helpers ------------------------------------------------------------------
function createMockWcWallet(overrides: Partial<WcWallet> = {}): WcWallet {
return {
id: 'wallet-id',
name: 'Test Wallet',
supports_wc: true,
mobile_link: 'testwalletapp://',
...overrides
}
}

// -- Tests --------------------------------------------------------------------
describe('ConnectUtil', () => {
describe('getWalletConnectWallets', () => {
beforeEach(() => {
vi.restoreAllMocks()
ConnectorController.state.connectors = []
OptionsController.state.featuredWalletIds = undefined
ConnectionController.state.wcBasic = false
vi.spyOn(AssetUtil, 'getWalletImageUrl').mockReturnValue('')
})

it('should filter out wallets without wc support from search results on mobile', () => {
vi.spyOn(CoreHelperUtil, 'isMobile').mockReturnValue(true)

const walletWithWc = createMockWcWallet({
id: 'wc-wallet',
name: 'WC Wallet',
supports_wc: true
})
const walletWithoutWc = createMockWcWallet({
id: 'no-wc-wallet',
name: 'No WC Wallet',
supports_wc: false,
mobile_link: 'noWcWalletapp://'
})

const result = ConnectUtil.getWalletConnectWallets([], [walletWithWc, walletWithoutWc])

expect(result.map(w => w.id)).toEqual(['wc-wallet'])
})

it('should include all search results on desktop regardless of wc support', () => {
vi.spyOn(CoreHelperUtil, 'isMobile').mockReturnValue(false)

const walletWithWc = createMockWcWallet({
id: 'wc-wallet',
name: 'WC Wallet',
supports_wc: true
})
const walletWithoutWc = createMockWcWallet({
id: 'no-wc-wallet',
name: 'No WC Wallet',
supports_wc: false
})

const result = ConnectUtil.getWalletConnectWallets([], [walletWithWc, walletWithoutWc])

expect(result.map(w => w.id)).toEqual(['wc-wallet', 'no-wc-wallet'])
})
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test name says desktop search results are included “regardless of wc support”, but that’s only true when ConnectionController.state.wcBasic is false (in wcBasic mode the filter will still exclude supports_wc: false). Consider renaming the test to reflect the wcBasic = false precondition, or add a separate assertion for the wcBasic=true behavior to avoid misleading coverage.

Copilot uses AI. Check for mistakes.

it('should preserve order of search results', () => {
vi.spyOn(CoreHelperUtil, 'isMobile').mockReturnValue(false)

const wallets = [
createMockWcWallet({ id: 'wallet-0', name: 'Wallet 0' }),
createMockWcWallet({ id: 'wallet-1', name: 'Wallet 1' }),
createMockWcWallet({ id: 'wallet-2', name: 'Wallet 2' })
]

const result = ConnectUtil.getWalletConnectWallets([], wallets)

expect(result).toHaveLength(3)
expect(result[0]?.id).toBe('wallet-0')
expect(result[1]?.id).toBe('wallet-1')
expect(result[2]?.id).toBe('wallet-2')
})
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test claims search result order is preserved, but getWalletConnectWallets() now calls markWalletsAsInstalled(), which can reorder results when there are installed wallets and/or featuredWalletIds is set. Consider tightening the test name to the specific conditions being asserted (e.g., no installed/featured sorting applied), or add a case that validates the expected reordering when installation/featured sorting is active.

Copilot uses AI. Check for mistakes.
})
})
Loading