-
Notifications
You must be signed in to change notification settings - Fork 53
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
Investigate Issues regards erroneous Invalid PIN #2317
Comments
Asked internal team members and no one was able to replicate. |
I added additional information on what the sequence of events are before the error message appears. |
Just a note for posterity: there are two separate, possibly related scenarios here. One, as Kim described in the steps to reproduce, where the user is being taken through onboarding again after they had already created a PIN and wallet. And two, as the video shows, where the user sees the login screen as usual, and then gets the error after entering their PIN there. |
I'm still not done investigating this issue. But further questions:
The reason I have these questions is that I have found two possible culprit sections of code so far. The first being that if they initially completed onboarding before we had onboarding versions (a few releases back), and then jump updated directly to 1.0.21 there is a bug where they would be sent through onboarding again. This is not likely the case, at least for the majority of these users, as they all got this update very soon after it was available - the reports started coming in almost the day of the release. The second bit of code I came across that may be the cause is the exclusion of the .afj folder (where the askar wallet is stored) from backup. If the user had the .afj folder backed up before this release, either from a different device or a previous installation, then got this release, and then had iCloud sync run, I wonder if during the sync it could have overwritten the .afj folder that is no longer being backed up from their current device with the latest release. This would be kind of convoluted bug but I would have to defer to someone more familiar with the backup exclusion code to know if it's actually possible this is the case. |
Additional question: Did these users recently turn off or on biometrics from the settings? Reason for this question, this issue seems to only affect iOS users and there was some iOS specific changes to the UseBiometry screen, but for any of those changes to have this impact, that screen would have to have been used. Also just adding, the fact that it's seemingly an iOS issue also puts more emphasis on the backup exclusion theory above as that code only affects iOS. |
One other thing I have been assuming - hard closing the app and reopening never fixes this issue. |
Here are the suspect changes that may or may not be the cause of this issue, some of which I have created fixes for: Splash screen onboarding initialization effect early returns: this is a new change in the latest release, and there was a bug here where if a user first setup the app before we had versioned onboarding it could cause onboarding to restart, which would result in the user creating a PIN again but without biometrics, so even if they recreated the same PIN it would not unlock their existing wallet. I already reverted this change in this PR which has been merged: .afj folder exclusion: this is another new change in the latest release. The bug has only occurred on iOS and this exclusion only applies to iOS devices. All of the storage items related to Askar go in the .afj folder, so if a user does iCloud sync and replaces their .afj folder with an older backup or from a different device they also had BC Wallet on, it could cause their keys to no longer unlock their wallet. I also came across some articles mentioning that the flag we are using to exclude this folder doesn't always work as expected, eg. it will reset the flag if a change is made: https://medium.com/@giulio.caggegi/excluding-files-from-icloud-backups-in-ios-78f3c64f26da UseBiometry permission check: this is another new change in the latest release. I have read through this code many times looking for potential causes of the Askar wallet key issue, and have found none. The change is also isolated to the UseBiometry screen and existing users won't see this screen unless they navigate to settings and intentionally change their biometrics. It is likely the cause of an unrelated SauceLabs issue but that's another matter. STATE_LOADED being dispatched before STATE_DISPATCH: I haven't been able to make this happen after trying many different interactions and scenarios, but if it did ever happen, it would explain onboarding being incorrectly restarted via only having the initial state when stateLoaded becomes true. This is unlikely to be the cause as it seems it was already in place before the latest release but maybe it is contributing to one of the other potential causes. Here is a PR to prevent even the possibility of this happening by combining them into one dispatch: New PersistentStorage: another new change in the latest release is this wrapper around AsyncStorage. I've noticed that the calls to store data are all done with the class itself, whereas the calls to load data are all done with an instance of the class. Requires more investigation but affects storage and state loading, so worth taking a closer look at in my opinion. I have taken a look at it and haven't found any fault, but it might be good for the creator of this change to check it as well. |
Support data
Steps to reproduce
NOTE: tried to enter dev mode and enable remote troubleshooting when the user was at the "is this app for you" screen but remote troubleshooting was disabled and could not be enabled.
Recording
Pin_error_recording.mp4
The text was updated successfully, but these errors were encountered: