feat(engine): an append-only hash chain for the book, and a column that stops saying NOT RECORDED (#721) - #734
Merged
Merged
Conversation
…at stops saying NOT RECORDED (#721) #703's activity export shipped with a `row_hash` column whose every cell read NOT RECORDED. That was honest and it was the whole of what could be said: none of the four stores the timeline merges hashed its rows. This is the record that column was built to read. THE CHAIN IS OVER EVENTS, NOT OVER THE BOOK `orders` rows are MUTATED -- `update_order` writes status, fills and fees as a venue reports them, sometimes days later via `execution.reconcile`. A hash chained over the order row itself would break on every legitimate fill, and a chain that cries wolf on ordinary operation is a chain an operator learns to ignore. The issue named both options and said the second is the one that actually delivers tamper-evidence for a mutable book. So `keel/data/audit.py` chains immutable STATEMENTS about the book. Each write to `orders`, `transactions` and both attestation tables appends one event, in the SAME transaction as the row it describes. The book stays mutable and queryable; the event stream is what verifies. Re-importing a transaction appends rather than rewrites -- two events for one `coinbase_id` is the record that a line arrived twice with different content, which for a store whose provenance is `imported-ledger` is exactly what should be visible. One chain, not one per table. A per-table chain would let an event be moved between streams undetectably, and a removed order event would only be visible to someone who thought to verify the order chain specifically. ONE CANONICAL FORM, AND A PROOF THAT IT DID NOT MOVE The issue's hardest constraint: `research/ledger.py` already decides what canonical JSON means here, and two canonicalisations that disagree produce two hashes for one row -- invisible at write time, surfacing months later as a chain that "cannot be verified", at exactly the moment someone is establishing whether a record was altered. `keel_core.hashchain` now holds the form and the walk; both stores import it. `keel/research/ledger.py` keeps deciding WHAT a trial commits to, because only it knows. The move is pinned by the one test in this repo that asserts against hashes computed by a previous version of the code: the 93 git-tracked rows in `docs/experiments/trials-ledger.jsonl` still verify byte-for-byte. Dropping `separators` from the canonicaliser fails it. `verify_links` also grew a `find_breaks` underneath it -- one walk, formatted two ways -- so the timeline can locate the first break without parsing English out of a message. THREE READINGS, AND THE MIDDLE ONE IS THE POINT The timeline's column now carries `chained`, `not chained` or `chain broken`, with `chain_status` beside `row_hash` rather than inferred from it. `not chained` is rows predating this and every engine-log row (the log is a FILE, not a chained store; a cycle row carrying a hash would be this codebase attesting to something it merely read). An honest gap, deliberately NOT a break, so upgrading into the chain does not open the timeline to a page of red. `chain broken` is a row whose event falls at or after the first break. Its hash is still SHOWN -- hiding it would destroy the value someone verifying would work from -- and the status refuses to call it evidence. A chain proves a sequence, so past a break the sequence is unproven, whatever the single row's own hash still says. Showing those as `chained` would present unverified values as evidence, which is the one thing the column exists to prevent. GREEN REQUIRES SOMETHING TO HAVE BEEN CHECKED `intact` is `no errors AND events exist`. An empty chain reports no breaks because there is nothing in it to break, and `keel doctor` says "audit chain empty -- unverified" rather than "verifies". That is `_chain_payload`'s four-state lesson from #708, on the trading side, where the store cannot be absent so there are three. WHAT IS REFUSED No backfill. No borrowing the trials ledger's hashes -- different domain, different records; the two stores share a canonicaliser and nothing else. No hash computed at export time. TWO SAFETY PROPERTIES THAT NEEDED TESTS TO STAY TRUE `write_transaction` is `BEGIN IMMEDIATE`, and the lock is asserted from BLOCK ENTRY. A deferred transaction takes its lock at the first WRITE -- by which point the chain head has already been read, and two writers racing both write the same `prev_hash` and fork the chain, silently, because each row verifies against the row it believes precedes it. The first version of that test appended before checking the lock, which passes under plain `BEGIN` and so proved nothing. The head read is `ORDER BY seq_id`, never `ts`. The first version of that test used two events, where the head read has one candidate and any ordering picks it; ordering by `ts` survived. Three events, timestamps deliberately out of order, is what shows the fork. Readers tolerate a database without the table; writers do not. `keel/web/ server.py` and `keel mcp`'s `_open_readonly_repo` both open a repo WITHOUT migrating, so a pre-v20 database reaching a reader is ordinary -- #718 shipped a reader that raised on exactly this and took all of `gather_findings` down. Checked via `sqlite_master` rather than caught, so a genuinely corrupt database is not reported as an un-upgraded one. A WRITE that cannot record its event still fails loudly: that is the failed chain write masquerading as an honest gap. Eleven mutants killed, including: the canonicaliser losing its separators, the head read taking no write lock, `prev_hash` dropping out of what a row commits to, the head read ordering by timestamp, `latest_hashes` forgetting which store a row came from, an empty chain counting as intact, the order row landing without its event, a row past a break still reading as chained, NOT RECORDED being truncated like a hash, and the export dropping the status column. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KZZxmspQXe5qJ9FAsG13s6
…as stranding the order row An independent review found four issues. The first reaches the venue. `write_transaction` HAD A JOIN BRANCH FOR NESTING THAT DOES NOT EXIST It read `if conn.in_transaction: yield; return` -- "somebody outside opened a transaction, so they own the commit". Nothing in this codebase nests these, so that branch was never reached by the nesting it was written for. What it WAS reached by is the accident. `sqlite3` runs in legacy implicit-transaction mode, so `in_transaction` is also True after any DML that has not been committed -- including one that raised and was swallowed. `execution/equity.py` swallows a failed `record_cycle_balance` by design, which is the rule the previous PR in this series added: a diagnostic write must never abort a cycle. That swallow left the connection mid-transaction. The next `insert_order` in the same cycle then took the join branch, returned a real `order_id`, and never committed -- and `broker.place_order` runs next. Measured end to end through the real swallow site: `orders visible to another connection: 0`. An order at the venue with no durable row behind it is exactly the row `execution.reconcile` exists to find. On main the identical sequence committed, because each writer ended in an unconditional `commit()`. So the same rule, violated from the other end: a swallowed RECORD failure was deciding whether the ORDER was recorded. Fixed twice over. The commit is now unconditional -- skipping the `BEGIN` on an open transaction is only about sqlite refusing to nest one and never means skipping the commit -- and `equity.py` rolls its swallowed write back, so the dirty connection does not reach the next writer at all. The lock invariant survives either path: legacy mode opens a transaction implicitly only on DML, so a connection already `in_transaction` has already taken the write lock. THE PAYLOAD NOW COMMITS AS THE BYTES THE COLUMN HOLDS The hash was taken over a parsed-then-re-encoded object, which puts a decode/encode round trip between the stored row and its hash -- and every such conversion is somewhere the two can drift. Hashing the stored string removes the question rather than answering it shape by shape. It is also the answer to the review's second finding. The console polls `/api/timeline` every 15 seconds and `chain_state` walks the whole chain, which is not negotiable -- a verdict over a suffix has not looked at the rows most likely to have been quietly edited, and a cached prefix verdict assumes precisely the thing being checked. But verification no longer PARSES: 20,000 events went from 277 ms to 134 ms. The cost that remains is stated in the docstring with its measured numbers, the way `DEFAULT_TIMELINE_LIMIT` and `export_rows` already state theirs next door, along with what to do if it ever stops fitting: bound what is CLAIMED and say so on the page, never cache the verdict. "" IS FALSEY TO PYTHON AND PERFECTLY INDEXABLE TO SQLITE `upsert_transaction` filed its event under `cursor.lastrowid` when `coinbase_id` was falsey, reasoning that a nullable column with distinct NULLs can never take the `DO UPDATE` branch. True of NULL. False of the empty string, which IS subject to the unique index -- so a re-upsert of a blank id updated an existing row while `lastrowid` held the last real INSERT's id. The timeline then showed one row the hash of a superseded event, marked `chained`, beside a phantom key no row resolved to. The id is read back now, and the invariant is pinned where it actually lives: the event's `entity_id` equals what `_transaction_rows` prints as the row's `reference`, across all three `coinbase_id` shapes. AND `or` TREATED THE EPOCH AS ABSENT `int(value or time.time())` on three timestamps. A row stamped at 0 got an event stamped `now`, and the event's `ts` is hashed -- so the chain would attest to a timestamp the row does not hold. Every real call site passes a non-zero value today, so this was a guard against a future caller, which is exactly when a silent fallback is worth making explicit. Also: `verify_events` and `latest_hashes` had no production callers and are gone; the tests that used them go through `chain_state`, which is the path anything real takes. And the export's status assertion was `in (the three words)`, which passes on any of them -- it now pins the pairing. Six mutants killed: the join branch returning, the swallow only logging, the falsey branch guessing the newest row, a zero timestamp falling back to now, the payload being re-encoded from the parsed object, and the export's status hard-coded to one word. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KZZxmspQXe5qJ9FAsG13s6
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
1 participant
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.
Closes #721.
#703's activity export shipped with a
row_hashcolumn whose every cell readNOT RECORDED. This is the record that column was built to read.The chain is over events, not over the book
ordersrows are mutated —update_orderwrites status, fills and fees as a venue reports them, sometimes days later viaexecution.reconcile. A hash chained over the order row would break on every legitimate fill, and a chain that cries wolf on ordinary operation is one an operator learns to ignore. The issue named both options and said the append-only event table is the one that actually delivers tamper-evidence for a mutable book.keel/data/audit.pychains immutable statements about the book. Each write toorders,transactionsand both attestation tables appends one event in the same transaction as the row it describes. Re-importing a transaction appends rather than rewrites — two events for onecoinbase_idis the record that a line arrived twice with different content.One chain, not one per table: a per-table chain would let an event be moved between streams undetectably.
One canonical form, and a proof it did not move
The issue's hardest constraint.
keel_core.hashchainnow holds the form and the walk;research/ledger.pyanddata/audit.pyboth import it, and each keeps deciding what its own rows commit to.The move is pinned by the one test in this repo that asserts against hashes computed by a previous version of the code: the 93 git-tracked rows in
docs/experiments/trials-ledger.jsonlstill verify byte-for-byte. Droppingseparatorsfrom the canonicaliser fails it.Three readings, and the middle one is the point
chainednot chainedchain brokenA chain proves a sequence, so past a break the sequence is unproven whatever the single row's own hash still says. Showing those as
chainedwould present unverified values as evidence, which is the one thing the column exists to prevent.chain_statussits besiderow_hash, never inferred from it — in the payload, in the table, and in the CSV.Green requires something to have been checked
intactis no errors and events exist. An empty chain reports no breaks because it has nothing in it to break, sokeel doctorsays "audit chain empty — unverified". That is_chain_payload's four-state lesson from #708, on the trading side, where the store cannot be absent so there are three.Refused
No backfill. No borrowing the trials ledger's hashes — different domain, different records; the two stores share a canonicaliser and nothing else. No hash computed at export time.
Two safety properties that needed tests to stay true
write_transactionisBEGIN IMMEDIATE, and the lock is asserted from block entry. A deferred transaction takes its lock at the first write — by which point the head has been read, and two writers racing both write the sameprev_hashand fork the chain silently. The first version of that test appended before checking the lock, which passes under plainBEGIN.The head read is
ORDER BY seq_id, neverts. The first version used two events, where the head read has one candidate and any ordering picks it; ordering bytssurvived. Three events with timestamps out of order is what shows the fork.Readers tolerate a database without the table; writers do not.
keel/web/server.pyandkeel mcp's_open_readonly_repoboth open a repo without migrating — #718 shipped a reader that raised on exactly this and took all ofgather_findingsdown. Checked viasqlite_masterrather than caught, so a corrupt database is not reported as an un-upgraded one. A write that cannot record its event still fails loudly.Acceptance
row_hashchained to its predecessorrow_hashcolumn carries real hashes for chained rows andNOT RECORDEDfor the rest, in the same file — pluschain_statusbeside itGates
E501is pre-existing, indocs/experiments/2026-09-02-equities-cost-fidelity.py, untouched here)prev_hashdropping out of what a row commits to · the head read ordering by timestamp ·latest_hashesforgetting which store a row came from · an empty chain counting as intact · the order row landing without its event · a row past a break still reading as chained ·NOT RECORDEDtruncated like a hash · an empty chain reported as verified · the export dropping the status columnNo schema change —
audit_eventsshipped as schema-only in v20 (#719's batch). This is its writer and its readers.🤖 Generated with Claude Code
https://claude.ai/code/session_01KZZxmspQXe5qJ9FAsG13s6