feat: integrate reputation manager into Lightpush behind a feature flag #3309
Conversation
|
You can find the image built from this PR at Built from d02f5f8 |
s-tikhomirov
force-pushed
the
feat/incentivization-poc-lightpush-integration-reputation
branch
from
2ffc878 to
321dc7e
Compare
s-tikhomirov
changed the title
|
Converting to draft after discussion with @jm-clius . I'd appreciate the opinion of @NagyZoltanPeter (cc @Ivansete-status ) on the following question. To introduce reputation-related functionality for Lightpush in this PR, I use a compilation flag
From nwaku's perspective, what would be the preferred way to develop reputation-related features? |
IMHO, compilation flag can just make things more complicated as you should sign or know that the binary is which and can also hide certain problems from other developers not using the flag. |
For the sake of completeness, is this - #3279 - the PR in question? I'm going to look at this more closely. I agree that it makes sense to introduce reputation for the new Lightpush, as long as it's coming up soon. |
Exactly, that one. |
s-tikhomirov
force-pushed
the
feat/incentivization-poc-lightpush-integration-reputation
branch
from
e8e36a3 to
0fc1862
Compare
|
|
||
| logScope: | ||
| topics = "waku lightpush client" | ||
|
|
||
| type WakuLightPushClient* = ref object | ||
| peerManager*: PeerManager | ||
| rng*: ref rand.HmacDrbgContext | ||
| reputationManager*: Option[ReputationManager] |
There was a problem hiding this comment.
I would move this inside PeerManager it will be needed to persist reputation state anyway and it would minimize the impact on the light push code.
| proc selectPeerForLightPush*( | ||
| wl: WakuLightPushClient | ||
| ): Future[Result[RemotePeerInfo, string]] {.async, gcsafe.} = | ||
| 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 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 peerResult |
There was a problem hiding this comment.
Seams a bit convoluted.
In my mind, a reputation ordered list of peers would work best. One would have to simply pick the first element, no computation needed.
edit: I see that the reputation is binary ATM maybe a number could be used instead? An exponentially smoothed moving average (simpler than it sounds) with a scoring of 1,0,-1 should work and not add too much complexity.
There was a problem hiding this comment.
Seams a bit convoluted.
I agree, I thought it would be good enough for a POC (and maybe it is), but your other idea of moving the reputation logic inside the PeerManager could simplify things indeed.
Description
This PR integrates a reputation system designed as part of the incentivization PoC into the Lightpush protocol behind a feature flag (
reputation).A Lightpush client uses the reputation manager to keep track of peers it used previously to publish messages. Peers that had failed at publishing a message get assigned a negative reputation and will not be chosen for future requests. If, however, a neutral- or positive-reputation peer isn't selected by the peer manager after a maximum number of attempts (
10), a negative-reputation peer is still used.This is a countinuaton of #3166, #3264, and #3293 with associated deliverable waku-org/pm#245.
Changes
publishToAnylogic for lightpush