From f704f838a9004283a5690921d96416cb6a6b7fe0 Mon Sep 17 00:00:00 2001 From: supaiku Date: Mon, 20 May 2019 16:41:53 +0200 Subject: [PATCH 1/8] fix: do not suspend peer when not ready --- packages/core-blockchain/src/state-machine.ts | 4 ++-- packages/core-p2p/src/peer-communicator.ts | 2 +- packages/core-p2p/src/peer-guard.ts | 4 ---- 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/packages/core-blockchain/src/state-machine.ts b/packages/core-blockchain/src/state-machine.ts index d5e3e30e24..ac3b0d7cf4 100644 --- a/packages/core-blockchain/src/state-machine.ts +++ b/packages/core-blockchain/src/state-machine.ts @@ -54,7 +54,7 @@ blockchainMachine.actionMap = (blockchain: Blockchain) => ({ } // tried to download but no luck after 5 tries (looks like network missing blocks) - if (stateStorage.noBlockCounter > 5 && blockchain.queue.length() === 0) { + if (stateStorage.noBlockCounter > 5 && blockchain.queue.idle()) { logger.info("Tried to sync 5 times to different nodes, looks like the network is missing blocks"); stateStorage.noBlockCounter = 0; @@ -101,7 +101,7 @@ blockchainMachine.actionMap = (blockchain: Blockchain) => ({ stateStorage.networkStart = false; blockchain.dispatch("SYNCFINISHED"); - } else if (blockchain.queue.length() === 0) { + } else if (blockchain.queue.idle()) { blockchain.dispatch("PROCESSFINISHED"); } }, diff --git a/packages/core-p2p/src/peer-communicator.ts b/packages/core-p2p/src/peer-communicator.ts index 94abac99e5..cb28e55ecf 100644 --- a/packages/core-p2p/src/peer-communicator.ts +++ b/packages/core-p2p/src/peer-communicator.ts @@ -20,7 +20,7 @@ export class PeerCommunicator implements P2P.IPeerCommunicator { public async downloadBlocks(peer: P2P.IPeer, fromBlockHeight: number): Promise { try { - this.logger.info(`Downloading blocks from height ${fromBlockHeight.toLocaleString()} via ${peer.ip}`); + this.logger.debug(`Downloading blocks from height ${fromBlockHeight.toLocaleString()} via ${peer.ip}`); return await this.getPeerBlocks(peer, fromBlockHeight); } catch (error) { diff --git a/packages/core-p2p/src/peer-guard.ts b/packages/core-p2p/src/peer-guard.ts index 309fab6d80..62856a1566 100644 --- a/packages/core-p2p/src/peer-guard.ts +++ b/packages/core-p2p/src/peer-guard.ts @@ -74,10 +74,6 @@ export class PeerGuard implements P2P.IPeerGuard { return this.createPunishment(this.offences.socketGotClosed); } - if (this.connector.hasError(peer, SocketErrors.AppNotReady)) { - return this.createPunishment(this.offences.applicationNotReady); - } - if (peer.latency === -1) { return this.createPunishment(this.offences.timeout); } From 66f311cddf222e4e4238c83ab5d8e3a9eaa6174e Mon Sep 17 00:00:00 2001 From: supaiku Date: Mon, 20 May 2019 19:37:24 +0200 Subject: [PATCH 2/8] fix: call saveBlocks before broadcast --- packages/core-blockchain/src/blockchain.ts | 47 +++++++++++----------- 1 file changed, 23 insertions(+), 24 deletions(-) diff --git a/packages/core-blockchain/src/blockchain.ts b/packages/core-blockchain/src/blockchain.ts index 6b7971a7d6..bcb2286225 100644 --- a/packages/core-blockchain/src/blockchain.ts +++ b/packages/core-blockchain/src/blockchain.ts @@ -375,11 +375,33 @@ export class Blockchain implements blockchain.IBlockchain { } } + if (acceptedBlocks.length > 0) { + try { + await this.database.saveBlocks(acceptedBlocks); + } catch (exceptionSaveBlocks) { + logger.error( + `Could not save ${acceptedBlocks.length} blocks to database : ${exceptionSaveBlocks.stack}`, + ); + + const resetToHeight = async height => { + try { + return await this.removeTopBlocks((await this.database.getLastBlock()).data.height - height); + } catch (e) { + logger.error(`Could not remove top blocks from database : ${e.stack}`); + + return resetToHeight(height); // keep trying, we can't do anything while this fails + } + }; + await resetToHeight(acceptedBlocks[0].data.height - 1); + + return this.processBlocks(blocks, callback); // keep trying, we can't do anything while this fails + } + } + if ( lastProcessResult === BlockProcessorResult.Accepted || lastProcessResult === BlockProcessorResult.DiscardedButCanBeBroadcasted ) { - // broadcast only current block const currentBlock: Interfaces.IBlock = blocks[blocks.length - 1]; const blocktime: number = config.getMilestone(currentBlock.data.height).blocktime; @@ -388,29 +410,6 @@ export class Blockchain implements blockchain.IBlockchain { } } - if (acceptedBlocks.length === 0) { - return callback([]); - } - - try { - await this.database.saveBlocks(acceptedBlocks); - } catch (exceptionSaveBlocks) { - logger.error(`Could not save ${acceptedBlocks.length} blocks to database : ${exceptionSaveBlocks.stack}`); - - const resetToHeight = async height => { - try { - return await this.removeTopBlocks((await this.database.getLastBlock()).data.height - height); - } catch (e) { - logger.error(`Could not remove top blocks from database : ${e.stack}`); - - return resetToHeight(height); // keep trying, we can't do anything while this fails - } - }; - await resetToHeight(acceptedBlocks[0].data.height - 1); - - return this.processBlocks(blocks, callback); // keep trying, we can't do anything while this fails - } - return callback(acceptedBlocks); } From 1a91aa0e79d462cf73b69c9d578976faed85cf0f Mon Sep 17 00:00:00 2001 From: supaiku Date: Mon, 20 May 2019 19:54:21 +0200 Subject: [PATCH 3/8] fix: unused import --- packages/core-p2p/src/peer-guard.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/core-p2p/src/peer-guard.ts b/packages/core-p2p/src/peer-guard.ts index 62856a1566..262693b2c7 100644 --- a/packages/core-p2p/src/peer-guard.ts +++ b/packages/core-p2p/src/peer-guard.ts @@ -2,7 +2,6 @@ import { app } from "@arkecosystem/core-container"; import { P2P } from "@arkecosystem/core-interfaces"; import { dato } from "@faustbrian/dato"; import { SCClientSocket } from "socketcluster-client"; -import { SocketErrors } from "./enums"; import { isValidVersion, isWhitelisted } from "./utils"; export class PeerGuard implements P2P.IPeerGuard { From 3479dd2a9d9c7ece954e3c840dcdb2b8f84b6536 Mon Sep 17 00:00:00 2001 From: supaiku Date: Mon, 20 May 2019 20:04:22 +0200 Subject: [PATCH 4/8] test: update mocks --- __tests__/unit/core-blockchain/mocks/blockchain.ts | 2 +- __tests__/unit/core-state/mocks/blockchain.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/__tests__/unit/core-blockchain/mocks/blockchain.ts b/__tests__/unit/core-blockchain/mocks/blockchain.ts index dfbada5eda..a4270914ce 100644 --- a/__tests__/unit/core-blockchain/mocks/blockchain.ts +++ b/__tests__/unit/core-blockchain/mocks/blockchain.ts @@ -3,7 +3,7 @@ import { p2p } from "./p2p"; import { transactionPool } from "./transactionPool"; export const blockchain = { - queue: { length: () => 0, push: () => undefined, clear: () => undefined }, + queue: { length: () => 0, push: () => undefined, clear: () => undefined, idle: () => undefined }, isStopped: false, setWakeUp: () => undefined, diff --git a/__tests__/unit/core-state/mocks/blockchain.ts b/__tests__/unit/core-state/mocks/blockchain.ts index dfbada5eda..a4270914ce 100644 --- a/__tests__/unit/core-state/mocks/blockchain.ts +++ b/__tests__/unit/core-state/mocks/blockchain.ts @@ -3,7 +3,7 @@ import { p2p } from "./p2p"; import { transactionPool } from "./transactionPool"; export const blockchain = { - queue: { length: () => 0, push: () => undefined, clear: () => undefined }, + queue: { length: () => 0, push: () => undefined, clear: () => undefined, idle: () => undefined }, isStopped: false, setWakeUp: () => undefined, From 32a7378e62272c81dce4a6a379eb82dc49f6abcf Mon Sep 17 00:00:00 2001 From: supaiku Date: Mon, 20 May 2019 20:11:32 +0200 Subject: [PATCH 5/8] test: more mocks --- __tests__/unit/core-blockchain/state-machine.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/__tests__/unit/core-blockchain/state-machine.test.ts b/__tests__/unit/core-blockchain/state-machine.test.ts index 88679caf04..4de46df90d 100644 --- a/__tests__/unit/core-blockchain/state-machine.test.ts +++ b/__tests__/unit/core-blockchain/state-machine.test.ts @@ -56,7 +56,7 @@ describe("State Machine", () => { describe("checkLastDownloadedBlockSynced", () => { it('should dispatch the event "NOTSYNCED" by default', async () => { blockchain.isSynced = jest.fn(() => false); - blockchain.queue.length = jest.fn(() => 1); + blockchain.queue.idle = jest.fn(() => false); await expect(actionMap.checkLastDownloadedBlockSynced).toDispatch(blockchain, "NOTSYNCED"); }); @@ -68,7 +68,7 @@ describe("State Machine", () => { it('should dispatch the event "NETWORKHALTED" if stateStorage.noBlockCounter > 5 and process queue is empty', async () => { blockchain.isSynced = jest.fn(() => false); - blockchain.queue.length = jest.fn(() => 0); + blockchain.queue.idle = jest.fn(() => true); stateStorage.noBlockCounter = 6; await expect(actionMap.checkLastDownloadedBlockSynced).toDispatch(blockchain, "NETWORKHALTED"); }); @@ -78,7 +78,7 @@ describe("State Machine", () => { - stateStorage.p2pUpdateCounter + 1 > 3 (network keeps missing blocks) - blockchain.p2p.checkNetworkHealth() returns a forked network status`, async () => { blockchain.isSynced = jest.fn(() => false); - blockchain.queue.length = jest.fn(() => 0); + blockchain.queue.idle = jest.fn(() => true); stateStorage.noBlockCounter = 6; stateStorage.p2pUpdateCounter = 3; From b43e481eea577c64b421bf4a5bfcacec910ca14d Mon Sep 17 00:00:00 2001 From: supaiku Date: Mon, 20 May 2019 20:51:13 +0200 Subject: [PATCH 6/8] test: obsolete --- __tests__/unit/core-p2p/peer-guard.test.ts | 9 --------- 1 file changed, 9 deletions(-) diff --git a/__tests__/unit/core-p2p/peer-guard.test.ts b/__tests__/unit/core-p2p/peer-guard.test.ts index 135677bf33..39dfe72966 100644 --- a/__tests__/unit/core-p2p/peer-guard.test.ts +++ b/__tests__/unit/core-p2p/peer-guard.test.ts @@ -68,14 +68,5 @@ describe("PeerGuard", () => { expect(reason).toBe("High Latency"); expect(convertToMinutes(until)).toBe(1); }); - - it('should return a 30 seconds suspension for "Application not ready"', () => { - connector.getError = jest.fn(() => SocketErrors.AppNotReady); - - const { until, reason } = guard.analyze(dummy); - - expect(reason).toBe("Application is not ready"); - expect(convertToMinutes(until)).toBe(0.5); - }); }); }); From 1c7042e8ad9f228c848bde4f9b3e133d714c66b4 Mon Sep 17 00:00:00 2001 From: supaiku Date: Mon, 20 May 2019 21:07:54 +0200 Subject: [PATCH 7/8] test: last one --- __tests__/unit/core-p2p/peer-guard.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/__tests__/unit/core-p2p/peer-guard.test.ts b/__tests__/unit/core-p2p/peer-guard.test.ts index 39dfe72966..6fa88a1b43 100644 --- a/__tests__/unit/core-p2p/peer-guard.test.ts +++ b/__tests__/unit/core-p2p/peer-guard.test.ts @@ -4,7 +4,6 @@ import "./mocks/core-container"; import { P2P } from "@arkecosystem/core-interfaces"; import { dato } from "@faustbrian/dato"; -import { SocketErrors } from "../../../packages/core-p2p/src/enums"; import { PeerConnector } from "../../../packages/core-p2p/src/peer-connector"; import { PeerGuard } from "../../../packages/core-p2p/src/peer-guard"; import { createStubPeer } from "../../helpers/peers"; From ccb9df93bdbb6818ab052f040ffc6bc34bf64ba4 Mon Sep 17 00:00:00 2001 From: Air1 Date: Tue, 21 May 2019 12:30:26 +0400 Subject: [PATCH 8/8] fix: explicitely dont punish when app not ready --- packages/core-p2p/src/peer-guard.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/core-p2p/src/peer-guard.ts b/packages/core-p2p/src/peer-guard.ts index 262693b2c7..f720a01b0d 100644 --- a/packages/core-p2p/src/peer-guard.ts +++ b/packages/core-p2p/src/peer-guard.ts @@ -2,6 +2,7 @@ import { app } from "@arkecosystem/core-container"; import { P2P } from "@arkecosystem/core-interfaces"; import { dato } from "@faustbrian/dato"; import { SCClientSocket } from "socketcluster-client"; +import { SocketErrors } from "./enums"; import { isValidVersion, isWhitelisted } from "./utils"; export class PeerGuard implements P2P.IPeerGuard { @@ -73,6 +74,10 @@ export class PeerGuard implements P2P.IPeerGuard { return this.createPunishment(this.offences.socketGotClosed); } + if (this.connector.hasError(peer, SocketErrors.AppNotReady)) { + return undefined; // no punishment when app is not ready + } + if (peer.latency === -1) { return this.createPunishment(this.offences.timeout); }