-
Notifications
You must be signed in to change notification settings - Fork 246
docs: rewrite and restructure docs #3026
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
base: main
Are you sure you want to change the base?
Conversation
|
The latest Buf updates on your PR. Results from workflow CI / buf-check (pull_request).
|
|
Claude finished @tac0turtle's task in 2m 11s —— View job Documentation Review CompleteI've completed a comprehensive review of the documentation rewrite and restructure. This is an ambitious PR that adds substantial new documentation (10,323 additions across 65 files), but there are several issues that should be addressed before merging. Critical Issues1. Complete File DuplicationLocation: These files are completely identical (759 lines each). This creates a significant maintenance burden and confusion for users. Recommendation:
2. Incorrect Code Example PatternLocation: The migration guide shows an incorrect pattern for the start command: RunE: func(cmd *cobra.Command, _ []string) error {
return server.Start(cmd, evabci_server.StartHandler())
},This is inconsistent with the correct pattern shown in:
Correct pattern: RunE: func(cmd *cobra.Command, _ []string) error {
return evabci.StartHandler(cmd, app.New)
},Impact: This will cause node startup failures for users following the migration guide. High Priority Issues3. Broken Internal LinkLocation: The link targets 4. Typo with Double SpaceLocation: "Every node" has an extra space. 5. Stray Character in MarkdownLocation: There's a trailing 6. Inconsistent CapitalizationLocation: Note says "evolve" instead of "Evolve" for consistency with brand naming throughout docs. Medium Priority Issues7. Inconsistent Terminology - Backoff CapLocations:
Recommendation: Verify the correct value from the code and make it consistent across all mentions. 8. Trailing Spaces in Markdown HeadingLocation: Markdown heading has trailing spaces. 9. Outdated Flag DescriptionLocation: The description of required DA flags mentions Documentation Completeness Issues10. Multiple Placeholder FilesSeveral new files contain only TODO comments or minimal placeholder content:
Recommendation:
Positive ObservationsExcellent Work:
RecommendationsBefore Merging:
Architecture & Design:
Style & Consistency:
SummaryThis documentation rewrite represents significant effort and substantially improves the project's documentation. However, the critical issues (duplicate files, incorrect code example) and numerous smaller issues should be resolved before merging. The incorrect code example in the migration guide is particularly concerning as it will directly impact users. Verdict: Needs revision before merge. The foundation is excellent, but the execution issues need cleanup. |
Summary of ChangesHello @tac0turtle, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the project's documentation, providing a more organized and in-depth resource for developers and operators. It covers fundamental concepts, practical implementation steps for different execution environments, operational best practices, and detailed technical specifications, making it easier for users to understand, integrate, and manage Evolve-based chains. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3026 +/- ##
==========================================
- Coverage 55.66% 55.61% -0.06%
==========================================
Files 116 116
Lines 11477 11477
==========================================
- Hits 6389 6383 -6
- Misses 4389 4394 +5
- Partials 699 700 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Code Review
This pull request introduces a significant rewrite and restructuring of the documentation, adding a large volume of new content across concepts, guides, and reference sections. The effort to improve the documentation is commendable. However, there are several areas that need attention before this can be merged.
Firstly, there's a notable amount of content duplication, particularly between docs/concepts/block-lifecycle.md and docs/reference/specs/block-manager.md. This could lead to maintenance issues and should be resolved, perhaps by using includes or linking between documents.
Secondly, a large number of placeholder files have been added, containing only TODO comments. These should be either completed or removed.
Finally, there are various inconsistencies in technical details, formatting issues (like the use of :::: instead of ::: for VitePress admonitions), minor typos, and some broken links. Addressing these points will greatly improve the quality and usability of the new documentation.
| RunE: func(cmd *cobra.Command, _ []string) error { | ||
| return server.Start(cmd, evabci_server.StartHandler()) | ||
| }, |
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.
The implementation of the start command's RunE function appears to be incorrect and is inconsistent with other guides like integration-guide.md and getting-started/cosmos/integrate-ev-abci.md. The use of server.Start wrapping the evabci_server.StartHandler is not the standard pattern and will likely cause an error, preventing the node from starting. Please make this consistent with the other guides.
| RunE: func(cmd *cobra.Command, _ []string) error { | |
| return server.Start(cmd, evabci_server.StartHandler()) | |
| }, | |
| RunE: func(cmd *cobra.Command, _ []string) error { | |
| return evabci_server.StartHandler(cmd, app.New) | |
| }, |
| - **Centralized State Management**: The `retryStrategy` struct manages attempt counts, backoff timing, and gas price adjustments | ||
| - **Multiple Backoff Types**: | ||
| - Exponential backoff for general failures (doubles each attempt, capped at `BlockTime`) | ||
| - Mempool-specific backoff (waits `MempoolTTL * BlockTime` for stuck transactions) | ||
| - Success-based backoff reset with gas price reduction |
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.
| @@ -0,0 +1,60 @@ | |||
| # P2P | |||
|
|
|||
| Every node (both full and light) runs a P2P client using [go-libp2p][go-libp2p] P2P networking stack for gossiping transactions in the chain's P2P network. The same P2P client is also used by the header and block sync services for gossiping headers and blocks. | |||
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.
There is a typo with a double space after "Every".
| Every node (both full and light) runs a P2P client using [go-libp2p][go-libp2p] P2P networking stack for gossiping transactions in the chain's P2P network. The same P2P client is also used by the header and block sync services for gossiping headers and blocks. | |
| Every node (both full and light) runs a P2P client using [go-libp2p][go-libp2p] P2P networking stack for gossiping transactions in the chain's P2P network. The same P2P client is also used by the header and block sync services for gossiping headers and blocks. |
| # Custom Precompiles | ||
|
|
||
| <!-- TODO: Create custom precompiles guide (ev-reth specific) | ||
| - What are precompiles | ||
| - ev-reth custom precompiles: | ||
| - Mint precompile | ||
| - Others as they exist | ||
| - Adding custom precompiles to ev-reth | ||
| - Chainspec activation configuration | ||
| - Reference: ev-reth repository | ||
| --> |
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.
This file is a placeholder with a TODO comment. Placeholder content should be completed or the file should be removed before merging a documentation rewrite PR. This comment applies to several other new files in this PR as well, such as docs/guides/operations/troubleshooting.md, docs/guides/operations/upgrades.md, docs/guides/running-nodes/aggregator.md, docs/guides/running-nodes/attester.md, docs/guides/running-nodes/light-node.md, and several files under docs/reference/.
docs/guides/da-layers/celestia.md
Outdated
| Now that we are posting to the Celestia DA instead of the local DA, the `evolve start` command requires three DA configuration flags: | ||
|
|
||
| - `--evnode.da.start_height` | ||
| - `--evnode.da.auth_token` | ||
| - `--evnode.da.namespace` |
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.
The description of required DA flags is outdated. It lists --evnode.da.namespace as a required flag, but this flag is deprecated. The text should be updated to reflect the use of --evnode.da.header_namespace and --evnode.da.data_namespace, as correctly shown in the command example later in the document.
| Now that we are posting to the Celestia DA instead of the local DA, the `evolve start` command requires three DA configuration flags: | |
| - `--evnode.da.start_height` | |
| - `--evnode.da.auth_token` | |
| - `--evnode.da.namespace` | |
| Now that we are posting to the Celestia DA instead of the local DA, the `evolve start` command requires several DA configuration flags: | |
| - `--evnode.da.start_height` | |
| - `--evnode.da.auth_token` | |
| - `--evnode.da.header_namespace` | |
| - `--evnode.da.data_namespace` |
|
|
||
| This guide covers how to set up a full node to run alongside a sequencer node in a Evolve-based blockchain network. A full node maintains a complete copy of the blockchain and helps validate transactions, improving the network's decentralization and security. | ||
|
|
||
| > **Note: The guide on how to run an evolve EVM full node can be found [in the evm section](./evm/single.md#setting-up-a-full-node).** |
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.
There's a typo in the note. It should be Evolve instead of evolve for consistency.
| > **Note: The guide on how to run an evolve EVM full node can be found [in the evm section](./evm/single.md#setting-up-a-full-node).** | |
| > **Note: The guide on how to run an Evolve EVM full node can be found [in the evm section](./evm/single.md#setting-up-a-full-node).** |
|
|
||
| ### Store | ||
|
|
||
| The [Store] is initialized with `DefaultStore`, an implementation of the [store interface] which is used for storing and retrieving blocks, commits, and state. | |
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.
There is a stray | character at the end of this line which should be removed.
| The [Store] is initialized with `DefaultStore`, an implementation of the [store interface] which is used for storing and retrieving blocks, commits, and state. | | |
| The [Store] is initialized with `DefaultStore`, an implementation of the [store interface] which is used for storing and retrieving blocks, commits, and state. |
| - Used by all node types (sequencer, full, and light) | ||
| - Essential for maintaining the canonical view of the chain | ||
|
|
||
| ### Data Sync Service |
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.
| - [Block Components](./block-manager.md): Explains the modular component architecture for block processing in Evolve. | ||
| - [Block Validity](./block-validity.md): Details the rules and checks for block validity within the protocol. | ||
| - [Data Availability (DA)](./da.md): Describes how Evolve ensures data availability and integrates with DA layers. | ||
| - [Full Node](./full_node.md): Outlines the architecture and operation of a full node in Evolve. |
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.
The link to the full node specification appears to be broken due to a filename mismatch. The target file is full-node.md, not full_node.md.
| - [Full Node](./full_node.md): Outlines the architecture and operation of a full node in Evolve. | |
| - [Full Node](./full-node.md): Outlines the architecture and operation of a full node in Evolve. |
- Updated Celestia guide to clarify prerequisites, installation, and configuration for connecting Evolve chains to Celestia. - Enhanced Local DA documentation with installation instructions, configuration options, and use cases for development and testing. - Expanded troubleshooting guide with detailed diagnostic commands, common issues, and solutions for node operations. - Created comprehensive upgrades guide covering minor and major upgrades, version compatibility, and rollback procedures. - Added aggregator node documentation detailing configuration, block production settings, and monitoring options. - Introduced attester node overview with configuration and use cases for low-latency applications. - Removed outdated light node documentation. - Improved formatting and clarity in ev-reth chainspec reference for better readability.
Overview
This pr restructures the docs and writes new sections in order to be better suited for users