Merge pull request #78 from HorizenOfficial/dev

2.20 to master

build.sbt

@@ -25,7 +25,7 @@ lazy val commonSettings = Seq(
     Wart.OptionPartial),
   organization := "io.horizen",
   organizationName := "Zen Blockchain Foundation",
-  version := "2.1.0",
+  version := "2.2.0",
   licenses := Seq("CC0" -> url("https://creativecommons.org/publicdomain/zero/1.0/legalcode")),
   homepage := Some(url("https://github.com/HorizenOfficial/Sparkz")),
   pomExtra :=

release-notes.md

@@ -1,3 +1,7 @@
+2.2.0
+---------
+* Improved banning mechanism for deprecated nodes (nodes not updated after an hardfork will start to ban updated nodes because receiving newer blocks without a known parent - this will prevent unnecessary network traffic)
+
 2.1.0
 ---------
 * Added support for forger's connections prioritization
@@ -153,4 +157,4 @@ Hybrid chain example got bugfixing and new tests.
 * ViewSynchronizer trait removed
 * Miner and MiningController are removed from the core
 * Maven artefact has been renamed from "scorex-basics" to "scorex-core"
-* blockFields method of a Block has been removed
+* blockFields method of a Block has been removed

src/main/scala/sparkz/core/network/NodeViewSynchronizer.scala

@@ -4,7 +4,7 @@ import akka.actor.{Actor, ActorRef, ActorSystem, Props}
 import sparkz.core.NodeViewHolder.ReceivableMessages.{GetNodeViewChanges, ModifiersFromRemote, TransactionsFromRemote}
 import sparkz.core.consensus.History._
 import sparkz.core.consensus.{History, HistoryReader, SyncInfo}
-import sparkz.core.network.ModifiersStatus.Requested
+import sparkz.core.network.ModifiersStatus.{Invalid, Requested}
 import sparkz.core.network.NetworkController.ReceivableMessages.{PenalizePeer, RegisterMessageSpecs, SendToNetwork, StartConnectingPeers}
 import sparkz.core.network.NodeViewSynchronizer.ReceivableMessages._
 import sparkz.core.network.message._
@@ -126,13 +126,13 @@ class NodeViewSynchronizer[TX <: Transaction, SI <: SyncInfo, SIS <: SyncInfoMes
       deliveryTracker.setHeld(mod.id)
 
     case SyntacticallyFailedModification(mod, _) =>
-      deliveryTracker.setInvalid(mod.id).foreach(penalizeMisbehavingPeer)
+      deliveryTracker.setInvalid(mod.id).foreach(penalizeAndDisconnectPeer)
 
     case SemanticallySuccessfulModifier(mod) =>
       broadcastModifierInv(mod)
 
     case SemanticallyFailedModification(mod, _) =>
-      deliveryTracker.setInvalid(mod.id).foreach(penalizeMisbehavingPeer)
+      deliveryTracker.setInvalid(mod.id).foreach(penalizeAndDisconnectPeer)
 
     case ChangedHistory(reader: HR) =>
       historyReaderOpt = Some(reader)
@@ -316,23 +316,30 @@ class NodeViewSynchronizer[TX <: Transaction, SI <: SyncInfo, SIS <: SyncInfoMes
     * Move `pmod` to `Invalid` if it is permanently invalid, to `Received` otherwise
     */
   @SuppressWarnings(Array("org.wartremover.warts.IsInstanceOf"))
-  private def validateAndSetStatus(remote: ConnectedPeer, pmod: PMOD): Boolean = {
-    historyReaderOpt match {
-      case Some(hr) =>
-        hr.applicableTry(pmod) match {
-          case Failure(e) if e.isInstanceOf[MalformedModifierError] =>
-            log.warn(s"Modifier ${pmod.encodedId} is permanently invalid", e)
-            deliveryTracker.setInvalid(pmod.id)
-            penalizeMisbehavingPeer(remote)
-            false
-          case _ =>
-            deliveryTracker.setReceived(pmod.id, remote)
-            true
-        }
-      case None =>
-        log.error("Got modifier while history reader is not ready")
-        deliveryTracker.setReceived(pmod.id, remote)
-        true
+  private[network] def validateAndSetStatus(remote: ConnectedPeer, pmod: PMOD): Boolean = {
+    if (deliveryTracker.status(pmod.parentId) == Invalid) {
+      log.warn(s"Modifier ${pmod.encodedId} is invalid because of its parent ${pmod.parentId}")
+      deliveryTracker.setInvalid(pmod.id)
+      penalizeAndDisconnectPeer(remote)
+      false
+    } else {
+      historyReaderOpt match {
+        case Some(hr) =>
+          hr.applicableTry(pmod) match {
+            case Failure(e) if e.isInstanceOf[MalformedModifierError] =>
+              log.warn(s"Modifier ${pmod.encodedId} is permanently invalid", e)
+              deliveryTracker.setInvalid(pmod.id)
+              penalizeMisbehavingPeer(remote)
+              false
+            case _ =>
+              deliveryTracker.setReceived(pmod.id, remote)
+              true
+          }
+        case None =>
+          log.error("Got modifier while history reader is not ready")
+          deliveryTracker.setReceived(pmod.id, remote)
+          true
+      }
     }
   }
 
@@ -424,6 +431,10 @@ class NodeViewSynchronizer[TX <: Transaction, SI <: SyncInfo, SIS <: SyncInfoMes
     networkControllerRef ! PenalizePeer(peer.connectionId.remoteAddress, PenaltyType.PermanentPenalty)
   }
 
+  protected def penalizeAndDisconnectPeer(peer: ConnectedPeer): Unit = {
+    networkControllerRef ! PenalizePeer(peer.connectionId.remoteAddress, PenaltyType.DisconnectPenalty(networkSettings))
+  }
+
   /**
     * Local node sending out objects requested to remote
     */

src/test/scala/sparkz/core/network/NodeViewSynchronizerSpecification.scala

@@ -14,7 +14,7 @@ import sparkz.core.serialization.SparkzSerializer
 import sparkz.core.settings.SparkzSettings
 import sparkz.core.transaction.Transaction
 import sparkz.core.utils.NetworkTimeProvider
-import sparkz.core.{ModifierTypeId, NodeViewModifier}
+import sparkz.core.{ModifierTypeId, NodeViewModifier, PersistentNodeViewModifier}
 import sparkz.util.ModifierId
 import sparkz.util.serialization.{Reader, Writer}
 
@@ -249,7 +249,7 @@ class NodeViewSynchronizerSpecification extends NetworkTests with TestImplementa
 
   it should "when HandshakedPeer message is received, statusTracker should add the peer" in {
     // Arrange
-    val nodeViewSynchronizerRef = createNodeViewSynchronizerAsTestActorRef(settings)
+    val nodeViewSynchronizerRef = createNodeViewSynchronizerAsTestActorRef(settings)._1
     val nodeViewSynchronizer = nodeViewSynchronizerRef.underlyingActor
 
     val statusTrackerField = classOf[NodeViewSynchronizer[_, _, _, _, _, _]].getDeclaredField("statusTracker")
@@ -265,6 +265,20 @@ class NodeViewSynchronizerSpecification extends NetworkTests with TestImplementa
     peersStatus(Unknown) should be(Set(peer))
   }
 
+  it should "penalize peer for sending invalid modifiers or continuation of invalid fork" in {
+    // Arrange
+    val (nodeViewSynchronizerRef, networkControllerProbe) = createNodeViewSynchronizerAsTestActorRef(settings, ModifiersStatus.Invalid)
+    val nodeViewSynchronizer = nodeViewSynchronizerRef.underlyingActor
+    networkControllerProbe.expectMsgType[RegisterMessageSpecs]
+    networkControllerProbe.expectMsgType[Any] //StartConnectingPeers
+
+    val result = nodeViewSynchronizer
+      .validateAndSetStatus(peer, TestModifier(ModifierId @@ "2", ModifierId @@ "1"))
+
+    result shouldBe false
+    networkControllerProbe.expectMsgType[PenalizePeer]
+  }
+
   def setupHistoryAndMempoolReaders(synchronizer: ActorRef): Unit = {
     val history = new TestHistory
     val mempool = new TestMempool {
@@ -325,15 +339,15 @@ class NodeViewSynchronizerSpecification extends NetworkTests with TestImplementa
     (nodeViewSynchronizerRef, networkControllerProbe, viewHolderProbe)
   }
 
-  private def createNodeViewSynchronizerAsTestActorRef(settings: SparkzSettings): TestActorRef[NodeViewSynchronizer[_, _, _, _, _, _]] = {
+  private def createNodeViewSynchronizerAsTestActorRef(settings: SparkzSettings, modStatus: ModifiersStatus = ModifiersStatus.Requested): (TestActorRef[NodeViewSynchronizer[_, _, _, TestModifier, _, _]], TestProbe) = {
     val networkControllerProbe = TestProbe()
     val viewHolderProbe = TestProbe()
     val timeProvider = new NetworkTimeProvider(settings.ntp)
 
     val modifierSerializers: Map[ModifierTypeId, SparkzSerializer[_ <: NodeViewModifier]] =
       Map(Transaction.ModifierTypeId -> TestTransactionSerializer)
 
-    TestActorRef(Props(
+    val nvs: TestActorRef[NodeViewSynchronizer[_, _, _, TestModifier, _, _]] = TestActorRef(Props(
       new NodeViewSynchronizer[TestTransaction,
         TestSyncInfo,
         TestSyncInfoMessageSpec.type,
@@ -350,13 +364,14 @@ class NodeViewSynchronizerSpecification extends NetworkTests with TestImplementa
         modifierSerializers
       ) {
         override val deliveryTracker: DeliveryTracker = new DeliveryTracker(context.system, settings.network, self) {
-          override def status(modifierId: ModifierId): ModifiersStatus = ModifiersStatus.Requested
+          override def status(modifierId: ModifierId): ModifiersStatus = modStatus
 
           override private[network] def clearStatusForModifier(id: ModifierId, oldStatus: ModifiersStatus): Unit = {}
 
           override def setInvalid(modifierId: ModifierId): Option[ConnectedPeer] = Some(peer)
         }
       }
     ))
+    (nvs, networkControllerProbe)
   }
 }

src/test/scala/sparkz/core/network/TestImplementations.scala

@@ -31,8 +31,7 @@ trait TestImplementations {
     override type NVCT = this.type
   }
 
-  class TestModifier(val id: ModifierId) extends PersistentNodeViewModifier {
-    override def parentId: ModifierId = ???
+  case class TestModifier(id: ModifierId, parentId: ModifierId) extends PersistentNodeViewModifier {
 
     override val modifierTypeId: ModifierTypeId = ModifierTypeId @@ (0: Byte)
     override type M = this.type