Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

When detecting a corrupted seed #13424

Closed
mrose17 opened this issue Mar 12, 2018 · 21 comments · Fixed by #13551 or #13576
Closed

When detecting a corrupted seed #13424

mrose17 opened this issue Mar 12, 2018 · 21 comments · Fixed by #13551 or #13576

Comments

@mrose17
Copy link
Member

mrose17 commented Mar 12, 2018

Test Plan:
#13576
#13551

If we detect a corrupted seed (when displaying the preferences panel), display a message with a link to an FAQ entry, that explains how to use the recovery file, and, failing that, how to contact to support.

@LaurenWags - please write the FAQ entry, ask catherine to review it, and @jonathansampson to update the website and give you the URL.

@NejcZdovc
Copy link
Contributor

@bradleyrichter can you please prepare text for this one

@bradleyrichter
Copy link
Contributor

@mrose17 I think a short description of the use case would be helpful for writing the associated elements.

For example:

Summary:
In certain circumstances, the user may arrive at the Brave Payments page in Browser Prefs and find no data. Rather than showing a blank page, we can display a message.

The only current known cause of this blank page occurrence is when we run into a corrupted seed upon fetching the user's ledger data.

The only known fix for this is to ask the user to recover their Brave Wallet.

If the user has not backed up their wallet, we might want to apologize.

@mrose17
Copy link
Member Author

mrose17 commented Mar 15, 2018

@bradleyrichter - the use case is very simple. the browser can easily detect if the seed is corrupted. in most cases, the reason is due to a bug in the recovery file generation code (that was fixed several revisions back). when the seed is corrupted, it is not possible to make a contribution. the user needs to fix this.

@LaurenWags - can you put together an entry for the FAQ at https://brave.com/faq-payments/ that explains to the user "how to recover" and then ask catherine to review it and then @jonathansampson to update the site. when that happens, we can put a link to the FAQ on the popup that blocks out the preferences panel. you may also want to suggest text in this issue as to what the message on the popup should say with the link, etc. thanks!

@LaurenWags
Copy link
Member

Yes @mrose17 this is on my list after the b-l and Android releases this week.

@mrose17
Copy link
Member Author

mrose17 commented Mar 15, 2018

thank you!

@bsclifton
Copy link
Member

Awesome- thanks for capturing this, @mrose17 😄 I think this will be a huge help for troubleshooting

@NejcZdovc
Copy link
Contributor

@bradleyrichter do you need anything else from me or @mrose17 ?

@bradleyrichter
Copy link
Contributor

bradleyrichter commented Mar 16, 2018

ok, the thing I was missing here was something like "When Brave Payments loads, it checks the brave wallet for contents. If the wallet is corrupted, it can not be loaded and the user can not perform any transactions. A backup to their wallet needs to be recovered by them, manually. (we can not automate this as we don't store the wallet backup code or it's location for obvious security reasons). If they do not have a backup, they can seek help through a link that takes them to the Brave.com Payments FAQ page.

Here is the recommended solution:

image

Text:
Title: Hello! Unfortunately your active wallet has been corrupted.

Body:
You must recover your backup wallet with your recovery keys before any transactions can be processed. We apologize for the inconvenience.

Link text: View the Brave Payments FAQ…
Link: https://brave.com/faq-payments/

Button Text: Recover your Brave Wallet

Button action:
The button will need to open the wallet recovery dialog. The alert dialog should close upon success of the wallet recovery. If recovery fails, then this alert dialog should remain open.

@NejcZdovc
Copy link
Contributor

adding this issue just for the reference #12672

@LaurenWags
Copy link
Member

First draft of FAQ entry sent to Catherine for feedback.

@LaurenWags
Copy link
Member

@mrose17 FAQ text was reviewed by Catherine and OK-ed. I have sent the text to @jonathansampson

@NejcZdovc
Copy link
Contributor

thank you @LaurenWags

@NejcZdovc NejcZdovc added this to the 0.22.x (Beta Channel) milestone Mar 21, 2018
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Mar 21, 2018
Resolves brave#13424

Auditors:

Test Plan:
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Mar 21, 2018
Resolves brave#13424

Auditors:

Test Plan:
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Mar 21, 2018
Resolves brave#13424

Auditors:

Test Plan:
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Mar 22, 2018
Resolves brave#13424

Auditors:

Test Plan:
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Mar 22, 2018
Resolves brave#13424

Auditors:

Test Plan:
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Mar 22, 2018
Resolves brave#13424

Auditors:

Test Plan:
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Mar 22, 2018
Resolves brave#13424

Auditors:

Test Plan:
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Mar 22, 2018
Resolves brave#13424

Auditors:

Test Plan:
@LaurenWags
Copy link
Member

LaurenWags commented Mar 23, 2018

NejcZdovc bradleyrichter the actual overlay differs slightly from expected overlay (see 13551) - the 'View the Brave Payments FAQ...' and 'Recover your Brave Wallet' buttons aren't on the same line like they are in 13551. Is this expected? cc kjozwiak

Logged #13582

@btlechowski
Copy link
Contributor

btlechowski commented Mar 27, 2018

When I removed one of the seeds, a crash was recorded in terminal, there was no recovery dialog and browser became unstable. Tested on Win7 v0.22.7.

It reproduces(fails) when there is uneven number of seeds and passes when there is even.

Also reproduced on Mac v0.22.7 by @LaurenWags .

Crash:

An uncaught exception occurred in the main process Uncaught Exception:
TypeError: Cannot read property 'isValidPassPhrase' of undefined
    at checkSeed (C:\Users\bartlomiej\AppData\Local\BraveBeta\app-0.22.7\resources\app.asar\app\browser\api\ledger.js:197:23)
    at Object.paymentPresent (C:\Users\bartlomiej\AppData\Local\BraveBeta\app-0.22.7\resources\app.asar\app\browser\api\ledger.js:184:13)
    at ledgerReducer (C:\Users\bartlomiej\AppData\Local\BraveBeta\app-0.22.7\resources\app.asar\app\browser\reducers\ledgerReducer.js:253:27)
    at reducers.reduce (C:\Users\bartlomiej\AppData\Local\BraveBeta\app-0.22.7\resources\app.asar\js\stores\appStore.js:191:24)
    at Array.reduce (<anonymous>)
    at applyReducers (C:\Users\bartlomiej\AppData\Local\BraveBeta\app-0.22.7\resources\app.asar\js\stores\appStore.js:189:68)
    at handleAppAction (C:\Users\bartlomiej\AppData\Local\BraveBeta\app-0.22.7\resources\app.asar\js\stores\appStore.js:253:16)
    at callbacks.forEach (C:\Users\bartlomiej\AppData\Local\BraveBeta\app-0.22.7\resources\app.asar\js\dispatcher\appDispatcher.js:107:7)
    at Array.forEach (<anonymous>)
    at AppDispatcher.dispatchToOwnRegisteredCallbacks (C:\Users\bartlomiej\AppData\Local\BraveBeta\app-0.22.7\resources\app.asar\js\dispatcher\appDispatcher.js:                                   106:20)
    at AppDispatcher.dispatchInternal (C:\Users\bartlomiej\AppData\Local\BraveBeta\app-0.22.7\resources\app.asar\js\dispatcher\appDispatcher.js:132:10)
    at runCallback (timers.js:672:20)
    at tryOnImmediate (timers.js:645:5)
    at processImmediate [as _immediateCallback] (timers.js:617:5)
[0327/153116.947:ERROR:http_transport_win.cc(273)] WinHttpSendRequest: A security error occurred (0x2f8f)

ledger-state.json

{
  "personaId": "826886b1-15e8-4d4c-a9e6-ae9dece977c3",
  "options": {
    "debugP": false,
    "loggingP": false,
    "rulesTestP": false,
    "verboseP": false,
    "server": {
      "protocol": "https:",
      "slashes": true,
      "auth": null,
      "host": "ledger.mercury.basicattentiontoken.org",
      "port": null,
      "hostname": "ledger.mercury.basicattentiontoken.org",
      "hash": null,
      "search": null,
      "query": null,
      "pathname": "/",
      "path": "/",
      "href": "https://ledger.mercury.basicattentiontoken.org/"
    },
    "version": "v2",
    "environment": "production",
    "prefix": "/v2"
  },
  "ballots": [],
  "transactions": [],
  "properties": {
    "setting": "adFree",
    "fee": {
      "currency": "BAT",
      "amount": 7.5
    },
    "days": 30,
    "wallet": {
      "keyinfo": {
        "seed": {
          "0": 9,
          "1": 209,
          "2": 27,
          "3": 236,
          "4": 138,
          "5": 194,
          "6": 124,
          "7": 107,
          "8": 91,
          "9": 227,
          "10": 62,
          "11": 44,
          "12": 35,
          "13": 56,
          "14": 168,
          "15": 54,
          "16": 77,
          "17": 108,
          "18": 139,
          "19": 245,
          "21": 10,
          "22": 172,
          "23": 232,
          "24": 48,
          "25": 137,
          "26": 5,
          "27": 4,
          "28": 174,
          "29": 121,
          "30": 175,
          "31": 47
        }
      },
      "paymentId": "aad63fc8-5018-427f-8a6c-d9798e6ab3bf",
      "addresses": {
        "BAT": "0xDe71e9a3CEaA93389d0ea328d30bdaD717C143Ca",
        "BTC": "17YnuXBYgNimaFV1zULTjfChYG3rBMpvCx",
        "CARD_ID": "53156eed-aaed-4a6e-84a2-4edbfc58d8b5",
        "ETH": "0xDe71e9a3CEaA93389d0ea328d30bdaD717C143Ca",
        "LTC": "LcdAXipT7gAJobUUcXZbWo1pLgKQMdEsps"
      }
    }
  },
  "ruleset": [
    {
      "condition": "SLD === 'youtube.com' && pathname.indexOf('/channel/') === 0",
      "consequent": "'youtube#channel:' + pathname.split('/')[2]",
      "description": "youtube channels"
    },
    {
      "condition": "/^[a-z][a-z].gov$/.test(SLD)",
      "consequent": "QLD + '.' + SLD",
      "description": "governmental sites"
    },
    {
      "condition": "TLD === 'gov' || /^go.[a-z][a-z]$/.test(TLD) || /^gov.[a-z][a-z]$/.test(TLD)",
      "consequent": "SLD",
      "description": "governmental sites"
    },
    {
      "condition": "SLD === 'keybase.pub'",
      "consequent": "QLD + '.' + SLD",
      "description": "keybase users"
    },
    {
      "condition": true,
      "consequent": "SLD",
      "description": "the default rule"
    }
  ],
  "persona": "{\"userId\":\"826886b115e8d4ca9e6ae9dece977c3\",\"registrarVK\":\"==========ANONLOGIN_VK_BEG==========\\n5Zo7fjdryszdwgAZO+HiK18yErXRoPvRkmYmo4E2UQV 4NMDAdwHg0z3LdGN3KkOoFLbza3SpeUWBcMKOUvJ8O1 1\\n7MpAGVMgs+tuMQs4pH+nKUh0M+rl21RyUAdhRHKGhq4 5LXnl7uUw0XwaSTIojyDh4hODTEcT5BAiAhPCGIItDM 1\\n2qtVvVwfxlDGzPSXXl0Ms94eycXiUJpthRp6uAjrjW8 Auhpv58KOuncZfXB1eekbOwTXloSAgPPRJ3A1g2a1MZ 1\\n41nN9zSpWB6xfefhy/BWOouZnE5Vz+Akgtu3WavASEJ AF4pxNogZT38xdPdxe51RIGaFXKNVj3K2vbfN3uzI1T 1\\n31ND6Q8qewDElWUU88rMWn4RZl95sjbSvoimlfsVFEg 8VjyRwkfd4dWwMQhShAIiyY0L9tOiJIR4PWlRIwyQs6 wLm/Ph3gh2ZtdqNbGJIcQM1LoCr+23zELEwUKvAnSL 1sWQdlB2UL0xSqgNcb9NObxiSof322GgHUg2nq43Y4l 1 0\\n===========ANONLOGIN_VK_END==========\",\"masterUserToken\":\"==========ANONLOGIN_CRED_BEG==========\\n826886b115e8d4ca9e6ae9dece977c3\\n63uIBfGn9B/vKcOj8LKoojhqJD0PRfN1Xq8E4Kc1SMo\\n6ODbzEHbvOc10RnAEda3mGFPGQhAPMKGY81P7JvwOe4 BMXEEmyDLz51l29CBLQtnN/ynTI3RaIu5mqEQXE4LTq 1\\n7YW6olyRW5wHt9KYUhBqqa92DWMezMysGbsTJGqoe9F\\n2m12HsWQGUbig3uVlFkHmtkQIQCEFRIdFkgi36uL/s8\\n===========ANONLOGIN_CRED_END==========\"}",
  "bootStamp": 1522157339057,
  "reconcileStamp": 1524749339057
}

@btlechowski btlechowski reopened this Mar 27, 2018
@NejcZdovc
Copy link
Contributor

NejcZdovc commented Mar 27, 2018

I notice this error as well, so I added check in this PR #13607. There will still be an error, but browser will work. If you found any other issues please create another issue.

@LaurenWags
Copy link
Member

Verified on macOS 10.12.6 x64 using the following build:

  • 0.22.9 e0fc152
  • libchromiumcontent: 65.0.3325.181
  • muon: 5.1.2

@kjozwiak
Copy link
Member

Re-opening the issue. When you corrupt the seed by removing a few entries, sometimes the warning overlay never appears, sometimes it does. It's really inconsistent. @LaurenWags ran into the same issue while we were attempting to debug.

The main case that's the most worrisome is when the seed is corrupted and appears as follows:

    "wallet": {
      "keyinfo": {
        "seed": {
          "0": 1
        }
      },

The above is usually the case that happens with users that experience this issue. I've ran into this issue once before about two months ago, and my seed appeared as "seed": { "0": 1 }. However, if you have the above seed, this fix doesn't work and the overlay never appears. When you click on "Backup", you'll see the full recovery code, however the seed has been corrupted. Which state is correct?

STR:

  • launch 0.22.9 e0fc152
  • enable payments via about:preferences#payments
  • once the wallet has been created and has pulled the latest BAT rates, close brave
  • open ledger-state.json and remove all the seeds so you're left with the above example
  • relaunch brave and visit about:preferences#payments

Notice that the corruption overlay isn't being displayed even though the seed in ledger-state.json has almost been completely removed.

  • click on Advanced Settings -> Backup your wallet
  • you'll notice that the entire recovery key is still there

Which state is correct? The seed has been almost removed yet the overlay is not appearing and the recovery file is still being listed.

  • macOS 10.13.3 x64 - Reproduced
  • Ubuntu 17.10 x64 - Reproduced

@bsclifton
Copy link
Member

Found the problem:
Code is fetching the passphrase from session-store-1 in ledger > info > passphrase instead of using what is in ledger-state.json

@bsclifton
Copy link
Member

After review, I don't think this is a problem really... but perhaps we should create another issue (for fixing the seed value in ledger-state.json).

Because the seed is intact in the state, you're able to continue backing up your seed. The ledger should also continue to reconcile just fine.

WDYT, @NejcZdovc?

@NejcZdovc
Copy link
Contributor

NejcZdovc commented Mar 29, 2018

Ok we are going in a wrong direction with this one.

So you need to have "valid broken" seed to actually trigger this overlay. If you change your seed to invalid one we will not save it into the state. So what happens is that on the initial read we try to parse seed from ledger-state.json and if it's not correct we will not save it into the state (you would still have an old one, correct one). State is then responsible for displaying backup keys, overlay, recovery, etc. This is the reason why you are not seeing an overlay.

Based on #12672 broken seed that users were seeing is

"keyinfo": {
   "seed": {
    "0": 0,
    "1": 0
  }

Let's use this valid broken seed for testing all seed related issues. Thing is that we would never save invalid broken seed into ledger-state.

I will fix one potential race condition in the meantime

@kjozwiak
Copy link
Member

Verified on Mint 18.3 x64 using the following build:

  • 0.22.11 56de947
  • libchromiumcontent: 65.0.3325.181
  • muon: 5.1.2

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