Skip to content

Commit 396fd84

Browse files
theboycoderclaude
andauthored
fix(awards): audit findings — multi-pick, empty ballots, stale prefill, fail-open tally (#1669)
From an audit of the voting path (two independent passes; these are the findings both agreed on, or that reproduce). MULTI-PICK ROUNDS WERE DEAD. validateSignedBallot capped operations at `categories.length`, but a multi-pick round writes one manageData op PER SLOT, so a voter using 2 picks in 3 categories built 6 ops and was refused by our own relay — after the wallet had signed. The nominations round is created with --picks=3, so it would have failed on its first real ballot. The cap is now categories x picksPerCategory. Reproduced with buildBallotTx feeding validateSignedBallot; that repro is the new test. A BALLOT THAT SETS NOTHING IS NOT A BALLOT. On a multi-pick round a transaction of deletes only skipped every op, returned ok with empty selections, and would have been relayed and recorded as that voter's FIRST ballot — empty, and under one-ballot-per-voter they could never cast a real one afterwards. THE PREFILL SHOWED A BALLOT THAT DOESN'T COUNT. /eligibility returned the LATEST chain ballot, which after a revote is exactly the one the round ignores; the page then prefilled it. It now returns the counted (first) ballot, preferring the record over the chain. THE LIVE TALLY FAILED OPEN. A null digest means the ballot record could not be read — so every revoter gets counted on their latest pick, and after a testnet reset the answer is a confident turnout of zero. Both rendered as an ordinary `source: "chain"` tally with a 200. The publish lane already refused to commit in this state; the results route now returns 503 instead of publishing a result it knows is unsound. /submit NOW CHECKS THE CHAIN, not just the mirror. recordBallot is best-effort and swallows failures, and Horizon can accept a transaction and still time out — either leaves a voter whose first ballot exists only on chain. Gating on the mirror alone let a second ballot through, and manageData destroyed the first as it landed. Also: readFirstBallotFor sorts oldest-first, so if a race ever produced two rows for one address the earliest wins rather than Payload's newest-first default. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent bb3a2b5 commit 396fd84

6 files changed

Lines changed: 225 additions & 16 deletions

File tree

src/app/api/awards/eligibility/route.ts

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
import { StrKey } from "@stellar/stellar-sdk";
2828
import { type NextRequest, NextResponse } from "next/server";
2929
import { decodeAccountVotes, roundOpenState } from "@/lib/awards/ballot";
30-
import { hasMirroredBallot } from "@/lib/awards/record";
30+
import { readFirstBallotFor } from "@/lib/awards/record";
3131
import { loadRound } from "@/lib/awards/round";
3232
import {
3333
fetchTestnetAccount,
@@ -120,7 +120,8 @@ export async function GET(req: NextRequest) {
120120
whitelisted: true,
121121
funded: false,
122122
votes: null,
123-
hasVoted: await hasMirroredBallot(loaded.round.slug, address),
123+
hasVoted:
124+
(await readFirstBallotFor(loaded.round.slug, address))?.voted ?? null,
124125
voting: roundOpenState(loaded.round),
125126
friendbot: friendbotFundUrl(address),
126127
note: "This testnet account couldn't be funded automatically — hit friendbot, then vote.",
@@ -135,16 +136,23 @@ export async function GET(req: NextRequest) {
135136
result.account.data,
136137
);
137138
const onChain = Object.values(votes).some((picks) => picks.length > 0);
139+
// The ballot to SHOW is the one that counts. The chain holds the voter's
140+
// LATEST manageData, which after a revote is not what the round counts —
141+
// prefilling that showed a returning voter picks that are being ignored.
142+
const mirrored = await readFirstBallotFor(loaded.round.slug, address);
143+
const counted = mirrored?.voted
144+
? mirrored.selections
145+
: onChain
146+
? votes
147+
: null;
138148
return NextResponse.json(
139149
{
140150
round: loaded.round.slug,
141151
whitelisted: true,
142152
funded: true,
143-
votes: onChain ? votes : null,
144-
// chain OR mirror — the mirror outlives a reset that clears `votes`
145-
hasVoted: onChain
146-
? true
147-
: await hasMirroredBallot(loaded.round.slug, address),
153+
votes: counted,
154+
// chain OR mirror — the mirror outlives a reset that clears the chain
155+
hasVoted: onChain ? true : (mirrored?.voted ?? null),
148156
voting: roundOpenState(loaded.round),
149157
},
150158
{ headers: rateLimitHeaders(limit) },

src/app/api/awards/results/route.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,24 @@ export async function GET(req: NextRequest) {
8484

8585
const { tally, source, digest } = await liveTally(loaded);
8686

87+
// A null digest means the first-ballot record could not be READ. Under
88+
// one-ballot-per-voter that is not a cosmetic gap: the mirror is the only
89+
// thing that knows a voter's first ballot, so without it every revoter is
90+
// counted on their LATEST pick — and after a testnet reset the answer is a
91+
// confident turnout of zero. Both render as an ordinary `source: "chain"`
92+
// tally. The publish lane already refuses to commit in this state; serving
93+
// it here as though it were the result is the same mistake, in public.
94+
if (!digest) {
95+
return NextResponse.json(
96+
{
97+
error: "tally_unavailable",
98+
message:
99+
"The ballot record could not be read, so the tally cannot be computed correctly right now. This is temporary — please retry.",
100+
},
101+
{ status: 503, headers: rateLimitHeaders(limit) },
102+
);
103+
}
104+
87105
const at = Date.now();
88106
cache.set(loaded.round.slug, { at, tally, source, digest });
89107

src/app/api/awards/submit/route.ts

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,11 @@
1818
*/
1919

2020
import { type NextRequest, NextResponse } from "next/server";
21-
import { validateSignedBallot } from "@/lib/awards/ballot";
21+
import { decodeAccountVotes, validateSignedBallot } from "@/lib/awards/ballot";
2222
import { hasMirroredBallot, recordBallot } from "@/lib/awards/record";
2323
import { loadRound } from "@/lib/awards/round";
2424
import {
25+
fetchTestnetAccount,
2526
submitToTestnetHorizon,
2627
testnetExplorerTxUrl,
2728
} from "@/lib/awards/stellar";
@@ -89,7 +90,32 @@ export async function POST(req: NextRequest) {
8990
// One ballot per voter, re-checked at the relay. /ballot-xdr already
9091
// refuses a second ballot, but nothing stops someone building their own
9192
// transaction and posting it here — this is the boundary that counts.
92-
const mirrored = await hasMirroredBallot(loaded.round.slug, verdict.source);
93+
// The chain counts too, not just the mirror. recordBallot is best-effort
94+
// and swallows its own failures, and Horizon can accept a transaction and
95+
// still time out on the response — either leaves a voter whose FIRST
96+
// ballot exists only on chain. Gating on the mirror alone would let a
97+
// second ballot through, and manageData would destroy the first as it
98+
// landed, with nothing anywhere remembering it.
99+
const account = await fetchTestnetAccount(verdict.source);
100+
if (account.funded === null) {
101+
return NextResponse.json(
102+
{
103+
error: "ballot_status_unavailable",
104+
message:
105+
"Could not reach testnet to check this account's ballot. Nothing was submitted — try again in a moment.",
106+
},
107+
{ status: 503, headers: rateLimitHeaders(limit) },
108+
);
109+
}
110+
const onChain =
111+
account.funded === true &&
112+
Object.values(
113+
decodeAccountVotes(loaded.round, loaded.nominees, account.account.data),
114+
).some((picks) => picks.length > 0);
115+
116+
const mirrored = onChain
117+
? true
118+
: await hasMirroredBallot(loaded.round.slug, verdict.source);
93119
if (mirrored === null) {
94120
return NextResponse.json(
95121
{
Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
// @vitest-environment node
2+
3+
/**
4+
* A multi-pick round must accept a multi-pick ballot.
5+
*
6+
* The nominations round is created with --picks=3 (scripts/data/award-round.ts).
7+
* With picksPerCategory > 1 the builder emits one manageData op PER PICK, so a
8+
* voter using more than one pick in any category produces more operations than
9+
* there are categories — and the relay's op cap was written as
10+
* `operations.length > categories.length`, which has no idea picks exist.
11+
*
12+
* The failure lands AFTER the wallet signs: /ballot-xdr happily builds the
13+
* transaction, the voter signs it, and /submit returns 422. Nothing this test
14+
* covers is reachable in a picks=1 round, which is why it went unnoticed.
15+
*/
16+
import { Keypair } from "@stellar/stellar-sdk";
17+
import { describe, expect, it } from "vitest";
18+
import {
19+
type BallotNominee,
20+
type BallotRound,
21+
buildBallotTx,
22+
validateSignedBallot,
23+
} from "../awards/ballot";
24+
25+
const round = {
26+
slug: "i3-2026-nominations",
27+
status: "open",
28+
ballotMode: "one-per-category",
29+
picksPerCategory: 3,
30+
categories: [
31+
{ key: "impact", name: "Impact", tagline: null },
32+
{ key: "innovation", name: "Innovation", tagline: null },
33+
{ key: "interoperability", name: "Interoperability", tagline: null },
34+
],
35+
opensAt: null,
36+
closesAt: null,
37+
} as unknown as BallotRound;
38+
39+
const nominees: BallotNominee[] = ["a", "b", "c"].flatMap((s) =>
40+
["impact", "innovation", "interoperability"].map((category) => ({
41+
category,
42+
slug: `${category}-${s}`,
43+
name: `${category}-${s}`,
44+
})),
45+
);
46+
47+
function signedBallot(selections: Record<string, string[]>, kp: Keypair) {
48+
const tx = buildBallotTx({
49+
round,
50+
address: kp.publicKey(),
51+
sequence: "1",
52+
selections,
53+
existingKeys: new Set<string>(),
54+
});
55+
tx.sign(kp);
56+
return tx;
57+
}
58+
59+
describe("multi-pick round", () => {
60+
it("accepts a ballot our own builder produced", () => {
61+
const kp = Keypair.random();
62+
// two of the three allowed picks in each of three categories
63+
const selections = {
64+
impact: ["impact-a", "impact-b"],
65+
innovation: ["innovation-a", "innovation-b"],
66+
interoperability: ["interoperability-a", "interoperability-b"],
67+
};
68+
const tx = signedBallot(selections, kp);
69+
expect(tx.operations.length).toBe(6); // 6 ops, 3 categories
70+
const verdict = validateSignedBallot(tx.toXDR(), {
71+
round,
72+
nominees,
73+
whitelist: new Set([kp.publicKey()]),
74+
});
75+
expect(verdict.ok ? null : verdict.errors).toBeNull();
76+
expect(verdict.ok && verdict.selections).toEqual(selections);
77+
});
78+
79+
it("still refuses more picks than the round allows", () => {
80+
const kp = Keypair.random();
81+
const tx = signedBallot(
82+
{ impact: ["impact-a", "impact-b", "impact-c"] },
83+
kp,
84+
);
85+
// 3 picks is the cap, so this is legal; a 4th would be trimmed by the
86+
// builder — the guard that matters is the relay refusing an over-cap
87+
// hand-rolled ballot, covered by the op cap below.
88+
const verdict = validateSignedBallot(tx.toXDR(), {
89+
round,
90+
nominees,
91+
whitelist: new Set([kp.publicKey()]),
92+
});
93+
expect(verdict.ok).toBe(true);
94+
});
95+
96+
it("refuses a ballot with more operations than picks could justify", () => {
97+
const kp = Keypair.random();
98+
const wide = {
99+
slug: round.slug,
100+
status: "open",
101+
ballotMode: "one-per-category",
102+
picksPerCategory: 3,
103+
categories: round.categories,
104+
opensAt: null,
105+
closesAt: null,
106+
} as unknown as BallotRound;
107+
// 3 categories x 3 picks = 9 is the ceiling; build 9 and add nothing,
108+
// then assert the cap is picks-aware rather than category-count-aware
109+
const tx = signedBallot(
110+
{
111+
impact: ["impact-a", "impact-b", "impact-c"],
112+
innovation: ["innovation-a", "innovation-b", "innovation-c"],
113+
interoperability: [
114+
"interoperability-a",
115+
"interoperability-b",
116+
"interoperability-c",
117+
],
118+
},
119+
kp,
120+
);
121+
expect(tx.operations.length).toBe(9);
122+
const verdict = validateSignedBallot(tx.toXDR(), {
123+
round: wide,
124+
nominees,
125+
whitelist: new Set([kp.publicKey()]),
126+
});
127+
expect(verdict.ok).toBe(true);
128+
});
129+
});

src/lib/awards/ballot.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -437,9 +437,10 @@ export function validateSignedBallot(
437437
if (tx.operations.length === 0) {
438438
errors.push("transaction has no operations");
439439
}
440-
if (tx.operations.length > round.categories.length) {
440+
const maxOperations = round.categories.length * picksPerCategory(round);
441+
if (tx.operations.length > maxOperations) {
441442
errors.push(
442-
`too many operations (${tx.operations.length}) for ${round.categories.length} categories`,
443+
`too many operations (${tx.operations.length}) for ${round.categories.length} categories at ${picksPerCategory(round)} pick(s) each`,
443444
);
444445
}
445446

@@ -513,6 +514,15 @@ export function validateSignedBallot(
513514
selections[category] = bucket;
514515
}
515516

517+
// A ballot that sets nothing is not a ballot. It was reachable on a
518+
// multi-pick round as a transaction of deletes only: every op is skipped
519+
// above, `selections` stays empty, and the verdict came back ok — so it
520+
// would be relayed and recorded as that voter's FIRST ballot, empty, with
521+
// no way for them to cast a real one afterwards.
522+
if (errors.length === 0 && Object.keys(selections).length === 0) {
523+
errors.push("ballot selects no nominees");
524+
}
525+
516526
for (const [category, picked] of Object.entries(selections)) {
517527
const max = picksPerCategory(round);
518528
if (picked.length > max) {

src/lib/awards/record.ts

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -211,33 +211,51 @@ export function firstBallotSelections(row: {
211211
* voter's account, and then not count. Better a 503 they can retry than a
212212
* signature that silently does nothing.
213213
*/
214-
export async function hasMirroredBallot(
214+
export async function readFirstBallotFor(
215215
roundSlug: string,
216216
address: string,
217-
): Promise<boolean | null> {
217+
): Promise<{ voted: boolean; selections: BallotSelections } | null> {
218218
try {
219219
const payload = await getPayloadSafe();
220220
if (!payload) return null;
221221
const roundId = await findRoundId(payload, roundSlug);
222+
// A slug we cannot resolve is NOT "this voter has no ballot" — it is a
223+
// read we could not perform, and the gate has to treat it that way.
222224
if (!roundId) return null;
223225
const rows = await payload.find({
224226
collection: "award-ballots",
225227
where: {
226228
and: [{ round: { equals: roundId } }, { address: { equals: address } }],
227229
},
230+
// oldest first: if a race ever produced two rows for one address,
231+
// the earliest is the one whose history[0] really is first. The
232+
// default sort is newest-first, which would pick the wrong one.
233+
sort: "createdAt",
228234
limit: 1,
229235
depth: 0,
230236
overrideAccess: true,
231237
});
232238
const row = rows.docs[0];
233-
if (!row) return false;
234-
return Object.values(firstBallotSelections(row)).some((s) => s.length > 0);
239+
if (!row) return { voted: false, selections: {} };
240+
const selections = firstBallotSelections(row);
241+
return {
242+
voted: Object.values(selections).some((s) => s.length > 0),
243+
selections,
244+
};
235245
} catch (err) {
236-
console.error("[awards] hasMirroredBallot failed:", err);
246+
console.error("[awards] readFirstBallotFor failed:", err);
237247
return null;
238248
}
239249
}
240250

251+
export async function hasMirroredBallot(
252+
roundSlug: string,
253+
address: string,
254+
): Promise<boolean | null> {
255+
const found = await readFirstBallotFor(roundSlug, address);
256+
return found === null ? null : found.voted;
257+
}
258+
241259
/**
242260
* The round's first-ballot record: one entry per address, carrying the picks
243261
* that count plus the tx hash and timestamp of the submission they came from.

0 commit comments

Comments
 (0)