-
Notifications
You must be signed in to change notification settings - Fork 175
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
feat: add superchain-registry #699
Conversation
WalkthroughThe recent changes introduce a new entry, "Superchain Registry," to the Changes
Sequence Diagram(s)The changes are primarily related to documentation and terminology updates, so a sequence diagram is not applicable in this context. 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: 5
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
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
pages/stack/superchain-registry.mdx
Outdated
It ensures transparency by documenting the configuration parameters for each chain, | ||
such as their protocol versions and governance settings. This registry is essential for maintaining consistency and | ||
security across the network by verifying that all included chains meet the criteria set forth by their | ||
respective Blockspace Charters [Optimism Collective](https://gov.optimism.io/t/season-6-introducing-blockspace-charters-superchain-first-governance/8133) |
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.
Ensure Adequate Explanation or Links for Semantic Versioning Lockfiles
The search results and inspection of markdown files did not reveal any additional documentation or links explaining the semantic versioning lockfiles (semver.yaml
). To improve clarity, consider adding a brief explanation or a link to relevant documentation within the pages/stack/superchain-registry.mdx
file.
Would you like me to draft a brief explanation or find a suitable link to include?
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.
yes
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.
@cpengilly, I've prepared a brief explanation and found a suitable link regarding semantic versioning lockfiles. Here's a draft that can be included in the pages/stack/superchain-registry.mdx
file:
### Semantic Versioning Lockfiles
Semantic Versioning Lockfiles (`semver.yaml`) are used to lock the versions of contracts and tools to specific releases. This ensures that all deployments are consistent and predictable, preventing issues that might arise from version mismatches. These lockfiles adhere to [Semantic Versioning](https://semver.org/), which is a versioning scheme for software that aims to convey meaning about the underlying changes with each release.
For more detailed information on Semantic Versioning, you can visit the official Semantic Versioning website [here](https://semver.org/).
This addition should help clarify the purpose and importance of semantic versioning lockfiles within the documentation. If this looks good to you, it can be added to the respective section in the pages/stack/superchain-registry.mdx
file.
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
## CheckSecurityConfigs | ||
|
||
The `CheckSecurityConfigs.s.sol` script is used in CI to perform security checks of OP Chains registered in the superchain directory. | ||
At high level, it performs checks to ensure privileges are properly granted to the right addresses. |
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.
Add the article "a" before "high level" to improve grammatical correctness.
- At high level, it performs checks to ensure privileges are properly granted to the right addresses.
+ At a high level, it performs checks to ensure privileges are properly granted to the right addresses.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
At high level, it performs checks to ensure privileges are properly granted to the right addresses. | |
At a high level, it performs checks to ensure privileges are properly granted to the right addresses. |
Tools
LanguageTool
[uncategorized] ~70-~70: Possible missing article found. (AI_HYDRA_LEO_MISSING_THE)
Context: ...istered in the superchain directory. At high level, it performs checks to ensure pri...
pages/stack/superchain-registry.mdx
Outdated
* Trusted portal. For example, `L1CrossDomainMessengerProxy` specifies | ||
the portal it trusts to deposit transactions and get L2 senders. | ||
* Trusted oracles. For example, `OptimismPortalProxy` specifies the L2 oracle they trust with the L2 state root information. | ||
<Callout type="info"> |
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.
Ensure consistent indentation for children elements in markdown.
- <Callout type="info">
+ <Callout type="info">
Also applies to: 100-100
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
<Callout type="info"> | |
<Callout type="info"> |
Tools
GitHub Check: lint
[warning] 90-90:
Don’t use mixed indentation for children, remove 2 spaces
</Callout> | ||
</Steps> | ||
|
||
As a result, here is a visualization of all the relationships the `CheckSecurityConfigs.s.sol` script checks: |
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.
Add the preposition "of" after "relationships" for grammatical accuracy.
- here is a visualization of all the relationships the `CheckSecurityConfigs.s.sol` script checks:
+ here is a visualization of all the relationships of the `CheckSecurityConfigs.s.sol` script checks:
Committable suggestion was skipped due to low confidence.
Tools
LanguageTool
[uncategorized] ~107-~107: Possible missing preposition found. (AI_HYDRA_LEO_MISSING_OF)
Context: ... visualization of all the relationships theCheckSecurityConfigs.s.sol
script che...
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
title: Superchain Registry Explainer | ||
lang: en-US | ||
description: Learn about Superchain configurations, member chains, contract permissions, and tools for contributing new chains or superchains. |
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.
Ensure consistency in the description for clarity.
The description in the metadata section could be enhanced for clarity and engagement. Consider revising it to something more detailed:
- description: Learn about Superchain configurations, member chains, contract permissions, and tools for contributing new chains or superchains.
+ description: Explore the intricacies of Superchain configurations, understand the roles of member chains, delve into contract permissions, and discover how you can contribute to the development of new chains or superchains within the Optimism ecosystem.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
title: Superchain Registry Explainer | |
lang: en-US | |
description: Learn about Superchain configurations, member chains, contract permissions, and tools for contributing new chains or superchains. | |
title: Superchain Registry Explainer | |
lang: en-US | |
description: Explore the intricacies of Superchain configurations, understand the roles of member chains, delve into contract permissions, and discover how you can contribute to the development of new chains or superchains within the Optimism ecosystem. |
A second module exists in this repo whose purpose is to validate the config exported by the `superchain` module. | ||
It is a separate module to avoid import cycles and polluting downstream dependencies with things like `go-ethereum` | ||
(which is used in the validation tests). | ||
The modules are tracked by a top-level go.work file. |
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.
Ensure consistent use of terminology and capitalization.
The term 'top-level' should be hyphenated when used as a compound adjective:
- The modules are tracked by a top level go.work file.
+ The modules are tracked by a top-level go.work file.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
The modules are tracked by a top-level go.work file. | |
The modules are tracked by a top-level go.work file. |
The Superchain Registry repository hosts Superchain-configuration data in a minimal human-readable form. | ||
This includes mainnet and testnet Superchain targets and their respective member chains. | ||
|
||
Other configuration, such as contract-permissions, and `SystemConfig` parameters are hosted and governed onchain. |
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.
Clarify the use of commas in complex lists.
The use of commas in the list of configurations is inconsistent. Consider adding a comma after 'contract-permissions' to improve readability:
- Other configuration, such as contract-permissions and `SystemConfig` parameters are hosted and governed onchain.
+ Other configuration, such as contract-permissions, and `SystemConfig` parameters are hosted and governed onchain.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Other configuration, such as contract-permissions, and `SystemConfig` parameters are hosted and governed onchain. | |
Other configuration, such as contract-permissions, and `SystemConfig` parameters are hosted and governed onchain. |
<Callout type="warning"> | ||
The [superchain registry](https://github.com/ethereum-optimism/superchain-registry/tree/main) is a work in progress and still must be approved by Optimism Governance. Until that time, the configuration described here is subject to change. | ||
</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.
Review the warning callout for clarity and actionability.
The warning callout could be more actionable by specifying what steps users should take or what they should be aware of during this interim period:
- The [superchain registry](https://github.com/ethereum-optimism/superchain-registry/tree/main) is a work in progress and still must be approved by Optimism Governance. Until that time, the configuration described here is subject to change.
+ The [superchain registry](https://github.com/ethereum-optimism/superchain-registry/tree/main) is currently under review and pending approval by Optimism Governance. Please note that the configurations described herein are provisional and subject to modifications. Stay updated by following our governance discussions.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
<Callout type="warning"> | |
The [superchain registry](https://github.com/ethereum-optimism/superchain-registry/tree/main) is a work in progress and still must be approved by Optimism Governance. Until that time, the configuration described here is subject to change. | |
</Callout> | |
<Callout type="warning"> | |
The [superchain registry](https://github.com/ethereum-optimism/superchain-registry/tree/main) is currently under review and pending approval by Optimism Governance. Please note that the configurations described herein are provisional and subject to modifications. Stay updated by following our governance discussions. | |
</Callout> |
the portal it trusts to deposit transactions and get L2 senders. | ||
* Trusted oracles. For example, `OptimismPortalProxy` specifies the L2 oracle they trust with the L2 state root information. | ||
<Callout type="info"> | ||
After the FPAC upgrade, the `OptimismPortalProxy` specifies the `DisputeGameFactory` they trust rather than the legacy `L2OutputOracle` contract. |
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.
Clarify the role of the DisputeGameFactory
post-upgrade.
The sentence structure could be improved for clarity. Consider rephrasing to explicitly state the change:
- After the FPAC upgrade, the `OptimismPortalProxy` specifies the `DisputeGameFactory` they trust rather than the legacy `L2OutputOracle` contract.
+ After the FPAC upgrade, the `OptimismPortalProxy` has transitioned from trusting the legacy `L2OutputOracle` contract to specifying the `DisputeGameFactory`.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
After the FPAC upgrade, the `OptimismPortalProxy` specifies the `DisputeGameFactory` they trust rather than the legacy `L2OutputOracle` contract. | |
After the FPAC upgrade, the `OptimismPortalProxy` has transitioned from trusting the legacy `L2OutputOracle` contract to specifying the `DisputeGameFactory`. |
|
||
## CheckSecurityConfigs | ||
|
||
The `CheckSecurityConfigs.s.sol` script is used in CI to perform security checks of OP Chains registered in the superchain directory. |
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.
Nit: This has recently been changed to a Go test. ethereum-optimism/superchain-registry#257
Closing this because we have a Superchain Explainer already in the docs. |
Description
A clear and concise description of the features you're adding in this pull request.
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