use getBlockByHash to get the best block number instead of getBlockByNumber

Author: Aurélien Richez

bytes/src/main/scala/io/iohk/ethereum/utils/Hex.scala

@@ -1,7 +1,5 @@
 package io.iohk.ethereum.utils
 
-import akka.util.ByteString
-
 object Hex {
   def toHexString(bytes: Array[Byte]): String =
     bytes.map("%02x".format(_)).mkString

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

@@ -139,7 +139,7 @@ abstract class CommonFakePeer(peerName: String, fakePeerCustomConfig: FakePeerCu
   )
 
   val blockchainMetadata = new BlockchainMetadata(
-    storagesInstance.storages.appStateStorage.getBestBlockData(),
+    storagesInstance.storages.appStateStorage.getBestBlockInfo(),
     storagesInstance.storages.appStateStorage.getLatestCheckpointBlockNumber()
   )
   val blockchainReader: BlockchainReader = BlockchainReader(storagesInstance.storages, blockchainMetadata)

src/main/scala/io/iohk/ethereum/db/storage/AppStateStorage.scala

@@ -1,17 +1,21 @@
 package io.iohk.ethereum.db.storage
 
+import java.math.BigInteger
+
 import akka.util.ByteString
+
+import scala.collection.immutable.ArraySeq
+
 import boopickle.Default.Unpickle
-import boopickle.Pickler
 import boopickle.DefaultBasic._
+import boopickle.Pickler
 
-import java.math.BigInteger
-import scala.collection.immutable.ArraySeq
 import io.iohk.ethereum.db.dataSource.DataSource
 import io.iohk.ethereum.db.dataSource.DataSourceBatchUpdate
 import io.iohk.ethereum.db.storage.AppStateStorage._
 import io.iohk.ethereum.domain.appstate.BestBlockInfo
-import io.iohk.ethereum.utils.ByteUtils.{byteSequenceToBuffer, compactPickledBytes}
+import io.iohk.ethereum.utils.ByteUtils.byteSequenceToBuffer
+import io.iohk.ethereum.utils.ByteUtils.compactPickledBytes
 import io.iohk.ethereum.utils.Hex
 import io.iohk.ethereum.utils.Picklers._
 
@@ -34,7 +38,7 @@ class AppStateStorage(val dataSource: DataSource) extends TransactionalKeyValueS
   def getBestBlockNumber(): BigInt =
     getBigInt(Keys.BestBlockNumber)
 
-  def getBestBlockData(): BestBlockInfo =
+  def getBestBlockInfo(): BestBlockInfo =
     BestBlockInfo( // FIXME default value for hash ?
       get(Keys.BestBlockHash).map(v => ByteString(Hex.decode(v))).getOrElse(ByteString.empty),
       getBigInt(Keys.BestBlockNumber)
@@ -96,10 +100,4 @@ object AppStateStorage {
     val LatestCheckpointBlockNumber = "LatestCheckpointBlockNumber"
   }
 
-  implicit private val bestBlockDataPickler: Pickler[BestBlockInfo] = generatePickler[BestBlockInfo]
-  private val bestBlockDataSerializer = (bestBlockData: BestBlockInfo) =>
-    compactPickledBytes(Pickle.intoBytes(bestBlockData))
-  private val bestBlockDataDeserializer =
-    (byteSequenceToBuffer _).andThen(Unpickle[BestBlockInfo].fromBytes)
-
 }

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

@@ -2,6 +2,7 @@ package io.iohk.ethereum.domain
 
 import akka.util.ByteString
 import scala.annotation.tailrec
+
 import io.iohk.ethereum.db.dataSource.DataSourceBatchUpdate
 import io.iohk.ethereum.db.storage._
 import io.iohk.ethereum.domain

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

@@ -1,9 +1,9 @@
 package io.iohk.ethereum.domain
 
-import io.iohk.ethereum.domain.appstate.BestBlockInfo
-
 import java.util.concurrent.atomic.AtomicReference
 
+import io.iohk.ethereum.domain.appstate.BestBlockInfo
+
 class BlockchainMetadata(bestBlockData: BestBlockInfo, latestCheckpointNumber: BigInt) {
   lazy val bestKnownBlockAndLatestCheckpoint: AtomicReference[BestBlockLatestCheckpointNumbers] =
     new AtomicReference(BestBlockLatestCheckpointNumbers(bestBlockData, latestCheckpointNumber))

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

@@ -97,13 +97,14 @@ class BlockchainReader(
 
   //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()
-      )
-    )
+    val bestKnownBlockinfo = blockchainMetadata.bestKnownBlockAndLatestCheckpoint.get().bestBlockInfo
+    log.debug("Trying to get best block with number {}", bestKnownBlockinfo.number)
+    getBlockByHash(bestKnownBlockinfo.hash)
+      .orElse {
+        val bestBlockInfo = appStateStorage.getBestBlockInfo()
+        log.warn("Falling back to best block in storage number {}", bestBlockInfo.number)
+        getBlockByHash(bestBlockInfo.hash)
+      }
   }
 
   def genesisHeader: BlockHeader =

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

@@ -1,6 +1,7 @@
 package io.iohk.ethereum.domain
 
 import akka.util.ByteString
+
 import io.iohk.ethereum.db.dataSource.DataSourceBatchUpdate
 import io.iohk.ethereum.db.storage.AppStateStorage
 import io.iohk.ethereum.db.storage.BlockBodiesStorage

src/main/scala/io/iohk/ethereum/nodebuilder/NodeBuilder.scala

@@ -176,7 +176,7 @@ trait BlockchainBuilder {
 
   private lazy val blockchainMetadata: BlockchainMetadata =
     new BlockchainMetadata(
-      storagesInstance.storages.appStateStorage.getBestBlockData(),
+      storagesInstance.storages.appStateStorage.getBestBlockInfo(),
       storagesInstance.storages.appStateStorage.getLatestCheckpointBlockNumber()
     )
   lazy val blockchainReader: BlockchainReader = BlockchainReader(storagesInstance.storages, blockchainMetadata)

src/test/scala/io/iohk/ethereum/jsonrpc/EthTxServiceSpec.scala

@@ -174,10 +174,12 @@ class EthTxServiceSpec
   }
 
   it should "return average gas price" in new TestSetup {
-    blockchain.saveBestKnownBlocks(ByteString.empty, 42)
+    private val block: Block =
+      Block(Fixtures.Blocks.Block3125369.header.copy(number = 42), Fixtures.Blocks.Block3125369.body)
     blockchainWriter
-      .storeBlock(Block(Fixtures.Blocks.Block3125369.header.copy(number = 42), Fixtures.Blocks.Block3125369.body))
+      .storeBlock(block)
       .commit()
+    blockchain.saveBestKnownBlocks(block.hash, block.number)
 
     val response = ethTxService.getGetGasPrice(GetGasPriceRequest())
     response.runSyncUnsafe() shouldEqual Right(GetGasPriceResponse(BigInt("20000000000")))

src/test/scala/io/iohk/ethereum/jsonrpc/JsonRpcControllerEthSpec.scala

@@ -413,10 +413,10 @@ class JsonRpcControllerEthSpec
   }
 
   it should "eth_gasPrice" in new JsonRpcControllerFixture {
-    blockchainWriter
-      .storeBlock(Block(Fixtures.Blocks.Block3125369.header.copy(number = 42), Fixtures.Blocks.Block3125369.body))
-      .commit()
-    blockchain.saveBestKnownBlocks(ByteString.empty, 42)
+    private val block: Block =
+      Block(Fixtures.Blocks.Block3125369.header.copy(number = 42), Fixtures.Blocks.Block3125369.body)
+    blockchainWriter.storeBlock(block).commit()
+    blockchain.saveBestKnownBlocks(block.hash, 42)
 
     val request: JsonRpcRequest = newJsonRpcRequest("eth_gasPrice")
 

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

@@ -17,6 +17,7 @@ import io.iohk.ethereum.consensus.validators.BlockHeaderError.HeaderParentNotFou
 import io.iohk.ethereum.consensus.validators._
 import io.iohk.ethereum.db.storage.MptStorage
 import io.iohk.ethereum.domain._
+import io.iohk.ethereum.domain.appstate.BestBlockInfo
 import io.iohk.ethereum.ledger.BlockQueue.Leaf
 import io.iohk.ethereum.mpt.LeafNode
 import io.iohk.ethereum.mpt.MerklePatriciaTrie
@@ -171,6 +172,9 @@ class BlockImportSpec extends AnyFlatSpec with Matchers with ScalaFutures {
     blockchainWriter.save(oldBlock4, Nil, oldWeight4, saveAsBestBlock = true)
 
     val ancestorForValidation: Block = getBlock(0, difficulty = 1)
+    storagesInstance.storages.appStateStorage
+      .putBestBlockData(BestBlockInfo(ancestorForValidation.hash, ancestorForValidation.number))
+      .commit()
     blockchainWriter.save(ancestorForValidation, Nil, ChainWeight.totalDifficultyOnly(1), saveAsBestBlock = false)
 
     val oldBranch = List(oldBlock2, oldBlock3, oldBlock4)