[Wallet] Report to analytics reveal statuses from validators #5315
Conversation
i1skn
requested review from
cmcewen,
gastonponti,
jeanregisser,
jmrossy and
mcortesi
as code owners
There was a problem hiding this comment.
looks good. i would say we'd want all the data you specified in the PR overview.
i'm going to sync with tim to understand the schema for multiple attempts and whether it makes sense to parse at the level of throwing the event here or to keep in the "status" field.
i'll get back to you on it!
i1skn
requested review from
barbaraliau,
critesjosh,
medhakothari and
a team
as code owners
| try { | ||
| const urlParams = new URLSearchParams({ | ||
| phoneNumber, | ||
| salt: pepper ?? '', |
There was a problem hiding this comment.
Out of scope here but just a nit, would be nice to have salt renamed in attestation service too: https://github.com/celo-org/celo-monorepo/blob/master/packages/utils/src/io.ts#L109
Since there's been changes there anyway seems like there's an opportunity to do that (in a backwards compat way) before the next release. cc @aslawson @timmoreton
There was a problem hiding this comment.
we've already published the attestation service, so unfortunately can't do it this iteration :/
@jmrossy yup, we are currently in the process of urging validators to enable TLS now |
|
@jmrossy I've addressed the feedback, PTAL. |
|
@jeanregisser can you please provide more details if we need to verify this task. |
|
No need to verify @Lss-Ankit thanks! |
|
Thanks @jeanregisser for the update. |
Description
Implementing a second part of https://github.com/celo-org/celo-labs/issues/574. The logic is the following:
proxyRevealStatus.TODO
yarn deployneeds to be run inpackages/attestation-proxyto deploy firebase function.Other changes
features. USE_PHONE_NUMBER_PRIVACYTested
proxyRevealStatusandproxyRevealtocelo-mobile-alfajoresand tested forcing using proxy in the codebase on alfajores.Related issues
Backwards compatibility
Yes