Skip to content

fix(oracle): require strict majority for weighted median#243

Merged
Thaleszh merged 3 commits into
KiiChain:mainfrom
Decussilva:fix/oracle-weighted-median-threshold
Jan 22, 2026
Merged

fix(oracle): require strict majority for weighted median#243
Thaleszh merged 3 commits into
KiiChain:mainfrom
Decussilva:fix/oracle-weighted-median-threshold

Conversation

@Decussilva

@Decussilva Decussilva commented Jan 21, 2026

Copy link
Copy Markdown
Contributor

Description

Fixes #236
spotted a bug in WeightedMedianWithAssertion - it uses >= for the majority check but that breaks with odd total power
example: if total power is 101, then 101/2 = 50 in go. so a validator with 50 power passes even tho thats only 49.5%
just changed >= to > so it needs actual majority

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

  • TestWeightedMedianOddTotalPower - 101 power case
  • TestWeightedMedianExactHalf - two validators with 50 each

PR Checklist:

  • Updated changelog with PR's intent
  • Lint with make lint-fix

@Decussilva
fix(oracle): use strict majority (>50%) for weighted median threshold
5b5d12c 
Fixes KiiChain#236

The WeightedMedianWithAssertion function incorrectly used >= for the majority check. With odd total power (e.g., 101), integer division gives threshold=50, allowing a validator with exactly 50 power (49.5%) to be selected as median. Changed to > to require strict majority.
@Decussilva Decussilva requested a review from jhelison as a code owner January 21, 2026 12:26
@coderabbitai

coderabbitai Bot commented Jan 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Walkthrough

The PR changes the oracle weighted median selection: WeightedMedianWithAssertion now uses a strict > comparison against totalPower/2 instead of >=, requiring cumulative power to be greater than half to select a vote. A changelog entry documenting this change was added. Two test functions validating odd and exact-half behavior were added to ballot_test.go; those test additions appear duplicated in the diff, resulting in duplicate test definitions.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: replacing >= with > to enforce strict majority in weighted median threshold calculation.
Description check ✅ Passed The description clearly explains the bug fix, provides the problematic example (101 total power), and documents the test additions and changelog update.
Linked Issues check ✅ Passed The PR changes directly address issue #236 by fixing the majority threshold from >= to > in WeightedMedianWithAssertion and adding regression tests for the 101 power and 50/50 split cases.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing issue #236: the ballot.go logic fix, corresponding test additions, and changelog entry. No unrelated modifications detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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 and usage tips.

@codecov

codecov Bot commented Jan 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@Decussilva
test(oracle): add regression tests for weighted median threshold fix
82488b9 
Added tests to verify strict majority (>50%) is required for weighted median.
@Decussilva Decussilva force-pushed the fix/oracle-weighted-median-threshold branch from ad25585 to 82488b9 Compare January 21, 2026 12:52
Thaleszh
Thaleszh reviewed Jan 21, 2026
View reviewed changes
Comment thread CHANGELOG.md Outdated
Thaleszh
Thaleszh approved these changes Jan 22, 2026
View reviewed changes

@Thaleszh Thaleszh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nicely done!

@Thaleszh Thaleszh merged commit 831a4e9 into KiiChain:main Jan 22, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jhelison jhelison Awaiting requested review from jhelison jhelison is a code owner

1 more reviewer

@Thaleszh Thaleszh Thaleszh approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[BUG] Oracle weighted median uses wrong threshold - accepts 49.5% as majority

2 participants

@Decussilva @Thaleszh