Page MenuHomePhabricator

[CommCoreModule] implement creating User Keys backup
AcceptedPublic

Authored by kamil on Thu, Nov 14, 8:50 AM.
Tags
None
Referenced Files
F3326429: D13934.id45825.diff
Wed, Nov 20, 7:22 AM
Unknown Object (File)
Tue, Nov 19, 8:55 PM
Unknown Object (File)
Sun, Nov 17, 8:17 PM
Unknown Object (File)
Sun, Nov 17, 6:59 PM
Unknown Object (File)
Sat, Nov 16, 6:16 PM
Unknown Object (File)
Sat, Nov 16, 6:16 PM
Unknown Object (File)
Sat, Nov 16, 6:15 PM
Unknown Object (File)
Thu, Nov 14, 5:56 PM
Subscribers

Details

Reviewers
bartek
tomek
Summary

ENG-9656.

There are several options on how to implement this:

  1. createUserKeysBackup: (backupSecret: string) -> We pass the secret (password or signature), and the SIWE message is read from SQLite. It's definitely the cleanest to use, but for SIWE users we first read the signature from SQLite to pass this again to C++ via JSI to then read the message again from the C++ level.
  2. createUserKeysBackup: (backupSecret: ?string) -> We can pass secret for password users, for SIWE read it directly from SQLite to avoid unnecessarily transferring data (this was pointed by @varun in https://phab.comm.dev/D13847#inline-78286). However, using nullable values raises concerns explained in https://phab.comm.dev/D11715?id=39388#inline-70515.
  3. createUserKeysBackup: (backupSecret: string) and createUserKeysBackupSIWE: () -> having separate JSI calls for SIWE and wallet users doing almost the same, for me it's the least clean, I prefer having one API to work regardless of user type.

I implemented 1, which is not the best in terms of transferring data but it's the cleanest one - but I am open to updating. Note, that this API won't be used a lot.

Depends on D13933

Test Plan

Tested in D13937

Diff Detail

Repository
rCOMM Comm
Branch
publish-client-backup
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

kamil held this revision as a draft.
kamil edited the summary of this revision. (Show Details)
kamil edited the summary of this revision. (Show Details)
kamil published this revision for review.Fri, Nov 15, 2:25 AM

I implemented 1, which is not the best in terms of transferring data but it's the cleanest one - but I am open to updating. Note, that this API won't be used a lot.

I agree with this reasoning

This revision is now accepted and ready to land.Wed, Nov 20, 7:46 AM