From 0a702140ce9ed0c53f467f8b09300099e0070ca8 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Tue, 11 Aug 2026 07:01:55 -0400 Subject: [PATCH] feat(compliance): attest the instrument wrapper, not just the underlying `AssetAttestation` is keyed on a base-leg symbol, so it can only ever describe the UNDERLYING. The honest attestation for the underlying of a BTC CFD is character-for-character BTC's existing spot one -- sector=payments, backing=native, pays_yield=False -- so the curation screen admitted it. Leverage, swap financing and counterparty exposure are properties of the CONTRACT, and no amount of care taken over the asset claim could ever surface them. Adds a separate `InstrumentAttestation` keyed per `(venue, product_id)` carrying a `wrapper` field. Admission now requires BOTH claims. Keyed on the product id rather than `(venue, asset)` because Coinbase -- the one venue keel already uses -- lists both `BTC-USD` and `BTC-PERP-USD` against the same base leg, so a per-asset wrapper claim would be factually wrong today, not merely imprecise once a second venue lands. The wrapper is attested, never computed. The id's shape cannot answer it: a cTrader CFD spells itself `BTC-USD` and `parse_spot_product_id` reads that as well-formed spot, which IS the gap. The venue's own `product_type` is its self-report about its own product, which makes it evidence to cite in `source` and not a substitute for a human's claim. `spot_instrument` (grammar) and `instrument_wrapper` (claim) are complementary and both fire for a derivative-shaped id attested as spot. `WAIVABLE_CRITERIA` stays `frozenset({"history"})` -- a waiver here would waive the charter, not a threshold. `instrument_wrapper` is deliberately NOT in `DATA_DERIVED_FAILURES`: like `settlement` it reads an attestation and never touches candles, so it stays assessable at zero bars. ACCEPTED CONSEQUENCE: `keel assets screen` now reports REJECT (`instrument_wrapper: UNATTESTED`) for every product, including BTC-USD/ETH-USD/ PAXG-USD, until the operator runs `keel assets attest-instrument` once per product. That is the fail-closed default and it is intended. Live trading is unaffected -- rail 1 gates live buys on `config.allowlist` directly, not on the screen. There is deliberately no backfill, no default-to-spot and no auto-attestation from venue metadata; the failure message names the exact command. Schema v10 adds `instrument_attestations`; the migration is stamp-only, because seeding `spot` rows for currently-allowlisted products would fabricate exactly the claim this gap is about, for the products the project is most likely to stop questioning. Closes #202 Co-Authored-By: Claude Opus 5 (1M context) --- keel/cli.py | 117 +++++++++++++- keel/compliance/screen.py | 119 ++++++++++++++- keel/data/db.py | 34 ++++- keel/data/repository.py | 44 ++++++ tests/commands/test_admission.py | 1 + tests/commands/test_tui.py | 1 + tests/compliance/test_assets_cli.py | 189 +++++++++++++++++++++++ tests/compliance/test_screen.py | 227 ++++++++++++++++++++++++++-- tests/data/test_db.py | 53 ++++++- tests/data/test_migrations.py | 2 +- tests/data/test_repository.py | 81 ++++++++++ tests/data/test_trade_outcomes.py | 4 +- tests/test_proposer.py | 8 +- 13 files changed, 851 insertions(+), 29 deletions(-) diff --git a/keel/cli.py b/keel/cli.py index 2ae5ebed..43cbe0aa 100644 --- a/keel/cli.py +++ b/keel/cli.py @@ -516,6 +516,24 @@ def assets_group() -> None: _LIQUIDITY_PROBE_DAYS = 180 +#: The venue every product screened here is listed on. +#: +#: ⚠️ A CONSTANT because it is currently a fact, not a configuration. The live path constructs +#: `keel/data/cb_client.py`'s `CoinbaseClient` directly, so there is exactly one venue these +#: product ids can mean, and an `InstrumentAttestation` is keyed on `(venue, product_id)` -- +#: which means the screen needs a venue id to look one up, and inventing a per-call parameter +#: for a value with one possible answer would be a knob whose only safe setting is its default. +#: +#: The broker-port migration replaces this with the adapter's own `BrokerCapabilities.venue` +#: (`packages/keel-broker-api/keel_broker_api/capabilities.py`), at which point the wrapper +#: statement recorded for `BTC-USD` on Coinbase correctly stops applying to `BTC-USD` somewhere +#: else -- which is issue #202's entire point and the reason the key is a pair. Until an adapter +#: handle actually reaches this function, reading a venue id off one would be reading it off +#: nothing: the same dead-gate pattern `capabilities.py` warns about, where a lookup that cannot +#: fail reads as a defence. +_VENUE = "coinbase" + + def _market_facts(repo: Repository, product: str, quote: str) -> screen_mod.MarketFacts: """Everything the screen can compute for itself from data we already hold.""" asset = product.split("-")[0] @@ -535,6 +553,8 @@ def _market_facts(repo: Repository, product: str, quote: str) -> screen_mod.Mark # Carried, not reduced: `screen_asset` applies rail 19's grammar to it, so the screen's # shape verdict and the rail's cannot disagree, and the verdict can name the id. product_id=product, + # The other half of the key the instrument statement is recorded under. See `_VENUE`. + venue=_VENUE, ) @@ -547,6 +567,11 @@ def _screen_product( all route through here, so none of them can drift onto a laxer path -- which is what makes "the same vetting process" a property of the code rather than an intention. Returns the facts alongside the verdict so a caller can explain WHY without recomputing them. + + The instrument statement is looked up HERE, next to the asset attestation, rather than being + threaded in by each caller -- that is what makes the wrapper criterion inherit the same + single-decision-point property as everything else on this path. Three callers get the new + check with no per-caller wiring, and none of them can be the one that forgot it. """ asset = product.split("-")[0] facts = _market_facts(repo, product, quote) @@ -564,8 +589,23 @@ def _screen_product( if raw is not None else None ) + raw_instrument = repo.get_instrument_attestation(_VENUE, product) + instrument = ( + screen_mod.InstrumentAttestation( + venue=raw_instrument["venue"], + product_id=raw_instrument["product_id"], + wrapper=raw_instrument["wrapper"], + source=raw_instrument["source"], + attested_by=raw_instrument["attested_by"], + attested_at=raw_instrument["attested_at"], + ) + if raw_instrument is not None + else None + ) waived = repo.get_screen_exceptions(asset) - return facts, screen_mod.screen_asset(facts, attestation, waived=waived) + return facts, screen_mod.screen_asset( + facts, attestation, waived=waived, instrument=instrument + ) # Never candidates: you cannot trade the currency you settle in, and fiat is funding rather than @@ -975,6 +1015,65 @@ def assets_attest( click.echo(f"attested {asset}: sector={sector} backing={backing} pays_yield={pays_yield}") +@assets_group.command("attest-instrument") +@click.option("--venue", default=_VENUE, show_default=True, help="Venue the product is listed on.") +@click.option("--product", required=True, help="Venue product id, e.g. BTC-USD.") +@click.option( + "--wrapper", + required=True, + type=click.Choice(sorted(screen_mod.KNOWN_WRAPPERS)), + help="What CONTRACT this listing is. Only 'spot' admits; every other value is a refusal.", +) +@click.option( + "--source", + required=True, + help="Where this was established: the venue's contract spec, its API docs, a filing.", +) +@click.option("--attested-by", required=True, help="Who established it.") +@click.pass_context +@with_disclaimer +def assets_attest_instrument( + ctx: click.Context, + venue: str, + product: str, + wrapper: str, + source: str, + attested_by: str, +) -> None: + """Record what CONTRACT a venue listing is. A claim about the PRODUCT, not the asset. + + `keel assets attest` says what the underlying is -- sector, backing, yield. This says what + you actually get when you buy this listing, and the two are genuinely independent: the honest + asset attestation for the underlying of a BTC CFD is BTC's existing, already-admitted one, so + nothing recorded there can ever surface the leverage, swap financing or counterparty exposure + that the CFD adds (issue #202). + + Keyed per `(venue, product)` rather than per asset because one venue lists several contracts + on the same base leg -- Coinbase quotes both `BTC-USD` and `BTC-PERP-USD` -- so a per-asset + wrapper claim would be wrong on the venue keel already uses, not merely imprecise later. + + This is ATTESTED and cannot be derived. The id's shape does not answer it (a CFD broker + spells its contract `BTC-USD`, identical to spot), and the venue's own `product_type` field + is its self-report about its own product -- excellent evidence to cite in `--source`, and not + a substitute for a human making the claim. + + Not passphrase-gated, for the same reason `keel assets attest` is not: an attestation cannot + itself place an order or raise a cap, and the screen it feeds only ever ADMITS to a list that + `guards.py` rail 1 still enforces per-trade. + """ + product = product.upper() # matches the uppercase ids `_screen_product` looks up by + repo = _open_repo(ctx) + repo.upsert_instrument_attestation( + venue=venue, + product_id=product, + wrapper=wrapper, + source=source, + attested_by=attested_by, + attested_at=int(time.time()), + ) + click.echo(f"attested {product} on {venue}: wrapper={wrapper}") + + @assets_group.command("exempt") @click.option("--asset", required=True, help="Asset code, e.g. PAXG.") @click.option( @@ -1052,11 +1151,16 @@ def assets_unexempt(ctx: click.Context, asset: str, criterion: str) -> None: @assets_group.command("list") @click.pass_context def assets_list(ctx: click.Context) -> None: - """List recorded attestations and any documented screen exceptions.""" + """List recorded attestations and any documented screen exceptions. + + Both KINDS of attestation are shown, because admission now requires both and an operator + reading only the asset list would see a fully-attested allowlist that still screens REJECT. + """ repo = _open_repo(ctx) rows = repo.get_asset_attestations() + instruments = repo.get_instrument_attestations() exceptions = repo.list_screen_exceptions() - if not rows and not exceptions: + if not rows and not instruments and not exceptions: click.echo("no attestations recorded") return for row in rows: @@ -1064,6 +1168,13 @@ def assets_list(ctx: click.Context) -> None: f"{row['asset']:<8} sector={row['sector']:<16} backing={row['backing']:<8} " f"pays_yield={bool(row['pays_yield'])!s:<5} by={row['attested_by']}" ) + if instruments: + click.echo("\ninstruments:") + for row in instruments: + click.echo( + f"{row['product_id']:<14} venue={row['venue']:<10} " + f"wrapper={row['wrapper']:<16} by={row['attested_by']}" + ) if exceptions: click.echo("\nexceptions:") for row in exceptions: diff --git a/keel/compliance/screen.py b/keel/compliance/screen.py index 2e5620f8..b974c70f 100644 --- a/keel/compliance/screen.py +++ b/keel/compliance/screen.py @@ -50,6 +50,14 @@ BACKING_NATIVE = "native" # a base-layer coin, neither a claim nor a warehouse receipt KNOWN_BACKINGS = frozenset({BACKING_AYN, BACKING_DAYN, BACKING_NATIVE}) +#: §71.4a: the allowlist is not juristically homogeneous, so admission has to name the CONTRACT, +#: not just the underlying. `spot` is the only wrapper this policy admits; every other name here +#: exists so that refusing it is an explicit, recorded classification rather than a shrug. +WRAPPER_SPOT = "spot" +KNOWN_WRAPPERS = frozenset( + {WRAPPER_SPOT, "cfd", "future", "perpetual", "option", "leveraged_token"} +) + #: Criteria a documented, human-recorded exception (`keel assets exempt`) may EVER waive. Only #: DATA/market criteria belong here -- history depth, liquidity, that kind of thing -- because #: those are facts about our own cache, not about the asset's shariah status. The shariah @@ -86,6 +94,39 @@ class AssetAttestation: attested_at: int +@dataclass(frozen=True) +class InstrumentAttestation: + """What CONTRACT a venue listing actually is. A separate claim from `AssetAttestation`. + + The two are complementary, and keeping them apart is the point rather than an accident of + layout. Sector, backing and yield are facts about the UNDERLYING -- they are true of BTC + wherever BTC is quoted. "What is this listing" is a fact about a VENUE'S PRODUCT, and the + honest asset attestation for the underlying of a BTC CFD is character-for-character BTC's + existing spot one: `sector=payments, backing=native, pays_yield=False`. That is issue #202 in + one sentence -- leverage, swap financing and counterparty exposure are properties of the + contract, so no amount of care taken over the asset claim can ever surface them. + + **Keyed on `product_id`, not on `(venue, asset)`.** Coinbase -- the one venue keel already + uses -- lists both `BTC-USD` and `BTC-PERP-USD` against the same base leg, so a per-asset + wrapper claim would be factually wrong today, not merely imprecise once a second venue lands. + The key has to be the thing being traded. + + **Attested, not computed, and that is the whole reason the type exists.** The id's shape + cannot answer it: a cTrader CFD spells itself `BTC-USD`, which is exactly the gap -- + `parse_spot_product_id` reads that as a well-formed spot id and is right to, because the + grammar is all it has. Nor is the venue's own metadata a substitute: `product_type` is the + venue's self-report about its own product, which makes it excellent INPUT to the human's + `source` and unacceptable as the claim itself. Fail closed, like every other attestation here. + """ + + venue: str + product_id: str + wrapper: str # one of KNOWN_WRAPPERS; only WRAPPER_SPOT admits + source: str # where this was established -- venue docs, a contract spec, a regulator filing + attested_by: str + attested_at: int + + @dataclass(frozen=True) class MarketFacts: """Everything the screen can compute for itself.""" @@ -104,6 +145,16 @@ class MarketFacts: #: so a construction site that forgets it must fail loudly at the call, not quietly at the #: verdict. product_id: str + #: The venue the product is listed on. Half of the key an `InstrumentAttestation` is recorded + #: under, and carried here so `screen_asset` can check that the statement on file is about + #: THIS listing rather than a same-named one elsewhere -- `BTC-USD` on Coinbase is spot and + #: `BTC-USD` on a CFD broker is not, and the id alone cannot tell them apart. + #: + #: NO default, for `product_id`'s reason exactly. A defaulted venue would have to name some + #: venue, and naming the venue keel currently trades on would make every forgotten call site + #: silently inherit "Coinbase, therefore spot" -- the fail-OPEN answer to the one question + #: this field was added to ask. A construction site that forgets it must fail at the call. + venue: str @dataclass(frozen=True) @@ -137,16 +188,23 @@ def screen_asset( attestation: AssetAttestation | None, policy: ScreenPolicy | None = None, waived: Mapping[str, str] | None = None, + instrument: InstrumentAttestation | None = None, ) -> ScreenResult: - """Deterministic admission decision. `attestation=None` fails closed. + """Deterministic admission decision. `attestation=None` and `instrument=None` both fail closed. + + TWO attestations are required, and they answer different questions. `attestation` says what + the UNDERLYING is; `instrument` says what the LISTING is. Either one missing is a rejection, + and both missing produce both failures in a single run rather than one at a time -- an + operator should learn every action they owe from one `keel assets screen`, not discover the + second only after satisfying the first. `waived` is `{criterion: rationale}` from a documented human exception (`keel assets exempt` / `repository.get_screen_exceptions`). It is consulted ONLY when a check would otherwise FAIL, and ONLY for criteria in `WAIVABLE_CRITERIA` -- a waiver for anything else - (a stray `screen_exceptions` row for, say, `attestation`) is silently ignored and that - criterion still fails closed. A waiver never affects any criterion other than its own, and a - blank/whitespace rationale is treated as no waiver at all (fail closed -- see the `.strip()` - check below, mirroring the unsourced-attestation guard further down). + (a stray `screen_exceptions` row for, say, `attestation` or `instrument_wrapper`) is silently + ignored and that criterion still fails closed. A waiver never affects any criterion other than + its own, and a blank/whitespace rationale is treated as no waiver at all (fail closed -- see + the `.strip()` check below, mirroring the unsourced-attestation guard further down). """ policy = policy or ScreenPolicy() # Filtered ONCE, up front, rather than inline per-branch: this is the actual defense-in-depth @@ -211,6 +269,57 @@ def screen_asset( "order for it" ) + # The ATTESTED half of the same question, and the reason `spot_instrument` above is not + # enough on its own. That check reads the id's GRAMMAR, which is all an id can offer and is + # exactly why it cannot close this gap: a cTrader CFD is spelled `BTC-USD`, parses clean, and + # is not spot. The two criteria are complementary, deliberately not merged, and both fire for + # a derivative-shaped id attested as spot -- a venue whose ids lie about the contract and a + # human who mis-states it are different failures, and collapsing them would let either hide + # behind the other. + # + # Not in `DATA_DERIVED_FAILURES`, for `settlement`'s reason: this consults an attestation and + # never touches candles, so it stays a real, assessable verdict at zero bars. A candidate we + # have never fetched is still one we can say "nobody has told us what contract this is" about. + # + # Not in `WAIVABLE_CRITERIA` either, and issue #202 says so explicitly. A waiver here would be + # a documented exception permitting a derivative, which is the charter, not a threshold. + if instrument is None or (instrument.venue, instrument.product_id) != ( + facts.venue, + facts.product_id, + ): + # A mismatch is treated as ABSENCE, not as a mismatch worth reporting in its own right. + # `_screen_product` looks the row up BY this pair, so the two can only diverge via a + # direct caller passing a statement about some other listing -- and a claim about a + # different product is not weaker evidence about this one, it is no evidence at all. + failures.append( + f"instrument_wrapper: UNATTESTED for {facts.product_id!r} on {facts.venue!r}. Which " + "CONTRACT a venue lists cannot be read off the id -- a CFD can spell itself exactly " + "like spot -- so an unattested listing is unknown, and unknown is a rejection (fail " + "closed). Record one with `keel assets attest-instrument`." + ) + else: + wrapper = instrument.wrapper.strip().lower() + if wrapper not in KNOWN_WRAPPERS: + failures.append( + f"instrument_wrapper: {wrapper!r} is not one of {sorted(KNOWN_WRAPPERS)} -- " + "classify it explicitly rather than leaving it open (§71.4a)" + ) + elif wrapper != WRAPPER_SPOT: + failures.append( + f"instrument_wrapper: {wrapper!r} -- keel is spot-only, and this listing is a " + "derivative on the underlying rather than the underlying itself. Leverage, swap " + "financing and counterparty exposure are properties of the CONTRACT, so they " + "survive any attestation about the asset: the base leg being admissible says " + "nothing about this wrapper (§65.6/§65.11, §71.4a)" + ) + if not instrument.source.strip(): + # Mirrors the unsourced-attestation guard below. Reported ALONGSIDE any wrapper + # verdict above rather than instead of it, because "spot, but nobody said where that + # came from" is precisely the unsourced claim that must not admit. + failures.append( + "instrument_wrapper: no source recorded -- an unsourced claim is not evidence" + ) + # -- attested shariah classification --------------------------------------- if attestation is None: failures.append( diff --git a/keel/data/db.py b/keel/data/db.py index 844d7483..a27be44b 100644 --- a/keel/data/db.py +++ b/keel/data/db.py @@ -19,7 +19,7 @@ from pathlib import Path from typing import Any -SCHEMA_VERSION = 9 +SCHEMA_VERSION = 10 # Creation order matters for readability (and for backends that validate FK targets eagerly); # SQLite itself only checks FK targets at DML time, but we still declare referenced tables first. @@ -239,6 +239,17 @@ ) """, """ + CREATE TABLE IF NOT EXISTS instrument_attestations ( + venue TEXT NOT NULL, + product_id TEXT NOT NULL, + wrapper TEXT NOT NULL, + source TEXT NOT NULL, + attested_by TEXT NOT NULL, + attested_at INTEGER NOT NULL, + PRIMARY KEY (venue, product_id) + ) + """, + """ CREATE TABLE IF NOT EXISTS screen_exceptions ( asset TEXT NOT NULL, criterion TEXT NOT NULL, @@ -420,6 +431,26 @@ def _migrate_v9_screen_exceptions(conn: sqlite3.Connection) -> None: """ +def _migrate_v10_instrument_attestations(conn: sqlite3.Connection) -> None: + """v10 adds `instrument_attestations`. Table creation is handled by `_SCHEMA_STATEMENTS`; + there is deliberately NO backfill. + + A row asserts a human established what CONTRACT a given venue listing actually is (spot, + CFD, perpetual, ...) against a named source. Seeding `spot` rows for the currently-allowlisted + products would fabricate exactly the claim this gap exists to demand -- and would do it for + the products the project is most likely to stop questioning. + + Like v9, this is a genuine no-op migration: `migrate()` runs every `_SCHEMA_STATEMENTS` + statement (all `IF NOT EXISTS`) before the version loop below, so a database already stamped + at v9 picks the table up from that pass alone. This step only exists to advance the stamp. + + An empty table correctly says nothing has been attested yet -- which, because the screen + fails closed on a missing instrument attestation, means every product reports REJECT until + the operator runs `keel assets attest-instrument`. That is the intended fail-closed default, + not a regression. + """ + + _MIGRATIONS: dict[int, Callable[[sqlite3.Connection], None]] = { 2: _migrate_v2_broker_subscriptions, 3: _migrate_v3_trade_outcomes, @@ -429,6 +460,7 @@ def _migrate_v9_screen_exceptions(conn: sqlite3.Connection) -> None: 7: _migrate_v7_profile, 8: _migrate_v8_autonomy_expiry, 9: _migrate_v9_screen_exceptions, + 10: _migrate_v10_instrument_attestations, } diff --git a/keel/data/repository.py b/keel/data/repository.py index 63f1ac93..9d187da1 100644 --- a/keel/data/repository.py +++ b/keel/data/repository.py @@ -742,6 +742,50 @@ def get_asset_attestations(self) -> list[dict]: rows = self._conn.execute("SELECT * FROM asset_attestations ORDER BY asset").fetchall() return [dict(row) for row in rows] + # -- instrument attestations -------------------------------------------------- + # Human-recorded claim about WHAT CONTRACT a venue listing is (spot/cfd/perpetual/future/ + # option/leveraged_token) -- keyed per (venue, product_id) rather than per asset, because one + # venue lists both BTC-USD (spot) and BTC-PERP-USD (perpetual) against the same base leg, so a + # per-asset wrapper claim would be factually wrong. Absent = unknown = rejected; see + # `keel/compliance/screen.py`. + + def upsert_instrument_attestation( + self, + venue: str, + product_id: str, + wrapper: str, + source: str, + attested_by: str, + attested_at: int, + ) -> None: + self._conn.execute( + """ + INSERT INTO instrument_attestations + (venue, product_id, wrapper, source, attested_by, attested_at) + VALUES (?, ?, ?, ?, ?, ?) + ON CONFLICT(venue, product_id) DO UPDATE SET + wrapper = excluded.wrapper, + source = excluded.source, + attested_by = excluded.attested_by, + attested_at = excluded.attested_at + """, + (venue, product_id, wrapper, source, attested_by, attested_at), + ) + self._conn.commit() + + def get_instrument_attestation(self, venue: str, product_id: str) -> dict | None: + row = self._conn.execute( + "SELECT * FROM instrument_attestations WHERE venue = ? AND product_id = ?", + (venue, product_id), + ).fetchone() + return dict(row) if row is not None else None + + def get_instrument_attestations(self) -> list[dict]: + rows = self._conn.execute( + "SELECT * FROM instrument_attestations ORDER BY venue, product_id" + ).fetchall() + return [dict(row) for row in rows] + # -- screen exceptions ------------------------------------------------------ # Documented, per-asset per-criterion waivers of an allowlist-screen admission criterion (KB # PAXG/history case). See `keel/compliance/screen.py` -- only criteria in `WAIVABLE_CRITERIA` diff --git a/tests/commands/test_admission.py b/tests/commands/test_admission.py index a2183588..792add02 100644 --- a/tests/commands/test_admission.py +++ b/tests/commands/test_admission.py @@ -245,6 +245,7 @@ def fake_screen(repo: Repository, product: str, quote: str): median_daily_volume=Decimal("2000000"), quotable_in_settlement_currency=True, product_id=product, + venue="coinbase", ) result = screen_mod.ScreenResult(asset=facts.asset, admitted=True) return facts, result diff --git a/tests/commands/test_tui.py b/tests/commands/test_tui.py index b0585b0f..e25b5d6a 100644 --- a/tests/commands/test_tui.py +++ b/tests/commands/test_tui.py @@ -1914,6 +1914,7 @@ def _screen(repo: Repository, product: str, quote: str): median_daily_volume=Decimal("2000000"), quotable_in_settlement_currency=True, product_id=product, + venue="coinbase", ) result = screen_mod.ScreenResult(asset=facts.asset, admitted=admitted) return facts, result diff --git a/tests/compliance/test_assets_cli.py b/tests/compliance/test_assets_cli.py index cccc98db..a3207a1e 100644 --- a/tests/compliance/test_assets_cli.py +++ b/tests/compliance/test_assets_cli.py @@ -58,6 +58,22 @@ def _attest(runner, db_path, config_path, asset, **over): ) +def _attest_instrument(runner, db_path, config_path, product, **over): + args = { + "--product": product, + "--wrapper": "spot", + "--source": "coinbase product spec", + "--attested-by": "tester", + } + args.update(over) + flat = [item for pair in args.items() for item in pair] + return runner.invoke( + cli, + ["--db", str(db_path), "--config", str(config_path), + "assets", "attest-instrument", *flat], + ) + + def test_an_unattested_asset_is_rejected_even_with_perfect_market_data( tmp_path, valid_config_path ): @@ -84,6 +100,9 @@ def test_attesting_admits_an_otherwise_clean_asset(tmp_path, valid_config_path): runner = CliRunner() for asset in ("BTC", "ETH", "PAXG"): assert _attest(runner, db_path, valid_config_path, asset).exit_code == 0 + assert _attest_instrument( + runner, db_path, valid_config_path, f"{asset}-USD" + ).exit_code == 0 result = runner.invoke( cli, ["--db", str(db_path), "--config", str(valid_config_path), "assets", "screen"] @@ -186,6 +205,9 @@ def test_exempt_admits_a_history_failing_asset_and_screen_prints_WAIVED( runner = CliRunner() attested = _attest(runner, db_path, valid_config_path, "PAXG", **{"--backing": "ayn"}) assert attested.exit_code == 0 + assert _attest_instrument( + runner, db_path, valid_config_path, "PAXG-USD" + ).exit_code == 0 # Before the exception: REJECT on history. before = runner.invoke( @@ -247,6 +269,9 @@ def test_exempt_normalizes_a_lowercase_asset_so_screening_still_finds_the_waiver runner = CliRunner() attested = _attest(runner, db_path, valid_config_path, "PAXG", **{"--backing": "ayn"}) assert attested.exit_code == 0 + assert _attest_instrument( + runner, db_path, valid_config_path, "PAXG-USD" + ).exit_code == 0 result = _exempt(runner, db_path, valid_config_path, **{"--asset": "paxg"}) assert result.exit_code == 0, result.output @@ -283,6 +308,9 @@ def test_unexempt_revokes_and_screen_rejects_again(tmp_path, valid_config_path): runner = CliRunner() attested = _attest(runner, db_path, valid_config_path, "PAXG", **{"--backing": "ayn"}) assert attested.exit_code == 0 + assert _attest_instrument( + runner, db_path, valid_config_path, "PAXG-USD" + ).exit_code == 0 assert _exempt(runner, db_path, valid_config_path).exit_code == 0 admitted = runner.invoke( @@ -562,6 +590,7 @@ def test_holdings_screen_agrees_with_assets_screen_for_the_same_asset( _seed_history(repo, "BTC-USD") runner = CliRunner() assert _attest(runner, db_path, valid_config_path, "BTC").exit_code == 0 + assert _attest_instrument(runner, db_path, valid_config_path, "BTC-USD").exit_code == 0 _with_broker(monkeypatch, _FakeBroker([_account("BTC", "0.5")])) screened = runner.invoke( @@ -825,6 +854,7 @@ def test_the_derived_failure_tags_actually_match_screen_asset_output(): median_daily_volume=Decimal(0), quotable_in_settlement_currency=False, product_id="SOL-EUR", + venue="coinbase", ) tags = {f.split(":")[0] for f in screen_mod.screen_asset(facts, None).failures} @@ -845,6 +875,7 @@ def test_a_lowercase_holding_is_screened_as_the_attested_uppercase_asset( _seed_history(repo, "BTC-USD") runner = CliRunner() assert _attest(runner, db_path, valid_config_path, "BTC").exit_code == 0 + assert _attest_instrument(runner, db_path, valid_config_path, "BTC-USD").exit_code == 0 _with_broker(monkeypatch, _FakeBroker([_account("btc", "0.5")])) result = _holdings(db_path, valid_config_path, "--screen") @@ -985,6 +1016,7 @@ def test_screen_still_ADMITS_a_well_formed_spot_pair(tmp_path, valid_config_path _seed_history(repo, "BTC-USD") runner = CliRunner() assert _attest(runner, db_path, valid_config_path, "BTC").exit_code == 0 + assert _attest_instrument(runner, db_path, valid_config_path, "BTC-USD").exit_code == 0 result = runner.invoke( cli, @@ -997,6 +1029,162 @@ def test_screen_still_ADMITS_a_well_formed_spot_pair(tmp_path, valid_config_path assert "spot_instrument" not in result.output +# -- instrument attestations (issue #202): the LISTING is a separate claim from the ASSET -------- +# +# `keel assets attest` says what the underlying is; `keel assets attest-instrument` says what +# CONTRACT this venue listing actually is. Admission now needs both, and the point of splitting +# them is that a spot-admissible underlying says nothing about a CFD/perp/future wrapped around +# it -- leverage, swap financing and counterparty exposure are properties of the CONTRACT. + + +def test_screen_REJECTS_a_fully_asset_attested_product_with_no_instrument_attestation( + tmp_path, valid_config_path +): + """The fail-closed default has to be ACTIONABLE, not just correct. An operator who has done + the asset-side work (sector, backing, source) and sees REJECT must be told the ONE remaining + thing they owe -- the exact command, not just the word 'unattested' -- or the fail-closed + default becomes a dead end instead of a checklist.""" + db_path = tmp_path / "t.db" + repo = _repo_at(db_path) + _seed_history(repo, "BTC-USD") + 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-USD"], + ) + + assert result.exit_code == 0, result.output + assert "REJECT" in result.output + assert "instrument_wrapper" in result.output + assert "keel assets attest-instrument" in result.output, ( + "a missing instrument attestation must name the exact remedy, not just say 'unattested'" + ) + + +def test_asset_and_spot_instrument_attestation_together_ADMIT(tmp_path, valid_config_path): + """The two attestations are complementary, not redundant -- both must be present to admit, + and both present with `--wrapper spot` is precisely the case that should clear the gate.""" + db_path = tmp_path / "t.db" + repo = _repo_at(db_path) + _seed_history(repo, "BTC-USD") + runner = CliRunner() + assert _attest(runner, db_path, valid_config_path, "BTC").exit_code == 0 + assert _attest_instrument(runner, db_path, valid_config_path, "BTC-USD").exit_code == 0 + + result = runner.invoke( + cli, + ["--db", str(db_path), "--config", str(valid_config_path), + "assets", "screen", "--products", "BTC-USD"], + ) + + assert result.exit_code == 0, result.output + assert "ADMIT" in result.output + + +def test_a_cfd_wrapper_on_an_admissible_underlying_still_REJECTS(tmp_path, valid_config_path): + """Issue #202's acceptance case at the CLI level: the underlying (BTC, spot-admissible) is + fully attested and would ADMIT as spot, but this listing is attested as a CFD. The contract, + not the underlying, is what this criterion polices -- leverage, swap financing and + counterparty exposure survive any care taken over the asset attestation, so ADMIT here would + be exactly the leak issue #202 was filed to close.""" + db_path = tmp_path / "t.db" + repo = _repo_at(db_path) + _seed_history(repo, "BTC-USD") + runner = CliRunner() + assert _attest(runner, db_path, valid_config_path, "BTC").exit_code == 0 + assert _attest_instrument( + runner, db_path, valid_config_path, "BTC-USD", **{"--wrapper": "cfd"} + ).exit_code == 0 + + result = runner.invoke( + cli, + ["--db", str(db_path), "--config", str(valid_config_path), + "assets", "screen", "--products", "BTC-USD"], + ) + + assert result.exit_code == 0, result.output + assert "REJECT" in result.output + assert "instrument_wrapper" in result.output + assert "'cfd'" in result.output, "the verdict must name the wrapper it refused" + + +def test_attest_instrument_rejects_an_unknown_wrapper_at_the_cli_boundary( + tmp_path, valid_config_path +): + """`--wrapper` is a `click.Choice` driven by `screen_mod.KNOWN_WRAPPERS` -- a made-up wrapper + name must be refused at the keyboard, the same boundary `assets attest --backing` and `assets + exempt --criterion` already enforce for their own Choice-typed options, rather than being + recorded and only failing later at screen time.""" + from keel.compliance import screen as screen_mod + + assert "banana" not in screen_mod.KNOWN_WRAPPERS, "fixture must actually be an unknown value" + + # The Choice must be DRIVEN by `screen_mod.KNOWN_WRAPPERS`, not a second, hardcoded list that + # could silently drift from it -- inspect the live Click param rather than assuming. + attest_instrument_cmd = cli.commands["assets"].commands["attest-instrument"] + wrapper_param = next(p for p in attest_instrument_cmd.params if p.name == "wrapper") + assert set(wrapper_param.type.choices) == screen_mod.KNOWN_WRAPPERS + + db_path = tmp_path / "t.db" + _repo_at(db_path) + + result = _attest_instrument( + CliRunner(), db_path, valid_config_path, "BTC-USD", **{"--wrapper": "banana"} + ) + + assert result.exit_code != 0 + assert "--wrapper" in result.output + + +def test_assets_list_renders_an_instrument_attestation(tmp_path, valid_config_path): + """Both KINDS of attestation must be visible from `assets list`, or an operator reading only + the asset half sees a fully-attested allowlist that still screens REJECT for a reason the + listing never shows them.""" + db_path = tmp_path / "t.db" + _repo_at(db_path) + runner = CliRunner() + assert _attest_instrument(runner, db_path, valid_config_path, "BTC-USD").exit_code == 0 + + result = runner.invoke( + cli, ["--db", str(db_path), "--config", str(valid_config_path), "assets", "list"] + ) + + assert "instruments:" in result.output + assert "BTC-USD" in result.output + assert "coinbase" in result.output + assert "spot" in result.output + + +def test_attest_instrument_normalizes_a_lowercase_product_so_screening_still_finds_it( + tmp_path, valid_config_path +): + """Mirrors `test_exempt_normalizes_a_lowercase_asset_so_screening_still_finds_the_waiver`: a + `--product btc-usd` statement must not silently no-op against the uppercase `BTC-USD` that + `_screen_product` looks the row up by -- an operator who typed the id in lowercase must not + be told UNATTESTED for a statement they already recorded.""" + db_path = tmp_path / "t.db" + repo = _repo_at(db_path) + _seed_history(repo, "BTC-USD") + runner = CliRunner() + assert _attest(runner, db_path, valid_config_path, "BTC").exit_code == 0 + + result = _attest_instrument( + runner, db_path, valid_config_path, "btc-usd" + ) + assert result.exit_code == 0, result.output + assert "BTC-USD" in result.output + + screened = runner.invoke( + cli, + ["--db", str(db_path), "--config", str(valid_config_path), + "assets", "screen", "--products", "BTC-USD"], + ) + assert "ADMIT" in screened.output + + # -- assets propose ----------------------------------------------------------------------------- @@ -1058,6 +1246,7 @@ def test_propose_and_screen_agree_for_the_same_asset(tmp_path, valid_config_path _seed_history(repo, "BTC-USD") runner = CliRunner() assert _attest(runner, db_path, valid_config_path, "BTC").exit_code == 0 + assert _attest_instrument(runner, db_path, valid_config_path, "BTC-USD").exit_code == 0 shortlist = _write_shortlist( tmp_path, [{"asset": "BTC", "rationale": "reserve asset", "sources": ["https://bitcoin.org"]}] ) diff --git a/tests/compliance/test_screen.py b/tests/compliance/test_screen.py index 60762788..86893598 100644 --- a/tests/compliance/test_screen.py +++ b/tests/compliance/test_screen.py @@ -7,7 +7,10 @@ import pytest from keel.compliance.screen import ( + KNOWN_WRAPPERS, + WAIVABLE_CRITERIA, AssetAttestation, + InstrumentAttestation, MarketFacts, ScreenPolicy, missing_history_lines, @@ -15,14 +18,30 @@ split_failures, ) +_VENUE = "coinbase" -def _facts(asset="BTC", bars=2000, volume="50000000", quotable=True, product=None) -> MarketFacts: + +def _facts( + asset="BTC", bars=2000, volume="50000000", quotable=True, product=None, venue=_VENUE +) -> MarketFacts: return MarketFacts( asset=asset, daily_bars=bars, median_daily_volume=Decimal(volume), quotable_in_settlement_currency=quotable, product_id=product if product is not None else f"{asset}-USD", + venue=venue, + ) + + +def _instrument(venue=_VENUE, product="BTC-USD", wrapper="spot", source="venue product spec"): + return InstrumentAttestation( + venue=venue, + product_id=product, + wrapper=wrapper, + source=source, + attested_by="tester", + attested_at=1_784_505_600, ) @@ -39,7 +58,7 @@ def _attestation(asset="BTC", sector="payments", backing="native", yield_=False, def test_a_clean_asset_is_admitted(): - result = screen_asset(_facts(), _attestation()) + result = screen_asset(_facts(), _attestation(), instrument=_instrument()) assert result.admitted is True assert result.failures == [] @@ -61,13 +80,183 @@ def test_a_missing_attestation_is_a_REJECTION_not_a_default_pass(): def test_a_missing_attestation_short_circuits_the_shariah_checks(): """No sector/backing VERDICT is invented for an asset nobody has classified. - Asserted as "exactly one failure, and it is the attestation one" rather than by grepping for - 'backing' -- the missing-attestation message legitimately mentions backing while explaining - why it cannot be judged. + This test's INTENT is the short-circuit -- that `haram_sector`, `riba_yield` and `backing` + produce no verdict at all when there is no attestation to judge. It used to assert that as + "exactly one failure", which was a proxy that stopped being true once a second, independent + missing-claim criterion existed. The intent is now asserted directly, by tag: no shariah tag + appears. (Grepping the message TEXT for 'backing' would false-positive -- the + missing-attestation message legitimately mentions backing while explaining why it cannot be + judged, which is exactly why the original test counted instead.) + + Both missing-class failures are expected together, and that is deliberate rather than + tolerated: an operator who runs `keel assets screen` once should be told BOTH claims they owe + -- the underlying's and the listing's -- not discover the second only after recording the + first and re-running. """ - result = screen_asset(_facts(), None) - assert len(result.failures) == 1 - assert result.failures[0].startswith("attestation: MISSING") + result = screen_asset(_facts(), None, instrument=None) + tags = [f.split(":")[0] for f in result.failures] + assert "haram_sector" not in tags + assert "riba_yield" not in tags + assert "backing" not in tags + assert sorted(tags) == ["attestation", "instrument_wrapper"] + + +def test_an_admitted_underlying_does_not_admit_an_unstated_wrapper(): + """Issue #202, the whole point: an honest attestation about BTC does not say what a BTC + listing IS. + + `sector=payments, backing=native, pays_yield=False` is a true claim about the underlying, and + it is equally true of spot BTC and of a BTC CFD -- swap financing, leverage and counterparty + exposure are properties of the CONTRACT, not of the coin. So an asset attestation alone must + not be able to admit anything, and with no instrument statement on file the screen fails + closed exactly as it does for a missing asset attestation. + """ + result = screen_asset(_facts(), _attestation()) + assert result.admitted is False + assert any(f.startswith("instrument_wrapper:") for f in result.failures) + + +def test_a_cfd_on_an_admitted_underlying_is_refused(): + """Issue #202's ACCEPTANCE case, and the one that used to pass silently. + + A cTrader-style CFD spells itself `BTC-USD` -- identical to Coinbase's spot id, so the shape + check passes it -- and its underlying's honest attestation is BTC's existing admitted one. It + is the wrapper claim, and nothing else in this module, that refuses it. + """ + facts = _facts(venue="ctrader") + result = screen_asset( + facts, + _attestation(), + instrument=_instrument(venue="ctrader", wrapper="cfd"), + ) + assert result.admitted is False + assert any(f.startswith("instrument_wrapper: 'cfd'") for f in result.failures) + # The shape check is NOT what caught it -- the id is a well-formed spot id, which is the gap. + assert not any(f.startswith("spot_instrument") for f in result.failures) + + +def test_an_attested_spot_listing_is_admitted(): + result = screen_asset(_facts(), _attestation(), instrument=_instrument(wrapper="spot")) + assert result.admitted is True + assert result.failures == [] + + +@pytest.mark.parametrize("wrapper", sorted(KNOWN_WRAPPERS - {"spot"})) +def test_no_known_wrapper_other_than_spot_admits(wrapper): + """Every name in the vocabulary except `spot` is a refusal. The vocabulary exists so that + refusing is an explicit classification, not so that some of its entries are tolerated.""" + result = screen_asset(_facts(), _attestation(), instrument=_instrument(wrapper=wrapper)) + assert result.admitted is False + assert any(f.startswith(f"instrument_wrapper: {wrapper!r}") for f in result.failures) + + +def test_an_unknown_wrapper_must_be_classified_not_assumed(): + """Mirrors the unknown-backing branch: an unrecognised name is not a pass.""" + result = screen_asset( + _facts(), _attestation(), instrument=_instrument(wrapper="probably spot") + ) + assert result.admitted is False + assert any("instrument_wrapper" in f and "not one of" in f for f in result.failures) + + +@pytest.mark.parametrize("wrapper", [" SPOT ", "Spot", "spot\n"]) +def test_the_wrapper_is_normalised_before_it_is_judged(wrapper): + """Same `.strip().lower()` treatment `sector` and `backing` already get -- a stored 'SPOT' + must not read as an unknown wrapper and reject an honestly-attested spot listing.""" + result = screen_asset(_facts(), _attestation(), instrument=_instrument(wrapper=wrapper)) + assert result.admitted is True + + +def test_a_statement_about_a_different_venue_is_no_evidence_about_this_one(): + """The identity check. A `BTC-USD` spot claim made about Coinbase says nothing about + `BTC-USD` on a CFD broker -- treating it as evidence is precisely the confusion #202 is + about, so a mismatch is handled as ABSENCE and fails closed.""" + result = screen_asset( + _facts(venue="ctrader"), + _attestation(), + instrument=_instrument(venue="coinbase", wrapper="spot"), + ) + assert result.admitted is False + assert any("instrument_wrapper: UNATTESTED" in f for f in result.failures) + assert any("'ctrader'" in f for f in result.failures) + + +def test_a_statement_about_a_different_product_is_no_evidence_about_this_one(): + """The same identity check on the other half of the key: Coinbase lists both `BTC-USD` and + `BTC-PERP-USD`, so a spot claim about one must not travel to the other.""" + result = screen_asset( + _facts(product="BTC-PERP-USD"), + _attestation(), + instrument=_instrument(product="BTC-USD", wrapper="spot"), + ) + assert result.admitted is False + assert any("instrument_wrapper: UNATTESTED" in f for f in result.failures) + + +@pytest.mark.parametrize("blank", ["", " ", "\t\n"]) +def test_an_unsourced_instrument_attestation_is_refused(blank): + """Mirrors the unsourced asset-attestation guard: 'spot, but nobody said where that came + from' is an unsourced claim, and an unsourced claim is not evidence.""" + result = screen_asset( + _facts(), _attestation(), instrument=_instrument(wrapper="spot", source=blank) + ) + assert result.admitted is False + assert any("instrument_wrapper: no source recorded" in f for f in result.failures) + + +def test_a_derivative_shaped_id_attested_as_spot_fails_BOTH_checks(): + """The two instrument criteria are complementary, and neither is allowed to hide behind the + other. A venue whose id says `BTC-PERP-USD` while a human attests `spot` is two separate + problems -- the shape and the claim disagree -- and the operator should see both.""" + result = screen_asset( + _facts(product="BTC-PERP-USD"), + _attestation(), + instrument=_instrument(product="BTC-PERP-USD", wrapper="spot"), + ) + assert result.admitted is False + assert any(f.startswith("spot_instrument") for f in result.failures) + + +def test_instrument_wrapper_is_never_waivable(): + """Issue #202 is explicit that no criterion it adds may be waivable, and `WAIVABLE_CRITERIA` + is pinned here rather than merely spot-checked: a documented exception permitting a + derivative would waive the charter, not a threshold.""" + assert WAIVABLE_CRITERIA == frozenset({"history"}) + + +def test_a_stray_waiver_for_instrument_wrapper_is_ignored_and_fails_closed(): + """Defence in depth for the pin above: even if a row reached `screen_exceptions` by hand, + the up-front `WAIVABLE_CRITERIA` filter drops it before any branch can read it.""" + result = screen_asset( + _facts(venue="ctrader"), + _attestation(), + waived={"instrument_wrapper": "someone tried to waive this"}, + instrument=_instrument(venue="ctrader", wrapper="cfd"), + ) + assert result.admitted is False + assert any(f.startswith("instrument_wrapper: 'cfd'") for f in result.failures) + assert not any("WAIVED" in w for w in result.warnings) + + +def test_the_wrapper_verdict_is_still_assessable_at_zero_bars(): + """Like `settlement`, this criterion reads an attestation and never touches candles, so it + stays a real verdict about the LISTING even with an empty cache -- it must not be suppressed + as "about our data" the way `history`/`liquidity` legitimately are.""" + from keel.compliance.screen import DATA_DERIVED_FAILURES + + assert "instrument_wrapper" not in DATA_DERIVED_FAILURES + facts = _facts(bars=0, volume="0") + result = screen_asset(facts, _attestation(), instrument=None) + about_the_asset, about_our_cache = split_failures(facts, result) + assert any(f.startswith("instrument_wrapper") for f in about_the_asset) + assert not any(f.startswith("instrument_wrapper") for f in about_our_cache) + + +def test_the_unattested_message_names_the_command_that_fixes_it(): + """The fail-closed default REJECTs every product until the operator attests it, so the + failure has to carry the exact remedy -- that message is how they are told.""" + result = screen_asset(_facts(), _attestation(), instrument=None) + assert any("keel assets attest-instrument" in f for f in result.failures) def test_an_unsourced_attestation_is_refused(): @@ -115,7 +304,11 @@ def test_an_unknown_backing_must_be_classified_not_assumed(): def test_asset_backed_is_admitted_but_warns_about_the_stricter_sarf_regime(): """PAXG's case: admitted, but §65.5's no-deferment rule is surfaced, not buried.""" - result = screen_asset(_facts(asset="PAXG"), _attestation(asset="PAXG", backing="ayn")) + result = screen_asset( + _facts(asset="PAXG"), + _attestation(asset="PAXG", backing="ayn"), + instrument=_instrument(product="PAXG-USD"), + ) assert result.admitted is True assert any("bay' al-sarf" in w for w in result.warnings) @@ -214,7 +407,10 @@ def test_every_failure_is_reported_not_just_the_first(): def test_policy_thresholds_are_configurable(): lenient = ScreenPolicy(min_daily_bars=100, min_median_daily_volume=Decimal("1")) - assert screen_asset(_facts(bars=200, volume="5"), _attestation(), lenient).admitted is True + result = screen_asset( + _facts(bars=200, volume="5"), _attestation(), lenient, instrument=_instrument() + ) + assert result.admitted is True # -- documented allowlist-screen exceptions (waivers) -------------------------- @@ -233,7 +429,10 @@ def test_insufficient_history_with_no_waiver_still_rejects(): def test_a_documented_history_waiver_admits_and_warns_loudly(): result = screen_asset( - _facts(bars=400), _attestation(), waived={"history": "PAXG: 441 bars, human-reviewed"} + _facts(bars=400), + _attestation(), + waived={"history": "PAXG: 441 bars, human-reviewed"}, + instrument=_instrument(), ) assert result.admitted is True assert not any("history" in f for f in result.failures) @@ -254,7 +453,10 @@ def test_a_blank_rationale_waiver_does_not_admit_undocumented_is_not_documented( def test_a_waiver_is_self_retiring_once_history_clears_the_floor(): """No leftover warning once the underlying condition it was granted for no longer holds.""" - result = screen_asset(_facts(bars=2000), _attestation(), waived={"history": "stale reason"}) + result = screen_asset( + _facts(bars=2000), _attestation(), waived={"history": "stale reason"}, + instrument=_instrument(), + ) assert result.admitted is True assert not any("WAIVED" in w for w in result.warnings) assert not any("history" in f for f in result.failures) @@ -291,6 +493,7 @@ def test_a_stray_non_waivable_key_alongside_a_real_waiver_is_dropped_not_honored _facts(bars=400), _attestation(), waived={"history": "documented reason", "settlement": "someone tried to waive this too"}, + instrument=_instrument(), ) assert result.admitted is True assert any("WAIVED" in w and "documented reason" in w for w in result.warnings) diff --git a/tests/data/test_db.py b/tests/data/test_db.py index 2315e252..1a332ee8 100644 --- a/tests/data/test_db.py +++ b/tests/data/test_db.py @@ -103,11 +103,11 @@ def test_agent_state_table_has_key_primary_key(): assert pk_columns == {"key"} -def test_schema_version_is_9(): +def test_schema_version_is_10(): """Deliberate tripwire: bump this literal consciously on every schema change.""" from keel.data.db import SCHEMA_VERSION - assert SCHEMA_VERSION == 9 + assert SCHEMA_VERSION == 10 def test_a_v6_database_migrates_up_and_gains_the_profile_table(tmp_path): @@ -215,3 +215,52 @@ def test_migrating_a_v8_database_twice_is_idempotent(tmp_path): assert int(conn.execute("SELECT version FROM schema_version").fetchone()["version"]) == ( SCHEMA_VERSION ) + + +def test_a_fresh_database_has_the_instrument_attestations_table(): + conn = connect(":memory:") + + migrate(conn) + + named = conn.execute( + "SELECT name FROM sqlite_master WHERE type='table' AND name='instrument_attestations'" + ).fetchone() + assert named is not None + + +def test_a_v9_database_migrates_up_and_gains_the_instrument_attestations_table(tmp_path): + """v10 is a documented no-op migration, same shape as v9: `_SCHEMA_STATEMENTS` (IF NOT + EXISTS) already creates the new table on an existing v9 DB before the version loop runs, so + the migration step itself has nothing to do.""" + from keel.data.db import SCHEMA_VERSION, connect, migrate + + conn = connect(str(tmp_path / "v9.db")) + migrate(conn) + conn.execute("UPDATE schema_version SET version = 9") + conn.commit() + + migrate(conn) + + assert int(conn.execute("SELECT version FROM schema_version").fetchone()["version"]) == ( + SCHEMA_VERSION + ) + named = conn.execute( + "SELECT name FROM sqlite_master WHERE type='table' AND name='instrument_attestations'" + ).fetchone() + assert named is not None, "v10 must add the instrument_attestations table" + (count,) = conn.execute("SELECT COUNT(*) FROM instrument_attestations").fetchone() + assert count == 0, "no backfill: seeding rows would fabricate an attestation nobody made" + + +def test_migrating_a_v9_database_twice_is_idempotent(tmp_path): + from keel.data.db import SCHEMA_VERSION, connect, migrate + + conn = connect(str(tmp_path / "v9_twice.db")) + migrate(conn) + conn.execute("UPDATE schema_version SET version = 9") + conn.commit() + migrate(conn) + migrate(conn) # must not raise + assert int(conn.execute("SELECT version FROM schema_version").fetchone()["version"]) == ( + SCHEMA_VERSION + ) diff --git a/tests/data/test_migrations.py b/tests/data/test_migrations.py index 4abbafe2..315364fb 100644 --- a/tests/data/test_migrations.py +++ b/tests/data/test_migrations.py @@ -46,7 +46,7 @@ def test_fresh_database_is_stamped_at_the_current_version() -> None: conn = db.connect(":memory:") db.migrate(conn) version = conn.execute("SELECT version FROM schema_version").fetchone()["version"] - assert version == db.SCHEMA_VERSION == 9 + assert version == db.SCHEMA_VERSION == 10 def test_fresh_database_gets_no_subscription_row() -> None: diff --git a/tests/data/test_repository.py b/tests/data/test_repository.py index 7ab96d88..fd07b919 100644 --- a/tests/data/test_repository.py +++ b/tests/data/test_repository.py @@ -483,6 +483,87 @@ def test_profile_readable_reports_damage_that_get_profile_hides(repo): assert repo.get_profile().autonomous is False # still fails closed, still no exception +# -- instrument attestations -------------------------------------------------- + + +def test_upsert_instrument_attestation_round_trips_through_get(repo): + repo.upsert_instrument_attestation( + venue="coinbase", + product_id="BTC-USD", + wrapper="spot", + source="https://x.invalid", + attested_by="tester", + attested_at=1_800_000_000, + ) + assert repo.get_instrument_attestation("coinbase", "BTC-USD") == { + "venue": "coinbase", + "product_id": "BTC-USD", + "wrapper": "spot", + "source": "https://x.invalid", + "attested_by": "tester", + "attested_at": 1_800_000_000, + } + + +def test_upsert_instrument_attestation_on_conflict_replaces_rather_than_duplicates(repo): + repo.upsert_instrument_attestation( + venue="coinbase", product_id="BTC-USD", wrapper="spot", source="s1", + attested_by="alice", attested_at=1_000, + ) + repo.upsert_instrument_attestation( + venue="coinbase", product_id="BTC-USD", wrapper="perpetual", source="s2", + attested_by="bob", attested_at=2_000, + ) + + row = repo.get_instrument_attestation("coinbase", "BTC-USD") + assert row["wrapper"] == "perpetual" + assert row["attested_by"] == "bob" + assert row["attested_at"] == 2_000 + assert repo.get_instrument_attestations() == [row] + + +def test_same_product_id_on_two_venues_are_independent_rows(repo): + """The whole point of the composite key: one venue's BTC-USD spot listing must not collide + with another venue's BTC-USD listing of a different wrapper.""" + repo.upsert_instrument_attestation( + venue="coinbase", product_id="BTC-USD", wrapper="spot", source="s1", + attested_by="a", attested_at=1, + ) + repo.upsert_instrument_attestation( + venue="kraken", product_id="BTC-USD", wrapper="cfd", source="s2", + attested_by="a", attested_at=1, + ) + + assert repo.get_instrument_attestation("coinbase", "BTC-USD")["wrapper"] == "spot" + assert repo.get_instrument_attestation("kraken", "BTC-USD")["wrapper"] == "cfd" + + +def test_get_instrument_attestation_returns_none_for_an_unknown_key(repo): + assert repo.get_instrument_attestation("coinbase", "BTC-USD") is None + + +def test_get_instrument_attestations_lists_all_rows_ordered_by_venue_then_product(repo): + repo.upsert_instrument_attestation( + venue="kraken", product_id="BTC-USD", wrapper="cfd", source="s", attested_by="a", + attested_at=1, + ) + repo.upsert_instrument_attestation( + venue="coinbase", product_id="ETH-USD", wrapper="spot", source="s", attested_by="a", + attested_at=1, + ) + repo.upsert_instrument_attestation( + venue="coinbase", product_id="BTC-USD", wrapper="spot", source="s", attested_by="a", + attested_at=1, + ) + + rows = repo.get_instrument_attestations() + assert [(r["venue"], r["product_id"]) for r in rows] == [ + ("coinbase", "BTC-USD"), + ("coinbase", "ETH-USD"), + ("kraken", "BTC-USD"), + ] + + # -- screen exceptions -------------------------------------------------------- diff --git a/tests/data/test_trade_outcomes.py b/tests/data/test_trade_outcomes.py index 9273505c..b5f105bc 100644 --- a/tests/data/test_trade_outcomes.py +++ b/tests/data/test_trade_outcomes.py @@ -35,11 +35,11 @@ def _outcome(**overrides: object) -> dict: return base -def test_schema_is_at_version_9() -> None: +def test_schema_is_at_version_10() -> None: conn = db.connect(":memory:") db.migrate(conn) version = conn.execute("SELECT version FROM schema_version").fetchone()["version"] - assert version == db.SCHEMA_VERSION == 9 + assert version == db.SCHEMA_VERSION == 10 def test_fresh_database_has_no_outcomes() -> None: diff --git a/tests/test_proposer.py b/tests/test_proposer.py index a3324056..a4ba88e8 100644 --- a/tests/test_proposer.py +++ b/tests/test_proposer.py @@ -113,6 +113,7 @@ def screen_fn(repo, product, quote): median_daily_volume=Decimal("2000000"), quotable_in_settlement_currency=True, product_id=product, + venue="coinbase", ) result = screen_mod.ScreenResult( asset=product.split("-")[0], @@ -155,7 +156,7 @@ def test_shariah_hypothesis_is_never_passed_to_the_gate(): def screen_fn(repo, product, quote): captured.append((repo, product, quote)) return ( - screen_mod.MarketFacts("SOL", 0, Decimal(0), True, "SOL-USD"), + screen_mod.MarketFacts("SOL", 0, Decimal(0), True, "SOL-USD", "coinbase"), screen_mod.ScreenResult("SOL", admitted=False, failures=["attestation: MISSING."]), ) @@ -179,7 +180,7 @@ def _report(admitted, bars, attested=False, hypothesis=None): ) def screen_fn(repo, product, quote): - facts = screen_mod.MarketFacts("SOL", bars, Decimal("0"), True, "SOL-USD") + facts = screen_mod.MarketFacts("SOL", bars, Decimal("0"), True, "SOL-USD", "coinbase") failures = ( [] if admitted @@ -273,7 +274,7 @@ def _report_with_real_failure_strings(bars): parsed = parse_proposal({"candidates": [_entry(asset="SOL")]}) def screen_fn(repo, product, quote): - facts = screen_mod.MarketFacts("SOL", bars, Decimal("0"), True, "SOL-USD") + facts = screen_mod.MarketFacts("SOL", bars, Decimal("0"), True, "SOL-USD", "coinbase") failures = [ f"history: {bars} daily bars < 1460 required", "liquidity: median daily volume 0 < 1000000 required", @@ -331,6 +332,7 @@ def test_data_derived_failures_tags_actually_match_screen_asset_output(): median_daily_volume=Decimal(0), quotable_in_settlement_currency=False, product_id="SOL-EUR", + venue="coinbase", ) tags = {f.split(":")[0] for f in screen_mod.screen_asset(facts, None).failures} missing = DATA_DERIVED_FAILURES - tags