From e09113181c125460b5b04819cfda4ef8750cae0c Mon Sep 17 00:00:00 2001 From: Sean King Date: Wed, 24 Nov 2021 17:12:43 +0100 Subject: [PATCH 1/7] feat: adding genesis validation + tests --- modules/apps/29-fee/types/genesis.go | 36 +++++ modules/apps/29-fee/types/genesis_test.go | 187 ++++++++++++++++++++-- modules/apps/29-fee/types/msgs.go | 87 ++++++---- 3 files changed, 265 insertions(+), 45 deletions(-) diff --git a/modules/apps/29-fee/types/genesis.go b/modules/apps/29-fee/types/genesis.go index 3f85192d930..eff429c673c 100644 --- a/modules/apps/29-fee/types/genesis.go +++ b/modules/apps/29-fee/types/genesis.go @@ -1,5 +1,11 @@ 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 +27,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 msg.Address into sdk.AccAddress") + } + + if rel.CounterpartyAddress == "" { + return ErrCounterpartyAddressEmpty + } + } + return nil } diff --git a/modules/apps/29-fee/types/genesis_test.go b/modules/apps/29-fee/types/genesis_test.go index 30516612cce..b11f0908e5b 100644 --- a/modules/apps/29-fee/types/genesis_test.go +++ b/modules/apps/29-fee/types/genesis_test.go @@ -1,39 +1,199 @@ package types_test -/* import ( "testing" "github.com/stretchr/testify/require" + "github.com/tendermint/tendermint/crypto/secp256k1" - "github.com/cosmos/ibc-go/modules/apps/transfer/types" + sdk "github.com/cosmos/cosmos-sdk/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)}} + invalidCoins = sdk.Coins{sdk.Coin{Denom: "invalidDenom", Amount: sdk.NewInt(100)}} ) func TestValidateGenesis(t *testing.T) { + var ( + packetId *channeltypes.PacketId + fee types.Fee + refundAcc string + sender string + counterparty string + portID = types.PortID + channelID = ibctesting.FirstChannelID + seq = uint64(1) + genState types.GenesisState + ) + 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 = types.NewPacketId( + "", + portID, + seq, + ) + }, - true, + false, + }, + { + "invalid packetId: invalid port", + func() { + packetId = types.NewPacketId( + channelID, + "", + seq, + ) + }, + false, + }, + { + "invalid packetId: invalid sequence", + func() { + packetId = types.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 client", - &types.GenesisState{ - PortId: "(INVALIDPORT)", + "invalid RegisteredRelayers: invalid sender", + func() { + sender = "" }, false, }, + { + "invalid RegisteredRelayers: invalid counterparty", + func() { + counterparty = "" + }, + false, + }, + } + + for _, tc := range testCases { + portID = types.PortID + channelID = ibctesting.FirstChannelID + seq = uint64(1) + + // build PacketId & Fee + packetId = types.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) { + testCases := []struct { + name string + genState *types.GenesisState + expPass bool + }{ + { + name: "default", + genState: types.DefaultGenesisState(), + expPass: true, + }, } for _, tc := range testCases { @@ -46,4 +206,3 @@ func TestValidateGenesis(t *testing.T) { } } } -*/ diff --git a/modules/apps/29-fee/types/msgs.go b/modules/apps/29-fee/types/msgs.go index bd04c041bb5..4b99e3924d1 100644 --- a/modules/apps/29-fee/types/msgs.go +++ b/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,60 @@ func NewIdentifiedPacketFee(packetId *channeltypes.PacketId, fee Fee, refundAddr } } +func (fee IdentifiedPacketFee) Validate() error { + if err := host.PortIdentifierValidator(fee.PacketId.PortId); err != nil { + return sdkerrors.Wrap(err, "invalid source port ID") + } + + if err := host.ChannelIdentifierValidator(fee.PacketId.ChannelId); err != nil { + return sdkerrors.Wrap(err, "invalid source channel ID") + } + + if fee.PacketId.Sequence == 0 { + return sdkerrors.Wrap(sdkerrors.ErrInvalidSequence, "packet sequence cannot be 0") + } + + _, 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 +} + // 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} } + +// Validates a PacketId +// TODO: move to channeltypes +/* +func (p channeltypes.PacketId) Validate() error { + if err := host.PortIdentifierValidator(p.PortId); err != nil { + return sdkerrors.Wrap(err, "invalid source port ID") + } + if err := host.ChannelIdentifierValidator(p.ChannelId); err != nil { + return sdkerrors.Wrap(err, "invalid source channel ID") + } + if p.Sequence == 0 { + return sdkerrors.Wrap(ErrInvalidPacket, "packet sequence cannot be 0") + } +} +*/ From d292e682ee45df4eb3c770fd9f627dd5b57b5f2f Mon Sep 17 00:00:00 2001 From: Sean King Date: Wed, 24 Nov 2021 17:16:17 +0100 Subject: [PATCH 2/7] fix: imports --- modules/apps/29-fee/types/genesis.go | 1 + modules/apps/29-fee/types/genesis_test.go | 16 +++++++--------- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/modules/apps/29-fee/types/genesis.go b/modules/apps/29-fee/types/genesis.go index eff429c673c..ff64153dc9e 100644 --- a/modules/apps/29-fee/types/genesis.go +++ b/modules/apps/29-fee/types/genesis.go @@ -3,6 +3,7 @@ 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" ) diff --git a/modules/apps/29-fee/types/genesis_test.go b/modules/apps/29-fee/types/genesis_test.go index b11f0908e5b..39b3140d70e 100644 --- a/modules/apps/29-fee/types/genesis_test.go +++ b/modules/apps/29-fee/types/genesis_test.go @@ -3,22 +3,21 @@ package types_test import ( "testing" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/stretchr/testify/require" "github.com/tendermint/tendermint/crypto/secp256k1" - sdk "github.com/cosmos/cosmos-sdk/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)}} - invalidCoins = sdk.Coins{sdk.Coin{Denom: "invalidDenom", Amount: sdk.NewInt(100)}} + 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) { @@ -31,7 +30,6 @@ func TestValidateGenesis(t *testing.T) { portID = types.PortID channelID = ibctesting.FirstChannelID seq = uint64(1) - genState types.GenesisState ) testCases := []struct { @@ -151,7 +149,7 @@ func TestValidateGenesis(t *testing.T) { tc.malleate() - genState = types.GenesisState{ + genState := types.GenesisState{ IdentifiedFees: []*types.IdentifiedPacketFee{ { PacketId: packetId, From 422d957a67907c077e18b10c089bcc003edf4f3c Mon Sep 17 00:00:00 2001 From: Sean King Date: Wed, 24 Nov 2021 17:22:28 +0100 Subject: [PATCH 3/7] Update modules/apps/29-fee/types/genesis.go --- modules/apps/29-fee/types/genesis.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/apps/29-fee/types/genesis.go b/modules/apps/29-fee/types/genesis.go index ff64153dc9e..5a0ebdeb9d2 100644 --- a/modules/apps/29-fee/types/genesis.go +++ b/modules/apps/29-fee/types/genesis.go @@ -50,7 +50,7 @@ func (gs GenesisState) Validate() error { for _, rel := range gs.RegisteredRelayers { _, err := sdk.AccAddressFromBech32(rel.Address) if err != nil { - return sdkerrors.Wrap(err, "failed to convert msg.Address into sdk.AccAddress") + return sdkerrors.Wrap(err, "failed to convert source relayer address into sdk.AccAddress") } if rel.CounterpartyAddress == "" { From e6d4584f0309173604077d322da7d4f0cf22d2c3 Mon Sep 17 00:00:00 2001 From: Sean King Date: Wed, 24 Nov 2021 17:23:27 +0100 Subject: [PATCH 4/7] fix: nit --- modules/apps/29-fee/types/genesis_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/apps/29-fee/types/genesis_test.go b/modules/apps/29-fee/types/genesis_test.go index 39b3140d70e..42fc5863bd8 100644 --- a/modules/apps/29-fee/types/genesis_test.go +++ b/modules/apps/29-fee/types/genesis_test.go @@ -27,9 +27,9 @@ func TestValidateGenesis(t *testing.T) { refundAcc string sender string counterparty string - portID = types.PortID - channelID = ibctesting.FirstChannelID - seq = uint64(1) + portID string + channelID string + seq uint64 ) testCases := []struct { From 9ca60b8e44a4755f81976cc2d77289da7aca023d Mon Sep 17 00:00:00 2001 From: Sean King Date: Wed, 24 Nov 2021 17:40:23 +0100 Subject: [PATCH 5/7] Update modules/apps/29-fee/types/genesis_test.go Co-authored-by: Aditya --- modules/apps/29-fee/types/genesis_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/modules/apps/29-fee/types/genesis_test.go b/modules/apps/29-fee/types/genesis_test.go index 42fc5863bd8..90fb9ad4f86 100644 --- a/modules/apps/29-fee/types/genesis_test.go +++ b/modules/apps/29-fee/types/genesis_test.go @@ -50,7 +50,6 @@ func TestValidateGenesis(t *testing.T) { portID, seq, ) - }, false, }, From 20ef9d89015bbb4983ecd249b0e0aac4a0f3a1ec Mon Sep 17 00:00:00 2001 From: Sean King Date: Wed, 24 Nov 2021 17:45:22 +0100 Subject: [PATCH 6/7] nit: imporve default gen val test --- modules/apps/29-fee/types/genesis_test.go | 23 ++--------------------- 1 file changed, 2 insertions(+), 21 deletions(-) diff --git a/modules/apps/29-fee/types/genesis_test.go b/modules/apps/29-fee/types/genesis_test.go index 90fb9ad4f86..f3ce33d13e0 100644 --- a/modules/apps/29-fee/types/genesis_test.go +++ b/modules/apps/29-fee/types/genesis_test.go @@ -181,25 +181,6 @@ func TestValidateGenesis(t *testing.T) { } func TestValidateDefaultGenesis(t *testing.T) { - testCases := []struct { - name string - genState *types.GenesisState - expPass bool - }{ - { - name: "default", - genState: types.DefaultGenesisState(), - expPass: true, - }, - } - - for _, tc := range testCases { - tc := tc - err := tc.genState.Validate() - if tc.expPass { - require.NoError(t, err, tc.name) - } else { - require.Error(t, err, tc.name) - } - } + err := types.DefaultGenesisState().Validate() + require.NoError(t, err) } From baa58a74d96ada9a8013d9ae397e99df66e12cac Mon Sep 17 00:00:00 2001 From: Sean King Date: Wed, 24 Nov 2021 18:15:44 +0100 Subject: [PATCH 7/7] chore: move packetId + val to channeltypes and use validate fn --- modules/apps/29-fee/keeper/genesis_test.go | 5 ++- modules/apps/29-fee/keeper/grpc_query_test.go | 11 +++--- modules/apps/29-fee/types/genesis_test.go | 8 ++-- modules/apps/29-fee/types/msgs.go | 38 +++---------------- modules/core/04-channel/types/packet.go | 22 +++++++++++ 5 files changed, 40 insertions(+), 44 deletions(-) diff --git a/modules/apps/29-fee/keeper/genesis_test.go b/modules/apps/29-fee/keeper/genesis_test.go index 4119891d380..7c5939c7b2f 100644 --- a/modules/apps/29-fee/keeper/genesis_test.go +++ b/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), diff --git a/modules/apps/29-fee/keeper/grpc_query_test.go b/modules/apps/29-fee/keeper/grpc_query_test.go index 1403326837a..560ccb6ffa3 100644 --- a/modules/apps/29-fee/keeper/grpc_query_test.go +++ b/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) diff --git a/modules/apps/29-fee/types/genesis_test.go b/modules/apps/29-fee/types/genesis_test.go index f3ce33d13e0..1a98142a870 100644 --- a/modules/apps/29-fee/types/genesis_test.go +++ b/modules/apps/29-fee/types/genesis_test.go @@ -45,7 +45,7 @@ func TestValidateGenesis(t *testing.T) { { "invalid packetId: invalid channel", func() { - packetId = types.NewPacketId( + packetId = channeltypes.NewPacketId( "", portID, seq, @@ -56,7 +56,7 @@ func TestValidateGenesis(t *testing.T) { { "invalid packetId: invalid port", func() { - packetId = types.NewPacketId( + packetId = channeltypes.NewPacketId( channelID, "", seq, @@ -67,7 +67,7 @@ func TestValidateGenesis(t *testing.T) { { "invalid packetId: invalid sequence", func() { - packetId = types.NewPacketId( + packetId = channeltypes.NewPacketId( channelID, portID, 0, @@ -129,7 +129,7 @@ func TestValidateGenesis(t *testing.T) { seq = uint64(1) // build PacketId & Fee - packetId = types.NewPacketId( + packetId = channeltypes.NewPacketId( portID, channelID, seq, diff --git a/modules/apps/29-fee/types/msgs.go b/modules/apps/29-fee/types/msgs.go index 4b99e3924d1..4653b69ce18 100644 --- a/modules/apps/29-fee/types/msgs.go +++ b/modules/apps/29-fee/types/msgs.go @@ -145,19 +145,13 @@ func NewIdentifiedPacketFee(packetId *channeltypes.PacketId, fee Fee, refundAddr } func (fee IdentifiedPacketFee) Validate() error { - if err := host.PortIdentifierValidator(fee.PacketId.PortId); err != nil { - return sdkerrors.Wrap(err, "invalid source port ID") - } - - if err := host.ChannelIdentifierValidator(fee.PacketId.ChannelId); err != nil { - return sdkerrors.Wrap(err, "invalid source channel ID") - } - - if fee.PacketId.Sequence == 0 { - return sdkerrors.Wrap(sdkerrors.ErrInvalidSequence, "packet sequence cannot be 0") + // validate PacketId + err := fee.PacketId.Validate() + if err != nil { + return sdkerrors.Wrap(err, "Invalid PacketId") } - _, err := sdk.AccAddressFromBech32(fee.RefundAddress) + _, err = sdk.AccAddressFromBech32(fee.RefundAddress) if err != nil { return sdkerrors.Wrap(err, "failed to convert RefundAddress into sdk.AccAddress") } @@ -179,25 +173,3 @@ func (fee IdentifiedPacketFee) Validate() error { return nil } - -// 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} -} - -// Validates a PacketId -// TODO: move to channeltypes -/* -func (p channeltypes.PacketId) Validate() error { - if err := host.PortIdentifierValidator(p.PortId); err != nil { - return sdkerrors.Wrap(err, "invalid source port ID") - } - if err := host.ChannelIdentifierValidator(p.ChannelId); err != nil { - return sdkerrors.Wrap(err, "invalid source channel ID") - } - if p.Sequence == 0 { - return sdkerrors.Wrap(ErrInvalidPacket, "packet sequence cannot be 0") - } -} -*/ diff --git a/modules/core/04-channel/types/packet.go b/modules/core/04-channel/types/packet.go index 5e9c56e62d8..962ab94ffc9 100644 --- a/modules/core/04-channel/types/packet.go +++ b/modules/core/04-channel/types/packet.go @@ -110,3 +110,25 @@ func (p Packet) ValidateBasic() error { } return nil } + +// NewPacketId returns a new instance of PacketId +func NewPacketId(channelId, portId string, seq uint64) *PacketId { + return &PacketId{ChannelId: channelId, PortId: portId, Sequence: seq} +} + +// Validates a PacketId +func (p PacketId) Validate() error { + if err := host.PortIdentifierValidator(p.PortId); err != nil { + return sdkerrors.Wrap(err, "invalid source port ID") + } + + if err := host.ChannelIdentifierValidator(p.ChannelId); err != nil { + return sdkerrors.Wrap(err, "invalid source channel ID") + } + + if p.Sequence == 0 { + return sdkerrors.Wrap(ErrInvalidPacket, "packet sequence cannot be 0") + } + + return nil +}