Merge pull request #2201 from oasisprotocol/lw/dedupe-usb-transport

Deduplicate logic in TransportWebUSB.create and requestDevice

Author: lukaw3d

.changelog/2201.trivial.md

@@ -0,0 +1 @@
+Deduplicate logic in TransportWebUSB.create and requestDevice

extension/src/ExtLedgerAccessPopup/ExtLedgerAccessPopup.tsx

@@ -10,7 +10,6 @@ import { Header } from 'app/components/Header'
 import { ErrorFormatter } from 'app/components/ErrorFormatter'
 import { AlertBox } from 'app/components/AlertBox'
 import { WalletErrors } from 'types/errors'
-import { requestDevice } from 'app/lib/ledger'
 import logotype from '../../../public/Icon Blue 192.png'
 import { CountdownButton } from 'app/components/CountdownButton'
 import TransportWebUSB from '@ledgerhq/hw-transport-webusb'
@@ -49,14 +48,11 @@ export function ExtLedgerAccessPopup() {
   const handleConnect = async () => {
     setConnection('connecting')
     try {
-      const device = await requestDevice()
-      const transport = await TransportWebUSB.create()
-      if (device && transport) {
-        setConnection('connected')
-        // Used to redirect after reopening wallet
-        window.localStorage.setItem('oasis_wallet_granted_usb_ledger_timestamp', Date.now().toString())
-        setTimeout(() => window.close(), 5_000)
-      }
+      await (await TransportWebUSB.create()).close() // Get access permissions
+      setConnection('connected')
+      // Used to redirect after reopening wallet
+      window.localStorage.setItem('oasis_wallet_granted_usb_ledger_timestamp', Date.now().toString())
+      setTimeout(() => window.close(), 5_000)
     } catch {
       setConnection('error')
     }

extension/src/ExtLedgerAccessPopup/__tests__/index.test.tsx

@@ -1,12 +1,10 @@
 import React from 'react'
 import { render, screen } from '@testing-library/react'
 import userEvent from '@testing-library/user-event'
-import { requestDevice } from 'app/lib/ledger'
 import { ExtLedgerAccessPopup } from '../ExtLedgerAccessPopup'
 import TransportWebUSB from '@ledgerhq/hw-transport-webusb'
 
-jest.mock('app/lib/ledger')
-jest.mock('@ledgerhq/hw-transport')
+jest.mock('@ledgerhq/hw-transport-webusb')
 
 describe('<ExtLedgerAccessPopup />', () => {
   it('should render component', () => {
@@ -16,8 +14,8 @@ describe('<ExtLedgerAccessPopup />', () => {
   })
 
   it('should render success state', async () => {
-    jest.mocked(requestDevice).mockResolvedValue({} as USBDevice)
-    jest.mocked(TransportWebUSB.create).mockResolvedValue({} as TransportWebUSB)
+    jest.mocked(TransportWebUSB.isSupported).mockResolvedValue(true)
+    jest.mocked(TransportWebUSB.create).mockResolvedValue({ close: () => {} } as TransportWebUSB)
 
     render(<ExtLedgerAccessPopup />)
 
@@ -28,12 +26,12 @@ describe('<ExtLedgerAccessPopup />', () => {
   })
 
   it('should render error state', async () => {
-    jest.mocked(requestDevice).mockRejectedValue(new Error('error'))
-    jest.mocked(TransportWebUSB.create).mockRejectedValue(new Error('error'))
+    jest.mocked(TransportWebUSB.isSupported).mockResolvedValue(true)
+    jest.mocked(TransportWebUSB.create).mockRejectedValue(new Error('Dummy error'))
 
     render(<ExtLedgerAccessPopup />)
 
-    userEvent.click(screen.getByRole('button'))
+    await userEvent.click(screen.getByRole('button'))
 
     expect(await screen.findByText('ledger.extension.failed')).toBeInTheDocument()
     expect(screen.getByLabelText('Status is critical')).toBeInTheDocument()

jest.config.js

@@ -24,9 +24,7 @@ const config = {
       },
     ],
   },
-  transformIgnorePatterns: [
-    '/node_modules/(?!(@ledgerhq/hw-transport-webusb|cborg|grommet/es6|grommet-icons/es6)/)',
-  ],
+  transformIgnorePatterns: ['/node_modules/(?!(cborg|grommet/es6|grommet-icons/es6)/)'],
   testMatch: [
     '<rootDir>/src/**/__tests__/**/*.{js,jsx,ts,tsx}',
     '<rootDir>/src/**/*.{spec,test}.{js,jsx,ts,tsx}',

src/app/lib/__tests__/ledger.test.ts

@@ -1,11 +1,9 @@
-import { canAccessBle, Ledger, LedgerSigner, requestDevice } from '../ledger'
+import { canAccessBle, Ledger, LedgerSigner } from '../ledger'
 import OasisApp from '@oasisprotocol/ledger'
 import { WalletError, WalletErrors } from 'types/errors'
 import { Wallet, WalletType } from 'app/state/wallet/types'
-import { isSupported, requestLedgerDevice } from '@ledgerhq/hw-transport-webusb/lib-es/webusb'
 import BleTransport from '@oasisprotocol/ionic-ledger-hw-transport-ble/lib'
 
-jest.mock('@ledgerhq/hw-transport-webusb/lib-es/webusb')
 jest.mock('@oasisprotocol/ionic-ledger-hw-transport-ble/lib', () => {
   return {
     isEnabled: jest.fn(),
@@ -22,16 +20,6 @@ function mockAppIsOpen(appName: string) {
   appInfo.mockResolvedValueOnce({ appName: appName, return_code: 0x9000, error_message: '' })
 }
 
-describe('Extension access', () => {
-  it('should return a ledger device when web usb is supported', async () => {
-    const device = {} as USBDevice
-    jest.mocked(isSupported).mockResolvedValue(true)
-    jest.mocked(requestLedgerDevice).mockResolvedValue(device)
-    const result = await requestDevice()
-    expect(result).toBe(device)
-  })
-})
-
 describe('Ledger Library', () => {
   afterEach(() => {
     jest.resetAllMocks()

src/app/lib/ledger.ts

@@ -5,7 +5,7 @@ import { LedgerWalletType, Wallet, WalletType } from 'app/state/wallet/types'
 import { WalletError, WalletErrors } from 'types/errors'
 import { hex2uint, publicKeyToAddress } from './helpers'
 import type Transport from '@ledgerhq/hw-transport'
-import { isSupported, requestLedgerDevice } from '@ledgerhq/hw-transport-webusb/lib-es/webusb'
+import TransportWebUSB from '@ledgerhq/hw-transport-webusb'
 import BleTransport from '@oasisprotocol/ionic-ledger-hw-transport-ble/lib'
 import { runtimeIs } from 'app/lib/runtimeIs'
 
@@ -16,7 +16,7 @@ interface LedgerAccount {
 }
 
 export async function canAccessNavigatorUsb(): Promise<boolean> {
-  return await isSupported()
+  return await TransportWebUSB.isSupported()
 }
 
 export async function canAccessBle(): Promise<boolean> {
@@ -26,12 +26,6 @@ export async function canAccessBle(): Promise<boolean> {
   return hasBLE && hasLEScan
 }
 
-export async function requestDevice(): Promise<USBDevice | undefined> {
-  if (await isSupported()) {
-    return await requestLedgerDevice()
-  }
-}
-
 function successOrThrowWalletError<T>(response: Response<T>, message: string) {
   try {
     return successOrThrow(response)

src/app/pages/OpenWalletPage/Features/FromLedger/index.tsx

@@ -35,11 +35,11 @@ export function FromLedger({ openLedgerAccessPopup }: SelectOpenMethodProps) {
   useEffect(() => {
     if (openLedgerAccessPopup) {
       // In default ext popup this gets auto-accepted / auto-rejected. In a tab or persistent popup it would
-      // prompt user to select a ledger device. TransportWebUSB.create seems to match requestDevice called in
-      // openLedgerAccessPopup.
-      // If TransportWebUSB.create() is rejected then call openLedgerAccessPopup and requestDevice. When user
+      // prompt user to select a ledger device.
+      // If TransportWebUSB.create() is rejected then call openLedgerAccessPopup. When user
       // confirms the prompt tell them to come back here. TransportWebUSB.create() will resolve.
       TransportWebUSB.create()
+        .then(t => t.close())
         .then(() => setHasUsbLedgerAccess(true))
         .catch(() => setHasUsbLedgerAccess(false))
     } else {