t12 contract 1.1: a second version alongside an untouched 1.0, dispatched on the version in the payload - #7
Merged
Merged
Conversation
…ched on the version in the payload the canonical hash covers every serialized field, so a hashed schema can't be edited in place: adding one field changes the digest of proposals already hashed, and any token minted over such a digest stops verifying. so 1.0 is frozen and 1.1 is a separate document (D32). - version.py: SCHEMA_VERSION_1_0 / _1_1 plus SCHEMA_VERSION = the version this codebase PRODUCES. old versions stay readable - ProposalV1_0 byte-for-byte as m1 shipped it; ProposalV1_1 adds `transfer` and an optional `valid_until`. no bare `Proposal` alias on purpose - a name whose meaning silently moves to the newest version is the ambiguity versioning exists to remove. Action split into ActionV1_0/ActionV1_1 for the same reason - tests/fixtures/proposal_v1_0.json + the pinned digest 9dbaa8bd...5a0803: the only thing that can PROVE "old proposals still hash the same". verified it has teeth - adding one optional field to 1.0 fails it immediately. the fixture is a realistic wire payload (keys out of order, +01:00 offset, trailing-zero amount), so it also asserts that canonicalization rather than the sender's formatting fixes the digest - same business fields hash DIFFERENTLY across versions, because schema_version is inside the hash - a cross-version replay is impossible by construction - parse_proposal(): discriminated union on schema_version. an unknown version is rejected rather than falling back to the newest, and a MISSING one is rejected too: in-process construction may default the version, a payload from outside has to say which contract it is. guessing is how a 1.0 document gets read as 1.1, and the digest that guess produces is wrong undetectably - valid_until in the past PARSES (a historical proposal must stay parseable forever - re-reading a stored proposal isn't the same act as accepting a new one). freshness is asked at the gate via is_expired(); what 1.1 does reject is valid_until <= created_at, a contradiction between two of its own fields whose verdict can never change with time - the T2 invariants now run against BOTH versions (parametrized): a new version must not quietly drop an invariant the old one had gate green, 165 passed.
…nt floor on value-moving actions the medium is a fail-open on the money path: is_expired() matched known classes with isinstance, so a future ProposalV1_2 carrying valid_until would return "not expired" for a proposal whose deadline passed months ago - and mypy stays happy when the union is widened, because the isinstance branch is still type-correct. the gate would then approve a stale proposal believing it had asked. - is_expired() reads valid_until STRUCTURALLY (a ProposalLike protocol + getattr), so a new version opts in the moment it exists rather than the moment someone remembers to extend the function. opting in by accident is harmless here; opting out by accident is the bug. pinned by a test with a stand-in next version - 1.1's digest is now pinned too (tests/fixtures/proposal_v1_1.json, 94c9ff9d...3d0a68). 1.1 is what this codebase PRODUCES, so from t13 its digests are the ones tokens are minted over - it is then exactly as frozen as 1.0. before this, adding a field to 1.1 broke zero tests; verified the reviewer's own example (chain_id) now fails - 1.1 requires amount > 0 for buy/sell/transfer. ge=0 is right for hold/alert, but a zero-amount transfer burns gas to do nothing and is what a prompt-injected recommend domain emits when it's flailing. 1.0 keeps its old rules - tightening a frozen version could make a stored proposal unparseable, which is the D32 mutation in another guise - EvidenceRef is shared across versions and therefore inside every version's freeze: widening `kind` would widen what a frozen 1.0 document may say without changing any existing digest. said so in the docstring and pinned the allowed kinds with a test, so adding one forces the "does this need a new version?" conversation - test_a_1_1_payload_is_not_readable_as_1_0 asserted something weaker than it claimed: the payload still carried schema_version 1.1, which fails the Literal before extra="forbid" is ever reached. it now relabels the tag to "1.0" first (also the realistic attack) and matches on the field name - the amount floor collided with test_negative_zero_hashes_as_zero, which used the default buy action while asserting signed-zero canonicalization; switched to hold, since the property under test isn't the business rule gate green, 174 passed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
First contract change since M1, and the one that has to be done carefully: the canonical hash is about to become load-bearing (T13 mints a token over it), so this is the last moment where "how do we evolve a hashed schema" is a cheap question.
why a second class and not two new fields
The canonical hash covers every serialized field. Add
valid_untilto 1.0 in place and every proposal that was already hashed gets a different digest — stored hashes stop reproducing, and any token minted over one becomes unverifiable. So 1.0 is frozen forever and 1.1 is a separate document (D32).schema_versionbeing inside the hash is what makes cross-version collision impossible by construction: the same business fields under 1.0 and 1.1 are two different documents with two different digests, so an approval of one can never be replayed as an approval of the other.what
version.py—SCHEMA_VERSION_1_0/SCHEMA_VERSION_1_1, andSCHEMA_VERSIONmeaning the version this codebase produces (now1.1). Older versions stay readable.ProposalV1_0— byte-for-byte as M1 shipped it.ProposalV1_1— adds thetransferaction and an optionalvalid_until.Actionsplit the same way (ActionV1_0/ActionV1_1), because 1.0's digest must not depend on what 1.1 allows.Proposalalias, deliberately. A name whose meaning silently moves to the newest version is exactly the ambiguity versioning exists to remove. Call sites that mean "what we produce today" useSCHEMA_VERSION; call sites that accept input useparse_proposal.parse_proposal()— a discriminated union onschema_version. An unknown version is rejected rather than falling back to the newest, and a missing one is rejected too: in-process construction may default the version, but a payload from outside has to say which contract it is. Guessing is how a 1.0 document gets read as 1.1, and the digest that guess produces is wrong in a way nothing downstream can detect.is_expired(proposal, at)— freshness asked as a question, at the gate. 1.0 has novalid_untiland never expires of its own accord; that isn't a loophole, because the token's own expiry and single-use still bound the approval. Refuses a naiveatrather than comparing against an unknown zone.the regression that gives D32 its teeth
tests/fixtures/proposal_v1_0.json+ a pinned digest (9dbaa8bd…5a0803). It's the only thing that can prove "old proposals still hash the same" — no amount of careful reading enforces it. Verified it fails when it should: adding a single optional field toProposalV1_0breaks it immediately (and I restored the file afterwards).The fixture is a realistic wire payload — keys out of order, a
+01:00offset, a trailing-zero amount — so it also asserts that canonicalization, not the sender's formatting, is what fixes the digest.what parses vs what the gate decides
valid_untilin the past parses fine. A historical proposal must stay parseable forever: re-reading a stored proposal is not the same act as accepting a new one, and if parsing rejected stale proposals the audit trail would rot and stored digests would become unverifiable. What 1.1 does reject isvalid_until <= created_at— a contradiction between two of the proposal's own fields, whose verdict can never change with the passage of time, so no stored proposal can become unparseable later. Anything that depends on "now" belongs at the gate (T16).tests
tests/test_contracts_versioning.py— the pinned 1.0 digest, cross-version digest difference,transferexpressible only in 1.1, a 1.1 payload not readable as 1.0 (extra="forbid"doing real work),parse_proposaldispatch + unknown/missing version, and thevalid_untilmatrix.tests/test_contracts.py— the T2 invariants (frozen, no unknown fields, float money rejected, negatives, naive datetimes, evidence required, empty strings, and all the canonical-hash properties) now run against both versions: a new version must not quietly drop an invariant the old one had.review round (a5caaf1)
The review verified the central claim independently — it reconstructed
main's pre-changeProposalmodel and hashed the fixture with it, getting exactly the pinned digest, so the 1.0 pin is genuine rather than self-referential. Five findings, all fixed:is_expiredfailed open for versions it didn't enumerate (medium). It narrowed withisinstance(proposal, ProposalV1_1), in a module whose entire thesis is that new shapes arrive as new classes. When aProposalV1_2lands carryingvalid_until, the mechanical change is to widen the unions — and mypy stays satisfied, because theisinstancebranch is still type-correct. The result would beis_expired(v1_2, now) == Falsefor a proposal whose deadline passed months ago, and a gate that approves a stale money-moving proposal believing it had asked. The deadline is now read structurally (aProposalLikeprotocol +getattr), so a new version opts in the moment it exists. Opting in by accident is harmless here; opting out by accident is the bug. Pinned by a test with a stand-in next version.ProposalV1_1broke zero tests. Nowtests/fixtures/proposal_v1_1.json+94c9ff9d…3d0a68are pinned as well, and I verified the reviewer's own example (chain_id: str | None = None) fails it.transfervalidated cleanly (low).ge=0is right forhold/alert, which carry no amount, but a zero-amount transfer burns gas to do nothing and is the shape a prompt-injected recommend domain emits when it's flailing. 1.1 — the version that adds the value-moving action — now requiresamount > 0forbuy/sell/transfer. 1.0 keeps its old rules: tightening validation on a frozen version could make a stored proposal unparseable, which is the D32 mutation in another guise.EvidenceRefis shared and unversioned whileActionwas split (low). That asymmetry is real: wideningkindwould widen what a frozen 1.0 document may say, without changing any existing digest — which is what makes it easy to miss. It stays shared (the shape is genuinely identical, and duplicating invites drift), but the docstring now says so explicitly and a test pins the allowedkindvalues, so adding one forces the "does this need a new proposal version?" conversation.test_a_1_1_payload_is_not_readable_as_1_0creditedextra="forbid", but the payload still carriedschema_version: "1.1", which fails theLiteral["1.0"]first — so the path it claimed to exercise was never reached, and it would have kept passing if cross-version narrowing became possible. It now relabels the tag to"1.0"(also the realistic attack) and matches on the field name.One knock-on: the new amount floor collided with
test_negative_zero_hashes_as_zero, which asserted signed-zero canonicalization using the defaultbuyaction. Switched tohold— the property under test is the canonicalization, not the business rule.Gate green: 174 passed.