Skip to content

Commit

Permalink
fix bug and add tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@sainoe
sainoe committed Oct 12, 2023
1 parent 584ce4e commit b26b453
Show file tree
Hide file tree
Showing 2 changed files with 163 additions and 87 deletions.
82 changes: 38 additions & 44 deletions x/ccv/provider/keeper/hooks.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,58 +197,21 @@ func (h Hooks) BeforeDelegationRemoved(_ sdk.Context, _ sdk.AccAddress, _ sdk.Va
//

// AfterProposalSubmission - call hook if registered
// After consumerAddition proposal submission, the consumer chainID is stored
// After a consumerAddition proposal submission, a record is created
// that maps the proposal ID to the consumer chain ID.
func (h Hooks) AfterProposalSubmission(ctx sdk.Context, proposalID uint64) {
p, ok := h.k.govKeeper.GetProposal(ctx, proposalID)
if !ok {
panic(fmt.Errorf("failed to get proposal %d in gov hook", proposalID))
}
msgs := p.GetMessages()

for _, msg := range msgs {
sdkMsg, isLegacyProposal := msg.GetCachedValue().(*v1.MsgExecLegacyContent)
if !isLegacyProposal {
continue
}

content, err := v1.LegacyContentFromMessage(sdkMsg)
if err != nil {
panic(fmt.Errorf("failed to get legacy proposal %d content in gov hook", proposalID))
}

consProp, ok := content.(*types.ConsumerAdditionProposal)
if ok {
h.k.SetProposedConsumerChain(ctx, consProp.ChainId, proposalID)
}
if p, ok := h.GetConsumerAdditionLegacyPropFromProp(ctx, proposalID); ok {
h.k.SetProposedConsumerChain(ctx, p.ChainId, proposalID)
}
}

// AfterProposalVotingPeriodEnded - call hook if registered
// After proposal voting ends, the consumer chainID in store is deleted.
// When a proposal passes, this chainID will be available in providerKeeper.GetAllPendingConsumerAdditionProps
// When a consumerAddition proposal passes, the consumer chainID is available in providerKeeper.GetAllPendingConsumerAdditionProps
// or providerKeeper.GetAllConsumerChains(ctx).
func (h Hooks) AfterProposalVotingPeriodEnded(ctx sdk.Context, proposalID uint64) {
p, ok := h.k.govKeeper.GetProposal(ctx, proposalID)
if !ok {
panic(fmt.Errorf("failed to get proposal %d in gov hook", proposalID))
}
msgs := p.GetMessages()

for _, msg := range msgs {
if sdkMsg, isLegacyProposal := msg.GetCachedValue().(*v1.MsgExecLegacyContent); isLegacyProposal {
content, err := v1.LegacyContentFromMessage(sdkMsg)
if err != nil {
panic(fmt.Errorf("failed to get legacy proposal %d content in gov hook", proposalID))
}

if content.ProposalType() != types.ProposalTypeConsumerAddition {
continue
}

h.k.DeleteProposedConsumerChainInStore(ctx, proposalID)
}

continue
if _, ok := h.GetConsumerAdditionLegacyPropFromProp(ctx, proposalID); ok {
h.k.DeleteProposedConsumerChainInStore(ctx, proposalID)
}
}

Expand All @@ -260,3 +223,34 @@ func (h Hooks) AfterProposalVote(ctx sdk.Context, proposalID uint64, voterAddr s

func (h Hooks) AfterProposalFailedMinDeposit(ctx sdk.Context, proposalID uint64) {
}

func (h Hooks) GetConsumerAdditionLegacyPropFromProp(
ctx sdk.Context,
proposalID uint64,
) (types.ConsumerAdditionProposal, bool) {
p, ok := h.k.govKeeper.GetProposal(ctx, proposalID)
if !ok {
panic(fmt.Errorf("failed to get proposal %d from store", proposalID))
}

// Iterate over the messages in the proposal
// Note that only ONE message can contain a consumer addition proposal
for _, msg := range p.GetMessages() {
sdkMsg, isLegacyProposal := msg.GetCachedValue().(*v1.MsgExecLegacyContent)
if !isLegacyProposal {
continue
}

content, err := v1.LegacyContentFromMessage(sdkMsg)
if err != nil {
panic(fmt.Errorf("failed to get legacy proposal %d from prop message", proposalID))
}

// returns if legacy prop is of ConsumerAddition proposal type
prop, ok := content.(*types.ConsumerAdditionProposal)
if ok {
return *prop, true
}
}
return types.ConsumerAdditionProposal{}, false
}
168 changes: 125 additions & 43 deletions x/ccv/provider/keeper/hooks_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,23 +1,24 @@
package keeper_test

import (
"fmt"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"

"testing"
"time"

authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
clienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types"
"cosmossdk.io/math"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"

"github.com/golang/mock/gomock"
"github.com/stretchr/testify/require"

sdk "github.com/cosmos/cosmos-sdk/types"
v1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1"
"github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1"

cryptotestutil "github.com/cosmos/interchain-security/v3/testutil/crypto"
testkeeper "github.com/cosmos/interchain-security/v3/testutil/keeper"
providerkeeper "github.com/cosmos/interchain-security/v3/x/ccv/provider/keeper"
"github.com/cosmos/interchain-security/v3/x/ccv/provider/types"
ccvtypes "github.com/cosmos/interchain-security/v3/x/ccv/types"
)

func TestValidatorConsensusKeyInUse(t *testing.T) {
Expand Down Expand Up @@ -90,60 +91,141 @@ func TestValidatorConsensusKeyInUse(t *testing.T) {
}
}

func TestAfterProposalSubmission(t *testing.T) {
// encodingConfig := appparams.MakeTestEncodingConfig()
// v1beta1.RegisterInterfaces(encodingConfig.InterfaceRegistry)
// v1.RegisterInterfaces(encodingConfig.InterfaceRegistry)
// cdc := encodingConfig.Codec
func TestAfterSubmissionAndAfterProposalVotingPeriodEnded(t *testing.T) {
acct := cryptotestutil.NewCryptoIdentityFromIntSeed(0)

k, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t))
defer ctrl.Finish()

newValidator := cryptotestutil.NewCryptoIdentityFromIntSeed(0)

content := types.NewConsumerAdditionProposal(
"chainID",
"description",
"chainID",
clienttypes.NewHeight(4, 5),
[]byte("gen_hash"),
[]byte("bin_hash"),
time.Now(),
ccvtypes.DefaultConsumerRedistributeFrac,
ccvtypes.DefaultBlocksPerDistributionTransmission,
"",
ccvtypes.DefaultHistoricalEntries,
ccvtypes.DefaultCCVTimeoutPeriod,
ccvtypes.DefaultTransferTimeoutPeriod,
ccvtypes.DefaultConsumerUnbondingPeriod,
propMsg, err := v1.NewLegacyContent(
testkeeper.GetTestConsumerAdditionProp(),
authtypes.NewModuleAddress("gov").String(),
)

propContent, err := v1.NewLegacyContent(content, authtypes.NewModuleAddress("gov").String())
require.NoError(t, err)

// _, err = v1.LegacyContentFromMessage(propContent)
// require.NoError(t, err)

prop, err := v1.NewProposal(
[]sdk.Msg{propContent},
[]sdk.Msg{propMsg},
0,
time.Now(),
time.Now(),
"",
"",
"",
// sdk.NewCoins(sdk.NewCoin("stake", math.OneInt())),
sdk.AccAddress(newValidator.SDKValOpAddress()),
sdk.AccAddress(acct.SDKValOpAddress()),
)

require.NoError(t, err)

k, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t))
defer ctrl.Finish()

// pass the prop to the mocked gov keeper,
// which is called by both the AfterProposalVotingPeriodEnded and
// AfterProposalSubmission gov hooks
gomock.InOrder(
mocks.MockGovKeeper.EXPECT().GetProposal(ctx, uint64(0)).Return(prop, true),
mocks.MockGovKeeper.EXPECT().GetProposal(ctx, prop.Id).Return(prop, true).Times(2),
)

tHooks := k.Hooks()
tHooks.AfterProposalSubmission(ctx, 0)
k.Hooks().AfterProposalSubmission(ctx, prop.Id)
// verify that the proposal ID is created
require.NotEmpty(t, k.GetProposedConsumerChain(ctx, prop.Id))

k.Hooks().AfterProposalVotingPeriodEnded(ctx, prop.Id)
// verify that the proposal ID is deleted
require.Empty(t, k.GetProposedConsumerChain(ctx, prop.Id))
}

func TestGetConsumerAdditionLegacyPropFromProp(t *testing.T) {
acct := cryptotestutil.NewCryptoIdentityFromIntSeed(0)
anotherAcct := cryptotestutil.NewCryptoIdentityFromIntSeed(1)

// create a dummy bank send message
dummyMsg := &banktypes.MsgSend{
FromAddress: sdk.AccAddress(acct.SDKValOpAddress()).String(),
ToAddress: sdk.AccAddress(anotherAcct.SDKValOpAddress()).String(),
Amount: sdk.NewCoins(sdk.NewCoin("stake", math.OneInt())),
}

testCases := map[string]struct {
propMsg func() sdk.Msg
// setup func(sdk.Context, k providerkeeper, proposalID uint64)
expPanic bool
expConsuAddProp bool
}{

"prop not found": {
propMsg: func() sdk.Msg {
return dummyMsg
},
expPanic: true,
},
"msgs in prop contain no legacy props": {
propMsg: func() sdk.Msg {
return dummyMsg
},
expConsuAddProp: false,
},
"msgs contain a legacy prop but not of ConsumerAdditionProposal type": {
propMsg: func() sdk.Msg {
textProp, err := v1.NewLegacyContent(
v1beta1.NewTextProposal("a title", "a legacy text prop"),
authtypes.NewModuleAddress("gov").String(),
)
require.NoError(t, err)

return textProp
},
expConsuAddProp: false,
},
"msg contains a prop of ConsumerAdditionProposal type - hook should create a new proposed chain": {
propMsg: func() sdk.Msg {
// create a dummy consumer addition prop
consuProp, err := v1.NewLegacyContent(
testkeeper.GetTestConsumerAdditionProp(),
authtypes.NewModuleAddress("gov").String(),
)
require.NoError(t, err)

return consuProp
},
expConsuAddProp: true,
},
}

fmt.Println(len(k.GetAllProposedConsumerChainIDs(ctx)))
for name, tc := range testCases {
t.Run(name, func(t *testing.T) {
k, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t))
defer ctrl.Finish()

// create a dummy prop for each test case
prop, err := v1.NewProposal(
[]sdk.Msg{tc.propMsg()},
0,
time.Now(),
time.Now(),
"",
"",
"",
sdk.AccAddress(acct.SDKValOpAddress()),
)
require.NoError(t, err)

if !tc.expPanic {
// pass the prop to the mocked gov keeper,
// which is called by the AfterProposalSubmission gov hook
gomock.InOrder(
mocks.MockGovKeeper.EXPECT().GetProposal(ctx, prop.Id).Return(prop, true),
)
} else {
gomock.InOrder(
mocks.MockGovKeeper.EXPECT().GetProposal(ctx, prop.Id).Return(v1.Proposal{}, false),
)
defer func() {
if r := recover(); r == nil {
require.Fail(t, r.(string))
}
}()
}

// retrieve consumer addition proposal
_, ok := k.Hooks().GetConsumerAdditionLegacyPropFromProp(ctx, prop.Id)
require.Equal(t, tc.expConsuAddProp, ok)
})
}
}

0 comments on commit b26b453

Please sign in to comment.