fix(core-p2p): prevent the list of peers to become too short (#1829)

* fix(core-p2p): prevent the list of peers to become too short This ports 9502585, 9c545bc and e42f4c7 from branch 'develop' to branch 'master'. * fix: config retrieval * chore: bump versions and update changelogs
ArkEcosystem · Dec 17, 2018 · e652300 · e652300
1 parent c6ef0d7
commit e652300
Show file tree

Hide file tree

Showing 8 changed files with 106 additions and 100 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -72,11 +72,10 @@ jobs:
             ./node_modules/.bin/cross-env ARK_ENV=test ./node_modules/.bin/jest
             ./packages/core-vote-report/ ./packages/core-transaction-pool/
             ./packages/core-snapshots-cli/ ./packages/core-logger-winston/
-            ./packages/core-api/ ./packages/core-event-emitter/
+            ./packages/core-http-utils/ ./packages/core-event-emitter/
             ./packages/core-elasticsearch/ ./packages/core-database-postgres/
-            ./packages/core-config/ ./packages/core-http-utils/
-            --detectOpenHandles --runInBand --forceExit --ci --coverage | tee
-            test_output.txt
+            ./packages/core-config/ ./packages/core/ --detectOpenHandles
+            --runInBand --forceExit --ci --coverage | tee test_output.txt
       - run:
           name: Last 1000 lines of test output
           when: on_fail
@@ -158,7 +157,7 @@ jobs:
             ./packages/core-test-utils/ ./packages/core-p2p/
             ./packages/core-json-rpc/ ./packages/core-forger/
             ./packages/core-error-tracker-bugsnag/ ./packages/core-debugger-cli/
-            ./packages/core-container/ ./packages/core/ --detectOpenHandles
+            ./packages/core-container/ ./packages/core-api/ --detectOpenHandles
             --runInBand --forceExit --ci --coverage | tee test_output.txt
       - run:
           name: Last 1000 lines of test output

diff --git a/packages/core-blockchain/lib/state-machine.js b/packages/core-blockchain/lib/state-machine.js
@@ -58,8 +58,6 @@ blockchainMachine.actionMap = blockchain => ({
       `Queued blocks (rebuild: ${blockchain.rebuildQueue.length()} process: ${blockchain.processQueue.length()})`,
     )
 
-    await blockchain.p2p.updateNetworkStatusIfNotEnoughPeers()
-
     if (
       blockchain.rebuildQueue.length() > 10000 ||
       blockchain.processQueue.length() > 10000

diff --git a/packages/core-p2p/CHANGELOG.md b/packages/core-p2p/CHANGELOG.md
@@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 ## Unreleased
 
+## 0.2.11 - 2018-12-17
+
+### Fixed
+
+- Prevent the list of peers to become too short
+
 ## 0.2.1 - 2018-12-11
 
 ### Fixed

diff --git a/packages/core-p2p/__tests__/monitor.test.js b/packages/core-p2p/__tests__/monitor.test.js
@@ -153,11 +153,12 @@ describe('Monitor', () => {
           peerMock.headers,
         ])
 
-      const peers = await monitor.discoverPeers()
+      await monitor.discoverPeers()
+      const peers = monitor.getPeers()
 
-      expect(peers).toBeObject()
+      expect(peers).toBeArray()
       expect(Object.keys(peers).length).toBe(6) // 5 from initial peers + 1 from peerMock
-      expect(peers[peerMock.ip]).toBeObject()
+      expect(peers.find(e => e.ip === peerMock.ip)).toBeDefined()
     })
   })
 

diff --git a/packages/core-p2p/lib/monitor.js b/packages/core-p2p/lib/monitor.js
@@ -35,6 +35,8 @@ class Monitor {
     this.peers = {}
     this.coldStartPeriod = dayjs().add(config.peers.coldStart || 30, 'seconds')
 
+    this.nextUpdateNetworkStatusScheduled = false
+
     // Holds temporary peers which are in the process of being accepted. Prevents that
     // peers who are not accepted yet, but send multiple requests in a short timeframe will
     // get processed multiple times in `acceptNewPeer`.
@@ -53,7 +55,7 @@ class Monitor {
 
     this.guard = guard.init(this)
 
-    this.__filterPeers()
+    this.__populateSeedPeers()
 
     if (this.config.skipDiscovery) {
       logger.warn(
@@ -98,7 +100,7 @@ class Monitor {
    * @return {Promise}
    */
   async updateNetworkStatus(networkStart) {
-    if (process.env.NODE_ENV === 'test') {
+    if (process.env.ARK_ENV === 'test' || process.env.NODE_ENV === 'test') {
       return
     }
 
@@ -117,53 +119,21 @@ class Monitor {
     }
 
     try {
-      if (process.env.ARK_ENV !== 'test') {
-        await this.discoverPeers()
-        await this.cleanPeers()
-
-        if (!this.hasMinimumPeers()) {
-          this.__addPeers(config.peers.list)
-
-          logger.info("Couldn't find enough peers, trying again in 5 seconds.")
-
-          await delay(5000)
-
-          return this.updateNetworkStatus()
-        }
-      }
+      await this.discoverPeers()
+      await this.cleanPeers()
     } catch (error) {
       logger.error(`Network Status: ${error.message}`)
-
-      this.__addPeers(config.peers.list)
-
-      logger.info('Failed to discover peers, trying again in 5 seconds.')
-
-      if (process.env.NODE_ENV !== 'test') {
-        await delay(5000)
-      }
-
-      return this.updateNetworkStatus()
     }
-  }
 
-  /**
-   * Updates the network status if not enough peers are available.
-   * NOTE: This is usually only necessary for nodes without incoming requests,
-   * since the available peers are depleting over time due to suspensions.
-   * @return {void}
-   */
-  async updateNetworkStatusIfNotEnoughPeers() {
-    if (!this.hasMinimumPeers() && process.env.ARK_ENV !== 'test') {
-      await this.updateNetworkStatus(this.config.networkStart)
+    let nextRunDelaySeconds = 600
+
+    if (!this.__hasMinimumPeers()) {
+      this.__populateSeedPeers();
+      nextRunDelaySeconds = 5;
+      logger.info(`Couldn't find enough peers. Falling back to seed peers.`);
     }
-  }
 
-  /**
-   * Returns if the minimum amount of peers are available.
-   * @return {Boolean}
-   */
-  hasMinimumPeers() {
-    return Object.keys(this.peers).length >= config.peers.minimumNetworkReach
+    this.__scheduleUpdateNetworkStatus(nextRunDelaySeconds)
   }
 
   /**
@@ -413,25 +383,26 @@ class Monitor {
 
   /**
    * Populate list of available peers from random peers.
-   * @return {Peer[]}
    */
   async discoverPeers() {
-    try {
-      const peers = await this.getRandomPeer().getPeers()
-
-      peers.forEach(peer => {
-        if (
-          Peer.isOk(peer) &&
-          !this.getPeer(peer.ip) &&
-          !this.guard.isMyself(peer)
-        ) {
-          this.__addPeer(peer)
+    const shuffledPeers = shuffle(this.getPeers())
+
+    for (const peer of shuffledPeers) {
+      try {
+        const hisPeers = await peer.getPeers()
+
+        for (const p of hisPeers) {
+          if (Peer.isOk(p) && !this.getPeer(p.ip) && !this.guard.isMyself(p)) {
+            this.__addPeer(p)
+          }
         }
-      })
+      } catch (error) {
+        // Just try with the next peer from shuffledPeers.
+      }
 
-      return this.peers
-    } catch (error) {
-      return this.discoverPeers()
+      if (this.__hasMinimumPeers()) {
+        return;
+      }
     }
   }
 
@@ -782,36 +753,6 @@ class Monitor {
     }
   }
 
-  /**
-   * Filter the initial seed list.
-   * @return {void}
-   */
-  __filterPeers() {
-    if (!config.peers.list) {
-      app.forceExit('No seed peers defined in peers.json :interrobang:')
-    }
-
-    let peers = config.peers.list.map(peer => {
-      peer.version = app.getVersion()
-      return peer
-    })
-
-    if (config.peers_backup) {
-      peers = { ...peers, ...config.peers_backup }
-    }
-
-    const filteredPeers = Object.values(peers).filter(
-      peer =>
-        !this.guard.isMyself(peer) ||
-        !this.guard.isValidPort(peer) ||
-        !this.guard.isValidVersion(peer),
-    )
-
-    for (const peer of filteredPeers) {
-      this.peers[peer.ip] = new Peer(peer.ip, peer.port)
-    }
-  }
-
   /**
    * Get last 10 block IDs from database.
    * @return {[]String}
@@ -896,6 +837,61 @@ class Monitor {
       this.__addPeer(peer)
     }
   }
+
+  /**
+   * Schedule the next update network status.
+   * @param {Number} nextUpdateInSeconds
+   * @returns {void}
+   */
+  async __scheduleUpdateNetworkStatus(nextUpdateInSeconds) {
+    if (this.nextUpdateNetworkStatusScheduled) {
+      return
+    }
+
+    this.nextUpdateNetworkStatusScheduled = true
+
+    await delay(nextUpdateInSeconds * 1000)
+
+    this.nextUpdateNetworkStatusScheduled = false
+
+    this.updateNetworkStatus(this.config.networkStart)
+  }
+
+  /**
+   * Returns if the minimum amount of peers are available.
+   * @return {Boolean}
+   */
+  __hasMinimumPeers() {
+    return Object.keys(this.peers).length >= config.peers.minimumNetworkReach
+  }
+
+  /**
+   * Populate the initial seed list.
+   * @return {void}
+   */
+  __populateSeedPeers() {
+    if (!config.peers.list) {
+      app.forceExit("No seed peers defined in peers.json :interrobang:")
+    }
+
+    let peers = config.peers.list.map(peer => {
+      peer.version = app.getVersion()
+      return peer
+    })
+
+    if (config.peers_backup) {
+      peers = { ...peers, ...config.peers_backup }
+    }
+
+    const filteredPeers = Object.values(peers).filter(
+      peer => !this.guard.isMyself(peer) || !this.guard.isValidPort(peer) || !this.guard.isValid
+    )
+
+    for (const peer of filteredPeers) {
+      delete this.guard.suspensions[peer.ip]
+      this.peers[peer.ip] = new Peer(peer.ip, peer.port)
+    }
+  }
 }
 
 module.exports = new Monitor()
diff --git a/packages/core-p2p/package.json b/packages/core-p2p/package.json
@@ -1,7 +1,7 @@
 {
   "name": "@arkecosystem/core-p2p",
   "description": "P2P API for Ark Core",
-  "version": "0.2.1",
+  "version": "0.2.11",
   "contributors": [
     "François-Xavier Thoorens <[email protected]>",
     "Kristjan Košič <[email protected]>",

diff --git a/packages/core/CHANGELOG.md b/packages/core/CHANGELOG.md
@@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 ## Unreleased
 
+## 2.0.16 - 2018-12-17
+
+### Fixed
+
+- Prevent the list of peers to become too short
+
 ## 2.0.15 - 2018-12-11
 
 ### Fixed

diff --git a/packages/core/package.json b/packages/core/package.json
@@ -1,7 +1,7 @@
 {
   "name": "@arkecosystem/core",
   "description": "Core of the Ark Blockchain",
-  "version": "2.0.15",
+  "version": "2.0.16",
   "contributors": [
     "François-Xavier Thoorens <[email protected]>",
     "Kristjan Košič <[email protected]>",