textileio / ios-textile

[DEPRECATED] iOS bindings for https://github.com/textileio/go-textile
MIT License
10 stars 6 forks source link

allows seed to be set during init #19

Closed andrewxhill closed 4 years ago

andrewxhill commented 5 years ago

@asutula kinda stab in the dark here, but I believe this would do it.

could we slip this into an upcoming update?

asutula commented 5 years ago

I'm trying to think through this a bit, and I'm not sure it's that simple. The way the this PR and the underlying go-textile code works, the seed is only taken into account when the repo is initialized the very first time. If you pass in a different seed at a later time, it is ignored. This would be a confusing API. Seems like we should just support switching accounts by passing in a different seed, but of course that could take work in both the sdk and go layers.

asutula commented 5 years ago

If you just need the ability to initialize with a seed the first time ASAP, I guess let's do something like this for now and just make a priority to sort out what i described in the previous comment.

andrewxhill commented 5 years ago

my thinking here was that if i passed this api a different repo location and seed, it would initialize a second account. or if i first stopped the node, removed the repo and init with a seed it would create a new repo for me. the client could simply delete the old repo location, as it's responsible for choosing that anyway. this would unblock the notes pairing

andrewxhill commented 5 years ago

What's the plan here? This is probably required for the getting started guide, as we don't want to encourage people to create wallets from scratch on the device unless they know impact down the road (difficulty in migrating primary wallet out of the app)

asutula commented 5 years ago

Seems like the plan needs to be to make a plan and act on it. There's a good chance I'm over-complicating this because of lack of understanding about the wallet (please let me know), but it seems to me like this is going to take some coordinated work between the sdks and go-textile. Here are a few questions that come to mind:

  1. You speak of passing in a seed. How did you ever get that seed to begin with? None of our apis return it ever.
  2. This suggests we may need some sort of Wallet api to access things like account seeds
  3. Unless we persist information about accounts that were already created on a device, we'll need the recovery phrase in order to get the wallet and account seeds. Should we be talking about recovery phrases at all?
  4. If I set up a new device, it seems like recovery phrase is the only option to recover my wallet accounts because any persistence about previously created accounts doesn't exist.
asutula commented 5 years ago

Oh I should add to that, I think any file system management that happens because of switching accounts should be done in the go layer.

andrewxhill commented 5 years ago

Sure, but like sander said we can build the APIs we want without having to full encode them in the node first. So what I want, is the ability to pass the pairing credentials from the desktop node to the Textile Notes app. I will do that via a QR code so the SDK doesn't need to do anything there.

Upon receipt, I plan to power down the local node and wipe the local repo (again handled by client here to just getting it working this first time).

Next I will run a new Textile.initialize, but I now need a way to pass in the credentials I got via the QR code.

sanderpick commented 5 years ago

You can get the seed. https://github.com/textileio/go-textile/blob/master/mobile/account.go#L21

asutula commented 5 years ago

Ok cool. Yea I guess the pairing use case avoids all my questions about recovery and wallet api.

Is the only credential you need to pass in the seed?

I think I see the flow in the sdk initializer that would work. Basically:

  1. Create the node like normal (this ignores any seed you passed in)
  2. Once it exists (or is started?), query for the existing seed.
  3. If the new seed is different than the existing seed, stop the node, delete the repo, re-init with new seed
asutula commented 5 years ago

Yea @sanderpick you can get the seed for the current repo, but not for other accounts. But that isn't needed for this pairing use case, so all good.

asutula commented 5 years ago

@sanderpick does the node need to be started to query for the seed? Or can it just be created?

sanderpick commented 5 years ago

The below issues are backlogged, but we can resurrect as needed. I agree we'll want to have the file system management live in the go-layer for account switching / deletion. But, I don't see an issue w/ an interim solution handled by the client so long as we can agree on the filesystem layout. I suggest doing is just like @carsonfarmer did on desktop.

re/ https://github.com/textileio/go-textile/issues/468 re/ https://github.com/textileio/go-textile/issues/469

sanderpick commented 5 years ago

@asutula -> https://github.com/textileio/go-textile/blob/master/mobile/account.go#L22

asutula commented 5 years ago

Ok. Makes it a little more tricky, cool.

sanderpick commented 5 years ago

@andrewxhill I would consider diving into these wallet api / account deletion issues. I feel like that would be more efficient than you doing the JS work, then me doing the Go work, when I think you could just do the Go work. Thoughts?

asutula commented 5 years ago

When taking into account this account switching stuff, the current initialize method feels super clunky.

I think what you started here is the right idea... initialize should just take a seed and not care where it came from. That should be the only initialize method.

That fulfills your paring use case.

Then it's a matter of providing a seed to a user that isn't bringing their own seed from a different source. That could be the first super basic implementation of a wallet API... Just create a wallet and return the account for the 0 index. We can extend that later.

sanderpick commented 5 years ago

I think what you started here is the right idea... initialize should just take a seed and not care where it came from. That should be the only initialize method.

The current init method does just take a seed. What would change?

Then it's a matter of providing a seed to a user that isn't bringing their own seed from a different source. That could be the first super basic implementation of a wallet API... Just create a wallet and return the account for the 0 index. We can extend that later.

Doesn't that already exist? You can create a wallet. You can get the seed at index 0.

asutula commented 5 years ago

Right, all that functionality does already exist, but it's all wrapped up inside the initialize method... We don't expose it to the sdk user at all. So what I'm suggesting and what @andrewxhill started here would expose more of it in the public sdk api.

sanderpick commented 5 years ago

Ahhhhh, ok 👍👍

andrewxhill commented 5 years ago

another bit to be clear here...

if you init with no seed, you get back a wallet seed correct? we still should be creating a secondary account seed to use as the primary in the app or not?

when you pair, you will be getting an account seed, not a wallet seed, correct?

just trying to get that straight because i think initializing with wallet versus account would lead to different outcomes if i have it straight in my head. and we probably only want to support initializing with an account seed, but if one isn't provided we would give them back both an account and a wallet?

asutula commented 5 years ago

@andrewxhill could we start this by assuming that any call to initialize with a seed is a pairing request? In that case, we could just stop the node, delete the repo, and re-initi with the seed without making it any more complicated than that.

andrewxhill commented 5 years ago

yeah same page, that would be my vote too. we may never have to support the other. but in the inverse direction, when you init from nothing, i'm curious what really should be reported to the users (wallet seed AND account seed?)

asutula commented 5 years ago

I think what you're calling a "wallet seed" is what I was calling a "recovery phrase".

The current initialize returns the recovery phrase. The account seed is derived, used to initialize the repo, then forgotten.

Your questions about which we should be returning and using would be ones for @sanderpick .

asutula commented 5 years ago

Not knowing much about the security implications of this idea, but it seem like the long term solution is for go-textile to keep a table of all the accounts (and their seeds) created on a device. Then the Wallet API would expose those to the sdk user so they could choose which account seed to initialize with.

I guess the short term band aid for that is to return the account seed as well as the recovery phrase and make it the sdk user's responsibility to keep track of the account seed for now.

andrewxhill commented 5 years ago

I see... yeah maybe no need to return account stuff at init time then.