Updated code per Marshall

- transition is called for all wallets
- sync is now done before transition
- added logic after transition (state updated via onLedgerCallback)

Auditors: @mrose17

Set wallet to v1 if v1 properties are found
(it should be transitioned on launch)

In the roundtrip callback, add `response` to param list when calling during an error

changes default debug mode

app/browser/api/ledger.js

@@ -50,8 +50,8 @@ let visitsByPublisher = {}
 let bootP
 let quitP
 const _internal = {
-  verboseP: process.env.LEDGER_VERBOSE || true,
-  debugP: process.env.LEDGER_DEBUG || true,
+  verboseP: process.env.LEDGER_VERBOSE || false,
+  debugP: process.env.LEDGER_DEBUG || false,
   ruleset: {
     raw: [],
     cooked: []
@@ -154,14 +154,18 @@ const notifications = {
     }, notifications.pollingInterval, state)
   },
   onLaunch: (state) => {
-    if (hasFunds(state)) {
-      // One time conversion of funds
-      const isNewInstall = state.get('firstRunTimestamp') === state.getIn(['migrations', 'batMercuryTimestamp'])
-      const hasUpgradedWallet = state.getIn(['migrations', 'batMercuryTimestamp']) !== state.getIn(['migrations', 'btcToBatTimestamp'])
-      if (!isNewInstall && !hasUpgradedWallet) {
-        module.exports.transitionWalletToBat(state)
-      }
+    if (!getSetting(settings.PAYMENTS_ENABLED)) {
+      return
+    }
 
+    // One time conversion of wallet
+    const isNewInstall = state.get('firstRunTimestamp') === state.getIn(['migrations', 'batMercuryTimestamp'])
+    const hasUpgradedWallet = state.getIn(['migrations', 'batMercuryTimestamp']) !== state.getIn(['migrations', 'btcToBatTimestamp'])
+    if (!isNewInstall && !hasUpgradedWallet) {
+      module.exports.transitionWalletToBat(state)
+    }
+
+    if (hasFunds(state)) {
       // Don't bother processing the rest, which are only notifications.
       if (!getSetting(settings.PAYMENTS_NOTIFICATIONS)) {
         return
@@ -1476,16 +1480,18 @@ const roundtrip = (params, options, callback) => {
       console.log('>>> ' + (body || '').split('\n').join('\n>>> '))
     }
 
-    if (err) return callback(err)
+    if (err) return callback(err, response)
 
     if (Math.floor(response.statusCode / 100) !== 2) {
-      return callback(new Error('HTTP response ' + response.statusCode) + ' for ' + params.method + ' ' + params.path)
+      return callback(
+        new Error('HTTP response ' + response.statusCode + ' for ' + params.method + ' ' + params.path),
+        response)
     }
 
     try {
       payload = rawP ? body : (response.statusCode !== 204) ? JSON.parse(body) : null
     } catch (err) {
-      return callback(err)
+      return callback(err, response)
     }
 
     try {
@@ -1946,8 +1952,16 @@ const onInitRead = (state, parsedData) => {
   try {
     let timeUntilReconcile
     clientprep()
+
+    const options = Object.assign({}, clientOptions)
+    try {
+      if (parsedData.properties.wallet.keychains.user) {
+        options.version = 'v1'
+      }
+    } catch (ex) {}
+
     client = ledgerClient(parsedData.personaId,
-      underscore.extend(parsedData.options, {roundtrip: roundtrip}, clientOptions),
+      underscore.extend(parsedData.options, {roundtrip: roundtrip}, options),
       parsedData)
 
     // Scenario: User enables Payments, disables it, waits 30+ days, then
@@ -1970,7 +1984,7 @@ const onInitRead = (state, parsedData) => {
       })
     }
   } catch (ex) {
-    console.error('ledger client creation error: ', ex)
+    console.error('ledger client creation error(1): ', ex)
     return state
   }
 
@@ -2222,28 +2236,44 @@ const deleteSynopsis = () => {
   synopsis.publishers = {}
 }
 
+let newClient = null
 const transitionWalletToBat = (state) => {
-  let newClient
+  let newPaymentId, result
 
-  try {
-    clientprep()
-    newClient = ledgerClient(null, underscore.extend({roundtrip: roundtrip}, clientOptions), null)
-  } catch (ex) {
-    console.error('exception during ledger client creation: ', ex)
+  if (newClient === true) return
+
+  if (!newClient) {
+    try {
+      clientprep()
+      newClient = ledgerClient(null, underscore.extend({roundtrip: roundtrip}, clientOptions), null)
+    } catch (ex) {
+      return console.error('ledger client creation error(2): ', ex)
+    }
+  }
+
+  newPaymentId = newClient.getPaymentId()
+  if (!newPaymentId) {
+    newClient.sync((err, result, delayTime) => {
+      if (err) {
+        return console.log('ledger client error(3): ' + JSON.stringify(err, null, 2) + (err.stack ? ('\n' + err.stack) : ''))
+      }
+
+      if (typeof delayTime === 'undefined') delayTime = random.randomInt({ min: 1, max: 500 })
+
+      setTimeout(() => transitionWalletToBat(state), delayTime)
+    })
     return
   }
 
   try {
-    // TODO: this line is breaking
-    // exception during ledger client transition:  TypeError: Cannot read property 'wallet' of undefined
-    client.transition(newClient.properties.wallet.paymentId, (err) => {
+    client.transition(newPaymentId, (err, properties) => {
       if (err) {
         console.error('ledger client transition error: ', err)
       } else {
-        // save previous client transactions
-        newClient.state.transactions = client.state.transactions
-        // persist client's state, overwriting the old state
+        result = newClient.transitioned(properties)
         client = newClient
+        newClient = true
+        appActions.onLedgerCallback(result, random.randomInt({ min: miliseconds.minute, max: 10 * miliseconds.minute }))
         appActions.onBitcoinToBatTransitioned()
       }
     })

package.json

@@ -88,7 +88,7 @@
     "aphrodite": "1.1.0",
     "async": "^2.0.1",
     "bat-balance": "^0.9.8",
-    "bat-client": "^0.9.20",
+    "bat-client": "^0.9.22",
     "bat-publisher": "^0.9.4",
     "bignumber.js": "^4.0.4",
     "bloodhound-js": "brave/bloodhound",

test/unit/app/browser/api/ledgerTest.js

@@ -20,8 +20,12 @@ describe('ledger api unit tests', function () {
   let paymentsEnabled
   let paymentsNotifications
   let ledgerClient
+
+  // spies
   let ledgerTransitionSpy
+  let ledgerTransitionedSpy
   let onBitcoinToBatTransitionedSpy
+  let onLedgerCallbackSpy
 
   before(function () {
     this.clock = sinon.useFakeTimers()
@@ -49,6 +53,7 @@ describe('ledger api unit tests', function () {
     })
     mockery.registerMock('../../../js/actions/appActions', appActions)
     onBitcoinToBatTransitionedSpy = sinon.spy(appActions, 'onBitcoinToBatTransitioned')
+    onLedgerCallbackSpy = sinon.spy(appActions, 'onLedgerCallback')
 
     // ledger client stubbing
     ledgerClient = sinon.stub()
@@ -68,18 +73,25 @@ describe('ledger api unit tests', function () {
       transition: function (paymentId, callback) {
         callback()
       },
+      getPaymentId: function () {
+        return 'payementIdGoesHere'
+      },
       properties: {
         wallet: {
           paymentId: 12345
         }
       },
+      transitioned: function () {
+        return {}
+      },
       state: {
         transactions: []
       }
     }
     ledgerClient.prototype.boolion = function (value) { return false }
     ledgerClient.prototype.getWalletPassphrase = function (state) {}
     ledgerTransitionSpy = sinon.spy(lc, 'transition')
+    ledgerTransitionedSpy = sinon.spy(lc, 'transitioned')
     ledgerClient.returns(lc)
     mockery.registerMock('bat-client', ledgerClient)
 
@@ -134,6 +146,8 @@ describe('ledger api unit tests', function () {
       const batState = ledgerApi.onBootStateFile(defaultAppState)
       ledgerTransitionSpy.reset()
       onBitcoinToBatTransitionedSpy.reset()
+      onLedgerCallbackSpy.reset()
+      ledgerTransitionedSpy.reset()
       ledgerClient.reset()
       ledgerApi.transitionWalletToBat(batState)
     })
@@ -143,8 +157,16 @@ describe('ledger api unit tests', function () {
     it('calls client.transition', function () {
       assert(ledgerTransitionSpy.calledOnce)
     })
-    it('calls AppActions.onBitcoinToBatTransitioned', function () {
-      assert(onBitcoinToBatTransitionedSpy.calledOnce)
+    describe('when transition completes', function () {
+      it('calls client.transitioned', function () {
+        assert(ledgerTransitionedSpy.calledOnce)
+      })
+      it('calls AppActions.onLedgerCallback', function () {
+        assert(onLedgerCallbackSpy.calledOnce)
+      })
+      it('calls AppActions.onBitcoinToBatTransitioned', function () {
+        assert(onBitcoinToBatTransitionedSpy.calledOnce)
+      })
     })
   })
 
@@ -340,13 +362,13 @@ describe('ledger api unit tests', function () {
               paymentsEnabled = true
               paymentsNotifications = true
             })
-            it('does not call ledger.transitionWalletToBat', function () {
+            it('calls ledger.transitionWalletToBat', function () {
               const ledgerStateWithoutBalance = ledgerStateWithBalance
                 .setIn(['ledger', 'info', 'balance'], 0)
                 .setIn(['migrations', 'batMercuryTimestamp'], 32145)
                 .setIn(['migrations', 'btcToBatTimestamp'], 32145)
               ledgerApi.notifications.onLaunch(ledgerStateWithoutBalance)
-              assert(transitionWalletToBatSpy.notCalled)
+              assert(transitionWalletToBatSpy.calledOnce)
             })
           })
 
@@ -375,7 +397,7 @@ describe('ledger api unit tests', function () {
             })
           })
 
-          describe('when payment notifications are enabled, payments are enabled, user has funds, user had wallet before BAT Mercury, and user not been shown message yet', function () {
+          describe('when payments are enabled and user had wallet before BAT Mercury', function () {
             before(function () {
               paymentsEnabled = true
               paymentsNotifications = true