search

harmony-one / bounties

Bounty program is to help the community take part in the development of the Harmony blockchain. It covers from core feature to validator tooling, from dApp development to DeFi integration.
MIT License
59 stars 23 forks source link

Identify root cause of Harmony Chrome Extension wallet's account import inconsistency #88

Closed givp closed 2 years ago

givp commented 2 years ago

Description

Several users have reported an issue with the Harmony Chrome Extension whereby upon restoration of a wallet, the wallet holds a different address from the original. This bounty is to investigate and identify the root cause.

Some users have described this issue happening when a seed phrase is used vs the private key.

Chrome Extension on App Store

Acceptance Criteria

Reward

$5,000 in ONE tokens

gitcoinbot commented 2 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

This issue now has a funding of 18418.6711 ONE (5010.08 USD @ $0.27/ONE) attached to it.

gitcoinbot commented 2 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

Work has been started.

These users each claimed they can complete the work by 265 years from now. Please review their action plans below:

1) 007tom has started work.

firstly must reproduce the bug

Learn more on the Gitcoin Issue Details page.

jotagep commented 2 years ago

Is this bounty open??

givp commented 2 years ago

Is this bounty open??

Yes it is.

Abdul00107 commented 2 years ago

I can fix this one , but the reward is bit low since it is complicated task, increase it a bit more and i can give you the solution with report with in 12 hours.

gitcoinbot commented 2 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

Work for 18418.6711 ONE (3259.64 USD @ $0.17/ONE) has been submitted by:

AgCaliva commented 2 years ago

Hi people, im working on this, after some testing i have found that: -Harmony Wallet has some bug generating the mnemonic phrase, so if you only saved this then probably all your assets are lost forever... -Harmony Wallet has some bug using the provided mnemonic phrase, so even if you have the right one you will not recover your wallet. -It generates correctly the private key and recovers wallet using it. So the only way that you can recover assets from a wallet created with this extension is using the private key by the time... its really a mess... Im working on issues. I dont understand or know what is talking about abdul00107. Will really try to get this ready in one or two days.

AgCaliva commented 2 years ago

Hi. after a better detailed test, i have found that some last statement was not correct. -Harmony Wallet its correctly generating the mnemonic phrase, so all the assets are ok if you only saved mnemonic phrase. -The bug in hmy wallet its in process of importing by mnemonic phrase only. -Also found that the project at least in development branch has some dependency problems, most of them caused by new node versions, and deprecated libs. I will upgrade you with correct data about correct compilation of project in Windows and Linux. -Probably need a day or two more for writing or fixing new implementation. For the ones who have stuck assets, you only need to import wallet using other wallet compatible like Metamask.

AgCaliva commented 2 years ago

Im not really sure what is happening but i only get recovery incorrect when import using Metamask using mnemonic phrase, if i use hmy wallet it imports correctly mnemonic phrase. For correct compilation of project you need to delete those dependencies: "node-sass": "^4.14.1", "extract-text-webpack-plugin": "^3.0.2", if project have some test account data for reproduce please provide me with the data, as far as i see the problem can be in metamask. it loads incorrect address. mail me at "edit: i deleted my mail to avoid spam"

EDIT: i see, i have reproduced the problem. -If i create wallet using metamask, then restoring account its only possible from metamask. Restoring from Harmony Wallet results in other address. -If i create wallet using harmony wallet, then restoring account using mnemonic phrase its only possible from harmony wallet. Restoring from metamask results other address. -I will continue researching for finding and fixing the issue.

AgCaliva commented 2 years ago

As far as i see the problem its that maybe harmony wallet creates this type of wallet: https://github.com/bitcoin/bips/blob/master/bip-0032.mediawiki

and metamask wallet creates this type: https://github.com/bitcoin/bips/blob/master/bip-0039.mediawiki those are not the same, Metamask its an HD wallet(recovers using mnemonic phrase), and harmony wallet can be non deterministic wallet (recovers from private key). EDIT: After some research i have found that Harmony its probably implementing BIP 44, which uses BIP32 as base, and Metamask its implementing BIP39. So you cant import wallet if you created from other wallet. So there is no real issue here. I think this is solved please let me know if you find something about this. The bug started being reported from 4 MAY, at least someone says that. It looks like there was some change in BIP implementation, so accounts created with previous BIP will not be imported in last versions. The explanation its that each BIP says how to index the address, so even if you have same keys it depends in BIP implementation which wallet you will retrieve.

SOLUTION: They can probably recover his wallet from using old version of extension. or compatible wallet implementing same bip, talk to confirm if it works. I recommend releases before January 2021 or before issue started being noticed.

Step by step guide for installing old extension version:

You need to select the version you was probably using when created your wallet from here: https://github.com/harmony-one/chrome-extension-wallet/releases

Download the zip, unzip and open your chrome-brave navigator. Go to Settings or Extensions settings (Manage extensions etc). Set up Developer Mode. Load unpacked. Now search for the folder unziped and press open.

Reference of BIP44 usage in project: https://github.com/harmony-one/harmony/blob/main/accounts/hd.go https://github.com/harmony-one/dapp-examples/blob/master/nodejs/testWallet.js As far as i know there is no reason to think this is related to security issue, i think those security issues may be caused by copy clipboard functions from some websites. We need info about how they saved his accounts if they remember just taking photo or they copy to clipboard. All wallets must encourage to use just paper against copy or taking screenshots to mitigate this risk.

I cant submit my work because you already have accepted other, and gitcoin cost you gas fees for doing that? https://gitcoin.co/issue/harmony-one/bounties/88/100027099

Please contact me to pay bounty.

AgCaliva commented 2 years ago

Hi people, can you provide the approximate wallet creation date so i can test the versions you probably used? Please add if you successfully already have recovered in the past( i guess not ).

Because to me it seems like a ramified problem, each people can have different problem depending on BIP used, so probably there are like 3 or 4 solutions. I dont know how harmony want me to proceed because its dangerous if i provide you with test versions. i cant confirm problem its solved by my self, but at least will try to fix obvious things in affected cases anyway they will need to check my implementation before letting you test it. Thanks in advance and hmy people contact me to clarify.

AgCaliva commented 2 years ago

By the time i have found that import and generation of keys is taking part in harmony-one/sdk Harmony SDK in js. So the problem its probably not related to wallet. I will test SDK to try to find out whats going on. At a simple glance it looks like it can be something messing with mnemonic phrase import function: in file /packages/harmony-account/src/wallet.ts, line 120: The derivation path being used can be m/44'/1023'/0'/0/0 or m/44'/60/0'/0/0. By default it seems to be m/44'/1023'/0'/0/0 being used, but i will debug to check if thats not true in some usecases. Why harmony wallet its using 1023? someone can contact me to discuss this kind of technical topics? Why SDK try to use 1023? but here it says to be using 60: https://github.com/harmony-one/harmony/blob/main/accounts/hd.go

Anyway the wallet its using a very old version of harmony-js SDK "@harmony-js/core": "0.1.36" Why are using that old library? it was published in Nov 27, 2019. Here i see before this commmit it was using "^0.1.43" so probably from May 4, 2020 aprox version. https://github.com/harmony-one/chrome-extension-wallet/commit/16847b8c4ca87d62b53cbb991413d45b0c5fe5e2 Maybe its a bug from SDK, but anyway the version used here its very very old. I would like to help please tell me where we can chat.

Its seems like sdk has been using different derivation paths, thats probably the cause people are getting other wallets. Here its using derivaption path m/44'/60/0'/0/0: https://github.com/harmony-one/sdk/commit/d7cd6fe65aa225d1ddc9e4a82bd6576a1a61084a And here we see the now implementation of m/44'/1023'/0'/0/0: https://github.com/harmony-one/sdk/commit/b1f6cd108d93f4bbd60950985bfa6027283b90ba

Those accounts created in dates that used other derivation will not be retrieved by last version of Harmony SDK, its needed to specify which derivation was used when created the account, probably the ones from deprecated wallet can retrieve their wallets by using an old version of Harmony SDK. Anyway i dont see any change in this repo that can change the derivation path, since those changes were much older. The only explanation its that SDK its buggy and it changed derivation path used by some reason, in some usecases (dont understand why its not using a derivation path fixed, using different cause this kind of situations). The derivation path only affects mnemonic phrases.

To clarify this is probably the problematic line: const path = this.messenger.chainType === ChainType.Harmony ? '1023' : '60';

https://github.com/harmony-one/sdk/blob/v0.1.57/packages/harmony-account/src/wallet.ts

I suggest to add to harmony sdk an option for setting the derivation path used in mnemonic phrases, and let people edit that config, like others wallets have done. Anyway its a misunderstanding of mnemonic phrases, those retrieve different address if use other derivation path, so in reality for retrieve account from mnemonic phrase you need the derivation path used too.

AgCaliva commented 2 years ago

@givp hi can someone mail me or answer something? what i need to get paid? which direction hmy want to take for this inconsistency? i have already suggested solution, the harmony-one/sdk needs to be modified. Bug its not from wallet( it needs to be upgraded too anyway). I have to wait two months?

cubanito201 commented 2 years ago

Same issue here, can't retrieve wallet with phrase. ugh still no solutions?

givp commented 2 years ago

@AgCaliva thank you for the investigative efforts. That gives us a great lead and we can now close this bounty which was intended to identify the root cause. I don't see your submission on Gitocoin, though. Can you please go ahead and submit it so we can pay you and wrap this up?

gitcoinbot commented 2 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

Work for 18418.6711 ONE (2285.26 USD @ $0.12/ONE) has been submitted by:

  1. @abdul00107
  2. @agcaliva

@givp please take a look at the submitted work:

gitcoinbot commented 2 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

The funding of 18418.6711 ONE (2639.08 USD @ $0.14/ONE) attached to this issue has been cancelled by the bounty submitter

givp commented 2 years ago

Bounty paid directly.