Skip to content
This repository has been archived by the owner on Jun 14, 2024. It is now read-only.

Waku v2 contentTopic type #180

Closed
kdeme opened this issue Sep 14, 2020 · 8 comments · Fixed by #217
Closed

Waku v2 contentTopic type #180

kdeme opened this issue Sep 14, 2020 · 8 comments · Fixed by #217
Assignees

Comments

@kdeme
Copy link
Contributor

kdeme commented Sep 14, 2020

Problem

In waku v2 the contentTopic is currently defined as an optional string.
There is problem with this definition:
In waku v1, this is an array of 4 bytes. When bridging waku v1 to waku v2 this gives an issue. Either we need to change it to an array of 4 bytes or we need some consistent way of converting this (probably having an overlap of topics). This is potentially also a problem for applications currently used to the waku v1 4 bytes topic.

Acceptance criteria

Make sure that contentTopic can be used in:

  • current use cases
  • bridging

Possible Solutions

  • Change definition to byte array of 4 bytes long.

Notes

See also bridging PR: #179

@kdeme kdeme added the waku-v2 label Sep 14, 2020
@oskarth
Copy link
Contributor

oskarth commented Sep 15, 2020

One thing I've had on my backlog but didn't turn into an issue yet is namespaced topics a la Eth2 p2p spec. What are your thoughts on this? That way perhaps we can have have something like /waku/v1/<prev-topichash>/encoding.

From p2p spec:

Topics are plain UTF-8 strings and are encoded on the wire as determined by protobuf (gossipsub messages are enveloped in protobuf messages). Topic strings have form: /eth2/ForkDigestValue/Name/Encoding. This defines both the type of data being sent on the topic and how the data field of the message is encoded. and https://github.com/ethereum/eth2.0-specs/blob/dev/specs/phase0/p2p-interface.md#topics-and-messages

@kdeme
Copy link
Contributor Author

kdeme commented Sep 15, 2020

Yeah, that sounds like a good idea for keeping compatibility when bridging, however allowing different topic type in the future.

We do have to watch out with how much information we provide in such string.
(Side node: ofcoursein eth2 this topic string is different from our contentTopic, as it is on pubsub level, and for Waku it is not)

@decanus
Copy link
Contributor

decanus commented Oct 14, 2020

So I started playing around with this in nim-waku, eg just changing it to bytes. It will most likely be a huge process so what I am thinking is maybe just using the hashed version of byte4 hex encoded?

@oskarth
Copy link
Contributor

oskarth commented Oct 15, 2020

What is the huge process involved?

@decanus
Copy link
Contributor

decanus commented Oct 15, 2020

@oskarth from playing around it would require quite a bit of code changes. And I am currently uncertain how easily it will be to change in certain scenarios. However, it makes more sense to do now rather than later.

@oskarth
Copy link
Contributor

oskarth commented Oct 16, 2020

I still don't understand what the code changes required are. Can you please be more specific?

As far as I can tell it is mostly casting a string to a byte string in a bunch of places...

@decanus
Copy link
Contributor

decanus commented Oct 16, 2020

@oskarth If I byte sequence is taken then the protobuf decoding could be a little weird. But other than that should be easy. I've also changed it to a fixed32 so in general should be fine.

@oskarth
Copy link
Contributor

oskarth commented Oct 19, 2020

Ok, is there a corresponding implementation issue? Would be good to link it

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants