Skip to content

Verify windowed custody entries on ingest - #687

Open
Volyarosh94 wants to merge 1 commit into
mainfrom
feat/custody-windowed-chain
Open

Volyarosh94 wants to merge 1 commit into
mainfrom
feat/custody-windowed-chain

Conversation

@Volyarosh94

Copy link
Copy Markdown
Contributor

What

  • Bumps libs/retina-custody to 294c0b0, the windowed hash chain. backend/uv.lock doesn't move.
  • services/custody.py holds one ingest_chain_entry for TCP CHAIN_ENTRY and POST /api/custody/chain-entry. The body is node_id, entry (HashChainEntry.to_dict()) and an optional batch (max 4 MiB). The server checks the hash, the signature, the link to the node's last verified window and, with a batch, detections_hash, then keeps the entry with its verdict. A malformed entry gets a 422 or a rejected ack and is not stored.
  • Keys come from node_identities. SignatureVerifier, the per-frame verify in the frame processor and its TCP stamp are gone; the parquet signature columns stay, nullable.
  • Status and chain report latest_window_end, verified and gaps. /verify runs verify_chain over what is held and caches the verdict per node. The test dashboard gets verified_entries_total, and the admin Custody page shows verified/entries, gaps and the latest window end.
  • _ensure_custody_data is removed. It wrote fake _verified hourly rows and SIM keys for every connected node, so a real node's windows would have linked to fake rows.
  • The body cap for /api/custody/chain-entry is 8256 KiB in the app and in nginx (main and API hosts). It is provisional until the 15-minute batch is measured on retina-test.
  • tests/test_custody.py (a copy of the library's tests) is deleted. Route and TCP tests use real chains from tests/custody_helpers.py.
  • Nothing under /v1/nodes changes, and contracts/nodes-v1.openapi.yaml is untouched.

Calls I made

  • Only a verified window anchors the next entry. Behind refused rows, an entry that links to the newest verified window is checked against it; anything else has to end after that window, or it is stored as stale window. Without this a replay passed as verified right after one refused row.
  • TCP CHAIN_ENTRY and REGISTER_KEY need an accepted HELLO, and a key registers only under the HELLO node. Before, any client could replace a node's key without a HELLO.
  • Entry fields are length-bounded and window times must be ISO UTC. The 422 for paths outside the node API no longer echoes input and ctx: a 4 MiB batch with a bad character made the default handler 500 and send it all back.
  • Snapshots written before this change still hold what the seeder fabricated. Restore drops it: identities with a -----SIM-KEY- key, hourly rows without window_end, and the seeded IQ commitments, matched on the seeder's own markers, with one log line counting what went. Production held about 24 such nodes, staging 65 and test 215.
  • /verify checks the held list, refused rows included, so a refused replay shows as a prev_hash issue there even though the windows around it were verified on receipt.

Tested

just check green up to web (5997 passed); web lint fails with main's same 54 problems, web typecheck, tests and build pass; the custody script and node.py ran against a local server, with the tampered and replayed windows refused and every other window verified.

Bump retina-custody to 294c0b0 and drop the per-frame signature path.
@Volyarosh94

Copy link
Copy Markdown
Contributor Author

Part of the custody server test: the stand-in node script is offworldlabs/retina-custody#7, and node.py on the new API is offworldlabs/retina-simulation#17. This PR can merge on its own; the droplet run comes after it is on retina-test.

@claude

claude Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Claude finished @Volyarosh94's task in 4m 21s —— View job


Review complete ✅

  • Gather context (PR diff, repo guidelines)
  • Understand the request (code review of PR Verify windowed custody entries on ingest #687)
  • Review core custody ingest logic (backend/services/custody.py, backend/routes/custody.py)
  • Review TCP handler changes (backend/services/tcp_handler.py)
  • Review snapshot restore logic (backend/services/state_snapshot.py)
  • Review nginx/body-cap and contract-adjacent changes
  • Post final review

Summary

This is a large, carefully-reasoned change, and the implementation matches what the PR description claims. I read services/custody.py, routes/custody.py, the TCP handler diff, state_snapshot.py's restore logic, main.py's body-size middleware, and the nginx template, and cross-checked the trickier behaviors against the test suite (test_custody_ingest.py, test_custody_routes.py, test_tcp_custody_protocol.py, test_state_snapshot.py).

Things I specifically checked and found correct/intentional (not bugs):

  • ingest_chain_entry stores a node-id-mismatched entry under the entry's own claimed node_id rather than the authenticated TCP hello_node_id, but always marks it verified: False first ("node_id mismatch", checked before signature verification). This is deliberate and explicitly covered by test_an_entry_for_another_node_is_a_node_id_mismatch (TCP) and test_node_id_mismatch_is_stored_under_the_entrys_node (HTTP) — a mismatched entry can never land as verified: True under someone else's chain, so it can't forge custody history, only add inert noise bounded by CHAIN_ENTRIES_MAX_PER_NODE. This is also a net security improvement over the prior TCP handler, which had no HELLO binding at all for REGISTER_KEY/CHAIN_ENTRY.
  • _link()'s fallback to an unconstrained prev_hash check (returning "") when the held tail is unverified and the new entry doesn't chain to the last verified window — this is intentional: the server has no trustworthy anchor to check hash continuity against in that case, so it falls back to requiring the window to end after the last verified one (staleness), while signature + node_id are still checked. Matches the "Calls I made" section and is covered by the replay/stale-window tests in test_custody_ingest.py and test_custody_verify_cache.py.
  • _restore_custody's seeded-data cleanup (_is_seeded_identity, _is_hourly_row, _is_seeded_iq_commitment, _drop_rows) correctly mirrors the exact fabrication logic of the removed _ensure_custody_data (same key prefix, same hour_utc/no-window_end shape, same capture_id/sha256 formula), and _drop_rows's "drop the node entirely only if it had rows and all were seeded" logic checks out against its own docstring.
  • The new node-API-external 422 handler (routes/nodes.py) strips input/ctx and serializes with ensure_ascii=True, allow_nan=False, so a hostile value (lone surrogate, NaN, huge echoed body) can no longer produce an unhandled 500 — this directly addresses the bug called out in the PR body, and the fix is sound (the escaped text is pure ASCII before it's ever encoded to bytes).
  • CUSTODY_CHAIN_ENTRY_MAX_BODY_BYTES (8256 KiB) in main.py's LimitUploadSize.detail_limits matches the 8256k nginx client_max_body_size on both the main and API host blocks.
  • SignatureVerifier/per-frame TCP signature path removal is complete — no leftover references in state.py, frame_processor.py, or tcp_handler.py's _enqueue_detection; the parquet columns are kept nullable as described.

I did not find a correctness or security bug in this diff. The ingest/verification state machine (_link, _is_stale, the ingest lock, the rolling cap, the verify-cache invalidation in routes/custody.py) is unusually well-documented inline and the test suite exercises the edge cases I'd have otherwise flagged (replay behind a rejected row, stale windows, cross-node forgery attempts, malformed/oversized fields, Unicode edge cases, snapshot restore of pre-PR seeded data). The main residual risk is operational, not code: the 8256 KiB cap is explicitly called out by the author as provisional pending a real 15-minute-batch measurement on retina-test — worth following up on but not a blocker here.

@Volyarosh94

Copy link
Copy Markdown
Contributor Author

Ran on retina-test at de93f2b:

  • On restore it dropped the seeded data: 223 identities, 11,859 hourly chain rows, 223 IQ commitments.
  • Stand-in, 2 × 900 s windows at 2 fps: every window verified with its batch. A full 15-minute window is 272,362 bytes (1,800 frames, about 151 bytes a frame), so the 4 MiB cap leaves room for node.py's ADS-B windows (about 2 MB measured locally).
  • Restart after a periodic snapshot: the node's three entries came back, and the next windows verified against the restored tail (5/5, no gaps).
  • Outside this PR: a restart within about a minute of the last post lost those entries, because main's start.sh doesn't wait for uvicorn's shutdown snapshot. Send sign-in and claim mail through a durable outbox #678 makes start.sh wait, so this goes away with that chain.

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.

1 participant