[ETCM-103] Minor renaming and cleanup

Author: KonradStaniec

src/main/resources/application.conf

@@ -261,17 +261,17 @@ mantis {
   blockchains {
     network = "etc"
 
-    etc { include "chains/etc-chain.conf"}
+    etc {include "chains/etc-chain.conf"}
 
-    eth { include "chains/eth-chain.conf"}
+    eth {include "chains/eth-chain.conf"}
 
-    mordor { include "chains/mordor-chain.conf"}
+    mordor {include "chains/mordor-chain.conf"}
 
-    ropsten { include "chains/ropsten-chain.conf"}
+    ropsten {include "chains/ropsten-chain.conf"}
 
-    test { include "chains/test-chain.conf"}
+    test {include "chains/test-chain.conf"}
 
-    testnet-internal { include "chains/testnet-internal-chain.conf"}
+    testnet-internal {include "chains/testnet-internal-chain.conf"}
   }
 
   sync {
@@ -397,17 +397,16 @@ mantis {
     # Current Size of ETC state trie is aroud 150M Nodes, so 200M is set to have some reserve
     # If the number of elements inserted into bloom filter would be significally higher that expected, then number
     # of false positives would rise which would degrade performance of state sync
-    state-sync-bloomFilter-size = 200000000
-
+    state-sync-bloom-filter-size = 200000000
 
     # Max number of mpt nodes held in memory in state sync, before saving them into database
     # 100k is around 60mb (each key-value pair has around 600bytes)
-    state-sync-persistBatch-size = 100000
+    state-sync-persist-batch-size = 100000
 
     # If new pivot block received from network will be less than fast sync current pivot block, the re-try to chose new
     # pivot will be scheduler after this time. Avarage block time in etc/eth is around 15s so after this time, most of
     # network peers should have new best block
-    pivot-block-reSchedule-interval =  15.seconds
+    pivot-block-reschedule-interval = 15.seconds
 
     # If for most network peers, the following condition will be true:
     # (peer.bestKnownBlock - pivot-block-offset) - node.curentPivotBlock > max-pivot-age

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

@@ -72,7 +72,7 @@ class FastSync(
   def startWithState(syncState: SyncState): Unit = {
     log.info(s"Starting with existing state and asking for new pivot block")
     val syncingHandler = new SyncingHandler(syncState)
-    syncingHandler.askForPivotBlockUpdate(NodeRestart)
+    syncingHandler.askForPivotBlockUpdate(SyncRestart)
   }
 
   def startFromScratch(): Unit = {
@@ -205,39 +205,48 @@ class FastSync(
       scheduler.scheduleOnce(syncConfig.pivotBlockReScheduleInterval, self, UpdatePivotBlock(updateReason))
     }
 
+    private def stalePivotAfterRestart(
+        newPivot: BlockHeader,
+        currentState: SyncState,
+        updateReason: PivotBlockUpdateReason
+    ): Boolean = {
+      newPivot.number == currentState.pivotBlock.number && updateReason.isSyncRestart
+    }
+
+    private def newPivotIsGoodEnough(
+        newPivot: BlockHeader,
+        currentState: SyncState,
+        updateReason: PivotBlockUpdateReason
+    ): Boolean = {
+      newPivot.number >= currentState.pivotBlock.number && !stalePivotAfterRestart(newPivot, currentState, updateReason)
+    }
+
     def waitingForPivotBlockUpdate(updateReason: PivotBlockUpdateReason): Receive = handleCommonMessages orElse {
-      case PivotBlockSelector.Result(pivotBlockHeader) =>
+      case PivotBlockSelector.Result(pivotBlockHeader)
+          if newPivotIsGoodEnough(pivotBlockHeader, syncState, updateReason) =>
         log.info(s"New pivot block with number ${pivotBlockHeader.number} received")
-        if (pivotBlockHeader.number >= syncState.pivotBlock.number) {
-          if (pivotBlockHeader.number == syncState.pivotBlock.number && updateReason.nodeRestart) {
-            // it can happen after quick node restart than pivot block has not changed in the network. To keep whole
-            // fast sync machinery running as expected we need to make sure that we will receive better pivot than current
-            log.info("Received stale pivot after restart, asking for new pivot")
-            reScheduleAskForNewPivot(updateReason)
-          } else {
-            updatePivotSyncState(updateReason, pivotBlockHeader)
-            syncState = syncState.copy(updatingPivotBlock = false)
-            context become this.receive
-            processSyncing()
-          }
-        } else {
-          log.info("Received target block is older than old one, re-scheduling asking for new one")
-          reScheduleAskForNewPivot(updateReason)
-        }
+        updatePivotSyncState(updateReason, pivotBlockHeader)
+        context become this.receive
+        processSyncing()
+
+      case PivotBlockSelector.Result(pivotBlockHeader)
+          if !newPivotIsGoodEnough(pivotBlockHeader, syncState, updateReason) =>
+        log.info("Received pivot block is older than old one, re-scheduling asking for new one")
+        reScheduleAskForNewPivot(updateReason)
 
       case PersistSyncState => persistSyncState()
 
       case UpdatePivotBlock(state) => updatePivotBlock(state)
     }
 
-    private def updatePivotBlock(state: PivotBlockUpdateReason): Unit = {
+    private def updatePivotBlock(updateReason: PivotBlockUpdateReason): Unit = {
       if (syncState.pivotBlockUpdateFailures <= syncConfig.maximumTargetUpdateFailures) {
         if (assignedHandlers.nonEmpty || syncState.blockChainWorkQueued) {
           log.info(s"Still waiting for some responses, rescheduling pivot block update")
-          scheduler.scheduleOnce(1.second, self, UpdatePivotBlock(state))
+          scheduler.scheduleOnce(1.second, self, UpdatePivotBlock(updateReason))
           processSyncing()
         } else {
-          askForPivotBlockUpdate(state)
+          askForPivotBlockUpdate(updateReason)
         }
       } else {
         log.warning(s"Sync failure! Number of pivot block update failures reached maximum.")
@@ -253,8 +262,9 @@ class FastSync(
             // Empty root has means that there were no transactions in blockchain, and Mpt trie is empty
             // Asking for this root would result only with empty transactions
             if (syncState.pivotBlock.stateRoot == ByteString(MerklePatriciaTrie.EmptyRootHash)) {
-              syncState = syncState.copy(stateSyncFinished = true)
+              syncState = syncState.copy(stateSyncFinished = true, updatingPivotBlock = false)
             } else {
+              syncState = syncState.copy(updatingPivotBlock = false)
               stateSyncRestartRequested = false
               syncStateScheduler ! StartSyncingTo(pivotBlockHeader.stateRoot, pivotBlockHeader.number)
             }
@@ -276,7 +286,7 @@ class FastSync(
           syncState =
             syncState.updatePivotBlock(pivotBlockHeader, syncConfig.fastSyncBlockValidationX, updateFailures = true)
 
-        case NodeRestart =>
+        case SyncRestart =>
           // in case of node restart we are sure that new pivotBlockHeader > current pivotBlockHeader
           syncState = syncState.updatePivotBlock(
             pivotBlockHeader,
@@ -541,7 +551,7 @@ class FastSync(
       (info.maxBlockNumber - syncConfig.pivotBlockOffset) - state.pivotBlock.number >= syncConfig.maxPivotBlockAge
     }
 
-    private def getPeerWithTooFreshNewBlock(
+    private def getPeersWithFreshEnoughPivot(
         peers: NonEmptyList[(Peer, PeerInfo)],
         state: SyncState,
         syncConfig: SyncConfig
@@ -569,7 +579,7 @@ class FastSync(
           (peerWithBestBlockInNetwork._2.maxBlockNumber - syncConfig.pivotBlockOffset) - syncState.pivotBlock.number
 
         val peersWithTooFreshPossiblePivotBlock =
-          getPeerWithTooFreshNewBlock(NonEmptyList.fromListUnsafe(currentPeers), syncState, syncConfig)
+          getPeersWithFreshEnoughPivot(NonEmptyList.fromListUnsafe(currentPeers), syncState, syncConfig)
 
         if (peersWithTooFreshPossiblePivotBlock.isEmpty) {
           log.info(
@@ -830,7 +840,8 @@ object FastSync {
       copy(
         pivotBlock = newPivot,
         safeDownloadTarget = newPivot.number + numberOfSafeBlocks,
-        pivotBlockUpdateFailures = if (updateFailures) pivotBlockUpdateFailures + 1 else pivotBlockUpdateFailures
+        pivotBlockUpdateFailures = if (updateFailures) pivotBlockUpdateFailures + 1 else pivotBlockUpdateFailures,
+        updatingPivotBlock = false
       )
 
     def isBlockchainWorkFinished: Boolean = {
@@ -861,14 +872,14 @@ object FastSync {
   case object ImportedPivotBlock extends HeaderProcessingResult
 
   sealed abstract class PivotBlockUpdateReason {
-    def nodeRestart: Boolean = this match {
+    def isSyncRestart: Boolean = this match {
       case ImportedLastBlock => false
       case LastBlockValidationFailed => false
-      case NodeRestart => true
+      case SyncRestart => true
     }
   }
 
   case object ImportedLastBlock extends PivotBlockUpdateReason
   case object LastBlockValidationFailed extends PivotBlockUpdateReason
-  case object NodeRestart extends PivotBlockUpdateReason
+  case object SyncRestart extends PivotBlockUpdateReason
 }

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

@@ -19,8 +19,8 @@ class SyncController(
     ommersPool: ActorRef,
     etcPeerManager: ActorRef,
     syncConfig: SyncConfig,
-    externalSchedulerOpt: Option[Scheduler] = None)
-    extends Actor
+    externalSchedulerOpt: Option[Scheduler] = None
+) extends Actor
     with ActorLogging {
 
   import SyncController._
@@ -29,8 +29,8 @@ class SyncController(
 
   override def receive: Receive = idle
 
-  def idle: Receive = {
-    case Start => start()
+  def idle: Receive = { case Start =>
+    start()
   }
 
   def runningFastSync(fastSync: ActorRef): Receive = {
@@ -41,8 +41,8 @@ class SyncController(
     case other => fastSync.forward(other)
   }
 
-  def runningRegularSync(regularSync: ActorRef): Receive = {
-    case other => regularSync.forward(other)
+  def runningRegularSync(regularSync: ActorRef): Receive = { case other =>
+    regularSync.forward(other)
   }
 
   def start(): Unit = {
@@ -54,15 +54,17 @@ class SyncController(
         startFastSync()
       case (true, true) =>
         log.warning(
-          s"do-fast-sync is set to $doFastSync but fast sync cannot start because it has already been completed")
+          s"do-fast-sync is set to $doFastSync but fast sync cannot start because it has already been completed"
+        )
         startRegularSync()
       case (true, false) =>
         startRegularSync()
       case (false, false) =>
         //Check whether fast sync was started before
         if (fastSyncStateStorage.getSyncState().isDefined) {
           log.warning(
-            s"do-fast-sync is set to $doFastSync but regular sync cannot start because fast sync hasn't completed")
+            s"do-fast-sync is set to $doFastSync but regular sync cannot start because fast sync hasn't completed"
+          )
           startFastSync()
         } else
           startRegularSync()
@@ -79,14 +81,17 @@ class SyncController(
         peerEventBus,
         etcPeerManager,
         syncConfig,
-        scheduler),
-      "fast-sync")
+        scheduler
+      ),
+      "fast-sync"
+    )
     fastSync ! FastSync.Start
     context become runningFastSync(fastSync)
   }
 
   def startRegularSync(): Unit = {
-    val peersClient = context.actorOf(PeersClient.props(etcPeerManager, peerEventBus, syncConfig, scheduler), "peers-client")
+    val peersClient =
+      context.actorOf(PeersClient.props(etcPeerManager, peerEventBus, syncConfig, scheduler), "peers-client")
     val regularSync = context.actorOf(
       RegularSync.props(
         peersClient,
@@ -97,7 +102,8 @@ class SyncController(
         syncConfig,
         ommersPool,
         pendingTransactionsManager,
-        scheduler),
+        scheduler
+      ),
       "regular-sync"
     )
     regularSync ! RegularSync.Start
@@ -118,7 +124,8 @@ object SyncController {
       pendingTransactionsManager: ActorRef,
       ommersPool: ActorRef,
       etcPeerManager: ActorRef,
-      syncConfig: SyncConfig): Props =
+      syncConfig: SyncConfig
+  ): Props =
     Props(
       new SyncController(
         appStateStorage,
@@ -130,7 +137,9 @@ object SyncController {
         pendingTransactionsManager,
         ommersPool,
         etcPeerManager,
-        syncConfig))
+        syncConfig
+      )
+    )
 
   case object Start
 }

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

@@ -110,13 +110,12 @@ class SyncStateScheduler(blockchain: Blockchain, bloomFilter: BloomFilter[ByteSt
     // complex due to pruning.
     val (nodes, newState) = state.getNodesToPersist
     nodes.foreach { case (hash, (data, reqType)) =>
+      bloomFilter.put(hash)
       reqType match {
         case _: CodeRequest =>
           blockchain.storeEvmCode(hash, data).commit()
-          bloomFilter.put(hash)
         case _: NodeRequest =>
           blockchain.saveNode(hash, data.toArray, targetBlockNumber)
-          bloomFilter.put(hash)
       }
     }
     newState
@@ -245,17 +244,10 @@ class SyncStateScheduler(blockchain: Blockchain, bloomFilter: BloomFilter[ByteSt
   }
 
   private def isRequestedHashAlreadyCommitted(state: SchedulerState, req: StateNodeRequest): Boolean = {
-    if (state.memBatch.contains(req.nodeHash)) {
-      true
-    } else {
-      if (bloomFilter.mightContain(req.nodeHash)) {
-        // hash might by in db double check
-        isInDatabase(req)
-      } else {
-        // hash is definitely not known
-        false
-      }
-    }
+    state.memBatch.contains(req.nodeHash) ||
+    (bloomFilter.mightContain(req.nodeHash) && isInDatabase(
+      req
+    )) // if hash is in bloom filter we need to double check on db
   }
 }
 

src/main/scala/io/iohk/ethereum/utils/Config.scala

@@ -167,9 +167,9 @@ object Config {
         fastSyncBlockValidationX = syncConfig.getInt("fast-sync-block-validation-x"),
         maxTargetDifference = syncConfig.getInt("max-target-difference"),
         maximumTargetUpdateFailures = syncConfig.getInt("maximum-target-update-failures"),
-        stateSyncBloomFilterSize = syncConfig.getInt("state-sync-bloomFilter-size"),
-        stateSyncPersistBatchSize = syncConfig.getInt("state-sync-persistBatch-size"),
-        pivotBlockReScheduleInterval = syncConfig.getDuration("pivot-block-reSchedule-interval").toMillis.millis,
+        stateSyncBloomFilterSize = syncConfig.getInt("state-sync-bloom-filter-size"),
+        stateSyncPersistBatchSize = syncConfig.getInt("state-sync-persist-batch-size"),
+        pivotBlockReScheduleInterval = syncConfig.getDuration("pivot-block-reschedule-interval").toMillis.millis,
         maxPivotBlockAge = syncConfig.getInt("max-pivot-block-age")
       )
     }

src/test/resources/application.conf

@@ -118,11 +118,11 @@ mantis {
     min-peers-to-choose-pivot-block = 2
     peers-to-choose-pivot-block-margin = 1
     pivot-block-offset = 500
-    state-sync-bloomFilter-size = 20000
+    state-sync-bloom-filter-size = 20000
 
-    state-sync-persistBatch-size = 10000
+    state-sync-persist-batch-size = 10000
 
-    pivot-block-reSchedule-interval = 15.seconds
+    pivot-block-reschedule-interval = 15.seconds
 
     max-pivot-block-age = 96
   }