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

Add section on compatiblity and waku v1 v2 bridge #179

Merged
merged 3 commits into from
Sep 15, 2020
Merged

Conversation

kdeme
Copy link
Contributor

@kdeme kdeme commented Sep 14, 2020

This should resolve #159

I will create a new issue for deciding on the optional encryption & signing of the payload.

Copy link
Contributor

@oskarth oskarth left a comment

Choose a reason for hiding this comment

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

Overall looks solid, some questions and comments

Publishing such packet will require the creation of a new `Message` with a
new `WakuMessage` as data field. The `data` and `topic` field from the Waku v1
`Envelope` MUST be copied to the `payload` and `contentTopic` fields of the
`WakuMessage`. Other fields such as nonce, expiry and ttl will be dropped as
Copy link
Contributor

Choose a reason for hiding this comment

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

Separate from this PR, but I wonder if there's something we are missing as we drop these fields

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, nonce is only there for PoW, which isn't there anymore.

I'm not sure how message lifetime works in pubsub. I've tried to figure it out before, but I only noticed the use of a timed cache. No information of ttl within the message format itself afaik.
But even if there was, I doubt you could insert ttl values manually (API probably wouldn't allow that?), so you'd have to hack it in there. Probably not the best idea.

specs/waku/waku-v2.md Show resolved Hide resolved
Waku v2 network. More specifically, the bridge SHOULD publish
this through the Waku Relay (PubSub domain).

Publishing such packet will require the creation of a new `Message` with a
Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose dealing with the contents of data field is out of scope of this PR and we can deal with it here #181 there's a question of Waku v2 receiving RLP envelopes here, but not much that can be done about it at this level I suppose.

Possibly some indication that it came from Waku v1 network? That way clients using Waku v2 can decide to drop support for RLPx in future when it doesn't feel like supporting that data content anymore. WDYT? Could be indicated in the contentTopic field perhaps, such as wakuv1/contenttopic (similar to eth2 namespaces). Or a separate field.

Copy link
Contributor Author

@kdeme kdeme Sep 15, 2020

Choose a reason for hiding this comment

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

The contents of data is indeed something to answer in #181.

But the idea is not to pass any RLP encoded data to the Waku v2 network. It is true that the Waku v1 Envelope type gets RLP encoded. But in the bridge it gets decoded and then the specific data and payload fields are used and placed in a new (protobuf encoded) WakuMessage.

Of course of there is any RLP encoded data within the data field, that is up to the application.

Copy link
Contributor

Choose a reason for hiding this comment

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

Currently the data field is part of the protocol (though optional as you note): https://specs.vac.dev/specs/waku/envelope-data-format.html - I suppose it can be read in a custom fashion, but it isn't a straightforward protobuf. Though the payload within the data field is.

Anyway, for the other issue

Co-authored-by: Oskar Thorén <ot@oskarthoren.com>
specs/waku/waku-v2.md Outdated Show resolved Hide resolved
Co-authored-by: Dean Eigenmann <7621705+decanus@users.noreply.github.com>
@oskarth oskarth merged commit 7ef7c68 into master Sep 15, 2020
@oskarth oskarth deleted the wakuv2bridge branch September 15, 2020 09:10
This pull request was closed.
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 this pull request may close these issues.

Waku v2: Briding with waku v1
3 participants