Page MenuHomePhabricator

[protos] Refactor user Identity message type
ClosedPublic

Authored by bartek on Apr 4 2024, 4:22 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Apr 26, 4:03 PM
Unknown Object (File)
Fri, Apr 26, 1:53 PM
Unknown Object (File)
Apr 16 2024, 8:40 PM
Unknown Object (File)
Apr 12 2024, 11:55 AM
Unknown Object (File)
Apr 11 2024, 10:31 PM
Unknown Object (File)
Apr 10 2024, 4:01 PM
Unknown Object (File)
Apr 10 2024, 3:45 PM
Unknown Object (File)
Apr 10 2024, 10:18 AM
Subscribers

Details

Summary

Addresses ENG-7532.

  • Replaced oneof with username and optional EthIdentity
  • Replaced social_proof with separate siwe message and signature fields

Depends on D11546

Test Plan

Registered password and wallet user. Called FindUserIdentity for both and verified the respponses.

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

bartek held this revision as a draft.
bartek published this revision for review.Apr 4 2024, 5:06 AM
bartek added inline comments.
services/identity/src/siwe.rs
49–58 ↗(On Diff #38764)

This is temporary. I get rid of this serialize-then-deserialize pattern in the next diff (D11548)

varun requested changes to this revision.Apr 5 2024, 7:21 AM
varun added inline comments.
keyserver/addons/rust-node-addon/src/identity_client/get_inbound_keys_for_user.rs
39–51 ↗(On Diff #38764)

how about something like this?

we eventually expect users to have usernames and wallet addresses, so i'm wondering if there's any need to have them be mutually exclusive here. we could then simplify this js code in user-responders.js:

const username = inboundKeysForUser.username
  ? inboundKeysForUser.username
  : inboundKeysForUser.walletAddress;

becomes

const username = inboundKeysForUser.username;
services/identity/src/grpc_utils.rs
257 ↗(On Diff #38764)

did you mean just "failed to construct wallet identity" ?

This revision now requires changes to proceed.Apr 5 2024, 7:21 AM
keyserver/addons/rust-node-addon/src/identity_client/get_inbound_keys_for_user.rs
39–51 ↗(On Diff #38764)

I'm not opposed to that.
I simply kept the previous logic here but can update it if you think that fits better.

services/identity/src/grpc_utils.rs
257 ↗(On Diff #38764)

yep, thanks

This revision is now accepted and ready to land.Apr 10 2024, 10:17 AM