Details

Reviewers

ashoat
• jon

Commits

rCOMM34449495726c: [native] make generateNonce callable from js

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

see gist: https://gist.github.com/vdhanan/bc034bf2e099b969724d25e04e77ba3b

Diff Detail

Repository

rCOMM Comm

Branch

varun/poc3 (branched from master)

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

varun created this revision.Mar 27 2023, 9:00 PM

Herald added subscribers: atul, tomek. · View Herald TranscriptMar 27 2023, 9:00 PM

varun added a parent revision: D7214: [native] introduce RustCallback.Mar 27 2023, 9:01 PM

varun edited the test plan for this revision. (Show Details)

varun added inline comments.Mar 27 2023, 9:06 PM

native/native_rust_library/src/lib.rs
94	i'd rather we use a new namespace than prefix all these functions with identity... but maintaining the convention for now
111	i'll rename counter to promise_id
118	hardcoding localhost for now. need to see how we set the keyserver address on native today

Harbormaster completed remote builds in B17723: Diff 24258.Mar 27 2023, 9:15 PM

varun requested review of this revision.Mar 27 2023, 9:15 PM

ashoat added inline comments.Mar 28 2023, 5:26 AM

native/native_rust_library/src/lib.rs
111	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

remove one-time setup

varun added a parent revision: D7221: [native] one-time setup for rust-C++ integration.Mar 28 2023, 10:15 AM

Harbormaster completed remote builds in B17752: Diff 24294.Mar 28 2023, 10:30 AM

• jon requested changes to this revision.Mar 28 2023, 10:43 AM

• 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.