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.go b/modules/apps/29-fee/types/genesis.go index 3f85192d930..5a0ebdeb9d2 100644 --- a/modules/apps/29-fee/types/genesis.go +++ b/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 } diff --git a/modules/apps/29-fee/types/genesis_test.go b/modules/apps/29-fee/types/genesis_test.go index 30516612cce..1a98142a870 100644 --- a/modules/apps/29-fee/types/genesis_test.go +++ b/modules/apps/29-fee/types/genesis_test.go @@ -1,44 +1,177 @@ 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 { @@ -46,4 +179,8 @@ func TestValidateGenesis(t *testing.T) { } } } -*/ + +func TestValidateDefaultGenesis(t *testing.T) { + err := types.DefaultGenesisState().Validate() + require.NoError(t, err) +} diff --git a/modules/apps/29-fee/types/msgs.go b/modules/apps/29-fee/types/msgs.go index bd04c041bb5..4653b69ce18 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,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 } 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 +}