Skip to content

Commit 5aa9953

Browse files
mswilkisonclaude
andcommitted
fix(frost): submit reject/conflict-only ROAST evidence snapshots (PR2b-2 review fold)
submitSnapshotIfActive skipped any snapshot whose Overflows map was empty, but a validation-blamable Reject (e.g. an attempt-context-hash mismatch) or a first-write-wins Conflict populates Evidence.Rejects / Evidence.Conflicts WITHOUT producing an Overflow. NextAttempt's exclusion path consumes snapshot.Rejects (next_attempt.go), so dropping reject/conflict-only snapshots silently starved the blame pipeline of exactly the validation evidence it needs -- newly reachable now that PR2b-2 enables the multi-seat submit path. Widen the emptiness test to all three evidence categories, and add a reject-only submission regression test. Folds Codex review P2 on PR #4087. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent df4d5fb commit 5aa9953

2 files changed

Lines changed: 60 additions & 6 deletions

File tree

pkg/frost/signing/roast_retry_submit.go

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -58,13 +58,20 @@ func submitSnapshotIfActive(
5858
return
5959
}
6060
evidence := recorder.Snapshot()
61-
if len(evidence.Overflows) == 0 {
62-
// Nothing observed worth submitting; emitting an empty
61+
if len(evidence.Overflows) == 0 &&
62+
len(evidence.Rejects) == 0 &&
63+
len(evidence.Conflicts) == 0 {
64+
// Truly nothing observed worth submitting; emitting an empty
6365
// snapshot is still meaningful in the ROAST protocol
64-
// (proof-of-attendance) but adds noise to the bundle.
65-
// Phase 4.3 chooses to skip empty submissions; Phase 5
66-
// orchestration may revisit this if attestations need to
67-
// be unconditional.
66+
// (proof-of-attendance) but adds noise to the bundle, so we
67+
// skip it. The emptiness test MUST consider all three evidence
68+
// categories, not just overflows: a validation-blamable Reject
69+
// (e.g. an attempt-context-hash mismatch) or a first-write-wins
70+
// Conflict populates Rejects/Conflicts WITHOUT any Overflow, and
71+
// NextAttempt's exclusion path consumes snapshot.Rejects
72+
// (next_attempt.go). Dropping a reject/conflict-only snapshot
73+
// here would silently starve the blame pipeline of exactly the
74+
// validation evidence it needs.
6875
return
6976
}
7077
snap := buildSignedSnapshot(deps, ctx, evidence)

pkg/frost/signing/roast_retry_submit_frost_roast_retry_test.go

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,53 @@ func TestSubmitSnapshotIfActive_MultiSeatSubmitsPerSeat(t *testing.T) {
307307
}
308308
}
309309

310+
// TestSubmitSnapshotIfActive_SubmitsRejectOnlySnapshot guards the fold of Codex's
311+
// PR2b-2 P2: a snapshot carrying ONLY reject evidence -- no overflow, no conflict
312+
// -- must still be signed and submitted. A validation-blamable Reject (e.g. an
313+
// attempt-context-hash mismatch) populates Evidence.Rejects without any Overflow,
314+
// and NextAttempt's exclusion path consumes snapshot.Rejects; the old overflow-only
315+
// emptiness check silently dropped such snapshots, starving the blame pipeline.
316+
func TestSubmitSnapshotIfActive_SubmitsRejectOnlySnapshot(t *testing.T) {
317+
ResetRoastRetryRegistrationForTest()
318+
ResetSessionHandleRegistryForTest()
319+
t.Cleanup(ResetRoastRetryRegistrationForTest)
320+
t.Cleanup(ResetSessionHandleRegistryForTest)
321+
322+
const selfMember group.MemberIndex = 1
323+
cap := newCaptureCoordinator(roast.NewInMemoryCoordinatorWithSigning(
324+
selfMember, &deterministicSigner{id: selfMember}, deterministicVerifier{},
325+
))
326+
RegisterRoastRetryCoordinator(RoastRetryDeps{
327+
Coordinator: cap,
328+
Signer: &deterministicSigner{id: selfMember},
329+
Verifier: deterministicVerifier{},
330+
SelfMember: uint32(selfMember),
331+
})
332+
333+
ctx := newTestContextForSubmit(t, "session-reject-only")
334+
handle, err := cap.BeginAttempt(ctx)
335+
if err != nil {
336+
t.Fatalf("begin: %v", err)
337+
}
338+
SetCurrentAttemptHandleForSession("session-reject-only", selfMember, handle, ctx)
339+
340+
// Only a reject -- no overflow, no conflict.
341+
recorder := attempt.NewBoundedRecorder()
342+
recorder.RecordReject(2, "attempt_context_hash_mismatch")
343+
submitSnapshotIfActive("session-reject-only", selfMember, recorder)
344+
345+
if len(cap.recordedFor) != 1 {
346+
t.Fatalf("reject-only snapshot must be submitted; got %d RecordEvidence calls", len(cap.recordedFor))
347+
}
348+
snap := cap.recordedSnp[0]
349+
if len(snap.Rejects) == 0 {
350+
t.Fatal("submitted snapshot must carry the reject evidence")
351+
}
352+
if len(snap.OperatorSignature) == 0 {
353+
t.Fatal("reject-only snapshot must be signed")
354+
}
355+
}
356+
310357
func TestSetCurrentAttemptHandleForSession_LaterBindingOverwrites(t *testing.T) {
311358
ResetSessionHandleRegistryForTest()
312359
t.Cleanup(ResetSessionHandleRegistryForTest)

0 commit comments

Comments
 (0)