Skip to content

fix(fiat-rates): if historical rate missing for given date, return -1#1545

Draft
cranycrane wants to merge 4 commits into
masterfrom
1543-return--1-for-missing-historical-fiat-rates-on-given-day
Draft

fix(fiat-rates): if historical rate missing for given date, return -1#1545
cranycrane wants to merge 4 commits into
masterfrom
1543-return--1-for-missing-historical-fiat-rates-on-given-day

Conversation

@cranycrane

@cranycrane cranycrane commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Return -1 for missing historical fiat rates on a given day

Closes #1543

What & why

For a historical timestamp, GetTickersForTimestamps previously scanned forward across days
for the closest available daily ticker and, if none was found, fell back to the current
ticker. This silently returned a rate from a different day (at worst, today's price) for dates
we have no data for - misleading for old/sparse history.

Now a historical lookup serves only the ticker stored for the resolved UTC day. If no rate
is stored for that day, the rate is left unset and the API returns -1. Timestamps newer
than the stored history continue to return the current ticker.

Behavior change

Case Before After
Rate stored for the day exact rate exact rate (unchanged)
No rate stored for the day closest other day / current rate -1
Timestamp after stored history current rate current rate (unchanged)

-1 previously meant only "invalid/unavailable currency"; it now also means "no stored rate
for that day". Affects /api/v2/tickers, /api/v2/multi-tickers, and balancehistory fiat
rates.

Changes

  • fiat/fiat_rates.go - drop the forward/closest fallback in the daily branch; serve only the
    rate resolved for the same UTC day.
  • common/metrics.go, configs/metrics.yaml - add Prometheus counter
    blockbook_fiat_rates_missing_day_lookups to track days resolved to an unavailable rate.
  • openapi.yaml - document the new resolution semantics and -1 meaning.
  • fiat/fiat_rates_test.go - coverage for the missing-day → -1 path and the metric.

Client impact

Clients must treat -1 as "rate unavailable" (not a real price).

@cranycrane cranycrane requested a review from pragmaxim June 5, 2026 14:48
@cranycrane cranycrane linked an issue Jun 5, 2026 that may be closed by this pull request
@cranycrane cranycrane self-assigned this Jun 8, 2026
@cranycrane cranycrane marked this pull request as draft June 9, 2026 07:00
@cranycrane

Copy link
Copy Markdown
Contributor Author

this would possible break some suite code. It needs to be fixed there first.

@pragmaxim

Copy link
Copy Markdown
Contributor

Partial-day fiat-rate holes (write-side detection/repair) — context for this PR

Surfaced during the review of #1560. We will not fix this in #1560; filing it here because this PR (return -1 for a missing historical rate) is exactly the read-side half of the problem, and it's worth capturing the full picture so the read-side and write-side stay coordinated.

The problem

The fiat-rate store keeps one record per day holding both base Rates and per-token TokenRates. A partial-day hole is a day whose record exists but lacks a specific series — e.g. a day has usd but not eur, or has base rates but is missing a given token's rate. Four layers conspire so these holes are created, never detected, and silently misread:

  1. They get persisted. Historical writes are per-series and independent, and partial results are stored:
    • Runtime daily loop: UpdateHistoricalTickers fetches each vsCurrency separately; a single failure does hadFailures = true; continue and still storeTickers the rest (fiat/coingecko.go). Same pattern for tokens in UpdateHistoricalTokenTickers.
    • A vsCurrency/token first tracked later → older days never carry it.
  2. They are never re-filled. The runtime loop keys off FiatRatesFindLastTicker(vsCurrency, token); once a series has a recent ticker, resolveHistoricalDays returns d == 0 and it never goes back to an older interior day it missed. (The in-code "the rates will be updated next run" comment is misleading for interior holes — they won't be.)
  3. They are invisible to reconcile. missingDayWindow + db.FiatRatesGetTickerTimestamps are whole-day key-only scans (by design, to stay cheap), so a day that exists-but-is-incomplete is treated as healthy and never repaired.
  4. They read as a wrong value, silently. For a missing series on day D, GetTickersForTimestamps forward-scans past D to the next day whose ticker passes IsSuitableTicker, returning a neighboring day's rate with no error/sentinel; loadDailyTickers similarly back-fills absent interior days to a neighbor pointer, and the DB token path masks the same way. This is the symptom this PR fixes.

Relationship to the #1560 changes

#1560 reworked fiat-rate fetching. Two of its changes bear directly on this:

  • It added an all-or-nothing startup reconciliation: an aborted/early-exited reconcile pass now discards its partial map instead of persisting base-only/partial days. So "abort leaves partial days" is no longer a source of these holes.
  • It deliberately left one write-side source: an isolated per-series fetch failure on an otherwise-clean run still persists a day missing that series (documented in the ReconcileHistoricalRates doc comment). That, plus the runtime-loop failures above and later-added series, are the remaining sources.
  • It routed historical/daily fetches to the no-rate-limit CDN, which (a) makes 429-induced partial holes rarer and (b) means any future repair fetch can hit the CDN freely.

Proposed solution (two complementary halves)

A. Read-side — this PR. Return the -1 sentinel when the resolved day's ticker exists but isn't suitable for the requested vsCurrency/token, instead of forward-scanning to a neighbor — in GetTickersForTimestamps, the DB token path, and by stopping loadDailyTickers's neighbor back-fill from masking real holes. This fixes the silently-wrong read on its own timeline, regardless of how fast any write-side repair catches up.

B. Write-side — detect + repair (separate follow-up, not in #1560).

  • Cheap ship-now increment (base currencies): the pack format stores Rates before the large TokenRates blob, so a Rates-only partial decode can detect base-currency holes cheaply (never allocating the hundreds-of-KB token map). Extend missingDayWindow to flag a present day as incomplete when its Rates set doesn't cover an expected base set (derive the expected set from the newest healthy day's Rates — no network, avoids supported_vs_currencies drift). Feed incomplete days into the existing reconcile fill path; fillFromMarketChart(fillMissingOnly=true) adds only the missing currencies without rewriting existing data, fetches via the CDN, and is abort-safe under fix(fiat-rates): make CoinGecko fetching resilient to 429 #1560's all-or-nothing model (the existing DB record keeps its data; the hole is retried next startup). Guards: keep partial days out of the gap_too_large count, bound re-fetch churn for genuinely-unfillable cells, and gate behind a config flag + an incomplete_day metric. Tokens deferred (detecting per-token holes needs decoding the big TokenRates blob).
  • Convergent end-state: a per-day coverage manifest — a small per-day record listing which currencies/tokens a day holds, written transactionally with each ticker. One index then serves both fix(fiat-rates): make CoinGecko fetching resilient to 429 #1560's wanted "per-day completion checkpoint" (incremental reconcile resume) and partial-hole detection for base and tokens without decoding values, plus a home for a negative-cache of genuinely-unfillable cells. Larger (new column family + DB versioning + a bounded one-time backfill migration), but it's the real root-cause fix and avoids doing detection twice.

Suggested sequencing

  1. Land this PR (read-side -1) — stops silently-wrong reads immediately, independent of the repair work.
  2. Optionally the Rates-only base-currency write-side increment (interim; superseded later).
  3. Per-day coverage manifest as the convergent target (unifies the reconcile checkpoint + base/token detection + negative-cache).

cc: tracked out of the #1560 review.

pragmaxim added a commit that referenced this pull request Jun 17, 2026
…void losing token series

ReconcileHistoricalRates accumulates every series (base coin x each vsCurrency, then every
token) into one shared per-day record, so a day record is only as complete as it will ever be
once all targets have run. The previous code persisted the partial map on any early exit
(shutdown/budget abort, Cloudflare ban, or DB fill error). After a base-phase or mid-token-phase
abort that left a day with base rates but no token rates; because stage-1 detection is whole-day
key-only, the day was then seen as "present" and never reconciled again -- silently dropping its
token series for good.

Make persistence all-or-nothing: write only after a clean, complete pass; on any early exit
discard the partial map and report 0 filled, so the next startup re-reconciles the whole gap.
Move the fetched-units/tokens metrics to the clean-completion path so discarded fetches are not
counted, and generalize observeFetchedToken to take a count.

Tradeoff: an aborted run no longer resumes from partial progress and refetches the gap next
startup (a future per-day completion checkpoint can restore that). An isolated per-series failure
on an otherwise-clean run still persists a partial-day hole -- tracked separately (see #1545).

Tests: TokenPhaseAbortDiscardsBaseOnly (verified to fail on the old persist-on-abort behavior)
and NoTokensPersistsBaseOnly (clean base-only pass still persists).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
pragmaxim added a commit that referenced this pull request Jun 17, 2026
…void losing token series

ReconcileHistoricalRates accumulates every series (base coin x each vsCurrency, then every
token) into one shared per-day record, so a day record is only as complete as it will ever be
once all targets have run. The previous code persisted the partial map on any early exit
(shutdown/budget abort, Cloudflare ban, or DB fill error). After a base-phase or mid-token-phase
abort that left a day with base rates but no token rates; because stage-1 detection is whole-day
key-only, the day was then seen as "present" and never reconciled again -- silently dropping its
token series for good.

Make persistence all-or-nothing: write only after a clean, complete pass; on any early exit
discard the partial map and report 0 filled, so the next startup re-reconciles the whole gap.
Move the fetched-units/tokens metrics to the clean-completion path so discarded fetches are not
counted, and generalize observeFetchedToken to take a count.

Tradeoff: an aborted run no longer resumes from partial progress and refetches the gap next
startup (a future per-day completion checkpoint can restore that). An isolated per-series failure
on an otherwise-clean run still persists a partial-day hole -- tracked separately (see #1545).

Tests: TokenPhaseAbortDiscardsBaseOnly (verified to fail on the old persist-on-abort behavior)
and NoTokensPersistsBaseOnly (clean base-only pass still persists).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@cranycrane cranycrane force-pushed the 1543-return--1-for-missing-historical-fiat-rates-on-given-day branch from b7a48bf to 846bde6 Compare June 23, 2026 17:11
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.

Return -1 for missing historical fiat rates on given day

2 participants