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

fix(storage): dont skip consistency checks for op-mainnet if using minimal bootstrap #11099

Merged
merged 9 commits into from
Sep 24, 2024

Conversation

joshieDo
Copy link
Collaborator

@joshieDo joshieDo commented Sep 21, 2024

reverting #9737

Storage consistency ensures that:

  • The number of receipts/headers/transactions that the DB expects to exist are the same number as existing on static files.
  • Transaction and block ranges in static files match the number of rows.

Currently, OVM import of receipts seems to be broken (#10580, #9739) (#9725).

Any node that synced to the tip after #9737 may be inconsistent (from a historical storage pov), and disabling this skip entirely would break them. Static files are not a database, so a migration may be possible, but more complex.

For now, leaving this check disabled for nodes which imported the ovm historical data (use minimal bootstrap instead). But it's important to understand that a sudden crash of the node MAY break it.

@joshieDo joshieDo added the A-op-reth Related to Optimism and op-reth label Sep 21, 2024
@joshieDo joshieDo changed the title fix: dont skip consistency checks for op-mainnet fix(storage): dont skip consistency checks for op-mainnet Sep 21, 2024
Copy link
Member

@emhane emhane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will fail for ovm chain

@emhane
Copy link
Member

emhane commented Sep 22, 2024

blocked by #9739

@emhane emhane added the S-blocked This cannot more forward until something else changes label Sep 22, 2024
@joshieDo joshieDo removed the S-blocked This cannot more forward until something else changes label Sep 22, 2024
@joshieDo joshieDo changed the title fix(storage): dont skip consistency checks for op-mainnet fix(storage): dont skip consistency checks for op-mainnet if using minimal bootstrap Sep 22, 2024
if provider.chain_spec().chain() == Chain::optimism_mainnet() &&
provider
.block_number(b256!(
"bee7192e575af30420cae0c7776304ac196077ee72b048970549e4f08e875453"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please define as constant in reth_optimism_primitives::bedrock

@joshieDo joshieDo marked this pull request as draft September 22, 2024 14:38
@joshieDo joshieDo marked this pull request as ready for review September 22, 2024 14:38
//
// If we detect an OVM import was done (block #1 <https://optimistic.etherscan.io/block/1>), skip it.
// More on [#11099](https://github.com/paradigmxyz/reth/pull/11099).
#[cfg(feature = "optimism")]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can keep this cfg, but not strictly required because not expensive to check and only reachable by op mainnet

@joshieDo joshieDo added this pull request to the merge queue Sep 24, 2024
Merged via the queue into main with commit 4da5f1f Sep 24, 2024
36 checks passed
@joshieDo joshieDo deleted the joshie/consi-op branch September 24, 2024 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-op-reth Related to Optimism and op-reth
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants