refactor(core-forger): memory leak, log spam, cleanup

__tests__/integration/core-forger/client.test.ts

@@ -5,6 +5,7 @@ import { httpie } from "@arkecosystem/core-utils";
 import "jest-extended";
 import nock from "nock";
 import { Client } from "../../../packages/core-forger/src/client";
+import { HostNoResponseError } from "../../../packages/core-forger/src/errors";
 import { sampleBlocks } from "./__fixtures__/blocks";
 
 jest.setTimeout(30000);
@@ -52,7 +53,7 @@ describe("Client", () => {
                             return requestBody;
                         });
 
-                    await client.__chooseHost();
+                    await client.selectHost();
 
                     const wasBroadcasted = await client.broadcast(sampleBlock.toJson());
                     expect(wasBroadcasted).toBeTruthy();
@@ -84,7 +85,7 @@ describe("Client", () => {
                     .get("/internal/transactions/forging")
                     .reply(200, { data: expectedResponse });
 
-                await client.__chooseHost();
+                await client.selectHost();
                 const response = await client.getTransactions();
 
                 expect(response).toEqual(expectedResponse);
@@ -100,7 +101,7 @@ describe("Client", () => {
                     .get("/internal/network/state")
                     .reply(200, { data: expectedResponse });
 
-                await client.__chooseHost();
+                await client.selectHost();
                 const response = await client.getNetworkState();
 
                 expect(response).toEqual(expectedResponse);
@@ -115,23 +116,22 @@ describe("Client", () => {
                 .get("/internal/blockchain/sync")
                 .reply(200);
 
+            await client.selectHost();
             await client.syncCheck();
 
             expect(httpie.get).toHaveBeenCalledWith(`${host}/internal/blockchain/sync`, expect.any(Object));
         });
     });
 
-    describe("getUsernames", () => {
-        it("should fetch usernames", async () => {
-            jest.spyOn(httpie, "get");
-            const expectedResponse = { foo: "bar" };
-            nock(host)
-                .get("/internal/utils/usernames")
-                .reply(200, { data: expectedResponse });
-
-            const response = await client.getUsernames();
+    describe("selectHost", () => {
+        it("should fallback to responsive host", async () => {
+            client = new Client(["http://127.0.0.2:4000", "http://127.0.0.3:4000", host]);
+            await expect(client.selectHost()).toResolve();
+        });
 
-            expect(response).toEqual(expectedResponse);
+        it("should throw error when no host is responsive", async () => {
+            client = new Client(["http://127.0.0.2:4000", "http://127.0.0.3:4000"]);
+            await expect(client.selectHost()).rejects.toThrowError(HostNoResponseError);
         });
     });
 
@@ -145,15 +145,15 @@ describe("Client", () => {
                     return [200];
                 });
 
-            await client.__chooseHost();
+            await client.selectHost();
             await client.emitEvent("foo", "bar");
 
             expect(httpie.post).toHaveBeenCalledWith(`${host}/internal/utils/events`, {
                 body: JSON.stringify({ event: "foo", body: "bar" }),
                 headers: {
                     "Content-Type": "application/json",
                     nethash: {},
-                    port: "4000",
+                    port: 4000,
                     version: "2.3.0",
                     "x-auth": "forger",
                 },

__tests__/unit/core-forger/manager.test.ts

@@ -35,18 +35,15 @@ describe("Forger Manager", () => {
         it("should be ok with configured delegates", async () => {
             const secret = "a secret";
             forgeManager.secrets = [secret];
-            // @ts-ignore
-            forgeManager.client.getUsernames.mockReturnValue([]);
 
             const delegates = await forgeManager.loadDelegates();
 
             expect(delegates).toBeArray();
             delegates.forEach(value => expect(value).toBeInstanceOf(Delegate));
-            expect(forgeManager.client.getUsernames).toHaveBeenCalled();
         });
     });
 
-    describe("__forgeNewBlock", () => {
+    describe("forgeNewBlock", () => {
         it("should forge a block", async () => {
             // NOTE: make sure we have valid transactions from an existing wallet
             const transactions = generateTransfer(
@@ -68,7 +65,7 @@ describe("Forger Manager", () => {
                 reward: 2 * 1e8,
             };
 
-            await forgeManager.__forgeNewBlock(del, round, {
+            await forgeManager.forgeNewBlock(del, round, {
                 lastBlockId: round.lastBlock.id,
                 nodeHeight: round.lastBlock.height,
             });
@@ -86,7 +83,7 @@ describe("Forger Manager", () => {
 
     describe("__monitor", () => {
         it("should emit failed event if error while monitoring", async () => {
-            forgeManager.client.getUsernames.mockRejectedValue(new Error("oh bollocks"));
+            forgeManager.client.getRound.mockRejectedValue(new Error("oh bollocks"));
 
             setTimeout(() => forgeManager.stop(), 1000);
             await forgeManager.__monitor();
@@ -95,12 +92,12 @@ describe("Forger Manager", () => {
         });
     });
 
-    describe("__getTransactionsForForging", () => {
+    describe("getTransactionsForForging", () => {
         it("should return zero transactions if none to forge", async () => {
             // @ts-ignore
             forgeManager.client.getTransactions.mockReturnValue({});
 
-            const transactions = await forgeManager.__getTransactionsForForging();
+            const transactions = await forgeManager.getTransactionsForForging();
 
             expect(transactions).toHaveLength(0);
             expect(forgeManager.client.getTransactions).toHaveBeenCalled();
@@ -111,7 +108,7 @@ describe("Forger Manager", () => {
                 transactions: [Transaction.fromData(sampleTransaction).serialized.toString("hex")],
             });
 
-            const transactions = await forgeManager.__getTransactionsForForging();
+            const transactions = await forgeManager.getTransactionsForForging();
 
             expect(transactions).toHaveLength(1);
             expect(forgeManager.client.getTransactions).toHaveBeenCalled();
@@ -120,31 +117,12 @@ describe("Forger Manager", () => {
         });
     });
 
-    describe("__isDelegateActivated", () => {
-        it("should be ok", async () => {
-            forgeManager.delegates = [
-                {
-                    username: "arkxdev",
-                    publicKey: "0310ad026647eed112d1a46145eed58b8c19c67c505a67f1199361a511ce7860c0",
-                },
-            ];
-
-            const forger = await forgeManager.__isDelegateActivated(
-                "0310ad026647eed112d1a46145eed58b8c19c67c505a67f1199361a511ce7860c0",
-            );
-
-            expect(forger).toBeObject();
-            expect(forger.username).toBe("arkxdev");
-            expect(forger.publicKey).toBe("0310ad026647eed112d1a46145eed58b8c19c67c505a67f1199361a511ce7860c0");
-        });
-    });
-
-    describe("__parseNetworkState", () => {
+    describe("parseNetworkState", () => {
         it("should be TRUE when quorum > 0.66", async () => {
             const networkState = new NetworkState(NetworkStateStatus.Default);
             Object.assign(networkState, { getQuorum: () => 0.9, nodeHeight: 100, lastBlockId: "1233443" });
 
-            const canForge = await forgeManager.__parseNetworkState(networkState, delegate);
+            const canForge = await forgeManager.parseNetworkState(networkState, delegate);
 
             expect(canForge).toBeTrue();
         });
@@ -153,7 +131,7 @@ describe("Forger Manager", () => {
             const networkState = new NetworkState(NetworkStateStatus.Unknown);
             Object.assign(networkState, { getQuorum: () => 1, nodeHeight: 100, lastBlockId: "1233443" });
 
-            const canForge = await forgeManager.__parseNetworkState(networkState, delegate);
+            const canForge = await forgeManager.parseNetworkState(networkState, delegate);
 
             expect(canForge).toBeFalse();
         });
@@ -162,21 +140,21 @@ describe("Forger Manager", () => {
             const networkState = new NetworkState(NetworkStateStatus.Default);
             Object.assign(networkState, { getQuorum: () => 0.65, nodeHeight: 100, lastBlockId: "1233443" });
 
-            const canForge = await forgeManager.__parseNetworkState(networkState, delegate);
+            const canForge = await forgeManager.parseNetworkState(networkState, delegate);
 
             expect(canForge).toBeFalse();
         });
 
         it("should be FALSE when coldStart is active", async () => {
             const networkState = new NetworkState(NetworkStateStatus.ColdStart);
-            const canForge = await forgeManager.__parseNetworkState(networkState, delegate);
+            const canForge = await forgeManager.parseNetworkState(networkState, delegate);
 
             expect(canForge).toBeFalse();
         });
 
         it("should be FALSE when minimumNetworkReach is not sufficient", async () => {
             const networkState = new NetworkState(NetworkStateStatus.BelowMinimumPeers);
-            const canForge = await forgeManager.__parseNetworkState(networkState, delegate);
+            const canForge = await forgeManager.parseNetworkState(networkState, delegate);
 
             expect(canForge).toBeFalse();
         });
@@ -200,7 +178,7 @@ describe("Forger Manager", () => {
                 },
             });
 
-            const canForge = await forgeManager.__parseNetworkState(networkState, delegate);
+            const canForge = await forgeManager.parseNetworkState(networkState, delegate);
             expect(canForge).toBeFalse();
         });
     });

packages/core-api/src/versions/1/delegates/controller.ts

@@ -95,15 +95,14 @@ export class DelegatesController extends Controller {
             const delegatesCount = this.config.getMilestone(lastBlock).activeDelegates;
             const currentSlot = slots.getSlotNumber(lastBlock.data.timestamp);
 
-            let activeDelegates = await this.databaseService.getActiveDelegates(lastBlock.data.height);
-            activeDelegates = activeDelegates.map(delegate => delegate.publicKey);
-
+            const activeDelegates = await this.databaseService.getActiveDelegates(lastBlock.data.height);
             const nextForgers = [];
+
             for (let i = 1; i <= delegatesCount && i <= limit; i++) {
                 const delegate = activeDelegates[(currentSlot + i) % delegatesCount];
 
                 if (delegate) {
-                    nextForgers.push(delegate);
+                    nextForgers.push(delegate.publicKey);
                 }
             }
 

packages/core-database-postgres/src/repositories/rounds.ts

@@ -11,7 +11,7 @@ export class RoundsRepository extends Repository implements Database.IRoundsRepo
      * @param  {Number} round
      * @return {Promise}
      */
-    public async findById(round) {
+    public async findById(round: number): Promise<Database.IRound[]> {
         return this.db.manyOrNone(sql.find, { round });
     }
 
@@ -20,15 +20,15 @@ export class RoundsRepository extends Repository implements Database.IRoundsRepo
      * @param  {Number} round
      * @return {Promise}
      */
-    public async delete(round) {
+    public async delete(round): Promise<void> {
         return this.db.none(sql.delete, { round });
     }
 
     /**
      * Get the model related to this repository.
      * @return {Round}
      */
-    public getModel() {
+    public getModel(): Round {
         return new Round(this.pgp);
     }
 }

packages/core-database/src/database-service.ts

@@ -24,7 +24,7 @@ export class DatabaseService implements Database.IDatabaseService {
     public blocksInCurrentRound: any[] = null;
     public stateStarted: boolean = false;
     public restoredDatabaseIntegrity: boolean = false;
-    public forgingDelegates: any[] = null;
+    public forgingDelegates: Database.IDelegateWallet[] = null;
     public cache: Map<any, any> = new Map();
 
     constructor(
@@ -141,7 +141,7 @@ export class DatabaseService implements Database.IDatabaseService {
         this.connection.enqueueDeleteRound(height);
     }
 
-    public async getActiveDelegates(height: number, delegates?: any[]) {
+    public async getActiveDelegates(height: number, delegates?: Database.IDelegateWallet[]) {
         const maxDelegates = this.config.getMilestone(height).activeDelegates;
         const round = Math.floor((height - 1) / maxDelegates) + 1;
 
@@ -151,7 +151,9 @@ export class DatabaseService implements Database.IDatabaseService {
 
         // When called during applyRound we already know the delegates, so we don't have to query the database.
         if (!delegates || delegates.length === 0) {
-            delegates = await this.connection.roundsRepository.findById(round);
+            delegates = ((await this.connection.roundsRepository.findById(
+                round,
+            )) as unknown) as Database.IDelegateWallet[];
         }
 
         const seedSource = round.toString();
@@ -169,6 +171,7 @@ export class DatabaseService implements Database.IDatabaseService {
 
         this.forgingDelegates = delegates.map(delegate => {
             delegate.round = +delegate.round;
+            delegate.username = this.walletManager.findByPublicKey(delegate.publicKey).username;
             return delegate;
         });
 

packages/core-database/src/wallet-manager.ts

@@ -187,7 +187,7 @@ export class WalletManager implements Database.IWalletManager {
      * @param height
      * @return {Array}
      */
-    public loadActiveDelegateList(maxDelegates: number, height?: number): any[] {
+    public loadActiveDelegateList(maxDelegates: number, height?: number): Database.IDelegateWallet[] {
         if (height > 1 && height % maxDelegates !== 1) {
             throw new Error("Trying to build delegates outside of round change");
         }
@@ -252,7 +252,7 @@ export class WalletManager implements Database.IWalletManager {
 
         this.logger.debug(`Loaded ${delegates.length} active ${pluralize("delegate", delegates.length)}`);
 
-        return delegates;
+        return delegates as Database.IDelegateWallet[];
     }
 
     /**

packages/core-forger/package.json

@@ -26,15 +26,12 @@
         "@arkecosystem/core-p2p": "^2.3.0-next.3",
         "@arkecosystem/core-utils": "^2.3.0-next.3",
         "@arkecosystem/crypto": "^2.3.0-next.3",
-        "delay": "^4.1.0",
         "lodash.isempty": "^4.4.0",
-        "lodash.sample": "^4.2.1",
         "lodash.uniq": "^4.5.0",
         "pluralize": "^7.0.0"
     },
     "devDependencies": {
         "@types/lodash.isempty": "^4.4.6",
-        "@types/lodash.sample": "^4.2.6",
         "@types/lodash.uniq": "^4.5.6",
         "@types/pluralize": "^0.0.29"
     },