From a8a9f7c95199ccc8b8a7f2b0c641543d4c9b9889 Mon Sep 17 00:00:00 2001
From: Edgar Aroutiounian <edgar.factorial@gmail.com>
Date: Mon, 2 Dec 2019 09:43:03 -0800
Subject: [PATCH] [votepower] Allow negative diff, explicit check sum to one
 (#1930)

* [votepower] Allow negative diff, explicit check sum to one

* [votepower] Handle error value from compute in test

* [votepower] Change log as suggested in PR comment

* [votepower] Check sum of voting power is 1 only when have at least one staked validator
---
 consensus/quorum/one-node-staked-vote.go |  5 ++++-
 consensus/votepower/roster.go            | 18 +++++++++++++++---
 consensus/votepower/roster_test.go       |  6 +++++-
 internal/chain/reward.go                 | 10 ++++++++--
 4 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/consensus/quorum/one-node-staked-vote.go b/consensus/quorum/one-node-staked-vote.go
index c539b71b6..c659ec3fc 100644
--- a/consensus/quorum/one-node-staked-vote.go
+++ b/consensus/quorum/one-node-staked-vote.go
@@ -185,7 +185,10 @@ func (v *stakedVoteWeight) SetVoters(
 	s, _ := v.ShardIDProvider()()
 	v.Reset([]Phase{Prepare, Commit, ViewChange})
 
-	roster := votepower.Compute(staked)
+	roster, err := votepower.Compute(staked)
+	if err != nil {
+		return nil, err
+	}
 	utils.Logger().Info().
 		Str("our-percentage", roster.OurVotingPowerTotalPercentage.String()).
 		Str("their-percentage", roster.TheirVotingPowerTotalPercentage.String()).
diff --git a/consensus/votepower/roster.go b/consensus/votepower/roster.go
index 43f7ce63c..8fd829ef2 100644
--- a/consensus/votepower/roster.go
+++ b/consensus/votepower/roster.go
@@ -5,6 +5,7 @@ import (
 	"github.com/harmony-one/harmony/internal/utils"
 	"github.com/harmony-one/harmony/numeric"
 	"github.com/harmony-one/harmony/shard"
+	"github.com/pkg/errors"
 )
 
 var (
@@ -12,6 +13,8 @@ var (
 	HarmonysShare = numeric.MustNewDecFromStr("0.68")
 	// StakersShare ..
 	StakersShare = numeric.MustNewDecFromStr("0.32")
+	// ErrVotingPowerNotEqualOne ..
+	ErrVotingPowerNotEqualOne = errors.New("voting power not equal to one")
 )
 
 type stakedVoter struct {
@@ -32,7 +35,7 @@ type Roster struct {
 }
 
 // Compute creates a new roster based off the shard.SlotList
-func Compute(staked shard.SlotList) *Roster {
+func Compute(staked shard.SlotList) (*Roster, error) {
 	roster := NewRoster()
 	for i := range staked {
 		if staked[i].TotalStake == nil {
@@ -82,7 +85,7 @@ func Compute(staked shard.SlotList) *Roster {
 	// NOTE Enforce voting power sums to one, give diff (expect tiny amt) to last staked voter
 	if diff := numeric.OneDec().Sub(
 		ourPercentage.Add(theirPercentage),
-	); diff.GT(numeric.ZeroDec()) && lastStakedVoter != nil {
+	); lastStakedVoter != nil {
 		lastStakedVoter.EffectivePercent = lastStakedVoter.EffectivePercent.Add(diff)
 		theirPercentage = theirPercentage.Add(diff)
 		utils.Logger().Info().
@@ -91,10 +94,19 @@ func Compute(staked shard.SlotList) *Roster {
 			Msg("sum of voting power of hmy & staked slots not equal to 1, gave diff to staked slot")
 	}
 
+	if lastStakedVoter != nil &&
+		!ourPercentage.Add(theirPercentage).Equal(numeric.OneDec()) {
+		utils.Logger().Error().
+			Str("theirs", theirPercentage.String()).
+			Str("ours", ourPercentage.String()).
+			Msg("Total voting power not equal 100 percent")
+		return nil, ErrVotingPowerNotEqualOne
+	}
+
 	roster.OurVotingPowerTotalPercentage = ourPercentage
 	roster.TheirVotingPowerTotalPercentage = theirPercentage
 
-	return roster
+	return roster, nil
 
 }
 
diff --git a/consensus/votepower/roster_test.go b/consensus/votepower/roster_test.go
index 88f143619..38bc99e47 100644
--- a/consensus/votepower/roster_test.go
+++ b/consensus/votepower/roster_test.go
@@ -81,7 +81,11 @@ func TestCompute(t *testing.T) {
 		expectedRoster.Voters[slot.BlsPublicKey] = newMember
 	}
 
-	computedRoster := Compute(slotList)
+	computedRoster, err := Compute(slotList)
+	if err != nil {
+		t.Error("Computed Roster failed on vote summation to one")
+	}
+
 	if !compareRosters(expectedRoster, computedRoster, t) {
 		t.Errorf("Compute Roster mismatch with expected Roster")
 	}
diff --git a/internal/chain/reward.go b/internal/chain/reward.go
index e6d10b45b..23ed40c32 100644
--- a/internal/chain/reward.go
+++ b/internal/chain/reward.go
@@ -219,7 +219,10 @@ func AccumulateRewards(
 			return err
 		}
 
-		votingPower := votepower.Compute(members)
+		votingPower, err := votepower.Compute(members)
+		if err != nil {
+			return err
+		}
 
 		for beaconMember := range payable {
 			// TODO Give out whatever leftover to the last voter/handle
@@ -271,7 +274,10 @@ func AccumulateRewards(
 					return err
 				}
 
-				votingPower := votepower.Compute(payableSigners)
+				votingPower, err := votepower.Compute(payableSigners)
+				if err != nil {
+					return err
+				}
 				for j := range payableSigners {
 					voter := votingPower.Voters[payableSigners[j].BlsPublicKey]
 					if !voter.IsHarmonyNode && !voter.EffectivePercent.IsZero() {