Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor how extension requests ledger access (don't rely on state sync) #2094

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
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
1 change: 1 addition & 0 deletions .changelog/2094.bugfix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Refactor how extension requests ledger access (don't rely on state sync)
11 changes: 5 additions & 6 deletions extension/src/ExtLedgerAccessPopup/ExtLedgerAccessPopup.tsx
Original file line number Diff line number Diff line change
@@ -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'
Expand All @@ -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 { MessageTypes } from '../../../src/utils/constants'

type ConnectionStatus = 'connected' | 'disconnected' | 'connecting' | 'error'
type ConnectionStatusIconPros = {
Expand Down Expand Up @@ -44,17 +42,18 @@ function ConnectionStatusIcon({ success = true, label, withMargin = false }: Con
)
}

const chrome = (window as any).chrome

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))
chrome?.runtime?.sendMessage({ type: MessageTypes.USB_LEDGER_PERMISSION_GRANTED })
}
} catch {
setConnection('error')
Expand Down
7 changes: 0 additions & 7 deletions extension/src/ExtLedgerAccessPopup/__tests__/index.test.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
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')

Expand All @@ -31,10 +28,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 () => {
Expand Down
8 changes: 4 additions & 4 deletions src/app/pages/OpenWalletPage/Features/FromLedger/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ import { useTranslation } from 'react-i18next'
import { Capacitor } from '@capacitor/core'

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 [supportsBleLedger, setSupportsBleLedger] = React.useState<boolean | undefined>(true)
Expand Down Expand Up @@ -46,11 +46,11 @@ export function FromLedger({ webExtensionUSBLedgerAccess }: SelectOpenMethodProp
<Box direction="row-responsive" justify="start" margin={{ top: 'medium' }} gap="medium">
<div>
<div>
{webExtensionUSBLedgerAccess ? (
{openLedgerAccessPopup ? (
<Button
disabled={!supportsUsbLedger}
style={{ width: 'fit-content' }}
onClick={webExtensionUSBLedgerAccess}
onClick={openLedgerAccessPopup}
label={t('ledger.extension.grantAccess', 'Grant access to your USB Ledger')}
primary
/>
Expand Down
49 changes: 42 additions & 7 deletions src/app/pages/OpenWalletPage/FromLedgerWebExtension.tsx
Original file line number Diff line number Diff line change
@@ -1,18 +1,53 @@
import React from 'react'
import { useHref, useNavigate } from 'react-router-dom'
import { useEffect, useState } from 'react'
import { useHref } from 'react-router-dom'
import { openLedgerAccessPopup } from 'utils/webextension'
import { FromLedger } from './Features/FromLedger'
import TransportWebUSB from '@ledgerhq/hw-transport-webusb'
import { MessageTypes, MessageType } from '../../../utils/constants'

const chrome = (window as any).chrome

export function FromLedgerWebExtension() {
const href = useHref('/open-wallet/connect-device')
const navigate = useNavigate()
const [hasUsbLedgerAccess, setHasUsbLedgerAccess] = useState<boolean | undefined>(undefined)

useEffect(() => {
const checkUsbLedgerAccess = async () => {
try {
// 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.
await TransportWebUSB.create()
setHasUsbLedgerAccess(true)
} catch (error) {
setHasUsbLedgerAccess(false)
}
}
checkUsbLedgerAccess()

const handleMessage = (message: { type: MessageType }) => {
if (message.type === MessageTypes.USB_LEDGER_PERMISSION_GRANTED) {
setHasUsbLedgerAccess(true)
}
}
chrome.runtime.onMessage.addListener(handleMessage)

return () => {
chrome.runtime.onMessage.removeListener(handleMessage)
}
}, [])

return (
<FromLedger
webExtensionUSBLedgerAccess={() => {
navigate('/open-wallet/ledger/usb')
openLedgerAccessPopup(href)
}}
openLedgerAccessPopup={
hasUsbLedgerAccess === false
? () => {
openLedgerAccessPopup(href)
}
: undefined
}
/>
)
}
5 changes: 5 additions & 0 deletions src/utils/constants.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export const MessageTypes = {
USB_LEDGER_PERMISSION_GRANTED: 'usb-ledger-permission-granted',
} as const

export type MessageType = (typeof MessageTypes)[keyof typeof MessageTypes]
Loading