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

Add import seed phrase. Closes #1727 #1742

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 4 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 main/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ const ctx = {
toggleOpenAtLogin: () => { throw new Error('never get here') },
isOpenAtLogin: () => { throw new Error('never get here') },
exportSeedPhrase: () => { throw new Error('never get here') },
importSeedPhrase: () => { throw new Error('never get here') },
showUI: () => { throw new Error('never get here') },
isShowingUI: false,
loadWebUIFromDist: serve({
Expand Down
4 changes: 4 additions & 0 deletions main/ipc.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,10 @@ function setupIpcMain (/** @type {Context} */ ctx) {
'station:exportSeedPhrase',
(_events) => ctx.exportSeedPhrase()
)
ipcMain.handle(
'station:importSeedPhrase',
(_events) => ctx.importSeedPhrase()
)
ipcMain.handle(
'station:saveModuleLogsAs',
(_events) => ctx.saveModuleLogsAs()
Expand Down
1 change: 1 addition & 0 deletions main/preload.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ contextBridge.exposeInMainWorld('electron', {
ipcRenderer.invoke('station:toggleOpenAtLogin'),
isOpenAtLogin: () => ipcRenderer.invoke('station:isOpenAtLogin'),
exportSeedPhrase: () => ipcRenderer.invoke('station:exportSeedPhrase'),
importSeedPhrase: () => ipcRenderer.invoke('station:importSeedPhrase'),
saveModuleLogsAs: () => ipcRenderer.invoke('station:saveModuleLogsAs'),
checkForUpdates: () => ipcRenderer.invoke('station:checkForUpdates')
},
Expand Down
13 changes: 13 additions & 0 deletions main/settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,19 @@ async function setup (ctx) {
clipboard.writeText(await wallet.getSeedPhrase())
}
}

ctx.importSeedPhrase = async () => {
const button = showDialogSync({
title: 'Import Seed Phrase',
// eslint-disable-next-line max-len
message: 'The seed phrase is used in order to back up your wallet, or move it to a different machine. Please copy it to your clipboard before proceeding. Please be cautious, as this will overwrite the seed phrase currently used, which will be permanently lost.',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like @patrickwoodhead to review this message.

Do we need to explain what is the seed phrase? I would expect the user to understand that if they decide to import the seed phrase,

I think it's also best to put the call to action as the last sentence in the message.

Should we also tell the user that they may want to export the current seed phrase?

Suggested change
message: 'The seed phrase is used in order to back up your wallet, or move it to a different machine. Please copy it to your clipboard before proceeding. Please be cautious, as this will overwrite the seed phrase currently used, which will be permanently lost.',
message: 'Please be cautious. This action will overwrite the seed phrase currently used, which will be permanently lost. Please copy the seed phrase to import to your clipboard before proceeding.',

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For reference, we're also not explaining what a seed phrase is in the export section.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In your suggestion it says Please copy the seed phrase to import, but I think it needs to be Please copye the seed phrase to your clipboard, to import

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to moving the CTA to the end

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I agree that we need to ask the user to be cautious as the first thing. Also, I'm a bit unclear what the CTA is asking the user to do. Is it asking them to copy their current seed phrase out, or copy their new one into the clipboard so that they can paste it into the input box?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The user needs to have their current seed phrase copied in their clipboard, from which we will then import it

type: 'info',
buttons: ['Cancel', 'Import from Clipboard and Restart']
})
if (button === 1) {
await wallet.setSeedPhrase(clipboard.readText())
juliangruber marked this conversation as resolved.
Show resolved Hide resolved
}
}
}

module.exports = {
Expand Down
1 change: 1 addition & 0 deletions main/typings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ export interface Context {
toggleOpenAtLogin: () => void;
isOpenAtLogin: () => boolean;
exportSeedPhrase: () => void;
importSeedPhrase: () => void;
}

export interface WalletSeed {
Expand Down
15 changes: 11 additions & 4 deletions main/wallet-backend.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ class WalletBackend {
this.transactions = []
this.disableKeytar = disableKeytar
this.onTransactionUpdate = onTransactionUpdate
this.keytarService = 'filecoin-station-wallet-0x'
}

/**
Expand Down Expand Up @@ -104,12 +105,11 @@ class WalletBackend {
* @returns {Promise<WalletSeed>}
*/
async getSeedPhrase () {
const service = 'filecoin-station-wallet-0x'
let seed
if (!this.disableKeytar) {
log.info('Reading the seed phrase from the keychain...')
try {
seed = await keytar.getPassword(service, 'seed')
seed = await keytar.getPassword(this.keytarService, 'seed')
} catch (err) {
throw new Error(
'Cannot read the seed phrase - did the user grant access?',
Expand All @@ -123,10 +123,17 @@ class WalletBackend {
}

seed = ethers.Wallet.createRandom().mnemonic.phrase
await this.setSeedPhrase(seed)
return { seed, isNew: true }
}

/**
* @param {string} seed
*/
async setSeedPhrase (seed) {
if (!this.disableKeytar) {
await keytar.setPassword(service, 'seed', seed)
await keytar.setPassword(this.keytarService, 'seed', seed)
}
return { seed, isNew: true }
}

async fetchBalance () {
Expand Down
14 changes: 13 additions & 1 deletion main/wallet.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
'use strict'

const { app } = require('electron')
const electronLog = require('electron-log')
const assert = require('assert')
const { getDestinationWalletAddress } = require('./station-config')
Expand Down Expand Up @@ -256,6 +257,16 @@ async function getSeedPhrase () {
return seed
}

/**
* @param {string} seed
* @returns {Promise<void>}
*/
async function setSeedPhrase (seed) {
await backend.setSeedPhrase(seed)
app.relaunch()
app.exit(0)
}

/**
* @param {string | ethers.utils.Bytes} message
* @returns {Promise<string>}
Expand All @@ -276,5 +287,6 @@ module.exports = {
signMessage,
transferAllFundsToDestinationWallet,
getTransactionsForUI,
getSeedPhrase
getSeedPhrase,
setSeedPhrase
}
1 change: 1 addition & 0 deletions renderer/src/assets/img/icons/import.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 4 additions & 0 deletions renderer/src/lib/station-config.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,10 @@ export function exportSeedPhrase () {
return window.electron.stationConfig.exportSeedPhrase()
}

export function importSeedPhrase () {
return window.electron.stationConfig.importSeedPhrase()
}

export function saveModuleLogsAs () {
return window.electron.stationConfig.saveModuleLogsAs()
}
Expand Down
18 changes: 17 additions & 1 deletion renderer/src/pages/settings/Settings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import Text from 'src/components/Text'
import {
checkForUpdates,
exportSeedPhrase,
importSeedPhrase,
isOpenAtLogin,
saveModuleLogsAs,
toggleOpenAtLogin
Expand All @@ -13,6 +14,7 @@ import Button from 'src/components/Button'
import UpdateIcon from 'src/assets/img/icons/update.svg?react'
import SaveIcon from 'src/assets/img/icons/save.svg?react'
import ExportIcon from 'src/assets/img/icons/export.svg?react'
import ImportIcon from 'src/assets/img/icons/import.svg?react'

const Settings = () => {
const [isOpenAtLoginChecked, setIsOpenAtLoginChecked] = useState<boolean>()
Expand Down Expand Up @@ -79,7 +81,7 @@ const Settings = () => {
</SettingsGroup>
<SettingsGroup name='Security'>
<SettingsGroupItem
title='Seed phrase'
title='Backup'
description={`Export your seed phrase for safekeeping,
allowing you to recover your cryptocurrency assets if necessary.`}
input={
Expand All @@ -93,6 +95,20 @@ const Settings = () => {
</Button>
}
/>
<SettingsGroupItem
title='Restore'
description={'Import your seed phrase from a previous export.'}
input={
<Button
type='button'
variant='secondary'
icon={<ImportIcon />}
onClick={importSeedPhrase}
>
Import seed phrase
</Button>
}
/>
</SettingsGroup>
</div>
</main>
Expand Down
12 changes: 11 additions & 1 deletion renderer/src/test/settings.test.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
import { act, fireEvent, render, screen, waitFor } from '@testing-library/react'
import { checkForUpdates, exportSeedPhrase, isOpenAtLogin, saveModuleLogsAs } from 'src/lib/station-config'
import {
checkForUpdates,
exportSeedPhrase,
importSeedPhrase,
isOpenAtLogin,
saveModuleLogsAs
} from 'src/lib/station-config'
import Settings from 'src/pages/settings/Settings'
import { describe, expect, test, vi } from 'vitest'

Expand All @@ -9,6 +15,7 @@ const mocks = vi.hoisted(() => {
return {
checkForUpdates: vi.fn(),
exportSeedPhrase: vi.fn(),
importSeedPhrase: vi.fn(),
saveModuleLogsAs: vi.fn()
}
})
Expand All @@ -33,6 +40,7 @@ describe('Settings page', () => {
beforeAll(() => {
vi.mocked(checkForUpdates).mockImplementation(mocks.checkForUpdates)
vi.mocked(exportSeedPhrase).mockImplementation(mocks.exportSeedPhrase)
vi.mocked(importSeedPhrase).mockImplementation(mocks.importSeedPhrase)
vi.mocked(saveModuleLogsAs).mockImplementation(mocks.saveModuleLogsAs)

render(<Settings />)
Expand All @@ -43,12 +51,14 @@ describe('Settings page', () => {
act(() => fireEvent.click(screen.getByText('Save module logs as...')))
act(() => fireEvent.click(screen.getByText('Check for updates')))
act(() => fireEvent.click(screen.getByText('Export seed phrase')))
act(() => fireEvent.click(screen.getByText('Import seed phrase')))
})

await waitFor(() => {
expect(mocks.saveModuleLogsAs).toHaveBeenCalledOnce()
expect(mocks.checkForUpdates).toHaveBeenCalledOnce()
expect(mocks.exportSeedPhrase).toHaveBeenCalledOnce()
expect(mocks.importSeedPhrase).toHaveBeenCalledOnce()
})
})
})
Expand Down
1 change: 1 addition & 0 deletions renderer/src/typings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ declare global {
toggleOpenAtLogin: () => void;
isOpenAtLogin: () => Promise<boolean>;
exportSeedPhrase: () => void;
importSeedPhrase: () => void;
saveModuleLogsAs: () => void;
checkForUpdates: () => void;
};
Expand Down
Loading