-
Notifications
You must be signed in to change notification settings - Fork 262
Smart contract section #1563
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?
Smart contract section #1563
Conversation
✅ Deploy Preview for docs-optimism ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
📝 WalkthroughWalkthroughThe pull request updates multiple documentation files to revise hyperlinks and references related to smart contracts. In several markdown files, links that previously pointed to Possibly related PRs
Suggested labels
Suggested reviewers
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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: 4
🧹 Nitpick comments (8)
pages/stack/smart-contracts/smart-contract-upgrades.mdx (2)
42-44
: Fix compound adjective in directory requirements.The phrase "Top level directory" should be hyphenated as it's a compound adjective modifying "names".
-Top level directory names must follow the [EIP-3770](https://eips.ethereum.org/EIPS/eip-3770) short name for the network (e.g., "mainnet", "sepolia"). +Top-level directory names must follow the [EIP-3770](https://eips.ethereum.org/EIPS/eip-3770) short name for the network (e.g., "mainnet", "sepolia").🧰 Tools
🪛 LanguageTool
[uncategorized] ~42-~42: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ccording to these strict requirements: Top level directory names must follow the [EIP-37...(EN_COMPOUND_ADJECTIVE_INTERNAL)
47-54
: Fix loose punctuation in list.There's a loose punctuation mark in the list of required files. The asterisks should be directly followed by the content without additional spaces. Also, ensure list formatting is consistent.
-* `README.md`: Describing precisely what the task will execute -* `Validation.md`: Detailing the expected state changes with justifications -* A foundry script for post-upgrade assertions: - * Use `SignFromJson.s.sol` for a Safe owned by EOA signers - * Use `NestedSignFromJson.s.sol` for a Safe owned by other Safes -* `input.json`: Defining the exact transaction for execution -* `.env`: Containing all environment variables specific to this task +* `README.md`: Describing precisely what the task will execute +* `Validation.md`: Detailing the expected state changes with justifications +* A foundry script for post-upgrade assertions: + * Use `SignFromJson.s.sol` for a Safe owned by EOA signers + * Use `NestedSignFromJson.s.sol` for a Safe owned by other Safes +* `input.json`: Defining the exact transaction for execution +* `.env`: Containing all environment variables specific to this task🧰 Tools
🪛 LanguageTool
[uncategorized] ~47-~47: Loose punctuation mark.
Context: ...directory must contain: *README.md
: Describing precisely what the task will...(UNLIKELY_OPENING_PUNCTUATION)
pages/stack/smart-contracts/upgrade-op-contracts-1-6-1-8.mdx (1)
23-23
: Import Statement:
The file currently imports{ Callout }
fromnextra/components
. This is acceptable if the callout component matches the design intent. However, if a card-based layout is desired later on (as seen in related files), consider updating the import to{ Card, Cards }
.pages/stack/smart-contracts/smart-contracts.mdx (5)
131-138
: Text Refinement in Safe Extensions Section:
In the segment describing the safe extensions upgrade:
- Change “1 week exit window” to “1‑week exit window” so that the compound adjective is correctly hyphenated.
- Insert a comma after “Additionally” to improve readability.
- Replace “able to” with “can” for conciseness.
These suggestions will help the text flow more naturally.🧰 Tools
🪛 LanguageTool
[uncategorized] ~136-~136: When a number forms part of an adjectival compound, use a hyphen.
Context: ...Superchain closer to satisfying the 1 week exit window requirement for Stage 1. ...(MISSING_HYPHEN)
[uncategorized] ~137-~137: A comma may be missing after the conjunctive/linking adverb ‘Additionally’.
Context: ...window requirement for Stage 1. * Additionally the Foundation is appointed to the new ...(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
[style] ~138-~138: As a shorter alternative for ‘able to’, consider using “can”.
Context: ...e new DeputyGuardian role which is able to act as Guardian through the Guardian Sa...(BE_ABLE_TO)
576-578
: GovernanceToken Description Improvement:
The current description appears fragmented:“The OP token used in governance and supporting voting and delegation. Implements EIP 2612 allowing signed approvals.”
Consider revising to a complete sentence such as:
“The OP token is used in governance—supporting voting and delegation—and implements EIP 2612 to allow signed approvals.”
This change will enhance clarity and grammatical correctness.🧰 Tools
🪛 LanguageTool
[uncategorized] ~576-~576: This verb may not be in the correct form. Consider using a different form for this context.
Context: ...en The OP token used in governance and supporting voting and delegation. Implements EIP 2...(AI_EN_LECTOR_REPLACEMENT_VERB_FORM)
640-641
: ProxyAdmin Description Conciseness:
The sentence “TheProxyAdmin
is the owner of all of the proxy contracts set at the predeploys.” can be made more concise. Consider revising it to:“The
ProxyAdmin
is the owner of all proxy contracts set at the predeploys.”
This removes the redundant “of all of” phrase.🧰 Tools
🪛 LanguageTool
[style] ~641-~641: Consider removing “of” to be more concise
Context: ...Admin TheProxyAdmin
is the owner of all of the proxy contracts set at the predeploys. ...(ALL_OF_THE)
652-664
: Fee Vault Wording:
Review the usage of “a certain amount of fees” in this section. If fees are considered a measurable (mass) quantity (e.g. ETH amounts), “amount” is appropriate. However, if referencing individual fee items, “number” might be more precise. Please double-check the intended meaning and adjust if necessary.🧰 Tools
🪛 LanguageTool
[uncategorized] ~653-~653: ‘Amount of’ should usually only be used with uncountable or mass nouns. Consider using “number” if this is not the case.
Context: ...nce the contract has received a certain amount of fees, the ETH can be withdrawn to an...(AMOUNTOF_TO_NUMBEROF)
[uncategorized] ~664-~664: ‘Amount of’ should usually only be used with uncountable or mass nouns. Consider using “number” if this is not the case.
Context: ...nce the contract has received a certain amount of fees, the ETH can be withdrawn to an...(AMOUNTOF_TO_NUMBEROF)
722-723
: Compound Adjective Consistency:
Ensure that compound adjectives are hyphenated when they modify a noun. For example, verify that terms related to the Ethereum Attestation Service (or similar compound descriptors) follow the project’s capitalization and hyphenation standards.🧰 Tools
🪛 LanguageTool
[uncategorized] ~723-~723: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...estation-service/eas-ponder-graph) * [Open Source Indexer](https://github.com/ethereum-at...(EN_COMPOUND_ADJECTIVE_INTERNAL)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
pages/operators/chain-operators/deploy/overview.mdx
(1 hunks)pages/operators/chain-operators/deploy/smart-contracts.mdx
(1 hunks)pages/operators/chain-operators/self-hosted.mdx
(1 hunks)pages/stack/_meta.json
(1 hunks)pages/stack/opcm.mdx
(1 hunks)pages/stack/smart-contracts.mdx
(1 hunks)pages/stack/smart-contracts/_meta.json
(1 hunks)pages/stack/smart-contracts/smart-contract-upgrades.mdx
(1 hunks)pages/stack/smart-contracts/smart-contracts.mdx
(1 hunks)pages/stack/smart-contracts/upgrade-op-contracts-1-6-1-8.mdx
(1 hunks)pages/superchain/addresses.mdx
(1 hunks)public/_redirects
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.mdx`: "ALWAYS review Markdown content THOROUGHLY with the following criteria: - First, check the frontmatter section at the top of the file: 1. For regular pages, ensure AL...
**/*.mdx
: "ALWAYS review Markdown content THOROUGHLY with the following criteria:
- First, check the frontmatter section at the top of the file:
- For regular pages, ensure ALL these fields are present and not empty:
--- title: [non-empty] lang: [non-empty] description: [non-empty] topic: [non-empty] personas: [non-empty array] categories: [non-empty array] content_type: [valid type] ---
- For landing pages (index.mdx or files with ), only these fields are required:
--- title: [non-empty] lang: [non-empty] description: [non-empty] topic: [non-empty] ---
- If any required fields are missing or empty, comment:
'This file appears to be missing required metadata. Please check keywords.config.yaml for valid options and add the required fields manually. You can validate your changes by running:pnpm validate-metadata ```'
- 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:
- Use sentence case, capitalizing only the first word.
- 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.
- 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.
"
pages/operators/chain-operators/deploy/smart-contracts.mdx
pages/superchain/addresses.mdx
pages/stack/opcm.mdx
pages/operators/chain-operators/deploy/overview.mdx
pages/operators/chain-operators/self-hosted.mdx
pages/stack/smart-contracts/smart-contract-upgrades.mdx
pages/stack/smart-contracts/upgrade-op-contracts-1-6-1-8.mdx
pages/stack/smart-contracts/smart-contracts.mdx
pages/stack/smart-contracts.mdx
🪛 LanguageTool
pages/stack/smart-contracts/smart-contract-upgrades.mdx
[uncategorized] ~42-~42: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ccording to these strict requirements: Top level directory names must follow the [EIP-37...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[uncategorized] ~47-~47: Loose punctuation mark.
Context: ...directory must contain: * README.md
: Describing precisely what the task will...
(UNLIKELY_OPENING_PUNCTUATION)
pages/stack/smart-contracts/upgrade-op-contracts-1-6-1-8.mdx
[uncategorized] ~48-~48: A punctuation mark might be missing here.
Context: ...and runner for executing upgrade scripts * Foundry for con...
(AI_EN_LECTOR_MISSING_PUNCTUATION)
pages/stack/smart-contracts/smart-contracts.mdx
[style] ~95-~95: Consider a shorter alternative to avoid wordiness.
Context: ...anchor state for the Fault Proof System in order to prevent referencing invalid anchor stat...
(IN_ORDER_TO_PREMIUM)
[uncategorized] ~136-~136: When a number forms part of an adjectival compound, use a hyphen.
Context: ...Superchain closer to satisfying the 1 week exit window requirement for Stage 1. ...
(MISSING_HYPHEN)
[uncategorized] ~137-~137: A comma may be missing after the conjunctive/linking adverb ‘Additionally’.
Context: ...window requirement for Stage 1. * Additionally the Foundation is appointed to the new ...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
[style] ~138-~138: As a shorter alternative for ‘able to’, consider using “can”.
Context: ...e new DeputyGuardian role which is able to act as Guardian through the Guardian Sa...
(BE_ABLE_TO)
[style] ~369-~369: The phrase ‘has the ability to’ might be wordy. Consider using “can”.
Context: ...uration of global superchain values. It has the ability to pause and unpause all withdrawals in th...
(HAS_THE_ABILITY_TO)
[uncategorized] ~576-~576: This verb may not be in the correct form. Consider using a different form for this context.
Context: ...en The OP token used in governance and supporting voting and delegation. Implements EIP 2...
(AI_EN_LECTOR_REPLACEMENT_VERB_FORM)
[style] ~641-~641: Consider removing “of” to be more concise
Context: ...Admin The ProxyAdmin
is the owner of all of the proxy contracts set at the predeploys. ...
(ALL_OF_THE)
[uncategorized] ~653-~653: ‘Amount of’ should usually only be used with uncountable or mass nouns. Consider using “number” if this is not the case.
Context: ...nce the contract has received a certain amount of fees, the ETH can be withdrawn to an...
(AMOUNTOF_TO_NUMBEROF)
[uncategorized] ~664-~664: ‘Amount of’ should usually only be used with uncountable or mass nouns. Consider using “number” if this is not the case.
Context: ...nce the contract has received a certain amount of fees, the ETH can be withdrawn to an...
(AMOUNTOF_TO_NUMBEROF)
[uncategorized] ~723-~723: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...estation-service/eas-ponder-graph) * [Open Source Indexer](https://github.com/ethereum-at...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Redirect rules - docs-optimism
- GitHub Check: Header rules - docs-optimism
- GitHub Check: Pages changed - docs-optimism
🔇 Additional comments (18)
pages/stack/_meta.json (1)
17-17
: Looks good - navigation item is maintained for smart contracts.The navigation entry for "smart contracts" is properly included in the stack navigation structure. This change aligns with the reorganization of the smart contracts documentation.
pages/stack/smart-contracts/smart-contract-upgrades.mdx (4)
1-21
: Frontmatter looks complete with all required fields.The frontmatter section correctly includes all required fields for a regular page:
- title
- description
- lang
- content_type
- topic
- personas (array)
- categories (array)
All fields contain appropriate non-empty values.
25-28
: Appropriate heading structure and introduction.The document starts with a clear H1 heading followed by a concise introduction that explains the purpose of the guide. The content uses sentence case for headings as required by the guidelines.
96-96
: Clear and important Docker memory requirement.The warning about Docker memory requirements is well-placed and crucial for preventing compilation failures. This practical information will help users avoid common setup issues.
106-106
: Critical configuration warning properly emphasized.The document appropriately highlights the critical nature of the
faultGameAbsolutePrestate
value configuration with clear emphasis, which is essential for the proper functioning of fault proof systems.pages/superchain/addresses.mdx (1)
25-25
: Updated link path to match new documentation structure.The hyperlink has been correctly updated to point to
/stack/smart-contracts/smart-contracts
instead of/stack/smart-contracts
, reflecting the reorganized documentation structure.public/_redirects (1)
171-171
: Added appropriate redirect for restructured documentation.A redirect rule has been added to ensure that users accessing the old path (
/stack/smart-contracts
) are automatically redirected to the new path (/stack/smart-contracts/smart-contracts
). This maintains backward compatibility while allowing for the documentation restructuring.pages/operators/chain-operators/deploy/overview.mdx (2)
1-19
: Metadata Validation and Frontmatter Compliance
The frontmatter section includes all the required fields with valid, non-empty values. All arrays (personas and categories) are properly populated.
37-42
: Updated Smart Contract Link in Callout
The hyperlink in the warning callout now correctly points to/stack/smart-contracts/smart-contracts
, which aligns with the updated documentation structure.pages/operators/chain-operators/deploy/smart-contracts.mdx (2)
1-19
: Frontmatter and Metadata Check
The frontmatter includes all required fields—title, lang, description, content_type, topic, personas, and categories—with appropriate values.
25-33
: Smart Contract Overview Link Update
The link to the smart contract overview is correctly updated to/stack/smart-contracts/smart-contracts
, ensuring consistency across the documentation.pages/stack/opcm.mdx (2)
1-19
: Frontmatter and Metadata Validation
The frontmatter is comprehensive with all required metadata fields present and non-empty. This ensures the document is properly categorized.
25-29
: Updated Link Reference in Content
The hyperlink for thePermissionedDisputeGame
now directs to/stack/smart-contracts/smart-contracts
, which aligns with the revised URL structure for smart contracts in the documentation.pages/stack/smart-contracts/_meta.json (1)
1-5
: Valid JSON Metadata File
The JSON file is correctly formatted and includes the expected keys:"smart-contracts"
,"smart-contract-upgrades"
, and"upgrade-op-contracts-1-6-1-8.mdx"
, each with an appropriate descriptive value.pages/operators/chain-operators/self-hosted.mdx (2)
1-18
: Frontmatter Inspection and Metadata Validity
The frontmatter section is complete with all required fields and non-empty values, ensuring the document meets metadata standards.
35-41
: Updated Smart Contracts Link
The hyperlinks for both "L1 smart contracts" and "L2 predeploy contracts" are updated to point to/stack/smart-contracts/smart-contracts
, which ensures that all references to smart contract resources are consistent. The link for "L2 preinstall contracts" remains appropriately directed.pages/stack/smart-contracts/upgrade-op-contracts-1-6-1-8.mdx (1)
1-21
: Frontmatter Validation:
The frontmatter is complete with all required fields (title, description, lang, content_type, topic, personas, categories, and is_imported_content). Please verify that the value used for “is_imported_content” (the string'false'
) conforms with the project’s metadata conventions.pages/stack/smart-contracts.mdx (1)
7-17
: Content and Card Links:
The use of<Cards>
with three<Card>
elements clearly presents the navigation options. Verify that each card’s title and hyperlink accurately direct users to the intended sections.
1. Create a new task directory in the appropriate network directory of the superchain-ops repository | ||
2. Copy the generated artifacts to the task directory for tracking and coordination | ||
3. Review the `Validation.md` file to understand and verify the expected state changes | ||
4. Simulate the transaction using Tenderly to confirm# Upgrading Optimism L1 Smart Contracts from v1.6.0 to v1.8.0 | ||
5. Collect required signatures from multisig signers after their independent verification |
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.
Formatting in Upgrade Steps:
There appears to be a formatting issue in the step-by-step instructions. In step 4, the text
4. Simulate the transaction using Tenderly to confirm# Upgrading Optimism L1 Smart Contracts from v1.6.0 to v1.8.0
merges a description with a heading marker (#
). Please ensure a proper newline or delimiter separates the confirmation note from the heading text.
pages/stack/smart-contracts.mdx
Outdated
--- | ||
title: Smart Contract overview | ||
description: Learn about the smart contracts that make up the OP Stack. | ||
title: Smart Contracts | ||
description: Documentation covering Smart Contract Upgrades, Smart Contracts, Upgrade Op Contracts 1 6 1 8 in the Smart Contracts section of the OP Stack ecosystem. | ||
lang: en-US | ||
content_type: guide | ||
topic: smart-contract-overview | ||
personas: | ||
- protocol-developer | ||
- chain-operator | ||
- app-developer | ||
categories: | ||
- mainnet | ||
- protocol | ||
- l1-contracts | ||
- predeploys | ||
- contract-addresses | ||
- contract-upgrades | ||
- architecture | ||
- fault-proofs | ||
is_imported_content: 'false' | ||
--- |
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.
Frontmatter Metadata:
For landing pages (i.e. those including <Cards>
), the required fields include title
, lang
, description
, and topic
. This file is missing the topic
field. Please add an appropriate topic value (for example, smart-contracts
) to meet the metadata requirements.
--- | ||
title: Smart Contract overview | ||
description: Learn about the smart contracts that make up the OP Stack. | ||
lang: en-US | ||
content_type: guide | ||
topic: smart-contract-overview | ||
personas: | ||
- protocol-developer | ||
- chain-operator | ||
- app-developer | ||
categories: | ||
- mainnet | ||
- protocol | ||
- l1-contracts | ||
- predeploys | ||
- contract-addresses | ||
- contract-upgrades | ||
- architecture | ||
- fault-proofs | ||
is_imported_content: 'false' | ||
--- |
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.
🛠️ Refactor suggestion
Frontmatter Validation and Title Consistency:
The frontmatter contains all required fields. However, the title is set as “Smart Contract overview” whereas the updated documentation guidelines call for “Smart Contracts” (as seen in other related files). Please update the title for consistency across the documentation.
is_imported_content: 'false' | ||
--- | ||
|
||
import { Callout } from 'nextra/components' |
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.
🛠️ Refactor suggestion
Import Statement Update:
Currently, the file imports { Callout }
from nextra/components
. To align with the new presentation style (which uses card layouts in related pages), consider updating this import to { Card, Cards }
if you plan to use those components in this section.
This is going to be a pain to review, i'd suggest we have this broken down into 2/3 PR's. |
It's just two new short docs, the rest of the changes are just redirects and link fixes |
Creates a new section on docs relating to smart contracts