feat: DevP2P #255
feat: DevP2P #255
Conversation
raa-dev
commented
May 5, 2024
raa-dev
commented
- devp2p section added into the sidebar
- devp2p page and it's scaffoling added
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
raa-dev
added
documentation
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
raa-dev
force-pushed
the
148-devp2p
branch
2 times, most recently
from
988b4ed to
1236e8a
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
Hey there I finally merged the two devp2p PRs into one. I cherry-picked @RazorClient meaningful commits. I think it's ready for review. Also, I don't know why but the spell check is still pointing two "words" but if you check where they come from, they are part of an hex address. I tried to wrap it into `` , but it didn't work and I do not think it worths to add them to the wordlist... any suggestions here? Any further suggestion from the review process, i'll be in touch. @raxhvl @taxmeifyoucan Pd. I suggest to mark the other PR #365 as abandoned/closed, and once the current one is ready for merging let's use the squash one. cc. @SiddharthV1 |
|
Thanks @raa-dev for wrapping this up. I will review it. Also fixed the spell check error. |
There was a problem hiding this comment.
Thanks a lot for putting this all together! I think the page is great.
Regarding technical details, maybe @angaz could provide more feedback
docs/wiki/EL/devp2p.md
Outdated
|
|
||
| > :warning: This article is a [stub](https://en.wikipedia.org/wiki/Wikipedia:Stub), help the wiki by [contributing](/contributing.md) and expanding it. | ||
| This section will cover the networking protocol used by the Execution Layer (EL). | ||
| First, we will provide some background on computer networks and then we will dive into the |
There was a problem hiding this comment.
I don't think EL networking page should go as far as explaining OSI, it can assume some prior knowledge. There is a lot of existing resources and your explanation is also good so maybe it could live somewhere in development resources and be referenced here and CL networking as knowledge dependency
There was a problem hiding this comment.
Just to be on the same page here. Would this imply to remove the OSI introduction section, right?
I took a look at the dev resources and most of them -if not all- were external sources. Want me to refer the books I based the section from there?
There was a problem hiding this comment.
I would move the section elsewhere and reference it maybe with other sources. You can create a page in development section dedicated to networking with your explanation and links to all relevant resources
There was a problem hiding this comment.
Moved the networking overview to the dev resources already. Left some useful information as part of the introduction though.
Please mark the conversation as resolved if you consider it that way, otherwise i'll follow up any further comment/suggestion
docs/wiki/EL/devp2p.md
Outdated
| Where ENR is the Ethereum Node Record, a standard format for connectivity for nodes. Which it is explained below. | ||
|
|
||
| --- | ||
| Currently, the execution clients are using the [Discv4 protocol](https://github.com/ethereum/devp2p/blob/master/discv4.md) for the discovery process, although it is planned to be migrated to [Discv5](https://github.com/ethereum/devp2p/blob/master/discv5/discv5.md) in the future. |
There was a problem hiding this comment.
I believe at least some clients already support both but v4 wasn't fully deprecated yet? The point is it's definitely used already
There was a problem hiding this comment.
Afaik the goal is a migration from v4 to v5. As of the discv5 page from devp2p says it's a WIP still, so I don't know if I'm missed something regarding to the deprecation.
I'll dig about this a bit in order to confirm it and update the section
There was a problem hiding this comment.
That's a pretty old page, discv5 is the standard today in CL and Portal network, EL is slower to transition but not sure what is the latest, which clients support it
There was a problem hiding this comment.
Good catch there, thanks for the comment! Really appreciated it! I took some time to dive in some of the execution clients in order to verify the discv5 status, and as you mentioned most of them implemented already. I added a table in the section to reflect this status as of today. In each one of the clients I attached the github references
-- Feat: Added recommendations
--- conflicts fixed on pectra page Co-authored-by: Mário Havel <[email protected]>
This comment was marked as outdated.
This comment was marked as outdated.
4 participants
