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.

Sign up for GitHub

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

Jump to bottom

[Wallet] Enable 8 digit code verification and ignore attestation services below 1.1.0 #6437

Merged
merged 6 commits into from
Jan 12, 2021
Merged

[Wallet] Enable 8 digit code verification and ignore attestation services below 1.1.0 #6437

merged 6 commits into from
Jan 12, 2021

Conversation

i1skn
Copy link
Contributor

@i1skn i1skn commented Jan 11, 2021 •
edited
Loading

Description

Enable 8 digit code verification and ignore attestation services below 1.1.0 as they do not support 8 digit codes.

Other changes

Force EOA address as a signer to provide signature for attestation services.

Tested

Android manual input and auto-import using attestation service 1.2.0 on alfajores.

Related issues

Backwards compatibility

Yes

@i1skn i1skn changed the title Enable 8 digit code verification and ignore attestation services below 1.1.0 [Wallet] Enable 8 digit code verification and ignore attestation services below 1.1.0 Jan 11, 2021
@i1skn i1skn force-pushed the i1skn/enable-8digit-verification branch from c7b8805 to e6f9d9d Compare January 11, 2021 19:30
gnardini
gnardini reviewed Jan 12, 2021
View reviewed changes
packages/mobile/src/identity/verification.ts
@@ -495,6 +497,13 @@ export function* requestAndRetrieveAttestations(

// Check if we have a sufficient set now by fetching the new total set
attestations = yield call(getActionableAttestations, attestationsWrapper, phoneHash, account)
if (features.SHORT_VERIFICATION_CODES) {
// we only support attestation service 1.1.0 and above for short codes
attestations = attestations.filter(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if one or two attestation services have an old version? We'd be missing attestations right? Should we ask for more in that case, is it unlikely to happen or is there somewhere else that's already handling that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the question! If any of the services has older version we would ignore this attestation service and request more, cause outer while loop would not exit.

@i1skn i1skn force-pushed the i1skn/enable-8digit-verification branch from 89cf289 to 0c80617 Compare January 12, 2021 17:02
@i1skn i1skn requested review from asaj and timmoreton as code owners January 12, 2021 17:02
@i1skn i1skn added the automerge Have PR merge automatically when checks pass label Jan 12, 2021
@mergify mergify bot merged commit 0a91ca1 into master Jan 12, 2021
@mergify mergify bot deleted the i1skn/enable-8digit-verification branch January 12, 2021 23:43
@Lss-Ankit
Copy link

Hi @gnardini

We have started verifying task using Android Internal Build V1.9.0(1004294331) & Test Flight Build V1.9.0(41) but we are blocked due to issue #3280 for "Verification error" popup while verifying phone number.

Can you please do the needful as we are blocked to perform this task

CC: @jeanregisser , @nityas

@Lss-Ankit Lss-Ankit mentioned this pull request Jan 27, 2021
Merged
@Lss-Ankit
Copy link

Hi @gnardini I have verified this issue using latest Android Internal Build V1.9.1(1004294332) & Test Flight Build V1.9.1(42) & observed following:
Observations:

If user use SIM card:

  • User able to receive 8 code verification code & user can complete verification flow.

If user use third party app:

  • Also it is observed that user not able to get attestation codes if user use any third party app to receive messages. As earlier we are able to perform verification using third party app also.Let me know is it expected??

  • Also let me know how we can check verification flow for other countries as we are not receiving code with out sim

Verified on devices: realme 6i(10.0),iPhone SE 2 (14.2)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gnardini gnardini gnardini approved these changes

@barbaraliau barbaraliau Awaiting requested review from barbaraliau

@cmcewen cmcewen Awaiting requested review from cmcewen

@gastonponti gastonponti Awaiting requested review from gastonponti

@jeanregisser jeanregisser Awaiting requested review from jeanregisser

@jmrossy jmrossy Awaiting requested review from jmrossy

@mcortesi mcortesi Awaiting requested review from mcortesi

@medhakothari medhakothari Awaiting requested review from medhakothari

@asaj asaj Awaiting requested review from asaj

@timmoreton timmoreton Awaiting requested review from timmoreton

Assignees
No one assigned
Labels
automerge Have PR merge automatically when checks pass wallet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[P1] [Stretch] Verification 8-digit code
4 participants
@i1skn @Lss-Ankit @gnardini @jeanregisser