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

Importing Brave Payments from Brave (old) is not working #2312

Closed
bsclifton opened this issue Nov 30, 2018 · 1 comment · Fixed by brave/brave-core#991
Closed

Importing Brave Payments from Brave (old) is not working #2312

bsclifton opened this issue Nov 30, 2018 · 1 comment · Fixed by brave/brave-core#991

Comments

@bsclifton
Copy link
Member

Description

Sometime after brave/brave-core#736 was merged to master, the behavior of the ledger API seems to have changed. This causes a situation where the import fails

Steps to Reproduce

  1. Fresh profile
  2. Build from source (master)
  3. Have a Muon profile setup, with a Brave Payments wallet (with a balance)
  4. Launch Brave Core
  5. Visit chrome://settings/importData, pick Brave (old), and only choose Brave Payments
  6. Run import process

Actual result:

Calls appear to happen in this order:

  1. Fresh profile; profile write calls CreateWallet
  2. GetWalletProperties (handler) is called BEFORE CreateWallet finishes
  3. GetWalletProperties shows a balance of 0; it attempts to backup the wallet (basically copying the ledger_state file to something like ledger_import_backup_1543557117056)
  4. The CreateWallet call (from step 1) finally finishes- RecoverWallet is now called
  5. The backup kicked off by step 3 finally gets executed; it sees a positive balance (because step 4 has executed). This cancels the import

Expected result:

The code should be robust enough to handle observers firing out of order. For example, in the above, step 2 should be skipped because the wallet hasn't been created yet.

Reproduces how often:

100%

Brave version (brave://version info)

@bsclifton
Copy link
Member Author

bsclifton commented Nov 30, 2018

Adding QA/No as this is already covered with test plan in brave/brave-core#736

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