feat!: return ceremony attestations on the notarization WebSocket - #6
feat!: return ceremony attestations on the notarization WebSocket#6Wondertan wants to merge 19 commits into
Conversation
bae5f1e to
d449da1
Compare
271d318 to
d4c4d87
Compare
cf8b26c to
f65795f
Compare
SupremaLex
left a comment
There was a problem hiding this comment.
Read server.rs end to end. The core move is right, there is one design problem I'd like changed, and two things I checked rather than assumed.
The core move is right
This deletes a stateful subsystem and replaces it with nothing: SessionMap, SessionEntry, SessionAttestation, SESSION_TTL, SESSION_SWEEP_INTERVAL, sweep_stale_sessions, admit_session, the background sweep task, the ready Notify, POST /session, GET /attestation/{id}, NotarizeQuery — for a Semaphore and a write back on the socket.
That removes a memory-DoS surface, a background task, a monotonic-vs-wall-clock hazard, a produced-but-never-fetched lifecycle, and this race, which silently discarded a completed proof:
"session {sid} gone before raw attestation could be stored (evicted/abandoned) — proof dropped"
It also converges with what libid-server-rs already assumes — "the notary answers a completed session on the socket the session ran over. It reads no attestation request." Two sides of one contract that previously disagreed.
The WebSocket rework is the strongest part. Splitting the pump into independent inbound/outbound tasks with a oneshot handoff, and the two comments justifying it — cancellation independence, and "this message boundary is the handoff: TLSNotary may read ahead within one WebSocket message, but it cannot consume this later one" — are load-bearing and correctly reasoned. try_acquire_owned before on_upgrade, so an over-capacity client gets a real 503 rather than a socket that just closes, and reject(Some("UPSTREAM_CONNECT_FAILED")) so a prover learns why, are both right.
Two things I checked rather than assumed
The deleted driver-death guard is not a regression. driver_finished_early, handle_mut() and the select! race are gone, and silent_connection_hits_the_deadline asserts a silent client takes the full 300s — which reads at first like trading a prompt failure for a five-minute hang, and with a semaphore that would be a cheap way to exhaust permits. But tests/driver_task_leak.rs covers the scenario the guard actually existed for: 20 connect-then-drop cycles, a 2s settle, num_alive_tasks() == 0. A handler wedged for 300s would show as 20 live tasks and fail it. Replaced, not dropped — worth saying out loud since the comment explaining the original hazard went with it.
CI: I have not run the suite on this branch; the above is from reading.
Inline comments below on the #[cfg(test)] seam (the one I'd like changed), the nested Result, the permit, and one question about a deleted test.
|
|
||
| let setup = async { | ||
| #[cfg(test)] | ||
| let root_store = state |
There was a problem hiding this comment.
Trust-anchor selection is the first of the two cfg forks. Under cfg(test) this reads proxy_test; shipped, it is unconditionally libid_tlsn::root_store(). Whichever branch is wrong, no test can reach it. See the note on the proxy_test field — an ordinary root_store field on NotaryState removes the fork.
There was a problem hiding this comment.
Addressed in bebcdaf: the verifier now always reads state.proxy_root_store through one production code path.
| handle.close(); | ||
|
|
||
| Ok((server_name, transcript, transcript_commitments)) | ||
| Ok::<_, Error>(Ok((server_name, transcript, transcript_commitments))) |
There was a problem hiding this comment.
Ok::<_, Error>(Ok(...)), then let setup_outcome = setup.await? and a match on the inner — the nesting encodes a real distinction (an error that must join the driver first, versus one that can propagate immediately) but nothing names it, so a reader has to reverse-engineer why there are two levels.
A two-variant enum would say it — SetupOutcome::Ready(..) | SetupOutcome::Rejected(Error) — or, if the shape stays, one sentence on setup stating what each level means. It is the sum-type-written-as-a-product smell: two different failures wearing the same Result.
7eb13e5 to
bebcdaf
Compare
a5a1d76 to
d2372e9
Compare
|
Ran this branch ( 1. No record comes back on a session that fully succeededProver side, one session: Notary side, same session:
Reproduced twice on separate runs, each with a fresh authorization code. Guess, not verified: 2. Panic when the platform returns non-2xxWhen the platform answers non-2xx, Backtrace, trimmed:
It kills one worker task. The process keeps running and keeps serving. The two are separate: changing only the OAuth |
|
@SupremaLex, the second one I will have to look at and it could be real, as I didnt tested the failure cases coming from platforms. The first however works well with wasm client and I am able to get attestations. Please investigate it. |
Five scratch files landed here that should not have: two unapplied patches against other repositories, and three write-ups of a debugging session. The patches target libid-rs and the TypeScript ceremony package, so nothing in this build consumes them and they would drift from what they patch with no way for a reader to tell. The write-ups belong where the work does. The findings themselves are on libid-rs#2 and libid-org/notary#6, which is where anyone acting on them is looking. Assisted-by: Claude Opus 5 Signed-off-by: SupremaLex <georglutsenko@gmail.com>
|
Point 2 is fixed in libid-org/mpz#2 and pinned here by 231ef89. The non-2xx return drops the TLSN session handle while its detached prover still owns an MPZ context. In alpha.6, dropping the executor cancelled queued tasks, so a still-awaited This is the minimal alpha.6-compatible lifetime part of upstream MPZ #403; it does not pull in that PR's global pool/API refactor. The notary suite passes all 15 tests, including the real ProxyMode and MPC return paths, and full clippy passes with warnings denied. Point 1 was a separate Rust-prover integration issue and is fixed by libid-rs@8954d84. This MPZ dependency fix does not change notary framing or close behavior. PR #6 still inherits #3's older |
Assisted-by: GPT-5 Signed-off-by: Wondertan <hlibwondertan@gmail.com>
Remove session creation and polling. Reclaim the ProxyMode channel, send one length-prefixed ceremony attestation, then close it. Pin the browser bundle to TLSNotary #1178 for prover-side finish(). Assisted-by: GPT-5 Signed-off-by: Wondertan <hlibwondertan@gmail.com>
Exercise a real local ProxyMode session through the WebSocket route, reclaim the prover channel, validate the signed ceremony attestation, and require EOF without any polling endpoint. Assisted-by: GPT-5 Signed-off-by: Wondertan <hlibwondertan@gmail.com>
Assisted-by: GPT-5 Signed-off-by: Wondertan <hlibwondertan@gmail.com>
Assisted-by: GPT-5 Signed-off-by: Wondertan <hlibwondertan@gmail.com>
Signed-off-by: Wondertan <hlibwondertan@gmail.com> Assisted-by: GPT-5
Signed-off-by: Wondertan <hlibwondertan@gmail.com> Assisted-by: GPT-5
Signed-off-by: Wondertan <hlibwondertan@gmail.com> Assisted-by: GPT-5
Signed-off-by: Wondertan <hlibwondertan@gmail.com> Assisted-by: GPT-5
Signed-off-by: Wondertan <hlibwondertan@gmail.com>
Signed-off-by: Wondertan <hlibwondertan@gmail.com>
Assisted-by: GPT-5 Signed-off-by: Wondertan <hlibwondertan@gmail.com>
Assisted-by: GPT-5 Signed-off-by: Wondertan <hlibwondertan@gmail.com>
Pin the notary and release WASM build to the libid TLSN integration revision containing upstream PRs #1178 and #1179. Assisted-by: GPT-5 Signed-off-by: Wondertan <hlibwondertan@gmail.com>
Promote the PoC-qualified TLSN pin with per-consumer RCOT and KOS domain separation. Repeat its MPZ workspace patches so the notary and SDK test clients resolve the same dependency graph as the browser WASM build. Depends on tlsnotary/tlsn#1173 and ethereum/mpz#446, alongside the existing #1178 and #1179 fork changes. Assisted-by: GPT-5 Signed-off-by: Wondertan <hlibwondertan@gmail.com>
Assisted-by: GPT-5 Signed-off-by: Wondertan <hlibwondertan@gmail.com>
Preserve the generated wrapper, WASM, and hashed web-spawn module paths in the release archive so CCDP can deploy them without rewriting generated source. Assisted-by: GPT-5 Signed-off-by: Wondertan <hlibwondertan@gmail.com>
Repin TLSN to its ownership-only integration commit and resolve the KOS domain-separation patch from libid-org/mpz at the same qualified commit. Assisted-by: GPT-5 Signed-off-by: Wondertan <hlibwondertan@gmail.com>
Pin the minimal alpha.6-compatible executor lifetime backport so early HTTP failures do not cancel active TLSN protocol tasks. Assisted-by: GPT-5 Signed-off-by: Wondertan <hlibwondertan@gmail.com>
231ef89 to
37e1950
Compare
Summary
latestalias{attested_data, notary_signature}record, then closeThis follows the later browser transport contract in libid-org/libid#13: the notary sends the canonical libID record directly after TLSNotary completes and reads no application-level request.
Browser WASM artifact
The release archive preserves exactly the generated runtime tree:
tlsn_wasm.js,tlsn_wasm_bg.wasm, andsnippets/web-spawn-<digest>/js/spawn.js. The build verifies the wrapper's generated import and stages the worker at that path unchanged. CCDP can therefore select those three resources, mount the tree beneath an immutable asset prefix, and apply worker-specific response headers only tospawn.js; the notary no longer needs to flatten, rewrite, or embed generated modules.Browser handoff race
The previous fixes changed verifier/driver shutdown ordering, but the invalid-message failure remained because the handoff itself had no boundary. The WASM adapter reads and buffers a whole WebSocket message;
finish()then discards that adapter. If the final TLSNotary bytes and attestation were emitted by one duplex-pump read, they became one WebSocket message and the buffered attestation was lost before the original browser IO tried to read it.The notary now drains and closes the TLSNotary side before sending the attestation as its own WebSocket binary message. WebSocket ordering provides a deterministic ownership boundary without another request or acknowledgement. The regression test asserts that the final WebSocket message is exactly one valid length-prefixed attestation. This is a framing implementation bug, not a fundamental limitation of same-WebSocket retrieval.
MPC regression and fix
Before #3's fix, the new ceremony attestation was retained only in the browser session map for later HTTP polling. MPC enters through the raw TCP listener and has no
sessionId, so that storage branch was unreachable: an MPC prover could complete TLS verification but could not retrieve the ceremony attestation.#3 fixes this by writing the single canonical attestation frame directly to TLSNotary's recovered MPC socket. No follow-up request or HTTP retrieval is involved. Because this PR is stacked on #3, it includes that fix and adds an end-to-end MPC regression test covering the real MPC exchange and final response.
Closes #5.
Dependencies
v0.3.0taglibid-org/tlsnintegration pinlibid-org/mpz@1dd2349dand repeated in the server because Cargo ignores dependency-workspace patchesUpstream TLSNotary
mainhas had no commits for over a month. If #1178, #1179, or #1173 and its MPZ dependency are not merged and released in a timely manner, we may retain thelibid-org/tlsnandlibid-org/mpzforks instead of blocking this work.The server and SDK test clients are pinned to
libid-org/tlsn@94aaaf33f3361d1218f9abb4c82b5c58a9199460, with MPZ resolved fromlibid-org/mpz@1dd2349d52aeea038d77fb0816f781c6b714fe77. The browser WASM build reads the same TLSN revision fromCargo.lock; that TLSN workspace carries the corresponding MPZ patch. The deadlock fix and its domain-separation follow-up must ship together, with matched server and browser artifacts.Before merge
v0.3.0-rc.3through CCDP in a real browser, then run one real X/GitHub ceremonyVerification
TLSN_WASM_FORCE=1 ./scripts/build-tlsn-wasm.sh --out <fresh-dir>builtlibid-org/tlsn@94aaaf33and staged only the wrapper, WASM, andsnippets/web-spawn-05868593a72e2d44/js/spawn.js; its archive retained the same tree and no rootspawn.jscargo +nightly-2026-08-10 fmt --all -- --checkcargo +1.97.1 clippy --all-targets --all-features -- -D warningscargo +1.97.1 test --allpassed all 15 tests, including real local Proxy-WebSocket and MPC recovered-socket protocol checksbash -n scripts/build-tlsn-wasm.shgit diff --checkv0.3.0-rc.3release workflow passed; the uploaded WASM archive has SHA-2561cd11e4397dc2d3a9749f16ae901c8d8e2ea97b539e80629d40cdb996551a7a8and the verified native tree above