Page MenuHomePhabricator

Introduce notifications crypto module to CommCoreModule
ClosedPublic

Authored by marcin on Mar 4 2024, 4:44 AM.
Tags
None
Referenced Files
F2118525: D11232.id38337.diff
Wed, Jun 26, 11:52 AM
Unknown Object (File)
Sat, Jun 22, 8:52 AM
Unknown Object (File)
Fri, Jun 21, 6:31 PM
Unknown Object (File)
Fri, Jun 21, 10:59 AM
Unknown Object (File)
Fri, Jun 21, 5:45 AM
Unknown Object (File)
Fri, Jun 21, 3:34 AM
Unknown Object (File)
Thu, Jun 20, 9:09 PM
Unknown Object (File)
Thu, Jun 20, 8:53 PM
Subscribers

Details

Summary

This differential modifies CommCoreModule to work with notifications account that is persisted in SQLite. At the point of this diff notifications crypto
account exists in two places: in SQLite and in notifs storage. This isn't a mistake since only the one in SQLite will be used for cryptographic purposes since notifs
specific code only works with session. Next diff will remove notifs account from notifs storage.

Test Plan
  1. Uninstall comm app from iOS/Android device.
  2. Apply this patch and build the app: https://gist.github.com/marcinwasowicz/47964be1bd6aeaabc5504b3fa440f340
  3. Tap each button that the patch introduces. Ensure that keys are logged to console.

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

marcin requested review of this revision.Mar 4 2024, 4:59 AM
kamil added inline comments.
native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp
351 ↗(On Diff #37792)
395 ↗(On Diff #37792)
399 ↗(On Diff #37792)
662 ↗(On Diff #37792)
694 ↗(On Diff #37792)

we return in line above sp we can make only if condition to make it more readable

695–696 ↗(On Diff #37792)

are you sure this is the right condition?

native/cpp/CommonCpp/NativeModules/CommCoreModule.h
30 ↗(On Diff #37792)

I think we should now rename it to contentCryptoModule

agree with @kamil 's suggestions, but lgtm otherwise

This revision is now accepted and ready to land.Mar 5 2024, 12:47 PM
native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp
454–462 ↗(On Diff #37792)

Isn't this done in the constructor of CryptoModule?

Rebase to account for changes from parent differential