From e8e36a3ab2bc1967431f3a45974ef26ba28f4a28 Mon Sep 17 00:00:00 2001 From: Sergei Tikhomirov Date: Wed, 5 Mar 2025 12:53:51 +0100 Subject: [PATCH] replace compile reputation flag with config flag (WIP) --- tests/waku_lightpush/lightpush_utils.nim | 7 ++-- tests/waku_lightpush/test_client.nim | 23 ++++++++----- waku/factory/external_config.nim | 8 +++++ waku/factory/node_factory.nim | 2 ++ waku/node/waku_node.nim | 4 +-- waku/waku_lightpush/client.nim | 43 +++++++++++------------- 6 files changed, 51 insertions(+), 36 deletions(-) diff --git a/tests/waku_lightpush/lightpush_utils.nim b/tests/waku_lightpush/lightpush_utils.nim index 582c3ec30b..4be10ea2b9 100644 --- a/tests/waku_lightpush/lightpush_utils.nim +++ b/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) diff --git a/tests/waku_lightpush/test_client.nim b/tests/waku_lightpush/test_client.nim index ee9f9415f1..5d5553e0a4 100644 --- a/tests/waku_lightpush/test_client.nim +++ b/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): - check client.reputationManager.getReputation(serverRemotePeerInfo.peerId) == + if client.reputationManager.isSome: + 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): - check client.reputationManager.getReputation( + if client.reputationManager.isSome: + 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) diff --git a/waku/factory/external_config.nim b/waku/factory/external_config.nim index 9bc073426b..9fe05829d0 100644 --- a/waku/factory/external_config.nim +++ b/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: diff --git a/waku/factory/node_factory.nim b/waku/factory/node_factory.nim index 625f1a77b2..567d6f0507 100644 --- a/waku/factory/node_factory.nim +++ b/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: diff --git a/waku/node/waku_node.nim b/waku/node/waku_node.nim index 5b4f9900ff..53b326570b 100644 --- a/waku/node/waku_node.nim +++ b/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, diff --git a/waku/waku_lightpush/client.nim b/waku/waku_lightpush/client.nim index 1401a6b470..ab513ab8fb 100644 --- a/waku/waku_lightpush/client.nim +++ b/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( - peerManager: peerManager, rng: rng, reputationManager: reputationManager.get() - ) - else: - WakuLightPushClient(peerManager: peerManager, rng: rng) + WakuLightPushClient( + peerManager: peerManager, rng: rng, reputationManager: reputationManager + ) proc addPublishObserver*(wl: WakuLightPushClient, obs: PublishObserver) = wl.publishObservers.add(obs) @@ -75,8 +72,8 @@ proc sendPushRequest( else: return err("unknown failure") - when defined(reputation): - wl.reputationManager.updateReputationFromResponse(peer.peerId, response) + if wl.reputationManager.isSome: + 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): - wl.reputationManager.setReputation(peer.peerId, some(false)) + if wl.reputationManager.isSome: + 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)): - return ok(candidate) - attempts += 1 + if wl.reputationManager.isSome(): + let reputation = wl.reputationManager.get().getReputation(candidate.peerId) + 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