etcm-678 etcm-660 fix for removing chain after the node restart

Author: pslaski

src/main/scala/io/iohk/ethereum/domain/Blockchain.scala

@@ -215,6 +215,19 @@ trait Blockchain {
   def getStateStorage: StateStorage
 
   def mptStateSavedKeys(): Observable[Either[IterationError, ByteString]]
+
+  /**
+    * Strict check if given block hash is in chain
+    * Using any of getXXXByHash is not always accurate - after restart the best block is often lower than before restart
+    * The result of that is returning data of blocks which we don't consider as a part of the chain anymore
+    * @param hash block hash
+    */
+  def isInChain(hash: ByteString): Boolean = {
+    (for {
+      header <- getBlockHeaderByHash(hash) if header.number <= getBestBlockNumber()
+      hash <- getHashByBlockNumber(header.number)
+    } yield header.hash == hash).getOrElse(false)
+  }
 }
 // scalastyle:on
 

src/main/scala/io/iohk/ethereum/ledger/BranchResolution.scala

@@ -11,8 +11,7 @@ class BranchResolution(blockchain: Blockchain) extends Logger {
       InvalidBranch
     } else {
       val knownParentOrGenesis = blockchain
-        .getBlockHeaderByHash(headers.head.parentHash)
-        .isDefined || headers.head.hash == blockchain.genesisHeader.hash
+        .isInChain(headers.head.parentHash) || headers.head.hash == blockchain.genesisHeader.hash
 
       if (!knownParentOrGenesis)
         UnknownBranch

src/test/scala/io/iohk/ethereum/domain/BlockchainSpec.scala

@@ -44,6 +44,15 @@ class BlockchainSpec extends AnyFlatSpec with Matchers with ScalaCheckPropertyCh
     assert(validBlock == block.get)
   }
 
+  it should "be able to do strict check of block existence in the chain" in new EphemBlockchainTestSetup {
+    val validBlock = Fixtures.Blocks.ValidBlock.block
+    blockchain.save(validBlock, Seq.empty, ChainWeight(100, 100), saveAsBestBlock = true)
+    assert(blockchain.isInChain(validBlock.hash))
+    // simulation of node restart
+    blockchain.saveBestKnownBlocks(validBlock.header.number - 1)
+    assert(!blockchain.isInChain(validBlock.hash))
+  }
+
   it should "be able to query a stored blockHeader by it's number" in new EphemBlockchainTestSetup {
     val validHeader = Fixtures.Blocks.ValidBlock.header
     blockchain.storeBlockHeader(validHeader).commit()

src/test/scala/io/iohk/ethereum/ledger/BranchResolutionSpec.scala

@@ -41,8 +41,7 @@ class BranchResolutionSpec
       val headers = getChainHeadersNel(5, 10)
 
       setGenesisHeader(genesisHeader) // Check genesis block
-      setBestBlockNumber(10)
-      setHeaderByHash(headers.head.parentHash, None)
+      setHeaderInChain(headers.head.parentHash, result = false)
 
       ledger.resolveBranch(headers) shouldEqual UnknownBranch
     }
@@ -52,21 +51,21 @@ class BranchResolutionSpec
         val headers = getChainHeadersNel(1, 10)
 
         setBestBlockNumber(10)
-        setHeaderByHash(headers.head.parentHash, Some(getBlock(0).header))
+        setHeaderInChain(headers.head.parentHash)
         setChainWeightByHash(headers.head.parentHash, ChainWeight.zero)
 
         val oldBlocks = getChain(1, 10, headers.head.parentHash, headers.head.difficulty - 1)
         oldBlocks.map(b => setBlockByNumber(b.header.number, Some(b)))
 
-        ledger.resolveBranch(headers) shouldEqual NewBetterBranch(oldBlocks.toList)
+        ledger.resolveBranch(headers) shouldEqual NewBetterBranch(oldBlocks)
       }
 
     "report no need for a chain switch the headers do not have chain weight greater than currently known branch" in
       new BranchResolutionTestSetup {
         val headers = getChainHeadersNel(1, 10)
 
         setBestBlockNumber(10)
-        setHeaderByHash(headers.head.parentHash, Some(getBlock(0).header))
+        setHeaderInChain(headers.head.parentHash)
         setChainWeightByHash(headers.head.parentHash, ChainWeight.zero)
 
         val oldBlocks = getChain(1, 10, headers.head.parentHash, headers.head.difficulty)
@@ -78,6 +77,7 @@ class BranchResolutionSpec
     "correctly handle a branch that goes up to the genesis block" in new BranchResolutionTestSetup {
       val headers = genesisHeader :: getChainHeadersNel(1, 10, genesisHeader.hash)
 
+      setHeaderInChain(genesisHeader.parentHash, result = false)
       setGenesisHeader(genesisHeader)
       setBestBlockNumber(10)
       setChainWeightByHash(genesisHeader.hash, ChainWeight.zero)
@@ -93,6 +93,7 @@ class BranchResolutionSpec
       val differentGenesis: BlockHeader = genesisHeader.copy(extraData = ByteString("I'm different ;("))
       val headers = differentGenesis :: getChainHeadersNel(1, 10, differentGenesis.hash)
 
+      setHeaderInChain(differentGenesis.parentHash, result = false)
       setGenesisHeader(genesisHeader)
       setBestBlockNumber(10)
 
@@ -104,7 +105,7 @@ class BranchResolutionSpec
       val commonParent = headers.toList(1)
 
       setBestBlockNumber(8)
-      setHeaderByHash(headers.head.parentHash, Some(getBlock(0).header))
+      setHeaderInChain(headers.head.parentHash)
       setChainWeightByHash(commonParent.hash, ChainWeight.zero)
 
       val oldBlocks = getChain(3, 8, commonParent.hash)
@@ -123,7 +124,7 @@ class BranchResolutionSpec
       val longerBranchLowerWeight = getChain(2, 10, commonParent.hash, difficulty = 100)
       val shorterBranchHigherWeight = getChainNel(2, 8, commonParent.hash, difficulty = 200)
 
-      setHeaderByHash(commonParent.hash, Some(commonParent.header))
+      setHeaderInChain(commonParent.hash)
       setChainWeightForBlock(commonParent, parentWeight)
       setBestBlockNumber(longerBranchLowerWeight.last.number)
       longerBranchLowerWeight.foreach(b => setBlockByNumber(b.number, Some(b)))
@@ -150,7 +151,7 @@ class BranchResolutionSpec
 
           val noCheckpointBranch = getChain(2, checkpointBranchLength + 2, commonParent.hash)
 
-          setHeaderByHash(commonParent.hash, Some(commonParent.header))
+          setHeaderInChain(commonParent.hash)
           setChainWeightForBlock(commonParent, parentWeight)
           setBestBlockNumber(noCheckpointBranch.last.number)
           noCheckpointBranch.foreach(b => setBlockByNumber(b.number, Some(b)))
@@ -176,7 +177,7 @@ class BranchResolutionSpec
 
           val noCheckpointBranch = getChainNel(2, checkpointBranchLength + 2, commonParent.hash)
 
-          setHeaderByHash(commonParent.hash, Some(commonParent.header))
+          setHeaderInChain(commonParent.hash)
           setChainWeightForBlock(commonParent, parentWeight)
           setBestBlockNumber(checkpointBranch.last.number)
           checkpointBranch.map(b => setBlockByNumber(b.number, Some(b)))

src/test/scala/io/iohk/ethereum/ledger/LedgerTestSetup.scala

@@ -393,15 +393,15 @@ trait MockBlockchain extends MockFactory { self: TestSetupWithVmAndValidators =>
       .once()
   }
 
-  def setHeaderByHash(hash: ByteString, header: Option[BlockHeader]): CallHandler1[ByteString, Option[BlockHeader]] =
-    (blockchain.getBlockHeaderByHash _).expects(hash).returning(header)
+  def setHeaderInChain(hash: ByteString, result: Boolean = true): CallHandler1[ByteString, Boolean] = {
+    (blockchain.isInChain _).expects(hash).returning(result)
+  }
 
   def setBlockByNumber(number: BigInt, block: Option[Block]): CallHandler1[BigInt, Option[Block]] =
     (blockchain.getBlockByNumber _).expects(number).returning(block)
 
   def setGenesisHeader(header: BlockHeader): Unit = {
     (() => blockchain.genesisHeader).expects().returning(header)
-    setHeaderByHash(header.parentHash, None)
   }
 }