Page MenuHomePhabricator

[native] add findUserIDForWalletAddress to CommRustModule
ClosedPublic

Authored by varun on Feb 15 2024, 1:30 PM.
Tags
None
Referenced Files
F3559669: D11098.id37281.diff
Fri, Dec 27, 6:27 AM
F3559430: D11098.id37314.diff
Fri, Dec 27, 6:06 AM
F3546342: D11098.diff
Thu, Dec 26, 2:36 PM
Unknown Object (File)
Tue, Dec 24, 4:02 AM
Unknown Object (File)
Tue, Dec 24, 4:02 AM
Unknown Object (File)
Tue, Dec 24, 4:02 AM
Unknown Object (File)
Tue, Dec 24, 4:02 AM
Unknown Object (File)
Sun, Dec 22, 3:44 AM
Subscribers

Details

Summary

CommRustModule bindings and rust client method implementation

Depends on D11084

Test Plan

called the JSI function from JS and successfully retrieved the user ID associated with a wallet address in my local DDB

diff --git a/native/account/siwe-panel.react.js b/native/account/siwe-panel.react.js
index 02dd3fbed..2b49412d4 100644
--- a/native/account/siwe-panel.react.js
+++ b/native/account/siwe-panel.react.js
@@ -28,6 +28,7 @@ import { UnknownErrorAlertDetails } from '../utils/alert-messages.js';
 import Alert from '../utils/alert.js';
 import { getContentSigningKey } from '../utils/crypto-utils.js';
 import { defaultLandingURLPrefix } from '../utils/url-utils.js';
+import { commRustModule } from '../native-modules.js';
 
 const commSIWE = `${defaultLandingURLPrefix}/siwe`;
 
@@ -110,6 +111,12 @@ function SIWEPanel(props: Props): React.Node {
         );
       }
 
+      const identityUserID = await commRustModule.findUserIDForWalletAddress(
+        '0xF8b57E5eD505bC6fAfFC765c1AC8569063664c28',
+      );
+      console.log(identityUserID);
+      const parsedIdentityResponse = JSON.parse(identityUserID);
+      console.log(parsedIdentityResponse.userID);
       const ed25519 = await getContentSigningKey();
       setPrimaryIdentityPublicKey(ed25519);
     })();
LOG  {"userID":"C81FF954-6216-4489-B23C-0EA145EA2003","isReserved":false}
LOG  C81FF954-6216-4489-B23C-0EA145EA2003

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

varun edited the test plan for this revision. (Show Details)
varun requested review of this revision.Feb 15 2024, 1:46 PM
  1. Can you explain more about why we need this?
  2. Why is it specific to wallet address rather than general to usernames?
  3. Does it call the identity service?
  1. Can you explain more about why we need this?

I just updated D11005 to use this, but the gist is, we need to check if a user already exists for a given wallet address so that we can decide whether to call identityWalletLogIn, identityWalletRegister, or onAccountDoesNotExist

Initially I wanted to just try login and check for a "does not exist" error, but we remove the nonce from DDB on the login call (i think this is the right thing to do to prevent replay attacks), so the client would have to get a new nonce and sign a new message to subsequently register, which would be a bad user experience.

  1. Why is it specific to wallet address rather than general to usernames?

I initially tried making it more general but because the RPC expects an Identifier enum (with variants WalletAddress and Username), we'd need to pass an enum from C++ to rust, which seemed like more trouble than it was worth.

  1. Does it call the identity service?

yes

We also need findUserIDForWalletAddress to determine whether to navigate to ExistingEthereumAccount from ConnectEthereum

Thanks for explaining!

but we remove the nonce from DDB on the login call (i think this is the right thing to do to prevent replay attacks)

I'm not so sure about the replay attack stuff. But looking at your code, it seems like FullscreenSIWEPanel is the only place that will need an extra round trip here, and that component should go away after we launch secondary device auth via primary device.

We also need findUserIDForWalletAddress to determine whether to navigate to ExistingEthereumAccount from ConnectEthereum

We'll need this one long-term.

One last question – why not use the existing RPC FindUserID? Is it just the C++-to-Rust enum thing?

native/native_rust_library/src/exact_user_search.rs
45 ↗(On Diff #37281)

Ah never mind, that's what it's doing

Makes sense to me. We can easily refactor this in the future when we find a better way of passing enums JS -> C++ -> Rust


Initially I wanted to just try login and check for a "does not exist" error, but we remove the nonce from DDB on the login call (i think this is the right thing to do to prevent replay attacks), so the client would have to get a new nonce and sign a new message to subsequently register, which would be a bad user experience.

Thanks for explaining!

but we remove the nonce from DDB on the login call (i think this is the right thing to do to prevent replay attacks)

I'm not so sure about the replay attack stuff. [...]

I think @varun just mentioned the same problem we have in the "restore" whitepaper protocol - we want to avoid running the SIWE login procedure twice and it's a bad UX

This revision is now accepted and ready to land.Feb 15 2024, 10:23 PM