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

Added force withdrawal guide #872

Merged
merged 15 commits into from
Sep 26, 2024
Merged

Added force withdrawal guide #872

merged 15 commits into from
Sep 26, 2024

Conversation

krofax
Copy link
Contributor

@krofax krofax commented Aug 30, 2024

Description

A short guide for forced withdrawals and deposits flow during sequencer downtime.

Tests

Please describe any tests you've added. If you've added no tests, or left important behavior untested, please explain why not.

Additional context

Add any other context about the problem you're solving.

Metadata

@krofax krofax requested a review from a team as a code owner August 30, 2024 10:49
Copy link

netlify bot commented Aug 30, 2024

Deploy Preview for docs-optimism ready!

Name Link
🔨 Latest commit 45affb8
🔍 Latest deploy log https://app.netlify.com/sites/docs-optimism/deploys/66e8257eb812dc0008e037ed
😎 Deploy Preview https://deploy-preview-872--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 Aug 30, 2024

Walkthrough

The changes introduce new metadata entries and comprehensive guides for forced withdrawals, deposits, and transactions in the OP stack rollup system. These updates clarify the mechanics and implications of forced transactions during sequencer downtime, detailing how different scenarios affect processing and ensuring users understand the operational framework.

Changes

File(s) Change Summary
pages/stack/protocol/rollup/_meta.json Added new entries for "forced-withdrawal": "Forced Withdrawal", "forced-deposit": "Forced Deposit", and "forced-transaction": "Forced Transaction" to the JSON structure.
pages/stack/protocol/rollup/forced-withdrawal.mdx Introduced a guide on forced withdrawal and deposit processes during sequencer downtime.
pages/stack/protocol/rollup/forced-deposit.mdx Added a guide on the forced deposit flow during sequencer downtime.
pages/stack/protocol/rollup/forced-transaction.mdx Added a guide on the forced transaction flow during sequencer downtime.

Assessment against linked issues

Objective Addressed Explanation
Forced Withdrawal for OP stack rollup (#840)

Possibly related PRs

  • docs: add information about the derivation pipeline #805: This PR adds new key-value pairs to the _meta.json file, which is directly related to the changes made in the main PR that also modifies the _meta.json file by adding new entries for forced withdrawal, deposit, and transaction.
  • Add info on expected internal reverts for withdrawal transactions #853: This PR enhances the documentation regarding the withdrawal process, which is relevant to the main PR's focus on forced withdrawal and deposit processes, providing additional context on transaction handling during withdrawals.
  • Details on the sequencer fee vault #858: This PR introduces details about the Sequencer Fee Vault, which is relevant to the overall transaction process, including withdrawals and deposits, as it discusses the management of transaction fees associated with these operations.

Suggested labels

documentation


Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 87b56ae and 45affb8.

Files selected for processing (2)
  • pages/stack/protocol/rollup/_meta.json (1 hunks)
  • pages/stack/protocol/rollup/forced-transaction.mdx (1 hunks)
Additional context used
Path-based instructions (1)
pages/stack/protocol/rollup/forced-transaction.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. Use bold for prominence instead of all caps or italics.
  • 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).
    "
LanguageTool
pages/stack/protocol/rollup/forced-transaction.mdx

[style] ~13-~13: As a shorter alternative for ‘able to’, consider using “can”.
Context: ...tions during sequencer downtime. Users are able to force-include deposit transactions, whi...

(BE_ABLE_TO)

Additional comments not posted (6)
pages/stack/protocol/rollup/_meta.json (1)

6-6: LGTM!

The addition of the "forced-transaction" key is consistent with the existing metadata structure and naming conventions. It aligns with the PR objective of providing a guide for forced transactions.

pages/stack/protocol/rollup/forced-transaction.mdx (5)

9-15: LGTM!

The introduction clearly states the purpose of the guide and emphasizes the importance of understanding the forced transaction flow during sequencer downtime. The language is clear, concise, and follows the documentation guidelines.

Tools
LanguageTool

[style] ~13-~13: As a shorter alternative for ‘able to’, consider using “can”.
Context: ...tions during sequencer downtime. Users are able to force-include deposit transactions, whi...

(BE_ABLE_TO)


16-21: LGTM!

The "Key Concepts" section clearly defines important terms used throughout the document, providing necessary context for understanding the forced transaction flow. The formatting is consistent and easy to follow.


29-50: LGTM!

The "Sequencer Downtime Scenarios" section clearly describes different scenarios of sequencer downtime and their impact on deposit transactions and the L2 chain state. The scenarios are well-organized, and the formatting is consistent, making it easy to understand the implications of each case.


53-58: LGTM!

The "Important Considerations" section highlights key points regarding forced transactions, emphasizing their role in mitigating risks during sequencer downtime and the speculative nature of actions within the 12-hour window. The language is clear, concise, and follows the documentation guidelines.


59-73: LGTM!

The "Example Scenario" section effectively illustrates the timing and effects of deposit transactions relative to the sequencing window, reinforcing the understanding of the forced transaction flow and the design's intent. The "Conclusion" section summarizes the key points and highlights the robustness of the forced transaction mechanism. The language is clear, concise, and follows the documentation guidelines.


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 generate interesting stats about this repository and render them as a table.
    -- @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 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.

@krofax krofax requested a review from a team August 30, 2024 10:52
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

pages/stack/protocol/rollup/forced-withdrawal.mdx Outdated Show resolved Hide resolved
pages/stack/protocol/rollup/forced-withdrawal.mdx Outdated Show resolved Hide resolved
krofax and others added 4 commits August 30, 2024 11:58
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: 1

pages/stack/protocol/rollup/forced-withdrawal.mdx Outdated Show resolved Hide resolved
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: 6

pages/stack/protocol/rollup/forced-deposit.mdx Outdated Show resolved Hide resolved
pages/stack/protocol/rollup/forced-deposit.mdx Outdated Show resolved Hide resolved
pages/stack/protocol/rollup/forced-deposit.mdx Outdated Show resolved Hide resolved
pages/stack/protocol/rollup/forced-deposit.mdx Outdated Show resolved Hide resolved
pages/stack/protocol/rollup/forced-deposit.mdx Outdated Show resolved Hide resolved
pages/stack/protocol/rollup/forced-deposit.mdx Outdated Show resolved Hide resolved
krofax and others added 2 commits September 3, 2024 11:36
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: Matt Solomon <matt@mattsolomon.dev>
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, codebase verification and nitpick comments (1)
pages/stack/protocol/rollup/forced-deposit.mdx (1)

13-14: Clarify the introduction of forced deposits.

Consider rephrasing the introduction to clarify the conditions under which users can initiate forced deposits:

-Users are able to force-include deposit transactions, which can initiate withdrawals, at any time.
+Users can initiate forced deposit transactions, which may also trigger withdrawals, at any time during sequencer downtime.

This rephrasing makes it more explicit that users can perform this action during sequencer downtime.

Tools
LanguageTool

[style] ~13-~13: As a shorter alternative for ‘able to’, consider using “can”.
Context: ...tions during sequencer downtime. Users are able to force-include deposit transactions, whi...

(BE_ABLE_TO)

pages/stack/protocol/rollup/forced-deposit.mdx Outdated Show resolved Hide resolved
pages/stack/protocol/rollup/forced-deposit.mdx Outdated Show resolved Hide resolved
pages/stack/protocol/rollup/forced-deposit.mdx Outdated Show resolved Hide resolved
pages/stack/protocol/rollup/forced-deposit.mdx Outdated Show resolved Hide resolved
krofax and others added 4 commits September 16, 2024 12:55
Co-authored-by: Matt Solomon <matt@mattsolomon.dev>
Co-authored-by: protolambda <proto@protolambda.com>
Co-authored-by: protolambda <proto@protolambda.com>
Co-authored-by: protolambda <proto@protolambda.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 (6)
pages/stack/protocol/rollup/forced-deposit.mdx (6)

2-4: Enhance the document description for better clarity and SEO.

Consider refining the description to provide more context and keywords:

-description: Learn the forced deposit flow during sequencer downtime.
+description: Comprehensive Guide to Managing Forced Deposit Transactions During Sequencer Downtime in Optimism Stack Rollups

11-14: Clarify the introduction and conditions for initiating forced deposits.

Consider rephrasing the introduction for better clarity:

-Users are able to force-include deposit transactions, which can initiate withdrawals, at any time.
+Users can initiate forced deposit transactions, which may also trigger withdrawals, at any time during sequencer downtime.

This change emphasizes the conditions (sequencer downtime) under which users can initiate forced deposits and avoids the phrase "are allowed to".

Tools
LanguageTool

[style] ~13-~13: As a shorter alternative for ‘able to’, consider using “can”.
Context: ...tions during sequencer downtime. Users are able to force-include deposit transactions, whi...

(BE_ABLE_TO)


18-19: Clarify the definitions of "Sequencing Window" and "Max Time Drift".

Consider updating the definitions as follows:

-*   **Sequencing Window**: A 12-hour rolling window to include L2 transactions, including native L2 transactions and deposit transactions.
+*   **Sequencing Window**: A 12-hour rolling window to include L2 transactions, encompassing both native L2 transactions and deposit transactions. This window ensures a chain derivation in case of persistent batch-submitter inactivity.

-*   **Max Time Drift**: 15 minutes, the maximum delay for including a deposit transaction, relative to the L2 chain.
+*   **Max Time Drift**: 30 minutes, the maximum delay for including a deposit transaction, relative to the timestamp represented by the L2 block.

These changes clarify that the sequencing window includes both types of transactions and ensures chain derivation during inactivity. The max time drift is also updated to 30 minutes and its relativity to the L2 block timestamp is specified.


48-49: Clarify the resumption of regular L2 transactions after prolonged downtime.

Consider updating the explanation as follows:

-    *   Regular L2 transactions resume afterward.
+    *   No regular L2 transactions are included until the L2 chain is within 12 hours of the chain.

This change clarifies that regular L2 transactions do not immediately resume after the sequencer returns, but only when the L2 chain is within the 12-hour window.


53-53: Clarify the nature and purpose of forced transactions.

Consider updating the explanation as follows:

-*   Forced transactions, through deposits (no need for deposited value), ensure timely execution of actions, mitigating risks like DEX price divergence during sequencer downtime.
+*   Forced transactions, which are deposits with no minimum value requirement, ensure timely execution of actions, mitigating risks such as DEX price divergence during sequencer downtime.

This change clarifies that forced transactions are deposits without a minimum value requirement and highlights their role in mitigating risks like DEX price divergence during downtime.


71-71: Refine the conclusion to emphasize the robustness and guarantees of the forced transaction mechanism.

Consider updating the conclusion as follows:

-The forced transaction mechanism on OP Stack chains provides a robust way to handle sequencer downtime, ensuring that critical transactions are included in a timely manner. While the 12-hour sequencer window introduces a degree of uncertainty during downtime, the system is designed to guarantee eventual consistency and transaction inclusion.
+The forced transaction mechanism on Optimism Stack chains provides a robust way to handle sequencer downtime, ensuring that critical transactions of the deposit type, with any value, are included in a timely manner. While the 12-hour sequencing window introduces a degree of uncertainty during downtime, the system is designed to guarantee eventual consistency and transaction inclusion.

This change emphasizes the robustness of the mechanism, clarifies that forced transactions are deposits with any value, and highlights the eventual consistency and inclusion guarantees.

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 710743b and 87b56ae.

Files selected for processing (1)
  • pages/stack/protocol/rollup/forced-deposit.mdx (1 hunks)
Additional context used
Path-based instructions (1)
pages/stack/protocol/rollup/forced-deposit.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. Use bold for prominence instead of all caps or italics.
  • 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).
    "
LanguageTool
pages/stack/protocol/rollup/forced-deposit.mdx

[style] ~13-~13: As a shorter alternative for ‘able to’, consider using “can”.
Context: ...tions during sequencer downtime. Users are able to force-include deposit transactions, whi...

(BE_ABLE_TO)

Additional comments not posted (1)
pages/stack/protocol/rollup/forced-deposit.mdx (1)

1-71: LGTM!

The forced deposit guide is well-structured, informative, and incorporates the necessary clarifications and improvements suggested in previous reviews. The document effectively explains the key concepts, scenarios, and considerations related to forced transactions during sequencer downtime.

Great work on this comprehensive guide!

Tools
LanguageTool

[style] ~13-~13: As a shorter alternative for ‘able to’, consider using “can”.
Context: ...tions during sequencer downtime. Users are able to force-include deposit transactions, whi...

(BE_ABLE_TO)

@krofax
Copy link
Contributor Author

krofax commented Sep 18, 2024

@protolambda mind doing a quick review one last time?

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.

nice work!

@sbvegan sbvegan merged commit 1fc176f into main Sep 26, 2024
7 of 8 checks passed
@sbvegan sbvegan deleted the forced-withdrawal branch September 26, 2024 21:54
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.

Forced Withdrawal for OP stack rollup
4 participants