Skip to content

Conversation

@joshieDo
Copy link
Collaborator

@joshieDo joshieDo commented Dec 23, 2025

During engine sync, we iterate over all stages in StageId::ALL and bump their checkpoints via update_pipeline_stages. During the pipeline consistency check, we iterate all checkpoints and compare them to the header stage.

However, the pipeline itself conditionally adds PruneSenderRecoveryStage only if sender pruning is enabled:

// If sender recovery prune mode is set, add the prune sender recovery stage.
.add_stage_opt(self.prune_modes.sender_recovery.map(|prune_mode| {
PruneSenderRecoveryStage::new(prune_mode, self.stages_config.prune.commit_threshold)
}))

This can result in the following weird scenario if sender pruning is not enabled:

  1. PruneSenderRecovery checkpoint is set to block 100 by engine sync (via update_pipeline_stages)
  2. Run unwind command to block 95. Since PruneSenderRecoveryStage is not in the pipeline, its checkpoint stays at 100 (ahead of other stages, but not an issue yet)
  3. Run pipeline forward to block 105. Once again, PruneSenderRecoveryStage is not in the pipeline and won't run, so its checkpoint stays at 100
  4. Now PruneSenderRecovery (100) < Headers (105), which triggers the inconsistency check:

or

  1. Engine sync to block 50 → update_pipeline_stages creates PruneSenderRecovery at 50
  2. Pipeline sync continues to block 100 → Headers at 100, but PruneSenderRecovery stuck at 50
  3. CLI unwind to block 60 → Headers at 60, PruneSenderRecovery still at 50
  4. reth node → consistency check: 50 < 60

if stage_checkpoint < first_stage_checkpoint {
debug!(
target: "consensus::engine",
first_stage_id = %first_stage,
first_stage_checkpoint,
inconsistent_stage_id = %stage_id,
inconsistent_stage_checkpoint = stage_checkpoint,
"Pipeline sync progress is inconsistent"
);
return self.blockchain_db().block_hash(first_stage_checkpoint);
}

@joshieDo joshieDo self-assigned this Dec 23, 2025
@joshieDo joshieDo added C-bug An unexpected or incorrect behavior A-staged-sync Related to staged sync (pipelines and stages) labels Dec 23, 2025
@joshieDo joshieDo added the A-engine Related to the engine implementation label Dec 23, 2025
@github-project-automation github-project-automation bot moved this to Backlog in Reth Tracker Dec 23, 2025
Copy link
Collaborator

@mediocregopher mediocregopher left a comment

Choose a reason for hiding this comment

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

Makes sense to me 👍

@github-project-automation github-project-automation bot moved this from Backlog to In Progress in Reth Tracker Dec 23, 2025
Comment on lines +1568 to +1569
// TODO: era_enabled should come from node config
let era_enabled = true;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

was not sure how to bring era_enabled here

hardcoding it to truth here, results in the same behaviour as before. And should be fine since we filter it out anyway in the pipeline consistency check.

@joshieDo joshieDo changed the title fix(stages): iterate over active stages only fix(stages): only iterate over active stages Dec 23, 2025
@joshieDo joshieDo marked this pull request as ready for review December 23, 2025 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-engine Related to the engine implementation A-staged-sync Related to staged sync (pipelines and stages) C-bug An unexpected or incorrect behavior

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

3 participants