diff --git a/x/ccv/provider/keeper/proposal.go b/x/ccv/provider/keeper/proposal.go index b235476260..931b4477ef 100644 --- a/x/ccv/provider/keeper/proposal.go +++ b/x/ccv/provider/keeper/proposal.go @@ -20,20 +20,9 @@ import ( ) // HandleConsumerAdditionProposal will receive the consumer chain's client state from the proposal. -// If the spawn time has already passed, then set the consumer chain. Otherwise store the client -// as a pending client, and set once spawn time has passed. -// -// Note: This method implements SpawnConsumerChainProposalHandler in spec. -// See: https://github.com/cosmos/ibc/blob/main/spec/app/ics-028-cross-chain-validation/methods.md#ccv-pcf-spccprop1 -// Spec tag: [CCV-PCF-SPCCPROP.1] +// The proposal is stored into state and if spawn time has passed the client will be created set in the next BeginBlocker. func (k Keeper) HandleConsumerAdditionProposal(ctx sdk.Context, p *types.ConsumerAdditionProposal) error { - if !ctx.BlockTime().Before(p.SpawnTime) { - // lockUbdOnTimeout is set to be false, regardless of what the proposal says, until we can specify and test issues around this use case more thoroughly - return k.CreateConsumerClient(ctx, p.ChainId, p.InitialHeight, false) - } - - err := k.SetPendingConsumerAdditionProp(ctx, p) - if err != nil { + if err := k.SetPendingConsumerAdditionProp(ctx, p); err != nil { return err } @@ -99,16 +88,8 @@ func (k Keeper) CreateConsumerClient(ctx sdk.Context, chainID string, // HandleConsumerRemovalProposal stops a consumer chain and released the outstanding unbonding operations. // If the stop time hasn't already passed, it stores the proposal as a pending proposal. -// -// This method implements StopConsumerChainProposalHandler from spec. -// See: https://github.com/cosmos/ibc/blob/main/spec/app/ics-028-cross-chain-validation/methods.md#ccv-pcf-stccprop1 -// Spec tag: [CCV-PCF-STCCPROP.1] +// If the stop time has passed the proposal will be executed in BeginBlocker. func (k Keeper) HandleConsumerRemovalProposal(ctx sdk.Context, p *types.ConsumerRemovalProposal) error { - - if !ctx.BlockTime().Before(p.StopTime) { - return k.StopConsumerChain(ctx, p.ChainId, false, true) - } - k.SetPendingConsumerRemovalProp(ctx, p.ChainId, p.StopTime) return nil } diff --git a/x/ccv/provider/keeper/proposal_test.go b/x/ccv/provider/keeper/proposal_test.go index 7f6ad1058e..f54bd0a97e 100644 --- a/x/ccv/provider/keeper/proposal_test.go +++ b/x/ccv/provider/keeper/proposal_test.go @@ -29,6 +29,8 @@ import ( // Tests the HandleConsumerAdditionProposal method against the SpawnConsumerChainProposalHandler spec. // See: https://github.com/cosmos/ibc/blob/main/spec/app/ics-028-cross-chain-validation/methods.md#ccv-pcf-spccprop1 // Spec tag: [CCV-PCF-SPCCPROP.1] +// +// NOTE: immediate handling of proposals is not necessary - all proposals are executed in BeginBlock. func TestHandleConsumerAdditionProposal(t *testing.T) { type testCase struct { @@ -36,8 +38,6 @@ func TestHandleConsumerAdditionProposal(t *testing.T) { prop *providertypes.ConsumerAdditionProposal // Time when prop is handled blockTime time.Time - // Whether it's expected that the spawn time has passed and client should be created - expCreatedClient bool } // Snapshot times asserted in tests @@ -46,7 +46,7 @@ func TestHandleConsumerAdditionProposal(t *testing.T) { tests := []testCase{ { - description: "ctx block time is after proposal's spawn time, expected that client is created", + description: "ctx block time is after proposal's spawn time, expected that client is persisted as pending - execution is in BeginBlocker", prop: providertypes.NewConsumerAdditionProposal( "title", "description", @@ -56,8 +56,7 @@ func TestHandleConsumerAdditionProposal(t *testing.T) { []byte("bin_hash"), now, // Spawn time ).(*providertypes.ConsumerAdditionProposal), - blockTime: hourFromNow, - expCreatedClient: true, + blockTime: hourFromNow, }, { description: `ctx block time is before proposal's spawn time, @@ -71,41 +70,29 @@ func TestHandleConsumerAdditionProposal(t *testing.T) { []byte("bin_hash"), hourFromNow, // Spawn time ).(*types.ConsumerAdditionProposal), - blockTime: now, - expCreatedClient: false, + blockTime: now, }, } for _, tc := range tests { // Common setup keeperParams := testkeeper.NewInMemKeeperParams(t) - providerKeeper, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, keeperParams) + providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, keeperParams) providerKeeper.SetParams(ctx, providertypes.DefaultParams()) ctx = ctx.WithBlockTime(tc.blockTime) - if tc.expCreatedClient { - // Mock calls are only asserted if we expect a client to be created. - gomock.InOrder( - testkeeper.GetMocksForCreateConsumerClient(ctx, &mocks, "chainID", clienttypes.NewHeight(2, 3))..., - ) - } - tc.prop.LockUnbondingOnTimeout = false // Full functionality not implemented yet. err := providerKeeper.HandleConsumerAdditionProposal(ctx, tc.prop) require.NoError(t, err) - if tc.expCreatedClient { - testCreatedConsumerClient(t, ctx, providerKeeper, tc.prop.ChainId, "clientID") - } else { - // check that stored pending prop is exactly the same as the initially instantiated prop - gotProposal, found := providerKeeper.GetPendingConsumerAdditionProp(ctx, tc.prop.SpawnTime, tc.prop.ChainId) - require.True(t, found) - require.Equal(t, *tc.prop, gotProposal) - // double check that a client for this chain does not exist - _, found = providerKeeper.GetConsumerClientId(ctx, tc.prop.ChainId) - require.False(t, found) - } + // check that stored pending prop is exactly the same as the initially instantiated prop + gotProposal, found := providerKeeper.GetPendingConsumerAdditionProp(ctx, tc.prop.SpawnTime, tc.prop.ChainId) + require.True(t, found) + require.Equal(t, *tc.prop, gotProposal) + // double check that a client for this chain does not exist + _, found = providerKeeper.GetConsumerClientId(ctx, tc.prop.ChainId) + require.False(t, found) ctrl.Finish() } } @@ -311,6 +298,8 @@ func TestPendingConsumerAdditionPropOrder(t *testing.T) { // // See: https://github.com/cosmos/ibc/blob/main/spec/app/ics-028-cross-chain-validation/methods.md#ccv-pcf-stccprop1 // Spec tag: [CCV-PCF-STCCPROP.1] +// +// NOTE: immediate handling of proposals is not necessary - all proposals are executed in BeginBlock. func TestHandleConsumerRemovalProposal(t *testing.T) { type testCase struct { @@ -319,8 +308,6 @@ func TestHandleConsumerRemovalProposal(t *testing.T) { prop *types.ConsumerRemovalProposal // Time when prop is handled blockTime time.Time - // Whether consumer chain should have been stopped - expStop bool } // Snapshot times asserted in tests @@ -337,7 +324,6 @@ func TestHandleConsumerRemovalProposal(t *testing.T) { now, ).(*providertypes.ConsumerRemovalProposal), blockTime: hourFromNow, // After stop time. - expStop: true, }, { description: "valid proposal: stop time has not yet been reached", @@ -348,7 +334,6 @@ func TestHandleConsumerRemovalProposal(t *testing.T) { hourFromNow, ).(*providertypes.ConsumerRemovalProposal), blockTime: now, // Before proposal's stop time - expStop: false, }, } @@ -356,31 +341,16 @@ func TestHandleConsumerRemovalProposal(t *testing.T) { // Common setup keeperParams := testkeeper.NewInMemKeeperParams(t) - providerKeeper, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, keeperParams) + providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, keeperParams) providerKeeper.SetParams(ctx, providertypes.DefaultParams()) ctx = ctx.WithBlockTime(tc.blockTime) - // Mock expectations and setup for stopping the consumer chain, if applicable - if tc.expStop { - testkeeper.SetupForStoppingConsumerChain(t, ctx, &providerKeeper, mocks) - } - // Note: when expStop is false, no mocks are setup, - // meaning no external keeper methods are allowed to be called. - err := providerKeeper.HandleConsumerRemovalProposal(ctx, tc.prop) require.NoError(t, err) - if tc.expStop { - // Expect no pending proposal to exist - found := providerKeeper.GetPendingConsumerRemovalProp(ctx, tc.prop.ChainId, tc.prop.StopTime) - require.False(t, found) - - testProviderStateIsCleaned(t, ctx, providerKeeper, tc.prop.ChainId, "channelID") - } else { - // Proposal should be stored as pending - found := providerKeeper.GetPendingConsumerRemovalProp(ctx, tc.prop.ChainId, tc.prop.StopTime) - require.True(t, found) - } + // Proposal should be stored as pending + found := providerKeeper.GetPendingConsumerRemovalProp(ctx, tc.prop.ChainId, tc.prop.StopTime) + require.True(t, found) // Assert mock calls from setup function ctrl.Finish() diff --git a/x/ccv/provider/proposal_handler_test.go b/x/ccv/provider/proposal_handler_test.go index c5cd78760f..f4fb995e35 100644 --- a/x/ccv/provider/proposal_handler_test.go +++ b/x/ccv/provider/proposal_handler_test.go @@ -3,7 +3,6 @@ package provider_test import ( sdk "github.com/cosmos/cosmos-sdk/types" distributiontypes "github.com/cosmos/cosmos-sdk/x/distribution/types" - "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" clienttypes "github.com/cosmos/ibc-go/v3/modules/core/02-client/types" @@ -27,26 +26,25 @@ func TestConsumerChainProposalHandler(t *testing.T) { hourFromNow := now.Add(time.Hour).UTC() testCases := []struct { - name string - content govtypes.Content - blockTime time.Time - expValidConsumerAddition bool - expValidConsumerRemoval bool + name string + content govtypes.Content + blockTime time.Time + expValidProposal bool }{ { name: "valid consumer addition proposal", content: types.NewConsumerAdditionProposal( "title", "description", "chainID", clienttypes.NewHeight(2, 3), []byte("gen_hash"), []byte("bin_hash"), now), - blockTime: hourFromNow, // ctx blocktime is after proposal's spawn time - expValidConsumerAddition: true, + blockTime: hourFromNow, // ctx blocktime is after proposal's spawn time + expValidProposal: true, }, { name: "valid consumer removal proposal", content: types.NewConsumerRemovalProposal( "title", "description", "chainID", now), - blockTime: hourFromNow, - expValidConsumerRemoval: true, + blockTime: hourFromNow, + expValidProposal: true, }, { name: "nil proposal", @@ -65,23 +63,15 @@ func TestConsumerChainProposalHandler(t *testing.T) { // Setup keeperParams := testkeeper.NewInMemKeeperParams(t) - providerKeeper, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, keeperParams) + providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, keeperParams) providerKeeper.SetParams(ctx, providertypes.DefaultParams()) ctx = ctx.WithBlockTime(tc.blockTime) - // Mock expectations depending on expected outcome - if tc.expValidConsumerAddition { - gomock.InOrder(testkeeper.GetMocksForCreateConsumerClient(ctx, &mocks, "chainID", clienttypes.NewHeight(2, 3))...) - } - if tc.expValidConsumerRemoval { - testkeeper.SetupForStoppingConsumerChain(t, ctx, &providerKeeper, mocks) - } - // Execution proposalHandler := provider.NewConsumerChainProposalHandler(providerKeeper) err := proposalHandler(ctx, tc.content) - if tc.expValidConsumerAddition || tc.expValidConsumerRemoval { + if tc.expValidProposal { require.NoError(t, err) } else { require.Error(t, err)