refactor(core-p2p): Remove unnecessary ping() call

packages/core-container/src/config/index.ts

@@ -49,12 +49,6 @@ class Config {
         this.config.exceptions = crypto.get("exceptions");
         this.config.milestones = crypto.get("milestones");
         this.config.genesisBlock = crypto.get("genesisBlock");
-
-        // Calculate milestone hash
-        const milestonesBuffer = Buffer.from(JSON.stringify(this.config.milestones));
-        this.config.milestoneHash = HashAlgorithms.sha256(milestonesBuffer)
-            .slice(0, 8)
-            .toString("hex");
     }
 }
 

packages/core-interfaces/src/core-p2p/peer.ts

@@ -18,7 +18,6 @@ export interface IPeer {
         ip: any;
         port: number;
         nethash: any;
-        milestoneHash: string;
         version: any;
         os: any;
         status: any;

packages/core-p2p/__tests__/court/guard.test.ts

@@ -5,15 +5,12 @@ import { defaults } from "../../src/defaults";
 import { Peer } from "../../src/peer";
 import { setUp, tearDown } from "../__support__/setup";
 
-const CORE_ENV = process.env.CORE_ENV;
 let guard;
 let peerMock;
 
 beforeAll(async () => {
     await setUp();
 
-    app.getConfig().set("milestoneHash", "dummy-milestone");
-
     guard = require("../../src/court/guard").guard;
     guard.config.set("minimumVersions", [">=2.0.0"]);
 });
@@ -28,25 +25,20 @@ beforeEach(async () => {
 
     // this peer is here to be ready for future use in tests (not added to initial peers)
     peerMock = new Peer("1.0.0.99", 4002);
-    peerMock.milestoneHash = "dummy-milestone";
     Object.assign(peerMock, peerMock.headers);
 });
 
 describe("Guard", () => {
     describe("isSuspended", () => {
         it("should return true", async () => {
-            process.env.CORE_ENV = "false";
             await guard.monitor.acceptNewPeer(peerMock);
-            process.env.CORE_ENV = CORE_ENV;
 
             expect(guard.isSuspended(peerMock)).toBe(true);
         });
 
         it("should return false because passed", async () => {
-            process.env.CORE_ENV = "false";
             await guard.monitor.acceptNewPeer(peerMock);
             guard.suspensions[peerMock.ip].until = dayjs().subtract(1, "minute");
-            process.env.CORE_ENV = CORE_ENV;
 
             expect(guard.isSuspended(peerMock)).toBe(false);
         });
@@ -104,7 +96,6 @@ describe("Guard", () => {
 
         const dummy = {
             nethash: "d9acd04bde4234a81addb8482333b4ac906bed7be5a9970ce8ada428bd083192",
-            milestoneHash: "dummy-milestone",
             version: "2.1.1",
             status: 200,
             state: {},
@@ -115,7 +106,6 @@ describe("Guard", () => {
 
             const { until, reason } = guard.__determineOffence({
                 nethash: "d9acd04bde4234a81addb8482333b4ac906bed7be5a9970ce8ada428bd083192",
-                milestoneHash: "dummy-milestone",
                 ip: "dummy-ip-addr",
             });
 
@@ -140,7 +130,6 @@ describe("Guard", () => {
         it('should return a 5 minute suspension for "Invalid Version"', () => {
             const { until, reason } = guard.__determineOffence({
                 nethash: "d9acd04bde4234a81addb8482333b4ac906bed7be5a9970ce8ada428bd083192",
-                milestoneHash: "dummy-milestone",
                 version: "1.0.0",
                 status: 200,
                 delay: 1000,
@@ -150,16 +139,6 @@ describe("Guard", () => {
             expect(convertToMinutes(until)).toBe(5);
         });
 
-        it('should return a 5 minute suspension for "Invalid Milestones"', () => {
-            const { until, reason } = guard.__determineOffence({
-                ...dummy,
-                milestoneHash: "wrong-milestone",
-            });
-
-            expect(reason).toBe("Invalid Milestones");
-            expect(convertToMinutes(until)).toBe(5);
-        });
-
         it('should return a 10 minutes suspension for "Node is not at height"', () => {
             guard.monitor.getNetworkHeight = jest.fn(() => 154);
 

packages/core-p2p/__tests__/monitor.test.ts

@@ -71,13 +71,9 @@ describe("Monitor", () => {
                 peerMock.headers,
             ]);
 
-            process.env.CORE_ENV = "false";
-
             await monitor.acceptNewPeer(peerMock);
 
             expect(monitor.peers[peerMock.ip]).toBeObject();
-
-            process.env.CORE_ENV = "test";
         });
     });
 
@@ -155,6 +151,7 @@ describe("Monitor", () => {
             ]);
             axiosMock.onGet(/.*\/peer\/list/).reply(() => [200, { peers: [] }, peerMock.headers]);
             await monitor.discoverPeers();
+            await monitor.cleanPeers();
 
             const height = await monitor.getNetworkHeight();
             expect(height).toBe(2);

packages/core-p2p/src/court/guard.ts

@@ -198,16 +198,6 @@ export class Guard {
         return nethash === config.get("network.nethash");
     }
 
-    /**
-     * Determine if the peer is has the same milestones.
-     * @param  {Peer}  peer
-     * @return {Boolean}
-     */
-    public isValidMilestoneHash(peer) {
-        const milestoneHash = peer.milestoneHash || (peer.headers && peer.headers.milestoneHash);
-        return milestoneHash === config.get("milestoneHash");
-    }
-
     /**
      * Determine if the peer has a valid port.
      * @param  {Peer}  peer
@@ -285,10 +275,6 @@ export class Guard {
             return this.__determinePunishment(peer, offences.INVALID_VERSION);
         }
 
-        if (!this.isValidMilestoneHash(peer)) {
-            return this.__determinePunishment(peer, offences.INVALID_MILESTONE_HASH);
-        }
-
         // NOTE: Suspending this peer only means that we no longer
         // will download blocks from him but he can still download blocks from us.
         const heightDifference = Math.abs(this.monitor.getNetworkHeight() - peer.state.height);

packages/core-p2p/src/monitor.ts

@@ -69,7 +69,7 @@ export class Monitor implements P2P.IMonitor {
         const cachedPeers = restorePeers();
         localConfig.set("peers", cachedPeers);
 
-        this.populateSeedPeers();
+        await this.populateSeedPeers();
 
         if (this.config.skipDiscovery) {
             logger.warn("Skipped peer discovery because the relay is in skip-discovery mode.");
@@ -121,7 +121,7 @@ export class Monitor implements P2P.IMonitor {
         let nextRunDelaySeconds = 600;
 
         if (!this.hasMinimumPeers()) {
-            this.populateSeedPeers();
+            await this.populateSeedPeers();
             nextRunDelaySeconds = 5;
             logger.info(`Couldn't find enough peers. Falling back to seed peers.`);
         }
@@ -134,18 +134,13 @@ export class Monitor implements P2P.IMonitor {
      * @param  {Peer} peer
      * @throws {Error} If invalid peer
      */
-    public async acceptNewPeer(peer) {
+    public async acceptNewPeer(peer, seed?: boolean) {
         if (this.config.disableDiscovery && !this.pendingPeers[peer.ip]) {
             logger.warn(`Rejected ${peer.ip} because the relay is in non-discovery mode.`);
             return;
         }
 
-        if (
-            !isValidPeer(peer) ||
-            this.guard.isSuspended(peer) ||
-            this.pendingPeers[peer.ip] ||
-            process.env.CORE_ENV === "test"
-        ) {
+        if (!isValidPeer(peer) || this.guard.isSuspended(peer) || this.pendingPeers[peer.ip]) {
             return;
         }
 
@@ -172,7 +167,7 @@ export class Monitor implements P2P.IMonitor {
             return this.guard.suspend(newPeer);
         }
 
-        if (!this.guard.isValidNetwork(peer)) {
+        if (!this.guard.isValidNetwork(peer) && !seed) {
             logger.debug(
                 `Rejected peer ${peer.ip} as it isn't on the same network. Expected: ${config.get(
                     "network.nethash",
@@ -182,24 +177,14 @@ export class Monitor implements P2P.IMonitor {
             return this.guard.suspend(newPeer);
         }
 
-        if (!this.guard.isValidMilestoneHash(newPeer)) {
-            logger.debug(
-                `Rejected peer ${peer.ip} as it has a different milestone hash. Expected: ${config.get(
-                    "milestoneHash",
-                )} - Received: ${peer.milestoneHash}`,
-            );
-
-            return this.guard.suspend(newPeer);
-        }
-
         if (this.getPeer(peer.ip)) {
             return;
         }
 
         try {
             this.pendingPeers[peer.ip] = true;
 
-            await newPeer.ping(1500);
+            await newPeer.ping(3000);
 
             this.peers[peer.ip] = newPeer;
 
@@ -388,12 +373,7 @@ export class Monitor implements P2P.IMonitor {
         for (const peer of shuffledPeers) {
             try {
                 const hisPeers = await peer.getPeers();
-
-                for (const p of hisPeers) {
-                    if (isValidPeer(p) && !this.getPeer(p.ip)) {
-                        this.addPeer(p);
-                    }
-                }
+                await Promise.all(hisPeers.map(p => this.acceptNewPeer(p)));
             } catch (error) {
                 // Just try with the next peer from shuffledPeers.
             }
@@ -769,35 +749,6 @@ export class Monitor implements P2P.IMonitor {
         }
     }
 
-    /**
-     * Add a new peer after it passes a few checks.
-     * @param  {Peer} peer
-     * @return {void}
-     */
-    private addPeer(peer) {
-        if (this.guard.isBlacklisted(peer)) {
-            return;
-        }
-
-        if (!this.guard.isValidVersion(peer)) {
-            return;
-        }
-
-        if (!this.guard.isValidNetwork(peer)) {
-            return;
-        }
-
-        if (!this.guard.isValidMilestoneHash(peer)) {
-            return;
-        }
-
-        if (!this.guard.isValidPort(peer)) {
-            return;
-        }
-
-        this.peers[peer.ip] = new Peer(peer.ip, peer.port);
-    }
-
     /**
      * Schedule the next update network status.
      * @param {Number} nextUpdateInSeconds
@@ -835,7 +786,7 @@ export class Monitor implements P2P.IMonitor {
      * Populate the initial seed list.
      * @return {void}
      */
-    private populateSeedPeers() {
+    private async populateSeedPeers() {
         const peerList = config.get("peers.list");
 
         if (!peerList) {
@@ -851,26 +802,12 @@ export class Monitor implements P2P.IMonitor {
             peers = { ...peers, ...localConfig.get("peers") };
         }
 
-        const filteredPeers: any[] = Object.values(peers).filter((peer: any) => {
-            if (!isValidPeer(peer)) {
-                return false;
-            }
-
-            if (!this.guard.isValidPort(peer)) {
-                return false;
-            }
-
-            if (!this.guard.isValidVersion(peer)) {
-                return false;
-            }
-
-            return true;
-        });
-
-        for (const peer of filteredPeers) {
-            delete this.guard.suspensions[peer.ip];
-            this.peers[peer.ip] = new Peer(peer.ip, peer.port);
-        }
+        return Promise.all(
+            Object.values(peers).map((peer: any) => {
+                delete this.guard.suspensions[peer.ip];
+                return this.acceptNewPeer(peer, true);
+            }),
+        );
     }
 }