Page MenuHomePhabricator

SecurePoll: Disallow elimination of already elected candidates
Closed, ResolvedPublicBUG REPORT

Description

What is the problem?

Elected candidates should never be considered for elimination.

In an edge case, it may be possible for an elected candidate's vote count to be considered the "lowest" in the ranking because the transfer of surplus to a hopeful candidate increases their vote count. This would cause the elected candidate to be declared eliminated.

(Quoted from https://gerrit.wikimedia.org/r/c/mediawiki/extensions/SecurePoll/+/715196)

This bug was found by @STran.

Event Timeline

Restricted Application added a subscriber: Aklapper. ยท View Herald TranscriptAug 30 2021, 1:04 PM

Change 715196 had a related patch set uploaded (by STran; author: STran):

[mediawiki/extensions/SecurePoll@master] Disallow elimination of already elected candidates

https://gerrit.wikimedia.org/r/715196

Change 715196 merged by jenkins-bot:

[mediawiki/extensions/SecurePoll@master] Disallow elimination of already elected candidates

https://gerrit.wikimedia.org/r/715196

@STran Could you check @phuedx's comment in https://gerrit.wikimedia.org/r/c/mediawiki/extensions/SecurePoll/+/715196/4#message-14424b9e3a217a9baed0d1078b8c2a45212122e9?

If I understand correctly, currently when we have 1 hopeful candidate (or all hopeful candidates are tied) $secondLowest will be one of the already elected candidates.

If all hopeful candidates are tied $lowest + $surplus < $secondLowest will always be true so they will always be eliminated. I guess this achieves the same outcome as this line, but is less explicit.

Moreover, after you implement T290027, $lowest + $surplus < $secondLowest will always be false when all hopeful candidates are tied or if there is one hopeful candidate (I think). I guess this might be ok as well if we want to keep iterating until there are no more votes to transfer, but is less explicit.

If I understand correctly, currently when we have 1 hopeful candidate (or all hopeful candidates are tied) $secondLowest will be one of the already elected candidates.

That's correct and why the check happens after all the rankings (I've also posted this to the patch)

If all hopeful candidates are tied $lowest + $surplus < $secondLowest will always be true so they will always be eliminated. I guess this achieves the same outcome as this line, but is less explicit.

Yes but it shouldn't reach the surplus check since the "only 1 remaining score" check happens first.

Moreover, after you implement T290027, $lowest + $surplus < $secondLowest will always be false when all hopeful candidates are tied or if there is one hopeful candidate (I think). I guess this might be ok as well if we want to keep iterating until there are no more votes to transfer, but is less explicit.

I believe this will be okay and expected? We wanted to iterate more just to be sure.

I am satisfied with the answers to T290001#7367390.

This is hard to test, as I don't know of a case where an elected candidate might have the lowest number of votes.

Since this change, I have retested most of the test elections here. I have not noticed any new problems related to this change.