[ETCM-969] move best block and genesis functions to blockchain reader (#1036)

* extract bestKnownBlockAndLatestCheckpoint in BlockchainMetadata

* move getBestBlockNumber and getBestBlock to blockchainReader

* remove TestBlockchainBuilder as it does not have any specific behaviour now

* move genesisHeader and genesisBlock to BlockchainReader

Author: Aurélien Richez

src/it/scala/io/iohk/ethereum/ledger/BlockImporterItSpec.scala

@@ -55,7 +55,7 @@ class BlockImporterItSpec
     testScheduler.awaitTermination(60.second)
   }
 
-  override lazy val blockQueue: BlockQueue = BlockQueue(blockchain, SyncConfig(Config.config))
+  override lazy val blockQueue: BlockQueue = BlockQueue(blockchain, blockchainReader, SyncConfig(Config.config))
 
   val genesis: Block = Block(
     Fixtures.Blocks.Genesis.header.copy(stateRoot = ByteString(MerklePatriciaTrie.EmptyRootHash)),
@@ -122,6 +122,7 @@ class BlockImporterItSpec
       fetcherProbe.ref,
       blockImport,
       blockchain,
+      blockchainReader,
       new BranchResolution(blockchain, blockchainReader),
       syncConfig,
       ommersPoolProbe.ref,
@@ -131,7 +132,7 @@ class BlockImporterItSpec
     )
   )
 
-  val genesisBlock = blockchain.genesisBlock
+  val genesisBlock = blockchainReader.genesisBlock
   val block1: Block = getBlock(genesisBlock.number + 1, parent = genesisBlock.header.hash)
   // new chain is shorter but has a higher weight
   val newBlock2: Block = getBlock(genesisBlock.number + 2, difficulty = 108, parent = block1.header.hash)
@@ -165,6 +166,7 @@ class BlockImporterItSpec
         fetcherProbe.ref,
         mkBlockImport(validators = successValidators),
         blockchain,
+        blockchainReader,
         new BranchResolution(blockchain, blockchainReader),
         syncConfig,
         ommersPoolProbe.ref,
@@ -178,7 +180,7 @@ class BlockImporterItSpec
     blockImporter ! BlockFetcher.PickedBlocks(NonEmptyList.fromListUnsafe(newBranch))
 
     //because the blocks are not valid, we shouldn't reorganise, but at least stay with a current chain, and the best block of the current chain is oldBlock4
-    eventually(blockchain.getBestBlock().get shouldEqual oldBlock4)
+    eventually(blockchainReader.getBestBlock().get shouldEqual oldBlock4)
   }
 
   it should "return a correct new best block after reorganising longer chain to a shorter one if its weight is bigger" in {
@@ -190,7 +192,7 @@ class BlockImporterItSpec
 
     blockImporter ! BlockFetcher.PickedBlocks(NonEmptyList.fromListUnsafe(newBranch))
 
-    eventually(blockchain.getBestBlock().get shouldEqual newBlock3)
+    eventually(blockchainReader.getBestBlock().get shouldEqual newBlock3)
   }
 
   it should "return Unknown branch, in case of PickedBlocks with block that has a parent that's not in the chain" in {
@@ -211,13 +213,13 @@ class BlockImporterItSpec
     blockchainWriter.save(oldBlock3, Nil, oldWeight3, saveAsBestBlock = true)
     blockchainWriter.save(oldBlock4, Nil, oldWeight4, saveAsBestBlock = true)
     // simulation of node restart
-    blockchain.saveBestKnownBlocks(blockchain.getBestBlockNumber() - 1)
+    blockchain.saveBestKnownBlocks(blockchainReader.getBestBlockNumber() - 1)
     blockchainWriter.save(newBlock4ParentOldBlock3, Nil, newBlock4WeightParentOldBlock3, saveAsBestBlock = true)
 
     //not reorganising anymore until oldBlock4(not part of the chain anymore), no block/ommer validation when not part of the chain, resolveBranch is returning UnknownBranch
     blockImporter ! BlockFetcher.PickedBlocks(NonEmptyList.fromListUnsafe(List(newBlock5ParentOldBlock4)))
 
-    eventually(blockchain.getBestBlock().get shouldEqual newBlock4ParentOldBlock3)
+    eventually(blockchainReader.getBestBlock().get shouldEqual newBlock4ParentOldBlock3)
   }
 
   it should "switch to a branch with a checkpoint" in {
@@ -230,7 +232,7 @@ class BlockImporterItSpec
 
     blockImporter ! BlockFetcher.PickedBlocks(NonEmptyList.fromListUnsafe(newBranch))
 
-    eventually(blockchain.getBestBlock().get shouldEqual oldBlock5WithCheckpoint)
+    eventually(blockchainReader.getBestBlock().get shouldEqual oldBlock5WithCheckpoint)
     eventually(blockchain.getLatestCheckpointBlockNumber() shouldEqual oldBlock5WithCheckpoint.header.number)
   }
 
@@ -244,13 +246,13 @@ class BlockImporterItSpec
 
     blockImporter ! BlockFetcher.PickedBlocks(NonEmptyList.fromListUnsafe(newBranch))
 
-    eventually(blockchain.getBestBlock().get shouldEqual newBlock4WithCheckpoint)
+    eventually(blockchainReader.getBestBlock().get shouldEqual newBlock4WithCheckpoint)
     eventually(blockchain.getLatestCheckpointBlockNumber() shouldEqual newBlock4WithCheckpoint.header.number)
   }
 
   it should "return a correct checkpointed block after receiving a request for generating a new checkpoint" in {
 
-    val parent = blockchain.getBestBlock().get
+    val parent = blockchainReader.getBestBlock().get
     val newBlock5: Block = getBlock(genesisBlock.number + 5, difficulty = 104, parent = parent.header.hash)
     val newWeight5 = newWeight3.increase(newBlock5.header)
 
@@ -263,12 +265,12 @@ class BlockImporterItSpec
     val checkpointBlock = checkpointBlockGenerator.generate(newBlock5, Checkpoint(signatures))
     blockImporter ! NewCheckpoint(checkpointBlock)
 
-    eventually(blockchain.getBestBlock().get shouldEqual checkpointBlock)
+    eventually(blockchainReader.getBestBlock().get shouldEqual checkpointBlock)
     eventually(blockchain.getLatestCheckpointBlockNumber() shouldEqual newBlock5.header.number + 1)
   }
 
   it should "ask BlockFetcher to resolve missing node" in {
-    val parent = blockchain.getBestBlock().get
+    val parent = blockchainReader.getBestBlock().get
     val newBlock: Block = getBlock(genesisBlock.number + 5, difficulty = 104, parent = parent.header.hash)
     val invalidBlock = newBlock.copy(header = newBlock.header.copy(beneficiary = Address(111).bytes))
 
@@ -277,6 +279,7 @@ class BlockImporterItSpec
         fetcherProbe.ref,
         mkBlockImport(validators = successValidators),
         blockchain,
+        blockchainReader,
         new BranchResolution(blockchain, blockchainReader),
         syncConfig,
         ommersPoolProbe.ref,

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

@@ -38,8 +38,14 @@ class FastSyncItSpec extends FlatSpecBase with Matchers with BeforeAndAfterAll {
       _ <- peer1.startFastSync().delayExecution(50.milliseconds)
       _ <- peer1.waitForFastSyncFinish()
     } yield {
-      assert(peer1.bl.getBestBlockNumber() == peer2.bl.getBestBlockNumber() - peer2.testSyncConfig.pivotBlockOffset)
-      assert(peer1.bl.getBestBlockNumber() == peer3.bl.getBestBlockNumber() - peer3.testSyncConfig.pivotBlockOffset)
+      assert(
+        peer1.blockchainReader
+          .getBestBlockNumber() == peer2.blockchainReader.getBestBlockNumber() - peer2.testSyncConfig.pivotBlockOffset
+      )
+      assert(
+        peer1.blockchainReader
+          .getBestBlockNumber() == peer3.blockchainReader.getBestBlockNumber() - peer3.testSyncConfig.pivotBlockOffset
+      )
     }
   }
 
@@ -56,8 +62,14 @@ class FastSyncItSpec extends FlatSpecBase with Matchers with BeforeAndAfterAll {
         val synchronizingPeerHaveAllData = peer1.containsExpectedDataUpToAccountAtBlock(1000, 500)
         // due to the fact that function generating state is deterministic both peer2 and peer3 ends up with exactly same
         // state, so peer1 can get whole trie from both of them.
-        assert(peer1.bl.getBestBlockNumber() == peer2.bl.getBestBlockNumber() - peer2.testSyncConfig.pivotBlockOffset)
-        assert(peer1.bl.getBestBlockNumber() == peer3.bl.getBestBlockNumber() - peer3.testSyncConfig.pivotBlockOffset)
+        assert(
+          peer1.blockchainReader
+            .getBestBlockNumber() == peer2.blockchainReader.getBestBlockNumber() - peer2.testSyncConfig.pivotBlockOffset
+        )
+        assert(
+          peer1.blockchainReader
+            .getBestBlockNumber() == peer3.blockchainReader.getBestBlockNumber() - peer3.testSyncConfig.pivotBlockOffset
+        )
         assert(trie.isDefined)
         assert(synchronizingPeerHaveAllData)
       }
@@ -83,8 +95,14 @@ class FastSyncItSpec extends FlatSpecBase with Matchers with BeforeAndAfterAll {
         val synchronizingPeerHaveAllData = peer1.containsExpectedDataUpToAccountAtBlock(1000, 500)
         // due to the fact that function generating state is deterministic both peer3 and peer4 ends up with exactly same
         // state, so peer1 can get whole trie from both of them.
-        assert(peer1.bl.getBestBlockNumber() == peer3.bl.getBestBlockNumber() - peer3.testSyncConfig.pivotBlockOffset)
-        assert(peer1.bl.getBestBlockNumber() == peer4.bl.getBestBlockNumber() - peer4.testSyncConfig.pivotBlockOffset)
+        assert(
+          peer1.blockchainReader
+            .getBestBlockNumber() == peer3.blockchainReader.getBestBlockNumber() - peer3.testSyncConfig.pivotBlockOffset
+        )
+        assert(
+          peer1.blockchainReader
+            .getBestBlockNumber() == peer4.blockchainReader.getBestBlockNumber() - peer4.testSyncConfig.pivotBlockOffset
+        )
         assert(trie.isDefined)
         assert(synchronizingPeerHaveAllData)
       }
@@ -110,8 +128,14 @@ class FastSyncItSpec extends FlatSpecBase with Matchers with BeforeAndAfterAll {
         val synchronizingPeerHaveAllData = peer1.containsExpectedDataUpToAccountAtBlock(1000, 500)
         // due to the fact that function generating state is deterministic both peer3 and peer4 ends up with exactly same
         // state, so peer1 can get whole trie from both of them.
-        assert(peer1.bl.getBestBlockNumber() == peer3.bl.getBestBlockNumber() - peer3.testSyncConfig.pivotBlockOffset)
-        assert(peer1.bl.getBestBlockNumber() == peer4.bl.getBestBlockNumber() - peer4.testSyncConfig.pivotBlockOffset)
+        assert(
+          peer1.blockchainReader
+            .getBestBlockNumber() == peer3.blockchainReader.getBestBlockNumber() - peer3.testSyncConfig.pivotBlockOffset
+        )
+        assert(
+          peer1.blockchainReader
+            .getBestBlockNumber() == peer4.blockchainReader.getBestBlockNumber() - peer4.testSyncConfig.pivotBlockOffset
+        )
         assert(trie.isDefined)
         assert(synchronizingPeerHaveAllData)
       }
@@ -125,7 +149,8 @@ class FastSyncItSpec extends FlatSpecBase with Matchers with BeforeAndAfterAll {
       _ <- peer1.startFastSync().delayExecution(50.milliseconds)
       _ <- peer1.waitForFastSyncFinish()
     } yield assert(
-      peer1.bl.getBestBlockNumber() == peer2.bl.getBestBlockNumber() - peer2.testSyncConfig.pivotBlockOffset
+      peer1.blockchainReader
+        .getBestBlockNumber() == peer2.blockchainReader.getBestBlockNumber() - peer2.testSyncConfig.pivotBlockOffset
     )
   }
 
@@ -139,7 +164,8 @@ class FastSyncItSpec extends FlatSpecBase with Matchers with BeforeAndAfterAll {
       _ <- peer1.startFastSync().delayExecution(50.milliseconds)
       _ <- peer1.waitForFastSyncFinish()
     } yield assert(
-      peer1.bl.getBestBlockNumber() == peer2.bl.getBestBlockNumber() - peer2.testSyncConfig.pivotBlockOffset
+      peer1.blockchainReader
+        .getBestBlockNumber() == peer2.blockchainReader.getBestBlockNumber() - peer2.testSyncConfig.pivotBlockOffset
     )
   }
 
@@ -155,7 +181,8 @@ class FastSyncItSpec extends FlatSpecBase with Matchers with BeforeAndAfterAll {
       _ <- peer1.startFastSync().delayExecution(50.milliseconds)
       _ <- peer1.waitForFastSyncFinish()
     } yield assert(
-      peer1.bl.getBestBlockNumber() == peer2.bl.getBestBlockNumber() - peer2.testSyncConfig.pivotBlockOffset
+      peer1.blockchainReader
+        .getBestBlockNumber() == peer2.blockchainReader.getBestBlockNumber() - peer2.testSyncConfig.pivotBlockOffset
     )
   }
 
@@ -179,8 +206,14 @@ class FastSyncItSpec extends FlatSpecBase with Matchers with BeforeAndAfterAll {
       val synchronizingPeerHaveAllData = peer1.containsExpectedDataUpToAccountAtBlock(3000, 1001)
       // due to the fact that function generating state is deterministic both peer3 and peer4 ends up with exactly same
       // state, so peer1 can get whole trie from both of them.
-      assert(peer1.bl.getBestBlockNumber() == peer3.bl.getBestBlockNumber() - peer3.testSyncConfig.pivotBlockOffset)
-      assert(peer1.bl.getBestBlockNumber() == peer4.bl.getBestBlockNumber() - peer4.testSyncConfig.pivotBlockOffset)
+      assert(
+        peer1.blockchainReader
+          .getBestBlockNumber() == peer3.blockchainReader.getBestBlockNumber() - peer3.testSyncConfig.pivotBlockOffset
+      )
+      assert(
+        peer1.blockchainReader
+          .getBestBlockNumber() == peer4.blockchainReader.getBestBlockNumber() - peer4.testSyncConfig.pivotBlockOffset
+      )
       assert(trie.isDefined)
       assert(synchronizingPeerHaveAllData)
     }
@@ -196,7 +229,8 @@ class FastSyncItSpec extends FlatSpecBase with Matchers with BeforeAndAfterAll {
       _ <- peer1.startFastSync().delayExecution(50.milliseconds)
       _ <- peer1.waitForFastSyncFinish()
     } yield assert(
-      peer1.bl.getBestBlockNumber() == peer3.bl.getBestBlockNumber() - peer3.testSyncConfig.pivotBlockOffset
+      peer1.blockchainReader
+        .getBestBlockNumber() == peer3.blockchainReader.getBestBlockNumber() - peer3.testSyncConfig.pivotBlockOffset
     )
   }
 

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

@@ -39,7 +39,7 @@ class RegularSyncItSpec extends FreeSpecBase with Matchers with BeforeAndAfterAl
           _ <- peer2.startRegularSync()
           _ <- peer2.connectToPeers(Set(peer1.node))
           _ <- peer2.waitForRegularSyncLoadLastBlock(blockNumber)
-        } yield assert(peer1.bl.getBestBlock().get.hash == peer2.bl.getBestBlock().get.hash)
+        } yield assert(peer1.blockchainReader.getBestBlock().get.hash == peer2.blockchainReader.getBestBlock().get.hash)
     }
 
     "given a previously mined blockchain" in customTestCaseResourceM(FakePeer.start2FakePeersRes()) {
@@ -52,7 +52,7 @@ class RegularSyncItSpec extends FreeSpecBase with Matchers with BeforeAndAfterAl
           _ <- peer2.startRegularSync()
           _ <- peer2.connectToPeers(Set(peer1.node))
           _ <- peer2.waitForRegularSyncLoadLastBlock(blockHeadersPerRequest + 1)
-        } yield assert(peer1.bl.getBestBlock().get.hash == peer2.bl.getBestBlock().get.hash)
+        } yield assert(peer1.blockchainReader.getBestBlock().get.hash == peer2.blockchainReader.getBestBlock().get.hash)
     }
   }
 
@@ -70,7 +70,7 @@ class RegularSyncItSpec extends FreeSpecBase with Matchers with BeforeAndAfterAl
       _ <- peer1.waitForRegularSyncLoadLastBlock(blockNumer + 2)
       _ <- peer1.mineNewBlocks(100.milliseconds, 2)(IdentityUpdate)
       _ <- peer2.waitForRegularSyncLoadLastBlock(blockNumer + 4)
-    } yield assert(peer1.bl.getBestBlock().get.hash == peer2.bl.getBestBlock().get.hash)
+    } yield assert(peer1.blockchainReader.getBestBlock().get.hash == peer2.blockchainReader.getBestBlock().get.hash)
   }
 
   "peers should keep being synced on checkpoints" in customTestCaseResourceM(
@@ -83,10 +83,10 @@ class RegularSyncItSpec extends FreeSpecBase with Matchers with BeforeAndAfterAl
       _ <- peer2.startRegularSync()
       _ <- peer2.connectToPeers(Set(peer1.node))
       _ <- peer2.waitForRegularSyncLoadLastBlock(blockNumber)
-      _ <- peer2.addCheckpointedBlock(peer2.bl.getBestBlock().get)
+      _ <- peer2.addCheckpointedBlock(peer2.blockchainReader.getBestBlock().get)
       _ <- peer1.waitForRegularSyncLoadLastBlock(blockNumber + 1)
     } yield {
-      assert(peer1.bl.getBestBlock().get.hash == peer2.bl.getBestBlock().get.hash)
+      assert(peer1.blockchainReader.getBestBlock().get.hash == peer2.blockchainReader.getBestBlock().get.hash)
       assert(peer1.bl.getLatestCheckpointBlockNumber() == peer2.bl.getLatestCheckpointBlockNumber())
     }
   }
@@ -103,14 +103,14 @@ class RegularSyncItSpec extends FreeSpecBase with Matchers with BeforeAndAfterAl
       _ <- peer2.waitForRegularSyncLoadLastBlock(blockNumber)
       _ <- peer2.mineNewBlocks(100.milliseconds, 2)(IdentityUpdate)
       _ <- peer2.waitForRegularSyncLoadLastBlock(blockNumber + 2)
-      _ <- peer2.addCheckpointedBlock(peer2.bl.getBestBlock().get)
+      _ <- peer2.addCheckpointedBlock(peer2.blockchainReader.getBestBlock().get)
       _ <- peer2.waitForRegularSyncLoadLastBlock(blockNumber + 3)
-      _ <- peer1.addCheckpointedBlock(peer1.bl.getBestBlock().get)
+      _ <- peer1.addCheckpointedBlock(peer1.blockchainReader.getBestBlock().get)
       _ <- peer2.waitForRegularSyncLoadLastBlock(blockNumber + 4)
       _ <- peer1.mineNewBlocks(100.milliseconds, 1)(IdentityUpdate)
       _ <- peer2.waitForRegularSyncLoadLastBlock(blockNumber + 5)
     } yield {
-      assert(peer1.bl.getBestBlock().get.hash == peer2.bl.getBestBlock().get.hash)
+      assert(peer1.blockchainReader.getBestBlock().get.hash == peer2.blockchainReader.getBestBlock().get.hash)
       assert(peer1.bl.getLatestCheckpointBlockNumber() == peer2.bl.getLatestCheckpointBlockNumber())
     }
   }
@@ -129,13 +129,10 @@ class RegularSyncItSpec extends FreeSpecBase with Matchers with BeforeAndAfterAl
       _ <- peer1.connectToPeers(Set(peer2.node))
       _ <- peer1.waitForRegularSyncLoadLastBlock(length)
     } yield {
-      assert(peer1.bl.getBestBlock().get.hash == peer2.bl.getBestBlock().get.hash)
-      assert(
-        peer1.bl.getBestBlock().get.number == peer2.bl.getBestBlock().get.number && peer1.bl
-          .getBestBlock()
-          .get
-          .number == length
-      )
+      assert(peer1.blockchainReader.getBestBlock().get.hash == peer2.blockchainReader.getBestBlock().get.hash)
+      val peer1BestBlockNumber = peer1.blockchainReader.getBestBlock().get.number
+      val peer2BestBlockNumber = peer2.blockchainReader.getBestBlock().get.number
+      assert(peer1BestBlockNumber == peer2BestBlockNumber && peer1BestBlockNumber == length)
       assert(peer1.bl.getLatestCheckpointBlockNumber() == peer2.bl.getLatestCheckpointBlockNumber())
     }
   }
@@ -147,7 +144,7 @@ class RegularSyncItSpec extends FreeSpecBase with Matchers with BeforeAndAfterAl
     for {
       _ <- peer1.importBlocksUntil(8)(IdentityUpdate)
       _ <- peer1.startRegularSync()
-      _ <- peer1.addCheckpointedBlock(peer1.bl.getBestBlock().get)
+      _ <- peer1.addCheckpointedBlock(peer1.blockchainReader.getBestBlock().get)
       _ <- peer1.waitForRegularSyncLoadLastBlock(9)
       _ <- peer2.importBlocksUntil(20)(IdentityUpdate)
       _ <- peer2.startRegularSync()
@@ -157,7 +154,7 @@ class RegularSyncItSpec extends FreeSpecBase with Matchers with BeforeAndAfterAl
       _ <- peer1.waitForRegularSyncLoadLastBlock(19)
     } yield assert(true)
   //these should pass
-//      assert(peer1.bl.getBestBlock().get.hash == peer2.bl.getBestBlock().get.hash )
+//      assert(peer1.blockchainReader.getBestBlock().get.hash == peer2.blockchainReader.getBestBlock().get.hash )
 //      assert(peer1.bl.getLatestCheckpointBlockNumber() == peer2.bl.getLatestCheckpointBlockNumber())
   }
 
@@ -179,8 +176,8 @@ class RegularSyncItSpec extends FreeSpecBase with Matchers with BeforeAndAfterAl
       _ <- peer2.waitForRegularSyncLoadLastBlock(blockNumer + 3)
     } yield {
       assert(
-        peer1.bl.getChainWeightByHash(peer1.bl.getBestBlock().get.hash) == peer2.bl.getChainWeightByHash(
-          peer2.bl.getBestBlock().get.hash
+        peer1.bl.getChainWeightByHash(peer1.blockchainReader.getBestBlock().get.hash) == peer2.bl.getChainWeightByHash(
+          peer2.blockchainReader.getBestBlock().get.hash
         )
       )
       (