gitcoinbot
commented
7 years ago This issue now has a funding of 1.0 ETH (308.2 USDT) attached to it. To view or claim this funding, click here.
Closed danfinlay closed 6 years ago
gitcoinbot
commented
7 years ago This issue now has a funding of 1.0 ETH (308.2 USDT) attached to it. To view or claim this funding, click here.
owocki
commented
7 years ago you'd think you could just hammer it with 10,000,000 tests for that seed/restore functionality which I'm sure they've done
Just curious, has this been done? If so, has all surface area of possible reproduction steps been covered? This might be an interesting way for someone to attempt to reproduce this.
Sounds like yes from this quote: https://github.com/MetaMask/metamask-extension/issues/1950#issuecomment-323834717
@kumavis has already run some tests on seed generating code and didn't get any red flags--I believe the core issue is something regarding state.
Maybe Kumavis or @danfinlay could post the code or describe his approach associated with said tests, described here: https://github.com/MetaMask/metamask-extension/issues/1950#issuecomment-323844631
danfinlay
commented
7 years ago @owocki wrote:
you'd think you could just hammer it with 10,000,000 tests for that seed/restore functionality which I'm sure they've done
This is indeed part of our HD keyring's test suite (Every PR runs 1k restores, but I've run millions while researching this).
What we haven't tested as rigorously is the larger surface of the integration layer. That's why I say if there is a bug, it would be at the Metamask Controller level, which is such a thick/complex controller that it currently lacks any real tests, and this is where I will probably write fuzzing logic if someone doesn't beat me to it.
danfinlay
commented
7 years ago To enrich this discussion, we’ve now opened a reddit AMA for encouraging devs to ask questions regarding this issue:
https://www.reddit.com/r/ethereum/comments/7d6aht/metamask_key_management_ama/
glitch003
commented
6 years ago I think I found something that could be related. When creating a new account, there's an event handler on the confirm password dialog for when the user hits enter (code here)
If you hold down the enter button, it seems to fire the event multiple times, sometimes after a delay, and this generates new seed phrase each time. See this video for an example.
I haven't dug any deeper yet, so maybe @danfinlay or someone can rule this out as being related or not.
danfinlay
commented
6 years ago @glitch003 Good find, I did find this scenario also and investigated a bit, but I don't want to say I fully ruled it out. It's definitely somewhat an open question.
What I did related to this:
I do think this is a fairly hot lead, but while it's suspicious, I haven't seen it reproduce the described problem yet. If you can, that's bounty-winning.
mikeseese
commented
6 years ago FWIW, I'm not 100% convinced that showing the user the wrong seed upon account creation is the entire story. Given the comment at https://github.com/MetaMask/metamask-extension/issues/1950#issuecomment-337040259, there could also be a restore issue (assuming there was no user error in this one circumstance)
danfinlay
commented
6 years ago @seesemichaelj I think you're right. I'll broaden the top post's award criteria to be less opinionated about what the bug is. If you can reproduce behavior that matches the described user experience, you'll be eligible for the bounty.
mikeseese
commented
6 years ago I don't think this is quite the issue, but I was able to reproduce it to generate a seed phrase for a keyring with a single identity. I then was able to get metamask to add identities/accounts to the keyring without the seed phrase changing. Therefore I would see multiple selectable accounts in the account selection menu. However anything but the first account seemed invalid (no ETH balance/etc)? If I locked and restored from seed, only the first account was imported and the other (invalid?) ones were not there. See the GIF below
EDIT (more details): I'm not sure if the code makes it easy for it to happen the other way (the seed is generated for not the first account). Is inducing artificial latency to reproduce permitted? This could simulate loads on the PC/network/IO/etc
EDIT 2: I guess the default function only imports the first account? I tried just doing normal operation: make a new account, create another, see seed hasnt changed, lock, restore from seed, only the first account is there, if I click "create account" it creates the 2nd account again. However, this was unintuitive to a user; they'd think that the second account was gone forever. It seems that the create-account is a deterministic process given some wallet/seed? Haha I REALLY hope this isn't the cause of this issue
EDIT 3: This was using the "multiple enters" method presented by @glitch003 with CPU throttling to simulate load and/or slow computers
mikeseese
commented
6 years ago I also tried to slow down different steps in the process like encryption and file IO and I still have issues getting it to show a seed that isn't for the "main" account
danfinlay
commented
6 years ago @seesemichaelj sorry for the slow reply, a long day when I'm only getting to email 20 hours later!
How did you get the gif state, where you have multiple accounts named "Account 1", but only some with loading balances? I assume you didn't edit those names? Do those accounts get generated by the same seed phrase?
I guess the default function only imports the first account? Haha I REALLY hope this isn't the cause of this issue
I can confirm this isn't the problem. We've had these users generate many of accounts from their backed up phrases, checking for any previous ones, with no luck.
mikeseese
commented
6 years ago @danfinlay haha no worries!
The gif state is pretty simple:
To answer the other questions you had: I did not rename them. They all were happening "simultaneously" and therefore all thought they were account 1. All of the "Account 1"s listed in MetaMask I saw had the same associated seed phrase (in terms of logging. I'm not sure if those addresses are reproducible via normal account adding using the seed). However there were previous accounts/seed phrases that seemed to be thrown away
I can confirm this isn't the problem. We've had these users generate many of accounts from their backed up phrases, checking for any previous ones, with no luck.
Good to hear! I do think this is a separate issue in terms of usability/UI although! Very unintuitive
Edit: I went back and did some testing. If I get into the GIF state and look at the second "Account 1" address, restore from seed phrase, then "Create Account", this "Account 2" does not match the second "Account 1". I proceeded to generate 6 more accounts and none of them matched the address of the second "Account 1". I can do more precising testing if we think this has some bearing. I also tried slowing down on account restore; this had no noticeable issues.
mikeseese
commented
6 years ago Do you know @danfinlay if occurences have been using English seed phrases (is that all you support)? Looking through the unicode set (as well as testing other OS's), I'm not seeing potential encoding issues on the characters. The only encoding discrepancies I've seen were punctuation (i.e. Microsoft doesn't use the default " ' or , characters but uses a special variant of them)
mikeseese
commented
6 years ago So I have been able to reproduce the problem with some finagling. I was able to generate a seed phrase, copy it somewhere else, return to the extension, the phrase was still there, go to my accounts page, see the account, lock, restore from the seed phrase I copied, and it showed up another account. I created some more accounts to make sure it wasn't a secondary/tertiary account with the same seed phrase. This is presented in the below GIF:
These are the steps I used:
With everything said, I don't believe this is very common case (as we have seen from bug reports provided), but I see it as a possible situation. I believe what's happening here is the account that you see at first is the account from a second seed phrase, the one that is generated on the second Enter, but there is some issue of why it doesn't show up in the popup window. I believe this is related to me leaving the popup to copy the address; if I stayed in the popup the whole time, I'm fairly confident the second seed phrase would show up (as seen in @glitch003's video) after the 8 seconds. Even if it did appropriately show up (even if you press "i copied it" before it showing up), the user could just ignore the contents and not realize it's different and click "NO - $*@& - I DID COPY IT!". I think there's some major async processes that need to be locked to only execute once to help prevent this error.
All of this said (v2): Who knows if this actually covers the problem. However, I believe some recoding to ensure one key ring is ever generated may resolve the problems. If you have tests that generate key rings, copy the address, then restore the keyring with the seed, and compare the address, and the tests do this thousands/millions of times, I think we may be rid of the problem.
Thoughts?
danfinlay
commented
6 years ago @seesemichaelj thanks so much for the thorough investigagion! I’ll be looking at this more closely soon, it’s late here now.
Eager to see the results!
danfinlay
commented
6 years ago Even if it did appropriately show up (even if you press "i copied it" before it showing up), the user could just ignore the contents and not realize it's different and click "NO - $*@& - I DID COPY IT!". I think there's some major async processes that need to be locked to only execute once to help prevent this error.
This is a plausible theory, with a concrete strategy for correction. The issue you've exposed, in summary:
create vault multiple times (needs a lock)The team will be consulting a bit more on this, but it looks eligible for the bounty to me. We'll communicate back here soon.
mikeseese
commented
6 years ago I dont think it matters either way, but I think in my specific example, it was opposite:
Sounds great! Let me know if you have any other questions.
Moejoe90
commented
6 years ago Have you thought about this old issue https://github.com/MetaMask/metamask-extension/issues/640 Maybe these users were very old and generated their phrases through the old settings and now the new versions fix the implementation but will generate a wrong address since it's not the same HD generation library, you even mention that I quote "So should we aim to expose the option to restore the bug at every layer of the stack, or simply offer an outdated build as a step in a recovery process?" Maybe you have thought about it but still worth mentioning
danfinlay
commented
6 years ago @Moejoe90 Good catch, we have definitely thought about issue #640. We have confirmed with multiple users that this was not the problem they were experiencing, by having them use ethereum wallets that use the old bitcore library (like eth-lightwallet).
owocki
commented
6 years ago hey @seesemichaelj was it you who claimed the issue at https://gitcoin.co/funding/details?url=https://github.com/MetaMask/metamask-extension/issues/2577 ?
mikeseese
commented
6 years ago yes @owocki it was me
dopeideas
commented
6 years ago Two questions:
danfinlay
commented
6 years ago @dopeideas we have a blog post summary coming very soon. You would only be eligible for another bounty if you found another way this could still happen.
kumavis
commented
6 years ago Here is the blog post https://medium.com/metamask/seed-phrase-issue-bounty-awarded-e1986e811021
thricepi
commented
6 years ago Hello everyone. I need help. I seem to be one of those who have a new address created for an erc20 token (cappasity) when trying to recover my metamask account with the seed words.
When i first used the seed phrase it gave me one account with a small amount of eth in it (account1). I tried (create account) to get the others to show. I was able to get my other eth accounts which all had small amounts of eth inside after doing this.
However my (cappsity) erc20 token is the most valuable token i had on metamask. I input the contract info: 0x04f2e7221fdb1b52a68169b25793e51478ff0329
To my shock I found 0 CAPP on it. The address associated with it is different than the address I had previously. I was able to verify with the CAPP team that my address was different than the one shown in metamask. I have the address the CAPP team showed me written down on paper. I checked etherscan witht the address provided by them and verified by me and I can see my account and the corresponding amount. However I cannot get access to my account balance. I tried everything.
What can be done to get access to my account?
ORIGINAL ADDRESS for CAPP TOKEN: 0xdbabcd8ba0a13837e61f1deba2ca9ffc5b82d7ef NEW ADDRESS (current) ADDRESS shown in metamask: 0x81144d5794ccd5a29390768a6de292705ad0b76f
NOTE: Is it possible to gain access to my accounts funds by using my metamask mnemonic seed phrase in myetherwallet? i see the same account address which I know about, (the one the CAPP team told me my funds are in). It is shown in the list of MEW addresses, but it says it has no eth equivalent balance in it. I am worried to proceed without help. I do not want to mess anything up.
Please help me!
bdresser
commented
6 years ago @thricepi sorry to hear about your trouble. There are a couple things we can try - please reach out to support@metamask.io and we'll be able to help you there.
thricepi
commented
6 years ago Hello @bdresser. Thank you for your reply. Over the last days I was talking to a metamask support employee. They were really patient and consistent in replying.
The good news is i got access to my tokens!
I will tell you which method was used so it could serve as a reminder in case someone needs help later with a similar situation.
I looked through all of my current metamask wallet accounts, which showed up after clicking the "create account" continuously. I did this after my last recovery attempt prior to speaking to you. There were quite a few accounts made so it took time
The very last account has the address '0xdbabcd8ba0a13837e61f1deba2ca9ffc5b82d7ef' I then input the contract address, ticker symbol, and decimal: contract info: 0x04f2e7221fdb1b52a68169b25793e51478ff0329
Token Symbol: CAPP Decimals: 2
It then appeared!
I feel people may say this is very obvious, but i was not aware i needed to search and find the address after creating all accounts over again and then select that specific address and follow that step up with inputting the contract info for the erc20 token after doing a recovery.
Now going through the motions it seems as if this makes sense. However, for the majority, particularly for any new or basic user it does not seem straight forward enough. Particularly since it is mostly mentioned the 12 word seed would be all that is needed to recover ether accts and erc20 tokens.
None the less. I learned a lot.
forte64
commented
6 years ago Help needed. Seed phrase is invalid
I tried send ETH from one account to another but had a broken internet connection while at it. The transaction failed. Then metamask crashed; it kept going on and on but did nothing.
Restarted the browser, no luck. Restated the machine too a few times but it didn't solve the problem either. Metamask kept going on and on after attempted signing in. I had to remove the extension and then reinstalled. The 12 seed phrase was copied CLEARLY and entered CORRECTLY. But metamask says "seed phrase in invalid".
I've got an important smart contract token as well as other tokens on the wallet (with multiple accounts). Is there a fix for this? I hope there's a fix. I've set an email to support@metamask.io
roo-shy
commented
3 years ago Customer reported, ticket reference on ZD: https://consensys.zendesk.com/agent/tickets/64454
Example my seed phase is 1 to 12 words in that order opens my wallet , now if I was to switch word 1 & 7 round I can open another wallet up .
This issue is the bounty summary version of #1950 and #2383.
A small number of users have reported that on restoring with their seed phrase, they were given different accounts then they first had with MetaMask. If this is an actual bug in MetaMask, it's a critical issue. However, we have been unable to reproduce this issue, and so we would like to offer a bounty to the larger community.
If you’re able to discover a condition in code (henceforth referred to as "The Bug"), or reproduce a behavior that could cause a MetaMask build version 3.0.0 or later to give a user an experience where they are shown a seed phrase, given some accounts, and later unable to restore those accounts with that seed phrase, you will be awarded this bounty. This could be an issue with showing a wrong seed phrase, an issue with restoring from seed, or something we haven't considered. Anything that reproduces this behavior will be considered a valid solution to this problem.
We will initially be issuing two parallel bounties:
Anyone who is able to find The Bug will receive both bounties. We will be gradually adding to both bounties, the longer the issue stands unsolved.
If you are concerned with this issue, you can contribute to the active bounty on either of those platforms, and this will only be paid out to a person who finds the bug, or it will be refunded after six months from our issue date of November 13, 2017 (May 13, 2018).