From dba67e6985a1578ad91897c5903ad2a4e12a912f Mon Sep 17 00:00:00 2001 From: mpoke Date: Fri, 30 Aug 2024 19:47:34 +0200 Subject: [PATCH] remove deprecated govv1beta1 validation --- x/ccv/provider/types/keys_test.go | 3 +- x/ccv/provider/types/legacy_proposal.go | 152 +---- x/ccv/provider/types/legacy_proposal_test.go | 557 ------------------- 3 files changed, 6 insertions(+), 706 deletions(-) delete mode 100644 x/ccv/provider/types/legacy_proposal_test.go diff --git a/x/ccv/provider/types/keys_test.go b/x/ccv/provider/types/keys_test.go index 7f475e0b14..741b036c78 100644 --- 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 index 3124cc3d87..9cc6ef2a77 100644 --- 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") } diff --git a/x/ccv/provider/types/legacy_proposal_test.go b/x/ccv/provider/types/legacy_proposal_test.go deleted file mode 100644 index 21f7a2da8a..0000000000 --- a/x/ccv/provider/types/legacy_proposal_test.go +++ /dev/null @@ -1,557 +0,0 @@ -package types_test - -import ( - fmt "fmt" - "testing" - "time" - - clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" - ibctmtypes "github.com/cosmos/ibc-go/v8/modules/light-clients/07-tendermint" - "github.com/golang/protobuf/proto" //nolint:staticcheck // see: https://github.com/cosmos/interchain-security/issues/236 - "github.com/stretchr/testify/require" - - "github.com/cosmos/cosmos-sdk/codec" - codectypes "github.com/cosmos/cosmos-sdk/codec/types" - govv1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1" - govv1beta1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1" - - "github.com/cosmos/interchain-security/v5/x/ccv/provider/types" -) - -func TestConsumerAdditionProposalValidateBasic(t *testing.T) { - initialHeight := clienttypes.NewHeight(2, 3) - - testCases := []struct { - name string - proposal govv1beta1.Content - expPass bool - }{ - { - "success", - types.NewConsumerAdditionProposal("title", "description", "chainID", initialHeight, []byte("gen_hash"), []byte("bin_hash"), time.Now(), - "0.75", - 10, - "", - 10000, - 100000000000, - 100000000000, - 100000000000, - 0, - 0, - 0, - nil, - nil, - 0, - false, - ), - true, - }, - { - "success with 0.0 fraction", - types.NewConsumerAdditionProposal("title", "description", "chainID", initialHeight, []byte("gen_hash"), []byte("bin_hash"), time.Now(), - "0.0", // fraction can be 0.0 but not empty - 10, - "", - 10000, - 100000000000, - 100000000000, - 100000000000, - 0, - 0, - 0, - nil, - nil, - 0, - false, - ), - true, - }, - { - "fails validate abstract - empty title", - types.NewConsumerAdditionProposal(" ", "description", "chainID", initialHeight, []byte("gen_hash"), []byte("bin_hash"), time.Now(), - "0.75", - 10, - "", - 10000, - 100000000000, - 100000000000, - 100000000000, - 0, - 0, - 0, - nil, - nil, - 0, - false, - ), - false, - }, - { - "chainID is empty", - types.NewConsumerAdditionProposal("title", "description", " ", initialHeight, []byte("gen_hash"), []byte("bin_hash"), time.Now(), - "0.75", - 10, - "", - 10000, - 100000000000, - 100000000000, - 100000000000, - 0, - 0, - 0, - nil, - nil, - 0, - false, - ), - false, - }, - { - "initial height is zero", - &types.ConsumerAdditionProposal{ - Title: "title", - Description: "description", - ChainId: "chainID", - InitialHeight: clienttypes.Height{}, - GenesisHash: []byte("gen_hash"), - BinaryHash: []byte("bin_hash"), - SpawnTime: time.Now(), - BlocksPerDistributionTransmission: 10, - CcvTimeoutPeriod: 100000000000, - TransferTimeoutPeriod: 100000000000, - ConsumerRedistributionFraction: "0.75", - DistributionTransmissionChannel: "", - HistoricalEntries: 10000, - UnbondingPeriod: 100000000000, - }, - false, - }, - { - "genesis hash is empty", - types.NewConsumerAdditionProposal("title", "description", "chainID", initialHeight, []byte(""), []byte("bin_hash"), time.Now(), - "0.75", - 10, - "", - 10000, - 100000000000, - 100000000000, - 100000000000, - 0, - 0, - 0, - nil, - nil, - 0, - false, - ), - false, - }, - { - "binary hash is empty", - types.NewConsumerAdditionProposal("title", "description", "chainID", initialHeight, []byte("gen_hash"), []byte(""), time.Now(), - "0.75", - 10, - "", - 10000, - 100000000000, - 100000000000, - 100000000000, - 0, - 0, - 0, - nil, - nil, - 0, - false, - ), - false, - }, - { - "spawn time is zero", - types.NewConsumerAdditionProposal("title", "description", "chainID", initialHeight, []byte("gen_hash"), []byte("bin_hash"), time.Time{}, - "0.75", - 10, - "", - 10000, - 100000000000, - 100000000000, - 100000000000, - 0, - 0, - 0, - nil, - nil, - 0, - false, - ), - false, - }, - { - "consumer redistribution fraction is invalid", - types.NewConsumerAdditionProposal("title", "description", "chainID", initialHeight, []byte("gen_hash"), []byte("bin_hash"), time.Now(), - "", // fraction can be 0.0 but not empty - 10, - "", - 10000, - 100000000000, - 100000000000, - 100000000000, - 0, - 0, - 0, - nil, - nil, - 0, - false, - ), - false, - }, - { - "blocks per distribution transmission is invalid", - types.NewConsumerAdditionProposal("title", "description", "chainID", initialHeight, []byte("gen_hash"), []byte("bin_hash"), time.Now(), - "0.75", - 0, - "", - 100000000000, - 10000, - 100000000000, - 100000000000, - 0, - 0, - 0, - nil, - nil, - 0, - false, - ), - false, - }, - { - "distribution transmission channel is invalid", - types.NewConsumerAdditionProposal("title", "description", "chainID", initialHeight, []byte("gen_hash"), []byte("bin_hash"), time.Now(), - "0.75", - 10, - "badchannel/", - 10000, - 100000000000, - 100000000000, - 100000000000, - 0, - 0, - 0, - nil, - nil, - 0, - false, - ), - false, - }, - { - "historical entries is invalid", - types.NewConsumerAdditionProposal("title", "description", "chainID", initialHeight, []byte("gen_hash"), []byte("bin_hash"), time.Now(), - "0.75", - 10, - "", - -2, - 100000000000, - 100000000000, - 100000000000, - 0, - 0, - 0, - nil, - nil, - 0, - false, - ), - false, - }, - { - "ccv timeout period is invalid", - types.NewConsumerAdditionProposal("title", "description", "chainID", initialHeight, []byte("gen_hash"), []byte("bin_hash"), time.Now(), - "0.75", - 10, - "", - 10000, - 0, - 100000000000, - 100000000000, - 0, - 0, - 0, - nil, - nil, - 0, - false, - ), - false, - }, - { - "transfer timeout period is invalid", - types.NewConsumerAdditionProposal("title", "description", "chainID", initialHeight, []byte("gen_hash"), []byte("bin_hash"), time.Now(), - "0.75", - 10, - "", - 10000, - 100000000000, - 0, - 100000000000, - 0, - 0, - 0, - nil, - nil, - 0, - false, - ), - false, - }, - { - "unbonding period is invalid", - types.NewConsumerAdditionProposal("title", "description", "chainID", initialHeight, []byte("gen_hash"), []byte("bin_hash"), time.Now(), - "0.75", - 10, - "", - 10000, - 100000000000, - 100000000000, - 0, - 0, - 0, - 0, - nil, - nil, - 0, - false, - ), - false, - }, - { - "top N is invalid", - types.NewConsumerAdditionProposal("title", "description", "chainID", initialHeight, []byte("gen_hash"), []byte("bin_hash"), time.Now(), - "0.75", - 10, - "", - 10000, - 100000000000, - 100000000000, - 100000000000, - 10, - 0, - 0, - nil, - nil, - 0, - false, - ), - false, - }, - { - "validators power cap is invalid", - types.NewConsumerAdditionProposal("title", "description", "chainID", initialHeight, []byte("gen_hash"), []byte("bin_hash"), time.Now(), - "0.75", - 10, - "", - 10000, - 100000000000, - 100000000000, - 100000000000, - 50, - 101, - 0, - nil, - nil, - 0, - false, - ), - false, - }, - { - "valid proposal with PSS features", - types.NewConsumerAdditionProposal("title", "description", "chainID", initialHeight, []byte("gen_hash"), []byte("bin_hash"), time.Now(), - "0.75", - 10, - "", - 10000, - 100000000000, - 100000000000, - 100000000000, - 0, - 34, - 101, - []string{"addr1"}, - []string{"addr2", "addr3"}, - 0, - false, - ), - true, - }, - } - - for _, tc := range testCases { - - err := tc.proposal.ValidateBasic() - if tc.expPass { - require.NoError(t, err, "valid case: %s should not return error. got %w", tc.name, err) - } else { - require.Error(t, err, "invalid case: '%s' must return error but got none", tc.name) - } - } -} - -func TestMarshalConsumerAdditionProposal(t *testing.T) { - content := types.NewConsumerAdditionProposal("title", "description", "chainID", clienttypes.NewHeight(0, 1), []byte("gen_hash"), []byte("bin_hash"), time.Now().UTC(), - "0.75", - 10, - "", - 10000, - 100000000000, - 100000000000, - 100000000000, - 0, - 0, - 0, - nil, - nil, - 0, - false, - ) - - cccp, ok := content.(*types.ConsumerAdditionProposal) - require.True(t, ok) - - // create codec - ir := codectypes.NewInterfaceRegistry() - types.RegisterInterfaces(ir) - govv1.RegisterInterfaces(ir) - clienttypes.RegisterInterfaces(ir) - ibctmtypes.RegisterInterfaces(ir) - cdc := codec.NewProtoCodec(ir) - - // marshal proposal - bz, err := cdc.MarshalJSON(cccp) - require.NoError(t, err) - - // unmarshal proposal - newCccp := &types.ConsumerAdditionProposal{} - err = cdc.UnmarshalJSON(bz, newCccp) - require.NoError(t, err) - - require.True(t, proto.Equal(cccp, newCccp), "unmarshalled proposal does not equal original proposal") -} - -func TestConsumerAdditionProposalString(t *testing.T) { - initialHeight := clienttypes.NewHeight(2, 3) - spawnTime := time.Now() - proposal := types.NewConsumerAdditionProposal( - "title", - "description", - "chainID", - initialHeight, - []byte("gen_hash"), - []byte("bin_hash"), - spawnTime, - "0.75", - 10001, - "", - 500000, - 100000000000, - 10000000000, - 100000000000, - 0, - 0, - 0, - []string{}, - []string{}, - 0, - false, - ) - - expect := fmt.Sprintf(`CreateConsumerChain Proposal - Title: title - Description: description - ChainID: chainID - InitialHeight: %s - GenesisHash: %s - BinaryHash: %s - SpawnTime: %s - ConsumerRedistributionFraction: %s - BlocksPerDistributionTransmission: %d - DistributionTransmissionChannel: %s - HistoricalEntries: %d - CcvTimeoutPeriod: %d - TransferTimeoutPeriod: %d - UnbondingPeriod: %d`, initialHeight, []byte("gen_hash"), []byte("bin_hash"), spawnTime, - "0.75", - 10001, - "", - 500000, - 100000000000, - 10000000000, - 100000000000) - - require.Equal(t, expect, proposal.String(), "string method for ConsumerAdditionProposal returned unexpected string") -} - -func TestChangeRewardDenomsProposalValidateBasic(t *testing.T) { - tcs := []struct { - name string - proposal govv1beta1.Content - expectError bool - expectPanic bool - }{ - { - name: "invalid change reward denoms proposal, none to add or remove", - proposal: types.NewChangeRewardDenomsProposal( - "title", "description", []string{}, []string{}), - expectError: true, - }, - { - name: "invalid change reward denoms proposal, same denom in both sets", - proposal: types.NewChangeRewardDenomsProposal( - "title", "description", []string{"denom1"}, []string{"denom1"}), - expectError: true, - }, - { - name: "valid change reward denoms proposal", - proposal: types.NewChangeRewardDenomsProposal( - "title", "description", []string{"denom1"}, []string{"denom2"}), - expectError: false, - }, - { - name: "invalid prop, invalid denom, will panic", - proposal: types.NewChangeRewardDenomsProposal( - "title", "description", []string{"!@blah"}, []string{"denom2"}), - expectPanic: true, - }, - } - - for _, tc := range tcs { - t.Run(tc.name, func(t *testing.T) { - if tc.expectPanic { - require.Panics(t, func() { tc.proposal.ValidateBasic() }) - return - } - err := tc.proposal.ValidateBasic() - if tc.expectError { - require.Error(t, err) - return - } - require.NoError(t, err) - }) - } -} - -func TestValidatePSSFeatures(t *testing.T) { - require.NoError(t, types.ValidatePSSFeatures(0, 0)) - require.NoError(t, types.ValidatePSSFeatures(50, 0)) - require.NoError(t, types.ValidatePSSFeatures(100, 0)) - require.NoError(t, types.ValidatePSSFeatures(0, 10)) - require.NoError(t, types.ValidatePSSFeatures(0, 100)) - require.NoError(t, types.ValidatePSSFeatures(50, 100)) - - require.Error(t, types.ValidatePSSFeatures(10, 0)) - require.Error(t, types.ValidatePSSFeatures(49, 0)) - require.Error(t, types.ValidatePSSFeatures(101, 0)) - require.Error(t, types.ValidatePSSFeatures(50, 101)) -}