From 75fcef6686ac4df0a9ecfddc69ac48f051b1b0df Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Wed, 22 Jul 2026 01:10:55 -0400 Subject: [PATCH 1/5] docs(spec): fix the -USD/USDC quote-currency mismatch Rail 13 guards config.quote_currency while an order spends its PRODUCT's quote leg -- which can make the rail PASS an order the account cannot fund. Co-Authored-By: Claude Opus 4.8 (1M context) --- config.yaml | 6 +- ...26-07-22-quote-currency-mismatch-design.md | 78 +++++++++++++++++++ 2 files changed, 81 insertions(+), 3 deletions(-) create mode 100644 docs/superpowers/specs/2026-07-22-quote-currency-mismatch-design.md diff --git a/config.yaml b/config.yaml index d85bf9e1..1dd0a52f 100644 --- a/config.yaml +++ b/config.yaml @@ -23,7 +23,7 @@ caps: # set explicit values here only if you want an extra per-order/per-day risk ceiling tighter # than the exposure/concentration caps below. Left at their non-binding default here so # risk-sized rule orders ($400-24k typical) aren't silently rejected. - max_exposure_usd: 5000 + max_exposure_usd: 25 max_per_asset_pct: 0.50 market_data: @@ -34,7 +34,7 @@ market_data: history_days: 365 auto_trade: - mode: paper + mode: confirm # NOTE: currently UNUSED by any code path -- it is NOT a kill-switch and setting it # true or false changes nothing. Use `keel kill` to halt trading. enabled: false @@ -58,7 +58,7 @@ money_mgmt: streak_cooloff_days: 0 dca: - budget_usd: 50 + budget_usd: 5 cadence_days: 7 # quote-currency the executor draws BUY notional from (rail 13, USDC-funding) -- never bank/ACH. diff --git a/docs/superpowers/specs/2026-07-22-quote-currency-mismatch-design.md b/docs/superpowers/specs/2026-07-22-quote-currency-mismatch-design.md new file mode 100644 index 00000000..5a8cf602 --- /dev/null +++ b/docs/superpowers/specs/2026-07-22-quote-currency-mismatch-design.md @@ -0,0 +1,78 @@ +# The `-USD` / `USDC` quote-currency mismatch — design + +**Date:** 2026-07-22 +**Status:** Approved design +**Trigger:** found during the first supervised live-order attempt. + +## The defect + +The codebase conflates two different things: + +- the **product's quote leg** — what an order actually spends (`BTC-USD` spends **USD**); +- **`config.quote_currency`** — a single global setting (default `USDC`). + +Nothing derives the first from the product, so three places disagree with reality: + +1. **Rail 13 (`usdc_funding`) guards the wrong balance.** `executor` fetches + `_fetch_available_quote(broker, config.quote_currency)` and the rail compares *that* to the + order notional — but the order settles in the product's quote leg. + - **False veto** (observed): $49 USD available, $0.25 USDC, `BTC-USD` $5 order → vetoed. + - **False pass** (the serious one): ample USDC, no USD → the rail **approves** an order that + spends USD the account does not have. That is exactly the "never draw from a linked + bank/ACH source" case rail 13 exists to prevent. A safety rail that can pass when it should + veto is worse than no rail, because it is trusted. +2. **The screen's settlement check is vacuous.** `quotable_in_settlement_currency = + product.endswith(f"-{quote}") or bool(candles)`. Every screened product is `-USD` while + `quote` is `USDC`, so it always falls through to `bool(candles)` — i.e. + `ScreenPolicy.require_settlement_quote` re-checks "do we have bars", which the history rail + already covers. One of four admission criteria does nothing. +3. **`config.quote_currency` never matched this deployment.** Products, cached history, rules and + the simulator are all `-USD` (`_default_sim_products`, `_history_product`, `keel fetch`). + +## Fix + +**One rule: the currency an order spends is a property of the PRODUCT, never of global config.** + +1. **`keel_core.products.quote_currency_of(product_id) -> str | None`** — the leg after the last + `-`, uppercased. `None` for a malformed id (no `-`, empty leg), so callers fail closed. +2. **Rail 13 guards the product's quote leg.** The executor fetches the balance of + `quote_currency_of(intent.product_id)`; an unresolvable product id yields `None`, which the + rail already treats as unknown and vetoes. The violation message names the actual currency + instead of hardcoding "USDC". The rail key stays `usdc_funding` (it is an identifier other + code and `LIVE_STATE_RAILS` match on). +3. **The screen's settlement check becomes real:** `quotable_in_settlement_currency = + quote_currency_of(product) == policy settlement currency`. The `or bool(candles)` escape is + removed — that clause is what made the criterion vacuous. +4. **`config.quote_currency: USD`** in the repo config and both templates, matching the products + actually traded and cached. With (3) real, leaving it at `USDC` would reject every `-USD` + asset — the setting has to describe reality, and reality is `-USD`. + +`config.quote_currency` keeps a clear, narrower meaning: **the settlement currency this +deployment trades in**, used to screen candidates and to exclude the settlement balance from +holdings. It is no longer used to decide which balance funds a given order. + +## Non-goals + +- No change to *which* products are traded, and no re-fetching history under a different quote. +- Rail 13's fail-closed semantics are unchanged; only the balance it reads changes. +- No new veto for "product quoted in a currency you did not configure" — that is a real gap + (§ below) but adding a veto the night after a live test is the wrong sequencing. + +## Known remaining gap (recorded, not fixed) + +Nothing yet *rejects an order* whose product quote leg differs from `config.quote_currency`; the +screen rejects such an asset at admission, but a live-seeded rule bypasses admission. Worth a +follow-up rail once this lands and the live path is proven. + +## Testing + +- `quote_currency_of`: `BTC-USD`→`USD`, `BTC-USDC`→`USDC`, `eth-usd`→`USD`, `BTC`→`None`, + `""`→`None`, `BTC-`→`None`. +- **The false-pass hole**: ample `config.quote_currency` balance but an empty product-quote + balance must VETO. This is the headline regression test. +- The false veto: sufficient product-quote balance passes even when the configured currency + balance is 0. +- A malformed product id fails closed. +- Screen: `-USD` product with settlement `USD` passes; `-USDC` product with settlement `USD` + fails **even with candles present** (proving the vacuous clause is gone). +- Both config templates stay byte-identical to the repo config where the existing test requires. From f4217c29e5c212d561d6d811e556a7ff10588d7c Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Wed, 22 Jul 2026 01:18:16 -0400 Subject: [PATCH 2/5] fix(rails): rail 13 must guard the currency the ORDER spends Rail 13 fetched the balance of config.quote_currency (USDC) and compared it to the notional -- but a BTC-USD order spends USD. The rail guarded a balance the order never touches, with two failure modes: false veto (observed live): $49 USD, $0.25 USDC, a $5 BTC-USD order -> vetoed false pass (the serious one): ample USDC, no USD -> the rail APPROVES an order the account cannot fund. That is exactly the 'never draw from a linked bank/ACH source' case rail 13 exists to prevent. A rail that can pass when it should veto is worse than no rail, because it is trusted. The currency an order spends is a property of the PRODUCT, never of global config. New keel_core.products.quote_currency_of() derives it; the executor fetches that balance; the rail names that currency in its violation (telling an operator to fund USDC for a USD-settled order sends them to the wrong place); an unresolvable product id yields None, which the rail already vetoes on. Two consequences of the same root cause, fixed with it: - MarketFacts.quotable_in_settlement_currency was 'endswith(-quote) or bool(candles)'. Every screened product is -USD while quote was USDC, so it always fell through to bool(candles) -- one of four admission criteria was re-checking 'do we have bars'. The fallback is gone; the check is now real. - quote_currency defaults to USD (config, templates, Config, DiscoveryPolicy) because that is what this deployment actually trades. With the settlement check real, USDC would reject every -USD asset. Also: stablecoins join fiat in the holdings exclusion -- cash held between positions is not a position. The operator's working config moves to a gitignored config.local.yaml; the repo config.yaml is the shipped template and should not carry one machine's live settings. Co-Authored-By: Claude Opus 4.8 (1M context) --- .gitignore | 3 + config.yaml | 15 +++-- keel/cli.py | 15 ++++- keel/compliance/screen.py | 2 +- keel/execution/executor.py | 24 +++++-- keel/execution/guards.py | 18 ++++-- keel/templates/config.live.yaml | 9 ++- keel/templates/config.yaml | 9 ++- packages/keel-core/keel_core/config.py | 4 +- packages/keel-core/keel_core/products.py | 25 ++++++++ tests/compliance/test_assets_cli.py | 24 +++---- tests/compliance/test_screen.py | 8 +-- tests/conftest.py | 2 +- tests/core/__init__.py | 0 tests/core/test_products.py | 25 ++++++++ tests/execution/test_executor.py | 75 +++++++++++++++++++++- tests/fixtures/config_golden_defaults.json | 2 +- tests/fixtures/config_golden_full.json | 2 +- tests/fixtures/config_golden_full.yaml | 2 +- tests/test_agent.py | 16 +++-- tests/test_config.py | 6 +- 21 files changed, 231 insertions(+), 55 deletions(-) create mode 100644 packages/keel-core/keel_core/products.py create mode 100644 tests/core/__init__.py create mode 100644 tests/core/test_products.py diff --git a/.gitignore b/.gitignore index 25946ca4..58f28012 100644 --- a/.gitignore +++ b/.gitignore @@ -30,3 +30,6 @@ transactions/ # Generated by the release workflow at build time; never committed. keel/_build_info.py + +# operator's own working config (never the shipped template) +config.local.yaml diff --git a/config.yaml b/config.yaml index 1dd0a52f..34e32a74 100644 --- a/config.yaml +++ b/config.yaml @@ -23,7 +23,7 @@ caps: # set explicit values here only if you want an extra per-order/per-day risk ceiling tighter # than the exposure/concentration caps below. Left at their non-binding default here so # risk-sized rule orders ($400-24k typical) aren't silently rejected. - max_exposure_usd: 25 + max_exposure_usd: 5000 max_per_asset_pct: 0.50 market_data: @@ -34,7 +34,7 @@ market_data: history_days: 365 auto_trade: - mode: confirm + mode: paper # NOTE: currently UNUSED by any code path -- it is NOT a kill-switch and setting it # true or false changes nothing. Use `keel kill` to halt trading. enabled: false @@ -58,11 +58,16 @@ money_mgmt: streak_cooloff_days: 0 dca: - budget_usd: 5 + budget_usd: 50 cadence_days: 7 -# quote-currency the executor draws BUY notional from (rail 13, USDC-funding) -- never bank/ACH. -quote_currency: USDC +# The settlement currency this deployment TRADES IN. It must match the quote leg of the +# products you actually trade: everything here is `-USD` (see `_default_sim_products` / +# `_history_product`), so this is USD. It is NOT used to decide which balance funds a given +# order -- that comes from the product itself (`quote_currency_of`), because BTC-USD spends USD +# whatever this says. Used to screen candidates and to exclude the settlement balance from +# `keel assets holdings`. +quote_currency: USD subscription: # The SIMULATOR's assumed fee-free monthly volume. The LIVE rail-14 cap is not set here -- diff --git a/keel/cli.py b/keel/cli.py index 91756180..4dd62b5c 100644 --- a/keel/cli.py +++ b/keel/cli.py @@ -54,6 +54,7 @@ from typing import Any import click +from keel_core.products import quote_currency_of from keel_core.subscription import BrokerSubscription, SubscriptionStatus from keel_core.telemetry import bind_venue @@ -619,7 +620,11 @@ def _market_facts(repo: Repository, product: str, quote: str) -> screen_mod.Mark asset=asset, daily_bars=len(candles), median_daily_volume=median, - quotable_in_settlement_currency=product.endswith(f"-{quote}") or bool(candles), + # A REAL check: does this product settle in the currency this deployment trades in? + # The former `or bool(candles)` fallback made it vacuous -- every screened product is + # `-USD`, so it always fell through to "do we have bars", which the history criterion + # already covers. One of four admission criteria was doing nothing. + quotable_in_settlement_currency=quote_currency_of(product) == quote.upper(), ) @@ -672,6 +677,12 @@ def _history_product(asset: str) -> str: {"USD", "EUR", "GBP", "CAD", "AUD", "JPY", "CHF", "SGD", "BRL", "MXN", "TRY", "INR", "KRW"} ) +# Stablecoins are cash EQUIVALENTS -- funding you hold between positions, not positions. They are +# excluded for the same reason fiat is: proposing the money as something to buy with the money is +# noise. (They would be REJECTED anyway -- unattested, and a yield-bearing one fails §28.4 -- so +# this only removes a redundant row, never an admission.) +_CASH_EQUIVALENTS = frozenset({"USDC", "USDT", "DAI", "PYUSD", "TUSD", "USDP", "GUSD"}) + @assets_group.command("holdings") @click.option( @@ -720,7 +731,7 @@ def assets_holdings(ctx: click.Context, min_balance: str, run_screen: bool) -> N " If this is an authentication error, check CDP_API_KEY/CDP_API_SECRET in .env." ) from exc - excluded = _FIAT_CURRENCIES | {quote.upper()} + excluded = _FIAT_CURRENCIES | _CASH_EQUIVALENTS | {quote.upper()} # Currency codes are compared UPPERCASED: a `usdc` balance is still the settlement currency, # and must not be presented as something tradable on a casing accident. holdings = sorted( diff --git a/keel/compliance/screen.py b/keel/compliance/screen.py index 17e915f7..e9b96253 100644 --- a/keel/compliance/screen.py +++ b/keel/compliance/screen.py @@ -191,7 +191,7 @@ class DiscoveryPolicy: five years of candles for. Everything that decides admission lives in `screen_asset`. """ - quote_currency: str = "USDC" + quote_currency: str = "USD" min_quote_24h_volume: Decimal = Decimal("5000000") diff --git a/keel/execution/executor.py b/keel/execution/executor.py index 94fb4884..08bdfcc0 100644 --- a/keel/execution/executor.py +++ b/keel/execution/executor.py @@ -63,6 +63,7 @@ from decimal import Decimal from typing import Any, Literal +from keel_core.products import quote_currency_of from keel_core.telemetry import log_event, log_exception from keel.config import Config @@ -256,14 +257,22 @@ def _withdrawals_enabled(repo: Any, now_ts: int) -> bool | None: return bool(enabled) -def _fetch_available_quote(broker: Any, quote_currency: str) -> Decimal | None: - """Live available `quote_currency` (default USDC) balance from `broker.get_accounts()`. +def _fetch_available_quote(broker: Any, quote_currency: str | None) -> Decimal | None: + """Live available balance of `quote_currency` from `broker.get_accounts()`. - `None` on any failure -- a broker error, a malformed response, or simply no account for - `quote_currency` -- so rail 13 (USDC-funding) fails closed rather than guessing. This is + `quote_currency` is the **product's own settlement leg** (`BTC-USD` -> `USD`), not + `config.quote_currency`: the currency an order spends is a property of the product. Checking + the configured currency instead could report a healthy balance for a currency the order never + touches, letting rail 13 PASS an order the account cannot fund -- precisely the "never draw + from a linked bank/ACH source" case the rail exists to prevent. + + `None` on any failure -- a broker error, a malformed response, an unresolvable product id, or + simply no account for that currency -- so rail 13 fails closed rather than guessing. This is the one broker call `execute()` makes *before* `guards.check` runs: it's an input the rail needs, not itself something the guard gate protects (no funds move, no order is placed). """ + if not quote_currency: + return None if broker is None: # Paper mode passes no broker. That is not an error and must not be logged as one -- # an ERROR per paper entry would fill the operator's log with noise about a condition @@ -279,7 +288,7 @@ def _fetch_available_quote(broker: Any, quote_currency: str) -> Decimal | None: currency = account.get("currency") if isinstance(account, dict) else getattr( account, "currency", None ) - if currency != quote_currency: + if (currency or "").upper() != quote_currency.upper(): continue balance = account.get("available_balance") if isinstance(account, dict) else getattr( account, "available_balance", None @@ -309,7 +318,10 @@ def _build_intent( qty = sizing.size(equity, config.risk_pct, setup.entry, setup.stop) stop = setup.stop - available_quote = _fetch_available_quote(broker, config.quote_currency) + # The PRODUCT's quote leg -- what this order actually spends. + available_quote = _fetch_available_quote( + broker, quote_currency_of(signal.product_id) + ) withdrawals = _withdrawals_enabled(repo, now_ts) return OrderIntent( diff --git a/keel/execution/guards.py b/keel/execution/guards.py index 2db981fd..9e9ad8a3 100644 --- a/keel/execution/guards.py +++ b/keel/execution/guards.py @@ -81,6 +81,7 @@ from decimal import Decimal from typing import Any +from keel_core.products import quote_currency_of from keel_core.subscription import SubscriptionStatus from keel_core.telemetry import log_event @@ -410,20 +411,29 @@ def check( # SELL is exempt -- it produces quote currency, it doesn't consume it (Issue #59). if not offline and is_buy: balance = intent.available_quote - if balance is None: + # The currency this order actually settles in, derived from the product -- NOT + # `config.quote_currency`. Naming the configured currency in the message would send an + # operator to fund a balance the order never touches. + required = quote_currency_of(intent.product_id) + if required is None: violations.append( - f"usdc_funding: available {config.quote_currency} balance is unknown/" + f"usdc_funding: cannot determine the settlement currency of " + f"{intent.product_id!r} -- failing closed, BUY vetoed" + ) + elif balance is None: + violations.append( + f"usdc_funding: available {required} balance is unknown/" "unavailable -- failing closed, BUY vetoed" ) elif balance <= 0: violations.append( - f"usdc_funding: available {config.quote_currency} balance {balance} is not " + f"usdc_funding: available {required} balance {balance} is not " "greater than 0" ) elif balance < intent.notional: shortfall = intent.notional - balance violations.append( - f"usdc_funding: available {config.quote_currency} balance {balance} is short " + f"usdc_funding: available {required} balance {balance} is short " f"{shortfall} of the {intent.notional} order notional" ) diff --git a/keel/templates/config.live.yaml b/keel/templates/config.live.yaml index 05bb0c83..68ede46a 100644 --- a/keel/templates/config.live.yaml +++ b/keel/templates/config.live.yaml @@ -72,8 +72,13 @@ dca: budget_usd: 50 cadence_days: 7 -# quote-currency the executor draws BUY notional from (rail 13, USDC-funding) -- never bank/ACH. -quote_currency: USDC +# The settlement currency this deployment TRADES IN. It must match the quote leg of the +# products you actually trade: everything here is `-USD` (see `_default_sim_products` / +# `_history_product`), so this is USD. It is NOT used to decide which balance funds a given +# order -- that comes from the product itself (`quote_currency_of`), because BTC-USD spends USD +# whatever this says. Used to screen candidates and to exclude the settlement balance from +# `keel assets holdings`. +quote_currency: USD subscription: # The SIMULATOR's assumed fee-free monthly volume. The LIVE rail-14 cap is not set here -- diff --git a/keel/templates/config.yaml b/keel/templates/config.yaml index d85bf9e1..34e32a74 100644 --- a/keel/templates/config.yaml +++ b/keel/templates/config.yaml @@ -61,8 +61,13 @@ dca: budget_usd: 50 cadence_days: 7 -# quote-currency the executor draws BUY notional from (rail 13, USDC-funding) -- never bank/ACH. -quote_currency: USDC +# The settlement currency this deployment TRADES IN. It must match the quote leg of the +# products you actually trade: everything here is `-USD` (see `_default_sim_products` / +# `_history_product`), so this is USD. It is NOT used to decide which balance funds a given +# order -- that comes from the product itself (`quote_currency_of`), because BTC-USD spends USD +# whatever this says. Used to screen candidates and to exclude the settlement balance from +# `keel assets holdings`. +quote_currency: USD subscription: # The SIMULATOR's assumed fee-free monthly volume. The LIVE rail-14 cap is not set here -- diff --git a/packages/keel-core/keel_core/config.py b/packages/keel-core/keel_core/config.py index 45fd486e..506f27ba 100644 --- a/packages/keel-core/keel_core/config.py +++ b/packages/keel-core/keel_core/config.py @@ -258,7 +258,7 @@ class Config: subscription: SubscriptionConfig = field(default_factory=SubscriptionConfig) tiers: tuple[TierConfig, ...] = field(default_factory=_default_tiers) fees: FeesConfig = field(default_factory=FeesConfig) - quote_currency: str = "USDC" + quote_currency: str = "USD" logging: LoggingConfig = field(default_factory=LoggingConfig) research: ResearchConfig = field(default_factory=ResearchConfig) @@ -508,7 +508,7 @@ def load_config(path: str | Path) -> Config: "-- set it with `keel subscription attest --venue coinbase --tier `." ) - quote_currency = raw.get("quote_currency", "USDC") + quote_currency = raw.get("quote_currency", "USD") if not isinstance(quote_currency, str) or not quote_currency: raise ConfigError(f"quote_currency: must be a non-empty string, got {quote_currency!r}") diff --git a/packages/keel-core/keel_core/products.py b/packages/keel-core/keel_core/products.py new file mode 100644 index 00000000..c4b0b749 --- /dev/null +++ b/packages/keel-core/keel_core/products.py @@ -0,0 +1,25 @@ +"""Facts derivable from a venue product id. + +The one rule this module exists to enforce: **the currency an order spends is a property of the +PRODUCT, not of global configuration.** `BTC-USD` settles in USD whatever `config.quote_currency` +says. Conflating the two let rail 13 check a balance the order never touches, which could pass an +order the account had no settled funds for -- the exact case that rail exists to prevent. +""" + +from __future__ import annotations + + +def quote_currency_of(product_id: str | None) -> str | None: + """The settlement leg of `product_id` (`"BTC-USD"` -> `"USD"`), uppercased. + + Returns `None` for anything that does not resolve to a currency -- no separator, an empty + base or quote leg, or a non-string. Callers must treat `None` as *unknown* and fail closed: + rail 13 already vetoes a BUY on an unknown balance, and an unresolvable product id is + precisely that. + """ + if not isinstance(product_id, str): + return None + base, separator, quote = product_id.rpartition("-") + if not separator or not base.strip() or not quote.strip(): + return None + return quote.strip().upper() diff --git a/tests/compliance/test_assets_cli.py b/tests/compliance/test_assets_cli.py index f53dbd10..5848be24 100644 --- a/tests/compliance/test_assets_cli.py +++ b/tests/compliance/test_assets_cli.py @@ -167,7 +167,7 @@ def _venue_product(pid, volume, **over): base = { "product_id": pid, "base_name": pid.split("-")[0], - "quote_currency_id": "USDC", + "quote_currency_id": "USD", "status": "online", "trading_disabled": False, "is_disabled": False, @@ -181,7 +181,7 @@ def _venue_product(pid, volume, **over): def test_discover_proposes_and_says_so_loudly(tmp_path, valid_config_path, monkeypatch): db_path = tmp_path / "t.db" _repo_at(db_path) - venue = _FakeVenue([_venue_product("SOL-USDC", "50000000")]) + venue = _FakeVenue([_venue_product("SOL-USD", "50000000")]) monkeypatch.setattr(cli_module, "_build_broker", lambda config: venue) result = CliRunner().invoke( @@ -198,7 +198,7 @@ def test_discover_excludes_the_current_allowlist(tmp_path, valid_config_path, mo db_path = tmp_path / "t.db" _repo_at(db_path) venue = _FakeVenue( - [_venue_product("BTC-USDC", "90000000"), _venue_product("SOL-USDC", "50000000")] + [_venue_product("BTC-USD", "90000000"), _venue_product("SOL-USD", "50000000")] ) monkeypatch.setattr(cli_module, "_build_broker", lambda config: venue) @@ -206,7 +206,7 @@ def test_discover_excludes_the_current_allowlist(tmp_path, valid_config_path, mo cli, ["--db", str(db_path), "--config", str(valid_config_path), "assets", "discover"] ) assert "SOL" in result.output - assert "BTC-USDC" not in result.output + assert "BTC-USD" not in result.output def test_probe_history_marks_candidates_without_a_four_year_series( @@ -215,8 +215,8 @@ def test_probe_history_marks_candidates_without_a_four_year_series( db_path = tmp_path / "t.db" _repo_at(db_path) venue = _FakeVenue( - [_venue_product("SOL-USDC", "50000000"), _venue_product("NEW-USDC", "40000000")], - history_for=frozenset({"SOL-USDC"}), + [_venue_product("SOL-USD", "50000000"), _venue_product("NEW-USD", "40000000")], + history_for=frozenset({"SOL-USD"}), ) monkeypatch.setattr(cli_module, "_build_broker", lambda config: venue) @@ -226,9 +226,9 @@ def test_probe_history_marks_candidates_without_a_four_year_series( "assets", "discover", "--probe-history"], ) assert result.exit_code == 0, result.output - assert set(venue.probe_calls) == {"SOL-USDC", "NEW-USDC"} - sol_line = next(ln for ln in result.output.splitlines() if "SOL-USDC" in ln) - new_line = next(ln for ln in result.output.splitlines() if "NEW-USDC" in ln) + assert set(venue.probe_calls) == {"SOL-USD", "NEW-USD"} + sol_line = next(ln for ln in result.output.splitlines() if "SOL-USD" in ln) + new_line = next(ln for ln in result.output.splitlines() if "NEW-USD" in ln) assert "yes" in sol_line assert "NO" in new_line @@ -244,7 +244,7 @@ class _BrokenProbe(_FakeVenue): def get_candles(self, *a, **k): raise RuntimeError("timeout") - venue = _BrokenProbe([_venue_product("SOL-USDC", "50000000")]) + venue = _BrokenProbe([_venue_product("SOL-USD", "50000000")]) monkeypatch.setattr(cli_module, "_build_broker", lambda config: venue) result = CliRunner().invoke( @@ -253,7 +253,7 @@ def get_candles(self, *a, **k): "assets", "discover", "--probe-history"], ) assert result.exit_code == 0 - sol_line = next(ln for ln in result.output.splitlines() if "SOL-USDC" in ln) + sol_line = next(ln for ln in result.output.splitlines() if "SOL-USD" in ln) assert "?" in sol_line assert "NO" not in sol_line @@ -327,8 +327,8 @@ def test_holdings_excludes_the_settlement_currency_and_fiat( holding_lines = [ln for ln in result.output.splitlines() if ln.startswith(" ")] assets_listed = {ln.split()[0] for ln in holding_lines if ln.split()} assert "BTC" in assets_listed - assert "USDC" not in assets_listed, "cannot trade the currency you settle in" assert "USD" not in assets_listed, "fiat is funding, not a position" + assert "USDC" not in assets_listed, "a stablecoin is cash held between positions" def test_holdings_filters_dust_by_min_balance(tmp_path, valid_config_path, monkeypatch): diff --git a/tests/compliance/test_screen.py b/tests/compliance/test_screen.py index 8388ce4d..e05f8691 100644 --- a/tests/compliance/test_screen.py +++ b/tests/compliance/test_screen.py @@ -153,7 +153,7 @@ def test_policy_thresholds_are_configurable(): # -- discovery (proposal stage) ------------------------------------------------ -def _product(pid="SOL-USDC", quote="USDC", volume="50000000", **over): +def _product(pid="SOL-USD", quote="USD", volume="50000000", **over): base = { "product_id": pid, "base_name": pid.split("-")[0], @@ -178,7 +178,7 @@ def test_discovery_keeps_liquid_online_products_in_the_settlement_currency(): def test_discovery_drops_the_wrong_quote_currency(): from keel.compliance.screen import discover_candidates - assert discover_candidates([_product(quote="USD")]) == [] + assert discover_candidates([_product(quote="USDC")]) == [] assert discover_candidates([_product(quote="BTC")]) == [] @@ -211,7 +211,7 @@ def test_discovery_excludes_assets_we_already_hold(): from keel.compliance.screen import discover_candidates found = discover_candidates( - [_product("BTC-USDC"), _product("SOL-USDC")], exclude_assets=frozenset({"BTC"}) + [_product("BTC-USD"), _product("SOL-USD")], exclude_assets=frozenset({"BTC"}) ) assert [c.asset for c in found] == ["SOL"] @@ -220,7 +220,7 @@ def test_discovery_ranks_by_liquidity(): from keel.compliance.screen import discover_candidates found = discover_candidates( - [_product("A-USDC", volume="10000000"), _product("B-USDC", volume="90000000")] + [_product("A-USD", volume="10000000"), _product("B-USD", volume="90000000")] ) assert [c.asset for c in found] == ["B", "A"] diff --git a/tests/conftest.py b/tests/conftest.py index 1450f90a..8f4e6ae0 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -100,7 +100,7 @@ def attest_subscription( budget_usd: 50 cadence_days: 7 -quote_currency: USDC +quote_currency: USD subscription: assumed_free_volume_usd: 500 diff --git a/tests/core/__init__.py b/tests/core/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/core/test_products.py b/tests/core/test_products.py new file mode 100644 index 00000000..233312a1 --- /dev/null +++ b/tests/core/test_products.py @@ -0,0 +1,25 @@ +"""`keel_core.products` -- deriving an order's settlement leg from its product id. + +The currency an order spends is a property of the PRODUCT, never of global config. Getting this +wrong let rail 13 guard a balance the order never touches. +""" + +from __future__ import annotations + +from keel_core.products import quote_currency_of + + +def test_the_quote_leg_is_the_part_after_the_last_dash(): + assert quote_currency_of("BTC-USD") == "USD" + assert quote_currency_of("BTC-USDC") == "USDC" + assert quote_currency_of("PAXG-USD") == "USD" + + +def test_it_is_case_normalised(): + assert quote_currency_of("eth-usd") == "USD" + + +def test_a_malformed_product_id_returns_None_so_callers_fail_closed(): + """`None` is what rail 13 already treats as 'unknown' and vetoes on.""" + for bad in ("BTC", "", " ", "BTC-", "-USD", None): + assert quote_currency_of(bad) is None, f"{bad!r} should not resolve to a currency" diff --git a/tests/execution/test_executor.py b/tests/execution/test_executor.py index 293c00f0..aa14e825 100644 --- a/tests/execution/test_executor.py +++ b/tests/execution/test_executor.py @@ -62,7 +62,12 @@ def __init__( place_success: bool = True, place_order_id: str = "broker-order-1", usdc_balance: Decimal | None = Decimal("1000000"), + balances: dict[str, Decimal | None] | None = None, ) -> None: + # `balances` models a REAL account: one entry per currency. Without it the fake funds + # both USD and USDC with `usdc_balance`, so tests that only mean "the account is funded" + # keep meaning that -- the mismatch tests below set the two independently on purpose. + self._balances = balances self._preview = preview or { "order_total": Decimal("50.00"), "commission_total": Decimal("0.30"), @@ -83,9 +88,19 @@ def __init__( def get_accounts(self) -> list[dict[str, Any]]: self.get_accounts_calls += 1 + if self._balances is not None: + return [ + {"currency": c, "available_balance": b} for c, b in self._balances.items() + ] if self._usdc_balance is None: - return [{"currency": "USDC", "available_balance": None}] - return [{"currency": "USDC", "available_balance": self._usdc_balance}] + return [ + {"currency": "USD", "available_balance": None}, + {"currency": "USDC", "available_balance": None}, + ] + return [ + {"currency": "USD", "available_balance": self._usdc_balance}, + {"currency": "USDC", "available_balance": self._usdc_balance}, + ] def preview_order(self, product_id: str, side: Any, order_configuration: dict) -> dict: self.preview_calls.append( @@ -1310,3 +1325,59 @@ def test_scale_out_has_no_production_caller(repo): "bracket, and record a trade outcome for the partial exit -- otherwise rail 16 will " f"count net winners as losses. Call sites: {callers}" ) + + +# -- rail 13 guards the PRODUCT's quote leg, not config.quote_currency ---------- +# +# The currency an order spends is a property of the product: BTC-USD spends USD whatever +# `config.quote_currency` says. Checking the configured currency instead could PASS an order the +# account cannot fund -- the exact case rail 13 exists to prevent. + + +def test_ample_configured_currency_does_NOT_fund_a_differently_quoted_product(repo): + """THE hole. config.quote_currency=USDC with a large USDC balance, zero USD, and a BTC-USD + order: the old code checked USDC, passed, and let an unfundable order through to the broker. + """ + broker = FakeBroker(balances={"USDC": Decimal("1000000"), "USD": Decimal("0")}) + signal = _enter_signal() # BTC-USD -> spends USD + + result = execute(signal, broker, repo, _config(), mode="autonomous", now_ts=NOW_TS) + + assert result.placed is False, "an order spending USD was funded from a USDC balance" + assert result.preview is None, "rails must veto before the broker is touched" + assert any(v.startswith("usdc_funding") for v in result.vetoed_by) + assert broker.place_calls == [] + assert repo.get_orders() == [] + + +def test_the_products_own_quote_balance_is_what_permits_the_buy(repo): + """The mirror case, and the false-veto the operator actually hit: funds are in USD, the + configured currency is empty, and a BTC-USD order should proceed.""" + broker = FakeBroker(balances={"USDC": Decimal("0"), "USD": Decimal("1000000")}) + signal = _enter_signal() + + result = execute(signal, broker, repo, _config(), mode="autonomous", now_ts=NOW_TS) + + assert result.placed is True, result.vetoed_by + + +def test_a_product_id_with_no_resolvable_quote_leg_fails_closed(repo): + broker = FakeBroker(balances={"USD": Decimal("1000000")}) + signal = _enter_signal(product_id="BTCUSD") # no separator -> unknown quote leg + + result = execute(signal, broker, repo, _config(), mode="autonomous", now_ts=NOW_TS) + + assert result.placed is False + assert any(v.startswith("usdc_funding") for v in result.vetoed_by) + + +def test_the_veto_message_names_the_currency_actually_required(repo): + """'insufficient USDC' on a USD-settled order sends the operator to fund the wrong thing.""" + broker = FakeBroker(balances={"USD": Decimal("1"), "USDC": Decimal("1000000")}) + signal = _enter_signal() + + result = execute(signal, broker, repo, _config(), mode="autonomous", now_ts=NOW_TS) + + message = next(v for v in result.vetoed_by if v.startswith("usdc_funding")) + assert "USD" in message + assert "USDC" not in message, f"message names the wrong currency: {message}" diff --git a/tests/fixtures/config_golden_defaults.json b/tests/fixtures/config_golden_defaults.json index 6e229176..c56a0452 100644 --- a/tests/fixtures/config_golden_defaults.json +++ b/tests/fixtures/config_golden_defaults.json @@ -45,7 +45,7 @@ "min_trades": 100, "min_win_rate": "0.55" }, - "quote_currency": "USDC", + "quote_currency": "USD", "research": { "pbo_max": "0.05", "slope_floor": "-0.5" diff --git a/tests/fixtures/config_golden_full.json b/tests/fixtures/config_golden_full.json index e3943abb..4be965cc 100644 --- a/tests/fixtures/config_golden_full.json +++ b/tests/fixtures/config_golden_full.json @@ -50,7 +50,7 @@ "min_trades": 42, "min_win_rate": "0.45" }, - "quote_currency": "USD", + "quote_currency": "USDC", "research": { "pbo_max": "0.1", "slope_floor": "-0.75" diff --git a/tests/fixtures/config_golden_full.yaml b/tests/fixtures/config_golden_full.yaml index b768bdd0..49422a67 100644 --- a/tests/fixtures/config_golden_full.yaml +++ b/tests/fixtures/config_golden_full.yaml @@ -77,7 +77,7 @@ fees: maker_pct: 0.0035 # Non-default on purpose (the default is USDC) -- see the `tiers` note above. -quote_currency: USD +quote_currency: USDC logging: verbose: true diff --git a/tests/test_agent.py b/tests/test_agent.py index 6d8c517f..f9a9e504 100644 --- a/tests/test_agent.py +++ b/tests/test_agent.py @@ -61,8 +61,12 @@ def __init__( self._order_seq = 0 def get_accounts(self) -> list[dict[str, Any]]: - """A comfortably large USDC balance -- rail 13 (USDC-funding) fails closed otherwise.""" - return [{"currency": "USDC", "available_balance": Decimal("1000000")}] + """Comfortable balances -- rail 13 fails closed otherwise. Both legs are funded because + rail 13 checks the PRODUCT's quote leg (BTC-USD spends USD), not config.quote_currency.""" + return [ + {"currency": "USD", "available_balance": Decimal("1000000")}, + {"currency": "USDC", "available_balance": Decimal("1000000")}, + ] def get_candles( self, product_id: str, granularity: Granularity, start: int, end: int @@ -630,7 +634,7 @@ def __init__(self, **kwargs: Any) -> None: self.balance = Decimal("10000") def get_accounts(self) -> list[dict[str, Any]]: - return [{"currency": "USDC", "available_balance": self.balance}] + return [{"currency": "USD", "available_balance": self.balance}] series = {(PRODUCT, Granularity.ONE_DAY): [_candle(1_000 + i * 86_400) for i in range(30)]} broker = _DecliningBroker(series=series) @@ -670,7 +674,7 @@ def test_equity_counts_the_net_held_qty_across_multiple_buys(repo: Repository) - broker = FakeBroker() equity = agent._mark_to_market_equity( - repo, broker, [PRODUCT], {PRODUCT: Decimal("100")}, "USDC" + repo, broker, [PRODUCT], {PRODUCT: Decimal("100")}, "USD" ) # 2.5 BTC held, not 0.5 -- cash is FakeBroker's 1_000_000 @@ -684,7 +688,7 @@ def test_equity_counts_a_held_position_whose_rule_is_no_longer_live(repo: Reposi broker = FakeBroker() equity = agent._mark_to_market_equity( - repo, broker, [], {PRODUCT: Decimal("100")}, "USDC" + repo, broker, [], {PRODUCT: Decimal("100")}, "USD" ) assert equity == Decimal("1000000") + Decimal("2") * Decimal("100") @@ -699,7 +703,7 @@ def test_equity_falls_back_to_avg_cost_when_a_held_product_has_no_price( _seed_open_position(repo, PRODUCT, Decimal("2"), Decimal("100"), ts=1_000) broker = FakeBroker() - equity = agent._mark_to_market_equity(repo, broker, [PRODUCT], {}, "USDC") + equity = agent._mark_to_market_equity(repo, broker, [PRODUCT], {}, "USD") assert equity == Decimal("1000000") + Decimal("2") * Decimal("100") diff --git a/tests/test_config.py b/tests/test_config.py index ff911a4d..aaf2c2cc 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -58,7 +58,7 @@ def test_load_config_caps_typed_and_correct(valid_config_path): def test_load_config_subscription_and_quote_currency_defaults(valid_config_path): config = load_config(valid_config_path) - assert config.quote_currency == "USDC" + assert config.quote_currency == "USD" assert config.subscription.assumed_free_volume_usd == Decimal("500") assert config.subscription.pacing == "opportunistic" @@ -78,7 +78,7 @@ def test_load_config_subscription_and_quote_currency_absent_falls_back_to_defaul config = load_config(path) - assert config.quote_currency == "USDC" + assert config.quote_currency == "USD" assert config.subscription.assumed_free_volume_usd == Decimal("500") assert config.subscription.pacing == "opportunistic" @@ -170,7 +170,7 @@ def test_the_rejection_message_points_at_attest(write_config) -> None: def test_load_config_quote_currency_empty_raises_configerror(write_config): - text = VALID_CONFIG_YAML.replace("quote_currency: USDC", "quote_currency: ''") + text = VALID_CONFIG_YAML.replace("quote_currency: USD", "quote_currency: ''") path = write_config(text) with pytest.raises(ConfigError, match="quote_currency"): From 1b6c14674e3161a63031843ec8ee4d36bcdcf496 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Wed, 22 Jul 2026 01:29:16 -0400 Subject: [PATCH 3/5] fix: address review -- vacuous tests, legacy configs, docs, and rail 11 equity Independent review found four blocking defects in the first cut: B1 THE TESTS DID NOT TEST THE FIX. _config() inherits the new USD default, so 'config.quote_currency=USDC' was false in the tests asserting it, and reverting the executor line left the suite green -- the regression barrier was the default flip, not the product derivation. Now passed explicitly, and mutation-checked: reverting the line fails all three. B2 A default change does not change configs on disk. _history_product and _default_sim_products hardcoded -USD while the (now real) settlement check compared against config, so every existing quote_currency: USDC deployment silently rejected every asset on an unfixable settlement failure. Both now derive from the configured currency, so the worst case is an honest 'no local history, run keel fetch'. Verified against a legacy config. B3 The go-live pre-flight told the operator to hold USDC -- precisely the currency that now produces the false veto this PR exists to fix. Corrected, along with README/operator-runbook claims that buys route through USDC. B4 Rail 11's equity still read only config.quote_currency, and this PR unblocks the trading that corrupts it: under-read equity latches a monotonic HWM and arms the total-drawdown breaker permanently on a flat account. Equity now sums settled cash across every quote leg in play. Also: 'settlement' removed from the data-derived suppression set (it no longer touches candles, so suppressing it would hide a real failure), executor docstring, case-insensitive discover comparison, and restored coverage for 'no account exists for the required currency'. Co-Authored-By: Claude Opus 4.8 (1M context) --- README.md | 3 +-- docs/go-live-runbook.md | 2 +- docs/operator-runbook.md | 7 ++++-- keel/agent.py | 25 ++++++++++++++++----- keel/cli.py | 31 +++++++++++++++++--------- keel/compliance/screen.py | 2 +- keel/execution/executor.py | 2 +- tests/execution/test_executor.py | 28 ++++++++++++++++++++--- tests/fixtures/config_golden_full.yaml | 2 +- tests/test_agent.py | 24 ++++++++++++++++++++ 10 files changed, 100 insertions(+), 26 deletions(-) diff --git a/README.md b/README.md index 45b7e455..d50e02ff 100644 --- a/README.md +++ b/README.md @@ -22,8 +22,7 @@ Runtime settings (allowlist, target weights, risk caps, market data granularitie ## Before trading live Read `docs/operator-runbook.md`. It lists the compliance obligations **no rail can enforce** — chiefly -that **interest/rewards on idle balances must be disabled** (Coinbase pays USDC rewards, and buys are -routed through USDC, so riba can accrue with no order placed). Every guard in `keel/execution/guards.py` +that **interest/rewards on idle balances must be disabled** (Coinbase pays USDC rewards on idle balances, so riba can accrue with no order placed). Every guard in `keel/execution/guards.py` inspects an order, so account-level obligations are invisible to all of them and are yours to verify. Note keel ships **inert**: rail 14 refuses live BUYs until a subscription is attested with diff --git a/docs/go-live-runbook.md b/docs/go-live-runbook.md index 354787c7..4a69d1cd 100644 --- a/docs/go-live-runbook.md +++ b/docs/go-live-runbook.md @@ -18,7 +18,7 @@ exchange, correctly, and we have a record of it". | ☐ | you have a **trade-enabled** CDP key (the read-only one cannot place orders) | | | ☐ | `.env` holds `CDP_API_KEY` / `CDP_API_SECRET`, and `.env` is git-ignored | credentials live only here — there is no vault | | ☐ | you are at a real terminal | confirmation and every halt-releasing command fail closed off a TTY | -| ☐ | funds are in **USDC**, not USD | rail 13 vetoes a BUY that is not funded from `quote_currency`, and never draws from a bank/ACH | +| ☐ | settled funds are in the **quote leg of the product you trade** (`BTC-USD` → **USD**) | rail 13 checks the balance of the currency the ORDER spends, derived from the product — not `config.quote_currency` — and never draws from a bank/ACH | ## 1. Bring the deployment up diff --git a/docs/operator-runbook.md b/docs/operator-runbook.md index 810e7ac4..a1fd2686 100644 --- a/docs/operator-runbook.md +++ b/docs/operator-runbook.md @@ -23,8 +23,11 @@ Coinbase account. ### 1. ⛔ Disable interest / rewards on idle balances — **required** -**Why.** Coinbase pays **USDC Rewards** on idle USDC balances. Rail 13 routes buys through USDC, so the -account holds a USDC quote balance between trades. Interest accruing on that balance is **riba** +**Why.** Coinbase pays **USDC Rewards** on idle USDC balances. This applies whenever you hold idle +USDC — which is the case if you settle in USDC, and remains the case for any USDC you keep aside. +(Since 2026-07-22 rail 13 checks the quote leg of the product being traded rather than a single +configured currency, so a `-USD` deployment holds USD between trades; the rewards concern still +applies to any USDC balance you do hold.) Interest accruing on that balance is **riba** (KB §56.3, grounded in §28.1 / §30.1) — and it accrues **with no order placed**, so no rail sees it. This is not a trading decision the system can veto; it is an account setting only you can change. diff --git a/keel/agent.py b/keel/agent.py index d1e78c47..981a777d 100644 --- a/keel/agent.py +++ b/keel/agent.py @@ -58,6 +58,7 @@ from decimal import Decimal from typing import Any +from keel_core.products import quote_currency_of from keel_core.telemetry import bind_cycle, log_event, new_cycle_id, unbind_cycle from keel.config import Config @@ -277,13 +278,27 @@ def _mark_to_market_equity( than on a loss. That is why this iterates `products` and not `price_by_product` -- a product missing from the price map is exactly the case the fallback exists for. - `None`, rather than a partial total, when the quote balance is unavailable: equity is simply - unknowable then, and a wrong one corrupts the high-water mark PERMANENTLY (an HWM never - falls, so an under-read arms the breaker on a phantom drawdown from then on). + Settled cash is summed across EVERY currency in play: `quote_currency` plus the quote leg of + each product being valued. Counting only the configured currency under-reads an account whose + cash sits in what its products actually settle in (a `BTC-USD` deployment configured for + USDC) -- and because the HWM is monotonic, that under-read arms rail 11 on a phantom + drawdown permanently. Currencies with no account contribute nothing rather than failing. + + `None`, rather than a partial total, when NO quote balance could be read at all: equity is + simply unknowable then, and a wrong one corrupts the high-water mark PERMANENTLY (an HWM + never falls, so an under-read arms the breaker on a phantom drawdown from then on). """ - quote = _fetch_available_quote(broker, quote_currency) - if quote is None: + currencies: list[str] = [] + for candidate in (quote_currency, *(quote_currency_of(p) for p in products)): + upper = (candidate or "").upper() + if upper and upper not in currencies: + currencies.append(upper) + + balances = [(c, _fetch_available_quote(broker, c)) for c in currencies] + if all(balance is None for _, balance in balances): + # Nothing readable at all -- not "zero cash", genuinely unknown. return None + quote = sum((balance for _, balance in balances if balance is not None), Decimal("0")) # Quantities come from `_held_position` (the filled-orders audit log), NEVER from # `position_rule:`. That key is exit-rule OWNERSHIP state: it is overwritten on diff --git a/keel/cli.py b/keel/cli.py index 4dd62b5c..8247eb63 100644 --- a/keel/cli.py +++ b/keel/cli.py @@ -657,18 +657,24 @@ def _screen_product( return facts, screen_mod.screen_asset(facts, attestation) -#: The product id this codebase uses for an asset's daily history. `assets screen`, -#: `keel simulate` and `keel fetch` all key on `-USD` (see `_default_sim_products`), so holdings -#: must too -- screening `{asset}-{quote_currency}` instead would find zero cached bars for every -#: asset and report "no local history" forever, which is worse than useless: it looks like a -#: verdict about the asset. -def _history_product(asset: str) -> str: - return f"{asset}-USD" +def _history_product(asset: str, quote: str) -> str: + """The product id for an asset, in the deployment's settlement currency. + + ONE source of truth, shared with `_default_sim_products`. Hardcoding `-USD` here while the + screen compared against `config.quote_currency` is what let a `quote_currency: USDC` config + reject every asset on a settlement failure it could never fix -- a default change does not + change configs already on disk. Deriving both from the same setting means the worst case is + an honest "no local history, run `keel fetch`", not a silent unfixable rejection. + """ + return f"{asset}-{quote.upper()}" # Failure classes that are DOWNSTREAM of having no cached history: with zero bars they report # on our data, not on the asset, so `assets holdings` must not print them as verdicts. -_DATA_DERIVED_FAILURES = frozenset({"liquidity", "settlement"}) +# `settlement` is deliberately NOT here: since it compares the product's quote leg to the +# configured settlement currency it no longer touches candles at all, so it is fully assessable +# with zero bars -- suppressing it would hide a real, actionable failure. +_DATA_DERIVED_FAILURES = frozenset({"liquidity"}) # Never candidates: you cannot trade the currency you settle in, and fiat is funding rather than # a position. Coinbase quotes many fiats, so the list is deliberately broad -- a missing one is @@ -764,7 +770,7 @@ def assets_holdings(ctx: click.Context, min_balance: str, run_screen: bool) -> N if not run_screen: continue - product = _history_product(asset) + product = _history_product(asset, quote) facts, result = _screen_product(repo, product, quote) click.echo(f" {result.summary} ({facts.daily_bars} daily bars cached)") @@ -1842,7 +1848,12 @@ def pnl(ctx: click.Context, asset: str | None, raw_marks: tuple[str, ...]) -> No def _default_sim_products(config: Config) -> list[str]: - return [f"{asset}-USD" for asset in config.allowlist] + """Allowlist assets as product ids, in the configured settlement currency. + + Shares `_history_product`'s derivation so `fetch`, `simulate`, `screen` and `holdings` + cannot disagree about which product an asset means. + """ + return [_history_product(asset, config.quote_currency) for asset in config.allowlist] def _parse_products_option(products: str | None, config: Config) -> list[str]: diff --git a/keel/compliance/screen.py b/keel/compliance/screen.py index e9b96253..3912f5c7 100644 --- a/keel/compliance/screen.py +++ b/keel/compliance/screen.py @@ -213,7 +213,7 @@ def discover_candidates( for product in products: product_id = product.get("product_id") or "" - if product.get("quote_currency_id") != policy.quote_currency: + if (product.get("quote_currency_id") or "").upper() != policy.quote_currency.upper(): continue if product.get("status") != "online": continue diff --git a/keel/execution/executor.py b/keel/execution/executor.py index 08bdfcc0..2d51be2b 100644 --- a/keel/execution/executor.py +++ b/keel/execution/executor.py @@ -44,7 +44,7 @@ profit-take) through the same guard+preview+place+log pipeline as a plain SELL leg. **USDC-funding balance (rail 13, Issue #59).** For a BUY `_build_intent` fetches the live -available `config.quote_currency` (default USDC) balance from `broker.get_accounts()` and hands +available balance of the PRODUCT's quote leg from `broker.get_accounts()` and hands it to `guards.check` via `OrderIntent.available_quote` -- guards itself has no broker access, by design. This happens *before* `guards.check` runs (the balance is an input to the rail, not something guarded itself), so it is the one broker call this module makes ahead of the guard diff --git a/tests/execution/test_executor.py b/tests/execution/test_executor.py index aa14e825..a470d8ea 100644 --- a/tests/execution/test_executor.py +++ b/tests/execution/test_executor.py @@ -1341,7 +1341,10 @@ def test_ample_configured_currency_does_NOT_fund_a_differently_quoted_product(re broker = FakeBroker(balances={"USDC": Decimal("1000000"), "USD": Decimal("0")}) signal = _enter_signal() # BTC-USD -> spends USD - result = execute(signal, broker, repo, _config(), mode="autonomous", now_ts=NOW_TS) + result = execute( + signal, broker, repo, _config(quote_currency="USDC"), + mode="autonomous", now_ts=NOW_TS, + ) assert result.placed is False, "an order spending USD was funded from a USDC balance" assert result.preview is None, "rails must veto before the broker is touched" @@ -1356,7 +1359,10 @@ def test_the_products_own_quote_balance_is_what_permits_the_buy(repo): broker = FakeBroker(balances={"USDC": Decimal("0"), "USD": Decimal("1000000")}) signal = _enter_signal() - result = execute(signal, broker, repo, _config(), mode="autonomous", now_ts=NOW_TS) + result = execute( + signal, broker, repo, _config(quote_currency="USDC"), + mode="autonomous", now_ts=NOW_TS, + ) assert result.placed is True, result.vetoed_by @@ -1376,8 +1382,24 @@ def test_the_veto_message_names_the_currency_actually_required(repo): broker = FakeBroker(balances={"USD": Decimal("1"), "USDC": Decimal("1000000")}) signal = _enter_signal() - result = execute(signal, broker, repo, _config(), mode="autonomous", now_ts=NOW_TS) + result = execute( + signal, broker, repo, _config(quote_currency="USDC"), + mode="autonomous", now_ts=NOW_TS, + ) message = next(v for v in result.vetoed_by if v.startswith("usdc_funding")) assert "USD" in message assert "USDC" not in message, f"message names the wrong currency: {message}" + + +def test_no_account_at_all_for_the_required_currency_fails_closed(repo): + """Distinct from a zero balance: the broker returns accounts, none of them the one this + order settles in. Silence about a currency is not evidence of funds in it.""" + broker = FakeBroker(balances={"EUR": Decimal("1000000")}) # no USD account + signal = _enter_signal() # BTC-USD + + result = execute(signal, broker, repo, _config(), mode="autonomous", now_ts=NOW_TS) + + assert result.placed is False + assert result.preview is None + assert any("unknown/unavailable" in v for v in result.vetoed_by) diff --git a/tests/fixtures/config_golden_full.yaml b/tests/fixtures/config_golden_full.yaml index 49422a67..b7fa5ebc 100644 --- a/tests/fixtures/config_golden_full.yaml +++ b/tests/fixtures/config_golden_full.yaml @@ -76,7 +76,7 @@ fees: taker_pct: 0.0075 maker_pct: 0.0035 -# Non-default on purpose (the default is USDC) -- see the `tiers` note above. +# Non-default on purpose (the default is USD) -- see the `tiers` note above. quote_currency: USDC logging: diff --git a/tests/test_agent.py b/tests/test_agent.py index f9a9e504..0a0ced61 100644 --- a/tests/test_agent.py +++ b/tests/test_agent.py @@ -1239,3 +1239,27 @@ def test_autonomy_still_applies_strictly_before_its_expiry(repo): assert result.mode == "autonomous" assert len(broker.place_calls) == 1 + + +def test_equity_counts_settled_cash_in_EVERY_quote_leg_being_traded(repo): + """Rail 11's HWM is monotonic, so an under-read arms a phantom drawdown PERMANENTLY. + + Counting only `config.quote_currency` under-reads an account whose settled cash sits in the + currency its products actually settle in -- and this is reachable now that rail 13 permits + such an order. Equity must see the cash that funds the trading. + """ + + class TwoCurrencyBroker: + def get_accounts(self): + return [ + {"currency": "USD", "available_balance": Decimal("1000")}, + {"currency": "USDC", "available_balance": Decimal("7")}, + ] + + equity = agent._mark_to_market_equity( + repo, TwoCurrencyBroker(), ["BTC-USD"], {}, "USDC" + ) + assert equity is not None + assert equity >= Decimal("1000"), ( + f"equity {equity} ignored the USD cash that funds BTC-USD orders" + ) From f3d71ed4de3f189e5845b9c711e92076b6c29579 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Wed, 22 Jul 2026 01:39:26 -0400 Subject: [PATCH 4/5] test: guard the fixes that could silently regress; own the settlement tautology MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second review round. No blocking defects were found; these are its non-blocking findings, and two of them matter. THE FIXES HAD NO TESTS. Reverting _history_product to a hardcoded -USD, or equity's all()->any(), or discover's .upper(), all left the suite green -- the same gap the first round caught for rail 13 itself. All four are now mutation- checked and CAUGHT. THE SETTLEMENT CRITERION IS TAUTOLOGICAL FOR PRODUCTS WE DERIVE, and my B2 fix made it so: _history_product builds {asset}-{quote} and the screen then asserts they match. Rather than claim it is doing work it isn't, the check is documented for what it actually guards -- an EXTERNALLY supplied product (--products, or a future venue/LLM-sourced list) whose quote leg would need a cross to settle, which is the §65.7 case. A test pins that BTC-EUR under USD settlement fails it, so the criterion is not dead. Also: monitor duplicated the -USD expression instead of calling the helper, so under a non-USD config it cached candles nothing else read; a comment still described a fallback this branch deleted; help text promised -USD regardless of configuration; and equity's no-FX-conversion assumption is now an explicit documented bound rather than an unstated one. Co-Authored-By: Claude Opus 4.8 (1M context) --- keel/agent.py | 8 +++++ keel/cli.py | 15 ++++++---- tests/compliance/test_assets_cli.py | 46 +++++++++++++++++++++++++++++ tests/compliance/test_screen.py | 12 ++++++++ tests/test_agent.py | 21 +++++++++++++ 5 files changed, 97 insertions(+), 5 deletions(-) diff --git a/keel/agent.py b/keel/agent.py index 981a777d..60f6fe3c 100644 --- a/keel/agent.py +++ b/keel/agent.py @@ -278,6 +278,14 @@ def _mark_to_market_equity( than on a loss. That is why this iterates `products` and not `price_by_product` -- a product missing from the price map is exactly the case the fallback exists for. + ⚠️ **Balances are summed at face value, with NO FX conversion.** That is correct for the + supported case -- one settlement currency, whose products all share that quote leg -- and it + is why `config.quote_currency` and the products' legs should agree. An account mixing, say, + USD and EUR would have them added 1:1 and over-read equity, which (the HWM being monotonic) + would arm rail 11 permanently. Reaching that state needs a product whose quote leg differs + from the settlement currency, which the admission screen rejects; a live-seeded rule can + bypass admission, so this is recorded as a known bound rather than claimed impossible. + Settled cash is summed across EVERY currency in play: `quote_currency` plus the quote leg of each product being valued. Counting only the configured currency under-reads an account whose cash sits in what its products actually settle in (a `BTC-USD` deployment configured for diff --git a/keel/cli.py b/keel/cli.py index 8247eb63..7fc930b9 100644 --- a/keel/cli.py +++ b/keel/cli.py @@ -669,8 +669,11 @@ def _history_product(asset: str, quote: str) -> str: return f"{asset}-{quote.upper()}" -# Failure classes that are DOWNSTREAM of having no cached history: with zero bars they report -# on our data, not on the asset, so `assets holdings` must not print them as verdicts. +# Failure classes that are DOWNSTREAM of having no cached history: with zero bars `liquidity` +# reports on our data (median volume is 0 *because* there are no bars), not on the asset, so +# `assets holdings` must not print it as a verdict. `settlement` is deliberately NOT here -- it +# compares the product's quote leg to the settlement currency and never touches candles, so it +# stays a real, assessable verdict even with zero bars. # `settlement` is deliberately NOT here: since it compares the product's quote leg to the # configured settlement currency it no longer touches candles at all, so it is fully assessable # with zero bars -- suppressing it would hide a real, actionable failure. @@ -1317,7 +1320,7 @@ def monitor( repo = _open_repo(ctx) config = _load_cfg(ctx) broker = _build_broker(config) - products = [f"{asset}-USD" for asset in config.allowlist] + products = _default_sim_products(config) granularities = list(config.market_data.granularities) interval = interval_sec if interval_sec is not None else config.auto_trade.interval_sec @@ -1682,7 +1685,8 @@ def _json_plain(value: Any) -> Any: @click.option( "--products", default=None, - help="Comma-separated product ids (default: config.yaml's allowlist mapped to -USD pairs).", + help="Comma-separated product ids (default: the allowlist, in the configured " + "settlement currency).", ) @click.option( "--kinds", @@ -2051,7 +2055,8 @@ def _default_report_path(now_ts: int) -> Path: @click.option( "--products", default=None, - help="Comma-separated product ids (default: config.yaml's allowlist mapped to -USD pairs).", + help="Comma-separated product ids (default: the allowlist, in the configured " + "settlement currency).", ) @click.option( "--contribution", diff --git a/tests/compliance/test_assets_cli.py b/tests/compliance/test_assets_cli.py index 5848be24..d2e0167d 100644 --- a/tests/compliance/test_assets_cli.py +++ b/tests/compliance/test_assets_cli.py @@ -568,3 +568,49 @@ def test_an_account_with_no_currency_field_does_not_crash( assert result.exit_code == 0, result.output assert "BTC" in result.output + + +# -- product ids derive from the configured settlement currency ---------------- + + +def test_product_ids_derive_from_the_configured_settlement_currency(tmp_path): + """Regression for the legacy-config break: hardcoding `-USD` here while the settlement check + compared against config made every `quote_currency: USDC` deployment reject every asset on a + settlement failure it could never fix. Both derivations must share one source.""" + from keel.cli import _default_sim_products, _history_product + from keel.config import load_config + from tests.conftest import VALID_CONFIG_YAML + + assert _history_product("BTC", "USD") == "BTC-USD" + assert _history_product("BTC", "usdc") == "BTC-USDC" + + legacy = tmp_path / "legacy.yaml" + legacy.write_text(VALID_CONFIG_YAML.replace("quote_currency: USD", "quote_currency: USDC")) + assert all(p.endswith("-USDC") for p in _default_sim_products(load_config(str(legacy)))) + + default = tmp_path / "default.yaml" + default.write_text(VALID_CONFIG_YAML) + assert all(p.endswith("-USD") for p in _default_sim_products(load_config(str(default)))) + + +def test_the_settlement_criterion_still_catches_an_EXTERNALLY_supplied_product( + tmp_path, valid_config_path +): + """Honesty about what this criterion does. For a product WE derive it is true by + construction. It is not dead: it catches a product supplied from outside that derivation -- + `--products`, or a future venue/LLM-sourced list -- whose quote leg would need a second + exchange leg (a cross) to settle. That is the §65.7 case it exists for.""" + db_path = tmp_path / "t.db" + repo = _repo_at(db_path) + _seed_history(repo, "BTC-EUR") + runner = CliRunner() + assert _attest(runner, db_path, valid_config_path, "BTC").exit_code == 0 + + result = runner.invoke( + cli, + ["--db", str(db_path), "--config", str(valid_config_path), + "assets", "screen", "--products", "BTC-EUR"], # settlement is USD + ) + + assert "settlement" in result.output, "a cross-settled product must fail the settlement check" + assert "REJECT" in result.output diff --git a/tests/compliance/test_screen.py b/tests/compliance/test_screen.py index e05f8691..f529b081 100644 --- a/tests/compliance/test_screen.py +++ b/tests/compliance/test_screen.py @@ -232,3 +232,15 @@ def test_discovery_proposes_but_never_admits(): (candidate,) = discover_candidates([_product()]) result = screen_asset(_facts(asset=candidate.asset), None) assert result.admitted is False + + +def test_discovery_matches_the_quote_currency_case_insensitively(): + """`quote_currency: usd` must not silently propose nothing while the screen accepts the same + product -- the two comparisons have to agree.""" + from keel.compliance.screen import DiscoveryPolicy, discover_candidates + + lowercase_venue = _product(pid="SOL-USD", quote="usd") + assert discover_candidates([lowercase_venue]), "lowercase venue quote id dropped everything" + assert discover_candidates( + [_product(pid="SOL-USD", quote="USD")], DiscoveryPolicy(quote_currency="usd") + ), "lowercase configured quote currency dropped everything" diff --git a/tests/test_agent.py b/tests/test_agent.py index 0a0ced61..6e45c812 100644 --- a/tests/test_agent.py +++ b/tests/test_agent.py @@ -1263,3 +1263,24 @@ def get_accounts(self): assert equity >= Decimal("1000"), ( f"equity {equity} ignored the USD cash that funds BTC-USD orders" ) + + +def test_equity_is_a_total_when_only_SOME_currencies_are_readable(repo): + """`None` means 'equity is unknowable', reserved for when NOTHING is readable. A currency the + account simply has no wallet for must contribute nothing, not void the whole reading -- that + would return None on a perfectly ordinary account and stall rail 11's equity tracking.""" + + class OnlyUsd: + def get_accounts(self): + return [{"currency": "USD", "available_balance": Decimal("1000")}] + + equity = agent._mark_to_market_equity(repo, OnlyUsd(), ["BTC-USD"], {}, "USDC") + assert equity == Decimal("1000"), f"expected a total, got {equity!r}" + + +def test_equity_is_None_only_when_NOTHING_is_readable(repo): + class NoAccounts: + def get_accounts(self): + return [] + + assert agent._mark_to_market_equity(repo, NoAccounts(), ["BTC-USD"], {}, "USDC") is None From 2fc35320f7f022813df246b7f600469749f3516d Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Wed, 22 Jul 2026 01:46:36 -0400 Subject: [PATCH 5/5] fix: equity misses cash for held products; correct the comment that matters Third review round, all non-blocking. The operative stale comment was the one I had NOT fixed. The version above the constant was corrected last round, but the one at the suppression site still described the "quotable_in_settlement_currency degenerates to bool(candles)" fallback that this very PR deletes -- and that is the comment a maintainer reads before re-adding "settlement" to _DATA_DERIVED_FAILURES. No test can catch that (a derived product can never fail settlement), so the comment now says so explicitly. The duplicated paragraph is removed. Equity's currency scan covered only the rule products, while the valuation loop covers those plus held_products(). A rule retired while its position is still open leaves that position marked to market with its funding currency unseen -- an under-read, and the HWM is monotonic, so it arms rail 11 permanently. Now scanned symmetrically, and mutation-checked. Also: the veto-message assertion strips the rail's lowercase tag, which is the only reason the "USDC not in message" check passed. Co-Authored-By: Claude Opus 4.8 (1M context) --- keel/agent.py | 3 ++- keel/cli.py | 17 +++++++++-------- tests/execution/test_executor.py | 7 +++++-- tests/test_agent.py | 14 ++++++++++++++ 4 files changed, 30 insertions(+), 11 deletions(-) diff --git a/keel/agent.py b/keel/agent.py index 60f6fe3c..a0c76937 100644 --- a/keel/agent.py +++ b/keel/agent.py @@ -297,7 +297,8 @@ def _mark_to_market_equity( never falls, so an under-read arms the breaker on a phantom drawdown from then on). """ currencies: list[str] = [] - for candidate in (quote_currency, *(quote_currency_of(p) for p in products)): + scanned = (*products, *repo.held_products()) + for candidate in (quote_currency, *(quote_currency_of(p) for p in scanned)): upper = (candidate or "").upper() if upper and upper not in currencies: currencies.append(upper) diff --git a/keel/cli.py b/keel/cli.py index 7fc930b9..2a4bf66d 100644 --- a/keel/cli.py +++ b/keel/cli.py @@ -674,9 +674,6 @@ def _history_product(asset: str, quote: str) -> str: # `assets holdings` must not print it as a verdict. `settlement` is deliberately NOT here -- it # compares the product's quote leg to the settlement currency and never touches candles, so it # stays a real, assessable verdict even with zero bars. -# `settlement` is deliberately NOT here: since it compares the product's quote leg to the -# configured settlement currency it no longer touches candles at all, so it is fully assessable -# with zero bars -- suppressing it would hide a real, actionable failure. _DATA_DERIVED_FAILURES = frozenset({"liquidity"}) # Never candidates: you cannot trade the currency you settle in, and fiat is funding rather than @@ -779,11 +776,15 @@ def assets_holdings(ctx: click.Context, min_balance: str, run_screen: bool) -> N failures = result.failures if facts.daily_bars == 0: - # The likeliest misreading of this whole feature. With no cached bars the liquidity - # and settlement checks CANNOT say anything about the asset -- median volume is 0 - # because there are no bars, and `quotable_in_settlement_currency` degenerates to - # `bool(candles)`. Printing them as findings would assert about the asset exactly - # what this message exists to deny, so they are shown as derived, not as verdicts. + # The likeliest misreading of this whole feature. With no cached bars `liquidity` + # cannot say anything about the asset -- median volume is 0 *because* there are no + # bars -- so printing it as a finding would assert about the asset exactly what this + # message exists to deny. It is shown as derived, not as a verdict. + # NOTE: `settlement` is deliberately NOT suppressed. It compares the product's quote + # leg to the settlement currency and never reads candles, so it stays assessable at + # zero bars. Do not add it to `_DATA_DERIVED_FAILURES` -- no test would catch that + # here (a derived product can never fail settlement), and it would hide a real + # verdict on any externally supplied product. derived = [f for f in failures if f.split(":")[0] in _DATA_DERIVED_FAILURES] failures = [f for f in failures if f not in derived] click.echo( diff --git a/tests/execution/test_executor.py b/tests/execution/test_executor.py index a470d8ea..59f3c6c6 100644 --- a/tests/execution/test_executor.py +++ b/tests/execution/test_executor.py @@ -1388,8 +1388,11 @@ def test_the_veto_message_names_the_currency_actually_required(repo): ) message = next(v for v in result.vetoed_by if v.startswith("usdc_funding")) - assert "USD" in message - assert "USDC" not in message, f"message names the wrong currency: {message}" + # Strip the rail's own lowercase tag before matching currency codes -- otherwise + # `"USDC" not in message` passes only by accident of the tag's casing. + body = message.split(":", 1)[1] + assert "USD" in body + assert "USDC" not in body, f"message names the wrong currency: {message}" def test_no_account_at_all_for_the_required_currency_fails_closed(repo): diff --git a/tests/test_agent.py b/tests/test_agent.py index 6e45c812..4e8c8bac 100644 --- a/tests/test_agent.py +++ b/tests/test_agent.py @@ -1284,3 +1284,17 @@ def get_accounts(self): return [] assert agent._mark_to_market_equity(repo, NoAccounts(), ["BTC-USD"], {}, "USDC") is None + + +def test_equity_finds_cash_for_a_HELD_product_whose_rule_was_retired(repo, monkeypatch): + """The valuation loop already covers `held_products()`; the currency scan must too. A + retired rule leaves its position marked to market while the cash funding it goes unseen -- + an under-read, and the HWM never falls.""" + + class EurOnly: + def get_accounts(self): + return [{"currency": "EUR", "available_balance": Decimal("500")}] + + monkeypatch.setattr(repo, "held_products", lambda: ["BTC-EUR"]) + equity = agent._mark_to_market_equity(repo, EurOnly(), [], {}, "USD") + assert equity == Decimal("500"), f"cash for a held product's quote leg was missed: {equity!r}"