simplify the requestable condition (#718)

* simplify the requestable condition

* Keep ASSERTION on non-positive index

* Check we're on the same fork before applying optimisation

* Fix assert msg

---------

Co-authored-by: HDegroote <[email protected]>

Author: mafintosh

lib/replicator.js

@@ -429,6 +429,8 @@ class Peer {
 
     this.needsSync = false
     this.syncsProcessing = 0
+    this.lastUpgradableLength = 0
+    this.lastUpgradableFork = 0
 
     this._remoteContiguousLength = 0
 
@@ -662,6 +664,10 @@ class Peer {
 
     if (length === this.remoteLength && fork === this.core.state.fork) {
       this.canUpgrade = upgrade
+      if (upgrade) {
+        this.lastUpgradableFork = fork
+        this.lastUpgradableLength = length
+      }
     }
 
     if (--this.syncsProcessing !== 0) return // ie not latest
@@ -1204,7 +1210,7 @@ class Peer {
 
       if (this._remoteHasBlock(index) === false) continue
       if (this.core.bitfield.get(index) === true) continue
-      if (!this._hasTreeParent(index)) continue
+      if (!this._canRequest(index)) continue
 
       // Check if this block is currently inflight - if so pick another
       const b = this.replicator._blocks.get(index)
@@ -1233,42 +1239,11 @@ class Peer {
     return false
   }
 
-  _hasTreeParent (index) {
+  _canRequest (index) {
+    if (!(index >= 0)) throw ASSERTION('bad index to _canRequest: ' + index)
     if (this.remoteLength >= this.core.state.length) return true
-    if (!(index >= 0)) throw ASSERTION('bad index to _hasTreeParent: ' + index)
-
-    const ite = flatTree.iterator(index * 2)
-
-    let span = 2
-    let length = 0
-    let cnt = 0
-
-    while (true) {
-      if (++cnt >= 64) {
-        throw ASSERTION('_hasTreeParent is stuck, index=' + index + ', length=' + length +
-          ', span=' + span + ', remoteLength=' + this.remoteLength + ', length=' + this.core.state.length)
-      }
-
-      ite.parent()
-
-      const left = (ite.index - ite.factor / 2 + 1) / 2
-      length = left + span
-
-      // if larger than local AND larger than remote - they share the root so its ok
-      if (length > this.core.state.length) {
-        if (length > this.remoteLength) return true
-        break
-      }
-
-      // its less than local but larger than remote so skip it
-      if (length > this.remoteLength) break
-
-      span *= 2
-      const first = this.core.bitfield.findFirst(true, left)
-      if (first > -1 && first < length) return true
-    }
-
-    // TODO: push to async queue and check against our local merkle tree if we actually can request this block
+    if (index < this.lastUpgradableLength && this.lastUpgradableFork === this.core.state.fork) return true
+    if (this.canUpgrade && this.syncsProcessing === 0) return true
     return false
   }
 
@@ -1293,9 +1268,7 @@ class Peer {
       return false
     }
 
-    if (!this._hasTreeParent(b.index)) {
-      return false
-    }
+    if (!this._canRequest(b.index)) return false
 
     const req = this._makeRequest(b.index >= length, b.priority, b.index + 1)
     if (req === null) return false
@@ -1306,7 +1279,7 @@ class Peer {
   }
 
   _requestRangeBlock (index, length) {
-    if (this.core.bitfield.get(index) === true || !this._hasTreeParent(index)) return false
+    if (this.core.bitfield.get(index) === true || !this._canRequest(index)) return false
 
     const b = this.replicator._blocks.add(index, PRIORITY.NORMAL)
     if (b.inflight.length > 0) {