Fix reversible-data cleanup destroying shared-fork blocks before they become irreversible (#333)

Eric Friasrequested to merge
haf333-fix-obsolete-reversible-cleanup into master

Fixes #333 (closed) — five mainnet HAF nodes (1.28.5 and 1.28.6) died on 2026-07-09/10 with fk_1_hive_irreversible_data violations in hive.set_irreversible(), and crash-loop on restart because the failing call is persisted in hived's HAF WAL.

Root cause

Every DELETE in hive.remove_obsolete_reversible_data() treated any row from an older fork_id as obsolete:

WHERE hbr.num <= __max_block_num OR hbr.fork_id < LEAST( __min_ctx_fork_id, __max_fork_id )

But rows at or below a fork's common block are shared by both branches and exist only under the old fork_id — the post-fork re-dump starts above the common block. If a fork arrives while consistent_block lags behind the fork's common block, the next set_irreversible(N) call (N below the fork point) copies up to N and then deletes the only copies of blocks in (N, common_block]. The following set_irreversible(N+1) finds nothing to copy and fails the FK. hived shuts down; on restart replay_wal_if_necessary re-executes the poisoned call and aborts, forever.

Under OBI the lag window is normally empty (LIB ≈ head−1), which is why this survived so long; a peer-flap-induced LIB stall — or even a routine 1-block lag, as one incident showed — opens it. See #333 (closed) for the full forensic evidence from five affected nodes.

Fix

An old-fork row is obsolete only if some later fork event actually superseded it, i.e. a fork with a higher id has its common block below the row's height (meaning the row was popped). Each DELETE now uses:

OR ( fork_id < LEAST( __min_ctx_fork_id, __max_fork_id )
     AND block_num > ( SELECT MIN(hf2.block_num) FROM hafd.fork hf2 WHERE hf2.id > fork_id ) )

hafd.fork is tiny (tens of rows over months), so the correlated subquery is cheap.

Regression test

set_irreversible_two_steps_over_fork_common_block_test.sql reproduces the incident shape: blocks 2–4 on fork 1, fork switch with common block 3 (only block 4 popped), then stepwise set_irreversible(2); set_irreversible(3); set_irreversible(4). The two-step sequence is essential — all existing tests call set_irreversible once, whose copy runs before its own cleanup, which is why this was never caught. Under the old code the test fails with the exact production FK violation; with the fix it passes and verifies blocks 2/3/4 land in hafd.blocks with the right hashes.

Also included

  • hive submodule bumped to current master (16a419df6), which includes hive!2064 (merged) — the p2p fix removing the punitive mass-disconnects that caused the LIB stalls exposing this bug. The two fixes complement each other but are independently correct.

Notes for reviewers

  • The __min_ctx_fork_id context protection is preserved unchanged; the new condition only narrows what qualifies as obsolete.
  • Same flawed predicate exists on develop — will cherry-pick after this merges.
  • Separate follow-up needed: a blessed repair procedure for already-stuck nodes (re-inserting the destroyed gap rows so the WAL replay can complete). We have two expendable stuck stacks available for validating it, plus ZFS snapshots of a third.

Merge request reports