HybridVoting: pickWinnerNSlices loses precision with integer division

[hudsonhrh]

Problem

VotingMath.pickWinnerNSlices (src/libs/VotingMath.sol:387) computes each option's score as:

uint256 classContribution = (perOptionPerClassRaw[opt][cls] * slices[cls]) / totalsRaw[cls];
score += classContribution;

This truncates to integer units per class contribution. When class totals are large (e.g., ERC20 balances with ~10^12 raw units) and slices are small (≤ 100), the division collapses huge value differences down to the same integer. Option scores that are meaningfully different on-chain end up identical, causing:

1. Spurious ties — options that have different levels of support are ranked equal.
2. First-iteration wins — the contract's if (score > hi) tie-break picks whichever option is iterated first, regardless of which is actually favored.
3. Auto-invalidation — strict-majority proposals (strict = true, which HybridVoting always uses) fail hi > second on the tie, setting isValid = false even when one option clearly leads.

Concrete Example

Argus proposal 65 (Sprint 20 Priorities), Gnosis HV 0xa9209afadf721c2a55ec5875cc4716a9f1c5b0b7:

• 3 voters, 2 classes (slice 80 / 20)
• Class totals: [300, 14_701_922_947_300]
• Option 0 raw: [65, 3_162_353_072_405]
• Option 1 raw: [65, 3_196_948_421_670] ← different (more token-weighted support)

With current math

Class	Option 0 contrib	Option 1 contrib
0 (slice 80)	(65 × 80) / 300 = 17	(65 × 80) / 300 = 17
1 (slice 20)	(3.16e12 × 20) / 1.47e13 = 4 (actual 4.30)	(3.20e12 × 20) / 1.47e13 = 4 (actual 4.35)
Score	21	21 (tied)

Contract picks option 0 (first iterated), marks isValid = false (strict majority fails). Option 1 was actually preferred by the token-weighted class but the integer math couldn't distinguish it.

With precision scaling (proposed fix)

Class	Option 0 contrib	Option 1 contrib
0	(65 × 80 × 10000) / 300 = 173_333	173_333
1	(3.16e12 × 20 × 10000) / 1.47e13 = 43_020	43_491
Score	216_353	216_824 ← option 1 wins

Threshold comparison scales too: hi >= thresholdPct × PRECISION (i.e., 216_824 >= 510_000). Same isValid outcome in this case (threshold still not met), but correct winner if the margin call ever matters.

Proposed Fix

Scale contributions by a PRECISION constant before the division:

uint256 constant PRECISION = 10000; // 4 decimal digits of fixed-point

function pickWinnerNSlices(
    uint256[][] memory perOptionPerClassRaw,
    uint256[] memory totalsRaw,
    uint8[] memory slices,
    uint8 thresholdPct,
    bool strict
) internal pure returns (uint256 win, bool ok, uint256 hi, uint256 second) {
    // ...
    uint256 classContribution = (perOptionPerClassRaw[opt][cls] * slices[cls] * PRECISION) / totalsRaw[cls];
    // ...
    bool thresholdMet = hi >= uint256(thresholdPct) * PRECISION;
    bool meetsMargin = strict ? (hi > second) : (hi >= second);
    ok = thresholdMet && meetsMargin;
}

Overflow safety

• perOptionPerClassRaw[opt][cls] is backed by uint128 (from classRaw), promoted to uint256.
• slices[cls] ≤ 100.
• Product: ≤ 2^128 × 100 × 10000 ≈ 3.4 × 10^44 — well under uint256 max (~1.15 × 10^77).

Backward compatibility

• pickWinnerNSlices is internal.
• Only caller is HybridVotingCore.announceWinner which discards hi/second: (winner, valid,,) = ... (HybridVotingCore.sol:213).
• Winner event payload unchanged.
• Historical proposals are unaffected (winner/isValid already stored on-chain from their original execution).
• Pending/future proposals may get different outcomes in genuinely-tied integer-math cases — that's the intent.

Related

• VotingMath.pickWinnerTwoSlice (line 323) has the same precision bug but appears to be dead code (no callers). Either apply the same fix or remove.
• VotingMath.pickWinnerMajority (line 283) already avoids the issue with cross-multiplication (hi * 100 >= totalWeight * thresholdPct). DDV is unaffected.

Frontend implications

The frontend currently matches the contract's integer math exactly (so UI display agrees with winner label). When this contract fix ships:

1. Revert poa-box/Poa-frontend#358 back to basis-point precision math, or
2. Update the frontend to use PRECISION = 10000 to match the upgraded contract.

Frontend PR can stay merged as-is until the contract is upgraded.

Test plan

• Unit tests for pickWinnerNSlices:
  • Close-but-distinct scores (like proposal 65) — ensure the one with higher raw score wins.
  • Exact ties — behavior unchanged (first iterated wins, isValid=false under strict).
  • Threshold boundary — 51% threshold with scaled math works equivalently.
• Integration tests: HybridVoting proposals with mixed DIRECT + ERC20 classes and varied weight distributions.

[hudsonhrh]

Addendum: the codebase already has precedent for this pattern

Investigating further — the cross-multiplication pattern for avoiding precision loss is already used elsewhere in the same library. pickWinnerNSlices is the outlier.

VotingMath.meetsThreshold

return (highestVote * 100 >= totalWeight * thresholdPct) && (highestVote > secondHighest);

Cross-multiplied to avoid the highestVote / totalWeight division.

VotingMath.pickWinnerMajority (line 304)

bool thresholdMet = (hi * 100 >= totalWeight * thresholdPct);

Same pattern. DDV is unaffected precisely because of this.

HybridVotingCore._calculateClassPower

if (cls.strategy == HybridVoting.ClassStrategy.DIRECT) {
    return 100; // Direct democracy: 1 person = 100 raw points
} else if (cls.strategy == HybridVoting.ClassStrategy.ERC20_BAL) {
    uint256 balance = IERC20(cls.asset).balanceOf(voter);
    if (balance < cls.minBalance) return 0;
    uint256 power = cls.quadratic ? VotingMath.sqrt(balance) : balance;
    return power * 100; // Scale to match existing system
}

Even raw-power calculation scales by 100 for unit alignment.

Why pickWinnerNSlices regressed

The N-slice algorithm can't use the simple lhs * 100 >= rhs * thresholdPct cross-multiplication because the LHS is a sum of fractions with different denominators:

score_opt = Σ_c (optRaw_c × slice_c / classTotal_c)

You can't rearrange this to avoid division without blowing up multiplicatively (Σ_c optRaw_c × slice_c × ∏_{i≠c} classTotal_i), which overflows for realistic token balances.

So a PRECISION constant (e.g., 10000) is the correct path here — it scales the numerator before dividing, matching the spirit of power * 100 already in the codebase, just with more headroom for the ratio case.

Updated recommendation

• Add uint256 internal constant PRECISION = 10000; to VotingMath
• Multiply by PRECISION inside the fraction in pickWinnerNSlices (line 387)
• Scale the threshold comparison: hi >= uint256(thresholdPct) * PRECISION
• Apply same fix to pickWinnerTwoSlice OR delete it (no callers)

This keeps the library internally consistent with its own precedent. No frontend change needed after deployment — the frontend's existing per-class-weighted math (from #356) will then naturally match the contract at 4-decimal precision.