fix(historical): bound batch miss resolution latency

[matheus1lva]

Summary

Miss-heavy batchHistorical requests issued one DeFiLlama request per pair and retried rate-limited calls on the same schedule. That increased provider fan-out and kept the response waiting on the slowest fallback.

Changes

• Resolve initial DeFiLlama misses through its batch endpoint, then send unresolved pairs through the source chain (a pair whose batch group failed skips the DeFiLlama single-coin lookup).
• Stop request-path DeFiLlama retries on the first 429, apply a 2.5-second per-attempt provider timeout, and return completed partial results when the five-second resolution deadline expires. Offline backfill keeps its explicit retry policy.
• Normalize batch timestamps to UTC day-end in edge-cache keys, then deduplicate and sort them. Range keys keep positional [start, end] arrays.
• Join exact database requests against inline VALUES instead of a requested CTE.

Testing

• npm run typecheck
• npm run lint
• npx vitest run --exclude test/enso.integration.test.ts (463 tests passed)
• npm test (463 tests passed; 2 live Enso tests failed because the external API returned 401)

[matheus1lva]

/review-workflow

[github-actions]

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

[matheus1lva]

/review

[github-actions]

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

[github-actions]

Summary

Reduces batchHistorical tail latency and 504s. Adds a provider-native DeFiLlama batch path (getBatchHistoricalPrices) that the historical registry tries first via a new resolveBatch, with unresolved pairs falling through to the existing per-source single-coin lookup; settlements are streamed back so a new 5s route-entry deadline can return already-completed partial results. Adds an opt-out for 429 retries (retryRateLimits) and applies it plus a 4s timeout to the request-path DeFiLlama client, leaving offline scripts on the old policy. Replaces the requested CTE in getBatchHistoricalPrices with an inline (VALUES …) join, and extends the edge-cache canonicalizer to normalize batchHistorical timestamp arrays by UTC day.

Checks not run: bun install fails in this sandbox (read-only tempdir) and node_modules is absent with no registry access, so biome check, tsc --noEmit, and vitest could not be executed. Findings below are from code reading only. No package.json change, so there are no new dependencies to evaluate against npm-policy. Visual verification skipped — no browser, and this is a JSON Worker API.

gitconfig-mask: sentinel

Issues

• src/sources/defillama/historical.ts:55 — Batch stage can eat the whole deadline, so the fallback never runs (medium) — Payload groups are fetched one after another, and the request-path client's 4s timeout is per request, not per route budget. Ten misses interleaved across ten tokens produce two groups (DEFI_LLAMA_TOKEN_BATCH is 5); if the first group hangs to its timeout it consumes 4 of the 5s, the second group is cut off, and the single-coin fallback this PR added and documented never executes — the response returns zero resolved prices for pairs that the fallback would have answered. The comment at src/registries/historical.ts:35-38 asserts the 4s timeout "leaves enough room" inside the 5s budget; that holds only for one sequential upstream call.
  • Done when: a batch of 10 misses spread over ≥6 tokens still reaches the single-coin fallback inside REQUEST_RESOLUTION_DEADLINE_MS when the first payload group exhausts its client timeout, with a test covering it, and the per-group isolation from 39a84b9 still holds.
  • Provenance: dd19f4e
• src/registries/historical.ts:39 — Exact route silently loses its 429 backoff (low) — createHistoricalSources is shared by /api/prices/historical/{timestamp}/{token} (src/routes/historical/exact.ts:23), not just batchHistorical, so a transient DeFiLlama 429 on a single-token lookup now surfaces on the first attempt instead of succeeding after the 1s/2s/4s backoff. The justification in the adjacent comment — a rate-limited batch retrying every member in lockstep — does not apply to a route that makes one upstream call, and docs/routes.md records the no-retry policy only under batchHistorical.
  • Done when: the exact route's 429 behavior is either restored or documented in docs/routes.md alongside the batchHistorical note, and the 4s timeout's effect on that route is stated.
  • Provenance: 39a84b9
• src/cache/edge.ts:74 — Cache key dedup lets an over-limit payload be served a cached 200 (low) — readEdgeCache runs before request validation (src/index.ts:84). Deduping normalized timestamps means {"ethereum:0xaaa…":[t,…]} with 91 identical entries canonicalizes to the same key as the single-entry form, so it returns a cached 200 when the edge is warm and the documented 400 (A maximum of 90 timestamps is allowed per token) when cold. The limit check reads the raw array length, which the key no longer preserves.
  • Done when: a payload that exceeds MAX_BATCH_TIMESTAMPS_PER_TOKEN gets the same rejection whether or not an equivalent request is already cached, and the same-UTC-day collision tests added in test/edge-cache.test.ts still pass.
  • Provenance: dd19f4e

Verdict

REQUEST_CHANGES

---

How This Was Reviewed

This review was conducted using the review-pr skill.

[murderteeth]

Summary

Adds a provider-native DeFiLlama batch path for batchHistorical misses, with unresolved pairs falling through to the source chain, a five-second route-entry deadline that returns completed partial results, and a retryRateLimits opt-out on the request-path client. The pre-cache parseBatchCoins guard in src/index.ts correctly closes the over-limit-payload hole that day-normalized cache keys would otherwise open.

Issues

• src/registries/historical.ts:116 - Fallback still starts only after the batch stage finishes (high) — If DeFiLlama is slow or returning 5xx, the batch call can consume the whole five-second budget before chainlink or on-chain resolution is ever attempted, so pairs those sources could have priced come back empty and unpersisted. Parallelizing the groups in 2bf1461 bounds one group, not the stage: the 2.5s cap is per attempt, and 5xx retries three times with 1s/2s sleeps — 10.5s against a 5s deadline.

  • Done when: with the DeFiLlama batch endpoint failing on every attempt, a batchHistorical request containing a chainlink- or on-chain-priced miss still returns that pair's price inside the route deadline, with a test covering it.
  • Provenance: dd19f4e

• src/sources/defillama/historical.ts:95 - A partly-failed batch still fans out one request per pair (medium) — The group error is reported only when every group came back empty. So if one group succeeds and another is rate-limited, each of the rate-limited group's pairs gets a fresh single-coin call — the fan-out this PR exists to remove, aimed at a provider that just returned 429, now with 429 retries disabled. docs/routes.md:284 promises the opposite.

  • Done when: with one payload group failing and another returning at least one usable price, no DeFiLlama single-coin request is issued for the failed group's coins during that same request.
  • Provenance: 39a84b9

• src/registries/historical.ts:121 - A succeeding group's pairs are blamed for another group's failure (medium) — When the batch call throws, every target is marked failed, including pairs whose own group returned 200 but matched no sample. Those skip the single-coin lookup that uses a wider search window and would likely have priced them, and their resolve-miss-failed log carries the other group's error instead of their own not-found.

  • Done when: when one group fails and another returns 200 with no matched sample, the second group's pairs are still tried against the DeFiLlama single-coin lookup and log their own NOT_FOUND.
  • Provenance: dd19f4e

Verdict

REQUEST_CHANGES

---

How This Was Reviewed

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

[matheus1lva]

All three findings addressed — plus one regression the fix for the first one introduced, caught before you had to.

1. Fallback started only after the batch stage — 3b206d5. HistoricalSourceRegistry.resolveBatch now races the batch stage against a 2.5s BATCH_STAGE_BUDGET_MS; pairs still pending at the cap fall through to chainlink/on-chain. Test: test/registries/registries.test.ts "falls through to the rest of the chain when the batch stage outruns its budget" (batch call hangs forever, fake timers advance 2.5s, chainlink delivers).

2. Partly-failed batch still fanned out per pair — 3b206d5. The global groupError throw is gone; each payload group reports only its own targets through a callback, so a failed group's pairs skip the single-coin lookup and a sibling group's keep it.

3. Succeeding group's pairs blamed for another group's failure — 3b206d5 for the reject path.

Side effect of that first fix, found in a self-review of 3b206d5 and fixed in b469f5d: the new timeout branch reintroduced finding 3 through a different door. markFailed([...pending.values()]) blamed every pending pair, and a pair whose own group had already answered 200 with no matched sample is still in pending — nothing removes it. So with ≥6 coins (2+ payload groups), group A hanging past 2.5s while group B answers promptly with no match meant group B's pairs were marked failed and skipped the DeFiLlama single-coin lookup — the exact cross-group misattribution this PR set out to remove. Same line also stamped UNAVAILABLE on unsupported-chain targets that were never in any payload, replacing their terminal NOT_FOUND.

Fix: onFailed becomes onSettled(targets, error?). A group reports its targets when it is done — with an error when it failed, without one when it answered. The registry keeps an answered set, and the timeout blames only batchTargets that are still pending and whose group never answered. New test covers the mixed case: sibling group answers, other group hangs past the cap, answered group's pair still reaches the single-coin lookup.

docs/routes.md updated to match. Known limit, not addressed: Promise.race does not cancel the in-flight batch, so abandoned 5xx retries can still consume provider capacity after the cap — response latency is bounded, upstream work is not. Fixing it means plumbing AbortSignal through the client.

[matheus1lva]

Merging this just to reduce the latency for danil on yearn.fi - will revisit.