Skip to content

Commit

Permalink
UX improvements
Browse files Browse the repository at this point in the history
  • Loading branch information
@NullSoldier
NullSoldier committed Oct 7, 2024
1 parent 6006059 commit cf9c9ae
Show file tree
Hide file tree
Showing 3 changed files with 129 additions and 59 deletions.
67 changes: 47 additions & 20 deletions ironfish-cli/src/commands/wallet/multisig/dkg/create.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,11 @@ export class DkgCreateCommand extends IronfishCommand {
const identities = await client.wallet.multisig.getIdentities()

if (ledger) {
const ledgerIdentity = await ui.ledgerAction(ledger, () => ledger.dkgGetIdentity(0))
const ledgerIdentity = await ui.ledger({
ledger,
message: 'Getting Ledger Identity',
action: () => ledger.dkgGetIdentity(0),
})

const foundIdentity = identities.content.identities.find(
(i) => i.identity === ledgerIdentity.toString('hex'),
Expand Down Expand Up @@ -333,9 +337,12 @@ export class DkgCreateCommand extends IronfishCommand {
}

// TODO(hughy): determine how to handle multiple identities using index
const { publicPackage, secretPackage } = await ui.ledgerAction(ledger, () =>
ledger.dkgRound1(0, identities, minSigners),
)
const { publicPackage, secretPackage } = await ui.ledger({
ledger,
message: 'Round1 on Ledger',
approval: true,
action: () => ledger.dkgRound1(0, identities, minSigners),
})

return {
round1: {
Expand Down Expand Up @@ -421,9 +428,12 @@ export class DkgCreateCommand extends IronfishCommand {
round2: { secretPackage: string; publicPackage: string }
}> {
// TODO(hughy): determine how to handle multiple identities using index
const { publicPackage, secretPackage } = await ui.ledgerAction(ledger, () =>
ledger.dkgRound2(0, round1PublicPackages, round1SecretPackage),
)
const { publicPackage, secretPackage } = await ui.ledger({
ledger,
message: 'Round2 on Ledger',
approval: true,
action: () => ledger.dkgRound2(0, round1PublicPackages, round1SecretPackage),
})

return {
round2: {
Expand Down Expand Up @@ -555,21 +565,33 @@ export class DkgCreateCommand extends IronfishCommand {
const round2FrostPackages = round2PublicPackages.map((pkg) => pkg.frostPackage)

// Perform round3 with Ledger
await ui.ledgerAction(ledger, () =>
ledger.dkgRound3(
0,
participants,
round1FrostPackages,
round2FrostPackages,
round2SecretPackage,
gskBytes,
),
)
await ui.ledger({
ledger,
message: 'Round3 on Ledger',
approval: true,
action: () =>
ledger.dkgRound3(
0,
participants,
round1FrostPackages,
round2FrostPackages,
round2SecretPackage,
gskBytes,
),
})

// Retrieve all multisig account keys and publicKeyPackage
const dkgKeys = await ui.ledgerAction(ledger, () => ledger.dkgRetrieveKeys())
const dkgKeys = await ui.ledger({
ledger,
message: 'Getting Ledger DKG keys',
action: () => ledger.dkgRetrieveKeys(),
})

const publicKeyPackage = await ui.ledgerAction(ledger, () => ledger.dkgGetPublicPackage())
const publicKeyPackage = await ui.ledger({
ledger,
message: 'Getting Ledger Public Package',
action: () => ledger.dkgGetPublicPackage(),
})

const accountImport = {
...dkgKeys,
Expand Down Expand Up @@ -597,7 +619,12 @@ export class DkgCreateCommand extends IronfishCommand {
this.log('Creating an encrypted backup of multisig keys from your Ledger device...')
this.log()

const encryptedKeys = await ui.ledgerAction(ledger, () => ledger.dkgBackupKeys())
const encryptedKeys = await ui.ledger({
ledger,
message: 'Backup DKG Keys',
approval: true,
action: () => ledger.dkgBackupKeys(),
})

this.log()
this.log('Encrypted Ledger Multisig Backup:')
Expand Down
39 changes: 29 additions & 10 deletions ironfish-cli/src/ledger/ledger.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at https://mozilla.org/MPL/2.0/. */
import { Assert, createRootLogger, Logger } from '@ironfish/sdk'
import { StatusCodes as LedgerStatusCodes, TransportStatusError } from '@ledgerhq/errors'
import TransportNodeHid from '@ledgerhq/hw-transport-node-hid'
import IronfishApp, {
KeyResponse,
Expand All @@ -26,9 +27,30 @@ export class Ledger {

tryInstruction = async <T>(instruction: (app: IronfishApp) => Promise<T>) => {
try {
await this.refreshConnection()
await this.connect()

Assert.isNotUndefined(this.app, 'Unable to establish connection with Ledger device')

// App info is a request to the dashboard CLA. The purpose of this it to
// produce a Locked Device error and works if an app is open or closed.
await this.app.appInfo()

// This is an app specific request. This is useful because this throws
// INS_NOT_SUPPORTED in the case that the app is locked which is useful to
// know versus the device is locked.
try {
await this.app.getVersion()
} catch (error) {
if (
error instanceof ResponseError &&
error.returnCode === LedgerStatusCodes.INS_NOT_SUPPORTED
) {
throw new LedgerAppLocked()
}

throw error
}

return await instruction(this.app)
} catch (error: unknown) {
if (LedgerPortIsBusyError.IsError(error)) {
Expand All @@ -37,6 +59,10 @@ export class Ledger {
throw new LedgerConnectError()
}

if (error instanceof TransportStatusError) {
throw new LedgerConnectError()
}

if (error instanceof ResponseError) {
if (error.returnCode === LedgerDeviceLockedError.returnCode) {
throw new LedgerDeviceLockedError(error)
Expand Down Expand Up @@ -78,9 +104,6 @@ export class Ledger {

const app = new IronfishApp(transport, this.isMultisig)

// If the app isn't open or the device is locked, this will throw an error.
await app.getVersion()

this.app = app
return { app, PATH: this.PATH }
} catch (e) {
Expand All @@ -94,12 +117,6 @@ export class Ledger {
close = () => {
void this.app?.transport.close()
}

protected refreshConnection = async () => {
if (!this.app) {
await this.connect()
}
}
}

export function isResponseAddress(response: KeyResponse): response is ResponseAddress {
Expand Down Expand Up @@ -156,6 +173,8 @@ export class LedgerDeviceLockedError extends LedgerResponseError {
static returnCode = 0x5515
}

export class LedgerAppLocked extends LedgerError {}

export class LedgerAppNotOpen extends LedgerResponseError {
static returnCodes = [
0x6d00, // Instruction not supported
Expand Down
82 changes: 53 additions & 29 deletions ironfish-cli/src/ui/ledger.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@

import { PromiseUtils } from '@ironfish/sdk'
import { ux } from '@oclif/core'
import inquirer from 'inquirer'
import {
Ledger,
LedgerAppLocked,
LedgerAppNotOpen,
LedgerClaNotSupportedError,
LedgerConnectError,
Expand All @@ -14,57 +16,79 @@ import {
LedgerPortIsBusyError,
} from '../ledger'

export async function ledgerAction<TResult>(
ledger: Ledger,
handler: () => TResult | Promise<TResult>,
action?: string,
): Promise<TResult> {
export async function ledger<TResult>({
ledger,
action,
message = 'Ledger',
approval,
}: {
ledger: Ledger
action: () => TResult | Promise<TResult>
message?: string
approval?: boolean
}): Promise<TResult> {
const wasRunning = ux.action.running
let statusAdded = false
let actionAdded = false

if (approval) {
message = `Approve ${message}`
}

if (!wasRunning) {
ux.action.start(message)
}

try {
// eslint-disable-next-line no-constant-condition
while (true) {
try {
const result = await handler()
const result = await action()
ux.action.stop()
return result
} catch (e) {
let status: string | undefined
if (e instanceof LedgerAppLocked) {
// If an app is running and it's locked, trying to poll the device
// will cause the Ledger device to hide the pin screen as the user
// is trying to enter their pin. When we run into this error, we
// cannot send any commands to the Ledger in the app's CLA.
ux.action.stop('Ledger App Locked')

await inquirer.prompt<{ retry: boolean }>([
{
name: 'retry',
message: `Ledger App Locked. Unlock and press enter to retry:`,
type: 'list',
choices: [
{
name: `Retry`,
value: true,
default: true,
},
],
},
])

if (e instanceof LedgerConnectError) {
status = 'Connect and unlock your Ledger'
if (!wasRunning) {
ux.action.start(message)
}
} else if (e instanceof LedgerConnectError) {
ux.action.status = 'Connect and unlock your Ledger'
} else if (e instanceof LedgerAppNotOpen) {
const appName = ledger.isMultisig ? 'Ironfish DKG' : 'Ironfish'
status = `Unlock your Ledger and open the ${appName}`
ux.action.status = `Open Ledger App ${appName}`
} else if (e instanceof LedgerDeviceLockedError) {
status = 'Unlock your Ledger'
ux.action.status = 'Unlock Ledger'
} else if (e instanceof LedgerPortIsBusyError) {
status = 'Ledger is busy, retrying'
ux.action.status = 'Ledger is busy, retrying'
} else if (e instanceof LedgerGPAuthFailed) {
status = 'Ledger handshake failed, retrying'
ux.action.status = 'Ledger handshake failed, retrying'
} else if (e instanceof LedgerClaNotSupportedError) {
const appName = ledger.isMultisig ? 'Ironfish DKG' : 'Ironfish'
status = `Wrong Ledger app. Please open ${appName}`
ux.action.status = `Wrong Ledger app. Please open ${appName}`
} else {
throw e
}

// Always show our custom action
if (action && !actionAdded) {
ux.action.start(action)
actionAdded = true
}

if (wasRunning || actionAdded) {
// Only update the status if someone else is using the action
ux.action.status = status
} else {
// Use the action if no one else is using it
ux.action.start(status)
}

statusAdded = true
await PromiseUtils.sleep(1000)
continue
Expand Down

0 comments on commit cf9c9ae

Please sign in to comment.