Skip to content

Commit

Permalink
Improve Ledger reliability (#235)
Browse files Browse the repository at this point in the history
  • Loading branch information
floating authored Sep 10, 2019
1 parent 76d81af commit ff52acf
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 16 deletions.
1 change: 0 additions & 1 deletion app/App/Panel/Main/Signer/Settings/RenameAccount/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ class RenameAccount extends React.Component {
this.observer = this.store.observer(() => {
this.accountId = this.store('selected.current')
this.accountName = this.store(`main.accounts.${this.accountId}.name`)
this.setState({ value: this.accountName })
})
}

Expand Down
40 changes: 26 additions & 14 deletions main/signers/ledger/Ledger/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ class Ledger extends Signer {
this.pause = false
this.coinbase = '0x'
this.handlers = {}
this.inUse = false
this.lastUse = Date.now()
this.network = store('main.connection.network')
this.networkObserver = store.observer(() => {
Expand Down Expand Up @@ -70,10 +69,9 @@ class Ledger extends Signer {
}

async getDevice () {
if (this.inUse) throw new Error('Device is in use')
if (this.pause) throw new Error('Device access is paused')
if (Date.now() - this.lastUse < 300) await this.wait(300)
this.releaseDevice()
this.inUse = true
await this.releaseDevice()
this.pause = true
this.currentDevice = new HID.HID(this.devicePath)
this.currentTransport = new TransportNodeHid(this.currentDevice)
Expand All @@ -87,7 +85,6 @@ class Ledger extends Signer {
delete this.currentDevice
this.lastUse = Date.now()
await this.wait(300)
this.inUse = false
this.pause = false
}

Expand All @@ -102,6 +99,7 @@ class Ledger extends Signer {
async getDeviceAddress (i, cb) {
if (this.pause) return cb(new Error('Device access is paused'))
try {

const { address } = await this.getAddress(this.getPath(i), false, true)
cb(null, address)
} catch (err) {
Expand Down Expand Up @@ -238,8 +236,8 @@ class Ledger extends Signer {

// Standard Methods
async signMessage (index, message, cb) {
if (this.pause) return cb(new Error('Device access is paused'))
try {
if (this.pause) throw new Error('Device access is paused')
const eth = await this.getDevice()
const result = await eth.signPersonalMessage(this.getPath(index), message.replace('0x', ''))
let v = (result['v'] - 27).toString(16)
Expand All @@ -248,13 +246,19 @@ class Ledger extends Signer {
await this.releaseDevice()
this.busyCount = 0
} catch (err) {
if (err.message.startsWith('cannot open device with path')) {
const deviceBusy = (
err.message.startsWith('cannot open device with path') ||
err.message === 'Device access is paused' ||
err.message === 'Invalid channel' ||
err.message === 'DisconnectedDevice'
)
if (deviceBusy) {
clearTimeout(this._signMessage)
if (++this.busyCount > 10) {
if (++this.busyCount > 20) {
this.busyCount = 0
return log.info('>>>>>>> Busy: Limit (10) hit, cannot open device with path, will not try again')
} else {
this._signMessage = setTimeout(() => this.signMessage(message, cb), 700)
this._signMessage = setTimeout(() => this.signMessage(index, message, cb), 700)
return log.info('>>>>>>> Busy: cannot open device with path, will try again (signMessage)')
}
}
Expand All @@ -265,8 +269,8 @@ class Ledger extends Signer {
}

async signTransaction (index, rawTx, cb) {
if (this.pause) return cb(new Error('Device access is paused'))
try {
if (this.pause) throw new Error('Device access is paused')
const eth = await this.getDevice()
if (parseInt(this.network) !== utils.hexToNumber(rawTx.chainId)) throw new Error('Signer signTx network mismatch')
const tx = new EthereumTx(rawTx)
Expand All @@ -290,17 +294,25 @@ class Ledger extends Signer {
cb(null, '0x' + _tx.serialize().toString('hex'))
this.releaseDevice()
} catch (err) {
if (err.message.startsWith('cannot open device with path')) {
const deviceBusy = (
err.message.startsWith('cannot open device with path') ||
err.message === 'Device access is paused' ||
err.message === 'Invalid channel' ||
err.message === 'DisconnectedDevice'
)
if (deviceBusy) {
clearTimeout(this._signTransaction)
if (++this.busyCount > 10) {
if (++this.busyCount > 20) {
this.busyCount = 0
cb(err)
return log.info('>>>>>>> Busy: Limit (10) hit, cannot open device with path, will not try again')
} else {
this._signTransaction = setTimeout(() => this.signTransaction(rawTx, cb), 700)
this._signTransaction = setTimeout(() => this.signTransaction(index, rawTx, cb), 700)
return log.info('>>>>>>> Busy: cannot open device with path, will try again (signTransaction)')
}
} else {
cb(err)
}
cb(err)
this.releaseDevice()
log.error(err)
}
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
"build": "electron-builder",
"release": "electron-builder -c.snap.publish=github",
"postinstall": "electron-builder install-app-deps",
"test": "standard && jest",
"test": "standard && jest --runInBand",
"test:clients": "mocha test/clients/*.test.js",
"local:rpc": "ganache-cli --account=\"0x2d6945dbddb8dcf5492004e6f720f8e971196ff61a61c4be99714ebc71e06c00, 5000000000000000000000\" --account=\"0xaef6a68a47c1628081e4e6df195f5f712ae4eb7da332a6d74dca06ae32a3e7ae,5000\""
},
Expand Down

0 comments on commit ff52acf

Please sign in to comment.