From c011807f7f2d291104178d533ab3cf8816cbcc8b Mon Sep 17 00:00:00 2001 From: MSalopek Date: Mon, 11 Nov 2024 19:55:44 +0100 Subject: [PATCH 01/11] feat!: enable unjail on pre-ccv chains --- x/ccv/consumer/keeper/validators.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/x/ccv/consumer/keeper/validators.go b/x/ccv/consumer/keeper/validators.go index 4386bb7b86..859434a95b 100644 --- a/x/ccv/consumer/keeper/validators.go +++ b/x/ccv/consumer/keeper/validators.go @@ -179,7 +179,13 @@ func (k Keeper) Jail(context.Context, sdk.ConsAddress) error { return nil } // 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 } +func (k Keeper) Unjail(sdkCtx context.Context, addr sdk.ConsAddress) error { + ctx := sdk.UnwrapSDKContext(sdkCtx) + if k.IsPrevStandaloneChain(ctx) && 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) { From 25f70204113c6392e88b7cd4e059eadc07530480 Mon Sep 17 00:00:00 2001 From: MSalopek Date: Mon, 11 Nov 2024 19:57:37 +0100 Subject: [PATCH 02/11] docs: update docstrings --- x/ccv/consumer/keeper/validators.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/x/ccv/consumer/keeper/validators.go b/x/ccv/consumer/keeper/validators.go index 859434a95b..70b2d20a33 100644 --- a/x/ccv/consumer/keeper/validators.go +++ b/x/ccv/consumer/keeper/validators.go @@ -174,11 +174,9 @@ 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 after the changeover is complete. // -// 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. +// 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.IsPrevStandaloneChain(ctx) && k.ChangeoverIsComplete(ctx) && k.standaloneStakingKeeper != nil { From 83bf87a3c78315ec5fdc433536d2fb7f489d0cbc Mon Sep 17 00:00:00 2001 From: MSalopek Date: Tue, 19 Nov 2024 11:22:36 +0100 Subject: [PATCH 03/11] feat!: enable unjail related functions --- x/ccv/consumer/keeper/validators.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/x/ccv/consumer/keeper/validators.go b/x/ccv/consumer/keeper/validators.go index 70b2d20a33..2f158d5ed2 100644 --- a/x/ccv/consumer/keeper/validators.go +++ b/x/ccv/consumer/keeper/validators.go @@ -85,7 +85,12 @@ func (k Keeper) IterateValidators(context.Context, func(index int64, validator s } // Validator - unimplemented on CCV keeper -func (k Keeper) Validator(ctx context.Context, addr sdk.ValAddress) (stakingtypes.ValidatorI, error) { +func (k Keeper) Validator(sdkCtx context.Context, addr sdk.ValAddress) (stakingtypes.ValidatorI, error) { + ctx := sdk.UnwrapSDKContext(sdkCtx) + if k.IsPrevStandaloneChain(ctx) && k.ChangeoverIsComplete(ctx) && k.standaloneStakingKeeper != nil { + return k.standaloneStakingKeeper.Validator(ctx, addr) + } + panic("unimplemented on CCV keeper") } @@ -186,7 +191,11 @@ func (k Keeper) Unjail(sdkCtx context.Context, addr sdk.ConsAddress) error { } // Delegation - unimplemented on CCV keeper -func (k Keeper) Delegation(ctx context.Context, addr sdk.AccAddress, valAddr sdk.ValAddress) (stakingtypes.DelegationI, error) { +func (k Keeper) Delegation(sdkCtx context.Context, addr sdk.AccAddress, valAddr sdk.ValAddress) (stakingtypes.DelegationI, error) { + ctx := sdk.UnwrapSDKContext(sdkCtx) + if k.IsPrevStandaloneChain(ctx) && k.ChangeoverIsComplete(ctx) && k.standaloneStakingKeeper != nil { + return k.standaloneStakingKeeper.Delegation(ctx, addr, valAddr) + } panic("unimplemented on CCV keeper") } From 831d178cac80fffe70bf112b321a9a9316059696 Mon Sep 17 00:00:00 2001 From: MSalopek Date: Tue, 19 Nov 2024 11:23:45 +0100 Subject: [PATCH 04/11] docs: update docstrings --- x/ccv/consumer/keeper/validators.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/ccv/consumer/keeper/validators.go b/x/ccv/consumer/keeper/validators.go index 2f158d5ed2..8d50a96edd 100644 --- a/x/ccv/consumer/keeper/validators.go +++ b/x/ccv/consumer/keeper/validators.go @@ -84,7 +84,7 @@ func (k Keeper) IterateValidators(context.Context, func(index int64, validator s return nil } -// Validator - 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.IsPrevStandaloneChain(ctx) && k.ChangeoverIsComplete(ctx) && k.standaloneStakingKeeper != nil { @@ -190,7 +190,7 @@ func (k Keeper) Unjail(sdkCtx context.Context, addr sdk.ConsAddress) error { return nil } -// Delegation - 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.IsPrevStandaloneChain(ctx) && k.ChangeoverIsComplete(ctx) && k.standaloneStakingKeeper != nil { From 4877f08cb314a0a4986079da55cea727efb175f7 Mon Sep 17 00:00:00 2001 From: MSalopek Date: Thu, 5 Dec 2024 17:24:26 +0100 Subject: [PATCH 05/11] fix: update handling (changeover completed checks) --- x/ccv/consumer/keeper/changeover.go | 2 +- x/ccv/consumer/keeper/validators.go | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/x/ccv/consumer/keeper/changeover.go b/x/ccv/consumer/keeper/changeover.go index 7f7ba4ad5b..044266e64c 100644 --- 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 index 8d50a96edd..34175263b2 100644 --- a/x/ccv/consumer/keeper/validators.go +++ b/x/ccv/consumer/keeper/validators.go @@ -87,11 +87,11 @@ func (k Keeper) IterateValidators(context.Context, func(index int64, validator s // 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.IsPrevStandaloneChain(ctx) && k.ChangeoverIsComplete(ctx) && k.standaloneStakingKeeper != nil { + if k.ChangeoverIsComplete(ctx) && k.standaloneStakingKeeper != nil { return k.standaloneStakingKeeper.Validator(ctx, addr) } - panic("unimplemented on CCV keeper") + return stakingtypes.Validator{}, errors.New("unimplemented on CCV keeper") } // IsJailed returns the outstanding slashing flag for the given validator address @@ -184,7 +184,7 @@ func (k Keeper) Jail(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.IsPrevStandaloneChain(ctx) && k.ChangeoverIsComplete(ctx) && k.standaloneStakingKeeper != nil { + if k.ChangeoverIsComplete(ctx) && k.standaloneStakingKeeper != nil { return k.standaloneStakingKeeper.Unjail(ctx, addr) } return nil @@ -193,10 +193,10 @@ func (k Keeper) Unjail(sdkCtx context.Context, addr sdk.ConsAddress) error { // 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.IsPrevStandaloneChain(ctx) && k.ChangeoverIsComplete(ctx) && k.standaloneStakingKeeper != nil { + if k.ChangeoverIsComplete(ctx) && k.standaloneStakingKeeper != nil { return k.standaloneStakingKeeper.Delegation(ctx, addr, valAddr) } - panic("unimplemented on CCV keeper") + return stakingtypes.Delegation{}, errors.New("unimplemented on CCV keeper") } // MaxValidators - unimplemented on CCV keeper From 84acb91ea82cdbee1074f6e9c5951330e9c1b622 Mon Sep 17 00:00:00 2001 From: MSalopek Date: Thu, 5 Dec 2024 17:27:57 +0100 Subject: [PATCH 06/11] fix comment --- x/ccv/consumer/keeper/validators.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/ccv/consumer/keeper/validators.go b/x/ccv/consumer/keeper/validators.go index 34175263b2..3fce5441a1 100644 --- a/x/ccv/consumer/keeper/validators.go +++ b/x/ccv/consumer/keeper/validators.go @@ -179,7 +179,7 @@ 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 is enabled for previously standalone chains after the changeover is complete. +// Unjail is enabled for previously standalone chains and chains implementing democracy staking. // // 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 { From b65fc99533bd98dfd8538e34694db3076e13b50a Mon Sep 17 00:00:00 2001 From: MSalopek Date: Mon, 9 Dec 2024 18:52:14 +0100 Subject: [PATCH 07/11] tests: add unjail tests --- tests/integration/democracy.go | 37 ++++++++++++++++++++++++ tests/integration/unbonding.go | 11 +++++++ testutil/integration/debug_test.go | 8 +++++ x/ccv/consumer/keeper/validators_test.go | 7 +++++ 4 files changed, 63 insertions(+) diff --git a/tests/integration/democracy.go b/tests/integration/democracy.go index 645cdb7805..e2d198ce2f 100644 --- a/tests/integration/democracy.go +++ b/tests/integration/democracy.go @@ -233,6 +233,43 @@ func (s *ConsumerDemocracyTestSuite) TestDemocracyMsgUpdateParams() { s.Assert().Equal(votersOldBalances, getAccountsBalances(s.consumerCtx(), bankKeeper, bondDenom, votingAccounts)) } +func (s *ConsumerDemocracyTestSuite) TestDemocracyValidatorUnjail() { + stakingKeeper := s.consumerApp.GetTestStakingKeeper() + + validators, err := stakingKeeper.GetAllValidators(s.consumerCtx()) + s.Require().NoError(err) + + // setting up pre-conditions + // validator[0] is expected to be jailed + expectJailed := validators[0] + expectJailed.Jailed = true + 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()) + } + } + + // the actual test if unjailing works + err = stakingKeeper.GetValidatorSet().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 index 3f13d7cd1f..7727240eaa 100644 --- 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 index 6092298a15..5c076a9f48 100644 --- 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/validators_test.go b/x/ccv/consumer/keeper/validators_test.go index 349a3b2b40..b24ef44dde 100644 --- 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) { From 928baa5317b84e0cc7df5ca0a55e6de5f84067be Mon Sep 17 00:00:00 2001 From: MSalopek Date: Mon, 9 Dec 2024 18:54:48 +0100 Subject: [PATCH 08/11] tests: improve unjail tests --- tests/integration/democracy.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/integration/democracy.go b/tests/integration/democracy.go index e2d198ce2f..69619d2aa5 100644 --- a/tests/integration/democracy.go +++ b/tests/integration/democracy.go @@ -235,6 +235,7 @@ func (s *ConsumerDemocracyTestSuite) TestDemocracyMsgUpdateParams() { func (s *ConsumerDemocracyTestSuite) TestDemocracyValidatorUnjail() { stakingKeeper := s.consumerApp.GetTestStakingKeeper() + consumerKeeper := s.consumerApp.GetConsumerKeeper() validators, err := stakingKeeper.GetAllValidators(s.consumerCtx()) s.Require().NoError(err) @@ -258,8 +259,9 @@ func (s *ConsumerDemocracyTestSuite) TestDemocracyValidatorUnjail() { } } - // the actual test if unjailing works - err = stakingKeeper.GetValidatorSet().Unjail(s.consumerCtx(), consAddr) + // 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() From e5edc4bf177d6466b810df48efeb558da9b8611e Mon Sep 17 00:00:00 2001 From: MSalopek Date: Mon, 9 Dec 2024 18:55:57 +0100 Subject: [PATCH 09/11] tests: improve unjail tests --- tests/integration/democracy.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/democracy.go b/tests/integration/democracy.go index 69619d2aa5..7e49734df7 100644 --- a/tests/integration/democracy.go +++ b/tests/integration/democracy.go @@ -243,10 +243,10 @@ func (s *ConsumerDemocracyTestSuite) TestDemocracyValidatorUnjail() { // setting up pre-conditions // validator[0] is expected to be jailed expectJailed := validators[0] - expectJailed.Jailed = true consAddr, err := expectJailed.GetConsAddr() s.Require().NoError(err) stakingKeeper.GetValidatorSet().Jail(s.consumerCtx(), consAddr) + s.consumerChain.NextBlock() validators, err = stakingKeeper.GetAllValidators(s.consumerCtx()) From 23b3a50c20b81d792b2a1deae13a1e5b6d780fda Mon Sep 17 00:00:00 2001 From: MSalopek Date: Tue, 10 Dec 2024 17:22:20 +0100 Subject: [PATCH 10/11] tests: appease linter --- tests/integration/democracy.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/integration/democracy.go b/tests/integration/democracy.go index 7e49734df7..993a0d1bec 100644 --- a/tests/integration/democracy.go +++ b/tests/integration/democracy.go @@ -233,6 +233,13 @@ 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() From 2b716a6275030acf2b0b7c1d7462508e82b881dc Mon Sep 17 00:00:00 2001 From: github-actions Date: Tue, 10 Dec 2024 16:25:54 +0000 Subject: [PATCH 11/11] Update testing documentation --- scripts/test_doc/test_documentation.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scripts/test_doc/test_documentation.md b/scripts/test_doc/test_documentation.md index d8cf18e62e..ac1085ecd0 100644 --- 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* Set up a democracy consumer chain.
* Create a new block.
* Check that rewards to democracy representatives, community pool, and provider redistribution account are distributed in the right proportions.
| [TestDemocracyMsgUpdateParams](../../tests/integration/democracy.go#L187) | TestDemocracyMsgUpdateParams checks that the consumer parameters can be updated through a governance proposal.
Details* Set up a democracy consumer chain.
* Submit a proposal containing changes to the consumer module parameters.
* Check that the proposal is executed, and the parameters are updated.
| + [TestDemocracyValidatorUnjail](../../tests/integration/democracy.go#L243) | TestDemocracyValidatorUnjail checks that the consumer validator can be unjailed when there is a standalone staking keeper available.
Details* Set up a democracy consumer chain.
* Jail a validator.
* Check that the validator is jailed.
* Unjail the validator.
* Check that the validator is unjailed.
| # [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* Set up CCV channel.
* Perform initial delegation of tokens followed by a partial undelegation (1/4 of the tokens).
* Verify that the staking unbonding operation is created as expected.
* Increment provider block height.
* Check that the unbonding operation has been completed.
* Verify that the token balances are correctly updated and the expected amount of tokens has been returned to the account.
| + [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* Set up CCV channel.
* Perform initial delegation of tokens followed by a partial undelegation (1/4 of the tokens).
* Verify that the staking unbonding operation is created as expected.
* Increment provider block height.
* Check that the unbonding operation has been completed.
* Verify that the token balances are correctly updated and the expected amount of tokens has been returned to the account.
| + [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. | # [valset_update.go](../../tests/integration/valset_update.go)