From acdd98217945f4f50e782e145eec41efced9c9ec Mon Sep 17 00:00:00 2001 From: meriadec Date: Wed, 30 May 2018 18:00:37 +0200 Subject: [PATCH] Prevent libcore race conditions --- src/commands/libcoreScanAccounts.js | 30 +++--- src/commands/libcoreSignAndBroadcast.js | 116 ++++++++++++------------ src/helpers/libcore.js | 16 ++-- src/helpers/withLibcore.js | 28 ++++++ src/init-ledger-core.js | 20 ---- 5 files changed, 112 insertions(+), 98 deletions(-) create mode 100644 src/helpers/withLibcore.js delete mode 100644 src/init-ledger-core.js diff --git a/src/commands/libcoreScanAccounts.js b/src/commands/libcoreScanAccounts.js index c20ab6d1..7044220d 100644 --- a/src/commands/libcoreScanAccounts.js +++ b/src/commands/libcoreScanAccounts.js @@ -4,6 +4,7 @@ import type { AccountRaw } from '@ledgerhq/live-common/lib/types' import { createCommand, Command } from 'helpers/ipc' import { Observable } from 'rxjs' import { scanAccountsOnDevice } from 'helpers/libcore' +import withLibcore from 'helpers/withLibcore' type Input = { devicePath: string, @@ -17,19 +18,22 @@ const cmd: Command = createCommand( ({ devicePath, currencyId }) => Observable.create(o => { // TODO scanAccountsOnDevice should directly return a Observable so we just have to pass-in - scanAccountsOnDevice({ - devicePath, - currencyId, - onAccountScanned: account => { - o.next(account) - }, - }).then( - () => { - o.complete() - }, - e => { - o.error(e) - }, + withLibcore(core => + scanAccountsOnDevice({ + core, + devicePath, + currencyId, + onAccountScanned: account => { + o.next(account) + }, + }).then( + () => { + o.complete() + }, + e => { + o.error(e) + }, + ), ) function unsubscribe() { diff --git a/src/commands/libcoreSignAndBroadcast.js b/src/commands/libcoreSignAndBroadcast.js index 5727b9f7..c08c63bd 100644 --- a/src/commands/libcoreSignAndBroadcast.js +++ b/src/commands/libcoreSignAndBroadcast.js @@ -2,11 +2,13 @@ import type { AccountRaw, OperationRaw } from '@ledgerhq/live-common/lib/types' import Btc from '@ledgerhq/hw-app-btc' +import { fromPromise } from 'rxjs/observable/fromPromise' +import { getCryptoCurrencyById } from '@ledgerhq/live-common/lib/helpers/currencies' + +import withLibcore from 'helpers/withLibcore' import { createCommand, Command } from 'helpers/ipc' import { withDevice } from 'helpers/deviceAccess' import { getWalletIdentifier } from 'helpers/libcore' -import { fromPromise } from 'rxjs/observable/fromPromise' -import { getCryptoCurrencyById } from '@ledgerhq/live-common/lib/helpers/currencies' type BitcoinLikeTransaction = { amount: number, @@ -24,70 +26,68 @@ type Result = $Exact const cmd: Command = createCommand( 'libcoreSignAndBroadcast', - ({ account, transaction, deviceId }) => { - // TODO: investigate why importing it on file scope causes trouble - const core = require('init-ledger-core')() - - return fromPromise( - withDevice(deviceId)(async transport => { - const hwApp = new Btc(transport) + ({ account, transaction, deviceId }) => + fromPromise( + withDevice(deviceId)(transport => + withLibcore(async core => { + const hwApp = new Btc(transport) - const WALLET_IDENTIFIER = await getWalletIdentifier({ - hwApp, - isSegwit: !!account.isSegwit, - currencyId: account.currencyId, - devicePath: deviceId, - }) + const WALLET_IDENTIFIER = await getWalletIdentifier({ + hwApp, + isSegwit: !!account.isSegwit, + currencyId: account.currencyId, + devicePath: deviceId, + }) - const njsWallet = await core.getWallet(WALLET_IDENTIFIER) - const njsAccount = await njsWallet.getAccount(account.index) - const bitcoinLikeAccount = njsAccount.asBitcoinLikeAccount() - const njsWalletCurrency = njsWallet.getCurrency() - const amount = core.createAmount(njsWalletCurrency, transaction.amount) - const fees = core.createAmount(njsWalletCurrency, transaction.feePerByte) - const transactionBuilder = bitcoinLikeAccount.buildTransaction() + const njsWallet = await core.getWallet(WALLET_IDENTIFIER) + const njsAccount = await njsWallet.getAccount(account.index) + const bitcoinLikeAccount = njsAccount.asBitcoinLikeAccount() + const njsWalletCurrency = njsWallet.getCurrency() + const amount = core.createAmount(njsWalletCurrency, transaction.amount) + const fees = core.createAmount(njsWalletCurrency, transaction.feePerByte) + const transactionBuilder = bitcoinLikeAccount.buildTransaction() - // TODO: check if is valid address. if not, it will fail silently on invalid + // TODO: check if is valid address. if not, it will fail silently on invalid - transactionBuilder.sendToAddress(amount, transaction.recipient) - // TODO: don't use hardcoded value for sequence (and first also maybe) - transactionBuilder.pickInputs(0, 0xffffff) - transactionBuilder.setFeesPerByte(fees) + transactionBuilder.sendToAddress(amount, transaction.recipient) + // TODO: don't use hardcoded value for sequence (and first also maybe) + transactionBuilder.pickInputs(0, 0xffffff) + transactionBuilder.setFeesPerByte(fees) - const builded = await transactionBuilder.build() - const sigHashType = core.helpers.bytesToHex( - njsWalletCurrency.bitcoinLikeNetworkParameters.SigHash, - ) + const builded = await transactionBuilder.build() + const sigHashType = core.helpers.bytesToHex( + njsWalletCurrency.bitcoinLikeNetworkParameters.SigHash, + ) - const currency = getCryptoCurrencyById(account.currencyId) - const signedTransaction = await core.signTransaction({ - hwApp, - transaction: builded, - sigHashType, - supportsSegwit: currency.supportsSegwit, - isSegwit: account.isSegwit, - }) + const currency = getCryptoCurrencyById(account.currencyId) + const signedTransaction = await core.signTransaction({ + hwApp, + transaction: builded, + sigHashType, + supportsSegwit: currency.supportsSegwit, + isSegwit: account.isSegwit, + }) - const txHash = await njsAccount - .asBitcoinLikeAccount() - .broadcastRawTransaction(signedTransaction) + const txHash = await njsAccount + .asBitcoinLikeAccount() + .broadcastRawTransaction(signedTransaction) - // optimistic operation - return { - id: txHash, - hash: txHash, - type: 'OUT', - value: amount, - blockHash: null, - blockHeight: null, - senders: [account.freshAddress], - recipients: [transaction.recipient], - accountId: account.id, - date: new Date().toISOString(), - } - }), - ) - }, + // optimistic operation + return { + id: txHash, + hash: txHash, + type: 'OUT', + value: amount, + blockHash: null, + blockHeight: null, + senders: [account.freshAddress], + recipients: [transaction.recipient], + accountId: account.id, + date: new Date().toISOString(), + } + }), + ), + ), ) export default cmd diff --git a/src/helpers/libcore.js b/src/helpers/libcore.js index 2418d376..070ae1f4 100644 --- a/src/helpers/libcore.js +++ b/src/helpers/libcore.js @@ -16,6 +16,7 @@ import type { AccountRaw, OperationRaw, OperationType } from '@ledgerhq/live-com import type { NJSAccount, NJSOperation } from '@ledgerhq/ledger-core/src/ledgercore_doc' type Props = { + core: Object, devicePath: string, currencyId: string, onAccountScanned: AccountRaw => void, @@ -24,13 +25,14 @@ type Props = { const { SHOW_LEGACY_NEW_ACCOUNT } = process.env export function scanAccountsOnDevice(props: Props): Promise { - const { devicePath, currencyId, onAccountScanned } = props + const { devicePath, currencyId, onAccountScanned, core } = props const currency = getCryptoCurrencyById(currencyId) return withDevice(devicePath)(async transport => { const hwApp = new Btc(transport) const commonParams = { + core, hwApp, currencyId, onAccountScanned, @@ -78,6 +80,7 @@ export async function getWalletIdentifier({ } async function scanAccountsOnDeviceBySegwit({ + core, hwApp, currencyId, onAccountScanned, @@ -96,12 +99,13 @@ async function scanAccountsOnDeviceBySegwit({ const WALLET_IDENTIFIER = await getWalletIdentifier({ hwApp, isSegwit, currencyId, devicePath }) // retrieve or create the wallet - const wallet = await getOrCreateWallet(WALLET_IDENTIFIER, currencyId, isSegwit) + const wallet = await getOrCreateWallet(core, WALLET_IDENTIFIER, currencyId, isSegwit) const accountsCount = await wallet.getAccountCount() // recursively scan all accounts on device on the given app // new accounts will be created in sqlite, existing ones will be updated const accounts = await scanNextAccount({ + core, wallet, hwApp, currencyId, @@ -119,6 +123,7 @@ async function scanAccountsOnDeviceBySegwit({ async function scanNextAccount(props: { // $FlowFixMe wallet: NJSWallet, + core: Object, hwApp: Object, currencyId: string, accountsCount: number, @@ -129,6 +134,7 @@ async function scanNextAccount(props: { showNewAccount: boolean, }): Promise { const { + core, wallet, hwApp, currencyId, @@ -140,9 +146,6 @@ async function scanNextAccount(props: { showNewAccount, } = props - // TODO: investigate why importing it on file scope causes trouble - const core = require('init-ledger-core')() - console.log(`>> Scanning account ${accountIndex} - isSegwit: ${isSegwit.toString()}`) // eslint-disable-line no-console // create account only if account has not been scanned yet @@ -187,12 +190,11 @@ async function scanNextAccount(props: { } async function getOrCreateWallet( + core: Object, WALLET_IDENTIFIER: string, currencyId: string, isSegwit: boolean, ): NJSWallet { - // TODO: investigate why importing it on file scope causes trouble - const core = require('init-ledger-core')() try { const wallet = await core.getWallet(WALLET_IDENTIFIER) return wallet diff --git a/src/helpers/withLibcore.js b/src/helpers/withLibcore.js new file mode 100644 index 00000000..0fc08212 --- /dev/null +++ b/src/helpers/withLibcore.js @@ -0,0 +1,28 @@ +// @flow + +const core = require('@ledgerhq/ledger-core') + +let instanciated = false +let queue = Promise.resolve() + +// TODO: `core` should be typed +type Job = Object => Promise + +export default function withLibcore(job: Job) { + if (!instanciated) { + core.instanciateWalletPool({ + // sqlite files will be located in the app local data folder + dbPath: process.env.LEDGER_LIVE_SQLITE_PATH, + }) + instanciated = true + } + queue = queue.then(() => { + try { + return job(core) + } catch (e) { + console.log(`withLibCore: Error in job`, e) // eslint-disable-line no-console + return Promise.resolve() + } + }) + return queue +} diff --git a/src/init-ledger-core.js b/src/init-ledger-core.js deleted file mode 100644 index 61e94e88..00000000 --- a/src/init-ledger-core.js +++ /dev/null @@ -1,20 +0,0 @@ -// Yep. That's a singleton. -// -// Electron needs to tell lib ledger core where to store the sqlite files, when -// instanciating wallet pool, but we don't need to do each everytime we -// require ledger-core, only the first time, so, eh. - -const core = require('@ledgerhq/ledger-core') - -let instanciated = false - -module.exports = () => { - if (!instanciated) { - core.instanciateWalletPool({ - // sqlite files will be located in the app local data folder - dbPath: process.env.LEDGER_LIVE_SQLITE_PATH, - }) - instanciated = true - } - return core -}