Skip to content

Fix history sync on testnet & Simplifying history sync artificial checkpoint blocks - #3479

Open
ii-cruz wants to merge 1 commit into
albatrossfrom
iicruz/history-sync-batch-set-gaps-fix
Open

Fix history sync on testnet & Simplifying history sync artificial checkpoint blocks#3479
ii-cruz wants to merge 1 commit into
albatrossfrom
iicruz/history-sync-batch-set-gaps-fix

Conversation

@ii-cruz

@ii-cruz ii-cruz commented Sep 10, 2025

Copy link
Copy Markdown
Member

What's in this pull request?

The extend history for syncing history nodes adds synthetic macro blocks if those are missing, so that it can recreate the FinalizeBatch inherents before pushing the history of an epoch. However, since we added the reward transactions to the body of the macro blocks, there is only one checkpoint block that can be empty - the first checkpoint block. This is because we pay rewards with a batch delay.
The pr #3758 did some hardening for the original assertion. However, it introduced a bug by rejecting the blocks when we have a gap of 2 batches. This shouldn't be the behaviour since we can have a valid code path for a gap, the case where we are in genesis block (batch 0) and batch 1 has no history, so the first item is on batch 2.

This PR:

  • Fixes the introduced bug in 3758
  • Removes and fixes extend_history_sync FIX MEs and outdated comments
  • Simplifies the addition of the synthetic missing checkpoint blocks to only do that when it's for checkpoint 1.
  • Adds unit tests for this edge cases

Pull request checklist

  • All tests pass. The project builds and runs.
  • I have resolved any merge conflicts.
  • I have resolved all clippy and rustfmt warnings.

@ii-cruz ii-cruz added this to the Next release milestone Sep 10, 2025
@ii-cruz ii-cruz self-assigned this Sep 10, 2025
@ii-cruz ii-cruz added the enhancement New feature or request label Sep 10, 2025
@ii-cruz
ii-cruz force-pushed the iicruz/history-sync-batch-set-gaps-fix branch 4 times, most recently from c0cf848 to e2217fe Compare September 10, 2025 16:15
@ii-cruz
ii-cruz marked this pull request as ready for review September 10, 2025 16:17
@ii-cruz
ii-cruz force-pushed the iicruz/history-sync-batch-set-gaps-fix branch from e2217fe to c24fe33 Compare September 10, 2025 16:24
@ii-cruz
ii-cruz force-pushed the iicruz/history-sync-batch-set-gaps-fix branch from 0a264aa to e063dcc Compare July 6, 2026 16:52
Copilot AI review requested due to automatic review settings July 6, 2026 16:52

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR simplifies extend_history_sync by tightening the “synthetic checkpoint insertion” logic to the first epoch scenario (where the first checkpoint macro block may have no history items due to delayed reward payouts). It also adds a regression test covering history sync for the first epoch.

Changes:

  • Simplify history-sync checkpoint insertion logic in Blockchain::push_history_sync / extend_history_sync.
  • Initialize prev_batch from the current chain head instead of assuming 0.
  • Add a new test that syncs the first epoch via (a) first checkpoint then election, and (b) directly to the election block.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
blockchain/src/blockchain/history_sync.rs Simplifies synthetic macro/checkpoint insertion while extending history during sync.
blockchain/tests/history_sync.rs Adds coverage for syncing the first epoch (checkpoint + election, and direct election).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread blockchain/src/blockchain/history_sync.rs
Comment thread blockchain/src/blockchain/history_sync.rs Outdated
@ii-cruz ii-cruz added the bug Something isn't working label Jul 6, 2026
@ii-cruz
ii-cruz force-pushed the iicruz/history-sync-batch-set-gaps-fix branch 6 times, most recently from f1efd54 to 5bd08a2 Compare July 6, 2026 20:22
@ii-cruz
ii-cruz requested review from jsdanielh and nibhar July 6, 2026 20:24
@ii-cruz
ii-cruz force-pushed the iicruz/history-sync-batch-set-gaps-fix branch 3 times, most recently from d23fff5 to 46c6fdd Compare July 7, 2026 15:31
@ii-cruz
ii-cruz marked this pull request as draft July 7, 2026 16:32
@ii-cruz
ii-cruz force-pushed the iicruz/history-sync-batch-set-gaps-fix branch from 46c6fdd to caece00 Compare July 7, 2026 19:44
@ii-cruz
ii-cruz marked this pull request as ready for review July 7, 2026 20:13
@ii-cruz ii-cruz changed the title Simplifying history sync artificial checkpoint blocks Fix history sync on testnet & Simplifying history sync artificial checkpoint blocks Jul 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants