Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature/filter #63

Merged
merged 16 commits into from
Jul 22, 2020
25 changes: 25 additions & 0 deletions waku/protocol/v2/filter.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import
tables

type

FilterMessageHandler* = proc(msg: seq[byte]) {.gcsafe, closure.}

Filter* = object
topics: seq[string] # @TODO TOPIC
handler: FilterMessageHandler

Filters* = Table[string, Filter]

proc init*(T: type Filter, topics: seq[string], handler: FilterMessageHandler): T =
result = T(
topics: topics,
handler: handler
)

proc notify*(filters: var Filters, topic: string, msg: seq[byte]) {.gcsafe.} =
for filter in filters.mvalues:
if filter.topics.len > 0 and topic notin filter.topics:
continue

filter.handler(msg)
7 changes: 7 additions & 0 deletions waku/protocol/v2/waku_protocol2.nim
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import strutils
import chronos, chronicles
import ./filter
import libp2p/protocols/pubsub/pubsub,
libp2p/protocols/pubsub/pubsubpeer,
libp2p/protocols/pubsub/floodsub,
Expand All @@ -28,6 +29,8 @@ type
text*: string
gossip_enabled*: bool

filters: Filters

method init(w: WakuSub) =
debug "init"
proc handler(conn: Connection, proto: string) {.async.} =
Expand Down Expand Up @@ -82,6 +85,10 @@ method subscribeTopic*(w: WakuSub,
proc handler(topic: string, data: seq[byte]) {.async, gcsafe.} =
info "Hit NOOP handler", topic

# Currently we are using the libp2p topic here.
# This will need to be change to a Waku topic.
w.filters.notify(topic, data)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you are mixing the GossipSub/FloodSub topic(s) here with the ones of Waku. The current idea/plan is that these are not the same.

So the way I see to do this is as follows: a Waku subscribe should subscribe a WakuSub handler on the "general" chosen topic (=GossipSub/FloodSub topic) for WakuSub communication, and use that WakuSub handler, to filter messages according the provided Waku topics, and then trigger the Waku handler.
I realize this probably sounds very confusing :), the naming isn't great.

And that would be in the case of full nodes. Light nodes would require another system probably.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, so then I have another problem. I need to decode the message. But from my understanding we do not yet have a message format do we? Hence my asking in nim-waku. Maybe @oskarth could also elaborate here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no idea what this PR is even trying to do

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do have a message format. It is documented here: https://specs.vac.dev/specs/waku/waku-v2.html#message

What isn't fully specced out there is: (a) how to deal with waku/content topics (not pubsub topics) and (b) how to deal with encryption of payload. Neither is necessary at this stage for the issues discussed on e.g. Monday.

For (a) there is vacp2p/rfc#156 and vacp2p/rfc#160 For (b) there is vacp2p/rfc#158

You don't need either to get a generic Topic Handler going.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, you make a good point that this extra filter that doesn't do anything currently (as it uses the same topic) is not needed to make this work. I thought it was fine as in my envision of the API, the filter would eventually filter messages on Waku topics which are provided through the subscribe call.


debug "subscribeTopic", topic=topic, subscribe=subscribe, peerId=peerId

if w.gossip_enabled:
Expand Down