Skip to content

fix(core): clean canonical metadata during rollback - #18

Open
Frozen wants to merge 1 commit into
mainfrom
fix/rollback-canonical-cleanup
Open

fix(core): clean canonical metadata during rollback#18
Frozen wants to merge 1 commit into
mainfrom
fix/rollback-canonical-cleanup

Conversation

@Frozen

@Frozen Frozen commented Aug 20, 2026

Copy link
Copy Markdown
Owner

Summary

  • delete the rolled-back canonical number mapping and any contiguous stale canonical suffix above it
  • purge HeaderChain's canonical cache after the batch is committed
  • preserve block bodies and state so a replacement branch can be imported by hash
  • add a regression test that warms both persistent and cached old-branch mappings, rolls back, and installs a replacement block at the same height

Root cause

BlockChainImpl.Rollback moved the current header/block/fast-block pointers but did not remove canonical number -> hash mappings or invalidate canonicalCache.

During the shard-0 recovery, the replacement branch became the current block chain, but number-based lookups continued to resolve headers from the abandoned branch. The first visible state divergence was block 92730047:

  • 92730047 % 64 == 63, so it is a reward-distribution boundary
  • reward processing reads the window 92729984..92730047 with GetHeaderByNumber
  • stale old-branch headers in that window produced a different payout and state root

The failed node computed 0x48704575...; the canonical block requires 0x834877ec....

Safety

  • cleanup runs only while chainmu is held and only during an explicit Rollback
  • deletion starts at the rolled-back height and stops at the first missing canonical mapping
  • canonical deletions are committed in one batch
  • bodies, headers, receipts, and state tries are not deleted

Verification

RED before the fix:

persistent canonical mapping survived rollback
persistent canonical mapping above rolled-back head survived

GREEN after the fix:

go test ./core ./core/rawdb -count=1
go test ./api/service/synchronize/legacysync ./cmd/harmony -count=1
go vet ./core

@greptile-apps

greptile-apps Bot commented Aug 20, 2026

Copy link
Copy Markdown

Greptile Summary

The PR makes rollback metadata updates atomic and removes stale canonical number mappings without deleting block data.

  • Computes replacement header, fast-block, and full-block heads before persisting changes.
  • Commits head pointers, validator-list changes, and canonical mapping deletions in one database batch.
  • Serializes canonical reads with rollback cleanup and purges the canonical cache after a successful commit.
  • Adds regression coverage for stale persistent/cache entries and failed batch writes.

Confidence Score: 5/5

The PR appears safe to merge with no actionable correctness or security issues identified.

Rollback now commits persistent head and canonical metadata atomically, retains in-memory heads on batch failure, and synchronizes canonical cache reads with cleanup without introducing a reachable lock cycle.

Important Files Changed

Filename Overview
core/blockchain_impl.go Refactors rollback to batch persistent metadata changes, remove stale canonical mappings, and publish in-memory state only after commit.
core/headerchain.go Adds read/write synchronization around canonical mapping cache access so rollback cleanup cannot race with number-based reads.
core/blockchain_impl_test.go Adds focused coverage for canonical mapping cleanup, cache invalidation, failed-batch atomicity, and canonical-read serialization.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Rollback requested] --> B[Resolve replacement heads]
    B --> C{Any head changed?}
    C -- No --> D[Return]
    C -- Yes --> E[Build database batch]
    E --> F[Write replacement head hashes]
    F --> G[Delete stale canonical suffix]
    G --> H{Batch commit succeeds?}
    H -- No --> I[Return error with in-memory heads unchanged]
    H -- Yes --> J[Purge canonical cache]
    J --> K[Publish replacement in-memory heads]
Loading

Reviews (1): Last reviewed commit: "fix(core): clean canonical metadata duri..." | Re-trigger Greptile

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.

1 participant