Skip to content

Commit

Permalink
feat: ledger enhancements (#1775)
Browse files Browse the repository at this point in the history
* feat: better error message for unknown errors

* feat: sign button label on one row

* fix: display actual error message

* refactor: single method for wrapping ledger errors

* fix: webhid transport
  • Loading branch information
0xKheops authored Jan 7, 2025
1 parent 6398774 commit e720c76
Show file tree
Hide file tree
Showing 14 changed files with 234 additions and 885 deletions.
1 change: 1 addition & 0 deletions apps/extension/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
"@hookform/resolvers": "3.9.0",
"@ledgerhq/hw-app-eth": "6.40.2",
"@ledgerhq/hw-transport": "6.31.4",
"@ledgerhq/hw-transport-webhid": "^6.30.0",
"@ledgerhq/hw-transport-webusb": "6.29.4",
"@metamask/eth-sig-util": "8.0.0",
"@polkadot-api/descriptors": "workspace:*",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
LedgerConnectionStatus,
LedgerConnectionStatusProps,
} from "@ui/domains/Account/LedgerConnectionStatus"
import { getCustomTalismanLedgerError } from "@ui/hooks/ledger/errors"
import { getTalismanLedgerError } from "@ui/hooks/ledger/errors"

type ConnectLedgerBaseProps = {
appName: string
Expand Down Expand Up @@ -51,7 +51,7 @@ export const ConnectLedgerBase: FC<ConnectLedgerBaseProps> = ({
})
onReadyChanged?.(true)
} catch (err) {
const error = getCustomTalismanLedgerError(err)
const error = getTalismanLedgerError(err)
log.error("ConnectLedgerSubstrateGeneric", { error })
setConnectionStatus({
status: "error",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { getEthLedgerDerivationPath, LedgerEthDerivationPathType } from "@extens
import { log } from "@extension/shared"
import { convertAddress } from "@talisman/util/convertAddress"
import { LedgerAccountDefEthereum } from "@ui/domains/Account/AccountAdd/AccountAddLedger/context"
import { getCustomTalismanLedgerError } from "@ui/hooks/ledger/errors"
import { getTalismanLedgerError } from "@ui/hooks/ledger/errors"
import { useLedgerEthereum } from "@ui/hooks/ledger/useLedgerEthereum"
import { AccountImportDef, useAccountImportBalances } from "@ui/hooks/useAccountImportBalances"
import { useAccounts, useEvmNetworks } from "@ui/state"
Expand Down Expand Up @@ -93,7 +93,7 @@ const useLedgerEthereumAccounts = (
message: t("Ledger is ready."),
})
} catch (err) {
const error = getCustomTalismanLedgerError(err)
const error = getTalismanLedgerError(err)
log.error("Failed to load page", { err })
setConnectionStatus({
status: "error",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { FormFieldContainer, FormFieldInputText, Tooltip, TooltipTrigger } from
import { convertAddress } from "@talisman/util/convertAddress"
import { LedgerAccountDefSubstrateGeneric } from "@ui/domains/Account/AccountAdd/AccountAddLedger/context"
import { getPolkadotLedgerDerivationPath } from "@ui/hooks/ledger/common"
import { getCustomTalismanLedgerError, TalismanLedgerError } from "@ui/hooks/ledger/errors"
import { getTalismanLedgerError, TalismanLedgerError } from "@ui/hooks/ledger/errors"
import { useLedgerSubstrateGeneric } from "@ui/hooks/ledger/useLedgerSubstrateGeneric"
import { AccountImportDef, useAccountImportBalances } from "@ui/hooks/useAccountImportBalances"
import { useAccounts, useChain, useChains } from "@ui/state"
Expand Down Expand Up @@ -122,7 +122,7 @@ const useLedgerSubstrateGenericAccounts = (
message: t("Ledger is ready."),
})
} catch (err) {
const error = getCustomTalismanLedgerError(err)
const error = getTalismanLedgerError(err)
log.error("Failed to load page", { err })
setConnectionStatus({
status: "error",
Expand Down Expand Up @@ -339,7 +339,7 @@ const useLedgerAccountAddress = (
message: t("Ledger is ready."),
})
} catch (err) {
const error = getCustomTalismanLedgerError(err)
const error = getTalismanLedgerError(err)
log.error("Failed to load page", { err })
setConnectionStatus({
status: "error",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { useTranslation } from "react-i18next"
import { isChainActive, SubstrateLedgerAppType } from "@extension/core"
import { log } from "@extension/shared"
import { convertAddress } from "@talisman/util/convertAddress"
import { getCustomTalismanLedgerError } from "@ui/hooks/ledger/errors"
import { getTalismanLedgerError } from "@ui/hooks/ledger/errors"
import { useLedgerSubstrateLegacy } from "@ui/hooks/ledger/useLedgerSubstrateLegacy"
import { AccountImportDef, useAccountImportBalances } from "@ui/hooks/useAccountImportBalances"
import { useAccounts, useActiveChainsState, useChain } from "@ui/state"
Expand Down Expand Up @@ -105,7 +105,7 @@ const useLedgerChainAccounts = (
message: t("Ledger is ready."),
})
} catch (err) {
const error = getCustomTalismanLedgerError(err)
const error = getTalismanLedgerError(err)
log.error("Failed to load page", { err })
setConnectionStatus({
status: "error",
Expand Down
2 changes: 1 addition & 1 deletion apps/extension/src/ui/domains/Sign/SignLedgerBase.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export const SignLedgerBase: FC<{
>
{!!onCancel && <Button onClick={onCancel}>{t("Cancel")}</Button>}
<Button primary processing={isProcessing} onClick={onSignClick} className="px-4">
{t("Approve on Ledger")}
{t("Sign on Ledger")}
</Button>
<ErrorMessageDrawer
name={error?.name}
Expand Down
6 changes: 3 additions & 3 deletions apps/extension/src/ui/domains/Sign/SignLedgerEthereum.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { useTranslation } from "react-i18next"

import { AccountJsonHardwareEthereum } from "@extension/core"
import { log } from "@extension/shared"
import { getCustomTalismanLedgerError } from "@ui/hooks/ledger/errors"
import { getTalismanLedgerError } from "@ui/hooks/ledger/errors"
import { useLedgerEthereum } from "@ui/hooks/ledger/useLedgerEthereum"

import { SignHardwareEthereumProps } from "./SignHardwareEthereum"
Expand Down Expand Up @@ -47,12 +47,12 @@ export const SignLedgerEthereum: FC<SignHardwareEthereumProps> = ({
const errCheck = err as Error & { statusCode?: number; reason?: string }
if (errCheck.reason === "invalid object key - maxPriorityFeePerGas") {
setError(
getCustomTalismanLedgerError(
getTalismanLedgerError(
t("Sorry, Talisman doesn't support signing transactions with Ledger on this network."),
),
)
} else {
const error = getCustomTalismanLedgerError(err)
const error = getTalismanLedgerError(err)
log.error("signLedger", { error })
setError(error)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { FC, useCallback } from "react"

import { AccountJsonHardwareSubstrate } from "@extension/core"
import { log } from "@extension/shared"
import { getCustomTalismanLedgerError } from "@ui/hooks/ledger/errors"
import { getTalismanLedgerError } from "@ui/hooks/ledger/errors"
import { useLedgerSubstrateAppByName } from "@ui/hooks/ledger/useLedgerSubstrateApp"
import { useLedgerSubstrateGeneric } from "@ui/hooks/ledger/useLedgerSubstrateGeneric"
import { useAccountByAddress } from "@ui/state"
Expand Down Expand Up @@ -44,7 +44,7 @@ export const SignLedgerSubstrateGeneric: FC<SignHardwareSubstrateProps> = ({
// await to keep loader spinning until popup closes
await onSigned({ signature })
} catch (err) {
const error = getCustomTalismanLedgerError(err)
const error = getTalismanLedgerError(err)
log.error("signLedger", { error })
setError(error)
} finally {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { useTranslation } from "react-i18next"

import { AccountJsonHardwareSubstrate } from "@extension/core"
import { log } from "@extension/shared"
import { getCustomTalismanLedgerError } from "@ui/hooks/ledger/errors"
import { getTalismanLedgerError } from "@ui/hooks/ledger/errors"
import { useLedgerSubstrateLegacy } from "@ui/hooks/ledger/useLedgerSubstrateLegacy"
import { useAccountByAddress } from "@ui/state"

Expand All @@ -28,7 +28,7 @@ export const SignLedgerSubstrateLegacy: FC<SignHardwareSubstrateProps> = ({

const signWithLedger = useCallback(async () => {
if (!payload || !onSigned || !account) return
if (!registry) return setError(getCustomTalismanLedgerError(t("Missing registry.")))
if (!registry) return setError(getTalismanLedgerError(t("Missing registry.")))

onSentToDevice?.(true)
setIsSigning(true)
Expand All @@ -39,7 +39,7 @@ export const SignLedgerSubstrateLegacy: FC<SignHardwareSubstrateProps> = ({
// await to keep loader spinning until popup closes
await onSigned({ signature })
} catch (err) {
const error = getCustomTalismanLedgerError(err)
const error = getTalismanLedgerError(err)
log.error("signLedger", { error })
setError(error)
} finally {
Expand Down
28 changes: 14 additions & 14 deletions apps/extension/src/ui/hooks/ledger/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ type NativeLedgerError = {
name?: string // if error raised by @zondax/*
statusCode?: number // if error raised by @zondax/*
returnCode?: number // if error raised by @ledgerhq/hw-transport
code?: number // if error raised by @ledgerhq/hw-transport-webusb
}

// used to generate an error-like object when using an api (substrate legacy app) that returns error message as part of the response without throwing it
Expand All @@ -26,6 +27,7 @@ export const getCustomNativeLedgerError = (
}

type TalismanLedgerErrorName =
| "Custom"
| "Unknown"
| "UnsupportedVersion"
| "InvalidApp"
Expand All @@ -46,15 +48,14 @@ export class TalismanLedgerError extends Error {
}
}

export const getCustomTalismanLedgerError = (errorOrMessage: unknown): TalismanLedgerError => {
if (errorOrMessage instanceof TalismanLedgerError) return errorOrMessage
export const getTalismanLedgerError = (
error: unknown,
appName: string = "Unknown App",
): TalismanLedgerError => {
if (error instanceof TalismanLedgerError) return error

if (typeof errorOrMessage === "string") return new TalismanLedgerError("Unknown", errorOrMessage)
if (typeof error === "string") return new TalismanLedgerError("Custom", error)

return getTalismanLedgerError(errorOrMessage as NativeLedgerError, "substrate")
}

export const getTalismanLedgerError = (error: unknown, appName: string): TalismanLedgerError => {
log.log("getTalismanLedgerError", { error })

const cause = error as NativeLedgerError
Expand Down Expand Up @@ -84,7 +85,7 @@ export const getTalismanLedgerError = (error: unknown, appName: string): Talisma
case "InvalidStateError":
return new TalismanLedgerError(
"Unknown",
t("Failed to connect to Ledger (invalid state)"),
t("Failed to connect to Ledger (invalid state). Ledger might be busy."),
{ cause },
)

Expand Down Expand Up @@ -115,7 +116,6 @@ export const getTalismanLedgerError = (error: unknown, appName: string): Talisma

// Polkadot specific errors, wrapped in simple Error object
// only message is available
// TODO Check if still a thing since ledger generic app
switch (cause.message) {
case "Timeout": // this one is throw by Talisman in case of timeout when calling ledger.getAddress
return new TalismanLedgerError("Timeout", t("Failed to connect to your Ledger (timeout)"), {
Expand Down Expand Up @@ -182,11 +182,10 @@ export const getTalismanLedgerError = (error: unknown, appName: string): Talisma
// eslint-disable-next-line no-console
DEBUG && console.warn("unmanaged ledger error", { error })

return new TalismanLedgerError(
"Unknown",
t("Failed to connect to your Ledger. Click here to retry."),
{ cause },
)
// If available, display the actual error message so our help-desk can understand what s going on
return new TalismanLedgerError("Unknown", cause.message ?? "Failed to connect to your Ledger", {
cause,
})
}

const getErrorFromCode = (code: number | undefined, appName: string, cause: unknown) => {
Expand All @@ -200,6 +199,7 @@ const getErrorFromCode = (code: number | undefined, appName: string, cause: unkn

case 27404: // locked
case 27010:
case 21781:
return new TalismanLedgerError("Locked", t("Please unlock your Ledger"), { cause })

case 28160: // non-compatible app
Expand Down
12 changes: 5 additions & 7 deletions apps/extension/src/ui/hooks/ledger/useLedgerSubstrateGeneric.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { useCallback, useRef } from "react"
import { useTranslation } from "react-i18next"

import { getPolkadotLedgerDerivationPath } from "./common"
import { getCustomTalismanLedgerError, getTalismanLedgerError, TalismanLedgerError } from "./errors"
import { getTalismanLedgerError, TalismanLedgerError } from "./errors"
import { useLedgerTransport } from "./useLedgerTransport"

type LedgerRequest<T> = (ledger: PolkadotGenericApp) => Promise<T>
Expand Down Expand Up @@ -93,19 +93,17 @@ const signPayload = async (

if (isJsonPayload(payload)) {
if (!payload.withSignedTransaction)
throw getCustomTalismanLedgerError(
throw getTalismanLedgerError(
t("This dapp needs to be updated in order to support Ledger signing."),
)
if (!registry) throw getCustomTalismanLedgerError(t("Missing registry."))
if (!registry) throw getTalismanLedgerError(t("Missing registry."))

const hasCheckMetadataHash = registry.metadata.extrinsic.signedExtensions.some(
(ext) => ext.identifier.toString() === "CheckMetadataHash",
)
if (!hasCheckMetadataHash)
throw getCustomTalismanLedgerError(
t("This network doesn't support Ledger Polkadot Generic App."),
)
if (!txMetadata) throw getCustomTalismanLedgerError(t("Missing short metadata"))
throw getTalismanLedgerError(t("This network doesn't support Ledger Polkadot Generic App."))
if (!txMetadata) throw getTalismanLedgerError(t("Missing short metadata"))

const unsigned = registry.createType("ExtrinsicPayload", payload)

Expand Down
71 changes: 49 additions & 22 deletions apps/extension/src/ui/hooks/ledger/useLedgerTransport.ts
Original file line number Diff line number Diff line change
@@ -1,35 +1,55 @@
import Transport from "@ledgerhq/hw-transport"
import TransportWebHID from "@ledgerhq/hw-transport-webhid"
import TransportWebUSB from "@ledgerhq/hw-transport-webusb"
import { sleep } from "@talismn/util"
import { log } from "extension-shared"
import { useCallback, useEffect, useRef } from "react"

import { getIsLedgerCapable } from "@ui/util/getIsLedgerCapable"
import { getIsLedgerCapable, LedgerTransportType } from "@ui/util/getIsLedgerCapable"

import { getTalismanLedgerError } from "./errors"

const LEDGER_IN_PROGRESS_ERROR = "An operation that changes interface state is in progress."

const safelyCreateTransport = async (attempt = 1): Promise<Transport> => {
if (!getIsLedgerCapable()) throw new Error("Ledger is not supported on your browser.")
const IS_USB_SUPPORTED = getIsLedgerCapable("usb")
const IS_HID_SUPPORTED = getIsLedgerCapable("hid")

const safelyCreateTransport = async (type: LedgerTransportType, attempt = 1) => {
if (attempt > 5) throw getTalismanLedgerError("Unable to connect to Ledger")

if (attempt > 5) throw new Error("Unable to connect to Ledger")
try {
return await TransportWebUSB.create()
} catch (e) {
if ((e as Error).message.includes(LEDGER_IN_PROGRESS_ERROR)) {
await new Promise((resolve) => setTimeout(resolve, 200 * attempt))
return await safelyCreateTransport(attempt + 1)
} else throw e
switch (type) {
case "usb":
return await TransportWebUSB.create()
case "hid":
return await TransportWebHID.create()
}
} catch (err) {
// in onboarding wizards, might need to wait for previous page/component to finish closing previous transport
if ((err as Error).message.includes(LEDGER_IN_PROGRESS_ERROR)) {
await sleep(200) // it should be almost instant but just in case, wait 1 second max (5 x 200ms)
return safelyCreateTransport(type, attempt + 1)
}
throw getTalismanLedgerError(err)
}
}

const safelyCloseTransport = async (transport: Transport | null, attempt = 1): Promise<void> => {
if (attempt > 5) throw new Error("Unable to disconnect Ledger")
const createTransport = async (): Promise<Transport> => {
try {
await transport?.close()
} catch (e) {
if ((e as Error).message.includes(LEDGER_IN_PROGRESS_ERROR)) {
await new Promise((resolve) => setTimeout(resolve, 100 * attempt))
return await safelyCloseTransport(transport, attempt + 1)
} else throw e
if (IS_USB_SUPPORTED) {
try {
return await safelyCreateTransport("usb")
} catch (err) {
if (!IS_HID_SUPPORTED) throw err
return await safelyCreateTransport("hid")
}
} else if (IS_HID_SUPPORTED) {
return await safelyCreateTransport("hid")
}

throw getTalismanLedgerError("Ledger is not supported on your browser.")
} catch (err) {
throw getTalismanLedgerError(err)
}
}

Expand All @@ -38,7 +58,7 @@ export const useLedgerTransport = () => {

const ensureTransport = useCallback(async () => {
if (!refTransport.current) {
refTransport.current = await safelyCreateTransport()
refTransport.current = await createTransport()
refTransport.current.on("disconnect", () => {
refTransport.current = null
})
Expand All @@ -50,13 +70,20 @@ export const useLedgerTransport = () => {
const closeTransport = useCallback(async () => {
if (!refTransport.current) return

await safelyCloseTransport(refTransport.current)
refTransport.current = null
try {
await refTransport.current.close()
} catch (err) {
log.error("Failed to close transport", { err })
} finally {
refTransport.current = null
}
}, [])

useEffect(() => {
return () => {
if (refTransport.current) safelyCloseTransport(refTransport.current).catch(log.error)
refTransport.current?.close().catch((err) => {
log.error("Failed to close transport on unmount", { err })
})
}
}, [])

Expand Down
10 changes: 9 additions & 1 deletion apps/extension/src/ui/util/getIsLedgerCapable.ts
Original file line number Diff line number Diff line change
@@ -1 +1,9 @@
export const getIsLedgerCapable = () => !!(window as unknown as { USB?: unknown }).USB
export type LedgerTransportType = "usb" | "hid"

export const getIsLedgerCapable = (type?: LedgerTransportType) => {
const ledgerWindow = window as unknown as { USB?: unknown; HID?: unknown }
const ledgerNavigator = window.navigator as unknown as { usb?: unknown; hid?: unknown }
if (type === "usb") return !!ledgerWindow.USB || !!ledgerNavigator.usb
if (type === "hid") return !!ledgerWindow.HID || !!ledgerNavigator.hid
return ledgerWindow.USB || ledgerWindow.HID || ledgerNavigator.usb || ledgerNavigator.hid
}
Loading

0 comments on commit e720c76

Please sign in to comment.