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

chore(primitives): remove static file stage #7116

Merged
merged 4 commits into from
Mar 12, 2024

Conversation

shekhirin
Copy link
Collaborator

@shekhirin shekhirin commented Mar 12, 2024

Cleanup after #6763

This PR does not remove the StageId::StaticFile variant, as it's unclear whether it's a breaking database change.

check_pipeline_consistency has a strong assumption that the first stage of the pipeline is Headers

/// Check if the pipeline is consistent (all stages have the checkpoint block numbers no less
/// than the checkpoint of the first stage).
///
/// This will return the pipeline target if:
/// * the pipeline was interrupted during its previous run
/// * a new stage was added
/// * stage data was dropped manually through `reth stage drop ...`
///
/// # Returns
///
/// A target block hash if the pipeline is inconsistent, otherwise `None`.
fn check_pipeline_consistency(&self) -> RethResult<Option<B256>> {
// If no target was provided, check if the stages are congruent - check if the
// checkpoint of the last stage matches the checkpoint of the first.
let first_stage_checkpoint = self
.blockchain
.get_stage_checkpoint(*StageId::ALL.first().unwrap())?
.unwrap_or_default()
.block_number;
// Skip the first stage as we've already retrieved it and comparing all other checkpoints
// against it.
for stage_id in StageId::ALL.iter().skip(1) {
let stage_checkpoint =
self.blockchain.get_stage_checkpoint(*stage_id)?.unwrap_or_default().block_number;
// If the checkpoint of any stage is less than the checkpoint of the first stage,
// retrieve and return the block hash of the latest header and use it as the target.
if stage_checkpoint < first_stage_checkpoint {
debug!(
target: "consensus::engine",
first_stage_checkpoint,
inconsistent_stage_id = %stage_id,
inconsistent_stage_checkpoint = stage_checkpoint,
"Pipeline sync progress is inconsistent"
);
return Ok(self.blockchain.block_hash(first_stage_checkpoint)?)
}
}
Ok(None)
}

But because we had StaticFile as the first stage without an actual stage implementation, its checkpoint was always zero, so the consistency check passed, and the node went to live sync instead of continuing the pipeline sync.

@shekhirin shekhirin added C-enhancement New feature or request A-staged-sync Related to staged sync (pipelines and stages) labels Mar 12, 2024
@shekhirin shekhirin requested a review from gakonst as a code owner March 12, 2024 15:45
@shekhirin shekhirin added C-debt Refactor of code section that is hard to understand or maintain and removed C-enhancement New feature or request labels Mar 12, 2024
@shekhirin shekhirin changed the title feat(primitives): remove static file stage chore(primitives): remove static file stage Mar 12, 2024
@shekhirin shekhirin disabled auto-merge March 12, 2024 15:52
@shekhirin shekhirin force-pushed the alexey/remove-static-files-stage branch 2 times, most recently from fe7516a to 89dd2c1 Compare March 12, 2024 15:59
@shekhirin shekhirin force-pushed the alexey/remove-static-files-stage branch from 89dd2c1 to 6801f5b Compare March 12, 2024 15:59
@shekhirin shekhirin added this pull request to the merge queue Mar 12, 2024
Merged via the queue into main with commit c15f2d5 Mar 12, 2024
27 checks passed
@shekhirin shekhirin deleted the alexey/remove-static-files-stage branch March 12, 2024 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-staged-sync Related to staged sync (pipelines and stages) C-debt Refactor of code section that is hard to understand or maintain
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants