Fix persistence edge-case issue with ledger client

[bsclifton]

• client is now persisted after sync
• file is now cleaned up after transition is complete (when new ledger file is being written)

Fixes #11494

Auditors: @evq

Submitter Checklist:

• Submitted a ticket for my issue if one did not already exist.
• Used Github auto-closing keywords in the commit message.
• Added/updated tests for this change (for new code or code which already has tests).
• Ran git rebase -i to squash commits (if needed).
• Tagged reviewers and labelled the pull request as needed.

Test Plan:

Reviewer Checklist:

Tests

• Adequate test coverage exists to prevent regressions
• Tests should be independent and work correctly when run individually or as a suite ref
• New files have MPL2 license header

[codecov-io]

Codecov Report

Merging #11495 into master will decrease coverage by 0.04%.
The diff coverage is 38.46%.

@@            Coverage Diff             @@
##           master   #11495      +/-   ##
==========================================
- Coverage    52.5%   52.45%   -0.05%     
==========================================
  Files         268      268              
  Lines       25241    25248       +7     
  Branches     4028     4030       +2     
==========================================
- Hits        13252    13245       -7     
- Misses      11989    12003      +14

Flag	Coverage Δ
#unittest	52.45% <38.46%> (-0.05%)	⬇️

Impacted Files	Coverage Δ
app/browser/api/ledger.js	31.11% <38.46%> (-0.53%)	⬇️

[evq]

There might still be an edge case where the callback from onBitcoinToBatTransitioned is handled first and the browser exits before onLedgerCallback is handled. So the ledger-state.json would be BTC, ledger-newstate.json would be BAT but the browser would not attempt to finish the transition process (since it would believe it was already over).

[evq]

I'm in favor of merging this as is and opening up a new issue for the above if it is indeed possible.

The timing required to hit the edge case I mentioned above would have to be much tighter than the edge case this fixes.

[NejcZdovc]

Can we change this to const newClientPath = pathName('ledger-newstate.json'), so that we call pathName only once. Same can be done with statePath

[bsclifton]

I looked at doing this, but since it uses electron.app.getPath() (which may or may not be initialized at the time module is loaded), I think changing that may be better to do in a different PR. Thanks for the suggestion though 😄

[bsclifton]

Ran into an issue while testing... Here's what I did:

• created branch new v1 wallet using Brave 0.18.x
• switch to this branch / rm node_modules / npm install / etc and pointed it at the 0.18.x session
• wallet upgrade shows as in progress 👍
• I quit and relaunched a few times- I verified that ledger-newstate.json gets persisted before and after after sync 👍
• I seem to be getting an error now, HTTP code 422

<<< POST https://ledger.mercury.basicattentiontoken.org/v2/registrar/persona/1ed2a2b0748089c8ccd81e7dac8c893
<<< content-type: application/json; charset=utf-8
<<< accept-encoding:
<<<
<<< {
<<<   "requestType": "httpSignature",
<<<   "request": {
<<<     "headers": {
<<<       "digest": "SHA-256=oe2fifEDcfI96boPcKaUUhe/HKWiuzyoBs10Gm+S4bg=",
<<<       "signature": "keyId=\"primary\",algorithm=\"ed25519\",headers=\"digest\",signature=\"vglrshpOIBjtoW6OBmW70pH7/RFK7vwzY+a3a9rDdwGxIU9VTBgj0Snm2AEHrqQUuR3mA+EX817oh/z/607FCg==\""
<<<     },
<<<     "body": {
<<<       "label": "4123099f-47e4-48ad-bc9f-6cdffebd81ed",
<<<       "currency": "BAT",
<<<       "publicKey": "429fb47298b7b058030c3d1bca1ca587585a81b1e47f0ecc9436e04306210043"
<<<     },
<<<     "octets": "{\"currency\":\"BAT\",\"label\":\"4123099f-47e4-48ad-bc9f-6cdffebd81ed\",\"publicKey\":\"429fb47298b7b058030c3d1bca1ca587585a81b1e47f0ecc9436e04306210043\"}"
<<<   },
<<<   "proof": "1ed2a2b0748089c8ccd81e7dac8c893\n7FxI5Yngr+RGK1uVGD9rWf/RMzpzMz2irqOoj6t9DOH 5xbHQ55+WSAbb5PjpJhKRGY/1u65rlE2BEXkt/HSBMe 1\n13D36D8a7pG56RdRGmI0NoNPY7AjP4YaYh+fTjIjFTv 8gwqQ7TkDtR9rJ6XNhQ6j5RpWY6CEbFKtCMPks4UmkM 1\n2YOcc2YrRugyGZOJrgDOGhBWwO/6+OsMCaJ0RbbjOx9\n5j3+EjAim1nxwiO7UbO8Gc17maim1QxYP5pMWTgUh2b 4WdiDvvTxgXwZc2xueVmFhnvAJ0CawmUaCDZ6Bsr5G5\n\n6PDzgyvawQBE+Ndu7yAV3XiGyV4gcnRKV74Q5QqQYvS 7i16apf3Fk4v3bdROu1fm9DEVrFRCFx0qhzY1woyJcB 1\nF5KSCVbKPt+9RXeAPt7WCaAwVjAeEf1UhxvsjhEYMTg\nBETV/Mnj5p0TjkQO6/MEbQrUPXdoRCXBfAH/fPDF+Ai 3UqevUsbtwxqNn9hVwaZeh13SlCYYDuaP9brmW9wz9n\n\n5wSFmTSkk3rgWr90uyHne1ZQ3c4xfsCuGMAPw0bHw56 4yIpZvGfdV+6pi0WMSZ55knoA1WJ0RK+dZdS2qXd01n 1\nEyxwGIpFJnew4/ltJlP9bmKSHIl/YOGai0VgBSxKVmm\n9cD1As+2yjj4+bgD56OlhDiRnihOvU4dgOtGskhfx8w 8RF3dVpPL0hSBfV1DM+VmDqNBBJw0qu0mDx2NvTFOcC\n\n3GdmB6jP/KLU/zk9ya2f0GO+O1rjkv/yiSKxDZm57XD 4P/fkCg3ka+pw0o+aByXOnx+CM9dkTNLi/nVVG6SHek 1\n3AmjLTzsgwrq7yy4VvobP6XC0YCINMGpdX+uEMEYcJ\nAjfAU8PaYhcZHNhBJtGUsnk1khsH1I9WS3mbYHmc3RK B1lMaDXW+LwBT7z4hK7QXarvMVO/7G4/uF482IAd0zA\n\n\n"
<<< }
[ response for POST https://ledger.mercury.basicattentiontoken.org/v2/registrar/persona/1ed2a2b0748089c8ccd81e7dac8c893 ]
>>> HTTP/1.1 422
>>> cache-control: no-cache
>>> connection: keep-alive
>>> content-length: 120
>>> content-type: application/json; charset=utf-8
>>> date: Fri, 13 Oct 2017 06:02:22 GMT
>>> server: Cowboy
>>> vary: accept-encoding
>>> via: 1.1 vegur
>>> x-rate-limit-limit: 60
>>> x-rate-limit-remaining: 59
>>> x-rate-limit-reset: 1507874601
>>>
>>> {"statusCode":422,"error":"Unprocessable Entity","message":"persona credential exists: 1ed2a2b0748089c8ccd81e7dac8c893"}
ledger client error(3): {}
Error: HTTP response 422 for POST /v2/registrar/persona/1ed2a2b0748089c8ccd81e7dac8c893
    at request.request (/Users/clifton/Documents/browser-laptop/app/browser/api/ledger.js:1491:9)
    at Function.defaultSession.webRequest.fetch (/Users/clifton/Documents/browser-laptop/js/lib/request.js:55:5)

[cezaraugusto]

sorry but what's the status of this? error mentioned by @bsclifton seems related to a different issue, is this blocked on something?

[bsclifton]

@cezaraugusto thanks for checking up on this 😄

I checked with @evq and we're good to merge this! I captured the above issue with brave-intl/bat-client#18 so that we can investigate further

[bsclifton]

master e644744
0.21.x a682e86
0.20.x b8f887c
0.19.x 4904078