docs: add postmortem of zktrie fork #272
Conversation
WalkthroughThe incident report describes a critical event on March 12, 2024, where a hardfork was necessary due to a vulnerability in the Changes
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
postmortems/assets/zktrie-deletion.svgis excluded by:!**/*.svg
Files selected for processing (1)
- postmortems/2024-03-12-zktrie-hardfork.md (1 hunks)
Additional comments: 4
postmortems/2024-03-12-zktrie-hardfork.md (4)
- 1-1: The document title and headings are clear and appropriately formatted. However, ensure that "ZKTrie" and "Hardfork" are correctly spelled as per your project's terminology, as static analysis tools flagged them as possible spelling mistakes. These are likely false positives but verifying consistency with project terminology is crucial.
- 5-40: Overall, the document provides a comprehensive overview of the incident, its causes, and the recovery process. It's well-structured and informative. However, consider adding a section on "Future Preventative Measures" to outline specific steps or technologies being implemented to prevent similar incidents. This could enhance the document's utility as a learning resource for the community.
- 10-10: The explanation of the
KromaZKTriebug is clear, but the technical details might be too complex for readers unfamiliar with blockchain internals. Consider adding a simplified summary or analogy to help less technical readers grasp the impact of the bug on the network.- 30-30: Including a visual representation (
zktrie-deletion.svg) is an excellent way to illustrate the bug's effect on the state tree. Ensure that the image is accessible and renders correctly on all platforms where the document might be viewed.
| Significant time was required for the chain to be fully recovered after this incident, resulting in a prolonged halt in | ||
| block generation. This was due to the absence of a rollback strategy during the upgrade process. In future upgrades, a | ||
| rollback strategy will always be provided alongside the upgrade. |
There was a problem hiding this comment.
The section on recovery and lessons learned is insightful. However, explicitly mentioning the development or adoption of new tools, if any, for monitoring or automating rollback processes in future incidents could provide additional value to readers.
|
|
||
| # Incident Summary | ||
|
|
||
| On March 12, 2024, at 15:16:25 UTC, a hardfork occurred at block number 8171899. |
There was a problem hiding this comment.
I guess 'fork' is the more appropriate word than 'hardfork' in this case.
There was a problem hiding this comment.
Changed to 'fork' as you mentioned. Thank you
0xHansLee
changed the title
0xHansLee
force-pushed
the
docs/zktrie-postmortem
branch
from
5a8801b to
acd78a9
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
postmortems/assets/zktrie-deletion.svgis excluded by:!**/*.svg
Files selected for processing (1)
- postmortems/2024-03-12-zktrie-hardfork.md (1 hunks)
| @@ -0,0 +1,85 @@ | |||
| # 2024-03-12 Chain Halt due to ZKTrie Upgrade Post-Mortem | |||
There was a problem hiding this comment.
The title of the document contains "Post-Mortem" which is commonly written as "Postmortem" or "Post-mortem". Consider standardizing the spelling for consistency with common industry practices.
- # 2024-03-12 Chain Halt due to ZKTrie Upgrade Post-Mortem
+ # 2024-03-12 Chain Halt due to ZKTrie Upgrade PostmortemCommittable 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.
| # 2024-03-12 Chain Halt due to ZKTrie Upgrade Post-Mortem | |
| # 2024-03-12 Chain Halt due to ZKTrie Upgrade Postmortem |
| should be removed, leaving an empty node. However, in this case, after deleting the node, another child node was | ||
| mistakenly set as the root node, altering the state tree and resulting in a different state root value. | ||
|
|
||
|  |
There was a problem hiding this comment.
Given the existing comment about the ambiguity of using "L" to denote a leaf or a left node, it's advisable to clarify the diagram or its description to ensure readers can easily understand the tree structure and the issue being described.
Consider adding a more detailed explanation or legend for the diagram to address the ambiguity and enhance understanding.
Summary by CodeRabbit