Skip to content

Commit

Permalink
feat!: automatically opt in validators that vote Yes on consumer addi…
Browse files Browse the repository at this point in the history
…tion proposals (#1629)

* init commit

* changed providerKeeper.GetProposedConsumerChain to return a  bool

* add logging mesages

* one more log message

* fix comment

* added one more test case of NO vote and made tabular test
  • Loading branch information
insumity authored and sainoe committed Mar 8, 2024
1 parent e82ee9f commit cdf139c
Show file tree
Hide file tree
Showing 6 changed files with 194 additions and 20 deletions.
53 changes: 41 additions & 12 deletions testutil/keeper/mocks.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

56 changes: 55 additions & 1 deletion x/ccv/provider/keeper/hooks.go
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -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) {
Expand Down
92 changes: 90 additions & 2 deletions x/ccv/provider/keeper/hooks_test.go
Original file line number Diff line number Diff line change
@@ -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"

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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))
})
}
}
5 changes: 3 additions & 2 deletions x/ccv/provider/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions x/ccv/provider/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand All @@ -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)
}
Expand Down
2 changes: 2 additions & 0 deletions x/ccv/types/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

0 comments on commit cdf139c

Please sign in to comment.