feat!: enable unjail on pre-ccv chains (#2396)

* feat!: enable unjail on pre-ccv chains * docs: update docstrings * feat!: enable unjail related functions * docs: update docstrings * fix: update handling (changeover completed checks) * fix comment * tests: add unjail tests * tests: improve unjail tests * tests: improve unjail tests * tests: appease linter * Update testing documentation --------- Co-authored-by: github-actions <[email protected]>
cosmos · Dec 11, 2024 · bdadedb · bdadedb
1 parent fe2ec1e
commit bdadedb
Show file tree

Hide file tree

Showing 7 changed files with 100 additions and 13 deletions.
diff --git a/scripts/test_doc/test_documentation.md b/scripts/test_doc/test_documentation.md
@@ -15,6 +15,7 @@
 |----------|-------------------|
  [TestDemocracyRewardsDistribution](../../tests/integration/democracy.go#L77) | TestDemocracyRewardsDistribution checks that rewards to democracy representatives, community pool, and provider redistribution account are done correctly.<details><summary>Details</summary>* Set up a democracy consumer chain.<br>* Create a new block.<br>* Check that rewards to democracy representatives, community pool, and provider redistribution account are distributed in the right proportions.</details> |
  [TestDemocracyMsgUpdateParams](../../tests/integration/democracy.go#L187) | TestDemocracyMsgUpdateParams checks that the consumer parameters can be updated through a governance proposal.<details><summary>Details</summary>* Set up a democracy consumer chain.<br>* Submit a proposal containing changes to the consumer module parameters.<br>* Check that the proposal is executed, and the parameters are updated.</details> |
+ [TestDemocracyValidatorUnjail](../../tests/integration/democracy.go#L243) | TestDemocracyValidatorUnjail checks that the consumer validator can be unjailed when there is a standalone staking keeper available.<details><summary>Details</summary>* Set up a democracy consumer chain.<br>* Jail a validator.<br>* Check that the validator is jailed.<br>* Unjail the validator.<br>* Check that the validator is unjailed.</details> |
 </details>
 
 # [distribution.go](../../tests/integration/distribution.go) 
@@ -127,7 +128,8 @@
 
 | Function | Short Description |
 |----------|-------------------|
- [TestUndelegationCompletion](../../tests/integration/unbonding.go#L16) | TestUndelegationCompletion tests that undelegations complete after the unbonding period elapses on the provider, regardless of the consumer's state<details><summary>Details</summary>* Set up CCV channel.<br>* Perform initial delegation of tokens followed by a partial undelegation (1/4 of the tokens).<br>* Verify that the staking unbonding operation is created as expected.<br>* Increment provider block height.<br>* Check that the unbonding operation has been completed.<br>* Verify that the token balances are correctly updated and the expected amount of tokens has been returned to the account.</details> |
+ [TestUndelegationCompletion](../../tests/integration/unbonding.go#L17) | TestUndelegationCompletion tests that undelegations complete after the unbonding period elapses on the provider, regardless of the consumer's state<details><summary>Details</summary>* Set up CCV channel.<br>* Perform initial delegation of tokens followed by a partial undelegation (1/4 of the tokens).<br>* Verify that the staking unbonding operation is created as expected.<br>* Increment provider block height.<br>* Check that the unbonding operation has been completed.<br>* Verify that the token balances are correctly updated and the expected amount of tokens has been returned to the account.</details> |
+ [TestConsumerUnjailNoOp](../../tests/integration/unbonding.go#L50) | TestConsumerUnjailNoOp check that consumerKeeper can call .Unjail() without error. This operation must only be available in case the app also implements a "standalone" staking keeper. |
 </details>
 
 # [valset_update.go](../../tests/integration/valset_update.go) 

diff --git a/tests/integration/democracy.go b/tests/integration/democracy.go
@@ -233,6 +233,52 @@ func (s *ConsumerDemocracyTestSuite) TestDemocracyMsgUpdateParams() {
 	s.Assert().Equal(votersOldBalances, getAccountsBalances(s.consumerCtx(), bankKeeper, bondDenom, votingAccounts))
 }
 
+// TestDemocracyValidatorUnjail checks that the consumer validator can be unjailed when there is a standalone staking keeper available.
+// @Long Description@
+// * Set up a democracy consumer chain.
+// * Jail a validator.
+// * Check that the validator is jailed.
+// * Unjail the validator.
+// * Check that the validator is unjailed.
+func (s *ConsumerDemocracyTestSuite) TestDemocracyValidatorUnjail() {
+	stakingKeeper := s.consumerApp.GetTestStakingKeeper()
+	consumerKeeper := s.consumerApp.GetConsumerKeeper()
+
+	validators, err := stakingKeeper.GetAllValidators(s.consumerCtx())
+	s.Require().NoError(err)
+
+	// setting up pre-conditions
+	// validator[0] is expected to be jailed
+	expectJailed := validators[0]
+	consAddr, err := expectJailed.GetConsAddr()
+	s.Require().NoError(err)
+	stakingKeeper.GetValidatorSet().Jail(s.consumerCtx(), consAddr)
+
+	s.consumerChain.NextBlock()
+
+	validators, err = stakingKeeper.GetAllValidators(s.consumerCtx())
+	s.Require().NoError(err)
+	for _, validator := range validators {
+		if validator.OperatorAddress == expectJailed.OperatorAddress {
+			s.Require().True(validator.IsJailed())
+		} else {
+			s.Require().False(validator.IsJailed())
+		}
+	}
+
+	// confirm unjail will not error and properly unjail
+	// in case of a consumer chain without standalone staking the call is a no-op
+	err = consumerKeeper.Unjail(s.consumerCtx(), consAddr)
+	s.Require().NoError(err)
+	s.consumerChain.NextBlock()
+
+	validators, err = stakingKeeper.GetAllValidators(s.consumerCtx())
+	s.Require().NoError(err)
+	for _, validator := range validators {
+		s.Require().False(validator.IsJailed())
+	}
+}
+
 func submitProposalWithDepositAndVote(govKeeper govkeeper.Keeper, ctx sdk.Context, msgs []sdk.Msg,
 	accounts []ibctesting.SenderAccount, proposer sdk.AccAddress, depositAmount sdk.Coins,
 ) error {

diff --git a/tests/integration/unbonding.go b/tests/integration/unbonding.go
@@ -2,6 +2,7 @@ package integration
 
 import (
 	"cosmossdk.io/math"
+	sdk "github.com/cosmos/cosmos-sdk/types"
 )
 
 // TestUndelegationCompletion tests that undelegations complete after
@@ -43,3 +44,13 @@ func (s *CCVTestSuite) TestUndelegationCompletion() {
 		"unexpected initial balance after unbonding; test: %s",
 	)
 }
+
+// TestConsumerUnjailNoOp check that consumerKeeper can call .Unjail() without error.
+// This operation must only be available in case the app also implements a "standalone" staking keeper.
+func (s *CCVTestSuite) TestConsumerUnjailNoOp() {
+	consumerKeeper := s.consumerApp.GetConsumerKeeper()
+
+	// this is a no-op
+	err := consumerKeeper.Unjail(s.consumerCtx(), sdk.ConsAddress([]byte{0x01, 0x02, 0x03}))
+	s.Require().NoError(err)
+}
diff --git a/testutil/integration/debug_test.go b/testutil/integration/debug_test.go
@@ -61,6 +61,10 @@ func TestDemocracyMsgUpdateParams(t *testing.T) {
 	runConsumerDemocracyTestByName(t, "TestDemocracyMsgUpdateParams")
 }
 
+func TestDemocracyUnjail(t *testing.T) {
+	runConsumerDemocracyTestByName(t, "TestDemocracyValidatorUnjail")
+}
+
 //
 // Distribution tests
 //
@@ -193,6 +197,10 @@ func TestUndelegationCompletion(t *testing.T) {
 	runCCVTestByName(t, "TestUndelegationCompletion")
 }
 
+func TestConsumerUnjailNoOp(t *testing.T) {
+	runCCVTestByName(t, "TestConsumerUnjailNoOp")
+}
+
 //
 // Val set update tests
 //

diff --git a/x/ccv/consumer/keeper/changeover.go b/x/ccv/consumer/keeper/changeover.go
@@ -9,7 +9,7 @@ import (
 // ChangeoverIsComplete returns whether the standalone to consumer changeover process is complete.
 func (k Keeper) ChangeoverIsComplete(ctx sdk.Context) bool {
 	if !k.IsPrevStandaloneChain(ctx) {
-		panic("ChangeoverIsComplete should only be called on previously standalone consumers")
+		return true
 	}
 	return ctx.BlockHeight() >= k.FirstConsumerHeight(ctx)
 }

diff --git a/x/ccv/consumer/keeper/validators.go b/x/ccv/consumer/keeper/validators.go
@@ -84,9 +84,14 @@ func (k Keeper) IterateValidators(context.Context, func(index int64, validator s
 	return nil
 }
 
-// Validator - unimplemented on CCV keeper
-func (k Keeper) Validator(ctx context.Context, addr sdk.ValAddress) (stakingtypes.ValidatorI, error) {
-	panic("unimplemented on CCV keeper")
+// Validator - unimplemented on CCV keeper but implemented on standalone keeper
+func (k Keeper) Validator(sdkCtx context.Context, addr sdk.ValAddress) (stakingtypes.ValidatorI, error) {
+	ctx := sdk.UnwrapSDKContext(sdkCtx)
+	if k.ChangeoverIsComplete(ctx) && k.standaloneStakingKeeper != nil {
+		return k.standaloneStakingKeeper.Validator(ctx, addr)
+	}
+
+	return stakingtypes.Validator{}, errors.New("unimplemented on CCV keeper")
 }
 
 // IsJailed returns the outstanding slashing flag for the given validator address
@@ -174,16 +179,24 @@ func (k Keeper) SlashWithInfractionReason(goCtx context.Context, addr sdk.ConsAd
 // the provider validator set will soon be in effect, and jailing is n/a.
 func (k Keeper) Jail(context.Context, sdk.ConsAddress) error { return nil }
 
-// Unjail - unimplemented on CCV keeper
+// Unjail is enabled for previously standalone chains and chains implementing democracy staking.
 //
-// This method should be a no-op even during a standalone to consumer changeover.
-// Once the upgrade has happened as a part of the changeover,
-// the provider validator set will soon be in effect, and jailing is n/a.
-func (k Keeper) Unjail(context.Context, sdk.ConsAddress) error { return nil }
+// This method should be a no-op for consumer chains that launched with the CCV module first.
+func (k Keeper) Unjail(sdkCtx context.Context, addr sdk.ConsAddress) error {
+	ctx := sdk.UnwrapSDKContext(sdkCtx)
+	if k.ChangeoverIsComplete(ctx) && k.standaloneStakingKeeper != nil {
+		return k.standaloneStakingKeeper.Unjail(ctx, addr)
+	}
+	return nil
+}
 
-// Delegation - unimplemented on CCV keeper
-func (k Keeper) Delegation(ctx context.Context, addr sdk.AccAddress, valAddr sdk.ValAddress) (stakingtypes.DelegationI, error) {
-	panic("unimplemented on CCV keeper")
+// Delegation - unimplemented on CCV keeper but implemented on standalone keeper
+func (k Keeper) Delegation(sdkCtx context.Context, addr sdk.AccAddress, valAddr sdk.ValAddress) (stakingtypes.DelegationI, error) {
+	ctx := sdk.UnwrapSDKContext(sdkCtx)
+	if k.ChangeoverIsComplete(ctx) && k.standaloneStakingKeeper != nil {
+		return k.standaloneStakingKeeper.Delegation(ctx, addr, valAddr)
+	}
+	return stakingtypes.Delegation{}, errors.New("unimplemented on CCV keeper")
 }
 
 // MaxValidators - unimplemented on CCV keeper

diff --git a/x/ccv/consumer/keeper/validators_test.go b/x/ccv/consumer/keeper/validators_test.go
@@ -151,6 +151,13 @@ func TestIsValidatorJailed(t *testing.T) {
 	isJailed3, err := consumerKeeper.IsValidatorJailed(ctx, consAddr)
 	require.NoError(t, err)
 	require.True(t, isJailed3)
+
+	// confirm that unjail returns no error and validator remains jailed
+	mocks.MockStakingKeeper.EXPECT().IsValidatorJailed(ctx, consAddr).Return(true, nil).Times(1)
+	require.NoError(t, consumerKeeper.Unjail(ctx, consAddr))
+	isJailed3, err = consumerKeeper.IsValidatorJailed(ctx, consAddr)
+	require.NoError(t, err)
+	require.True(t, isJailed3)
 }
 
 func TestSlash(t *testing.T) {