-
Notifications
You must be signed in to change notification settings - Fork 243
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
challenger docs #759
challenger docs #759
Conversation
cpengilly
commented
Jun 19, 2024
- explainer
- how to guide
- explainer - how to guide
WalkthroughThe latest changes include the addition of a comprehensive guide for configuring and utilizing Changes
Recent review detailsConfiguration used: .coderabbit.yaml Files selected for processing (2)
Additional context usedPath-based instructions (2)
LanguageTool
Additional comments not posted (3)
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
✅ Deploy Preview for docs-optimism ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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: 12
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.
I feel like a lot of this still sounds like my very rough notes. Not sure what style we usually use for our docs but worth reviewing the language to fit with that - there's a lot of "we" and "you" which feels overly informal through here for example.
Co-authored-by: Adrian Sutton <adrian@symphonious.net>
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: 5
Outside diff range and nitpick comments (5)
pages/builders/chain-operators/deploy/smart-contracts.mdx (1)
Line range hint
60-60
: Suggested Grammar Improvement: Add a comma for clarity.The sentence would benefit from a comma after "contracts" for better readability.
- ... for each of the contracts as well as a shared SuperchainConfig contract. + ... for each of the contracts, as well as a shared SuperchainConfig contract.pages/stack/protocol/fault-proofs/challenger.mdx (4)
10-12
: Clarify the link to OP-Challenger Explainer.The link to the OP-Challenger Explainer might benefit from more context about what the reader can expect to find there, especially since it's referenced as a starting point for those new to the topic.
34-35
: Clarify the role of 'cannon' and 'op-program'.The role and function of the
cannon
andop-program
executables could be expanded upon to help users understand why they are significant within theop-challenger
setup.
65-67
: Clarify the implications of a 'poisoned prestate'.The concept of a "poisoned prestate" is introduced here but might be too jargon-heavy without sufficient explanation. Consider adding a definition or a more detailed description to aid understanding.
Tools
LanguageTool
[grammar] ~65-~65: The progressive verb “agree” is used incorrectly. Consider changing it to the simple tense. (PROGRESSIVE_VERB_AGREE)
Context: ...t to have a "poisoned prestate" where it's implicitly agreeing to something false. Then, once the hon...
94-96
: Clarify the implications of lacking a blob archiver.The scenario described here is critical and could benefit from a more detailed explanation or example to illustrate the consequences of not having a blob archiver.
Tools
LanguageTool
[style] ~96-~96: Consider using a different verb for a more formal wording. (FIX_RESOLVE)
Context: ...ve 3.5 days on your side of the game to fix the problem by switching to a beacon no...
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/fault-proofs/challenger.mdx (1)
10-12
: Clarify the Document's Scope in the IntroductionThe introduction could be enhanced by specifying what aspects of the OP-Challenger are covered in this document versus what is covered in the linked explainer. This helps set the right expectations for the reader.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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: 7
Outside diff range and nitpick comments (1)
pages/stack/protocol/fault-proofs/challenger.mdx (1)
10-10
: Ensure Proper Capitalization in TitlesThe title "OP-Challenger Explainer" should follow the proper title case as per the additional instructions. Each major word in the title should be capitalized.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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.
lgtm: I just left a non-blocking nit and comment about the download link
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: 8
Outside diff range and nitpick comments (6)
pages/stack/protocol/fault-proofs/cannon.mdx (6)
Line range hint
71-71
: Simplify language for clarity.The word "important" might be overused. Consider simplifying the sentence to make it more direct.
- how memory is stored isn't important, as it can hold the entire monolithic memory within the Go runtime. + how memory is stored is not crucial, as it can hold the entire monolithic memory within the Go runtime.
Line range hint
73-73
: Simplify the phrase to enhance readability.The phrase "in order to" can often be simplified to "to" without losing meaning.
- such that only small portions are needed in order to run a MIPS instruction onchain. + such that only small portions are needed to run a MIPS instruction onchain.
Line range hint
85-85
: Correct the punctuation.There seems to be an unnecessary space before the comma, which might be a typographical error.
- [generalized Merkle tree index specification](https://github.com/ethereum/consensus-specs/blob/dev/ssz/merkle-proofs.md#generalized-merkle-tree-index). `page.go`, as the name implies, defines memory pages... + [generalized Merkle tree index specification](https://github.com/ethereum/consensus-specs/blob/dev/ssz/merkle-proofs.md#generalized-merkle-tree-index). `page.go`, as the name implies, defines memory pages...
Line range hint
98-98
: Grammar correction needed.The verb form should match the plural subject in the sentence.
- as long as the memory region the Merkle root covers has not been written to. + as long as the memory region the Merkle root covers have not been written to.
Line range hint
122-122
: Consider rephrasing to avoid redundancy.The phrase "exactly the same" is somewhat redundant. Consider using "identical" instead to reduce wordiness.
- should be exactly the same as the post state generated by the `mipsevm`. + should be identical to the post state generated by the `mipsevm`.
Line range hint
192-192
: Consider rephrasing to avoid redundancy.The phrase "exactly the same" is somewhat redundant. Consider using "identical" instead to reduce wordiness.
- In fact, they must produce **exactly the same results** given the same instruction, memory, and register state. + In fact, they must produce **identical results** given the same instruction, memory, and register state.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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: 7