From bc320ec7622e8e2e228b91d80e7a9d9a97bc1ecc Mon Sep 17 00:00:00 2001 From: Francis O'Brien <42175565+F-OBrien@users.noreply.github.com> Date: Mon, 30 Mar 2026 21:44:34 +0100 Subject: [PATCH] Allow Ledger generic app accounts to sign on any chain (#1613) * feat(ui): allow generic Ledger app to derive from any chain via polkadot slip44 * feat: allow Ledger generic app accounts to sign on any chain * fix(extension-ui): guard generic ledger init and stabilize i18n deps --- .../extension-base/src/background/types.ts | 2 +- .../src/Popup/Accounts/Account.spec.tsx | 26 +++- .../src/Popup/Accounts/Account.tsx | 11 +- .../extension-ui/src/Popup/ImportLedger.tsx | 54 ++++++--- .../src/Popup/Signing/Request/index.tsx | 15 ++- packages/extension-ui/src/hooks/useLedger.ts | 114 ++++++++++-------- packages/extension-ui/src/messaging.ts | 2 +- 7 files changed, 140 insertions(+), 84 deletions(-) diff --git a/packages/extension-base/src/background/types.ts b/packages/extension-base/src/background/types.ts index dcdff5fb..b707ecf6 100644 --- a/packages/extension-base/src/background/types.ts +++ b/packages/extension-base/src/background/types.ts @@ -201,7 +201,7 @@ export interface RequestAccountCreateHardware { accountIndex: number; address: string; addressOffset: number; - genesisHash: HexString; + genesisHash?: HexString | null; hardwareType: string; name: string; type: KeypairType; diff --git a/packages/extension-ui/src/Popup/Accounts/Account.spec.tsx b/packages/extension-ui/src/Popup/Accounts/Account.spec.tsx index b9252d28..8428b3ae 100644 --- a/packages/extension-ui/src/Popup/Accounts/Account.spec.tsx +++ b/packages/extension-ui/src/Popup/Accounts/Account.spec.tsx @@ -12,6 +12,9 @@ import React from 'react'; import { act } from 'react-dom/test-utils'; import { MemoryRouter } from 'react-router'; +import { settings } from '@polkadot/ui-settings'; + +import { SettingsContext } from '../../components/index.js'; import * as messaging from '../../messaging.js'; import { flushAllPromises } from '../../testHelpers.js'; import Account from './Account.js'; @@ -34,11 +37,13 @@ describe('Account component', () => { let wrapper: ReactWrapper; const VALID_ADDRESS = 'HjoBp62cvsWDA3vtNMWxz6c9q13ReEHi9UGHK7JbZweH5g5'; // eslint-disable-next-line @typescript-eslint/no-unsafe-return - const mountAccountComponent = (additionalAccountProperties: Record): ReactWrapper => mount( + const mountAccountComponent = (additionalAccountProperties: Record, ledgerApp: 'generic' | 'migration' | 'chainSpecific' = 'chainSpecific'): ReactWrapper => mount( - + + + ); it('shows Export option if account is not external', async () => { @@ -79,7 +84,7 @@ describe('Account component', () => { }); it('does not show genesis hash selection dropsown if account is hardware', async () => { - wrapper = mountAccountComponent({ isExternal: true, isHardware: true }); + wrapper = mountAccountComponent({ isExternal: true, isHardware: true }, 'chainSpecific'); wrapper.find('.settings').first().simulate('click'); await act(flushAllPromises); @@ -88,4 +93,15 @@ describe('Account component', () => { expect(wrapper.find('a.menuItem').at(1).text()).toBe('Forget Account'); expect(wrapper.find('.genesisSelection').exists()).toBe(false); }); + + it('shows genesis hash selection for hardware account in generic mode', async () => { + wrapper = mountAccountComponent({ isExternal: true, isHardware: true }, 'generic'); + wrapper.find('.settings').first().simulate('click'); + await act(flushAllPromises); + + expect(wrapper.find('a.menuItem').length).toBe(2); + expect(wrapper.find('a.menuItem').at(0).text()).toBe('Rename'); + expect(wrapper.find('a.menuItem').at(1).text()).toBe('Forget Account'); + expect(wrapper.find('.genesisSelection').exists()).toBe(true); + }); }); diff --git a/packages/extension-ui/src/Popup/Accounts/Account.tsx b/packages/extension-ui/src/Popup/Accounts/Account.tsx index 885d6050..9da09cd9 100644 --- a/packages/extension-ui/src/Popup/Accounts/Account.tsx +++ b/packages/extension-ui/src/Popup/Accounts/Account.tsx @@ -8,7 +8,7 @@ import React, { useCallback, useContext, useEffect, useMemo, useState } from 're import { canDerive } from '@polkadot/extension-base/utils'; -import { AccountContext, Address, Checkbox, Dropdown, Link, MenuDivider } from '../../components/index.js'; +import { AccountContext, Address, Checkbox, Dropdown, Link, MenuDivider, SettingsContext } from '../../components/index.js'; import { useGenesisHashOptions, useTranslation } from '../../hooks/index.js'; import { editAccount, tieAccount } from '../../messaging.js'; import { Name } from '../../partials/index.js'; @@ -33,8 +33,10 @@ function Account ({ address, className, genesisHash, isExternal, isHardware, isH const [editedName, setName] = useState(name); const [checked, setChecked] = useState(false); const genesisOptions = useGenesisHashOptions(); + const { ledgerApp } = useContext(SettingsContext); const { selectedAccounts = [], setSelectedAccounts } = useContext(AccountContext); const isSelected = useMemo(() => selectedAccounts?.includes(address) || false, [address, selectedAccounts]); + const canEditGenesis = !isHardware || ledgerApp === 'generic'; useEffect(() => { setChecked(isSelected); @@ -50,7 +52,7 @@ function Account ({ address, className, genesisHash, isExternal, isHardware, isH const _onChangeGenesis = useCallback( (genesisHash?: HexString | null): void => { - tieAccount(address, genesisHash ?? null) + tieAccount(address, genesisHash || null) .catch(console.error); }, [address] @@ -105,7 +107,7 @@ function Account ({ address, className, genesisHash, isExternal, isHardware, isH > {t('Forget Account')} - {!isHardware && ( + {canEditGenesis && ( <>
@@ -120,7 +122,7 @@ function Account ({ address, className, genesisHash, isExternal, isHardware, isH )} - ), [_onChangeGenesis, _toggleEdit, address, genesisHash, genesisOptions, isExternal, isHardware, t, type]); + ), [_onChangeGenesis, _toggleEdit, address, canEditGenesis, genesisHash, genesisOptions, isExternal, t, type]); return (
@@ -138,6 +140,7 @@ function Account ({ address, className, genesisHash, isExternal, isHardware, isH className='address' genesisHash={genesisHash} isExternal={isExternal} + isHardware={isHardware} isHidden={isHidden} name={editedName} parentName={parentName} diff --git a/packages/extension-ui/src/Popup/ImportLedger.tsx b/packages/extension-ui/src/Popup/ImportLedger.tsx index f5057987..e5887d44 100644 --- a/packages/extension-ui/src/Popup/ImportLedger.tsx +++ b/packages/extension-ui/src/Popup/ImportLedger.tsx @@ -5,11 +5,11 @@ import type { HexString } from '@polkadot/util/types'; import { faSync } from '@fortawesome/free-solid-svg-icons'; import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; -import React, { useCallback, useContext, useEffect, useRef, useState } from 'react'; +import React, { useCallback, useContext, useEffect, useMemo, useRef, useState } from 'react'; import { settings } from '@polkadot/ui-settings'; -import { ActionContext, Address, Button, ButtonArea, Dropdown, Switch, VerticalSpace, Warning } from '../components/index.js'; +import { ActionContext, Address, Button, ButtonArea, Dropdown, SettingsContext, Switch, VerticalSpace, Warning } from '../components/index.js'; import { useLedger, useTranslation } from '../hooks/index.js'; import { createAccountHardware } from '../messaging.js'; import { Header, Name } from '../partials/index.js'; @@ -23,10 +23,12 @@ interface AccOption { interface NetworkOption { text: string; - value: string | null; + value: string; } const AVAIL: number[] = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19]; +const SELECT_NETWORK = ''; +const ALLOW_ANY_NETWORK = '__allow_any_network__'; interface Props { className?: string; @@ -41,8 +43,10 @@ function ImportLedger ({ className }: Props): React.ReactElement { const [genesis, setGenesis] = useState(null); const [isEthereum, setIsEthereum] = useState(false); const onAction = useContext(ActionContext); + const { ledgerApp } = useContext(SettingsContext); const [name, setName] = useState(null); - const isChainSpecific = settings.ledgerApp === 'chainSpecific'; + const isGenericLedgerApp = ledgerApp === 'generic'; + const isChainSpecific = ledgerApp === 'chainSpecific'; const { address, error: ledgerError, isLoading: ledgerLoading, isLocked: ledgerLocked, refresh, type, warning: ledgerWarning } = useLedger(genesis, accountIndex, addressOffset, isEthereum); useEffect(() => { @@ -67,20 +71,32 @@ function ImportLedger ({ className }: Props): React.ReactElement { value }))); - const networkOps = useRef( - [{ - text: t('Select network'), - value: '' - }, - ...ledgerChains.map(({ displayName, genesisHash }): NetworkOption => ({ - text: displayName, - value: genesisHash[0] - }))] + const onNetworkChange = useCallback((value: string) => { + if (value === ALLOW_ANY_NETWORK) { + setGenesis(null); + + return; + } + + setGenesis(value ? value as HexString : null); + }, []); + + const networkOps = useMemo( + () => [ + ...(isGenericLedgerApp + ? [{ text: t('Allow use on any chain'), value: ALLOW_ANY_NETWORK }] + : [{ text: t('Select network'), value: SELECT_NETWORK }]), + ...ledgerChains.map(({ displayName, genesisHash }): NetworkOption => ({ + text: displayName, + value: genesisHash[0] + })) + ], + [isGenericLedgerApp, t] ); const _onSave = useCallback( () => { - if (address && genesis && name && type) { + if (address && name && type) { setIsBusy(true); createAccountHardware(address, 'ledger', accountIndex, addressOffset, name, genesis, type) @@ -127,11 +143,11 @@ function ImportLedger ({ className }: Props): React.ReactElement { - {!!genesis && !!address && !ledgerError && ( + {!!address && !ledgerError && ( {t('Import Account')} diff --git a/packages/extension-ui/src/Popup/Signing/Request/index.tsx b/packages/extension-ui/src/Popup/Signing/Request/index.tsx index 15fe0ba2..df91772e 100644 --- a/packages/extension-ui/src/Popup/Signing/Request/index.tsx +++ b/packages/extension-ui/src/Popup/Signing/Request/index.tsx @@ -43,12 +43,17 @@ function isRawPayload (payload: SignerPayloadJSON | SignerPayloadRaw): payload i return !!(payload as SignerPayloadRaw).data; } -export default function Request ({ account: { accountIndex, addressOffset, genesisHash, isExternal, isHardware, type }, buttonText, isFirst, request, signId, url }: Props): React.ReactElement | null { +export default function Request ({ account: { accountIndex, addressOffset, genesisHash: accountGenesisHash, isExternal, isHardware, type }, buttonText, isFirst, request, signId, url }: Props): React.ReactElement | null { const onAction = useContext(ActionContext); const [{ hexBytes, payload }, setData] = useState({ hexBytes: null, payload: null }); const [error, setError] = useState(null); const { t } = useTranslation(); - const chain = useMetadata(genesisHash); + // Use payload genesis for transaction-signing flow. Account genesis can be null + // for allow-any accounts and should not drive payload decoding/signing setup. + const payloadGenesisHash = !isRawPayload(request.payload) + ? request.payload.genesisHash + : null; + const chain = useMetadata(payloadGenesisHash); useEffect((): void => { // When the chain and request are ready, configure the chain's registry. @@ -153,12 +158,12 @@ export default function Request ({ account: { accountIndex, addressOffset, genes isExternal={isExternal} />
- {isExternal && !isHardware && genesisHash + {isExternal && !isHardware && accountGenesisHash ? ( @@ -171,7 +176,7 @@ export default function Request ({ account: { accountIndex, addressOffset, genes ) } - {isExternal && !isHardware && !genesisHash && ( + {isExternal && !isHardware && !accountGenesisHash && ( <> {t('"Allow use on any network" is not supported to show a QR code. You must associate this account with a network.')} diff --git a/packages/extension-ui/src/hooks/useLedger.ts b/packages/extension-ui/src/hooks/useLedger.ts index 1b3fb1bf..b90f0a0f 100644 --- a/packages/extension-ui/src/hooks/useLedger.ts +++ b/packages/extension-ui/src/hooks/useLedger.ts @@ -56,35 +56,37 @@ function getState (): StateBase { }; } -function retrieveLedger (genesis: string, ledgerApp: string): LedgerGeneric | Ledger { - let ledger: LedgerGeneric | Ledger | null = null; - +function retrieveLedger (genesis: string | null, ledgerApp: string): LedgerGeneric | Ledger { const { isLedgerCapable } = getState(); assert(isLedgerCapable, 'Incompatible browser, only Chrome is supported'); + const transport = getTransportType(); + + if (ledgerApp === 'generic') { + // Generic app always uses Polkadot's slip44, regardless of chain genesis. + return new LedgerGeneric(transport, 'polkadot', knownLedger['polkadot']); + } + + // Shouldn't happen but guard to satisfy the compiler + assert(genesis, 'Genesis hash is required to connect to the Ledger in non-generic mode'); + const def = getNetwork(genesis); assert(def, 'There is no known Ledger app available for this chain'); assert(def.slip44, 'Slip44 is not available for this network, please report an issue to update this chains slip44'); - const transport = getTransportType(); - - 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(transport, def.network, knownLedger['polkadot']); - } else if (ledgerApp === 'migration') { - ledger = new LedgerGeneric(transport, def.network, knownLedger[def.network]); - } else if (ledgerApp === 'chainSpecific') { - 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(transport, def.network, knownLedger['polkadot']); + if (ledgerApp === 'migration') { + return new LedgerGeneric(transport, def.network, knownLedger[def.network]); } - return ledger; + if (ledgerApp === 'chainSpecific') { + return new Ledger(transport, def.network); + } + + // This will never get touched since it will always hit the above two. This satisfies the compiler. + return new LedgerGeneric(transport, 'polkadot', knownLedger['polkadot']); } export default function useLedger (genesis?: string | null, accountIndex = 0, addressOffset = 0, isEthereum = false): State { @@ -97,47 +99,59 @@ export default function useLedger (genesis?: string | null, accountIndex = 0, ad const [type, setType] = useState(null); const { t } = useTranslation(); const { ledgerApp } = useContext(SettingsContext); + const tRef = useRef(t); // 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(null); - const handleGetAddressError = (e: Error, genesis: string) => { + // Keep a stable reference so effects and callbacks don't depend on + // i18n function identity changes. + tRef.current = t; + + const handleGetAddressError = useCallback((e: Error, genesis: string, ledgerApp: string) => { setIsLoading(false); - const { network } = getNetwork(genesis) || { network: 'unknown network' }; + const { network } = getNetwork(genesis) || { network: ledgerApp === 'generic' ? 'Polkadot' : 'unknown network' }; const warningMessage = e.message.includes('Code: 26628') - ? t('Is your ledger locked?') + ? tRef.current('Is your ledger locked?') : null; const errorMessage = e.message.includes('App does not seem to be open') - ? t('App "{{network}}" does not seem to be open', { replace: { network } }) + ? tRef.current('App "{{network}}" does not seem to be open', { replace: { network } }) : e.message; setIsLocked(true); setWarning(warningMessage); - setError(t( + setError(tRef.current( 'Ledger error: {{errorMessage}}', { replace: { errorMessage } } )); console.error(e); setAddress(null); setType(null); - }; + }, []); const { ledger, ledgerInitError } = useMemo(() => { - if (refreshCount > 0 || genesis) { - if (!genesis) { - return { ledger: null, ledgerInitError: null }; - } - - try { - return { ledger: retrieveLedger(genesis, ledgerApp), ledgerInitError: null }; - } catch (error) { - return { ledger: null, ledgerInitError: (error as Error).message }; - } + // undefined means no ledger connection attempt. + // null means connect using the generic app without a specific chain genesis. + // Prevents hook instances that only check isLedgerCapable or isLedgerEnabled + // from attempting a connection when ledgerApp === 'generic'. + if (genesis === undefined) { + return { ledger: null, ledgerInitError: null }; } - return { ledger: null, ledgerInitError: null }; + // Generic app connects without a specific chain genesis (always derives from polkadot). + const canConnect = ledgerApp === 'generic' || refreshCount > 0 || !!genesis; + + if (!canConnect || (ledgerApp !== 'generic' && !genesis)) { + return { ledger: null, ledgerInitError: null }; + } + + try { + return { ledger: retrieveLedger(genesis ?? null, ledgerApp), ledgerInitError: null }; + } catch (error) { + return { ledger: null, ledgerInitError: (error as Error).message }; + } }, [genesis, ledgerApp, refreshCount]); useEffect(() => { @@ -147,7 +161,7 @@ export default function useLedger (genesis?: string | null, accountIndex = 0, ad useEffect(() => { let isStale = false; - if (!ledger || !genesis) { + if (!ledger || (ledgerApp !== 'generic' && !genesis)) { if (prevLedgerRef.current) { prevLedgerRef.current.disconnect().catch(console.error); prevLedgerRef.current = null; @@ -172,7 +186,7 @@ export default function useLedger (genesis?: string | null, accountIndex = 0, ad const onAddressError = (e: Error): void => { if (!isStale) { - handleGetAddressError(e, genesis); + handleGetAddressError(e, genesis ?? '', ledgerApp); } }; @@ -190,10 +204,14 @@ export default function useLedger (genesis?: string | null, accountIndex = 0, ad } const chosenNetwork = chains.find(({ genesisHash }) => genesisHash === genesis as HexString); - // Use the chain's SS58 prefix when known; fall back to 42 (substrate default). const ss58Prefix = chosenNetwork?.ss58Format ?? 42; + if (ledgerApp === 'migration') { + // Migration app is only expected on known, mapped chains. + assert(chosenNetwork, tRef.current('This network is not available, please report an issue to update the known chains')); + } + if (ledgerApp === 'generic' || ledgerApp === 'migration') { if (isEthereum) { (ledger as LedgerGeneric).getAddressEcdsa(false, accountIndex, addressOffset) @@ -206,14 +224,15 @@ export default function useLedger (genesis?: string | null, accountIndex = 0, ad }); }).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); + (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) @@ -238,10 +257,7 @@ export default function useLedger (genesis?: string | null, accountIndex = 0, ad 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, ledgerApp, isEthereum]); + }, [accountIndex, addressOffset, genesis, handleGetAddressError, ledger, ledgerApp, isEthereum]); const ledgerRef = useRef(ledger); diff --git a/packages/extension-ui/src/messaging.ts b/packages/extension-ui/src/messaging.ts index 28715f61..238e24ba 100644 --- a/packages/extension-ui/src/messaging.ts +++ b/packages/extension-ui/src/messaging.ts @@ -143,7 +143,7 @@ export async function createAccountExternal (name: string, address: string, gene return sendMessage('pri(accounts.create.external)', { address, genesisHash, name }); } -export async function createAccountHardware (address: string, hardwareType: string, accountIndex: number, addressOffset: number, name: string, genesisHash: HexString, type: KeypairType): Promise { +export async function createAccountHardware (address: string, hardwareType: string, accountIndex: number, addressOffset: number, name: string, genesisHash: HexString | null, type: KeypairType): Promise { return sendMessage('pri(accounts.create.hardware)', { accountIndex, address, addressOffset, genesisHash, hardwareType, name, type }); }