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

@@ -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()) })
+}

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")
+}

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 {