From 8a8e7a04efd07983dbf0e165aef9b1009f3e88ec Mon Sep 17 00:00:00 2001 From: Simon Noetzlin Date: Tue, 11 Jun 2024 11:14:06 +0200 Subject: [PATCH] test: Add integration test reproducing the LastValidators exceeding MaxValidators bug (#1945) * Add test reproducing the LastValidators exceeding MaxValidators * formatting * Update tests/integration/unbonding.go Co-authored-by: insumity * Update tests/integration/unbonding.go Co-authored-by: Philip Offtermatt <57488781+p-offtermatt@users.noreply.github.com> * document --------- Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: insumity Co-authored-by: Philip Offtermatt <57488781+p-offtermatt@users.noreply.github.com> --- tests/integration/unbonding.go | 49 ++++++++++++++++++++++++++++++ testutil/integration/debug_test.go | 4 +++ testutil/integration/interfaces.go | 4 +++ testutil/keeper/mocks.go | 28 +++++++++++++++++ 4 files changed, 85 insertions(+) diff --git a/tests/integration/unbonding.go b/tests/integration/unbonding.go index 7f87516444..a8398843aa 100644 --- a/tests/integration/unbonding.go +++ b/tests/integration/unbonding.go @@ -461,3 +461,52 @@ func (s *CCVTestSuite) TestRedelegationProviderFirst() { // Check that ccv unbonding op has been deleted checkCCVUnbondingOp(s, s.providerCtx(), s.consumerChain.ChainID, valsetUpdateID, false) } + +// This test reproduces a fixed bug when an inactive validator enters back into the active set. +// It used to cause a panic in the provider module hook called by AfterUnbondingInitiated +// during the staking module EndBlock. +func (s *CCVTestSuite) TestTooManyLastValidators() { + sk := s.providerApp.GetTestStakingKeeper() + + // get current staking params + p := sk.GetParams(s.providerCtx()) + + // get validators, which are all active at the moment + vals := sk.GetAllValidators(s.providerCtx()) + s.Require().Equal(len(vals), len(sk.GetLastValidators(s.providerCtx()))) + + // jail a validator + val := vals[0] + consAddr, err := val.GetConsAddr() + s.Require().NoError(err) + sk.Jail(s.providerCtx(), consAddr) + + // save the current number of bonded vals + lastVals := sk.GetLastValidators(s.providerCtx()) + + // pass one block to apply the validator set changes + // (calls ApplyAndReturnValidatorSetUpdates in the the staking module EndBlock) + s.providerChain.NextBlock() + + // verify that the number of bonded validators is decreased by one + s.Require().Equal(len(lastVals)-1, len(sk.GetLastValidators(s.providerCtx()))) + + // update maximum validator to equal the number of bonded validators + p.MaxValidators = uint32(len(sk.GetLastValidators(s.providerCtx()))) + sk.SetParams(s.providerCtx(), p) + + // pass one block to apply validator set changes + s.providerChain.NextBlock() + + // unjail validator + // Note that since validators are sorted in descending order, the unjailed validator + // enters the active set again since it's ranked first by voting power. + sk.Unjail(s.providerCtx(), consAddr) + + // pass another block to update the validator set + // which causes a panic due to a GetLastValidator call in + // ApplyAndReturnValidatorSetUpdates where the staking module has a inconsistent state + s.Require().NotPanics(s.providerChain.NextBlock) + s.Require().NotPanics(func() { sk.ApplyAndReturnValidatorSetUpdates(s.providerCtx()) }) + s.Require().NotPanics(func() { sk.GetLastValidators(s.providerCtx()) }) +} diff --git a/testutil/integration/debug_test.go b/testutil/integration/debug_test.go index 2481e865ab..d97c41f7d0 100644 --- a/testutil/integration/debug_test.go +++ b/testutil/integration/debug_test.go @@ -303,3 +303,7 @@ func TestAllocateTokensToValidator(t *testing.T) { func TestMultiConsumerRewardsDistribution(t *testing.T) { runCCVTestByName(t, "TestMultiConsumerRewardsDistribution") } + +func TestTooManyLastValidators(t *testing.T) { + runCCVTestByName(t, "TestTooManyLastValidators") +} diff --git a/testutil/integration/interfaces.go b/testutil/integration/interfaces.go index fe3382b524..5324449c9f 100644 --- a/testutil/integration/interfaces.go +++ b/testutil/integration/interfaces.go @@ -3,6 +3,7 @@ package integration import ( "time" + stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" ibctesting "github.com/cosmos/ibc-go/v7/testing" "cosmossdk.io/math" @@ -106,6 +107,9 @@ type TestStakingKeeper interface { ) (ubd types.UnbondingDelegation, found bool) GetAllValidators(ctx sdk.Context) (validators []types.Validator) GetValidatorSet() types.ValidatorSet + GetParams(ctx sdk.Context) stakingtypes.Params + SetParams(ctx sdk.Context, p stakingtypes.Params) error + ApplyAndReturnValidatorSetUpdates(ctx sdk.Context) (updates []abci.ValidatorUpdate, err error) } type TestBankKeeper interface { diff --git a/testutil/keeper/mocks.go b/testutil/keeper/mocks.go index ffa76ad40c..0ad09a0be6 100644 --- a/testutil/keeper/mocks.go +++ b/testutil/keeper/mocks.go @@ -119,6 +119,20 @@ func (mr *MockStakingKeeperMockRecorder) GetLastValidators(ctx interface{}) *gom return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetLastValidators", reflect.TypeOf((*MockStakingKeeper)(nil).GetLastValidators), ctx) } +// GetParams mocks base method. +func (m *MockStakingKeeper) GetParams(ctx types0.Context) types5.Params { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetParams", ctx) + ret0, _ := ret[0].(types5.Params) + return ret0 +} + +// GetParams indicates an expected call of GetParams. +func (mr *MockStakingKeeperMockRecorder) GetParams(ctx interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetParams", reflect.TypeOf((*MockStakingKeeper)(nil).GetParams), ctx) +} + // GetRedelegationByUnbondingID mocks base method. func (m *MockStakingKeeper) GetRedelegationByUnbondingID(ctx types0.Context, id uint64) (types5.Redelegation, bool) { m.ctrl.T.Helper() @@ -357,6 +371,20 @@ func (mr *MockStakingKeeperMockRecorder) PutUnbondingOnHold(ctx, id interface{}) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "PutUnbondingOnHold", reflect.TypeOf((*MockStakingKeeper)(nil).PutUnbondingOnHold), ctx, id) } +// SetParams mocks base method. +func (m *MockStakingKeeper) SetParams(ctx types0.Context, p types5.Params) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "SetParams", ctx, p) + ret0, _ := ret[0].(error) + return ret0 +} + +// SetParams indicates an expected call of SetParams. +func (mr *MockStakingKeeperMockRecorder) SetParams(ctx, p interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetParams", reflect.TypeOf((*MockStakingKeeper)(nil).SetParams), ctx, p) +} + // Slash mocks base method. func (m *MockStakingKeeper) Slash(arg0 types0.Context, arg1 types0.ConsAddress, arg2, arg3 int64, arg4 types0.Dec) math.Int { m.ctrl.T.Helper()