daostack / migration

A repo for handeling the migration of DAOstack contracts and DAOs.
https://www.npmjs.com/package/@daostack/migration
GNU General Public License v3.0
17 stars 17 forks source link

use mnemonic instead of deterministic to maintain consistent account addresses #20

Closed dkent600 closed 5 years ago

dkent600 commented 5 years ago

Anyone using MetaMask with an app use arc.js and running against ganache is going to find that the ganache accounts they have imported into MM are no longer valid. These users will thus have to import new accounts. The need to do this will not be intuitive to the user, but there is an easy solution to help avoid this pain.

(Also affected would be any other dependencies on the ganache account mnemonic/addresses.)

The problem arrises because in order to migrate contracts to the same addresses as the migration repo does, one has to use the "--deterministic" flag, just like the migration repo does. But this results in different account addresses than arc.js has produced in the past, because the mnemonics used in the two cases are different.

So I request that the migration repo use the "--mnemoic" flag instead of "--deterministc". These are equivalent. The mnemonic to maintain consistent account addresses, is: "behave pipe turkey animal voyage dial relief menu blush match jeans general".

There is an additional advantage to doing this: We avoid the risk that ganache changes the mnemonic used by "-deterministic" -- the risk that users would again have to reimport their accounts or otherwise alter their dependencies on test user account addresses.

dkent600 commented 5 years ago

PS -- also consider supplying a constant "networkId" to ganache. This would enable people to identify the network as coming from the migration repo. Arc.js has been using an id of "1512051714758", and it would be good to adopt that, again, for consistency across releases.

jellegerbrandy commented 5 years ago

The mnemonic for the --deterministic option is myth like bonus scare over problem client lizard pioneer submit female collect

jellegerbrandy commented 5 years ago

Can you make a separete issue for the networkId? I agree completely with your remark, but I cannot get it to work.

dkent600 commented 5 years ago

@jellegerbrandy

We should use "behave pipe turkey animal voyage dial relief menu blush match jeans general" to maintain the account addresses that have been generated until now by arc.js.

Be sure you run the migration from a fresh instance of ganache -- the resulting addresses will be different each time you migrate thereafter.

dkent600 commented 5 years ago

@jellegerbrandy For networkId: https://github.com/daostack/migration/issues/30

jellegerbrandy commented 5 years ago

We are using the --deterministic accounts now through the whole stack, and also alchemy and arcjs (within alchemy) uses those addresses. I pushed for that a few weeks ago (with the argument that it is better to use default values than use or custom defined ones). Given that this all works, I'd say we do not go back.

dkent600 commented 5 years ago

@jellegerbrandy @tsuberim

The community may not agree that it "all works" when they will find their ganache accounts are no longer valid.

Using a custom mnemonic has the further advantage that we have control over the accounts. Is there any guarantee that the mnemonic for deterministic will never change?