Repository navigation
Conversation
…gaps foldTranscript now returns warnings: gap, seq ordering and timestamp monotonicity per room, plus a generic note that ts/seq are venue metadata not covered by the Ed25519 signature (room|nonce|line only). A file supplier can rewrite ts to move a reveal across refundAfterMs and flip claimed<->refunded with all signatures valid — the transcript is coordination, not settlement proof; verify on the rail. This closes the timestamp-malleability half of flop-labs#93's scope note, and the gap detection covers the same missing-row flip as flop-labs#93 C without refusing legitimate partial windows. audit-export.mjs and tclk_apply_transcript now surface warnings; SPEC 2 documents the boundary.
|
Ran this branch against the transcript shapes from #93 rather than reading it, because the two Method: one contract, offer and accept on the board, The duplicate case is the one I want to flag as a point in your favour. Your check compares The case it missesDelete the reveal and put an unsigned row in its place, so the positions read The fold marks the padding row The cause is that What mine does not doOrdering and So the two seem to be complements rather than alternatives: yours catches ordering, timestamps |
Unsigned/BAD rows still appear as BAD in steps but must not make a censored verified seq look contiguous. Verified 1,3 padded with unverified 2 at seq 2 now correctly trips gap detected, matching thangvmt's flop-labs#94 shape. Pairwise seq check already handled 1,2,2,4 duplicate.
|
Thanks for running it against #93 shapes, good catch on the unsigned padding. Fixed: Yours (verified gap + |
jokazza84
left a comment
There was a problem hiding this comment.
I re-ran the current head addd5db6d7cfffaa70f3ceb7f1de1754e94b4a4b. The verified-row change also fixes the null-record regression from the previous head, so that part now holds.
There is still one correctness gap in the offline-audit claim: the gap detector treats seq as evidence of completeness even though this PR correctly documents that seq is not covered by the signature.
I built one authenticated contract with offer/accept on tclk-offers and lock/reveal/refund at deal-room seq 1,2,3:
honest
every verifyTranscriptRecord: true
fold: claimed
warnings: generic unsigned-metadata warning only
audit fatal predicate: false
resulting exit: 0
delete reveal; change refund metadata seq 3 -> 2
every verifyTranscriptRecord: true
fold: refunded
warnings: the same generic warning only
audit fatal predicate: false
resulting exit: 0
No signed byte changes: the signature is over room|nonce|line, so changing seq does not invalidate the refund record. The supplied file therefore removes the claim, reports a contiguous 1,2, changes the terminal outcome, and still passes the new selective fatal check.
The per-room warnings are useful diagnostics for accidental truncation or reordering, but an offline file cannot use unsigned seq to establish completeness against its supplier. Please make that limitation structural in audit-export.mjs: for example, do not give a terminal fold a proof-like success status when the only completeness evidence is unsigned metadata (or distinguish “replay succeeded” from “audit evidence verified”). A focused regression for delete-and-renumber should pin whichever contract you choose.
…upports
The tool printed one line, `fold → claimed`, and exited 0. A reader hears that as an audit
result. It is not one: the fold says what these records amount to, never that these are all the
records, and the gap check cannot close that difference because `seq` is venue metadata outside
the signed preimage. A supplier that deletes a row and renumbers what it keeps leaves no gap,
and one that drops the last row leaves none either.
The conclusion is now two claims that cannot be read as one:
```
replay → refunded
evidence → completeness NOT established. A gap is evidence of absence; the absence of
a gap is not evidence of presence …
Exit 0 means the replay reached a terminal status. It does not mean audited.
```
The honest transcript prints that too. Nothing this tool can see distinguishes a complete file
from a renumbered one, so nothing it prints should sound like it can. A detected gap still says
INCOMPLETE and exits 1, which is the one claim the counting does support.
Raised by @jokazza84 on flop-labs#97, against that PR's version of the same check. It applies here
unchanged, and prose in the caveat line was the wrong place for it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
@jokazza84's point applies to #94 unchanged, and I checked rather than assumed. Delete the What I changed there, in case it is useful here. The conclusion is now two claims that cannot be The honest transcript prints that same evidence line. Nothing either of our checks can see The caveat was already in my output as prose and it was not enough. It sat under a line that |
A reordering check built on `seq` is defeated by editing `seq`. Both open branches on flop-labs#93 read venue metadata to decide order, and flop-labs#93's matrix records that relabelling a row beats them: moving one row the supplier already holds costs a character and reads the same afterwards. `nonce` is different. It is inside the signed preimage `room|nonce|line`, and the venue requires it to exceed the last nonce that key used in that room, so a signer's own records carry an ordering the supplier cannot rewrite. Reordering two of them moves their nonces out of order, and putting the nonces back means forging a signature over a different preimage. `checkNonceOrder(records)` reports those positions. Scope is per `(room, sender)`, which is the scope the venue enforces: two parties give two attested chains and no attested interleaving, so it makes no claim about a payer's record relative to a payee's. It does not detect deletion or end-truncation — nothing in an export does — and it ignores records whose signature does not verify, because an unverified nonce is asserted rather than attested. Deliberately a separate exported function rather than a change to `foldTranscript`'s result shape, so it lands whichever of flop-labs#94/flop-labs#97 goes in and either can call it. Measured on live exports before writing it: grouping signed records by sender in export order, per-key nonce order held in 585 records over 24 multi-record keys on one mailbox room and 55 over 7 on `signing-messages`, with zero violations, and 12,387 over 565 keys on `tclk-offers` with one. That one is a pair the venue itself ordered against its own clock — the key's nonces are ordered and the venue's `seq` is not — which is the boundary the doc comment names: the venue only looks for a key's last nonce within the newest portion of a room it scans, so a long busy board can legitimately show a lower nonce. A derived deal room is never near it. SPEC.md and the code agree; this adds a check over what §2 already says about which fields a signature covers, and changes no wire format. Hostile counterparty: nothing. The function reads records and returns positions; it holds no secret, performs no write, and refuses to draw a conclusion from a record it could not authenticate. CI locally: build clean; `pnpm -r --include-workspace-root test` gives 108 passing against 103 before, plus mcp's 40 and 33. The one failure is `schema-conformance` on a Windows checkout, present identically before this change and tracked as flop-labs#90.
jokazza84 review: delete reveal + renumber refund 3->2 leaves verified 1,2 contiguous with all signatures valid (seq/ts unsigned), so gap check cannot prove completeness. audit-export now prints replay vs evidence separately — replay -> status, evidence -> completeness NOT established / INCOMPLETE — exit 0 means replay hit terminal, not audited. Honest files print it too. Adds delete-and-renumber regression pinning the limit.
@jokazza84 Thanks - fixed in
Done:
Added: |
|
Patch is up as PASdeco#1 against this branch, carrying the three pieces @sv named on #94. Two corrections to what I had on #94, both found while porting rather than after:
The MCP fixture numbered Fresh clone at @PASdeco yours to take or leave, and happy to reshape it if you would rather fold the check in differently. |
luch91
left a comment
There was a problem hiding this comment.
I audited the current head aedd1c087415e453decd757565910ac8e4ed4d0c against main.
The revised implementation now:
- computes per-room ordering/gap warnings from authenticated rows only, while retaining rejected rows in
steps; - preserves the distinction between a malformed/reordered supplied stream and a partial but otherwise valid window;
- separates
replay -> statusfromevidence -> completeness NOT establishedinaudit-export.mjs; - keeps
tclk_apply_transcriptwarnings visible to MCP callers; and - pins the important delete-and-renumber case: all retained signatures still verify, the replay can change terminal status, and no gap warning can prove completeness when
seqis unsigned.
The current source and tests are consistent with the documented trust boundary. GitHub does not expose a full runtime check for this fork head here, so this approval is a source-level review and does not claim an additional local test run.
|
Independent validation against current main I used 2,000 deterministic, locally signed The ~2.93x result matches the source path: each valid record is verified once while building The functional checks otherwise pass: build passes; all 101 root tests excluding the unrelated Windows CRLF schema-conformance failure pass; MCP 40/40 and Worker 33/33 pass. Current main reproduces that same CRLF failure (#90/#121). Could the first traversal retain each record's verification result (and decoded frame where available) and reuse it for gap analysis, deadline detection, and the fold? A focused regression can instrument or inject the verifier if you want a structural one-call-per-record assertion; otherwise a sufficiently large deterministic timing test would be much more fragile. This is not a wire/state-machine objection—the replay/evidence wording and delete-and-renumber coverage address the earlier correctness concern. |
|
Reconciliation result after running both current heads locally:
I also attempted the merge of #110 into #97. Intended combined behavior
Merge blocker: authenticate once per call#97 currently calls Please factor a per-call authenticated-record analysis/cache before combining the branches: each record should be verified once, with that verification result (and decoded frame where useful) reused by the warning analysis, deadline detection, fold, and nonce-order walk. The public That leaves the security meaning precise: gap checks detect some omissions, nonce order detects some reorderings even after unsigned |
luch91
left a comment
There was a problem hiding this comment.
The latest exact-head validation found a material performance regression: the new warning pass verifies each record repeatedly (about 2.93× the current main cost on a deterministic 2,000-record run). The combined #110 review also identifies another full verification walk if nonce-order analysis is added. Please introduce a per-call authenticated-record analysis/cache so gap analysis, deadline detection, folding, and nonce-order checks reuse one verification result per record, then add focused coverage and rerun the full workspace gates.
|
One interaction with #206 that the fixtures here don't cover. I ran
If this merges as is, the preserved-handshake flow in #206 would end at exit 1 whenever the saved offer and accept are not adjacent on the board. #206 says to keep the original seq, and a non-adjacent pair gets through only if its unsigned seqs are rewritten to be adjacent, which is the kind of edit this PR warns auditors about. As thangvmt put it on #93, a gap between one contract's board rows carries no information. Locally, skipping the gap branch for I can share the fixture generator if that helps. |
Fixes #96
Problem
src/transcript.ts:103signatures coverroom|nonce|lineonly.seq/tsare venue metadata (transcriptRecord:131->Date.parse) butfoldTranscript:236usesrecord.timestampMsasnowMsfor every deadline guard (machine.ts:121,158,178). A file supplier can rewritetsand flipclaimed↔refundedwith all signatures valid. Same silent flip happens if a row is deleted (#93) - gap, no BAD verdict.This PR does not refuse to fold (partial windows/post-reap are legitimate), it reports:
TranscriptFoldResult.warnings: string[]gap detected (seq 1->3),seq not strictly increasing,timestamp goes backwardsaccept/lock/reveal/refund) is present: “timestamps and seq are venue metadata, not covered by signature, verify settlement on the rail”examples/audit-export.mjsprintsWARN:and exits 1 on gap/ordering/backwards; generic note is informational.mcp/src/tools.ts:tclk_apply_transcriptforwardswarnings.SPEC.md:62documents the boundary: transcript is coordination, not settlement proof.Verification
pnpm build && pnpm test- 103/104 (oneschema-conformanceCRLF artifact pre-existing Windows CRLF checkout fails generated protocol field freshness check without content drift #90),mcp40/40,worker33/33repro_warn.mjs(honestclaimed-> generic note only; tamperedreveal.ts=T+7210000->timestamp goes backwards+refunded; deletedreveal->gap detected+refunded)No wire change. Follow-up could sort by
(room,seq)(#93 B) but reporting is enough for audit use.