Skip to content
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

docs: Update Silent Patch Policy Broken Link #10329

Merged
merged 1 commit into from Apr 29, 2024

Conversation

SanShi2023
Copy link
Contributor

Update Silent Patch Policy Broken Link

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

  • Fixes #[Link to Issue]

Copy link
Contributor

coderabbitai bot commented Apr 27, 2024

Walkthrough

Walkthrough

The update primarily involves a modification to a URL link within the public disclosure section of a document. This change is made to ensure the document aligns with the Geth team's updated policy on silent patches. The adjustment reflects a shift in the URL structure on the Geth website, directing readers to the correct resource regarding their policy.

Changes

File Path Change Summary
docs/.../2022-02-02-inflation-vuln.md Updated URL in public disclosure section to reflect new link structure.

Recent Review Details

Configuration used: .coderabbit.yml
Review profile: CHILL

Commits Files that changed from the base of the PR and between 826a7bd and 3062c6b.
Files selected for processing (1)
  • docs/postmortems/2022-02-02-inflation-vuln.md (1 hunks)
Additional Context Used
LanguageTool (124)
docs/postmortems/2022-02-02-inflation-vuln.md (124)

Near line 8: Possible spelling mistake found.
Context: ... A vulnerability in Optimism’s fork of [Geth](https://github.com/ethereum/go-ethereu...


Near line 8: Possible typo detected.
Context: ...-ethereum) (which we refer to as L2Geth) was reported to us by [Jay ...


Near line 9: Possible spelling mistake found.
Context: ...eeman](https://twitter.com/saurik) (AKA saurik) on February 2nd, 2022. If exploited, t...


Near line 19: This sentence does not start with an uppercase letter.
Context: ...urs of the initial report. ## Lead up saurik had been engaging with our code, and [o...


Near line 22: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ...ue+author%3Asaurik+) for several months prior to identifying this bug. We launched our ...


Near line 24: Possible spelling mistake found.
Context: ...identifying this bug. We launched our [Immunefi-hosted bug bounty program](https://immunefi.co...


Near line 26: Possible spelling mistake found.
Context: ...he program was $2,000,042. According to saurik, his decision to hunt for bugs in our c...


Near line 27: Possible spelling mistake found.
Context: ... a topic to speak about at the upcoming ETHDenver conference. ## The Vulnerability Cont...


Near line 37: Possible spelling mistake found.
Context: ... A thorough description can be found in saurik's [writeup](https://www.saurik.com/opti...


Near line 37: Possible spelling mistake found.
Context: ...h description can be found in saurik's writeup....


Near line 41: The preposition “on” seems more likely in this position.
Context: ...s not exploited, so there was no impact to ordinary users. However, the issue requ...


Near line 47: Possible spelling mistake found.
Context: ...ion. ## Detection Jay Freeman (a.k.a. saurik) reported the bug to us via security@op...


Near line 48: Possible spelling mistake found.
Context: ...o. He first attempted to report via our Immunefi bounty program, but decided to email us...


Near line 53: Did you mean the communication tool “Slack” (= proper noun, capitalized)?
Context: ...a small subset of the team in a private slack channel. The timeline and activities we...


Near line 58: The official name of this software platform is spelled with a capital “H”.
Context: ...as follows: ### Timeline (UTC) (Using github handles as identifiers) - 2022-02-02 1...


Near line 60: Possible spelling mistake found.
Context: ...les as identifiers) - 2022-02-02 1625: smartcontracts receives an e-mail from saurik claiming...


Near line 60: Possible spelling mistake found.
Context: ... smartcontracts receives an e-mail from saurik claiming to have found a critical iss...


Near line 62: Possible spelling mistake found.
Context: ...to securityoptimism.io. - 2022-02-02 X: saurik messaged smartcontracts on Discord to m...


Near line 62: Possible spelling mistake found.
Context: ...ism.io. - 2022-02-02 X: saurik messaged smartcontracts on Discord to make sure we checked the ...


Near line 65: Possible spelling mistake found.
Context: ... #security on Slack. - 2022-02-02 1758: tynes and smartcontracts confirm the issue on...


Near line 65: Possible spelling mistake found.
Context: ... on Slack. - 2022-02-02 1758: tynes and smartcontracts confirm the issue on the huddle. - 2022...


Near line 66: Possible spelling mistake found.
Context: ...issue on the huddle. - 2022-02-02 1812: mslipper joins the huddle and alerts infrastruct...


Near line 68: Possible spelling mistake found.
Context: ...te will be required. - 2022-02-02 1906: tynes cuts the following builds: - Mainnet:...


Near line 69: Possible spelling mistake found.
Context: ...6: tynes cuts the following builds: - Mainnet: 0.5.8_b6f79171 - Kovan: `0.5.9_d4c...


Near line 70: Possible spelling mistake found.
Context: ...ilds: - Mainnet: 0.5.8_b6f79171 - Kovan: 0.5.9_d4c6d824 - 2022-02-20 1930: op...


Near line 71: Possible spelling mistake found.
Context: ...an: 0.5.9_d4c6d824 - 2022-02-20 1930: optimisticben deploys to Kovan and mainnet. - 2022-02...


Near line 71: Possible spelling mistake found.
Context: ...22-02-20 1930: optimisticben deploys to Kovan and mainnet. - 2022-02-02 2021: mslippe...


Near line 71: Possible spelling mistake found.
Context: ...930: optimisticben deploys to Kovan and mainnet. - 2022-02-02 2021: mslipper gives inst...


Near line 72: Possible spelling mistake found.
Context: ...o Kovan and mainnet. - 2022-02-02 2021: mslipper gives instructions to infra providers o...


Near line 72: To make your text as clear as possible to all readers, do not use this foreign term. Possible alternatives are “below” or “further on” (in a document).
Context: ...02 2021: mslipper gives instructions to infra providers on how to upgrade. - 2022-02-...


Near line 73: Possible spelling mistake found.
Context: ...s on how to upgrade. - 2022-02-02 2150: Infura upgrades both Kovan and mainnet. - 2022...


Near line 73: Possible spelling mistake found.
Context: ...- 2022-02-02 2150: Infura upgrades both Kovan and mainnet. - 2022-02-03 0457: Alchemy...


Near line 73: Possible spelling mistake found.
Context: ...02 2150: Infura upgrades both Kovan and mainnet. - 2022-02-03 0457: Alchemy upgrades bo...


Near line 74: Possible spelling mistake found.
Context: ... 2022-02-03 0457: Alchemy upgrades both Kovan and mainnet. - 2022-02-03 2309: Quickno...


Near line 74: Possible spelling mistake found.
Context: ...3 0457: Alchemy upgrades both Kovan and mainnet. - 2022-02-03 2309: Quicknode upgrades ...


Near line 75: Possible spelling mistake found.
Context: ...h Kovan and mainnet. - 2022-02-03 2309: Quicknode upgrades mainnet. - 2022-02-03 1432: Qu...


Near line 75: Possible spelling mistake found.
Context: .... - 2022-02-03 2309: Quicknode upgrades mainnet. - 2022-02-03 1432: Quicknode upgrades ...


Near line 76: Possible spelling mistake found.
Context: ...de upgrades mainnet. - 2022-02-03 1432: Quicknode upgrades Kovan. - 2022-02-03 1945: smar...


Near line 76: Possible spelling mistake found.
Context: .... - 2022-02-03 1432: Quicknode upgrades Kovan. - 2022-02-03 1945: smartcontracts aler...


Near line 77: Possible spelling mistake found.
Context: ...node upgrades Kovan. - 2022-02-03 1945: smartcontracts alerts Boba. - 2022-02-03 2300: Boba pa...


Near line 77: Possible spelling mistake found.
Context: ... 2022-02-03 1945: smartcontracts alerts Boba. - 2022-02-03 2300: Boba patches mainne...


Near line 78: Possible spelling mistake found.
Context: ...ntracts alerts Boba. - 2022-02-03 2300: Boba patches mainnet. - 2022-02-03 2300: sma...


Near line 78: Possible spelling mistake found.
Context: ...s Boba. - 2022-02-03 2300: Boba patches mainnet. - 2022-02-03 2300: smartcontracts aler...


Near line 79: Possible spelling mistake found.
Context: ...oba patches mainnet. - 2022-02-03 2300: smartcontracts alerts Metis. They patched mainnet at s...


Near line 79: Possible spelling mistake found.
Context: ... 2022-02-03 2300: smartcontracts alerts Metis. They patched mainnet at sometime overn...


Near line 79: Possible spelling mistake found.
Context: ...artcontracts alerts Metis. They patched mainnet at sometime overnight. - 2022-02-04 161...


Near line 79: The adverb “sometime” means “at some point in the future”. Did you mean “some time” (which often means “a long span of time” as in the phrase “for some time” or “in some time”)?
Context: ...s alerts Metis. They patched mainnet at sometime overnight. - 2022-02-04 1617: smartcont...


Near line 80: Possible spelling mistake found.
Context: ... sometime overnight. - 2022-02-04 1617: smartcontracts opens [PR #2146](https://github.com/e...


Near line 83: Possible spelling mistake found.
Context: ...licly disclosing it. - 2022-02-06 0250: mslipper merges the finalized patch into PR #214...


Near line 90: Possible spelling mistake found.
Context: ... 3 lines long, it ensures that when the SELFDESTRUCT operation is called in an account, its...


Near line 91: Possible spelling mistake found.
Context: ...s called in an account, its balance (in OVM_ETH) is also immediately set to zero. ...


Near line 95: It appears that a comma is missing.
Context: ...t to zero. ## Lessons learned In this section we outline the lessons learned, and how...


Near line 99: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...ve occurred since the incident. ### On overly-optimistic code reviews This bug was (obviously) ...


Near line 101: Consider adding a comma after ‘Naturally’ for more clarity.
Context: ... not caught by our code review process. Naturally we want to understand why that is, by l...


Near line 110: Consider removing “of” to be more concise
Context: ...149 lines. The PR was well scoped, and all of the changes were relevant according to its ...


Near line 112: Possible spelling mistake found.
Context: ... description: > Refactors the usage of OVM_ETH so we can get most remaining integr...


Near line 112: A comma may be missing after the conjunctive/linking adverb ‘Also’.
Context: ...aining integration tests working again. Also > reworks vm.UsingOVM to be `rcfg.Usi...


Near line 113: Add a space between sentences.
Context: ...ion tests working again. Also > reworks vm.UsingOVM to be rcfg.UsingOVM where rcfg is ...


Near line 113: Add a space between sentences.
Context: ...ain. Also > reworks vm.UsingOVM to be rcfg.UsingOVM where rcfg is a new package within t...


Near line 113: Possible spelling mistake found.
Context: ...m.UsingOVMto bercfg.UsingOVMwherercfg` is a new package within the rollup > f...


Near line 113: Possible spelling mistake found.
Context: ...here rcfg is a new package within the rollup > folder. Was required in order to avoi...


Near line 114: This sentence seems to be incomplete. Insert a noun before ‘Was’ to make the sentence complete.
Context: ...new package within the rollup > folder. Was required in order to avoid an import cy...


Near line 114: Consider a shorter alternative to avoid wordiness.
Context: ...ithin the rollup > folder. Was required in order to avoid an import cycle. The PR was revi...


Near line 126: Possible spelling mistake found.
Context: ...hitectural update (which we refer as a 'regenesis') to Optimism. The regenesis removed th...


Near line 127: Possible spelling mistake found.
Context: ...efer as a 'regenesis') to Optimism. The regenesis removed the OVM contracts, and enabled ...


Near line 127: Possible spelling mistake found.
Context: ... to Optimism. The regenesis removed the OVM contracts, and enabled EVM equivalence....


Near line 130: Possible spelling mistake found.
Context: ... size of the update can be seen in the [regenesis 0.5.0 PR](https://github.com/ethereum-o...


Near line 137: Possible spelling mistake found.
Context: ...r/l2geth) codebase, which is a fork of [Geth](https://github.com/ethereum/go-ethereu...


Near line 137: Possible spelling mistake found.
Context: ...tps://github.com/ethereum/go-ethereum). Geth itself is already a very complex codeba...


Near line 138: Consider a shorter alternative to avoid wordiness.
Context: ...ebase. The changes introduced to L2Geth in order to support the OVM made it much more compl...


Near line 138: Possible spelling mistake found.
Context: ...duced to L2Geth in order to support the OVM made it much more complex, such that ve...


Near line 141: Possible spelling mistake found.
Context: ...w it worked. The changes made for this regenesis mostly removed this complexity, and mov...


Near line 142: Possible missing preposition found.
Context: ...and moved the behavior of L2Geth closer that of Geth. Unfortunately L2Geth had alrea...


Near line 142: Possible spelling mistake found.
Context: ...d the behavior of L2Geth closer that of Geth. Unfortunately L2Geth had already diver...


Near line 142: A comma might be missing here.
Context: ...behavior of L2Geth closer that of Geth. Unfortunately L2Geth had already diverged significant...


Near line 143: Possible spelling mistake found.
Context: ...nificantly, and the abstractions of the OVM leaked in enough that a change made in ...


Near line 146: Possible spelling mistake found.
Context: ...nces elsewhere. More specifically: the OVM used OVM_ETH, and ERC20 token rather ...


Near line 146: Possible spelling mistake found.
Context: ...where. More specifically: the OVM used OVM_ETH, and ERC20 token rather than nativ...


Near line 147: Possible spelling mistake found.
Context: ...lances were no longer kept in the state trie. However the EVM's SELFDESTRUCT works...


Near line 147: A comma may be missing after the conjunctive/linking adverb ‘However’.
Context: ... were no longer kept in the state trie. However the EVM's SELFDESTRUCT works by delet...


Near line 147: Possible spelling mistake found.
Context: ...pt in the state trie. However the EVM's SELFDESTRUCT works by deleting the balance in the t...


Near line 148: Possible spelling mistake found.
Context: ...Tworks by deleting the balance in the trie. In addition,SELFDESTRUCT` was not im...


Near line 148: Possible spelling mistake found.
Context: ...g the balance in the trie. In addition, SELFDESTRUCT was not implemented in the OVM, meanin...


Near line 148: Possible spelling mistake found.
Context: ...ELFDESTRUCT` was not implemented in the OVM, meaning it was not present to remind u...


Near line 166: Possible spelling mistake found.
Context: ...refer to balance and value transfer. 1. SELFDESTRUCT involves value transfer. 1. Does SELFDE...


Near line 167: Possible spelling mistake found.
Context: ...STRUCT involves value transfer. 1. Does SELFDESTRUCT behave the same way after the change? ...


Near line 171: Possible spelling mistake found.
Context: ...ave an audit on the changes made to the regenesis. The rationale for this was that: 1. t...


Near line 173: This sentence does not start with an uppercase letter.
Context: ...s. The rationale for this was that: 1. the changes were mostly deleting code and s...


Near line 173: Possible spelling mistake found.
Context: ... simplifying the system by removing the OVM, and 1. the availability of qualified a...


Near line 179: Possible spelling mistake found.
Context: ...vily modified from an upstream project (Geth) which very few people fully understand...


Near line 182: If the ‘because’ clause is essential to the meaning, do not use a comma before the clause.
Context: ...pe of changes. Perhaps most importantly, because the actual location of the bug was in a...


Near line 182: This phrase is redundant. Consider using “outside”.
Context: ...ctual location of the bug was in a file outside of the PR, it was not considered. This is ...


Near line 185: Consider a shorter alternative to avoid wordiness.
Context: ... simply by "reviewing more carefully". In order to catch an issue like this, we as reviewe...


Near line 195: Possible spelling mistake found.
Context: ...tic-specs)) will use a [fresh fork of Geth](https://github.com/ethereum-optimism/o...


Near line 196: Possible spelling mistake found.
Context: ...be easily rebased to track the upstream Geth repository. - We will ensure the common...


Near line 198: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...g our code review process, to introduce measure which will: 1. encourage authors to ...


Near line 208: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ake it a hard requirement not to deploy high risk code without an audit. ### Maximizing ...


Near line 212: Possible spelling mistake found.
Context: ...ng channels Our bounty program page on Immunefi did not list L2Geth as in scope, which ...


Near line 212: Possible spelling mistake found.
Context: ... not list L2Geth as in scope, which led saurik to report through our security@optimism...


Near line 213: ‘in the habit of’ might be wordy. Consider a shorter alternative.
Context: ...onally, not all members of the team are in the habit of checking email at the start of their da...


Near line 218: Possible spelling mistake found.
Context: ...* 1. We have extended the scope of the Immunefi program to include L2Geth. **Actions p...


Near line 223: The official name of this software platform is spelled with a capital “H”.
Context: ...s, including websites, chat forums, and github repos. 1. We will set up automated aler...


Near line 225: Possible spelling mistake found.
Context: ...ew who has access to both the email and Immunefi reporting channel, and ensure the gr...


Near line 228: A determiner may be missing.
Context: ...know. ### Adhering to the principle of least privilege Early in the process, the ex...


Near line 230: Did you mean the communication tool “Slack” (= proper noun, capitalized)?
Context: ... issue was openly discussed in a public slack channel, although the details of the vu...


Near line 232: A determiner may be missing.
Context: ...cribed. This violates the [principle of least privilege](https://en.wikipedia.org/wik...


Near line 238: Did you mean the communication tool “Slack” (= proper noun, capitalized)?
Context: ...licitly prescribes the use of a private slack channel, and the principle of least pri...


Near line 240: Possible spelling mistake found.
Context: ...in general. ### Communicating with the whitehat Communication with saurik was initiall...


Near line 242: Possible spelling mistake found.
Context: ...g with the whitehat Communication with saurik was initially done mostly in a direct m...


Near line 243: Possible spelling mistake found.
Context: ...ded communication overhead, and reduced saurik's ability to participate in the respons...


Near line 246: Possible spelling mistake found.
Context: ...en we received a review of the fix from saurik, who was able to suggest a better appro...


Near line 247: Possible spelling mistake found.
Context: ...gest a better approach. Consulting with saurik on the fix before implementing would ha...


Near line 249: Possible spelling mistake found.
Context: ...ing would have saved time. Keeping the whitehat better informed should also help to bui...


Near line 253: Possible spelling mistake found.
Context: ...establishing a private channel with the whitehat and the full response team, as well as ...


Near line 272: Possible spelling mistake found.
Context: ... we will adopt a process similar to the Geth team’s [silent patch policy](https://ge...


Near line 276: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ...tly notify a subset of downstream users prior to the public announcement. **Action take...


Near line 283: A comma may be missing after the conjunctive/linking adverb ‘However’.
Context: ...ility without it having been exploited. However this incident has also revealed that we...


Near line 284: The plural noun “criteria” cannot be used with the article “a”. Did you mean “a clear criterion” or “clear criteria”?
Context: ...t has also revealed that we do not have a clear criteria for deciding whether or not to disable ...


Near line 284: Consider shortening this phrase to just ‘whether’, unless you mean ‘regardless of whether’.
Context: ... not have a clear criteria for deciding whether or not to disable the sequencer, or pause smar...


Near line 286: Consider adding a comma after ‘Ultimately’ for more clarity.
Context: ...e sequencer, or pause smart contracts. Ultimately this will be a decision made in the mom...


Near line 287: Did you mean “at the moment” (=currently)?
Context: ...Ultimately this will be a decision made in the moment with the full available context. Althou...


Near line 288: A comma might be missing here.
Context: ...gh it is not possible to anticipate all scenarios we outline some basic criteria to infor...


Near line 294: Consider a shorter alternative to avoid wordiness.
Context: ... is ongoing: we should disable or pause in order to prevent further damage. - If we suspect...


Near line 302: The conjunction “so that” does not have a comma in front.
Context: ...and alerting checks we run on the system, so that we will be alerted to events such as th...

Additional comments not posted (1)
docs/postmortems/2022-02-02-inflation-vuln.md (1)

273-273: Update URL to correct resource.

Ensure the new URL is accessible and correctly points to the intended section on silent patches as described.


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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@protolambda protolambda added this pull request to the merge queue Apr 29, 2024
Merged via the queue into ethereum-optimism:develop with commit f72faa9 Apr 29, 2024
69 of 71 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants