From fb3df67deeba60e4b1b17bc6fb1fa03624630a11 Mon Sep 17 00:00:00 2001
From: cgeek <cem.moreau@gmail.com>
Date: Mon, 28 Aug 2017 14:25:46 +0200
Subject: [PATCH] [fix] #1093 Do not try to switch again on a same fork while
 HEAD remains unchanged

---
 app/lib/blockchain/Switcher.ts   | 18 ++++++++++++++----
 app/service/BlockchainService.ts |  7 +++++--
 test/fast/fork-resolution-3-3.ts | 16 ++++++++--------
 3 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/app/lib/blockchain/Switcher.ts b/app/lib/blockchain/Switcher.ts
index 1bf69f008..e6ae98923 100644
--- a/app/lib/blockchain/Switcher.ts
+++ b/app/lib/blockchain/Switcher.ts
@@ -21,6 +21,7 @@ export class Switcher<T extends SwitchBlock> {
 
   constructor(
     private dao:SwitcherDao<T>,
+    private invalidForks:string[],
     private avgGenTime:number,
     private forkWindowSize:number,
     private switchOnHeadAdvance:number,
@@ -36,7 +37,7 @@ export class Switcher<T extends SwitchBlock> {
       const numberStart = current.number + this.switchOnHeadAdvance
       const timeStart = current.medianTime + this.switchOnHeadAdvance * this.avgGenTime
       // Phase 1: find potential chains
-      const suites = await this.findPotentialSuites(current, numberStart, timeStart)
+      const suites = await this.findPotentialSuites(numberStart, timeStart)
       if (suites.length) {
         this.logger && this.logger.info("Fork resolution: %s potential suite(s) found...", suites.length)
       }
@@ -62,19 +63,18 @@ export class Switcher<T extends SwitchBlock> {
   async findPotentialSuitesHeads(current:T) {
     const numberStart = current.number - this.forkWindowSize
     const timeStart = current.medianTime - this.forkWindowSize * this.avgGenTime
-    const suites = await this.findPotentialSuites(current, numberStart, timeStart)
+    const suites = await this.findPotentialSuites(numberStart, timeStart)
     return suites.map(suite => suite[suite.length - 1])
   }
 
   /**
    * Looks at the potential blocks that could form fork chains in the sandbox, and sort them to have a maximum of unique
    * chains.
-   * @param {SwitchBlock} current HEAD of local blockchain.
    * @param numberStart The minimum number of a fork block.
    * @param timeStart The minimum medianTime of a fork block.
    * @returns {SwitchBlock[][]} The suites found.
    */
-  private async findPotentialSuites(current:T, numberStart:number, timeStart:number) {
+  private async findPotentialSuites(numberStart:number, timeStart:number) {
     const suites:T[][] = []
     const potentials:T[] = await this.dao.getPotentials(numberStart, timeStart, numberStart + this.forkWindowSize)
     const knownForkBlocks:{ [k:string]: boolean } = {}
@@ -115,6 +115,12 @@ export class Switcher<T extends SwitchBlock> {
             previous = await this.dao.getSandboxBlock(previousNumber, previousHash)
             if (previous) {
               knownForkBlocks[BlockDTO.fromJSONObject(previous).blockstamp] = true
+              const alreadyKnownInvalidBlock = this.invalidForks.indexOf([previous.number, previous.hash].join('-')) !== -1
+              if (alreadyKnownInvalidBlock) {
+                // Incorrect = not found
+                this.logger && this.logger.info("Fork resolution: block #%s-%s is known as incorrect. Skipping.", previous.number, previous.hash.substr(0, 8))
+                previous = null
+              }
             }
           }
         }
@@ -164,6 +170,7 @@ export class Switcher<T extends SwitchBlock> {
           this.logger && this.logger.info("Fork resolution: suite %s/%s added block#%s-%s", j, suites.length, s[i].number, s[i].hash)
           successfulBlocks.push(s[i])
         } catch (e) {
+          this.invalidForks.push([s[i].number, s[i].hash].join('-'))
           this.logger && this.logger.info("Fork resolution: suite %s/%s REFUSED block#%s: %s", j, suites.length, s[0].number + i, e && e.message)
           added = false
         }
@@ -171,6 +178,9 @@ export class Switcher<T extends SwitchBlock> {
       }
       // Pop the successfuly added blocks
       if (successfulBlocks.length) {
+        for (const b of successfulBlocks) {
+          this.invalidForks.push([b.number, b.hash].join('-'))
+        }
         const addedToHeadLevel = successfulBlocks[successfulBlocks.length-1].number - current.number
         this.logger && this.logger.info("Fork resolution: suite %s/%s reached HEAD + %s. Now rolling back.", j, suites.length, addedToHeadLevel)
         await this.dao.revertTo(forkPoint)
diff --git a/app/service/BlockchainService.ts b/app/service/BlockchainService.ts
index 9631d6fdd..e10c4bec6 100644
--- a/app/service/BlockchainService.ts
+++ b/app/service/BlockchainService.ts
@@ -28,6 +28,7 @@ export class BlockchainService extends FIFOService {
   selfPubkey:string
   quickSynchronizer:QuickSynchronizer
   switcherDao:SwitcherDao<BlockDTO>
+  invalidForks:string[] = []
 
   constructor(private server:any, fifoPromiseHandler:GlobalFifoPromise) {
     super(fifoPromiseHandler)
@@ -119,7 +120,7 @@ export class BlockchainService extends FIFOService {
 
   async branches() {
     const current = await this.current()
-    const switcher = new Switcher(this.switcherDao, this.conf.avgGenTime, this.conf.forksize, this.conf.switchOnHeadAdvance, this.logger)
+    const switcher = new Switcher(this.switcherDao, this.invalidForks, this.conf.avgGenTime, this.conf.forksize, this.conf.switchOnHeadAdvance, this.logger)
     const heads = await switcher.findPotentialSuitesHeads(current)
     return heads.concat([current])
   }
@@ -197,6 +198,8 @@ export class BlockchainService extends FIFOService {
             bcEvent: OtherConstants.BC_EVENT.HEAD_CHANGED,
             block: addedBlock
           })
+          // Clear invalid forks' cache
+          this.invalidForks.splice(0, this.invalidForks.length)
         } catch (e) {
           this.logger.error(e)
           added = false
@@ -210,7 +213,7 @@ export class BlockchainService extends FIFOService {
   }
 
   async forkResolution() {
-    const switcher = new Switcher(this.switcherDao, this.conf.avgGenTime, this.conf.forksize, this.conf.switchOnHeadAdvance, this.logger)
+    const switcher = new Switcher(this.switcherDao, this.invalidForks, this.conf.avgGenTime, this.conf.forksize, this.conf.switchOnHeadAdvance, this.logger)
     const newCurrent = await switcher.tryToFork()
     if (newCurrent) {
       this.push({
diff --git a/test/fast/fork-resolution-3-3.ts b/test/fast/fork-resolution-3-3.ts
index 2a8e3215b..fc9a64a2b 100644
--- a/test/fast/fork-resolution-3-3.ts
+++ b/test/fast/fork-resolution-3-3.ts
@@ -1,4 +1,4 @@
-import * as assert from 'assert'
+import * as assert from "assert"
 import {SwitchBlock, Switcher, SwitcherDao} from "../../app/lib/blockchain/Switcher"
 import {NewLogger} from "../../app/lib/logger"
 
@@ -27,7 +27,7 @@ describe("Fork resolution 3-3 algo", () => {
       Block.from("C15"),
       Block.from("C16")
     ])
-    const switcher = new Switcher(new TestingSwitcherDao(bc, sbx), avgGenTime, forkWindowSize, switchOnHeadAdvance, logger)
+    const switcher = new Switcher(new TestingSwitcherDao(bc, sbx), [], avgGenTime, forkWindowSize, switchOnHeadAdvance, logger)
     await switcher.tryToFork()
     assert.equal(bc.current.number, 16)
     assert.equal(bc.current.hash, "C16")
@@ -49,7 +49,7 @@ describe("Fork resolution 3-3 algo", () => {
       Block.from("C14"),
       Block.from("C15")
     ])
-    const switcher = new Switcher(new TestingSwitcherDao(bc, sbx), avgGenTime, forkWindowSize, switchOnHeadAdvance)
+    const switcher = new Switcher(new TestingSwitcherDao(bc, sbx), [], avgGenTime, forkWindowSize, switchOnHeadAdvance)
     await switcher.tryToFork()
     assert.equal(bc.current.number, 13)
     assert.equal(bc.current.hash, "B13")
@@ -69,7 +69,7 @@ describe("Fork resolution 3-3 algo", () => {
       Block.from("C14"),
       Block.from("C15")
     ])
-    const switcher = new Switcher(new TestingSwitcherDao(bc, sbx), avgGenTime, forkWindowSize, switchOnHeadAdvance)
+    const switcher = new Switcher(new TestingSwitcherDao(bc, sbx), [], avgGenTime, forkWindowSize, switchOnHeadAdvance)
     await switcher.tryToFork()
     assert.equal(bc.current.number, 13)
     assert.equal(bc.current.hash, "B13")
@@ -94,7 +94,7 @@ describe("Fork resolution 3-3 algo", () => {
       Block.from("C15"),
       Block.from("C16")
     ])
-    const switcher = new Switcher(new TestingSwitcherDao(bc, sbx), avgGenTime, forkWindowSize, switchOnHeadAdvance)
+    const switcher = new Switcher(new TestingSwitcherDao(bc, sbx), [], avgGenTime, forkWindowSize, switchOnHeadAdvance)
     await switcher.tryToFork()
     assert.equal(bc.current.number, 13)
     assert.equal(bc.current.hash, "B13")
@@ -118,7 +118,7 @@ describe("Fork resolution 3-3 algo", () => {
       Block.from("C15"),
       Block.from("C16")
     ])
-    const switcher = new Switcher(new TestingSwitcherDao(bc, sbx), avgGenTime, forkWindowSize, switchOnHeadAdvance)
+    const switcher = new Switcher(new TestingSwitcherDao(bc, sbx), [], avgGenTime, forkWindowSize, switchOnHeadAdvance)
     await switcher.tryToFork()
     assert.equal(bc.current.number, 13)
     assert.equal(bc.current.hash, "B13")
@@ -141,7 +141,7 @@ describe("Fork resolution 3-3 algo", () => {
       Block.from("C15"),
       Block.from("C16")
     ])
-    const switcher = new Switcher(new TestingSwitcherDao(bc, sbx), avgGenTime, forkWindowSize, switchOnHeadAdvance)
+    const switcher = new Switcher(new TestingSwitcherDao(bc, sbx), [], avgGenTime, forkWindowSize, switchOnHeadAdvance)
     await switcher.tryToFork()
     assert.equal(bc.current.number, 13)
     assert.equal(bc.current.hash, "B13")
@@ -170,7 +170,7 @@ describe("Fork resolution 3-3 algo", () => {
       Block.from("E14"),
       Block.from("E15")
     ])
-    const switcher = new Switcher(new TestingSwitcherDao(bc, sbx), avgGenTime, forkWindowSize, 1)
+    const switcher = new Switcher(new TestingSwitcherDao(bc, sbx), [], avgGenTime, forkWindowSize, 1)
     await switcher.tryToFork()
     assert.equal(16, bc.current.number)
     assert.equal("D16", bc.current.hash)
-- 
GitLab