Page MenuHomePhabricator

Update native backup code to handle siwe backup message
ClosedPublic

Authored by marcin on Apr 22 2024, 11:25 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Nov 24, 12:12 AM
Unknown Object (File)
Thu, Nov 21, 7:28 AM
Unknown Object (File)
Fri, Nov 8, 6:00 PM
Unknown Object (File)
Nov 3 2024, 6:30 AM
Unknown Object (File)
Oct 12 2024, 6:36 PM
Unknown Object (File)
Oct 12 2024, 6:36 PM
Unknown Object (File)
Oct 12 2024, 6:36 PM
Unknown Object (File)
Oct 12 2024, 6:36 PM
Subscribers

Details

Summary

This diff modifies native rust code to attach backup message to upload request provided the file exists.

Test Plan

For now test that backup upload works for password users. Very next diff will enable testing for SIWE users with just one button click

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Apr 22 2024, 11:46 AM
Harbormaster failed remote builds in B28364: Diff 39354!

I think you should rename the generic backup_message to siwe_backup_message

LGTM but maybe @bartek could double check Rust code

native/native_rust_library/src/backup.rs
55–66 ↗(On Diff #39388)

I think this could be simplified a bit

shared/backup_client/src/lib.rs
74–77 ↗(On Diff #39388)

but you'll have to make form mutable

This revision is now accepted and ready to land.Apr 26 2024, 12:36 AM
native/backup/use-client-backup.js
54 ↗(On Diff #39388)

is there a way to make it nullable to avoid mocking with an empty string?

native/backup/use-client-backup.js
54 ↗(On Diff #39388)

Actually this is a bit problematic. We can change the types so that user passes undefined or null instead of empty string. However the user MUST explicitly pass undefined or null - otherwise JSI will crash. Unfortunately if we change the type in CommCoreModuleJSISchema.js to sth like this:

+createNewBackup: (
    backupSecret: string,
    siweBackupMsg: ?string,
  ) => Promise<void>;

Flow won't complain if the user doesn't undefined explicitly so it won't prevent potential crash. Therefore it is best to create new JSI specifically for siwe backup creation.

P.S. JSI crashes since it accesses arguments by accessing array. So if the user doesn't provide anything for backup message JSI will try to access out of array bounds.

  1. Rename msg_backup to siwe_backup_msg
  2. Introcuce separate backup creation JSI calls for SIWE and password users to avoid macking empty string or potential crashes if dev forgets to specify null