From 51d80be7f0d516767297bdebec5e0d653774b51c Mon Sep 17 00:00:00 2001 From: Thomas Bruyelle Date: Fri, 10 Mar 2023 16:42:06 +0100 Subject: [PATCH 1/2] feat: pause unbondings during equivocation proposal voting period Fix #747 The change registers 2 gov module hooks in the provider module: - `AfterProposalDeposit`: if the proposal is an equivocation proposal in voting period, then call `PutUnbondingOnHold` for each unbondings of each validators found in the proposal. - `AfterProposalVotingPeriodEnded`: if the proposal is an equivocation proposal, then call `UnbondingCanComplete` for each unbondings of each validators found in the proposal. A new key is also added, to store the equivocation proposal ID for which unbondings have been put on hold. This covers 2 specific cases: - The gov module allows additional deposits even if the proposal is already in voting period. So when `AfterProposalDeposit` is invoked, we have to make sure the proposal is in voting period for the first time before puting the unbondings on hold. This is simply handled by checking if the proposal ID exists in the store at the beginning of the hook, and then storing it if not. - If the provider chain is upgraded with this change and there's already an equivocation proposal in voting period, `AfterProposalVotingPeriodEnded` could be invoked without the initial `AfterProposalDeposit`, so some unbondings could be un-paused while they hadn't been paused (conflicts with `AfterUnbondingInitiated` hook). To prevent that, we check the proposal ID exists in the store, which means `AfterProposalDeposit` has been called prevouisly. Co-authored-by: Albert Le Batteux --- app/provider/app.go | 14 +- tests/e2e/common.go | 50 +++++ tests/e2e/proposal.go | 243 +++++++++++++++++++++++++ testutil/e2e/interfaces.go | 3 + testutil/keeper/mocks.go | 111 ++++++++--- testutil/keeper/unit_test_helpers.go | 2 + x/ccv/provider/keeper/hooks.go | 122 ++++++++++++- x/ccv/provider/keeper/proposal.go | 18 ++ x/ccv/provider/keeper/proposal_test.go | 16 ++ x/ccv/provider/keeper/relay_test.go | 6 +- x/ccv/provider/types/keys.go | 13 ++ x/ccv/types/expected_keepers.go | 7 + 12 files changed, 573 insertions(+), 32 deletions(-) create mode 100644 tests/e2e/proposal.go diff --git a/app/provider/app.go b/app/provider/app.go index 2037ee131a..6553b629f4 100644 --- a/app/provider/app.go +++ b/app/provider/app.go @@ -376,7 +376,7 @@ func New( stakingtypes.NewMultiStakingHooks( app.DistrKeeper.Hooks(), app.SlashingKeeper.Hooks(), - app.ProviderKeeper.Hooks(), + app.ProviderKeeper.Hooks(app.GovKeeper), ), ) @@ -426,7 +426,7 @@ func New( AddRoute(providertypes.RouterKey, ibcprovider.NewProviderProposalHandler(app.ProviderKeeper)). AddRoute(ibcclienttypes.RouterKey, ibcclient.NewClientProposalHandler(app.IBCKeeper.ClientKeeper)) - app.GovKeeper = govkeeper.NewKeeper( + govKeeper := govkeeper.NewKeeper( appCodec, keys[govtypes.StoreKey], app.GetSubspace(govtypes.ModuleName), @@ -435,6 +435,11 @@ func New( &stakingKeeper, govRouter, ) + app.GovKeeper = *govKeeper.SetHooks( + govtypes.NewMultiGovHooks( + app.ProviderKeeper.Hooks(govKeeper), + ), + ) app.TransferKeeper = ibctransferkeeper.NewKeeper( appCodec, @@ -763,6 +768,11 @@ func (app *App) GetE2eStakingKeeper() e2e.E2eStakingKeeper { return app.StakingKeeper } +// GetE2eGovKeeper implements the ProviderApp interface. +func (app *App) GetE2eGovKeeper() e2e.E2eGovKeeper { + return app.GovKeeper +} + // GetE2eBankKeeper implements the ProviderApp interface. func (app *App) GetE2eBankKeeper() e2e.E2eBankKeeper { return app.BankKeeper diff --git a/tests/e2e/common.go b/tests/e2e/common.go index 5e2390b887..cd66d7b787 100644 --- a/tests/e2e/common.go +++ b/tests/e2e/common.go @@ -299,6 +299,56 @@ func checkStakingUnbondingOps(s *CCVTestSuite, id uint64, found bool, onHold boo } } +// unbondingsOnHold is a handy struct to hold the different kinds of unbonding +// refcount. +type unbondingsOnHold struct { + unbondingDelegationRefCount int64 + validatorUnbondingRefCount int64 + redelegationRefCount int64 +} + +// checkStakingUnbondingOnHoldRefCount checks that valAddr's unbonding refcounts +// match expected. +func checkStakingUnbondingOnHoldRefCount(s *CCVTestSuite, valAddr sdk.ValAddress, expected unbondingsOnHold) { + + // check unbondingDelegation + ubds := s.providerApp.GetE2eStakingKeeper().GetUnbondingDelegationsFromValidator(s.providerCtx(), valAddr) + if expected.unbondingDelegationRefCount == 0 { + s.Assert().Empty(ubds, "expected no unbonding delegation") + } else { + s.Assert().NotEmpty(ubds, "no unbonding delegation found") + for _, ubd := range ubds { + s.Assert().NotEmpty(ubd.Entries, "no unbonding delegation entries found") + for _, entry := range ubd.Entries { + s.Assert().Equal(expected.unbondingDelegationRefCount, entry.UnbondingOnHoldRefCount, + "wrong unbonding delegation ref count") + } + } + } + + // check redelegation + reds := s.providerApp.GetE2eStakingKeeper().GetRedelegationsFromSrcValidator(s.providerCtx(), valAddr) + if expected.redelegationRefCount == 0 { + s.Assert().Empty(reds, "expected no redelegation") + } else { + s.Assert().NotEmpty(reds, "no redelegation found") + for _, ubd := range reds { + s.Assert().NotEmpty(ubd.Entries, "no redelegation entries found") + for _, entry := range ubd.Entries { + s.Assert().Equal(expected.redelegationRefCount, entry.UnbondingOnHoldRefCount, + "wrong redelegation ref count") + } + } + } + + // check validator unbonding + val, found := s.providerApp.GetE2eStakingKeeper().GetValidator(s.providerCtx(), valAddr) + if s.Assert().True(found, "validator not found") { + s.Assert().Equal(expected.validatorUnbondingRefCount, val.UnbondingOnHoldRefCount, + "wrong validator unbonding ref count") + } +} + func checkCCVUnbondingOp(s *CCVTestSuite, providerCtx sdk.Context, chainID string, valUpdateID uint64, found bool, msgAndArgs ...interface{}) { entries := s.providerApp.GetProviderKeeper().GetUnbondingOpsFromIndex(providerCtx, chainID, valUpdateID) if found { diff --git a/tests/e2e/proposal.go b/tests/e2e/proposal.go new file mode 100644 index 0000000000..f2496db6c0 --- /dev/null +++ b/tests/e2e/proposal.go @@ -0,0 +1,243 @@ +package e2e + +import ( + "time" + + sdk "github.com/cosmos/cosmos-sdk/types" + evidencetypes "github.com/cosmos/cosmos-sdk/x/evidence/types" + govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" + providertypes "github.com/cosmos/interchain-security/x/ccv/provider/types" +) + +func (s *CCVTestSuite) TestPauseUnbondingOnEquivocationProposal() { + tests := []struct { + name string + setup func() govtypes.Content + expectedWithoutProp unbondingsOnHold + expectedDuringProp unbondingsOnHold + }{ + { + // Assert a text proposal doesn't pause any existing unbondings + name: "text proposal and unbonding delegation", + setup: func() govtypes.Content { + var ( + bondAmt = sdk.NewInt(10000000) + delAddr = s.providerChain.SenderAccount.GetAddress() + ) + // delegate bondAmt and undelegate it + delegateAndUndelegate(s, delAddr, bondAmt, 1) + + return govtypes.NewTextProposal("title", "desc") + }, + expectedWithoutProp: unbondingsOnHold{ + unbondingDelegationRefCount: 1, + }, + expectedDuringProp: unbondingsOnHold{ + unbondingDelegationRefCount: 1, + }, + }, + { + // Assert an equivocation proposal pauses nothing if there's no existing + // unbondings. + name: "equivocation proposal and no unbonding", + setup: func() govtypes.Content { + var ( + val, _ = s.getValByIdx(0) + consAddr, _ = val.GetConsAddr() + ) + return providertypes.NewEquivocationProposal("title", "desc", + []*evidencetypes.Equivocation{{ + Height: 1, + Power: 1, + Time: time.Now(), + ConsensusAddress: consAddr.String(), + }}, + ) + }, + expectedWithoutProp: unbondingsOnHold{}, + expectedDuringProp: unbondingsOnHold{}, + }, + { + // Assert an equivocation proposal pauses unbonding delegations + name: "equivocation proposal and unbonding delegation", + setup: func() govtypes.Content { + // create an unbonding entry of type UnbondingDelegation + var ( + bondAmt = sdk.NewInt(10000000) + delAddr = s.providerChain.SenderAccount.GetAddress() + val, _ = s.getValByIdx(0) + consAddr, _ = val.GetConsAddr() + ) + // delegate bondAmt and undelegate it + delegateAndUndelegate(s, delAddr, bondAmt, 1) + + return providertypes.NewEquivocationProposal("title", "desc", + []*evidencetypes.Equivocation{{ + Height: 1, + Power: 1, + Time: time.Now(), + ConsensusAddress: consAddr.String(), + }}, + ) + }, + expectedWithoutProp: unbondingsOnHold{ + unbondingDelegationRefCount: 1, + }, + expectedDuringProp: unbondingsOnHold{ + unbondingDelegationRefCount: 2, + }, + }, + { + // Assert an equivocation proposal pauses redelegations + name: "equivocation proposal and redelegate", + setup: func() govtypes.Content { + // create an unbonding entry of type UnbondingDelegation + var ( + bondAmt = sdk.NewInt(10000000) + delAddr = s.providerChain.SenderAccount.GetAddress() + val, valSrcAddr = s.getValByIdx(0) + _, valDstAddr = s.getValByIdx(1) + consAddr, _ = val.GetConsAddr() + ) + // delegate bondAmt and redelegate it + delegateAndRedelegate(s, delAddr, valSrcAddr, valDstAddr, bondAmt) + + return providertypes.NewEquivocationProposal("title", "desc", + []*evidencetypes.Equivocation{{ + Height: 1, + Power: 1, + Time: time.Now(), + ConsensusAddress: consAddr.String(), + }}, + ) + }, + expectedWithoutProp: unbondingsOnHold{ + redelegationRefCount: 1, + }, + expectedDuringProp: unbondingsOnHold{ + redelegationRefCount: 2, + }, + }, + { + // Assert an equivocation proposal pauses validator unbonding + name: "equivocation proposal and validator unbonding", + setup: func() govtypes.Content { + var ( + delAddr = s.providerChain.SenderAccount.GetAddress() + val, valAddr = s.getValByIdx(0) + consAddr, _ = val.GetConsAddr() + ) + // unbond validator by undelegate all his delegation (test setup only + // delegates from delAddr, there's no validator self delegation) + undelegate(s, delAddr, valAddr, sdk.NewDec(1)) + + return providertypes.NewEquivocationProposal("title", "desc", + []*evidencetypes.Equivocation{{ + Height: 1, + Power: 1, + Time: time.Now(), + ConsensusAddress: consAddr.String(), + }}, + ) + }, + expectedWithoutProp: unbondingsOnHold{ + unbondingDelegationRefCount: 1, + validatorUnbondingRefCount: 1, + }, + expectedDuringProp: unbondingsOnHold{ + unbondingDelegationRefCount: 2, + validatorUnbondingRefCount: 2, + }, + }, + } + submitProp := func(s *CCVTestSuite, content govtypes.Content) uint64 { + proposal, err := s.providerApp.GetE2eGovKeeper().SubmitProposal(s.providerCtx(), content) + s.Require().NoError(err) + return proposal.ProposalId + } + addDepositProp := func(s *CCVTestSuite, proposalID uint64, depositAmt sdk.Coins) { + depositorAddr := s.providerChain.SenderAccount.GetAddress() + _, err := s.providerApp.GetE2eGovKeeper().AddDeposit( + s.providerCtx(), proposalID, depositorAddr, depositAmt, + ) + s.Require().NoError(err) + } + waitFor := func(s *CCVTestSuite, period time.Duration) { + s.providerChain.CurrentHeader.Time = s.providerChain.CurrentHeader.Time.Add(period) + // Need to advance 2 blocks because the time set at the line above is only + // applied for the second block + s.providerChain.NextBlock() + s.providerChain.NextBlock() + } + for i, tt := range tests { + s.Run(tt.name, func() { + if i+1 < len(tests) { + // reset suite to reset provider client + defer s.SetupTest() + } + + //----------------------------------------- + // Setup + + var ( + govContent = tt.setup() + val, valAddr = s.getValByIdx(0) + consAddr, _ = val.GetConsAddr() + govKeeper = s.providerApp.GetE2eGovKeeper() + dustAmt = sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(1))) + minDepositAmt = govKeeper.GetDepositParams(s.providerCtx()).MinDeposit + ) + // Equivocation proposal requires validator signing info and a slash log + s.setDefaultValSigningInfo(*s.providerChain.Vals.Validators[0]) + s.providerApp.GetProviderKeeper().SetSlashLog(s.providerCtx(), + providertypes.NewProviderConsAddress(consAddr)) + // Reduce voting period + votingParams := govKeeper.GetVotingParams(s.providerCtx()) + votingParams.VotingPeriod = 3 * time.Second + govKeeper.SetVotingParams(s.providerCtx(), votingParams) + // Reduce deposit period + depositParams := govKeeper.GetDepositParams(s.providerCtx()) + depositParams.MaxDepositPeriod = 3 * time.Second + govKeeper.SetDepositParams(s.providerCtx(), depositParams) + // must move one block forward because unbonding validators are detected + // during EndBlock. + s.providerChain.NextBlock() + checkStakingUnbondingOnHoldRefCount(s, valAddr, tt.expectedWithoutProp) + + //----------------------------------------- + // #1 Create a proposal, activate it and wait for voting period + + proposalID := submitProp(s, govContent) + checkStakingUnbondingOnHoldRefCount(s, valAddr, tt.expectedWithoutProp) + // assert that until voting period starts, there's no pause triggered + addDepositProp(s, proposalID, dustAmt) + checkStakingUnbondingOnHoldRefCount(s, valAddr, tt.expectedWithoutProp) + // activate prop + addDepositProp(s, proposalID, minDepositAmt) + checkStakingUnbondingOnHoldRefCount(s, valAddr, tt.expectedDuringProp) + // assert that an additionnal deposit done after the activation doesn't + // trigger additionnal pauses + addDepositProp(s, proposalID, dustAmt) + checkStakingUnbondingOnHoldRefCount(s, valAddr, tt.expectedDuringProp) + // ends the voting period + waitFor(s, votingParams.VotingPeriod) + checkStakingUnbondingOnHoldRefCount(s, valAddr, tt.expectedWithoutProp) + s.Assert().False( + s.providerApp.GetProviderKeeper().HasEquivocationProposal(s.providerCtx(), proposalID), + "proposalID should be removed from store after voting period", + ) + + //----------------------------------------- + // #2 Create an other proposal but let it expire (not enough deposit) + + proposalID = submitProp(s, govContent) + checkStakingUnbondingOnHoldRefCount(s, valAddr, tt.expectedWithoutProp) + waitFor(s, depositParams.MaxDepositPeriod) + checkStakingUnbondingOnHoldRefCount(s, valAddr, tt.expectedWithoutProp) + s.Assert().False( + s.providerApp.GetProviderKeeper().HasEquivocationProposal(s.providerCtx(), proposalID), + "proposalID shouldn't be registered if proposal didn't reach the voting period", + ) + }) + } +} diff --git a/testutil/e2e/interfaces.go b/testutil/e2e/interfaces.go index a5b8e1962f..6a15ae3828 100644 --- a/testutil/e2e/interfaces.go +++ b/testutil/e2e/interfaces.go @@ -38,6 +38,8 @@ type ProviderApp interface { GetE2eSlashingKeeper() E2eSlashingKeeper // Returns a distribution keeper interface with more capabilities than the expected_keepers interface GetE2eDistributionKeeper() E2eDistributionKeeper + // Returns a gov keeper interface with more capabilities than the expected_keepers interface + GetE2eGovKeeper() E2eGovKeeper } // The interface that any consumer app must implement to be compatible with e2e tests @@ -140,6 +142,7 @@ type E2eMintKeeper interface { type E2eGovKeeper interface { GetDepositParams(ctx sdk.Context) govtypes.DepositParams + SetDepositParams(sdk.Context, govtypes.DepositParams) GetVotingParams(ctx sdk.Context) govtypes.VotingParams SetVotingParams(ctx sdk.Context, votingParams govtypes.VotingParams) SubmitProposal(ctx sdk.Context, content govtypes.Content) (govtypes.Proposal, error) diff --git a/testutil/keeper/mocks.go b/testutil/keeper/mocks.go index 66fc77dbce..4ab6388a94 100644 --- a/testutil/keeper/mocks.go +++ b/testutil/keeper/mocks.go @@ -13,14 +13,15 @@ import ( types0 "github.com/cosmos/cosmos-sdk/x/auth/types" types1 "github.com/cosmos/cosmos-sdk/x/capability/types" types2 "github.com/cosmos/cosmos-sdk/x/evidence/types" - types3 "github.com/cosmos/cosmos-sdk/x/slashing/types" - types4 "github.com/cosmos/cosmos-sdk/x/staking/types" - types5 "github.com/cosmos/ibc-go/v4/modules/core/02-client/types" - types6 "github.com/cosmos/ibc-go/v4/modules/core/03-connection/types" - types7 "github.com/cosmos/ibc-go/v4/modules/core/04-channel/types" + types3 "github.com/cosmos/cosmos-sdk/x/gov/types" + types4 "github.com/cosmos/cosmos-sdk/x/slashing/types" + types5 "github.com/cosmos/cosmos-sdk/x/staking/types" + types6 "github.com/cosmos/ibc-go/v4/modules/core/02-client/types" + types7 "github.com/cosmos/ibc-go/v4/modules/core/03-connection/types" + types8 "github.com/cosmos/ibc-go/v4/modules/core/04-channel/types" exported "github.com/cosmos/ibc-go/v4/modules/core/exported" gomock "github.com/golang/mock/gomock" - types8 "github.com/tendermint/tendermint/abci/types" + types9 "github.com/tendermint/tendermint/abci/types" ) // MockStakingKeeper is a mock of StakingKeeper interface. @@ -74,11 +75,39 @@ func (mr *MockStakingKeeperMockRecorder) GetLastValidatorPower(ctx, operator int return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetLastValidatorPower", reflect.TypeOf((*MockStakingKeeper)(nil).GetLastValidatorPower), ctx, operator) } +// GetRedelegationsFromSrcValidator mocks base method. +func (m *MockStakingKeeper) GetRedelegationsFromSrcValidator(arg0 types.Context, arg1 types.ValAddress) []types5.Redelegation { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetRedelegationsFromSrcValidator", arg0, arg1) + ret0, _ := ret[0].([]types5.Redelegation) + return ret0 +} + +// GetRedelegationsFromSrcValidator indicates an expected call of GetRedelegationsFromSrcValidator. +func (mr *MockStakingKeeperMockRecorder) GetRedelegationsFromSrcValidator(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetRedelegationsFromSrcValidator", reflect.TypeOf((*MockStakingKeeper)(nil).GetRedelegationsFromSrcValidator), arg0, arg1) +} + +// GetUnbondingDelegationsFromValidator mocks base method. +func (m *MockStakingKeeper) GetUnbondingDelegationsFromValidator(arg0 types.Context, arg1 types.ValAddress) []types5.UnbondingDelegation { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetUnbondingDelegationsFromValidator", arg0, arg1) + ret0, _ := ret[0].([]types5.UnbondingDelegation) + return ret0 +} + +// GetUnbondingDelegationsFromValidator indicates an expected call of GetUnbondingDelegationsFromValidator. +func (mr *MockStakingKeeperMockRecorder) GetUnbondingDelegationsFromValidator(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUnbondingDelegationsFromValidator", reflect.TypeOf((*MockStakingKeeper)(nil).GetUnbondingDelegationsFromValidator), arg0, arg1) +} + // GetValidator mocks base method. -func (m *MockStakingKeeper) GetValidator(ctx types.Context, addr types.ValAddress) (types4.Validator, bool) { +func (m *MockStakingKeeper) GetValidator(ctx types.Context, addr types.ValAddress) (types5.Validator, bool) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetValidator", ctx, addr) - ret0, _ := ret[0].(types4.Validator) + ret0, _ := ret[0].(types5.Validator) ret1, _ := ret[1].(bool) return ret0, ret1 } @@ -90,10 +119,10 @@ func (mr *MockStakingKeeperMockRecorder) GetValidator(ctx, addr interface{}) *go } // GetValidatorByConsAddr mocks base method. -func (m *MockStakingKeeper) GetValidatorByConsAddr(ctx types.Context, consAddr types.ConsAddress) (types4.Validator, bool) { +func (m *MockStakingKeeper) GetValidatorByConsAddr(ctx types.Context, consAddr types.ConsAddress) (types5.Validator, bool) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetValidatorByConsAddr", ctx, consAddr) - ret0, _ := ret[0].(types4.Validator) + ret0, _ := ret[0].(types5.Validator) ret1, _ := ret[1].(bool) return ret0, ret1 } @@ -105,10 +134,10 @@ func (mr *MockStakingKeeperMockRecorder) GetValidatorByConsAddr(ctx, consAddr in } // GetValidatorUpdates mocks base method. -func (m *MockStakingKeeper) GetValidatorUpdates(ctx types.Context) []types8.ValidatorUpdate { +func (m *MockStakingKeeper) GetValidatorUpdates(ctx types.Context) []types9.ValidatorUpdate { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetValidatorUpdates", ctx) - ret0, _ := ret[0].([]types8.ValidatorUpdate) + ret0, _ := ret[0].([]types9.ValidatorUpdate) return ret0 } @@ -171,7 +200,7 @@ func (mr *MockStakingKeeperMockRecorder) PutUnbondingOnHold(ctx, id interface{}) } // Slash mocks base method. -func (m *MockStakingKeeper) Slash(arg0 types.Context, arg1 types.ConsAddress, arg2, arg3 int64, arg4 types.Dec, arg5 types4.InfractionType) { +func (m *MockStakingKeeper) Slash(arg0 types.Context, arg1 types.ConsAddress, arg2, arg3 int64, arg4 types.Dec, arg5 types5.InfractionType) { m.ctrl.T.Helper() m.ctrl.Call(m, "Slash", arg0, arg1, arg2, arg3, arg4, arg5) } @@ -210,6 +239,44 @@ func (mr *MockStakingKeeperMockRecorder) UnbondingTime(ctx interface{}) *gomock. return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UnbondingTime", reflect.TypeOf((*MockStakingKeeper)(nil).UnbondingTime), ctx) } +// MockGovKeeper is a mock of GovKeeper interface. +type MockGovKeeper struct { + ctrl *gomock.Controller + recorder *MockGovKeeperMockRecorder +} + +// MockGovKeeperMockRecorder is the mock recorder for MockGovKeeper. +type MockGovKeeperMockRecorder struct { + mock *MockGovKeeper +} + +// NewMockGovKeeper creates a new mock instance. +func NewMockGovKeeper(ctrl *gomock.Controller) *MockGovKeeper { + mock := &MockGovKeeper{ctrl: ctrl} + mock.recorder = &MockGovKeeperMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockGovKeeper) EXPECT() *MockGovKeeperMockRecorder { + return m.recorder +} + +// GetProposal mocks base method. +func (m *MockGovKeeper) GetProposal(ctx types.Context, proposalID uint64) (types3.Proposal, bool) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetProposal", ctx, proposalID) + ret0, _ := ret[0].(types3.Proposal) + ret1, _ := ret[1].(bool) + return ret0, ret1 +} + +// GetProposal indicates an expected call of GetProposal. +func (mr *MockGovKeeperMockRecorder) GetProposal(ctx, proposalID interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetProposal", reflect.TypeOf((*MockGovKeeper)(nil).GetProposal), ctx, proposalID) +} + // MockEvidenceKeeper is a mock of EvidenceKeeper interface. type MockEvidenceKeeper struct { ctrl *gomock.Controller @@ -283,10 +350,10 @@ func (mr *MockSlashingKeeperMockRecorder) DowntimeJailDuration(arg0 interface{}) } // GetValidatorSigningInfo mocks base method. -func (m *MockSlashingKeeper) GetValidatorSigningInfo(ctx types.Context, address types.ConsAddress) (types3.ValidatorSigningInfo, bool) { +func (m *MockSlashingKeeper) GetValidatorSigningInfo(ctx types.Context, address types.ConsAddress) (types4.ValidatorSigningInfo, bool) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetValidatorSigningInfo", ctx, address) - ret0, _ := ret[0].(types3.ValidatorSigningInfo) + ret0, _ := ret[0].(types4.ValidatorSigningInfo) ret1, _ := ret[1].(bool) return ret0, ret1 } @@ -401,10 +468,10 @@ func (mr *MockChannelKeeperMockRecorder) ChanCloseInit(ctx, portID, channelID, c } // GetChannel mocks base method. -func (m *MockChannelKeeper) GetChannel(ctx types.Context, srcPort, srcChan string) (types7.Channel, bool) { +func (m *MockChannelKeeper) GetChannel(ctx types.Context, srcPort, srcChan string) (types8.Channel, bool) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetChannel", ctx, srcPort, srcChan) - ret0, _ := ret[0].(types7.Channel) + ret0, _ := ret[0].(types8.Channel) ret1, _ := ret[1].(bool) return ret0, ret1 } @@ -519,10 +586,10 @@ func (m *MockConnectionKeeper) EXPECT() *MockConnectionKeeperMockRecorder { } // GetConnection mocks base method. -func (m *MockConnectionKeeper) GetConnection(ctx types.Context, connectionID string) (types6.ConnectionEnd, bool) { +func (m *MockConnectionKeeper) GetConnection(ctx types.Context, connectionID string) (types7.ConnectionEnd, bool) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetConnection", ctx, connectionID) - ret0, _ := ret[0].(types6.ConnectionEnd) + ret0, _ := ret[0].(types7.ConnectionEnd) ret1, _ := ret[1].(bool) return ret0, ret1 } @@ -777,7 +844,7 @@ func (m *MockIBCTransferKeeper) EXPECT() *MockIBCTransferKeeperMockRecorder { } // SendTransfer mocks base method. -func (m *MockIBCTransferKeeper) SendTransfer(ctx types.Context, sourcePort, sourceChannel string, token types.Coin, sender types.AccAddress, receiver string, timeoutHeight types5.Height, timeoutTimestamp uint64) error { +func (m *MockIBCTransferKeeper) SendTransfer(ctx types.Context, sourcePort, sourceChannel string, token types.Coin, sender types.AccAddress, receiver string, timeoutHeight types6.Height, timeoutTimestamp uint64) error { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "SendTransfer", ctx, sourcePort, sourceChannel, token, sender, receiver, timeoutHeight, timeoutTimestamp) ret0, _ := ret[0].(error) @@ -814,10 +881,10 @@ func (m *MockIBCCoreKeeper) EXPECT() *MockIBCCoreKeeperMockRecorder { } // ChannelOpenInit mocks base method. -func (m *MockIBCCoreKeeper) ChannelOpenInit(goCtx context.Context, msg *types7.MsgChannelOpenInit) (*types7.MsgChannelOpenInitResponse, error) { +func (m *MockIBCCoreKeeper) ChannelOpenInit(goCtx context.Context, msg *types8.MsgChannelOpenInit) (*types8.MsgChannelOpenInitResponse, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "ChannelOpenInit", goCtx, msg) - ret0, _ := ret[0].(*types7.MsgChannelOpenInitResponse) + ret0, _ := ret[0].(*types8.MsgChannelOpenInitResponse) ret1, _ := ret[1].(error) return ret0, ret1 } diff --git a/testutil/keeper/unit_test_helpers.go b/testutil/keeper/unit_test_helpers.go index ab0eeef2ea..b4f1c82352 100644 --- a/testutil/keeper/unit_test_helpers.go +++ b/testutil/keeper/unit_test_helpers.go @@ -88,6 +88,7 @@ type MockedKeepers struct { *MockIBCTransferKeeper *MockIBCCoreKeeper *MockEvidenceKeeper + *MockGovKeeper } // NewMockedKeepers instantiates a struct with pointers to properly instantiated mocked keepers. @@ -105,6 +106,7 @@ func NewMockedKeepers(ctrl *gomock.Controller) MockedKeepers { MockIBCTransferKeeper: NewMockIBCTransferKeeper(ctrl), MockIBCCoreKeeper: NewMockIBCCoreKeeper(ctrl), MockEvidenceKeeper: NewMockEvidenceKeeper(ctrl), + MockGovKeeper: NewMockGovKeeper(ctrl), } } diff --git a/x/ccv/provider/keeper/hooks.go b/x/ccv/provider/keeper/hooks.go index 15f3dc5bbc..f74499f26f 100644 --- a/x/ccv/provider/keeper/hooks.go +++ b/x/ccv/provider/keeper/hooks.go @@ -3,25 +3,38 @@ package keeper import ( "fmt" + sdk "github.com/cosmos/cosmos-sdk/types" + govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" - sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/interchain-security/x/ccv/provider/types" providertypes "github.com/cosmos/interchain-security/x/ccv/provider/types" + ccv "github.com/cosmos/interchain-security/x/ccv/types" "github.com/cosmos/interchain-security/x/ccv/utils" ) // Wrapper struct type Hooks struct { - k *Keeper + k *Keeper + govKeeper ccv.GovKeeper } -var _ stakingtypes.StakingHooks = Hooks{} +var ( + _ stakingtypes.StakingHooks = Hooks{} + _ govtypes.GovHooks = Hooks{} +) // Returns new provider hooks -func (k *Keeper) Hooks() Hooks { - return Hooks{k} +func (k *Keeper) Hooks(govKeeper ccv.GovKeeper) Hooks { + return Hooks{ + k: k, + govKeeper: govKeeper, + } } +//----------------------------------------- +// Staking Hooks + // This stores a record of each unbonding op from staking, allowing us to track which consumer chains have unbonded func (h Hooks) AfterUnbondingInitiated(ctx sdk.Context, ID uint64) error { var consumerChainIDS []string @@ -132,3 +145,102 @@ func (h Hooks) AfterValidatorBeginUnbonding(_ sdk.Context, _ sdk.ConsAddress, _ } func (h Hooks) BeforeDelegationRemoved(_ sdk.Context, _ sdk.AccAddress, _ sdk.ValAddress) { } + +//----------------------------------------- +// Gov Hooks + +func (h Hooks) AfterProposalSubmission(ctx sdk.Context, proposalID uint64) {} + +func (h Hooks) AfterProposalDeposit(ctx sdk.Context, proposalID uint64, _ sdk.AccAddress) { + if h.k.HasEquivocationProposal(ctx, proposalID) { + // already handled, skip + return + } + prop, found := h.govKeeper.GetProposal(ctx, proposalID) + if !found { + panic(fmt.Sprintf("cannot find proposal %d", proposalID)) + } + if prop.Status != govtypes.StatusVotingPeriod { + // skip if proposal is not in voting period + return + } + eqProp, ok := prop.GetContent().(*types.EquivocationProposal) + if !ok { + // skip if not an equivocation proposal + return + } + // Mark proposal has handled + h.k.SetEquivocationProposal(ctx, proposalID) + for _, eq := range eqProp.Equivocations { + ids, err := h.getUnbondingOpsIDsForValidator(ctx, eq.ConsensusAddress) + if err != nil { + panic(fmt.Errorf("can get unbondings of validator '%s': %w", eq.ConsensusAddress, err)) + } + for _, id := range ids { + err := h.k.stakingKeeper.PutUnbondingOnHold(ctx, id) + if err != nil { + panic(fmt.Errorf("cannot PutUnbondingOnHold for id %d: %w", id, err)) + } + } + } +} + +func (h Hooks) AfterProposalVote(_ sdk.Context, _ uint64, _ sdk.AccAddress) {} +func (h Hooks) AfterProposalFailedMinDeposit(_ sdk.Context, _ uint64) {} + +func (h Hooks) AfterProposalVotingPeriodEnded(ctx sdk.Context, proposalID uint64) { + prop, found := h.govKeeper.GetProposal(ctx, proposalID) + if !found { + panic(fmt.Sprintf("cannot find proposal %d", proposalID)) + } + eqProp, ok := prop.GetContent().(*types.EquivocationProposal) + if !ok { + // skip if not an equivocation proposal + return + } + for _, eq := range eqProp.Equivocations { + ids, err := h.getUnbondingOpsIDsForValidator(ctx, eq.ConsensusAddress) + if err != nil { + panic(fmt.Errorf("can get unbondings of validator '%s': %w", eq.ConsensusAddress, err)) + } + for _, id := range ids { + err := h.k.stakingKeeper.UnbondingCanComplete(ctx, id) + if err != nil { + panic(fmt.Errorf("cannot UnbondingCanComplete for id %d: %w", id, err)) + } + } + } + // Remove equivocation proposal flag + h.k.DeleteEquivocationProposal(ctx, proposalID) +} + +// getUnbondingOpsIDsForValidator returns all pending unbonding operations for +// validator consensus address consAddrStr. +func (h Hooks) getUnbondingOpsIDsForValidator(ctx sdk.Context, consAddrStr string) ([]uint64, error) { + consAddr, err := sdk.ConsAddressFromBech32(consAddrStr) + if err != nil { + return nil, fmt.Errorf("cannot convert '%s' to consensus address", consAddrStr) + } + val, found := h.k.stakingKeeper.GetValidatorByConsAddr(ctx, consAddr) + if !found { + return nil, fmt.Errorf("cannot find validator for consensus address '%s'", consAddr) + } + var ids []uint64 + // pause all unbonding delegation + ubds := h.k.stakingKeeper.GetUnbondingDelegationsFromValidator(ctx, val.GetOperator()) + for _, ubd := range ubds { + for _, entry := range ubd.Entries { + ids = append(ids, entry.UnbondingId) + } + } + // pause all redelegations + reds := h.k.stakingKeeper.GetRedelegationsFromSrcValidator(ctx, val.GetOperator()) + for _, red := range reds { + for _, entry := range red.Entries { + ids = append(ids, entry.UnbondingId) + } + } + // pause all unbonding validator + ids = append(ids, val.UnbondingIds...) + return ids, nil +} diff --git a/x/ccv/provider/keeper/proposal.go b/x/ccv/provider/keeper/proposal.go index c60051e372..4699fbdf8c 100644 --- a/x/ccv/provider/keeper/proposal.go +++ b/x/ccv/provider/keeper/proposal.go @@ -613,3 +613,21 @@ func (k Keeper) HandleEquivocationProposal(ctx sdk.Context, p *types.Equivocatio } return nil } + +// SetEquivocationProposal sets the equivocation proposal flag for proposalID. +func (k Keeper) SetEquivocationProposal(ctx sdk.Context, proposalID uint64) { + store := ctx.KVStore(k.storeKey) + store.Set(types.EquivocationProposalKey(proposalID), []byte{}) +} + +// HasEquivocationProposal returns the equivocation proposal flag for proposalID. +func (k Keeper) HasEquivocationProposal(ctx sdk.Context, proposalID uint64) bool { + store := ctx.KVStore(k.storeKey) + return store.Get(types.EquivocationProposalKey(proposalID)) != nil +} + +// DeleteEquivocationProposal deletes the equivocation proposal flag for proposalID. +func (k Keeper) DeleteEquivocationProposal(ctx sdk.Context, proposalID uint64) { + store := ctx.KVStore(k.storeKey) + store.Delete(types.EquivocationProposalKey(proposalID)) +} diff --git a/x/ccv/provider/keeper/proposal_test.go b/x/ccv/provider/keeper/proposal_test.go index 3c9dc27863..cef935b4b1 100644 --- a/x/ccv/provider/keeper/proposal_test.go +++ b/x/ccv/provider/keeper/proposal_test.go @@ -1061,3 +1061,19 @@ func TestHandleEquivocationProposal(t *testing.T) { ctrl.Finish() } } + +func TestEquivocationProposal(t *testing.T) { + var ( + require = require.New(t) + k, ctx, _, _ = testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) + proposalID = uint64(1) + ) + + require.False(k.HasEquivocationProposal(ctx, proposalID), "proposalID shouldn't exist") + + k.SetEquivocationProposal(ctx, proposalID) + require.True(k.HasEquivocationProposal(ctx, proposalID), "proposalID should exist") + + k.DeleteEquivocationProposal(ctx, proposalID) + require.False(k.HasEquivocationProposal(ctx, proposalID), "proposalID shouldn't exist") +} diff --git a/x/ccv/provider/keeper/relay_test.go b/x/ccv/provider/keeper/relay_test.go index 229975ca19..8f039172e1 100644 --- a/x/ccv/provider/keeper/relay_test.go +++ b/x/ccv/provider/keeper/relay_test.go @@ -567,7 +567,7 @@ func TestHandleVSCMaturedPacket(t *testing.T) { // Start first unbonding without any consumers registered var unbondingOpId uint64 = 1 - err := pk.Hooks().AfterUnbondingInitiated(ctx, unbondingOpId) + err := pk.Hooks(mocks.MockGovKeeper).AfterUnbondingInitiated(ctx, unbondingOpId) require.NoError(t, err) // Check that no unbonding op was stored _, found := pk.GetUnbondingOp(ctx, unbondingOpId) @@ -585,7 +585,7 @@ func TestHandleVSCMaturedPacket(t *testing.T) { gomock.InOrder( mocks.MockStakingKeeper.EXPECT().PutUnbondingOnHold(ctx, unbondingOpId).Return(nil), ) - err = pk.Hooks().AfterUnbondingInitiated(ctx, unbondingOpId) + err = pk.Hooks(mocks.MockGovKeeper).AfterUnbondingInitiated(ctx, unbondingOpId) require.NoError(t, err) // Check that an unbonding op was stored expectedChains := []string{"chain-1"} @@ -612,7 +612,7 @@ func TestHandleVSCMaturedPacket(t *testing.T) { gomock.InOrder( mocks.MockStakingKeeper.EXPECT().PutUnbondingOnHold(ctx, id).Return(nil), ) - err = pk.Hooks().AfterUnbondingInitiated(ctx, id) + err = pk.Hooks(mocks.MockGovKeeper).AfterUnbondingInitiated(ctx, id) require.NoError(t, err) } // Check that the unbonding ops were stored diff --git a/x/ccv/provider/types/keys.go b/x/ccv/provider/types/keys.go index 07773a91f3..3743e33ee1 100644 --- a/x/ccv/provider/types/keys.go +++ b/x/ccv/provider/types/keys.go @@ -127,6 +127,10 @@ const ( // SlashLogBytePrefix is the byte prefix that will store the mapping from provider address to boolean // denoting whether the provider address has commited any double signign infractions SlashLogBytePrefix + + // EquivocationProposalBytePrefix is the byte prefix that will store the + // equivocation proposal id. + EquivocationProposalBytePrefix ) // PortKey returns the key to the port ID in the store @@ -448,3 +452,12 @@ func ParseChainIdAndConsAddrKey(prefix byte, bz []byte) (string, sdk.ConsAddress func SlashLogKey(providerAddr ProviderConsAddress) []byte { return append([]byte{SlashLogBytePrefix}, providerAddr.ToSdkConsAddr().Bytes()...) } + +// EquivocationProposalKey returns the key with the following format: +// bytePrefix | uint64(proposalID) +func EquivocationProposalKey(proposalID uint64) []byte { + return ccvutils.AppendMany( + []byte{EquivocationProposalBytePrefix}, + sdk.Uint64ToBigEndian(proposalID), + ) +} diff --git a/x/ccv/types/expected_keepers.go b/x/ccv/types/expected_keepers.go index 6ce079f454..12684a2442 100644 --- a/x/ccv/types/expected_keepers.go +++ b/x/ccv/types/expected_keepers.go @@ -8,6 +8,7 @@ import ( auth "github.com/cosmos/cosmos-sdk/x/auth/types" capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types" evidencetypes "github.com/cosmos/cosmos-sdk/x/evidence/types" + govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" slashingtypes "github.com/cosmos/cosmos-sdk/x/slashing/types" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" clienttypes "github.com/cosmos/ibc-go/v4/modules/core/02-client/types" @@ -23,6 +24,8 @@ import ( // so we do not need a registry module between the staking module and CCV. type StakingKeeper interface { GetValidatorUpdates(ctx sdk.Context) []abci.ValidatorUpdate + GetUnbondingDelegationsFromValidator(sdk.Context, sdk.ValAddress) []stakingtypes.UnbondingDelegation + GetRedelegationsFromSrcValidator(sdk.Context, sdk.ValAddress) []stakingtypes.Redelegation UnbondingCanComplete(ctx sdk.Context, id uint64) error UnbondingTime(ctx sdk.Context) time.Duration GetValidatorByConsAddr(ctx sdk.Context, consAddr sdk.ConsAddress) (validator stakingtypes.Validator, found bool) @@ -37,6 +40,10 @@ type StakingKeeper interface { GetLastTotalPower(ctx sdk.Context) sdk.Int } +type GovKeeper interface { + GetProposal(ctx sdk.Context, proposalID uint64) (govtypes.Proposal, bool) +} + type EvidenceKeeper interface { HandleEquivocationEvidence(ctx sdk.Context, evidence *evidencetypes.Equivocation) } From 6df4b95cb4aac2cc27e7275993ea1e89cf742b94 Mon Sep 17 00:00:00 2001 From: Thomas Bruyelle Date: Wed, 22 Mar 2023 13:46:38 +0100 Subject: [PATCH 2/2] replace waitFor with existing incrementTime --- tests/e2e/proposal.go | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/tests/e2e/proposal.go b/tests/e2e/proposal.go index f2496db6c0..1ce0341976 100644 --- a/tests/e2e/proposal.go +++ b/tests/e2e/proposal.go @@ -162,13 +162,6 @@ func (s *CCVTestSuite) TestPauseUnbondingOnEquivocationProposal() { ) s.Require().NoError(err) } - waitFor := func(s *CCVTestSuite, period time.Duration) { - s.providerChain.CurrentHeader.Time = s.providerChain.CurrentHeader.Time.Add(period) - // Need to advance 2 blocks because the time set at the line above is only - // applied for the second block - s.providerChain.NextBlock() - s.providerChain.NextBlock() - } for i, tt := range tests { s.Run(tt.name, func() { if i+1 < len(tests) { @@ -220,7 +213,7 @@ func (s *CCVTestSuite) TestPauseUnbondingOnEquivocationProposal() { addDepositProp(s, proposalID, dustAmt) checkStakingUnbondingOnHoldRefCount(s, valAddr, tt.expectedDuringProp) // ends the voting period - waitFor(s, votingParams.VotingPeriod) + incrementTime(s, votingParams.VotingPeriod) checkStakingUnbondingOnHoldRefCount(s, valAddr, tt.expectedWithoutProp) s.Assert().False( s.providerApp.GetProviderKeeper().HasEquivocationProposal(s.providerCtx(), proposalID), @@ -232,7 +225,7 @@ func (s *CCVTestSuite) TestPauseUnbondingOnEquivocationProposal() { proposalID = submitProp(s, govContent) checkStakingUnbondingOnHoldRefCount(s, valAddr, tt.expectedWithoutProp) - waitFor(s, depositParams.MaxDepositPeriod) + incrementTime(s, depositParams.MaxDepositPeriod) checkStakingUnbondingOnHoldRefCount(s, valAddr, tt.expectedWithoutProp) s.Assert().False( s.providerApp.GetProviderKeeper().HasEquivocationProposal(s.providerCtx(), proposalID),