fix/update-filter-spec #203
Conversation
|
@decanus please link the issue this is addressing |
|
@oskarth created issues: Unsubscribe was already an issue therefore ignored. |
Co-authored-by: Oskar Thorén <ot@oskarthoren.com>
| // space for optional request id | ||
| repeated ContentFilter contentFilters = 2; | ||
| optional string topic = 3; | ||
| string topic = 1; |
There was a problem hiding this comment.
This is the WakuRelay pubsub topic right?
I understand why you add it, but I'm still wondering whether we need that additional flexibility.
There was a problem hiding this comment.
Because the topics might be different. We discussed this in a call I believe, essentially a filter is made up of pubsub topic and content filter. Omitting this would lead to a lot of unspecified behavior, especially as we move to multiple pubsub topics I believe.
specs/waku/v2/waku-filter.md
Outdated
|
|
||
| TODO Specify mechanism for telling it won't honor (normal-no service-spam case) | ||
| The `request_id` MUST be a uniquely generated string. When a `MessagePush` is sent | ||
| the `request_id` MUST match the `request_id` of the `FilterRequest` whose filters |
There was a problem hiding this comment.
What stops a node from sending messages with invalid contentTopics but correct request_id? Don't we still have to check the contentTopics?
There was a problem hiding this comment.
Is there still a use for this if we have to check all topics anyhow?
There was a problem hiding this comment.
@kdeme wdym? the requestId? I think so yes. It seems like its pretty standard across our protocols now so even if just for that we should probably keep it.
There was a problem hiding this comment.
I didn't mean for having a requestId. I mean specifically for returning that requestId in the MessagePush
|
@oskarth I think this is ready to merge now/ |
|
|
||
| # Abstract | ||
|
|
||
| Filter spec. | ||
| `WakuFilter` is a protocol that enables subscribing to messages that a peer receives. This is a more lightweight version of `WakuRelay` specifically designed for bandwidth restricted devices. This is due to the fact that light nodes subscribe to full-nodes and only receive the messages they desire. |
There was a problem hiding this comment.
honestly I don't think this addresses the feedback I gave before, it still isn't very clear what this is or how it works. But let's move on...
|
It isn't clear to me if #201 (comment) was addressed or not. The abstract needs work but we can do this later separately. |
| # Changelog | ||
|
|
||
| 2.0.0-beta1 | ||
| Initial draft version. Released 2020-10-05 <!-- @TODO LINK --> |
|
@oskarth yes its been done, but I am not quite sure how to make it clear in a spec that a cluster node was updated. |
Some fixes to the spec, syncing it with some of the dev work that happened. Additionally claryfing some todos and moving it into draft.
closes #201