Merge pull request #387 from isocolsky/fix/scan

Fix/scan

lib/blockchainexplorers/insight.js

@@ -23,7 +23,7 @@ var _parseErr = function(err, res) {
     log.warn('Insight error: ', err);
     return "Insight Error";
   }
-  log.warn("Insight " + res.request.href  + " Returned Status: " + res.statusCode);
+  log.warn("Insight " + res.request.href + " Returned Status: " + res.statusCode);
   return "Error querying the blockchain";
 };
 
@@ -45,7 +45,7 @@ Insight.prototype.getUnspentUtxos = function(addresses, cb) {
   };
 
   request(args, function(err, res, unspent) {
-    if (err || res.statusCode !== 200) return cb(_parseErr(err,res));
+    if (err || res.statusCode !== 200) return cb(_parseErr(err, res));
     return cb(null, unspent);
   });
 };
@@ -64,7 +64,7 @@ Insight.prototype.broadcast = function(rawTx, cb) {
   };
 
   request(args, function(err, res, body) {
-    if (err || res.statusCode !== 200) return cb(_parseErr(err,res));
+    if (err || res.statusCode !== 200) return cb(_parseErr(err, res));
     return cb(null, body ? body.txid : null);
   });
 };
@@ -74,12 +74,13 @@ Insight.prototype.getTransaction = function(txid, cb) {
   var args = {
     method: 'GET',
     url: url,
+    json: true,
   };
 
   request(args, function(err, res, tx) {
-    if (res && res.statusCode == 404 ) return cb();
+    if (res && res.statusCode == 404) return cb();
-    if (err || res.statusCode !== 200) 
+    if (err || res.statusCode !== 200)
-      return cb(_parseErr(err,res));
+      return cb(_parseErr(err, res));
 
     return cb(null, tx);
   });
@@ -100,7 +101,7 @@ Insight.prototype.getTransactions = function(addresses, from, to, cb) {
   };
 
   request(args, function(err, res, txs) {
-    if (err || res.statusCode !== 200) return cb(_parseErr(err,res));
+    if (err || res.statusCode !== 200) return cb(_parseErr(err, res));
 
     if (_.isObject(txs) && txs.items)
       txs = txs.items;
@@ -112,10 +113,21 @@ Insight.prototype.getTransactions = function(addresses, from, to, cb) {
   });
 };
 
-Insight.prototype.getAddressActivity = function(addresses, cb) {
+Insight.prototype.getAddressActivity = function(address, cb) {
-  this.getTransactions(addresses, null, null, function(err, result) {
+  var url = this.url + this.apiPrefix + '/addr/' + address;
-    if (err) return cb(err);
+  var args = {
-    return cb(null, result && result.length > 0);
+    method: 'GET',
+    url: url,
+    json: true,
+  };
+
+  request(args, function(err, res, result) {
+    if (res && res.statusCode == 404) return cb();
+    if (err || res.statusCode !== 200)
+      return cb(_parseErr(err, res));
+
+    var nbTxs = result.unconfirmedTxApperances + result.txApperances;
+    return cb(null, nbTxs > 0);
   });
 };
 
@@ -131,7 +143,7 @@ Insight.prototype.estimateFee = function(nbBlocks, cb) {
     json: true,
   };
   request(args, function(err, res, body) {
-    if (err || res.statusCode !== 200) return cb(_parseErr(err,res));
+    if (err || res.statusCode !== 200) return cb(_parseErr(err, res));
     return cb(null, body);
   });
 };

lib/server.js

@@ -64,8 +64,7 @@ WalletService.BACKOFF_TIME = 2;
 
 // Fund scanning parameters
 WalletService.SCAN_CONFIG = {
-  scanWindow: 20,
+  maxGap: 20,
-  derivationDelay: 10, // in milliseconds
 };
 
 WalletService.FEE_LEVELS = [{
@@ -1878,43 +1877,30 @@ WalletService.prototype.scan = function(opts, cb) {
 
   opts = opts || {};
 
-  function deriveAddresses(size, derivator, cb) {
+  function checkActivity(address, network, cb) {
-    async.mapSeries(_.range(size), function(i, next) {
+    var bc = self._getBlockchainExplorer(network);
-      setTimeout(function() {
+    bc.getAddressActivity(address, cb);
-        next(null, derivator.derive());
-      }, WalletService.SCAN_CONFIG.derivationDelay)
-    }, cb);
-  };
-
-  function checkActivity(addresses, networkName, cb) {
-    var bc = self._getBlockchainExplorer(networkName);
-    bc.getAddressActivity(addresses, cb);
   };
 
   function scanBranch(derivator, cb) {
-    var activity = true;
+    var inactiveCounter = 0;
     var allAddresses = [];
-    var networkName;
+    var gap = WalletService.SCAN_CONFIG.maxGap;
-    async.whilst(function() {
-      return activity;
-    }, function(next) {
-      deriveAddresses(WalletService.SCAN_CONFIG.scanWindow, derivator, function(err, addresses) {
-        if (err) return next(err);
-        networkName = networkName || Bitcore.Address(addresses[0].address).toObject().network;
-        checkActivity(_.pluck(addresses, 'address'), networkName, function(err, thereIsActivity) {
-          if (err) return next(err);
 
-          activity = thereIsActivity;
+    async.whilst(function() {
-          if (thereIsActivity) {
+      return inactiveCounter < gap;
-            allAddresses.push(addresses);
+    }, function(next) {
-          } else {
+      var address = derivator.derive();
-            derivator.rewind(WalletService.SCAN_CONFIG.scanWindow);
+      checkActivity(address.address, address.network, function(err, activity) {
-          }
+        if (err) return next(err);
-          next();
+
-        });
+        allAddresses.push(address);
+        inactiveCounter = activity ? 0 : inactiveCounter + 1;
+        next();
       });
     }, function(err) {
-      return cb(err, _.flatten(allAddresses));
+      derivator.rewind(gap);
+      return cb(err, _.dropRight(allAddresses, gap));
     });
   };
 

test/integration/server.js

@@ -237,8 +237,8 @@ helpers.stubFeeLevels = function(levels) {
 };
 
 helpers.stubAddressActivity = function(activeAddresses) {
-  blockchainExplorer.getAddressActivity = function(addresses, cb) {
+  blockchainExplorer.getAddressActivity = function(address, cb) {
-    return cb(null, _.intersection(activeAddresses, addresses).length > 0);
+    return cb(null, _.contains(activeAddresses, address));
   };
 };
 
@@ -4771,8 +4771,7 @@ describe('Wallet service', function() {
     describe('1-of-1 wallet (BIP44 & P2PKH)', function() {
       beforeEach(function(done) {
         this.timeout(5000);
-        WalletService.SCAN_CONFIG.scanWindow = 2;
+        WalletService.SCAN_CONFIG.maxGap = 2;
-        WalletService.SCAN_CONFIG.derivationDelay = 0;
 
         helpers.createAndJoinWallet(1, 1, function(s, w) {
           server = s;
@@ -4794,9 +4793,7 @@ describe('Wallet service', function() {
           'm/0/0',
           'm/0/1',
           'm/0/2',
-          'm/0/3',
           'm/1/0',
-          'm/1/1',
         ];
         server.scan({}, function(err) {
           should.not.exist(err);
@@ -4810,13 +4807,96 @@ describe('Wallet service', function() {
               _.difference(paths, expectedPaths).length.should.equal(0);
               server.createAddress({}, function(err, address) {
                 should.not.exist(err);
-                address.path.should.equal('m/0/4');
+                address.path.should.equal('m/0/3');
                 done();
               });
             });
           });
         });
       });
+
+      it('should not go beyond max gap', function(done) {
+        helpers.stubAddressActivity(
+          ['1L3z9LPd861FWQhf3vDn89Fnc9dkdBo2CG', // m/0/0
+            '1GdXraZ1gtoVAvBh49D4hK9xLm6SKgesoE', // m/0/2
+            '1DY9exavapgnCUWDnSTJe1BPzXcpgwAQC4', // m/0/5
+            '1LD7Cr68LvBPTUeXrr6YXfGrogR7TVj3WQ', // m/1/3
+          ]);
+        var expectedPaths = [
+          'm/0/0',
+          'm/0/1',
+          'm/0/2',
+        ];
+        server.scan({}, function(err) {
+          should.not.exist(err);
+          server.getWallet({}, function(err, wallet) {
+            should.not.exist(err);
+            wallet.scanStatus.should.equal('success');
+            server.storage.fetchAddresses(wallet.id, function(err, addresses) {
+              should.exist(addresses);
+              addresses.length.should.equal(expectedPaths.length);
+              var paths = _.pluck(addresses, 'path');
+              _.difference(paths, expectedPaths).length.should.equal(0);
+              server.createAddress({}, function(err, address) {
+                should.not.exist(err);
+                address.path.should.equal('m/0/3');
+                // A rescan should see the m/0/5 address initially beyond the gap
+                server.scan({}, function(err) {
+                  server.createAddress({}, function(err, address) {
+                    should.not.exist(err);
+                    address.path.should.equal('m/0/6');
+                    done();
+                  });
+                });
+              });
+            });
+          });
+        });
+      });
+
+      it('should not affect indexes on new wallet', function(done) {
+        helpers.stubAddressActivity([]);
+        server.scan({}, function(err) {
+          should.not.exist(err);
+          server.getWallet({}, function(err, wallet) {
+            should.not.exist(err);
+            wallet.scanStatus.should.equal('success');
+            server.storage.fetchAddresses(wallet.id, function(err, addresses) {
+              should.not.exist(err);
+              addresses.length.should.equal(0);
+              server.createAddress({}, function(err, address) {
+                should.not.exist(err);
+                address.path.should.equal('m/0/0');
+                done();
+              });
+            });
+          });
+        });
+      });
+
+      it('should not rewind already generated addresses on error', function(done) {
+        server.createAddress({}, function(err, address) {
+          should.not.exist(err);
+          address.path.should.equal('m/0/0');
+          blockchainExplorer.getAddressActivity = sinon.stub().callsArgWith(1, 'dummy error');
+          server.scan({}, function(err) {
+            should.exist(err);
+            err.toString().should.equal('dummy error');
+            server.getWallet({}, function(err, wallet) {
+              should.not.exist(err);
+              wallet.scanStatus.should.equal('error');
+              wallet.addressManager.receiveAddressIndex.should.equal(1);
+              wallet.addressManager.changeAddressIndex.should.equal(0);
+              server.createAddress({}, function(err, address) {
+                should.not.exist(err);
+                address.path.should.equal('m/0/1');
+                done();
+              });
+            });
+          });
+        });
+      });
+
       it('should restore wallet balance', function(done) {
         async.waterfall([
 
@@ -4858,6 +4938,7 @@ describe('Wallet service', function() {
           done();
         });
       });
+
       it('should abort scan if there is an error checking address activity', function(done) {
         blockchainExplorer.getAddressActivity = sinon.stub().callsArgWith(1, 'dummy error');
         server.scan({}, function(err) {
@@ -4882,8 +4963,7 @@ describe('Wallet service', function() {
 
       beforeEach(function(done) {
         this.timeout(5000);
-        WalletService.SCAN_CONFIG.scanWindow = 2;
+        WalletService.SCAN_CONFIG.maxGap = 2;
-        WalletService.SCAN_CONFIG.derivationDelay = 0;
 
         helpers.createAndJoinWallet(1, 2, {
           supportBIP44AndP2PKH: false
@@ -4907,9 +4987,7 @@ describe('Wallet service', function() {
           'm/2147483647/0/0',
           'm/2147483647/0/1',
           'm/2147483647/0/2',
-          'm/2147483647/0/3',
           'm/2147483647/1/0',
-          'm/2147483647/1/1',
         ];
         server.scan({}, function(err) {
           should.not.exist(err);
@@ -4923,7 +5001,7 @@ describe('Wallet service', function() {
               _.difference(paths, expectedPaths).length.should.equal(0);
               server.createAddress({}, function(err, address) {
                 should.not.exist(err);
-                address.path.should.equal('m/2147483647/0/4');
+                address.path.should.equal('m/2147483647/0/3');
                 done();
               });
             });
@@ -4940,15 +5018,11 @@ describe('Wallet service', function() {
           ]);
         var expectedPaths = [
           'm/2147483647/0/0',
-          'm/2147483647/0/1',
           'm/2147483647/1/0',
-          'm/2147483647/1/1',
           'm/0/0/0',
           'm/0/0/1',
           'm/1/0/0',
-          'm/1/0/1',
           'm/1/1/0',
-          'm/1/1/1',
         ];
         server.scan({
           includeCopayerBranches: true
@@ -4971,8 +5045,7 @@ describe('Wallet service', function() {
     var scanConfigOld = WalletService.SCAN_CONFIG;
     beforeEach(function(done) {
       this.timeout(5000);
-      WalletService.SCAN_CONFIG.scanWindow = 2;
+      WalletService.SCAN_CONFIG.maxGap = 2;
-      WalletService.SCAN_CONFIG.derivationDelay = 0;
 
       helpers.createAndJoinWallet(1, 1, {
         supportBIP44AndP2PKH: false
@@ -4997,9 +5070,7 @@ describe('Wallet service', function() {
         'm/2147483647/0/0',
         'm/2147483647/0/1',
         'm/2147483647/0/2',
-        'm/2147483647/0/3',
         'm/2147483647/1/0',
-        'm/2147483647/1/1',
       ];
       server.messageBroker.onMessage(function(n) {
         if (n.type == 'ScanFinished') {
@@ -5014,7 +5085,7 @@ describe('Wallet service', function() {
               _.difference(paths, expectedPaths).length.should.equal(0);
               server.createAddress({}, function(err, address) {
                 should.not.exist(err);
-                address.path.should.equal('m/2147483647/0/4');
+                address.path.should.equal('m/2147483647/0/3');
                 done();
               });
             })