Page MenuHomePhabricator

[identity] modify get user identity rpc to take user ids and return identity map with farcaster id

Authored by will on Wed, May 8, 10:41 PM.
Referenced Files
Unknown Object (File)
Fri, May 17, 8:35 AM
Unknown Object (File)
Tue, May 14, 10:25 PM
Unknown Object (File)
Sun, May 12, 5:45 PM
Unknown Object (File)
Sat, May 11, 10:05 PM
Unknown Object (File)
Fri, May 10, 4:22 AM
Unknown Object (File)
Fri, May 10, 4:22 AM
Unknown Object (File)
Fri, May 10, 4:21 AM



This diff modifies our protos for the get user identity rpc to take in a list of user ids and return their corresponding identities.

Additionally, for our farcaster work, the Identity message will now also include a farcaster id field

Depends on D11951

Test Plan

Deployed on staging and called the rpcs with a list of user ids, including a default username user, an ethereum user, a default user with farcaster id, and a non-existent user.

The rpc returned all the proper Identity info and didn't include the non-existent userID

Diff Detail

rCOMM Comm
Lint Not Applicable
Tests Not Applicable

Event Timeline

will requested review of this revision.Wed, May 8, 10:56 PM
bartek added a reviewer: ashoat.

Rust part is ok

Adding @ashoat due to proto changes

42–43 ↗(On Diff #39960)

Not related to this diff but I realized we have no Farcaster integration tests.
Idea for a simple integration test:

  • Copy-paste this test.
  • Call client.link_farcaster_account() before querying
  • Call find_user_identities() but instead of username, check for fid being present
744 ↗(On Diff #39960)

We talked about adding more logs in the last retro

754 ↗(On Diff #39960)
98 ↗(On Diff #39960)

I was wondering if it's more correct to have it inside EthereumIdentity, but the approach here is more consistent with what you agreed on in this discussion.

So it probably should stay this way

98 ↗(On Diff #39960)

To clarify for anyone else reading this. You can have a farcaster_id without an EthereumIdentity so the two should remain separate

Proto changes look good!

225 ↗(On Diff #39960)

Don't need this newline

This revision is now accepted and ready to land.Thu, May 9, 4:11 PM
will marked 3 inline comments as done.Wed, May 15, 8:37 AM
will added inline comments.
42–43 ↗(On Diff #39960)