Add block numbers of network upgrades #912
Conversation
WalkthroughThe changes involve updates to the network upgrades documentation, specifically enhancing the table that lists various upgrades by adding block numbers to their activation timestamps for both the OP Mainnet and OP Sepolia. This includes upgrades such as Granite, Fjord, Ecotone, Delta, and Canyon, while maintaining the existing governance approval links. The timestamp format has been slightly adjusted to include the block number. Changes
Possibly related issues
Possibly related PRs
Suggested reviewers
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 using PR comments)
Other keywords and placeholders
Documentation and Community
|
✅ Deploy Preview for docs-optimism ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
@sbvegan how can i confirm that the values in this PR are correct? |
|
@fvictorio, thank you for taking the time to investigate this and update our documentation. However, our L2 network upgrades (hard forks) are actually activated by timestamp and not block number. My initial gut feeling is to not add the block numbers they activated at because it doesn't reflect the protocols behavior and might cause confusion. Perhaps we can make a more emphasis on the timestamp activation and link to specs that might describe this in more detail. @krofax to check the activation timestamps, you can use the superchain-registry as the canoncial source of truth: |
|
@sbvegan thanks for the answer. I know Optimism's upgrades are timestamp-based and not block based, but sometimes you want to know at which block the activation happened because the JSON-RPC layer is still based in block numbers. For example, in EDR (Hardhat's network simulation engine) we have some tests that fork Mainnet and Optimism at different blocks and replay the transactions to make sure our logic matches the one in the live chain. We normally want to do this for more than one hardfork, and so we need to know the block numbers where it happened. And while it's not super hard to obtain them manually (as I did in this PR), it's frustrating that there's no place in the docs that tells you these values. I understand that this might be misleading, but the very first sentence before that table explicitly says that upgrades are activated by timestamps. If you still think the trade-off is not worth it, then I think there are two options:
|
There was a problem hiding this comment.
@fvictorio This makes sense and if its useful to you we'll include it in the docs for reference. However, I can't promise that we'll be able to maintain these block heights very well. Can you:
- Update the blockheight language to say something like, "at or around block x"?
- Add back ticks around the block number `?
Thank you for fixing the timestamps!
|
Done, thanks! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
pages/builders/node-operators/network-upgrades.mdx (2)
26-30: Clarify activation mechanism in the table headerThe addition of block numbers provides valuable information for developers. However, to avoid potential confusion, consider adding a note in the table header or as a footnote to clarify that upgrades are activated by timestamps, not block numbers.
Consider adding the following note above or below the table:
Note: Network upgrades are activated based on timestamps. Block numbers are provided for reference only and may vary slightly due to block time variations.This clarification aligns with the concerns raised in the PR discussion while maintaining the useful information for developers.
Line range hint
1-200: Minor style improvementsThe document generally follows the coding guidelines, but there are a few areas where we can improve consistency and clarity:
- Use sentence case for subheadings under "Summary of Changes" for consistency.
- Consider using bold text instead of all caps for emphasis in some places.
- Ensure proper nouns are consistently capitalized (e.g., "Optimism Collective").
Here are some suggested changes:
Change subheadings under "Summary of Changes" to sentence case:
### Limit `bn256Pairing` precompile input size ### Reduce channel timeout to 50Replace all-caps emphasis with bold:
- IMPORTANT: Your task is to ensure that each instruction is followed for each code segment you review. + **Important:** Your task is to ensure that each instruction is followed for each code segment you review.Capitalize proper nouns consistently:
- governance system votes on proposals. + Governance system votes on proposals.These changes will improve the overall consistency and readability of the document.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- pages/builders/node-operators/network-upgrades.mdx (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
pages/builders/node-operators/network-upgrades.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).
"
🔇 Additional comments (1)
pages/builders/node-operators/network-upgrades.mdx (1)
Line range hint
1-200: Overall assessment: Approved with minor suggestionsThe changes made to this document, particularly the addition of block numbers to the activation table, align well with the PR objectives and provide valuable information for developers. The content is informative, well-structured, and generally adheres to the coding guidelines.
To further improve the document:
- Consider adding a clarification note about the timestamp-based activation mechanism.
- Implement the suggested minor style improvements for consistency and clarity.
These changes will enhance the document's accuracy and readability while maintaining its informative nature.
|
@sbvegan checking here to know if this is good to go? |
The network upgrades table is very useful but sometimes you want to know the exact block number at which an ugprade was activated. I added these numbers in the table, following a methodology I'll explain below. I'd recommend someone triple-checks them with some other approach (I guess this information must be somewhere else?).
My approach to get the block numbers was mainly taking the timestamp and using etherscan "Timestamp to Block" feature (op mainnet, op sepolia) to get the block number.
Then, for all upgrades except Delta and Granite, I ran a separate check to confim this was the right block number:
isFjord()inGasPriceOraclereturned true in that block, and reverted in the previous one.Delta doesn't seem to have any way to check its activation using only the JSON-RPC layer, so I just used the result of Etherscan, with one caveat: for mainnet, the value I got was
116480611, but all the other mainnet blocks I got ended in 12, so I used116480612, since it's too much of a coincidence (and the timestamp-to-block tool can have off-by-one errors).For Granite I guess I could check if the
bn256Pairingprecompile has the additional validation, but that seems more work than I can put into this today 😅Also: the timestamp of the Fjord upgrade for OP Sepolia seems wrong. It had a different date than mainnet, but the UNIX timestamp was the same. I guess it was a copy-paste error.