[ETCM-331] Separate RateLimit validation in a different trait

Author: SoseMagarian

src/main/resources/application.conf

@@ -202,7 +202,7 @@ mantis {
         # any domain.
         cors-allowed-origins = []
 
-        # Ip Limit tracking for JSON-RPC requests
+        # Rate Limit for JSON-RPC requests
         # Limits the amount of request the same ip can perform in a given amount of time
         rate-limit {
           # If enabled, restrictions are applied

src/main/scala/io/iohk/ethereum/jsonrpc/server/http/JsonRpcHttpServer.scala

@@ -1,7 +1,6 @@
 package io.iohk.ethereum.jsonrpc.server.http
 
 import java.security.SecureRandom
-import java.time.Clock
 
 import akka.actor.ActorSystem
 import akka.http.scaladsl.model._
@@ -11,7 +10,6 @@ import ch.megard.akka.http.cors.javadsl.CorsRejection
 import ch.megard.akka.http.cors.scaladsl.CorsDirectives._
 import ch.megard.akka.http.cors.scaladsl.model.HttpOriginMatcher
 import ch.megard.akka.http.cors.scaladsl.settings.CorsSettings
-import com.twitter.util.LruMap
 import de.heikoseeberger.akkahttpjson4s.Json4sSupport
 import io.iohk.ethereum.faucet.jsonrpc.FaucetJsonRpcController
 import io.iohk.ethereum.jsonrpc._
@@ -28,7 +26,7 @@ import com.typesafe.config.{Config => TypesafeConfig}
 
 import scala.concurrent.duration.{FiniteDuration, _}
 
-trait JsonRpcHttpServer extends Json4sSupport {
+trait JsonRpcHttpServer extends Json4sSupport with RateLimit {
   val jsonRpcController: JsonRpcBaseController
   val jsonRpcHealthChecker: JsonRpcHealthChecker
   val config: JsonRpcHttpServerConfig
@@ -39,10 +37,6 @@ trait JsonRpcHttpServer extends Json4sSupport {
 
   def corsAllowedOrigins: HttpOriginMatcher
 
-  val latestRequestTimestamps = new LruMap[RemoteAddress, Long](config.rateLimit.latestTimestampCacheSize)
-
-  val clock: Clock = Clock.systemUTC()
-
   val corsSettings = CorsSettings.defaultSettings
     .withAllowGenericHttpRequests(true)
     .withAllowedOrigins(corsAllowedOrigins)
@@ -71,17 +65,16 @@ trait JsonRpcHttpServer extends Json4sSupport {
   }
 
   def handleRequest(clientAddress: RemoteAddress, request: JsonRpcRequest): StandardRoute = {
+    //FIXME: FaucetJsonRpcController.Status should be part of a Healthcheck request or alike.
+    // As a temporary solution, it is being excluded from the Rate Limit.
     if (config.rateLimit.enabled && request.method != FaucetJsonRpcController.Status) {
-      handleRestrictedRequest(clientAddress, request)
+      handleRateLimitedRequest(clientAddress, request)
     } else complete(jsonRpcController.handleRequest(request).runToFuture)
   }
 
-  def handleRestrictedRequest(clientAddress: RemoteAddress, request: JsonRpcRequest): StandardRoute = {
-    val timeMillis = clock.instant().toEpochMilli
-    val latestRequestTimestamp = latestRequestTimestamps.getOrElse(clientAddress, 0L)
-
-    if (latestRequestTimestamp + config.rateLimit.minRequestInterval.toMillis < timeMillis) {
-      latestRequestTimestamps.put(clientAddress, timeMillis)
+  def handleRateLimitedRequest(clientAddress: RemoteAddress, request: JsonRpcRequest): StandardRoute = {
+    if (isRequestAvailable(clientAddress)) {
+      log.warn(s"Request limit exceeded for ip 1 ${clientAddress.toIP.getOrElse("unknown")}")
       complete(jsonRpcController.handleRequest(request).runToFuture)
     } else complete(StatusCodes.TooManyRequests)
   }
@@ -144,15 +137,15 @@ object JsonRpcHttpServer extends Logger {
       case _ => Left(s"Cannot start JSON RPC server: Invalid mode ${config.mode} selected")
     }
 
-  trait RateLimit {
+  trait RateLimitConfig {
     val enabled: Boolean
     val minRequestInterval: FiniteDuration
     val latestTimestampCacheSize: Int
   }
 
-  object RateLimit {
-    def apply(rateLimitConfig: TypesafeConfig): RateLimit =
-      new RateLimit {
+  object RateLimitConfig {
+    def apply(rateLimitConfig: TypesafeConfig): RateLimitConfig =
+      new RateLimitConfig {
         override val enabled: Boolean = rateLimitConfig.getBoolean("enabled")
         override val minRequestInterval: FiniteDuration =
           rateLimitConfig.getDuration("min-request-interval").toMillis.millis
@@ -166,7 +159,7 @@ object JsonRpcHttpServer extends Logger {
     val interface: String
     val port: Int
     val corsAllowedOrigins: HttpOriginMatcher
-    val rateLimit: RateLimit
+    val rateLimit: RateLimitConfig
   }
 
   object JsonRpcHttpServerConfig {
@@ -181,7 +174,7 @@ object JsonRpcHttpServer extends Logger {
 
         override val corsAllowedOrigins = ConfigUtils.parseCorsAllowedOrigins(rpcHttpConfig, "cors-allowed-origins")
 
-        override val rateLimit = RateLimit(rpcHttpConfig.getConfig("rate-limit"))
+        override val rateLimit = RateLimitConfig(rpcHttpConfig.getConfig("rate-limit"))
       }
     }
   }

src/main/scala/io/iohk/ethereum/jsonrpc/server/http/RateLimit.scala

@@ -0,0 +1,26 @@
+package io.iohk.ethereum.jsonrpc.server.http
+
+import java.time.Clock
+
+import akka.http.scaladsl.model.RemoteAddress
+import com.twitter.util.LruMap
+import io.iohk.ethereum.jsonrpc.server.http.JsonRpcHttpServer.JsonRpcHttpServerConfig
+import io.iohk.ethereum.utils.Logger
+
+trait RateLimit extends Logger {
+
+  val config: JsonRpcHttpServerConfig
+
+  val latestRequestTimestamps = new LruMap[RemoteAddress, Long](config.rateLimit.latestTimestampCacheSize)
+
+  val clock: Clock = Clock.systemUTC()
+
+  def isRequestAvailable(clientAddress: RemoteAddress): Boolean = {
+    val timeMillis = clock.instant().toEpochMilli
+    val latestRequestTimestamp = latestRequestTimestamps.getOrElse(clientAddress, 0L)
+
+    val response = latestRequestTimestamp + config.rateLimit.minRequestInterval.toMillis < timeMillis
+    if (response) latestRequestTimestamps.put(clientAddress, timeMillis)
+    response
+  }
+}

src/test/scala/io/iohk/ethereum/jsonrpc/server/http/JsonRpcHttpServerSpec.scala

@@ -4,20 +4,23 @@ import java.net.InetAddress
 import java.time.{Clock, Instant, ZoneId}
 import java.util.concurrent.TimeUnit
 
+import akka.actor.ActorSystem
 import akka.http.scaladsl.model._
 import akka.http.scaladsl.model.headers.{HttpOrigin, Origin}
 import akka.http.scaladsl.server.Route
 import akka.http.scaladsl.testkit.ScalatestRouteTest
 import akka.util.ByteString
 import ch.megard.akka.http.cors.scaladsl.model.HttpOriginMatcher
-import io.iohk.ethereum.jsonrpc.server.http.JsonRpcHttpServer.{JsonRpcHttpServerConfig, RateLimit}
+import io.iohk.ethereum.jsonrpc.server.http.JsonRpcHttpServer.{JsonRpcHttpServerConfig, RateLimitConfig}
 import io.iohk.ethereum.jsonrpc.{JsonRpcController, JsonRpcHealthChecker, JsonRpcResponse}
 import monix.eval.Task
 import org.json4s.JsonAST.{JInt, JString}
 import org.scalamock.scalatest.MockFactory
 import org.scalatest.flatspec.AnyFlatSpec
 import org.scalatest.matchers.should.Matchers
 import akka.http.scaladsl.model.headers._
+import io.iohk.ethereum.utils.Logger
+import io.iohk.ethereum.jsonrpc.server.controllers.JsonRpcBaseController
 
 import scala.concurrent.duration.FiniteDuration
 
@@ -108,7 +111,7 @@ class JsonRpcHttpServerSpec extends AnyFlatSpec with Matchers with ScalatestRout
     val postRequest =
       HttpRequest(HttpMethods.POST, uri = "/", entity = HttpEntity(MediaTypes.`application/json`, jsonRequest))
 
-    postRequest ~> Route.seal(mockJsonRpcHttpServerWithIpRestriction.route) ~> check {
+    postRequest ~> Route.seal(mockJsonRpcHttpServerWithRateLimit.route) ~> check {
       status shouldEqual StatusCodes.OK
       responseAs[String] shouldEqual """{"jsonrpc":"2.0","result":"this is a response","id":1}"""
     }
@@ -123,11 +126,11 @@ class JsonRpcHttpServerSpec extends AnyFlatSpec with Matchers with ScalatestRout
     val postRequest =
       HttpRequest(HttpMethods.POST, uri = "/", entity = HttpEntity(MediaTypes.`application/json`, jsonRequest))
 
-    postRequest ~> Route.seal(mockJsonRpcHttpServerWithIpRestriction.route) ~> check {
+    postRequest ~> Route.seal(mockJsonRpcHttpServerWithRateLimit.route) ~> check {
       status shouldEqual StatusCodes.OK
       responseAs[String] shouldEqual """{"jsonrpc":"2.0","result":"this is a response","id":1}"""
     }
-    postRequest ~> Route.seal(mockJsonRpcHttpServerWithIpRestriction.route) ~> check {
+    postRequest ~> Route.seal(mockJsonRpcHttpServerWithRateLimit.route) ~> check {
       status shouldEqual StatusCodes.TooManyRequests
     }
   }
@@ -143,7 +146,7 @@ class JsonRpcHttpServerSpec extends AnyFlatSpec with Matchers with ScalatestRout
     val postRequest =
       HttpRequest(HttpMethods.POST, uri = "/", entity = HttpEntity(MediaTypes.`application/json`, jsonRequest))
 
-    postRequest ~> Route.seal(mockJsonRpcHttpServerWithIpRestriction.route) ~> check {
+    postRequest ~> Route.seal(mockJsonRpcHttpServerWithRateLimit.route) ~> check {
       status === StatusCodes.MethodNotAllowed
     }
   }
@@ -158,17 +161,17 @@ class JsonRpcHttpServerSpec extends AnyFlatSpec with Matchers with ScalatestRout
     val postRequest =
       HttpRequest(HttpMethods.POST, uri = "/", entity = HttpEntity(MediaTypes.`application/json`, jsonRequest))
 
-    postRequest ~> Route.seal(mockJsonRpcHttpServerWithIpRestriction.route) ~> check {
+    postRequest ~> Route.seal(mockJsonRpcHttpServerWithRateLimit.route) ~> check {
       status shouldEqual StatusCodes.OK
       responseAs[String] shouldEqual """{"jsonrpc":"2.0","result":"this is a response","id":1}"""
     }
-    postRequest ~> Route.seal(mockJsonRpcHttpServerWithIpRestriction.route) ~> check {
+    postRequest ~> Route.seal(mockJsonRpcHttpServerWithRateLimit.route) ~> check {
       status shouldEqual StatusCodes.TooManyRequests
     }
 
-    fakeClock.advanceTime(10)
+    fakeClock.advanceTime(2 * serverConfigWithRateLimit.rateLimit.minRequestInterval.toMillis)
 
-    postRequest ~> Route.seal(mockJsonRpcHttpServerWithIpRestriction.route) ~> check {
+    postRequest ~> Route.seal(mockJsonRpcHttpServerWithRateLimit.route) ~> check {
       status shouldEqual StatusCodes.OK
       responseAs[String] shouldEqual """{"jsonrpc":"2.0","result":"this is a response","id":1}"""
     }
@@ -192,18 +195,18 @@ class JsonRpcHttpServerSpec extends AnyFlatSpec with Matchers with ScalatestRout
         jsonRequest)
       )
 
-    postRequest ~> Route.seal(mockJsonRpcHttpServerWithIpRestriction.route) ~> check {
+    postRequest ~> Route.seal(mockJsonRpcHttpServerWithRateLimit.route) ~> check {
       status shouldEqual StatusCodes.OK
       responseAs[String] shouldEqual """{"jsonrpc":"2.0","result":"this is a response","id":1}"""
     }
-    postRequest2 ~> Route.seal(mockJsonRpcHttpServerWithIpRestriction.route) ~> check {
+    postRequest2 ~> Route.seal(mockJsonRpcHttpServerWithRateLimit.route) ~> check {
       status shouldEqual StatusCodes.OK
       responseAs[String] shouldEqual """{"jsonrpc":"2.0","result":"this is a response","id":1}"""
     }
   }
 
   trait TestSetup extends MockFactory {
-    val rateLimitConfig = new RateLimit {
+    val rateLimitConfig = new RateLimitConfig {
       override val enabled: Boolean = false
       override val minRequestInterval: FiniteDuration = FiniteDuration.apply(5, TimeUnit.SECONDS)
       override val latestTimestampCacheSize: Int = 1024
@@ -215,51 +218,66 @@ class JsonRpcHttpServerSpec extends AnyFlatSpec with Matchers with ScalatestRout
       override val interface: String = ""
       override val port: Int = 123
       override val corsAllowedOrigins = HttpOriginMatcher.*
-      override val rateLimit: RateLimit = rateLimitConfig
+      override val rateLimit: RateLimitConfig = rateLimitConfig
     }
 
-    val mockJsonRpcController = mock[JsonRpcController]
-    val mockJsonRpcHealthChecker = mock[JsonRpcHealthChecker]
-    val mockJsonRpcHttpServer = new JsonRpcHttpServer {
-      override val jsonRpcController = mockJsonRpcController
-      override val jsonRpcHealthChecker = mockJsonRpcHealthChecker
-      override val config: JsonRpcHttpServerConfig = serverConfig
-
-      def run(): Unit = ()
-
-      override def corsAllowedOrigins: HttpOriginMatcher = config.corsAllowedOrigins
+    val rateLimitEnabledConfig = new RateLimitConfig {
+      override val enabled: Boolean = true
+      override val minRequestInterval: FiniteDuration = FiniteDuration.apply(5, TimeUnit.SECONDS)
+      override val latestTimestampCacheSize: Int = 1024
     }
 
-    val corsAllowedOrigin = HttpOrigin("http://localhost:3333")
-
-    val mockJsonRpcHttpServerWithCors = new JsonRpcHttpServer {
-      override val jsonRpcController = mockJsonRpcController
-      override val jsonRpcHealthChecker = mockJsonRpcHealthChecker
-      override val config: JsonRpcHttpServerConfig = serverConfig
-
-      def run(): Unit = ()
-
-      override def corsAllowedOrigins: HttpOriginMatcher = HttpOriginMatcher(corsAllowedOrigin)
+    val serverConfigWithRateLimit = new JsonRpcHttpServerConfig {
+      override val mode: String = "mockJsonRpc"
+      override val enabled: Boolean = true
+      override val interface: String = ""
+      override val port: Int = 123
+      override val corsAllowedOrigins = HttpOriginMatcher.*
+      override val rateLimit: RateLimitConfig = rateLimitEnabledConfig
     }
 
+    val mockJsonRpcController = mock[JsonRpcController]
+    val mockJsonRpcHealthChecker = mock[JsonRpcHealthChecker]
     val fakeClock = new FakeClock
-    val mockJsonRpcHttpServerWithIpRestriction = new JsonRpcHttpServer {
-      override val jsonRpcController = mockJsonRpcController
-      override val jsonRpcHealthChecker = mockJsonRpcHealthChecker
-      override val config: JsonRpcHttpServerConfig = serverConfig
 
-      override val clock: Clock = fakeClock
+    val mockJsonRpcHttpServer = new FakeJsonRpcHttpServer(
+      mockJsonRpcController,
+      mockJsonRpcHealthChecker,
+      serverConfig,
+      serverConfig.corsAllowedOrigins,
+      fakeClock)
 
-      override val config.rateLimit.`enabled` = true
-      override val config.rateLimit.minRequestInterval: FiniteDuration = config.rateLimit.minRequestInterval
-
-      def run(): Unit = ()
-
-      override def corsAllowedOrigins: HttpOriginMatcher = config.corsAllowedOrigins
-    }
+    val corsAllowedOrigin = HttpOrigin("http://localhost:3333")
+    val mockJsonRpcHttpServerWithCors = new FakeJsonRpcHttpServer(
+      mockJsonRpcController,
+      mockJsonRpcHealthChecker,
+      serverConfig,
+      HttpOriginMatcher(corsAllowedOrigin),
+      fakeClock)
+
+    val mockJsonRpcHttpServerWithRateLimit = new FakeJsonRpcHttpServer(
+      mockJsonRpcController,
+      mockJsonRpcHealthChecker,
+      serverConfigWithRateLimit,
+      serverConfigWithRateLimit.corsAllowedOrigins,
+      fakeClock)
   }
 }
 
+class FakeJsonRpcHttpServer(
+   val jsonRpcController: JsonRpcBaseController,
+   val jsonRpcHealthChecker: JsonRpcHealthChecker,
+   val config: JsonRpcHttpServerConfig,
+   val cors: HttpOriginMatcher,
+   val testClock: Clock
+  )(implicit val actorSystem: ActorSystem)
+  extends JsonRpcHttpServer
+    with Logger {
+    def run(): Unit = ()
+    override def corsAllowedOrigins: HttpOriginMatcher = cors
+    override val clock = testClock
+}
+
 class FakeClock extends Clock {
 
   var time: Instant = Instant.now()

src/universal/conf/faucet.conf

@@ -118,7 +118,7 @@ mantis {
         # any domain.
         cors-allowed-origins = []
 
-        # Ip Limit tracking for JSON-RPC requests
+        # Rate Limit for JSON-RPC requests
         # Limits the amount of request the same ip can perform in a given amount of time
         rate-limit {
           # If enabled, restrictions are applied