Refactor how extension requests ledger access (don't rely on state sync) and close popup

.changelog/2118.internal.md

@@ -0,0 +1 @@
+Refactor how extension requests ledger access (don't rely on state sync)

extension/src/ExtLedgerAccessPopup/ExtLedgerAccessPopup.tsx

@@ -1,6 +1,5 @@
-import React, { useState } from 'react'
+import { useState } from 'react'
 import { useTranslation } from 'react-i18next'
-import { useDispatch } from 'react-redux'
 import { Box } from 'grommet/es6/components/Box'
 import { Button } from 'grommet/es6/components/Button'
 import { Spinner } from 'grommet/es6/components/Spinner'
@@ -11,10 +10,9 @@ import { Header } from 'app/components/Header'
 import { ErrorFormatter } from 'app/components/ErrorFormatter'
 import { AlertBox } from 'app/components/AlertBox'
 import { WalletErrors } from 'types/errors'
-import { importAccountsActions } from 'app/state/importaccounts'
 import { requestDevice } from 'app/lib/ledger'
-import { WalletType } from '../../../src/app/state/wallet/types'
 import logotype from '../../../public/Icon Blue 192.png'
+import { CountdownButton } from 'app/components/CountdownButton'
 
 type ConnectionStatus = 'connected' | 'disconnected' | 'connecting' | 'error'
 type ConnectionStatusIconPros = {
@@ -46,15 +44,16 @@ function ConnectionStatusIcon({ success = true, label, withMargin = false }: Con
 
 export function ExtLedgerAccessPopup() {
   const { t } = useTranslation()
-  const dispatch = useDispatch()
   const [connection, setConnection] = useState<ConnectionStatus>('disconnected')
   const handleConnect = async () => {
     setConnection('connecting')
     try {
       const device = await requestDevice()
       if (device) {
         setConnection('connected')
-        dispatch(importAccountsActions.enumerateAccountsFromLedger(WalletType.UsbLedger))
+        // 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')
@@ -65,8 +64,8 @@ export function ExtLedgerAccessPopup() {
     <Box
       style={{ minHeight: '100dvh' }}
       justify="center"
-      align="center"
-      pad="medium"
+      align="stretch"
+      pad="xlarge"
       background="background-back"
     >
       <Box
@@ -83,22 +82,9 @@ export function ExtLedgerAccessPopup() {
           {t('ledger.extension.grantAccess', 'Grant access to your Ledger')}
         </Header>
         <Box gap="medium">
-          <ol>
-            <li>
-              {t(
-                'ledger.instructionSteps.connectUsbLedger',
-                'Connect your USB Ledger device to the computer',
-              )}
-            </li>
-            <li>{t('ledger.instructionSteps.closeLedgerLive', 'Close Ledger Live app on the computer')}</li>
-            <li>{t('ledger.instructionSteps.openOasisApp', 'Open the Oasis App on your Ledger device')}</li>
-            <li>
-              {t(
-                'ledger.extension.instructionStep',
-                'Once device is connected continue the operation in the wallet app',
-              )}
-            </li>
-          </ol>
+          <p>
+            {t('ledger.instructionSteps.connectUsbLedger', 'Connect your USB Ledger device to the computer')}
+          </p>
 
           {connection === 'connecting' && (
             <Box
@@ -114,7 +100,14 @@ export function ExtLedgerAccessPopup() {
             </Box>
           )}
           {connection === 'connected' && (
-            <ConnectionStatusIcon label={t('ledger.extension.succeed', 'Device connected')} />
+            <Box>
+              <ConnectionStatusIcon label={t('ledger.extension.succeed', 'Device connected')} withMargin />
+
+              <CountdownButton
+                onClick={() => window.close()}
+                label={t('ledger.extension.closingPopup', 'Closing... Please re-open the wallet app')}
+              />
+            </Box>
           )}
           {connection === 'error' && (
             <Box margin={{ bottom: 'medium' }}>

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

@@ -8,10 +8,10 @@ exports[`<ExtLedgerAccessPopup /> should render component 1`] = `
   display: flex;
   box-sizing: border-box;
   max-width: 100%;
-  -webkit-align-items: center;
-  -webkit-box-align: center;
-  -ms-flex-align: center;
-  align-items: center;
+  -webkit-align-items: stretch;
+  -webkit-box-align: stretch;
+  -ms-flex-align: stretch;
+  align-items: stretch;
   background-color: #EDEDED;
   color: #444444;
   min-width: 0;
@@ -23,7 +23,7 @@ exports[`<ExtLedgerAccessPopup /> should render component 1`] = `
   -webkit-justify-content: center;
   -ms-flex-pack: center;
   justify-content: center;
-  padding: 24px;
+  padding: 96px;
 }
 
 .c1 {
@@ -190,7 +190,7 @@ exports[`<ExtLedgerAccessPopup /> should render component 1`] = `
 
 @media only screen and (max-width:768px) {
   .c0 {
-    padding: 12px;
+    padding: 48px;
   }
 }
 
@@ -253,20 +253,9 @@ exports[`<ExtLedgerAccessPopup /> should render component 1`] = `
       <div
         class="c4"
       >
-        <ol>
-          <li>
-            ledger.instructionSteps.connectUsbLedger
-          </li>
-          <li>
-            ledger.instructionSteps.closeLedgerLive
-          </li>
-          <li>
-            ledger.instructionSteps.openOasisApp
-          </li>
-          <li>
-            ledger.extension.instructionStep
-          </li>
-        </ol>
+        <p>
+          ledger.instructionSteps.connectUsbLedger
+        </p>
         <div
           class="c5"
         />

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

@@ -2,18 +2,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 { importAccountsActions } from 'app/state/importaccounts'
 import { ExtLedgerAccessPopup } from '../ExtLedgerAccessPopup'
-import { WalletType } from '../../../../src/app/state/wallet/types'
 
 jest.mock('app/lib/ledger')
 
-const mockDispatch = jest.fn()
-jest.mock('react-redux', () => ({
-  useSelector: jest.fn(),
-  useDispatch: () => mockDispatch,
-}))
-
 describe('<ExtLedgerAccessPopup />', () => {
   it('should render component', () => {
     const { container } = render(<ExtLedgerAccessPopup />)
@@ -30,11 +22,6 @@ describe('<ExtLedgerAccessPopup />', () => {
 
     expect(await screen.findByText('ledger.extension.succeed')).toBeInTheDocument()
     expect(screen.getByLabelText('Status is okay')).toBeInTheDocument()
-    expect(screen.queryByRole('button')).not.toBeInTheDocument()
-    expect(mockDispatch).toHaveBeenCalledWith({
-      payload: WalletType.UsbLedger,
-      type: importAccountsActions.enumerateAccountsFromLedger.type,
-    })
   })
 
   it('should render error state', async () => {
@@ -46,6 +33,5 @@ describe('<ExtLedgerAccessPopup />', () => {
 
     expect(await screen.findByText('ledger.extension.failed')).toBeInTheDocument()
     expect(screen.getByLabelText('Status is critical')).toBeInTheDocument()
-    expect(mockDispatch).not.toHaveBeenCalled()
   })
 })

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

@@ -7,14 +7,16 @@ import { Text } from 'grommet/es6/components/Text'
 import { canAccessBle, canAccessNavigatorUsb } from '../../../../lib/ledger'
 import { useTranslation } from 'react-i18next'
 import { Capacitor } from '@capacitor/core'
+import TransportWebUSB from '@ledgerhq/hw-transport-webusb'
 
 type SelectOpenMethodProps = {
-  webExtensionUSBLedgerAccess?: () => void
+  openLedgerAccessPopup?: () => void
 }
 
-export function FromLedger({ webExtensionUSBLedgerAccess }: SelectOpenMethodProps) {
+export function FromLedger({ openLedgerAccessPopup }: SelectOpenMethodProps) {
   const { t } = useTranslation()
   const [supportsUsbLedger, setSupportsUsbLedger] = React.useState<boolean | undefined>(true)
+  const [hasUsbLedgerAccess, setHasUsbLedgerAccess] = React.useState<boolean | undefined>(undefined)
   const [supportsBleLedger, setSupportsBleLedger] = React.useState<boolean | undefined>(true)
 
   useEffect(() => {
@@ -31,6 +33,25 @@ export function FromLedger({ webExtensionUSBLedgerAccess }: SelectOpenMethodProp
     getLedgerSupport()
   }, [])
 
+  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
+      // confirms the prompt tell them to come back here. TransportWebUSB.create() will resolve.
+      TransportWebUSB.create()
+        .then(() => setHasUsbLedgerAccess(true))
+        .catch(() => setHasUsbLedgerAccess(false))
+    } else {
+      // Assume true in web app. enumerateAccountsFromLedger will call TransportWebUSB.create in next steps
+      // and will prompt user to select a ledger device.
+      setHasUsbLedgerAccess(true)
+    }
+  }, [openLedgerAccessPopup])
+
+  const shouldOpenUsbLedgerAccessPopup = openLedgerAccessPopup && !hasUsbLedgerAccess
+
   return (
     <Box
       round="5px"
@@ -46,11 +67,14 @@ export function FromLedger({ webExtensionUSBLedgerAccess }: SelectOpenMethodProp
       <Box direction="row-responsive" justify="start" margin={{ top: 'medium' }} gap="medium">
         <div>
           <div>
-            {webExtensionUSBLedgerAccess ? (
+            {shouldOpenUsbLedgerAccessPopup ? (
               <Button
                 disabled={!supportsUsbLedger}
                 style={{ width: 'fit-content' }}
-                onClick={webExtensionUSBLedgerAccess}
+                onClick={() => {
+                  openLedgerAccessPopup()
+                  window.close()
+                }}
                 label={t('ledger.extension.grantAccess', 'Grant access to your USB Ledger')}
                 primary
               />

src/app/pages/OpenWalletPage/FromLedgerWebExtension.tsx

@@ -1,16 +1,14 @@
 import React from 'react'
-import { useHref, useNavigate } from 'react-router-dom'
+import { useHref } from 'react-router-dom'
 import { openLedgerAccessPopup } from 'utils/webextension'
 import { FromLedger } from './Features/FromLedger'
 
 export function FromLedgerWebExtension() {
   const href = useHref('/open-wallet/connect-device')
-  const navigate = useNavigate()
 
   return (
     <FromLedger
-      webExtensionUSBLedgerAccess={() => {
-        navigate('/open-wallet/ledger/usb')
+      openLedgerAccessPopup={() => {
         openLedgerAccessPopup(href)
       }}
     />

src/app/useRouteRedirects.tsx

@@ -9,7 +9,17 @@ export const useRouteRedirects = () => {
   const navigate = useNavigate()
 
   useEffect(() => {
-    if (address) {
+    const hasRecentlyGrantedUsbAccess =
+      Date.now() - parseInt(window.localStorage.getItem('oasis_wallet_granted_usb_ledger_timestamp') ?? '0') <
+      5 * 60 * 1000
+
+    if (hasRecentlyGrantedUsbAccess) {
+      // When we implement auto-locking, this won't correctly redirect if wallet
+      // gets locked within 5min (useEffect called with address=undefined before unlocking).
+      navigate(`/open-wallet/ledger`)
+      // Delay is needed as workaround for React.StrictMode.
+      setTimeout(() => window.localStorage.removeItem('oasis_wallet_granted_usb_ledger_timestamp'), 1000)
+    } else if (address) {
       navigate(`/account/${address}`)
     }
   }, [address, navigate])

src/locales/en/translation.json

@@ -233,10 +233,10 @@
   "language": "Language",
   "ledger": {
     "extension": {
+      "closingPopup": "Closing... Please re-open the wallet app",
       "connect": "Connect Ledger device",
       "failed": "Connection failed",
       "grantAccess": "Grant access to your Ledger",
-      "instructionStep": "Once device is connected continue the operation in the wallet app",
       "succeed": "Device connected"
     },
     "instructionSteps": {