From dbb9df13f205c854596bb06e68baf00c6c64ef5b Mon Sep 17 00:00:00 2001 From: Jeroen Ubbink Date: Tue, 25 Aug 2026 15:42:15 +0200 Subject: [PATCH] fix(pricing): keep an unusable rate out of the money math Prices reach the node from upstream catalogs, an operator's admin edit, a legacy database row and the BTC/USD feed. json.loads accepts the bare NaN and Infinity literals and overflows 1e999 to inf, so any of those sources can deliver a value that is not a price. Three guards let one through: - The token-rate gate tested truthiness, so NaN and inf reached the token math and raised ValueError/OverflowError *after* the response was served. The streaming handlers swallow that, so the request went unbilled. A negative rate produced a negative charge, which settlement subtracts from the balance. - An upstream-reported cost component was clamped with max(0.0, ...), which passes inf and NaN through. A non-finite component poisoned the proportional split in _calculate_from_usd_cost (inf / inf is NaN); the exception was absorbed by the broad handler around the USD path, so a request whose total was perfectly valid fell through to token estimation and was billed a fraction of what the upstream charged. Each spelling is now coerced before the fallback chooses between them, so a malformed first field cannot win the `or` and hide the usable figure beside it. - An exchange quote that was zero, negative or non-finite was accepted as the node's BTC/USD price, repricing every model on the node. Each now declines to price rather than billing a nonsensical amount. Co-Authored-By: Claude Opus 5 --- routstr/payment/cost_calculation.py | 58 +++- routstr/payment/price.py | 72 ++++- tests/unit/test_pricing_rate_validation.py | 317 +++++++++++++++++++++ 3 files changed, 424 insertions(+), 23 deletions(-) create mode 100644 tests/unit/test_pricing_rate_validation.py diff --git a/routstr/payment/cost_calculation.py b/routstr/payment/cost_calculation.py index 86495728..a51f633d 100644 --- a/routstr/payment/cost_calculation.py +++ b/routstr/payment/cost_calculation.py @@ -201,13 +201,14 @@ async def calculate_cost( cost_details = usage_data.get("cost_details", {}) if not isinstance(cost_details, dict): cost_details = {} - input_usd = _coerce_usd( - cost_details.get("input_cost") - or cost_details.get("upstream_inference_prompt_cost") + # Coerce each spelling before choosing between them: `inf` and `NaN` + # are truthy, so a malformed first field would otherwise win the + # fallback and the usable figure beside it would never be read. + input_usd = _coerce_usd(cost_details.get("input_cost")) or _coerce_usd( + cost_details.get("upstream_inference_prompt_cost") ) - output_usd = _coerce_usd( - cost_details.get("output_cost") - or cost_details.get("upstream_inference_completions_cost") + output_usd = _coerce_usd(cost_details.get("output_cost")) or _coerce_usd( + cost_details.get("upstream_inference_completions_cost") ) cache_pricing_rates: tuple[float, float, float, float] | None = None if cache_read_tokens > 0 or cache_creation_tokens > 0: @@ -267,9 +268,22 @@ async def calculate_cost( else: input_rate, output_rate, cache_read_rate, cache_creation_rate = pricing_rates - if not (input_rate and output_rate): + # Local import mirrors this module's existing lazy pricing imports. + from .models import is_usable_rate + + # An unusable rate is not "no pricing" to Python's truthiness: `NaN` and a + # negative float are both truthy, so they sailed past this gate — the one + # guard meant to catch a rate that cannot be billed on — and reached the + # token math, which raises `ValueError` on `NaN` and `OverflowError` on + # `inf` *after* the response was served (the streaming handlers swallow + # that, so the request goes unbilled), while a negative produced a negative + # charge. Ask whether each rate is usable rather than whether it is truthy. + rates = (input_rate, output_rate, cache_read_rate, cache_creation_rate) + if not all(is_usable_rate(rate) for rate in rates) or not ( + input_rate and output_rate + ): logger.warning( - "No token pricing configured — billing at flat MaxCostData. " + "No usable token pricing — billing at flat MaxCostData. " "Token counts %s in the upstream response but cannot be " "priced; the request will appear in dashboards with the " "raw counts and a fixed max-cost charge.", @@ -279,6 +293,8 @@ async def calculate_cost( "model": response_data.get("model", "unknown"), "input_tokens": input_tokens, "output_tokens": output_tokens, + "input_rate": input_rate, + "output_rate": output_rate, }, ) return MaxCostData( @@ -313,15 +329,35 @@ async def calculate_cost( def _coerce_usd(value: object) -> float: - """Coerce a value to USD float, handling various formats safely.""" + """Coerce an upstream-reported USD figure to a usable amount, else ``0.0``. + + These values come straight off the upstream response, where ``json.loads`` + accepts the bare ``NaN``/``Infinity`` literals and overflows ``1e999`` to + ``inf``. A non-finite figure is not a cost, and letting one through poisoned + the proportional split in ``_calculate_from_usd_cost`` (``inf / inf`` is + ``NaN``): the resulting exception was absorbed by the broad handler around + the USD path, so a request whose *total* cost was perfectly valid fell + through to token-estimated pricing and was billed a fraction of what the + upstream charged. + + ``0.0`` means "no usable figure" to every caller, which is the same thing an + absent field means, so the caller's existing ``> 0`` checks handle it. + """ + # Local import mirrors this module's existing lazy pricing imports. + from .models import is_usable_rate + if value is None or isinstance(value, bool): return 0.0 if not isinstance(value, (int, float, str)): return 0.0 try: - return max(0.0, float(value)) - except (TypeError, ValueError): + # An oversized integer raises OverflowError, not ValueError. + amount = float(value) + except (TypeError, ValueError, OverflowError): return 0.0 + # `is_usable_rate` also rejects negatives, which the previous `max(0.0, …)` + # clamped to zero — same outcome, stated once instead of inline. + return amount if is_usable_rate(amount) else 0.0 def _resolve_usd_cost(usage_data: dict, response_data: dict) -> float: diff --git a/routstr/payment/price.py b/routstr/payment/price.py index ad614322..93ed68e9 100644 --- a/routstr/payment/price.py +++ b/routstr/payment/price.py @@ -12,16 +12,68 @@ BTC_USD_PRICE: float | None = None SATS_USD_PRICE: float | None = None +def _parse_quote(raw: object, exchange: str) -> float | None: + """Coerce an exchange quote to a price, or ``None`` if it is not one. + + Every quote passes through here because the aggregator takes the ``min()`` + of what it collects: an unusable quote does not merely join the sample, it + *wins* it, and the result is the rate every model and every request on the + node is priced at. A zero divides by zero on the USD cost path, ``NaN`` + raises out of the integer conversion in settlement, and a negative rate + produces a negative charge that is credited back to the caller. + + ``is_usable_rate`` is the same predicate the billable-rate guards use, so + "finite and non-negative" has one definition; a quote is stricter still and + must be positive, since a BTC price of zero is a broken feed, not free money. + Imported lazily because ``payment.models`` imports this module. + """ + from .models import is_usable_rate + + # A boolean is a shape change, not a price: `float(True)` is a finite, + # positive 1.0 that passes every numeric guard below and then wins the + # `min()`, pricing the node at one dollar per bitcoin. + if isinstance(raw, bool): + logger.warning( + "Non-numeric price quote — ignoring this exchange", + extra={"exchange": exchange, "quote": repr(raw)}, + ) + return None + + try: + price = float(raw) # type: ignore[arg-type] + except (TypeError, ValueError, OverflowError) as e: + logger.warning( + "Unparseable price quote — ignoring this exchange", + extra={ + "error": str(e), + "error_type": type(e).__name__, + "exchange": exchange, + "quote": repr(raw), + }, + ) + return None + + if not is_usable_rate(price) or price <= 0: + logger.warning( + "Unusable price quote — ignoring this exchange", + extra={"exchange": exchange, "quote": price}, + ) + return None + + return price + + async def _kraken_btc_usd(client: httpx.AsyncClient) -> float | None: """Fetch BTC/USD price from Kraken API.""" api = "https://api.kraken.com/0/public/Ticker?pair=XBTUSD" try: response = await client.get(api) price_data = response.json() - price = float(price_data["result"]["XXBTZUSD"]["c"][0]) - - return price - except (httpx.RequestError, KeyError) as e: + return _parse_quote(price_data["result"]["XXBTZUSD"]["c"][0], "kraken") + except (httpx.RequestError, KeyError, IndexError, TypeError, ValueError) as e: + # A payload whose *shape* changed raises IndexError/TypeError, and a + # non-JSON body raises ValueError; unhandled, one exchange's bad day + # aborted the whole aggregation instead of dropping a single quote. logger.warning( "Kraken API error", extra={ @@ -39,10 +91,8 @@ async def _coinbase_btc_usd(client: httpx.AsyncClient) -> float | None: try: response = await client.get(api) price_data = response.json() - price = float(price_data["data"]["amount"]) - - return price - except (httpx.RequestError, KeyError) as e: + return _parse_quote(price_data["data"]["amount"], "coinbase") + except (httpx.RequestError, KeyError, IndexError, TypeError, ValueError) as e: logger.warning( "Coinbase API error", extra={ @@ -60,10 +110,8 @@ async def _binance_btc_usdt(client: httpx.AsyncClient) -> float | None: try: response = await client.get(api) price_data = response.json() - price = float(price_data["price"]) - - return price - except (httpx.RequestError, KeyError) as e: + return _parse_quote(price_data["price"], "binance") + except (httpx.RequestError, KeyError, IndexError, TypeError, ValueError) as e: logger.warning( "Binance API error", extra={ diff --git a/tests/unit/test_pricing_rate_validation.py b/tests/unit/test_pricing_rate_validation.py new file mode 100644 index 00000000..db2f5100 --- /dev/null +++ b/tests/unit/test_pricing_rate_validation.py @@ -0,0 +1,317 @@ +"""Tests that an unusable rate never reaches the money math. + +A billable rate is usable only when it is finite and non-negative. Prices reach +the node from upstream catalogs, an operator's admin edit, a legacy database row +and the BTC/USD feed, and each of those can deliver ``NaN``, ``±inf`` or a +negative — ``json.loads`` accepts the bare ``NaN``/``Infinity`` literals and +overflows ``1e999`` to ``inf``. + +These tests cover the guards between such a value and a charge: the token-rate +gate that decides a model cannot be priced, the upstream-reported USD cost, the +exchange-rate feed, and the stored-row read path. They assert the node declines +to price the request rather than billing a nonsensical amount or raising after +the response has already been served. +""" + +from __future__ import annotations + +import math +from collections.abc import Iterator +from typing import Any +from unittest.mock import patch + +import pytest + +from routstr.payment.cost_calculation import ( + CostData, + MaxCostData, + calculate_cost, +) +from routstr.payment.models import ( + Architecture, + Model, + Pricing, +) + + +@pytest.fixture(autouse=True) +def patch_sats_usd_price() -> Iterator[None]: + """Pin the exchange rate; these tests are about the rates, not the feed.""" + with patch("routstr.payment.cost_calculation.sats_usd_price", return_value=5.0e-5): + yield + + +def _architecture() -> Architecture: + return Architecture( + modality="text", + input_modalities=["text"], + output_modalities=["text"], + tokenizer="unknown", + instruct_type=None, + ) + + +def _model(sats_pricing: Pricing) -> Model: + return Model( + id="m", + name="m", + created=0, + description="d", + context_length=8192, + architecture=_architecture(), + pricing=Pricing(prompt=1e-06, completion=2e-06), + sats_pricing=sats_pricing, + ) + + +def _usage_response() -> dict[str, Any]: + return {"model": "m", "usage": {"prompt_tokens": 1000, "completion_tokens": 500}} + + +@pytest.mark.parametrize( + "bad_rate", + [float("nan"), float("inf"), -5.0], + ids=["nan", "inf", "negative"], +) +@pytest.mark.asyncio +async def test_unusable_token_rate_falls_back_to_max_cost(bad_rate: float) -> None: + """An unusable configured rate must not be billed on. + + The "no token pricing configured" gate is a truthiness test, and ``NaN`` and + negative floats are both truthy, so an unusable rate passes the guard that + exists to catch it. It then reaches the integer conversion in the token math, + which raises ``ValueError`` for ``NaN`` and ``OverflowError`` for ``inf`` — + after the upstream response has already been served, where the streaming + handlers swallow it and the request goes unbilled. + """ + model = _model(Pricing(prompt=bad_rate, completion=1.0)) + + cost = await calculate_cost(_usage_response(), max_cost=1234, model_obj=model) + + assert isinstance(cost, MaxCostData) + assert cost.total_msats == 1234 + + +@pytest.mark.parametrize( + "junk", [float("inf"), float("nan"), "Infinity"], ids=["inf", "nan", "inf-string"] +) +@pytest.mark.asyncio +async def test_junk_cost_component_still_bills_the_reported_total(junk: Any) -> None: + """A malformed component must not discard the upstream's real total cost. + + ``cost_details`` only splits the total across input and output; the total is + the authoritative billed amount. A non-finite component poisons the + proportional allocation (``inf / inf`` is ``NaN``), which raised out of the + USD path and was swallowed by the broad handler around it — so the request + silently fell through to token-estimated pricing and was billed at a small + fraction of what the upstream actually charged. + """ + model = _model(Pricing(prompt=1e-06, completion=2e-06)) + response = { + "model": "m", + "usage": { + "prompt_tokens": 1000, + "completion_tokens": 500, + "cost": 0.01, + "cost_details": {"input_cost": junk, "output_cost": 0.004}, + }, + } + + cost = await calculate_cost(response, max_cost=9999, model_obj=model) + + assert isinstance(cost, CostData) + # $0.01 at 5.0e-5 USD/sat = 200 sats = 200_000 msats. + assert cost.total_msats == 200000 + assert cost.total_usd == pytest.approx(0.01) + + +@pytest.mark.asyncio +async def test_junk_cost_component_falls_back_to_its_alternate_field() -> None: + """A malformed component must not shadow the field that would have replaced it. + + Each side of the split has two spellings and the second is a fallback for a + missing first. ``inf`` and ``NaN`` are both truthy, so a malformed + ``input_cost`` won that choice before anything checked whether it was a + number, and the usable figure beside it was never read — the input side was + then billed at nothing and the whole total landed on output. + """ + model = _model(Pricing(prompt=1e-06, completion=2e-06)) + response = { + "model": "m", + "usage": { + "prompt_tokens": 1000, + "completion_tokens": 500, + "cost": 0.01, + "cost_details": { + "input_cost": float("inf"), + "upstream_inference_prompt_cost": 0.006, + "output_cost": 0.004, + }, + }, + } + + cost = await calculate_cost(response, max_cost=9999, model_obj=model) + + assert isinstance(cost, CostData) + # $0.01 at 5.0e-5 USD/sat = 200_000 msats, split 0.006 : 0.004. + assert cost.total_msats == 200000 + assert (cost.input_msats, cost.output_msats) == (120000, 80000) + + +@pytest.mark.asyncio +async def test_non_finite_reported_cost_is_not_a_cost() -> None: + """An upstream-reported ``Infinity`` cost is junk, not an infinite charge. + + ``json.loads`` accepts the bare ``Infinity`` literal, so a compromised or + buggy upstream can put one in ``usage.cost``. It must not be treated as a + positive USD cost at all — the request falls through to the node's own token + pricing instead. + """ + model = _model(Pricing(prompt=1e-06, completion=2e-06)) + response = { + "model": "m", + "usage": { + "prompt_tokens": 1000, + "completion_tokens": 500, + "cost": float("inf"), + }, + } + + cost = await calculate_cost(response, max_cost=9999, model_obj=model) + + assert isinstance(cost, CostData) + assert math.isfinite(cost.total_usd) + assert cost.total_msats == 2 + + +class _ExchangeResponse: + def __init__(self, payload: dict[str, Any]) -> None: + self._payload = payload + + def json(self) -> dict[str, Any]: + return self._payload + + +class _ExchangeClient: + """Answers each exchange endpoint with a caller-supplied quote.""" + + def __init__(self, quotes: dict[str, Any]) -> None: + self._quotes = quotes + + async def get(self, url: str) -> _ExchangeResponse: + if "kraken" in url: + return _ExchangeResponse( + {"result": {"XXBTZUSD": {"c": [self._quotes["kraken"]]}}} + ) + if "coinbase" in url: + return _ExchangeResponse({"data": {"amount": self._quotes["coinbase"]}}) + return _ExchangeResponse({"price": self._quotes["binance"]}) + + +class _AsyncCtx: + def __init__(self, client: _ExchangeClient) -> None: + self._client = client + + async def __aenter__(self) -> _ExchangeClient: + return self._client + + async def __aexit__(self, *exc: object) -> bool: + return False + + +@pytest.fixture +def refresh_price_with() -> Iterator[Any]: + """Refresh the node's BTC/USD price from caller-supplied exchange quotes. + + Restores the module's cached price afterwards so one test cannot set the + rate another one bills at. + """ + import routstr.payment.price as price_module + + previous = (price_module.BTC_USD_PRICE, price_module.SATS_USD_PRICE) + + async def _run(quotes: dict[str, Any], last_good: float | None = None) -> None: + price_module.BTC_USD_PRICE = last_good + price_module.SATS_USD_PRICE = ( + None if last_good is None else last_good / 100_000_000 + ) + with patch.object( + price_module.httpx, + "AsyncClient", + lambda *a, **k: _AsyncCtx(_ExchangeClient(quotes)), + ): + await price_module._update_prices() + + yield _run + + price_module.BTC_USD_PRICE, price_module.SATS_USD_PRICE = previous + + +@pytest.mark.parametrize( + "bad_quote", + ["0", "0.00000000", "-1", "NaN", "Infinity", "N/A"], + ids=["zero", "zero-padded", "negative", "nan", "infinity", "non-numeric"], +) +@pytest.mark.asyncio +async def test_unusable_exchange_quote_does_not_set_the_node_price( + bad_quote: str, refresh_price_with: Any +) -> None: + """One exchange returning junk must not set the price the node bills at. + + The feed takes the ``min()`` of the quotes it collects, so an unusable quote + does not merely join the sample — it *wins*, and poisons the rate every model + and every request is priced at until the next refresh. Zero then divides by + zero on the USD path, ``NaN`` raises out of the integer conversion, and a + negative rate produces a negative charge that settlement credits back to the + caller. + + The two healthy quotes must still price the node. + """ + from routstr.payment.price import btc_usd_price + + await refresh_price_with( + {"kraken": bad_quote, "coinbase": "100000.0", "binance": "100000.0"} + ) + + assert btc_usd_price() == pytest.approx(100000.0) + + +@pytest.mark.asyncio +async def test_boolean_exchange_quote_does_not_set_the_node_price( + refresh_price_with: Any, +) -> None: + """A boolean in the price field is a shape change, not a $1 bitcoin. + + ``float(True)`` is ``1.0``, which is finite and positive, so a payload whose + price field turned into a boolean passes every numeric guard — and then + *wins* the ``min()``, pricing the whole node at one dollar per bitcoin. The + node's other coercions all reject ``bool`` before the numeric check for this + reason; this one is the exception. + """ + from routstr.payment.price import btc_usd_price + + await refresh_price_with( + {"kraken": True, "coinbase": "100000.0", "binance": "100000.0"} + ) + + assert btc_usd_price() == pytest.approx(100000.0) + + +@pytest.mark.asyncio +async def test_all_quotes_unusable_keeps_the_last_good_price( + refresh_price_with: Any, +) -> None: + """When every quote is junk the node keeps the last price it trusted. + + Adopting ``0`` or ``NaN`` because it was the only thing on offer would take + out billing for every model at once; skipping the update degrades to a stale + rate, which is the safe direction and what an unreachable exchange already + does. + """ + from routstr.payment.price import btc_usd_price + + await refresh_price_with( + {"kraken": "0", "coinbase": "NaN", "binance": "-3"}, last_good=90000.0 + ) + + assert btc_usd_price() == pytest.approx(90000.0)