Skip to content

Commit

Permalink
[Wallet] Change redux storage implementation to file system (#6273)
Browse files Browse the repository at this point in the history
### Description

AsyncStorage has a 6MB limit on Android. It can be modified, and we should not even be close to 6MB, but when the limit is reached the whole storage is deleted, no backup is used. Given the risk of losing the whole state even if it's unlikely we're changing the implementation to use the file system which has no such limitation.

Also added 2 analytics events:
- One which uploads every app open or once a day (if it's kept in background) the size of the redux store
- One which represents an error in the case that there's no account stored on the redux store but there is one on the geth keychain.

### Other changes

- On one of my tests, I noticed that the size of the Redux store was getting bigger and bigger. Upon closer inspection, it was the history we use for the CELO/cUSD exchange rate that was growing at a rate of ~100kb sometimes when restarting the app. It was not consistent, it happened for around an hour and then completely stopped happening and couldn't reproduce it anymore. In the time where it was happening I managed to see that the starting time to fetch the rates was being sent as a month ago, which is the default. It sounds like some race condition, but we're already waiting for the store to be populated before starting the sagas, so it's unlikely to be that. I changed the implementation to use the value of `lastTimeUpdated`, which is a value that we were storing but not using. Not completely sure this will fix the issue, but it is definitely possible that this was causing the resetting of the accounts that we saw. We'll be able to confirm if this is the issue with the analytics event that uploads the size of the store.
- When testing the case where the redux store doesn't match the geth keychain I got an error when trying to re-import the address. We were checking `e === ErrorMessages.GETH_ACCOUNT_ALREADY_EXISTS` when we needed to compare against `e.message`.

### Tested

Manually

### Related issues

- Part of #5792

### Backwards compatibility

Yes
  • Loading branch information
gnardini authored Dec 21, 2020
1 parent de416da commit 296f608
Show file tree
Hide file tree
Showing 12 changed files with 114 additions and 18 deletions.
3 changes: 3 additions & 0 deletions packages/mobile/__mocks__/redux-persist-fs-storage.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export const DocumentDirectoryPath = jest.fn()

export default () => {}
1 change: 1 addition & 0 deletions packages/mobile/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@
"react-redux": "^7.2.0",
"redux": "^4.0.5",
"redux-persist": "^6.0.0",
"redux-persist-fs-storage": "^1.3.0",
"redux-saga": "^1.1.3",
"reselect": "^4.0.0",
"sleep-promise": "^8.0.1",
Expand Down
6 changes: 6 additions & 0 deletions packages/mobile/src/analytics/Events.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export enum AppEvents {
user_restart = 'user_restart',
fetch_balance = 'fetch_balance',
fetch_balance_error = 'fetch_balance_error',
redux_keychain_mismatch = 'redux_keychain_mismatch',
}

export enum HomeEvents {
Expand Down Expand Up @@ -343,6 +344,10 @@ export enum ContractKitEvents {
init_contractkit_finish = 'init_contractkit_finish',
}

export enum PerformanceEvents {
redux_store_size = 'redux_store_size',
}

export type AnalyticsEventType =
| AppEvents
| HomeEvents
Expand All @@ -360,3 +365,4 @@ export type AnalyticsEventType =
| CeloExchangeEvents
| GethEvents
| NetworkEvents
| PerformanceEvents
13 changes: 12 additions & 1 deletion packages/mobile/src/analytics/Properties.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
InviteEvents,
NetworkEvents,
OnboardingEvents,
PerformanceEvents,
RequestEvents,
SendEvents,
SettingsEvents,
Expand Down Expand Up @@ -55,6 +56,9 @@ interface AppEventsProperties {
dollarBalance?: string
goldBalance?: string
}
[AppEvents.redux_keychain_mismatch]: {
account: string
}
}

interface HomeEventsProperties {
Expand Down Expand Up @@ -779,6 +783,12 @@ interface ContractKitEventsProperties {
[ContractKitEvents.init_contractkit_finish]: undefined
}

interface PerformanceProperties {
[PerformanceEvents.redux_store_size]: {
size: number
}
}

export type AnalyticsPropertiesList = AppEventsProperties &
HomeEventsProperties &
SettingsEventsProperties &
Expand All @@ -796,4 +806,5 @@ export type AnalyticsPropertiesList = AppEventsProperties &
FiatExchangeEventsProperties &
GethEventsProperties &
NetworkEventsProperties &
ContractKitEventsProperties
ContractKitEventsProperties &
PerformanceProperties
6 changes: 4 additions & 2 deletions packages/mobile/src/app/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import ErrorBoundary from 'src/app/ErrorBoundary'
import { isE2EEnv } from 'src/config'
import i18n from 'src/i18n'
import NavigatorWrapper from 'src/navigator/NavigatorWrapper'
import { waitUntilSagasFinishLoading } from 'src/redux/sagas'
import { persistor, store } from 'src/redux/store'
import Logger from 'src/utils/Logger'

Expand Down Expand Up @@ -65,7 +66,7 @@ export class App extends React.Component<Props> {

const url = await Linking.getInitialURL()
if (url) {
this.handleOpenURL({ url })
await this.handleOpenURL({ url })
}

this.logAppLoadTime()
Expand Down Expand Up @@ -93,7 +94,8 @@ export class App extends React.Component<Props> {
Linking.removeEventListener('url', this.handleOpenURL)
}

handleOpenURL = (event: any) => {
handleOpenURL = async (event: any) => {
await waitUntilSagasFinishLoading()
store.dispatch(openDeepLink(event.url))
}

Expand Down
12 changes: 5 additions & 7 deletions packages/mobile/src/firebase/saga.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { exchangeHistorySelector, ExchangeRate, MAX_HISTORY_RETENTION } from 'sr
import { Actions, firebaseAuthorized } from 'src/firebase/actions'
import { initializeAuth, initializeCloudMessaging, setUserLanguage } from 'src/firebase/firebase'
import Logger from 'src/utils/Logger'
import { getRemoteTime } from 'src/utils/time'
import { getAccount } from 'src/web3/saga'
import { currentAccountSelector } from 'src/web3/selectors'

Expand Down Expand Up @@ -68,7 +69,7 @@ export function* watchLanguage() {
yield takeEvery(AppActions.SET_LANGUAGE, syncLanguageSelection)
}

function celoGoldExchangeRateHistoryChannel(latestExchangeRate: ExchangeRate) {
function celoGoldExchangeRateHistoryChannel(lastTimeUpdated: number) {
const errorCallback = (error: Error) => {
Logger.warn(TAG, error.toString())
}
Expand All @@ -86,9 +87,7 @@ function celoGoldExchangeRateHistoryChannel(latestExchangeRate: ExchangeRate) {
}

// timestamp + 1 is used because .startAt is inclusive
const startAt = latestExchangeRate
? latestExchangeRate.timestamp + 1
: now - MAX_HISTORY_RETENTION
const startAt = lastTimeUpdated + 1 || now - MAX_HISTORY_RETENTION

const cancel = () => {
firebase
Expand All @@ -112,12 +111,11 @@ function celoGoldExchangeRateHistoryChannel(latestExchangeRate: ExchangeRate) {
export function* subscribeToCeloGoldExchangeRateHistory() {
yield call(waitForFirebaseAuth)
const history = yield select(exchangeHistorySelector)
const latestExchangeRate = history.celoGoldExchangeRates[history.celoGoldExchangeRates.length - 1]
const chan = yield call(celoGoldExchangeRateHistoryChannel, latestExchangeRate)
const chan = yield call(celoGoldExchangeRateHistoryChannel, history.lastTimeUpdated)
try {
while (true) {
const exchangeRates = yield take(chan)
const now = Date.now()
const now = getRemoteTime()
yield put(updateCeloGoldExchangeRateHistory(exchangeRates, now))
}
} catch (error) {
Expand Down
1 change: 1 addition & 0 deletions packages/mobile/src/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ declare module 'react-native-platform-touchable'
declare module 'react-native-shake'
declare module 'numeral'
declare module 'redux-persist/lib/stateReconciler/autoMergeLevel2'
declare module 'redux-persist-fs-storage'
declare module '@celo/react-native-sms-retriever'
declare module 'futoin-hkdf'
declare module 'react-native-share'
Expand Down
12 changes: 12 additions & 0 deletions packages/mobile/src/redux/sagas.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { sleep } from '@celo/utils/lib/async'
import { AnyAction } from 'redux'
import { call, select, spawn, takeEvery } from 'redux-saga/effects'
import { accountSaga } from 'src/account/saga'
Expand All @@ -22,6 +23,7 @@ import { sendSaga } from 'src/send/saga'
import { sentrySaga } from 'src/sentry/saga'
import { stableTokenSaga } from 'src/stableToken/saga'
import { transactionSaga } from 'src/transactions/saga'
import { checkAccountExistenceSaga } from 'src/utils/accountChecker'
import Logger from 'src/utils/Logger'
import { web3Saga } from 'src/web3/saga'

Expand Down Expand Up @@ -66,6 +68,13 @@ function* loggerSaga() {
})
}

let sagasFinishedLoading = false
export async function waitUntilSagasFinishLoading() {
while (!sagasFinishedLoading) {
await sleep(100)
}
}

export function* rootSaga() {
// Delay all sagas until rehydrate is done
// This prevents them from running with missing state
Expand Down Expand Up @@ -97,4 +106,7 @@ export function* rootSaga() {
yield spawn(inviteSaga)
yield spawn(importSaga)
yield spawn(dappKitSaga)
yield spawn(checkAccountExistenceSaga)

sagasFinishedLoading = true
}
44 changes: 39 additions & 5 deletions packages/mobile/src/redux/store.ts
Original file line number Diff line number Diff line change
@@ -1,24 +1,58 @@
import AsyncStorage from '@react-native-community/async-storage'
import * as Sentry from '@sentry/react-native'
import { applyMiddleware, compose, createStore } from 'redux'
import { createMigrate, persistReducer, persistStore } from 'redux-persist'
import { createMigrate, getStoredState, persistReducer, persistStore } from 'redux-persist'
import FSStorage from 'redux-persist-fs-storage'
import autoMergeLevel2 from 'redux-persist/lib/stateReconciler/autoMergeLevel2'
import createSagaMiddleware from 'redux-saga'
import { PerformanceEvents } from 'src/analytics/Events'
import ValoraAnalytics from 'src/analytics/ValoraAnalytics'
import { migrations } from 'src/redux/migrations'
import rootReducer from 'src/redux/reducers'
import { rootSaga } from 'src/redux/sagas'
import Logger from 'src/utils/Logger'
import { ONE_DAY_IN_MILLIS } from 'src/utils/time'

const timeBetweenStoreSizeEvents = ONE_DAY_IN_MILLIS
let lastEventTime = Date.now()

const persistConfig: any = {
key: 'root',
version: 7, // default is -1, increment as we make migrations
storage: AsyncStorage,
keyPrefix: `reduxStore-`, // the redux-persist default is `persist:` which doesn't work with some file systems.
storage: FSStorage(),
blacklist: ['home', 'geth', 'networkInfo', 'alert', 'fees', 'recipients', 'imports'],
stateReconciler: autoMergeLevel2,
migrate: createMigrate(migrations, { debug: true }),
serialize: (data: any) => {
// We're using this to send the size of the store to analytics while using the default implementation of JSON.stringify.
const stringifiedData = JSON.stringify(data)
// if data._persist or any other key is present the whole state is present (the content of the keys are
// sometimes serialized independently).
if (data._persist && Date.now() > lastEventTime + timeBetweenStoreSizeEvents) {
lastEventTime = Date.now()
ValoraAnalytics.track(PerformanceEvents.redux_store_size, {
size: stringifiedData.length,
})
}
return stringifiedData
},
timeout: null,
}

if (__DEV__) {
persistConfig.timeout = 10000
}
// We used to use AsyncStorage to save the state, but moved to file system storage because of problems with Android
// maximum size limits. To keep backwards compatibility, we first try to read from the file system but if nothing is found
// it means it's an old version so we read the state from AsyncStorage.
persistConfig.getStoredState = (config: any) =>
getStoredState(config)
.then(
(state) =>
state ?? getStoredState({ ...config, storage: AsyncStorage, keyPrefix: 'persist:' })
)
.catch((error) => {
Sentry.captureException(error)
Logger.error('redux/store', 'Failed to retrieve redux state.', error)
})

const persistedReducer = persistReducer(persistConfig, rootReducer)

Expand Down
21 changes: 21 additions & 0 deletions packages/mobile/src/utils/accountChecker.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { UnlockableWallet } from '@celo/wallet-base'
import { call, select } from 'redux-saga/effects'
import { AppEvents } from 'src/analytics/Events'
import ValoraAnalytics from 'src/analytics/ValoraAnalytics'
import { getWallet } from 'src/web3/contracts'
import { currentAccountSelector } from 'src/web3/selectors'

export function* checkAccountExistenceSaga() {
const wallet: UnlockableWallet = yield call(getWallet)
if (!wallet) {
return
}
const gethAccounts: string[] = yield wallet.getAccounts()
const reduxAddress: string = yield select(currentAccountSelector)
if (!reduxAddress && gethAccounts.length > 0) {
// TODO: Try to recover access to the account.
ValoraAnalytics.track(AppEvents.redux_keychain_mismatch, {
account: gethAccounts[0],
})
}
}
4 changes: 2 additions & 2 deletions packages/mobile/src/web3/saga.ts
Original file line number Diff line number Diff line change
Expand Up @@ -220,8 +220,8 @@ export function* assignAccountFromPrivateKey(privateKey: string, mnemonic: strin
yield call([wallet, wallet.addAccount], privateKey, password)
} catch (e) {
if (
e === RpcWalletErrors.AccountAlreadyExists ||
e === ErrorMessages.GETH_ACCOUNT_ALREADY_EXISTS
e.message === RpcWalletErrors.AccountAlreadyExists ||
e.message === ErrorMessages.GETH_ACCOUNT_ALREADY_EXISTS
) {
Logger.warn(TAG + '@assignAccountFromPrivateKey', 'Attempted to import same account')
} else {
Expand Down
9 changes: 8 additions & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -29573,7 +29573,7 @@ react-native-exit-app@^1.1.0:
version "1.0.0"
resolved "git://github.com/kristiansorens/react-native-flag-secure-android#e234251220f5d745eec8ebde3e83d4d369e81a14"

react-native-fs@*, react-native-fs@^2.16.6:
react-native-fs@*, react-native-fs@^2.14.1, react-native-fs@^2.16.6:
version "2.16.6"
resolved "https://registry.yarnpkg.com/react-native-fs/-/react-native-fs-2.16.6.tgz#2901789a43210a35a0ef0a098019bbef3af395fd"
integrity sha512-ZWOooD1AuFoAGY3HS2GY7Qx2LZo4oIg6AK0wbC68detxwvX75R/q9lRqThXNKP6vIo2VHWa0fYUo/SrLw80E8w==
Expand Down Expand Up @@ -30274,6 +30274,13 @@ redux-mock-store@^1.5.3:
dependencies:
lodash.isplainobject "^4.0.6"

redux-persist-fs-storage@^1.3.0:
version "1.3.0"
resolved "https://registry.yarnpkg.com/redux-persist-fs-storage/-/redux-persist-fs-storage-1.3.0.tgz#395d0c4b40985e04c480512ee1c469d6d2a9abd9"
integrity sha512-32FMEF5apIwlm7bnBq6VTmbK7sVVZ0bpTpMt8CUw5MMDXcsSfT4BSM5t6Y15V6KBRL9O11GDKAAV8a3/Hfj2Tg==
dependencies:
react-native-fs "^2.14.1"

redux-persist@^6.0.0:
version "6.0.0"
resolved "https://registry.yarnpkg.com/redux-persist/-/redux-persist-6.0.0.tgz#b4d2972f9859597c130d40d4b146fecdab51b3a8"
Expand Down

0 comments on commit 296f608

Please sign in to comment.