From d19546b08f70f02b4d668d06cea0f935ea2f6867 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABtan=20Renaudeau?= Date: Fri, 27 Jul 2018 14:49:40 +0200 Subject: [PATCH] Optimize a bit libcoreSignAndBroadcast and libcoreSyncAccount perf --- src/bridge/LibcoreBridge.js | 92 ++++++++++--------- src/commands/libcoreSignAndBroadcast.js | 51 +++++++--- src/commands/libcoreSyncAccount.js | 9 +- src/components/EnsureDeviceApp.js | 4 +- .../Receive/steps/04-step-receive-funds.js | 4 +- src/helpers/bip32.js | 8 -- src/helpers/libcore.js | 32 +++++-- src/logger/logger.js | 2 +- 8 files changed, 120 insertions(+), 82 deletions(-) diff --git a/src/bridge/LibcoreBridge.js b/src/bridge/LibcoreBridge.js index db25372c..40413e7c 100644 --- a/src/bridge/LibcoreBridge.js +++ b/src/bridge/LibcoreBridge.js @@ -120,41 +120,48 @@ const LibcoreBridge: WalletBridge = { }, synchronize: account => - libcoreSyncAccount.send({ rawAccount: encodeAccount(account) }).pipe( - map(rawSyncedAccount => { - const syncedAccount = decodeAccount(rawSyncedAccount) - return account => { - const accountOps = account.operations - const syncedOps = syncedAccount.operations - const patch: $Shape = { - id: syncedAccount.id, - freshAddress: syncedAccount.freshAddress, - freshAddressPath: syncedAccount.freshAddressPath, - balance: syncedAccount.balance, - blockHeight: syncedAccount.blockHeight, - lastSyncDate: new Date(), - } - - const hasChanged = - accountOps.length !== syncedOps.length || // size change, we do a full refresh for now... - (accountOps.length > 0 && - syncedOps.length > 0 && - (accountOps[0].accountId !== syncedOps[0].accountId || - accountOps[0].id !== syncedOps[0].id || // if same size, only check if the last item has changed. - accountOps[0].blockHeight !== syncedOps[0].blockHeight)) - - if (hasChanged) { - patch.operations = syncedAccount.operations - patch.pendingOperations = [] // For now, we assume a change will clean the pendings. - } - - return { - ...account, - ...patch, + libcoreSyncAccount + .send({ + accountId: account.id, + freshAddressPath: account.freshAddressPath, + index: account.index, + currencyId: account.currency.id, + }) + .pipe( + map(rawSyncedAccount => { + const syncedAccount = decodeAccount(rawSyncedAccount) + return account => { + const accountOps = account.operations + const syncedOps = syncedAccount.operations + const patch: $Shape = { + id: syncedAccount.id, + freshAddress: syncedAccount.freshAddress, + freshAddressPath: syncedAccount.freshAddressPath, + balance: syncedAccount.balance, + blockHeight: syncedAccount.blockHeight, + lastSyncDate: new Date(), + } + + const hasChanged = + accountOps.length !== syncedOps.length || // size change, we do a full refresh for now... + (accountOps.length > 0 && + syncedOps.length > 0 && + (accountOps[0].accountId !== syncedOps[0].accountId || + accountOps[0].id !== syncedOps[0].id || // if same size, only check if the last item has changed. + accountOps[0].blockHeight !== syncedOps[0].blockHeight)) + + if (hasChanged) { + patch.operations = syncedAccount.operations + patch.pendingOperations = [] // For now, we assume a change will clean the pendings. + } + + return { + ...account, + ...patch, + } } - } - }), - ), + }), + ), pullMoreOperations: () => Promise.reject(notImplemented), @@ -201,11 +208,15 @@ const LibcoreBridge: WalletBridge = { .catch(() => BigNumber(0)) .then(totalFees => a.balance.minus(totalFees || 0)), - signAndBroadcast: (account, transaction, deviceId) => { - const encodedAccount = encodeAccount(account) // FIXME no need to send the whole account over the threads - return libcoreSignAndBroadcast + signAndBroadcast: (account, transaction, deviceId) => + libcoreSignAndBroadcast .send({ - account: encodedAccount, + accountId: account.id, + currencyId: account.currency.id, + xpub: account.xpub, + freshAddress: account.freshAddress, + freshAddressPath: account.freshAddressPath, + index: account.index, transaction: serializeTransaction(transaction), deviceId, }) @@ -215,14 +226,13 @@ const LibcoreBridge: WalletBridge = { case 'broadcasted': return { type: 'broadcasted', - operation: decodeOperation(encodedAccount, e.operation), + operation: decodeOperation(encodeAccount(account), e.operation), } default: return e } }), - ) - }, + ), addPendingOperation: (account, operation) => ({ ...account, diff --git a/src/commands/libcoreSignAndBroadcast.js b/src/commands/libcoreSignAndBroadcast.js index 8da3d7b7..e479c9f3 100644 --- a/src/commands/libcoreSignAndBroadcast.js +++ b/src/commands/libcoreSignAndBroadcast.js @@ -2,11 +2,11 @@ import logger from 'logger' import { BigNumber } from 'bignumber.js' -import type { AccountRaw, OperationRaw } from '@ledgerhq/live-common/lib/types' +import type { OperationRaw } from '@ledgerhq/live-common/lib/types' import Btc from '@ledgerhq/hw-app-btc' import { Observable } from 'rxjs' import { getCryptoCurrencyById } from '@ledgerhq/live-common/lib/helpers/currencies' -import { isSegwitAccount } from 'helpers/bip32' +import { isSegwitPath } from 'helpers/bip32' import { libcoreAmountToBigNumber, bigNumberToLibcoreAmount } from 'helpers/libcore' import withLibcore from 'helpers/withLibcore' @@ -21,7 +21,12 @@ type BitcoinLikeTransaction = { } type Input = { - account: AccountRaw, // FIXME there is no reason we send the whole AccountRaw + accountId: string, + currencyId: string, + xpub: string, + freshAddress: string, + freshAddressPath: string, + index: number, transaction: BitcoinLikeTransaction, deviceId: string, } @@ -32,13 +37,18 @@ type Result = { type: 'signed' } | { type: 'broadcasted', operation: OperationRa const cmd: Command = createCommand( 'libcoreSignAndBroadcast', - ({ account, transaction, deviceId }) => + ({ accountId, currencyId, xpub, freshAddress, freshAddressPath, index, transaction, deviceId }) => Observable.create(o => { let unsubscribed = false const isCancelled = () => unsubscribed withLibcore(core => doSignAndBroadcast({ - account, + accountId, + currencyId, + xpub, + freshAddress, + freshAddressPath, + index, transaction, deviceId, core, @@ -151,7 +161,12 @@ async function signTransaction({ } export async function doSignAndBroadcast({ - account, + accountId, + currencyId, + xpub, + freshAddress, + freshAddressPath, + index, transaction, deviceId, core, @@ -159,7 +174,12 @@ export async function doSignAndBroadcast({ onSigned, onOperationBroadcasted, }: { - account: AccountRaw, + accountId: string, + currencyId: string, + xpub: string, + freshAddress: string, + freshAddressPath: string, + index: number, transaction: BitcoinLikeTransaction, deviceId: string, core: *, @@ -167,10 +187,10 @@ export async function doSignAndBroadcast({ onSigned: () => void, onOperationBroadcasted: (optimisticOp: $Exact) => void, }): Promise { - const { walletName } = accountIdHelper.decode(account.id) + const { walletName } = accountIdHelper.decode(accountId) const njsWallet = await core.getPoolInstance().getWallet(walletName) if (isCancelled()) return - const njsAccount = await njsWallet.getAccount(account.index) + const njsAccount = await njsWallet.getAccount(index) if (isCancelled()) return const bitcoinLikeAccount = njsAccount.asBitcoinLikeAccount() const njsWalletCurrency = njsWallet.getCurrency() @@ -194,16 +214,16 @@ export async function doSignAndBroadcast({ const hasTimestamp = !!njsWalletCurrency.bitcoinLikeNetworkParameters.UsesTimestampedTransaction // TODO: const timestampDelay = njsWalletCurrency.bitcoinLikeNetworkParameters.TimestampDelay - const currency = getCryptoCurrencyById(account.currencyId) + const currency = getCryptoCurrencyById(currencyId) const signedTransaction = await withDevice(deviceId)(async transport => signTransaction({ hwApp: new Btc(transport), - currencyId: account.currencyId, + currencyId, transaction: builded, sigHashType: parseInt(sigHashType, 16), supportsSegwit: !!currency.supportsSegwit, - isSegwit: isSegwitAccount(account), + isSegwit: isSegwitPath(freshAddressPath), hasTimestamp, }), ) @@ -221,7 +241,7 @@ export async function doSignAndBroadcast({ // NB we don't check isCancelled() because the broadcast is not cancellable now! onOperationBroadcasted({ - id: `${account.xpub}-${txHash}-OUT`, + id: `${xpub}-${txHash}-OUT`, hash: txHash, type: 'OUT', value: BigNumber(transaction.amount) @@ -230,9 +250,10 @@ export async function doSignAndBroadcast({ fee: fee.toString(), blockHash: null, blockHeight: null, - senders: [account.freshAddress], + // FIXME for senders and recipients, can we ask the libcore? + senders: [freshAddress], recipients: [transaction.recipient], - accountId: account.id, + accountId, date: new Date().toISOString(), }) } diff --git a/src/commands/libcoreSyncAccount.js b/src/commands/libcoreSyncAccount.js index 380e8151..bbc94644 100644 --- a/src/commands/libcoreSyncAccount.js +++ b/src/commands/libcoreSyncAccount.js @@ -8,13 +8,16 @@ import { syncAccount } from 'helpers/libcore' import withLibcore from 'helpers/withLibcore' type Input = { - rawAccount: AccountRaw, // FIXME there is no reason we send the whole AccountRaw + accountId: string, + freshAddressPath: string, + currencyId: string, + index: number, } type Result = AccountRaw -const cmd: Command = createCommand('libcoreSyncAccount', ({ rawAccount }) => - fromPromise(withLibcore(core => syncAccount({ rawAccount, core }))), +const cmd: Command = createCommand('libcoreSyncAccount', accountInfos => + fromPromise(withLibcore(core => syncAccount({ ...accountInfos, core }))), ) export default cmd diff --git a/src/components/EnsureDeviceApp.js b/src/components/EnsureDeviceApp.js index 26dc46a7..4748a0a9 100644 --- a/src/components/EnsureDeviceApp.js +++ b/src/components/EnsureDeviceApp.js @@ -11,7 +11,7 @@ import logger from 'logger' import getAddress from 'commands/getAddress' import { createCancelablePolling } from 'helpers/promise' import { standardDerivation } from 'helpers/derivations' -import { isSegwitAccount } from 'helpers/bip32' +import { isSegwitPath } from 'helpers/bip32' import { BtcUnmatchedApp } from 'helpers/getAddressForCurrency/btc' import DeviceInteraction from 'components/DeviceInteraction' @@ -124,7 +124,7 @@ async function getAddressFromAccountOrCurrency(device, account, currency) { path: account ? account.freshAddressPath : standardDerivation({ currency, segwit: false, x: 0 }), - segwit: account ? isSegwitAccount(account) : false, + segwit: account ? isSegwitPath(account.freshAddressPath) : false, }) .toPromise() return address diff --git a/src/components/modals/Receive/steps/04-step-receive-funds.js b/src/components/modals/Receive/steps/04-step-receive-funds.js index 1b090509..5faff059 100644 --- a/src/components/modals/Receive/steps/04-step-receive-funds.js +++ b/src/components/modals/Receive/steps/04-step-receive-funds.js @@ -5,7 +5,7 @@ import React, { PureComponent } from 'react' import TrackPage from 'analytics/TrackPage' import getAddress from 'commands/getAddress' -import { isSegwitAccount } from 'helpers/bip32' +import { isSegwitPath } from 'helpers/bip32' import Box from 'components/base/Box' import CurrentAddressForAccount from 'components/CurrentAddressForAccount' import { DisconnectedDevice, WrongDeviceForAccount } from 'config/errors' @@ -29,7 +29,7 @@ export default class StepReceiveFunds extends PureComponent { currencyId: account.currency.id, devicePath: device.path, path: account.freshAddressPath, - segwit: isSegwitAccount(account), + segwit: isSegwitPath(account.freshAddressPath), verify: true, } const { address } = await getAddress.send(params).toPromise() diff --git a/src/helpers/bip32.js b/src/helpers/bip32.js index 665f4422..decfdc44 100644 --- a/src/helpers/bip32.js +++ b/src/helpers/bip32.js @@ -1,16 +1,11 @@ // @flow -import type { Account, AccountRaw } from '@ledgerhq/live-common/lib/types' - type SplitConfig = { coinType: number, } export const isSegwitPath = (path: string): boolean => path.startsWith("49'") -export const isSegwitAccount = (account: Account | AccountRaw): boolean => - isSegwitPath(account.freshAddressPath) - export const isUnsplitPath = (path: string, splitConfig: SplitConfig) => { try { const coinType = parseInt(path.split('/')[1], 10) @@ -19,6 +14,3 @@ export const isUnsplitPath = (path: string, splitConfig: SplitConfig) => { return false } } - -export const isUnsplitAccount = (account: Account | AccountRaw, splitConfig: ?SplitConfig) => - !!splitConfig && isUnsplitPath(account.freshAddressPath, splitConfig) diff --git a/src/helpers/libcore.js b/src/helpers/libcore.js index 216ba02a..a9ed673d 100644 --- a/src/helpers/libcore.js +++ b/src/helpers/libcore.js @@ -12,7 +12,7 @@ import { SHOW_LEGACY_NEW_ACCOUNT } from 'config/constants' import type { AccountRaw, OperationRaw, OperationType } from '@ledgerhq/live-common/lib/types' import type { NJSAccount, NJSOperation } from '@ledgerhq/ledger-core/src/ledgercore_doc' -import { isSegwitAccount, isUnsplitAccount } from 'helpers/bip32' +import { isSegwitPath, isUnsplitPath } from 'helpers/bip32' import * as accountIdHelper from 'helpers/accountId' import { createCustomErrorClass, deserializeError } from './errors' import { getAccountPlaceholderName, getNewAccountPlaceholderName } from './accountName' @@ -482,10 +482,22 @@ function buildOperationRaw({ } } -export async function syncAccount({ rawAccount, core }: { core: *, rawAccount: AccountRaw }) { - const decodedAccountId = accountIdHelper.decode(rawAccount.id) - const isSegwit = isSegwitAccount(rawAccount) - const isUnsplit = isUnsplitAccount(rawAccount, SPLITTED_CURRENCIES[rawAccount.currencyId]) +export async function syncAccount({ + accountId, + freshAddressPath, + currencyId, + index, + core, +}: { + core: *, + accountId: string, + freshAddressPath: string, + currencyId: string, + index: number, +}) { + const decodedAccountId = accountIdHelper.decode(accountId) + const isSegwit = isSegwitPath(freshAddressPath) + const isUnsplit = isUnsplitPath(freshAddressPath, SPLITTED_CURRENCIES[currencyId]) let njsWallet try { njsWallet = await core.getPoolInstance().getWallet(decodedAccountId.walletName) @@ -494,7 +506,7 @@ export async function syncAccount({ rawAccount, core }: { core: *, rawAccount: A njsWallet = await getOrCreateWallet( core, decodedAccountId.walletName, - rawAccount.currencyId, + currencyId, isSegwit, isUnsplit, ) @@ -502,10 +514,10 @@ export async function syncAccount({ rawAccount, core }: { core: *, rawAccount: A let njsAccount try { - njsAccount = await njsWallet.getAccount(rawAccount.index) + njsAccount = await njsWallet.getAccount(index) } catch (e) { logger.warn(`Have to recreate the account... (${e.message})`) - const extendedInfos = await njsWallet.getExtendedKeyAccountCreationInfo(rawAccount.index) + const extendedInfos = await njsWallet.getExtendedKeyAccountCreationInfo(index) extendedInfos.extendedKeys.push(decodedAccountId.xpub) njsAccount = await njsWallet.newAccountWithExtendedKeyInfo(extendedInfos) } @@ -521,9 +533,9 @@ export async function syncAccount({ rawAccount, core }: { core: *, rawAccount: A njsAccount, isSegwit, isUnsplit, - accountIndex: rawAccount.index, + accountIndex: index, wallet: njsWallet, - currencyId: rawAccount.currencyId, + currencyId, core, ops, }) diff --git a/src/logger/logger.js b/src/logger/logger.js index 13fcdee1..d6768c54 100644 --- a/src/logger/logger.js +++ b/src/logger/logger.js @@ -146,7 +146,7 @@ const logNetwork = !__DEV__ || DEBUG_NETWORK const logAnalytics = !__DEV__ || DEBUG_ANALYTICS const logApdu = !__DEV__ || DEBUG_DEVICE -const blacklistTooVerboseCommandInput = ['libcoreSyncAccount', 'libcoreSignAndBroadcast'] +const blacklistTooVerboseCommandInput = [] const blacklistTooVerboseCommandResponse = [ 'libcoreSyncAccount', 'libcoreScanAccounts',