Skip to content

feat(transcript): detect reordering the supplier cannot renumber away - #110

Open
hayulpapax wants to merge 3 commits into
flop-labs:mainfrom
hayulpapax:feat/nonce-order-check
Open

hayulpapax wants to merge 3 commits into
flop-labs:mainfrom
hayulpapax:feat/nonce-order-check

Conversation

@hayulpapax

Copy link
Copy Markdown

Closes nothing on its own — this adds the one ordering check on #93 that a supplier cannot renumber away, alongside rather than instead of #94 and #97.

What changes for a caller

A new exported checkNonceOrder(records) returning NonceOrderIssue[]. It reports positions where one signer's records are supplied out of the order that signer numbered them. examples/audit-export.mjs calls it and refuses to treat the fold as evidence when it fires.

Nothing else changes: foldTranscript's result shape is untouched, deliberately, so this lands whichever of #94/#97 goes in and either can call it.

Why nonce rather than seq

seq and timestampMs are venue metadata. A file supplier can rewrite both, which is what makes @alitiknazoglu's relabelling case in #93 beat a pairwise seq walk — moving one row the supplier already holds costs a character and reads the same afterwards.

nonce 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 reordering two records from one signer moves their nonces out of order, and putting the nonces back means forging a signature over a different preimage.

Scope is per (room, sender) — the scope the venue actually enforces. Two parties give two attested chains and no attested interleaving, so this says nothing about a payer's record relative to a payee's. It does not detect deletion or end-truncation; nothing in an export does. Records whose signature does not verify are skipped, since an unverified nonce is asserted rather than attested.

Measured before writing it

Grouping signed records by sender in export order, on live rooms 2026-09-05:

room signed records keys with 2+ rows nonce order violations
mb-9f28e3dca219 585 24 0
signing-messages 55 7 0
tclk-offers 12,387 565 1

The single violation is a pair the venue ordered against its own clock: the key's nonces are ordered and the venue's seq is not. That is the boundary the doc comment names — the venue looks for a key's last nonce only within the newest portion of a room it scans, so a long busy board can legitimately show a lower nonce once older traffic buries the record that would have refused it. A derived deal room, a handful of records from two keys, is never near it.

(Same reading also cleared #97's fatal timestamp goes backwards: the venue's ts is non-monotonic 107 times in 12,414 board records, worst by 44.6 minutes, but across 3,500 real offer/accept pairs the number where the accept carries an earlier ts than its offer is zero. The inversions are between unrelated contracts, which a fold never compares. Detail in the issue thread.)

Tests

Five, in tests/transcript.test.ts:

  • an honest deal reports nothing
  • two records of one signer supplied swapped are caught
  • the same swap with seq and timestampMs renumbered to hide it is still caught — the regression that fails without this change
  • two signers whose records interleave, the later one carrying the lower nonce, are not faulted
  • a record whose signature does not verify is ignored rather than treated as an ordering claim

SPEC

SPEC.md and the code agree. This adds a check over what §2 already states about which fields a signature covers, and changes no wire format — the golden vectors are untouched.

Hostile counterparty

Nothing. It reads records and returns positions; it holds no secret, performs no write, and declines to conclude anything from a record it could not authenticate.

CI

pnpm install --frozen-lockfile, pnpm -r --include-workspace-root build, pnpm -r --include-workspace-root test run locally: build clean, 108 root tests passing against 103 before, mcp/ 40 and 33 passing.

One failure locally, tests/schema-conformance.test.ts, identical before and after this branch: generate-frame-fields.mjs --check on a Windows checkout, which is #90 and has fixes open in #91 and #103.

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.
@alitiknazoglu

Copy link
Copy Markdown

@thangvmt asked for the nonce consequence to be read as reasoning rather than measurement, so I implemented @hayulpapax's rule as stated — per signer, per room, nonce must not decrease in file order — and ran it over the cases in the matrix. It closes reordering. It does not close relabelling, and the reason it does not is that the condition is the supplier's to choose.

Deal room written in this order: lock(payer) 1, heartbeat(payee) 2, reveal(payee) 3, refund(payer) 4, then two spare heartbeats written last, one from each party.

supplied file nonce violations caught
honest 1,2,3,4 0 — (no false alarm)
reveal deleted 1,2,4 0 no, as you both said
scrambled 4,1,3,2 2 yes
relabelled payee spare into the hole 0 no
relabelled payer spare into the hole 1 yes

The scrambled row is the result worth having: two violations, and correcting either means signing a different preimage. That is the first check in this thread whose evidence an editor cannot rewrite, and it closes reorder-plus-renumber, which #94 and #97 both leave open.

The bracketing condition is not "shares a signer"

The stated condition was that the substituted row shares a signer with another retained row. The payee spare does share a signer — the retained heartbeat at position 2 is also the payee's — and it is still missed, because the payee's two remaining rows are in increasing nonce order. The spare was written after the heartbeat, so a walk over the payee's rows sees n(hb) -> n(spare) and rises.

The condition that actually bites is narrower: the substituted row must be followed in the file by a row from the same signer carrying a lower nonce. The payer spare is caught for exactly that reason — the payer's refund sits after it and was written earlier, so the walk sees the nonce come back down.

In the case from the matrix the row being replaced is the payee's reveal, and it is that signer's last row in the deal. Nothing from the payee follows it. There is nothing to bracket against, and the walk is silent.

Which is a choice the supplier makes

Either party may send a heartbeat, so a supplier holding spares from both keys picks the one whose signer leaves no later row to bracket it. Against a three-frame deal room that is a one-line decision, not a constraint. So the rule does not need widening to catch this — the gap is not in the rule, it is that the attack selects around it.

None of that argues against sending the check. It is the only ordering evidence in the thread that lives inside the signature, and reorder-plus-renumber is currently open in both branches, which is reason enough on its own. My suggestion is only that it ship described as what the measurement supports — closes reordering, does not close deletion, end-truncation, or relabelling — rather than as covering the relabelling case, which would put a third "no gap detected" style claim into a tool whose whole problem in this thread has been over-reading its own silence.

Method: rule implemented from @hayulpapax's description, not from their code, which I have not seen. Signed records only, grouped by room|sender, walked in supplied file order, nonces compared as integers. Synthetic transcript on main @ 5cc4ab9, one process per case.

The review implemented the rule from its description, ran it against the flop-labs#93
matrix, and found the stated bracketing condition wrong. "Shares a signer" is
not what makes a substitution visible. A swap surfaces only when the moved
record is followed in the supplied order by a record from the same signer
carrying a lower nonce, so substituting a signer's *final* record leaves nothing
to bracket against and the walk stays silent — and a supplier holding spare
frames from both parties simply uses the one whose signer has no later row.

That is a choice available to the supplier rather than a gap the rule can be
widened to close, so this does not change the check. It changes what the check
says about itself.

The doc comment now names reordering, including reorder-plus-renumber, as what
it closes, and names deletion, end-truncation, cross-signer reordering and
substitution as what it does not. The CHANGELOG entry says the same. A test
asserts both halves — silent when the substituted row is that signer's last,
firing when the same substitution is bracketed — so the claim cannot quietly
widen later.

Recording the limit as an executable assertion rather than prose is the point:
this thread's recurring failure has been a tool over-reading its own silence.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@hayulpapax

Copy link
Copy Markdown
Author

You are right, and thank you for implementing it from the description rather than taking the claim — that is the second time on this thread that a rebuild has corrected the person who stated the rule, mine included.

The bracketing condition I gave was wrong. "Shares a signer" is not what makes a substitution visible; what makes it visible is being followed in the supplied order by a record from the same signer carrying a lower nonce. Your payee case is the proof: the payee's retained heartbeat sits before the spare, so the walk sees the nonces rise and says nothing. Substituting a signer's final record has nothing to bracket against, and in the #93 matrix the row being replaced is the payee's reveal, which is exactly that.

And the part that settles it is that the supplier chooses. Holding spares from both keys, picking the one whose signer leaves no later row is a one-line decision against a three-frame room. So this is not a gap to widen the rule into — the rule is doing what it does, and the attack selects around it.

Pushed, and it changes what the check says rather than what it does:

  • The doc comment now names what it closes — reordering, including reorder-plus-renumber — and names deletion, end-truncation, cross-signer reordering and substitution as what it does not, with your condition stated as the reason rather than as a caveat.
  • The CHANGELOG entry says the same, in the same words.
  • A test asserts both halves: silent when the substituted row is that signer's last, firing when the same substitution is bracketed by a later row from that signer. Your two matrix rows, executable.

That last one is the response I actually wanted to make to your closing point. This thread's recurring failure has been a tool over-reading its own silence — GAP on an empty room, no gap detected on a censored one — and a caveat in prose is the form of statement that rots first. An assertion that fails if someone later widens the claim does not.

Nothing else moved: same function, same scope, foldTranscript still untouched.

CI locally: build clean, 109 root tests passing against 103 on main, mcp/ 40 and 33. The one failure is schema-conformance on a Windows checkout, identical before this branch and tracked as #90.

@luch91 luch91 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new check currently compares against a highest nonce, not the previous supplied record. In checkNonceOrder, after value <= previous.value it pushes an issue and returns without updating highest. For three authenticated records from one (room, sender) with record nonces 5, 3, 4, the function therefore reports both 5→3 and 5→4. The documented/implemented rule is that a signer’s nonce must not decrease in supplied order: only 5→3 decreases, and 3→4 is increasing. Please retain the current record as the comparison predecessor even after reporting a violation (or otherwise make the intended all-prior comparison explicit and adjust the contract/tests). A regression test with three valid signed records in 5,3,4 order would pin this boundary.

@luch91 luch91 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for clarifying the intended per-signer file-order rule and documenting its limits. I rechecked the exact head 65fb59359a09e2c1d7b7f437be9bd1050126bade; the implementation issue remains in checkNonceOrder: when value <= previous.value, it pushes an issue and immediately returns, so highest is not updated to the current record. For authenticated records from one (room, sender) with nonces 5, 3, 4, the current code therefore reports both the 5→3 and 5→4 rows, although only the second supplied row decreases relative to its immediate predecessor. Please either update the stored predecessor after reporting, or make an explicit all-prior comparison and adjust the contract/tests. A three-record regression covering 5,3,4 would pin the chosen semantics. The new matrix tests cover substitution limits but do not exercise this three-record violation path.

…L byte

@luch91 is right, twice over, and the second reading was the one that named it
precisely: after `value <= previous.value` the walk pushed an issue and returned
without advancing the stored record, so the map held a running maximum while the
documented rule is about the immediately preceding record. For one signer's
nonces 5, 3, 4 that reports 5>3 and 5>4 — one displacement counted twice, and the
second row naming a predecessor already walked past, when 3 -> 4 does not go
backwards at all.

The doc comment already said "followed in the supplied order by a record from the
same signer carrying a lower nonce". The code said something else. Advancing the
entry after reporting is the smaller of the two options offered, and the one that
matches what is already written, so the contract is unchanged.

The variable was called `highest`, which is most of why the early return read as
correct; it is `previousBySigner` now.

A regression covering exactly that case: three authenticated records from one
(room, sender) supplied 5, 3, 4, asserting a single issue at index 1 against
previousIndex 0. It fails on the previous commit and passes here.

Also in this diff: `src/transcript.ts` carried a literal NUL byte, in the key
separator between room and sender. It worked — nothing can collide with it — but
a control character in source is damage rather than a choice: invisible in an
editor, and enough for some tooling to treat the file as binary. It is a space
now, which is unambiguous since neither a room name nor a did:key can contain one.

111 tests pass, build clean.
@hayulpapax

Copy link
Copy Markdown
Author

You are right, and the second pass named it more precisely than the first: the problem is not the comparison but what happens after it. value <= previous.value pushed the issue and returned without advancing the entry, so the map held a running maximum while the doc comment three lines above describes the immediately preceding record — "followed in the supplied order by a record from the same signer carrying a lower nonce". For 5, 3, 4 that is one displacement reported twice, and the second row names a predecessor the walk had already passed, when 3 → 4 does not go backwards at all.

Taking the first of your two options, since it is what the contract already says. Pushed as 39a74fb:

  • the entry advances after reporting, so the comparison is against the predecessor
  • highest is now previousBySigner — the old name is most of why the early return read as correct
  • the regression you asked for: three authenticated records from one (room, sender) supplied 5, 3, 4, asserting one issue at index 1 / previousIndex 0 with the nonces named. It fails on 65fb593 and passes here, so it pins the semantics rather than the current output.

You were also right that the matrix tests did not exercise this path — they cover which substitutions are visible, and never put three records from one signer in a violating order. That is now its own case rather than a corner of an existing one.

One thing I found while doing this, unrelated to your review and worth flagging on the diff: src/transcript.ts was carrying a literal NUL byte as the key separator between room and sender. It worked, since nothing can collide with it, but a control character in source is damage rather than a design choice — invisible in an editor, and enough for some tooling to treat the file as binary. It is a space in this push. Neither a room name nor a did:key can contain one, so the separator stays unambiguous.

111 tests pass, build clean.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants