Skip to content

Commit 85e6894

Browse files
dzajkowskidzajkowski
committed
[ETCM-841] Align unit tests to changes in flow of protocol negotiation
1 parent d6dab6d commit 85e6894

File tree

10 files changed

+99
-69
lines changed

10 files changed

+99
-69
lines changed

‎src/it/scala/io/iohk/ethereum/sync/util/CommonFakePeer.scala‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ abstract class CommonFakePeer(peerName: String, fakePeerCustomConfig: FakePeerCu
172172
override val capabilities: List[Capability] = blockchainConfig.capabilities
173173
}
174174

175-
lazy val handshaker: Handshaker[PeerInfo] = EtcHandshaker(handshakerConfiguration)
175+
lazy val handshaker: Capability => Handshaker[PeerInfo] = c => EtcHandshaker(handshakerConfiguration, c)
176176

177177
lazy val authHandshaker: AuthHandshaker = AuthHandshaker(nodeKey, secureRandom)
178178

@@ -193,7 +193,7 @@ abstract class CommonFakePeer(peerName: String, fakePeerCustomConfig: FakePeerCu
193193
EthereumMessageDecoder,
194194
discoveryConfig,
195195
blacklist,
196-
Capability.best(blockchainConfig.capabilities)
196+
blockchainConfig.capabilities
197197
),
198198
"peer-manager"
199199
)

‎src/it/scala/io/iohk/ethereum/txExecTest/util/DumpChainApp.scala‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ object DumpChainApp extends App with NodeKeyBuilder with SecureRandomBuilder wit
9696
override val capabilities: List[Capability] = blockchainConfig.capabilities
9797
}
9898

99-
lazy val handshaker: Handshaker[PeerInfo] = EtcHandshaker(handshakerConfiguration)
99+
lazy val handshaker: Capability => Handshaker[PeerInfo] = c => EtcHandshaker(handshakerConfiguration, c)
100100

101101
val peerMessageBus = actorSystem.actorOf(PeerEventBusActor.props)
102102

@@ -116,7 +116,7 @@ object DumpChainApp extends App with NodeKeyBuilder with SecureRandomBuilder wit
116116
messageDecoder = EthereumMessageDecoder,
117117
discoveryConfig = discoveryConfig,
118118
blacklist = blacklist,
119-
bestProtocolVersion = Capability.best(blockchainConfig.capabilities)
119+
capabilities = blockchainConfig.capabilities
120120
),
121121
"peer-manager"
122122
)

‎src/main/scala/io/iohk/ethereum/network/rlpx/FrameCodec.scala‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,15 @@ class FrameCodec(private val secrets: Secrets) {
2525

2626
private val allZerosIV = Array.fill[Byte](16)(0)
2727

28-
private val enc: StreamCipher = {
28+
//needs to be lazy to enable mocking
29+
private lazy val enc: StreamCipher = {
2930
val cipher = new SICBlockCipher(new AESEngine)
3031
cipher.init(true, new ParametersWithIV(new KeyParameter(secrets.aes), allZerosIV))
3132
cipher
3233
}
3334

34-
private val dec: StreamCipher = {
35+
//needs to be lazy to enable mocking
36+
private lazy val dec: StreamCipher = {
3537
val cipher = new SICBlockCipher(new AESEngine)
3638
cipher.init(false, new ParametersWithIV(new KeyParameter(secrets.aes), allZerosIV))
3739
cipher

‎src/main/scala/io/iohk/ethereum/network/rlpx/MessageCodec.scala‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ class MessageCodec(frameCodec: FrameCodec, messageDecoder: MessageDecoder, proto
2929
frames map { frame =>
3030
val frameData = frame.payload.toArray
3131
val payloadTry =
32-
if (remotePeer2PeerVersion >= EtcHelloExchangeState.P2pVersion) {
32+
if (remotePeer2PeerVersion >= EtcHelloExchangeState.P2pVersion && frame.`type` != Hello.code) {
3333
decompressData(frameData)
3434
} else {
3535
Success(frameData)

‎src/main/scala/io/iohk/ethereum/network/rlpx/RLPxConnectionHandler.scala‎

Lines changed: 28 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import cats.data.NonEmptyList
99
import io.iohk.ethereum.network.p2p.messages.Capability
1010
import io.iohk.ethereum.network.p2p.messages.WireProtocol.Hello
1111
import io.iohk.ethereum.network.p2p.{Message, MessageDecoder, MessageSerializable, NetworkMessageDecoder}
12-
import io.iohk.ethereum.network.rlpx.RLPxConnectionHandler.RLPxConfiguration
12+
import io.iohk.ethereum.network.rlpx.RLPxConnectionHandler.{HelloExtractor, RLPxConfiguration}
1313
import io.iohk.ethereum.utils.ByteUtils
1414
import org.bouncycastle.util.encoders.Hex
1515

@@ -33,7 +33,8 @@ class RLPxConnectionHandler(
3333
capabilities: List[Capability],
3434
authHandshaker: AuthHandshaker,
3535
messageCodecFactory: (FrameCodec, MessageDecoder, Capability, Long) => MessageCodec,
36-
rlpxConfiguration: RLPxConfiguration
36+
rlpxConfiguration: RLPxConfiguration,
37+
extractor: Secrets => HelloExtractor
3738
) extends Actor
3839
with ActorLogging {
3940

@@ -152,17 +153,15 @@ class RLPxConnectionHandler(
152153
result match {
153154
case AuthHandshakeSuccess(secrets, remotePubKey) =>
154155
log.info(s"Auth handshake succeeded for peer $peerId")
155-
val frameCodec = new FrameCodec(secrets)
156-
val frames = frameCodec.readFrames(remainingData)
156+
val e = extractor(secrets)
157157
val messageCodecOpt = for {
158-
frame <- frames.headOption
159-
hello <- extractHello(frame)
158+
r <- e.readHello(remainingData)
159+
(hello, restFrames) = r
160160
protocolVersion <- Capability.negotiate(hello.capabilities.toList, capabilities)
161161
p2pVersion = hello.p2pVersion
162-
messageCodec = messageCodecFactory(frameCodec, messageDecoder, protocolVersion, p2pVersion)
162+
messageCodec = messageCodecFactory(e.frameCodec, messageDecoder, protocolVersion, p2pVersion)
163163
_ = context.parent ! ConnectionEstablished(remotePubKey, protocolVersion)
164164
_ = context.parent ! MessageReceived(hello)
165-
restFrames = frames.drop(1)
166165
_ = {
167166
if (restFrames.nonEmpty) {
168167
val messagesSoFar = messageCodec.readFrames(restFrames) // omit hello
@@ -185,17 +184,6 @@ class RLPxConnectionHandler(
185184
context stop self
186185
}
187186

188-
private def extractHello(frame: Frame): Option[Hello] = {
189-
val frameData = frame.payload.toArray
190-
if(frame.`type` == Hello.code) {
191-
val m = NetworkMessageDecoder.fromBytes(frame.`type`, frameData, Capability.Capabilities.Eth63Capability)
192-
Some(m.asInstanceOf[Hello])
193-
} else {
194-
log.error("Unable to find 'Hello'")
195-
None
196-
}
197-
}
198-
199187
def processMessage(messageTry: Try[Message]): Unit = messageTry match {
200188
case Success(message) =>
201189
context.parent ! MessageReceived(message)
@@ -322,7 +310,9 @@ object RLPxConnectionHandler {
322310
rlpxConfiguration: RLPxConfiguration
323311
): Props =
324312
Props(
325-
new RLPxConnectionHandler(messageDecoder, capabilities, authHandshaker, messageCodecFactory, rlpxConfiguration)
313+
new RLPxConnectionHandler(
314+
messageDecoder, capabilities, authHandshaker, messageCodecFactory, rlpxConfiguration, HelloExtractor.apply
315+
)
326316
)
327317

328318
def messageCodecFactory(
@@ -358,4 +348,22 @@ object RLPxConnectionHandler {
358348
val waitForTcpAckTimeout: FiniteDuration
359349
}
360350

351+
case class HelloExtractor(secrets: Secrets) {
352+
lazy val frameCodec = new FrameCodec(secrets)
353+
354+
def readHello(remainingData: ByteString): Option[(Hello, Seq[Frame])] = {
355+
val frames = frameCodec.readFrames(remainingData)
356+
frames.headOption.flatMap(extractHello).map(h => (h, frames.drop(1)))
357+
}
358+
}
359+
360+
private def extractHello(frame: Frame): Option[Hello] = {
361+
val frameData = frame.payload.toArray
362+
if(frame.`type` == Hello.code) {
363+
val m = NetworkMessageDecoder.fromBytes(frame.`type`, frameData, Capability.Capabilities.Eth63Capability)
364+
Some(m.asInstanceOf[Hello])
365+
} else {
366+
None
367+
}
368+
}
361369
}

‎src/test/scala/io/iohk/ethereum/network/PeerActorHandshakingSpec.scala‎

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package io.iohk.ethereum.network
22

33
import java.net.{InetSocketAddress, URI}
4-
54
import akka.actor.{ActorSystem, Props}
65
import akka.testkit.{TestActorRef, TestProbe}
76
import akka.util.ByteString
@@ -16,7 +15,7 @@ import io.iohk.ethereum.network.handshaker._
1615
import io.iohk.ethereum.network.p2p.Message
1716
import io.iohk.ethereum.network.p2p.messages.Capability.Capabilities._
1817
import io.iohk.ethereum.network.p2p.messages.BaseETH6XMessages.Status
19-
import io.iohk.ethereum.network.p2p.messages.ProtocolVersions
18+
import io.iohk.ethereum.network.p2p.messages.{Capability, ProtocolVersions}
2019
import io.iohk.ethereum.network.p2p.messages.WireProtocol.{Disconnect, Hello, Pong}
2120
import io.iohk.ethereum.network.rlpx.RLPxConnectionHandler
2221
import io.iohk.ethereum.utils.Config
@@ -31,12 +30,12 @@ class PeerActorHandshakingSpec extends AnyFlatSpec with Matchers {
3130
import DefaultValues._
3231

3332
val peerActorHandshakeSucceeds =
34-
peerActor(MockHandshakerAlwaysSucceeds(defaultStatus, defaultBlockNumber, defaultForkAccepted))
33+
peerActor(_ => MockHandshakerAlwaysSucceeds(defaultStatus, defaultBlockNumber, defaultForkAccepted))
3534

3635
//Establish probe rlpxconnection
3736
peerActorHandshakeSucceeds ! ConnectTo(uri)
3837
rlpxConnectionProbe.expectMsg(RLPxConnectionHandler.ConnectTo(uri))
39-
rlpxConnectionProbe.reply(RLPxConnectionHandler.ConnectionEstablished(ByteString()))
38+
rlpxConnectionProbe.reply(RLPxConnectionHandler.ConnectionEstablished(ByteString(), ProtocolVersions.PV63))
4039

4140
//Test that the handshake succeeded
4241
val sender = TestProbe()(system)
@@ -48,12 +47,12 @@ class PeerActorHandshakingSpec extends AnyFlatSpec with Matchers {
4847

4948
import DefaultValues._
5049

51-
val peerActorHandshakeFails = peerActor(MockHandshakerAlwaysFails(defaultReasonDisconnect))
50+
val peerActorHandshakeFails = peerActor(_ => MockHandshakerAlwaysFails(defaultReasonDisconnect))
5251

5352
//Establish probe rlpxconnection
5453
peerActorHandshakeFails ! ConnectTo(uri)
5554
rlpxConnectionProbe.expectMsg(RLPxConnectionHandler.ConnectTo(uri))
56-
rlpxConnectionProbe.reply(RLPxConnectionHandler.ConnectionEstablished(ByteString()))
55+
rlpxConnectionProbe.reply(RLPxConnectionHandler.ConnectionEstablished(ByteString(), ProtocolVersions.PV63))
5756

5857
//Test that the handshake failed
5958
rlpxConnectionProbe.expectMsg(RLPxConnectionHandler.SendMessage(Disconnect(defaultReasonDisconnect)))
@@ -64,12 +63,12 @@ class PeerActorHandshakingSpec extends AnyFlatSpec with Matchers {
6463

6564
import DefaultValues._
6665

67-
val peerActorHandshakeRequiresHello = peerActor(MockHandshakerRequiresHello())
66+
val peerActorHandshakeRequiresHello = peerActor(_ => MockHandshakerRequiresHello())
6867

6968
//Establish probe rlpxconnection
7069
peerActorHandshakeRequiresHello ! ConnectTo(uri)
7170
rlpxConnectionProbe.expectMsg(RLPxConnectionHandler.ConnectTo(uri))
72-
rlpxConnectionProbe.reply(RLPxConnectionHandler.ConnectionEstablished(ByteString()))
71+
rlpxConnectionProbe.reply(RLPxConnectionHandler.ConnectionEstablished(ByteString(), ProtocolVersions.PV63))
7372

7473
rlpxConnectionProbe.expectMsg(RLPxConnectionHandler.SendMessage(defaultHello))
7574
peerActorHandshakeRequiresHello ! RLPxConnectionHandler.MessageReceived(defaultHello)
@@ -84,12 +83,12 @@ class PeerActorHandshakingSpec extends AnyFlatSpec with Matchers {
8483

8584
import DefaultValues._
8685

87-
val peerActorHandshakeRequiresHello = peerActor(MockHandshakerRequiresHello())
86+
val peerActorHandshakeRequiresHello = peerActor(_ => MockHandshakerRequiresHello())
8887

8988
//Establish probe rlpxconnection
9089
peerActorHandshakeRequiresHello ! ConnectTo(uri)
9190
rlpxConnectionProbe.expectMsg(RLPxConnectionHandler.ConnectTo(uri))
92-
rlpxConnectionProbe.reply(RLPxConnectionHandler.ConnectionEstablished(ByteString()))
91+
rlpxConnectionProbe.reply(RLPxConnectionHandler.ConnectionEstablished(ByteString(), ProtocolVersions.PV63))
9392

9493
rlpxConnectionProbe.expectMsg(RLPxConnectionHandler.SendMessage(defaultHello))
9594
time.advance(defaultTimeout * 2)
@@ -102,12 +101,12 @@ class PeerActorHandshakingSpec extends AnyFlatSpec with Matchers {
102101

103102
import DefaultValues._
104103

105-
val peerActorHandshakeRequiresHello = peerActor(MockHandshakerRequiresHello())
104+
val peerActorHandshakeRequiresHello = peerActor(_ => MockHandshakerRequiresHello())
106105

107106
//Establish probe rlpxconnection
108107
peerActorHandshakeRequiresHello ! ConnectTo(uri)
109108
rlpxConnectionProbe.expectMsg(RLPxConnectionHandler.ConnectTo(uri))
110-
rlpxConnectionProbe.reply(RLPxConnectionHandler.ConnectionEstablished(ByteString()))
109+
rlpxConnectionProbe.reply(RLPxConnectionHandler.ConnectionEstablished(ByteString(), ProtocolVersions.PV63))
111110

112111
rlpxConnectionProbe.expectMsg(RLPxConnectionHandler.SendMessage(defaultHello))
113112
peerActorHandshakeRequiresHello ! RLPxConnectionHandler.MessageReceived(defaultStatusMsg)
@@ -120,12 +119,12 @@ class PeerActorHandshakingSpec extends AnyFlatSpec with Matchers {
120119

121120
import DefaultValues._
122121

123-
val peerActorHandshakeRequiresHello = peerActor(MockHandshakerRequiresHello())
122+
val peerActorHandshakeRequiresHello = peerActor(_ => MockHandshakerRequiresHello())
124123

125124
//Establish probe rlpxconnection
126125
peerActorHandshakeRequiresHello ! ConnectTo(uri)
127126
rlpxConnectionProbe.expectMsg(RLPxConnectionHandler.ConnectTo(uri))
128-
rlpxConnectionProbe.reply(RLPxConnectionHandler.ConnectionEstablished(ByteString()))
127+
rlpxConnectionProbe.reply(RLPxConnectionHandler.ConnectionEstablished(ByteString(), ProtocolVersions.PV63))
129128

130129
rlpxConnectionProbe.expectMsg(RLPxConnectionHandler.SendMessage(defaultHello))
131130
peerActorHandshakeRequiresHello ! RLPxConnectionHandler.MessageReceived(Pong()) //Ignored
@@ -151,7 +150,7 @@ class PeerActorHandshakingSpec extends AnyFlatSpec with Matchers {
151150
val peerMessageBus = TestProbe()
152151
val knownNodesManager = TestProbe()
153152

154-
def peerActor(handshaker: Handshaker[PeerInfo]): TestActorRef[PeerActor[PeerInfo]] = TestActorRef(
153+
def peerActor(handshaker: Capability => Handshaker[PeerInfo]): TestActorRef[PeerActor[PeerInfo]] = TestActorRef(
155154
Props(
156155
new PeerActor(
157156
new InetSocketAddress("127.0.0.1", 0),

‎src/test/scala/io/iohk/ethereum/network/handshaker/EtcHandshakerSpec.scala‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -98,9 +98,9 @@ class EtcHandshakerSpec extends AnyFlatSpec with Matchers {
9898
bestHash = firstBlock.header.hash
9999
)
100100

101-
initHandshakerWithoutResolver.nextMessage.map(_.messageToSend) shouldBe Right(localHello: HelloEnc)
101+
initHandshakerWithoutResolverETC64.nextMessage.map(_.messageToSend) shouldBe Right(localHello: HelloEnc)
102102

103-
val handshakerAfterHelloOpt = initHandshakerWithoutResolver.applyMessage(remoteHello)
103+
val handshakerAfterHelloOpt = initHandshakerWithoutResolverETC64.applyMessage(remoteHello)
104104
assert(handshakerAfterHelloOpt.isDefined)
105105
handshakerAfterHelloOpt.get.nextMessage.map(_.messageToSend.underlyingMsg) shouldBe Right(newLocalStatusMsg)
106106

@@ -232,7 +232,7 @@ class EtcHandshakerSpec extends AnyFlatSpec with Matchers {
232232
it should "fail if the remote peer doesn't support PV63/PV64" in new RemotePeerPV63Setup {
233233
val pv62Capability = ProtocolVersions.PV62
234234
val handshakerAfterHelloOpt =
235-
initHandshakerWithResolver.applyMessage(remoteHello.copy(capabilities = Seq(pv62Capability)))
235+
initHandshakerWithResolverETH62.applyMessage(remoteHello.copy(capabilities = Seq(pv62Capability)))
236236
assert(handshakerAfterHelloOpt.isDefined)
237237
handshakerAfterHelloOpt.get.nextMessage.leftSide shouldBe Left(
238238
HandshakeFailure(Disconnect.Reasons.IncompatibleP2pProtocolVersion)
@@ -286,9 +286,15 @@ class EtcHandshakerSpec extends AnyFlatSpec with Matchers {
286286
}
287287

288288
val initHandshakerWithoutResolver = EtcHandshaker(
289-
new MockEtcHandshakerConfiguration(List(ProtocolVersions.PV64, ProtocolVersions.PV63))
289+
new MockEtcHandshakerConfiguration(List(ProtocolVersions.PV64, ProtocolVersions.PV63)), ProtocolVersions.PV63
290+
)
291+
val initHandshakerWithoutResolverETC64 = EtcHandshaker(
292+
new MockEtcHandshakerConfiguration(List(ProtocolVersions.PV64, ProtocolVersions.PV63)), ProtocolVersions.PV64
290293
)
291-
val initHandshakerWithResolver = EtcHandshaker(etcHandshakerConfigurationWithResolver)
294+
295+
val initHandshakerWithResolver = EtcHandshaker(etcHandshakerConfigurationWithResolver, ProtocolVersions.PV63)
296+
297+
val initHandshakerWithResolverETH62 = EtcHandshaker(etcHandshakerConfigurationWithResolver, ProtocolVersions.PV62)
292298

293299
val firstBlock =
294300
genesisBlock.copy(header = genesisBlock.header.copy(parentHash = genesisBlock.header.hash, number = 1))

‎src/test/scala/io/iohk/ethereum/network/p2p/MessageCodecSpec.scala‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package io.iohk.ethereum.network.p2p
22

33
import akka.util.ByteString
44
import io.iohk.ethereum.network.handshaker.EtcHelloExchangeState
5+
import io.iohk.ethereum.network.handshaker.EtcHelloExchangeState.P2pVersion
56
import io.iohk.ethereum.network.p2p.messages.Capability.Capabilities._
67
import io.iohk.ethereum.network.p2p.messages.BaseETH6XMessages.Status
78
import io.iohk.ethereum.network.p2p.messages.ProtocolVersions
@@ -26,6 +27,9 @@ class MessageCodecSpec extends AnyFlatSpec with Matchers {
2627
}
2728

2829
it should "compress messages when remote side advertises p2p version larger or equal 5" in new TestSetup {
30+
override lazy val negotiatedRemoteP2PVersion: Long = 5L
31+
override lazy val negotiatedLocalP2PVersion: Long = 4L
32+
2933
val remoteHello = remoteMessageCodec.encodeMessage(helloV5)
3034
val localReceivedRemoteHello = messageCodec.readMessages(remoteHello)
3135

@@ -73,6 +77,8 @@ class MessageCodecSpec extends AnyFlatSpec with Matchers {
7377
trait TestSetup extends SecureChannelSetup {
7478
val frameCodec = new FrameCodec(secrets)
7579
val remoteFrameCodec = new FrameCodec(remoteSecrets)
80+
lazy val negotiatedRemoteP2PVersion: Long = 5L
81+
lazy val negotiatedLocalP2PVersion: Long = 5L
7682

7783
val helloV5 = Hello(
7884
p2pVersion = EtcHelloExchangeState.P2pVersion,
@@ -94,8 +100,8 @@ class MessageCodecSpec extends AnyFlatSpec with Matchers {
94100

95101
val decoder = NetworkMessageDecoder orElse EthereumMessageDecoder
96102

97-
val messageCodec = new MessageCodec(frameCodec, decoder, ProtocolVersions.PV63)
98-
val remoteMessageCodec = new MessageCodec(remoteFrameCodec, decoder, ProtocolVersions.PV63)
103+
val messageCodec = new MessageCodec(frameCodec, decoder, ProtocolVersions.PV63, negotiatedLocalP2PVersion)
104+
val remoteMessageCodec = new MessageCodec(remoteFrameCodec, decoder, ProtocolVersions.PV63, negotiatedRemoteP2PVersion)
99105

100106
}
101107

0 commit comments

Comments
 (0)