Skip to content

sec: require strict majority in 1v1 winner-vote consensus#4580

Merged
Celant merged 1 commit into
v32from
fix/1v1-winner-vote-consensus
Jul 12, 2026
Merged

sec: require strict majority in 1v1 winner-vote consensus#4580
Celant merged 1 commit into
v32from
fix/1v1-winner-vote-consensus

Conversation

@Celant

@Celant Celant commented Jul 12, 2026

Copy link
Copy Markdown
Member

Add approved & assigned issue number here:

Resolves #4136

Description:

VoteRound.result() (used by handleWinner in GameServer.ts to reach consensus on a reported game winner) accepted an exact tie as a majority: votes * 2 >= totalUniqueIPs. In a 1v1 game the electorate is 2 unique IPs, so a single client's vote alone satisfied 1 * 2 >= 2 and was accepted immediately — letting one player unilaterally declare themselves the winner without any agreement from the other player, and without the server ever checking the report against the actual simulation outcome.

This fixes the comparison to require a strict majority (votes * 2 > totalUniqueIPs), so a 1v1 now requires both clients to agree. Legitimate forfeit-on-disconnect is unaffected: once a player actually disconnects, the electorate shrinks to 1 unique IP, and the sole remaining vote still resolves immediately (1 * 2 > 1).

Same fix as previously proposed in #4137, adapted to the current VoteTally.ts (which didn't exist at the time of that PR) plus corrected test coverage in tests/server/VoteTally.test.ts for the affected tie cases.

This targets v32 for a hotfix deploy; a backport PR to main follows separately.

Please complete the following:

  • I have added screenshots for all UI updates
  • I process any text displayed to the user through translateText() and I've added it to the en.json file
  • I have added relevant tests to the test directory

Please put your Discord username so you can be contacted if a bug or regression is found:

jish

…er spoof

VoteRound.result() treated an exact tie (votes * 2 >= total) as a
majority. With a 1v1 game's electorate of 2 unique IPs, a single
client's vote alone satisfied that check (1 * 2 >= 2), letting one
player unilaterally declare themselves the winner without the other
player agreeing. Requiring a strict majority (votes * 2 > total) closes
this while still resolving immediately once the electorate shrinks to 1
(e.g. the other player actually disconnects).

Resolves #4136
@coderabbitai

coderabbitai Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 174201ce-91e5-4e50-9237-9afe244df324

📥 Commits

Reviewing files that changed from the base of the PR and between dcc18d5 and 2458dd7.

📒 Files selected for processing (2)
  • src/server/VoteTally.ts
  • tests/server/VoteTally.test.ts

Walkthrough

VoteRound.result() now requires a strict majority rather than accepting ties. Its documentation and tests were updated to cover tied electorates, winning majorities, and resolution after the electorate shrinks to one elector.

Changes

Vote consensus

Layer / File(s) Summary
Enforce and validate strict majority
src/server/VoteTally.ts, tests/server/VoteTally.test.ts
The result condition changed from inclusive half to strict majority, and tests now reject ties while confirming valid majority and single-elector outcomes.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Poem

Halfway votes now wait in line,
Ties resolve no winner’s sign.
One voice left can still prevail,
Strict majorities set the sail.
> guards the victory trail.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the strict-majority winner consensus fix.
Description check ✅ Passed The description matches the winner-vote consensus bug fix and its 1v1 disconnect behavior.
Linked Issues check ✅ Passed The change enforces strict majority and preserves the disconnect case required by #4136.
Out of Scope Changes check ✅ Passed Only the consensus logic and related tests changed, with no unrelated scope visible.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Celant Celant added this to the Backlog milestone Jul 12, 2026
Celant added a commit that referenced this pull request Jul 12, 2026
…4581)

**Add approved & assigned issue number here:**

Resolves #4136

## Description:

Backport of #4580 to `main`. `VoteRound.result()` (used by
`handleWinner` in `GameServer.ts` to reach consensus on a reported game
winner) accepted an exact tie as a majority: `votes * 2 >=
totalUniqueIPs`. In a 1v1 game the electorate is 2 unique IPs, so a
single client's vote alone satisfied `1 * 2 >= 2` and was accepted
immediately — letting one player unilaterally declare themselves the
winner without any agreement from the other player, and without the
server ever checking the report against the actual simulation outcome.

This fixes the comparison to require a strict majority (`votes * 2 >
totalUniqueIPs`), so a 1v1 now requires both clients to agree.
Legitimate forfeit-on-disconnect is unaffected: once a player actually
disconnects, the electorate shrinks to 1 unique IP, and the sole
remaining vote still resolves immediately (`1 * 2 > 1`).

The `v32` hotfix (#4580) is being deployed first; this PR keeps `main`
in sync with the same fix.

## Please complete the following:

- [x] I have added screenshots for all UI updates
- [x] I process any text displayed to the user through translateText()
and I've added it to the en.json file
- [x] I have added relevant tests to the test directory

## Please put your Discord username so you can be contacted if a bug or
regression is found:

jish
@Celant Celant merged commit 9d340e5 into v32 Jul 12, 2026
16 of 17 checks passed
@Celant Celant deleted the fix/1v1-winner-vote-consensus branch July 12, 2026 13:12
@github-project-automation github-project-automation Bot moved this from Triage to Complete in OpenFront Release Management Jul 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Complete

Development

Successfully merging this pull request may close these issues.

1 participant