-
Notifications
You must be signed in to change notification settings - Fork 986
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 Lost the syncing state in fallback recovery, when first enter the seed phrase for the wrong account #21298
Conversation
Jenkins BuildsClick to see older builds (21)
|
27eda90
to
0fe1e3c
Compare
100% of end-end tests have passed
Passed tests (7)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestWalletOneDevice:
Class TestWalletMultipleDevice:
Class TestCommunityMultipleDeviceMerged:
Class TestCommunityOneDeviceMerged:
|
@Parveshdhull |
Hi @Horupa-Olena, Thank you very much for testing the PR. Could you please share a screen recording so that I can replicate the issue? |
@Parveshdhull Sorry, I forgot add the video because it's simple flow of account creation and don't need any special actions. Screenrecorder-2024-09-23-14-18-58-880.mp4 |
Thank you, @Horupa-Olena, for sharing the video. Interestingly, I was unable to reproduce this issue when I tried to create an account from the profiles screen instead of the intro. The issue should be fixed now. |
16e7240
to
1cfa9c5
Compare
Also rebased the PR, so that fix from #21300 is also included. |
100% of end-end tests have passed
Passed tests (7)Click to expandClass TestCommunityOneDeviceMerged:
Class TestWalletMultipleDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestWalletOneDevice:
Class TestCommunityMultipleDeviceMerged:
|
@Parveshdhull The main issue has been fixed. However, on iOS, I encountered the following issue: ISSUE 1: ios onboarding: missing screen after password creation, "generating keys..." screen appears insteadSteps:
Expected results: Actual results: video.1.mp4 |
@Parveshdhull @jrainville
video.1.mp4I assume some changes were made on the status-go side, as these issues were not present in the main PR for the fallback flow. Could you confirm that notifications changes by the status-go side or I mistake? So I can create separately bug reports for this issues |
1cfa9c5
to
0b1f557
Compare
Hi @Horupa-Olena, Thank you very much for reporting these issues. Could you please confirm whether you are using a new account for testing? (Old notifications may have been fetched from the last fallback recovery flow.) - #21090 (comment) |
0b1f557
to
97a906b
Compare
@Parveshdhull Thank you for your quick fix. This issue still reproduces if I use the old profile. The difference is that in this PR, if I use a new account, I receive an incorrect notification. Previously, the first sync on a new account worked fine. |
Thank you for finding this issue. Should be fixed now. |
Issue already logged here and have not been worked on yet: #21260 |
Oh, you mean even if for new accounts we are showing wrong notification? Sorry I missed this. |
Weird, I just tested latest build and I am seeing correct notifications. |
@Parveshdhull Agreed. I think (but not sure) it can be some changes on status-go side, because this issue was found in nightly build. I see it only once in #21090, but now it reproduce more oten.
|
100% of end-end tests have passed
Passed tests (7)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestCommunityOneDeviceMerged:
Class TestWalletOneDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestWalletMultipleDevice:
|
@Parveshdhull Thank for your fix!
This fix is ok!
This fix is ok, too. |
Could you confirm, for this issue, we need to create a separate bug report, right? |
@Parveshdhull I just tested this on Desktop -> Desktop and I couldn't reproduce, so my guess is that you call As far as I can see, it's not a status-go issue, because it would reproduce on the Desktop side then. |
If this is happening even for fresh account then I think its probably a new bug in UI side. But if its happening on the same account which we used earlier or fallback recovery then its the same issue. - #21090 (comment) |
@Parveshdhull unfortunately it's reproduce on new accounts too |
Thank you for confirming, issue should be fixed now. |
1f274ee
to
1e6b812
Compare
Please let me know if PR also fixes #21260 |
71% of end-end tests have passed
Failed tests (2)Click to expandClass TestWalletMultipleDevice:
Passed tests (5)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMerged:
Class TestCommunityOneDeviceMerged:
Class TestWalletOneDevice:
|
@Parveshdhull Hey! Thanks for your fix!
This issue is fixed. But for old accounts (old = account that have few tries to syncs before) issue from #21260 still reproduce. |
@Parveshdhull I don't see any other issues. The PR is ready to be merged. |
… seed phrase for the wrong account
1bb1f3b
to
4f6f9c0
Compare
fixes #21258
status: ready