Improve Ledger reliability and signing (WebHID, lifecycle, mismatch guard) (#1611)

* fix: prefer WebHID over WebUSB for Ledger transport to fix Flex/Nano X connectivity

* fix(ui): release Ledger transport with explicit disconnect lifecycle

* fix(ui): align Ledger app mode with reactive settings context

* refactor(ui): keep useLedger render phase pure for init errors

* fix(ui): apply popup-aware routing to Ledger connection links

* fix(ui): harden Ledger signing error handling and mismatch UX

- Remove chain.name being set as specName from merkleizeMetadata payload
- Use reactive SettingsContext ledgerApp instead of settings.get() snapshot
- Guard no-metadata signing path with setIsBusy(false) + early return
- Detect signer mismatch by normalizing addresses and block signing when mismatched

packages/extension-ui/package.json

@@ -29,7 +29,7 @@
     "@polkadot/extension-chains": "0.62.7-1-x",
     "@polkadot/extension-dapp": "0.62.7-1-x",
     "@polkadot/extension-inject": "0.62.7-1-x",
-    "@polkadot/hw-ledger": "^13.5.9",
+    "@polkadot/hw-ledger": "^14.0.3",
     "@polkadot/keyring": "^13.5.9",
     "@polkadot/networks": "^13.5.9",
     "@polkadot/react-identicon": "^3.16.4",

packages/extension-ui/src/Popup/Signing/LedgerSign.tsx

@@ -11,13 +11,13 @@ import type { HexString } from '@polkadot/util/types';
 
 import { faSync } from '@fortawesome/free-solid-svg-icons';
 import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
-import React, { useCallback, useEffect, useState } from 'react';
+import React, { useCallback, useContext, useState } from 'react';
 
-import settings from '@polkadot/ui-settings';
+import { assert, objectSpread, stringShorten, u8aToHex } from '@polkadot/util';
-import { assert, objectSpread, u8aToHex } from '@polkadot/util';
+import { decodeAddress } from '@polkadot/util-crypto';
 import { merkleizeMetadata } from '@polkadot-api/merkleize-metadata';
 
-import { Button, Warning } from '../../components/index.js';
+import { Button, SettingsContext, Warning } from '../../components/index.js';
 import { useLedger, useMetadata, useTranslation } from '../../hooks/index.js';
 import { styled } from '../../styled.js';
 
@@ -42,7 +42,6 @@ function getMetadataProof (chain: Chain, payload: SignerPayloadJSON) {
   const merkleizedMetadata = merkleizeMetadata(m, {
     base58Prefix: chain.ss58Format,
     decimals: chain.tokenDecimals,
-    specName: chain.name.toLowerCase(),
     specVersion: chain.specVersion,
     tokenSymbol: chain.tokenSymbol
   });
@@ -56,25 +55,48 @@ function getMetadataProof (chain: Chain, payload: SignerPayloadJSON) {
   };
 }
 
+function matchesExpectedSigner (derivedAddress: string, expectedAddress: string, isEthereum: boolean): boolean {
+  if (isEthereum) {
+    return derivedAddress.toLowerCase() === expectedAddress.toLowerCase();
+  }
+
+  try {
+    return u8aToHex(decodeAddress(derivedAddress)) === u8aToHex(decodeAddress(expectedAddress));
+  } catch {
+    return derivedAddress === expectedAddress;
+  }
+}
+
 function LedgerSign ({ accountIndex, addressOffset, className, error, genesisHash, isEthereum = false, onSignature, payloadExt, payloadJson, setError }: Props): React.ReactElement<Props> {
   const [isBusy, setIsBusy] = useState(false);
   const { t } = useTranslation();
+  const { ledgerApp } = useContext(SettingsContext);
   const chain = useMetadata(genesisHash);
-  const { error: ledgerError, isLoading: ledgerLoading, isLocked: ledgerLocked, ledger, refresh, type: _ledgerType, warning: ledgerWarning } = useLedger(genesisHash, accountIndex, addressOffset, isEthereum);
+  const { address, error: ledgerError, isLoading: ledgerLoading, isLocked: ledgerLocked, ledger, refresh, type: _ledgerType, warning: ledgerWarning } = useLedger(genesisHash, accountIndex, addressOffset, isEthereum);
-
+  const expectedAddress = payloadJson?.address;
-  useEffect(() => {
+  const signerMismatchError = address && expectedAddress && !matchesExpectedSigner(address, expectedAddress, isEthereum)
-    if (ledgerError) {
+    ? t('Address mismatch: derived {{derived}}, expected {{expected}}. Check that the correct Ledger device is connected and the correct app is selected.', {
-      setError(ledgerError);
+      replace: {
-    }
+        derived: stringShorten(address, 8),
-  }, [chain, ledgerError, setError]);
+        expected: stringShorten(expectedAddress, 8)
+      }
+    })
+    : null;
+  const signerMismatchHint = signerMismatchError
+    ? t('Possible cause: the Ledger App setting differs from the app originally used to derive this account.')
+    : null;
 
   const _onRefresh = useCallback(() => {
-    refresh();
     setError(null);
+    refresh();
   }, [refresh, setError]);
 
   const _onSignLedger = useCallback(
     (): void => {
+      if (signerMismatchError) {
+        return;
+      }
+
       if (!ledger || !payloadJson || !onSignature || !chain || !payloadExt) {
         if (!chain) {
           setError('No chain information found. You may need to update/upload the metadata.');
@@ -87,11 +109,14 @@ function LedgerSign ({ accountIndex, addressOffset, className, error, genesisHas
       setError(null);
       setIsBusy(true);
 
-      const currApp = settings.get().ledgerApp;
+      const currApp = ledgerApp;
 
       if (currApp === 'generic' || currApp === 'migration') {
         if (!chain?.definition.rawMetadata) {
           setError('No metadata found for this chain. You must upload the metadata to the extension in order to use Ledger.');
+          setIsBusy(false);
+
+          return;
         }
 
         const { raw, txMetadata } = getMetadataProof(chain, payloadJson);
@@ -152,9 +177,12 @@ function LedgerSign ({ accountIndex, addressOffset, className, error, genesisHas
           });
       }
     },
-    [accountIndex, addressOffset, chain, ledger, onSignature, payloadJson, payloadExt, setError, isEthereum]
+    [accountIndex, addressOffset, chain, ledger, ledgerApp, onSignature, payloadJson, payloadExt, setError, signerMismatchError, isEthereum]
   );
 
+  const activeError = ledgerError || error;
+  const hasActiveError = !!activeError;
+
   return (
     <div className={className}>
       {!!ledgerWarning && (
@@ -162,17 +190,22 @@ function LedgerSign ({ accountIndex, addressOffset, className, error, genesisHas
           {ledgerWarning}
         </Warning>
       )}
-      {error && (
+      {hasActiveError && (
         <Warning isDanger>
-          {error}
+          {activeError}
         </Warning>
       )}
-      {
+      {signerMismatchError && !hasActiveError && (
-        <Warning>
+        <Warning isDanger>
-          {`You are using the Ledger ${settings.ledgerApp.toUpperCase()} App. If you would like to switch it, please go to "MANAGE LEDGER APP" in the extension's settings.`}
+          {signerMismatchError}
         </Warning>
-      }
+      )}
-      {(ledgerLocked || error)
+      {signerMismatchHint && !hasActiveError && (
+        <Warning>
+          {signerMismatchHint}
+        </Warning>
+      )}
+      {(ledgerLocked || hasActiveError)
         ? (
           <Button
             isBusy={isBusy || ledgerLoading}
@@ -185,6 +218,7 @@ function LedgerSign ({ accountIndex, addressOffset, className, error, genesisHas
         : (
           <Button
             isBusy={isBusy || ledgerLoading}
+            isDisabled={!!signerMismatchError}
             onClick={_onSignLedger}
           >
             {t('Sign on Ledger')}

packages/extension-ui/src/hooks/useLedger.ts

@@ -9,13 +9,14 @@ import type { Network } from '@polkadot/networks/types';
 import type { HexString } from '@polkadot/util/types';
 import type { KeypairType } from '@polkadot/util-crypto/types';
 
-import { useCallback, useEffect, useMemo, useState } from 'react';
+import { useCallback, useContext, useEffect, useMemo, useRef, useState } from 'react';
 
 import { Ledger, LedgerGeneric } from '@polkadot/hw-ledger';
 import { knownLedger } from '@polkadot/networks/defaults';
 import { settings } from '@polkadot/ui-settings';
 import { assert } from '@polkadot/util';
 
+import { SettingsContext } from '../components/contexts.js';
 import chains from '../util/chains.js';
 import ledgerChains from '../util/legerChains.js';
 import useTranslation from './useTranslation.js';
@@ -40,8 +41,14 @@ function getNetwork (genesisHash: string): Network | undefined {
   return ledgerChains.find(({ genesisHash: [hash] }) => hash === genesisHash);
 }
 
+function getTransportType (): 'hid' | 'webusb' {
+  // prefer WebHID over WebUSB.
+  return ('hid' in navigator) ? 'hid' : 'webusb';
+}
+
 function getState (): StateBase {
-  const isLedgerCapable = !!(window as unknown as { USB?: unknown }).USB;
+  const w = window as unknown as { USB?: unknown; hid?: unknown };
+  const isLedgerCapable = !!(w.hid || w.USB);
 
   return {
     isLedgerCapable,
@@ -49,11 +56,9 @@ function getState (): StateBase {
   };
 }
 
-function retrieveLedger (genesis: string): LedgerGeneric | Ledger {
+function retrieveLedger (genesis: string, ledgerApp: string): LedgerGeneric | Ledger {
   let ledger: LedgerGeneric | Ledger | null = null;
 
-  const currApp = settings.get().ledgerApp;
-
   const { isLedgerCapable } = getState();
 
   assert(isLedgerCapable, 'Incompatible browser, only Chrome is supported');
@@ -64,21 +69,19 @@ function retrieveLedger (genesis: string): LedgerGeneric | Ledger {
 
   assert(def.slip44, 'Slip44 is not available for this network, please report an issue to update this chains slip44');
 
-  // All chains use the `slip44` from polkadot in their derivation path in ledger.
+  const transport = getTransportType();
-  // This interface is specific to the underlying PolkadotGenericApp.
-  ledger = new LedgerGeneric('webusb', def.network, knownLedger['polkadot']);
 
-  if (currApp === 'generic') {
+  if (ledgerApp === 'generic') {
     // All chains use the `slip44` from polkadot in their derivation path in ledger.
     // This interface is specific to the underlying PolkadotGenericApp.
-    ledger = new LedgerGeneric('webusb', def.network, knownLedger['polkadot']);
+    ledger = new LedgerGeneric(transport, def.network, knownLedger['polkadot']);
-  } else if (currApp === 'migration') {
+  } else if (ledgerApp === 'migration') {
-    ledger = new LedgerGeneric('webusb', def.network, knownLedger[def.network]);
+    ledger = new LedgerGeneric(transport, def.network, knownLedger[def.network]);
-  } else if (currApp === 'chainSpecific') {
+  } else if (ledgerApp === 'chainSpecific') {
-    ledger = new Ledger('webusb', def.network);
+    ledger = new Ledger(transport, def.network);
   } else {
     // This will never get touched since it will always hit the above two. This satisfies the compiler.
-    ledger = new LedgerGeneric('webusb', def.network, knownLedger['polkadot']);
+    ledger = new LedgerGeneric(transport, def.network, knownLedger['polkadot']);
   }
 
   return ledger;
@@ -87,12 +90,16 @@ function retrieveLedger (genesis: string): LedgerGeneric | Ledger {
 export default function useLedger (genesis?: string | null, accountIndex = 0, addressOffset = 0, isEthereum = false): State {
   const [isLoading, setIsLoading] = useState(false);
   const [isLocked, setIsLocked] = useState(false);
-  const [refreshLock, setRefreshLock] = useState(false);
+  const [refreshCount, setRefreshCount] = useState(0);
   const [warning, setWarning] = useState<string | null>(null);
   const [error, setError] = useState<string | null>(null);
   const [address, setAddress] = useState<string | null>(null);
   const [type, setType] = useState<KeypairType | null>(null);
   const { t } = useTranslation();
+  const { ledgerApp } = useContext(SettingsContext);
+  // Holds the ledger from the previous effect run so we can close its
+  // transport when the network changes and a new instance is created.
+  const prevLedgerRef = useRef<LedgerGeneric | Ledger | null>(null);
 
   const handleGetAddressError = (e: Error, genesis: string) => {
     setIsLoading(false);
@@ -117,90 +124,148 @@ export default function useLedger (genesis?: string | null, accountIndex = 0, ad
     setType(null);
   };
 
-  const ledger = useMemo(() => {
+  const { ledger, ledgerInitError } = useMemo(() => {
-    setError(null);
+    if (refreshCount > 0 || genesis) {
-    setIsLocked(false);
-    setRefreshLock(false);
-
-    // this trick allows to refresh the ledger on demand
-    // when it is shown as locked and the user has actually
-    // unlocked it, which we can't know.
-    if (refreshLock || genesis) {
       if (!genesis) {
-        return null;
+        return { ledger: null, ledgerInitError: null };
       }
 
       try {
-        return retrieveLedger(genesis);
+        return { ledger: retrieveLedger(genesis, ledgerApp), ledgerInitError: null };
       } catch (error) {
-        setError((error as Error).message);
+        return { ledger: null, ledgerInitError: (error as Error).message };
       }
     }
 
-    return null;
+    return { ledger: null, ledgerInitError: null };
-  }, [genesis, refreshLock]);
+  }, [genesis, ledgerApp, refreshCount]);
 
   useEffect(() => {
+    setError(ledgerInitError);
+  }, [ledgerInitError]);
+
+  useEffect(() => {
+    let isStale = false;
+
     if (!ledger || !genesis) {
+      if (prevLedgerRef.current) {
+        prevLedgerRef.current.disconnect().catch(console.error);
+        prevLedgerRef.current = null;
+      }
+
+      setIsLoading(false);
+      setIsLocked(false);
+      setWarning(null);
       setAddress(null);
       setType(null);
 
-      return;
+      return () => {
+        isStale = true;
+      };
     }
 
+    const runIfCurrent = (fn: () => void): void => {
+      if (!isStale) {
+        fn();
+      }
+    };
+
+    const onAddressError = (e: Error): void => {
+      if (!isStale) {
+        handleGetAddressError(e, genesis);
+      }
+    };
+
     setIsLoading(true);
     setError(null);
     setWarning(null);
 
-    // This is used with a genesisHash only when importing the Ledger account
+    const prevLedger = prevLedgerRef.current;
-    // and when signing with Ledger. In both cases, the genesisHash is known and
-    // will be in this array.
-    const chosenNetwork = chains.find(({ genesisHash }) => genesisHash === genesis as HexString);
 
-    // Just in case, but this shouldn't be triggered
+    prevLedgerRef.current = ledger;
-    assert(chosenNetwork, t('This network is not available, please report an issue to update the known chains'));
-    const currApp = settings.get().ledgerApp;
 
-    if (currApp === 'generic' || currApp === 'migration') {
+    const fetchLedgerAddress = () => {
-      if (isEthereum) {
+      if (isStale) {
-        (ledger as LedgerGeneric).getAddressEcdsa(false, accountIndex, addressOffset)
+        return;
-          .then((res) => {
-            setIsLoading(false);
-            setAddress(`0x${res.address}`);
-            setType('ethereum');
-          }).catch((e: Error) => {
-            handleGetAddressError(e, genesis);
-          });
-      } else {
-        (ledger as LedgerGeneric).getAddress(chosenNetwork.ss58Format, false, accountIndex, addressOffset)
-          .then((res) => {
-            setIsLoading(false);
-            setAddress(res.address);
-            setType('ed25519');
-          }).catch((e: Error) => {
-            handleGetAddressError(e, genesis);
-          });
       }
-    } else if (currApp === 'chainSpecific') {
+
-      (ledger as Ledger).getAddress(false, accountIndex, addressOffset)
+      const chosenNetwork = chains.find(({ genesisHash }) => genesisHash === genesis as HexString);
-        .then((res) => {
+
-          setIsLoading(false);
+      // Use the chain's SS58 prefix when known; fall back to 42 (substrate default).
-          setAddress(res.address);
+      const ss58Prefix = chosenNetwork?.ss58Format ?? 42;
-          setType('ed25519');
+
-        }).catch((e: Error) => {
+      if (ledgerApp === 'generic' || ledgerApp === 'migration') {
-          handleGetAddressError(e, genesis);
+        if (isEthereum) {
-        });
+          (ledger as LedgerGeneric).getAddressEcdsa(false, accountIndex, addressOffset)
+            .then((res) => {
+              runIfCurrent(() => {
+                setIsLoading(false);
+                setIsLocked(false);
+                setAddress(`0x${res.address}`);
+                setType('ethereum');
+              });
+            }).catch(onAddressError);
+        } else {
+          (ledger as LedgerGeneric).getAddress(ss58Prefix, false, accountIndex, addressOffset).then((res) => {
+            runIfCurrent(() => {
+              setIsLoading(false);
+              setIsLocked(false);
+              setAddress(res.address);
+              setType('ed25519');
+            });
+          }).catch(onAddressError);
+        }
+      } else if (ledgerApp === 'chainSpecific') {
+        (ledger as Ledger).getAddress(false, accountIndex, addressOffset)
+          .then((res) => {
+            runIfCurrent(() => {
+              setIsLoading(false);
+              setIsLocked(false);
+              setAddress(res.address);
+              setType('ed25519');
+            });
+          }).catch(onAddressError);
+      }
+    };
+
+    // Disconnect the previous instance before opening the new one (network or ledger app change).
+    if (prevLedger && prevLedger !== ledger) {
+      prevLedger.disconnect().catch(console.error).finally(fetchLedgerAddress);
+    } else {
+      fetchLedgerAddress();
     }
+
+    return () => {
+      isStale = true;
+    };
   // If the dependency array is exhaustive, with t, the translation function, it
   // triggers a useless re-render when ledger device is connected.
   // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [accountIndex, addressOffset, genesis, ledger, isEthereum]);
+  }, [accountIndex, addressOffset, genesis, ledger, ledgerApp, isEthereum]);
+
+  const ledgerRef = useRef(ledger);
+
+  // Keep in sync so the unmount cleanup always closes the latest instance.
+  ledgerRef.current = ledger;
+
+  // On unmount, release the transport so the next session can open it.
+  useEffect(() => {
+    return () => {
+      ledgerRef.current?.disconnect().catch(console.error);
+    };
+  }, []);
 
   const refresh = useCallback(() => {
-    setRefreshLock(true);
+    if (ledger) {
-    setError(null);
+      // Prevent the address-fetch effect from disconnecting this instance again.
-    setWarning(null);
+      prevLedgerRef.current = null;
-  }, []);
+      ledger.disconnect()
+        .catch(console.error)
+        .finally(() => setRefreshCount((c) => c + 1));
+    } else {
+      setRefreshCount((c) => c + 1);
+    }
+  }, [ledger]);
 
   return ({ ...getState(), address, error, isLoading, isLocked, ledger, refresh, type, warning });
 }

packages/extension-ui/src/partials/MenuAdd.tsx

@@ -101,8 +101,9 @@ function MenuAdd ({ className, reference }: Props): React.ReactElement<Props> {
           ? (
             <Link
               isDisabled={!isLedgerCapable}
+              onClick={isPopup ? _onOpenLedgerConnect : undefined}
               title={ (!isLedgerCapable && t('Ledger devices can only be connected with Chrome browser')) || ''}
-              to={ledgerPath}
+              to={isPopup ? undefined : ledgerPath}
             >
               <FontAwesomeIcon
                 icon={faUsb}
@@ -112,7 +113,10 @@ function MenuAdd ({ className, reference }: Props): React.ReactElement<Props> {
             </Link>
           )
           : (
-            <Link onClick={_onOpenLedgerConnect}>
+            <Link
+              onClick={isPopup ? _onOpenLedgerConnect : undefined}
+              to={isPopup ? undefined : ledgerPath}
+            >
               <FontAwesomeIcon
                 icon={faUsb}
                 rotation={270}

packages/extension/public/locales/en/translation.json

@@ -197,5 +197,7 @@
   "You are starting a Parachain": "",
   "For all other operations, your balances are already on Asset Hub.": "",
   "I Understand": "",
-  "Do not teleport balances to the Relay Chain unless:": ""
+  "Do not teleport balances to the Relay Chain unless:": "",
+  "Address mismatch: derived {{derived}}, expected {{expected}}. Check that the correct Ledger device is connected and the correct app is selected.": "",
+  "Possible cause: the Ledger App setting differs from the app originally used to derive this account.": ""
 }

yarn.lock

@@ -1184,7 +1184,7 @@ __metadata:
     "@polkadot/extension-dapp": "npm:0.62.7-1-x"
     "@polkadot/extension-inject": "npm:0.62.7-1-x"
     "@polkadot/extension-mocks": "npm:0.62.7-1-x"
-    "@polkadot/hw-ledger": "npm:^13.5.9"
+    "@polkadot/hw-ledger": "npm:^14.0.3"
     "@polkadot/keyring": "npm:^13.5.9"
     "@polkadot/networks": "npm:^13.5.9"
     "@polkadot/react-identicon": "npm:^3.16.4"
@@ -1253,32 +1253,32 @@ __metadata:
   languageName: unknown
   linkType: soft
 
-"@polkadot/hw-ledger-transports@npm:13.5.9":
+"@polkadot/hw-ledger-transports@npm:14.0.3":
-  version: 13.5.9
+  version: 14.0.3
-  resolution: "@polkadot/hw-ledger-transports@npm:13.5.9"
+  resolution: "@polkadot/hw-ledger-transports@npm:14.0.3"
   dependencies:
     "@ledgerhq/hw-transport": "npm:^6.31.4"
     "@ledgerhq/hw-transport-node-hid-singleton": "npm:^6.31.5"
     "@ledgerhq/hw-transport-webhid": "npm:^6.29.4"
     "@ledgerhq/hw-transport-webusb": "npm:^6.29.4"
-    "@polkadot/util": "npm:13.5.9"
+    "@polkadot/util": "npm:14.0.3"
     tslib: "npm:^2.8.0"
   dependenciesMeta:
     "@ledgerhq/hw-transport-node-hid-singleton":
       optional: true
-  checksum: 10/2ea7ee7873d6a3208342a5c5919c4e1ca6fe78ee014f3442a0bb094243cb28d179cfefe550e81a6b64a26e7af8daaa2b45f2b5615fbd6796c266ff0d57c1076c
+  checksum: 10/5cf48a41079e06ea4c46df4c4aaf2efcca786b19f9eecb0c7c0ba505a1f5a98f730518cff41e3b0fa27f09d6dc74329e2f2b982a2dd8f1c7e59db9d65b90eabe
   languageName: node
   linkType: hard
 
-"@polkadot/hw-ledger@npm:^13.5.9":
+"@polkadot/hw-ledger@npm:^14.0.3":
-  version: 13.5.9
+  version: 14.0.3
-  resolution: "@polkadot/hw-ledger@npm:13.5.9"
+  resolution: "@polkadot/hw-ledger@npm:14.0.3"
   dependencies:
-    "@polkadot/hw-ledger-transports": "npm:13.5.9"
+    "@polkadot/hw-ledger-transports": "npm:14.0.3"
-    "@polkadot/util": "npm:13.5.9"
+    "@polkadot/util": "npm:14.0.3"
     "@zondax/ledger-substrate": "npm:1.1.1"
     tslib: "npm:^2.8.0"
-  checksum: 10/92f598cd8a761e7bce0e6eeb00279ace512dbb34265dab55f23c0ea58ec854c894c64d50765a768d9e30c7bc8825779e33a254f0abb35595c749ed19b7dfe50c
+  checksum: 10/1f4746a3fd7f311211c451ba26ff1cd11bcfc729c27bc9516e619e5df2d57a9e79e94b163d2e00448864680c7afa0d20b933932134bafa709af091db1e6d9b10
   languageName: node
   linkType: hard