rpc: Fix response time grow over time (#3537)

* rpc: store validator info periodly

* increase ValidatorSetStoreInterval

also

- unexpose it
- add a comment
- refactor code
- add a benchmark, which shows that 100000 results in ~ 100ms to get 100
validators

* make the change non-breaking

* expand comment

* rename valSetStoreInterval to valSetCheckpointInterval

* change the panic msg

* add a test and changelog entry

* update changelog entry

* update changelog entry

* add a link to PR

* fix test

* Update CHANGELOG_PENDING.md

Co-Authored-By: melekes <anton.kalyaev@gmail.com>

* update comment

* use MaxInt64 func

CHANGELOG_PENDING.md

@@ -20,5 +20,14 @@
 - [p2p] [\#3463](https://github.com/tendermint/tendermint/pull/3463) Do not log "Can't add peer's address to addrbook" error for a private peer
 
 ### BUG FIXES:
+
+- [state] [\#3438](https://github.com/tendermint/tendermint/pull/3438) 
+  Persist validators every 100000 blocks even if no changes to the set
+  occurred (@guagualvcha). This
+  1) Prevents possible DoS attack using `/validators` or `/status` RPC
+  endpoints. Before response time was growing linearly with height if no
+  changes were made to the validator set.
+  2) Fixes performance degradation in `ExecCommitBlock` where we call
+  `LoadValidators` for each `Evidence` in the block.
 - [p2p] \#2716 Check if we're already connected to peer right before dialing it (@melekes)
 - [docs] \#3514 Fix block.Header.Time description (@melekes)

state/store.go

@@ -9,6 +9,14 @@ import (
 	"github.com/tendermint/tendermint/types"
 )
 
+const (
+	// persist validators every valSetCheckpointInterval blocks to avoid
+	// LoadValidators taking too much time.
+	// https://github.com/tendermint/tendermint/pull/3438
+	// 100000 results in ~ 100ms to get 100 validators (see BenchmarkLoadValidators)
+	valSetCheckpointInterval = 100000
+)
+
 //------------------------------------------------------------------------
 
 func calcValidatorsKey(height int64) []byte {
@@ -182,25 +190,38 @@ func LoadValidators(db dbm.DB, height int64) (*types.ValidatorSet, error) {
 	if valInfo == nil {
 		return nil, ErrNoValSetForHeight{height}
 	}
-
 	if valInfo.ValidatorSet == nil {
-		valInfo2 := loadValidatorsInfo(db, valInfo.LastHeightChanged)
+		lastStoredHeight := lastStoredHeightFor(height, valInfo.LastHeightChanged)
+		valInfo2 := loadValidatorsInfo(db, lastStoredHeight)
 		if valInfo2 == nil {
-			panic(
-				fmt.Sprintf(
-					"Couldn't find validators at height %d as last changed from height %d",
-					valInfo.LastHeightChanged,
-					height,
-				),
-			)
+			// TODO (melekes): remove the below if condition in the 0.33 major
+			// release and just panic. Old chains might panic otherwise if they
+			// haven't saved validators at intermediate (%valSetCheckpointInterval)
+			// height yet.
+			// https://github.com/tendermint/tendermint/issues/3543
+			valInfo2 = loadValidatorsInfo(db, valInfo.LastHeightChanged)
+			lastStoredHeight = valInfo.LastHeightChanged
+			if valInfo2 == nil {
+				panic(
+					fmt.Sprintf("Couldn't find validators at height %d (height %d was originally requested)",
+						lastStoredHeight,
+						height,
+					),
+				)
+			}
 		}
-		valInfo2.ValidatorSet.IncrementProposerPriority(int(height - valInfo.LastHeightChanged)) // mutate
+		valInfo2.ValidatorSet.IncrementProposerPriority(int(height - lastStoredHeight)) // mutate
 		valInfo = valInfo2
 	}
 
 	return valInfo.ValidatorSet, nil
 }
 
+func lastStoredHeightFor(height, lastHeightChanged int64) int64 {
+	checkpointHeight := height - height%valSetCheckpointInterval
+	return cmn.MaxInt64(checkpointHeight, lastHeightChanged)
+}
+
 // CONTRACT: Returned ValidatorsInfo can be mutated.
 func loadValidatorsInfo(db dbm.DB, height int64) *ValidatorsInfo {
 	buf := db.Get(calcValidatorsKey(height))
@@ -221,18 +242,20 @@ func loadValidatorsInfo(db dbm.DB, height int64) *ValidatorsInfo {
 }
 
 // saveValidatorsInfo persists the validator set.
-// `height` is the effective height for which the validator is responsible for signing.
-// It should be called from s.Save(), right before the state itself is persisted.
-// If the validator set did not change after processing the latest block,
-// only the last height for which the validators changed is persisted.
+//
+// `height` is the effective height for which the validator is responsible for
+// signing. It should be called from s.Save(), right before the state itself is
+// persisted.
 func saveValidatorsInfo(db dbm.DB, height, lastHeightChanged int64, valSet *types.ValidatorSet) {
 	if lastHeightChanged > height {
 		panic("LastHeightChanged cannot be greater than ValidatorsInfo height")
 	}
 	valInfo := &ValidatorsInfo{
 		LastHeightChanged: lastHeightChanged,
 	}
-	if lastHeightChanged == height {
+	// Only persist validator set if it was updated or checkpoint height (see
+	// valSetCheckpointInterval) is reached.
+	if height == lastHeightChanged || height%valSetCheckpointInterval == 0 {
 		valInfo.ValidatorSet = valSet
 	}
 	db.Set(calcValidatorsKey(height), valInfo.Bytes())

state/store_test.go

@@ -0,0 +1,67 @@
+package state
+
+import (
+	"fmt"
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	cfg "github.com/tendermint/tendermint/config"
+	dbm "github.com/tendermint/tendermint/libs/db"
+	"github.com/tendermint/tendermint/types"
+)
+
+func TestSaveValidatorsInfo(t *testing.T) {
+	// test we persist validators every valSetCheckpointInterval blocks
+	stateDB := dbm.NewMemDB()
+	val, _ := types.RandValidator(true, 10)
+	vals := types.NewValidatorSet([]*types.Validator{val})
+
+	// TODO(melekes): remove in 0.33 release
+	// https://github.com/tendermint/tendermint/issues/3543
+	saveValidatorsInfo(stateDB, 1, 1, vals)
+	saveValidatorsInfo(stateDB, 2, 1, vals)
+	assert.NotPanics(t, func() {
+		_, err := LoadValidators(stateDB, 2)
+		if err != nil {
+			panic(err)
+		}
+	})
+	//ENDREMOVE
+
+	saveValidatorsInfo(stateDB, valSetCheckpointInterval, 1, vals)
+
+	loadedVals, err := LoadValidators(stateDB, valSetCheckpointInterval)
+	assert.NoError(t, err)
+	assert.NotZero(t, loadedVals.Size())
+}
+
+func BenchmarkLoadValidators(b *testing.B) {
+	const valSetSize = 100
+
+	config := cfg.ResetTestRoot("state_")
+	defer os.RemoveAll(config.RootDir)
+	dbType := dbm.DBBackendType(config.DBBackend)
+	stateDB := dbm.NewDB("state", dbType, config.DBDir())
+	state, err := LoadStateFromDBOrGenesisFile(stateDB, config.GenesisFile())
+	if err != nil {
+		b.Fatal(err)
+	}
+	state.Validators = genValSet(valSetSize)
+	state.NextValidators = state.Validators.CopyIncrementProposerPriority(1)
+	SaveState(stateDB, state)
+
+	for i := 10; i < 10000000000; i *= 10 { // 10, 100, 1000, ...
+		saveValidatorsInfo(stateDB, int64(i), state.LastHeightValidatorsChanged, state.NextValidators)
+
+		b.Run(fmt.Sprintf("height=%d", i), func(b *testing.B) {
+			for n := 0; n < b.N; n++ {
+				_, err := LoadValidators(stateDB, int64(i))
+				if err != nil {
+					b.Fatal(err)
+				}
+			}
+		})
+	}
+}