Skip to content

fix(tier3): validator average_score divides by validated+rejected instead of validated count - #7964

Open
Vyacheslav-Tomashevskiy wants to merge 2 commits into
Scottcjn:mainfrom
Vyacheslav-Tomashevskiy:fix/validator-average-score-denominator
Open

fix(tier3): validator average_score divides by validated+rejected instead of validated count#7964
Vyacheslav-Tomashevskiy wants to merge 2 commits into
Scottcjn:mainfrom
Vyacheslav-Tomashevskiy:fix/validator-average-score-denominator

Conversation

@Vyacheslav-Tomashevskiy

Copy link
Copy Markdown
Contributor

Problem

ValidatorAgent.validate_poa_proof (tier3/agents/validator_agent.py) maintains average_score as a running mean over validated scores — rejected proofs contribute no score to the numerator (the else branch only bumps total_rejected). But the update divided by the combined count:

if valid:
    self.stats["total_validated"] += 1
    total = self.stats["total_validated"] + self.stats["total_rejected"]   # <-- wrong divisor
    self.stats["average_score"] = (
        (self.stats["average_score"] * (total - 1) + score) / total
    )

Because the numerator only ever sums valid scores, dividing by validated + rejected dilutes the mean whenever any proof was rejected.

Impact

  • Reject one proof, then validate one scoring 100 → total_validated=1, total_rejected=1, so average_score = (0*1 + 100) / 2 = 50.0 instead of 100.0.
  • Sequence 90 (valid), reject, 60 (valid) → reports 80.0, correct mean is 75.0.

The error appears whenever any proof is rejected and surfaces through get_stats() and up through PipelineOrchestrator.get_stats() as validator_stats.

Fix

Divide by total_validated — the count of scores actually summed — so numerator and denominator agree:

if valid:
    self.stats["total_validated"] += 1
    n = self.stats["total_validated"]
    self.stats["average_score"] = (
        (self.stats["average_score"] * (n - 1) + score) / n
    )

The sibling RewardAgent (tier3/agents/reward_agent.py) already follows this rule for average_reward — it divides the summed value by total_distributed, the count actually accumulated.

Test

Added test_average_score_excludes_rejected_from_denominator to tier3/tests/test_pipeline.py: reject one proof, validate one (score 100), assert average_score == 100. Fails on main (50.0), passes with the fix. The existing test_validation_stats_tracking only ever validates (no rejects), so it never exercised this path. Full tier3/tests/test_pipeline.py suite: 33 passed.

…tead of validated count

ValidatorAgent.validate_poa_proof maintains average_score as a running
mean over VALIDATED scores (rejected proofs contribute no score to the
numerator). But the update divided by total_validated + total_rejected,
so any rejected proof dilutes the mean.

Example: reject one proof, then validate one scoring 100 -> reported
average_score is 50.0 instead of 100.0. The error appears whenever any
proof is rejected and surfaces through get_stats() /
PipelineOrchestrator.get_stats().

Fix: divide by total_validated (the count actually summed), matching the
numerator. Add a regression test that fails on main (50.0) and passes
with the fix; full tier3 pipeline suite stays green (33 passed).
@github-actions

Copy link
Copy Markdown
Contributor

Welcome to RustChain! Thanks for your first pull request.

Before we review, please make sure:

  • Non-doc PRs have a BCOS-L1 or BCOS-L2 label
  • Doc-only PRs are exempt from BCOS tier labels when they only touch docs/**, *.md, or common image/PDF files
  • New code files include an SPDX license header
  • You've tested your changes against the live node

Bounty tiers: Micro (1-10 RTC) | Standard (20-50) | Major (75-100) | Critical (100-150)

A maintainer will review your PR soon. Thanks for contributing!

@github-actions github-actions Bot added BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) size/S PR: 11-50 lines labels Jul 14, 2026

@IcanBENCHurCAT IcanBENCHurCAT 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.

LGTM — clean fix. average_score should only divide by validated proofs, not validated+rejected. Well-documented with a clear regression test. ✅

@IcanBENCHurCAT IcanBENCHurCAT 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.

LGTM — clean fix. average_score divides by validated count only, not validated+rejected. Well-documented with clear regression test. ✅

@Scottcjn

Copy link
Copy Markdown
Owner

Verified, with a scope note. The bug is real: average_score summed only validated scores into the numerator but divided by validated + rejected, diluting the mean whenever a proof was rejected. The fix divides by validated-only, matching the summed terms. But this is an internal tier3 experimental-pipeline telemetry metric (get_stats), not consensus or on-chain reward math, and tier3 is not imported by the live node, so 'economically sensitive validator scoring' overstates it. Correct, well-scoped fix. Merge.

CI is green.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) size/S PR: 11-50 lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants