-
Notifications
You must be signed in to change notification settings - Fork 260
Smart contract overview update #808
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
Conversation
WalkthroughThe updates enhance the OP Stack documentation by introducing a dedicated section for smart contracts, improving organizational clarity. A detailed guide on smart contracts was added, highlighting key functionalities and upgrades. Concurrently, a metadata entry related to smart contracts was removed from the rollup protocol, indicating a shift in focus. Several URL redirections were also revised to streamline navigation and provide clearer access to content. Changes
Recent review detailsConfiguration used: .coderabbit.yaml Files selected for processing (2)
Files skipped from review due to trivial changes (1)
Additional context usedLanguageTool
Additional comments not posted (4)
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 Configuration 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: 10
Outside diff range, codebase verification and nitpick comments (5)
pages/stack/smart-contracts.mdx (5)
9-11
: Clarify the introduction.The introduction is clear and informative but can be more concise.
- This guide provides an overview of the functionality of the smart contract components. You can also find [contract addresses](/chain/addresses) on OP Mainnet. + This guide provides an overview of smart contract functionality. Find [contract addresses](/chain/addresses) on OP Mainnet.
13-17
: Improve clarity and grammar.The description of Layer 1 contracts can be clearer and grammatically correct.
- The layer 1 contracts of the OP Stack are deployed on Ethereum. Their primary purpose is to facilitate the cross domain message passing and maintain the valid state root of the layer 2. + The Layer 1 contracts of the OP Stack are deployed on Ethereum. Their primary purpose is to facilitate cross-domain message passing and maintain the valid state root of Layer 2.
25-33
: Improve Callout formatting.The Callout content should be more concise and clear.
- For contract releases, refer to the GitHub release notes for a given release, which will list the specific contracts being released—**not all contracts are considered production ready within a release**, and many are under active development. These release pages are linked below. - Tags of the form `v<semver>`, such as `v1.1.4`, indicate releases of all Go code only, and **DO NOT** include smart contracts. + For contract releases, refer to the GitHub release notes, which list specific contracts being released. **Not all contracts are production-ready within a release**, and many are under active development. Release pages are linked below. + Tags like `v<semver>`, such as `v1.1.4`, indicate releases of all Go code only and **DO NOT** include smart contracts.
394-394
: Simplify phrase.‘Prior to’ might be wordy. Consider a shorter alternative.
- prior to the Ecotone upgrade + before the Ecotone upgradeTools
LanguageTool
[style] ~394-~394: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ...ed to compute the L1 portion of the fee prior to the Ecotone upgrade are: *scalar
...(EN_WORDINESS_PREMIUM_PRIOR_TO)
498-498
: Remove unnecessary "of".Consider removing “of” to be more concise.
- The `ProxyAdmin` is the owner of all of the proxy contracts + The `ProxyAdmin` is the owner of all the proxy contractsTools
LanguageTool
[style] ~498-~498: 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)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
The broken links are unrelated to this PR and because there are so many I think it should be addressed in a different one |
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, codebase verification and nitpick comments (9)
words.txt (1)
252-252
: Correct capitalization for 'Predeploys'.Ensure that the term 'Predeploys' is correctly capitalized and used consistently. The following instances need correction:
words.txt
: 'predeploys'public/_redirects
: 'modifying-predeploys'pages/stack/smart-contracts.mdx
: 'predeploys'pages/builders/chain-operators/self-hosted.mdx
: 'predeploy contracts'pages/builders/chain-operators/tutorials/modifying-predeploys.mdx
: 'predeploys'Analysis chain
Correct capitalization for 'Predeploys'.
Ensure that the term 'Predeploys' is correctly capitalized and used consistently.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage and capitalization of 'Predeploys' in the codebase. # Test: Search for the term 'Predeploys'. Expect: Correct capitalization and consistent usage. rg --type python -A 5 $'Predeploys'Length of output: 65
Script:
#!/bin/bash # Description: Verify the usage and capitalization of 'Predeploys' in the codebase. # Test: Search for the term 'Predeploys' across all files. Expect: Correct capitalization and consistent usage. rg -i 'Predeploys'Length of output: 4139
pages/stack/smart-contracts.mdx (8)
1-5
: Correct title casing and description.Ensure the title and description follow proper title case and sentence case conventions.
- title: Smart Contract Overview - description: Learn about the smart contracts that make up the OP Stack. + title: Smart Contract Overview + description: Learn about the smart contracts that make up the OP stack.
11-11
: Use proper nouns.Replace 'You can also find' with 'Users can also find' for consistency.
- You can also find [contract addresses](/chain/addresses) on OP Mainnet. + Users can also find [contract addresses](/chain/addresses) on OP Mainnet.
15-17
: Use the Oxford comma.Add a comma after 'message passing' for clarity.
- facilitate the cross domain message passing and maintain the + facilitate the cross domain message passing, and maintain the
21-23
: Correct preposition usage.Replace 'as' with 'with' for clarity.
- versioned as `<component-name>/v<semver>` + versioned with `<component-name>/v<semver>`
25-33
: Use proper title casing and emphasis.Ensure proper title casing and use bold for emphasis instead of all caps.
- **DO NOT** include smart contracts. + **Do not** include smart contracts.
332-332
: Add missing comma.Add a comma after "precompiles".
- They are similar to precompiles but instead run directly in the EVM instead of running native code outside the EVM. + They are similar to precompiles, but instead run directly in the EVM instead of running native code outside the EVM.Tools
LanguageTool
[uncategorized] ~332-~332: Possible missing comma found.
Context: ...s state. They are similar to precompiles but instead run directly in the EVM instead...(AI_HYDRA_LEO_MISSING_COMMA)
418-418
: Use concise language.Replace 'prior to' with 'before' for conciseness.
- prior to the Ecotone upgrade are: + before the Ecotone upgrade are:Tools
LanguageTool
[style] ~418-~418: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ...ed to compute the L1 portion of the fee prior to the Ecotone upgrade are: *scalar
...(EN_WORDINESS_PREMIUM_PRIOR_TO)
516-516
: Remove redundant 'of'.Remove 'of' to be more concise.
- The `ProxyAdmin` is the owner of all of the proxy contracts set at the predeploys. + The `ProxyAdmin` is the owner of all the proxy contracts set at the predeploys.Tools
LanguageTool
[style] ~516-~516: 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)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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.
I haven't reviewed the "Layer 2 Contracts (Predeploys)" section and below yet, will continue there tomorrow. Overall this is really great, thank you!
Co-authored-by: Matt Solomon <[email protected]>
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
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: 3
Outside diff range, codebase verification and nitpick comments (11)
pages/stack/smart-contracts.mdx (11)
11-11
: Ensure clarity and proper documentation style.The sentence should be clear and follow the documentation guidelines.
- This guide provides an overview of the functionality of the smart contract components. You can also find [contract addresses](/chain/addresses) on OP Mainnet. + This guide provides an overview of the functionality of the smart contract components. Find [contract addresses](/chain/addresses) on OP Mainnet.
23-23
: Ensure clarity and proper documentation style.The sentence should be clear and follow the documentation guidelines.
- Contract releases have a component name of `op-contracts` and therefore are tagged as `op-contract/vX.Y.Z`. + Contract releases have a component name of `op-contracts` and are tagged as `op-contract/vX.Y.Z`.
31-33
: Ensure clarity and proper documentation style.The sentence should be clear and follow the documentation guidelines.
- Releases or tags of the form `v<semver>` without a component name, such as `v1.1.4`, indicate releases of all Go code - only, and **DO NOT** include smart contracts. DO NOT use these releases for deploying - smart contracts—only deploy from `op-contracts/vX.Y.Z` + Releases or tags of the form `v<semver>` without a component name, such as `v1.1.4`, indicate releases of all Go code only and **DO NOT** include smart contracts. DO NOT use these releases for deploying smart contracts—only deploy from `op-contracts/vX.Y.Z`.
39-40
: Ensure clarity and proper documentation style.The note should be clear and follow the documentation guidelines.
- Note: While these are a governance approved contract release, the recommended - release for new production chains is `op-contracts/v1.3.0`. + Note: While these are a governance-approved contract release, the recommended release for new production chains is `op-contracts/v1.3.0`.
47-48
: Ensure clarity and proper documentation style.The sentence should be clear and follow the documentation guidelines.
- Increasing the Security Council Safe's signing threshold, from 4 to 10, out - of 13 owners. This meets the 75% threshold requirement for a Stage 1 rollup - outlined in [L2Beat's Stages framework](https://medium.com/l2beat/stages-update-security-council-requirements-4c79cea8ef52) + Increasing the Security Council Safe's signing threshold from 4 to 10 out of 13 owners. This meets the 75% threshold requirement for a Stage 1 rollup outlined in [L2Beat's Stages framework](https://medium.com/l2beat/stages-update-security-council-requirements-4c79cea8ef52).
52-53
: Ensure clarity and proper documentation style.The sentence should be clear and follow the documentation guidelines.
- Additionally the Foundation is appointed to the new DeputyGuardian role - which is able to act as Guardian through the Guardian Safe. This + Additionally, the Foundation is appointed to the new DeputyGuardian role, which can act as Guardian through the Guardian Safe. ThisTools
LanguageTool
[uncategorized] ~52-~52: A comma may be missing after the conjunctive/linking adverb ‘Additionally’.
Context: ...dow 5 requirement for Stage 1. * Additionally the Foundation is appointed to the new ...(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
[style] ~53-~53: As a shorter alternative for ‘able to’, consider using “can”.
Context: ... new DeputyGuardian role which is able to act as Guardian through the Guardian Sa...(BE_ABLE_TO)
100-101
: Ensure clarity and proper documentation style.The sentence should be clear and follow the documentation guidelines.
- As part of a responsible and safe rollout of Fault Proofs, it preserves - the ability for the guardian to override if necessary to maintain security. + As part of a responsible and safe rollout of Fault Proofs, it preserves the ability for the guardian to override if necessary to maintain security.
108-110
: Ensure clarity and proper documentation style.The sentence should be clear and follow the documentation guidelines.
- Combined with the Guardian, Security Council Threshold and L2 ProxyAdmin - Ownership changes 23 proposals, this satisfies the criteria to have OP Chains - reach Stage 1 status. + Combined with the Guardian, Security Council Threshold, and L2 ProxyAdmin Ownership changes 23 proposals, this satisfies the criteria to have OP Chains reach Stage 1 status.
119-125
: Ensure clarity and proper documentation style.The list should be clear and follow the documentation guidelines.
- FaultDisputeGame: [1.2.0](https://github.com/ethereum-optimism/optimism/blob/op-contracts/v1.4.0/packages/contracts-bedrock/src/dispute/FaultDisputeGame.sol#L73) - PermissionedDisputeGame: [1.2.0](https://github.com/ethereum-optimism/optimism/blob/op-contracts/v1.4.0/packages/contracts-bedrock/src/dispute/PermissionedDisputeGame.sol) - DisputeGameFactory: [1.0.0](https://github.com/ethereum-optimism/optimism/blob/op-contracts/v1.4.0/packages/contracts-bedrock/src/dispute/DisputeGameFactory.sol#L25) - AnchorStateRegistry: [1.0.0](https://github.com/ethereum-optimism/optimism/blob/op-contracts/v1.4.0/packages/contracts-bedrock/src/dispute/AnchorStateRegistry.sol#L28) - DelayedWETH: [1.0.0](https://github.com/ethereum-optimism/optimism/blob/op-contracts/v1.4.0/packages/contracts-bedrock/src/dispute/weth/DelayedWETH.sol#L25) - MIPS: [1.0.1](https://github.com/ethereum-optimism/optimism/blob/op-contracts/v1.4.0/packages/contracts-bedrock/src/cannon/MIPS.sol#L47) - PreimageOracle: [1.0.0](https://github.com/ethereum-optimism/optimism/blob/op-contracts/v1.4.0/packages/contracts-bedrock/src/cannon/PreimageOracle.sol#L33) + * FaultDisputeGame: [1.2.0](https://github.com/ethereum-optimism/optimism/blob/op-contracts/v1.4.0/packages/contracts-bedrock/src/dispute/FaultDisputeGame.sol#L73) + * PermissionedDisputeGame: [1.2.0](https://github.com/ethereum-optimism/optimism/blob/op-contracts/v1.4.0/packages/contracts-bedrock/src/dispute/PermissionedDisputeGame.sol) + * DisputeGameFactory: [1.0.0](https://github.com/ethereum-optimism/optimism/blob/op-contracts/v1.4.0/packages/contracts-bedrock/src/dispute/DisputeGameFactory.sol#L25) + * AnchorStateRegistry: [1.0.0](https://github.com/ethereum-optimism/optimism/blob/op-contracts/v1.4.0/packages/contracts-bedrock/src/dispute/AnchorStateRegistry.sol#L28) + * DelayedWETH: [1.0.0](https://github.com/ethereum-optimism/optimism/blob/op-contracts/v1.4.0/packages/contracts-bedrock/src/dispute/weth/DelayedWETH.sol#L25) + * MIPS: [1.0.1](https://github.com/ethereum-optimism/optimism/blob/op-contracts/v1.4.0/packages/contracts-bedrock/src/cannon/MIPS.sol#L47) + * PreimageOracle: [1.0.0](https://github.com/ethereum-optimism/optimism/blob/op-contracts/v1.4.0/packages/contracts-bedrock/src/cannon/PreimageOracle.sol#L33)
129-130
: Ensure clarity and proper documentation style.The list should be clear and follow the documentation guidelines.
- OptimismPortal: [3.10.0](https://github.com/ethereum-optimism/optimism/blob/op-contracts/v1.4.0/packages/contracts-bedrock/src/L1/OptimismPortal2.sol#L144) - SystemConfig: [2.2.0](https://github.com/ethereum-optimism/optimism/blob/547ea72d9849e13ce169fd31df0f9197651b3f86/packages/contracts-bedrock/src/L1/SystemConfig.sol#L111) + * OptimismPortal: [3.10.0](https://github.com/ethereum-optimism/optimism/blob/op-contracts/v1.4.0/packages/contracts-bedrock/src/L1/OptimismPortal2.sol#L144) + * SystemConfig: [2.2.0](https://github.com/ethereum-optimism/optimism/blob/547ea72d9849e13ce169fd31df0f9197651b3f86/packages/contracts-bedrock/src/L1/SystemConfig.sol#L111)
134-134
: Ensure clarity and proper documentation style.The list should be clear and follow the documentation guidelines.
- L2OutputOracle: 1.8.0 + * L2OutputOracle: 1.8.0
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: 6
Outside diff range, codebase verification and nitpick comments (15)
pages/stack/smart-contracts.mdx (15)
1-11
: Use title case for headers.The title "Smart Contract Overview" should be in title case.
- # Smart Contract Overview + # Smart Contract Overview
11-11
: Clarify link description.The link description should specify that it is for OP Mainnet contract addresses.
- You can also find [contract addresses](/chain/addresses) on OP Mainnet. + You can also find [contract addresses on OP Mainnet](/chain/addresses).
13-17
: Correct preposition usage.The phrase "facilitate the cross domain message passing" should use "facilitate cross-domain message passing" for clarity.
- purpose is to facilitate the cross domain message passing and maintain the + purpose is to facilitate cross-domain message passing and maintain the
21-23
: Clarify versioning description.The description of versioning should be more explicit for clarity.
- All production releases are always tagged, versioned as `<component-name>/v<semver>`. + All production releases are always tagged and versioned as `<component-name>/v<semver>`.
31-33
: Add missing punctuation.Add a period at the end of the sentence for consistency.
- only, and **DO NOT** include smart contracts. DO NOT use these releases for deploying + only, and **DO NOT** include smart contracts. DO NOT use these releases for deploying.
39-40
: Clarify note.The note should be more explicit about the governance approval.
- Note: While these are a governance approved contract release, the recommended + Note: While these contracts are governance-approved, the recommended
94-95
: Clarify note.The note should be more explicit about the governance approval.
- Note: While these are a governance approved contract release, the recommended + Note: While these contracts are governance-approved, the recommended
149-151
: Clarify note.The note should be more explicit about the recommendation.
- This is the current recommended contract release for new production chains. + This is the currently recommended contract release for new production chains.
184-185
: Simplify wording.Simplify the phrase "has the ability to" to "can".
- It has the ability to pause and unpause all withdrawals + It can pause and unpause all withdrawals
289-291
: Simplify wording.Simplify the phrase "has the ability to" to "can".
- It has the ability to pause and unpause all withdrawals + It can pause and unpause all withdrawalsTools
LanguageTool
[style] ~290-~290: The phrase ‘has the ability to’ might be wordy. Consider using “can”.
Context: ...ration of global superchain values. It has the ability to pause and unpause all withdrawals in th...(HAS_THE_ABILITY_TO)
386-387
: Add missing comma.Add a comma after "precompiles".
- They are similar to precompiles but instead run directly in the EVM + They are similar to precompiles, but instead run directly in the EVMTools
LanguageTool
[uncategorized] ~386-~386: Possible missing comma found.
Context: ...s state. They are similar to precompiles but instead run directly in the EVM instead...(AI_HYDRA_LEO_MISSING_COMMA)
389-391
: Add missing comma.Add a comma after "multiclient".
- easier for multiclient implementations as well as allowing for more integration + easier for multiclient implementations, as well as allowing for more integrationTools
LanguageTool
[uncategorized] ~389-~389: Possible missing comma found.
Context: ...piles to make it easier for multiclient implementations as well as allowing for more integratio...(AI_HYDRA_LEO_MISSING_COMMA)
472-473
: Simplify wording.The phrase "prior to" might be wordy. Consider using "before".
- prior to the Ecotone upgrade are: + before the Ecotone upgrade are:Tools
LanguageTool
[style] ~472-~472: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ...ed to compute the L1 portion of the fee prior to the Ecotone upgrade are: *scalar
...(EN_WORDINESS_PREMIUM_PRIOR_TO)
570-570
: Remove redundant word.Consider removing “of” to be more concise.
- The `ProxyAdmin` is the owner of all of the proxy contracts set at the + The `ProxyAdmin` is the owner of all the proxy contracts set at theTools
LanguageTool
[style] ~570-~570: 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)
671-674
: Correct preposition usage.The usual preposition to use after “fills” is “with”, not “the”.
- fills the role of the + fills the role with the
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
The `FaultDisputeGame` contract instances that each act as a host to a proposal | ||
about the state of the OP Stack chain at a given block number. |
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.
Typo in this sentence? "The FaultDisputeGame
contract instances that each act" is confusing me
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.
Yeah this is a frankenstein's monster from pulling this out of the dipsutegamefactory
In the legacy system, the `GasPriceOracle` was a permissioned contract that was | ||
pushed the L1 base fee and the L2 gas price by an offchain actor. The offchain | ||
actor observes the L1 blockheaders to get the L1 base fee as well as the gas | ||
usage on L2 to compute what the L2 gas price should be based on a congestion | ||
control algorithm. | ||
|
||
After Bedrock, the `GasPriceOracle` is no longer a permissioned contract and |
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.
Delete legacy pre-Bedrock info and just explain what it currently is?
<Callout> | ||
There is a legacy contract under this same name. | ||
</Callout> |
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.
What does this mean? I'm not familiar with this (doesn't mean it's not true, just unclear what this means exactly and if this matters to readers)
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.
idk where this came from tbh, I'm going to remove it
can be refunded on L2. | ||
</Callout> | ||
|
||
The `L2ERC721Bridge` is a contract which works together with the L1 ERC721 bridge to |
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.
The `L2ERC721Bridge` is a contract which works together with the L1 ERC721 bridge to | |
The `L2ERC721Bridge` is a contract which works together with the `L1ERC721Bridge` to |
Description