Skip to content

Commit e492683

Browse files
committed
mirror_worker: accept the mirror checkpoint size as upload_end
The spec requires the mirror to accept upload_end equal to the mirror checkpoint's tree size, not just a known pending value; resolve_target_ pending only matched the current pending or a ticket-carried past pending, so a client targeting the already-committed size with no valid ticket got a spurious 409. Accept upload_end == committed.size using the committed checkpoint as the target. cosign_and_serve now skips the /commit dispatch when the upload only reaches the committed size, which also avoids redundantly rewriting R2 and appending a duplicate cosignature line. Addresses bonk #264 review on resolve_target_pending.
1 parent 04c42cf commit e492683

1 file changed

Lines changed: 34 additions & 2 deletions

File tree

crates/mirror_worker/src/add_entries.rs

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,22 @@ async fn cosign_and_serve(
418418
let cosig_body =
419419
tlog_witness::serialize_add_checkpoint_response(std::slice::from_ref(&note_sig));
420420

421+
// When the upload only reaches the mirror checkpoint's current size,
422+
// the checkpoint at that size is already committed and served. There
423+
// is nothing to advance, and re-committing would redundantly rewrite
424+
// R2 and append a duplicate cosignature line to the served note, so
425+
// return a fresh cosignature without dispatching `/commit`. (Committed
426+
// is monotonic, so a stale-low snapshot only skips this optimization,
427+
// never the other way.)
428+
if header.upload_end <= snapshot.committed.size {
429+
return Ok((
430+
StatusCode::OK,
431+
[(CONTENT_TYPE, "text/plain; charset=utf-8")],
432+
cosig_body,
433+
)
434+
.into_response());
435+
}
436+
421437
// The served checkpoint is the log's signed note with the mirror's
422438
// cosignature appended. The DO writes it to R2 while advancing the
423439
// durable checkpoint under its commit lock.
@@ -590,15 +606,18 @@ async fn fetch_snapshot(env: &Env, origin: &str) -> Result<MirrorStateSnapshot>
590606
}
591607

592608
/// Resolve the target pending checkpoint that this `add-entries`
593-
/// request is uploading toward. Either:
609+
/// request is uploading toward. Any of:
594610
///
595611
/// * `upload_end == snapshot.pending.size`: use the current pending.
612+
/// * `upload_end == snapshot.committed.size`: use the mirror
613+
/// checkpoint, which the spec requires the mirror to accept as a
614+
/// valid `upload_end` independent of any ticket.
596615
/// * The ticket round-trips and yields a past pending whose
597616
/// embedded checkpoint has size `upload_end` and verifies against
598617
/// the trusted log keys: use that.
599618
///
600619
/// Returns `Err(reason)` (a static `&str` describing why) when
601-
/// neither path produces a target. The frontend turns the error into
620+
/// none of these produce a target. The frontend turns the error into
602621
/// a 409 with `text/x.tlog.mirror-info`.
603622
fn resolve_target_pending(
604623
env: &Env,
@@ -617,6 +636,19 @@ fn resolve_target_pending(
617636
return Ok(snapshot.pending.clone());
618637
}
619638

639+
// Spec: the mirror MUST also accept `upload_end` equal to the mirror
640+
// checkpoint's tree size, whatever the ticket says. Everything up to
641+
// it is already committed and served, so the committed checkpoint is
642+
// the target and nothing new is persisted; `cosign_and_serve` returns
643+
// a fresh cosignature without re-advancing.
644+
if snapshot.committed.size > 0 && header.upload_end == snapshot.committed.size {
645+
return Ok(PendingCheckpoint {
646+
size: snapshot.committed.size,
647+
hash: snapshot.committed.hash,
648+
signed_note_bytes: snapshot.committed.signed_note_bytes.clone(),
649+
});
650+
}
651+
620652
// Try the ticket. An empty ticket can't carry a past pending, so
621653
// there's nothing to fall back to.
622654
if header.ticket.is_empty() {

0 commit comments

Comments
 (0)