Skip to content

Commit

Permalink
feat(extension,cardano,core): LW-9808 revamp hardware wallet onboardi…
Browse files Browse the repository at this point in the history
…ng (#1020)

* feat(extension,cardano,core): revamp hardware wallet connection in onboarding

* feat(extension,cardano,staking): bump hardware-ledger, web-extension and wallet SDK packages

* feat(extension): changes after self review

* feat(extension): improve error cases

* feat(extension): improve error handling in creation and reorganize the code

* feat(cardano): allow only supported ledger devices

* test(extension): fix unit tests of useWalletManager hook

* refactor(extension): use enums insted of string unions

* feat(extension): update hw walet creation generic error message

* feat(extension,cardano,core): add unit tests, restrict allowed HW devices

* refactor(extension): remove redundant eslint disable comment

* fix(extension): fix typo

---------

Co-authored-by: Emir Hodzic <[email protected]>
  • Loading branch information
szymonmaslowski and emiride authored Apr 15, 2024
1 parent 19f1506 commit d7d5505
Show file tree
Hide file tree
Showing 41 changed files with 1,023 additions and 687 deletions.
4 changes: 2 additions & 2 deletions apps/browser-extension-wallet/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@
"@cardano-sdk/input-selection": "0.12.26",
"@cardano-sdk/tx-construction": "0.18.2",
"@cardano-sdk/util": "0.15.0",
"@cardano-sdk/wallet": "0.35.2",
"@cardano-sdk/web-extension": "0.26.1",
"@cardano-sdk/wallet": "0.36.0",
"@cardano-sdk/web-extension": "0.26.2",
"@emurgo/cip14-js": "~3.0.1",
"@koralabs/handles-public-api-interfaces": "^1.6.6",
"@lace/cardano": "0.1.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import SpyInstance = jest.SpyInstance;
const mockEmip3decrypt = jest.fn();
const mockEmip3encrypt = jest.fn();
const mockConnectDevice = jest.fn();
const mockGetHwExtendedAccountPublicKey = jest.fn();
const mockRestoreWalletFromKeyAgent = jest.fn();
const mockSwitchKeyAgents = jest.fn();
const mockLedgerCheckDeviceConnection = jest.fn();
Expand Down Expand Up @@ -80,6 +81,7 @@ jest.mock('@lace/cardano', () => {
restoreWalletFromKeyAgent: mockRestoreWalletFromKeyAgent,
switchKeyAgents: mockSwitchKeyAgents,
connectDevice: mockConnectDevice,
getHwExtendedAccountPublicKey: mockGetHwExtendedAccountPublicKey,
KeyManagement: {
...actual.Wallet.KeyManagement,
emip3decrypt: mockEmip3decrypt,
Expand Down Expand Up @@ -382,7 +384,7 @@ describe('Testing useWalletManager hook', () => {
describe('createHardwareWallet', () => {
test('should use cardano manager to create wallet', async () => {
const walletId = 'walletId';
mockLedgerGetXpub.mockResolvedValue('pubkey');
mockGetHwExtendedAccountPublicKey.mockResolvedValue('pubkey');
(walletApiUi.walletRepository as any).addWallet = jest.fn().mockResolvedValue(walletId);
(walletApiUi.walletRepository as any).addAccount = jest.fn().mockResolvedValue(undefined);
(walletApiUi.walletManager as any).activate = jest.fn().mockResolvedValue(undefined);
Expand Down Expand Up @@ -696,11 +698,11 @@ describe('Testing useWalletManager hook', () => {
},
{
type: WalletType.Trezor,
prepare: () => mockTrezorGetXpub.mockResolvedValueOnce(extendedAccountPublicKey)
prepare: () => mockGetHwExtendedAccountPublicKey.mockResolvedValueOnce(extendedAccountPublicKey)
},
{
type: WalletType.Ledger,
prepare: () => mockLedgerGetXpub.mockResolvedValueOnce(extendedAccountPublicKey)
prepare: () => mockGetHwExtendedAccountPublicKey.mockResolvedValueOnce(extendedAccountPublicKey)
}
];

Expand Down
96 changes: 51 additions & 45 deletions apps/browser-extension-wallet/src/hooks/useWalletManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,6 @@ export interface CreateWallet {
chainId?: Wallet.Cardano.ChainId;
}

export interface SetWallet {
walletInstance: Wallet.CardanoWallet;
chainName?: Wallet.ChainName;
mnemonicVerificationFrequency?: string;
}

export interface CreateHardwareWallet {
accountIndex?: number;
name: string;
Expand All @@ -66,6 +60,14 @@ type WalletManagerAddAccountProps = {

type ActivateWalletProps = Omit<WalletManagerActivateProps, 'chainId'>;

type CreateHardwareWalletRevampedParams = {
accountIndex: number;
name: string;
connection: Wallet.HardwareWalletConnection;
};

type CreateHardwareWalletRevamped = (params: CreateHardwareWalletRevampedParams) => Promise<Wallet.CardanoWallet>;

export interface UseWalletManager {
walletManager: WalletManagerApi;
walletRepository: WalletRepositoryApi<Wallet.WalletMetadata, Wallet.AccountMetadata>;
Expand All @@ -78,7 +80,9 @@ export interface UseWalletManager {
createWallet: (args: CreateWallet) => Promise<Wallet.CardanoWallet>;
activateWallet: (args: Omit<WalletManagerActivateProps, 'chainId'>) => Promise<void>;
createHardwareWallet: (args: CreateHardwareWallet) => Promise<Wallet.CardanoWallet>;
createHardwareWalletRevamped: CreateHardwareWalletRevamped;
connectHardwareWallet: (model: Wallet.HardwareWallets) => Promise<Wallet.DeviceConnection>;
connectHardwareWalletRevamped: typeof connectHardwareWalletRevamped;
saveHardwareWallet: (wallet: Wallet.CardanoWallet, chainName?: Wallet.ChainName) => Promise<void>;
/**
* @returns active wallet id after deleting the wallet; undefined if deleted the last wallet
Expand All @@ -95,27 +99,6 @@ const clearBytes = (bytes: Uint8Array) => {
}
};

const getHwExtendedAccountPublicKey = async (
walletType: Wallet.HardwareWallets,
accountIndex: number,
deviceConnection?: Wallet.DeviceConnection
) => {
switch (walletType) {
case WalletType.Ledger:
await Wallet.Ledger.LedgerKeyAgent.checkDeviceConnection(Wallet.KeyManagement.CommunicationType.Web);
return Wallet.Ledger.LedgerKeyAgent.getXpub({
communicationType: Wallet.KeyManagement.CommunicationType.Web,
deviceConnection: typeof deviceConnection !== 'boolean' ? deviceConnection : undefined,
accountIndex
});
case WalletType.Trezor:
return Wallet.Trezor.TrezorKeyAgent.getXpub({
communicationType: Wallet.KeyManagement.CommunicationType.Web,
accountIndex
});
}
};

const getExtendedAccountPublicKey = async (
wallet: AnyBip32Wallet<Wallet.WalletMetadata, Wallet.AccountMetadata>,
accountIndex: number,
Expand Down Expand Up @@ -143,7 +126,7 @@ const getExtendedAccountPublicKey = async (
}
case WalletType.Ledger:
case WalletType.Trezor:
return getHwExtendedAccountPublicKey(wallet.type, accountIndex);
return Wallet.getHwExtendedAccountPublicKey(wallet.type, accountIndex);
}
};

Expand Down Expand Up @@ -210,6 +193,9 @@ const encryptMnemonic = async (mnemonic: string[], passphrase: Uint8Array) => {
export const connectHardwareWallet = async (model: Wallet.HardwareWallets): Promise<Wallet.DeviceConnection> =>
await Wallet.connectDevice(model);

const connectHardwareWalletRevamped = async (usbDevice: USBDevice): Promise<Wallet.HardwareWalletConnection> =>
Wallet.connectDeviceRevamped(usbDevice);

export const useWalletManager = (): UseWalletManager => {
const {
walletLock,
Expand Down Expand Up @@ -241,25 +227,21 @@ export const useWalletManager = (): UseWalletManager => {
return (storedChain?.chainName && chainIdFromName(storedChain.chainName)) || DEFAULT_CHAIN_ID;
}, [currentChain]);

/**
* Creates a Ledger or Trezor hardware wallet
* and saves it in browser storage with the data to lock/unlock it
*/
const createHardwareWallet = useCallback(
async ({
accountIndex = 0,
deviceConnection,
name,
connectedDevice
}: CreateHardwareWallet): Promise<Wallet.CardanoWallet> => {
const extendedAccountPublicKey = await getHwExtendedAccountPublicKey(
connectedDevice,
accountIndex,
deviceConnection
);
const createHardwareWalletRevamped = useCallback<CreateHardwareWalletRevamped>(
async ({ accountIndex, connection, name }) => {
let extendedAccountPublicKey;
try {
extendedAccountPublicKey = await Wallet.getHwExtendedAccountPublicKey(
connection.type,
accountIndex,
connection.type === WalletType.Ledger ? connection.value : undefined
);
} catch (error: unknown) {
throw error;
}
const addWalletProps: AddWalletProps<Wallet.WalletMetadata, Wallet.AccountMetadata> = {
metadata: { name, lastActiveAccountIndex: accountIndex },
type: connectedDevice,
type: connection.type,
accounts: [
{
extendedAccountPublicKey,
Expand Down Expand Up @@ -291,6 +273,28 @@ export const useWalletManager = (): UseWalletManager => {
[getCurrentChainId]
);

/**
* Creates a Ledger or Trezor hardware wallet
* and saves it in browser storage with the data to lock/unlock it
*/
const createHardwareWallet = useCallback(
async ({
accountIndex = 0,
deviceConnection,
name,
connectedDevice
}: CreateHardwareWallet): Promise<Wallet.CardanoWallet> =>
createHardwareWalletRevamped({
accountIndex,
connection: {
type: connectedDevice,
value: typeof deviceConnection !== 'boolean' ? deviceConnection : undefined
},
name
}),
[createHardwareWalletRevamped]
);

const tryMigrateToWalletRepository = useCallback(async (): Promise<
AnyWallet<Wallet.WalletMetadata, Wallet.AccountMetadata>[] | undefined
> => {
Expand Down Expand Up @@ -742,7 +746,9 @@ export const useWalletManager = (): UseWalletManager => {
loadWallet,
createWallet,
createHardwareWallet,
createHardwareWalletRevamped,
connectHardwareWallet,
connectHardwareWalletRevamped,
saveHardwareWallet,
deleteWallet,
switchNetwork,
Expand Down
34 changes: 24 additions & 10 deletions apps/browser-extension-wallet/src/lib/translations/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -494,9 +494,6 @@
"browserView.walletSetup.mnemonicResetModal.content": "In order to keep you safe, we'll show you a new set of 24 words.",
"browserView.walletSetup.mnemonicResetModal.cancel": "Cancel",
"browserView.walletSetup.mnemonicResetModal.confirm": "OK",
"browserView.walletSetup.confirmExperimentalHwDapp.header": "Limited support for Dapp connection",
"browserView.walletSetup.confirmExperimentalHwDapp.content": "This current version does not support signing transactions through the Dapp connection feature with hardware wallets. Please stay tuned for upcoming releases and new features through @lace on Twitter.",
"browserView.walletSetup.confirmExperimentalHwDapp.confirm": "OK",
"browserView.crypto.emptyDashboard.welcome": "Welcome",
"browserView.crypto.emptyDashboard.addSomeFundsYoStartYourJourney": "Add some funds to start your journey",
"browserView.crypto.emptyDashboard.useThisAddressOrScanTheQRCodeToTransferFunds": "Use this address or scan the QR code to transfer funds",
Expand Down Expand Up @@ -647,13 +644,11 @@
"browserView.staking.details.noFundsModal.buttons.confirm": "Add funds",
"browserView.staking.details.errors.utxoFullyDepleted": "UTxO has been fully depleted",
"browserView.staking.details.errors.utxoBalanceInsufficient": "Balance Insufficient",
"browserView.onboarding.commonError.title": "Oops! Something went wrong",
"browserView.onboarding.commonError.description": "Please check your hardware device.",
"browserView.onboarding.commonError.ok": "OK",
"browserView.onboarding.notDetectedError.title": "Failed to detect device",
"browserView.onboarding.notDetectedError.description": "Please make sure your device is unlocked and the Cardano app is open.",
"browserView.onboarding.notDetectedError.agree": "Agree",
"browserView.onboarding.notDetectedError.trezorDescription": "Please make sure your device is unlocked.",
"browserView.onboarding.errorDialog.title": "Opps! Something went wrong",
"browserView.onboarding.errorDialog.cta": "OK",
"browserView.onboarding.errorDialog.messageDeviceDisconnected": "Please check your hardware device connection and for Ledger, if Cardano App is open",
"browserView.onboarding.errorDialog.messagePublicKeyExportRejected": "Public key export unsuccessful. User declined action on hardware wallet device.",
"browserView.onboarding.errorDialog.messageGeneric": "Try connecting you device again",
"browserView.onboarding.startOver.title": "Are you sure you want to start again?",
"browserView.onboarding.startOver.description": "Connection to this device will be cancelled and you will need to re-connect.",
"browserView.onboarding.startOver.cancel": "Cancel",
Expand Down Expand Up @@ -1263,6 +1258,18 @@
"core.walletSetupConnectHardwareWalletStep.supportedDevices": "Lace is supporting Ledger Nano X, Nano S and Nano S Plus",
"core.walletSetupConnectHardwareWalletStep.connectDevice": "Just connect your device to your computer, unlock and open the Cardano app to hit continue.",
"core.walletSetupConnectHardwareWalletStep.connectDeviceFull": "Just connect your device to your computer and unlock it to continue. If you're using a Ledger device, make sure you open the Cardano App.",
"core.walletSetupConnectHardwareWalletStepRevamp.title": "Connect your device",
"core.walletSetupConnectHardwareWalletStepRevamp.subTitleLedgerOnly": "Lace supports Ledger Nano X, Nano S, Nano S Plus. Unlock your device and open the Cardano app.",
"core.walletSetupConnectHardwareWalletStepRevamp.subTitle": "Lace supports Ledger Nano X, Nano S, Nano S Plus, Trezor Model T. Unlock your device and for Ledger, open the Cardano app.",
"core.walletSetupConnectHardwareWalletStepRevamp.errorMessage.userGestureRequired": "It appears the page reload interrupted the search for connected hardware wallet devices.",
"core.walletSetupConnectHardwareWalletStepRevamp.errorMessage.devicePickerRejected": "No hardware wallet device was chosen.",
"core.walletSetupConnectHardwareWalletStepRevamp.errorMessage.deviceLocked": "Your hardware wallet device seems to be locked. Please unlock it to proceed.",
"core.walletSetupConnectHardwareWalletStepRevamp.errorMessage.deviceBusy": "Your hardware wallet device seems to be busy with some other App. Please ensure it is free to connect.",
"core.walletSetupConnectHardwareWalletStepRevamp.errorMessage.cardanoAppNotOpen": "Cardano App is not open on your hardware wallet device. Please open it to proceed.",
"core.walletSetupConnectHardwareWalletStepRevamp.errorMessage.generic": "Something went wrong. Please try again.",
"core.walletSetupConnectHardwareWalletStepRevamp.errorCta": "Try again",
"core.walletSetupCreateStep.title": "Creating your wallet",
"core.walletSetupCreateStep.description": "Confirm exporting your public key on your hardware wallet device to create your Lace wallet.",
"core.walletSetupRestoreStep.title": "Restoring your wallet",
"core.walletSetupMnemonicStep.writePassphrase": "Write down your secret passphrase",
"core.walletSetupMnemonicStep.enterPassphrase": "Enter your secret passphrase",
Expand Down Expand Up @@ -1392,6 +1399,13 @@
"multiWallet.confirmationDialog.description": "You'll have to start over.",
"multiWallet.confirmationDialog.cancel": "Go back",
"multiWallet.confirmationDialog.confirm": "Proceed",
"multiWallet.errorDialog.commonError.title": "Oops! Something went wrong",
"multiWallet.errorDialog.commonError.description": "Please check your hardware device.",
"multiWallet.errorDialog.commonError.ok": "OK",
"multiWallet.errorDialog.notDetectedError.title": "Failed to detect device",
"multiWallet.errorDialog.notDetectedError.description": "Please make sure your device is unlocked and the Cardano app is open.",
"multiWallet.errorDialog.notDetectedError.agree": "Agree",
"multiWallet.errorDialog.notDetectedError.trezorDescription": "Please make sure your device is unlocked.",
"multiWallet.activated.wallet": "Wallet \"{{walletName}}\" activated",
"multiWallet.activated.account": "Account \"{{accountName}}\" activated",
"multiWallet.popupHwAccountEnable": "Hardware wallets require the <0>expanded view</0> to enable accounts",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export const SetupHardwareWallet = ({ shouldShowDialog$ }: ConfirmationDialog):
const { t } = useTranslation();
const { connectHardwareWallet, createHardwareWallet, walletRepository } = useWalletManager();
const analytics = useAnalyticsContext();
const disconnectHardwareWallet$ = useMemo(() => new Subject<HIDConnectionEvent>(), []);
const disconnectHardwareWallet$ = useMemo(() => new Subject<USBConnectionEvent>(), []);

const hardwareWalletProviders = useMemo(
(): Providers => ({
Expand Down Expand Up @@ -83,14 +83,14 @@ export const SetupHardwareWallet = ({ shouldShowDialog$ }: ConfirmationDialog):
);

useEffect(() => {
const onHardwareWalletDisconnect = (event: HIDConnectionEvent) => {
const onHardwareWalletDisconnect = (event: USBConnectionEvent) => {
disconnectHardwareWallet$.next(event);
};

navigator.hid.addEventListener('disconnect', onHardwareWalletDisconnect);
navigator.usb.addEventListener('disconnect', onHardwareWalletDisconnect);

return () => {
navigator.hid.removeEventListener('disconnect', onHardwareWalletDisconnect);
navigator.usb.removeEventListener('disconnect', onHardwareWalletDisconnect);
disconnectHardwareWallet$.complete();
};
}, [disconnectHardwareWallet$]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,15 +59,15 @@ describe('Multi Wallet Setup/Hardware Wallet', () => {
let providers = {} as {
connectHardwareWallet: jest.Mock;
createWallet: jest.Mock;
disconnectHardwareWallet$: Subject<HIDConnectionEvent>;
disconnectHardwareWallet$: Subject<USBConnectionEvent>;
shouldShowDialog$: Subject<boolean>;
};

beforeEach(() => {
providers = {
connectHardwareWallet: jest.fn(),
createWallet: jest.fn(),
disconnectHardwareWallet$: new Subject<HIDConnectionEvent>(),
disconnectHardwareWallet$: new Subject<USBConnectionEvent>(),
shouldShowDialog$: new Subject()
};
});
Expand Down Expand Up @@ -101,7 +101,7 @@ describe('Multi Wallet Setup/Hardware Wallet', () => {
await selectAccountStep();

act(() => {
providers.disconnectHardwareWallet$.next({ device: { opened: true } } as HIDConnectionEvent);
providers.disconnectHardwareWallet$.next({ device: { opened: true } } as USBConnectionEvent);
});

await waitFor(() => expect(screen.queryByText('Oops! Something went wrong')).toBeInTheDocument());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ interface State {
setAccount: (account: number) => void;
resetConnection: () => void;
createWallet: () => Promise<void>;
disconnectHardwareWallet$: Observable<HIDConnectionEvent>;
disconnectHardwareWallet$: Observable<USBConnectionEvent>;
}

// eslint-disable-next-line unicorn/no-null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { WalletSetupConnectHardwareWalletStep } from '@lace/core';
import React, { useState } from 'react';
import { useTranslation } from 'react-i18next';
import { useHistory } from 'react-router';
import { isTrezorHWSupported } from '../../../wallet-setup/helpers';
import { Wallet } from '@lace/cardano';
import { useHardwareWallet } from '../context';
import { walletRoutePaths } from '@routes';
Expand All @@ -11,6 +10,8 @@ import { WalletType } from '@cardano-sdk/web-extension';
import { useAnalyticsContext } from '@providers';
import { PostHogAction } from '@lace/common';

export const isTrezorHWSupported = (): boolean => process.env.USE_TREZOR_HW === 'true';

interface State {
error?: 'notDetectedLedger' | 'notDetectedTrezor';
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,29 @@
import React, { useEffect, useState } from 'react';
import { useHistory } from 'react-router';
import { useHardwareWallet } from '../context';
import { ErrorDialog } from '../../../wallet-setup/components/ErrorDialog';
import { makeErrorDialog } from '../../../wallet-setup/components/HardwareWalletFlow';
import { walletRoutePaths } from '@routes';

type Errors = 'notDetectedLedger' | 'notDetectedTrezor' | 'common';

const ErrorDialog = makeErrorDialog<Errors>({
common: {
title: 'multiWallet.errorDialog.commonError.title',
description: 'multiWallet.errorDialog.commonError.description',
confirm: 'multiWallet.errorDialog.commonError.ok'
},
notDetectedLedger: {
title: 'multiWallet.errorDialog.notDetectedError.title',
description: 'multiWallet.errorDialog.notDetectedError.description',
confirm: 'multiWallet.errorDialog.notDetectedError.agree'
},
notDetectedTrezor: {
title: 'multiWallet.errorDialog.notDetectedError.title',
description: 'multiWallet.errorDialog.notDetectedError.trezorDescription',
confirm: 'multiWallet.errorDialog.notDetectedError.agree'
}
});

interface State {
error?: Errors;
}
Expand All @@ -21,7 +39,7 @@ export const ErrorHandling = ({ error, onRetry }: Props): JSX.Element => {
const [state, setState] = useState<State>({});

useEffect(() => {
const subscription = disconnectHardwareWallet$.subscribe((event: HIDConnectionEvent) => {
const subscription = disconnectHardwareWallet$.subscribe((event: USBConnectionEvent) => {
if (event.device.opened) {
setState({ error: 'common' });
}
Expand Down
Loading

0 comments on commit d7d5505

Please sign in to comment.