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

fix users who already restored wallets but UI displays incorrect addresses #2172

Closed
kjozwiak opened this issue Nov 17, 2018 · 12 comments · Fixed by brave/brave-core#1173
Closed

Comments

@kjozwiak
Copy link
Member

Description

We originally fixed #1831 but didn't fix the case when a user has already restored but still has the incorrect address appearing in their UI. We've gotten several more reports from users who sent BAT to their address but never received it as it was sent to the incorrect address as they previously restored. @jonathansampson was also affected by this.

@evq mentioned that the deposit addresses are in the wallet info response and we should be able to compare those values to the locally stored values and fix the issue automatically.

Steps to Reproduce

STR would be similar to #1831. Requires you to get the wallet into the broken state where it shows the incorrect addresses in the UI after restoring a previous wallet. Updating to a new version should automatically fix these users as well.

Actual result:

#1831 addressed new installs but didn't address users who might have already restored wallets. Users who update to a new version should be fixed automatically.

Expected result:

Users affected by this issue should be automatically fixed and the correct addresses should be displayed in the UI so they don't send funds to the incorrect addresses.

Reproduces how often:

100% reproducible when you have an account affected by this issue.

Brave version (brave://version info)

Brave 0.57.4 Chromium: 71.0.3578.31 (Official Build) dev (64-bit)
Revision c88fdf2a4ce19a713615ca4fbde7a0d0b5fe2363-refs/branch-heads/3578@{#427}
OS Mac OS X

Additional Information

@jonathansampson @evq please add anymore information if I've missed something!

@kjozwiak kjozwiak added feature/rewards priority/P2 A bad problem. We might uplift this to the next planned release. labels Nov 17, 2018
@kjozwiak kjozwiak added this to the 1.x Backlog milestone Nov 17, 2018
@NejcZdovc
Copy link
Contributor

@evq which endpoint should we use to get this data? can we just do it as part of balance check?

@NejcZdovc NejcZdovc self-assigned this Nov 19, 2018
@kjozwiak
Copy link
Member Author

@evq @NejcZdovc any news on this one? We should really try getting this into 0.57.x as there's a few folks still affected by this 😢

@btlechowski
Copy link

@brave/legacy_qa To get the wallet into broken state, version 0.56.6 of Brave is required. Unfortunately there are no Windows standalone installers for that version https://github.com/brave/brave-browser/releases/tag/v0.56.6. Any ideas how we could verify this issue on Windows?

@srirambv
Copy link
Contributor

Doesn't editing the seed values corrupt the wallet ? @NejcZdovc thoguhts

@kjozwiak
Copy link
Member Author

Doesn't editing the seed values corrupt the wallet ? @NejcZdovc thoguhts

With this issue, it wasn't due to the seed being corrupted but more about ledger_state and the UI having addresses mismatched. We need a way to have two addresses. One address in the UI and a completely different in ledger_state.

@NejcZdovc any idea's how we can reproduce this case? As @btlechowski mentioned, 0.56.x didn't have standalone installers for Win so there's no way of actually getting that version on Windows unless DevOps builds one (not sure if that's even possible).

@btlechowski
Copy link

btlechowski commented Jan 23, 2019

I think the original issues is not fixed. Tested on Linux.
Steps:

  1. Clean Install 0.56.6
  2. Enable rewards
  3. Backup ledger_state
  4. Backup recovery codes
  5. Clean launch (delete the profile)
  6. Enable rewards
  7. Backup ledger_state
  8. Use recovery codes from 4. <- you should get a corrupted wallet
  9. Backup ledger_state
  10. Install 0.59.26
  11. Compare wallet addresses

Reproduced twice with above STR.

@NejcZdovc
Copy link
Contributor

@btlechowski why would you get corrupted wallet if you do wallet recovery in step 8?

@LaurenWags
Copy link
Member

Issue does not appear to be resolved on macOS either. These were the steps I used to test (adapted from #1831):

  1. Install 0.56.6 dmg (this is a Beta version)
  2. Launch with clean profile and enable rewards.
  3. Note backup words.
  4. Note addresses displayed in Add Funds.
  5. Note paymentId, addresses, and CARD_ID from ledger_state.
  6. Use restore functionality to Restore a wallet.
  7. Note backup words (they updated to the words from the restored wallet and are different than step 3).
  8. Note addresses in Add Funds (still the same as step 4).
  9. Open ledger_state and note paymentId (this was updated, so it's different than step 5) addresses and CARD_ID (all still the same as step 5).
  10. Go to brave://settings/help to trigger update check.
  11. Relaunch or close/launch manually. You will now have 0.59.26
  12. Backup words for wallet match step 7 and paymentId matches step 9 as expected.
  13. Addresses in Add Funds are still the same as step 4/8 - FAIL
  14. Addresses and CARD_ID are still the same as step 5/9 - FAIL
Brave 0.59.26 Chromium: 72.0.3626.64 (Official Build) beta(64-bit)
Revision eaa9668e80ce5405e7f1902579558ea725c06ca1-refs/branch-heads/3626@{#708}
OS Mac OS X

@btlechowski
Copy link

btlechowski commented Jan 23, 2019

@NejcZdovc because 0.56.6 don't have the fix from #1831. So if you recover wallet on 0.56.6, you will get a corrupted one.
To my understanding the whole point of this issue is to fix users who recovered the wallet before #1831, so their corrupted wallets are fixed.

@LaurenWags
Copy link
Member

@NejcZdovc when I follow the test plan from brave/brave-core#1173, after modifying the addresses in ledger_state and launching Brave, I get this message about a corrupted wallet, so I can't check via the test plan.
screen shot 2019-01-23 at 3 55 01 pm

@LaurenWags
Copy link
Member

LaurenWags commented Jan 23, 2019

For #2172 (comment) I logged a follow up issue: #3060

Since I am unable to test this issue per the test plan (#2172 (comment)), I came across a different set of steps that allowed me to verify the fix for this issue. Here are the steps I used:

  1. Install muon (probably 0.25.304 or whatever the last released version we did). Set up data for import (a few bookmarks, history, and definitely enable Payments. Fund wallet as well). Note backup words, wallet addresses, and address/paymentId/CARD_ID from ledger-state file.
  2. Install 0.57.18 (Release version) and enable rewards. Note Addresses and backup words as displayed in the UI. Close Rewards page. Close this version. Navigate to ledger_state file and note addresses, paymentID, Card_ID.
  3. Reopen On 0.57.18, import muon data from Brave > Import Bookmarks and Settings in menu (note, I could only get this problem by importing this way. Importing a different way i.e. from brave://welcome or hamburger menu did not produce the issue).
  4. Go to Rewards and verify that UI still shows addresses and backup words from step 2
  5. Go to ledger_state and verify it shows same values for addresses/paymentId/CARD_ID from muon (step 1)
    At this point you have the UI showing the old values and the ledger_state file showing updated values.
  6. Update 0.57.18 to current released version 0.58.21 by going to chrome://settings/help to trigger the update.
  7. Once updated, relaunch Brave to verify update, then close Brave.
  8. Rename your profile to be a Beta profile.
  9. Launch 0.59.28 Beta version. Navigate to Rewards page.
  10. Verify that the UI shows the addresses that are in ledger_state and match muon.
    NOTE - backup words are still incorrect. I have logged a separate issue for this: UI is not reflecting updated backup words for wallet (incorrect backup words shown) #3061
Brave 0.59.28 Chromium: 72.0.3626.64 (Official Build) beta(64-bit)
Revision eaa9668e80ce5405e7f1902579558ea725c06ca1-refs/branch-heads/3626@{#708}
OS Mac OS X

@btlechowski
Copy link

btlechowski commented Jan 24, 2019

Verification passed on

Brave 0.59.28 Chromium: 72.0.3626.64 (Official Build) beta(64-bit)
Revision eaa9668e80ce5405e7f1902579558ea725c06ca1-refs/branch-heads/3626@{#708}
OS Windows

Used STR from #2172 (comment)
Used profile provided by @LaurenWags . Thanks!
Encountered #3061 while testing.

Verification passed on

Brave 0.59.28 Chromium: 72.0.3626.64 (Official Build) beta(64-bit)
Revision eaa9668e80ce5405e7f1902579558ea725c06ca1-refs/branch-heads/3626@{#708}
OS Linux

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

Successfully merging a pull request may close this issue.

6 participants