mirror_worker: add the optional sign-subtree endpoint - #273
Conversation
|
/bigbonk review |
![ask-bonk[bot]](https://avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
There was a problem hiding this comment.
The implementation is solid and closely follows the established pattern in witness_worker. The core logic — stateless checkpoint verification, subtree consistency proof, signing with timestamp=0 — is correct. clippy, tests, and fmt all pass on this branch.
Two doc-comment issues:
-
add-entriesreferences insign_subtreedoc comment (filed inline): The doc comment refers toadd-entriesthree times but the mirror doesn't have that HTTP endpoint — the cosignature is produced on the commit path after entry ingestion. Thewitness_worker's equivalent doc says "the same cosigner emitted byadd-checkpoint". Please update these references to the correct endpoint/operation name. -
Module-level route list not updated (
frontend_worker.rslines 4-16): The module header lists onlyPOST /add-checkpoint,GET /metadata, andGET /. The newPOST /sign-subtreeroute should be added between/add-checkpointand/metadata, with a[signsub]link reference matching the one inlib.rs.
|
Review submitted. Here's a summary of the findings: Overall: The implementation is correct and closely mirrors the established Two doc-comment issues requiring changes:
|
Add the POST /sign-subtree route to the frontend_worker module doc's route list with a [signsub] link, matching lib.rs. Addresses bonk #273 finding 2.
|
/bigbonk review again |
|
Good. Now I have a clear picture. Let me finalize the review. Summary of findings: The previous review raised two issues:
Both issues from the previous review are resolved. Re-review of the implementation: The implementation is correct and well-structured:
No actionable findings remain. |
Implement the tlog-witness sign-subtree cosigning path: verify the requested subtree is consistent with a reference checkpoint this mirror has cosigned, then countersign the subtree with the concrete SubtreeV1CheckpointSigner (the algorithm-agnostic CheckpointSigner trait object cannot reach sign_subtree). Give MirrorSigner an actual signing capability for this: it now holds a boxed SubtreeV1CheckpointSigner built at load, since the foundation only stored the public key (the mirror never cosigned before). Later cosigning endpoints (add-entries) reuse this. Adds the /sign-subtree route, a request-body size cap, the UnprocessableEntity (422) and ReferenceCheckpointNotCosignedByThisMirror errors.
Add the POST /sign-subtree route to the frontend_worker module doc's route list with a [signsub] link, matching lib.rs. Addresses bonk #273 finding 2.
- Return the Display form of a malformed reference-checkpoint parse error, matching add-checkpoint, so the Rust type name stays out of the 400 body. - Reuse tlog_witness::MAX_REQUEST_BODY_SIZE for the sign-subtree body cap instead of duplicating the constant the parser already enforces. - Log the previously-silent rejections (unknown origin 404, subtree bounds 400s, consistency-proof 422) with the subtree range and checkpoint size, so operators can tell which check failed. - Map the unexpected verify errors (MismatchedVerifier/AmbiguousKey, unreachable over a one-element verifier list) to 500 rather than blaming the client with 400.
e39bc5d to
28593fb
Compare
2 participants
Implement the tlog-witness sign-subtree cosigning path: verify the requested subtree is consistent with a reference checkpoint this mirror has cosigned, then countersign the subtree with SubtreeV1CheckpointSigner.