Merge branch 'develop' into ETCM-521-integration-tests

robinraju · web-flow · commit 9c2080c52d56 · 2021-03-22T19:41:10.000+05:30
diff --git a/src/it/scala/io/iohk/ethereum/sync/RegularSyncItSpec.scala b/src/it/scala/io/iohk/ethereum/sync/RegularSyncItSpec.scala
@@ -3,6 +3,7 @@ package io.iohk.ethereum.sync
 import com.typesafe.config.ConfigValueFactory
 import io.iohk.ethereum.FreeSpecBase
 import io.iohk.ethereum.metrics.{Metrics, MetricsConfig}
+import io.iohk.ethereum.network.PeerId
 import io.iohk.ethereum.sync.util.RegularSyncItSpecUtils.FakePeer
 import io.iohk.ethereum.sync.util.SyncCommonItSpec._
 import io.iohk.ethereum.utils.Config
@@ -27,6 +28,24 @@ class RegularSyncItSpec extends FreeSpecBase with Matchers with BeforeAndAfterAl
     testScheduler.awaitTermination(120.second)
   }
 
+  "a peer should reorganise when receives a checkpoint older than the current best from a peer" in customTestCaseResourceM(
+    FakePeer.start2FakePeersRes()
+  ) { case (peer1, peer2) =>
+    for {
+      _ <- peer1.importBlocksUntil(20)(IdentityUpdate)
+      _ <- peer2.importBlocksUntil(30)(IdentityUpdate)
+      _ <- peer1.startRegularSync()
+      _ <- peer2.startRegularSync()
+      _ <- peer1.addCheckpointedBlock(peer1.bl.getBestBlock().get)
+      _ <- peer1.waitForRegularSyncLoadLastBlock(21)
+      _ <- peer2.getCheckpointFromPeer(peer1.bl.getBestBlock().get, PeerId("Peer1"))
+      _ <- peer2.waitForRegularSyncLoadLastBlock(21)
+    } yield {
+      assert(peer1.bl.getBestBlock().get.hash == peer2.bl.getBestBlock().get.hash)
+      assert(peer1.bl.getLatestCheckpointBlockNumber() == peer2.bl.getLatestCheckpointBlockNumber())
+    }
+  }
+
   "peer 2 should sync to the top of peer1 blockchain" - {
     "given a previously imported blockchain" in customTestCaseResourceM(FakePeer.start2FakePeersRes()) {
       case (peer1, peer2) =>
@@ -76,6 +95,91 @@ class RegularSyncItSpec extends FreeSpecBase with Matchers with BeforeAndAfterAl
     }
   }
 
+  "peers should keep being synced on checkpoints" in customTestCaseResourceM(
+    FakePeer.start2FakePeersRes()
+  ) { case (peer1, peer2) =>
+    val blockNumber: Int = 2000
+    for {
+      _ <- peer1.importBlocksUntil(blockNumber)(IdentityUpdate)
+      _ <- peer1.startRegularSync()
+      _ <- peer2.startRegularSync()
+      _ <- peer2.connectToPeers(Set(peer1.node))
+      _ <- peer2.waitForRegularSyncLoadLastBlock(blockNumber)
+      _ <- peer2.addCheckpointedBlock(peer2.bl.getBestBlock().get)
+      _ <- peer1.waitForRegularSyncLoadLastBlock(blockNumber + 1)
+    } yield {
+      assert(peer1.bl.getBestBlock().get.hash == peer2.bl.getBestBlock().get.hash)
+      assert(peer1.bl.getLatestCheckpointBlockNumber() == peer2.bl.getLatestCheckpointBlockNumber())
+    }
+  }
+
+  "peers should keep being synced on checkpoints even if 2 checkpoints are issued to different forks at the same time" in customTestCaseResourceM(
+    FakePeer.start2FakePeersRes()
+  ) { case (peer1, peer2) =>
+    val blockNumber: Int = 2000
+    for {
+      _ <- peer1.importBlocksUntil(blockNumber)(IdentityUpdate)
+      _ <- peer1.startRegularSync()
+      _ <- peer2.startRegularSync()
+      _ <- peer2.connectToPeers(Set(peer1.node))
+      _ <- peer2.waitForRegularSyncLoadLastBlock(blockNumber)
+      _ <- peer2.mineNewBlocks(100.milliseconds, 2)(IdentityUpdate)
+      _ <- peer2.waitForRegularSyncLoadLastBlock(blockNumber + 2)
+      _ <- peer2.addCheckpointedBlock(peer2.bl.getBestBlock().get)
+      _ <- peer2.waitForRegularSyncLoadLastBlock(blockNumber + 3)
+      _ <- peer1.addCheckpointedBlock(peer1.bl.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.bl.getLatestCheckpointBlockNumber() == peer2.bl.getLatestCheckpointBlockNumber())
+    }
+  }
+
+  "peers should chose the branch with a checkpoint discarding blocks that come after the checkpoint" in customTestCaseResourceM(
+    FakePeer.start2FakePeersRes()
+  ) { case (peer1, peer2) =>
+    val length = 26
+    for {
+      _ <- peer1.importBlocksUntil(20)(IdentityUpdate)
+      _ <- peer2.importBlocksUntil(30)(IdentityUpdate)
+      _ <- peer1.startRegularSync()
+      _ <- peer2.startRegularSync()
+      _ <- peer2.addCheckpointedBlock(peer2.bl.getBlockByNumber(25).get)
+      _ <- peer2.waitForRegularSyncLoadLastBlock(length)
+      _ <- 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.bl.getLatestCheckpointBlockNumber() == peer2.bl.getLatestCheckpointBlockNumber())
+    }
+  }
+
+  //TODO: investigate why reorganisation is not triggered after 2 nodes with conflicting branches connect
+  "peers should choose the branch with a checkpoint even if it's shorter" in customTestCaseResourceM(
+    FakePeer.start2FakePeersRes()
+  ) { case (peer1, peer2) =>
+    for {
+      _ <- peer1.importBlocksUntil(8)(IdentityUpdate)
+      _ <- peer1.startRegularSync()
+      _ <- peer1.addCheckpointedBlock(peer1.bl.getBestBlock().get)
+      _ <- peer1.waitForRegularSyncLoadLastBlock(9)
+      _ <- peer2.importBlocksUntil(20)(IdentityUpdate)
+      _ <- peer2.startRegularSync()
+      _ <- peer2.connectToPeers(Set(peer1.node))
+      //without new added blocks the syncing and reorganisation are not triggered
+      _ <- peer1.mineNewBlocks(500.milliseconds, 10)(IdentityUpdate)
+      _ <- peer1.waitForRegularSyncLoadLastBlock(19)
+    } yield {
+      assert(true)
+      //these should pass
+//      assert(peer1.bl.getBestBlock().get.hash == peer2.bl.getBestBlock().get.hash )
+//      assert(peer1.bl.getLatestCheckpointBlockNumber() == peer2.bl.getLatestCheckpointBlockNumber())
+    }
+  }
+
   "peers with divergent chains will be forced to resolve branches" in customTestCaseResourceM(
     FakePeer.start2FakePeersRes()
   ) { case (peer1, peer2) =>
diff --git a/src/it/scala/io/iohk/ethereum/sync/util/RegularSyncItSpecUtils.scala b/src/it/scala/io/iohk/ethereum/sync/util/RegularSyncItSpecUtils.scala
@@ -6,14 +6,21 @@ import cats.effect.Resource
 import io.iohk.ethereum.Mocks.MockValidatorsAlwaysSucceed
 import io.iohk.ethereum.blockchain.sync.regular.BlockBroadcast.BlockToBroadcast
 import io.iohk.ethereum.blockchain.sync.regular.BlockBroadcasterActor.BroadcastBlock
-import io.iohk.ethereum.blockchain.sync.regular.RegularSync
+import io.iohk.ethereum.blockchain.sync.regular.BlockImporter.Start
+import io.iohk.ethereum.blockchain.sync.regular.{BlockBroadcast, BlockBroadcasterActor, BlockFetcher, BlockImporter, RegularSync}
+import io.iohk.ethereum.blockchain.sync.regular.RegularSync.NewCheckpoint
 import io.iohk.ethereum.blockchain.sync.{PeersClient, SyncProtocol}
+import io.iohk.ethereum.checkpointing.CheckpointingTestHelpers
 import io.iohk.ethereum.consensus.Protocol.NoAdditionalEthashData
 import io.iohk.ethereum.consensus.blocks.CheckpointBlockGenerator
 import io.iohk.ethereum.consensus.ethash.{EthashConfig, EthashConsensus}
 import io.iohk.ethereum.consensus.{ConsensusConfig, FullConsensusConfig, ethash}
+import io.iohk.ethereum.crypto
 import io.iohk.ethereum.domain._
 import io.iohk.ethereum.ledger._
+import io.iohk.ethereum.network.PeerEventBusActor.PeerEvent.MessageFromPeer
+import io.iohk.ethereum.network.PeerId
+import io.iohk.ethereum.network.p2p.messages.CommonMessages.NewBlock
 import io.iohk.ethereum.nodebuilder.VmSetup
 import io.iohk.ethereum.ommers.OmmersPool
 import io.iohk.ethereum.sync.util.SyncCommonItSpecUtils.FakePeerCustomConfig.defaultConfig
@@ -23,6 +30,7 @@ import io.iohk.ethereum.utils._
 import io.iohk.ethereum.vm.EvmConfig
 import monix.eval.Task
 import monix.execution.Scheduler
+
 import scala.concurrent.duration._
 object RegularSyncItSpecUtils {
 
@@ -66,6 +74,31 @@ object RegularSyncItSpecUtils {
 
     lazy val validators = buildEthashConsensus().validators
 
+    val broadcasterRef: ActorRef = system.actorOf(
+      BlockBroadcasterActor
+        .props(new BlockBroadcast(etcPeerManager), peerEventBus, etcPeerManager, syncConfig, system.scheduler),
+      "block-broadcaster"
+    )
+
+    val fetcher: ActorRef =
+      system.actorOf(
+        BlockFetcher.props(peersClient, peerEventBus, regularSync, syncConfig, validators.blockValidator),
+        "block-fetcher"
+      )
+
+    lazy val blockImporter = system.actorOf(
+      BlockImporter.props(
+        fetcher,
+        ledger,
+        bl,
+        syncConfig,
+        ommersPool,
+        broadcasterRef,
+        pendingTransactionsManager,
+        checkpointBlockGenerator,
+        regularSync
+      ))
+
     lazy val regularSync = system.actorOf(
       RegularSync.props(
         peersClient,
@@ -132,6 +165,19 @@ object RegularSyncItSpecUtils {
       } else Task(())
     }
 
+    def addCheckpointedBlock(parent: Block): Task[Unit] = Task {
+      val signatures = CheckpointingTestHelpers.createCheckpointSignatures(
+        Seq(crypto.generateKeyPair(secureRandom)),
+        parent.hash
+      )
+      regularSync ! NewCheckpoint(parent.header.hash, signatures)
+    }
+
+    def getCheckpointFromPeer(checkpoint: Block, peerId: PeerId): Task[Unit] = Task {
+      blockImporter ! Start
+      fetcher ! MessageFromPeer(NewBlock(checkpoint, checkpoint.header.difficulty), peerId)
+    }
+
     private def getMptForBlock(block: Block) = {
       bl.getWorldStateProxy(
         blockNumber = block.number,
diff --git a/src/main/scala/io/iohk/ethereum/blockchain/sync/regular/BlockImporter.scala b/src/main/scala/io/iohk/ethereum/blockchain/sync/regular/BlockImporter.scala
@@ -329,7 +329,7 @@ class BlockImporter(
         Right(Nil)
       case UnknownBranch =>
         val currentBlock = blocks.head.number.min(startingBlockNumber)
-        val goingBackTo = currentBlock - syncConfig.branchResolutionRequestSize
+        val goingBackTo = (currentBlock - syncConfig.branchResolutionRequestSize).max(0)
         val msg = s"Unknown branch, going back to block nr $goingBackTo in order to resolve branches"
 
         log.info(msg)
diff --git a/src/main/scala/io/iohk/ethereum/blockchain/sync/regular/RegularSync.scala b/src/main/scala/io/iohk/ethereum/blockchain/sync/regular/RegularSync.scala
@@ -102,9 +102,7 @@ class RegularSync(
       context become running(newState)
     case ProgressProtocol.ImportedBlock(blockNumber, isCheckpoint, internally) =>
       log.info(s"Imported new block [number = $blockNumber, internally = $internally]")
-      val newState =
-        if (isCheckpoint) progressState.copy(currentBlock = blockNumber, bestKnownNetworkBlock = blockNumber)
-        else progressState.copy(currentBlock = blockNumber)
+      val newState = progressState.copy(currentBlock = blockNumber)
       if (internally && isCheckpoint) {
         fetcher ! InternalCheckpointImport(blockNumber)
       } else if (internally) {
diff --git a/src/test/scala/io/iohk/ethereum/blockchain/sync/PivotBlockSelectorSpec.scala b/src/test/scala/io/iohk/ethereum/blockchain/sync/PivotBlockSelectorSpec.scala
@@ -447,7 +447,7 @@ class PivotBlockSelectorSpec
 
     override def defaultSyncConfig: SyncConfig = super.defaultSyncConfig.copy(
       doFastSync = true,
-      branchResolutionRequestSize = 20,
+      branchResolutionRequestSize = 30,
       checkForNewBlockInterval = 1.second,
       blockHeadersPerRequest = 10,
       blockBodiesPerRequest = 10,
diff --git a/src/test/scala/io/iohk/ethereum/blockchain/sync/SyncControllerSpec.scala b/src/test/scala/io/iohk/ethereum/blockchain/sync/SyncControllerSpec.scala
@@ -516,7 +516,7 @@ class SyncControllerSpec
 
     override def defaultSyncConfig: SyncConfig = super.defaultSyncConfig.copy(
       doFastSync = true,
-      branchResolutionRequestSize = 20,
+      branchResolutionRequestSize = 30,
       checkForNewBlockInterval = 1.second,
       blockHeadersPerRequest = 10,
       blockBodiesPerRequest = 10,
diff --git a/src/test/scala/io/iohk/ethereum/blockchain/sync/TestSyncConfig.scala b/src/test/scala/io/iohk/ethereum/blockchain/sync/TestSyncConfig.scala
@@ -10,7 +10,7 @@ trait TestSyncConfig extends SyncConfigBuilder {
     printStatusInterval = 1.second,
     persistStateSnapshotInterval = 2.seconds,
     pivotBlockOffset = 500,
-    branchResolutionRequestSize = 2,
+    branchResolutionRequestSize = 30,
     blacklistDuration = 5.seconds,
     criticalBlacklistDuration = 10.seconds,
     syncRetryInterval = 1.second,
