-
Notifications
You must be signed in to change notification settings - Fork 369
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Consider not persisting exchange rate history #6284
Labels
Comments
gnardini
added
wallet
enhancement
an improvement to an existing feature
Priority: P2
Major
Component: CELO
labels
Dec 17, 2020
Upping priority to P1 because we have reason to believe this is bloating redux state to the point that it's triggering app crashes |
mergify bot
pushed a commit
that referenced
this issue
Jan 27, 2021
…on (#6488) ### Description This PR enables Flipper on iOS along with `Redux Debugger` and `Reactotron` plugins. This allows viewing / debugging the following: - React DevTools (Components and Profiling) - Network connections - View hierarchy - Redux State / Actions - AsyncStorage - App preferences - Hermes - and more ;) It also opens the door for creating/adding our own Flipper plugins (for instance for viewing transactions 😉). Setup instructions were added to the mobile README. ### Details When we upgraded to React Native 0.63.x and Reanimated 2, the introduction of TurboModules broke the ability to use remote debugging with the Chrome DevTools, see [this discussion with more details](react-native-community/discussions-and-proposals#206). As an alternative we can use Flipper to provide similar debugging functionalities. Flipper was already working on Android, but on iOS there was a conflict between the OpenSSL lib shipped as part of `react-native-fast-crypto` and the one included by `Flipper`. To workaround this problem, we now use [a "headers" only version of `OpenSSL`](https://github.com/celo-org/OpenSSL-headers) that makes `Flipper` happy while building. The 2nd issue was that Flipper in React Native doesn't work with `use_frameworks!` enabled in the `Podfile`, which we need because we have Swift dependencies, which don't work without it. I bent CocoaPods a little more so that everything Flipper related is built as static frameworks. I've integrated both `Redux Debugger` and `Reactotron` as they provide different lenses to debug the app. There's also a `redux-saga` plugin for `Reactotron` but it triggered some odd unhandled promise rejections, so I didn't include it. While working on this I noticed the state in the `exchange` reducer is very big and made the `Redux Debugger` plugin quite unusable within Flipper. There's an issue open which should address this (#6284). In the meantime I've filtered out that part of the state in `Redux Debugger`. `Reactotron` can still be used to see it and is not affected. Overall this gives us more tools to see what is happening within the app. ### Tested - Flipper works on both iOS and Android along with their `Redux Debugger` and `Reactotron` plugins. - iOS release builds don't include Flipper (Flipper modules are built for release builds, but NOT linked - this makes the build take more time, but unfortunately no simple way around this with CocoaPods for now, or we'd need to manage multiple `Podfile.lock` files, see facebook/flipper#1275). <img width="1554" alt="Screenshot 2021-01-15 at 11 38 52" src="https://user-images.githubusercontent.com/57791/104722566-4bfe4800-572e-11eb-973c-63c7af58b94f.png"> <img width="1554" alt="Screenshot 2021-01-15 at 11 39 19" src="https://user-images.githubusercontent.com/57791/104722570-4f91cf00-572e-11eb-9bb7-4fe0abf4336c.png"> <img width="1554" alt="Screenshot 2021-01-15 at 12 30 47" src="https://user-images.githubusercontent.com/57791/104722693-836cf480-572e-11eb-8ef0-83e54a52dfe8.png"> <img width="1554" alt="Screenshot 2021-01-15 at 11 40 26" src="https://user-images.githubusercontent.com/57791/104722580-5587b000-572e-11eb-886e-8fff2da8cffd.png"> <img width="1554" alt="Screenshot 2021-01-15 at 11 40 48" src="https://user-images.githubusercontent.com/57791/104722594-5a4c6400-572e-11eb-86bd-873add6027b1.png"> <img width="1554" alt="Screenshot 2021-01-15 at 11 47 17" src="https://user-images.githubusercontent.com/57791/104722632-6801e980-572e-11eb-8412-d86a8a29d383.png"> <img width="1554" alt="Screenshot 2021-01-15 at 11 49 12" src="https://user-images.githubusercontent.com/57791/104722649-6df7ca80-572e-11eb-8ac5-bae41abd6488.png"> <img width="1554" alt="Screenshot 2021-01-15 at 11 49 32" src="https://user-images.githubusercontent.com/57791/104722659-7223e800-572e-11eb-9734-59512f3987f9.png"> ### Related issues - Discussed on Slack: https://celo-org.slack.com/archives/CL7BVQPHB/p1610040529072600 ### Backwards compatibility Yes
mergify bot
pushed a commit
to valora-inc/wallet
that referenced
this issue
Apr 20, 2021
### Description We had a bug in the fetching of the exchange rates that made us fetch the whole thing every time there was an update, which is every 30 minutes. The number of elements to fetch each time depended on how much time it passed since the last use of the app, for new users and for users that didn't open the app in a couple months we would load all the history for three months. We were also fetching the whole exchange rate history from Firebase and filtering it in memory instead of only fetching the latest three months. We tried to filter, but we were sending `1` as `startAt` which returns everything. With the current changes, we load everything since the max between the last update and three months ago and then we listen only for new items, we don't reload everything again. ### Other changes - Added a migration to delete the exchange rate history from Redux since it was likely that it was repeated a bunch of times. ### Tested Mostly manually, by making changes and adding and modifying items in the RTDB to see that the behavior was the expected one. ### Related issues - Fixes celo-org/celo-monorepo#6284 ### Backwards compatibility N/A
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Expected Behavior
Exchange rate history is not persisted in the Redux state. See if it's cached at the Firebase lib layer.
Current Behavior
We have a bug in our code where the exchange rate history gets stored multiple times in our redux state.
Quoting from this issue: #6273
One comment by @jeanregisser was:
The text was updated successfully, but these errors were encountered: