AlphaWallet / alpha-wallet-ios

An advanced Ethereum/EVM mobile wallet
https://www.alphawallet.com
MIT License
598 stars 373 forks source link

Importing a seedphrase only imports the first derived account #6383

Open nicexe opened 1 year ago

nicexe commented 1 year ago

name: Bug Report about: Using AlphaWallet, but it's not working as you expect?


Describe the bug When importing a seed phrase, only the first account of the derivation tree is imported.

Steps to reproduce (REQUIRED)

  1. Import from a seed phrase (you can use test test test test test test test test test test test junk)
  2. Check the imported account (should be 0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266)
  3. Add new wallet and check its seed phrase

Expect 4a. Both accounts should have the same seed phrase (test test test test test test test test test test test junk)

Observed 4b. The second account created has a different seed phrase than the one used in step 1 (not test test test test test test test test test test test junk)

Screenshots IMG_5742 IMG_5743 IMG_5744 IMG_5745 IMG_5746 IMG_5747 IMG_5748 IMG_5749 IMG_5750 IMG_5751 IMG_5752 IMG_5753

Version or build number number Version number: 3.61 (471)

nicexe commented 1 year ago

The first addresses derived from the seed phrase should be (in this specific order):

0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266
0x70997970C51812dc3A010C7d01b50e0d17dc79C8
0x3C44CdDdB6a900fa2b585dd299e03d12FA4293BC
0x90F79bf6EB2c4f870365E785982E1f101E93b906
0x15d34AAf54267DB7D7c367839AAf71A00a2C6A65
0x9965507D1a55bcC2695C58ba16FB37d819B0A4dc
0x976EA74026E726554dB657fA54763abd0C3a0aa9
0x14dC79964da2C08b23698B3D3cc7Ca32193d9955
0x23618e81E3f5cdF7f54C3d65f7FBc0aBf5B21E8f
0xa0Ee7A142d267C1f36714E4a8F75612F20a79720
0xBcd4042DE499D14e55001CcbB24a551F3b954096
0x71bE63f3384f5fb98995898A86B02Fb2426c5788
0xFABB0ac9d68B0B445fB7357272Ff202C5651694a
0x1CBd3b2770909D4e10f157cABC84C7264073C9Ec
0xdF3e18d64BC6A983f673Ab319CCaE4f1a57C7097
0xcd3B766CCDd6AE721141F452C550Ca635964ce71
0x2546BcD3c84621e976D8185a91A922aE77ECEc30
0xbDA5747bFD65F08deb54cb465eB87D40e51B197E
0xdD2FD4581271e230360230F9337D5c0430Bf44C0
0x8626f6940E2eb28930eFb4CeF49B2d1F2C9C1199
hboon commented 1 year ago

Hi @nicexe

Our existing UI creates and imports multiple seeds (and seed phrase) and only generate the 0th account for each.

For context, it's a bit of a trade off: when we added support HD wallet support a long time ago, we already supported raw keys and Keystore/JSON files and if we generated multiple addresses per seed (admittedly like how they were used to be used), the UI could be potentially overwhelming as we also want to let users import and create multiple seeds.

We also always use the Ethereum mainnet coin type = 60, see https://github.com/AlphaWallet/alpha-wallet-ios/blob/47d406ec521738b586f507ed163018b61d780adc/modules/AlphaWalletFoundation/AlphaWalletFoundation/KeyManagement/EtherKeystore.swift#L867, which again is not how it was meant to be.

Interestingly, we haven't heard much about users wanting to be able to derive/generate other addresses or about the coin type being fixed. Would you help elaborate a little — Do you actively see out this feature in mobile wallets, etc?

nicexe commented 1 year ago

In the case of Metamask Mobile, they also have coin type 60 (that is not configurable) but the user on-boarding is a bit different.

They force the user to create (generate a new seed-phrase) or import an existing wallet (using a seed-phrase, not a raw private key) and only after there is a seed-phrase set the user can import raw private keys or keystore/json files. Creating a new account gets the next index from the HD wallet (from the seed-phrase initially set or restored).

The downside is that on Metamask Mobile you can only have one seed-phrase set and every new account you create is derived from that. You can still import raw private keys or keystore/json files but those accounts are marked as "imported".

hboon commented 1 year ago

Right, thanks.

Ideally, we at AlphaWallet would want to support it this way, but just haven't came up with the user interface to expose it yet (and no demand to prioritize it yet):

seed 0 address 0
seed 0 address 1

seed 1 address 0
seed 1 address 1

raw private key 1
raw private key 2

In an attempt to simplify, we had to restrict to only generating address 0 at the moment because we really want to be able to support multiple seeds.