move getBestBlockNumber and getBestBlock to blockchainReader

Author: Aurélien Richez

src/main/scala/io/iohk/ethereum/blockchain/sync/SyncController.scala

@@ -120,6 +120,7 @@ class SyncController(
         peerEventBus,
         blockImport,
         blockchain,
+        blockchainReader,
         new BranchResolution(blockchain, blockchainReader),
         validators.blockValidator,
         blacklist,

src/main/scala/io/iohk/ethereum/blockchain/sync/fast/FastSyncBranchResolver.scala

@@ -17,7 +17,7 @@ trait FastSyncBranchResolver {
 
   // TODO [ETCM-676] move to [[Blockchain]] and make sure it's atomic
   def discardBlocksAfter(lastValidBlock: BigInt): Unit =
-    discardBlocks(lastValidBlock, blockchain.getBestBlockNumber())
+    discardBlocks(lastValidBlock, blockchainReader.getBestBlockNumber())
 
   // TODO [ETCM-676] move to [[Blockchain]] and make sure it's atomic
   private def discardBlocks(fromBlock: BigInt, toBlock: BigInt): Unit = {

src/main/scala/io/iohk/ethereum/blockchain/sync/fast/FastSyncBranchResolverActor.scala

@@ -59,9 +59,9 @@ class FastSyncBranchResolverActor(
         log.debug(
           "Starting branch resolution now with peer {} and block number {}",
           peerWithInfo,
-          blockchain.getBestBlockNumber()
+          blockchainReader.getBestBlockNumber()
         )
-        requestRecentBlockHeaders(peer, blockchain.getBestBlockNumber())
+        requestRecentBlockHeaders(peer, blockchainReader.getBestBlockNumber())
       case None =>
         log.info("Waiting for peers, rescheduling StartBranchResolver")
         timers.startSingleTimer(RestartTimerKey, StartBranchResolver, 1.second)

src/main/scala/io/iohk/ethereum/blockchain/sync/regular/BlockImporter.scala

@@ -37,6 +37,7 @@ class BlockImporter(
     fetcher: ActorRef,
     blockImport: BlockImport,
     blockchain: Blockchain,
+    blockchainReader: BlockchainReader,
     branchResolution: BranchResolution,
     syncConfig: SyncConfig,
     ommersPool: ActorRef,
@@ -311,7 +312,7 @@ class BlockImporter(
         Right(Nil)
     }
 
-  private def bestKnownBlockNumber: BigInt = blockchain.getBestBlockNumber()
+  private def bestKnownBlockNumber: BigInt = blockchainReader.getBestBlockNumber()
 
   private def getBehavior(newBehavior: NewBehavior, blockImportType: BlockImportType): Behavior = newBehavior match {
     case Running                             => running
@@ -326,6 +327,7 @@ object BlockImporter {
       fetcher: ActorRef,
       blockImport: BlockImport,
       blockchain: Blockchain,
+      blockchainReader: BlockchainReader,
       branchResolution: BranchResolution,
       syncConfig: SyncConfig,
       ommersPool: ActorRef,
@@ -338,6 +340,7 @@ object BlockImporter {
         fetcher,
         blockImport,
         blockchain,
+        blockchainReader,
         branchResolution,
         syncConfig,
         ommersPool,

src/main/scala/io/iohk/ethereum/blockchain/sync/regular/RegularSync.scala

@@ -10,7 +10,6 @@ import akka.actor.Scheduler
 import akka.actor.SupervisorStrategy
 import akka.actor.typed.scaladsl.adapter._
 import akka.actor.typed.{ActorRef => TypedActorRef}
-
 import io.iohk.ethereum.blockchain.sync.Blacklist
 import io.iohk.ethereum.blockchain.sync.SyncProtocol
 import io.iohk.ethereum.blockchain.sync.SyncProtocol.Status
@@ -20,10 +19,8 @@ import io.iohk.ethereum.blockchain.sync.regular.RegularSync.NewCheckpoint
 import io.iohk.ethereum.blockchain.sync.regular.RegularSync.ProgressProtocol
 import io.iohk.ethereum.blockchain.sync.regular.RegularSync.ProgressState
 import io.iohk.ethereum.consensus.validators.BlockValidator
-import io.iohk.ethereum.domain.Block
-import io.iohk.ethereum.domain.Blockchain
-import io.iohk.ethereum.ledger.BlockImport
-import io.iohk.ethereum.ledger.BranchResolution
+import io.iohk.ethereum.domain.{Block, Blockchain, BlockchainReader}
+import io.iohk.ethereum.ledger.{BlockImport, BranchResolution}
 import io.iohk.ethereum.utils.ByteStringUtils
 import io.iohk.ethereum.utils.Config.SyncConfig
 
@@ -33,6 +30,7 @@ class RegularSync(
     peerEventBus: ActorRef,
     blockImport: BlockImport,
     blockchain: Blockchain,
+    blockchainReader: BlockchainReader,
     branchResolution: BranchResolution,
     blockValidator: BlockValidator,
     blacklist: Blacklist,
@@ -62,6 +60,7 @@ class RegularSync(
         fetcher.toClassic,
         blockImport,
         blockchain,
+        blockchainReader,
         branchResolution,
         syncConfig,
         ommersPool,
@@ -133,6 +132,7 @@ object RegularSync {
       peerEventBus: ActorRef,
       blockImport: BlockImport,
       blockchain: Blockchain,
+      blockchainReader: BlockchainReader,
       branchResolution: BranchResolution,
       blockValidator: BlockValidator,
       blacklist: Blacklist,
@@ -148,6 +148,7 @@ object RegularSync {
         peerEventBus,
         blockImport,
         blockchain,
+        blockchainReader,
         branchResolution,
         blockValidator,
         blacklist,

src/main/scala/io/iohk/ethereum/consensus/pow/PoWConsensus.scala

@@ -105,7 +105,7 @@ class PoWConsensus private (
                   node.syncController,
                   node.ethMiningService,
                   blockCreator,
-                  blockchain,
+                  blockchainReader,
                   blockchainConfig.forkBlockNumbers.ecip1049BlockNumber
                 ),
                 "PoWMinerCoordinator",

src/main/scala/io/iohk/ethereum/consensus/pow/PoWMiningCoordinator.scala

@@ -12,12 +12,8 @@ import monix.execution.Scheduler
 import scala.concurrent.duration.DurationInt
 
 import io.iohk.ethereum.consensus.pow.PoWMiningCoordinator.CoordinatorProtocol
-import io.iohk.ethereum.consensus.pow.miners.EthashDAGManager
-import io.iohk.ethereum.consensus.pow.miners.EthashMiner
-import io.iohk.ethereum.consensus.pow.miners.KeccakMiner
-import io.iohk.ethereum.consensus.pow.miners.Miner
-import io.iohk.ethereum.domain.Block
-import io.iohk.ethereum.domain.Blockchain
+import io.iohk.ethereum.consensus.pow.miners.{EthashDAGManager, EthashMiner, KeccakMiner, Miner}
+import io.iohk.ethereum.domain.{Block, Blockchain, BlockchainReader}
 import io.iohk.ethereum.jsonrpc.EthMiningService
 
 object PoWMiningCoordinator {
@@ -53,7 +49,7 @@ object PoWMiningCoordinator {
       syncController: ClassicActorRef,
       ethMiningService: EthMiningService,
       blockCreator: PoWBlockCreator,
-      blockchain: Blockchain,
+      blockchainReader: BlockchainReader,
       ecip1049BlockNumber: Option[BigInt]
   ): Behavior[CoordinatorProtocol] =
     Behaviors
@@ -63,7 +59,7 @@ object PoWMiningCoordinator {
           syncController,
           ethMiningService,
           blockCreator,
-          blockchain,
+          blockchainReader,
           ecip1049BlockNumber
         )
       )
@@ -74,7 +70,7 @@ class PoWMiningCoordinator private (
     syncController: ClassicActorRef,
     ethMiningService: EthMiningService,
     blockCreator: PoWBlockCreator,
-    blockchain: Blockchain,
+    blockchainReader: BlockchainReader,
     ecip1049BlockNumber: Option[BigInt]
 ) extends AbstractBehavior[CoordinatorProtocol](context) {
 
@@ -98,7 +94,7 @@ class PoWMiningCoordinator private (
 
     case MineNext =>
       log.debug("Received message MineNext")
-      blockchain
+      blockchainReader
         .getBestBlock()
         .fold {
           log.error("Unable to get block for mining: blockchain.getBestBlock() returned None")

src/main/scala/io/iohk/ethereum/consensus/pow/miners/MockedMiner.scala

@@ -60,7 +60,7 @@ class MockedMiner(
               sender() ! MiningError(error)
           }
         case None =>
-          val parentBlock = blockchain.getBestBlock()
+          val parentBlock = blockchainReader.getBestBlock()
           startMiningBlocks(mineBlocks, parentBlock.get)
       }
   }

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

@@ -74,10 +74,6 @@ trait Blockchain {
     */
   def getChainWeightByHash(blockhash: ByteString): Option[ChainWeight]
 
-  def getBestBlockNumber(): BigInt
-
-  def getBestBlock(): Option[Block]
-
   def getLatestCheckpointBlockNumber(): BigInt
 
   def removeBlock(hash: ByteString, withState: Boolean): Unit
@@ -114,42 +110,15 @@ class BlockchainImpl(
 
   override def isInChain(hash: ByteString): Boolean =
     (for {
-      header <- blockchainReader.getBlockHeaderByHash(hash) if header.number <= getBestBlockNumber()
+      header <- blockchainReader.getBlockHeaderByHash(hash) if header.number <= blockchainReader.getBestBlockNumber()
       hash <- blockchainReader.getHashByBlockNumber(header.number)
     } yield header.hash == hash).getOrElse(false)
 
   override def getChainWeightByHash(blockhash: ByteString): Option[ChainWeight] = chainWeightStorage.get(blockhash)
 
-  override def getBestBlockNumber(): BigInt = {
-    val bestSavedBlockNumber = appStateStorage.getBestBlockNumber()
-    val bestKnownBlockNumber = blockchainMetadata.bestKnownBlockAndLatestCheckpoint.get().bestBlockNumber
-    log.debug(
-      "Current best saved block number {}. Current best known block number {}",
-      bestSavedBlockNumber,
-      bestKnownBlockNumber
-    )
-
-    // The cached best block number should always be more up-to-date than the one on disk, we are keeping access to disk
-    // above only for logging purposes
-    bestKnownBlockNumber
-  }
-
   override def getLatestCheckpointBlockNumber(): BigInt =
     blockchainMetadata.bestKnownBlockAndLatestCheckpoint.get().latestCheckpointNumber
 
-  //returns the best known block if it's available in the storage, otherwise the best stored block
-  override def getBestBlock(): Option[Block] = {
-    val bestBlockNumber = getBestBlockNumber()
-    log.debug("Trying to get best block with number {}", bestBlockNumber)
-    blockchainReader
-      .getBlockByNumber(bestBlockNumber)
-      .orElse(
-        blockchainReader.getBlockByNumber(
-          appStateStorage.getBestBlockNumber()
-        )
-      )
-  }
-
   override def getAccount(address: Address, blockNumber: BigInt): Option[Account] =
     getAccountMpt(blockNumber) >>= (_.get(address))
 
@@ -207,7 +176,7 @@ class BlockchainImpl(
   def getReadOnlyMptStorage(): MptStorage = stateStorage.getReadOnlyStorage
 
   private def persistBestBlocksData(): Unit = {
-    val currentBestBlockNumber = getBestBlockNumber()
+    val currentBestBlockNumber = blockchainReader.getBestBlockNumber()
     val currentBestCheckpointNumber = getLatestCheckpointBlockNumber()
     log.debug(
       "Persisting app info data into database. Persisted block number is {}. " +
@@ -265,7 +234,7 @@ class BlockchainImpl(
     log.debug(s"Trying to remove block ${block.idTag}")
 
     val txList = block.body.transactionList
-    val bestBlockNumber = getBestBlockNumber()
+    val bestBlockNumber = blockchainReader.getBestBlockNumber()
     val latestCheckpointNumber = getLatestCheckpointBlockNumber()
 
     val blockNumberMappingUpdates =

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

@@ -1,21 +1,26 @@
 package io.iohk.ethereum.domain
 
 import akka.util.ByteString
-
-import io.iohk.ethereum.db.storage.BlockBodiesStorage
-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.db.storage.{
+  AppStateStorage,
+  BlockBodiesStorage,
+  BlockHeadersStorage,
+  BlockNumberMappingStorage,
+  ReceiptStorage,
+  StateStorage
+}
 import io.iohk.ethereum.mpt.MptNode
+import io.iohk.ethereum.utils.Logger
 
 class BlockchainReader(
     blockHeadersStorage: BlockHeadersStorage,
     blockBodiesStorage: BlockBodiesStorage,
     blockNumberMappingStorage: BlockNumberMappingStorage,
     stateStorage: StateStorage,
-    receiptStorage: ReceiptStorage
-) {
+    receiptStorage: ReceiptStorage,
+    appStateStorage: AppStateStorage,
+    blockchainMetadata: BlockchainMetadata
+) extends Logger {
 
   /** Allows to query a blockHeader by block hash
     *
@@ -81,16 +86,46 @@ class BlockchainReader(
     * @return Receipts if found
     */
   def getReceiptsByHash(blockhash: ByteString): Option[Seq[Receipt]] = receiptStorage.get(blockhash)
+
+  def getBestBlockNumber(): BigInt = {
+    val bestSavedBlockNumber = appStateStorage.getBestBlockNumber()
+    val bestKnownBlockNumber = blockchainMetadata.bestKnownBlockAndLatestCheckpoint.get().bestBlockNumber
+    log.debug(
+      "Current best saved block number {}. Current best known block number {}",
+      bestSavedBlockNumber,
+      bestKnownBlockNumber
+    )
+
+    // The cached best block number should always be more up-to-date than the one on disk, we are keeping access to disk
+    // above only for logging purposes
+    bestKnownBlockNumber
+  }
+
+  //returns the best known block if it's available in the storage, otherwise the best stored block
+  def getBestBlock(): Option[Block] = {
+    val bestBlockNumber = getBestBlockNumber()
+    log.debug("Trying to get best block with number {}", bestBlockNumber)
+    getBlockByNumber(bestBlockNumber).orElse(
+      getBlockByNumber(
+        appStateStorage.getBestBlockNumber()
+      )
+    )
+  }
 }
 
 object BlockchainReader {
 
-  def apply(storages: BlockchainStorages): BlockchainReader = new BlockchainReader(
+  def apply(
+      storages: BlockchainStorages,
+      blockchainMetadata: BlockchainMetadata
+  ): BlockchainReader = new BlockchainReader(
     storages.blockHeadersStorage,
     storages.blockBodiesStorage,
     storages.blockNumberMappingStorage,
     storages.stateStorage,
-    storages.receiptStorage
+    storages.receiptStorage,
+    storages.appStateStorage,
+    blockchainMetadata
   )
 
 }

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

@@ -27,7 +27,7 @@ class CheckpointingService(
   import CheckpointingService._
 
   def getLatestBlock(req: GetLatestBlockRequest): ServiceResponse[GetLatestBlockResponse] = {
-    lazy val bestBlockNum = blockchain.getBestBlockNumber()
+    lazy val bestBlockNum = blockchainReader.getBestBlockNumber()
     lazy val blockToReturnNum =
       if (req.checkpointingInterval != 0)
         bestBlockNum - bestBlockNum % req.checkpointingInterval

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

@@ -53,7 +53,7 @@ class EthBlocksService(
     * @return Current block number the client is on.
     */
   def bestBlockNumber(req: BestBlockNumberRequest): ServiceResponse[BestBlockNumberResponse] = Task {
-    Right(BestBlockNumberResponse(blockchain.getBestBlockNumber()))
+    Right(BestBlockNumberResponse(blockchainReader.getBestBlockNumber()))
   }
 
   /** Implements the eth_getBlockTransactionCountByHash method that fetches the number of txs that a certain block has.

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

@@ -20,9 +20,7 @@ import io.iohk.ethereum.consensus.ConsensusConfig
 import io.iohk.ethereum.consensus.blocks.PendingBlockAndState
 import io.iohk.ethereum.consensus.pow.EthashUtils
 import io.iohk.ethereum.crypto.kec256
-import io.iohk.ethereum.domain.Address
-import io.iohk.ethereum.domain.BlockHeader
-import io.iohk.ethereum.domain.Blockchain
+import io.iohk.ethereum.domain.{Address, BlockHeader, BlockchainReader}
 import io.iohk.ethereum.jsonrpc.AkkaTaskOps._
 import io.iohk.ethereum.jsonrpc.server.controllers.JsonRpcBaseController.JsonRpcConfig
 import io.iohk.ethereum.ommers.OmmersPool
@@ -51,7 +49,7 @@ object EthMiningService {
 }
 
 class EthMiningService(
-    blockchain: Blockchain,
+    blockchainReader: BlockchainReader,
     blockchainConfig: BlockchainConfig,
     consensus: Consensus,
     jsonRpcConfig: JsonRpcConfig,
@@ -83,7 +81,7 @@ class EthMiningService(
   def getWork(req: GetWorkRequest): ServiceResponse[GetWorkResponse] =
     consensus.ifEthash { ethash =>
       reportActive()
-      blockchain.getBestBlock() match {
+      blockchainReader.getBestBlock() match {
         case Some(block) =>
           Task.parZip2(getOmmersFromPool(block.hash), getTransactionsFromPool).map { case (ommers, pendingTxs) =>
             val blockGenerator = ethash.blockGenerator
@@ -114,7 +112,7 @@ class EthMiningService(
       reportActive()
       Task {
         ethash.blockGenerator.getPrepared(req.powHeaderHash) match {
-          case Some(pendingBlock) if blockchain.getBestBlockNumber() <= pendingBlock.block.header.number =>
+          case Some(pendingBlock) if blockchainReader.getBestBlockNumber() <= pendingBlock.block.header.number =>
             import pendingBlock._
             syncingController ! SyncProtocol.MinedBlock(
               block.copy(header = block.header.copy(nonce = req.nonce, mixHash = req.mixHash))