fix(eibc,delaydack): add validation to eibc and delayedack genesis state

CHANGELOG.md

@@ -87,6 +87,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### Bug Fixes
 
+- (eibc,delayedack) [#942](https://github.com/dymensionxyz/dymension/issues/942) Add missing genesis validation
 - (rollapp) [#317](https://github.com/dymensionxyz/research/issues/317) Prevent overflow on rollapp state update
 - (code standards) [#932](https://github.com/dymensionxyz/dymension/issues/932) Dry out existing middlewares to make use of new .GetValidTransfer* functions which take care of parsing and validating the fungible packet, and querying and validating any associated rollapp and finalizations
 - (code standards) [#932](https://github.com/dymensionxyz/dymension/issues/932) Removes the obsolete ValidateRollappId func and sub routines

x/common/types/rollapp_packet.go

@@ -13,6 +13,30 @@ func (r RollappPacket) LogString() string {
 		r.RollappId, r.Type, r.Status, r.Packet.SourcePort, r.Packet.SourceChannel, r.Packet.Sequence, r.Error, r.ProofHeight)
 }
 
+func (r RollappPacket) ValidateBasic() error {
+	if r.RollappId == "" {
+		return fmt.Errorf("rollapp id cannot be empty")
+	}
+	if len(r.Relayer) == 0 {
+		return fmt.Errorf("status cannot be empty")
+	}
+	if r.OriginalTransferTarget != "" {
+		if _, err := sdk.AccAddressFromBech32(r.OriginalTransferTarget); err != nil {
+			return fmt.Errorf("original transfer target: %w", err)
+		}
+	}
+	if r.ProofHeight == 0 {
+		return fmt.Errorf("proof height revision height cannot be zero")
+	}
+	if r.Packet == nil {
+		return fmt.Errorf("packet cannot be nil")
+	}
+	if err := r.Packet.ValidateBasic(); err != nil {
+		return fmt.Errorf("packet: %w", err)
+	}
+	return nil
+}
+
 func (r RollappPacket) GetEvents() []sdk.Attribute {
 	eventAttributes := []sdk.Attribute{
 		sdk.NewAttribute(AttributeKeyRollappId, r.RollappId),

x/delayedack/types/errors.go

@@ -7,6 +7,7 @@ import (
 var (
 	ErrCanOnlyUpdatePendingPacket = errorsmod.Register(ModuleName, 1, "can only update pending packet")
 	ErrRollappPacketDoesNotExist  = errorsmod.Register(ModuleName, 2, "rollapp packet does not exist")
+	ErrRollappPacketAlreadyExists = errorsmod.Register(ModuleName, 3, "rollapp packet already exists")
 	ErrUnknownRequest             = errorsmod.Register(ModuleName, 8, "unknown request")
 	ErrBadEIBCFee                 = errorsmod.Register(ModuleName, 10, "provided eibc fee is invalid")
 )

x/delayedack/types/genesis.go

@@ -1,7 +1,6 @@
 package types
 
-// DefaultIndex is the default global index
-const DefaultIndex uint64 = 1
+import "github.com/dymensionxyz/dymension/v3/x/common/types"
 
 // DefaultGenesis returns the default genesis state
 func DefaultGenesis() *GenesisState {
@@ -13,5 +12,15 @@ func DefaultGenesis() *GenesisState {
 // Validate performs basic genesis state validation returning an error upon any
 // failure.
 func (gs GenesisState) Validate() error {
+	rollappPacketMap := make(map[string]struct{})
+	for _, rollappPacket := range gs.GetRollappPackets() {
+		if err := rollappPacket.ValidateBasic(); err != nil {
+			return err
+		}
+		if _, ok := rollappPacketMap[string(types.RollappPacketKey(&rollappPacket))]; ok {
+			return ErrRollappPacketAlreadyExists
+		}
+		rollappPacketMap[string(types.RollappPacketKey(&rollappPacket))] = struct{}{}
+	}
 	return gs.Params.Validate()
 }

x/delayedack/types/genesis_test.go

@@ -0,0 +1,87 @@
+package types_test
+
+import (
+	"testing"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	clienttypes "github.com/cosmos/ibc-go/v6/modules/core/02-client/types"
+	chantypes "github.com/cosmos/ibc-go/v6/modules/core/04-channel/types"
+	"github.com/stretchr/testify/require"
+
+	ctypes "github.com/dymensionxyz/dymension/v3/x/common/types"
+	"github.com/dymensionxyz/dymension/v3/x/delayedack/types"
+)
+
+func TestGenesisState_Validate(t *testing.T) {
+	for _, tc := range []struct {
+		desc     string
+		genState *types.GenesisState
+		valid    bool
+	}{
+		{
+			desc:     "default is valid",
+			genState: types.DefaultGenesis(),
+			valid:    true,
+		}, {
+			desc: "valid genesis state",
+			genState: &types.GenesisState{
+				Params: types.Params{
+					EpochIdentifier: "hour",
+					BridgingFee:     sdk.NewDecWithPrec(1, 1),
+				},
+				RollappPackets: []ctypes.RollappPacket{validRollappPacket},
+			},
+			valid: true,
+		}, {
+			desc: "invalid params",
+			genState: &types.GenesisState{
+				Params: types.Params{
+					EpochIdentifier: "",
+					BridgingFee:     sdk.Dec{},
+				},
+			},
+			valid: false,
+		}, {
+			desc:     "invalid rollapp packet",
+			genState: &types.GenesisState{RollappPackets: []ctypes.RollappPacket{{}}, Params: types.DefaultParams()},
+			valid:    false,
+		}, {
+			desc: "duplicate rollapp packet",
+			genState: &types.GenesisState{RollappPackets: []ctypes.RollappPacket{
+				validRollappPacket,
+				validRollappPacket,
+			}, Params: types.DefaultParams()},
+			valid: false,
+		},
+	} {
+		t.Run(tc.desc, func(t *testing.T) {
+			err := tc.genState.Validate()
+			if tc.valid {
+				require.NoError(t, err)
+			} else {
+				require.Error(t, err)
+			}
+		})
+	}
+}
+
+var validRollappPacket = ctypes.RollappPacket{
+	RollappId: "1",
+	Packet: &chantypes.Packet{
+		Sequence:           1,
+		SourcePort:         "transfer",
+		SourceChannel:      "channel-0",
+		DestinationPort:    "transfer",
+		DestinationChannel: "channel-1",
+		Data:               []byte("data"),
+		TimeoutHeight:      clienttypes.NewHeight(0, 1),
+		TimeoutTimestamp:   1,
+	},
+	Acknowledgement:        nil,
+	Status:                 ctypes.Status_PENDING,
+	ProofHeight:            100,
+	Relayer:                []byte("cosmos1"),
+	Type:                   ctypes.RollappPacket_ON_RECV,
+	Error:                  "error",
+	OriginalTransferTarget: "cosmos18wvvwfmq77a6d8tza4h5sfuy2yj3jj88yqg82a",
+}

x/eibc/types/errors.go

@@ -9,6 +9,7 @@ import (
 // x/eibc module sentinel errors
 var (
 	ErrInvalidOrderID               = errorsmod.Register(ModuleName, 3, "Invalid order ID")
+	ErrDemandOrderAlreadyExist      = errorsmod.Register(ModuleName, 4, "Demand order already exists")
 	ErrDemandOrderDoesNotExist      = errorsmod.Register(ModuleName, 5, "Demand order does not exist")
 	ErrDemandOrderInactive          = errorsmod.Register(ModuleName, 6, "Demand order inactive")
 	ErrFulfillerAddressDoesNotExist = errorsmod.Register(ModuleName, 7, "Fulfiller address does not exist")

x/eibc/types/genesis.go

@@ -13,5 +13,15 @@ func DefaultGenesis() *GenesisState {
 // Validate performs basic genesis state validation returning an error upon any
 // failure.
 func (gs GenesisState) Validate() error {
+	demandOrdersMap := make(map[string]struct{})
+	for _, demandOrder := range gs.GetDemandOrders() {
+		if err := demandOrder.Validate(); err != nil {
+			return err
+		}
+		if _, ok := demandOrdersMap[demandOrder.Id]; ok {
+			return ErrDemandOrderAlreadyExist
+		}
+		demandOrdersMap[demandOrder.Id] = struct{}{}
+	}
 	return gs.Params.Validate()
 }

x/eibc/types/genesis_test.go

@@ -3,8 +3,10 @@ package types_test
 import (
 	"testing"
 
-	"github.com/dymensionxyz/dymension/v3/x/eibc/types"
+	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/stretchr/testify/require"
+
+	"github.com/dymensionxyz/dymension/v3/x/eibc/types"
 )
 
 func TestGenesisState_Validate(t *testing.T) {
@@ -17,11 +19,33 @@ func TestGenesisState_Validate(t *testing.T) {
 			desc:     "default is valid",
 			genState: types.DefaultGenesis(),
 			valid:    true,
-		},
-		{
-			desc:     "valid genesis state",
-			genState: &types.GenesisState{},
-			valid:    true,
+		}, {
+			desc: "valid genesis state",
+			genState: &types.GenesisState{
+				Params:       validParams,
+				DemandOrders: []types.DemandOrder{validDemandOrder},
+			},
+			valid: true,
+		}, {
+			desc: "invalid params",
+			genState: &types.GenesisState{
+				Params: types.Params{
+					TimeoutFee: sdk.NewDec(-1),
+					ErrackFee:  sdk.NewDec(-1),
+				},
+			},
+			valid: false,
+		}, {
+			desc:     "invalid demand order",
+			genState: &types.GenesisState{DemandOrders: []types.DemandOrder{{}}, Params: types.DefaultParams()},
+			valid:    false,
+		}, {
+			desc: "duplicate demand order",
+			genState: &types.GenesisState{DemandOrders: []types.DemandOrder{
+				validDemandOrder,
+				validDemandOrder,
+			}, Params: types.DefaultParams()},
+			valid: false,
 		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
@@ -34,3 +58,16 @@ func TestGenesisState_Validate(t *testing.T) {
 		})
 	}
 }
+
+var validDemandOrder = types.DemandOrder{
+	Id:        "1",
+	Price:     sdk.Coins{sdk.NewInt64Coin("denom", 2)},
+	Fee:       sdk.Coins{sdk.NewInt64Coin("denom", 1)},
+	Recipient: "cosmos18wvvwfmq77a6d8tza4h5sfuy2yj3jj88yqg82a",
+}
+
+var validParams = types.Params{
+	EpochIdentifier: "hour",
+	TimeoutFee:      sdk.NewDecWithPrec(1, 1),
+	ErrackFee:       sdk.NewDecWithPrec(1, 1),
+}

x/eibc/types/params.go

@@ -1,7 +1,7 @@
 package types
 
 import (
-	fmt "fmt"
+	"fmt"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"
@@ -47,15 +47,23 @@ func DefaultParams() Params {
 // ParamSetPairs get the params.ParamSet
 func (p *Params) ParamSetPairs() paramtypes.ParamSetPairs {
 	return paramtypes.ParamSetPairs{
-		paramtypes.NewParamSetPair(KeyEpochIdentifier, &p.EpochIdentifier, func(_ interface{}) error { return nil }),
+		paramtypes.NewParamSetPair(KeyEpochIdentifier, &p.EpochIdentifier, validateEpochIdentifier),
 		paramtypes.NewParamSetPair(KeyTimeoutFee, &p.TimeoutFee, validateTimeoutFee),
 		paramtypes.NewParamSetPair(KeyErrAckFee, &p.ErrackFee, validateErrAckFee),
 	}
 }
 
 // Validate validates the set of params
 func (p Params) Validate() error {
-	// TODO(danwt): need to validate fees again?
+	if err := validateEpochIdentifier(p.EpochIdentifier); err != nil {
+		return fmt.Errorf("epoch identifier: %w", err)
+	}
+	if err := validateTimeoutFee(p.TimeoutFee); err != nil {
+		return fmt.Errorf("timeout fee: %w", err)
+	}
+	if err := validateErrAckFee(p.ErrackFee); err != nil {
+		return fmt.Errorf("error acknowledgement fee: %w", err)
+	}
 	return nil
 }
 
@@ -65,6 +73,17 @@ func (p Params) String() string {
 	return string(out)
 }
 
+func validateEpochIdentifier(i interface{}) error {
+	v, ok := i.(string)
+	if !ok {
+		return fmt.Errorf("invalid parameter type: %T", i)
+	}
+	if len(v) == 0 {
+		return fmt.Errorf("epoch identifier cannot be empty")
+	}
+	return nil
+}
+
 func validateTimeoutFee(i interface{}) error {
 	v, ok := i.(sdk.Dec)
 	if !ok {