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

Implement wallet import/migration for existing users. Follow up to #24 #1215

Closed
srirambv opened this issue Sep 20, 2018 · 8 comments
Closed

Comments

@srirambv
Copy link
Contributor

srirambv commented Sep 20, 2018

Test plan

See brave/brave-core#736

Description

Implement wallet import/migration for existing users. Follow up to #24 Current import only works for Bookmarks/Stats/Password/History/Cookies.

Brave version (chrome://version info)

Brave 0.55.5 Chromium: 70.0.3538.16 (Official Build) dev (64-bit)
Revision 16ed95b41bb05e565b11fb66ac33c660b721f778-refs/branch-heads/3538@{#306}
OS Linux
@kjozwiak
Copy link
Member

kjozwiak commented Sep 20, 2018

We need to have a discussion to figure out what we're going to import from muon in terms of payments.. Example:

  • currently accepted grants
  • current balance
  • payments history reports
  • ledger table
  • payment settings (allow videos etc..)
  • previously created crypto addresses (litcoin, ETH, BTC)

@bbondy
Copy link
Member

bbondy commented Sep 22, 2018

I think as long as wallet is recoverable we shouldn't block 0.55.x on anything else. Reports can still be obtained for example in the old browser. Let me know if you disagree or feel strongly, I'll put to 1.x for now.

@bbondy bbondy modified the milestones: Releasable builds 0.55.x, 1.x Backlog Sep 22, 2018
@rebron rebron modified the milestones: 1.x Backlog, 1.0 (0.56.x) Sep 28, 2018
@garrettr garrettr self-assigned this Oct 11, 2018
@garrettr
Copy link
Contributor

We need to keep the following use case in mind while developing this feature:

  1. User downloads Brave Browser (e.g. 0.55, which does not have wallet auto-import) and creates a new Brave Rewards wallet.
  2. User decides they want to import their old wallet from browser-laptop.

As it stands today, if a user imports their old wallet after creating a new wallet, the imported wallet will replace the one that was already created. This could confuse users, and if they've already funded the new wallet, it could lead to them losing funds.

Some ideas for mitigation:

  1. If a user tries to import a wallet when one already exists, merge the wallets (are there any reasons why we might not want to do this?).
  2. If a user tries to import a wallet when one already exists, warn them of the conflict and abort wallet import.
  3. Re-order the brave://welcome first-run flow so "Import bookmarks and settings" comes before "Enable Brave Rewards." This should reduce (but will not eliminate) the probability that a conflict will occur.

@bbondy
Copy link
Member

bbondy commented Oct 20, 2018

The only reason not to do # 1 would be based on complexity. Do you have a way to do this in a reasonable timeframe? Mainly keeping in mind that we need to upgrade users for 0.56.x timeline.

If we fall to # 2 I would think allow overwrite if 0 balance or ask if they would like to proceed even know there's a balance. I think a user could for example backup their wallet and restore to profile 2 and then legit want to replace profile 1's wallet.

For # 3 I think we should do this too, I think I seen some slack conversations for this, but please go ahead and do this if you get sign-off from product.

@NejcZdovc NejcZdovc modified the milestones: 1.0, 0.56.x - Beta Oct 22, 2018
@mandar-brave
Copy link

mandar-brave commented Oct 24, 2018

@garrettr @NejcZdovc @evq @bsclifton
Once the Muon users are upgraded, their reconcile dates will be changed. However, to ensure the dates are spread out evenly across the month (to ensure privacy for every user), the dates need to randomly spread from the time of upgrade from day 25 to day 40 for every upgrade at the client end itself. The reconcile dates assume that the upgrade for Muon users will be staggered for the first two weeks from the server end (day 0 to day 14 is upgrading at 5% a day)

@rebron
Copy link
Collaborator

rebron commented Oct 24, 2018

@garrettr
For "3. Re-order the brave://welcome first-run flow so "Import bookmarks and settings" comes before "Enable Brave Rewards." This should reduce (but will not eliminate) the probability that a conflict will occur."

This looks like a good solution.
@rossmoody put together a new simplified welcome flow to support this change.

@kjozwiak kjozwiak modified the milestones: 1.x Backlog, 0.57.x - Beta Nov 26, 2018
@bsclifton
Copy link
Member

Fixed with brave/brave-core#736

@btlechowski
Copy link

btlechowski commented Dec 6, 2018

Verification Passed on

Brave 0.57.17 Chromium: 71.0.3578.80 (Official Build) (64-bit)
Revision 2ac50e7249fbd55e6f517a28131605c9fb9fe897-refs/branch-heads/3578@{#860}
OS Windows 7

Used STR from brave/brave-core#736
When the wallet already exists in b-c, the import is aborted.
Also imported using --upgrade-from-muon flag

Verified import is canceled when Brave Payments in b-l are turned off
Verified that following settings were imported:

  • Minimum page time before logging a visit
  • Minimum visits for publisher relevancy
  • Allow contributions to non-verified sites
  • Allow contributions to videos
  • Wallet (this includes Grants; attached to wallet by way of server query) Checked addresses in UI and ledger_state
  • Monthly Budget
  • Pinned sites (Tips)
  • Turning auto-contribute on/off based on ledger table and pinned sites

Verified passed with

Brave 0.57.18 Chromium: 71.0.3578.80 (Official Build) (64-bit)
Revision 2ac50e7249fbd55e6f517a28131605c9fb9fe897-refs/branch-heads/3578@{#860}
OS Mac OS X
  • Verified via test plan in Import Brave Payments data from Muon brave-core#736
  • Verified via --upgrade-from-muon flag and manual Import.
  • Verified import is canceled when Brave Payments in b-l are turned off
  • Verified that settings were imported.
  • Verified wallet imported (Checked addresses in UI and ledger_state)
  • Verified various monthly budget amounts imported (and were lowered if necessary due to recurring tips)
  • Verified if they existed, pinned sites became recurring tips with their amount calculated with % pinned and monthly budget

Verification passed on

Brave 0.57.18 Chromium: 71.0.3578.80 (Official Build) (64-bit)
Revision 2ac50e7249fbd55e6f517a28131605c9fb9fe897-refs/branch-heads/3578@{#860}
OS Linux
  • Verified manual import via --upgrade-from-muon imports payments
  • Verified all payments settings are retained on rewards
  • Verified wallet balance is retained
  • Verified recovery code is same as in payments
  • Verified pinned publishers became reccuring tips with rounded values
  • Verified import fails when payments is turned off but wallet exists on muon

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