-
Notifications
You must be signed in to change notification settings - Fork 324
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
Fix crash bug by ensuring getTypeByName does not return null #692
base: master
Are you sure you want to change the base?
Conversation
If an unknown coin type is read from user data, it will cause getTypeByName to return null. The currentCurrencyMap will then have an asset with key equal to null, which should never happen. That will later cause a crash in CurrencySwitcher.setCurrency. This fix here is to use the BTC coin type as the default instead of returning null.
Its not reason to return BTC for every unknown coin. It will produce not correct behavior. Should handle maybe with exception or handle null in concrete place. |
An exception will crash the application at startup. It will be unrecoverable for the end-user as the app will be bricked. Defaulting to bitcoin as a fallback is commonly used in the Mycelium codebase where there are no other alternatives.
If preferred we can instead allocate a new CryptoCurrency object with the name that was passed to the function and some default properties. Allowing the application to load is important so the end-user can be given the opportunity to resolve the issue with the problematic account while still retaining access to all their other accounts.
|
I like this idea with new Unknown CryptoCurrency object. |
If an unknown coin type is read from user data, it will cause getTypeByName to return null. The currentCurrencyMap will then have an asset with key equal to null, which should never happen. That will later cause a crash in CurrencySwitcher.setCurrency. The fix here is to allocate a new Unknown CryptoCurrency object instead of returning null.
I have updated the pull request with the discussed changes. Thank you for the review! |
How long do reviewed pull requests usually take to be completed in this project? |
If an unknown coin type is read from user data, it will cause getTypeByName to return null. The currentCurrencyMap will then have an asset with key equal to null, which should never happen. That will later cause a crash in CurrencySwitcher.setCurrency. This fix here is to use the BTC coin type as the default instead of returning null.