[ETCM-969] simple Branch implementation (#1039)

* create Branch class with getBlockByNumber and getHashByBlockNumber functions

* make getBlockByNumber private in blockchain reader

* Create EmptyBlochainBranch and make getBestChain return it if there no known best block

* Rephrase scaladoc comment and remove TODO

Author: Aurélien Richez

src/it/scala/io/iohk/ethereum/sync/RegularSyncItSpec.scala

@@ -124,7 +124,7 @@ class RegularSyncItSpec extends FreeSpecBase with Matchers with BeforeAndAfterAl
       _ <- peer2.importBlocksUntil(30)(IdentityUpdate)
       _ <- peer1.startRegularSync()
       _ <- peer2.startRegularSync()
-      _ <- peer2.addCheckpointedBlock(peer2.blockchainReader.getBlockByNumber(25).get)
+      _ <- peer2.addCheckpointedBlock(peer2.blockchainReader.getBestBranch().getBlockByNumber(25).get)
       _ <- peer2.waitForRegularSyncLoadLastBlock(length)
       _ <- peer1.connectToPeers(Set(peer2.node))
       _ <- peer1.waitForRegularSyncLoadLastBlock(length)
@@ -181,8 +181,8 @@ class RegularSyncItSpec extends FreeSpecBase with Matchers with BeforeAndAfterAl
         )
       )
       (
-        peer1.blockchainReader.getBlockByNumber(blockNumer + 1),
-        peer2.blockchainReader.getBlockByNumber(blockNumer + 1)
+        peer1.blockchainReader.getBestBranch().getBlockByNumber(blockNumer + 1),
+        peer2.blockchainReader.getBestBranch().getBlockByNumber(blockNumer + 1)
       ) match {
         case (Some(blockP1), Some(blockP2)) =>
           assert(peer1.bl.getChainWeightByHash(blockP1.hash) == peer2.bl.getChainWeightByHash(blockP2.hash))

src/it/scala/io/iohk/ethereum/sync/util/RegularSyncItSpecUtils.scala

@@ -187,6 +187,7 @@ object RegularSyncItSpecUtils {
       Task(blockNumber match {
         case Some(bNumber) =>
           blockchainReader
+            .getBestBranch()
             .getBlockByNumber(bNumber)
             .getOrElse(throw new RuntimeException(s"block by number: $bNumber doesn't exist"))
         case None => blockchainReader.getBestBlock().get

src/it/scala/io/iohk/ethereum/txExecTest/util/DumpChainApp.scala

@@ -26,6 +26,7 @@ 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
@@ -101,7 +102,9 @@ object DumpChainApp
 
   val blockchain: Blockchain = new BlockchainMock(genesisHash)
   val blockchainReader: BlockchainReader = mock[BlockchainReader]
-  (blockchainReader.getHashByBlockNumber _).expects(*).returning(Some(genesisHash))
+  val bestChain: Branch = mock[Branch]
+  (blockchainReader.getBestBranch _).expects().anyNumberOfTimes().returning(bestChain)
+  (bestChain.getHashByBlockNumber _).expects(*).returning(Some(genesisHash))
 
   val nodeStatus: NodeStatus =
     NodeStatus(key = nodeKey, serverStatus = ServerStatus.NotListening, discoveryStatus = ServerStatus.NotListening)

src/main/scala/io/iohk/ethereum/consensus/pow/validators/OmmersValidator.scala

@@ -29,8 +29,11 @@ trait OmmersValidator {
   ): Either[OmmersError, OmmersValid] = {
 
     val getBlockHeaderByHash: ByteString => Option[BlockHeader] = blockchainReader.getBlockHeaderByHash
+    val bestBranch = blockchainReader.getBestBranch()
     val getNBlocksBack: (ByteString, Int) => List[Block] =
-      (_, n) => ((blockNumber - n) until blockNumber).toList.flatMap(blockchainReader.getBlockByNumber)
+      (_, n) =>
+        ((blockNumber - n) until blockNumber).toList
+          .flatMap(nb => bestBranch.getBlockByNumber(nb))
 
     validate(parentHash, blockNumber, ommers, getBlockHeaderByHash, getNBlocksBack)
   }

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

@@ -103,7 +103,7 @@ class BlockchainImpl(
   override def isInChain(hash: ByteString): Boolean =
     (for {
       header <- blockchainReader.getBlockHeaderByHash(hash) if header.number <= blockchainReader.getBestBlockNumber()
-      hash <- blockchainReader.getHashByBlockNumber(header.number)
+      hash <- blockchainReader.getBestBranch().getHashByBlockNumber(header.number)
     } yield header.hash == hash).getOrElse(false)
 
   override def getChainWeightByHash(blockhash: ByteString): Option[ChainWeight] = chainWeightStorage.get(blockhash)
@@ -223,7 +223,7 @@ class BlockchainImpl(
     val latestCheckpointNumber = getLatestCheckpointBlockNumber()
 
     val blockNumberMappingUpdates =
-      if (blockchainReader.getHashByBlockNumber(block.number).contains(blockHash))
+      if (blockchainReader.getBestBranch().getHashByBlockNumber(block.number).contains(blockHash))
         removeBlockNumberMapping(block.number)
       else blockNumberMappingStorage.emptyBatchUpdate
 
@@ -292,7 +292,7 @@ class BlockchainImpl(
   ): BigInt =
     if (blockNumberToCheck > 0) {
       val maybePreviousCheckpointBlockNumber = for {
-        currentBlock <- blockchainReader.getBlockByNumber(blockNumberToCheck)
+        currentBlock <- blockchainReader.getBestBranch().getBlockByNumber(blockNumberToCheck)
         if currentBlock.hasCheckpoint &&
           currentBlock.number < latestCheckpointBlockNumber
       } yield currentBlock.number

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

@@ -8,6 +8,9 @@ import io.iohk.ethereum.db.storage.BlockHeadersStorage
 import io.iohk.ethereum.db.storage.BlockNumberMappingStorage
 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.mpt.MptNode
 import io.iohk.ethereum.utils.Logger
 
@@ -48,31 +51,12 @@ class BlockchainReader(
       body <- getBlockBodyByHash(hash)
     } yield Block(header, body)
 
-  /** Returns a block hash given a block number
-    *
-    * @param number Number of the searchead block
-    * @return Block hash if found
-    */
-  def getHashByBlockNumber(number: BigInt): Option[ByteString] =
-    blockNumberMappingStorage.get(number)
-
   def getBlockHeaderByNumber(number: BigInt): Option[BlockHeader] =
     for {
       hash <- getHashByBlockNumber(number)
       header <- getBlockHeaderByHash(hash)
     } yield header
 
-  /** Allows to query for a block based on it's number
-    *
-    * @param number Block number
-    * @return Block if it exists
-    */
-  def getBlockByNumber(number: BigInt): Option[Block] =
-    for {
-      hash <- getHashByBlockNumber(number)
-      block <- getBlockByHash(hash)
-    } yield block
-
   /** Returns MPT node searched by it's hash
     * @param hash Node Hash
     * @return MPT node
@@ -86,6 +70,18 @@ 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 getBestBlockNumber(): BigInt = {
     val bestSavedBlockNumber = appStateStorage.getBestBlockNumber()
     val bestKnownBlockNumber = blockchainMetadata.bestKnownBlockAndLatestCheckpoint.get().bestBlockNumber
@@ -116,6 +112,25 @@ class BlockchainReader(
 
   def genesisBlock: Block =
     getBlockByNumber(0).get
+
+  /** Allows to query for a block based on it's number
+    *
+    * @param number Block number
+    * @return Block if it exists
+    */
+  private def getBlockByNumber(number: BigInt): Option[Block] =
+    for {
+      hash <- getHashByBlockNumber(number)
+      block <- getBlockByHash(hash)
+    } yield block
+
+  /** Returns a block hash given a block number
+    *
+    * @param number Number of the searchead block
+    * @return Block hash if found
+    */
+  private def getHashByBlockNumber(number: BigInt): Option[ByteString] =
+    blockNumberMappingStorage.get(number)
 }
 
 object BlockchainReader {

src/main/scala/io/iohk/ethereum/domain/branch/BestBranch.scala

@@ -0,0 +1,35 @@
+package io.iohk.ethereum.domain.branch
+import akka.util.ByteString
+
+import io.iohk.ethereum.db.storage.BlockNumberMappingStorage
+import io.iohk.ethereum.domain.Block
+import io.iohk.ethereum.domain.BlockHeader
+import io.iohk.ethereum.domain.BlockchainReader
+
+/** A Branch instance which only works for the best canonical branch or a subset of this branch.
+  * This implementation uses the existing storage indexes to access blocks by number more efficiently.
+  */
+class BestBranch(
+    tipBlockHeader: BlockHeader,
+    bestChainBlockNumberMappingStorage: BlockNumberMappingStorage,
+    blockchainReader: BlockchainReader
+) extends Branch {
+
+  /* The following assumptions are made in this class :
+   *  - The whole branch exists in storage
+   *  - The various metadata and index are consistent
+   */
+
+  override def getBlockByNumber(number: BigInt): Option[Block] =
+    if (tipBlockHeader.number >= number && number >= 0) {
+      for {
+        hash <- getHashByBlockNumber(number)
+        block <- blockchainReader.getBlockByHash(hash)
+      } yield block
+    } else None
+
+  override def getHashByBlockNumber(number: BigInt): Option[ByteString] =
+    if (tipBlockHeader.number >= number && number >= 0) {
+      bestChainBlockNumberMappingStorage.get(number)
+    } else None
+}

src/main/scala/io/iohk/ethereum/domain/branch/Branch.scala

@@ -0,0 +1,16 @@
+package io.iohk.ethereum.domain.branch
+
+import akka.util.ByteString
+
+import io.iohk.ethereum.domain.Block
+
+/** An interface to manipulate blockchain branches */
+trait 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]
+
+}

src/main/scala/io/iohk/ethereum/domain/branch/EmptyBranch$.scala

@@ -0,0 +1,11 @@
+package io.iohk.ethereum.domain.branch
+import akka.util.ByteString
+
+import io.iohk.ethereum.domain.Block
+
+object EmptyBranch$ extends Branch {
+
+  override def getBlockByNumber(number: BigInt): Option[Block] = None
+
+  override def getHashByBlockNumber(number: BigInt): Option[ByteString] = None
+}

src/main/scala/io/iohk/ethereum/jsonrpc/CheckpointingService.scala

@@ -36,7 +36,7 @@ class CheckpointingService(
       req.parentCheckpoint.forall(blockchainReader.getBlockHeaderByHash(_).exists(_.number < blockToReturnNum))
 
     Task {
-      blockchainReader.getBlockByNumber(blockToReturnNum)
+      blockchainReader.getBestBranch().getBlockByNumber(blockToReturnNum)
     }.flatMap {
       case Some(b) if isValidParent =>
         Task.now(Right(GetLatestBlockResponse(Some(BlockInfo(b.hash, b.number)))))

src/main/scala/io/iohk/ethereum/jsonrpc/EthProofService.scala

@@ -205,6 +205,7 @@ class EthProofService(
   private def resolveBlock(blockParam: BlockParam): Either[JsonRpcError, ResolvedBlock] = {
     def getBlock(number: BigInt): Either[JsonRpcError, Block] =
       blockchainReader
+        .getBestBranch()
         .getBlockByNumber(number)
         .toRight(JsonRpcError.InvalidParams(s"Block $number not found"))
 

src/main/scala/io/iohk/ethereum/jsonrpc/EthTxService.scala

@@ -156,8 +156,9 @@ class EthTxService(
     val bestBlock = blockchainReader.getBestBlockNumber()
 
     Task {
+      val bestBranch = blockchainReader.getBestBranch()
       val gasPrice = ((bestBlock - blockDifference) to bestBlock)
-        .flatMap(blockchainReader.getBlockByNumber)
+        .flatMap(nb => bestBranch.getBlockByNumber(nb))
         .flatMap(_.body.transactionList)
         .map(_.tx.gasPrice)
       if (gasPrice.nonEmpty) {

src/main/scala/io/iohk/ethereum/jsonrpc/ResolveBlock.scala

@@ -34,6 +34,7 @@ trait ResolveBlock {
 
   private def getBlock(number: BigInt): Either[JsonRpcError, Block] =
     blockchainReader
+      .getBestBranch()
       .getBlockByNumber(number)
       .toRight(JsonRpcError.InvalidParams(s"Block $number not found"))
 

src/main/scala/io/iohk/ethereum/jsonrpc/TestService.scala

@@ -372,7 +372,7 @@ class TestService(
 
     val blockOpt = request.parameters.blockHashOrNumber
       .fold(
-        number => blockchainReader.getBlockByNumber(number),
+        number => blockchainReader.getBestBranch().getBlockByNumber(number),
         blockHash => blockchainReader.getBlockByHash(blockHash)
       )
 
@@ -411,7 +411,10 @@ class TestService(
   def storageRangeAt(request: StorageRangeRequest): ServiceResponse[StorageRangeResponse] = {
 
     val blockOpt = request.parameters.blockHashOrNumber
-      .fold(number => blockchainReader.getBlockByNumber(number), hash => blockchainReader.getBlockByHash(hash))
+      .fold(
+        number => blockchainReader.getBestBranch().getBlockByNumber(number),
+        hash => blockchainReader.getBlockByHash(hash)
+      )
 
     (for {
       block <- blockOpt.toRight(StorageRangeResponse(complete = false, Map.empty, None))

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

@@ -305,7 +305,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.getBlockByNumber(fromNumber) match {
+      blockchainReader.getBestBranch().getBlockByNumber(fromNumber) match {
         case Some(block) if block.header.hash == parent || fromNumber == 0 =>
           acc
 

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

@@ -41,7 +41,9 @@ class BlockValidation(
           val remaining = n - queuedBlocks.length - 1
 
           val numbers = (block.header.number - remaining) until block.header.number
-          val blocks = (numbers.toList.flatMap(blockchainReader.getBlockByNumber) :+ block) ::: queuedBlocks
+          val bestBranch = blockchainReader.getBestBranch()
+          val blocks =
+            (numbers.toList.flatMap(nb => bestBranch.getBlockByNumber(nb)) :+ block) ::: queuedBlocks
           blocks
       }
     }

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

@@ -72,10 +72,12 @@ class BranchResolution(blockchain: Blockchain, blockchainReader: BlockchainReade
     }
   }
 
-  private def getTopBlocksFromNumber(from: BigInt): List[Block] =
+  private def getTopBlocksFromNumber(from: BigInt): List[Block] = {
+    val bestBranch = blockchainReader.getBestBranch()
     (from to blockchainReader.getBestBlockNumber())
-      .flatMap(blockchainReader.getBlockByNumber)
+      .flatMap(nb => bestBranch.getBlockByNumber(nb))
       .toList
+  }
 }
 
 sealed trait BranchResolutionResult

src/main/scala/io/iohk/ethereum/testmode/TestEthBlockServiceWrapper.scala

@@ -75,7 +75,8 @@ class TestEthBlockServiceWrapper(
     .getBlockByNumber(request)
     .map(
       _.map { blockByBlockResponse =>
-        val fullBlock = blockchainReader.getBlockByNumber(blockByBlockResponse.blockResponse.get.number).get
+        val bestBranch = blockchainReader.getBestBranch()
+        val fullBlock = bestBranch.getBlockByNumber(blockByBlockResponse.blockResponse.get.number).get
         BlockByNumberResponse(blockByBlockResponse.blockResponse.map(response => toEthResponse(fullBlock, response)))
       }
     )

src/main/scala/io/iohk/ethereum/transactions/TransactionHistoryService.scala

@@ -33,7 +33,7 @@ class TransactionHistoryService(
     val getLastCheckpoint = Task(blockchain.getLatestCheckpointBlockNumber()).memoizeOnSuccess
     val txnsFromBlocks = Observable
       .from(fromBlocks.reverse)
-      .mapParallelOrdered(10)(blockNr => Task(blockchainReader.getBlockByNumber(blockNr)))(
+      .mapParallelOrdered(10)(blockNr => Task(blockchainReader.getBestBranch().getBlockByNumber(blockNr)))(
         OverflowStrategy.Unbounded
       )
       .collect { case Some(block) => block }

src/test/scala/io/iohk/ethereum/consensus/pow/validators/StdOmmersValidatorSpec.scala

@@ -79,7 +79,9 @@ class StdOmmersValidatorSpec extends AnyFlatSpec with Matchers with ScalaCheckPr
 
   it should "report failure if there is an ommer which that is not parent of an ancestor" in new BlockUtils {
     val getNBlocksBack: (ByteString, Int) => List[Block] =
-      (_, n) => ((ommersBlockNumber - n) until ommersBlockNumber).toList.flatMap(blockchainReader.getBlockByNumber)
+      (_, n) =>
+        ((ommersBlockNumber - n) until ommersBlockNumber).toList
+          .flatMap(nb => blockchainReader.getBestBranch().getBlockByNumber(nb))
 
     ommersValidator.validateOmmersAncestors(
       ommersBlockParentHash,
@@ -462,6 +464,7 @@ class StdOmmersValidatorSpec extends AnyFlatSpec with Matchers with ScalaCheckPr
       .and(blockchainWriter.storeBlock(block95))
       .and(blockchainWriter.storeBlock(block96))
       .commit()
+    blockchain.saveBestKnownBlocks(block96.number)
 
   }
 }

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

@@ -44,7 +44,8 @@ class BlockchainSpec extends AnyFlatSpec with Matchers with ScalaCheckPropertyCh
   it should "be able to store a block and retrieve it by number" in new EphemBlockchainTestSetup {
     val validBlock = Fixtures.Blocks.ValidBlock.block
     blockchainWriter.storeBlock(validBlock).commit()
-    val block = blockchainReader.getBlockByNumber(validBlock.header.number)
+    blockchain.saveBestKnownBlocks(validBlock.number)
+    val block = blockchainReader.getBestBranch().getBlockByNumber(validBlock.header.number)
     block.isDefined should ===(true)
     validBlock should ===(block.get)
   }
@@ -67,8 +68,10 @@ class BlockchainSpec extends AnyFlatSpec with Matchers with ScalaCheckPropertyCh
   }
 
   it should "not return a value if not stored" in new EphemBlockchainTestSetup {
-    blockchainReader.getBlockByNumber(Fixtures.Blocks.ValidBlock.header.number).isEmpty should ===(true)
-    blockchainReader.getBlockByHash(Fixtures.Blocks.ValidBlock.header.hash).isEmpty should ===(true)
+    blockchainReader
+      .getBestBranch()
+      .getBlockByNumber(Fixtures.Blocks.ValidBlock.header.number) shouldBe None
+    blockchainReader.getBlockByHash(Fixtures.Blocks.ValidBlock.header.hash) shouldBe None
   }
 
   it should "be able to store a block with checkpoint and retrieve it and checkpoint" in new EphemBlockchainTestSetup {