Clear the global session cache when a node is unjailed

codec/codec.go

@@ -50,6 +50,7 @@ const (
 	BlockSizeModifyKey           = "BLOCK"
 	RSCALKey                     = "RSCAL"
 	VEDITKey                     = "VEDIT"
+	ClearUnjailedValSessionKey   = "CRVAL"
 )
 
 func GetCodecUpgradeHeight() int64 {
@@ -281,6 +282,24 @@ func (cdc *Codec) IsOnNamedFeatureActivationHeight(height int64, key string) boo
 	return UpgradeFeatureMap[key] != 0 && height == UpgradeFeatureMap[key]
 }
 
+// IsOnNamedFeatureActivationHeightWithTolerance is used to enable certain
+// business logic within some tolerance (i.e. only a few blocks) of feature
+// activation to have more confidence in the feature's release and avoid
+// non-deterministic or hard-to-predict behaviour.
+func (cdc *Codec) IsOnNamedFeatureActivationHeightWithTolerance(
+	height int64,
+	featureKey string,
+	tolerance int64,
+) bool {
+	upgradeHeight := UpgradeFeatureMap[featureKey]
+	if upgradeHeight == 0 {
+		return false
+	}
+	minHeight := upgradeHeight - tolerance
+	maxHeight := upgradeHeight + tolerance
+	return height >= minHeight && height <= maxHeight
+}
+
 // Upgrade Utils for feature map
 
 // SliceToExistingMap merge slice to existing map

codec/codec_test.go

@@ -0,0 +1,58 @@
+package codec
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// This test should never be run in parallel because this replaces
+// the global variable `UpgradeFeatureMap`.
+func TestCodec_IsOnNamedFeatureActivationHeightWithTolerance(t *testing.T) {
+	upgradeKey := ClearUnjailedValSessionKey
+	upgradeHeight := int64(20000)
+	tolerance := int64(5)
+
+	// have to init global map / not mock friendly.
+	originalUpgradeFeatureMap := UpgradeFeatureMap
+	UpgradeFeatureMap = map[string]int64{upgradeKey: upgradeHeight}
+	t.Cleanup(func() {
+		UpgradeFeatureMap = originalUpgradeFeatureMap
+	})
+
+	codec := NewCodec(nil)
+
+	// Test zero values / out of bounds
+	assert.False(t, codec.IsOnNamedFeatureActivationHeightWithTolerance(
+		0,
+		upgradeKey,
+		tolerance,
+	))
+	assert.False(t, codec.IsOnNamedFeatureActivationHeightWithTolerance(
+		upgradeHeight-tolerance-1,
+		upgradeKey,
+		tolerance,
+	))
+	assert.False(t, codec.IsOnNamedFeatureActivationHeightWithTolerance(
+		upgradeHeight+tolerance+1,
+		upgradeKey,
+		tolerance,
+	))
+
+	// Test in bounds
+	assert.True(t, codec.IsOnNamedFeatureActivationHeightWithTolerance(
+		upgradeHeight-tolerance,
+		upgradeKey,
+		tolerance,
+	))
+	assert.True(t, codec.IsOnNamedFeatureActivationHeightWithTolerance(
+		upgradeHeight,
+		upgradeKey,
+		tolerance,
+	))
+	assert.True(t, codec.IsOnNamedFeatureActivationHeightWithTolerance(
+		upgradeHeight+tolerance,
+		upgradeKey,
+		tolerance,
+	))
+}

x/gov/module.go

@@ -10,6 +10,7 @@ import (
 	"github.com/pokt-network/pocket-core/types/module"
 	"github.com/pokt-network/pocket-core/x/gov/keeper"
 	"github.com/pokt-network/pocket-core/x/gov/types"
+	pc "github.com/pokt-network/pocket-core/x/pocketcore/types"
 	abci "github.com/tendermint/tendermint/abci/types"
 )
 
@@ -120,6 +121,18 @@ func (am AppModule) BeginBlock(ctx sdk.Ctx, req abci.RequestBeginBlock) {
 
 	ActivateAdditionalParametersACL(ctx, am)
 
+	// Around this upgrade height, we clear the global session cache to prevent
+	// any non-deterministic cache consistency issues.
+	// See #1529 for more details.
+	const clearSessionCacheTolerance = 2
+	if am.keeper.GetCodec().IsOnNamedFeatureActivationHeightWithTolerance(
+		ctx.BlockHeight(),
+		codec.ClearUnjailedValSessionKey,
+		clearSessionCacheTolerance,
+	) {
+		pc.ClearSessionCache(pc.GlobalSessionCache)
+	}
+
 	u := am.keeper.GetUpgrade(ctx)
 	if ctx.AppVersion() < u.Version && ctx.BlockHeight() >= u.UpgradeHeight() && ctx.BlockHeight() != 0 {
 		ctx.Logger().Error("MUST UPGRADE TO NEXT VERSION: ", u.Version)

x/nodes/keeper/valStateChanges.go

@@ -4,15 +4,14 @@ import (
 	"bytes"
 	"encoding/hex"
 	"fmt"
-	"github.com/tendermint/tendermint/libs/strings"
 	"time"
 
-	"github.com/pokt-network/pocket-core/crypto"
-	abci "github.com/tendermint/tendermint/abci/types"
-
 	"github.com/pokt-network/pocket-core/codec"
+	"github.com/pokt-network/pocket-core/crypto"
 	sdk "github.com/pokt-network/pocket-core/types"
 	"github.com/pokt-network/pocket-core/x/nodes/types"
+	abci "github.com/tendermint/tendermint/abci/types"
+	"github.com/tendermint/tendermint/libs/strings"
 )
 
 // UpdateTendermintValidators - Apply and return accumulated updates to the staked validator set
@@ -179,7 +178,7 @@ func (k Keeper) ValidateValidatorStaking(ctx sdk.Ctx, validator types.Validator,
 	return nil
 }
 
-//ValidateValidatorMsgSigner Check Validator Signature
+// ValidateValidatorMsgSigner Check Validator Signature
 func ValidateValidatorMsgSigner(validator types.Validator, signerAddress sdk.Address, k Keeper) (sdk.Error, bool) {
 	//check if outputAddress is defined, if not only the operator/node signature is valid
 	if validator.OutputAddress == nil {
@@ -652,6 +651,18 @@ func (k Keeper) UnjailValidator(ctx sdk.Ctx, addr sdk.Address) {
 		k.Logger(ctx).Error(fmt.Sprintf("cannot unjail already unjailed validator, validator: %v at height %d\n", validator, ctx.BlockHeight()))
 		return
 	}
+
+	// On and after this upgrade height, we start clearing the global session
+	// cache every time a node is unjailed to prevent any non-deterministic cache
+	// consistency issues.  We have been doing the same in other events such as
+	// editstake, unstake, and jail.  See #1529 for more details.
+	if k.Cdc.IsAfterNamedFeatureActivationHeight(
+		ctx.BlockHeight(),
+		codec.ClearUnjailedValSessionKey,
+	) {
+		k.PocketKeeper.ClearSessionCache()
+	}
+
 	validator.Jailed = false
 	k.SetValidator(ctx, validator)
 	k.ResetValidatorSigningInfo(ctx, addr)

x/pocketcore/types/service_test.go

@@ -2,17 +2,14 @@ package types
 
 import (
 	"encoding/hex"
-	"github.com/pokt-network/pocket-core/codec"
-	types2 "github.com/pokt-network/pocket-core/codec/types"
-	"github.com/pokt-network/pocket-core/crypto"
-	exported2 "github.com/pokt-network/pocket-core/x/apps/exported"
-	"github.com/pokt-network/pocket-core/x/auth"
-	"github.com/pokt-network/pocket-core/x/gov"
 	"reflect"
 	"testing"
 	"time"
 
+	"github.com/pokt-network/pocket-core/codec"
+	"github.com/pokt-network/pocket-core/crypto"
 	sdk "github.com/pokt-network/pocket-core/types"
+	exported2 "github.com/pokt-network/pocket-core/x/apps/exported"
 	appsType "github.com/pokt-network/pocket-core/x/apps/types"
 	"github.com/pokt-network/pocket-core/x/nodes/exported"
 	nodesTypes "github.com/pokt-network/pocket-core/x/nodes/types"
@@ -331,7 +328,7 @@ type MockPosKeeper struct {
 type MockPocketKeeper struct{}
 
 func (m MockPocketKeeper) Codec() *codec.Codec {
-	return makeTestCodec()
+	panic("implement me")
 }
 
 func (m MockPocketKeeper) SessionNodeCount(ctx sdk.Ctx) (res int64) {
@@ -396,12 +393,3 @@ func (m MockPosKeeper) BlocksPerSession(ctx sdk.Ctx) (res int64) {
 func (m MockPosKeeper) StakeDenom(ctx sdk.Ctx) (res string) {
 	panic("implement me")
 }
-
-func makeTestCodec() *codec.Codec {
-	var cdc = codec.NewCodec(types2.NewInterfaceRegistry())
-	auth.RegisterCodec(cdc)
-	gov.RegisterCodec(cdc)
-	sdk.RegisterCodec(cdc)
-	crypto.RegisterAmino(cdc.AminoCodec().Amino)
-	return cdc
-}