treat validator updates as set

state/execution.go

@@ -2,7 +2,6 @@ package state
 
 import (
 	"fmt"
-	"strings"
 	"time"
 
 	abci "github.com/tendermint/tendermint/abci/types"
@@ -143,7 +142,7 @@ func (blockExec *BlockExecutor) ApplyBlock(state State, blockID types.BlockID, b
 		return state, err
 	}
 	if len(validatorUpdates) > 0 {
-		blockExec.logger.Info("Updates to validators", "updates", makeValidatorUpdatesLogString(validatorUpdates))
+		blockExec.logger.Info("Updates to validators", "updates", types.ValidatorListString(validatorUpdates))
 	}
 
 	// Update the state with the block and responses.
@@ -368,70 +367,6 @@ func validateValidatorUpdates(abciUpdates []abci.ValidatorUpdate,
 	return nil
 }
 
-// If more or equal than 1/3 of total voting power changed in one block, then
-// a light client could never prove the transition externally. See
-// ./lite/doc.go for details on how a light client tracks validators.
-func updateValidators(currentSet *types.ValidatorSet, updates []*types.Validator) error {
-	for _, valUpdate := range updates {
-		// should already have been checked
-		if valUpdate.VotingPower < 0 {
-			return fmt.Errorf("Voting power can't be negative %v", valUpdate)
-		}
-
-		address := valUpdate.Address
-		_, val := currentSet.GetByAddress(address)
-		// valUpdate.VotingPower is ensured to be non-negative in validation method
-		if valUpdate.VotingPower == 0 { // remove val
-			_, removed := currentSet.Remove(address)
-			if !removed {
-				return fmt.Errorf("Failed to remove validator %X", address)
-			}
-		} else if val == nil { // add val
-			// make sure we do not exceed MaxTotalVotingPower by adding this validator:
-			totalVotingPower := currentSet.TotalVotingPower()
-			updatedVotingPower := valUpdate.VotingPower + totalVotingPower
-			overflow := updatedVotingPower > types.MaxTotalVotingPower || updatedVotingPower < 0
-			if overflow {
-				return fmt.Errorf(
-					"Failed to add new validator %v. Adding it would exceed max allowed total voting power %v",
-					valUpdate,
-					types.MaxTotalVotingPower)
-			}
-			// TODO: issue #1558 update spec according to the following:
-			// Set ProposerPriority to -C*totalVotingPower (with C ~= 1.125) to make sure validators can't
-			// unbond/rebond to reset their (potentially previously negative) ProposerPriority to zero.
-			//
-			// Contract: totalVotingPower < MaxTotalVotingPower to ensure ProposerPriority does
-			// not exceed the bounds of int64.
-			//
-			// Compute ProposerPriority = -1.125*totalVotingPower == -(totalVotingPower + (totalVotingPower >> 3)).
-			valUpdate.ProposerPriority = -(totalVotingPower + (totalVotingPower >> 3))
-			added := currentSet.Add(valUpdate)
-			if !added {
-				return fmt.Errorf("Failed to add new validator %v", valUpdate)
-			}
-		} else { // update val
-			// make sure we do not exceed MaxTotalVotingPower by updating this validator:
-			totalVotingPower := currentSet.TotalVotingPower()
-			curVotingPower := val.VotingPower
-			updatedVotingPower := totalVotingPower - curVotingPower + valUpdate.VotingPower
-			overflow := updatedVotingPower > types.MaxTotalVotingPower || updatedVotingPower < 0
-			if overflow {
-				return fmt.Errorf(
-					"Failed to update existing validator %v. Updating it would exceed max allowed total voting power %v",
-					valUpdate,
-					types.MaxTotalVotingPower)
-			}
-
-			updated := currentSet.Update(valUpdate)
-			if !updated {
-				return fmt.Errorf("Failed to update validator %X to %v", address, valUpdate)
-			}
-		}
-	}
-	return nil
-}
-
 // updateState returns a new State updated according to the header and responses.
 func updateState(
 	state State,
@@ -448,7 +383,7 @@ func updateState(
 	// Update the validator set with the latest abciResponses.
 	lastHeightValsChanged := state.LastHeightValidatorsChanged
 	if len(validatorUpdates) > 0 {
-		err := updateValidators(nValSet, validatorUpdates)
+		err := nValSet.UpdateWithChangeSet(validatorUpdates)
 		if err != nil {
 			return state, fmt.Errorf("Error changing validator set: %v", err)
 		}
@@ -552,13 +487,3 @@ func ExecCommitBlock(
 	// ResponseCommit has no error or log, just data
 	return res.Data, nil
 }
-
-// Make pretty string for validatorUpdates logging
-func makeValidatorUpdatesLogString(vals []*types.Validator) string {
-	chunks := make([]string, len(vals))
-	for i, val := range vals {
-		chunks[i] = fmt.Sprintf("%s:%d", val.Address, val.VotingPower)
-	}
-
-	return strings.Join(chunks, ",")
-}

state/execution_test.go

@@ -280,7 +280,7 @@ func TestUpdateValidators(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			updates, err := types.PB2TM.ValidatorUpdates(tc.abciUpdates)
 			assert.NoError(t, err)
-			err = updateValidators(tc.currentSet, updates)
+			err = tc.currentSet.UpdateWithChangeSet(updates)
 			if tc.shouldErr {
 				assert.Error(t, err)
 			} else {

state/state_test.go

@@ -322,7 +322,8 @@ func TestProposerFrequency(t *testing.T) {
 		vals := make([]*types.Validator, N)
 		totalVotePower := int64(0)
 		for j := 0; j < N; j++ {
-			votePower := int64(cmn.RandInt() % maxPower)
+			// make sure votePower > 0
+			votePower := int64(cmn.RandInt()%maxPower) + 1
 			totalVotePower += votePower
 			privVal := types.NewMockPV()
 			pubKey := privVal.GetPubKey()
@@ -424,49 +425,71 @@ func TestProposerPriorityDoesNotGetResetToZero(t *testing.T) {
 	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 := curTotal
-	// while adding we compute prio == -1.125 * total:
-	wantVal2ProposerPrio := -(totalPowerBefore2 + (totalPowerBefore2 >> 3))
-	wantVal2ProposerPrio = wantVal2ProposerPrio + val2VotingPower
-	// then increment:
+	// Steps from adding new validator:
+	// 0 - val1 prio is 0, TVP after add:
+	wantVal1Prio := int64(0)
 	totalPowerAfter := val1VotingPower + val2VotingPower
-	// mostest:
-	wantVal2ProposerPrio = wantVal2ProposerPrio - totalPowerAfter
-	avg := big.NewInt(0).Add(big.NewInt(val1VotingPower), big.NewInt(wantVal2ProposerPrio))
+	// 1. Add - Val2 should be initially added with (-123) =>
+	wantVal2Prio := -(totalPowerAfter + (totalPowerAfter >> 3))
+	// 2. Scale - noop
+	// 3. Center - with avg, resulting val2:-61, val1:62
+	avg := big.NewInt(0).Add(big.NewInt(wantVal1Prio), big.NewInt(wantVal2Prio))
 	avg.Div(avg, big.NewInt(2))
-	wantVal2ProposerPrio = wantVal2ProposerPrio - avg.Int64()
-	wantVal1Prio := 0 + val1VotingPower - avg.Int64()
+	wantVal2Prio = wantVal2Prio - avg.Int64() // -61
+	wantVal1Prio = wantVal1Prio - avg.Int64() // 62
+
+	// 4. Steps from IncrementProposerPriority
+	wantVal1Prio = wantVal1Prio + val1VotingPower // 72
+	wantVal2Prio = wantVal2Prio + val2VotingPower // 39
+	wantVal1Prio = wantVal1Prio - totalPowerAfter // -38 as val1 is proposer
+
 	assert.Equal(t, wantVal1Prio, updatedVal1.ProposerPriority)
-	assert.Equal(t, wantVal2ProposerPrio, addedVal2.ProposerPriority)
+	assert.Equal(t, wantVal2Prio, addedVal2.ProposerPriority)
 
 	// Updating a validator does not reset the ProposerPriority to zero:
+	// 1. Add - Val2 VotingPower change to 1 =>
 	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)
+	// this will cause the diff of priorities (77)
 	// to be larger than threshold == 2*totalVotingPower (22):
 	updatedState3, err := updateState(updatedState2, blockID, &block.Header, abciResponses, validatorUpdates)
 	assert.NoError(t, err)
 
 	require.Equal(t, len(updatedState3.NextValidators.Validators), 2)
 	_, prevVal1 := updatedState3.Validators.GetByAddress(val1PubKey.Address())
+	_, prevVal2 := updatedState3.Validators.GetByAddress(val2PubKey.Address())
+	_, updatedVal1 = updatedState3.NextValidators.GetByAddress(val1PubKey.Address())
 	_, updatedVal2 := updatedState3.NextValidators.GetByAddress(val2PubKey.Address())
 
-	// 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())
+	// 2. Scale
+	// old prios: v1(10):-38, v2(1):39
+	wantVal1Prio = prevVal1.ProposerPriority
+	wantVal2Prio = prevVal2.ProposerPriority
+	// scale to diffMax = 22 = 2 * tvp, diff=39-(-38)=77
+	// new totalPower
+	totalPower := updatedVal1.VotingPower + updatedVal2.VotingPower
+	dist := wantVal2Prio - wantVal1Prio
+	// ratio := (dist + 2*totalPower - 1) / 2*totalPower = 98/22 = 4
+	ratio := (dist + 2*totalPower - 1) / (2 * totalPower)
+	// v1(10):-38/4, v2(1):39/4
+	wantVal1Prio /= ratio // -9
+	wantVal2Prio /= ratio // 9
+
+	// 3. Center - noop
+	// 4. IncrementProposerPriority() ->
+	// v1(10):-9+10, v2(1):9+1 -> v2 proposer so substract tvp(11)
+	// v1(10):1, v2(1):-1
+	wantVal2Prio += updatedVal2.VotingPower // 10 -> prop
+	wantVal1Prio += updatedVal1.VotingPower // 1
+	wantVal2Prio -= totalPower              // -1
+
+	assert.Equal(t, wantVal2Prio, updatedVal2.ProposerPriority)
 	assert.Equal(t, wantVal1Prio, updatedVal1.ProposerPriority)
 }
 
@@ -527,22 +550,22 @@ func TestProposerPriorityProposerAlternates(t *testing.T) {
 	_, oldVal1 := updatedState2.Validators.GetByAddress(val1PubKey.Address())
 	_, updatedVal2 := updatedState2.NextValidators.GetByAddress(val2PubKey.Address())
 
-	totalPower = val1VotingPower // no update
-	v2PrioWhenAddedVal2 := -(totalPower + (totalPower >> 3))
-	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 * 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
+	// 1. Add
+	val2VotingPower := val1VotingPower
+	totalPower = val1VotingPower + val2VotingPower           // 20
+	v2PrioWhenAddedVal2 := -(totalPower + (totalPower >> 3)) // -22
+	// 2. Scale - noop
+	// 3. Center
+	avgSum := big.NewInt(0).Add(big.NewInt(v2PrioWhenAddedVal2), big.NewInt(oldVal1.ProposerPriority))
+	avg := avgSum.Div(avgSum, big.NewInt(2))                   // -11
+	expectedVal2Prio := v2PrioWhenAddedVal2 - avg.Int64()      // -11
+	expectedVal1Prio := oldVal1.ProposerPriority - avg.Int64() // 11
+	// 4. Increment
+	expectedVal2Prio = expectedVal2Prio + val2VotingPower // -11 + 10 = -1
+	expectedVal1Prio = expectedVal1Prio + val1VotingPower // 11 + 10 == 21
+	expectedVal1Prio = expectedVal1Prio - totalPower      // 1, val1 proposer
+
 	assert.EqualValues(t, expectedVal1Prio, updatedVal1.ProposerPriority)
-	// val2 prio when added: -(totalVotingPower + (totalVotingPower >> 3)) == -11
-	// -> -11 + 10  (voting power) - (-1) (avg) == 0
 	assert.EqualValues(t, expectedVal2Prio, updatedVal2.ProposerPriority, "unexpected proposer priority for validator: %v", updatedVal2)
 
 	validatorUpdates, err = types.PB2TM.ValidatorUpdates(abciResponses.EndBlock.ValidatorUpdates)
@@ -551,34 +574,40 @@ func TestProposerPriorityProposerAlternates(t *testing.T) {
 	updatedState3, err := updateState(updatedState2, blockID, &block.Header, abciResponses, validatorUpdates)
 	assert.NoError(t, err)
 
-	// proposer changes from now on (every iteration) as long as there are no changes in the validator set:
-	assert.NotEqual(t, updatedState3.Validators.Proposer.Address, updatedState3.NextValidators.Proposer.Address)
+	assert.Equal(t, updatedState3.Validators.Proposer.Address, updatedState3.NextValidators.Proposer.Address)
 
 	assert.Equal(t, updatedState3.Validators, updatedState2.NextValidators)
 	_, updatedVal1 = updatedState3.NextValidators.GetByAddress(val1PubKey.Address())
-	_, oldVal1 = updatedState3.Validators.GetByAddress(val1PubKey.Address())
 	_, updatedVal2 = updatedState3.NextValidators.GetByAddress(val2PubKey.Address())
-	_, oldVal2 := updatedState3.Validators.GetByAddress(val2PubKey.Address())
 
-	// val2 will be proposer:
-	assert.Equal(t, val2PubKey.Address(), updatedState3.NextValidators.Proposer.Address)
-	// check if expected proposer prio is matched:
+	// val1 will still be proposer:
+	assert.Equal(t, val1PubKey.Address(), updatedState3.NextValidators.Proposer.Address)
 
-	expectedVal1Prio2 := oldVal1.ProposerPriority + val1VotingPower
-	expectedVal2Prio2 := oldVal2.ProposerPriority + val1VotingPower - totalPower
-	avgSum = big.NewInt(expectedVal1Prio + expectedVal2Prio)
-	avg = avgSum.Div(avgSum, big.NewInt(2))
-	expectedVal1Prio -= avg.Int64()
-	expectedVal2Prio -= avg.Int64()
+	// check if expected proposer prio is matched:
+	// Increment
+	expectedVal2Prio2 := expectedVal2Prio + val2VotingPower // -1 + 10 = 9
+	expectedVal1Prio2 := expectedVal1Prio + val1VotingPower // 1 + 10 == 11
+	expectedVal1Prio2 = expectedVal1Prio2 - totalPower      // -9, val1 proposer
 
-	// -19 + 10 - 0 (avg) == -9
 	assert.EqualValues(t, expectedVal1Prio2, updatedVal1.ProposerPriority, "unexpected proposer priority for validator: %v", updatedVal2)
-	// 0 + 10 - 0 (avg) - 20 (total) == -10
 	assert.EqualValues(t, expectedVal2Prio2, updatedVal2.ProposerPriority, "unexpected proposer priority for validator: %v", updatedVal2)
 
 	// no changes in voting power and both validators have same voting power
 	// -> proposers should alternate:
 	oldState := updatedState3
+	abciResponses = &ABCIResponses{
+		EndBlock: &abci.ResponseEndBlock{ValidatorUpdates: nil},
+	}
+	validatorUpdates, err = types.PB2TM.ValidatorUpdates(abciResponses.EndBlock.ValidatorUpdates)
+	require.NoError(t, err)
+
+	oldState, err = updateState(oldState, blockID, &block.Header, abciResponses, validatorUpdates)
+	assert.NoError(t, err)
+	expectedVal1Prio2 = 1
+	expectedVal2Prio2 = -1
+	expectedVal1Prio = -9
+	expectedVal2Prio = 9
+
 	for i := 0; i < 1000; i++ {
 		// no validator updates:
 		abciResponses := &ABCIResponses{

types/validator.go

@@ -3,6 +3,7 @@ package types
 import (
 	"bytes"
 	"fmt"
+	"strings"
 
 	"github.com/tendermint/tendermint/crypto"
 	cmn "github.com/tendermint/tendermint/libs/common"
@@ -68,6 +69,16 @@ func (v *Validator) String() string {
 		v.ProposerPriority)
 }
 
+// Make pretty string for logging
+func ValidatorListString(vals []*Validator) string {
+	chunks := make([]string, len(vals))
+	for i, val := range vals {
+		chunks[i] = fmt.Sprintf("%s:%d", val.Address, val.VotingPower)
+	}
+
+	return strings.Join(chunks, ",")
+}
+
 // Bytes computes the unique encoding of a validator with a given voting power.
 // These are the bytes that gets hashed in consensus. It excludes address
 // as its redundant with the pubkey. This also excludes ProposerPriority