chore: use type for rate limit config (#3489)

* chore: use type for rate limit config

Use type instead of `seq[string]` for rate limit config earlier.
Enables to fail faster (at config time) if the string is malformated

Also enables using object in some scenarios.

* test: remove import warnings

* improve naming and add tests

apps/chat2bridge/chat2bridge.nim

@@ -23,6 +23,7 @@ import
     waku_store,
     factory/builder,
     common/utils/matterbridge_client,
+    common/rate_limit/setting,
   ],
   # Chat 2 imports
   ../chat2/chat2,

tests/common/test_all.nim

@@ -1,3 +1,5 @@
+{.used.}
+
 import
   ./test_base64_codec,
   ./test_confutils_envvar,

tests/factory/test_all.nim

@@ -1 +1,3 @@
+{.used.}
+
 import ./test_external_config, ./test_node_factory, ./test_waku_conf

tests/factory/test_waku_conf.nim

@@ -281,3 +281,18 @@ suite "Waku Conf - extMultiaddrs":
     )
     for m in multiaddrs:
       check m in resMultiaddrs
+
+suite "Waku Conf Builder - rate limits":
+  test "Valid rate limit passed via string":
+    ## Setup
+    var builder = RateLimitConfBuilder.init()
+
+    ## Given
+    let rateLimitsStr = @["lightpush:2/2ms", "10/2m", "store: 3/3s"]
+    builder.withRateLimits(rateLimitsStr)
+
+    ## When
+    let res = builder.build()
+
+    ## Then
+    assert res.isOk(), $res.error

tests/incentivization/test_all.nim

@@ -1 +1,3 @@
+{.used.}
+
 import ./test_rpc_codec, ./test_poc_eligibility, ./test_poc_reputation

tests/node/test_all.nim

@@ -1,3 +1,5 @@
+{.used.}
+
 import
   ./test_wakunode_filter,
   ./test_wakunode_legacy_lightpush,

tests/waku_lightpush/test_all.nim

@@ -1 +1,3 @@
+{.used.}
+
 import ./test_client, ./test_ratelimit

tests/waku_lightpush_legacy/test_all.nim

@@ -1 +1,3 @@
+{.used.}
+
 import ./test_client, ./test_ratelimit

tests/waku_peer_exchange/test_all.nim

@@ -1 +1,3 @@
+{.used.}
+
 import ./test_protocol, ./test_rpc_codec

waku/factory/builder.nim

@@ -43,8 +43,8 @@ type
     switchSendSignedPeerRecord: Option[bool]
     circuitRelay: Relay
 
-    #Rate limit configs for non-relay req-resp protocols
-    rateLimitSettings: Option[seq[string]]
+    # Rate limit configs for non-relay req-resp protocols
+    rateLimitSettings: Option[ProtocolRateLimitSettings]
 
   WakuNodeBuilderResult* = Result[void, string]
 
@@ -127,7 +127,7 @@ proc withPeerManagerConfig*(
 proc withColocationLimit*(builder: var WakuNodeBuilder, colocationLimit: int) =
   builder.colocationLimit = colocationLimit
 
-proc withRateLimit*(builder: var WakuNodeBuilder, limits: seq[string]) =
+proc withRateLimit*(builder: var WakuNodeBuilder, limits: ProtocolRateLimitSettings) =
   builder.rateLimitSettings = some(limits)
 
 proc withCircuitRelay*(builder: var WakuNodeBuilder, circuitRelay: Relay) =
@@ -219,11 +219,9 @@ proc build*(builder: WakuNodeBuilder): Result[WakuNode, string] =
       switch = switch,
       peerManager = peerManager,
       rng = rng,
+      rateLimitSettings = builder.rateLimitSettings.get(DefaultProtocolRateLimit),
     )
   except Exception:
     return err("failed to build WakuNode instance: " & getCurrentExceptionMsg())
 
-  if builder.rateLimitSettings.isSome():
-    ?node.setRateLimits(builder.rateLimitSettings.get())
-
   ok(node)

waku/factory/conf_builder/conf_builder.nim

@@ -8,10 +8,11 @@ import
   ./discv5_conf_builder,
   ./web_socket_conf_builder,
   ./metrics_server_conf_builder,
+  ./rate_limit_conf_builder,
   ./rln_relay_conf_builder
 
 export
   waku_conf_builder, filter_service_conf_builder, store_sync_conf_builder,
   store_service_conf_builder, rest_server_conf_builder, dns_discovery_conf_builder,
   discv5_conf_builder, web_socket_conf_builder, metrics_server_conf_builder,
-  rln_relay_conf_builder
+  rate_limit_conf_builder, rln_relay_conf_builder

waku/factory/conf_builder/rate_limit_conf_builder.nim

@@ -0,0 +1,29 @@
+import chronicles, std/[net, options], results
+import waku/common/rate_limit/setting
+
+logScope:
+  topics = "waku conf builder rate limit"
+
+type RateLimitConfBuilder* = object
+  strValue: Option[seq[string]]
+  objValue: Option[ProtocolRateLimitSettings]
+
+proc init*(T: type RateLimitConfBuilder): RateLimitConfBuilder =
+  RateLimitConfBuilder()
+
+proc withRateLimits*(b: var RateLimitConfBuilder, rateLimits: seq[string]) =
+  b.strValue = some(rateLimits)
+
+proc build*(b: RateLimitConfBuilder): Result[ProtocolRateLimitSettings, string] =
+  if b.strValue.isSome() and b.objValue.isSome():
+    return err("Rate limits conf must only be set once on the builder")
+
+  if b.objValue.isSome():
+    return ok(b.objValue.get())
+
+  if b.strValue.isSome():
+    let rateLimits = ProtocolRateLimitSettings.parse(b.strValue.get()).valueOr:
+      return err("Invalid rate limits settings:" & $error)
+    return ok(rateLimits)
+
+  return ok(DefaultProtocolRateLimit)

waku/factory/conf_builder/waku_conf_builder.nim

@@ -23,6 +23,7 @@ import
   ./discv5_conf_builder,
   ./web_socket_conf_builder,
   ./metrics_server_conf_builder,
+  ./rate_limit_conf_builder,
   ./rln_relay_conf_builder
 
 logScope:
@@ -74,6 +75,7 @@ type WakuConfBuilder* = object
   rlnRelayConf*: RlnRelayConfBuilder
   storeServiceConf*: StoreServiceConfBuilder
   webSocketConf*: WebSocketConfBuilder
+  rateLimitConf*: RateLimitConfBuilder
   # End conf builders
   relay: Option[bool]
   lightPush: Option[bool]
@@ -116,8 +118,6 @@ type WakuConfBuilder* = object
 
   agentString: Option[string]
 
-  rateLimits: Option[seq[string]]
-
   maxRelayPeers: Option[int]
   relayShardedPeerManagement: Option[bool]
   relayServiceRatio: Option[string]
@@ -134,6 +134,7 @@ proc init*(T: type WakuConfBuilder): WakuConfBuilder =
     rlnRelayConf: RlnRelayConfBuilder.init(),
     storeServiceConf: StoreServiceConfBuilder.init(),
     webSocketConf: WebSocketConfBuilder.init(),
+    rateLimitConf: RateLimitConfBuilder.init(),
   )
 
 proc withNetworkConf*(b: var WakuConfBuilder, networkConf: NetworkConf) =
@@ -241,9 +242,6 @@ proc withAgentString*(b: var WakuConfBuilder, agentString: string) =
 proc withColocationLimit*(b: var WakuConfBuilder, colocationLimit: int) =
   b.colocationLimit = some(colocationLimit)
 
-proc withRateLimits*(b: var WakuConfBuilder, rateLimits: seq[string]) =
-  b.rateLimits = some(rateLimits)
-
 proc withMaxRelayPeers*(b: var WakuConfBuilder, maxRelayPeers: int) =
   b.maxRelayPeers = some(maxRelayPeers)
 
@@ -489,6 +487,10 @@ proc build*(
 
   let webSocketConf = builder.webSocketConf.build().valueOr:
     return err("WebSocket Conf building failed: " & $error)
+
+  let rateLimit = builder.rateLimitConf.build().valueOr:
+    return err("Rate limits Conf building failed: " & $error)
+
   # End - Build sub-configs
 
   let logLevel =
@@ -583,7 +585,6 @@ proc build*(
   # TODO: use `DefaultColocationLimit`. the user of this value should
   # probably be defining a config object
   let colocationLimit = builder.colocationLimit.get(5)
-  let rateLimits = builder.rateLimits.get(newSeq[string](0))
 
   # TODO: is there a strategy for experimental features? delete vs promote
   let relayShardedPeerManagement = builder.relayShardedPeerManagement.get(false)
@@ -643,7 +644,7 @@ proc build*(
     colocationLimit: colocationLimit,
     maxRelayPeers: builder.maxRelayPeers,
     relayServiceRatio: builder.relayServiceRatio.get("60:40"),
-    rateLimits: rateLimits,
+    rateLimit: rateLimit,
     circuitRelayClient: builder.circuitRelayClient.get(false),
     staticNodes: builder.staticNodes,
     relayShardedPeerManagement: relayShardedPeerManagement,

waku/factory/external_config.nim

@@ -1016,6 +1016,6 @@ proc toWakuConf*(n: WakuNodeConf): ConfResult[WakuConf] =
   b.webSocketConf.withKeyPath(n.websocketSecureKeyPath)
   b.webSocketConf.withCertPath(n.websocketSecureCertPath)
 
-  b.withRateLimits(n.rateLimits)
+  b.rateLimitConf.withRateLimits(n.rateLimits)
 
   return b.build()

waku/factory/node_factory.nim

@@ -122,7 +122,7 @@ proc initNode(
       relayServiceRatio = conf.relayServiceRatio,
       shardAware = conf.relayShardedPeerManagement,
     )
-  builder.withRateLimit(conf.rateLimits)
+  builder.withRateLimit(conf.rateLimit)
   builder.withCircuitRelay(relay)
 
   let node =

waku/factory/waku_conf.nim

@@ -12,6 +12,7 @@ import
   ../discovery/waku_discv5,
   ../node/waku_metrics,
   ../common/logging,
+  ../common/rate_limit/setting,
   ../waku_enr/capabilities,
   ./networks_config
 
@@ -127,8 +128,7 @@ type WakuConf* {.requiresInit.} = ref object
 
   colocationLimit*: int
 
-  # TODO: use proper type
-  rateLimits*: seq[string]
+  rateLimit*: ProtocolRateLimitSettings
 
   # TODO: those could be in a relay conf object
   maxRelayPeers*: Option[int]

waku/node/waku_node.nim

@@ -128,6 +128,7 @@ proc new*(
     enr: enr.Record,
     switch: Switch,
     peerManager: PeerManager,
+    rateLimitSettings: ProtocolRateLimitSettings = DefaultProtocolRateLimit,
     # TODO: make this argument required after tests are updated
     rng: ref HmacDrbgContext = crypto.newRng(),
 ): T {.raises: [Defect, LPError, IOError, TLSStreamProtocolError].} =
@@ -144,7 +145,7 @@ proc new*(
     enr: enr,
     announcedAddresses: netConfig.announcedAddresses,
     topicSubscriptionQueue: queue,
-    rateLimitSettings: DefaultProtocolRateLimit,
+    rateLimitSettings: rateLimitSettings,
   )
 
   return node
@@ -1563,10 +1564,3 @@ proc isReady*(node: WakuNode): Future[bool] {.async: (raises: [Exception]).} =
     return true
   return await node.wakuRlnRelay.isReady()
   ## TODO: add other protocol `isReady` checks
-
-proc setRateLimits*(node: WakuNode, limits: seq[string]): Result[void, string] =
-  let rateLimitConfig = ProtocolRateLimitSettings.parse(limits)
-  if rateLimitConfig.isErr():
-    return err("invalid rate limit settings:" & rateLimitConfig.error)
-  node.rateLimitSettings = rateLimitConfig.get()
-  return ok()