feat: adding genesis validation + tests (#561)

* feat: adding genesis validation + tests

* fix: imports

* Update modules/apps/29-fee/types/genesis.go

* fix: nit

* Update modules/apps/29-fee/types/genesis_test.go

Co-authored-by: Aditya <adityasripal@gmail.com>

* nit: imporve default gen val test

* chore: move packetId + val to channeltypes and use validate fn

Co-authored-by: Aditya <adityasripal@gmail.com>

modules/apps/29-fee/keeper/genesis_test.go

@@ -3,6 +3,7 @@ package keeper_test
 import (
 	"github.com/cosmos/ibc-go/modules/apps/29-fee/types"
 	transfertypes "github.com/cosmos/ibc-go/modules/apps/transfer/types"
+	channeltypes "github.com/cosmos/ibc-go/modules/core/04-channel/types"
 	ibctesting "github.com/cosmos/ibc-go/testing"
 )
 
@@ -11,7 +12,7 @@ func (suite *KeeperTestSuite) TestInitGenesis() {
 
 	// build PacketId & Fee
 	refundAcc := suite.chainA.SenderAccount.GetAddress()
-	packetId := types.NewPacketId(
+	packetId := channeltypes.NewPacketId(
 		ibctesting.FirstChannelID,
 		types.PortID,
 		uint64(1),
@@ -73,7 +74,7 @@ func (suite *KeeperTestSuite) TestExportGenesis() {
 
 	// setup & escrow the packet fee
 	refundAcc := suite.chainA.SenderAccount.GetAddress()
-	packetId := types.NewPacketId(
+	packetId := channeltypes.NewPacketId(
 		ibctesting.FirstChannelID,
 		types.PortID,
 		uint64(1),

modules/apps/29-fee/keeper/grpc_query_test.go

@@ -7,6 +7,7 @@ import (
 	"github.com/cosmos/cosmos-sdk/types/query"
 
 	"github.com/cosmos/ibc-go/modules/apps/29-fee/types"
+	channeltypes "github.com/cosmos/ibc-go/modules/core/04-channel/types"
 	ibctesting "github.com/cosmos/ibc-go/testing"
 )
 
@@ -17,8 +18,8 @@ func (suite *KeeperTestSuite) TestQueryIncentivizedPacket() {
 	)
 
 	// setup
-	validPacketId := types.NewPacketId(ibctesting.FirstChannelID, types.PortID, 1)
-	invalidPacketId := types.NewPacketId(ibctesting.FirstChannelID, types.PortID, 2)
+	validPacketId := channeltypes.NewPacketId(ibctesting.FirstChannelID, types.PortID, 1)
+	invalidPacketId := channeltypes.NewPacketId(ibctesting.FirstChannelID, types.PortID, 2)
 	identifiedPacketFee := types.NewIdentifiedPacketFee(
 		validPacketId,
 		types.Fee{
@@ -110,9 +111,9 @@ func (suite *KeeperTestSuite) TestQueryIncentivizedPackets() {
 			func() {
 				refundAcc := suite.chainA.SenderAccount.GetAddress()
 
-				fee1 := types.NewIdentifiedPacketFee(types.NewPacketId(ibctesting.FirstChannelID, types.PortID, 1), fee, refundAcc.String(), []string(nil))
-				fee2 := types.NewIdentifiedPacketFee(types.NewPacketId(ibctesting.FirstChannelID, types.PortID, 2), fee, refundAcc.String(), []string(nil))
-				fee3 := types.NewIdentifiedPacketFee(types.NewPacketId(ibctesting.FirstChannelID, types.PortID, 3), fee, refundAcc.String(), []string(nil))
+				fee1 := types.NewIdentifiedPacketFee(channeltypes.NewPacketId(ibctesting.FirstChannelID, types.PortID, 1), fee, refundAcc.String(), []string(nil))
+				fee2 := types.NewIdentifiedPacketFee(channeltypes.NewPacketId(ibctesting.FirstChannelID, types.PortID, 2), fee, refundAcc.String(), []string(nil))
+				fee3 := types.NewIdentifiedPacketFee(channeltypes.NewPacketId(ibctesting.FirstChannelID, types.PortID, 3), fee, refundAcc.String(), []string(nil))
 
 				expPackets = []*types.IdentifiedPacketFee{}
 				expPackets = append(expPackets, fee1, fee2, fee3)

modules/apps/29-fee/types/genesis.go

@@ -1,5 +1,12 @@
 package types
 
+import (
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
+
+	host "github.com/cosmos/ibc-go/modules/core/24-host"
+)
+
 // NewGenesisState creates a 29-fee GenesisState instance.
 func NewGenesisState(identifiedFees []*IdentifiedPacketFee, feeEnabledChannels []*FeeEnabledChannel, registeredRelayers []*RegisteredRelayerAddress) *GenesisState {
 	return &GenesisState{
@@ -21,5 +28,35 @@ func DefaultGenesisState() *GenesisState {
 // Validate performs basic genesis state validation returning an error upon any
 // failure.
 func (gs GenesisState) Validate() error {
+	// Validate IdentifiedPacketFees
+	for _, fee := range gs.IdentifiedFees {
+		err := fee.Validate()
+		if err != nil {
+			return err
+		}
+	}
+
+	// Validate FeeEnabledChannels
+	for _, feeCh := range gs.FeeEnabledChannels {
+		if err := host.PortIdentifierValidator(feeCh.PortId); err != nil {
+			return sdkerrors.Wrap(err, "invalid source port ID")
+		}
+		if err := host.ChannelIdentifierValidator(feeCh.ChannelId); err != nil {
+			return sdkerrors.Wrap(err, "invalid source channel ID")
+		}
+	}
+
+	// Validate RegisteredRelayers
+	for _, rel := range gs.RegisteredRelayers {
+		_, err := sdk.AccAddressFromBech32(rel.Address)
+		if err != nil {
+			return sdkerrors.Wrap(err, "failed to convert source relayer address into sdk.AccAddress")
+		}
+
+		if rel.CounterpartyAddress == "" {
+			return ErrCounterpartyAddressEmpty
+		}
+	}
+
 	return nil
 }

modules/apps/29-fee/types/genesis_test.go

@@ -1,49 +1,186 @@
 package types_test
 
-/*
 import (
 	"testing"
 
+	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/stretchr/testify/require"
+	"github.com/tendermint/tendermint/crypto/secp256k1"
 
-	"github.com/cosmos/ibc-go/modules/apps/transfer/types"
+	"github.com/cosmos/ibc-go/modules/apps/29-fee/types"
+	channeltypes "github.com/cosmos/ibc-go/modules/core/04-channel/types"
+	ibctesting "github.com/cosmos/ibc-go/testing"
+)
+
+var (
+	addr1       = sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String()
+	addr2       = sdk.AccAddress("testaddr2").String()
+	validCoins  = sdk.Coins{sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: sdk.NewInt(100)}}
+	validCoins2 = sdk.Coins{sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: sdk.NewInt(200)}}
+	validCoins3 = sdk.Coins{sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: sdk.NewInt(300)}}
 )
 
 func TestValidateGenesis(t *testing.T) {
+	var (
+		packetId     *channeltypes.PacketId
+		fee          types.Fee
+		refundAcc    string
+		sender       string
+		counterparty string
+		portID       string
+		channelID    string
+		seq          uint64
+	)
+
 	testCases := []struct {
 		name     string
-		genState *types.GenesisState
+		malleate func()
 		expPass  bool
 	}{
 		{
-			name:     "default",
-			genState: types.DefaultGenesisState(),
-			expPass:  true,
+			"valid genesis",
+			func() {},
+			true,
 		},
 		{
-			"valid genesis",
-			&types.GenesisState{
-				PortId: "portidone",
+			"invalid packetId: invalid channel",
+			func() {
+				packetId = channeltypes.NewPacketId(
+					"",
+					portID,
+					seq,
+				)
 			},
-			true,
+			false,
+		},
+		{
+			"invalid packetId: invalid port",
+			func() {
+				packetId = channeltypes.NewPacketId(
+					channelID,
+					"",
+					seq,
+				)
+			},
+			false,
 		},
 		{
-			"invalid client",
-			&types.GenesisState{
-				PortId: "(INVALIDPORT)",
+			"invalid packetId: invalid sequence",
+			func() {
+				packetId = channeltypes.NewPacketId(
+					channelID,
+					portID,
+					0,
+				)
+			},
+			false,
+		},
+		{
+			"invalid packetId: invalid fee",
+			func() {
+				fee = types.Fee{
+					sdk.Coins{},
+					sdk.Coins{},
+					sdk.Coins{},
+				}
+			},
+			false,
+		},
+		{
+			"invalid packetId: invalid refundAcc",
+			func() {
+				refundAcc = ""
+			},
+			false,
+		},
+		{
+			"invalid FeeEnabledChannel: invalid ChannelID",
+			func() {
+				channelID = ""
+			},
+			false,
+		},
+		{
+			"invalid FeeEnabledChannel: invalid PortID",
+			func() {
+				portID = ""
+			},
+			false,
+		},
+		{
+			"invalid RegisteredRelayers: invalid sender",
+			func() {
+				sender = ""
+			},
+			false,
+		},
+		{
+			"invalid RegisteredRelayers: invalid counterparty",
+			func() {
+				counterparty = ""
 			},
 			false,
 		},
 	}
 
 	for _, tc := range testCases {
-		tc := tc
-		err := tc.genState.Validate()
+		portID = types.PortID
+		channelID = ibctesting.FirstChannelID
+		seq = uint64(1)
+
+		// build PacketId & Fee
+		packetId = channeltypes.NewPacketId(
+			portID,
+			channelID,
+			seq,
+		)
+		fee = types.Fee{
+			validCoins,
+			validCoins2,
+			validCoins3,
+		}
+
+		refundAcc = addr1
+
+		// relayer addresses
+		sender = addr1
+		counterparty = addr2
+
+		tc.malleate()
+
+		genState := types.GenesisState{
+			IdentifiedFees: []*types.IdentifiedPacketFee{
+				{
+					PacketId:      packetId,
+					Fee:           fee,
+					RefundAddress: refundAcc,
+					Relayers:      nil,
+				},
+			},
+			FeeEnabledChannels: []*types.FeeEnabledChannel{
+				{
+					PortId:    portID,
+					ChannelId: channelID,
+				},
+			},
+			RegisteredRelayers: []*types.RegisteredRelayerAddress{
+				{
+					Address:             sender,
+					CounterpartyAddress: counterparty,
+				},
+			},
+		}
+
+		err := genState.Validate()
 		if tc.expPass {
 			require.NoError(t, err, tc.name)
 		} else {
 			require.Error(t, err, tc.name)
 		}
 	}
 }
-*/
+
+func TestValidateDefaultGenesis(t *testing.T) {
+	err := types.DefaultGenesisState().Validate()
+	require.NoError(t, err)
+}

modules/apps/29-fee/types/msgs.go

@@ -111,42 +111,15 @@ func NewMsgPayPacketFeeAsync(identifiedPacketFee IdentifiedPacketFee) *MsgPayPac
 
 // ValidateBasic performs a basic check of the MsgPayPacketFeeAsync fields
 func (msg MsgPayPacketFeeAsync) ValidateBasic() error {
-	// validate channelId
-	err := host.ChannelIdentifierValidator(msg.IdentifiedPacketFee.PacketId.ChannelId)
-	if err != nil {
-		return err
-	}
-
-	// validate portId
-	err = host.PortIdentifierValidator(msg.IdentifiedPacketFee.PacketId.PortId)
-	if err != nil {
-		return err
-	}
-
 	// signer check
-	_, err = sdk.AccAddressFromBech32(msg.IdentifiedPacketFee.RefundAddress)
+	_, err := sdk.AccAddressFromBech32(msg.IdentifiedPacketFee.RefundAddress)
 	if err != nil {
 		return sdkerrors.Wrap(err, "failed to convert msg.Signer into sdk.AccAddress")
 	}
 
-	// enforce relayer is nil
-	if msg.IdentifiedPacketFee.Relayers != nil {
-		return ErrRelayersNotNil
-	}
-
-	// ensure sequence is not 0
-	if msg.IdentifiedPacketFee.PacketId.Sequence == 0 {
-		return sdkerrors.ErrInvalidSequence
-	}
-
-	// if any of the fee's are invalid return an error
-	if !msg.IdentifiedPacketFee.Fee.AckFee.IsValid() || !msg.IdentifiedPacketFee.Fee.ReceiveFee.IsValid() || !msg.IdentifiedPacketFee.Fee.TimeoutFee.IsValid() {
-		return sdkerrors.ErrInvalidCoins
-	}
-
-	// if all three fee's are zero or empty return an error
-	if msg.IdentifiedPacketFee.Fee.AckFee.IsZero() && msg.IdentifiedPacketFee.Fee.ReceiveFee.IsZero() && msg.IdentifiedPacketFee.Fee.TimeoutFee.IsZero() {
-		return sdkerrors.ErrInvalidCoins
+	err = msg.IdentifiedPacketFee.Validate()
+	if err != nil {
+		return sdkerrors.Wrap(err, "Invalid IdentifiedPacketFee")
 	}
 
 	return nil
@@ -171,8 +144,32 @@ func NewIdentifiedPacketFee(packetId *channeltypes.PacketId, fee Fee, refundAddr
 	}
 }
 
-// NewPacketId returns a new instance of PacketId
-// TODO: move to channeltypes
-func NewPacketId(channelId, portId string, seq uint64) *channeltypes.PacketId {
-	return &channeltypes.PacketId{ChannelId: channelId, PortId: portId, Sequence: seq}
+func (fee IdentifiedPacketFee) Validate() error {
+	// validate PacketId
+	err := fee.PacketId.Validate()
+	if err != nil {
+		return sdkerrors.Wrap(err, "Invalid PacketId")
+	}
+
+	_, err = sdk.AccAddressFromBech32(fee.RefundAddress)
+	if err != nil {
+		return sdkerrors.Wrap(err, "failed to convert RefundAddress into sdk.AccAddress")
+	}
+
+	// if any of the fee's are invalid return an error
+	if !fee.Fee.AckFee.IsValid() || !fee.Fee.ReceiveFee.IsValid() || !fee.Fee.TimeoutFee.IsValid() {
+		return sdkerrors.ErrInvalidCoins
+	}
+
+	// if all three fee's are zero or empty return an error
+	if fee.Fee.AckFee.IsZero() && fee.Fee.ReceiveFee.IsZero() && fee.Fee.TimeoutFee.IsZero() {
+		return sdkerrors.ErrInvalidCoins
+	}
+
+	// enforce relayer is nil
+	if fee.Relayers != nil {
+		return ErrRelayersNotNil
+	}
+
+	return nil
 }