From e5efe1507c70d703c17b60dc189650e814eb18d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABtan=20Renaudeau?= Date: Sat, 19 May 2018 09:48:44 +0200 Subject: [PATCH 1/2] remove unused old sync sync happens in BridgeSyncContext now & via the bridge --- src/bridge/BridgeSyncContext.js | 2 + src/components/IsUnlocked.js | 21 ------- src/config/constants.js | 1 - src/renderer/events.js | 104 +------------------------------- 4 files changed, 4 insertions(+), 124 deletions(-) diff --git a/src/bridge/BridgeSyncContext.js b/src/bridge/BridgeSyncContext.js index b6939096..16b21d32 100644 --- a/src/bridge/BridgeSyncContext.js +++ b/src/bridge/BridgeSyncContext.js @@ -157,6 +157,8 @@ class Provider extends Component { setTimeout(syncLoop, 2 * 1000) } + // TODO we might want to call sync straight away when new accounts got added (it will happen every 10s anyway) + api: BridgeSync render() { diff --git a/src/components/IsUnlocked.js b/src/components/IsUnlocked.js index d269520b..9a9c7226 100644 --- a/src/components/IsUnlocked.js +++ b/src/components/IsUnlocked.js @@ -6,7 +6,6 @@ import { connect } from 'react-redux' import { compose } from 'redux' import styled from 'styled-components' import { translate } from 'react-i18next' -import type { Account } from '@ledgerhq/live-common/lib/types' import type { Settings, T } from 'types/common' import IconLockScreen from 'icons/LockScreen' @@ -15,11 +14,9 @@ import { ErrorMessageInput } from 'components/base/Input' import get from 'lodash/get' -import { startSyncAccounts, stopSyncAccounts } from 'renderer/events' import { setEncryptionKey } from 'helpers/db' import { fetchAccounts } from 'actions/accounts' -import { getAccounts } from 'reducers/accounts' import { isLocked, unlock } from 'reducers/application' import Box from 'components/base/Box' @@ -30,7 +27,6 @@ type InputValue = { } type Props = { - accounts: Account[], children: any, fetchAccounts: Function, isLocked: boolean, @@ -44,7 +40,6 @@ type State = { } const mapStateToProps = state => ({ - accounts: getAccounts(state), isLocked: isLocked(state), settings: state.settings, }) @@ -84,22 +79,6 @@ class IsUnlocked extends Component { ...defaultState, } - componentWillMount() { - if (this.props.isLocked) { - stopSyncAccounts() - } - } - - componentWillReceiveProps(nextProps) { - if (this.props.isLocked && !nextProps.isLocked) { - startSyncAccounts(nextProps.accounts) - } - - if (!this.props.isLocked && nextProps.isLocked) { - stopSyncAccounts() - } - } - shouldComponentUpdate(nextProps) { if (nextProps.isLocked) { return true diff --git a/src/config/constants.js b/src/config/constants.js index 3783f597..aab97216 100644 --- a/src/config/constants.js +++ b/src/config/constants.js @@ -1,5 +1,4 @@ export const CHECK_UPDATE_DELAY = 5e3 -export const SYNC_ACCOUNT_DELAY = 3e3 export const MODAL_ADD_ACCOUNT = 'MODAL_ADD_ACCOUNT' export const MODAL_OPERATION_DETAILS = 'MODAL_OPERATION_DETAILS' diff --git a/src/renderer/events.js b/src/renderer/events.js index 9c5445c1..cebd47cc 100644 --- a/src/renderer/events.js +++ b/src/renderer/events.js @@ -5,15 +5,11 @@ import { ipcRenderer } from 'electron' import objectPath from 'object-path' import debug from 'debug' -import type { Account } from '@ledgerhq/live-common/lib/types' -import { CHECK_UPDATE_DELAY, SYNC_ACCOUNT_DELAY } from 'config/constants' +import { CHECK_UPDATE_DELAY } from 'config/constants' -import { getAccounts, getAccountById } from 'reducers/accounts' -import { isLocked } from 'reducers/application' import { setUpdateStatus } from 'reducers/update' -import { updateAccount } from 'actions/accounts' import { addDevice, removeDevice } from 'actions/devices' import i18n from 'renderer/i18n/electron' @@ -24,16 +20,11 @@ const d = { update: debug('lwd:update'), } -const { DISABLED_SYNC, DISABLED_AUTO_SYNC } = process.env - type MsgPayload = { type: string, data: any, } -let syncAccountsInProgress = false -let syncAccountsTimeout - export function sendEvent(channel: string, msgType: string, data: any) { ipcRenderer.send(channel, { type: msgType, @@ -48,68 +39,12 @@ export function sendSyncEvent(channel: string, msgType: string, data: any): any }) } -export default ({ store, locked }: { store: Object, locked: boolean }) => { +export default ({ store }: { store: Object, locked: boolean }) => { const handlers = { dispatch: ({ type, payload }) => store.dispatch({ type, payload }), application: { changeLanguage: lang => i18n.changeLanguage(lang), }, - account: { - sync: { - success: account => { - if (syncAccountsInProgress) { - const state = store.getState() - const existingAccount = getAccountById(state, account.id) - - if (!existingAccount) { - return - } - - const { name, balance, balanceByDay, operations } = existingAccount - - if (account.operations.length > 0) { - d.sync(`Update account - ${name}`) - const updatedAccount = { - ...account, - balance: balance + account.balance, - balanceByDay: Object.keys(balanceByDay).reduce((result, k) => { - result[k] = balanceByDay[k] + (account.balanceByDay[k] || 0) - return result - }, {}), - index: account.index || existingAccount.index, - operations: [...operations, ...account.operations], - } - store.dispatch(updateAccount(updatedAccount)) - } - } - }, - }, - }, - accounts: { - sync: { - start: () => { - if (!syncAccountsInProgress) { - const state = store.getState() - const accounts = getAccounts(state) - const locked = isLocked(state) - - if (!locked && !DISABLED_SYNC) { - startSyncAccounts(accounts) - } - } - }, - stop: stopSyncAccounts, - success: () => { - if (syncAccountsInProgress && !DISABLED_AUTO_SYNC) { - d.sync('Sync accounts - success') - syncAccountsTimeout = setTimeout(() => { - const accounts = getAccounts(store.getState()) - startSyncAccounts(accounts) - }, SYNC_ACCOUNT_DELAY) - } - }, - }, - }, device: { add: device => { d.device('Device - add') @@ -145,47 +80,12 @@ export default ({ store, locked }: { store: Object, locked: boolean }) => { // Start detection when we plug/unplug devices sendEvent('devices', 'listen') - const state = store.getState() - - if (!locked) { - const accounts = getAccounts(state) - - // Start accounts sync only if we have accounts - if (accounts.length > 0 && !DISABLED_SYNC) { - startSyncAccounts(accounts) - } - } - if (__PROD__) { // Start check of eventual updates checkUpdates() } } -export function startSyncAccounts(accounts: Account[]) { - d.sync('Sync accounts - start') - syncAccountsInProgress = true - sendEvent('accounts', 'sync', { - accounts: accounts.map(account => { - const { id, currency, walletPath, addresses, index, operations } = account - return { - id, - currencyId: currency.id, - allAddresses: addresses, - currentIndex: index, - walletPath, - operations, - } - }), - }) -} - -export function stopSyncAccounts() { - d.sync('Sync accounts - stop') - syncAccountsInProgress = false - clearTimeout(syncAccountsTimeout) -} - export function checkUpdates() { d.update('Update - check') setTimeout(() => sendEvent('msg', 'updater.init'), CHECK_UPDATE_DELAY) From df72d04638ac63cd7e0a6ff7c73e9fd3c69cc259 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABtan=20Renaudeau?= Date: Sat, 19 May 2018 10:11:08 +0200 Subject: [PATCH 2/2] Bugfix settings to always be saved even without DB: there were a bug were the currency exchange settings was never saved DB: is a too tricky pattern that we'll drop in the future. the problem with this is that we actually miss cases where a DB: should be triggered ultimately an action should be atomic and each changes it affect should be saved without extra magic for performance purpose we can just === check if things have actually changed and we can do this at many levels in the future, we will try to reorganize things to allow this better, for instance splitting latest countervalues vs historical countervalues so you only save the latest more frequently.. --- src/middlewares/db.js | 7 +++++-- src/reducers/settings.js | 33 +++++++++++++++++---------------- 2 files changed, 22 insertions(+), 18 deletions(-) diff --git a/src/middlewares/db.js b/src/middlewares/db.js index 3ac08297..6cc5e566 100644 --- a/src/middlewares/db.js +++ b/src/middlewares/db.js @@ -3,7 +3,7 @@ import db from 'helpers/db' import { getAccounts } from 'reducers/accounts' -import { settingsExportSelector } from 'reducers/settings' +import { settingsExportSelector, areSettingsLoaded } from 'reducers/settings' import CounterValues from 'helpers/countervalues' export default store => next => action => { @@ -11,8 +11,8 @@ export default store => next => action => { const [, type] = action.type.split(':') store.dispatch({ type, payload: action.payload }) const state = store.getState() - db.set('settings', settingsExportSelector(state)) db.set('accounts', getAccounts(state)) + // ^ TODO ultimately we'll do same for accounts to drop DB: pattern } else { const oldState = store.getState() const res = next(action) @@ -20,6 +20,9 @@ export default store => next => action => { if (oldState.countervalues !== newState.countervalues) { db.set('countervalues', CounterValues.exportSelector(newState)) } + if (areSettingsLoaded(newState) && oldState.settings !== newState.settings) { + db.set('settings', settingsExportSelector(newState)) + } return res } } diff --git a/src/reducers/settings.js b/src/reducers/settings.js index 377cfd36..5ba2d60e 100644 --- a/src/reducers/settings.js +++ b/src/reducers/settings.js @@ -14,6 +14,7 @@ import type { Settings, CurrencySettings } from 'types/common' import type { State } from 'reducers' export type SettingsState = { + loaded: boolean, // is the settings loaded from db (it not we don't save them) hasCompletedOnboarding: boolean, counterValue: string, language: string, @@ -35,21 +36,6 @@ const localeSplit = window.navigator.language.split('-') const language = (localeSplit[0] || 'en').toLowerCase() const region = (localeSplit[1] || 'US').toUpperCase() -const defaultState: SettingsState = { - hasCompletedOnboarding: false, - counterValue: 'USD', - language, - orderAccounts: 'balance|asc', - password: { - isEnabled: false, - value: '', - }, - marketIndicator: 'western', - currenciesSettings: {}, - region, - developerMode: false, -} - const CURRENCY_DEFAULTS_SETTINGS: CurrencySettings = { confirmationsToSpend: 10, minConfirmationsToSpend: 10, // FIXME DROP @@ -65,7 +51,19 @@ const CURRENCY_DEFAULTS_SETTINGS: CurrencySettings = { } const state: SettingsState = { - ...defaultState, + hasCompletedOnboarding: false, + counterValue: 'USD', + language, + orderAccounts: 'balance|asc', + password: { + isEnabled: false, + value: '', + }, + marketIndicator: 'western', + currenciesSettings: {}, + region, + developerMode: false, + loaded: false, } function asCryptoCurrency(c: Currency): ?CryptoCurrency { @@ -107,6 +105,7 @@ const handlers: Object = { FETCH_SETTINGS: (state: SettingsState, { payload: settings }: { payload: Settings }) => ({ ...state, ...settings, + loaded: true, }), } @@ -144,6 +143,8 @@ export const localeSelector = (state: State) => { export const getOrderAccounts = (state: State) => state.settings.orderAccounts +export const areSettingsLoaded = (state: State) => state.settings.loaded + export const currencySettingsSelector = ( state: State, currency: CryptoCurrency,