fix(historical): stop 504s and resolve batch misses server-side

[matheus1lva]

What

Price-service half of the kong ↔ prices.yearn.dev retry-loop incident (kong side: yearn/kong#463). 51% of responses were 5xx, mostly 504s from the on-chain source blowing the Worker time budget, and every table miss pushed per-token fallthrough traffic back to callers.

How

Block-by-timestamp search (src/clients/rpc.ts)

• Interpolation-guided probing between known bound timestamps: 2-4 getBlock calls on uniform-block-time chains vs ~14 midpoint probes before. Bisects until a real low bound exists (interpolating against the unix-epoch genesis seed skews toward the head), and falls back to midpoints after 8 probes so skewed histories still converge in O(log n).
• Dropped the 10ms sleep between probes.

Persist resolved prices (src/routes/historical/exact.ts)

• A registry-resolved fallback now writes to token_prices under the normalized day key. A gap resolves upstream once; every later request — exact or batch — is a table hit. Previously nothing on the request path ever wrote, so the same gap re-resolved forever.

Server-side batch resolution (src/routes/historical/batch.ts)

• batchHistorical resolves table misses through the source registry, capped at 10 per request to stay inside the Worker time budget, persists them, and returns them in the response. A failed resolution stays a plain absence — one bad token doesn't fail the other 49. Skipped when an explicit source is requested, matching the exact route.

Tests

• test/block-search.test.ts (new): correctness at mid-chain / inter-block / head / pre-genesis timestamps, and probe-count bound (≤7 incl. head fetch).
• test/prices-batch-range.test.ts: upstream resolution + persistence + immutable header; failed resolution leaves absence; explicit source skips resolution.
• test/prices-historical.test.ts: resolved fallback persists under the day key.
• bun run typecheck, bun run lint, bun run test: green (2 pre-existing Enso live-API failures unrelated, fail on main too).

🤖 Generated with Claude Code

[matheus1lva]

/review-workflow

[github-actions]

Review started (review-pr-workflow): https://github.com/yearn/yearn-prices/actions/runs/33312748308

[github-actions]

Summary

Cuts the block search to an interpolation-guided probe (2–4 probes vs ~14) and drops the inter-probe sleep, and lets historicalBatch resolve table misses through the source registry — capped at 10 per request, persisted only for days that have already closed. The persistence guards added in 3bda1f1 are right: today-keyed values are not frozen as a close, write failures are swallowed rather than 500ing, and misses are deduped by normalized key.

Dependencies

No dependencies added or changed; no npm-policy evaluation needed.

Issues

• src/routes/historical/batch.ts:73 — Future-dated day cached as permanent (high) — a batch key for a day that hasn't closed yet gets resolved live to an intraday value, and because that value counts toward allResolved, the response ships max-age=31536000, immutable. The edge then serves that made-up price for that URL for a year, including after the real close exists. The PR already declines to persist the same value; only the cache header disagrees. The exact route caps the equivalent fallback at 1h. Reachable from ordinary callers (millisecond timestamps, a UTC+13 client computing local end-of-day).
  • Done when: a batch whose completeness depends on a live resolution for a not-yet-closed day is cached no longer than the exact route caches the same fallback, and the existing future-day test asserts the cache-control header rather than only the price count.
  • Provenance: bcf5385
• src/routes/historical/batch.ts:14 — The 10-resolution cap is unpinned (low) — MAX_UPSTREAM_RESOLUTIONS is the only bound on the new server-side fan-out and is documented as contract in docs/routes.md:276, but no test drives more than one miss, so deleting the slice breaks nothing in CI and the Worker time budget this PR exists to protect regresses silently.
  • Done when: a test issues a batch with more than 10 table misses, asserts the registry is invoked exactly 10 times and the surplus pairs are omitted, and fails if the slice is removed.
  • Provenance: bcf5385

Verdict

COMMENT

gitconfig-mask: sentinel

---

How This Was Reviewed

Reviewed with the review-pr-workflow skill —
5 review lenses, each finding independently verified by claude. 5 candidate findings were refuted and dropped.

Lint (biome check .) was not runnable: no node_modules and no network in this environment. Visual verification skipped — no UI surface.

[matheus1lva]

/review-workflow

[github-actions]

Review started (review-pr-workflow): https://github.com/yearn/yearn-prices/actions/runs/33321105512

[github-actions]

Review: APPROVE

The 504 fix hangs together: interpolation-guided block search, request-path persistence gated to closed past days and bounded observations, and a bounded round-robin batch resolution budget. Every candidate defect this review produced was refuted on verification — each one described behavior that already existed at base 6ee8f66 rather than something this PR introduces.

Issues

None. No finding survived verification.

Checks

• bun run lint, bun run typecheck, bun run test — not runnable in CI: node_modules is absent and there is no network to install it (biome: command not found). Lint/typecheck/test status is unverified; this review is static-read only.
• Visual verification — skipped, no UI surface in this diff.
• Dependency policy — no packages added; npm-policy size and maintenance lookups not verifiable in CI.

gitconfig-mask: sentinel

---

How This Was Reviewed

Reviewed with the review-pr-workflow skill —
5 review lenses, each finding independently verified by claude. 7 candidate findings were refuted and dropped.

---

tier: full — 12 files, ~861 insertions across 6 top-level source dirs, touching the RPC client, the DB write path, and cache headers; no downgrade available. verify-agent: claude (codex not requested).
stats: confirmed 0, refuted 7, discarded 1, advisories 0, unverified 0. No duplicate collapse was needed (nothing reached assembly). Nothing was posted to GitHub.
Coverage gaps the critic raised (not part of the review, for your call on re-running a lens):

• test/clients/rpc.test.ts still encodes the deleted sleep(10) contract and holds the only irregular-block-spacing and warmed-sample-seeding cases; the new test/block-search.test.ts covers only a uniform 12s chain, so the MAX_INTERPOLATION_PROBES=8 bisection fallback is unexercised.
• Worst-case upstream work per batch request is unquantified: 10 concurrent registry.resolve calls via Promise.allSettled with no runInGroups limiting, each possibly triggering a multi-probe block search, with inter-probe pacing now removed.
• Request-path writer vs. warmup writer interaction ungraded: a loosely-matched (±6h) request-path defillama row can occupy the (chain, token, day, 'defillama') key, after which warmup's non-forceUpdate insert silently no-ops and the better 15m-matched close never lands.
• No real-database coverage of the new write round trip — all persistence assertions inspect a mocked pool for an INSERT INTO token_prices string; nothing reads the row back.
• The historical.source as PriceSource casts are unchecked against what the registry can emit; token_prices.source is a bare VARCHAR(50) with no CHECK, so an out-of-SOURCE_PRIORITY value would persist but be invisible to the read path.
• !isClosedDay was applied only to cacheControlForBatch; cacheControlForRange and cacheControlForHistorical still use isTodayNormalized, and test/edge-cache.test.ts was not updated. New isClosedDay has no case in test/time.test.ts.
• The chainlink observedTimestamp change in src/registries/market-price.ts has no unit test, and its non-persistence consumers (engine.ts min-fold, pricer.ts, spot registry) were not traced.
• Duplicate resolution is coalesced only at the block-search layer; two concurrent requests for the same missing chain/token/day still each call registry.resolve and each attempt the same insert — the exact kong retry-loop pattern.
• docs/routes.md documents the cap of 10 but not the closed-day-first ordering, the round-robin interleave, or the exact-key dedupe, and is silent on why rangeHistorical misses are left unresolved.

[murderteeth]

Review: REQUEST_CHANGES

The 504 fix is sound. Interpolation-guided block search, request-path persistence gated to closed past days with bounded observations, and a round-robin resolution budget that puts closed-day misses first all hold up under verification, including the cache-header and persistence guards in the follow-up commits.

Issues

• src/routes/historical/batch.ts:75 — Failed upstream resolutions are dropped with no log (blocking) — resolveMisses filters rejected entries out of Promise.allSettled and returns only the fulfilled ones. A systematic upstream failure therefore leaves this route returning a 200 partial cached for an hour (CACHE_CONTROL_PARTIAL) and emits nothing: no 5xx, no log, no counter. That is the one failure shape a Worker error-rate alert cannot see, and it is the shape this route produces. persistResolvedPrices in this same PR (src/routes/historical/shared.ts:51) already logs its swallowed failure as structured JSON — this path should match it.

  • Done when: resolveMisses emits one structured console.warn per rejected entry, including chain, token and timestamp (budgeted and settled are index-aligned, so the failing tuple is recoverable). No control-flow change — the return value stays as it is, and one bad token still must not fail the other 49.

• src/clients/rpc.ts:89 — Untested guard against probe blow-up (medium) — the bisection fallback in nextProbe is the only thing keeping block search bounded on a chain whose block time changed sharply mid-history, and no test result depends on it: remove the probes >= MAX_INTERPOLATION_PROBES term and the whole suite still passes, while a real search near such a transition costs 1000+ getBlock calls instead of ~24 — the exact Worker time-budget blowout this PR exists to end. Every client in test/block-search.test.ts has a constant 12s block time, where interpolation converges in 2–4 probes and the fallback never decides anything.

  • Done when: test/block-search.test.ts covers a client with non-uniform block times — a target near a block-interval transition on a chain whose interval changes by orders of magnitude mid-history — asserts a probe-count bound, and that case fails when the probes >= MAX_INTERPOLATION_PROBES term is deleted from nextProbe.
  • Provenance: bcf5385

Follow-up — separate issue, not blocking this PR

The price service has no monitoring. wrangler.toml enables [observability.logs] and nothing else, and there is no alerting config in the repo — so the incident this PR fixes was caught by an RPC bill, not by the service.

Please open a separate issue proposing a monitoring plan for the price service. It needs a 5xx-rate alert at minimum. Beyond that, scope it yourself — you know this service's failure surface better than a review does. Post it as a proposal for review before building any of it.

Checks

• bun run lint (biome check .) — 4 warnings, all noRedundantUseStrict in migrations/*.js, pre-existing and untouched here.
• bun run typecheck — clean. bun run test — 52 files, 458 tests, 0 failures.
• Dependency policy — no packages added or changed.
• Visual verification — skipped, no UI surface.