Normalize priorities to not exceed total voting power (#3049)

* more proposer priority tests

 - test that we don't reset to zero when updating / adding
 - test that same power validators alternate

* add another test to track / simulate similar behaviour as in #2960

* address some of Chris' review comments

* address some more of Chris' review comments

* temporarily pushing branch with the following changes:
The total power might change if:
   - a validator is added
   - a validator is removed
   - a validator is updated

Decrement the accums (of all validators) directly after any of these events
(by the inverse of the change)

* Fix 2960 by re-normalizing / scaling priorities to be in bounds of total
power, additionally:

 - remove heap where it doesn't make sense
 - avg. only at the end of IncrementProposerPriority instead of each
   iteration
 - update (and slightly improve)
   TestAveragingInIncrementProposerPriorityWithVotingPower to reflect
   above changes

* Fix 2960 by re-normalizing / scaling priorities to be in bounds of total
power, additionally:

 - remove heap where it doesn't make sense
 - avg. only at the end of IncrementProposerPriority instead of each
   iteration
 - update (and slightly improve)
   TestAveragingInIncrementProposerPriorityWithVotingPower to reflect
   above changes

* fix tests

* add comment

* update changelog pending & some minor changes

* comment about division will floor the result & fix typo

* Update TestLargeGenesisValidator:
 - remove TODO and increase large genesis validator's voting power
accordingly

* move changelog entry to P2P Protocol

* Ceil instead of flooring when dividing & update test

* quickly fix failing TestProposerPriorityDoesNotGetResetToZero:

 - divide by Ceil((maxPriority - minPriority) / 2*totalVotingPower)

* fix typo: rename getValWitMostPriority -> getValWithMostPriority

* test proposer frequencies

* return absolute value for diff. keep testing

* use for loop for div

* cleanup, more tests

* spellcheck

* get rid of using floats: manually ceil where necessary

* Remove float, simplify, fix tests to match chris's proof (#3157)

CHANGELOG_PENDING.md

@@ -18,6 +18,8 @@ Special thanks to external contributors on this release:
   * [merkle] \#2713 Merkle trees now match the RFC 6962 specification
 
 * P2P Protocol
+  - [consensus] \#2960 normalize priorities to not exceed `2*TotalVotingPower` to mitigate unfair proposer selection 
+    heavily preferring earlier joined validators in the case of an early bonded large validator unbonding
 
 ### FEATURES:
 

p2p/metrics.go

@@ -72,7 +72,7 @@ func PrometheusMetrics(namespace string, labelsAndValues ...string) *Metrics {
 // NopMetrics returns no-op Metrics.
 func NopMetrics() *Metrics {
 	return &Metrics{
-		Peers: discard.NewGauge(),
+		Peers:                 discard.NewGauge(),
 		PeerReceiveBytesTotal: discard.NewCounter(),
 		PeerSendBytesTotal:    discard.NewCounter(),
 		PeerPendingSendBytes:  discard.NewGauge(),

state/state_test.go

@@ -3,6 +3,7 @@ package state
 import (
 	"bytes"
 	"fmt"
+	"math"
 	"math/big"
 	"testing"
 
@@ -264,14 +265,133 @@ func TestOneValidatorChangesSaveLoad(t *testing.T) {
 	}
 }
 
+func TestProposerFrequency(t *testing.T) {
+
+	// some explicit test cases
+	testCases := []struct {
+		powers []int64
+	}{
+		// 2 vals
+		{[]int64{1, 1}},
+		{[]int64{1, 2}},
+		{[]int64{1, 100}},
+		{[]int64{5, 5}},
+		{[]int64{5, 100}},
+		{[]int64{50, 50}},
+		{[]int64{50, 100}},
+		{[]int64{1, 1000}},
+
+		// 3 vals
+		{[]int64{1, 1, 1}},
+		{[]int64{1, 2, 3}},
+		{[]int64{1, 2, 3}},
+		{[]int64{1, 1, 10}},
+		{[]int64{1, 1, 100}},
+		{[]int64{1, 10, 100}},
+		{[]int64{1, 1, 1000}},
+		{[]int64{1, 10, 1000}},
+		{[]int64{1, 100, 1000}},
+
+		// 4 vals
+		{[]int64{1, 1, 1, 1}},
+		{[]int64{1, 2, 3, 4}},
+		{[]int64{1, 1, 1, 10}},
+		{[]int64{1, 1, 1, 100}},
+		{[]int64{1, 1, 1, 1000}},
+		{[]int64{1, 1, 10, 100}},
+		{[]int64{1, 1, 10, 1000}},
+		{[]int64{1, 1, 100, 1000}},
+		{[]int64{1, 10, 100, 1000}},
+	}
+
+	for caseNum, testCase := range testCases {
+		// run each case 5 times to sample different
+		// initial priorities
+		for i := 0; i < 5; i++ {
+			valSet := genValSetWithPowers(testCase.powers)
+			testProposerFreq(t, caseNum, valSet)
+		}
+	}
+
+	// some random test cases with up to 300 validators
+	maxVals := 100
+	maxPower := 1000
+	nTestCases := 5
+	for i := 0; i < nTestCases; i++ {
+		N := cmn.RandInt() % maxVals
+		vals := make([]*types.Validator, N)
+		totalVotePower := int64(0)
+		for j := 0; j < N; j++ {
+			votePower := int64(cmn.RandInt() % maxPower)
+			totalVotePower += votePower
+			privVal := types.NewMockPV()
+			pubKey := privVal.GetPubKey()
+			val := types.NewValidator(pubKey, votePower)
+			val.ProposerPriority = cmn.RandInt64()
+			vals[j] = val
+		}
+		valSet := types.NewValidatorSet(vals)
+		valSet.RescalePriorities(totalVotePower)
+		testProposerFreq(t, i, valSet)
+	}
+}
+
+// new val set with given powers and random initial priorities
+func genValSetWithPowers(powers []int64) *types.ValidatorSet {
+	size := len(powers)
+	vals := make([]*types.Validator, size)
+	totalVotePower := int64(0)
+	for i := 0; i < size; i++ {
+		totalVotePower += powers[i]
+		val := types.NewValidator(ed25519.GenPrivKey().PubKey(), powers[i])
+		val.ProposerPriority = cmn.RandInt64()
+		vals[i] = val
+	}
+	valSet := types.NewValidatorSet(vals)
+	valSet.RescalePriorities(totalVotePower)
+	return valSet
+}
+
+// test a proposer appears as frequently as expected
+func testProposerFreq(t *testing.T, caseNum int, valSet *types.ValidatorSet) {
+	N := valSet.Size()
+	totalPower := valSet.TotalVotingPower()
+
+	// run the proposer selection and track frequencies
+	runMult := 1
+	runs := int(totalPower) * runMult
+	freqs := make([]int, N)
+	for i := 0; i < runs; i++ {
+		prop := valSet.GetProposer()
+		idx, _ := valSet.GetByAddress(prop.Address)
+		freqs[idx] += 1
+		valSet.IncrementProposerPriority(1)
+	}
+
+	// assert frequencies match expected (max off by 1)
+	for i, freq := range freqs {
+		_, val := valSet.GetByIndex(i)
+		expectFreq := int(val.VotingPower) * runMult
+		gotFreq := freq
+		abs := int(math.Abs(float64(expectFreq - gotFreq)))
+
+		// max bound on expected vs seen freq was proven
+		// to be 1 for the 2 validator case in
+		// https://github.com/cwgoes/tm-proposer-idris
+		// and inferred to generalize to N-1
+		bound := N - 1
+		require.True(t, abs <= bound, fmt.Sprintf("Case %d val %d (%d): got %d, expected %d", caseNum, i, N, gotFreq, expectFreq))
+	}
+}
+
+// TestProposerPriorityDoesNotGetResetToZero assert that we preserve accum when calling updateState
+// see https://github.com/tendermint/tendermint/issues/2718
 func TestProposerPriorityDoesNotGetResetToZero(t *testing.T) {
-	// assert that we preserve accum when calling updateState:
-	// https://github.com/tendermint/tendermint/issues/2718
 	tearDown, _, state := setupTestCase(t)
 	defer tearDown(t)
-	origVotingPower := int64(10)
+	val1VotingPower := int64(10)
 	val1PubKey := ed25519.GenPrivKey().PubKey()
-	val1 := &types.Validator{Address: val1PubKey.Address(), PubKey: val1PubKey, VotingPower: origVotingPower}
+	val1 := &types.Validator{Address: val1PubKey.Address(), PubKey: val1PubKey, VotingPower: val1VotingPower}
 
 	state.Validators = types.NewValidatorSet([]*types.Validator{val1})
 	state.NextValidators = state.Validators
@@ -288,8 +408,9 @@ func TestProposerPriorityDoesNotGetResetToZero(t *testing.T) {
 	require.NoError(t, err)
 	updatedState, err := updateState(state, blockID, &block.Header, abciResponses, validatorUpdates)
 	assert.NoError(t, err)
-
-	assert.Equal(t, -origVotingPower, updatedState.NextValidators.Validators[0].ProposerPriority)
+	curTotal := val1VotingPower
+	// one increment step and one validator: 0 + power - total_power == 0
+	assert.Equal(t, 0+val1VotingPower-curTotal, updatedState.NextValidators.Validators[0].ProposerPriority)
 
 	// add a validator
 	val2PubKey := ed25519.GenPrivKey().PubKey()
@@ -301,22 +422,33 @@ func TestProposerPriorityDoesNotGetResetToZero(t *testing.T) {
 	assert.NoError(t, err)
 
 	require.Equal(t, len(updatedState2.NextValidators.Validators), 2)
+	_, updatedVal1 := updatedState2.NextValidators.GetByAddress(val1PubKey.Address())
 	_, addedVal2 := updatedState2.NextValidators.GetByAddress(val2PubKey.Address())
 	// adding a validator should not lead to a ProposerPriority equal to zero (unless the combination of averaging and
 	// incrementing would cause so; which is not the case here)
-	totalPowerBefore2 := origVotingPower                                                      // 10
-	wantVal2ProposerPrio := -(totalPowerBefore2 + (totalPowerBefore2 >> 3)) + val2VotingPower // 89
-	avg := (0 + wantVal2ProposerPrio) / 2                                                     // 44
-	wantVal2ProposerPrio -= avg                                                               // 45
-	totalPowerAfter := origVotingPower + val2VotingPower                                      // 110
-	wantVal2ProposerPrio -= totalPowerAfter                                                   // -65
-	assert.Equal(t, wantVal2ProposerPrio, addedVal2.ProposerPriority)                         // not zero == -65
+	totalPowerBefore2 := curTotal
+	// while adding we compute prio == -1.125 * total:
+	wantVal2ProposerPrio := -(totalPowerBefore2 + (totalPowerBefore2 >> 3))
+	wantVal2ProposerPrio = wantVal2ProposerPrio + val2VotingPower
+	// then increment:
+	totalPowerAfter := val1VotingPower + val2VotingPower
+	// mostest:
+	wantVal2ProposerPrio = wantVal2ProposerPrio - totalPowerAfter
+	avg := big.NewInt(0).Add(big.NewInt(val1VotingPower), big.NewInt(wantVal2ProposerPrio))
+	avg.Div(avg, big.NewInt(2))
+	wantVal2ProposerPrio = wantVal2ProposerPrio - avg.Int64()
+	wantVal1Prio := 0 + val1VotingPower - avg.Int64()
+	assert.Equal(t, wantVal1Prio, updatedVal1.ProposerPriority)
+	assert.Equal(t, wantVal2ProposerPrio, addedVal2.ProposerPriority)
 
 	// Updating a validator does not reset the ProposerPriority to zero:
 	updatedVotingPowVal2 := int64(1)
 	updateVal := abci.ValidatorUpdate{PubKey: types.TM2PB.PubKey(val2PubKey), Power: updatedVotingPowVal2}
 	validatorUpdates, err = types.PB2TM.ValidatorUpdates([]abci.ValidatorUpdate{updateVal})
 	assert.NoError(t, err)
+
+	// this will cause the diff of priorities (31)
+	// to be larger than threshold == 2*totalVotingPower (22):
 	updatedState3, err := updateState(updatedState2, blockID, &block.Header, abciResponses, validatorUpdates)
 	assert.NoError(t, err)
 
@@ -324,11 +456,18 @@ func TestProposerPriorityDoesNotGetResetToZero(t *testing.T) {
 	_, prevVal1 := updatedState3.Validators.GetByAddress(val1PubKey.Address())
 	_, updatedVal2 := updatedState3.NextValidators.GetByAddress(val2PubKey.Address())
 
-	expectedVal1PrioBeforeAvg := prevVal1.ProposerPriority + prevVal1.VotingPower // -44 + 10 == -34
-	wantVal2ProposerPrio = wantVal2ProposerPrio + updatedVotingPowVal2            // -64
-	avg = (wantVal2ProposerPrio + expectedVal1PrioBeforeAvg) / 2                  // (-64-34)/2 == -49
-	wantVal2ProposerPrio = wantVal2ProposerPrio - avg                             // -15
-	assert.Equal(t, wantVal2ProposerPrio, updatedVal2.ProposerPriority)           // -15
+	// divide previous priorities by 2 == CEIL(31/22) as diff > threshold:
+	expectedVal1PrioBeforeAvg := prevVal1.ProposerPriority/2 + prevVal1.VotingPower
+	wantVal2ProposerPrio = wantVal2ProposerPrio/2 + updatedVotingPowVal2
+	// val1 will be proposer:
+	total := val1VotingPower + updatedVotingPowVal2
+	expectedVal1PrioBeforeAvg = expectedVal1PrioBeforeAvg - total
+	avgI64 := (wantVal2ProposerPrio + expectedVal1PrioBeforeAvg) / 2
+	wantVal2ProposerPrio = wantVal2ProposerPrio - avgI64
+	wantVal1Prio = expectedVal1PrioBeforeAvg - avgI64
+	assert.Equal(t, wantVal2ProposerPrio, updatedVal2.ProposerPriority)
+	_, updatedVal1 = updatedState3.NextValidators.GetByAddress(val1PubKey.Address())
+	assert.Equal(t, wantVal1Prio, updatedVal1.ProposerPriority)
 }
 
 func TestProposerPriorityProposerAlternates(t *testing.T) {
@@ -338,9 +477,9 @@ func TestProposerPriorityProposerAlternates(t *testing.T) {
 	// have the same voting power (and the 2nd was added later).
 	tearDown, _, state := setupTestCase(t)
 	defer tearDown(t)
-	origVotinPower := int64(10)
+	val1VotingPower := int64(10)
 	val1PubKey := ed25519.GenPrivKey().PubKey()
-	val1 := &types.Validator{Address: val1PubKey.Address(), PubKey: val1PubKey, VotingPower: origVotinPower}
+	val1 := &types.Validator{Address: val1PubKey.Address(), PubKey: val1PubKey, VotingPower: val1VotingPower}
 
 	// reset state validators to above validator
 	state.Validators = types.NewValidatorSet([]*types.Validator{val1})
@@ -361,12 +500,14 @@ func TestProposerPriorityProposerAlternates(t *testing.T) {
 	assert.NoError(t, err)
 
 	// 0 + 10 (initial prio) - 10 (avg) - 10 (mostest - total) = -10
-	assert.Equal(t, -origVotinPower, updatedState.NextValidators.Validators[0].ProposerPriority)
+	totalPower := val1VotingPower
+	wantVal1Prio := 0 + val1VotingPower - totalPower
+	assert.Equal(t, wantVal1Prio, updatedState.NextValidators.Validators[0].ProposerPriority)
 	assert.Equal(t, val1PubKey.Address(), updatedState.NextValidators.Proposer.Address)
 
 	// add a validator with the same voting power as the first
 	val2PubKey := ed25519.GenPrivKey().PubKey()
-	updateAddVal := abci.ValidatorUpdate{PubKey: types.TM2PB.PubKey(val2PubKey), Power: origVotinPower}
+	updateAddVal := abci.ValidatorUpdate{PubKey: types.TM2PB.PubKey(val2PubKey), Power: val1VotingPower}
 	validatorUpdates, err = types.PB2TM.ValidatorUpdates([]abci.ValidatorUpdate{updateAddVal})
 	assert.NoError(t, err)
 
@@ -386,16 +527,18 @@ func TestProposerPriorityProposerAlternates(t *testing.T) {
 	_, oldVal1 := updatedState2.Validators.GetByAddress(val1PubKey.Address())
 	_, updatedVal2 := updatedState2.NextValidators.GetByAddress(val2PubKey.Address())
 
-	totalPower := origVotinPower
+	totalPower = val1VotingPower // no update
 	v2PrioWhenAddedVal2 := -(totalPower + (totalPower >> 3))
-	v2PrioWhenAddedVal2 = v2PrioWhenAddedVal2 + origVotinPower       // -11 + 10 == -1
-	v1PrioWhenAddedVal2 := oldVal1.ProposerPriority + origVotinPower // -10 + 10 == 0
+	v2PrioWhenAddedVal2 = v2PrioWhenAddedVal2 + val1VotingPower       // -11 + 10 == -1
+	v1PrioWhenAddedVal2 := oldVal1.ProposerPriority + val1VotingPower // -10 + 10 == 0
+	totalPower = 2 * val1VotingPower                                  // now we have to validators with that power
+	v1PrioWhenAddedVal2 = v1PrioWhenAddedVal2 - totalPower            // mostest
 	// have to express the AVG in big.Ints as -1/2 == -1 in big.Int while -1/2 == 0 in int64
 	avgSum := big.NewInt(0).Add(big.NewInt(v2PrioWhenAddedVal2), big.NewInt(v1PrioWhenAddedVal2))
 	avg := avgSum.Div(avgSum, big.NewInt(2))
 	expectedVal2Prio := v2PrioWhenAddedVal2 - avg.Int64()
-	totalPower = 2 * origVotinPower // 10 + 10
-	expectedVal1Prio := oldVal1.ProposerPriority + origVotinPower - avg.Int64() - totalPower
+	totalPower = 2 * val1VotingPower // 10 + 10
+	expectedVal1Prio := oldVal1.ProposerPriority + val1VotingPower - avg.Int64() - totalPower
 	// val1's ProposerPriority story: -10 (see above) + 10 (voting pow) - (-1) (avg) - 20 (total) == -19
 	assert.EqualValues(t, expectedVal1Prio, updatedVal1.ProposerPriority)
 	// val2 prio when added: -(totalVotingPower + (totalVotingPower >> 3)) == -11
@@ -421,10 +564,12 @@ func TestProposerPriorityProposerAlternates(t *testing.T) {
 	assert.Equal(t, val2PubKey.Address(), updatedState3.NextValidators.Proposer.Address)
 	// check if expected proposer prio is matched:
 
-	avgSum = big.NewInt(oldVal1.ProposerPriority + origVotinPower + oldVal2.ProposerPriority + origVotinPower)
+	expectedVal1Prio2 := oldVal1.ProposerPriority + val1VotingPower
+	expectedVal2Prio2 := oldVal2.ProposerPriority + val1VotingPower - totalPower
+	avgSum = big.NewInt(expectedVal1Prio + expectedVal2Prio)
 	avg = avgSum.Div(avgSum, big.NewInt(2))
-	expectedVal1Prio2 := oldVal1.ProposerPriority + origVotinPower - avg.Int64()
-	expectedVal2Prio2 := oldVal2.ProposerPriority + origVotinPower - avg.Int64() - totalPower
+	expectedVal1Prio -= avg.Int64()
+	expectedVal2Prio -= avg.Int64()
 
 	// -19 + 10 - 0 (avg) == -9
 	assert.EqualValues(t, expectedVal1Prio2, updatedVal1.ProposerPriority, "unexpected proposer priority for validator: %v", updatedVal2)
@@ -468,9 +613,8 @@ func TestProposerPriorityProposerAlternates(t *testing.T) {
 func TestLargeGenesisValidator(t *testing.T) {
 	tearDown, _, state := setupTestCase(t)
 	defer tearDown(t)
-	// TODO: increase genesis voting power to sth. more  close to MaxTotalVotingPower with changes that
-	// fix with tendermint/issues/2960; currently, the last iteration would take forever though
-	genesisVotingPower := int64(types.MaxTotalVotingPower / 100000000000000)
+
+	genesisVotingPower := int64(types.MaxTotalVotingPower / 1000)
 	genesisPubKey := ed25519.GenPrivKey().PubKey()
 	// fmt.Println("genesis addr: ", genesisPubKey.Address())
 	genesisVal := &types.Validator{Address: genesisPubKey.Address(), PubKey: genesisPubKey, VotingPower: genesisVotingPower}
@@ -494,11 +638,11 @@ func TestLargeGenesisValidator(t *testing.T) {
 		blockID := types.BlockID{block.Hash(), block.MakePartSet(testPartSize).Header()}
 
 		updatedState, err := updateState(oldState, blockID, &block.Header, abciResponses, validatorUpdates)
-		// no changes in voting power (ProposerPrio += VotingPower == 0 in 1st round; than shiftByAvg == no-op,
+		// no changes in voting power (ProposerPrio += VotingPower == Voting in 1st round; than shiftByAvg == 0,
 		// than -Total == -Voting)
-		// -> no change in ProposerPrio (stays -Total == -VotingPower):
+		// -> no change in ProposerPrio (stays zero):
 		assert.EqualValues(t, oldState.NextValidators, updatedState.NextValidators)
-		assert.EqualValues(t, -genesisVotingPower, updatedState.NextValidators.Proposer.ProposerPriority)
+		assert.EqualValues(t, 0, updatedState.NextValidators.Proposer.ProposerPriority)
 
 		oldState = updatedState
 	}
@@ -508,7 +652,6 @@ func TestLargeGenesisValidator(t *testing.T) {
 	// see how long it takes until the effect wears off and both begin to alternate
 	// see: https://github.com/tendermint/tendermint/issues/2960
 	firstAddedValPubKey := ed25519.GenPrivKey().PubKey()
-	// fmt.Println("first added addr: ", firstAddedValPubKey.Address())
 	firstAddedValVotingPower := int64(10)
 	firstAddedVal := abci.ValidatorUpdate{PubKey: types.TM2PB.PubKey(firstAddedValPubKey), Power: firstAddedValVotingPower}
 	validatorUpdates, err := types.PB2TM.ValidatorUpdates([]abci.ValidatorUpdate{firstAddedVal})
@@ -598,10 +741,33 @@ func TestLargeGenesisValidator(t *testing.T) {
 		}
 		count++
 	}
-	// first proposer change happens after this many iters; we probably want to lower this number:
-	// TODO: change with https://github.com/tendermint/tendermint/issues/2960
-	firstProposerChangeExpectedAfter := 438
+	updatedState = curState
+	// the proposer changes after this number of blocks
+	firstProposerChangeExpectedAfter := 1
 	assert.Equal(t, firstProposerChangeExpectedAfter, count)
+	// store proposers here to see if we see them again in the same order:
+	numVals := len(updatedState.Validators.Validators)
+	proposers := make([]*types.Validator, numVals)
+	for i := 0; i < 100; i++ {
+		// no updates:
+		abciResponses := &ABCIResponses{
+			EndBlock: &abci.ResponseEndBlock{ValidatorUpdates: nil},
+		}
+		validatorUpdates, err := types.PB2TM.ValidatorUpdates(abciResponses.EndBlock.ValidatorUpdates)
+		require.NoError(t, err)
+
+		block := makeBlock(updatedState, updatedState.LastBlockHeight+1)
+		blockID := types.BlockID{block.Hash(), block.MakePartSet(testPartSize).Header()}
+
+		updatedState, err = updateState(updatedState, blockID, &block.Header, abciResponses, validatorUpdates)
+		if i > numVals { // expect proposers to cycle through after the first iteration (of numVals blocks):
+			if proposers[i%numVals] == nil {
+				proposers[i%numVals] = updatedState.NextValidators.Proposer
+			} else {
+				assert.Equal(t, proposers[i%numVals], updatedState.NextValidators.Proposer)
+			}
+		}
+	}
 }
 
 func TestStoreLoadValidatorsIncrementsProposerPriority(t *testing.T) {

types/validator_set.go

@@ -13,13 +13,13 @@ import (
 )
 
 // The maximum allowed total voting power.
-// We set the ProposerPriority of freshly added validators to -1.125*totalVotingPower.
-// To compute 1.125*totalVotingPower efficiently, we do:
-// totalVotingPower + (totalVotingPower >> 3) because
-// x + (x >> 3) = x + x/8 = x * (1 + 0.125).
-// MaxTotalVotingPower is the largest int64 `x` with the property that `x + (x >> 3)` is
-// still in the bounds of int64.
-const MaxTotalVotingPower = int64(8198552921648689607)
+// It needs to be sufficiently small to, in all cases::
+// 1. prevent clipping in incrementProposerPriority()
+// 2. let (diff+diffMax-1) not overflow in IncrementPropposerPriotity()
+// (Proof of 1 is tricky, left to the reader).
+// It could be higher, but this is sufficiently large for our purposes,
+// and leaves room for defensive purposes.
+const MaxTotalVotingPower = int64(math.MaxInt64) / 8
 
 // ValidatorSet represent a set of *Validator at a given height.
 // The validators can be fetched by address or index.
@@ -78,44 +78,57 @@ func (vals *ValidatorSet) IncrementProposerPriority(times int) {
 		panic("Cannot call IncrementProposerPriority with non-positive times")
 	}
 
-	const shiftEveryNthIter = 10
+	// Cap the difference between priorities to be proportional to 2*totalPower by
+	// re-normalizing priorities, i.e., rescale all priorities by multiplying with:
+	//  2*totalVotingPower/(maxPriority - minPriority)
+	diffMax := 2 * vals.TotalVotingPower()
+	vals.RescalePriorities(diffMax)
+
 	var proposer *Validator
 	// call IncrementProposerPriority(1) times times:
 	for i := 0; i < times; i++ {
-		shiftByAvgProposerPriority := i%shiftEveryNthIter == 0
-		proposer = vals.incrementProposerPriority(shiftByAvgProposerPriority)
-	}
-	isShiftedAvgOnLastIter := (times-1)%shiftEveryNthIter == 0
-	if !isShiftedAvgOnLastIter {
-		validatorsHeap := cmn.NewHeap()
-		vals.shiftByAvgProposerPriority(validatorsHeap)
+		proposer = vals.incrementProposerPriority()
 	}
+	vals.shiftByAvgProposerPriority()
+
 	vals.Proposer = proposer
 }
 
-func (vals *ValidatorSet) incrementProposerPriority(subAvg bool) *Validator {
-	for _, val := range vals.Validators {
-		// Check for overflow for sum.
-		val.ProposerPriority = safeAddClip(val.ProposerPriority, val.VotingPower)
+func (vals *ValidatorSet) RescalePriorities(diffMax int64) {
+	// NOTE: This check is merely a sanity check which could be
+	// removed if all tests would init. voting power appropriately;
+	// i.e. diffMax should always be > 0
+	if diffMax == 0 {
+		return
 	}
 
-	validatorsHeap := cmn.NewHeap()
-	if subAvg { // shift by avg ProposerPriority
-		vals.shiftByAvgProposerPriority(validatorsHeap)
-	} else { // just update the heap
+	// Caculating ceil(diff/diffMax):
+	// Re-normalization is performed by dividing by an integer for simplicity.
+	// NOTE: This may make debugging priority issues easier as well.
+	diff := computeMaxMinPriorityDiff(vals)
+	ratio := (diff + diffMax - 1) / diffMax
+	if ratio > 1 {
 		for _, val := range vals.Validators {
-			validatorsHeap.PushComparable(val, proposerPriorityComparable{val})
+			val.ProposerPriority /= ratio
 		}
 	}
+}
 
+func (vals *ValidatorSet) incrementProposerPriority() *Validator {
+	for _, val := range vals.Validators {
+		// Check for overflow for sum.
+		newPrio := safeAddClip(val.ProposerPriority, val.VotingPower)
+		val.ProposerPriority = newPrio
+	}
 	// Decrement the validator with most ProposerPriority:
-	mostest := validatorsHeap.Peek().(*Validator)
+	mostest := vals.getValWithMostPriority()
 	// mind underflow
 	mostest.ProposerPriority = safeSubClip(mostest.ProposerPriority, vals.TotalVotingPower())
 
 	return mostest
 }
 
+// should not be called on an empty validator set
 func (vals *ValidatorSet) computeAvgProposerPriority() int64 {
 	n := int64(len(vals.Validators))
 	sum := big.NewInt(0)
@@ -131,11 +144,38 @@ func (vals *ValidatorSet) computeAvgProposerPriority() int64 {
 	panic(fmt.Sprintf("Cannot represent avg ProposerPriority as an int64 %v", avg))
 }
 
-func (vals *ValidatorSet) shiftByAvgProposerPriority(validatorsHeap *cmn.Heap) {
+// compute the difference between the max and min ProposerPriority of that set
+func computeMaxMinPriorityDiff(vals *ValidatorSet) int64 {
+	max := int64(math.MinInt64)
+	min := int64(math.MaxInt64)
+	for _, v := range vals.Validators {
+		if v.ProposerPriority < min {
+			min = v.ProposerPriority
+		}
+		if v.ProposerPriority > max {
+			max = v.ProposerPriority
+		}
+	}
+	diff := max - min
+	if diff < 0 {
+		return -1 * diff
+	} else {
+		return diff
+	}
+}
+
+func (vals *ValidatorSet) getValWithMostPriority() *Validator {
+	var res *Validator
+	for _, val := range vals.Validators {
+		res = res.CompareProposerPriority(val)
+	}
+	return res
+}
+
+func (vals *ValidatorSet) shiftByAvgProposerPriority() {
 	avgProposerPriority := vals.computeAvgProposerPriority()
 	for _, val := range vals.Validators {
 		val.ProposerPriority = safeSubClip(val.ProposerPriority, avgProposerPriority)
-		validatorsHeap.PushComparable(val, proposerPriorityComparable{val})
 	}
 }
 
@@ -508,20 +548,6 @@ func (valz ValidatorsByAddress) Swap(i, j int) {
 	valz[j] = it
 }
 
-//-------------------------------------
-// Use with Heap for sorting validators by ProposerPriority
-
-type proposerPriorityComparable struct {
-	*Validator
-}
-
-// We want to find the validator with the greatest ProposerPriority.
-func (ac proposerPriorityComparable) Less(o interface{}) bool {
-	other := o.(proposerPriorityComparable).Validator
-	larger := ac.CompareProposerPriority(other)
-	return bytes.Equal(larger.Address, ac.Address)
-}
-
 //----------------------------------------
 // For testing
 

types/validator_set_test.go

@@ -392,10 +392,16 @@ func TestAveragingInIncrementProposerPriority(t *testing.T) {
 func TestAveragingInIncrementProposerPriorityWithVotingPower(t *testing.T) {
 	// Other than TestAveragingInIncrementProposerPriority this is a more complete test showing
 	// how each ProposerPriority changes in relation to the validator's voting power respectively.
+	// average is zero in each round:
+	vp0 := int64(10)
+	vp1 := int64(1)
+	vp2 := int64(1)
+	total := vp0 + vp1 + vp2
+	avg := (vp0 + vp1 + vp2 - total) / 3
 	vals := ValidatorSet{Validators: []*Validator{
-		{Address: []byte{0}, ProposerPriority: 0, VotingPower: 10},
-		{Address: []byte{1}, ProposerPriority: 0, VotingPower: 1},
-		{Address: []byte{2}, ProposerPriority: 0, VotingPower: 1}}}
+		{Address: []byte{0}, ProposerPriority: 0, VotingPower: vp0},
+		{Address: []byte{1}, ProposerPriority: 0, VotingPower: vp1},
+		{Address: []byte{2}, ProposerPriority: 0, VotingPower: vp2}}}
 	tcs := []struct {
 		vals                  *ValidatorSet
 		wantProposerPrioritys []int64
@@ -407,95 +413,89 @@ func TestAveragingInIncrementProposerPriorityWithVotingPower(t *testing.T) {
 			vals.Copy(),
 			[]int64{
 				// Acumm+VotingPower-Avg:
-				0 + 10 - 12 - 4, // mostest will be subtracted by total voting power (12)
-				0 + 1 - 4,
-				0 + 1 - 4},
+				0 + vp0 - total - avg, // mostest will be subtracted by total voting power (12)
+				0 + vp1,
+				0 + vp2},
 			1,
 			vals.Validators[0]},
 		1: {
 			vals.Copy(),
 			[]int64{
-				(0 + 10 - 12 - 4) + 10 - 12 + 4, // this will be mostest on 2nd iter, too
-				(0 + 1 - 4) + 1 + 4,
-				(0 + 1 - 4) + 1 + 4},
+				(0 + vp0 - total) + vp0 - total - avg, // this will be mostest on 2nd iter, too
+				(0 + vp1) + vp1,
+				(0 + vp2) + vp2},
 			2,
 			vals.Validators[0]}, // increment twice -> expect average to be subtracted twice
 		2: {
 			vals.Copy(),
 			[]int64{
-				((0 + 10 - 12 - 4) + 10 - 12) + 10 - 12 + 4, // still mostest
-				((0 + 1 - 4) + 1) + 1 + 4,
-				((0 + 1 - 4) + 1) + 1 + 4},
+				0 + 3*(vp0-total) - avg, // still mostest
+				0 + 3*vp1,
+				0 + 3*vp2},
 			3,
 			vals.Validators[0]},
 		3: {
 			vals.Copy(),
 			[]int64{
-				0 + 4*(10-12) + 4 - 4, // still mostest
-				0 + 4*1 + 4 - 4,
-				0 + 4*1 + 4 - 4},
+				0 + 4*(vp0-total), // still mostest
+				0 + 4*vp1,
+				0 + 4*vp2},
 			4,
 			vals.Validators[0]},
 		4: {
 			vals.Copy(),
 			[]int64{
-				0 + 4*(10-12) + 10 + 4 - 4, // 4 iters was mostest
-				0 + 5*1 - 12 + 4 - 4,       // now this val is mostest for the 1st time (hence -12==totalVotingPower)
-				0 + 5*1 + 4 - 4},
+				0 + 4*(vp0-total) + vp0, // 4 iters was mostest
+				0 + 5*vp1 - total,       // now this val is mostest for the 1st time (hence -12==totalVotingPower)
+				0 + 5*vp2},
 			5,
 			vals.Validators[1]},
 		5: {
 			vals.Copy(),
 			[]int64{
-				0 + 6*10 - 5*12 + 4 - 4, // mostest again
-				0 + 6*1 - 12 + 4 - 4,    // mostest once up to here
-				0 + 6*1 + 4 - 4},
+				0 + 6*vp0 - 5*total, // mostest again
+				0 + 6*vp1 - total,   // mostest once up to here
+				0 + 6*vp2},
 			6,
 			vals.Validators[0]},
 		6: {
 			vals.Copy(),
 			[]int64{
-				0 + 7*10 - 6*12 + 4 - 4, // in 7 iters this val is mostest 6 times
-				0 + 7*1 - 12 + 4 - 4,    // in 7 iters this val is mostest 1 time
-				0 + 7*1 + 4 - 4},
+				0 + 7*vp0 - 6*total, // in 7 iters this val is mostest 6 times
+				0 + 7*vp1 - total,   // in 7 iters this val is mostest 1 time
+				0 + 7*vp2},
 			7,
 			vals.Validators[0]},
 		7: {
 			vals.Copy(),
 			[]int64{
-				0 + 8*10 - 7*12 + 4 - 4, // mostest
-				0 + 8*1 - 12 + 4 - 4,
-				0 + 8*1 + 4 - 4},
+				0 + 8*vp0 - 7*total, // mostest again
+				0 + 8*vp1 - total,
+				0 + 8*vp2},
 			8,
 			vals.Validators[0]},
 		8: {
 			vals.Copy(),
 			[]int64{
-				0 + 9*10 - 7*12 + 4 - 4,
-				0 + 9*1 - 12 + 4 - 4,
-				0 + 9*1 - 12 + 4 - 4}, // mostest
+				0 + 9*vp0 - 7*total,
+				0 + 9*vp1 - total,
+				0 + 9*vp2 - total}, // mostest
 			9,
 			vals.Validators[2]},
 		9: {
 			vals.Copy(),
 			[]int64{
-				0 + 10*10 - 8*12 + 4 - 4, // after 10 iters this is mostest again
-				0 + 10*1 - 12 + 4 - 4,    // after 6 iters this val is "mostest" once and not in between
-				0 + 10*1 - 12 + 4 - 4},   // in between 10 iters this val is "mostest" once
+				0 + 10*vp0 - 8*total, // after 10 iters this is mostest again
+				0 + 10*vp1 - total,   // after 6 iters this val is "mostest" once and not in between
+				0 + 10*vp2 - total},  // in between 10 iters this val is "mostest" once
 			10,
 			vals.Validators[0]},
 		10: {
 			vals.Copy(),
 			[]int64{
-				// shift twice inside incrementProposerPriority (shift every 10th iter);
-				// don't shift at the end of IncremenctProposerPriority
-				// last avg should be zero because
-				// ProposerPriority of validator 0: (0 + 11*10 - 8*12 - 4) == 10
-				// ProposerPriority of validator 1 and 2: (0 + 11*1 - 12 - 4) == -5
-				// and (10 + 5 - 5) / 3 == 0
-				0 + 11*10 - 8*12 - 4 - 12 - 0,
-				0 + 11*1 - 12 - 4 - 0,  // after 6 iters this val is "mostest" once and not in between
-				0 + 11*1 - 12 - 4 - 0}, // after 10 iters this val is "mostest" once
+				0 + 11*vp0 - 9*total,
+				0 + 11*vp1 - total,  // after 6 iters this val is "mostest" once and not in between
+				0 + 11*vp2 - total}, // after 10 iters this val is "mostest" once
 			11,
 			vals.Validators[0]},
 	}