search

celo-org / celo-monorepo

Official repository for core projects comprising the Celo platform
https://celo.org
Apache License 2.0
699 stars 369 forks source link

DappKit should return the phone number identifier for a Valora wallet #7428

Closed codyborn closed 3 years ago

codyborn commented 3 years ago

Certain dapps like ImpactMarket would like to check the verification status and uniqueness of a phone number for one of their users. This can help with simple fraud prevention if a user is creating multiple accounts with the same phone number or claiming to own a number that is not truly verified. Today the Dapp developer will have to process events to see the address->PN identifier relationship since the only mapping that's stored on chain is from PN Identifier->Address.

DappKit/WalletConnect should return the phone number identifier for a Valora wallet. This allows easy lookup of verification status for an account.

aslawson commented 3 years ago

Should be super easy to add.

codyborn commented 3 years ago

@nambrot queued it up🏌️‍♂️ https://github.com/celo-org/celo-monorepo/compare/nambrot/dappkit-pepper?expand=1 https://github.com/celo-org/wallet/compare/nambrot/pepper-dappkit?expand=1

aslawson commented 3 years ago

@nambrot is anything needed to make these pull requests? ^ were you hoping for someone else to test it?

nambrot commented 3 years ago

Oh I didn't just want to drive-by create PRs without consulting y'all, but IMO these should be pretty straightiforward, so happy to just create them and get them merged

aslawson commented 3 years ago

sounds good. looks like your impl returns the pepper rather than identifier. as part of this, can you could add a blurb under here about how the dappkit will:

  1. return both phoneNumber and pepper
  2. how to derive the identifier
  3. how to confirm request mappings from onchain in a way the dev can confirm if its a singular mapping (https://docs.celo.org/developer-guide/sdk-code-reference/summary-2/modules/_wrappers_attestations_.attestationswrapper#lookupidentifiers)
aslawson commented 3 years ago

@nambrot ^ just wanted to make sure we also have the documentation for how devs can access this new feature

aslawson commented 3 years ago

Also made a separate ticket to add this functionality to WalletConnect as well https://app.zenhub.com/workspaces/devx-backlog-board-6009f3d1a7a39b001018f159/issues/celo-org/celo-monorepo/7584

nambrot commented 3 years ago

Was this what you were envisioning? https://github.com/celo-org/celo-monorepo/pull/7546/commits/6160f74b7409b5af1f4e869db07fe4fc29cbec53

aslawson commented 3 years ago

@nambrot added a comment. otherwise looks good 👍