Skip to content

feat(api): harden HAF indexer replay safety - #31

Open
KareemN1936 wants to merge 1 commit into
developfrom
feat/trello-32-indexer-reorg-replay
Open

KareemN1936 wants to merge 1 commit into
developfrom
feat/trello-32-indexer-reorg-replay

Conversation

@KareemN1936

Copy link
Copy Markdown
Collaborator

Summary

  • Persist Hive block hash metadata on indexed events and the HAF indexer watermark.
  • Add fork detection that rewinds rebuildable HiveEvent rows from the divergent block and replays canonical HAF rows.
  • Add an explicit indexer:replay -- <from-block> path plus documentation for rebuild bounds.
  • Expand indexer tests for hash checkpoints, idempotent crash replay, reorg convergence, and explicit replay.

Trello

Verification

  • npm test --workspace=@hivelore/api (217 passed, 2 skipped because TEST_DATABASE_ADMIN_URL is not configured locally)
  • npm run typecheck --workspace=@hivelore/api
  • npm run lint --workspace=@hivelore/api
  • npm run build --workspace=@hivelore/api
  • npm run db:validate
  • git diff --check

Note: avoid running API commands that invoke prisma generate in parallel locally; Prisma can collide while replacing src/generated/prisma on Windows.

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown

Vercel preview is ready:

https://hivelore-cegmk9g0h-kareemn1936s-projects.vercel.app

Commit: 9e41579

@mcfarhat mcfarhat left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Requesting changes on one defect. I want to lead with what's right, because most of this is.

The hard part — idempotent replay — you got correct. I verified rather than assumed it: the upsert is keyed on (transactionId, operationIndex), and the replay boundary is genuinely inclusive — replayFromBlock seeds the watermark with lastProcessedOperationIndex: -1, so shouldProcessRow lets operation index 0 through and the rewound block really is re-projected rather than skipped. Your crash-replay test is testing the thing it claims to test.

The rewind is also complete, which I checked separately because it's the obvious way this design goes wrong. projectHiveOperation writes only HiveEvent; I grepped for anything else materialising from it and there's nothing outside runner.ts. So deleting HiveEvent from the fork point really does undo the projection — there's no derived table left holding stale counts.

And you fixed a latent bug nobody asked you to. On develop, normalizeHafOperation resolved the operation index via operation_id ?? operationIndex ?? op_in_trx ?? op_pos, but shouldProcessRow used operation_id ?? operationIndex ?? op_pos. Any row carrying op_in_trx but not op_pos would have had the watermark and the stored index disagree. You aligned them in this PR and didn't mention it. Worth mentioning next time — that's a real find.

The migration is additive and safe, and the README is honest about rebuild bounds instead of implying an SLO you can't hold.


Blocking

findForkBlock treats stream-adjacency as chain-adjacency

haf-sync.ts:342 compares a row's previous_block_hash against the checkpoint hash without first checking that the row is the block after the checkpoint:

if (previousBlockHash && previousBlockHash !== watermark.lastProcessedBlockHash) {
  return blockNumber - 1;
}

The operation stream is filtered to HiveLore operations, so the next row is very often not the next block. When the checkpoint is at block 100 and the next relevant operation is at block 104, that row's parent hash is block 103's — which of course doesn't match block 100's. A healthy chain reads as a fork.

That alone would only cost a wasted DELETE. What makes it blocking is the multi-page case. Checkpoint at 100, page 1 carries block 101, page 2 carries block 102 — contiguous blocks, healthy chain, no reorg anywhere:

PROBE E -> projected = 2 | rolledBack = 1 | surviving = block-102-op:1

Page 1 projects block 101. Page 2 computes a fork at 101 and deletes it. The event is gone and is never re-projected, because the watermark has already advanced past it — no ordinary run will re-fetch that block. Recovery needs a manual --from-block replay, and you'd have to already know which blocks you lost.

Note the run reports projected = 2 while one row survives. It logs success.

Two things are compounding here. The first is the missing contiguity check. The second is at haf-sync.ts:138: watermark is read once per run and never advanced as rows are projected — only the database row moves, via saveWatermark. So every page after the first compares against the checkpoint the run started with, which is what puts page 2 a block behind reality.

There's a second-order effect worth knowing about: when the spurious rewind fires it sets lastProcessedBlockHash to null, and findForkBlock returns early on a null hash. So after one false positive, fork detection is off for the rest of that run — a genuine reorg arriving on a later page produces no rollback at all. Two bugs partly masking each other, which is why this isn't obvious from reading it.

Why the suite is green

row() in haf-sync.test.ts hardcodes previousBlockHash: \block-${input.block - 1}-a``, and every scenario uses consecutive or identical block numbers. There's no test where the stream skips a block, so the one assumption the fork logic rests on is never exercised.


What I'd like to see

  1. Guard the parent-hash comparison so it only applies to the block immediately after the checkpoint — blockNumber === lastProcessedBlock + 1. I tried it locally: PROBE E goes to rolledBack = 0 with both events surviving, and all four of your tests still pass.
  2. Advance the in-memory watermark as you project, so pages after the first compare against current state. The guard is still needed for real gaps, but this removes the underlying class of bug rather than the symptom.
  3. A test with a gap in it — checkpoint at 100, next operation at 104, healthy chain, assert no rewind is issued. And one with contiguous blocks split across two pages, asserting the page-1 event survives. Both fail on the current code.

Not blocking

  • deleteMany is typed Promise<{ count: number } | unknown> with a runtime shape check at haf-sync.ts:298. Prisma always returns { count }; the defensive branch reads as uncertainty about an API that is actually stable. Type it and drop the check.
  • Even with the guard, a reorg that arrives mid-run on a later page won't be rewound in that run. The next run converges, so I'd leave it — but it's worth a comment so the next reader doesn't think it's covered.

The design is sound and the replay semantics are right. It's one conceptual slip: the next block in the stream and the next block on the chain are only the same thing when nothing in between was filtered out.

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.

2 participants