[ETCM-1042] Switch to a case class implementation for blockchain branch (#1073)

Aurélien Richez · web-flow · commit b9f3c0b87e35 · 2021-07-30T17:56:44.000+02:00
* [ETCM-1042] store only hash and number in best branch

* [ETCM-1042] move getBlockByNumber to BlockchainReader and out of Branch interface

* [ETCM-1042] move getHashByBlockNumber to BlockchainReader and out of Branch interface

* [ETCM-1042] move isInChain to BlockchainReader and out of Branch interface

* [ETCM-1042] remove old branch interface

* [ETCM-1042] rename BestBranchSubset to BestBranch
diff --git a/src/it/scala/io/iohk/ethereum/sync/RegularSyncItSpec.scala b/src/it/scala/io/iohk/ethereum/sync/RegularSyncItSpec.scala
@@ -124,7 +124,9 @@ class RegularSyncItSpec extends FreeSpecBase with Matchers with BeforeAndAfterAl
       _ <- peer2.importBlocksUntil(30)(IdentityUpdate)
       _ <- peer1.startRegularSync()
       _ <- peer2.startRegularSync()
-      _ <- peer2.addCheckpointedBlock(peer2.blockchainReader.getBestBranch().getBlockByNumber(25).get)
+      _ <- peer2.addCheckpointedBlock(
+        peer2.blockchainReader.getBlockByNumber(peer2.blockchainReader.getBestBranch(), 25).get
+      )
       _ <- peer2.waitForRegularSyncLoadLastBlock(length)
       _ <- peer1.connectToPeers(Set(peer2.node))
       _ <- peer1.waitForRegularSyncLoadLastBlock(length)
@@ -181,8 +183,8 @@ class RegularSyncItSpec extends FreeSpecBase with Matchers with BeforeAndAfterAl
         )
       )
       (
-        peer1.blockchainReader.getBestBranch().getBlockByNumber(blockNumer + 1),
-        peer2.blockchainReader.getBestBranch().getBlockByNumber(blockNumer + 1)
+        peer1.blockchainReader.getBlockByNumber(peer1.blockchainReader.getBestBranch(), blockNumer + 1),
+        peer2.blockchainReader.getBlockByNumber(peer2.blockchainReader.getBestBranch(), blockNumer + 1)
       ) match {
         case (Some(blockP1), Some(blockP2)) =>
           assert(peer1.bl.getChainWeightByHash(blockP1.hash) == peer2.bl.getChainWeightByHash(blockP2.hash))
diff --git a/src/it/scala/io/iohk/ethereum/sync/util/RegularSyncItSpecUtils.scala b/src/it/scala/io/iohk/ethereum/sync/util/RegularSyncItSpecUtils.scala
@@ -189,8 +189,7 @@ object RegularSyncItSpecUtils {
       Task(blockNumber match {
         case Some(bNumber) =>
           blockchainReader
-            .getBestBranch()
-            .getBlockByNumber(bNumber)
+            .getBlockByNumber(blockchainReader.getBestBranch(), bNumber)
             .getOrElse(throw new RuntimeException(s"block by number: $bNumber doesn't exist"))
         case None => blockchainReader.getBestBlock().get
       }).flatMap { block =>
diff --git a/src/it/scala/io/iohk/ethereum/txExecTest/util/DumpChainApp.scala b/src/it/scala/io/iohk/ethereum/txExecTest/util/DumpChainApp.scala
@@ -26,7 +26,6 @@ import io.iohk.ethereum.db.storage.pruning.PruningMode
 import io.iohk.ethereum.domain.BlockHeader.HeaderExtraFields.HefEmpty
 import io.iohk.ethereum.domain.Blockchain
 import io.iohk.ethereum.domain._
-import io.iohk.ethereum.domain.branch.Branch
 import io.iohk.ethereum.jsonrpc.ProofService.EmptyStorageValueProof
 import io.iohk.ethereum.jsonrpc.ProofService.StorageProof
 import io.iohk.ethereum.jsonrpc.ProofService.StorageProofKey
@@ -102,9 +101,7 @@ object DumpChainApp
 
   val blockchain: Blockchain = new BlockchainMock(genesisHash)
   val blockchainReader: BlockchainReader = mock[BlockchainReader]
-  val bestChain: Branch = mock[Branch]
-  (blockchainReader.getBestBranch _).expects().anyNumberOfTimes().returning(bestChain)
-  (bestChain.getHashByBlockNumber _).expects(*).returning(Some(genesisHash))
+  (blockchainReader.getHashByBlockNumber _).expects(*, *).returning(Some(genesisHash))
 
   val nodeStatus: NodeStatus =
     NodeStatus(key = nodeKey, serverStatus = ServerStatus.NotListening, discoveryStatus = ServerStatus.NotListening)
diff --git a/src/main/scala/io/iohk/ethereum/consensus/pow/validators/OmmersValidator.scala b/src/main/scala/io/iohk/ethereum/consensus/pow/validators/OmmersValidator.scala
@@ -34,7 +34,7 @@ trait OmmersValidator {
     val getNBlocksBack: (ByteString, Int) => List[Block] =
       (_, n) =>
         ((blockNumber - n) until blockNumber).toList
-          .flatMap(nb => bestBranch.getBlockByNumber(nb))
+          .flatMap(nb => blockchainReader.getBlockByNumber(bestBranch, nb))
 
     validate(parentHash, blockNumber, ommers, getBlockHeaderByHash, getNBlocksBack)
   }
diff --git a/src/main/scala/io/iohk/ethereum/domain/Blockchain.scala b/src/main/scala/io/iohk/ethereum/domain/Blockchain.scala
@@ -211,7 +211,7 @@ class BlockchainImpl(
     val latestCheckpointNumber = getLatestCheckpointBlockNumber()
 
     val blockNumberMappingUpdates =
-      if (blockchainReader.getBestBranch().getHashByBlockNumber(block.number).contains(blockHash))
+      if (blockchainReader.getHashByBlockNumber(blockchainReader.getBestBranch(), block.number).contains(blockHash))
         removeBlockNumberMapping(block.number)
       else blockNumberMappingStorage.emptyBatchUpdate
 
@@ -280,7 +280,7 @@ class BlockchainImpl(
   ): BigInt =
     if (blockNumberToCheck > 0) {
       val maybePreviousCheckpointBlockNumber = for {
-        currentBlock <- blockchainReader.getBestBranch().getBlockByNumber(blockNumberToCheck)
+        currentBlock <- blockchainReader.getBlockByNumber(blockchainReader.getBestBranch(), blockNumberToCheck)
         if currentBlock.hasCheckpoint &&
           currentBlock.number < latestCheckpointBlockNumber
       } yield currentBlock.number
diff --git a/src/main/scala/io/iohk/ethereum/domain/BlockchainReader.scala b/src/main/scala/io/iohk/ethereum/domain/BlockchainReader.scala
@@ -10,7 +10,7 @@ import io.iohk.ethereum.db.storage.ReceiptStorage
 import io.iohk.ethereum.db.storage.StateStorage
 import io.iohk.ethereum.domain.branch.BestBranch
 import io.iohk.ethereum.domain.branch.Branch
-import io.iohk.ethereum.domain.branch.EmptyBranch$
+import io.iohk.ethereum.domain.branch.EmptyBranch
 import io.iohk.ethereum.mpt.MptNode
 import io.iohk.ethereum.utils.Logger
 
@@ -71,16 +71,14 @@ class BlockchainReader(
   def getReceiptsByHash(blockhash: ByteString): Option[Seq[Receipt]] = receiptStorage.get(blockhash)
 
   /** get the current best stored branch */
-  def getBestBranch(): Branch =
-    getBestBlock()
-      .map { block =>
-        new BestBranch(
-          block.header,
-          blockNumberMappingStorage,
-          this
-        )
-      }
-      .getOrElse(EmptyBranch$)
+  def getBestBranch(): Branch = {
+    val number = getBestBlockNumber()
+    blockNumberMappingStorage
+      .get(number)
+      .orElse(blockNumberMappingStorage.get(appStateStorage.getBestBlockNumber()))
+      .map(hash => BestBranch(hash, number))
+      .getOrElse(EmptyBranch)
+  }
 
   def getBestBlockNumber(): BigInt = {
     val bestSavedBlockNumber = appStateStorage.getBestBlockNumber()
@@ -113,6 +111,36 @@ class BlockchainReader(
   def genesisBlock: Block =
     getBlockByNumber(0).get
 
+  /** Returns a block inside this branch based on its number */
+  def getBlockByNumber(branch: Branch, number: BigInt): Option[Block] = branch match {
+    case BestBranch(_, tipBlockNumber) if tipBlockNumber >= number && number >= 0 =>
+      for {
+        hash <- getHashByBlockNumber(number)
+        block <- getBlockByHash(hash)
+      } yield block
+    case EmptyBranch | BestBranch(_, _) => None
+  }
+
+  /** Returns a block hash for the block at the given height if any */
+  def getHashByBlockNumber(branch: Branch, number: BigInt): Option[ByteString] = branch match {
+    case BestBranch(_, tipBlockNumber) =>
+      if (tipBlockNumber >= number && number >= 0) {
+        blockNumberMappingStorage.get(number)
+      } else None
+
+    case EmptyBranch => None
+  }
+
+  /** Checks if given block hash is in this chain. (i.e. is an ancestor of the tip block) */
+  def isInChain(branch: Branch, hash: ByteString): Boolean = branch match {
+    case BestBranch(_, tipBlockNumber) =>
+      (for {
+        header <- getBlockHeaderByHash(hash) if header.number <= tipBlockNumber
+        hashFromBestChain <- getHashByBlockNumber(branch, header.number)
+      } yield header.hash == hashFromBestChain).getOrElse(false)
+    case EmptyBranch => false
+  }
+
   /** Allows to query for a block based on it's number
     *
     * @param number Block number
diff --git a/src/main/scala/io/iohk/ethereum/domain/branch/BestBranch.scala b/src/main/scala/io/iohk/ethereum/domain/branch/BestBranch.scala
diff --git a/src/main/scala/io/iohk/ethereum/domain/branch/Branch.scala b/src/main/scala/io/iohk/ethereum/domain/branch/Branch.scala
@@ -2,17 +2,8 @@ package io.iohk.ethereum.domain.branch
 
 import akka.util.ByteString
 
-import io.iohk.ethereum.domain.Block
+sealed trait Branch
 
-/** An interface to manipulate blockchain branches */
-trait Branch {
+case class BestBranch(tipBlockHash: ByteString, tipBlockNumber: BigInt) extends Branch
 
-  /** Returns a block inside this branch based on its number */
-  def getBlockByNumber(number: BigInt): Option[Block]
-
-  /** Returns a block hash for the block at the given height if any */
-  def getHashByBlockNumber(number: BigInt): Option[ByteString]
-
-  /** Checks if given block hash is in this chain. (i.e. is an ancestor of the tip block) */
-  def isInChain(hash: ByteString): Boolean
-}
+case object EmptyBranch extends Branch
diff --git a/src/main/scala/io/iohk/ethereum/domain/branch/EmptyBranch$.scala b/src/main/scala/io/iohk/ethereum/domain/branch/EmptyBranch$.scala
diff --git a/src/main/scala/io/iohk/ethereum/jsonrpc/CheckpointingService.scala b/src/main/scala/io/iohk/ethereum/jsonrpc/CheckpointingService.scala
@@ -9,15 +9,13 @@ import io.iohk.ethereum.blockchain.sync.regular.RegularSync.NewCheckpoint
 import io.iohk.ethereum.consensus.blocks.CheckpointBlockGenerator
 import io.iohk.ethereum.crypto.ECDSASignature
 import io.iohk.ethereum.domain.Block
-import io.iohk.ethereum.domain.Blockchain
 import io.iohk.ethereum.domain.BlockchainReader
 import io.iohk.ethereum.domain.Checkpoint
 import io.iohk.ethereum.ledger.BlockQueue
 import io.iohk.ethereum.utils.ByteStringUtils
 import io.iohk.ethereum.utils.Logger
 
 class CheckpointingService(
-    blockchain: Blockchain,
     blockchainReader: BlockchainReader,
     blockQueue: BlockQueue,
     checkpointBlockGenerator: CheckpointBlockGenerator,
@@ -36,7 +34,7 @@ class CheckpointingService(
       req.parentCheckpoint.forall(blockchainReader.getBlockHeaderByHash(_).exists(_.number < blockToReturnNum))
 
     Task {
-      blockchainReader.getBestBranch().getBlockByNumber(blockToReturnNum)
+      blockchainReader.getBlockByNumber(blockchainReader.getBestBranch(), blockToReturnNum)
     }.flatMap {
       case Some(b) if isValidParent =>
         Task.now(Right(GetLatestBlockResponse(Some(BlockInfo(b.hash, b.number)))))
diff --git a/src/main/scala/io/iohk/ethereum/jsonrpc/EthProofService.scala b/src/main/scala/io/iohk/ethereum/jsonrpc/EthProofService.scala
@@ -205,8 +205,7 @@ class EthProofService(
   private def resolveBlock(blockParam: BlockParam): Either[JsonRpcError, ResolvedBlock] = {
     def getBlock(number: BigInt): Either[JsonRpcError, Block] =
       blockchainReader
-        .getBestBranch()
-        .getBlockByNumber(number)
+        .getBlockByNumber(blockchainReader.getBestBranch(), number)
         .toRight(JsonRpcError.InvalidParams(s"Block $number not found"))
 
     def getLatestBlock(): Either[JsonRpcError, Block] =
diff --git a/src/main/scala/io/iohk/ethereum/jsonrpc/EthTxService.scala b/src/main/scala/io/iohk/ethereum/jsonrpc/EthTxService.scala
@@ -158,7 +158,7 @@ class EthTxService(
     Task {
       val bestBranch = blockchainReader.getBestBranch()
       val gasPrice = ((bestBlock - blockDifference) to bestBlock)
-        .flatMap(nb => bestBranch.getBlockByNumber(nb))
+        .flatMap(nb => blockchainReader.getBlockByNumber(bestBranch, nb))
         .flatMap(_.body.transactionList)
         .map(_.tx.gasPrice)
       if (gasPrice.nonEmpty) {
diff --git a/src/main/scala/io/iohk/ethereum/jsonrpc/ResolveBlock.scala b/src/main/scala/io/iohk/ethereum/jsonrpc/ResolveBlock.scala
@@ -34,8 +34,7 @@ trait ResolveBlock {
 
   private def getBlock(number: BigInt): Either[JsonRpcError, Block] =
     blockchainReader
-      .getBestBranch()
-      .getBlockByNumber(number)
+      .getBlockByNumber(blockchainReader.getBestBranch(), number)
       .toRight(JsonRpcError.InvalidParams(s"Block $number not found"))
 
   private def getLatestBlock(): Either[JsonRpcError, Block] =
diff --git a/src/main/scala/io/iohk/ethereum/jsonrpc/TestService.scala b/src/main/scala/io/iohk/ethereum/jsonrpc/TestService.scala
@@ -368,7 +368,7 @@ class TestService(
 
     val blockOpt = request.parameters.blockHashOrNumber
       .fold(
-        number => blockchainReader.getBestBranch().getBlockByNumber(number),
+        number => blockchainReader.getBlockByNumber(blockchainReader.getBestBranch(), number),
         blockHash => blockchainReader.getBlockByHash(blockHash)
       )
 
@@ -408,7 +408,7 @@ class TestService(
 
     val blockOpt = request.parameters.blockHashOrNumber
       .fold(
-        number => blockchainReader.getBestBranch().getBlockByNumber(number),
+        number => blockchainReader.getBlockByNumber(blockchainReader.getBestBranch(), number),
         hash => blockchainReader.getBlockByHash(hash)
       )
 
diff --git a/src/main/scala/io/iohk/ethereum/ledger/BlockImport.scala b/src/main/scala/io/iohk/ethereum/ledger/BlockImport.scala
@@ -86,7 +86,7 @@ class BlockImport(
   private def isPossibleNewBestBlock(newBlock: BlockHeader, currentBestBlock: BlockHeader): Boolean =
     newBlock.parentHash == currentBestBlock.hash && newBlock.number == currentBestBlock.number + 1
 
-  private def measureBlockMetrics(importResult: BlockImportResult)(implicit blockchainConfig: BlockchainConfig): Unit =
+  private def measureBlockMetrics(importResult: BlockImportResult): Unit =
     importResult match {
       case BlockImportedToTop(blockImportData) =>
         blockImportData.foreach(blockData => BlockMetrics.measure(blockData.block, blockchainReader.getBlockByHash))
@@ -318,7 +318,7 @@ class BlockImport(
   private def removeBlocksUntil(parent: ByteString, fromNumber: BigInt): List[BlockData] = {
     @tailrec
     def removeBlocksUntil(parent: ByteString, fromNumber: BigInt, acc: List[BlockData]): List[BlockData] =
-      blockchainReader.getBestBranch().getBlockByNumber(fromNumber) match {
+      blockchainReader.getBlockByNumber(blockchainReader.getBestBranch(), fromNumber) match {
         case Some(block) if block.header.hash == parent || fromNumber == 0 =>
           acc
 
diff --git a/src/main/scala/io/iohk/ethereum/ledger/BlockValidation.scala b/src/main/scala/io/iohk/ethereum/ledger/BlockValidation.scala
@@ -46,7 +46,7 @@ class BlockValidation(
           val numbers = (block.header.number - remaining) until block.header.number
           val bestBranch = blockchainReader.getBestBranch()
           val blocks =
-            (numbers.toList.flatMap(nb => bestBranch.getBlockByNumber(nb)) :+ block) ::: queuedBlocks
+            (numbers.toList.flatMap(nb => blockchainReader.getBlockByNumber(bestBranch, nb)) :+ block) ::: queuedBlocks
           blocks
       }
     }
diff --git a/src/main/scala/io/iohk/ethereum/ledger/BranchResolution.scala b/src/main/scala/io/iohk/ethereum/ledger/BranchResolution.scala
@@ -16,8 +16,10 @@ class BranchResolution(blockchain: Blockchain, blockchainReader: BlockchainReade
       InvalidBranch
     } else {
       val knownParentOrGenesis = blockchainReader
-        .getBestBranch()
-        .isInChain(headers.head.parentHash) || headers.head.hash == blockchainReader.genesisHeader.hash
+        .isInChain(
+          blockchainReader.getBestBranch(),
+          headers.head.parentHash
+        ) || headers.head.hash == blockchainReader.genesisHeader.hash
 
       if (!knownParentOrGenesis)
         UnknownBranch
@@ -76,7 +78,7 @@ class BranchResolution(blockchain: Blockchain, blockchainReader: BlockchainReade
   private def getTopBlocksFromNumber(from: BigInt): List[Block] = {
     val bestBranch = blockchainReader.getBestBranch()
     (from to blockchainReader.getBestBlockNumber())
-      .flatMap(nb => bestBranch.getBlockByNumber(nb))
+      .flatMap(nb => blockchainReader.getBlockByNumber(bestBranch, nb))
       .toList
   }
 }
diff --git a/src/main/scala/io/iohk/ethereum/nodebuilder/NodeBuilder.scala b/src/main/scala/io/iohk/ethereum/nodebuilder/NodeBuilder.scala
@@ -570,7 +570,6 @@ trait CheckpointingServiceBuilder {
 
   lazy val checkpointingService =
     new CheckpointingService(
-      blockchain,
       blockchainReader,
       blockQueue,
       checkpointBlockGenerator,
diff --git a/src/main/scala/io/iohk/ethereum/testmode/TestEthBlockServiceWrapper.scala b/src/main/scala/io/iohk/ethereum/testmode/TestEthBlockServiceWrapper.scala
@@ -76,7 +76,7 @@ class TestEthBlockServiceWrapper(
     .map(
       _.map { blockByBlockResponse =>
         val bestBranch = blockchainReader.getBestBranch()
-        val fullBlock = bestBranch.getBlockByNumber(blockByBlockResponse.blockResponse.get.number).get
+        val fullBlock = blockchainReader.getBlockByNumber(bestBranch, blockByBlockResponse.blockResponse.get.number).get
         BlockByNumberResponse(blockByBlockResponse.blockResponse.map(response => toEthResponse(fullBlock, response)))
       }
     )
diff --git a/src/main/scala/io/iohk/ethereum/transactions/TransactionHistoryService.scala b/src/main/scala/io/iohk/ethereum/transactions/TransactionHistoryService.scala
@@ -33,7 +33,9 @@ class TransactionHistoryService(
     val getLastCheckpoint = Task(blockchain.getLatestCheckpointBlockNumber()).memoizeOnSuccess
     val txnsFromBlocks = Observable
       .from(fromBlocks.reverse)
-      .mapParallelOrdered(10)(blockNr => Task(blockchainReader.getBestBranch().getBlockByNumber(blockNr)))(
+      .mapParallelOrdered(10)(blockNr =>
+        Task(blockchainReader.getBlockByNumber(blockchainReader.getBestBranch(), blockNr))
+      )(
         OverflowStrategy.Unbounded
       )
       .collect { case Some(block) => block }
diff --git a/src/test/scala/io/iohk/ethereum/consensus/pow/validators/StdOmmersValidatorSpec.scala b/src/test/scala/io/iohk/ethereum/consensus/pow/validators/StdOmmersValidatorSpec.scala
@@ -81,7 +81,7 @@ class StdOmmersValidatorSpec extends AnyFlatSpec with Matchers with ScalaCheckPr
     val getNBlocksBack: (ByteString, Int) => List[Block] =
       (_, n) =>
         ((ommersBlockNumber - n) until ommersBlockNumber).toList
-          .flatMap(nb => blockchainReader.getBestBranch().getBlockByNumber(nb))
+          .flatMap(nb => blockchainReader.getBlockByNumber(blockchainReader.getBestBranch(), nb))
 
     ommersValidator.validateOmmersAncestors(
       ommersBlockParentHash,
diff --git a/src/test/scala/io/iohk/ethereum/domain/BlockchainSpec.scala b/src/test/scala/io/iohk/ethereum/domain/BlockchainSpec.scala
diff --git a/src/test/scala/io/iohk/ethereum/jsonrpc/CheckpointingServiceSpec.scala b/src/test/scala/io/iohk/ethereum/jsonrpc/CheckpointingServiceSpec.scala
diff --git a/src/test/scala/io/iohk/ethereum/ledger/LedgerTestSetup.scala b/src/test/scala/io/iohk/ethereum/ledger/LedgerTestSetup.scala

Original file line number	Diff line number	Diff line change
`@@ -34,7 +34,7 @@ trait OmmersValidator {`
`34`	`34`	`val getNBlocksBack: (ByteString, Int) => List[Block] =`
`35`	`35`	`(_, n) =>`
`36`	`36`	`((blockNumber - n) until blockNumber).toList`
`37`		`- .flatMap(nb => bestBranch.getBlockByNumber(nb))`
	`37`	`+ .flatMap(nb => blockchainReader.getBlockByNumber(bestBranch, nb))`
`38`	`38`
`39`	`39`	`validate(parentHash, blockNumber, ommers, getBlockHeaderByHash, getNBlocksBack)`
`40`	`40`	`}`