fixes wrong logic

eth_p2p.nim

@@ -35,7 +35,7 @@ proc newEthereumNode*(keys: KeyPair,
                       chain: AbstractChainDB,
                       clientId = "nim-eth-p2p/0.2.0", # TODO: read this value from nimble somehow
                       addAllCapabilities = true,
-                      useSnappyCompression: bool = false): EthereumNode =
+                      useCompression: bool = false): EthereumNode =
   new result
   result.keys = keys
   result.networkId = networkId
@@ -46,7 +46,7 @@ proc newEthereumNode*(keys: KeyPair,
   result.connectionState = ConnectionState.None
 
   when useSnappy:
-    if useSnappyCompression:
+    if useCompression:
       result.protocolVersion = devp2pSnappyVersion
     else:
       result.protocolVersion = devp2pVersion

eth_p2p/mock_peers.nim

@@ -28,6 +28,8 @@ type
 
     expectedMsgs: Deque[ExpectedMsg]
     receivedMsgsCount: int
+    when useSnappy:
+      useCompression*: bool
 
 var
   nextUnusedMockPort = 40304
@@ -152,6 +154,12 @@ macro expect*(mock: MockConf, MsgType: type, handler: untyped = nil): untyped =
     newCall(bindSym"makeProtoMsgPair", MsgType.getType),
     newCall(bindSym"toAction", handler))
 
+template compression(m: MockConf): bool =
+  when useSnappy:
+    m.useCompression
+  else:
+    false
+
 proc newMockPeer*(userConfigurator: proc (m: MockConf)): EthereumNode =
   var mockConf = new MockConf
   mockConf.keys = newKeyPair()
@@ -169,7 +177,8 @@ proc newMockPeer*(userConfigurator: proc (m: MockConf)): EthereumNode =
                              mockConf.networkId,
                              mockConf.chain,
                              mockConf.clientId,
-                             addAllCapabilities = false)
+                             addAllCapabilities = false,
+                             mockConf.compression())
 
   mockConf.handshakes.sort do (lhs, rhs: ExpectedMsg) -> int:
     # this is intentially sorted in reverse order, so we

eth_p2p/rlpx.nim

@@ -261,7 +261,9 @@ proc linkSendFailureToReqFuture[S, R](sendFut: Future[S], resFut: Future[R]) =
 
 template compressMsg(peer: Peer, data: Bytes): Bytes =
   when useSnappy:
-    if peer.snappyEnabled: snappy.compress(data)
+    if peer.snappyEnabled and
+      peer.network.protocolVersion == devp2pSnappyVersion:
+      snappy.compress(data)
     else: data
   else:
     data
@@ -422,7 +424,8 @@ proc recvMsg*(peer: Peer): Future[tuple[msgId: int, msgData: Rlp]] {.async.} =
   decryptedBytes.setLen(decryptedBytesCount)
 
   when useSnappy:
-    if peer.network.protocolVersion == devp2pSnappyVersion:
+    if peer.network.protocolVersion == devp2pSnappyVersion and
+      peer.snappyEnabled:
       decryptedBytes = snappy.uncompress(decryptedBytes)
       if decryptedBytes.len == 0:
         await peer.disconnectAndRaise(BreachOfProtocol,
@@ -1189,7 +1192,14 @@ template baseProtocolVersion(node: EthereumNode): untyped =
   else:
     devp2pVersion
 
-template checkPeerCap(peer: Peer, handshake: HandShake) =
+template baseProtocolVersion(node: EthereumNode, peer: Peer): untyped =
+  when useSnappy:
+    if peer.snappyEnabled: node.protocolVersion
+    else: devp2pVersion
+  else:
+    devp2pVersion
+
+template checkPeerProtocolVersion(peer: Peer, handshake: HandShake) =
   when useSnappy:
     peer.snappyEnabled = handshake.version >= devp2pSnappyVersion.uint
 
@@ -1201,7 +1211,7 @@ template getVersion(handshake: HandShake): uint =
 
 template baseProtocolVersion(peer: Peer): uint =
   when useSnappy:
-    if result.snappyEnabled: devp2pSnappyVersion
+    if peer.snappyEnabled: devp2pSnappyVersion
     else: devp2pVersion
   else:
     devp2pVersion
@@ -1215,7 +1225,6 @@ proc rlpxConnect*(node: EthereumNode, remote: Node): Future[Peer] {.async.} =
   var ok = false
   try:
     result.transport = await connect(ta)
-
     var handshake = newHandshake({Initiator, EIP8}, int(node.baseProtocolVersion()))
     handshake.host = node.keys
 
@@ -1238,7 +1247,7 @@ proc rlpxConnect*(node: EthereumNode, remote: Node): Future[Peer] {.async.} =
       ret = handshake.decodeAckMessage(ackMsg)
     check ret
 
-    result.checkPeerCap(handshake)
+    result.checkPeerProtocolVersion(handshake)
     initSecretState(handshake, ^authMsg, ackMsg, result)
 
     # if handshake.remoteHPubkey != remote.node.pubKey:
@@ -1300,8 +1309,8 @@ proc rlpxAccept*(node: EthereumNode,
       ret = handshake.decodeAuthMessage(authMsg)
     check ret
 
-    result.checkPeerCap(handshake)
+    result.checkPeerProtocolVersion(handshake)
-    handshake.version = uint8(node.baseProtocolVersion())
+    handshake.version = uint8(node.baseProtocolVersion(result))
 
     var ackMsg: array[AckMessageMaxEIP8, byte]
     var ackMsgLen: int

tests/tserver.nim

@@ -90,10 +90,10 @@ template asyncTest(name, body: untyped) =
     waitFor scenario()
 
 asyncTest "network with 3 peers using custom protocols":
-  const usecompression = defined(useSnappy)
+  const useCompression = defined(useSnappy)
   let localKeys = newKeyPair()
   let localAddress = localAddress(30303)
-  var localNode = newEthereumNode(localKeys, localAddress, 1, nil, useSnappyCompression = useCompression)
+  var localNode = newEthereumNode(localKeys, localAddress, 1, nil, useCompression = useCompression)
   localNode.initProtocolStates()
   localNode.startListening()
 
@@ -119,6 +119,9 @@ asyncTest "network with 3 peers using custom protocols":
       echo "got xyz req"
       await peer.xyzRes("mock peer data")
 
+    when useCompression:
+      m.useCompression = useCompression
+
   discard await mock1.rlpxConnect(localNode)
   let mock2Connection = await localNode.rlpxConnect(mock2)