diff --git a/x/ccv/provider/keeper/msg_server.go b/x/ccv/provider/keeper/msg_server.go index 7cac362a04..c4afb9bfc3 100644 --- a/x/ccv/provider/keeper/msg_server.go +++ b/x/ccv/provider/keeper/msg_server.go @@ -188,25 +188,23 @@ func (k msgServer) OptIn(goCtx context.Context, msg *types.MsgOptIn) (*types.Msg } if msg.ConsumerKey != "" { - k.Keeper.HandleOptIn(ctx, msg.ChainId, providerAddr, &msg.ConsumerKey) - ctx.EventManager().EmitEvents(sdk.Events{ - sdk.NewEvent( - ccvtypes.EventTypeOptIn, - sdk.NewAttribute(types.AttributeProviderValidatorAddress, msg.ProviderAddr), - sdk.NewAttribute(types.AttributeConsumerConsensusPubKey, msg.ConsumerKey), - ), - }) + err = k.Keeper.HandleOptIn(ctx, msg.ChainId, providerAddr, &msg.ConsumerKey) } else { - k.Keeper.HandleOptIn(ctx, msg.ChainId, providerAddr, nil) + err = k.Keeper.HandleOptIn(ctx, msg.ChainId, providerAddr, nil) + } - ctx.EventManager().EmitEvents(sdk.Events{ - sdk.NewEvent( - ccvtypes.EventTypeOptIn, - sdk.NewAttribute(types.AttributeProviderValidatorAddress, msg.ProviderAddr), - ), - }) + if err != nil { + return nil, err } + ctx.EventManager().EmitEvents(sdk.Events{ + sdk.NewEvent( + ccvtypes.EventTypeOptIn, + sdk.NewAttribute(types.AttributeProviderValidatorAddress, msg.ProviderAddr), + sdk.NewAttribute(types.AttributeConsumerConsensusPubKey, msg.ConsumerKey), + ), + }) + return &types.MsgOptInResponse{}, nil } @@ -222,7 +220,10 @@ func (k msgServer) OptOut(goCtx context.Context, msg *types.MsgOptOut) (*types.M return nil, err } - k.Keeper.HandleOptOut(ctx, msg.ChainId, providerAddr) + err = k.Keeper.HandleOptOut(ctx, msg.ChainId, providerAddr) + if err != nil { + return nil, err + } ctx.EventManager().EmitEvents(sdk.Events{ sdk.NewEvent( diff --git a/x/ccv/provider/keeper/partial_set_security.go b/x/ccv/provider/keeper/partial_set_security.go index 1251eda372..2cfdaa7292 100644 --- a/x/ccv/provider/keeper/partial_set_security.go +++ b/x/ccv/provider/keeper/partial_set_security.go @@ -1,6 +1,7 @@ package keeper import ( + errorsmod "cosmossdk.io/errors" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/interchain-security/v4/x/ccv/provider/types" ) @@ -11,7 +12,13 @@ type OptedInValidator struct { BlockHeight uint64 } -func (k Keeper) HandleOptIn(ctx sdk.Context, chainID string, providerAddr types.ProviderConsAddress, consumerKey *string) { +func (k Keeper) HandleOptIn(ctx sdk.Context, chainID string, providerAddr types.ProviderConsAddress, consumerKey *string) error { + if !k.IsConsumerProposedOrRegistered(ctx, chainID) { + return errorsmod.Wrapf( + types.ErrUnknownConsumerChainId, + "opting in to an unknown consumer chain id: %s", chainID) + } + if k.IsToBeOptedOut(ctx, chainID, providerAddr) { // a validator to be opted in cancels out with a validator to be opted out k.DeleteToBeOptedOut(ctx, chainID, providerAddr) @@ -21,11 +28,19 @@ func (k Keeper) HandleOptIn(ctx sdk.Context, chainID string, providerAddr types. } if consumerKey != nil { - // TODO: assign consumer key in this case + // TODO (PR 1586): assign consumer key in this case } + + return nil } -func (k Keeper) HandleOptOut(ctx sdk.Context, chainID string, providerAddr types.ProviderConsAddress) { +func (k Keeper) HandleOptOut(ctx sdk.Context, chainID string, providerAddr types.ProviderConsAddress) error { + if !k.IsConsumerProposedOrRegistered(ctx, chainID) { + return errorsmod.Wrapf( + types.ErrUnknownConsumerChainId, + "opting out of an unknown consumer chain id: %s", chainID) + } + if k.IsToBeOptedIn(ctx, chainID, providerAddr) { // a validator to be opted out cancels out a validator to be opted in k.DeleteToBeOptedIn(ctx, chainID, providerAddr) @@ -33,4 +48,6 @@ func (k Keeper) HandleOptOut(ctx sdk.Context, chainID string, providerAddr types // a validator can only be set for opt out if it is opted in and not already set for opt out k.SetToBeOptedOut(ctx, chainID, providerAddr) } + + return nil } diff --git a/x/ccv/provider/keeper/partial_set_security_test.go b/x/ccv/provider/keeper/partial_set_security_test.go index 34e99f61f9..6321c5a7d6 100644 --- a/x/ccv/provider/keeper/partial_set_security_test.go +++ b/x/ccv/provider/keeper/partial_set_security_test.go @@ -13,9 +13,13 @@ func TestHandleOptIn(t *testing.T) { providerAddr := types.NewProviderConsAddress([]byte("providerAddr")) + // trying to opt in to a non-proposed and non-registered chain returns an error + require.Error(t, providerKeeper.HandleOptIn(ctx, "unknownChainID", providerAddr, nil)) + // if validator (`providerAddr`) is to be opted out, then we cancel that the validator is about // to be opted out and do not consider the validator to opt in providerKeeper.SetToBeOptedOut(ctx, "chainID", providerAddr) + providerKeeper.SetProposedConsumerChain(ctx, "chainID", 1) require.True(t, providerKeeper.IsToBeOptedOut(ctx, "chainID", providerAddr)) providerKeeper.HandleOptIn(ctx, "chainID", providerAddr, nil) require.False(t, providerKeeper.IsToBeOptedIn(ctx, "chainID", providerAddr)) @@ -33,16 +37,22 @@ func TestHandleOptOut(t *testing.T) { providerAddr := types.NewProviderConsAddress([]byte("providerAddr")) + // trying to opt out from a non-proposed and non-registered chain returns an error + require.Error(t, providerKeeper.HandleOptOut(ctx, "unknownChainID", providerAddr)) + // if validator (`providerAddr`) is to be opted in, then we cancel that the validator is about // to be opted out and do not consider the validator to opt out providerKeeper.SetToBeOptedIn(ctx, "chainID", providerAddr) + providerKeeper.SetProposedConsumerChain(ctx, "chainID", 1) require.True(t, providerKeeper.IsToBeOptedIn(ctx, "chainID", providerAddr)) - providerKeeper.HandleOptOut(ctx, "chainID", providerAddr) + err := providerKeeper.HandleOptOut(ctx, "chainID", providerAddr) + require.NoError(t, err) require.False(t, providerKeeper.IsToBeOptedOut(ctx, "chainID", providerAddr)) require.False(t, providerKeeper.IsToBeOptedIn(ctx, "chainID", providerAddr)) // if validator (`providerAddr`) is not opted in, then the validator cannot be opted out providerKeeper.DeleteOptedIn(ctx, "chainID", providerAddr) - providerKeeper.HandleOptOut(ctx, "chainID", providerAddr) + err = providerKeeper.HandleOptOut(ctx, "chainID", providerAddr) + require.NoError(t, err) require.False(t, providerKeeper.IsToBeOptedOut(ctx, "chainID", providerAddr)) }