Merge pull request #1898 from tendermint/1893-err-if-zero-power-val

state: err if 0 power validator is added to the validator set

state/execution.go

@@ -5,10 +5,10 @@ import (
 
 	fail "github.com/ebuchman/fail-test"
 	abci "github.com/tendermint/tendermint/abci/types"
-	"github.com/tendermint/tendermint/proxy"
-	"github.com/tendermint/tendermint/types"
 	dbm "github.com/tendermint/tendermint/libs/db"
 	"github.com/tendermint/tendermint/libs/log"
+	"github.com/tendermint/tendermint/proxy"
+	"github.com/tendermint/tendermint/types"
 )
 
 //-----------------------------------------------------------------------------
@@ -278,20 +278,24 @@ func updateValidators(currentSet *types.ValidatorSet, abciUpdates []abci.Validat
 
 	// these are tendermint types now
 	for _, valUpdate := range updates {
+		if valUpdate.VotingPower < 0 {
+			return fmt.Errorf("Voting power can't be negative %v", valUpdate)
+		}
+
 		address := valUpdate.Address
 		_, val := currentSet.GetByAddress(address)
-		if val == nil {
-			// add val
-			added := currentSet.Add(valUpdate)
-			if !added {
-				return fmt.Errorf("Failed to add new validator %v", valUpdate)
-			}
-		} else if valUpdate.VotingPower == 0 {
+		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
+			added := currentSet.Add(valUpdate)
+			if !added {
+				return fmt.Errorf("Failed to add new validator %v", valUpdate)
+			}
 		} else {
 			// update val
 			updated := currentSet.Update(valUpdate)

state/execution_test.go

@@ -149,6 +149,89 @@ func TestBeginBlockByzantineValidators(t *testing.T) {
 	}
 }
 
+func TestUpdateValidators(t *testing.T) {
+	pubkey1 := crypto.GenPrivKeyEd25519().PubKey()
+	val1 := types.NewValidator(pubkey1, 10)
+	pubkey2 := crypto.GenPrivKeyEd25519().PubKey()
+	val2 := types.NewValidator(pubkey2, 20)
+
+	testCases := []struct {
+		name string
+
+		currentSet  *types.ValidatorSet
+		abciUpdates []abci.Validator
+
+		resultingSet *types.ValidatorSet
+		shouldErr    bool
+	}{
+		{
+			"adding a validator is OK",
+
+			types.NewValidatorSet([]*types.Validator{val1}),
+			[]abci.Validator{{[]byte{}, types.TM2PB.PubKey(pubkey2), 20}},
+
+			types.NewValidatorSet([]*types.Validator{val1, val2}),
+			false,
+		},
+		{
+			"updating a validator is OK",
+
+			types.NewValidatorSet([]*types.Validator{val1}),
+			[]abci.Validator{{[]byte{}, types.TM2PB.PubKey(pubkey1), 20}},
+
+			types.NewValidatorSet([]*types.Validator{types.NewValidator(pubkey1, 20)}),
+			false,
+		},
+		{
+			"removing a validator is OK",
+
+			types.NewValidatorSet([]*types.Validator{val1, val2}),
+			[]abci.Validator{{[]byte{}, types.TM2PB.PubKey(pubkey2), 0}},
+
+			types.NewValidatorSet([]*types.Validator{val1}),
+			false,
+		},
+
+		{
+			"removing a non-existing validator results in error",
+
+			types.NewValidatorSet([]*types.Validator{val1}),
+			[]abci.Validator{{[]byte{}, types.TM2PB.PubKey(pubkey2), 0}},
+
+			types.NewValidatorSet([]*types.Validator{val1}),
+			true,
+		},
+
+		{
+			"adding a validator with negative power results in error",
+
+			types.NewValidatorSet([]*types.Validator{val1}),
+			[]abci.Validator{{[]byte{}, types.TM2PB.PubKey(pubkey2), -100}},
+
+			types.NewValidatorSet([]*types.Validator{val1}),
+			true,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			err := updateValidators(tc.currentSet, tc.abciUpdates)
+			if tc.shouldErr {
+				assert.Error(t, err)
+			} else {
+				require.Equal(t, tc.resultingSet.Size(), tc.currentSet.Size())
+
+				assert.Equal(t, tc.resultingSet.TotalVotingPower(), tc.currentSet.TotalVotingPower())
+
+				assert.Equal(t, tc.resultingSet.Validators[0].Address, tc.currentSet.Validators[0].Address)
+				if tc.resultingSet.Size() > 1 {
+					assert.Equal(t, tc.resultingSet.Validators[1].Address, tc.currentSet.Validators[1].Address)
+				}
+			}
+		})
+	}
+}
+
 //----------------------------------------------------------------------------
 
 // make some bogus txs