Page MenuHomePhabricator

[native] Expose get keyserver keys Identity RPC
ClosedPublic

Authored by tomek on Dec 22 2023, 7:12 AM.
Tags
None
Referenced Files
F2171089: D10449.diff
Tue, Jul 2, 4:49 PM
Unknown Object (File)
Sat, Jun 29, 4:30 AM
Unknown Object (File)
Thu, Jun 27, 9:05 AM
Unknown Object (File)
Tue, Jun 25, 9:29 PM
Unknown Object (File)
Tue, Jun 25, 11:15 AM
Unknown Object (File)
Tue, Jun 25, 11:15 AM
Unknown Object (File)
Tue, Jun 25, 11:15 AM
Unknown Object (File)
Tue, Jun 25, 11:15 AM
Subscribers

Details

Summary

Add Rust and C++ code that allows calling this function.

https://linear.app/comm/issue/ENG-6083/allow-calling-getkeyserverkeys-on-native

Depends on D10403

Test Plan

Call this function in a code introduced by D10327 and check if the keys are returned.

Diff Detail

Repository
rCOMM Comm
Branch
handler-4
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

tomek added inline comments.
native/cpp/CommonCpp/NativeModules/CommRustModule.cpp
284–286 ↗(On Diff #34986)

This approach is consistent with other functions from this file, but I strongly suspect it is incorrect - as it ignores the error. The first function using this approach was introduced in https://phab.comm.dev/D8657 - @kamil is this the right approach?

Harbormaster returned this revision to the author for changes because remote builds failed.Dec 22 2023, 7:26 AM
Harbormaster failed remote builds in B25324: Diff 34986!
tomek requested review of this revision.Dec 22 2023, 7:34 AM
native/cpp/CommonCpp/NativeModules/CommRustModule.cpp
284–286 ↗(On Diff #34986)

In D8657 I only moved methods from CommCoreModule (D8654) without changing implementation or updating anything - but yes, probably you're right and methods were implemented incorrectly

native/cpp/CommonCpp/NativeModules/CommRustModule.cpp
273 ↗(On Diff #34986)

this also turned out to be incorrect, here is the proposed fix: D10550

284–286 ↗(On Diff #34986)

I've added D10530 to fix entire file, would be nice if you could implement the same here

Fix error handling and capturing

This revision is now accepted and ready to land.Jan 9 2024, 3:57 AM