fix: use dynamic array size for IRV tie handling#251
Conversation
IRVResult.sol allocates the winners array with a hardcoded size of 2 on the tie path. If 3 or more candidates are tied during IRV elimination, only 2 get returned and the rest are silently dropped. This gives wrong election results for any 3+ way tie. Changed new uint256[](2) to new uint256[](count) where count is the number of tied candidates already computed in the same scope.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis change modifies the tie-handling logic in an IRV (Instant Runoff Voting) result calculator smart contract. The winners array allocation during tie scenarios now uses the actual count of tied candidates instead of a fixed size, ensuring the returned array accurately reflects all tied winners. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment Tip You can customize the high-level summary generated by CodeRabbit.Configure the |
Problem
In
IRVResult.sol, the winners array on the tie path is allocated with a hardcoded size of 2:If 3 or more candidates are tied during an IRV elimination round, only the first 2 make it into the result array. The rest are silently dropped, which gives wrong election outcomes.
Fix
Changed the allocation to use
count, which is already computed in the same scope as the number of tied candidates:No logic changes, just the array size is corrected.
Found this while writing edge case tests for 3-way ties.
Summary by CodeRabbit