Skip to content

feat: new backup transfer method#3489

Closed
dignifiedquire wants to merge 29 commits into
masterfrom
iroh-share
Closed

feat: new backup transfer method#3489
dignifiedquire wants to merge 29 commits into
masterfrom
iroh-share

Conversation

@dignifiedquire

@dignifiedquire dignifiedquire commented Jul 7, 2022

Copy link
Copy Markdown
Collaborator

⚠️ There be dragons 🐉
There be exciting things

Idea: use p2p based transfer methods to move backups from one device to the other.

Implementation: Use iroh a new implementation of IPFS to build iroh-share, a simple protocol that moves data from device a to device b.

Local Testing

Using the new commands send-backup and receive-backup <Ticket> you can test this on a local machine or between two machines running the repl example.

Current Restrictions

  • Both devices must be in the same network to discover each other

TODOs

Before Merging

  • finish iroh-share
  • test on very large files
  • add progress bar
  • add indicator for finishing
  • review current security properties
  • fixup todos
  • add documentation
  • finish C-api

Future

  • improve iroh-shares hole punching so it does not need to be on the same network anymore
  • add python bindings
  • add node bindings

Comment thread src/blob.rs
pub fn suffix(&self) -> Option<&str> {
let ext = self.name.rsplit('.').next();
if ext == Some(&self.name) {
if ext == Some(self.name.as_str()) {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to make these changes because things would suddenly not compile anymore otherwise, not sure what happened there, seems like a compiler issue 🤷

@Simon-Laux

Simon-Laux commented Jul 10, 2022

Copy link
Copy Markdown
Member

In which direction does this go? only "scan to import" for now?
Though I guess it's probably easy to switch directions once the connection works at all (to also allow scan code from computer (when computer has no camera) to export from phone to that computer)

@dignifiedquire dignifiedquire changed the title [WIP] feat: new backup transfer method feat: new backup transfer method Aug 29, 2022
@dignifiedquire dignifiedquire requested review from link2xt and r10s August 29, 2022 12:46
@dignifiedquire dignifiedquire marked this pull request as ready for review August 29, 2022 13:03
@link2xt

link2xt commented Sep 10, 2022

Copy link
Copy Markdown
Collaborator

librocksdb takes forever to build, why is it needed as a dependency? Is it possible to disable it as a feature somehow?
I see iroh supports different stores: https://github.com/n0-computer/iroh/tree/main/stores

Comment thread Cargo.toml Outdated
Comment thread Cargo.toml Outdated
Comment thread Cargo.toml Outdated
Comment thread deltachat-ffi/Cargo.toml
Comment thread deltachat-ffi/deltachat.h
Comment on lines +2612 to +2652
* The path must exist and being accessible at least until the other device has received all data.
* The folder is not cleaned up by the backup send;
* a temporary directory seems to be good place therefore.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a UI developer, but this seems a bit awkward and something that the core could all handle internally?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have strong opinions in either direction @r10s what do you think?

@r10s r10s Sep 23, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in general, it is nicer if the UI would not need to take care about such details.

but not sure how easy it really is for the core to really fulfill the requirements. so, as a pragmatic approach, i am also fine with leaving things up to the UIs for now, where we can more safely pass a directly that is really handled by the OS.

Comment thread deltachat-ffi/deltachat.h
* The path must exist and being accessible at least until the other device has received all data.
* The folder is not cleaned up by the backup send;
* a temporary directory seems to be good place therefore.
* @param passphrase Used to at-rest-encrypt the backuped database in `folder` before sending,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @param passphrase Used to at-rest-encrypt the backuped database in `folder` before sending,
* @param passphrase Used to at-rest-encrypt the backup in `folder` before sending,

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no this is correct, only the database is encrypted

Comment thread deltachat-ffi/deltachat.h
* similar to dc_imex().
* The same passphrase must be given to dc_receive_backup() on the other device.
* If NULL or empty string is given, the sent backup is not encrypted at rest.
* The transport of the backup is always encrypted additionally.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the transport is encrypted why is this passphrase still needed? Would be nice if this described what it is protecting against as well.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This passphrase is there to encrypt the sql database. It is the same as the existing other backup methods. I personally would love to drop it, but didn't want to open that can of worms.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @r10s

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, i'd also better leave that can of worms closed, at least for now :)

Comment thread deltachat-ffi/deltachat.h


/**
* Waits for the sending to finish and frees the backup sender object.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is thee a way to abort the sending?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or resume any time later (incl. never)?

Comment thread deltachat-ffi/deltachat.h
* but on a device that has scanned the QR code generated by dc_backup_sender_qr().
*
* While dc_receive_backup() returns immediately, the started job may take a while;
* you can stop it using dc_stop_ongoing_process().

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An ongoing process is IIRC a global modal that can not be dismissed. I guess this is normally done from the "create account" screen or similar so that is what is desired?

Comment thread examples/repl/cmdline.rs Outdated
Comment thread src/imex.rs
Comment thread src/imex.rs
Comment thread src/imex.rs Outdated
Comment thread src/imex.rs Outdated
Comment thread src/imex.rs
Comment thread src/imex.rs
Comment thread src/imex.rs
Comment thread src/imex.rs Outdated
Comment thread src/imex.rs
@dignifiedquire

Copy link
Copy Markdown
Collaborator Author

librocksdb takes forever to build, why is it needed as a dependency? Is it possible to disable it as a feature somehow?
I see iroh supports different stores: https://github.com/n0-computer/iroh/tree/main/stores

@link2xt no it is not optional, those stores are just for experimentation. RocksDB is the only supported DB in iroh atm

@flub flub left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

foo-shm and foo-wal are checked in accidentally i think.

CI isn't happy yet, but i think this is probably mostly fine

@dignifiedquire

Copy link
Copy Markdown
Collaborator Author

foo-shm and foo-wal are checked in accidentally i think.

ups, fixed

@dignifiedquire

Copy link
Copy Markdown
Collaborator Author

iroh requires rust1.63, so this bumps the msrv for dc

@r10s

r10s commented Mar 6, 2023

Copy link
Copy Markdown
Contributor

closing, this is superseded by #4007

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants