diff --git a/testutil/keeper/mocks.go b/testutil/keeper/mocks.go index 5f9d9b2694..b183e8b377 100644 --- a/testutil/keeper/mocks.go +++ b/testutil/keeper/mocks.go @@ -473,18 +473,6 @@ func (mr *MockSlashingKeeperMockRecorder) GetValidatorSigningInfo(ctx, address i return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetValidatorSigningInfo", reflect.TypeOf((*MockSlashingKeeper)(nil).GetValidatorSigningInfo), ctx, address) } -// SetValidatorSigningInfo mocks base method. -func (m *MockSlashingKeeper) SetValidatorSigningInfo(ctx types0.Context, address types0.ConsAddress, info types3.ValidatorSigningInfo) { - m.ctrl.T.Helper() - m.ctrl.Call(m, "SetValidatorSigningInfo", ctx, address, info) -} - -// SetValidatorSigningInfo indicates an expected call of SetValidatorSigningInfo. -func (mr *MockSlashingKeeperMockRecorder) SetValidatorSigningInfo(ctx, address interface{}, info interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetValidatorSigningInfo", reflect.TypeOf((*MockSlashingKeeper)(nil).SetValidatorSigningInfo), ctx, address, info) -} - // IsTombstoned mocks base method. func (m *MockSlashingKeeper) IsTombstoned(arg0 types0.Context, arg1 types0.ConsAddress) bool { m.ctrl.T.Helper() @@ -511,6 +499,18 @@ func (mr *MockSlashingKeeperMockRecorder) JailUntil(arg0, arg1, arg2 interface{} return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "JailUntil", reflect.TypeOf((*MockSlashingKeeper)(nil).JailUntil), arg0, arg1, arg2) } +// SetValidatorSigningInfo mocks base method. +func (m *MockSlashingKeeper) SetValidatorSigningInfo(ctx types0.Context, address types0.ConsAddress, info types3.ValidatorSigningInfo) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "SetValidatorSigningInfo", ctx, address, info) +} + +// SetValidatorSigningInfo indicates an expected call of SetValidatorSigningInfo. +func (mr *MockSlashingKeeperMockRecorder) SetValidatorSigningInfo(ctx, address, info interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetValidatorSigningInfo", reflect.TypeOf((*MockSlashingKeeper)(nil).SetValidatorSigningInfo), ctx, address, info) +} + // SlashFractionDoubleSign mocks base method. func (m *MockSlashingKeeper) SlashFractionDoubleSign(ctx types0.Context) types0.Dec { m.ctrl.T.Helper() @@ -1217,3 +1217,32 @@ func (mr *MockGovKeeperMockRecorder) GetProposal(ctx, proposalID interface{}) *g mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetProposal", reflect.TypeOf((*MockGovKeeper)(nil).GetProposal), ctx, proposalID) } + +// GetProposals mocks base method. +func (m *MockGovKeeper) GetProposals(ctx types0.Context) v1.Proposals { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetProposals", ctx) + ret0, _ := ret[0].(v1.Proposals) + return ret0 +} + +// GetProposals indicates an expected call of GetProposals. +func (mr *MockGovKeeperMockRecorder) GetProposals(ctx interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetProposals", reflect.TypeOf((*MockGovKeeper)(nil).GetProposals), ctx) +} + +// GetVote mocks base method. +func (m *MockGovKeeper) GetVote(ctx types0.Context, proposalID uint64, voterAddr types0.AccAddress) (v1.Vote, bool) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetVote", ctx, proposalID, voterAddr) + ret0, _ := ret[0].(v1.Vote) + ret1, _ := ret[1].(bool) + return ret0, ret1 +} + +// GetVote indicates an expected call of GetVote. +func (mr *MockGovKeeperMockRecorder) GetVote(ctx, proposalID, voterAddr interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetVote", reflect.TypeOf((*MockGovKeeper)(nil).GetVote), ctx, proposalID, voterAddr) +} diff --git a/x/ccv/provider/keeper/hooks.go b/x/ccv/provider/keeper/hooks.go index 88590a9875..fcf8339187 100644 --- a/x/ccv/provider/keeper/hooks.go +++ b/x/ccv/provider/keeper/hooks.go @@ -1,8 +1,8 @@ package keeper import ( + "cosmossdk.io/math" "fmt" - sdk "github.com/cosmos/cosmos-sdk/types" sdkgov "github.com/cosmos/cosmos-sdk/x/gov/types" v1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1" @@ -175,7 +175,61 @@ func (h Hooks) AfterProposalVotingPeriodEnded(ctx sdk.Context, proposalID uint64 func (h Hooks) AfterProposalDeposit(ctx sdk.Context, proposalID uint64, depositorAddr sdk.AccAddress) { } +// AfterProposalVote opts in validators that vote YES (with 100% weight) on a `ConsumerAdditionProposal`. If a +// validator votes multiple times, only the last vote would be considered on whether the validator is opted in or not. func (h Hooks) AfterProposalVote(ctx sdk.Context, proposalID uint64, voterAddr sdk.AccAddress) { + validator, found := h.k.stakingKeeper.GetValidator(ctx, voterAddr.Bytes()) + if !found { + return + } + + consAddr, err := validator.GetConsAddr() + if err != nil { + h.k.Logger(ctx).Error("could not extract validator's consensus address", + "error", err.Error(), + "validator acc addr", voterAddr, + ) + return + } + + chainID, found := h.k.GetProposedConsumerChain(ctx, proposalID) + if !found { + return + } + + vote, found := h.k.govKeeper.GetVote(ctx, proposalID, voterAddr) + if !found { + h.k.Logger(ctx).Error("could not find vote for validator", + "validator acc addr", voterAddr, + "proposalID", proposalID, + ) + return + } + + if len(vote.Options) == 1 && vote.Options[0].Option == v1.VoteOption_VOTE_OPTION_YES { + // only consider votes that vote YES with 100% weight + weight, err := sdk.NewDecFromStr(vote.Options[0].Weight) + if err != nil { + h.k.Logger(ctx).Error("could not extract decimal value from vote's weight", + "vote", vote, + "error", err.Error(), + ) + return + } + if !weight.Equal(math.LegacyNewDec(1)) { + h.k.Logger(ctx).Error("single vote does not have a weight of 1", + "vote", vote, + ) + return + } + + // in the validator is already to-be-opted in, the `SetToBeOptedIn` is a no-op + h.k.SetToBeOptedIn(ctx, chainID, providertypes.NewProviderConsAddress(consAddr)) + } else { + // if vote is not a YES vote with 100% weight, we delete the validator from to-be-opted in + // if the validator is not to-be-opted in, the `DeleteToBeOptedIn` is a no-op + h.k.DeleteToBeOptedIn(ctx, chainID, providertypes.NewProviderConsAddress(consAddr)) + } } func (h Hooks) AfterProposalFailedMinDeposit(ctx sdk.Context, proposalID uint64) { diff --git a/x/ccv/provider/keeper/hooks_test.go b/x/ccv/provider/keeper/hooks_test.go index f4aad8d441..140bb8a4ce 100644 --- a/x/ccv/provider/keeper/hooks_test.go +++ b/x/ccv/provider/keeper/hooks_test.go @@ -1,6 +1,10 @@ package keeper_test import ( + codectypes "github.com/cosmos/cosmos-sdk/codec/types" + "github.com/cosmos/cosmos-sdk/crypto/keys/ed25519" + stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" + "github.com/cosmos/interchain-security/v4/x/ccv/provider/types" "testing" "time" @@ -123,11 +127,13 @@ func TestAfterPropSubmissionAndVotingPeriodEnded(t *testing.T) { k.Hooks().AfterProposalSubmission(ctx, prop.Id) // verify that the proposal ID is created - require.NotEmpty(t, k.GetProposedConsumerChain(ctx, prop.Id)) + _, found := k.GetProposedConsumerChain(ctx, prop.Id) + require.True(t, found) k.Hooks().AfterProposalVotingPeriodEnded(ctx, prop.Id) // verify that the proposal ID is deleted - require.Empty(t, k.GetProposedConsumerChain(ctx, prop.Id)) + _, found = k.GetProposedConsumerChain(ctx, prop.Id) + require.False(t, found) } func TestGetConsumerAdditionLegacyPropFromProp(t *testing.T) { @@ -223,3 +229,85 @@ func TestGetConsumerAdditionLegacyPropFromProp(t *testing.T) { }) } } + +func TestAfterProposalVoteWithYesVote(t *testing.T) { + k, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) + defer ctrl.Finish() + + providerConsPubKey := ed25519.GenPrivKeyFromSecret([]byte{1}).PubKey() + pkAny, _ := codectypes.NewAnyWithValue(providerConsPubKey) + providerAddr := types.NewProviderConsAddress(providerConsPubKey.Address().Bytes()) + + options := []*v1.WeightedVoteOption{{Option: v1.OptionYes, Weight: "1"}} + k.SetProposedConsumerChain(ctx, "chainID", 1) + + gomock.InOrder( + mocks.MockStakingKeeper.EXPECT().GetValidator(ctx, gomock.Any()).Return( + stakingtypes.Validator{ConsensusPubkey: pkAny}, true), + mocks.MockGovKeeper.EXPECT().GetVote(ctx, gomock.Any(), gomock.Any()).Return( + v1.Vote{ProposalId: 1, Voter: "voter", Options: options}, true, + ), + ) + + require.False(t, k.IsToBeOptedIn(ctx, "chainID", providerAddr)) + k.Hooks().AfterProposalVote(ctx, 1, sdk.AccAddress{}) + require.True(t, k.IsToBeOptedIn(ctx, "chainID", providerAddr)) +} + +func TestAfterProposalVoteWithNoVote(t *testing.T) { + testCases := []struct { + name string + options []*v1.WeightedVoteOption + setup func(sdk.Context, []*v1.WeightedVoteOption, testkeeper.MockedKeepers, *codectypes.Any) + }{ + { + "Weighted vote with 100% NO", + []*v1.WeightedVoteOption{{Option: v1.OptionNo, Weight: "1"}}, + func(ctx sdk.Context, options []*v1.WeightedVoteOption, + mocks testkeeper.MockedKeepers, pubKey *codectypes.Any) { + gomock.InOrder( + mocks.MockStakingKeeper.EXPECT().GetValidator(ctx, gomock.Any()).Return( + stakingtypes.Validator{ConsensusPubkey: pubKey}, true), + mocks.MockGovKeeper.EXPECT().GetVote(ctx, gomock.Any(), gomock.Any()).Return( + v1.Vote{ProposalId: 1, Voter: "voter", Options: options}, true, + ), + ) + }, + }, + { + "Weighted vote with 99.9% YES and 0.1% NO", + []*v1.WeightedVoteOption{{Option: v1.OptionYes, Weight: "0.999"}, {Option: v1.OptionNo, Weight: "0.001"}}, + func(ctx sdk.Context, options []*v1.WeightedVoteOption, + mocks testkeeper.MockedKeepers, pubKey *codectypes.Any) { + gomock.InOrder( + mocks.MockStakingKeeper.EXPECT().GetValidator(ctx, gomock.Any()).Return( + stakingtypes.Validator{ConsensusPubkey: pubKey}, true), + mocks.MockGovKeeper.EXPECT().GetVote(ctx, gomock.Any(), gomock.Any()).Return( + v1.Vote{ProposalId: 1, Voter: "voter", Options: options}, true, + ), + ) + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + k, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) + defer ctrl.Finish() + + providerConsPubKey := ed25519.GenPrivKeyFromSecret([]byte{1}).PubKey() + pkAny, _ := codectypes.NewAnyWithValue(providerConsPubKey) + providerAddr := types.NewProviderConsAddress(providerConsPubKey.Address().Bytes()) + + k.SetProposedConsumerChain(ctx, "chainID", 1) + + tc.setup(ctx, tc.options, mocks, pkAny) + + // set the validator to-be-opted in first to assert that a NO vote removes the validator from to-be-opted in + k.SetToBeOptedIn(ctx, "chainID", providerAddr) + require.True(t, k.IsToBeOptedIn(ctx, "chainID", providerAddr)) + k.Hooks().AfterProposalVote(ctx, 1, sdk.AccAddress{}) + require.False(t, k.IsToBeOptedIn(ctx, "chainID", providerAddr)) + }) + } +} diff --git a/x/ccv/provider/keeper/keeper.go b/x/ccv/provider/keeper/keeper.go index ea052ec5fd..42bcb12ace 100644 --- a/x/ccv/provider/keeper/keeper.go +++ b/x/ccv/provider/keeper/keeper.go @@ -186,9 +186,10 @@ func (k Keeper) SetProposedConsumerChain(ctx sdk.Context, chainID string, propos } // GetProposedConsumerChain returns the proposed chainID for the given consumerAddition proposal ID. -func (k Keeper) GetProposedConsumerChain(ctx sdk.Context, proposalID uint64) string { +func (k Keeper) GetProposedConsumerChain(ctx sdk.Context, proposalID uint64) (string, bool) { store := ctx.KVStore(k.storeKey) - return string(store.Get(types.ProposedConsumerChainKey(proposalID))) + consumerChain := store.Get(types.ProposedConsumerChainKey(proposalID)) + return string(consumerChain), consumerChain != nil } // DeleteProposedConsumerChainInStore deletes the consumer chainID from store diff --git a/x/ccv/provider/keeper/keeper_test.go b/x/ccv/provider/keeper/keeper_test.go index 6eb28675ba..35380a67aa 100644 --- a/x/ccv/provider/keeper/keeper_test.go +++ b/x/ccv/provider/keeper/keeper_test.go @@ -552,7 +552,7 @@ func TestSetProposedConsumerChains(t *testing.T) { for _, test := range tests { providerKeeper.SetProposedConsumerChain(ctx, test.chainID, test.proposalID) - cID := providerKeeper.GetProposedConsumerChain(ctx, test.proposalID) + cID, _ := providerKeeper.GetProposedConsumerChain(ctx, test.proposalID) require.Equal(t, cID, test.chainID) } } @@ -574,9 +574,9 @@ func TestDeleteProposedConsumerChainInStore(t *testing.T) { for _, test := range tests { providerKeeper.SetProposedConsumerChain(ctx, test.chainID, test.proposalID) providerKeeper.DeleteProposedConsumerChainInStore(ctx, test.deleteProposalID) - cID := providerKeeper.GetProposedConsumerChain(ctx, test.proposalID) + cID, found := providerKeeper.GetProposedConsumerChain(ctx, test.proposalID) if test.ok { - require.Equal(t, cID, "") + require.False(t, found) } else { require.Equal(t, cID, test.chainID) } diff --git a/x/ccv/types/expected_keepers.go b/x/ccv/types/expected_keepers.go index a2ef7ab465..df531e09ca 100644 --- a/x/ccv/types/expected_keepers.go +++ b/x/ccv/types/expected_keepers.go @@ -151,4 +151,6 @@ type ScopedKeeper interface { type GovKeeper interface { GetProposal(ctx sdk.Context, proposalID uint64) (v1.Proposal, bool) + GetProposals(ctx sdk.Context) (proposals v1.Proposals) + GetVote(ctx sdk.Context, proposalID uint64, voterAddr sdk.AccAddress) (vote v1.Vote, found bool) }