Page MenuHomePhabricator

[native] Expose get keyserver keys Identity RPC
ClosedPublic

Authored by tomek on Dec 22 2023, 7:12 AM.
Tags
None
Referenced Files
F3352135: D10449.diff
Sat, Nov 23, 4:45 AM
Unknown Object (File)
Wed, Nov 6, 4:59 AM
Unknown Object (File)
Oct 18 2024, 7:40 AM
Unknown Object (File)
Oct 18 2024, 7:40 AM
Unknown Object (File)
Oct 18 2024, 7:40 AM
Unknown Object (File)
Oct 18 2024, 7:40 AM
Unknown Object (File)
Oct 18 2024, 7:40 AM
Unknown Object (File)
Oct 18 2024, 7:39 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
Lint
Lint Not Applicable
Unit
Tests Not Applicable

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