Skip to content

Commit 127524c

Browse files
lukasz-golebiewskilukasz-golebiewski
authored
[ETCM-354] Peer fork ID validation (#1024)
* [ETCM-354] Peer fork ID validation * [ETCM-354] Apply review suggestions * [ETCM-354] Update sbt sha * [ETCM-354] Add scaladoc for validatePeer * [ETCM-354] Implement another round of review suggestions * [ETCM-354] Fix typo * [ETCM-354] More review suggestions
1 parent 4425097 commit 127524c

File tree

4 files changed

+245
-2
lines changed

4 files changed

+245
-2
lines changed

‎nix/overlay.nix

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ rev: final: prev: {
33

44
mantis = final.callPackage ./mantis.nix {
55
src = ../.;
6-
depsSha256 = "sha256-QXIsF46M9gKhLD8EWMsyxDRWQNzgzWv3pctRJEEHbYM=";
6+
depsSha256 = "sha256-vzp0pLLhuXFvb+DIVFeiIviBho6K0e5Xymo617EgIm8=";
77
};
88

99
mantis-hash = final.mantis.override {

‎project/Dependencies.scala

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,9 @@ object Dependencies {
9494
"ch.qos.logback" % "logback-classic" % "1.2.3",
9595
"com.typesafe.scala-logging" %% "scala-logging" % "3.9.2",
9696
"net.logstash.logback" % "logstash-logback-encoder" % "6.4",
97-
"org.codehaus.janino" % "janino" % "3.1.2"
97+
"org.codehaus.janino" % "janino" % "3.1.2",
98+
"org.typelevel" %% "log4cats-core" % "2.1.1",
99+
"org.typelevel" %% "log4cats-slf4j" % "1.3.1"
98100
)
99101

100102
val crypto = Seq("org.bouncycastle" % "bcprov-jdk15on" % "1.66")

‎src/main/scala/io/iohk/ethereum/forkid/ForkIdValidator.scala

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
package io.iohk.ethereum.forkid
2+
3+
import akka.util.ByteString
4+
import cats.Monad
5+
import cats.data.EitherT._
6+
import cats.implicits._
7+
import io.iohk.ethereum.utils.BigIntExtensionMethods._
8+
import io.iohk.ethereum.utils.BlockchainConfig
9+
import io.iohk.ethereum.utils.ByteUtils._
10+
import monix.eval.Task
11+
import org.typelevel.log4cats.Logger
12+
import org.typelevel.log4cats.slf4j.Slf4jLogger
13+
14+
import java.util.zip.CRC32
15+
16+
sealed trait ForkIdValidationResult
17+
case object Connect extends ForkIdValidationResult
18+
case object ErrRemoteStale extends ForkIdValidationResult
19+
case object ErrLocalIncompatibleOrStale extends ForkIdValidationResult
20+
21+
object ForkIdValidator {
22+
23+
implicit val unsafeLogger = Slf4jLogger.getLogger[Task]
24+
25+
val maxUInt64 = (BigInt(0x7fffffffffffffffL) << 1) + 1 // scalastyle:ignore magic.number
26+
27+
/** Tells whether it makes sense to connect to a peer or gives a reason why it isn't a good idea.
28+
*
29+
* @param genesisHash - hash of the genesis block of the current chain
30+
* @param config - local client's blockchain configuration
31+
* @param currentHeight - number of the block at the current tip
32+
* @param remoteId - ForkId announced by the connecting peer
33+
* @return One of:
34+
* - [[io.iohk.ethereum.forkid.Connect]] - It is safe to connect to the peer
35+
* - [[io.iohk.ethereum.forkid.ErrRemoteStale]] - Remote is stale, don't connect
36+
* - [[io.iohk.ethereum.forkid.ErrLocalIncompatibleOrStale]] - Local is incompatible or stale, don't connect
37+
*/
38+
def validatePeer[F[_]: Monad: Logger](
39+
genesisHash: ByteString,
40+
config: BlockchainConfig
41+
)(currentHeight: BigInt, remoteForkId: ForkId): F[ForkIdValidationResult] = {
42+
val forks = ForkId.gatherForks(config)
43+
validatePeer[F](genesisHash, forks)(currentHeight, remoteForkId)
44+
}
45+
46+
private[forkid] def validatePeer[F[_]: Monad: Logger](
47+
genesisHash: ByteString,
48+
forks: List[BigInt]
49+
)(currentHeight: BigInt, remoteId: ForkId): F[ForkIdValidationResult] = {
50+
val checksums: Vector[BigInt] = calculateChecksums(genesisHash, forks)
51+
52+
// find the first unpassed fork and it's index
53+
val (unpassedFork, unpassedForkIndex) =
54+
forks.zipWithIndex.find { case (fork, _) => currentHeight < fork }.getOrElse((maxUInt64, forks.length))
55+
56+
// The checks are left biased -> whenever a result is found we need to short circuit
57+
val validate = (for {
58+
_ <- liftF(Logger[F].trace(s"Before checkMatchingHashes"))
59+
matching <- fromEither[F](
60+
checkMatchingHashes(checksums(unpassedForkIndex), remoteId, currentHeight).toLeft("hashes didn't match")
61+
)
62+
_ <- liftF(Logger[F].trace(s"checkMatchingHashes result: $matching"))
63+
_ <- liftF(Logger[F].trace(s"Before checkSubset"))
64+
sub <- fromEither[F](checkSubset(checksums, forks, remoteId, unpassedForkIndex).toLeft("not in subset"))
65+
_ <- liftF(Logger[F].trace(s"checkSubset result: $sub"))
66+
_ <- liftF(Logger[F].trace(s"Before checkSuperset"))
67+
sup <- fromEither[F](checkSuperset(checksums, remoteId, unpassedForkIndex).toLeft("not in superset"))
68+
_ <- liftF(Logger[F].trace(s"checkSuperset result: $sup"))
69+
_ <- liftF(Logger[F].trace(s"No check succeeded"))
70+
_ <- fromEither[F](Either.left[ForkIdValidationResult, Unit](ErrLocalIncompatibleOrStale))
71+
} yield ()).value
72+
73+
for {
74+
_ <- Logger[F].debug(s"Validating $remoteId")
75+
_ <- Logger[F].trace(s" list: $forks")
76+
_ <- Logger[F].trace(s"Unpassed fork $unpassedFork was found at index $unpassedForkIndex")
77+
res <- validate.map(_.swap)
78+
_ <- Logger[F].debug(s"Validation result is: $res")
79+
} yield (res.getOrElse(Connect))
80+
}
81+
82+
private def calculateChecksums(
83+
genesisHash: ByteString,
84+
forks: List[BigInt]
85+
): Vector[BigInt] = {
86+
val crc = new CRC32()
87+
crc.update(genesisHash.asByteBuffer)
88+
val genesisChecksum = BigInt(crc.getValue())
89+
90+
genesisChecksum +: (forks.map { fork =>
91+
crc.update(bigIntToBytes(fork, 8))
92+
BigInt(crc.getValue())
93+
}).toVector
94+
}
95+
96+
/**
97+
* 1) If local and remote FORK_HASH matches, compare local head to FORK_NEXT.
98+
* The two nodes are in the same fork state currently.
99+
* They might know of differing future forks, but that’s not relevant until the fork triggers (might be postponed, nodes might be updated to match).
100+
* 1a) A remotely announced but remotely not passed block is already passed locally, disconnect, since the chains are incompatible.
101+
* 1b) No remotely announced fork; or not yet passed locally, connect.
102+
*/
103+
private def checkMatchingHashes(
104+
checksum: BigInt,
105+
remoteId: ForkId,
106+
currentHeight: BigInt
107+
): Option[ForkIdValidationResult] =
108+
remoteId match {
109+
case ForkId(hash, _) if checksum != hash => None
110+
case ForkId(_, Some(next)) if currentHeight >= next => Some(ErrLocalIncompatibleOrStale)
111+
case _ => Some(Connect)
112+
}
113+
114+
/**
115+
* 2) If the remote FORK_HASH is a subset of the local past forks and the remote FORK_NEXT matches with the locally following fork block number, connect.
116+
* Remote node is currently syncing. It might eventually diverge from us, but at this current point in time we don’t have enough information.
117+
*/
118+
def checkSubset(
119+
checksums: Vector[BigInt],
120+
forks: List[BigInt],
121+
remoteId: ForkId,
122+
i: Int
123+
): Option[ForkIdValidationResult] =
124+
checksums
125+
.zip(forks)
126+
.take(i)
127+
.collectFirst {
128+
case (sum, fork) if sum == remoteId.hash => if (fork == remoteId.next.getOrElse(0)) Connect else ErrRemoteStale
129+
}
130+
131+
/**
132+
* 3) If the remote FORK_HASH is a superset of the local past forks and can be completed with locally known future forks, connect.
133+
* Local node is currently syncing. It might eventually diverge from the remote, but at this current point in time we don’t have enough information.
134+
*/
135+
def checkSuperset(checksums: Vector[BigInt], remoteId: ForkId, i: Int): Option[ForkIdValidationResult] = {
136+
checksums.drop(i).collectFirst { case sum if sum == remoteId.hash => Connect }
137+
}
138+
139+
}

‎src/test/scala/io/iohk/ethereum/forkid/ForkIdValidatorSpec.scala

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
package io.iohk.ethereum.forkid
2+
3+
import akka.util.ByteString
4+
import io.iohk.ethereum.forkid.ForkId._
5+
import io.iohk.ethereum.utils.Config._
6+
import io.iohk.ethereum.utils.ForkBlockNumbers
7+
import monix.eval.Task
8+
import monix.execution.Scheduler.Implicits.global
9+
import org.bouncycastle.util.encoders.Hex
10+
import org.scalatest.matchers.should._
11+
import org.scalatest.wordspec.AnyWordSpec
12+
13+
import scala.concurrent.duration._
14+
15+
import ForkIdValidator._
16+
17+
class ForkIdValidatorSpec extends AnyWordSpec with Matchers {
18+
19+
val config = blockchains
20+
21+
val ethGenesisHash = ByteString(Hex.decode("d4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3"))
22+
23+
"ForkIdValidator" must {
24+
"correctly validate ETH peers" in {
25+
// latest fork at the time of writing those assertions (in the spec) was Petersburg
26+
val ethForksList: List[BigInt] = List(1150000, 1920000, 2463000, 2675000, 4370000, 7280000)
27+
28+
def validatePeer(head: BigInt, remoteForkId: ForkId) =
29+
ForkIdValidator
30+
.validatePeer[Task](ethGenesisHash, ethForksList)(head, remoteForkId)
31+
.runSyncUnsafe(Duration(1, SECONDS))
32+
33+
// Local is mainnet Petersburg, remote announces the same. No future fork is announced.
34+
validatePeer(7987396, ForkId(0x668db0afL, None)) shouldBe Connect
35+
36+
// Local is mainnet Petersburg, remote announces the same. Remote also announces a next fork
37+
// at block 0xffffffff, but that is uncertain.
38+
validatePeer(7279999, ForkId(0xa00bc324L, Some(ForkIdValidator.maxUInt64))) shouldBe Connect
39+
40+
// Local is mainnet currently in Byzantium only (so it's aware of Petersburg), remote announces
41+
// also Byzantium, and it's also aware of Petersburg (e.g. updated node before the fork). We
42+
// don't know if Petersburg passed yet (will pass) or not.
43+
validatePeer(7279999, ForkId(0xa00bc324L, Some(7280000))) shouldBe Connect
44+
45+
// Local is mainnet Petersburg, remote announces the same. Remote also announces a next fork
46+
// at block 0xffffffff, but that is uncertain.
47+
validatePeer(7987396, ForkId(0x668db0afL, Some(ForkIdValidator.maxUInt64))) shouldBe Connect
48+
49+
// Local is mainnet currently in Byzantium only (so it's aware of Petersburg), remote announces
50+
// also Byzantium, but it's not yet aware of Petersburg (e.g. non updated node before the fork).
51+
// In this case we don't know if Petersburg passed yet or not.
52+
validatePeer(7279999, ForkId(0xa00bc324L, None)) shouldBe Connect
53+
54+
validatePeer(7279999, ForkId(0xa00bc324L, Some(7280000))) shouldBe Connect
55+
56+
// Local is mainnet currently in Byzantium only (so it's aware of Petersburg), remote announces
57+
// also Byzantium, and it's also aware of some random fork (e.g. misconfigured Petersburg). As
58+
// neither forks passed at neither nodes, they may mismatch, but we still connect for now.
59+
validatePeer(7279999, ForkId(0xa00bc324L, Some(ForkIdValidator.maxUInt64))) shouldBe Connect
60+
61+
// Local is mainnet Petersburg, remote announces Byzantium + knowledge about Petersburg. Remote
62+
// is simply out of sync, accept.
63+
validatePeer(7987396, ForkId(0xa00bc324L, Some(7280000))) shouldBe Connect
64+
65+
// Local is mainnet Petersburg, remote announces Spurious + knowledge about Byzantium. Remote
66+
// is definitely out of sync. It may or may not need the Petersburg update, we don't know yet.
67+
validatePeer(7987396, ForkId(0x3edd5b10L, Some(4370000))) shouldBe Connect
68+
69+
// Local is mainnet Byzantium, remote announces Petersburg. Local is out of sync, accept.
70+
validatePeer(7279999, ForkId(0x668db0afL, None)) shouldBe Connect
71+
72+
// Local is mainnet Spurious, remote announces Byzantium, but is not aware of Petersburg. Local
73+
// out of sync. Local also knows about a future fork, but that is uncertain yet.
74+
validatePeer(4369999, ForkId(0xa00bc324L, None)) shouldBe Connect
75+
76+
// Local is mainnet Petersburg. remote announces Byzantium but is not aware of further forks.
77+
// Remote needs software update.
78+
validatePeer(7987396, ForkId(0xa00bc324L, None)) shouldBe ErrRemoteStale
79+
80+
// Local is mainnet Petersburg, and isn't aware of more forks. Remote announces Petersburg +
81+
// 0xffffffff. Local needs software update, reject.
82+
validatePeer(7987396, ForkId(0x5cddc0e1L, None)) shouldBe ErrLocalIncompatibleOrStale
83+
84+
// Local is mainnet Byzantium, and is aware of Petersburg. Remote announces Petersburg +
85+
// 0xffffffff. Local needs software update, reject.
86+
validatePeer(7279999, ForkId(0x5cddc0e1L, None)) shouldBe ErrLocalIncompatibleOrStale
87+
88+
// Local is mainnet Petersburg, remote is Rinkeby Petersburg.
89+
validatePeer(7987396, ForkId(0xafec6b27L, None)) shouldBe ErrLocalIncompatibleOrStale
90+
91+
// Local is mainnet Petersburg, far in the future. Remote announces Gopherium (non existing fork)
92+
// at some future block 88888888, for itself, but past block for local. Local is incompatible.
93+
//
94+
// This case detects non-upgraded nodes with majority hash power (typical Ropsten mess).
95+
validatePeer(88888888, ForkId(0x668db0afL, Some(88888888))) shouldBe ErrLocalIncompatibleOrStale
96+
97+
// Local is mainnet Byzantium. Remote is also in Byzantium, but announces Gopherium (non existing
98+
// fork) at block 7279999, before Petersburg. Local is incompatible.
99+
validatePeer(7279999, ForkId(0xa00bc324L, Some(7279999L))) shouldBe ErrLocalIncompatibleOrStale
100+
}
101+
}
102+
}

0 commit comments

Comments
 (0)