From 0d7c3ec07836b7daa94589f93ecac41163406c71 Mon Sep 17 00:00:00 2001 From: Heimdall Date: Mon, 9 Nov 2026 20:55:03 +0000 Subject: [PATCH] Mind intake: nonce prune is best-effort and never fails a valid request; test the five-lost-races contended branch; doc the retried reveal's phase judgement Co-Authored-By: Claude Sonnet 5.5 --- docs/sealed-ballot-motion.md | 4 +- .../Features/Motions/MindIntakeService.cs | 15 +++++++- tests/Bifrost.Web.Tests/MindIntakeTests.cs | 37 +++++++++++++++++++ tests/Bifrost.Web.Tests/SealedBallotTests.cs | 34 +++++++++++++++++ 4 files changed, 86 insertions(+), 4 deletions(-) diff --git a/docs/sealed-ballot-motion.md b/docs/sealed-ballot-motion.md index 7119dda..d47fa9d 100644 --- a/docs/sealed-ballot-motion.md +++ b/docs/sealed-ballot-motion.md @@ -16,7 +16,7 @@ publication at close, no mediator. Code: `Features/Motions/SealedBallotService.c `SealedBallotCloser` sweep if nobody reads, writes status `Closed` and a digest of all commitments, then returns every revealed ballot (voter, candidate, one-sentence reason, salt, commitment) at once. The close freezes the reveal set: `Motion.RevealSeq` (bumped by every accepted reveal) and `Status` are concurrency tokens, so a reveal racing the close either lands before it (the close re-reads and its digest covers that reveal) or is - refused with 409 ("the motion closed"). A conflict from another voter's reveal while the window is open is re-read and retried, up to five times, then answered 409 "contended; send it again"; the same 409 "contended" is what `/result` answers if its close loses five races in a row; the digest covers `reveals:` plus every commitment's revealed/unrevealed state. A reveal after + refused with 409 ("the motion closed"). A conflict from another voter's reveal while the window is open is re-read and retried, up to five times (a retried reveal is judged for phase once, at the start of the request, but it lands in the frozen digest through `RevealSeq` like any other), then answered 409 "contended; send it again"; the same 409 "contended" is what `/result` answers if its close loses five races in a row; the digest covers `reveals:` plus every commitment's revealed/unrevealed state. A reveal after the deadline or after close is refused. Anyone can recompute each commitment from the published fields. Nobody can close early, extend, edit or break a tie. Canonical document (`gamecult.sealed_ballot.v0`): `{schema, motionId, round, candidate, reason, salt}`. Commitment = @@ -119,7 +119,7 @@ wrong key, tampered field, persona not matching the signing key and a malformed Only after a verified signature: a motion id different from the route: 400; `issuedAt` more than 10 minutes from the server clock: 400; a replayed nonce (stored per persona, spent only after the signature verifies): 409; then the verb itself (wrong phase 409, bad document 400, reveal not matching the persona-bound commitment 400). Nonces older than 20 minutes are pruned on -each accepted envelope, which is safe because the clock window already refuses their envelopes. A database failure that is not +each accepted envelope, in a separate best-effort write (a prune that fails, for example two requests deleting the same rows, never fails the request), which is safe because the clock window already refuses their envelopes. A database failure that is not a nonce collision is a server fault, not a replay. **Limits.** Per-source (client IP after forwarded-header processing; an IPv6 /64 is not grouped) fixed-window budget on the diff --git a/src/Bifrost.Web/Features/Motions/MindIntakeService.cs b/src/Bifrost.Web/Features/Motions/MindIntakeService.cs index 2fa39f1..0b111fb 100644 --- a/src/Bifrost.Web/Features/Motions/MindIntakeService.cs +++ b/src/Bifrost.Web/Features/Motions/MindIntakeService.cs @@ -182,8 +182,19 @@ public sealed class MindIntakeService( throw new SealedBallotException(SealedBallotFailure.WrongPhase, "That nonce was already used."); } - var cutoff = now - 2 * Skew; - dbContext.MindNonces.RemoveRange(await dbContext.MindNonces.Where(x => x.SeenAtUtc < cutoff).ToListAsync(cancellationToken)); + // Pruning is housekeeping and runs apart from the write that spends the nonce: two requests can load and delete the same + // expired rows, and the loser's concurrency exception must never fail a valid request. + try + { + var cutoff = now - 2 * Skew; + dbContext.MindNonces.RemoveRange(await dbContext.MindNonces.Where(x => x.SeenAtUtc < cutoff).ToListAsync(cancellationToken)); + await dbContext.SaveChangesAsync(cancellationToken); + } + catch (DbUpdateException) + { + dbContext.ChangeTracker.Clear(); + } + dbContext.MindNonces.Add(new MindNonce { PersonaId = persona, Nonce = envelope.Nonce!, SeenAtUtc = now }); try { diff --git a/tests/Bifrost.Web.Tests/MindIntakeTests.cs b/tests/Bifrost.Web.Tests/MindIntakeTests.cs index 29b0684..776bbac 100644 --- a/tests/Bifrost.Web.Tests/MindIntakeTests.cs +++ b/tests/Bifrost.Web.Tests/MindIntakeTests.cs @@ -373,4 +373,41 @@ public sealed class MindIntakeTests(TestWebApplicationFactory factory) : IClassF await Assert.ThrowsAsync(() => service.HandleAsync(motion, Sign(key, "mindx", "commit", motion, commitment: new string('a', 64)), default)); } + + private sealed class FailOnNonceDelete : SaveChangesInterceptor + { + public override ValueTask> SavingChangesAsync(DbContextEventData e, InterceptionResult r, CancellationToken ct = default) => + e.Context!.ChangeTracker.Entries().Any(x => x.State == EntityState.Deleted) + ? throw new DbUpdateConcurrencyException("the other request deleted these rows first") + : ValueTask.FromResult(r); + } + + [Fact] + public async Task A_prune_that_loses_a_race_never_fails_a_valid_request() + { + var clock = TimeProvider.System; + await using var db = new BifrostDbContext(new DbContextOptionsBuilder() + .UseInMemoryDatabase($"intake-prune-{Guid.NewGuid():N}").AddInterceptors(new FailOnNonceDelete()).Options); + var key = Key.New(); + var user = new UserAccount { DisplayName = "m", GitHubLogin = "m", BifrostIdentity = "m", NormalizedBifrostIdentity = "M", NormalizedGitHubLogin = "M" }; + user.Membership = new Domain.Membership { UserAccountId = user.Id, Status = MembershipStatus.Active }; + var now = DateTimeOffset.UtcNow; + var motion = new Motion + { + CreatedByUserAccountId = user.Id, Scope = MotionScope.Management, Kind = MotionKind.SealedBallot, Title = "t", Summary = "s", Candidates = "Ana\nBo", + OpensAtUtc = now, CommitClosesAtUtc = now.AddHours(1), ClosesAtUtc = now.AddHours(2), CreatedAtUtc = now + }; + db.UserAccounts.Add(user); + db.Motions.Add(motion); + db.MindKeys.Add(new MindKey { PersonaId = "mindx", UserAccountId = user.Id, PublicKey = key.Public, RegisteredAtUtc = now }); + db.MindNonces.Add(new MindNonce { PersonaId = "mindx", Nonce = "expired-nonce-expired-1", SeenAtUtc = now.AddHours(-2) }); + await db.SaveChangesAsync(); + var audit = new AuditTrailService(db, clock); + var service = new MindIntakeService(db, new SealedBallotService(db, audit, clock), audit, clock); + + var receipt = await service.HandleAsync(motion.Id, Sign(key, "mindx", "commit", motion.Id, commitment: new string('a', 64)), default); + + Assert.Equal(new string('a', 64), receipt.Commitment); + Assert.Equal(1, await db.MindNonces.CountAsync(x => x.Nonce != "expired-nonce-expired-1")); // the nonce was still spent + } } diff --git a/tests/Bifrost.Web.Tests/SealedBallotTests.cs b/tests/Bifrost.Web.Tests/SealedBallotTests.cs index 38c3879..29ebd19 100644 --- a/tests/Bifrost.Web.Tests/SealedBallotTests.cs +++ b/tests/Bifrost.Web.Tests/SealedBallotTests.cs @@ -397,4 +397,38 @@ public sealed class SealedBallotTests Assert.Equal(SealedBallotFailure.Invalid, ex.Failure); Assert.Null((await rig.Db.SealedBallots.SingleAsync()).RevealedAtUtc); } + + // Moves RevealSeq from another context just before every save, so each attempt of a reveal loses its race. + private sealed class LoseEveryRevealRace(string name, Guid motionId) : SaveChangesInterceptor + { + public override async ValueTask> SavingChangesAsync(DbContextEventData eventData, InterceptionResult result, CancellationToken cancellationToken = default) + { + if (eventData.Context!.ChangeTracker.Entries().Any(e => e.State == EntityState.Modified)) + { + await using var other = new BifrostDbContext(new DbContextOptionsBuilder().UseInMemoryDatabase(name).Options); + (await other.Motions.SingleAsync(x => x.Id == motionId)).RevealSeq++; + await other.SaveChangesAsync(); + } + + return result; + } + } + + [Fact] + public async Task Five_lost_reveal_races_answer_contended_and_record_nothing() + { + var name = $"sealed-contended-{Guid.NewGuid():N}"; + var seeder = Peer(name, T0); + var (actor, motion) = await SeedPeerWorld(seeder.Db, seeder.Service); + var revealer = Peer(name, T0.AddMinutes(90), new LoseEveryRevealRace(name, motion.Id)); + + var ex = await Assert.ThrowsAsync(() => + revealer.Service.RevealAsync(actor, motion.Id, new SealedBallotDocument(SealedBallotDocument.SchemaId, motion.Id, 1, "Ana", "Mine.", Salt), default)); + + Assert.Equal(SealedBallotFailure.WrongPhase, ex.Failure); + Assert.Contains("contended", ex.Message); + await using var check = Peer(name, T0.AddMinutes(90)).Db; + Assert.Null((await check.SealedBallots.AsNoTracking().SingleAsync()).RevealedAtUtc); + Assert.Equal(5, (await check.Motions.AsNoTracking().SingleAsync()).RevealSeq); // five rival writes, none of them this reveal + } }