Skip to content

Commit

Permalink
feat: 🔧 prevent jailing high stake validators (#963)
Browse files Browse the repository at this point in the history
  • Loading branch information
byte-bandit authored Aug 31, 2023
1 parent 0a6de50 commit 43fa07f
Show file tree
Hide file tree
Showing 8 changed files with 65 additions and 11 deletions.
1 change: 1 addition & 0 deletions app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -528,6 +528,7 @@ func New(
app.GetSubspace(valsetmoduletypes.ModuleName),
app.StakingKeeper,
minimumPigeonVersion,
sdk.DefaultPowerReduction,
)

consensusRegistry := consensusmodulekeeper.NewRegistry()
Expand Down
1 change: 1 addition & 0 deletions testutil/keeper/valset.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ func ValsetKeeper(t testing.TB) (*keeper.Keeper, sdk.Context) {
paramsSubspace,
nil,
"v1.4.0",
sdk.DefaultPowerReduction,
)

k.EvmKeeper = mocks.NewEvmKeeper(t)
Expand Down
5 changes: 3 additions & 2 deletions x/gravity/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"strconv"
"strings"

sdkmath "cosmossdk.io/math"
tmbytes "github.com/cometbft/cometbft/libs/bytes"
"github.com/cometbft/cometbft/libs/log"
"github.com/cosmos/cosmos-sdk/codec"
Expand All @@ -33,7 +34,7 @@ type Keeper struct {
bankKeeper types.BankKeeper
SlashingKeeper types.SlashingKeeper
DistributionKeeper types.DistributionKeeper
PowerReduction sdk.Int
PowerReduction sdkmath.Int
hooks types.GravityHooks
ReceiverModuleAccounts map[string]string
SenderModuleAccounts map[string]string
Expand All @@ -49,7 +50,7 @@ func NewKeeper(
bankKeeper types.BankKeeper,
slashingKeeper types.SlashingKeeper,
distributionKeeper types.DistributionKeeper,
powerReduction sdk.Int,
powerReduction sdkmath.Int,
receiverModuleAccounts map[string]string,
senderModuleAccounts map[string]string,
) Keeper {
Expand Down
23 changes: 22 additions & 1 deletion x/paloma/keeper/keeper_integration_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package keeper_test

import (
"fmt"
"math/big"
"testing"

Expand Down Expand Up @@ -39,7 +40,9 @@ var _ = Describe("jailing validators with missing external chain infos", func()
})

BeforeEach(func() {
vals = testutil.GenValidators(3, 100)
// Generate enough validators to ensure jailing is not prevented due to high
// amount of stake in network.
vals = testutil.GenValidators(10, 100)

for _, val := range vals {
a.StakingKeeper.SetValidator(ctx, val)
Expand All @@ -52,6 +55,7 @@ var _ = Describe("jailing validators with missing external chain infos", func()
// val[0] has everything
// val[1] has only one chain
// val[2] doesn't have anything
// All other vals have everything
err := a.ValsetKeeper.AddExternalChainInfo(ctx, vals[0].GetOperator(), []*valsettypes.ExternalChainInfo{
{
ChainType: "evm",
Expand All @@ -76,6 +80,23 @@ var _ = Describe("jailing validators with missing external chain infos", func()
},
})
Expect(err).To(BeNil())
for i, v := range vals[3:] {
err := a.ValsetKeeper.AddExternalChainInfo(ctx, v.GetOperator(), []*valsettypes.ExternalChainInfo{
{
ChainType: "evm",
ChainReferenceID: "c1",
Address: fmt.Sprintf("abc%d", i+3),
Pubkey: []byte(fmt.Sprintf("abc%d", i+3)),
},
{
ChainType: "evm",
ChainReferenceID: "c2",
Address: fmt.Sprintf("abcd%d", i+3),
Pubkey: []byte(fmt.Sprintf("abcd%d", i+3)),
},
})
Expect(err).To(BeNil())
}
})

BeforeEach(func() {
Expand Down
2 changes: 2 additions & 0 deletions x/valset/keeper/keep_alive_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@ func TestJailingInactiveValidators(t *testing.T) {
valBuild := func(id int, toBeJailed bool) (*mocks.StakingValidatorI, sdk.ValAddress) {
val := sdk.ValAddress(fmt.Sprintf("validator_%d", id))
vali := mocks.NewStakingValidatorI(t)
stake := sdk.DefaultPowerReduction
ms.StakingKeeper.On("Validator", mock.Anything, val).Return(vali)
vali.On("IsJailed").Return(false)
vali.On("GetConsensusPower", k.powerReduction).Return(stake.Int64())
vali.On("IsBonded").Return(true)
vali.On("GetOperator").Return(val)
vali.On("GetStatus").Return(stakingtypes.Bonded)
Expand Down
29 changes: 21 additions & 8 deletions x/valset/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,21 @@ import (
const (
snapshotIDKey = "snapshot-id"
maxNumOfAllowedExternalAccounts = 100
cJailingNetworkShareProtection = 0.25
)

type Keeper struct {
cdc codec.BinaryCodec
storeKey storetypes.StoreKey
memKey storetypes.StoreKey
paramstore paramtypes.Subspace
staking types.StakingKeeper
ider keeperutil.IDGenerator
EvmKeeper types.EvmKeeper

EvmKeeper types.EvmKeeper
SnapshotListeners []types.OnSnapshotBuiltListener

cdc codec.BinaryCodec
ider keeperutil.IDGenerator
memKey storetypes.StoreKey
minimumPigeonVersion string
paramstore paramtypes.Subspace
powerReduction sdkmath.Int
staking types.StakingKeeper
storeKey storetypes.StoreKey
}

func NewKeeper(
Expand All @@ -47,6 +48,7 @@ func NewKeeper(
ps paramtypes.Subspace,
staking types.StakingKeeper,
minimumPigeonVersion string,
powerReduction sdkmath.Int,
) *Keeper {
// set KeyTable if it has not already been set
if !ps.HasKeyTable() {
Expand All @@ -60,6 +62,7 @@ func NewKeeper(
paramstore: ps,
staking: staking,
minimumPigeonVersion: minimumPigeonVersion,
powerReduction: powerReduction,
}
k.ider = keeperutil.NewIDGenerator(keeperutil.StoreGetterFn(func(ctx sdk.Context) sdk.KVStore {
return prefix.NewStore(ctx.KVStore(k.storeKey), []byte("IDs"))
Expand Down Expand Up @@ -525,16 +528,26 @@ func (k Keeper) Jail(ctx sdk.Context, valAddr sdk.ValAddress, reason string) err
if val.IsJailed() {
return ErrValidatorAlreadyJailed.Format(valAddr.String())
}

consensusPower := val.GetConsensusPower(k.powerReduction)
totalConsensusPower := int64(0)
count := 0
k.staking.IterateValidators(ctx, func(_ int64, val stakingtypes.ValidatorI) bool {
if val.IsBonded() && !val.IsJailed() {
totalConsensusPower += val.GetConsensusPower(k.powerReduction)
count++
}
return false
})

if count == 1 {
return ErrCannotJailValidator.Format(valAddr).WrapS("number of active validators would be zero then")
}

if float64(consensusPower)/float64(totalConsensusPower) > cJailingNetworkShareProtection {
return ErrCannotJailValidator.Format(valAddr).WrapS("validator stake holds over %v percent of entire network", cJailingNetworkShareProtection)
}

cons, err := val.GetConsAddr()
if err != nil {
return err
Expand Down
14 changes: 14 additions & 0 deletions x/valset/keeper/keeper_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,5 +95,19 @@ var _ = Describe("jaling validators", func() {
Expect(err).To(MatchError(keeper.ErrValidatorAlreadyJailed))
})
})

Context("jailing validator with too much network stake", func() {
BeforeEach(func() {
By("change validator stake")
validators := testutil.GenValidators(1, 100)
a.StakingKeeper.SetValidator(ctx, validators[0])
a.StakingKeeper.SetValidatorByConsAddr(ctx, validators[0])
val = validators[0].GetOperator()
})
It("returns an error", func() {
err := a.ValsetKeeper.Jail(ctx, val, "i am bored")
Expect(err).To(MatchError(keeper.ErrCannotJailValidator))
})
})
})
})
1 change: 1 addition & 0 deletions x/valset/keeper/setup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ func newValsetKeeper(t testing.TB) (*Keeper, mockedServices, sdk.Context) {
paramsSubspace,
ms.StakingKeeper,
"v1.4.0",
sdk.DefaultPowerReduction,
)

k.EvmKeeper = ms.EvmKeeper
Expand Down

0 comments on commit 43fa07f

Please sign in to comment.