[ETCM-1048] Remove bestKnownBlockAndLatestCheckpoint cache

Author: Leonor Boga

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

@@ -39,7 +39,6 @@ import io.iohk.ethereum.db.storage.pruning.PruningMode
 import io.iohk.ethereum.domain.Block
 import io.iohk.ethereum.domain.Blockchain
 import io.iohk.ethereum.domain.BlockchainImpl
-import io.iohk.ethereum.domain.BlockchainMetadata
 import io.iohk.ethereum.domain.BlockchainReader
 import io.iohk.ethereum.domain.BlockchainWriter
 import io.iohk.ethereum.domain.ChainWeight
@@ -138,13 +137,9 @@ abstract class CommonFakePeer(peerName: String, fakePeerCustomConfig: FakePeerCu
     )
   )
 
-  val blockchainMetadata = new BlockchainMetadata(
-    storagesInstance.storages.appStateStorage.getBestBlockNumber(),
-    storagesInstance.storages.appStateStorage.getLatestCheckpointBlockNumber()
-  )
-  val blockchainReader: BlockchainReader = BlockchainReader(storagesInstance.storages, blockchainMetadata)
-  val blockchainWriter: BlockchainWriter = BlockchainWriter(storagesInstance.storages, blockchainMetadata)
-  val bl: BlockchainImpl = BlockchainImpl(storagesInstance.storages, blockchainReader, blockchainMetadata)
+  val blockchainReader: BlockchainReader = BlockchainReader(storagesInstance.storages)
+  val blockchainWriter: BlockchainWriter = BlockchainWriter(storagesInstance.storages)
+  val bl: BlockchainImpl = BlockchainImpl(storagesInstance.storages, blockchainReader)
   val evmCodeStorage = storagesInstance.storages.evmCodeStorage
 
   val genesis: Block = Block(

src/it/scala/io/iohk/ethereum/txExecTest/ECIP1017Test.scala

@@ -80,10 +80,9 @@ class ECIP1017Test extends AnyFlatSpec with Matchers {
 
     (startBlock to endBlock).foreach { blockToExecute =>
       val storages = FixtureProvider.prepareStorages(blockToExecute - 1, fixtures)
-      val blockchainMetadata = getNewBlockchainMetadata
-      val blockchainReader = BlockchainReader(storages, blockchainMetadata)
-      val blockchainWriter = BlockchainWriter(storages, blockchainMetadata)
-      val blockchain = BlockchainImpl(storages, blockchainReader, blockchainMetadata)
+      val blockchainReader = BlockchainReader(storages)
+      val blockchainWriter = BlockchainWriter(storages)
+      val blockchain = BlockchainImpl(storages, blockchainReader)
       val blockValidation =
         new BlockValidation(mining, blockchainReader, BlockQueue(blockchain, blockchainReader, syncConfig))
       val blockExecution =

src/it/scala/io/iohk/ethereum/txExecTest/ForksTest.scala

@@ -56,10 +56,9 @@ class ForksTest extends AnyFlatSpec with Matchers {
 
     (startBlock to endBlock).foreach { blockToExecute =>
       val storages = FixtureProvider.prepareStorages(blockToExecute - 1, fixtures)
-      val blockchainMetadata = getNewBlockchainMetadata
-      val blockchainReader = BlockchainReader(storages, blockchainMetadata)
-      val blockchainWriter = BlockchainWriter(storages, blockchainMetadata)
-      val blockchain = BlockchainImpl(storages, blockchainReader, blockchainMetadata)
+      val blockchainReader = BlockchainReader(storages)
+      val blockchainWriter = BlockchainWriter(storages)
+      val blockchain = BlockchainImpl(storages, blockchainReader)
       val blockValidation =
         new BlockValidation(mining, blockchainReader, BlockQueue(blockchain, blockchainReader, syncConfig))
       val blockExecution =

src/it/scala/io/iohk/ethereum/txExecTest/ScenarioSetup.scala

@@ -10,10 +10,8 @@ import io.iohk.ethereum.ledger.VMImpl
 trait ScenarioSetup extends EphemBlockchainTestSetup {
   protected val testBlockchainStorages: BlockchainStorages
 
-  val blockchainMetadata = getNewBlockchainMetadata
-  override lazy val blockchainReader: BlockchainReader = BlockchainReader(testBlockchainStorages, blockchainMetadata)
-  override lazy val blockchainWriter: BlockchainWriter = BlockchainWriter(testBlockchainStorages, blockchainMetadata)
-  override lazy val blockchain: BlockchainImpl =
-    BlockchainImpl(testBlockchainStorages, blockchainReader, blockchainMetadata)
+  override lazy val blockchainReader: BlockchainReader = BlockchainReader(testBlockchainStorages)
+  override lazy val blockchainWriter: BlockchainWriter = BlockchainWriter(testBlockchainStorages)
+  override lazy val blockchain: BlockchainImpl = BlockchainImpl(testBlockchainStorages, blockchainReader)
   override lazy val vm: VMImpl = new VMImpl
 }

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

@@ -184,12 +184,10 @@ class BlockchainMock(genesisHash: ByteString) extends Blockchain {
       ethCompatibleStorage: Boolean
   ): StorageProof = EmptyStorageValueProof(StorageProofKey(position))
 
-  override def removeBlock(hash: ByteString, withState: Boolean = true): Unit = ???
+  override def removeBlock(hash: ByteString): Unit = ???
 
   override def getChainWeightByHash(blockhash: ByteString): Option[ChainWeight] = ???
 
-  def getAccount(address: Address, blockNumber: BigInt): Option[Account] = ???
-
   override def getAccountStorageAt(rootHash: ByteString, position: BigInt, ethCompatibleStorage: Boolean): ByteString =
     ???
 
@@ -198,8 +196,6 @@ class BlockchainMock(genesisHash: ByteString) extends Blockchain {
 
   def getBestBlockNumber(): BigInt = ???
 
-  def saveBlockNumber(number: BigInt, hash: NodeHash): Unit = ???
-
   def saveBestKnownBlocks(bestBlockNumber: BigInt, latestCheckpointNumber: Option[BigInt] = None): Unit = ???
 
   def getBestBlock(): Option[Block] = ???

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

@@ -70,7 +70,7 @@ class SyncController(
   def start(): Unit = {
     import syncConfig.doFastSync
 
-    appStateStorage.putSyncStartingBlock(appStateStorage.getBestBlockNumber())
+    appStateStorage.putSyncStartingBlock(appStateStorage.getBestBlockNumber()).commit
     (appStateStorage.isFastSyncDone(), doFastSync) match {
       case (false, true) =>
         startFastSync()

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

@@ -567,16 +567,13 @@ class FastSync(
     }
 
     // TODO [ETCM-676]: Move to blockchain and make sure it's atomic
-    private def discardLastBlocks(startBlock: BigInt, blocksToDiscard: Int): Unit = {
+    private def discardLastBlocks(startBlock: BigInt, blocksToDiscard: Int): Unit =
       // TODO (maybe ETCM-77): Manage last checkpoint number too
-      appStateStorage.putBestBlockNumber((startBlock - blocksToDiscard - 1).max(0)).commit()
-
       (startBlock to ((startBlock - blocksToDiscard).max(1)) by -1).foreach { n =>
         blockchainReader.getBlockHeaderByNumber(n).foreach { headerToRemove =>
-          blockchain.removeBlock(headerToRemove.hash, withState = false)
+          blockchain.removeBlock(headerToRemove.hash)
         }
       }
-    }
 
     private def validateHeader(header: BlockHeader, peer: Peer): Either[HeaderProcessingResult, BlockHeader] = {
       val shouldValidate = header.number >= syncState.nextBlockToFullyValidate
@@ -790,7 +787,7 @@ class FastSync(
             |Peers waiting_for_response/connected: ${assignedHandlers.size}/${handshakedPeers.size} (${blacklistedIds.size} blacklisted).
             |State: ${syncState.downloadedNodesCount}/${syncState.totalNodesCount} nodes.
             |""".stripMargin.replace("\n", " "),
-        appStateStorage.getBestBlockNumber()
+        blockchainReader.getBestBlockNumber()
       )
       log.debug(
         s"""|Connection status: connected({})/
@@ -1136,9 +1133,8 @@ class FastSync(
 
       if (fullBlocks.nonEmpty) {
         val bestReceivedBlock = fullBlocks.maxBy(_.number)
-        val lastStoredBestBlockNumber = appStateStorage.getBestBlockNumber()
+        val lastStoredBestBlockNumber = blockchainReader.getBestBlockNumber()
         if (lastStoredBestBlockNumber < bestReceivedBlock.number) {
-          blockchain.saveBestKnownBlocks(bestReceivedBlock.number)
           // TODO ETCM-1089 move direct calls to storages to blockchain or blockchain writer
           appStateStorage
             .putBestBlockNumber(bestReceivedBlock.number)

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

@@ -25,7 +25,7 @@ trait FastSyncBranchResolver {
     blocksToBeRemoved.foreach { toBeRemoved =>
       blockchainReader
         .getBlockHeaderByNumber(toBeRemoved)
-        .foreach(header => blockchain.removeBlock(header.hash, withState = false))
+        .foreach(header => blockchain.removeBlock(header.hash))
     }
   }
 

src/main/scala/io/iohk/ethereum/consensus/ConsensusImpl.scala

@@ -333,7 +333,7 @@ class ConsensusImpl(
             weight <- blockchain.getChainWeightByHash(hash)
           } yield BlockData(block, receipts, weight)
 
-          blockchain.removeBlock(hash, withState = true)
+          blockchain.removeBlock(hash)
 
           removeBlocksUntil(parent, fromNumber - 1, blockDataOpt.map(_ :: acc).getOrElse(acc))
 

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

@@ -62,7 +62,7 @@ trait Blockchain {
 
   def getLatestCheckpointBlockNumber(): BigInt
 
-  def removeBlock(hash: ByteString, withState: Boolean): Unit
+  def removeBlock(hash: ByteString): Unit
 
   def saveBestKnownBlocks(bestBlockNumber: BigInt, latestCheckpointNumber: Option[BigInt] = None): Unit
 
@@ -77,15 +77,13 @@ class BlockchainImpl(
     protected val transactionMappingStorage: TransactionMappingStorage,
     protected val appStateStorage: AppStateStorage,
     protected val stateStorage: StateStorage,
-    blockchainReader: BlockchainReader,
-    blockchainMetadata: BlockchainMetadata
+    blockchainReader: BlockchainReader
 ) extends Blockchain
     with Logger {
 
   override def getChainWeightByHash(blockhash: ByteString): Option[ChainWeight] = chainWeightStorage.get(blockhash)
 
-  override def getLatestCheckpointBlockNumber(): BigInt =
-    blockchainMetadata.bestKnownBlockAndLatestCheckpoint.get().latestCheckpointNumber
+  override def getLatestCheckpointBlockNumber(): BigInt = appStateStorage.getLatestCheckpointBlockNumber()
 
   override def getAccountStorageAt(
       rootHash: ByteString,
@@ -128,22 +126,6 @@ class BlockchainImpl(
 
   def getReadOnlyMptStorage(): MptStorage = stateStorage.getReadOnlyStorage
 
-  private def persistBestBlocksData(): Unit = {
-    val currentBestBlockNumber = blockchainReader.getBestBlockNumber()
-    val currentBestCheckpointNumber = getLatestCheckpointBlockNumber()
-    log.debug(
-      "Persisting app info data into database. Persisted block number is {}. " +
-        "Persisted checkpoint number is {}",
-      currentBestBlockNumber,
-      currentBestCheckpointNumber
-    )
-
-    appStateStorage
-      .putBestBlockNumber(currentBestBlockNumber)
-      .and(appStateStorage.putLatestCheckpointBlockNumber(currentBestCheckpointNumber))
-      .commit()
-  }
-
   override def saveBestKnownBlocks(bestBlockNumber: BigInt, latestCheckpointNumber: Option[BigInt] = None): Unit =
     latestCheckpointNumber match {
       case Some(number) =>
@@ -153,28 +135,29 @@ class BlockchainImpl(
     }
 
   private def saveBestKnownBlock(bestBlockNumber: BigInt): Unit =
-    blockchainMetadata.bestKnownBlockAndLatestCheckpoint.updateAndGet(_.copy(bestBlockNumber = bestBlockNumber))
+    appStateStorage.putBestBlockNumber(bestBlockNumber).commit()
 
   private def saveBestKnownBlockAndLatestCheckpointNumber(number: BigInt, latestCheckpointNumber: BigInt): Unit =
-    blockchainMetadata.bestKnownBlockAndLatestCheckpoint.set(
-      BestBlockLatestCheckpointNumbers(number, latestCheckpointNumber)
-    )
+    appStateStorage
+      .putBestBlockNumber(number)
+      .and(appStateStorage.putLatestCheckpointBlockNumber(latestCheckpointNumber))
+      .commit()
 
   private def removeBlockNumberMapping(number: BigInt): DataSourceBatchUpdate =
     blockNumberMappingStorage.remove(number)
 
-  override def removeBlock(blockHash: ByteString, withState: Boolean): Unit = {
+  override def removeBlock(blockHash: ByteString): Unit = {
     val maybeBlock = blockchainReader.getBlockByHash(blockHash)
 
     maybeBlock match {
-      case Some(block) => removeBlock(block, withState)
+      case Some(block) => removeBlock(block)
       case None =>
         log.warn(s"Attempted removing block with hash ${ByteStringUtils.hash2string(blockHash)} that we don't have")
     }
   }
 
   // scalastyle:off method.length
-  private def removeBlock(block: Block, withState: Boolean): Unit = {
+  private def removeBlock(block: Block): Unit = {
     val blockHash = block.hash
 
     log.debug(s"Trying to remove block ${block.idTag}")
@@ -229,17 +212,12 @@ class BlockchainImpl(
       .and(latestCheckpointNumberUpdates)
       .commit()
 
-    saveBestKnownBlocks(newBestBlockNumber, Some(newLatestCheckpointNumber))
     log.debug(
       "Removed block with hash {}. New best block number - {}, new best checkpoint block number - {}",
       ByteStringUtils.hash2string(blockHash),
       newBestBlockNumber,
       newLatestCheckpointNumber
     )
-
-    // not transactional part
-    if (withState)
-      stateStorage.onBlockRollback(block.number, bestBlockNumber)(() => persistBestBlocksData())
   }
   // scalastyle:on method.length
 
@@ -288,8 +266,7 @@ trait BlockchainStorages {
 object BlockchainImpl {
   def apply(
       storages: BlockchainStorages,
-      blockchainReader: BlockchainReader,
-      metadata: BlockchainMetadata
+      blockchainReader: BlockchainReader
   ): BlockchainImpl =
     new BlockchainImpl(
       blockHeadersStorage = storages.blockHeadersStorage,
@@ -300,7 +277,6 @@ object BlockchainImpl {
       transactionMappingStorage = storages.transactionMappingStorage,
       appStateStorage = storages.appStateStorage,
       stateStorage = storages.stateStorage,
-      blockchainReader = blockchainReader,
-      blockchainMetadata = metadata
+      blockchainReader = blockchainReader
     )
 }

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

@@ -21,8 +21,7 @@ class BlockchainReader(
     blockNumberMappingStorage: BlockNumberMappingStorage,
     stateStorage: StateStorage,
     receiptStorage: ReceiptStorage,
-    appStateStorage: AppStateStorage,
-    blockchainMetadata: BlockchainMetadata
+    appStateStorage: AppStateStorage
 ) extends Logger {
 
   /** Allows to query a blockHeader by block hash
@@ -81,29 +80,13 @@ class BlockchainReader(
       .getOrElse(EmptyBranch)
   }
 
-  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
-  }
+  def getBestBlockNumber(): BigInt = appStateStorage.getBestBlockNumber()
 
-  //returns the best known block if it's available in the storage, otherwise the best stored block
+  //returns the best known block if it's available in the storage
   def getBestBlock(): Option[Block] = {
     val bestBlockNumber = getBestBlockNumber()
     log.debug("Trying to get best block with number {}", bestBlockNumber)
-    getBlockByNumber(bestBlockNumber).orElse(
-      getBlockByNumber(
-        appStateStorage.getBestBlockNumber()
-      )
-    )
+    getBlockByNumber(bestBlockNumber)
   }
 
   def genesisHeader: BlockHeader =
@@ -199,16 +182,14 @@ class BlockchainReader(
 object BlockchainReader {
 
   def apply(
-      storages: BlockchainStorages,
-      blockchainMetadata: BlockchainMetadata
+      storages: BlockchainStorages
   ): BlockchainReader = new BlockchainReader(
     storages.blockHeadersStorage,
     storages.blockBodiesStorage,
     storages.blockNumberMappingStorage,
     storages.stateStorage,
     storages.receiptStorage,
-    storages.appStateStorage,
-    blockchainMetadata
+    storages.appStateStorage
   )
 
 }