fix!: provider msg validation (#2197)

* validate MsgAssignConsumerKey * validate MsgSubmitConsumerMisbehaviour and MsgSubmitConsumerDoubleVoting * move RemoveConsumer * refactor: rearrange messages in msg.go * validate MsgOptIn and MsgOptOut * validate MsgSetConsumerCommissionRate * remove ValidateBasic for deprecated msgs * remove deprecated GetSigners * remove deprecated GetSignBytes * remove deprecated govv1beta1 validation * remove Route and Type * Update x/ccv/provider/keeper/msg_server.go Co-authored-by: insumity <[email protected]> * apply review suggestions * apply review suggestions --------- Co-authored-by: insumity <[email protected]>
cosmos · Sep 2, 2024 · e1b1dc2 · e1b1dc2
1 parent 6262e42
commit e1b1dc2
Show file tree

Hide file tree

Showing 6 changed files with 246 additions and 1,249 deletions.
diff --git a/x/ccv/provider/keeper/msg_server.go b/x/ccv/provider/keeper/msg_server.go
@@ -87,45 +87,6 @@ func (k msgServer) AssignConsumerKey(goCtx context.Context, msg *types.MsgAssign
 	return &types.MsgAssignConsumerKeyResponse{}, nil
 }
 
-// RemoveConsumer defines an RPC handler method for MsgRemoveConsumer
-func (k msgServer) RemoveConsumer(goCtx context.Context, msg *types.MsgRemoveConsumer) (*types.MsgRemoveConsumerResponse, error) {
-	ctx := sdk.UnwrapSDKContext(goCtx)
-
-	resp := types.MsgRemoveConsumerResponse{}
-
-	consumerId := msg.ConsumerId
-	ownerAddress, err := k.Keeper.GetConsumerOwnerAddress(ctx, consumerId)
-	if err != nil {
-		return &resp, errorsmod.Wrapf(types.ErrNoOwnerAddress, "cannot retrieve owner address %s", ownerAddress)
-	}
-
-	if msg.Signer != ownerAddress {
-		return &resp, errorsmod.Wrapf(types.ErrUnauthorized, "expected owner address %s, got %s", ownerAddress, msg.Signer)
-	}
-
-	phase := k.Keeper.GetConsumerPhase(ctx, consumerId)
-	if phase != types.ConsumerPhase_CONSUMER_PHASE_LAUNCHED {
-		return nil, errorsmod.Wrapf(types.ErrInvalidPhase,
-			"chain with consumer id: %s has to be in its launched phase", consumerId)
-	}
-
-	previousStopTime, err := k.Keeper.GetConsumerStopTime(ctx, consumerId)
-	if err == nil {
-		if err := k.Keeper.RemoveConsumerToBeStopped(ctx, consumerId, previousStopTime); err != nil {
-			return &resp, errorsmod.Wrapf(ccvtypes.ErrInvalidConsumerState, "cannot remove previous stop time: %s", err.Error())
-		}
-	}
-
-	if err := k.Keeper.SetConsumerStopTime(ctx, consumerId, msg.StopTime); err != nil {
-		return &resp, errorsmod.Wrapf(types.ErrInvalidStopTime, "cannot set stop time: %s", err.Error())
-	}
-	if err := k.Keeper.AppendConsumerToBeStopped(ctx, consumerId, msg.StopTime); err != nil {
-		return &resp, errorsmod.Wrapf(ccvtypes.ErrInvalidConsumerState, "cannot set consumer to be stop: %s", err.Error())
-	}
-
-	return &resp, nil
-}
-
 // ChangeRewardDenoms defines a rpc handler method for MsgChangeRewardDenoms
 func (k msgServer) ChangeRewardDenoms(goCtx context.Context, msg *types.MsgChangeRewardDenoms) (*types.MsgChangeRewardDenomsResponse, error) {
 	ctx := sdk.UnwrapSDKContext(goCtx)
@@ -495,6 +456,45 @@ func (k msgServer) UpdateConsumer(goCtx context.Context, msg *types.MsgUpdateCon
 	return &resp, nil
 }
 
+// RemoveConsumer defines an RPC handler method for MsgRemoveConsumer
+func (k msgServer) RemoveConsumer(goCtx context.Context, msg *types.MsgRemoveConsumer) (*types.MsgRemoveConsumerResponse, error) {
+	ctx := sdk.UnwrapSDKContext(goCtx)
+
+	resp := types.MsgRemoveConsumerResponse{}
+
+	consumerId := msg.ConsumerId
+	ownerAddress, err := k.Keeper.GetConsumerOwnerAddress(ctx, consumerId)
+	if err != nil {
+		return &resp, errorsmod.Wrapf(types.ErrNoOwnerAddress, "cannot retrieve owner address %s", ownerAddress)
+	}
+
+	if msg.Signer != ownerAddress {
+		return &resp, errorsmod.Wrapf(types.ErrUnauthorized, "expected owner address %s, got %s", ownerAddress, msg.Signer)
+	}
+
+	phase := k.Keeper.GetConsumerPhase(ctx, consumerId)
+	if phase != types.ConsumerPhase_CONSUMER_PHASE_LAUNCHED {
+		return nil, errorsmod.Wrapf(types.ErrInvalidPhase,
+			"chain with consumer id: %s has to be in its launched phase", consumerId)
+	}
+
+	previousStopTime, err := k.Keeper.GetConsumerStopTime(ctx, consumerId)
+	if err == nil {
+		if err := k.Keeper.RemoveConsumerToBeStopped(ctx, consumerId, previousStopTime); err != nil {
+			return &resp, errorsmod.Wrapf(ccvtypes.ErrInvalidConsumerState, "cannot remove previous stop time: %s", err.Error())
+		}
+	}
+
+	if err := k.Keeper.SetConsumerStopTime(ctx, consumerId, msg.StopTime); err != nil {
+		return &resp, errorsmod.Wrapf(types.ErrInvalidStopTime, "cannot set stop time: %s", err.Error())
+	}
+	if err := k.Keeper.AppendConsumerToBeStopped(ctx, consumerId, msg.StopTime); err != nil {
+		return &resp, errorsmod.Wrapf(ccvtypes.ErrInvalidConsumerState, "cannot set consumer to be stopped: %s", err.Error())
+	}
+
+	return &resp, nil
+}
+
 func (k msgServer) ConsumerAddition(_ context.Context, _ *types.MsgConsumerAddition) (*types.MsgConsumerAdditionResponse, error) {
 	return nil, fmt.Errorf("`MsgConsumerAddition` is deprecated. Use `MsgCreateConsumer`")
 }

diff --git a/x/ccv/provider/types/errors.go b/x/ccv/provider/types/errors.go
@@ -10,34 +10,22 @@ var (
 	ErrInvalidConsumerRemovalProp              = errorsmod.Register(ModuleName, 2, "invalid consumer removal proposal")
 	ErrUnknownConsumerId                       = errorsmod.Register(ModuleName, 3, "no consumer chain with this consumer id")
 	ErrUnknownConsumerChannelId                = errorsmod.Register(ModuleName, 4, "no consumer chain with this channel id")
-	ErrInvalidConsumerConsensusPubKey          = errorsmod.Register(ModuleName, 5, "empty consumer consensus public key")
 	ErrInvalidConsumerId                       = errorsmod.Register(ModuleName, 6, "invalid consumer id")
-	ErrConsumerKeyNotFound                     = errorsmod.Register(ModuleName, 7, "consumer key not found")
-	ErrNoValidatorConsumerAddress              = errorsmod.Register(ModuleName, 8, "error getting validator consumer address")
-	ErrNoValidatorProviderAddress              = errorsmod.Register(ModuleName, 9, "error getting validator provider address")
 	ErrConsumerKeyInUse                        = errorsmod.Register(ModuleName, 10, "consumer key is already in use by a validator")
 	ErrCannotAssignDefaultKeyAssignment        = errorsmod.Register(ModuleName, 11, "cannot re-assign default key assignment")
-	ErrInvalidConsumerParams                   = errorsmod.Register(ModuleName, 12, "invalid consumer params")
-	ErrInvalidProviderAddress                  = errorsmod.Register(ModuleName, 13, "invalid provider address")
 	ErrInvalidConsumerRewardDenom              = errorsmod.Register(ModuleName, 14, "invalid consumer reward denom")
-	ErrInvalidDepositorAddress                 = errorsmod.Register(ModuleName, 15, "invalid depositor address")
 	ErrInvalidConsumerClient                   = errorsmod.Register(ModuleName, 16, "ccv channel is not built on correct client")
-	ErrDuplicateConsumerChain                  = errorsmod.Register(ModuleName, 17, "consumer chain already exists")
 	ErrConsumerChainNotFound                   = errorsmod.Register(ModuleName, 18, "consumer chain not found")
-	ErrInvalidConsumerCommissionRate           = errorsmod.Register(ModuleName, 19, "consumer commission rate is invalid")
 	ErrCannotOptOutFromTopN                    = errorsmod.Register(ModuleName, 20, "cannot opt out from a Top N chain")
-	ErrNoUnconfirmedVSCPacket                  = errorsmod.Register(ModuleName, 21, "no unconfirmed vsc packet for this chain id")
 	ErrInvalidConsumerModificationProposal     = errorsmod.Register(ModuleName, 22, "invalid consumer modification proposal")
 	ErrNoUnbondingTime                         = errorsmod.Register(ModuleName, 23, "provider unbonding time not found")
-	ErrInvalidAddress                          = errorsmod.Register(ModuleName, 24, "invalid address")
 	ErrUnauthorized                            = errorsmod.Register(ModuleName, 25, "unauthorized")
 	ErrBlankConsumerChainID                    = errorsmod.Register(ModuleName, 26, "consumer chain id must not be blank")
 	ErrInvalidPhase                            = errorsmod.Register(ModuleName, 27, "cannot perform action in the current phase of consumer chain")
 	ErrInvalidConsumerMetadata                 = errorsmod.Register(ModuleName, 28, "invalid consumer metadata")
 	ErrInvalidPowerShapingParameters           = errorsmod.Register(ModuleName, 29, "invalid power shaping parameters")
 	ErrInvalidConsumerInitializationParameters = errorsmod.Register(ModuleName, 30, "invalid consumer initialization parameters")
 	ErrCannotUpdateMinimumPowerInTopN          = errorsmod.Register(ModuleName, 31, "cannot update minimum power in Top N")
-	ErrNoChainId                               = errorsmod.Register(ModuleName, 32, "missing chain id for consumer chain")
 	ErrNoConsumerGenesis                       = errorsmod.Register(ModuleName, 33, "missing consumer genesis")
 	ErrInvalidConsumerGenesis                  = errorsmod.Register(ModuleName, 34, "invalid consumer genesis")
 	ErrNoConsumerId                            = errorsmod.Register(ModuleName, 35, "missing consumer id")
@@ -51,4 +39,11 @@ var (
 	ErrInvalidStopTime                         = errorsmod.Register(ModuleName, 43, "invalid stop time")
 	ErrInvalidMsgCreateConsumer                = errorsmod.Register(ModuleName, 44, "invalid create consumer message")
 	ErrInvalidMsgUpdateConsumer                = errorsmod.Register(ModuleName, 45, "invalid update consumer message")
+	ErrInvalidMsgAssignConsumerKey             = errorsmod.Register(ModuleName, 46, "invalid assign consumer key message")
+	ErrInvalidMsgSubmitConsumerMisbehaviour    = errorsmod.Register(ModuleName, 47, "invalid submit consumer misbehaviour message")
+	ErrInvalidMsgSubmitConsumerDoubleVoting    = errorsmod.Register(ModuleName, 48, "invalid submit consumer double voting message")
+	ErrInvalidMsgOptIn                         = errorsmod.Register(ModuleName, 49, "invalid opt in message")
+	ErrInvalidMsgOptOut                        = errorsmod.Register(ModuleName, 50, "invalid opt out message")
+	ErrInvalidMsgSetConsumerCommissionRate     = errorsmod.Register(ModuleName, 51, "invalid set consumer commission rate message")
+	ErrInvalidMsgChangeRewardDenoms            = errorsmod.Register(ModuleName, 52, "invalid change reward denoms message")
 )
diff --git a/x/ccv/provider/types/keys_test.go b/x/ccv/provider/types/keys_test.go
@@ -11,7 +11,6 @@ import (
 
 	cryptoutil "github.com/cosmos/interchain-security/v5/testutil/crypto"
 	providerkeeper "github.com/cosmos/interchain-security/v5/x/ccv/provider/keeper"
-	"github.com/cosmos/interchain-security/v5/x/ccv/provider/types"
 	providertypes "github.com/cosmos/interchain-security/v5/x/ccv/provider/types"
 )
 
@@ -201,7 +200,7 @@ func getAllFullyDefinedKeys() [][]byte {
 		providertypes.ConsumerCommissionRateKey("13", providertypes.NewProviderConsAddress([]byte{0x05})),
 		providertypes.MinimumPowerInTopNKey("13"),
 		providertypes.ConsumerAddrsToPruneV2Key("13", time.Time{}),
-		providerkeeper.GetValidatorKey(types.LastProviderConsensusValsPrefix(), providertypes.NewProviderConsAddress([]byte{0x05})),
+		providerkeeper.GetValidatorKey(providertypes.LastProviderConsensusValsPrefix(), providertypes.NewProviderConsAddress([]byte{0x05})),
 		providertypes.ConsumerIdKey(),
 		providertypes.ConsumerIdToChainIdKey("13"),
 		providertypes.ConsumerIdToOwnerAddressKey("13"),

diff --git a/x/ccv/provider/types/legacy_proposal.go b/x/ccv/provider/types/legacy_proposal.go
@@ -1,21 +1,13 @@
 package types
 
 import (
-	"errors"
 	"fmt"
-	"strings"
 	time "time"
 
 	clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types"
 
-	errorsmod "cosmossdk.io/errors"
-	"cosmossdk.io/math"
-
 	evidencetypes "cosmossdk.io/x/evidence/types"
-	sdk "github.com/cosmos/cosmos-sdk/types"
 	govv1beta1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1"
-
-	ccvtypes "github.com/cosmos/interchain-security/v5/x/ccv/types"
 )
 
 const (
@@ -100,79 +92,9 @@ func (cccp *ConsumerAdditionProposal) ProposalType() string {
 	return ProposalTypeConsumerAddition
 }
 
-// ValidatePSSFeatures returns an error if the `topN` and `validatorsPowerCap` parameters are no in the correct ranges
-func ValidatePSSFeatures(topN, validatorsPowerCap uint32) error {
-	// Top N corresponds to the top N% of validators that have to validate the consumer chain and can only be 0 (for an
-	// Opt In chain) or in the range [50, 100] (for a Top N chain).
-	if topN != 0 && (topN < 50 || topN > 100) {
-		return fmt.Errorf("Top N can either be 0 or in the range [50, 100]")
-	}
-
-	if validatorsPowerCap != 0 && validatorsPowerCap > 100 {
-		return fmt.Errorf("validators' power cap has to be in the range [1, 100]")
-	}
-
-	return nil
-}
-
 // ValidateBasic runs basic stateless validity checks
 func (cccp *ConsumerAdditionProposal) ValidateBasic() error {
-	if err := govv1beta1.ValidateAbstract(cccp); err != nil {
-		return err
-	}
-
-	if strings.TrimSpace(cccp.ChainId) == "" {
-		return errorsmod.Wrap(ErrInvalidConsumerAdditionProposal, "consumer chain id must not be blank")
-	}
-
-	if cccp.InitialHeight.IsZero() {
-		return errorsmod.Wrap(ErrInvalidConsumerAdditionProposal, "initial height cannot be zero")
-	}
-
-	if len(cccp.GenesisHash) == 0 {
-		return errorsmod.Wrap(ErrInvalidConsumerAdditionProposal, "genesis hash cannot be empty")
-	}
-	if len(cccp.BinaryHash) == 0 {
-		return errorsmod.Wrap(ErrInvalidConsumerAdditionProposal, "binary hash cannot be empty")
-	}
-
-	if cccp.SpawnTime.IsZero() {
-		return errorsmod.Wrap(ErrInvalidConsumerAdditionProposal, "spawn time cannot be zero")
-	}
-
-	if err := ccvtypes.ValidateStringFraction(cccp.ConsumerRedistributionFraction); err != nil {
-		return errorsmod.Wrapf(ErrInvalidConsumerAdditionProposal, "consumer redistribution fraction is invalid: %s", err)
-	}
-
-	if err := ccvtypes.ValidatePositiveInt64(cccp.BlocksPerDistributionTransmission); err != nil {
-		return errorsmod.Wrap(ErrInvalidConsumerAdditionProposal, "blocks per distribution transmission cannot be < 1")
-	}
-
-	if err := ccvtypes.ValidateDistributionTransmissionChannel(cccp.DistributionTransmissionChannel); err != nil {
-		return errorsmod.Wrap(ErrInvalidConsumerAdditionProposal, "distribution transmission channel")
-	}
-
-	if err := ccvtypes.ValidatePositiveInt64(cccp.HistoricalEntries); err != nil {
-		return errorsmod.Wrap(ErrInvalidConsumerAdditionProposal, "historical entries cannot be < 1")
-	}
-
-	if err := ccvtypes.ValidateDuration(cccp.CcvTimeoutPeriod); err != nil {
-		return errorsmod.Wrap(ErrInvalidConsumerAdditionProposal, "ccv timeout period cannot be zero")
-	}
-
-	if err := ccvtypes.ValidateDuration(cccp.TransferTimeoutPeriod); err != nil {
-		return errorsmod.Wrap(ErrInvalidConsumerAdditionProposal, "transfer timeout period cannot be zero")
-	}
-
-	if err := ccvtypes.ValidateDuration(cccp.UnbondingPeriod); err != nil {
-		return errorsmod.Wrap(ErrInvalidConsumerAdditionProposal, "unbonding period cannot be zero")
-	}
-
-	err := ValidatePSSFeatures(cccp.Top_N, cccp.ValidatorsPowerCap)
-	if err != nil {
-		return errorsmod.Wrapf(ErrInvalidConsumerAdditionProposal, "invalid PSS features: %s", err.Error())
-	}
-	return nil
+	return fmt.Errorf("ConsumerAdditionProposal is deprecated")
 }
 
 // String returns the string representation of the ConsumerAdditionProposal.
@@ -226,18 +148,7 @@ func (sccp *ConsumerRemovalProposal) ProposalType() string { return ProposalType
 
 // ValidateBasic runs basic stateless validity checks
 func (sccp *ConsumerRemovalProposal) ValidateBasic() error {
-	if err := govv1beta1.ValidateAbstract(sccp); err != nil {
-		return err
-	}
-
-	if strings.TrimSpace(sccp.ChainId) == "" {
-		return errorsmod.Wrap(ErrInvalidConsumerRemovalProp, "consumer chain id must not be blank")
-	}
-
-	if sccp.StopTime.IsZero() {
-		return errorsmod.Wrap(ErrInvalidConsumerRemovalProp, "spawn time cannot be zero")
-	}
-	return nil
+	return fmt.Errorf("ConsumerRemovalProposal is deprecated")
 }
 
 // NewConsumerModificationProposal creates a new consumer modification proposal.
@@ -274,19 +185,7 @@ func (cccp *ConsumerModificationProposal) ProposalType() string {
 
 // ValidateBasic runs basic stateless validity checks
 func (cccp *ConsumerModificationProposal) ValidateBasic() error {
-	if err := govv1beta1.ValidateAbstract(cccp); err != nil {
-		return err
-	}
-
-	if strings.TrimSpace(cccp.ChainId) == "" {
-		return errorsmod.Wrap(ErrInvalidConsumerModificationProposal, "consumer chain id must not be blank")
-	}
-
-	err := ValidatePSSFeatures(cccp.Top_N, cccp.ValidatorsPowerCap)
-	if err != nil {
-		return errorsmod.Wrapf(ErrInvalidConsumerModificationProposal, "invalid PSS features: %s", err.Error())
-	}
-	return nil
+	return fmt.Errorf("ConsumerModificationProposal is deprecated")
 }
 
 // NewEquivocationProposal creates a new equivocation proposal.
@@ -310,18 +209,7 @@ func (sp *EquivocationProposal) ProposalType() string {
 
 // ValidateBasic runs basic stateless validity checks
 func (sp *EquivocationProposal) ValidateBasic() error {
-	if err := govv1beta1.ValidateAbstract(sp); err != nil {
-		return err
-	}
-	if len(sp.Equivocations) == 0 {
-		return errors.New("invalid equivocation proposal: empty equivocations")
-	}
-	for i := 0; i < len(sp.Equivocations); i++ {
-		if err := sp.Equivocations[i].ValidateBasic(); err != nil {
-			return err
-		}
-	}
-	return nil
+	return fmt.Errorf("EquivocationProposal is deprecated")
 }
 
 func NewChangeRewardDenomsProposal(title, description string,
@@ -345,35 +233,5 @@ func (crdp *ChangeRewardDenomsProposal) ProposalType() string {
 
 // ValidateBasic runs basic stateless validity checks on a ChangeRewardDenomsProposal.
 func (crdp *ChangeRewardDenomsProposal) ValidateBasic() error {
-	emptyDenomsToAdd := len(crdp.DenomsToAdd) == 0
-	emptyDenomsToRemove := len(crdp.DenomsToRemove) == 0
-	// Return error if both sets are empty or nil
-	if emptyDenomsToAdd && emptyDenomsToRemove {
-		return fmt.Errorf(
-			"invalid change reward denoms proposal: both denoms to add and denoms to remove are empty")
-	}
-
-	// Return error if a denom is in both sets
-	for _, denomToAdd := range crdp.DenomsToAdd {
-		for _, denomToRemove := range crdp.DenomsToRemove {
-			if denomToAdd == denomToRemove {
-				return fmt.Errorf(
-					"invalid change reward denoms proposal: %s cannot be both added and removed", denomToAdd)
-			}
-		}
-	}
-
-	// Return error if any denom is "invalid"
-	for _, denom := range crdp.DenomsToAdd {
-		if !sdk.NewCoin(denom, math.NewInt(1)).IsValid() {
-			return fmt.Errorf("invalid change reward denoms proposal: %s is not a valid denom", denom)
-		}
-	}
-	for _, denom := range crdp.DenomsToRemove {
-		if !sdk.NewCoin(denom, math.NewInt(1)).IsValid() {
-			return fmt.Errorf("invalid change reward denoms proposal: %s is not a valid denom", denom)
-		}
-	}
-
-	return nil
+	return fmt.Errorf("ChangeRewardDenomsProposal is deprecated")
 }