-
Notifications
You must be signed in to change notification settings - Fork 158
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
Transaction Finality Explainer #987
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for docs-optimism ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
WalkthroughThe changes introduce a new documentation file, Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
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? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
Documentation and Community
|
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.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
pages/stack/protocol/rollup/finality.mdx (1)
1-87
: Excellent documentation on transaction finality.This document provides a comprehensive and well-structured explanation of transaction finality in Ethereum and OP Stack. The content is informative, clear, and adheres to the provided coding guidelines. The use of Callout components and bullet points enhances readability and helps to highlight important information.
As a final step, I recommend a thorough proofread of the entire document to catch any minor issues that might have been overlooked. Pay special attention to the grammar correction suggested earlier in the "Finality on Ethereum" section.
🧰 Tools
🪛 LanguageTool
[grammar] ~17-~17: With the plural noun ‘transactions’, the verb inflection ‘achieves’ is not correct.
Context: ...rk conditions, transactions on Ethereum achieves finality in about 15 minutes. * Conse...(NNS_IN_NNP_VBZ)
[uncategorized] ~46-~46: The preposition ‘of’ seems more likely in this position.
Context: ...hereum to establish a view of the state on an OP Stack chain: * Rollup nodes po...(AI_HYDRA_LEO_REPLACE_ON_OF)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
public/img/op-stack/protocol/tx-finality.png
is excluded by!**/*.png
📒 Files selected for processing (2)
- pages/stack/protocol/rollup/finality.mdx (1 hunks)
- words.txt (1 hunks)
✅ Files skipped from review due to trivial changes (1)
- words.txt
🧰 Additional context used
📓 Path-based instructions (1)
pages/stack/protocol/rollup/finality.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 headers, 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).
- Use sentence case for H1, H2, and H3 headers, capitalizing only the first word and any proper nouns.
- For all headers (H1, H2, H3), do not change the capitalization of proper nouns; keep them as they are.
"
🪛 LanguageTool
pages/stack/protocol/rollup/finality.mdx
[grammar] ~17-~17: With the plural noun ‘transactions’, the verb inflection ‘achieves’ is not correct.
Context: ...rk conditions, transactions on Ethereum achieves finality in about 15 minutes. * Conse...(NNS_IN_NNP_VBZ)
[uncategorized] ~46-~46: The preposition ‘of’ seems more likely in this position.
Context: ...hereum to establish a view of the state on an OP Stack chain: * Rollup nodes po...(AI_HYDRA_LEO_REPLACE_ON_OF)
🔇 Additional comments (5)
pages/stack/protocol/rollup/finality.mdx (5)
1-11
: LGTM: Metadata and introduction are well-structured and informative.The metadata provides clear information about the document's content, and the introduction offers a concise definition of transaction finality. The language used is appropriate and adheres to the provided guidelines.
27-42
: LGTM: Clear explanation of OP Stack finality.This section effectively explains how OP Stack derives security from Ethereum and clearly defines the three stages of finality. The use of the Callout component adds valuable information about the OP Stack's security measures.
44-52
: LGTM: Clear explanation of the settlement layer.This section effectively explains how Ethereum establishes a view of the state on an OP Stack chain. The use of bullet points helps in presenting the information clearly.
Regarding the static analysis suggestion to replace "on" with "of" in the phrase "state on an OP Stack chain", the current preposition "on" is correct and more appropriate in this context. It accurately conveys the idea of the state existing on the OP Stack chain, rather than being a property of it.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~46-~46: The preposition ‘of’ seems more likely in this position.
Context: ...hereum to establish a view of the state on an OP Stack chain: * Rollup nodes po...(AI_HYDRA_LEO_REPLACE_ON_OF)
54-62
: LGTM: Clear explanation of sequencer reliability.This section effectively addresses concerns about centralized sequencers and explains how consensus rules and network nodes ensure reliability. The information is presented clearly and concisely using bullet points.
64-87
: LGTM: Comprehensive explanation of the challenge period.This section provides a clear and detailed explanation of the 7-day withdrawal challenge period in OP Stack rollups. It effectively clarifies the distinction between transaction finality and the challenge period, which is crucial for understanding the system. The use of bullet points and the Callout component enhances the presentation of this complex information.
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
pages/stack/protocol/rollup/_meta.json (1)
6-6
: Remove trailing space in "Transaction Finality "There's an extra space at the end of the value for the "transaction-finality" key. While this won't affect functionality, it's best to remove it for consistency and cleanliness.
Here's the suggested change:
- "transaction-finality": "Transaction Finality ", + "transaction-finality": "Transaction Finality",
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
- pages/stack/protocol/rollup/_meta.json (1 hunks)
- pages/stack/protocol/rollup/finality.mdx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- pages/stack/protocol/rollup/finality.mdx
🧰 Additional context used
🔇 Additional comments (1)
pages/stack/protocol/rollup/_meta.json (1)
6-6
: LGTM: New entry for transaction finality added.The addition of the "transaction-finality" entry aligns well with the PR objectives to enhance documentation on transaction finality in the OP Stack. Its placement in the file is logical within the context of the rollup process flow.
I have a doubt regarding the explanation posted above: As far as I understand optimistic rollups:
Quoting docs "The transaction needs to be written to L1 (Ethereum). This is typically performed by op-batcher, but any user can send an L1 transaction to submit an L2 transaction, in which case op-batcher is bypassed." Again quoting from the explanation: A successful challenge can also be for a batch that has been posted not necessarily withdrawals. (A batch is posted every 10 minutes) and would definitely cause a reorg- as far as I understand. @zainbacchus @sbvegan can you confirm If I am correct? |
Description
Created a section on the docs to cover the nuances of transaction finality, sequencer policy and challenge periods to debunk the belief that OP Stack chains take 7 days for transaction finalization.
Tests
Additional context
Metadata
Include a link to any github issues that this may close in the following form: