From 14ef24a9fb2f43a65f56e499705d96a0c3a06661 Mon Sep 17 00:00:00 2001 From: bznein Date: Tue, 15 Oct 2024 14:10:39 +0100 Subject: [PATCH 1/3] chore: add ValidateBasic and test for MsgRecvPacket --- modules/core/04-channel/v2/types/msgs.go | 26 ++++++- modules/core/04-channel/v2/types/msgs_test.go | 73 +++++++++++++++++++ 2 files changed, 97 insertions(+), 2 deletions(-) diff --git a/modules/core/04-channel/v2/types/msgs.go b/modules/core/04-channel/v2/types/msgs.go index cb06c5f90ef..1a59c7a10f2 100644 --- a/modules/core/04-channel/v2/types/msgs.go +++ b/modules/core/04-channel/v2/types/msgs.go @@ -6,7 +6,8 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" clienttypes "github.com/cosmos/ibc-go/v9/modules/core/02-client/types" - commitmenttypes "github.com/cosmos/ibc-go/v9/modules/core/23-commitment/types/v2" + commitmenttypesv1 "github.com/cosmos/ibc-go/v9/modules/core/23-commitment/types" + commitmenttypesv2 "github.com/cosmos/ibc-go/v9/modules/core/23-commitment/types/v2" host "github.com/cosmos/ibc-go/v9/modules/core/24-host" ibcerrors "github.com/cosmos/ibc-go/v9/modules/core/errors" ) @@ -46,7 +47,7 @@ func (msg *MsgProvideCounterparty) ValidateBasic() error { } // NewMsgCreateChannel creates a new MsgCreateChannel instance -func NewMsgCreateChannel(clientID string, merklePathPrefix commitmenttypes.MerklePath, signer string) *MsgCreateChannel { +func NewMsgCreateChannel(clientID string, merklePathPrefix commitmenttypesv2.MerklePath, signer string) *MsgCreateChannel { return &MsgCreateChannel{ Signer: signer, ClientId: clientID, @@ -91,6 +92,27 @@ func NewMsgRecvPacket(packet Packet, proofCommitment []byte, proofHeight clientt } } +// ValidateBasic performs basic checks on a MsgRecvPacket. +func (msg *MsgRecvPacket) ValidateBasic() error { + if err := msg.Packet.ValidateBasic(); err != nil { + return err + } + + if len(msg.ProofCommitment) == 0 { + return errorsmod.Wrap(commitmenttypesv1.ErrInvalidProof, "proof commitment can not be empty") + } + + if msg.ProofHeight.IsZero() { + return errorsmod.Wrap(clienttypes.ErrInvalidHeight, "proof height can not be zero") + } + + _, err := sdk.AccAddressFromBech32(msg.Signer) + if err != nil { + return errorsmod.Wrapf(ibcerrors.ErrInvalidAddress, "string could not be parsed as address: %v", err) + } + return nil +} + // NewMsgAcknowledgement creates a new MsgAcknowledgement instance func NewMsgAcknowledgement(packet Packet, acknowledgement Acknowledgement, proofAcked []byte, proofHeight clienttypes.Height, signer string) *MsgAcknowledgement { return &MsgAcknowledgement{ diff --git a/modules/core/04-channel/v2/types/msgs_test.go b/modules/core/04-channel/v2/types/msgs_test.go index 6d867221b54..55408bde52e 100644 --- a/modules/core/04-channel/v2/types/msgs_test.go +++ b/modules/core/04-channel/v2/types/msgs_test.go @@ -6,11 +6,13 @@ import ( "github.com/stretchr/testify/suite" + clienttypes "github.com/cosmos/ibc-go/v9/modules/core/02-client/types" "github.com/cosmos/ibc-go/v9/modules/core/04-channel/v2/types" commitmenttypes "github.com/cosmos/ibc-go/v9/modules/core/23-commitment/types" host "github.com/cosmos/ibc-go/v9/modules/core/24-host" ibcerrors "github.com/cosmos/ibc-go/v9/modules/core/errors" ibctesting "github.com/cosmos/ibc-go/v9/testing" + "github.com/cosmos/ibc-go/v9/testing/mock" ) type TypesTestSuite struct { @@ -19,6 +21,8 @@ type TypesTestSuite struct { coordinator *ibctesting.Coordinator chainA *ibctesting.TestChain chainB *ibctesting.TestChain + + proof []byte } func (s *TypesTestSuite) SetupTest() { @@ -142,3 +146,72 @@ func (s *TypesTestSuite) TestMsgCreateChannelValidateBasic() { } } } + +func (s *TypesTestSuite) TestMsgRecvPacketValidateBasic() { + var msg *types.MsgRecvPacket + testCases := []struct { + name string + malleate func() + expError error + }{ + { + name: "success", + malleate: func() {}, + }, + { + name: "failure: invalid packet", + malleate: func() { + msg.Packet.Data = []types.PacketData{} + }, + expError: types.ErrInvalidPacket, + }, + { + name: "failure: invalid proof commitment", + malleate: func() { + msg.ProofCommitment = []byte{} + }, + expError: commitmenttypes.ErrInvalidProof, + }, + { + name: "failure: invalid proof height", + malleate: func() { + msg.ProofHeight = clienttypes.ZeroHeight() + }, + expError: clienttypes.ErrInvalidHeight, + }, + { + name: "failure: invalid signer", + malleate: func() { + msg.Signer = "" + }, + expError: ibcerrors.ErrInvalidAddress, + }, + } + for _, tc := range testCases { + s.Run(tc.name, func() { + packet := types.NewPacket(1, + ibctesting.FirstChannelID, ibctesting.FirstChannelID, + s.chainA.GetTimeoutTimestamp(), + types.PacketData{ + SourcePort: ibctesting.MockPort, + DestinationPort: ibctesting.MockPort, + Payload: types.NewPayload("ics20-1", "json", mock.MockPacketData), + }, + ) + + msg = types.NewMsgRecvPacket(packet, []byte("foo"), s.chainA.GetTimeoutHeight(), s.chainA.SenderAccount.GetAddress().String()) + + tc.malleate() + + err := msg.ValidateBasic() + + expPass := tc.expError == nil + + if expPass { + s.Require().NoError(err) + } else { + ibctesting.RequireErrorIsOrContains(s.T(), err, tc.expError) + } + }) + } +} From 539c56e0dd778b8f3972a5a7829059fae11c6d2a Mon Sep 17 00:00:00 2001 From: bznein Date: Tue, 15 Oct 2024 14:13:36 +0100 Subject: [PATCH 2/3] merge --- modules/core/04-channel/v2/types/msgs_test.go | 69 +++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/modules/core/04-channel/v2/types/msgs_test.go b/modules/core/04-channel/v2/types/msgs_test.go index e649869d907..d089a5dc460 100644 --- a/modules/core/04-channel/v2/types/msgs_test.go +++ b/modules/core/04-channel/v2/types/msgs_test.go @@ -215,3 +215,72 @@ func (s *TypesTestSuite) TestMsgSendPacketValidateBasic() { }) } } + +func (s *TypesTestSuite) TestMsgRecvPacketValidateBasic() { + var msg *types.MsgRecvPacket + testCases := []struct { + name string + malleate func() + expError error + }{ + { + name: "success", + malleate: func() {}, + }, + { + name: "failure: invalid packet", + malleate: func() { + msg.Packet.Data = []types.PacketData{} + }, + expError: types.ErrInvalidPacket, + }, + { + name: "failure: invalid proof commitment", + malleate: func() { + msg.ProofCommitment = []byte{} + }, + expError: commitmenttypes.ErrInvalidProof, + }, + { + name: "failure: invalid proof height", + malleate: func() { + msg.ProofHeight = clienttypes.ZeroHeight() + }, + expError: clienttypes.ErrInvalidHeight, + }, + { + name: "failure: invalid signer", + malleate: func() { + msg.Signer = "" + }, + expError: ibcerrors.ErrInvalidAddress, + }, + } + for _, tc := range testCases { + s.Run(tc.name, func() { + packet := types.NewPacket(1, + ibctesting.FirstChannelID, ibctesting.FirstChannelID, + s.chainA.GetTimeoutTimestamp(), + types.PacketData{ + SourcePort: ibctesting.MockPort, + DestinationPort: ibctesting.MockPort, + Payload: types.NewPayload("ics20-1", "json", mock.MockPacketData), + }, + ) + + msg = types.NewMsgRecvPacket(packet, []byte("foo"), s.chainA.GetTimeoutHeight(), s.chainA.SenderAccount.GetAddress().String()) + + tc.malleate() + + err := msg.ValidateBasic() + + expPass := tc.expError == nil + + if expPass { + s.Require().NoError(err) + } else { + ibctesting.RequireErrorIsOrContains(s.T(), err, tc.expError) + } + }) + } +} From 0e42fbe8f14d489d9ac6fc660fbedeafa2501e9c Mon Sep 17 00:00:00 2001 From: DimitrisJim Date: Thu, 17 Oct 2024 12:57:20 +0300 Subject: [PATCH 3/3] chore: address reviews. --- modules/core/04-channel/v2/types/msgs.go | 16 +++++------- modules/core/04-channel/v2/types/msgs_test.go | 26 ++++--------------- modules/core/keeper/events_test.go | 1 + modules/core/keeper/msg_server.go | 1 + modules/core/keeper/msg_server_test.go | 1 + 5 files changed, 14 insertions(+), 31 deletions(-) diff --git a/modules/core/04-channel/v2/types/msgs.go b/modules/core/04-channel/v2/types/msgs.go index 2afc72fd45f..2d1091c16f5 100644 --- a/modules/core/04-channel/v2/types/msgs.go +++ b/modules/core/04-channel/v2/types/msgs.go @@ -6,9 +6,9 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" clienttypes "github.com/cosmos/ibc-go/v9/modules/core/02-client/types" + channeltypesv1 "github.com/cosmos/ibc-go/v9/modules/core/04-channel/types" commitmenttypesv1 "github.com/cosmos/ibc-go/v9/modules/core/23-commitment/types" commitmenttypesv2 "github.com/cosmos/ibc-go/v9/modules/core/23-commitment/types/v2" - channeltypesv1 "github.com/cosmos/ibc-go/v9/modules/core/04-channel/types" host "github.com/cosmos/ibc-go/v9/modules/core/24-host" ibcerrors "github.com/cosmos/ibc-go/v9/modules/core/errors" ) @@ -19,6 +19,9 @@ var ( _ sdk.Msg = (*MsgCreateChannel)(nil) _ sdk.HasValidateBasic = (*MsgCreateChannel)(nil) + + _ sdk.Msg = (*MsgRecvPacket)(nil) + _ sdk.HasValidateBasic = (*MsgRecvPacket)(nil) ) // NewMsgProvideCounterparty creates a new MsgProvideCounterparty instance @@ -123,23 +126,16 @@ func NewMsgRecvPacket(packet Packet, proofCommitment []byte, proofHeight clientt // ValidateBasic performs basic checks on a MsgRecvPacket. func (msg *MsgRecvPacket) ValidateBasic() error { - if err := msg.Packet.ValidateBasic(); err != nil { - return err - } - if len(msg.ProofCommitment) == 0 { return errorsmod.Wrap(commitmenttypesv1.ErrInvalidProof, "proof commitment can not be empty") } - if msg.ProofHeight.IsZero() { - return errorsmod.Wrap(clienttypes.ErrInvalidHeight, "proof height can not be zero") - } - _, err := sdk.AccAddressFromBech32(msg.Signer) if err != nil { return errorsmod.Wrapf(ibcerrors.ErrInvalidAddress, "string could not be parsed as address: %v", err) } - return nil + + return msg.Packet.ValidateBasic() } // NewMsgAcknowledgement creates a new MsgAcknowledgement instance diff --git a/modules/core/04-channel/v2/types/msgs_test.go b/modules/core/04-channel/v2/types/msgs_test.go index d089a5dc460..1cebdd19647 100644 --- a/modules/core/04-channel/v2/types/msgs_test.go +++ b/modules/core/04-channel/v2/types/msgs_test.go @@ -6,24 +6,23 @@ import ( "github.com/stretchr/testify/suite" - clienttypes "github.com/cosmos/ibc-go/v9/modules/core/02-client/types" channeltypesv1 "github.com/cosmos/ibc-go/v9/modules/core/04-channel/types" "github.com/cosmos/ibc-go/v9/modules/core/04-channel/v2/types" commitmenttypes "github.com/cosmos/ibc-go/v9/modules/core/23-commitment/types" host "github.com/cosmos/ibc-go/v9/modules/core/24-host" ibcerrors "github.com/cosmos/ibc-go/v9/modules/core/errors" ibctesting "github.com/cosmos/ibc-go/v9/testing" - "github.com/cosmos/ibc-go/v9/testing/mock" + mockv2 "github.com/cosmos/ibc-go/v9/testing/mock/v2" ) +var testProof = []byte("test") + type TypesTestSuite struct { suite.Suite coordinator *ibctesting.Coordinator chainA *ibctesting.TestChain chainB *ibctesting.TestChain - - proof []byte } func (s *TypesTestSuite) SetupTest() { @@ -241,13 +240,6 @@ func (s *TypesTestSuite) TestMsgRecvPacketValidateBasic() { }, expError: commitmenttypes.ErrInvalidProof, }, - { - name: "failure: invalid proof height", - malleate: func() { - msg.ProofHeight = clienttypes.ZeroHeight() - }, - expError: clienttypes.ErrInvalidHeight, - }, { name: "failure: invalid signer", malleate: func() { @@ -258,17 +250,9 @@ func (s *TypesTestSuite) TestMsgRecvPacketValidateBasic() { } for _, tc := range testCases { s.Run(tc.name, func() { - packet := types.NewPacket(1, - ibctesting.FirstChannelID, ibctesting.FirstChannelID, - s.chainA.GetTimeoutTimestamp(), - types.PacketData{ - SourcePort: ibctesting.MockPort, - DestinationPort: ibctesting.MockPort, - Payload: types.NewPayload("ics20-1", "json", mock.MockPacketData), - }, - ) + packet := types.NewPacket(1, ibctesting.FirstChannelID, ibctesting.SecondChannelID, s.chainA.GetTimeoutTimestamp(), mockv2.NewMockPacketData(mockv2.ModuleNameA, mockv2.ModuleNameB)) - msg = types.NewMsgRecvPacket(packet, []byte("foo"), s.chainA.GetTimeoutHeight(), s.chainA.SenderAccount.GetAddress().String()) + msg = types.NewMsgRecvPacket(packet, testProof, s.chainA.GetTimeoutHeight(), s.chainA.SenderAccount.GetAddress().String()) tc.malleate() diff --git a/modules/core/keeper/events_test.go b/modules/core/keeper/events_test.go index 1143782ddc8..359322833f0 100644 --- a/modules/core/keeper/events_test.go +++ b/modules/core/keeper/events_test.go @@ -2,6 +2,7 @@ package keeper_test import ( "testing" + "github.com/stretchr/testify/require" sdk "github.com/cosmos/cosmos-sdk/types" diff --git a/modules/core/keeper/msg_server.go b/modules/core/keeper/msg_server.go index 92348c0de03..7b987945e61 100644 --- a/modules/core/keeper/msg_server.go +++ b/modules/core/keeper/msg_server.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + errorsmod "cosmossdk.io/errors" sdk "github.com/cosmos/cosmos-sdk/types" diff --git a/modules/core/keeper/msg_server_test.go b/modules/core/keeper/msg_server_test.go index bea3fccc45a..547d5702304 100644 --- a/modules/core/keeper/msg_server_test.go +++ b/modules/core/keeper/msg_server_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + upgradetypes "cosmossdk.io/x/upgrade/types" sdk "github.com/cosmos/cosmos-sdk/types"