replace compile reputation flag with config flag

tests/waku_lightpush/lightpush_utils.nim

@@ -25,10 +25,13 @@ proc newTestWakuLightpushNode*(
 
   return proto
 
-proc newTestWakuLightpushClient*(switch: Switch): WakuLightPushClient =
+proc newTestWakuLightpushClient*(
+  switch: Switch,
+  reputationEnabled: bool = false
+  ): WakuLightPushClient =
   let peerManager = PeerManager.new(switch)
   let reputationManager =
-    if defined(reputation):
+    if reputationEnabled:
       some(ReputationManager.new())
     else:
       none(ReputationManager)

tests/waku_lightpush/test_client.nim

@@ -75,7 +75,8 @@ suite "Waku Lightpush Client":
     server = await newTestWakuLightpushNode(serverSwitch, handler)
     serverFailsLightpush =
       await newTestWakuLightpushNode(serverSwitchFailsLightpush, handlerFailsLightpush)
-    client = newTestWakuLightpushClient(clientSwitch)
+    ## FIXME: how to pass reputationEnabled from config to here? should we?
+    client = newTestWakuLightpushClient(clientSwitch, reputationEnabled = true)
 
     await allFutures(
       serverSwitch.start(), serverSwitchFailsLightpush.start(), clientSwitch.start()
@@ -330,8 +331,8 @@ suite "Waku Lightpush Client":
       # Then the response is positive
       assertResultOk publishResponse
 
-      when defined(reputation):
+      if client.reputationManager.isSome:
-        check client.reputationManager.getReputation(serverRemotePeerInfo.peerId) ==
+        check client.reputationManager.get().getReputation(serverRemotePeerInfo.peerId) ==
           some(true)
 
     # TODO: Improve: Add more negative responses variations
@@ -351,8 +352,8 @@ suite "Waku Lightpush Client":
       # Then the response is negative
       check not publishResponse.isOk()
 
-      when defined(reputation):
+      if client.reputationManager.isSome:
-        check client.reputationManager.getReputation(
+        check client.reputationManager.get().getReputation(
           serverRemotePeerInfoFailsLightpush.peerId
         ) == some(false)
 
@@ -386,6 +387,9 @@ suite "Waku Lightpush Client":
 
       check not publishResponse1.isOk()
 
+      if client.reputationManager.isSome:
+        client.reputationManager.get().setReputation(serverRemotePeerInfoFailsLightpush.peerId, some(false))
+
       # add a peer that supports the Lightpush protocol to the client's PeerManager
       client.peerManager.addPeer(serverRemotePeerInfo) # supports Lightpush
 
@@ -394,12 +398,15 @@ suite "Waku Lightpush Client":
 
       check publishResponse2.isOk()
 
-      when defined(reputation):
+      if client.reputationManager.isSome:
+        client.reputationManager.get().setReputation(serverRemotePeerInfo.peerId, some(true))
+
+      if client.reputationManager.isSome:
         # the reputation of a failed peer is negative
-        check client.reputationManager.getReputation(
+        check client.reputationManager.get().getReputation(
           serverRemotePeerInfoFailsLightpush.peerId
         ) == some(false)
 
         # the reputation of a successful peer is positive
-        check client.reputationManager.getReputation(serverRemotePeerInfo.peerId) ==
+        check client.reputationManager.get().getReputation(serverRemotePeerInfo.peerId) ==
           some(true)

waku/factory/external_config.nim

@@ -496,6 +496,14 @@ hence would have reachability issues.""",
       name: "lightpushnode"
     .}: string
 
+    ## Reputation config
+    ## FIXME: should be set to false by default
+    reputationEnabled* {.
+      desc: "Enable client-side reputation for light protocols: true|false",
+      defaultValue: true,
+      name: "reputation"
+    .}: bool
+
     ## Reliability config
     reliabilityEnabled* {.
       desc:

waku/factory/node_factory.nim

@@ -370,6 +370,8 @@ proc setupProtocols(
     else:
       return err("failed to set node waku lightpush peer: " & lightPushNode.error)
 
+  ## TODO: initialize reputation manager here??
+
   # Filter setup. NOTE Must be mounted after relay
   if conf.filter:
     try:

waku/node/waku_node.nim

@@ -1009,10 +1009,10 @@ proc mountLightPush*(
 
   node.switch.mount(node.wakuLightPush, protocolMatcher(WakuLightPushCodec))
 
-proc mountLightPushClient*(node: WakuNode) =
+proc mountLightPushClient*(node: WakuNode, reputationEnabled: bool = false) =
   info "mounting light push client"
 
-  node.wakuLightpushClient = WakuLightPushClient.new(node.peerManager, node.rng)
+  node.wakuLightpushClient = WakuLightPushClient.new(node.peerManager, node.rng, reputationEnabled)
 
 proc lightpushPublish*(
     node: WakuNode,

waku/waku_lightpush/client.nim

@@ -20,21 +20,18 @@ logScope:
 type WakuLightPushClient* = ref object
   peerManager*: PeerManager
   rng*: ref rand.HmacDrbgContext
-  reputationManager*: ReputationManager
+  reputationManager*: Option[ReputationManager]
   publishObservers: seq[PublishObserver]
 
 proc new*(
     T: type WakuLightPushClient,
     peerManager: PeerManager,
     rng: ref rand.HmacDrbgContext,
-    reputationManager: Option[ReputationManager] = none(ReputationManager),
+    reputationManager: Option[ReputationManager],
 ): T =
-  if reputationManager.isSome:
+  WakuLightPushClient(
-    WakuLightPushClient(
+    peerManager: peerManager, rng: rng, reputationManager: reputationManager
-      peerManager: peerManager, rng: rng, reputationManager: reputationManager.get()
+  )
-    )
-  else:
-    WakuLightPushClient(peerManager: peerManager, rng: rng)
 
 proc addPublishObserver*(wl: WakuLightPushClient, obs: PublishObserver) =
   wl.publishObservers.add(obs)
@@ -75,8 +72,8 @@ proc sendPushRequest(
     else:
       return err("unknown failure")
 
-  when defined(reputation):
+  if wl.reputationManager.isSome:
-    wl.reputationManager.updateReputationFromResponse(peer.peerId, response)
+    wl.reputationManager.get().updateReputationFromResponse(peer.peerId, response)
 
   return ok()
 
@@ -92,8 +89,8 @@ proc publish*(
   let pushRequest = PushRequest(pubSubTopic: pubSubTopic, message: message)
   let pushResult = await wl.sendPushRequest(pushRequest, peer)
   if pushResult.isErr:
-    when defined(reputation):
+    if wl.reputationManager.isSome:
-      wl.reputationManager.setReputation(peer.peerId, some(false))
+      wl.reputationManager.get().setReputation(peer.peerId, some(false))
     return err(pushResult.error)
 
   for obs in wl.publishObservers:
@@ -110,29 +107,27 @@ proc publish*(
 proc selectPeerForLightPush*(
     wl: WakuLightPushClient
 ): Future[Result[RemotePeerInfo, string]] {.async, gcsafe.} =
-  ## If reputation flag is defined, try to ensure the selected peer is not bad-rep.
-  ## Repeat peer selection until either maxAttempts is exceeded,
-  ## or a good-rep or neutral-rep peer is found.
-  ## Note: this procedure CAN return a bad-rep peer if maxAttempts is exceeded.
   let maxAttempts = if defined(reputation): 10 else: 1
   var attempts = 0
   var peerResult: Result[RemotePeerInfo, string]
   while attempts < maxAttempts:
     let candidate = wl.peerManager.selectPeer(WakuLightPushCodec, none(PubsubTopic)).valueOr:
       return err("could not retrieve a peer supporting WakuLightPushCodec")
-    if not (wl.reputationManager.getReputation(candidate.peerId) == some(false)):
+    if wl.reputationManager.isSome():
-      return ok(candidate)
+      let reputation = wl.reputationManager.get().getReputation(candidate.peerId)
-    attempts += 1
+      info "Peer selected", peerId = candidate.peerId, reputation = $reputation, attempts = $attempts
+      if (reputation == some(false)):
+        attempts += 1
+        continue
+    return ok(candidate)
   warn "Maximum reputation-based retries exceeded; continuing with a bad-reputation peer."
-  # Return last candidate even if it has bad reputation
   return peerResult
 
 proc publishToAny*(
     wl: WakuLightPushClient, pubSubTopic: PubsubTopic, message: WakuMessage
 ): Future[WakuLightPushResult[string]] {.async, gcsafe.} =
-  ## Publish a message via a peer that we get from the peer manager
-
   info "publishToAny", msg_hash = computeMessageHash(pubSubTopic, message).to0xHex
-
   let peer = ?await wl.selectPeerForLightPush()
-  return await wl.publish(pubSubTopic, message, peer)
+  let publishResult = await wl.publish(pubSubTopic, message, peer)
+  info "Publish result", result = $publishResult
+  return publishResult