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

[WIP]: Fallback flow for pairing #20766

Closed
wants to merge 3 commits into from

Conversation

cammellos
Copy link
Contributor

@cammellos cammellos commented Jul 16, 2024

This PR adds a fallback flow in case syncing does not work.

The flow is as follow:

  1. If the syncing fails, the empty device (A2) will be prompted to enter their seed phrase.
  2. Once logged in A2 will show a popup with their own installation id
  3. On A1, it should show a prompt with A2 installation ID and an option to "Enable and Sync"
  4. If enable and sync is clicked, the devices should be now paired, and data transfer should happen.

Devices need to be connected to the internet for this to work, but don't need to be on the same network.

I have tested the happy path yesterday, and the two devices are paired correctly, there's some issues with the actual data being synchronized, but @qfrank will be looking into it, he will be taking over the status-go pr.

@xAlisher can share the design and flows.

Things left to do:

fire pair installation when generating a code, to facilitate correct discovery

When displaying a QR code from the sending device, it would be nice if we could call SendPairInstallation RPC, so that metadata is available.

Styling & translations

Styling and translations needs to be checked

Double modal

When/if the data confirmation modal is merged, there could be a clash of modals when logging in if the user of the receiving pair is on mobile network (i.e two modals opened at the same time), something to watch out for.

Testing

Please use this PR for both A1 & A2, as this is a breaking change 2.30 will only local pair with 2.30. Or you can use desktop if @jrainville has finished with his part.

Client A1 is logged in
Client A2 is logged out

  1. Client A1 shows a QR code
  2. Client A2 goes offline/mobile data, Client A2 scans a QR code (it should fail).
  3. If connection fails on A2, the user will be prompted to enter a seed phrase. Make sure you are connected now.
  4. If the generated account matches the key-uid from the QR code, A2 should call FinishPairingThroughSeedPhraseProcess with the installation id passed in the QR code. This will send installation information over waku. The user should be shown its own installation id and prompted to check the other device.
  5. Client A1 will receive new installation data through waku, if they are still on the qr code page, they should show a popup to the user showing the received installation id, and a way to Enable and Sync, which should call the EnableAndSyncInstallation endpoint. This should finish the fallback syncing flow.

Other areas of testing: Pairing flow when no error is returned.

Status-go pr: status-im/status-go#5567

@cammellos cammellos self-assigned this Jul 16, 2024
@status-im-auto
Copy link
Member

status-im-auto commented Jul 16, 2024

Jenkins Builds

Click to see older builds (40)
Commit #️⃣ Finished (UTC) Duration Platform Result
b08974d #2 2024-07-16 09:09:07 ~3 min tests 📄log
b08974d #2 2024-07-16 09:10:11 ~4 min ios 📄log
✔️ b08974d #2 2024-07-16 09:12:44 ~6 min android-e2e 🤖apk 📲
✔️ b08974d #2 2024-07-16 09:13:24 ~7 min android 🤖apk 📲
b5ff042 #3 2024-07-16 10:23:18 ~2 min ios 📄log
b5ff042 #3 2024-07-16 10:23:50 ~3 min tests 📄log
✔️ b5ff042 #3 2024-07-16 10:26:59 ~6 min android-e2e 🤖apk 📲
✔️ b5ff042 #3 2024-07-16 10:28:31 ~7 min android 🤖apk 📲
c2d708a #4 2024-07-16 12:18:06 ~3 min tests 📄log
✔️ c2d708a #4 2024-07-16 12:21:16 ~6 min android-e2e 🤖apk 📲
✔️ c2d708a #4 2024-07-16 12:23:12 ~8 min android 🤖apk 📲
✔️ c2d708a #4 2024-07-16 12:25:42 ~11 min ios 📱ipa 📲
173e6be #5 2024-07-19 08:08:24 ~3 min tests 📄log
✔️ 173e6be #5 2024-07-19 08:13:34 ~8 min android-e2e 🤖apk 📲
✔️ 173e6be #5 2024-07-19 08:14:27 ~9 min android 🤖apk 📲
✔️ 173e6be #5 2024-07-19 08:15:51 ~11 min ios 📱ipa 📲
e9aee39 #6 2024-07-22 07:33:40 ~2 min tests 📄log
✔️ e9aee39 #6 2024-07-22 07:38:12 ~7 min android-e2e 🤖apk 📲
✔️ e9aee39 #6 2024-07-22 07:38:31 ~7 min android 🤖apk 📲
✔️ e9aee39 #6 2024-07-22 07:39:48 ~9 min ios 📱ipa 📲
43a22b6 #7 2024-07-22 10:34:14 ~2 min tests 📄log
✔️ 43a22b6 #7 2024-07-22 10:37:54 ~6 min android 🤖apk 📲
✔️ 43a22b6 #7 2024-07-22 10:38:51 ~7 min android-e2e 🤖apk 📲
✔️ 43a22b6 #7 2024-07-22 10:40:37 ~8 min ios 📱ipa 📲
49a7b20 #9 2024-07-22 15:23:17 ~3 min tests 📄log
✔️ 49a7b20 #9 2024-07-22 15:27:16 ~7 min android-e2e 🤖apk 📲
✔️ 49a7b20 #9 2024-07-22 15:28:11 ~8 min android 🤖apk 📲
✔️ 49a7b20 #9 2024-07-22 15:31:01 ~11 min ios 📱ipa 📲
c5d8235 #10 2024-07-23 13:34:41 ~3 min tests 📄log
✔️ c5d8235 #10 2024-07-23 13:38:55 ~7 min android-e2e 🤖apk 📲
✔️ c5d8235 #10 2024-07-23 13:39:55 ~8 min android 🤖apk 📲
✔️ c5d8235 #10 2024-07-23 13:42:32 ~11 min ios 📱ipa 📲
451532f #12 2024-07-24 07:36:24 ~3 min tests 📄log
✔️ 451532f #12 2024-07-24 07:41:17 ~8 min android-e2e 🤖apk 📲
✔️ 451532f #12 2024-07-24 07:42:24 ~9 min ios 📱ipa 📲
✔️ 451532f #12 2024-07-24 07:42:30 ~9 min android 🤖apk 📲
0ccd21c #14 2024-07-25 09:38:43 ~5 min tests 📄log
✔️ 0ccd21c #14 2024-07-25 09:42:58 ~10 min android-e2e 🤖apk 📲
✔️ 0ccd21c #14 2024-07-25 09:43:59 ~11 min android 🤖apk 📲
✔️ 0ccd21c #14 2024-07-25 09:44:34 ~11 min ios 📱ipa 📲
Commit #️⃣ Finished (UTC) Duration Platform Result
aa28678 #15 2024-07-25 09:52:51 ~7 min tests 📄log
✔️ aa28678 #15 2024-07-25 09:54:11 ~9 min ios 📱ipa 📲
✔️ aa28678 #15 2024-07-25 09:54:59 ~9 min android-e2e 🤖apk 📲
✔️ aa28678 #15 2024-07-25 09:57:12 ~12 min android 🤖apk 📲
a8a08c8 #16 2024-07-25 10:38:56 ~3 min tests 📄log
✔️ a8a08c8 #16 2024-07-25 10:41:37 ~6 min android 🤖apk 📲
✔️ a8a08c8 #16 2024-07-25 10:43:22 ~7 min android-e2e 🤖apk 📲
✔️ a8a08c8 #16 2024-07-25 10:44:21 ~8 min ios 📱ipa 📲

@cammellos cammellos force-pushed the feature/fallback-pairing-seed branch 4 times, most recently from fa92da3 to 49a7b20 Compare July 22, 2024 15:19
@cammellos cammellos force-pushed the feature/fallback-pairing-seed branch 3 times, most recently from 29f62e5 to 451532f Compare July 24, 2024 07:32
@cammellos cammellos changed the title Feature/fallback pairing seed Fallback flow for pairing Jul 24, 2024
@cammellos cammellos marked this pull request as ready for review July 24, 2024 07:49
@churik churik self-assigned this Jul 25, 2024
@Parveshdhull Parveshdhull force-pushed the feature/fallback-pairing-seed branch from 451532f to bd530c4 Compare July 25, 2024 09:29
@Parveshdhull Parveshdhull force-pushed the feature/fallback-pairing-seed branch from 0ccd21c to aa28678 Compare July 25, 2024 09:44
@Parveshdhull Parveshdhull self-assigned this Jul 25, 2024
@Parveshdhull
Copy link
Member

Parveshdhull commented Jul 25, 2024

Issues so far

  • when A1 and A2 can sync successfully without waku, I saw A1 show the popup, this is not we should expect.
  • based on issue1, press continue, A2 will stuck as 3rd screenshot(A2 screenshot2) shows, restart A2 can login successfully. (Please also test fix onboarding navigation to enable notification screen and blur issues #20725, as issue/fix is related to that)
  • Syncing progress screen style is broken (only taking half screen)

I am working on these issues. CC @qfrank @churik

UPD: pushing fix for last 2 issues in separate PR - #20883

@Parveshdhull Parveshdhull changed the title Fallback flow for pairing [WIP]: Fallback flow for pairing Jul 25, 2024
@Parveshdhull Parveshdhull marked this pull request as draft July 25, 2024 11:03
@Parveshdhull
Copy link
Member

Marking the PR as draft for now, as its moved to next milestone

@ilmotta
Copy link
Contributor

ilmotta commented Jul 25, 2024

On top of what @Parveshdhull shared above #20766 (comment) Given the circumstances and the tight schedule for 2.30 and available capacity for QA, we have decided that:

  1. We should try to get the status-go PR merged before 2.30 is out add fallback flow to pairing status-go#5567 because it's safe to do so.
  2. On the clients, we can adapt the connection string to adjust to the breaking change and still be compatible with 2.29.
  3. We are not going to try to implement the UI changes for 2.30.

cc @qfrank

@qfrank
Copy link
Contributor

qfrank commented Jul 26, 2024

On the clients, we can adapt the connection string to adjust to the breaking change and still be compatible with 2.29.

yeah, v2.30 can parse QR generated by v2.29, but not the other way around. We may ignore the opposite use case, I mean ppl won't use v2.29 to scan the QR generated by v2.30 right?


We are not going to try to implement the UI changes for 2.30.

so once status-go PR get merged, we actually won't support fallback flow for pairing for v2.30? according to this message, we will support it in v2.30.1 ?

@ilmotta

@ilmotta
Copy link
Contributor

ilmotta commented Jul 26, 2024

On the clients, we can adapt the connection string to adjust to the breaking change and still be compatible with 2.29.

yeah, v2.30 can parse QR generated by v2.29, but not the other way around. We may ignore the opposite use case, I mean ppl won't use v2.29 to scan the QR generated by v2.30 right? 

We are not going to try to implement the UI changes for 2.30.

so once status-go PR get merged, we actually won't support fallback flow for pairing for v2.30? according to this message, we will support it in v2.30.1 ?

@ilmotta

Yes for both questions @qfrank :) We are descoping the fallback feature (on the UI/front) for 2.30 due to risks and capacity to QA the work adequately in the face of other priorities.

Edit: Further discussions with the desktop team led to a different decision. @cammellos will reach out to you @qfrank until Monday for more technical details.

@ilmotta
Copy link
Contributor

ilmotta commented Jul 29, 2024

Hey @qfrank, can we close this PR for now given PR status-im/status-go#5614 won't require changes in clients once merged?

@qfrank
Copy link
Contributor

qfrank commented Jul 30, 2024

Hey @qfrank, can we close this PR for now given PR status-im/status-go#5614 won't require changes in clients once merged?

I think so, we can reopen it later or create new one if needed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants