Skip to content
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

Updates to el-architecture #184

Conversation

SiddharthV1
Copy link
Contributor

@SiddharthV1 SiddharthV1 commented Apr 16, 2024

I enthusiastically merged the previous PR before reading through all of the comments. This PR is used to monitor any revisions to the recommendations that were made during the previous PR's review.

PR Checklist

Architecture File

  • Include the changes that @taxmeifyoucan already made when reviewing my previous PR.
  • Maintain a single, top-level heading.
  • Content Standardization
  • Expand the Overview
  • Move code over from the specs file into architecture
  • Absorb content from EPFsg presentations
  • Add introductory sections on EVM, devp2p, MPT

Specs File
(may do a separate PR for this since this file is long and any work on it is time consuming)

  • Content standardization
  • Fix LLM introduced artifacts and artificial phrasing in the specs document
  • Fix Typos and grammar issue
  • Maintain a single, top-level heading.

El architecture file update :
Added the modifications that were already done by Mario during
the review of my last PR
Prettier ends up breaking the table by adding an additional
column in the header
@SiddharthV1 SiddharthV1 self-assigned this Apr 16, 2024
@SiddharthV1 SiddharthV1 changed the title Updates to el-architecute Updates to el-architecture Apr 16, 2024
@taxmeifyoucan
Copy link
Contributor

Thanks for following up on this! I will leave detailed review later as the content progresses. Reading it now, the structure and wording is great, just keep an eye on those extra spaces, capital letters, etc

@taxmeifyoucan taxmeifyoucan mentioned this pull request Apr 24, 2024
16 tasks

Note: client specific overviews will go here

### Reth
Copy link
Contributor

Choose a reason for hiding this comment

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

The part dedicated to reth is pretty client specific and thorough. It could become its own page similar to #191 with extra info for contributors and make this page leaner.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds great, will do that at some point today


2. **Blob Pools**: Unlike legacy pools, blob pools maintain a priority heap for transaction eviction but incorporate distinct mechanisms for operation. Notably, the implementation of blob pools is well-documented, with an extensive comments section available for review [here](https://github.com/ethereum/go-ethereum/blob/064f37d6f67a012eea0bf8d410346fb1684004b4/core/txpool/blobpool/blobpool.go#L132). A key feature of blob pools is the use of logarithmic functions in their eviction queues.

### EVM
Copy link
Contributor

Choose a reason for hiding this comment

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

Following pages - EVM, devp2p, mpt, rlp, etc have their own dedicate page. You can just provide a context and refer people to other pages to keep it relatively simple :)

@taxmeifyoucan
Copy link
Contributor

Closing in favor of #224

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

Successfully merging this pull request may close these issues.

2 participants