Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

restore wallet functionality does not support older 16 word wallets #1193

Closed
LaurenWags opened this issue Sep 19, 2018 · 6 comments · Fixed by brave/brave-core#530
Closed

Comments

@LaurenWags
Copy link
Member

LaurenWags commented Sep 19, 2018

Description

Brave Payments on browser-laptop allows for wallets to be recovered with either the old 16 words or the newer 24 words. Brave Rewards only allows for the newer 24. Since the switch to 24 was done without user interaction/knowledge, it's possible some users do not know and only have the 16 words in their backup file. Once they move to brave-core, if they try to recover their wallet using these words it will not work for them.

Steps to Reproduce

  1. Have backup words for a wallet with 16 words.
  2. Attempt to recover a wallet on Brave Rewards with 16 words. (currently you may encounter staging env is being used for wallets #1171)

Actual result:

Wallet recovery with 16 words fails

Expected result:

Wallet would be recovered with 16 words. However, as it is in browser-laptop, after recovering the wallet with 16 words, when user goes to backup wallet, the 24 words should be displayed. This does create a disconnect for the user as their backup phrase is now different than what they entered, however we probably do want to promote the newer 24 words.

Reproduces how often:

easily

Brave version (chrome://version info)

all Brave Rewards enabled versions

Reproducible on current release:

no

Website problems only:

  • Does the issue resolve itself when disabling Brave Shields? n/a
  • Is the issue reproducible on the latest version of Chrome? n/a

Additional Information

brave/browser-laptop#14856 (comment)
brave/browser-laptop#13313 (comment)
brave-intl/bat-client#48

@bbondy bbondy added this to the 1.x Backlog milestone Sep 22, 2018
@NejcZdovc NejcZdovc modified the milestones: 1.x Backlog, Releasable builds 0.55.x Sep 24, 2018
jasonrsadler pushed a commit to brave-intl/bat-native-ledger that referenced this issue Sep 28, 2018
jasonrsadler pushed a commit to brave-intl/bat-native-ledger that referenced this issue Sep 28, 2018
jasonrsadler pushed a commit to brave-intl/bat-native-ledger that referenced this issue Sep 28, 2018
jasonrsadler pushed a commit to brave-intl/bat-native-ledger that referenced this issue Sep 28, 2018
Added tests

linting
jasonrsadler pushed a commit to brave-intl/bat-native-ledger that referenced this issue Sep 28, 2018
Added tests

linting
jasonrsadler pushed a commit to brave-intl/bat-native-ledger that referenced this issue Sep 29, 2018
jasonrsadler pushed a commit to brave-intl/bat-native-ledger that referenced this issue Oct 1, 2018
Refactored loading wordlist from pak
jasonrsadler pushed a commit to brave-intl/bat-native-ledger that referenced this issue Oct 1, 2018
Refactored loading wordlist from pak
jasonrsadler pushed a commit to brave-intl/bat-native-ledger that referenced this issue Oct 1, 2018
Refactored loading wordlist from pak

linting and cleanup
jasonrsadler pushed a commit to brave-intl/bat-native-ledger that referenced this issue Oct 1, 2018
Refactored loading wordlist from pak

linting and cleanup

Added note for mobile to not check for legacy wallet keys
jasonrsadler pushed a commit to brave-intl/bat-native-ledger that referenced this issue Oct 1, 2018
Refactored loading wordlist from pak

linting and cleanup

Added note for mobile to not check for legacy wallet keys
jasonrsadler pushed a commit to brave-intl/bat-native-ledger that referenced this issue Oct 1, 2018
Refactored loading wordlist from pak

linting and cleanup

Added note for mobile to not check for legacy wallet keys

Corrected wordlist size comparison
jasonrsadler pushed a commit to brave-intl/bat-native-ledger that referenced this issue Oct 2, 2018
jasonrsadler pushed a commit to brave-intl/bat-native-ledger that referenced this issue Oct 2, 2018
wip --

Addressing comments
jasonrsadler pushed a commit to brave-intl/bat-native-ledger that referenced this issue Oct 2, 2018
wip --

Addressing comments
jasonrsadler pushed a commit to brave-intl/bat-native-ledger that referenced this issue Oct 3, 2018
wip --

Addressing comments

Addressing additional comments
jasonrsadler pushed a commit to brave-intl/bat-native-ledger that referenced this issue Oct 3, 2018
wip --

Addressing comments

Addressing additional comments

Adding handling to onRecoverWallet with niceware failure
jasonrsadler pushed a commit to brave-intl/bat-native-ledger that referenced this issue Oct 3, 2018
wip --

Addressing comments

Addressing additional comments

Adding handling to onRecoverWallet with niceware failure

cleanup
jasonrsadler pushed a commit to brave-intl/bat-native-ledger that referenced this issue Oct 3, 2018
wip --

Addressing comments

Addressing additional comments

Adding handling to onRecoverWallet with niceware failure

cleanup

linting
jasonrsadler pushed a commit to brave-intl/bat-native-ledger that referenced this issue Oct 3, 2018
wip --

Addressing comments

Addressing additional comments

Adding handling to onRecoverWallet with niceware failure

cleanup

linting

Corrected actual result value getting used
jasonrsadler pushed a commit to brave-intl/bat-native-ledger that referenced this issue Oct 3, 2018
wip --

Addressing comments

Addressing additional comments

Adding handling to onRecoverWallet with niceware failure

cleanup

linting

Corrected actual result value getting used

Updated tests
jasonrsadler pushed a commit to brave-intl/bat-native-ledger that referenced this issue Oct 3, 2018
wip --

Addressing comments

Addressing additional comments

Adding handling to onRecoverWallet with niceware failure

cleanup

linting

Corrected actual result value getting used

Updated tests

Removed commented lines
jasonrsadler pushed a commit to brave-intl/bat-native-ledger that referenced this issue Oct 6, 2018
wip --

Addressing comments

Addressing additional comments

Adding handling to onRecoverWallet with niceware failure

cleanup

linting

Corrected actual result value getting used

Updated tests

Removed commented lines
@NejcZdovc NejcZdovc reopened this Oct 8, 2018
@srirambv
Copy link
Contributor

srirambv commented Oct 10, 2018

Verification Passed on

Brave 0.55.12 Chromium: 70.0.3538.45 (Official Build) beta (64-bit)
Revision cbdc32e4334458954e9def214d7e5fa1ca1960eb-refs/branch-heads/3538@{#830}
OS Linux
  • Verified 16 word wallet is restored correctly with BAT in it
  • Verified 24 word wallet is restored correctly with BAT in it
  • Verified wrong words doesn't restore wallet and throws error

One observation, error doesn't go away when user tries to re-enter the recover code.

Verification passed on

Brave 0.55.12 Chromium: 70.0.3538.45 (Official Build) (64-bit)
Revision cbdc32e4334458954e9def214d7e5fa1ca1960eb-refs/branch-heads/3538@{#830}
OS Windows 7
  • Verified 16 word wallet is restored (thanks @LaurenWags for the 16 word wallet)
  • Verified wrong words doesn't restore wallet and throws error

Verified passed with

Brave 0.55.12 Chromium: 70.0.3538.45 (Official Build) beta(64-bit)
Revision cbdc32e4334458954e9def214d7e5fa1ca1960eb-refs/branch-heads/3538@{#830}
OS Mac OS X

@shayanb
Copy link

shayanb commented Nov 15, 2019

Is this option removed from Brave 1.0.0 ? It seems that only 24 words recovery phrase is acceptable.

@LaurenWags
Copy link
Member Author

@shayanb I was just able to import a 16 word Rewards wallet file as well as copy/paste the recovery words without issue using 1.0.0. Are you trying to do this on brave://rewards or brave://wallet? Are you seeing an error message?

Brave 1.0.0 Chromium: 78.0.3904.97 (Official Build) (64-bit)
Revision 021b9028c246d820be17a10e5b393ee90f41375e-refs/branch-heads/3904@{#859}
OS macOS Version 10.13.6 (Build 17G5019)

@shayanb
Copy link

shayanb commented Nov 16, 2019

Thanks @LaurenWags . I was using the restore wallet brave://wallet functionality, it worked with brave://rewards, although seems that the wallet is empty :-?

@LaurenWags
Copy link
Member Author

@shayanb thanks for the confirmation. brave://wallet is for the crypto wallet, which is separate and not related to Brave Rewards at all.

As for your wallet being empty when restoring, it's possible that it had grants which have since expired. We have not had grants in awhile I believe and we stopped doing 16 word wallets in favor of the 24 word wallets even longer ago, so this is my suspicion. Is this a wallet from a long time ago?

@shayanb
Copy link

shayanb commented Nov 18, 2019

Yes, probably early on when BAT was implemented in Brave. Anyhow thanks for the help. It's all good now :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment