get10101 / itchysats

CFDs on Bitcoin.
https://itchysats.network
MIT License
61 stars 17 forks source link

Make it easier to back up the seed-file #1065

Closed da-kami closed 2 years ago

da-kami commented 2 years ago

After our discussion in https://github.com/itchysats/itchysats/issues/865 and #1016 we dropped the feature to backup and re-initialize the ItchySats (wallet) seed.

Allow users of the binary to backup their seed in textual form (so they don't have to backup a complete file, the generated seed-file is still binary), so they can save it in e.g. a password safe.

thomaseizinger commented 2 years ago

Related discussion: https://github.com/itchysats/itchysats/discussions/1088

bonomat commented 2 years ago

I think this is somewhat important for our current focus of providing integrations for all the nodes.

It's not possible to move from our current seed to a seed phrase (bip39), so for our current solution we would need a different backup solution.

klochowicz commented 2 years ago

should we consider migrating to bip39, at least for new users? (it could also tie in with providing better randomness when creating wallet, or optionally initiating one from an externally generated seed words).

bonomat commented 2 years ago

should we consider migrating to bip39, at least for new users?

I think it's a good idea but we can't break backwards compatibility and can't convert the old key to bip39. I propose the following:

1) simply keep what we have and let the user download the taker_seed through the frontend

or

2) make use of bip39 and provide a path to upgrade

if first start (no taker_seed) exists, we start with an empty wallet.

if there is a seed file present we:

bonomat commented 2 years ago

How is it going with this?

klochowicz commented 2 years ago

I have investigated it a bit, and came to the following conclusions (please correct me if I'm wrong)

Moreover, as we don't really have binary users pre-electron bundle, we could simply make our new seed-phrases BIP-39 compatible and allow for easy backup-restore solution. (note: restoring could also allow for using externally generated seeds).

As for where I'm at, I'm right now experimenting with generating BIP-39 compatible seed and then will proceed to allowing its easy export (I figured out that this could be a first reviewable step).

bonomat commented 2 years ago

I almost agree :) Some inline comments:

I have investigated it a bit, and came to the following conclusions (please correct me if I'm wrong)

* bip39 feature would be intended only for binary users (more precisely, wherever we are _not_ initialising the wallet using `--app-seed` parameter). 

Yes, for all cases where we do not pass in --app-seed.

Ideally, for Umbrel etc we would integrate with the provided wallet instead of having a wallet altogether. Do you mean this feature?

Especially restoring via the UI from a seed other than Umbrel seed would be very dangerous for the user, as they would need to keep backups of multiple seeds, greatly increasing changes of something going wrong

👍 Meaning, if --app-seed was provided we could disable the wallet backup feature.

* I could see some appeal in "exporting" seed from ItchySats running on umbrel, but then BIP-39 word list is what we really need (also, simply withdrawing to a different wallet is an arguably superior option, and it's there already).

I don't think this is possible due to the same reason we can't generate a BIP-39 seed phrase out of our current seed.

Moreover, as we don't really have binary users pre-electron bundle, we could simply make our new seed-phrases BIP-39 compatible and allow for easy backup-restore solution. (note: restoring could also allow for using externally generated seeds).

We do, at this time: RaspiBlitz uses the binary and seed file. I don't know about all the other nodes but --app-seed sounds unique to Umbrel to me.

As for where I'm at, I'm right now experimenting with generating BIP-39 compatible seed and then will proceed to allowing its easy export (I figured out that this could be a first reviewable step).

Again, you can't generate a BIP-39 seed from our current seed. You can create a new wallet using a seed phrase. To be brutal, this is actually not the ticket. The ticket talks about backing up the wallet seed. This could either be done by exporting the taker_seed file or by exposing the wallet private key.

klochowicz commented 2 years ago

Thank you for bringing up RaspiBlitz, I completely forgot about these users.

I was not generating BIP-39 from our current seed (that's impossible), I was trialling a separate one for the new wallet.

l'll leave BIP-39 discussions out of this ticket, as the ticket can be addressed without it at all. I will go with simply exporting taker_seed file.

bonomat commented 2 years ago

oh, I might have misunderstood you then :) Please excuse.

l'll leave BIP-39 discussions out of this ticket, as the ticket can be addressed without it at all. I will go with simply exporting taker_seed file.

Sounds good. I think this is a good idea.