Page MenuHomePhabricator

[native-rust-library] Upload/download user backup
ClosedPublic

Authored by michal on Dec 8 2023, 8:35 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Dec 26, 10:22 PM
Unknown Object (File)
Mon, Dec 23, 9:46 AM
Unknown Object (File)
Mon, Dec 23, 9:46 AM
Unknown Object (File)
Mon, Dec 23, 9:46 AM
Unknown Object (File)
Mon, Dec 23, 9:46 AM
Unknown Object (File)
Mon, Dec 23, 9:46 AM
Unknown Object (File)
Fri, Dec 20, 1:48 PM
Unknown Object (File)
Fri, Dec 20, 1:47 PM
Subscribers

Details

Summary

ENG-5345 and ENG-5562

Moves the upload/download logic from JS code to rust code. Uses the previously introduced backup client. The JS code that is no longer needed will be removed in the next diff but I wanted to keep this one smaller.

We no longer return encrypted values up to JS from Rust, and we don't send the downloaded encrypted bytes down to Rust as now it can handle download/upload by itself.

Depends on D10263

Test Plan

Tested on physical iOS and Android device:

  • enable backups
  • test restoring the backup -> data integrity == true
  • change user store (send a friend request)
  • test restoring the backup -> data integrity == true

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

native/backup/use-client-backup.js
44–48 ↗(On Diff #34441)

This is for testing purposes only, as we currently don't set the auth metadata anywhere. This will only run if someone enables backups (which only staff can do).

87–94 ↗(On Diff #34441)

We no longer have specific errors for each step except for data integrity becase

  1. The code runs on Rust now so it's harder to test
  2. This code was useful for testing the backup service but now it we know that it works
  3. If any of these steps fails than data integrity will also fail so we still check everything
Harbormaster returned this revision to the author for changes because remote builds failed.Dec 8 2023, 9:01 AM
Harbormaster failed remote builds in B24907: Diff 34441!
bartek added 1 blocking reviewer(s): kamil.

LGTM but I'd like @kamil to take a look as well, he has the most context on how this should work

LGTM

native/backup/use-client-backup.js
18

wondering if we should update the call site as this now returns a much simpler value than previously, but probably it's okay as it is

44–48

I would add a comment here just to remember that currentUserID should be a value from Identity, not the value we have in store assigned by keyserver because I am afraid this could cause some confusion or bugs when someone enables backup after we enable Identity and could override proper value - but still, it's only for testing purposes so probably it's okay

68–69

shouldn't we add try {} catch(e) {}?

This revision is now accepted and ready to land.Dec 15 2023, 4:59 AM
native/backup/use-client-backup.js
44–48

I think these ids are going to be the same in the end. I will create a linear task for removing this code anyway.

68–69

Any exceptions are caught in the calling function (in backup menu).