Page MenuHomePhabricator

[native] make generateNonce callable from js
ClosedPublic

Authored by varun on Mar 27 2023, 9:00 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Dec 7, 7:21 AM
Unknown Object (File)
Thu, Dec 5, 8:34 PM
Unknown Object (File)
Thu, Dec 5, 8:34 PM
Unknown Object (File)
Wed, Dec 4, 3:23 AM
Unknown Object (File)
Wed, Dec 4, 3:23 AM
Unknown Object (File)
Wed, Dec 4, 3:23 AM
Unknown Object (File)
Sun, Dec 1, 4:26 PM
Unknown Object (File)
Tue, Nov 26, 7:58 AM
Subscribers

Details

Summary

rust: add generate_nonce to the cxx bridge. in the fn implementation we construct
a client to talk to the identity service, and then call the generate_nonce RPC.

if we get an error constructing the client or calling the RPC, we convert it to
a string call stringCallback with it. otherwise we call the callback with the
nonce.

c++: implement generateNonce in CommCoreModule, which just calls the Rust fn
exposed by CXX with an atomically incremented promise ID

js: codegen some JSI functions

Test Plan

Diff Detail

Repository
rCOMM Comm
Branch
varun/poc3 (branched from master)
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

native/native_rust_library/src/lib.rs
94 ↗(On Diff #24258)

i'd rather we use a new namespace than prefix all these functions with identity... but maintaining the convention for now

111 ↗(On Diff #24258)

i'll rename counter to promise_id

118 ↗(On Diff #24258)

hardcoding localhost for now. need to see how we set the keyserver address on native today

varun requested review of this revision.Mar 27 2023, 9:15 PM
native/native_rust_library/src/lib.rs
111 ↗(On Diff #24258)

Are these changes necessary every time a new function is introduced? If not, this isn't a good "dummy diff"

I don't feel strongly about the rust code, which is ephemeral anyway.

Can't comment on the jsi code.

In D7215#214574, @jon wrote:

I don't feel strongly about the rust code, which is ephemeral anyway.

Can't comment on the jsi code.

i don't think the rust code is ephemeral, besides the hardcoding of the socket addr

jon added inline comments.
native/native_rust_library/src/lib.rs
115–134 ↗(On Diff #24294)

Sorry, read the "dummy diff" and somehow confused it with the "42 callback" code.

I would prefer something like the following, cuts down on branching logic and I think it's easier to read. Also if sending back async results will be more common, then the handle utility function can be reused often.

This revision now requires changes to proceed.Mar 28 2023, 10:43 AM

change counter to promise_id

addressing jon's feedback

native/native_rust_library/src/lib.rs
118 ↗(On Diff #24297)

Why are we hardcoding localhost here?

native/native_rust_library/src/lib.rs
118 ↗(On Diff #24297)

i'm planning to introduce configurability in a later diff. just want to unblock jon asap

moved additional one-time setup to parent diff

rust looks okay to me. Just don't like that the callee has to have awareness of the promise ID.

This revision is now accepted and ready to land.Mar 28 2023, 3:10 PM