Skip to content
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

Open
cvarjao opened this issue Dec 11, 2024 · 7 comments
Open

Investigate Issues regards erroneous Invalid PIN #2317

cvarjao opened this issue Dec 11, 2024 · 7 comments
Assignees
Labels
bug Something isn't working

Comments

@cvarjao
Copy link
Member

cvarjao commented Dec 11, 2024

Support data

Timestamp Android/iOS OS Version App Version New Phone? Forgot PIN? Error?
Dec 13, 2024 12:10 iOS iPhone 13 iOS 18.2 1.0.21 No No, uses face ID AskarWallet: Incorrect key for wallet 'walletid'
Dec 10, 2024, 3:15 PM iOS No AskarWallet: Incorrect key for wallet 'walletid'
Dec 10, 2024, 2:51 PM
Dec 10, 2024, 11:16 AM iOS iPhone 14 18.11 1.0.21 No No
Dec 09, 2024, 12:58 PM No
Dec 05, 2024, 12:50 PM iOS iPhone 14 Pro Max 1.0.21 No AskarWallet: Incorrect key for wallet 'walletid'
Dec 03, 2024 2:28 PM Yes
Dec 03. 2024, 2:18 PM iOS No Timeout has occurred
Nov 22, 2024, 1:18 PM Yes
Nov 20, 2024, 3:26 PM Yes No
Aug 12, 2024 3:25 PM Yes No
  • Support emails before Nov 20th are not PIN related all the way until August
  • new release of 1.0.21 happened on Dec 02, 2024

Steps to reproduce

  • App is updated
  • Open the BC Wallet app
  • User is presented with the "Is this app for you" screen, taps continue
  • User is presented with the onboarding screens, taps next
  • User is presented with the Terms and conditions, taps continue
  • User is presented with the PIN creation screen, enters a PIN and taps continue
  • Initialization
  • error message : "AskarWallet: Incorrect key for wallet 'walletid'

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
@knguyenBC
Copy link

Asked internal team members and no one was able to replicate.

@cvarjao cvarjao added the bug Something isn't working label Dec 11, 2024
@bryce-mcmath bryce-mcmath self-assigned this Dec 13, 2024
@knguyenBC
Copy link

I added additional information on what the sequence of events are before the error message appears.

@bryce-mcmath
Copy link
Contributor

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.

@bryce-mcmath
Copy link
Contributor

I'm still not done investigating this issue. But further questions:

  1. When was the last time they logged into BC Wallet successfully?
  2. Have they ever had BC Wallet on another device?
  3. Do they have iCloud backup on either device?

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.

@bryce-mcmath
Copy link
Contributor

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.

@bryce-mcmath
Copy link
Contributor

One other thing I have been assuming - hard closing the app and reopening never fixes this issue.

@bryce-mcmath
Copy link
Contributor

bryce-mcmath commented Dec 31, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants