[backport/v0.41.x] Removed GetValidator caching to fix concurrency error (#8546)

CHANGELOG.md

@@ -61,6 +61,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 * (simapp) [\#8418](https://github.com/cosmos/cosmos-sdk/pull/8418) Add balance coin to supply when adding a new genesis account
 * (x/bank) [\#8417](https://github.com/cosmos/cosmos-sdk/pull/8417) Validate balances and coin denom metadata on genesis
+* (x/staking) [\#8546](https://github.com/cosmos/cosmos-sdk/pull/8546) Fix caching bug where concurrent calls to GetValidator could cause a node to crash
 
 ## [v0.40.1](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.40.1) - 2021-01-19
 

x/staking/keeper/keeper.go

@@ -12,8 +12,6 @@ import (
  "github.com/cosmos/cosmos-sdk/x/staking/types"
 )
 
-const aminoCacheSize = 500
-
 // Implements ValidatorSet interface
 var _ types.ValidatorSet = Keeper{}
 
@@ -28,7 +26,6 @@ type Keeper struct {
  bankKeeper types.BankKeeper
  hooks types.StakingHooks
  paramstore paramtypes.Subspace
- validatorCache map[string]cachedValidator
  validatorCacheList *list.List
 }
 
@@ -58,7 +55,6 @@ func NewKeeper(
  bankKeeper: bk,
  paramstore: ps,
  hooks: nil,
- validatorCache: make(map[string]cachedValidator, aminoCacheSize),
  validatorCacheList: list.New(),
  }
 }

x/staking/keeper/validator.go

@@ -10,22 +10,6 @@ import (
  "github.com/cosmos/cosmos-sdk/x/staking/types"
 )
 
-// Cache the amino decoding of validators, as it can be the case that repeated slashing calls
-// cause many calls to GetValidator, which were shown to throttle the state machine in our
-// simulation. Note this is quite biased though, as the simulator does more slashes than a
-// live chain should, however we require the slashing to be fast as noone pays gas for it.
-type cachedValidator struct {
- val types.Validator
- marshalled string // marshalled amino bytes for the validator object (not operator address)
-}
-
-func newCachedValidator(val types.Validator, marshalled string) cachedValidator {
- return cachedValidator{
- val: val,
- marshalled: marshalled,
- }
-}
-
 // get a single validator
 func (k Keeper) GetValidator(ctx sdk.Context, addr sdk.ValAddress) (validator types.Validator, found bool) {
  store := ctx.KVStore(k.storeKey)
@@ -35,30 +19,7 @@ func (k Keeper) GetValidator(ctx sdk.Context, addr sdk.ValAddress) (validator ty
  return validator, false
  }
 
- // If these amino encoded bytes are in the cache, return the cached validator
- strValue := string(value)
- if val, ok := k.validatorCache[strValue]; ok {
- valToReturn := val.val
- // Doesn't mutate the cache's value
- valToReturn.OperatorAddress = addr.String()
-
- return valToReturn, true
- }
-
- // amino bytes weren't found in cache, so amino unmarshal and add it to the cache
- validator = types.MustUnmarshalValidator(k.cdc, value)
- cachedVal := newCachedValidator(validator, strValue)
- k.validatorCache[strValue] = newCachedValidator(validator, strValue)
- k.validatorCacheList.PushBack(cachedVal)
-
- // if the cache is too big, pop off the last element from it
- if k.validatorCacheList.Len() > aminoCacheSize {
- valToRemove := k.validatorCacheList.Remove(k.validatorCacheList.Front()).(cachedValidator)
- delete(k.validatorCache, valToRemove.marshalled)
- }
-
  validator = types.MustUnmarshalValidator(k.cdc, value)
-
  return validator, true
 }
 

x/staking/keeper/validator_bench_test.go

@@ -0,0 +1,29 @@
+package keeper_test
+
+import "testing"
+
+func BenchmarkGetValidator(b *testing.B) {
+ // 900 is the max number we are allowed to use in order to avoid simapp.CreateTestPubKeys
+ // panic: encoding/hex: odd length hex string
+ var powersNumber = 900
+
+ var totalPower int64 = 0
+ var powers = make([]int64, powersNumber)
+ for i := range powers {
+ powers[i] = int64(i)
+ totalPower += int64(i)
+ }
+
+ app, ctx, _, valAddrs, vals := initValidators(b, totalPower, len(powers), powers)
+
+ for _, validator := range vals {
+ app.StakingKeeper.SetValidator(ctx, validator)
+ }
+
+ b.ResetTimer()
+ for n := 0; n < b.N; n++ {
+ for _, addr := range valAddrs {
+ _, _ = app.StakingKeeper.GetValidator(ctx, addr)
+ }
+ }
+}

x/staking/keeper/validator_test.go

@@ -19,13 +19,13 @@ import (
  "github.com/cosmos/cosmos-sdk/x/staking/types"
 )
 
-func newMonikerValidator(t *testing.T, operator sdk.ValAddress, pubKey cryptotypes.PubKey, moniker string) types.Validator {
+func newMonikerValidator(t testing.TB, operator sdk.ValAddress, pubKey cryptotypes.PubKey, moniker string) types.Validator {
  v, err := types.NewValidator(operator, pubKey, types.Description{Moniker: moniker})
  require.NoError(t, err)
  return v
 }
 
-func bootstrapValidatorTest(t *testing.T, power int64, numAddrs int) (*simapp.SimApp, sdk.Context, []sdk.AccAddress, []sdk.ValAddress) {
+func bootstrapValidatorTest(t testing.TB, power int64, numAddrs int) (*simapp.SimApp, sdk.Context, []sdk.AccAddress, []sdk.ValAddress) {
  _, app, ctx := createTestInput()
 
  addrDels, addrVals := generateAddresses(app, ctx, numAddrs)
@@ -43,12 +43,13 @@ func bootstrapValidatorTest(t *testing.T, power int64, numAddrs int) (*simapp.Si
  return app, ctx, addrDels, addrVals
 }
 
-func initValidators(t *testing.T, power int64, numAddrs int, powers []int64) (*simapp.SimApp, sdk.Context, []sdk.AccAddress, []sdk.ValAddress, []types.Validator) {
- app, ctx, addrs, valAddrs := bootstrapValidatorTest(t, 1000, 20)
+func initValidators(t testing.TB, power int64, numAddrs int, powers []int64) (*simapp.SimApp, sdk.Context, []sdk.AccAddress, []sdk.ValAddress, []types.Validator) {
+ app, ctx, addrs, valAddrs := bootstrapValidatorTest(t, power, numAddrs)
+ pks := simapp.CreateTestPubKeys(numAddrs)
 
  vs := make([]types.Validator, len(powers))
  for i, power := range powers {
- vs[i] = teststaking.NewValidator(t, sdk.ValAddress(addrs[i]), PKs[i])
+ vs[i] = teststaking.NewValidator(t, sdk.ValAddress(addrs[i]), pks[i])
  tokens := sdk.TokensFromConsensusPower(power)
  vs[i], _ = vs[i].AddTokensFromDel(tokens)
  }

x/staking/teststaking/validator.go

@@ -11,7 +11,7 @@ import (
 )
 
 // NewValidator is a testing helper method to create validators in tests
-func NewValidator(t *testing.T, operator sdk.ValAddress, pubKey cryptotypes.PubKey) types.Validator {
+func NewValidator(t testing.TB, operator sdk.ValAddress, pubKey cryptotypes.PubKey) types.Validator {
  v, err := types.NewValidator(operator, pubKey, types.Description{})
  require.NoError(t, err)
  return v