registration fails due to expired nonce
ClosedPublic
Actions

Authored by ashoat on May 12 2024, 7:34 PM.

Details

Reviewers

inka

Commits

rCOMMef68e14230ae: [native] Display meaningful alert if RegistrationTerms/FullscreenSIWEPanel…

Summary

This addresses ENG-7666 and ENG-4087.

Doing RegistartionTerms and FullscreenSIWEPanel in one diff because they required shared changes to RegistrationServerCallInput. Will annotate diff with some more details inline.

Would normally put @varun on the review here, but going to ask @inka instead because he is out.

Depends on D12009

Test Plan

I tested (or will test) this diff stack as follows:

Be in a multi-keyserver environment, testing SIWE with iOS simulator
Do a SIWE and then wait 2 minutes to let the nonce expire in the following screens:
- FullscreenSIWEPanel for an account that doesn't exist yet when the new registration flow is disabled
- FullscreenSIWEPanel for an account that doesn't exist yet when the new registration flow is enabled
- FullscreenSIWEPanel for an account that does exist
- New registration flow for an account that doesn't exist yet (RegistrationTerms)
- New registration flow for an account that does exist (ExistingEthereumAccount)
Make sure there are no duplicate Alerts, that in all cases an Alert is shown, and that the "back" action activates when the user confirms the Alert

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

ashoat created this revision.May 12 2024, 7:34 PM

Herald added a subscriber: tomek. · View Herald TranscriptMay 12 2024, 7:34 PM

ashoat added inline comments.May 12 2024, 7:39 PM

native/account/fullscreen-siwe-panel.react.js
105 ↗	(On Diff #40103)	I removed this `throw e` line and replaced the next one with a `return` because I felt it was confusing to read. There is no point to throwing an exception here, since this eventually bubbles up through the `onMessage` callback we pass to `WebView`. I don't think throwing an exception there has any particular meaning, and trying to maintain the property that we always throw an exception from this function if an error occurs is hard
108 ↗	(On Diff #40103)	We `throw` here to make sure we still trigger the `Alert` on line 132. We want some `Alert` to be shown in any error case
127–128 ↗	(On Diff #40103)	This is new and probably should have been here when I introduced the `registrationServerCall` here in D11767