test: Add integration test reproducing the LastValidators exceeding M…

…axValidators bug (#1945)

* Add test reproducing the LastValidators exceeding MaxValidators

* formatting

* Update tests/integration/unbonding.go

Co-authored-by: insumity <[email protected]>

* Update tests/integration/unbonding.go

Co-authored-by: Philip Offtermatt <[email protected]>

* document

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: insumity <[email protected]>
Co-authored-by: Philip Offtermatt <[email protected]>

tests/integration/unbonding.go

@@ -4,7 +4,8 @@ import (
 	"time"
 
 	"cosmossdk.io/math"
-
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
 	providerkeeper "github.com/cosmos/interchain-security/v5/x/ccv/provider/keeper"
 	ccv "github.com/cosmos/interchain-security/v5/x/ccv/types"
 )
@@ -467,3 +468,61 @@ 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()
+
+	getLastValsFn := func(ctx sdk.Context) []stakingtypes.Validator {
+		lastVals, err := sk.GetLastValidators(s.providerCtx())
+		s.Require().NoError(err)
+		return lastVals
+	}
+
+	// get current staking params
+	p, err := sk.GetParams(s.providerCtx())
+	s.Require().NoError(err)
+
+	// get validators, which are all active at the moment
+	vals, err := sk.GetAllValidators(s.providerCtx())
+	s.Require().NoError(err)
+
+	s.Require().Equal(len(vals), len(getLastValsFn(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 := getLastValsFn(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(getLastValsFn(s.providerCtx())))
+
+	// update maximum validator to equal the number of bonded validators
+	p.MaxValidators = uint32(len(getLastValsFn(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() { getLastValsFn(s.providerCtx()) })
+}

testutil/integration/debug_test.go

@@ -324,3 +324,7 @@ func TestAllocateTokensToValidator(t *testing.T) {
 func TestMultiConsumerRewardsDistribution(t *testing.T) {
 	runCCVTestByName(t, "TestMultiConsumerRewardsDistribution")
 }
+
+func TestTooManyLastValidators(t *testing.T) {
+	runCCVTestByName(t, "TestTooManyLastValidators")
+}

testutil/integration/interfaces.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"time"
 
+	abci "github.com/cometbft/cometbft/abci/types"
 	ibctesting "github.com/cosmos/ibc-go/v8/testing"
 
 	"cosmossdk.io/core/comet"
@@ -108,6 +109,9 @@ type TestStakingKeeper interface {
 	GetUnbondingDelegation(ctx context.Context, delAddr sdk.AccAddress, valAddr sdk.ValAddress) (ubd stakingtypes.UnbondingDelegation, err error)
 	GetAllValidators(ctx context.Context) (validators []stakingtypes.Validator, err error)
 	GetValidatorSet() stakingtypes.ValidatorSet
+	GetParams(ctx context.Context) (stakingtypes.Params, error)
+	SetParams(ctx context.Context, p stakingtypes.Params) error
+	ApplyAndReturnValidatorSetUpdates(ctx context.Context) (updates []abci.ValidatorUpdate, err error)
 }
 
 type TestBankKeeper interface {