Reject abandoned chain anchor hashes - #13
Conversation
Greptile SummaryThis PR implements a prevention-focused emergency recovery boundary that rejects exact abandoned-chain anchors and validates recovered shard-0 state before networked startup.
Confidence Score: 5/5The PR appears safe to merge because no blocking failure remains after the checkpoint and validator-manifest startup issues were fixed and the persisted-head limitation was explicitly accepted with an offline recovery requirement. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| consensus/engine/rejected_block.go | Defines exact abandoned-anchor hashes and a shared fail-closed hash validator used by the recovery checks. |
| consensus/recovery_checkpoint.go | Pins and validates the retained checkpoint tuple, canonical ancestry, persisted heads, state availability, and commit certificate. |
| consensus/recovery_validator_list.go | Supplies the release validator-list count and digest and validates the exact ordered persisted list. |
| cmd/harmony/main.go | Adds verified offline rollback and repair, networked startup validation, recovery isolation, and safe consensus initialization. |
| core/recovery_freeze.go | Centralizes the post-boundary recovery feature policy used to suppress unsafe payloads and metadata advancement. |
| core/rejected_block.go | Enforces exact-hash and embedded-reference rejection in core block-processing paths. |
| internal/params/emergency_recovery.go | Defines the retained recovery block and ViewID floor used across recovery enforcement. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Start Harmony node] --> B{Offline maintenance?}
B -->|Yes| C[Open database for verified rollback]
C --> D[Verify retained checkpoint state and certificate]
D --> E[Repair validator list and roll back canonical head]
E --> F[Close database successfully]
B -->|No| G{Mainnet shard 0?}
G -->|Yes| H[Force network isolation]
H --> I[Validate checkpoint ancestry, persisted heads, certificate, and validator manifest]
I --> J[Initialize recovery ViewID floor and deterministic leader]
G -->|No| J
J --> K[Process consensus and block imports]
K --> L{Rejected exact hash or embedded reference?}
L -->|Yes| M[Reject before caches or metadata writes]
L -->|No| N[Continue normal validation]
N --> O{Recovery feature freeze active?}
O -->|Yes| P[Suppress frozen payloads and crosslink rollup]
O -->|No| Q[Normal processing]
Reviews (12): Last reviewed commit: "fix: make recovery rollback restartable" | Re-trigger Greptile
|
Shard-aware rollback is still required before this recovery binary can be used for both shards. The node already opens its local shard chain through
At the moment, The fix should use one fail-closed recovery manifest selected by Release-material status currently visible in this PR:
Until those values are supplied and the target selection is made shard-aware, shard 0 and shard 1 must not be documented as equally supported by the recovery binary. |
Frozen
left a comment
There was a problem hiding this comment.
Blocking recovery issues found. This PR is authored by the current GitHub account, so this is submitted as a COMMENT review rather than REQUEST_CHANGES; treat the inline findings as release blockers.
|
Confirmed retained state root for the shard-0 recovery checkpoint. The value to use for block
This exact |
|
Release constants are now pinned on PR head Shard-0 retained checkpoint:
Validator-list manifest at that exact retained state:
The manifest was independently reproduced from two read-only shard-0 database copies. Both copies matched the pinned block hash/state root and produced The prior state-root commit passed all PR checks. The new manifest head is awaiting its own CI run. The staged-sync stale-body shortcut and rollback no-progress loop remain separate release blockers and are not claimed fixed by these constant updates. |
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
|
Update: fixed the recovery rollback failure mode.
The staged-sync changes were removed from this PR. Recovery will start from the prepared snapshot with sync disabled, so Regression coverage includes incomplete ancestry, no-progress rollback, and successful multi-block rollback. [Test] All three packages pass at the rewritten PR head. |
38d9ca0 to
d53ebc6
Compare
|
Follow-up high-assurance review found two additional rollback durability gaps. They are fixed in
Added regressions cover wrong-target ancestry, missing ancestry, rollback no-progress, certificate-write failure before head movement, successful exact-target rollback, and stale persisted-head keys. Fresh local validation on this exact commit:
Repository-wide native verification also completed successfully on this exact commit: The staged-sync stale-body issue remains intentionally outside the currently isolated shard-0 rollout and is still a blocker before staged sync is re-enabled on an unpurged database. |
Summary
92,730,03494,978,278EmergencyRecoveryViewIDFloorto1,000,000,00092,730,035) by exact hash:0x5de06979a333f20afb8b245a8cf44472dc5bfc7383a57ddee48e1809bcee7c5d94,978,279) by exact hash:0xc936581d391b74a620bf6636519834b14a9a2d4e9a5154867c8407f219d8a878CommitOffChainDatalatest-crosslink rollup on post-boundary mainnet shard 0 blocks so abandoned crosslink records cannot advance local last-continuous markers during recoveryRejecting an abandoned branch anchor prevents its descendants from attaching through normal parent validation. The embedded-reference checks also prevent a valid outer block from reintroducing crosslink or CX metadata tied to a rejected source block.
The off-chain rollup freeze prevents empty recovery blocks from repeatedly scanning stale crosslink records and advancing the local last-crosslink pointer, which could otherwise emit stale heartbeats and impair crosslink liveness when the feature is re-enabled.
This PR prevents new acceptance on a clean or recovered database. It intentionally does not repair a database that already has a rejected anchor or descendant as its persisted head. Deployment over affected data still requires offline rollback/purge and verification of the retained head before starting network services.
[Test]
Previous CI on commit
458591a38passed:consensus,consensus/engine, andinternal/paramscorein both unit-test jobsFor commit
e1d43c235:gofmtgit diff --checkTestEmergencyRecoverySkipsLatestCrossLinkRollupcovering post-boundary mainnet freeze, retained mainnet behavior, testnet behavior, and non-beacon behaviorgo test ./core -run '^TestEmergencyRecoverySkipsLatestCrossLinkRollup$' -count=1is blocked by missing host BLS headers; CI runs with the repository BLS setupThe previous overall unit-test jobs failed later in the unrelated
core/state/snapshottestTestDiffLayerExternalInvalidationPartialFlatten. Localmake testcannot reach repository tests because the legacyharmonyone/localnet-test:latestrunner requires a TTY, ships Go 1.16 while the repository requires Go 1.24, and cannot resolve the host worktree.gitpointer inside the container.