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

Authored by michal on Dec 8 2023, 8:35 AM.

Details

Reviewers

bartek
kamil
varun

Commits

rCOMM3fa26e7d7165: [native-rust-library] Upload/download user backup

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

michal created this revision.Dec 8 2023, 8:35 AM

Herald added subscribers: tomek, ashoat. · View Herald TranscriptDec 8 2023, 8:35 AM

michal added inline comments.Dec 8 2023, 8:38 AM

native/backup/use-client-backup.js
44–48	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	We no longer have specific errors for each step except for data integrity becase The code runs on Rust now so it's harder to test This code was useful for testing the backup service but now it we know that it works If any of these steps fails than data integrity will also fail so we still check everything

michal edited the summary of this revision. (Show Details)Dec 8 2023, 8:40 AM

michal added a child revision: D10265: [native/lib] Cleanup existing JS backup code.Dec 8 2023, 8:43 AM

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!

michal requested review of this revision.Dec 12 2023, 2:12 AM

Rebase

Harbormaster completed remote builds in B24987: Diff 34527.Dec 12 2023, 4:29 AM

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 ↗	(On Diff #34527)	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 ↗	(On Diff #34527)	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 ↗	(On Diff #34527)	shouldn't we add `try {} catch(e) {}`?

This revision is now accepted and ready to land.Dec 15 2023, 4:59 AM

michal added inline comments.Jan 3 2024, 6:34 AM

native/backup/use-client-backup.js
44–48 ↗	(On Diff #34527)	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 ↗	(On Diff #34527)	Any exceptions are caught in the calling function (in backup menu).