-
Notifications
You must be signed in to change notification settings - Fork 97
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
chore: remove decentralised verification paths #4274
Conversation
Co-authored-by: Jean Regisser <[email protected]>
1 build decreased size
Celo (test) 1.70.0 (135)⚖️ Compare build Total install size change: ⬇️ 1.0 MB (-1.61%) Largest size changes
🛸 Powered by Emerge Tools |
Codecov Report
@@ Coverage Diff @@
## main #4274 +/- ##
==========================================
+ Coverage 84.06% 84.28% +0.21%
==========================================
Files 712 707 -5
Lines 26319 25972 -347
Branches 3445 3427 -18
==========================================
- Hits 22126 21891 -235
+ Misses 4127 4015 -112
Partials 66 66
... and 5 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, this feels good! ✂️ 💥
REVOKE_VERIFICATION = 'IDENTITY/REVOKE_VERIFICATION', | ||
REVOKE_VERIFICATION_STATE = 'IDENTITY/REVOKE_VERIFICATION_STATE', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there more actions we can remove around here?
Seeing:
- SET_SEEN_VERIFICATION_NUX
- FETCH_ADDRESSES_AND_VALIDATION_STATUS
- UPDATE_WALLET_TO_ACCOUNT_ADDRESS
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these all seem to be used...am i missing something?
- SET_SEEN_VERIFICATION_NUX seems to be used for determining the initial route (the name is confusing but seems more like a "HAS_SKIPPED_PHONE_VERIFICATION" action)
- FETCH_ADDRESSES_AND_VALIDATION_STATUS seems to be used for the phone number lookup
- UPDATE_WALLET_TO_ACCOUNT_ADDRESS is used to fetch the DEK
i'm happy to remove any of these if we don't need the flows anymore but the code paths look still alive for these
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ahah ok, well was just trying to see if there were more actions we could now remove :D
}) | ||
}) | ||
} | ||
function* navigateToQuotaPurchaseScreen() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to see this one gone.
@kathaypacific I was surprised to see we still get successful migrations after all this time. About 97 in the last 30 days. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great! 🚀
### Description This PR removes: - remote config flag for decentralizedVerificationEnabled - stuff related to decentralised phone number lookups with ODIS - stuff relating to ODIS quota purchases - decentralised verification analytics events which are no longer used - decentralised revoke mapping - translations relating to decentralised verification - unlikely we'll want to update this - fetch lost accounts on firebase Note: background migration from decentralised verification to CPV was not removed since it seems there's still traffic to that endpoint. This flow will only work if the user has a cached salt for their own number. ### Test plan CI ### Related issues - Fixes RET-880 ### Backwards compatibility Y --------- Co-authored-by: Jean Regisser <[email protected]>
Description
This PR removes:
Note: background migration from decentralised verification to CPV was not removed since it seems there's still traffic to that endpoint. This flow will only work if the user has a cached salt for their own number.
Test plan
CI
Related issues
Backwards compatibility
Y