Merge PR #3033: Fix negative stake & invariance bug

* Fix negative stake & invariance bug

* Merge PR #3037: Updates to negative stake fix

* Update invariant; fix lint

* Fix linter

* Add comment & TODO

cmd/gaia/app/invariants.go

@@ -18,7 +18,7 @@ func (app *GaiaApp) runtimeInvariants() []simulation.Invariant {
 		distrsim.ValAccumInvariants(app.distrKeeper, app.stakeKeeper),
 		stakesim.SupplyInvariants(app.bankKeeper, app.stakeKeeper,
 			app.feeCollectionKeeper, app.distrKeeper, app.accountKeeper),
-		stakesim.PositivePowerInvariant(app.stakeKeeper),
+		stakesim.NonNegativePowerInvariant(app.stakeKeeper),
 	}
 }
 

cmd/gaia/cmd/gaiareplay/main.go

@@ -127,7 +127,9 @@ func run(rootDir string) {
 	if err != nil {
 		panic(err)
 	}
-	defer proxyApp.Stop()
+	defer func() {
+		_ = proxyApp.Stop()
+	}()
 
 	var state tmsm.State = tmsm.LoadState(tmDB)
 	if state.LastBlockHeight == 0 {

types/decimal.go

@@ -272,6 +272,10 @@ func (d Dec) Format(s fmt.State, verb rune) {
 }
 
 func (d Dec) String() string {
+	isNeg := d.IsNegative()
+	if d.IsNegative() {
+		d = d.Neg()
+	}
 	bz, err := d.Int.MarshalText()
 	if err != nil {
 		return ""
@@ -298,6 +302,9 @@ func (d Dec) String() string {
 		bzWDec[inputSize-10] = byte('.')
 		copy(bzWDec[inputSize-9:], bz[inputSize-10:])
 	}
+	if isNeg {
+		return "-" + string(bzWDec)
+	}
 	return string(bzWDec)
 }
 

x/stake/genesis.go

@@ -38,7 +38,7 @@ func InitGenesis(ctx sdk.Context, keeper Keeper, data types.GenesisState) (res [
 
 		// Manually set indices for the first time
 		keeper.SetValidatorByConsAddr(ctx, validator)
-		keeper.SetValidatorByPowerIndex(ctx, validator, data.Pool)
+		keeper.SetValidatorByPowerIndex(ctx, validator)
 		keeper.OnValidatorCreated(ctx, validator.OperatorAddr)
 
 		// Set timeslice if necessary

x/stake/handler_test.go

@@ -55,8 +55,7 @@ func TestValidatorByPowerIndex(t *testing.T) {
 	// verify that the by power index exists
 	validator, found := keeper.GetValidator(ctx, validatorAddr)
 	require.True(t, found)
-	pool := keeper.GetPool(ctx)
-	power := keep.GetValidatorsByPowerIndexKey(validator, pool)
+	power := keep.GetValidatorsByPowerIndexKey(validator)
 	require.True(t, keep.ValidatorByPowerIndexExists(ctx, keeper, power))
 
 	// create a second validator keep it bonded
@@ -85,12 +84,11 @@ func TestValidatorByPowerIndex(t *testing.T) {
 	// but the new power record should have been created
 	validator, found = keeper.GetValidator(ctx, validatorAddr)
 	require.True(t, found)
-	pool = keeper.GetPool(ctx)
-	power2 := GetValidatorsByPowerIndexKey(validator, pool)
+	power2 := GetValidatorsByPowerIndexKey(validator)
 	require.True(t, keep.ValidatorByPowerIndexExists(ctx, keeper, power2))
 
 	// now the new record power index should be the same as the original record
-	power3 := GetValidatorsByPowerIndexKey(validator, pool)
+	power3 := GetValidatorsByPowerIndexKey(validator)
 	require.Equal(t, power2, power3)
 
 	// unbond self-delegation

x/stake/keeper/delegation_test.go

@@ -240,7 +240,7 @@ func TestUndelegateSelfDelegation(t *testing.T) {
 	keeper.SetDelegation(ctx, selfDelegation)
 
 	// create a second delegation to this validator
-	keeper.DeleteValidatorByPowerIndex(ctx, validator, pool)
+	keeper.DeleteValidatorByPowerIndex(ctx, validator)
 	validator, pool, issuedShares = validator.AddTokensFromDel(pool, sdk.NewInt(10))
 	require.Equal(t, int64(10), issuedShares.RoundInt64())
 	keeper.SetPool(ctx, pool)
@@ -288,7 +288,7 @@ func TestUndelegateFromUnbondingValidator(t *testing.T) {
 	keeper.SetDelegation(ctx, selfDelegation)
 
 	// create a second delegation to this validator
-	keeper.DeleteValidatorByPowerIndex(ctx, validator, pool)
+	keeper.DeleteValidatorByPowerIndex(ctx, validator)
 	validator, pool, issuedShares = validator.AddTokensFromDel(pool, sdk.NewInt(10))
 	require.Equal(t, int64(10), issuedShares.RoundInt64())
 	keeper.SetPool(ctx, pool)
@@ -365,7 +365,7 @@ func TestUndelegateFromUnbondedValidator(t *testing.T) {
 	keeper.SetDelegation(ctx, selfDelegation)
 
 	// create a second delegation to this validator
-	keeper.DeleteValidatorByPowerIndex(ctx, validator, pool)
+	keeper.DeleteValidatorByPowerIndex(ctx, validator)
 	validator, pool, issuedShares = validator.AddTokensFromDel(pool, sdk.NewInt(10))
 	require.Equal(t, int64(10), issuedShares.RoundInt64())
 	keeper.SetPool(ctx, pool)
@@ -444,7 +444,7 @@ func TestUnbondingAllDelegationFromValidator(t *testing.T) {
 	keeper.SetDelegation(ctx, selfDelegation)
 
 	// create a second delegation to this validator
-	keeper.DeleteValidatorByPowerIndex(ctx, validator, pool)
+	keeper.DeleteValidatorByPowerIndex(ctx, validator)
 	validator, pool, issuedShares = validator.AddTokensFromDel(pool, sdk.NewInt(10))
 	require.Equal(t, int64(10), issuedShares.RoundInt64())
 	keeper.SetPool(ctx, pool)
@@ -689,7 +689,7 @@ func TestRedelegateFromUnbondingValidator(t *testing.T) {
 	keeper.SetDelegation(ctx, selfDelegation)
 
 	// create a second delegation to this validator
-	keeper.DeleteValidatorByPowerIndex(ctx, validator, pool)
+	keeper.DeleteValidatorByPowerIndex(ctx, validator)
 	validator, pool, issuedShares = validator.AddTokensFromDel(pool, sdk.NewInt(10))
 	require.Equal(t, int64(10), issuedShares.RoundInt64())
 	keeper.SetPool(ctx, pool)
@@ -773,7 +773,7 @@ func TestRedelegateFromUnbondedValidator(t *testing.T) {
 	keeper.SetDelegation(ctx, selfDelegation)
 
 	// create a second delegation to this validator
-	keeper.DeleteValidatorByPowerIndex(ctx, validator, pool)
+	keeper.DeleteValidatorByPowerIndex(ctx, validator)
 	validator, pool, issuedShares = validator.AddTokensFromDel(pool, sdk.NewInt(10))
 	validator.BondIntraTxCounter = 1
 	require.Equal(t, int64(10), issuedShares.RoundInt64())

x/stake/keeper/key.go

@@ -61,7 +61,7 @@ func AddressFromLastValidatorPowerKey(key []byte) []byte {
 // Power index is the key used in the power-store, and represents the relative
 // power ranking of the validator.
 // VALUE: validator operator address ([]byte)
-func GetValidatorsByPowerIndexKey(validator types.Validator, pool types.Pool) []byte {
+func GetValidatorsByPowerIndexKey(validator types.Validator) []byte {
 	// NOTE the address doesn't need to be stored because counter bytes must always be different
 	return getValidatorPowerRank(validator)
 }

x/stake/keeper/test_common.go

@@ -202,9 +202,8 @@ func ValidatorByPowerIndexExists(ctx sdk.Context, keeper Keeper, power []byte) b
 
 // update validator for testing
 func TestingUpdateValidator(keeper Keeper, ctx sdk.Context, validator types.Validator) types.Validator {
-	pool := keeper.GetPool(ctx)
 	keeper.SetValidator(ctx, validator)
-	keeper.SetValidatorByPowerIndex(ctx, validator, pool)
+	keeper.SetValidatorByPowerIndex(ctx, validator)
 	keeper.ApplyAndReturnValidatorSetUpdates(ctx)
 	validator, found := keeper.GetValidator(ctx, validator.OperatorAddr)
 	if !found {

x/stake/keeper/val_state_change.go

@@ -160,10 +160,9 @@ func (k Keeper) jailValidator(ctx sdk.Context, validator types.Validator) {
 		panic(fmt.Sprintf("cannot jail already jailed validator, validator: %v\n", validator))
 	}
 
-	pool := k.GetPool(ctx)
 	validator.Jailed = true
 	k.SetValidator(ctx, validator)
-	k.DeleteValidatorByPowerIndex(ctx, validator, pool)
+	k.DeleteValidatorByPowerIndex(ctx, validator)
 }
 
 // remove a validator from jail
@@ -172,28 +171,26 @@ func (k Keeper) unjailValidator(ctx sdk.Context, validator types.Validator) {
 		panic(fmt.Sprintf("cannot unjail already unjailed validator, validator: %v\n", validator))
 	}
 
-	pool := k.GetPool(ctx)
 	validator.Jailed = false
 	k.SetValidator(ctx, validator)
-	k.SetValidatorByPowerIndex(ctx, validator, pool)
+	k.SetValidatorByPowerIndex(ctx, validator)
 }
 
 // perform all the store operations for when a validator status becomes bonded
 func (k Keeper) bondValidator(ctx sdk.Context, validator types.Validator) types.Validator {
 
-	pool := k.GetPool(ctx)
-
-	k.DeleteValidatorByPowerIndex(ctx, validator, pool)
+	k.DeleteValidatorByPowerIndex(ctx, validator)
 
 	validator.BondHeight = ctx.BlockHeight()
 
 	// set the status
+	pool := k.GetPool(ctx)
 	validator, pool = validator.UpdateStatus(pool, sdk.Bonded)
 	k.SetPool(ctx, pool)
 
 	// save the now bonded validator record to the two referenced stores
 	k.SetValidator(ctx, validator)
-	k.SetValidatorByPowerIndex(ctx, validator, pool)
+	k.SetValidatorByPowerIndex(ctx, validator)
 
 	// delete from queue if present
 	k.DeleteValidatorQueue(ctx, validator)
@@ -209,17 +206,17 @@ func (k Keeper) bondValidator(ctx sdk.Context, validator types.Validator) types.
 // perform all the store operations for when a validator status begins unbonding
 func (k Keeper) beginUnbondingValidator(ctx sdk.Context, validator types.Validator) types.Validator {
 
-	pool := k.GetPool(ctx)
 	params := k.GetParams(ctx)
 
-	k.DeleteValidatorByPowerIndex(ctx, validator, pool)
+	k.DeleteValidatorByPowerIndex(ctx, validator)
 
 	// sanity check
 	if validator.Status != sdk.Bonded {
 		panic(fmt.Sprintf("should not already be unbonded or unbonding, validator: %v\n", validator))
 	}
 
 	// set the status
+	pool := k.GetPool(ctx)
 	validator, pool = validator.UpdateStatus(pool, sdk.Unbonding)
 	k.SetPool(ctx, pool)
 
@@ -228,7 +225,7 @@ func (k Keeper) beginUnbondingValidator(ctx sdk.Context, validator types.Validat
 
 	// save the now unbonded validator record and power index
 	k.SetValidator(ctx, validator)
-	k.SetValidatorByPowerIndex(ctx, validator, pool)
+	k.SetValidatorByPowerIndex(ctx, validator)
 
 	// Adds to unbonding validator queue
 	k.InsertValidatorQueue(ctx, validator)

x/stake/keeper/validator.go

@@ -99,26 +99,25 @@ func (k Keeper) SetValidatorByConsAddr(ctx sdk.Context, validator types.Validato
 }
 
 // validator index
-func (k Keeper) SetValidatorByPowerIndex(ctx sdk.Context, validator types.Validator, pool types.Pool) {
+func (k Keeper) SetValidatorByPowerIndex(ctx sdk.Context, validator types.Validator) {
 	// jailed validators are not kept in the power index
 	if validator.Jailed {
 		return
 	}
 	store := ctx.KVStore(k.storeKey)
-	store.Set(GetValidatorsByPowerIndexKey(validator, pool), validator.OperatorAddr)
+	store.Set(GetValidatorsByPowerIndexKey(validator), validator.OperatorAddr)
 }
 
 // validator index
-func (k Keeper) DeleteValidatorByPowerIndex(ctx sdk.Context, validator types.Validator, pool types.Pool) {
+func (k Keeper) DeleteValidatorByPowerIndex(ctx sdk.Context, validator types.Validator) {
 	store := ctx.KVStore(k.storeKey)
-	store.Delete(GetValidatorsByPowerIndexKey(validator, pool))
+	store.Delete(GetValidatorsByPowerIndexKey(validator))
 }
 
 // validator index
 func (k Keeper) SetNewValidatorByPowerIndex(ctx sdk.Context, validator types.Validator) {
 	store := ctx.KVStore(k.storeKey)
-	pool := k.GetPool(ctx)
-	store.Set(GetValidatorsByPowerIndexKey(validator, pool), validator.OperatorAddr)
+	store.Set(GetValidatorsByPowerIndexKey(validator), validator.OperatorAddr)
 }
 
 //___________________________________________________________________________
@@ -127,8 +126,8 @@ func (k Keeper) SetNewValidatorByPowerIndex(ctx sdk.Context, validator types.Val
 func (k Keeper) AddValidatorTokensAndShares(ctx sdk.Context, validator types.Validator,
 	tokensToAdd sdk.Int) (valOut types.Validator, addedShares sdk.Dec) {
 
+	k.DeleteValidatorByPowerIndex(ctx, validator)
 	pool := k.GetPool(ctx)
-	k.DeleteValidatorByPowerIndex(ctx, validator, pool)
 	validator, pool, addedShares = validator.AddTokensFromDel(pool, tokensToAdd)
 	// increment the intra-tx counter
 	// in case of a conflict, the validator which least recently changed power takes precedence
@@ -137,32 +136,32 @@ func (k Keeper) AddValidatorTokensAndShares(ctx sdk.Context, validator types.Val
 	k.SetIntraTxCounter(ctx, counter+1)
 	k.SetValidator(ctx, validator)
 	k.SetPool(ctx, pool)
-	k.SetValidatorByPowerIndex(ctx, validator, pool)
+	k.SetValidatorByPowerIndex(ctx, validator)
 	return validator, addedShares
 }
 
 // Update the tokens of an existing validator, update the validators power index key
 func (k Keeper) RemoveValidatorTokensAndShares(ctx sdk.Context, validator types.Validator,
 	sharesToRemove sdk.Dec) (valOut types.Validator, removedTokens sdk.Dec) {
 
+	k.DeleteValidatorByPowerIndex(ctx, validator)
 	pool := k.GetPool(ctx)
-	k.DeleteValidatorByPowerIndex(ctx, validator, pool)
 	validator, pool, removedTokens = validator.RemoveDelShares(pool, sharesToRemove)
 	k.SetValidator(ctx, validator)
 	k.SetPool(ctx, pool)
-	k.SetValidatorByPowerIndex(ctx, validator, pool)
+	k.SetValidatorByPowerIndex(ctx, validator)
 	return validator, removedTokens
 }
 
 // Update the tokens of an existing validator, update the validators power index key
 func (k Keeper) RemoveValidatorTokens(ctx sdk.Context, validator types.Validator, tokensToRemove sdk.Dec) types.Validator {
 
+	k.DeleteValidatorByPowerIndex(ctx, validator)
 	pool := k.GetPool(ctx)
-	k.DeleteValidatorByPowerIndex(ctx, validator, pool)
 	validator, pool = validator.RemoveTokens(pool, tokensToRemove)
 	k.SetValidator(ctx, validator)
 	k.SetPool(ctx, pool)
-	k.SetValidatorByPowerIndex(ctx, validator, pool)
+	k.SetValidatorByPowerIndex(ctx, validator)
 	return validator
 }
 
@@ -195,12 +194,18 @@ func (k Keeper) RemoveValidator(ctx sdk.Context, address sdk.ValAddress) {
 		panic("Cannot call RemoveValidator on bonded or unbonding validators")
 	}
 
+	// if any tokens remain, remove from pool (burning the tokens).
+	// this happens if shares are zero but tokens are not.
+	// TODO: Remove once https://github.com/cosmos/cosmos-sdk/pull/2958 is merged
+	pool := k.GetPool(ctx)
+	pool.LooseTokens = pool.LooseTokens.Sub(validator.Tokens)
+	k.SetPool(ctx, pool)
+
 	// delete the old validator record
 	store := ctx.KVStore(k.storeKey)
-	pool := k.GetPool(ctx)
 	store.Delete(GetValidatorKey(address))
 	store.Delete(GetValidatorByConsAddrKey(sdk.ConsAddress(validator.ConsPubKey.Address())))
-	store.Delete(GetValidatorsByPowerIndexKey(validator, pool))
+	store.Delete(GetValidatorsByPowerIndexKey(validator))
 
 	// call hook if present
 	if k.hooks != nil {