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

Add public docs for OP Stack network topologies #1167

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

krofax
Copy link
Contributor

@krofax krofax commented Dec 9, 2024

Description

This PR introduces public-facing documentation outlining acceptable network topologies for OP Stack network deployments.

Summary:

  • Externalized content from Zach’s internal network design documentation to address confusion noted in support tickets.

  • Provides an example configuration with detailed recommendations for various node types, including sequencers, Tx ingress nodes, archive nodes, and public RPC nodes.

  • Highlights best practices for redundancy, security, and monitoring.

  • Incorporates YAML configuration snippets for easier implementation.

Tests

Additional context

Metadata

Include a link to any github issues that this may close in the following form:

@krofax krofax requested a review from a team as a code owner December 9, 2024 13:54
Copy link

netlify bot commented Dec 9, 2024

Deploy Preview for docs-optimism ready!

Name Link
🔨 Latest commit 25a479e
🔍 Latest deploy log https://app.netlify.com/sites/docs-optimism/deploys/675aefc5d133ae0008202054
😎 Deploy Preview https://deploy-preview-1167--docs-optimism.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Contributor

coderabbitai bot commented Dec 9, 2024

📝 Walkthrough
📝 Walkthrough

Walkthrough

The pull request introduces several modifications to two primary files: words.txt and architecture.mdx. In words.txt, multiple new terms have been added, including "Bootnode," "bootnode," "Snapsync," "snapsync," and "monitorism," among others. Existing terms have also undergone normalization, such as changing "accountqueue" to "ACCOUNTQUEUE" and "blocklogs" to "BLOCKLOGS." The overall structure of the file remains intact, with no deletions or significant alterations beyond these adjustments.

In architecture.mdx, the document has been expanded to include a new section on the "Sequencer," detailing its role in block creation and interaction with other components. Additional subsections cover topics like "Sequencer isolation" and "Replica node," providing insights into security measures and scaling RPC requests. A new section on "Bootnodes" has been added, explaining their role in peer discovery and including configuration examples for various node types. These changes enhance the document's comprehensiveness regarding the architecture and operational guidelines for the rollup protocol.

Possibly related PRs

  • 3 new node pages #679: This PR includes changes to the words.txt file, which is directly related to the main PR that also modifies words.txt by adding and normalizing terms.
  • Deploy section #729: This PR also modifies the words.txt file, adding new entries and making changes that align with the normalization efforts described in the main PR.
  • adding op-conductor documentation #818: The changes in this PR to the words.txt file involve adding new terms, which relates to the main PR's focus on expanding and normalizing entries in the same file.
  • op-deployer + opcm documentation #934: This PR introduces new entries in the words.txt file, which is consistent with the main PR's goal of adding and standardizing terms.
  • superchainERC20 #986: The modifications to words.txt in this PR include adding new terms, which aligns with the main PR's changes aimed at enhancing the vocabulary within the file.
  • Op deployer updates #1020: This PR includes updates to words.txt, focusing on normalization and consistency, which is a key aspect of the main PR's objectives.
  • adding block time research #1034: The changes in this PR to words.txt involve adding and modifying terms, which directly relates to the main PR's focus on enhancing the file's content.
  • Clean up explainer #1107: This PR updates the explainer.mdx file, which includes references to terms that may be affected by the changes in words.txt, indicating a connection to the main PR's focus on terminology.
  • Add clarification re L1Admin #1145: The updates in this PR to the explainer.mdx file include clarifications that may reference terms in words.txt, linking it to the main PR's changes.
  • Move USDC Section to OP Mainnet-Specific #1160: This PR's updates to the documentation regarding USDC may reference terms in words.txt, connecting it to the main PR's focus on terminology normalization.

Suggested labels

tutorial

Suggested reviewers

  • zainbacchus
  • sbvegan

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Experiment)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (5)
pages/stack/network-topology.mdx (5)

22-46: Maintain consistent capitalization of technical terms.

Update the following terms to match the project's terminology:

  • Line 24: "rollup" should be "Rollup" (as defined in words.txt)
  • Line 43: "Tx" should be consistent throughout the document

115-117: Enhance security context for Public RPC configuration.

Consider adding explanations for:

  • Why transaction pool gossip should be disabled
  • Which specific RPCs should be whitelisted and why
  • Security implications of consensus_aware mode

131-131: Add missing comma in list.

Add a comma after "sequencers" in the list of critical roles.

-*   Redundancy: Always have backup nodes for critical roles like sequencers and archive nodes.
+*   Redundancy: Always have backup nodes for critical roles like sequencers, and archive nodes.
🧰 Tools
🪛 LanguageTool

[uncategorized] ~131-~131: Possible missing comma found.
Context: ...: Always have backup nodes for critical roles like sequencers and archive nodes. * ...

(AI_HYDRA_LEO_MISSING_COMMA)


143-150: Add comments explaining critical configuration values.

Consider adding inline comments to explain:

  • The security implications of P2P_NO_DISCOVERY
  • The purpose of P2P_PEER_BANNING
  • The format expected for <static peer list>

160-166: Consider adding more common FAQs.

Suggested additional topics:

  • Recommended hardware requirements for different node types
  • Backup and disaster recovery procedures
  • Monitoring best practices
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between e869986 and 7c81f23.

⛔ Files ignored due to path filters (1)
  • public/img/op-stack/protocol/opstack-network-topology.png is excluded by !**/*.png
📒 Files selected for processing (3)
  • pages/stack/_meta.json (1 hunks)
  • pages/stack/network-topology.mdx (1 hunks)
  • words.txt (2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
pages/stack/network-topology.mdx (1)

Pattern **/*.mdx: "ALWAYS review Markdown content THOROUGHLY with the following criteria:

  • Use proper nouns in place of personal pronouns like 'We' and 'Our' to maintain consistency in communal documentation.
  • Avoid gender-specific language and use the imperative form.
  • Monitor capitalization for emphasis. Avoid using all caps, italics, or bold for emphasis.
  • Ensure proper nouns are capitalized in sentences.
  • Apply the Oxford comma.
  • Use proper title case for buttons, tab names, page names, and links. Sentence case should be used for body content and short phrases, even in links.
  • Use correct spelling and grammar at all times (IMPORTANT).
  • For H1, H2, and H3 headers:
    1. Use sentence case, capitalizing only the first word.
    2. Preserve the capitalization of proper nouns, technical terms, and acronyms as defined in the 'nouns.txt' file located in the root directory of the project.
    3. Do not automatically lowercase words that appear in the 'nouns.txt' file, regardless of their position in the header.
  • Flag any headers that seem to inconsistently apply these rules for manual review.
  • When reviewing capitalization, always refer to the 'nouns.txt' file for the correct capitalization of proper nouns and technical terms specific to the project.
    "
🪛 LanguageTool
pages/stack/network-topology.mdx

[uncategorized] ~131-~131: Possible missing comma found.
Context: ...: Always have backup nodes for critical roles like sequencers and archive nodes. * ...

(AI_HYDRA_LEO_MISSING_COMMA)

words.txt

[duplication] ~45-~45: Möglicher Tippfehler: ein Wort wird wiederholt
Context: ...mfilter BLTZ Bootcamp Bootnode bootnode BOOTNODES Bootnodes bootnodes bottlenecked Brotli brotli Ca...

(GERMAN_WORD_REPEAT_RULE)

🔇 Additional comments (4)
pages/stack/network-topology.mdx (2)

1-13: LGTM! Clear and well-structured frontmatter and introduction.

The metadata and introduction effectively set up the document's purpose and scope.


14-20: LGTM! Clear overview with supporting diagram.

The section effectively introduces the network architecture with visual aid.

pages/stack/_meta.json (1)

7-7: LGTM! Metadata entry properly formatted.

The new entry maintains consistency with existing entries and correctly references the new documentation.

words.txt (1)

44-45: LGTM! Terminology additions are complete and well-organized.

The new entries properly capture network topology terminology with appropriate variations.

Also applies to: 344-345

🧰 Tools
🪛 LanguageTool

[duplication] ~45-~45: Möglicher Tippfehler: ein Wort wird wiederholt
Context: ...mfilter BLTZ Bootcamp Bootnode bootnode BOOTNODES Bootnodes bootnodes bottlenecked Brotli brotli Ca...

(GERMAN_WORD_REPEAT_RULE)

Copy link
Collaborator

@sbvegan sbvegan left a comment

Choose a reason for hiding this comment

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

@krofax can you please incorporate this issue as well: #918

I'd prefer if these details were added to the architecture page instead of spinning up a new page. I think its best if its under the chain operator section for now because its more practical guidance and not protocol specific information

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Outside diff range and nitpick comments (5)
words.txt (1)

44-45: Consider consolidating duplicate entries

The terms "Bootnode" and "bootnode" appear consecutively. Consider keeping only one form to maintain consistency throughout the documentation.

🧰 Tools
🪛 LanguageTool

[duplication] ~45-~45: Möglicher Tippfehler: ein Wort wird wiederholt
Context: ...mfilter BLTZ Bootcamp Bootnode bootnode BOOTNODES Bootnodes bootnodes bottlenecked Brotli brotli Ca...

(GERMAN_WORD_REPEAT_RULE)

pages/builders/chain-operators/architecture.mdx (4)

58-58: Add link to peer-management-service documentation

The reference to peer-management-service should include more context or documentation link for better understanding.

-Sequencer op-node should have p2p discovery disabled and only be statically peered with other internal nodes (or use [peer-management-service](https://github.com/ethereum-optimism/infra/tree/main/peer-mgmt-service) to define peering network)
+Sequencer op-node should have p2p discovery disabled and only be statically peered with other internal nodes (or use [peer-management-service](https://github.com/ethereum-optimism/infra/tree/main/peer-mgmt-service) to define peering network). For more information, see the [peer management documentation](/builders/chain-operators/tools/peer-management).

130-130: Improve sentence structure

The sentence about disk snapshots needs better structure.

-*   You can also use these nodes taking disk snapshots for disaster recovery.
+*   You can also use these nodes to create disk snapshots for disaster recovery purposes.
🧰 Tools
🪛 LanguageTool

[uncategorized] ~130-~130: This verb may not be in the correct form. Consider using a different form for this context.
Context: ..." ``` * You can also use these nodes taking disk snapshots for disaster recovery. ...

(AI_EN_LECTOR_REPLACEMENT_VERB_FORM)


122-122: Improve sentence clarity and fix typo

The sentence about archive nodes needs better structure and has a typo.

-We recommend setting up some archive nodes for internal RPC usage, primary used by the challenger, proposer and security monitoring tools like [monitorism](/builders/chain-operators/tools/chain-monitoring#monitorism).
+We recommend setting up archive nodes for internal RPC usage, primarily used by the challenger, proposer, and security monitoring tools like [monitorism](/builders/chain-operators/tools/chain-monitoring#monitorism).

179-183: Consider adding comments to YAML configuration

The YAML configuration would benefit from inline comments explaining each setting's purpose.

 ```yaml
-GETH_NETRESTRICT: "10.0.0.0/8" # Restrict P2P to internal IPs
-GETH_P2P_NO_DISCOVERY: "true"
-GETH_P2P_STATIC: "<static peer list>"
+# Restrict P2P communication to internal network
+GETH_NETRESTRICT: "10.0.0.0/8"
+# Disable automatic peer discovery for security
+GETH_P2P_NO_DISCOVERY: "true"
+# List of trusted peer nodes
+GETH_P2P_STATIC: "<static peer list>"

</blockquote></details>

</blockquote></details>

<details>
<summary>📜 Review details</summary>

**Configuration used: .coderabbit.yaml**
**Review profile: CHILL**

<details>
<summary>📥 Commits</summary>

Reviewing files that changed from the base of the PR and between 7c81f237c1dc36ffa032fa608ac21cbc5a1529f0 and 153223dd0c2364f953a2b2c8332482b04e7111a3.

</details>

<details>
<summary>📒 Files selected for processing (2)</summary>

* `pages/builders/chain-operators/architecture.mdx` (3 hunks)
* `words.txt` (3 hunks)

</details>

<details>
<summary>🧰 Additional context used</summary>

<details>
<summary>📓 Path-based instructions (1)</summary>

<details>
<summary>pages/builders/chain-operators/architecture.mdx (1)</summary>

Pattern `**/*.mdx`: "ALWAYS review Markdown content THOROUGHLY with the following criteria:
- Use proper nouns in place of personal pronouns like 'We' and 'Our' to maintain consistency in communal documentation.
- Avoid gender-specific language and use the imperative form.
- Monitor capitalization for emphasis. Avoid using all caps, italics, or bold for emphasis.
- Ensure proper nouns are capitalized in sentences.
- Apply the Oxford comma.
- Use proper title case for buttons, tab names, page names, and links. Sentence case should be used for body content and short phrases, even in links.
- Use correct spelling and grammar at all times (IMPORTANT).
- For H1, H2, and H3 headers:
  1. Use sentence case, capitalizing only the first word.
  2. Preserve the capitalization of proper nouns, technical terms, and acronyms as defined in the 'nouns.txt' file located in the root directory of the project.
  3. Do not automatically lowercase words that appear in the 'nouns.txt' file, regardless of their position in the header.
- Flag any headers that seem to inconsistently apply these rules for manual review.
- When reviewing capitalization, always refer to the 'nouns.txt' file for the correct capitalization of proper nouns and technical terms specific to the project.
"


</details>

</details><details>
<summary>🪛 LanguageTool</summary>

<details>
<summary>words.txt</summary>

[duplication] ~45-~45: Möglicher Tippfehler: ein Wort wird wiederholt
Context: ...mfilter BLTZ Bootcamp Bootnode bootnode BOOTNODES Bootnodes bootnodes bottlenecked Brotli brotli Ca...

(GERMAN_WORD_REPEAT_RULE)

</details>
<details>
<summary>pages/builders/chain-operators/architecture.mdx</summary>

[uncategorized] ~130-~130: This verb may not be in the correct form. Consider using a different form for this context.
Context: ..." ```  *   You can also use these nodes taking disk snapshots for disaster recovery.  ...

(AI_EN_LECTOR_REPLACEMENT_VERB_FORM)

---

[duplication] ~147-~147: Possible typo: you repeated a word
Context: ...th/core/alltools/).     *   If you want to to use geth for snapync bootnodes, you may...

(ENGLISH_WORD_REPEAT_RULE)

</details>

</details>

</details>

</details>

<!-- This is an auto-generated comment by CodeRabbit for review status -->

words.txt Show resolved Hide resolved
pages/builders/chain-operators/architecture.mdx Outdated Show resolved Hide resolved
@krofax
Copy link
Contributor Author

krofax commented Dec 11, 2024

@krofax can you please incorporate this issue as well: #918

I'd prefer if these details were added to the architecture page instead of spinning up a new page. I think its best if its under the chain operator section for now because its more practical guidance and not protocol specific information

@sbvegan I've updated the docs to reflect the changes.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (4)
pages/builders/chain-operators/architecture.mdx (4)

54-56: Improve clarity in the Sequencer description

The sentence structure could be clearer to better explain the Sequencer's role.

-The Sequencer node works with the batcher and proposer to create new blocks. So it should
-handle the state changing RPC request `eth_sendRawTransaction`. It can be peered with
-replica nodes to gossip new `unsafe` blocks to the rest of the network.
+The Sequencer node collaborates with the batcher and proposer to create new blocks. It handles
+state-changing RPC requests such as `eth_sendRawTransaction`. The Sequencer can be peered with
+replica nodes to propagate new `unsafe` blocks across the network.

58-59: Improve formatting of technical reference

The sentence contains a URL that should be properly formatted as a link.

-Sequencer op-node should have p2p discovery disabled and only be statically peered with other internal nodes (or use [peer-management-service](https://github.com/ethereum-optimism/infra/tree/main/peer-mgmt-service) to define peering network)
+The Sequencer op-node should have P2P discovery disabled and only be statically peered with other internal nodes. For defining the peering network, you can use the [peer-management-service](https://github.com/ethereum-optimism/infra/tree/main/peer-mgmt-service).

106-112: Enhance configuration documentation

The YAML configuration would benefit from additional comments explaining each setting's purpose.

 ```yaml
-GETH_ROLLUP_DISABLETXPOOLGOSSIP: "false"
-GETH_TXPOOL_JOURNALREMOTES: "false"
-GETH_TXPOOL_LIFETIME: "1h"
-GETH_TXPOOL_NOLOCALS: "true"
-GETH_NETRESTRICT: "10.0.0.0/8" # Restrict P2P to internal IPs
+# Enable transaction pool gossip for network-wide transaction propagation
+GETH_ROLLUP_DISABLETXPOOLGOSSIP: "false"
+# Disable journaling of remote transactions to reduce disk I/O
+GETH_TXPOOL_JOURNALREMOTES: "false"
+# Set transaction lifetime in the pool to 1 hour
+GETH_TXPOOL_LIFETIME: "1h"
+# Disable special handling of local transactions
+GETH_TXPOOL_NOLOCALS: "true"
+# Restrict P2P communication to internal network for security
+GETH_NETRESTRICT: "10.0.0.0/8"

171-174: Improve formatting of consensus layer peering section

The section could be more structured with bullet points for better readability.

-Ensure proper peer configurations for consensus layer clients. Use static peer lists for critical components and restrict P2P traffic to internal IP ranges.
+Ensure proper peer configurations for consensus layer clients:
+* Use static peer lists for critical components
+* Restrict P2P traffic to internal IP ranges
+* Monitor peer count and connection stability
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 153223d and 873dd1e.

📒 Files selected for processing (1)
  • pages/builders/chain-operators/architecture.mdx (3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
pages/builders/chain-operators/architecture.mdx (1)

Pattern **/*.mdx: "ALWAYS review Markdown content THOROUGHLY with the following criteria:

  • Use proper nouns in place of personal pronouns like 'We' and 'Our' to maintain consistency in communal documentation.
  • Avoid gender-specific language and use the imperative form.
  • Monitor capitalization for emphasis. Avoid using all caps, italics, or bold for emphasis.
  • Ensure proper nouns are capitalized in sentences.
  • Apply the Oxford comma.
  • Use proper title case for buttons, tab names, page names, and links. Sentence case should be used for body content and short phrases, even in links.
  • Use correct spelling and grammar at all times (IMPORTANT).
  • For H1, H2, and H3 headers:
    1. Use sentence case, capitalizing only the first word.
    2. Preserve the capitalization of proper nouns, technical terms, and acronyms as defined in the 'nouns.txt' file located in the root directory of the project.
    3. Do not automatically lowercase words that appear in the 'nouns.txt' file, regardless of their position in the header.
  • Flag any headers that seem to inconsistently apply these rules for manual review.
  • When reviewing capitalization, always refer to the 'nouns.txt' file for the correct capitalization of proper nouns and technical terms specific to the project.
    "
🪛 LanguageTool
pages/builders/chain-operators/architecture.mdx

[duplication] ~147-~147: Possible typo: you repeated a word
Context: ...th/core/alltools/). * If you want to to use geth for snapync bootnodes, you may...

(ENGLISH_WORD_REPEAT_RULE)

🔇 Additional comments (1)
pages/builders/chain-operators/architecture.mdx (1)

147-147: Fix typo in sentence

There's a duplicate word "to" in the sentence, and "snapync" is misspelled.

-*   If you want to to use geth for snapync bootnodes, you may want to just make the Full Snapsync Nodes serve as your bootnodes as well.
+*   If you want to use geth for snapsync bootnodes, you may want to just make the Full Snapsync Nodes serve as your bootnodes as well.
🧰 Tools
🪛 LanguageTool

[duplication] ~147-~147: Possible typo: you repeated a word
Context: ...th/core/alltools/). * If you want to to use geth for snapync bootnodes, you may...

(ENGLISH_WORD_REPEAT_RULE)

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (5)
pages/builders/chain-operators/architecture.mdx (5)

58-58: Maintain consistent capitalization of technical terms

Change "op-node" to "op-node" for consistency with other mentions in the documentation.


68-72: Add descriptions for configuration parameters

The YAML configuration lacks explanations for the parameters. Consider adding comments to explain:

  • Why P2P discovery is disabled
  • Why peer banning is disabled
  • The format expected for the static peer list
 # Recommended sequencer configuration
-OP_NODE_P2P_NO_DISCOVERY: "true"
-OP_NODE_P2P_PEER_BANNING: "false"
-OP_NODE_P2P_STATIC: "<static peer list>"
+# Disable P2P discovery for security
+OP_NODE_P2P_NO_DISCOVERY: "true"
+# Disable peer banning to maintain stable connections with trusted peers
+OP_NODE_P2P_PEER_BANNING: "false"
+# Comma-separated list of peer multiaddrs (e.g., /ip4/1.2.3.4/tcp/9003/p2p/[peer-id])
+OP_NODE_P2P_STATIC: "<static peer list>"

89-89: Clarify the archive node requirement

The sentence about proposer requirements could be clearer.

-  To run a rollup, you need a minimum of one archive node. This is required by the proposer as the data that it needs can be older than the data available to a full node.  Note that since the proposer doesn't care what archive node it points to, you can technically point it towards an archive node that isn't the sequencer.
+  To run a rollup, you need a minimum of one archive node. The proposer requires an archive node because it needs access to historical data that exceeds the retention period of a full node. The proposer can use any archive node, not necessarily the sequencer's archive node.

106-112: Document configuration parameters

Add descriptions for each configuration parameter to help users understand their purpose.

 ```yaml
+# Enable transaction pool gossip between peers
 GETH_ROLLUP_DISABLETXPOOLGOSSIP: "false"
+# Disable persistence of remote transactions
 GETH_TXPOOL_JOURNALREMOTES: "false"
+# Set transaction pool entry lifetime
 GETH_TXPOOL_LIFETIME: "1h"
+# Disable special treatment for local submitted transactions
 GETH_TXPOOL_NOLOCALS: "true"
+# Restrict P2P communication to internal network
 GETH_NETRESTRICT: "10.0.0.0/8" # Restrict P2P to internal IPs

---

`179-183`: **Add format example for static peer list**

Include an example of the expected format for static peers.

```diff
 ```yaml
 GETH_NETRESTRICT: "10.0.0.0/8" # Restrict P2P to internal IPs
 GETH_P2P_NO_DISCOVERY: "true"
-GETH_P2P_STATIC: "<static peer list>"
+# Format: enode://[node-id]@[ip]:[port]
+GETH_P2P_STATIC: "enode://[email protected]:30303,enode://[email protected]:30303"

</blockquote></details>

</blockquote></details>

<details>
<summary>📜 Review details</summary>

**Configuration used: .coderabbit.yaml**
**Review profile: CHILL**
**Plan: Pro**

<details>
<summary>📥 Commits</summary>

Reviewing files that changed from the base of the PR and between 873dd1e19ea5ad8b62ceaa01ea9ed0277a7165e6 and 25a479ee0b6ddaafc1e955ed293a1877711b8ebe.

</details>

<details>
<summary>📒 Files selected for processing (1)</summary>

* `pages/builders/chain-operators/architecture.mdx` (3 hunks)

</details>

<details>
<summary>🧰 Additional context used</summary>

<details>
<summary>📓 Path-based instructions (1)</summary>

<details>
<summary>pages/builders/chain-operators/architecture.mdx (1)</summary>

Pattern `**/*.mdx`: "ALWAYS review Markdown content THOROUGHLY with the following criteria:
- Use proper nouns in place of personal pronouns like 'We' and 'Our' to maintain consistency in communal documentation.
- Avoid gender-specific language and use the imperative form.
- Monitor capitalization for emphasis. Avoid using all caps, italics, or bold for emphasis.
- Ensure proper nouns are capitalized in sentences.
- Apply the Oxford comma.
- Use proper title case for buttons, tab names, page names, and links. Sentence case should be used for body content and short phrases, even in links.
- Use correct spelling and grammar at all times (IMPORTANT).
- For H1, H2, and H3 headers:
  1. Use sentence case, capitalizing only the first word.
  2. Preserve the capitalization of proper nouns, technical terms, and acronyms as defined in the 'nouns.txt' file located in the root directory of the project.
  3. Do not automatically lowercase words that appear in the 'nouns.txt' file, regardless of their position in the header.
- Flag any headers that seem to inconsistently apply these rules for manual review.
- When reviewing capitalization, always refer to the 'nouns.txt' file for the correct capitalization of proper nouns and technical terms specific to the project.
"


</details>

</details>

</details>

<details>
<summary>🔇 Additional comments (4)</summary>

<details>
<summary>pages/builders/chain-operators/architecture.mdx (4)</summary>

`74-78`: **LGTM!**

The section clearly explains the Sequencer's isolation requirements and security considerations.

---

`147-147`: **Fix typo in sentence**

There's a duplicate word "to" in the sentence.

```diff
-    *   If you want to to use geth for snapsync bootnodes, you may want to just make the Full Snapsync Nodes serve as your bootnodes as well.
+    *   If you want to use geth for snapsync bootnodes, you may want to just make the Full Snapsync Nodes serve as your bootnodes as well.

153-168: LGTM!

The section provides a clear overview of essential offchain components.


201-203: LGTM!

The next steps provide clear guidance for further reading.

@krofax
Copy link
Contributor Author

krofax commented Jan 10, 2025

@krofax can you please incorporate this issue as well: #918
I'd prefer if these details were added to the architecture page instead of spinning up a new page. I think its best if its under the chain operator section for now because its more practical guidance and not protocol specific information

@sbvegan I've updated the docs to reflect the changes.

@sbvegan Checking here for a review when you have the time.

@@ -49,6 +49,46 @@ amount of data required to reproduce L2 blocks.

<OpProposerDescriptionShort />

Copy link
Collaborator

Choose a reason for hiding this comment

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

Lets add a short description of op-challenger at the top here

@@ -61,8 +101,87 @@ to route RPC methods, retry failed requests, load balance, etc. Users sending
`eth_sendRawTransaction` requests can have their requests forwarded directly to the
Sequencer. All other RPC requests can be forwarded to replica nodes.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remove the example image and the sentence: "Users sending
eth_sendRawTransaction requests can have their requests forwarded directly to the
Sequencer. All other RPC requests can be forwarded to replica nodes."

Its out of date

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