-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add data-structures dir with rlp page #5758
Conversation
Gatsby Cloud Build Reportethereum-org-website-dev 🎉 Your build was successful! See the Deploy preview here. Build Details🕐 Build time: 15m PerformanceLighthouse report
|
Co-authored-by: Joshua <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jmcook1186 this is looking great. Mostly nitpicks or site conventions I've highlighted. I've suggested simplifying the language in places or using a more active tone. Feel free to disregard what you don't find appropriate/correct or ask for any clarification :-)
Co-authored-by: Joshua <[email protected]>
Co-authored-by: Joshua <[email protected]>
Co-authored-by: Joshua <[email protected]>
Co-authored-by: Joshua <[email protected]>
Co-authored-by: Joshua <[email protected]>
Co-authored-by: Joshua <[email protected]>
Co-authored-by: Joshua <[email protected]>
Co-authored-by: Joshua <[email protected]>
Co-authored-by: Joshua <[email protected]>
Co-authored-by: Joshua <[email protected]>
Co-authored-by: Joshua <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had a look at the MPT section, but not yet at RLP and SSZ. Had some nits but generally looks good
src/content/developers/docs/data-structures/patricia-merkle-trie/index.md
Outdated
Show resolved
Hide resolved
src/content/developers/docs/data-structures/patricia-merkle-trie/index.md
Outdated
Show resolved
Hide resolved
src/content/developers/docs/data-structures/patricia-merkle-trie/index.md
Outdated
Show resolved
Hide resolved
src/content/developers/docs/data-structures/patricia-merkle-trie/index.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jmcook1186 Sorry for delays reviewing this, but nice work! This looks great overall, and with a couple small adjustments I think we can get this in soon.
In summary:
- I agree we should consider expanding this bucket from "Data structures" to perhaps "Data structures and encoding"
- Just need to clean up the path and links to make sure we use either
patricia-merkle-trie
orpatricia-merkle-tries
- Couple small typos I left fix suggestions for
Otherwise I feel like this is ready to be brought in and any additional fine-tuning can be raised as issues.
src/content/developers/docs/data-structures/patricia-merkle-trie/index.md
Outdated
Show resolved
Hide resolved
src/content/developers/docs/data-structures/patricia-merkle-trie/index.md
Outdated
Show resolved
Hide resolved
committing the straightforward suggestions from Wackerow and S1na reviews Co-authored-by: Sina Mahmoodi <[email protected]> Co-authored-by: Paul Wackerow <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I worked through the RLP page and some of the SSZ page. It might be easier to break them each into a PR. MPT and RLP are mostly good to go IMO but SSZ might need some more review.
Also something else is: specially the MPT and RLP pages seem to have as their audience people wanting to implement them. They're detailed explanations of each. I think there's also room for a more high-level explanation for people wanting to use them, maybe with some examples from JS or Python libraries. I don't mean for this PR, but in general.
src/content/developers/docs/data-structures-and-encoding/index.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple of broken links to fix that I'll come back to later today, but other than that this is good to merge 👍
|
||
Patricia Merkle Tries are structures that encode key-value pairs into a deterministic and cryptographically authenticated trie. These are used extensively across Ethereum's execution layer. | ||
|
||
[More on Patricia Merkle Tries](/developers/docs/data-structures/patricia-merkle-trie) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Broken link
Description
This PR adds a "data-structures" directory to /developers/docs/ and initializes with recursive-length prefix (RLP) material migrated over from eth.wiki. I added a basic landing page at
/developers/docs/data-structures/index.md
with placeholders for other pages soon to be migrated across. For the RLP page itself (/developers/docs/data-structures/rlp/index.md
the information is migrated from eth.wiki almost verbatim except for removing dead links, adding two introductory sentences and bringing the formatting in line with other ethereum.org pages.The data-structures directory can be a bucket for other related materials to be migrated over from eth.wiki and ethdocs.org that do not yet have a sensible home on ethereum.org.
update: 30th March 22 I have now added pages on SSZ and Patricia Merkle Tries to the data structures directory. The material on SSZ is new, the material on Patricia Merkle tries is migrated almost verbatim from eth.wiki except for very minor addition to make clear these structures are ubiquitous on the execution layer and formatting for ethereum.org. All new material is added to the site menus.
The additions look like this in the dir tree:
Related Issue
This resolves suggested PR 10 from #5690 and will enable several other PRs from #5690 and #5731.