update: topic naming #579

kaiserd · 2023-03-03T14:25:52Z

Background

This PR

shortens the topic-name part indicating static sharding from static-rshard to `rs
adjusts 23/WAKU2-TOPICS to reflect updates made by 51/WAKU2-RELAY-SHARDING.
removes the superfluous /{encoding} part of the pub sub topic name, including removing /proto from the default topic.
(Copied from a note added to 23/ with this PR:)

In previous versions of this document, the default topic was /waku/2/default-waku/proto.
The now deprecated /proto part indicated that the data field in PubSub is serialized/encoded as protobuf.
The inspiration for this format was taken from
Ethereum 2 P2P spec.
However, because the payload of messages transmitted over 11/WAKU2-RELAY must be a 14/WAKU2-MESSAGE,
which specifies the wire format as protobuf,/proto is the only valid encoding.
This makes the /proto indication obsolete.
The encoding of the payload field of a Waku Message is indicated by the /{encoding} part of the content topic name.
Specifying an encoding is only significant for the actual payload/data field.
Waku preserves this option by allowing to specify an encoding for the WakuMessage payload field as part of the content topic name.

Open

~~adjust test vectors in 14/WAKU2-MESSAGE to the updated default topic name~~
~~adjust 36/WAKU2-BINDINGS-API to the updated default topic name~~
(edit: we will keep the old default topic for backwards compatibility.)

Future

Adjust the recommendation in 23/WAKU2-TOPICS regarding usage of the default topic.
- give a rough user count threshold estimate where it is worth moving to a sharded setup
- this will be done after MVP (with insights gained from MVP)

jm-clius

Roughly agree with these changes, except changing the default pubsub topic itself, which would be an unnecessarily complicated migration (it is used by Status, for example).

content/docs/rfcs/23/README.md

jm-clius · 2023-03-03T14:37:31Z

content/docs/rfcs/23/README.md


-`/waku/2/default-waku/proto`
+`/waku/2/default-waku/`


I'm not sure we need to change the default named pubsub topic (this would require a finicky migration). We can just mention that for legacy reasons it has the /proto encoding trailer, but that we no longer support semantic interpretation of that suffix. In other words, it still adheres to the format /waku/2/{topic-name}, but for legacy reasons {topic-name} is default-waku/proto. The same goes for the recently introduced /waku/2/dev-waku/proto topic.

Agree with @jm-clius. For compatibility with the existing implementations, we should support both topics as the default topic:

/waku/2/default-waku/proto

/waku/2/default-waku

The same way Protocol labs does with the multiaddr protocol ipfs, which is an alternate form for p2p for backward compatibility reasons.

I agree with keeping /waku/2/default-waku/proto
Having two default topics would be a third option.
If I understand correctly, @jm-clius suggestion is only use /waku/2/default-waku/proto as the default topic.
While /waku/2/default-waku would be a valid topic, it is is not declared as a default.

Co-authored-by: Hanno Cornelius <68783915+jm-clius@users.noreply.github.com>

kaiserd · 2023-03-03T14:43:44Z

Agreed. I will change that back and explain the new format with another example.

LNSD · 2023-03-03T15:15:07Z

content/docs/rfcs/23/README.md

 ### Topic sharding example

-Topic sharding is currently not supported by default, but is planned for the future in order to deal with increased network traffic.
-Here's an example of what this might look like:
+The following is an example of named sharding, as specified in [51/WAKU2-RELAY-SHARDING](/spec/51).

 ```
-waku/2/waku-9_shard-0/proto
+waku/2/waku-9_shard-0/
 ...
-waku/2/waku-9_shard-9/proto
+waku/2/waku-9_shard-9/
 ```

 This indicates explicitly that the network traffic has been partitioned into 10 buckets.

-### Compression example
-
-Not yet implemented, but would be easy to add with:
-
-`/waku/2/default-waku/proto_snappy`
+Besides named sharding, [51/WAKU2-RELAY-SHARDING](/spec/51) specifies two more sharding methods: static sharding and automatic sharding.


This section does not convince me because this is referencing something that references this RFC (I see a cyclic dependency 😅). I would not specify anything about sharding in RFC 23.

Additionally, you have already provided some examples above, rs/0/2. So I would leave this part for RFC 51.

The reason I mention that here is: 23/ gives some advice on how to shard, and how to use topics beyond just naming.
If we leave this in, imo, it makes sense to ref to 51/.

I see two options:

leave it as it is (inlc. the cyclic ref)

remove suggestions beyond naming (also the advice to use the default topic) from 23/.

Imo, the second option would be cleaner, and I agree the cyclic ref is not nice.
I chose 1) to have minimal changes, but happy to switch to 2).
Wdyt? Also cc @jm-clius

(2) makes the most sense.

I've been trying to find specific guidelines on cyclic dependencies and cross-referencing in RFCs when evolving a concept. This of course cannot be done for normative references, but IMO, since this is not a normative RFC and 51 is not a normative reference, I don't have a major problem to have an appendix/section which is not part of the main text that briefly points people to the evolution of ideas in this RFC in 51. It makes navigating to the latest "full picture" a bit easier.

Just to be sure, that would be (1) in the list?
So, keeping the suggestions as well as refs to /51

(To me both are fine. Both have different advantages. And I agree with the point of easier navigation :)).

Mmm, I had more in mind simplifying this whole section to a line or two in an appendix ("Future work"?) with possible reference to 51 for easier navigation (but, in other words, the specification for named sharding and default pubsub topic happens there, not in 23). But I don't have a strong opinion here, because we tend to be more pragmatic when it's not a normative spec. Might make sense to just do the simplest thing here for now.

I like this the most :).
Just kept the topic recommendation in 23/ to not change too much, given the RFC moved to draft state.
But as you said, it is informational and should be fine.
I'll adjust it, and we can have another round of discussion on this.
It is not a blocker, because in any case we agreed on how the implementation should look like :).

LNSD · 2023-03-03T15:15:26Z

content/docs/rfcs/23/README.md

@@ -184,3 +197,5 @@ Copyright and related rights waived via
 8. [15/WAKU-BRIDGE](/spec/15)

 9. [26/WAKU-PAYLOAD](/spec/26)
+
+10. [51/WAKU2-RELAY-SHARDING](/spec/51)


Same thing with the references.

Same reason as above.

LNSD · 2023-03-03T15:20:27Z

content/docs/rfcs/51/README.md

 With named sharding, managing discovery falls into the responsibility of apps.

-The default Waku pubsub topic `/waku/2/default-waku/proto` can be seen as a named shard available to all app protocols.
+The default Waku pubsub topic `/waku/2/default-waku` can be seen as a named shard available to all app protocols.


The same comment as @jm-clius pointed out, for compatibility with the existing implementations, we should support both topics as the default topic:

/waku/2/default-waku/proto

/waku/2/default-waku

The same way Protocol labs does with the multiaddr protocol ipfs which is an alternate form for p2p for backward compatibility reasons.

(see above)

kaiserd · 2023-03-06T06:29:28Z

Thanks for the feedback :). My replys are in-line.
I'll update the PR once we argee on cyclic ref, vs. removing sharding info from 23/.

alrevuelta

As I can see we are removing proto from the gossipsub topic but its still in the content topic? Is that correct or am I missing something?

alrevuelta · 2023-03-07T16:47:17Z

content/docs/rfcs/23/README.md


 ```
-waku/2/waku-9_shard-0/proto
+waku/2/waku-9_shard-0/


why not waku/2/rs/9/0? like in the other example?

This is an example for named sharding, not static sharding.

kaiserd · 2023-03-08T07:37:56Z

As I can see we are removing proto from the gossipsub topic but its still in the content topic? Is that correct or am I missing something?

That is correct. Reason is in the RFC update (this PR).

kaiserd · 2023-03-09T11:24:51Z

Updates in 233fa61 :

minimize reference to 51/ in 23/
- still, imo, some reference to 51/ makes sense to give context (as @jm-clius suggested)
move more normative parts of 23/ into 51/
- the suggestion to use the default topic now moved into the security considerations section of 51/
some editing towards RFC parlance

Wdyt?
(Adding the rs encoding option to 51/ will be part of a new PR.)

jm-clius

LGTM!

content/docs/rfcs/51/README.md

Co-authored-by: Hanno Cornelius <68783915+jm-clius@users.noreply.github.com>

LNSD

LGTM ✅

update: topic naming

60c87a1

kaiserd requested review from richard-ramos, LNSD, jm-clius and fryorcraken March 3, 2023 14:25

kaiserd mentioned this pull request Mar 3, 2023

Roadmap(SeM): Secure Scaling vacp2p/research#154

Closed

13 tasks

jm-clius reviewed Mar 3, 2023

View reviewed changes

kaiserd and others added 2 commits March 3, 2023 15:42

Update content/docs/rfcs/23/README.md

d4706fc

Co-authored-by: Hanno Cornelius <68783915+jm-clius@users.noreply.github.com>

Update content/docs/rfcs/23/README.md

f45d38c

Co-authored-by: Hanno Cornelius <68783915+jm-clius@users.noreply.github.com>

LNSD reviewed Mar 3, 2023

View reviewed changes

LNSD mentioned this pull request Mar 3, 2023

feat(message): differentiate content and pubsub topic namespacing waku-org/nwaku#1589

Merged

5 tasks

alrevuelta reviewed Mar 7, 2023

View reviewed changes

update: 23/ more informational, 51/ normative

233fa61

kaiserd requested review from LNSD and jm-clius March 9, 2023 11:25

jm-clius approved these changes Mar 9, 2023

View reviewed changes

content/docs/rfcs/51/README.md Outdated Show resolved Hide resolved

content/docs/rfcs/51/README.md Outdated Show resolved Hide resolved

kaiserd and others added 2 commits March 9, 2023 17:24

Update content/docs/rfcs/51/README.md

95248e6

Co-authored-by: Hanno Cornelius <68783915+jm-clius@users.noreply.github.com>

Update content/docs/rfcs/51/README.md

cc1bd8b

Co-authored-by: Hanno Cornelius <68783915+jm-clius@users.noreply.github.com>

LNSD approved these changes Mar 13, 2023

View reviewed changes

kaiserd merged commit f767fd1 into master Mar 13, 2023

kaiserd deleted the update/topic-naming branch March 13, 2023 09:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update: topic naming #579

update: topic naming #579

kaiserd commented Mar 3, 2023 •

edited

Loading

jm-clius left a comment

jm-clius Mar 3, 2023

LNSD Mar 3, 2023

kaiserd Mar 6, 2023

kaiserd commented Mar 3, 2023

LNSD Mar 3, 2023

LNSD Mar 3, 2023

kaiserd Mar 6, 2023

jm-clius Mar 6, 2023

kaiserd Mar 6, 2023

jm-clius Mar 6, 2023

kaiserd Mar 6, 2023

LNSD Mar 3, 2023

kaiserd Mar 6, 2023

LNSD Mar 3, 2023 •

edited

Loading

kaiserd Mar 6, 2023

kaiserd commented Mar 6, 2023

alrevuelta left a comment

alrevuelta Mar 7, 2023 •

edited

Loading

kaiserd Mar 8, 2023

kaiserd commented Mar 8, 2023

kaiserd commented Mar 9, 2023 •

edited

Loading

jm-clius left a comment

LNSD left a comment


		`/waku/2/default-waku/proto`
		`/waku/2/default-waku/`

update: topic naming #579

update: topic naming #579

Conversation

kaiserd commented Mar 3, 2023 • edited Loading

Background

Open

Future

jm-clius left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kaiserd commented Mar 3, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LNSD Mar 3, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kaiserd commented Mar 6, 2023

alrevuelta left a comment

Choose a reason for hiding this comment

alrevuelta Mar 7, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kaiserd commented Mar 8, 2023

kaiserd commented Mar 9, 2023 • edited Loading

jm-clius left a comment

Choose a reason for hiding this comment

LNSD left a comment

Choose a reason for hiding this comment

kaiserd commented Mar 3, 2023 •

edited

Loading

LNSD Mar 3, 2023 •

edited

Loading

alrevuelta Mar 7, 2023 •

edited

Loading

kaiserd commented Mar 9, 2023 •

edited

Loading