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

spec/waku-discovery #99

Merged
merged 2 commits into from
Feb 7, 2020
Merged

spec/waku-discovery #99

merged 2 commits into from
Feb 7, 2020

Conversation

decanus
Copy link
Contributor

@decanus decanus commented Feb 3, 2020

This seems to cover it, @oskarth do you feel this requires more detail?

@decanus decanus requested a review from oskarth February 3, 2020 23:57
waku/waku.md Outdated
@@ -495,11 +495,9 @@ Notes useful for implementing Waku mode.

### Node discovery

[Discovery v4](https://github.com/ethereum/devp2p/wiki/Discovery-Overview) SHOULD NOT be used, because it doesn't distinguish between capabilities. It will thus have a hard time finding Waku/Whisper nodes.
[Discovery v5](https://github.com/ethereum/devp2p/blob/master/discv5/discv5.md) MAY be used. However, it is quite bandwidth heavy for resource restricted devices.
Copy link
Contributor

Choose a reason for hiding this comment

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

Which draft version?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

wdym?

Copy link
Contributor

Choose a reason for hiding this comment

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

Discovery v5 isn't finalized, so it would probably be a good idea to specify which version we are targeting

E.g. Draft of October 2019. (ethereum/devp2p@193bc09) (can be in a footnote if is too wordy)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah fair, I am not entirely sure of which version I think @adambabik or @cammellos can answer that.

Choose a reason for hiding this comment

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

We use the deprecated version. It has been officially deprecated and it's not compliant with the current discv5 spec.

I would only keep info about EIP-1459 here.

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, I could remove the entire discv5 stuff, @oskarth how do you feel?

Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is the Waku spec and not Status v1 reality, sure

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.

Looks OK to me, I'd let Adam have a look at it as well (he's back on Thu)

@decanus decanus merged commit 03949cf into master Feb 7, 2020
@decanus decanus deleted the spec/waku-discovery branch February 7, 2020 12:57
kdeme pushed a commit that referenced this pull request May 18, 2020
* changed waku discovery

* Update waku.md
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.

3 participants