feat: Add backup and restore #1562

holzeis · 2023-11-10T14:32:13Z

Backup and restore

When you do things right, people won't be sure you've done anything at all

Introduces the TenTenOneStorage layer encapsulating the rust-lightning and rust-dlc storage.
Introduces a 10101 DlcStorageProvider trait with a SledStorageProvider and a InMemoryDLCStoreProvider implementation introducing an abstraction layer for storage.
Removes the simple-wallet and dlc-sled-storage-provider crates.
Makes the ln_dlc_node crate indifferent about the used storage, allowing us to use in-memory implementations of rust-lightning and rust-dlc data for the tests.
Introduces a RemoteBackupClient implementation that sends backups asynchronously to the coordinator.
Backups are symmetrically (aes256-gcm-siv) encrypted and decrypted on the client side. The key is derived from the users secret key. The coordinator only receives and stores encrypted data.
The users intent for creating a backup or restoring a backup is achieved by verifying a signature on the backup (value) or restore (node_id) message.
Adds the rusqlite dependency to create a read-only backup connection, that creates an online backup after the following events: PaymentClaimed, PaymentSent, PaymentFailed, PositionUpdateNotification, PositionClosedNotification, OrderUpdateNotification, OrderFilledWith, SpendableOutputs.
The user backups are stored on the coordinator in a sled database in the subdirectory user_backups.

Minor onboarding UI/UX improvements

The user is not asked for an email address when restoring from a backup.

Removed app bar from welcome screen

Added visual improvements on the loading screen
Wait for restore / initial backup to complete on loading screen
Remove transitions from initial screens
Remove splash screen once on the loading screen

Compatibility

The change is fully compatible with older versions. Tested with an upgrade from 1.5.0. However, for the upgrade path we have to run a full backup at startup. This is achieved through a preference setting, which is by default true unless the user is creating a new wallet or restores a wallet.

The dlc-store of the actions did not adhere to the same pattern as other data stored in the dlc sled storage. (didn't use a tree, but rather stored the data directly into the sled db). I opted to ignore, that since handling that case would have made the implementation more complex, while the actions can be reconstructed in a on_channel_reestablish event.

Testing

Manually tested the upgrade path from a pre-backup version. (see video)
Added an e2e test for restoring from a backup. Note, I had to make the storage mutable to update the states after stopping the app. This required me to introduce some unsafe code e1ac6f2. This actually only needed for testing, but I wasn't able to compile the corresponding unsafe parts only for #[cfg(test)] as the test are always compiled for #[cfg(not(test))].
Copied the sled database tests from rust-dlc, but noticed that they do not cover all scenarios. I found quite some issues during manual testing. However, my confidence in our SledStorageProvider is pretty high.
Introduced TenTenOneInMemoryStorage for the ln_dlc_node crate tests, meaning that no rust-lightning or rust-dlc data is stored to disk during those tests.

Future Work

There is no requirement for the coordinator to provide that API. The backup and restore API could be scaled nicely distributing the load.
The backups are stored into a sled database. We might want to consider using a managed nosql database for storing user backups. e.g. a managed MongoDB.
Preferences are not backed up. We might want to revisit that if we are storing more important data in the preferences.

Demo

Start the app from latest main
Fund the app and opens a position
Stop everything
Checkout feat/backup-app
Start everything up again
Close and open a position.
Copy seed phrases
Delete the app
Install the app again
Restore app from seed phrase
Close restored position

backup_and_restore.mov

bonomat

Only went over the first commit so far... will continue later.

coordinator/src/backup.rs

coordinator/src/orderbook/websocket.rs

bonomat · 2023-11-10T21:34:56Z

coordinator/src/routes.rs

@@ -542,3 +553,55 @@ pub async fn collaborative_revert_confirm(
    })?;
    Ok(Json(serialize_hex(&raw_tx)))
 }
+
+// TODO(holzeis): There is no reason the backup and restore api has to run on the coordinator. On
+// the contrary it would be much more reasonable to have the backup and restore api run separately.


Definitely. Maybe we should create from the getgo at least a different subdomain/path/port for the backup to be able to move it.

As we are already using a reverse proxy, we could simple direct any request to /api/backup or /api/restore to a different service once we move it. This could be completely transparent to the app.

The problem is though that we are hardcoding the IP in the app. If we were to use backup.10101.finance instead, we could move it indepdently.

The problem is though that we are hardcoding the IP in the app.

Agreed, but this is unrelated to this change. e.g. we could change the configuration to use the domain instead of the ip. e.g. api.10101.finance and then configure the reverse proxy to route any traffic on api.10101.finance/api/backup to another service.

One caveat I see in using a dedicated subdomain is that this would introduce another configuration. What benefits do you see in using a dedicated subdomain?

It will be hard to roll this out at a later stage or move the backup to a new server without breaking existing apps.

i.e.

user_a runs version 1.6.0 and backs-up his app on 10101.finance

we migrate the backup service to backup.10101.finance

user_a cannot create/restore a backup before upgrading to the new version

I guess breaking changes are OK. Are we handling errors correctly, i.e. what if the backup service is not working (e.g. for an old version because we moved it already)?

we migrate the backup service to backup.10101.finance
user_a cannot create/restore a backup before upgrading to the new version

I don't see an issue here. We can simply add an additional nginx config that points all the backup/restore requests from user_a to the new service.

But, let's not argue about that. If you feel strongly about it. I can add it as a follow up to this PR.

I don't see an issue here. We can simply add an additional nginx config that points all the backup/restore requests from user_a to the new service.

Can we do this if the new service runs on a different machine/ip?

Yes, we would need to configure a redirect then.

coordinator/src/routes.rs

coordinator/src/backup.rs

coordinator/src/storage.rs

crates/ln-dlc-storage/src/lib.rs

crates/orderbook-client/src/lib.rs

mobile/lib/util/file.dart

mobile/native/src/config/api.rs

mobile/native/src/ln_dlc/mod.rs

mobile/native/src/storage.rs

bonomat · 2023-11-11T23:36:38Z

mobile/lib/common/loading_screen.dart

@@ -38,25 +39,27 @@ class _LoadingScreenState extends State<LoadingScreen> {
      if (isSeedFilePresent) {
        runBackend(context).then((value) {
          logger.i("Backend started");
-        });
+
+          if (!hasEmailAddress) {


Given the size of the PR, I think these changes should be in its own PR.

Hmm.. you mean the full backup could have been separated into a separate PR?

bonomat · 2023-11-11T23:40:55Z

mobile/lib/common/loading_screen.dart

+          if (isFullBackupRequired) {
+            setState(() => message = "Creating initial backup!");
+            fullBackup().then((value) {
+              Preferences.instance.setFullBackupRequired(false).then((value) {
+                start(context, position);
+              });


The logic in this file is getting out of hand :(

Agreed 🙈. We probably should use some kind of wizard here.

mobile/lib/common/loading_screen.dart

mobile/lib/common/routes.dart

holzeis · 2023-11-13T08:56:01Z

Added an e2e test for restoring from a backup. Note, I had to make the storage mutable to update the states after stopping the app. This required me to introduce some unsafe code e1ac6f2. This actually only needed for testing, but I wasn't able to compile the corresponding unsafe parts only for #[cfg(test)] as the test are always compiled for #[cfg(not(test))].

Addressed with 168fcf4

bonomat

Pretty awesome. I tried it and it works like magic 🪄🚀🌕

luckysori

LGTM except for:

Encrypted backup scheme.
Numerous type parameters and trait bounds added in ln-dlc-node. Maybe unavoidable for now, but it makes me feel uneasy.

I did not review some of the unrelated changes to the main feature.

luckysori · 2023-11-13T04:36:01Z

Cargo.toml

-dlc-sled-storage-provider = { git = "https://github.com/p2pderivatives/rust-dlc", rev = "4e104b4" }
 p2pd-oracle-client = { git = "https://github.com/p2pderivatives/rust-dlc", rev = "4e104b4" }
 dlc-trie = { git = "https://github.com/p2pderivatives/rust-dlc", rev = "4e104b4" }
-simple-wallet = { git = "https://github.com/p2pderivatives/rust-dlc", rev = "4e104b4" }


🙃 It would have been nice to have a separate patch replacing these dependencies without changes. Then it's easier to judge the diff.

coordinator/src/backup.rs

mobile/native/src/storage.rs

mobile/native/src/cipher.rs

crates/tests-e2e/tests/restore_from_backup.rs

luckysori

LGTM!

### Replace rust-dlc SledStorageProvider with DlcStorageProvider This removes the dependency to the `dlc-sled-storage-provider` and the `simple-wallet`, however, it does not change the underlying implementation. The new DlcStorageProvider is simply introducing a new abstraction layer to allow for replacing sled with another implementation in preparation to hook in the backup. ### Add data dir to config Note, I didn't put it on the environments on the client side as it requires the context to be initialized and that would have meant bigger changes, than I wanted to deal with. ### Verify users intent The users intent is verified by verifying the signature on the backup and restore messages. ### Symmetric backup cipher Backups are encrypted and decrypted on the client side. ### Add full backup support If the app is upgraded to the new version it will run an initial full backup to ensure that the backup is complete. - Combine states into a single state file For testing I had to make the state mutable as otherwise I wasn't able to start a new app node. ### Improve the app onboarding flow - The user should not be required to input the email when restoring from a backup. Note, the solution is a bit hacky, and ideally we also backup the preferences. - Removed app bar from welcome screen - Added visual improvements on the loading screen - Wait for restore / initial backup to complete on loading screen - Remove transitions from initial screens - Remove splash screen once on the loading screen

This prevents the candlestick widget not being sometimes not initialized

Setting viewportFraction: 1.0 uses the full space of the screen instead of only 80% (default).

holzeis requested review from bonomat and luckysori November 10, 2023 14:32

holzeis self-assigned this Nov 10, 2023

holzeis force-pushed the feat/backup-app branch from 1184c31 to bf091de Compare November 10, 2023 14:56

holzeis linked an issue Nov 10, 2023 that may be closed by this pull request

Create app backup #1098

Closed

4 tasks

holzeis force-pushed the feat/backup-app branch 2 times, most recently from 0a1214b to bfae40b Compare November 10, 2023 21:15

bonomat reviewed Nov 10, 2023

View reviewed changes

holzeis force-pushed the feat/backup-app branch from bfae40b to 37b6cbe Compare November 11, 2023 21:41