feat(indexer): raw block archive in object storage

[baktun14]

Why

Closes CON-808

Indexer v2 stores only typed data in Postgres, and that is only safe if raw blocks can be replayed without hammering RPC nodes: handler fixes, new decode modules, and derived-table rebuilds all need a cheap replay path. This adds the raw block archive from the design doc (L-2): zstd-compressed raw /block and /block_results JSON in GCS, written during live sync and read back by backfill. It also unblocks per-module replay (L-12) and the 8h mainnet backfill target (L-11).

What

• ARCHIVE_BUCKET (optional) turns the archive on. Unset, both roles behave exactly as today and the boot log says ARCHIVE_DISABLED.
• Live sync stages one object per block before the DB commit, so no block is ever committed without being archived. If GCS stays down, sync retries through the existing wrapper and then halts instead of committing unarchived blocks.
• Backfill reads each height archive-first (1,000-block chunk, then staged single, then RPC) and compacts any fully covered aligned range into a chunk as a side effect, deleting the staged singles it consumed. There is no compactor process and no new table: keys are deterministic and the bucket is the source of truth for coverage.
• Writes use ifGenerationMatch: 0, so re-running a writer over an already-archived range rewrites nothing (a 412 counts as success).
• Keys are namespaced by the chain id from /status (e.g. sandbox-2/...) so sandbox resets cannot mix archives. Compression is Node 24's built-in zstd. The only new dependency is @google-cloud/storage 7.21.0, the newest version that clears the repo's min-release-age guard.
• ARCHIVE_STORAGE_API_ENDPOINT points the client at an emulator for local verification. The SDK's own STORAGE_EMULATOR_HOST looks like it works but silently breaks reads against fake-gcs-server (the client drops the /storage/v1 path prefix, every download 404s, and the archive falls back to RPC). Found while driving the flow end-to-end, which is also why the override exists.

Verified end-to-end against sandbox RPC, fake-gcs-server, and a scratch Postgres:

• sync staged 20 of 20 committed blocks with the full raw payloads, including fields outside our narrow TS projections (AC1)
• a 2,000-block RPC-fed backfill compacted two chunks (AC3); replaying the same range from the archive made zero /block or /block_results calls, counted through a logging proxy, and ran in 2.2s vs 80s RPC-fed (AC2)
• the replay left chunk generations byte-identical (AC4)
• a backfill killed mid-chunk resumed from its checkpoint; the interrupted range fell back to staged singles (600 of them, exactly the resumed heights) and the next range chunked normally
• with ARCHIVE_BUCKET unset, both roles ran normally and wrote nothing to the bucket

Accepted tradeoffs from the plan, flagged for review:

• blocks are archived before the continuity check (deliberate, so raw blocks survive decoder bugs), which means a poisoned RPC node could pin a bad block into the immutable archive for a height that sync then refuses to commit; a replay of that range trips the same continuity check, so the divergence is detectable but needs a manual object delete to heal (documented in the README)

• sync couples ingestion to GCS availability (the halt policy is deliberate), and the staged PUT adds roughly 50ms per block during sync catch-up; use the backfill role for large catch-ups

• staged singles accumulate between replays (about 14k/day), and heights 1-999 stay staged forever because chunk [0..999] can never fill (heights start at 1)

• a chunk-eligible range interrupted mid-run loses its in-memory buffer; re-fetched blocks land as staged singles until a later full-range replay heals them into a chunk

• chunk compression blocks the event loop for 0.2-0.5s once per 1,000 blocks during replays; a promisified variant is a drop-in if that ever matters

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (1)

• main

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 68136cd6-3b6c-4f65-b365-084769fbe2c8

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

_{Comment @coderabbitai help to get the list of available commands.}

[socket-security]

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff	Package	Supply Chain Security	Vulnerability	Quality	Maintenance	License
	@​google-cloud/​storage@​7.21.0

View full report

[claude]

Additional findings (outside current diff — PR may have been updated during review):

• 🟡 apps/chain-indexer/src/pipeline/sync-runner.service.ts:115-121 — Both SyncRunnerService#logArchiveState and BackfillRunnerService#logArchiveState (apps/chain-indexer/src/pipeline/sync-runner.service.ts:115-121, apps/chain-indexer/src/pipeline/backfill-runner.service.ts:111-117) are byte-identical private methods that log ARCHIVE_ENABLED/ARCHIVE_DISABLED based on archive.isEnabled(). Consider hoisting this into a shared helper or a method on BlockArchiveService (which already owns isEnabled() and the bucket config) so a future change to the log shape doesn'''t need to be made in two places.

  Extended reasoning...

  Both SyncRunnerService and BackfillRunnerService define a private #logArchiveState() method that is byte-for-byte identical:

  #logArchiveState(): void {
    if (this.#archive.isEnabled()) {
      this.#logger.info({ event: "ARCHIVE_ENABLED", bucket: this.#config.ARCHIVE_BUCKET });
    } else {
      this.#logger.info({ event: "ARCHIVE_DISABLED" });
    }
  }

  Both are net-new in this PR (part of the raw block archive feature), and both are called once at startup — sync-runner.service.ts from #run() right after SYNC_STARTED, and backfill-runner.service.ts from #run() right after BACKFILL_STARTED.

  Why this isn'''t prevented by existing code: each runner is an independent class with its own private fields (#archive, #config, #logger), constructed via its own tsyringe registration. Nothing forces the two to share behavior, so the same log-shape logic was written twice by hand.

  Impact: this is a maintenance/consistency risk, not a functional bug today. If the log shape ever needs to change — e.g. adding a chainId field once it'''s resolved, or renaming the bucket field — a future contributor has to remember to update both files. Missing one produces silent drift: one runner logs the new shape, the other still logs the old one, and nothing in CI catches it because both are independently valid, compiling, and tested.

  Step-by-step proof of the duplication and its drift risk:

  1. Open apps/chain-indexer/src/pipeline/sync-runner.service.ts:115-121 — #logArchiveState checks this.#archive.isEnabled() and logs {event: "ARCHIVE_ENABLED", bucket: this.#config.ARCHIVE_BUCKET} or {event: "ARCHIVE_DISABLED"}.
  2. Open apps/chain-indexer/src/pipeline/backfill-runner.service.ts:111-117 — the method body is character-for-character the same.
  3. Suppose a future PR wants to also log the resolved chain id (available via BlockArchiveService) alongside ARCHIVE_ENABLED. A contributor editing sync-runner.service.ts has no compiler or test signal pointing them at the matching method in backfill-runner.service.ts; if they miss it, sync and backfill silently report different log shapes for the same underlying feature.

  Suggested fix: move the branching onto BlockArchiveService itself, e.g. a logState(logger: LoggerService): void method (it already exposes isEnabled() and could read ARCHIVE_BUCKET from its own injected config), and have both runners call this.#archive.logState(this.#logger) instead of maintaining their own copies. Alternatively, extract a small shared helper function that both call directly. This is a pure code-quality/reuse issue — behavior is unchanged either way — so it does not block merging.

[stalniy]

question: can't this be implemented with functional tests?

[baktun14]

This drives the real indexer binary against a live sandbox RPC, a scratch Postgres, and a fake-gcs emulator, so it is a manual end-to-end recipe rather than something functional tests fully replace. Carving out functional coverage for the archive paths is a worthwhile follow-up; keeping the skill here for now, happy to discuss whether some of it should move into functional tests.

[claude]

Beyond the inline findings, I also checked whether the backfill's already-complete checkpoint short-circuit (backfill-runner.service.ts) undermines the README's documented archive-healing replay — it doesn't: the healing scenarios described (a backfill killed mid-chunk, a poisoned block needing a manual re-archive) all leave the checkpoint short of toHeight, so a resume goes through #backfillRange/ArchiveBlockSource normally rather than hitting the already-complete short-circuit. That short-circuit only skips work when you rerun an already-fully-completed range unchanged, which is outside what the README promises to heal.

Extended reasoning...

Checked planBackfill (backfill-planner.ts) and its use in BackfillRunnerService#run: already-complete is only returned when checkpointHeight >= toHeight. Every healing scenario the PR documents (mid-chunk kill leaving staged singles, a bad block pinned into the archive before a manual delete) implies the checkpoint stopped short of the range end, so the next run's plan is run, not already-complete, and normally re-walks the range through the new ArchiveBlockSource. The short-circuit only matters for the separate case of deliberately rerunning a range that already fully committed, which the README never claims heals the archive.

[claude]

🟡 Even after the recent fix for a throwing node_info-less /status (e3386fc), resolveChainId still permanently caches a bad chain id if /status resolves with node_info.network present but empty/undefined — since that never throws, .catch never fires, and every subsequent chunk/staged-block key gets built with an undefined/empty prefix for the process's lifetime. Pre-existing narrow edge case since this method's original commit; suggest validating that network is a non-empty string and throwing otherwise.

Extended reasoning...

BlockArchiveService#resolveChainId (block-archive.service.ts:139-148) caches the chain id as:

this.#chainId ??= this.#pool
  .getStatus()
  .then(status => status.node_info.network)
  .catch(error => {
    this.#chainId = null;
    throw error;
  });

This is the fixed form from commit e3386fc, which correctly routes a throw inside the fulfillment handler (e.g. node_info missing entirely) through .catch() so the cache resets and the next call re-fetches. But that fix only helps when the access actually throws. If /status resolves with node_info present but network absent (node_info: {}) or an empty string (node_info: { network: "" }), status.node_info.network evaluates to undefined or "" without throwing at all. The derived promise resolves successfully with that bad value, .catch() never runs, and this.#chainId permanently caches a promise resolving to undefined/"" — for the entire process lifetime, since nothing ever rejects to trigger the ??= re-fetch.

RpcClientPool#getStatus() does zero runtime validation of the response shape (a bare cast in rpc-client-pool.service.ts), so nothing upstream would catch a proxy or non-CometBFT-compliant node returning a node_info object without a populated network field.

Once poisoned, every subsequent chunkKey/stagedBlockKey call builds keys like undefined/chunks/... or /chunks/... (leading slash when empty), silently defeating the chain-id namespacing the README explicitly promises ("namespaced by the chain id ... so a sandbox chain reset starts a fresh namespace instead of mixing archives") — with no error, warning, or log anywhere, and no self-healing path since nothing ever rejects.

Step-by-step proof:

1. #resolveChainId() is called for the first time; this.#chainId is null, so the .then().catch() chain above runs.
2. getStatus() resolves with { node_info: {}, sync_info: {...} } (a proxy or non-standard RPC that keeps the node_info key but omits network).
3. status.node_info.network evaluates to undefined — no throw. The derived promise resolves with undefined.
4. .catch() never runs since nothing rejected; this.#chainId now holds a permanently-resolved promise whose value is undefined.
5. Every future getChunk/putChunkIfAbsent/putStagedBlockIfAbsent/deleteStagedBlocks call awaits this same cached promise and builds keys under the literal undefined/ prefix, forever — even after the RPC node starts returning well-formed /status responses again. Recovery requires a full process restart.

This is a distinct gap from the already-fixed throwing case: the existing dedicated poison test (block-archive.service.spec.ts, "refetches the chain id after a malformed status response poisons the first call") only covers a /status body missing node_info entirely, which throws and is caught by .catch(). It does not cover node_info: { network: "" } or node_info: {}, which resolve cleanly and slip past the fix.

Fix: validate that network is a non-empty string inside the .then() handler and throw if not, e.g. .then(status => { const network = status.node_info.network; if (!network) throw new Error("status.node_info.network is missing"); return network; }), so the existing .catch() reset also covers this case.

Impact is bounded: within a single process, reads and writes consistently use the same bad prefix, so the archive stays internally self-consistent — the failure mode is losing the documented chain-reset isolation guarantee, not data corruption or a crash. Triggering it requires a non-compliant /status response (a proxy stripping the field, or a non-CometBFT node), which is why this is a narrow edge case worth fixing but not release-blocking.

[claude]

Beyond the inline nit findings, I also checked whether a successful chunk flush could leave entry.chunkFetch stale (cached as resolved-to-null) and force spurious RPC re-fetches for blocks that were just archived into the new chunk. It doesn't cause incorrect behavior in practice: the walk is strictly ascending and the flushed entry is evicted before any height in that range would be requested again, so the stale cache is never actually read.

Extended reasoning...

This PR is a large (XL), new subsystem (raw block archive in GCS) with real correctness issues already found and fixed twice in this thread for the same two-argument-.then() cache-reset anti-pattern, plus a third occurrence caught and fixed again in a later commit — a pattern that recurred because the self-healing cache logic is hand-rolled three separate times (flagged as a nit in this run). Given the size, the number of non-trivial correctness bugs already surfaced and fixed across review rounds, and the accepted-tradeoffs the author explicitly flagged for review in the PR description (archive-before-continuity-check ordering, staged-object accumulation, GCS-availability coupling), this remains squarely in human-review territory; I'm not approving. This note only records one additional candidate — a stale chunkFetch cache surviving a successful flush — that I traced through the ascending-walk/eviction logic and confirmed is not reachable in practice, so a future pass doesn't need to re-derive it from scratch.

[claude]

🟡 BlockArchiveService#resolveChainId, ArchiveBlockSource#fetchChunk, and ArchiveBlockSource#flush each hand-roll the same 'cache a promise, null the field on failure so the next call re-fetches' pattern against three different fields. This isn't hypothetical: the first two were independently written with the broken two-argument .then(onFulfilled, onRejected) form, which silently fails to reset the cache when onFulfilled itself throws — the same bug found and fixed twice in this PR (e3386fc, aa0620e) because the logic was copy-pasted instead of shared. A single cachedUntilRejected<T>(fetch: () => Promise<T>): () => Promise<T> helper would let all three (and any future cache) share one correct implementation.

Extended reasoning...

What the duplication is. Three separate places in this PR implement the identical self-healing memoization contract — cache an in-flight promise on a field, and if it ever fails, null the field so the next caller re-fetches instead of awaiting a poisoned promise forever:

• BlockArchiveService#resolveChainId (block-archive.service.ts:139-148) caches this.#chainId.
• ArchiveBlockSource#fetchChunk (archive-block-source.ts:98-106) caches entry.chunkFetch.
• ArchiveBlockSource#flush/#flushChunk (archive-block-source.ts:122-130) caches entry.flush.

Each is a correct, hand-rolled instance of the same pattern against a different field, rather than one shared implementation reused three times.

Why this isn't a hypothetical DRY nitpick. The duplication has already caused a real, repeated bug in this exact PR thread. resolveChainId and fetchChunk were both originally written using the two-argument promise.then(onFulfilled, onRejected) form. That form's second argument only fires if the source promise (getStatus() / getChunk()) rejects — it does not fire when onFulfilled itself throws after the source promise already resolved (e.g. a malformed /status response missing node_info, or a corrupt chunk record failing to map). In both cases the derived promise assigned to the cache field rejected, but the reset handler never ran, so the field was left holding a permanently-rejected-but-truthy promise — poisoning every future call until process restart. This was found and fixed twice, independently, in commits e3386fc (resolveChainId) and aa0620e (fetchChunk), because the pattern was copy-pasted rather than centralized, so the same class of mistake had to be discovered and repaired at each call site separately.

Why nothing currently prevents a third recurrence. There is no shared utility encoding 'memoize a promise-returning fetch and self-reset on any failure, including a throw inside the success handler.' Each of the three call sites is free to reimplement the contract by hand, and two of the three already got it wrong on first attempt in the same way. A fourth cache added later (or a refactor of one of these three) has no guardrail against reintroducing the exact same two-argument .then() foot-gun.

Impact if left as-is. None of the three sites are currently broken — the fix commits corrected the two .then(onFulfilled, onRejected) sites, and #flush already used a correct try/catch-around-await variant. This is a maintainability/reuse issue, not a live correctness bug: it doesn't block merge on its own, but the duplication is a proven liability rather than a theoretical one.

Suggested fix. Extract a small shared helper, e.g. function cachedUntilRejected<T>(fetch: () => Promise<T>): () => Promise<T>, that returns a closure holding the cached promise and resetting it via try/catch (or .then(onFulfilled).catch(onRejected)) on any failure — including a throw inside the success path. resolveChainId and fetchChunk could then delegate to it directly; flush's per-RangeEntry cache could store the closure on the entry at creation. This removes the risk of a fifth hand-rolled copy reintroducing the same mistake, and consolidates the now-correct-but-duplicated logic into one implementation that only needs to be tested once.

[claude]

🟡 block-archive.service.spec.ts and archive-block-source.spec.ts each define their own local buildChunkRecords helper that maps buildRawBlockRecord over a height range — identical logic, differing only in parameter shape (fromHeight/toHeight vs a ChunkRange object). This duplicate was missed when the sibling buildRawBlockRecord builder was hoisted into test/fakes/build-raw-block-record.ts in commit aa0620e. Hoisting a single exported buildRawBlockRecords(range: ChunkRange) into that same fakes file would complete the consolidation and keep both specs in sync if RawBlockRecord/ChunkRange ever change.

Extended reasoning...

What the duplication looks like

block-archive.service.spec.ts (near line 200):

function buildChunkRecords(fromHeight: number, toHeight: number): RawBlockRecord[] {
  return Array.from({ length: toHeight - fromHeight + 1 }, (_, index) => buildRawBlockRecord(fromHeight + index));
}

archive-block-source.spec.ts (near line 155):

function buildChunkRecords(range: ChunkRange): RawBlockRecord[] {
  return Array.from({ length: range.end - range.start + 1 }, (_, index) => buildRawBlockRecord(range.start + index));
}

Both bodies do the exact same thing — map buildRawBlockRecord over an inclusive height range — differing only in whether the range is passed as two numbers or a ChunkRange object. This is the identical duplication class the PR already fixed for the underlying buildRawBlockRecord/buildRecord builder in commit aa0620e, which hoisted that function into test/fakes/build-raw-block-record.ts and pointed all three archive specs at it. That consolidation just didn't extend to the range-mapping wrapper built on top of it, so the same pattern reappeared one level up.

Why nothing catches this today: both copies are internally correct and every test using them currently passes — this is not a correctness bug, it's a maintenance-drift risk. If RawBlockRecord or ChunkRange ever changes shape, a contributor updating one copy could easily miss the other since the two spec files aren't colocated and neither imports from the other.

Impact: none at runtime. The only cost is that a future change to RawBlockRecord/ChunkRange requires remembering to update two independent local helpers instead of one shared one — the exact problem the team already solved for buildRawBlockRecord itself.

Fix: add an exported buildRawBlockRecords(range: ChunkRange): RawBlockRecord[] to test/fakes/build-raw-block-record.ts (implemented as Array.from({ length: range.end - range.start + 1 }, (_, i) => buildRawBlockRecord(range.start + i))), then have both spec files import and call it instead of defining their own copy. block-archive.service.spec.ts's call site would just wrap its fromHeight/toHeight pair into a { start, end } object at the call site.

Step-by-step proof:

1. Open archive-block-source.spec.ts around line 155 — buildChunkRecords(range: ChunkRange) maps buildRawBlockRecord(range.start + index) over range.end - range.start + 1 entries.
2. Open block-archive.service.spec.ts around line 200 — buildChunkRecords(fromHeight, toHeight) maps buildRawBlockRecord(fromHeight + index) over toHeight - fromHeight + 1 entries.
3. Substituting range.start/range.end for fromHeight/toHeight makes the two function bodies textually identical — confirming this is pure parameter-shape divergence, not different behavior.
4. Both specs already import buildRawBlockRecord from the shared test/fakes/build-raw-block-record.ts file created in aa0620e, so adding buildRawBlockRecords there is a one-file, low-risk change with no assertion changes needed at either call site.

This is a pure test-fixture DRY cleanup with no behavioral implication; it doesn't block merge.