fix(historical): resolve current-day lookups at now, not future end-of-day - #49
Conversation
|
APPROVE Fix matches the incident's root cause: only the live-resolve argument is clamped with the existing How This Was ReviewedReviewed with the review-pr-workflow skill — |
|
/review-workflow |
|
Review started (review-pr-workflow): https://github.com/yearn/yearn-prices/actions/runs/33075013630 |
Review: PR #49 — fix(historical): resolve current-day lookups at now, not future end-of-dayVerdict: APPROVE
IssuesNone. No defect survived verification. gitconfig-mask: sentinel How This Was ReviewedReviewed with the review-pr-workflow skill — tier: full — 2 files / ~30 net lines would downgrade to
|
murderteeth
left a comment
There was a problem hiding this comment.
Review: PR #49 — fix(historical): resolve current-day lookups at now, not future end-of-day
Verdict: APPROVE
The clamp itself is right and matches the batch path's existing use of toFetchTimestamp. Lint is green as of faaa513; all four CI jobs pass.
Issues
- Current-day on-chain fallback got slower (medium) — resolving at
nowmeans the chain head has usually moved past that timestamp by the time the on-chain source runs, so the cheap "target is in the future, return head" shortcut inestimateBlockByTimestampno longer fires. In a cold isolate the block lookup becomes a sleeping binary search (~26 sequentialgetBlockcalls instead of 1), on exactly the current-day requests this PR is repairing — worst on fast-block L2s. It amortizes to ~3 calls once the isolate has samples, so this is added latency, not failure.Done when:with a stubbed client whose head timestamp is one second newer than the resolve timestamp and no remembered samples, a current-day on-chain lookup resolves in single-digitgetBlockcalls, and a test pins that count.Provenance:d63d0ba
How This Was Reviewed
Reviewed with the review-pr-workflow skill —
5 review lenses, each finding independently verified by claude. 0 candidate findings were refuted and dropped.
Problem
Since the 08/25 deploy, Kong's current-day price lookups fail ~50% of the time (incident report: https://artifacts.yearn.dev/94e791faa5bb87e377c76cb2cc7a4cfe.md).
parseTimestampSegmentnormalizes every requested timestamp to end-of-day 23:59:59. That key is right for the DB read (today's rows are stored at EOD), but the exact route passed the same normalized value to the live-resolve fallback. For a block a few minutes old that timestamp is up to ~24h in the future: DeFiLlama's 6h search width around a future timestamp finds nothing until late in the UTC day, and the on-chain source maps a future timestamp to a block. Result: 404s and edge timeouts on nearly every current-day miss, worst right after UTC midnight (87% failure in the 00:00 hour on Aug 27).Fix
Clamp the resolve timestamp with the existing
toFetchTimestamphelper (already used by the batch warmup source): current-day requests resolve atnow, past days are unchanged. The DB lookup key, responsetimestampfield, and cache-control behavior stay as they were.No service-side backfill needed
scripts/warmup-prices.ts) and kept writing correct rows throughout.After merge
USE_PRICE_SERVICE(Render env). This fix removes the current-day regression (~38% of the zeros), but ~55% are tokens the service has never carried — those still return 404 and becomepriceUsd: 0in Kong until the coverage gap is closed or the flag is rolled back.packages/scripts/src/quality-assurance/pair:tvl-detect-gaps.ts --start 2026-08-25to list affected vaults, thentvl-backfill.ts --update tvlsto rewrite them.gap:tokens per run are the worklist; the historical gap backfill (feat(backfill): add historical price gap backfill #40) is the tool. Gate cutover on a measured coverage comparison against the legacy chain.Scope
This PR is only the current-day live-resolve fix. Coverage-gap work, Kong-side caching, and alerting on zero-price rates are follow-ups.