Skip to content

fix: retry price ranges left empty by a transient decimals failure#67

Merged
suchapalaver merged 1 commit into
mainfrom
fix/skip-degraded-price-cache
May 26, 2026
Merged

fix: retry price ranges left empty by a transient decimals failure#67
suchapalaver merged 1 commit into
mainfrom
fix/skip-degraded-price-cache

Conversation

@suchapalaver

Copy link
Copy Markdown
Contributor

Summary

A persistent decimals() fetch failure while scanning a block range caused
the affected swaps to be dropped and the resulting zeroed/partial price to be
cached as authoritative — for both the gap and the whole query window. Every
later call for that range then returned the empty result, so a transient RPC
hiccup was frozen in and downstream consumers concluded the token had no swaps.
This makes the calculator decline to cache a range degraded by a decimals
failure, so it rescans on a later call. Closes #66.

What's in the diff

  • src/price/calculator.rs
    • process_gap_for_price now returns a private GapScan { result, degraded };
      degraded is set whenever a swap is dropped because its decimals couldn't be
      fetched. A genuinely-empty gap returns degraded: false.
    • calculate_price_between_blocks skips the per-gap cache insert for a degraded
      gap, and skips the full-range covering write-back whenever any gap degraded
      (the covering write would otherwise collapse the disjoint segments into one
      entry that re-caches the degraded data for the whole window). Each case logs
      at warn.

How it preserves the cache invariants

Clean gaps in the same call are still cached as disjoint segments; only the
degraded gap is left out. On the next call, calculate_gaps merges the cached
clean segments and reports the degraded range as a gap, so just that range is
rescanned — no over-counting, no stale empty aggregate.

Acceptance check (from #66)

  • Distinguish "scanned, genuinely empty" from "scanned but degraded by a
    fetch failure" — relevant.is_empty() returns degraded: false and is cached;
    a dropped-swap gap returns degraded: true and is not cached.
  • Avoid caching the degraded case as authoritative — per-gap insert and the
    full-range write-back are both skipped when degraded.
  • Respect the cache's disjoint-range invariants — clean segments remain
    disjoint; the degraded range resurfaces as a gap for rescanning.

Test plan

  • cargo test --all-features — 97/97 (price + cache suites) pass
  • cargo test (default) and cargo test --no-default-features pass
  • cargo clippy --all-targets --all-features -- -D warnings clean
  • cargo fmt clean
  • End-to-end retry-path verification requires a provider that fails
    decimals(); per the project convention (provider-dependent calculator paths
    live in examples/, not tests) this isn't covered by a unit test. The caching
    mechanics it relies on are covered by the existing PriceCache tests.

Note (deliberately out of scope)

The returned TokenPriceResult for a degraded range is still the partial/zeroed
aggregate with no partial-failure flag on it — a caller doing its own caching
could re-freeze it. Surfacing such a signal is the alternative the issue framed
("mirror the combined-retrieval partial-failure reporting"); this PR takes the
narrower skip-the-cache approach. Happy to file a follow-up if the signal is wanted.

When a token's decimals() call failed while scanning a block range, the
affected swaps were dropped and the resulting zeroed/partial price was
cached for both the gap and the whole query window. Every later call for
that range then returned the empty result, freezing the transient failure
in even after the RPC recovered.

A gap whose swaps were dropped by a decimals fetch failure is now treated
as degraded and left out of the cache, and the full-range write-back is
skipped whenever any gap degraded. A later call rescans the affected
range instead of serving the stale empty aggregate. Genuinely empty
ranges are still cached, so the retry only applies to failures.
@suchapalaver

Copy link
Copy Markdown
Contributor Author

Self-review pass

Pre-commit /code-review (high effort, 5 angles) + post-PR /pr-review, findings triaged below.

# Finding Status
1 Degraded gap could over-count if covering write-back collapsed segments Not a bug: full-range write-back is skipped whenever any gap degraded; clean segments stay disjoint and the degraded range resurfaces as a gap. Verified against existing PriceCache invariant tests.
2 degraded could falsely suppress caching of a complete scan Not a bug: degraded is only set on a dropped swap (decimals failure); relevant.is_empty() and normalize_against_pairNone (structural) both leave it false.
3 Mutex held across .await Not a bug: guards scoped to the else branches, dropped before merge, no await under lock.
4 No unit test for the retry path Skipped with justification: exercising it needs a provider that fails decimals(); per project convention provider-dependent calculator paths live in examples/, not tests. The caching mechanics it relies on are covered by existing PriceCache tests.
5 Returned result for a degraded range carries no partial-failure signal Out of scope by design — the issue framed signal-surfacing as the alternative to skip-the-cache. Noted in the PR body; can file a follow-up if wanted.

No blockers remaining. Confident to ship.

@suchapalaver suchapalaver merged commit 96e05f4 into main May 26, 2026
11 checks passed
@suchapalaver suchapalaver deleted the fix/skip-degraded-price-cache branch May 26, 2026 21:07
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.

1 participant