diff --git a/modules/core/04-channel/v2/client/cli/cli.go b/modules/core/04-channel/v2/client/cli/cli.go index 5c5a897b0f9..69952be9f81 100644 --- a/modules/core/04-channel/v2/client/cli/cli.go +++ b/modules/core/04-channel/v2/client/cli/cli.go @@ -5,14 +5,14 @@ import ( "github.com/cosmos/cosmos-sdk/client" - "github.com/cosmos/ibc-go/v9/modules/core/packet-server/types" + "github.com/cosmos/ibc-go/v9/modules/core/04-channel/v2/types" ) -// GetQueryCmd returns the query commands for the IBC packet-server. +// GetQueryCmd returns the query commands for the IBC channel/v2. func GetQueryCmd() *cobra.Command { queryCmd := &cobra.Command{ Use: types.SubModuleName, - Short: "IBC packet-server query subcommands", + Short: "IBC channel/v2 query subcommands", DisableFlagParsing: true, SuggestionsMinimumDistance: 2, RunE: client.ValidateCmd, @@ -25,18 +25,19 @@ func GetQueryCmd() *cobra.Command { return queryCmd } -// NewTxCmd returns the command to submit transactions defined for the IBC packet-server. +// NewTxCmd returns the command to submit transactions defined for IBC channel/v2. func NewTxCmd() *cobra.Command { txCmd := &cobra.Command{ Use: types.SubModuleName, - Short: "IBC packet-server transaction subcommands", + Short: "IBC channel/v2 transaction subcommands", DisableFlagParsing: true, SuggestionsMinimumDistance: 2, RunE: client.ValidateCmd, } txCmd.AddCommand( - newProvideCounterpartyCmd(), + newCreateChannelTxCmd(), + newProvideCounterpartyTxCmd(), ) return txCmd diff --git a/modules/core/04-channel/v2/client/cli/tx.go b/modules/core/04-channel/v2/client/cli/tx.go index 9ff268d4843..418f01815e7 100644 --- a/modules/core/04-channel/v2/client/cli/tx.go +++ b/modules/core/04-channel/v2/client/cli/tx.go @@ -1,7 +1,9 @@ package cli import ( + "encoding/hex" "fmt" + "strings" "github.com/spf13/cobra" @@ -11,11 +13,42 @@ import ( "github.com/cosmos/cosmos-sdk/version" "github.com/cosmos/ibc-go/v9/modules/core/04-channel/v2/types" + commitmenttypesv2 "github.com/cosmos/ibc-go/v9/modules/core/23-commitment/types/v2" "github.com/cosmos/ibc-go/v9/modules/core/exported" ) +// newCreateChannelTxCmd defines the command to create an IBC channel/v2. +func newCreateChannelTxCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "create-channel [client-identifier] [merkle-path-prefix]", + Args: cobra.ExactArgs(2), + Short: "create an IBC channel/v2", + Long: `Creates an IBC channel/v2 using the client identifier representing the counterparty chain and the hex-encoded merkle path prefix under which the counterparty stores packet flow information.`, + Example: fmt.Sprintf("%s tx %s %s create-channel 07-tendermint-0 696263,657572656b61", version.AppName, exported.ModuleName, types.SubModuleName), + RunE: func(cmd *cobra.Command, args []string) error { + clientCtx, err := client.GetClientTxContext(cmd) + if err != nil { + return err + } + + clientID := args[0] + merklePathPrefix, err := parseMerklePathPrefix(args[2]) + if err != nil { + return err + } + + msg := types.NewMsgCreateChannel(clientID, merklePathPrefix, clientCtx.GetFromAddress().String()) + + return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), msg) + }, + } + + flags.AddTxFlagsToCmd(cmd) + return cmd +} + // newProvideCounterpartyCmd defines the command to provide the counterparty channel identifier to an IBC channel. -func newProvideCounterpartyCmd() *cobra.Command { +func newProvideCounterpartyTxCmd() *cobra.Command { cmd := &cobra.Command{ Use: "provide-counterparty [channel-identifier] [counterparty-channel-identifier]", Args: cobra.ExactArgs(2), @@ -43,3 +76,18 @@ func newProvideCounterpartyCmd() *cobra.Command { flags.AddTxFlagsToCmd(cmd) return cmd } + +// parseMerklePathPrefix parses a comma-separated list of hex-encoded strings into a MerklePath. +func parseMerklePathPrefix(merklePathPrefixString string) (commitmenttypesv2.MerklePath, error) { + var keyPath [][]byte + hexPrefixes := strings.Split(merklePathPrefixString, ",") + for _, hexPrefix := range hexPrefixes { + prefix, err := hex.DecodeString(hexPrefix) + if err != nil { + return commitmenttypesv2.MerklePath{}, fmt.Errorf("invalid hex merkle path prefix: %w", err) + } + keyPath = append(keyPath, prefix) + } + + return commitmenttypesv2.MerklePath{KeyPath: keyPath}, nil +} diff --git a/modules/core/04-channel/v2/keeper/grpc_query.go b/modules/core/04-channel/v2/keeper/grpc_query.go index fde351da818..10343b516ab 100644 --- a/modules/core/04-channel/v2/keeper/grpc_query.go +++ b/modules/core/04-channel/v2/keeper/grpc_query.go @@ -16,7 +16,7 @@ import ( var _ types.QueryServer = (*queryServer)(nil) -// queryServer implements the packet-server types.QueryServer interface. +// queryServer implements the channel/v2 types.QueryServer interface. type queryServer struct { *Keeper } diff --git a/modules/core/04-channel/v2/keeper/msg_server.go b/modules/core/04-channel/v2/keeper/msg_server.go index 123fd44310e..7356040ccee 100644 --- a/modules/core/04-channel/v2/keeper/msg_server.go +++ b/modules/core/04-channel/v2/keeper/msg_server.go @@ -11,8 +11,8 @@ import ( channeltypesv1 "github.com/cosmos/ibc-go/v9/modules/core/04-channel/types" channeltypesv2 "github.com/cosmos/ibc-go/v9/modules/core/04-channel/v2/types" ibcerrors "github.com/cosmos/ibc-go/v9/modules/core/errors" + internalerrors "github.com/cosmos/ibc-go/v9/modules/core/internal/errors" telemetryv2 "github.com/cosmos/ibc-go/v9/modules/core/internal/v2/telemetry" - coretypes "github.com/cosmos/ibc-go/v9/modules/core/types" ) var _ channeltypesv2.MsgServer = &Keeper{} @@ -127,7 +127,7 @@ func (k *Keeper) RecvPacket(ctx context.Context, msg *channeltypesv2.MsgRecvPack writeFn() } else { // Modify events in cached context to reflect unsuccessful acknowledgement - sdkCtx.EventManager().EmitEvents(convertToErrorEvents(cacheCtx.EventManager().Events())) + sdkCtx.EventManager().EmitEvents(internalerrors.ConvertToErrorEvents(cacheCtx.EventManager().Events())) } ack.AcknowledgementResults = append(ack.AcknowledgementResults, channeltypesv2.AcknowledgementResult{ @@ -233,24 +233,3 @@ func (k *Keeper) ProvideCounterparty(goCtx context.Context, msg *channeltypesv2. return &channeltypesv2.MsgProvideCounterpartyResponse{}, nil } - -// convertToErrorEvents converts all events to error events by appending the -// error attribute prefix to each event's attribute key. -// TODO: https://github.com/cosmos/ibc-go/issues/7436 -func convertToErrorEvents(events sdk.Events) sdk.Events { - if events == nil { - return nil - } - - newEvents := make(sdk.Events, len(events)) - for i, event := range events { - newAttributes := make([]sdk.Attribute, len(event.Attributes)) - for j, attribute := range event.Attributes { - newAttributes[j] = sdk.NewAttribute(coretypes.ErrorAttributeKeyPrefix+attribute.Key, attribute.Value) - } - - newEvents[i] = sdk.NewEvent(coretypes.ErrorAttributeKeyPrefix+event.Type, newAttributes...) - } - - return newEvents -} diff --git a/modules/core/04-channel/v2/keeper/msg_server_test.go b/modules/core/04-channel/v2/keeper/msg_server_test.go index 83eb1e8b9ff..a75dfdee074 100644 --- a/modules/core/04-channel/v2/keeper/msg_server_test.go +++ b/modules/core/04-channel/v2/keeper/msg_server_test.go @@ -11,7 +11,6 @@ import ( channeltypesv1 "github.com/cosmos/ibc-go/v9/modules/core/04-channel/types" channeltypesv2 "github.com/cosmos/ibc-go/v9/modules/core/04-channel/v2/types" commitmenttypes "github.com/cosmos/ibc-go/v9/modules/core/23-commitment/types" - hostv2 "github.com/cosmos/ibc-go/v9/modules/core/24-host/v2" 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" @@ -20,9 +19,10 @@ import ( func (suite *KeeperTestSuite) TestMsgSendPacket() { var ( - path *ibctesting.Path - msg *channeltypesv2.MsgSendPacket - expectedPacket channeltypesv2.Packet + path *ibctesting.Path + expectedPacket channeltypesv2.Packet + timeoutTimestamp uint64 + packetData channeltypesv2.PacketData ) testCases := []struct { @@ -39,7 +39,7 @@ func (suite *KeeperTestSuite) TestMsgSendPacket() { name: "failure: timeout elapsed", malleate: func() { // ensure a message timeout. - msg.TimeoutTimestamp = uint64(1) + timeoutTimestamp = uint64(1) }, expError: channeltypesv1.ErrTimeoutElapsed, }, @@ -62,14 +62,14 @@ func (suite *KeeperTestSuite) TestMsgSendPacket() { { name: "failure: counterparty not found", malleate: func() { - msg.SourceChannel = "foo" + path.EndpointA.ChannelID = "foo" }, expError: channeltypesv1.ErrChannelNotFound, }, { name: "failure: route to non existing app", malleate: func() { - msg.PacketData[0].SourcePort = "foo" + packetData.SourcePort = "foo" }, expError: fmt.Errorf("no route for foo"), }, @@ -84,19 +84,19 @@ func (suite *KeeperTestSuite) TestMsgSendPacket() { path = ibctesting.NewPath(suite.chainA, suite.chainB) path.SetupV2() - timeoutTimestamp := suite.chainA.GetTimeoutTimestamp() - msg = channeltypesv2.NewMsgSendPacket(path.EndpointA.ChannelID, timeoutTimestamp, suite.chainA.SenderAccount.GetAddress().String(), mockv2.NewMockPacketData(mockv2.ModuleNameA, mockv2.ModuleNameB)) + timeoutTimestamp = suite.chainA.GetTimeoutTimestamp() + packetData = mockv2.NewMockPacketData(mockv2.ModuleNameA, mockv2.ModuleNameB) - expectedPacket = channeltypesv2.NewPacket(1, path.EndpointA.ChannelID, path.EndpointB.ChannelID, timeoutTimestamp, mockv2.NewMockPacketData(mockv2.ModuleNameA, mockv2.ModuleNameB)) + expectedPacket = channeltypesv2.NewPacket(1, path.EndpointA.ChannelID, path.EndpointB.ChannelID, timeoutTimestamp, packetData) tc.malleate() - res, err := path.EndpointA.Chain.SendMsgs(msg) + packet, err := path.EndpointA.MsgSendPacket(timeoutTimestamp, packetData) expPass := tc.expError == nil if expPass { suite.Require().NoError(err) - suite.Require().NotNil(res) + suite.Require().NotEmpty(packet) ck := path.EndpointA.Chain.GetSimApp().IBCKeeper.ChannelKeeperV2 @@ -108,6 +108,8 @@ func (suite *KeeperTestSuite) TestMsgSendPacket() { suite.Require().True(ok) suite.Require().Equal(uint64(2), nextSequenceSend, "next sequence send was not incremented correctly") + suite.Require().Equal(expectedPacket, packet) + } else { suite.Require().Error(err) ibctesting.RequireErrorIsOrContains(suite.T(), err, tc.expError) @@ -119,8 +121,7 @@ func (suite *KeeperTestSuite) TestMsgSendPacket() { func (suite *KeeperTestSuite) TestMsgRecvPacket() { var ( path *ibctesting.Path - msg *channeltypesv2.MsgRecvPacket - recvPacket channeltypesv2.Packet + packet channeltypesv2.Packet expectedAck channeltypesv2.Acknowledgement ) @@ -169,7 +170,7 @@ func (suite *KeeperTestSuite) TestMsgRecvPacket() { { name: "success: NoOp", malleate: func() { - suite.chainB.App.GetIBCKeeper().ChannelKeeperV2.SetPacketReceipt(suite.chainB.GetContext(), recvPacket.SourceChannel, recvPacket.Sequence) + suite.chainB.App.GetIBCKeeper().ChannelKeeperV2.SetPacketReceipt(suite.chainB.GetContext(), packet.SourceChannel, packet.Sequence) expectedAck = channeltypesv2.Acknowledgement{} }, }, @@ -177,7 +178,7 @@ func (suite *KeeperTestSuite) TestMsgRecvPacket() { name: "failure: counterparty not found", malleate: func() { // change the destination id to a non-existent channel. - recvPacket.DestinationChannel = "not-existent-channel" + packet.DestinationChannel = "not-existent-channel" }, expError: channeltypesv2.ErrChannelNotFound, }, @@ -185,7 +186,7 @@ func (suite *KeeperTestSuite) TestMsgRecvPacket() { name: "failure: invalid proof", malleate: func() { // proof verification fails because the packet commitment is different due to a different sequence. - recvPacket.Sequence = 10 + packet.Sequence = 10 }, expError: commitmenttypes.ErrInvalidProof, }, @@ -201,15 +202,10 @@ func (suite *KeeperTestSuite) TestMsgRecvPacket() { path.SetupV2() timeoutTimestamp := suite.chainA.GetTimeoutTimestamp() - msgSendPacket := channeltypesv2.NewMsgSendPacket(path.EndpointA.ChannelID, timeoutTimestamp, suite.chainA.SenderAccount.GetAddress().String(), mockv2.NewMockPacketData(mockv2.ModuleNameA, mockv2.ModuleNameB)) - res, err := path.EndpointA.Chain.SendMsgs(msgSendPacket) + var err error + packet, err = path.EndpointA.MsgSendPacket(timeoutTimestamp, mockv2.NewMockPacketData(mockv2.ModuleNameA, mockv2.ModuleNameB)) suite.Require().NoError(err) - suite.Require().NotNil(res) - - suite.Require().NoError(path.EndpointB.UpdateClient()) - - recvPacket = channeltypesv2.NewPacket(1, path.EndpointA.ChannelID, path.EndpointB.ChannelID, timeoutTimestamp, mockv2.NewMockPacketData(mockv2.ModuleNameA, mockv2.ModuleNameB)) // default expected ack is a single successful recv result for moduleB. expectedAck = channeltypesv2.Acknowledgement{ @@ -226,27 +222,19 @@ func (suite *KeeperTestSuite) TestMsgRecvPacket() { tc.malleate() - // get proof of packet commitment from chainA - packetKey := hostv2.PacketCommitmentKey(recvPacket.SourceChannel, recvPacket.Sequence) - proof, proofHeight := path.EndpointA.QueryProof(packetKey) - - msg = channeltypesv2.NewMsgRecvPacket(recvPacket, proof, proofHeight, suite.chainB.SenderAccount.GetAddress().String()) - - res, err = path.EndpointB.Chain.SendMsgs(msg) - suite.Require().NoError(path.EndpointA.UpdateClient()) + err = path.EndpointB.MsgRecvPacket(packet) ck := path.EndpointB.Chain.GetSimApp().IBCKeeper.ChannelKeeperV2 expPass := tc.expError == nil if expPass { suite.Require().NoError(err) - suite.Require().NotNil(res) // packet receipt should be written - _, ok := ck.GetPacketReceipt(path.EndpointB.Chain.GetContext(), recvPacket.SourceChannel, recvPacket.Sequence) + _, ok := ck.GetPacketReceipt(path.EndpointB.Chain.GetContext(), packet.SourceChannel, packet.Sequence) suite.Require().True(ok) - ackWritten := ck.HasPacketAcknowledgement(path.EndpointB.Chain.GetContext(), recvPacket.DestinationChannel, recvPacket.Sequence) + ackWritten := ck.HasPacketAcknowledgement(path.EndpointB.Chain.GetContext(), packet.DestinationChannel, packet.Sequence) if len(expectedAck.AcknowledgementResults) == 0 || expectedAck.AcknowledgementResults[0].RecvPacketResult.Status == channeltypesv2.PacketStatus_Async { // ack should not be written for async app or if the packet receipt was already present. @@ -256,13 +244,13 @@ func (suite *KeeperTestSuite) TestMsgRecvPacket() { suite.Require().True(ackWritten) expectedBz := channeltypesv2.CommitAcknowledgement(expectedAck) - actualAckBz := ck.GetPacketAcknowledgement(path.EndpointB.Chain.GetContext(), recvPacket.DestinationChannel, recvPacket.Sequence) + actualAckBz := ck.GetPacketAcknowledgement(path.EndpointB.Chain.GetContext(), packet.DestinationChannel, packet.Sequence) suite.Require().Equal(expectedBz, actualAckBz) } } else { ibctesting.RequireErrorIsOrContains(suite.T(), err, tc.expError) - _, ok := ck.GetPacketReceipt(path.EndpointB.Chain.GetContext(), recvPacket.SourceChannel, recvPacket.Sequence) + _, ok := ck.GetPacketReceipt(path.EndpointB.Chain.GetContext(), packet.SourceChannel, packet.Sequence) suite.Require().False(ok) } }) @@ -344,9 +332,9 @@ func (suite *KeeperTestSuite) TestProvideCounterparty() { func (suite *KeeperTestSuite) TestMsgAcknowledgement() { var ( - path *ibctesting.Path - msgAckPacket *channeltypesv2.MsgAcknowledgement - recvPacket channeltypesv2.Packet + path *ibctesting.Path + packet channeltypesv2.Packet + ack channeltypesv2.Acknowledgement ) testCases := []struct { name string @@ -360,7 +348,7 @@ func (suite *KeeperTestSuite) TestMsgAcknowledgement() { { name: "success: NoOp", malleate: func() { - suite.chainA.App.GetIBCKeeper().ChannelKeeperV2.SetPacketCommitment(suite.chainA.GetContext(), recvPacket.SourceChannel, recvPacket.Sequence, []byte{}) + suite.chainA.App.GetIBCKeeper().ChannelKeeperV2.SetPacketCommitment(suite.chainA.GetContext(), packet.SourceChannel, packet.Sequence, []byte{}) // Modify the callback to return an error. // This way, we can verify that the callback is not executed in a No-op case. @@ -369,13 +357,6 @@ func (suite *KeeperTestSuite) TestMsgAcknowledgement() { } }, }, - { - name: "failure: invalid signer", - malleate: func() { - msgAckPacket.Signer = "" - }, - expError: errors.New("empty address string is not allowed"), - }, { name: "failure: callback fails", malleate: func() { @@ -389,21 +370,21 @@ func (suite *KeeperTestSuite) TestMsgAcknowledgement() { name: "failure: counterparty not found", malleate: func() { // change the source id to a non-existent channel. - msgAckPacket.Packet.SourceChannel = "not-existent-channel" + packet.SourceChannel = "not-existent-channel" }, expError: channeltypesv2.ErrChannelNotFound, }, { name: "failure: invalid commitment", malleate: func() { - suite.chainA.App.GetIBCKeeper().ChannelKeeperV2.SetPacketCommitment(suite.chainA.GetContext(), recvPacket.SourceChannel, recvPacket.Sequence, []byte("foo")) + suite.chainA.App.GetIBCKeeper().ChannelKeeperV2.SetPacketCommitment(suite.chainA.GetContext(), packet.SourceChannel, packet.Sequence, []byte("foo")) }, expError: channeltypesv2.ErrInvalidPacket, }, { name: "failure: failed membership verification", malleate: func() { - msgAckPacket.ProofHeight = clienttypes.ZeroHeight() + ack.AcknowledgementResults[0].RecvPacketResult.Acknowledgement = mock.MockFailPacketData }, expError: errors.New("failed packet acknowledgement verification"), }, @@ -417,28 +398,16 @@ func (suite *KeeperTestSuite) TestMsgAcknowledgement() { timeoutTimestamp := suite.chainA.GetTimeoutTimestamp() + var err error // Send packet from A to B - msgSendPacket := channeltypesv2.NewMsgSendPacket(path.EndpointA.ChannelID, timeoutTimestamp, suite.chainA.SenderAccount.GetAddress().String(), mockv2.NewMockPacketData(mockv2.ModuleNameA, mockv2.ModuleNameB)) - res, err := path.EndpointA.Chain.SendMsgs(msgSendPacket) + packet, err = path.EndpointA.MsgSendPacket(timeoutTimestamp, mockv2.NewMockPacketData(mockv2.ModuleNameA, mockv2.ModuleNameB)) suite.Require().NoError(err) - suite.Require().NotNil(res) - suite.Require().NoError(path.EndpointB.UpdateClient()) - // Receive packet on B - recvPacket = channeltypesv2.NewPacket(1, path.EndpointA.ChannelID, path.EndpointB.ChannelID, timeoutTimestamp, mockv2.NewMockPacketData(mockv2.ModuleNameA, mockv2.ModuleNameB)) - // get proof of packet commitment from chainA - packetKey := hostv2.PacketCommitmentKey(recvPacket.SourceChannel, recvPacket.Sequence) - proof, proofHeight := path.EndpointA.QueryProof(packetKey) - - // Construct msgRecvPacket to be sent to B - msgRecvPacket := channeltypesv2.NewMsgRecvPacket(recvPacket, proof, proofHeight, suite.chainB.SenderAccount.GetAddress().String()) - res, err = suite.chainB.SendMsgs(msgRecvPacket) + err = path.EndpointB.MsgRecvPacket(packet) suite.Require().NoError(err) - suite.Require().NotNil(res) - suite.Require().NoError(path.EndpointA.UpdateClient()) // Construct expected acknowledgement - ack := channeltypesv2.Acknowledgement{ + ack = channeltypesv2.Acknowledgement{ AcknowledgementResults: []channeltypesv2.AcknowledgementResult{ { AppName: mockv2.ModuleNameB, @@ -450,21 +419,14 @@ func (suite *KeeperTestSuite) TestMsgAcknowledgement() { }, } - // Consttruct MsgAcknowledgement - packetKey = hostv2.PacketAcknowledgementKey(recvPacket.DestinationChannel, recvPacket.Sequence) - proof, proofHeight = path.EndpointB.QueryProof(packetKey) - msgAckPacket = channeltypesv2.NewMsgAcknowledgement(recvPacket, ack, proof, proofHeight, suite.chainA.SenderAccount.GetAddress().String()) - tc.malleate() // Finally, acknowledge the packet on A - res, err = suite.chainA.SendMsgs(msgAckPacket) + err = path.EndpointA.MsgAcknowledgePacket(packet, ack) expPass := tc.expError == nil - if expPass { suite.Require().NoError(err) - suite.NotNil(res) } else { ibctesting.RequireErrorIsOrContains(suite.T(), err, tc.expError, "expected error %q, got %q instead", tc.expError, err) } diff --git a/modules/core/04-channel/v2/types/errors.go b/modules/core/04-channel/v2/types/errors.go index bb112a09696..2873867295b 100644 --- a/modules/core/04-channel/v2/types/errors.go +++ b/modules/core/04-channel/v2/types/errors.go @@ -10,4 +10,5 @@ var ( ErrInvalidPacket = errorsmod.Register(SubModuleName, 4, "invalid packet") ErrInvalidPayload = errorsmod.Register(SubModuleName, 5, "invalid payload") ErrSequenceSendNotFound = errorsmod.Register(SubModuleName, 6, "sequence send not found") + ErrInvalidPacketData = errorsmod.Register(SubModuleName, 7, "invalid packet data") ) diff --git a/modules/core/04-channel/v2/types/msgs.go b/modules/core/04-channel/v2/types/msgs.go index 1a59c7a10f2..2afc72fd45f 100644 --- a/modules/core/04-channel/v2/types/msgs.go +++ b/modules/core/04-channel/v2/types/msgs.go @@ -8,6 +8,7 @@ import ( clienttypes "github.com/cosmos/ibc-go/v9/modules/core/02-client/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" ) @@ -82,6 +83,34 @@ func NewMsgSendPacket(sourceChannel string, timeoutTimestamp uint64, signer stri } } +// ValidateBasic performs basic checks on a MsgSendPacket. +func (msg *MsgSendPacket) ValidateBasic() error { + if err := host.ChannelIdentifierValidator(msg.SourceChannel); err != nil { + return err + } + + if msg.TimeoutTimestamp == 0 { + return errorsmod.Wrap(channeltypesv1.ErrInvalidTimeout, "timeout must not be 0") + } + + if len(msg.PacketData) != 1 { + return errorsmod.Wrapf(ErrInvalidPacketData, "packet data must be of length 1, got %d instead", len(msg.PacketData)) + } + + for _, pd := range msg.PacketData { + if err := pd.ValidateBasic(); err != nil { + return err + } + } + + _, err := sdk.AccAddressFromBech32(msg.Signer) + if err != nil { + return errorsmod.Wrapf(ibcerrors.ErrInvalidAddress, "string could not be parsed as address: %v", err) + } + + return nil +} + // NewMsgRecvPacket creates a new MsgRecvPacket instance. func NewMsgRecvPacket(packet Packet, proofCommitment []byte, proofHeight clienttypes.Height, signer string) *MsgRecvPacket { return &MsgRecvPacket{ diff --git a/modules/core/04-channel/v2/types/msgs_test.go b/modules/core/04-channel/v2/types/msgs_test.go index 55408bde52e..e649869d907 100644 --- a/modules/core/04-channel/v2/types/msgs_test.go +++ b/modules/core/04-channel/v2/types/msgs_test.go @@ -7,6 +7,7 @@ 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" @@ -147,8 +148,8 @@ func (s *TypesTestSuite) TestMsgCreateChannelValidateBasic() { } } -func (s *TypesTestSuite) TestMsgRecvPacketValidateBasic() { - var msg *types.MsgRecvPacket +func (s *TypesTestSuite) TestMsgSendPacketValidateBasic() { + var msg *types.MsgSendPacket testCases := []struct { name string malleate func() @@ -159,25 +160,32 @@ func (s *TypesTestSuite) TestMsgRecvPacketValidateBasic() { malleate: func() {}, }, { - name: "failure: invalid packet", + name: "failure: invalid source channel", malleate: func() { - msg.Packet.Data = []types.PacketData{} + msg.SourceChannel = "" }, - expError: types.ErrInvalidPacket, + expError: host.ErrInvalidID, }, { - name: "failure: invalid proof commitment", + name: "failure: invalid timestamp", malleate: func() { - msg.ProofCommitment = []byte{} + msg.TimeoutTimestamp = 0 }, - expError: commitmenttypes.ErrInvalidProof, + expError: channeltypesv1.ErrInvalidTimeout, }, { - name: "failure: invalid proof height", + name: "failure: invalid length for packetdata", malleate: func() { - msg.ProofHeight = clienttypes.ZeroHeight() + msg.PacketData = []types.PacketData{} }, - expError: clienttypes.ErrInvalidHeight, + expError: types.ErrInvalidPacketData, + }, + { + name: "failure: invalid packetdata", + malleate: func() { + msg.PacketData[0].DestinationPort = "" + }, + expError: host.ErrInvalidID, }, { name: "failure: invalid signer", @@ -189,24 +197,16 @@ 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), - }, + msg = types.NewMsgSendPacket( + ibctesting.FirstChannelID, s.chainA.GetTimeoutTimestamp(), + s.chainA.SenderAccount.GetAddress().String(), + types.PacketData{SourcePort: ibctesting.MockPort, DestinationPort: ibctesting.MockPort, Payload: types.NewPayload("ics20-1", "json", ibctesting.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 { diff --git a/modules/core/internal/errors/errors.go b/modules/core/internal/errors/errors.go new file mode 100644 index 00000000000..99acf8c02a5 --- /dev/null +++ b/modules/core/internal/errors/errors.go @@ -0,0 +1,25 @@ +package errors + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + + coretypes "github.com/cosmos/ibc-go/v9/modules/core/types" +) + +// ConvertToErrorEvents converts all events to error events by appending the +// error attribute prefix to each event's attribute key. +func ConvertToErrorEvents(events sdk.Events) sdk.Events { + if events == nil { + return nil + } + + newEvents := make(sdk.Events, len(events)) + for i, event := range events { + newEvents[i] = sdk.NewEvent(coretypes.ErrorAttributeKeyPrefix + event.Type) + for _, attribute := range event.Attributes { + newEvents[i] = newEvents[i].AppendAttributes(sdk.NewAttribute(coretypes.ErrorAttributeKeyPrefix+attribute.Key, attribute.Value)) + } + } + + return newEvents +} diff --git a/modules/core/keeper/events_test.go b/modules/core/keeper/events_test.go index 98915ac13c1..1143782ddc8 100644 --- a/modules/core/keeper/events_test.go +++ b/modules/core/keeper/events_test.go @@ -2,12 +2,11 @@ package keeper_test import ( "testing" - "github.com/stretchr/testify/require" sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/ibc-go/v9/modules/core/keeper" + internalerrors "github.com/cosmos/ibc-go/v9/modules/core/internal/errors" "github.com/cosmos/ibc-go/v9/modules/core/types" ) @@ -99,7 +98,7 @@ func TestConvertToErrorEvents(t *testing.T) { tc.malleate() - newEvents := keeper.ConvertToErrorEvents(events) + newEvents := internalerrors.ConvertToErrorEvents(events) require.Equal(t, expEvents, newEvents) }) } diff --git a/modules/core/keeper/export_test.go b/modules/core/keeper/export_test.go deleted file mode 100644 index 81d79ea7aa9..00000000000 --- a/modules/core/keeper/export_test.go +++ /dev/null @@ -1,9 +0,0 @@ -package keeper - -import sdk "github.com/cosmos/cosmos-sdk/types" - -// ConvertToErrorEvents is a wrapper around convertToErrorEvents -// to allow the function to be directly called in tests. -func ConvertToErrorEvents(events sdk.Events) sdk.Events { - return convertToErrorEvents(events) -} diff --git a/modules/core/keeper/msg_server.go b/modules/core/keeper/msg_server.go index a59bfc28490..92348c0de03 100644 --- a/modules/core/keeper/msg_server.go +++ b/modules/core/keeper/msg_server.go @@ -4,7 +4,6 @@ import ( "context" "errors" "fmt" - errorsmod "cosmossdk.io/errors" sdk "github.com/cosmos/cosmos-sdk/types" @@ -15,8 +14,8 @@ import ( channeltypes "github.com/cosmos/ibc-go/v9/modules/core/04-channel/types" porttypes "github.com/cosmos/ibc-go/v9/modules/core/05-port/types" ibcerrors "github.com/cosmos/ibc-go/v9/modules/core/errors" + internalerrors "github.com/cosmos/ibc-go/v9/modules/core/internal/errors" "github.com/cosmos/ibc-go/v9/modules/core/internal/telemetry" - coretypes "github.com/cosmos/ibc-go/v9/modules/core/types" ) var ( @@ -442,7 +441,7 @@ func (k *Keeper) RecvPacket(goCtx context.Context, msg *channeltypes.MsgRecvPack writeFn() } else { // Modify events in cached context to reflect unsuccessful acknowledgement - ctx.EventManager().EmitEvents(convertToErrorEvents(cacheCtx.EventManager().Events())) + ctx.EventManager().EmitEvents(internalerrors.ConvertToErrorEvents(cacheCtx.EventManager().Events())) } // Set packet acknowledgement only if the acknowledgement is not nil. @@ -999,26 +998,6 @@ func (k *Keeper) UpdateChannelParams(goCtx context.Context, msg *channeltypes.Ms return &channeltypes.MsgUpdateParamsResponse{}, nil } -// convertToErrorEvents converts all events to error events by appending the -// error attribute prefix to each event's attribute key. -func convertToErrorEvents(events sdk.Events) sdk.Events { - if events == nil { - return nil - } - - newEvents := make(sdk.Events, len(events)) - for i, event := range events { - newAttributes := make([]sdk.Attribute, len(event.Attributes)) - for j, attribute := range event.Attributes { - newAttributes[j] = sdk.NewAttribute(coretypes.ErrorAttributeKeyPrefix+attribute.Key, attribute.Value) - } - - newEvents[i] = sdk.NewEvent(coretypes.ErrorAttributeKeyPrefix+event.Type, newAttributes...) - } - - return newEvents -} - // getPacketHandlerAndModule returns the appropriate packet handler // given the protocol version. An error is returned if the protocol // version is not supported. diff --git a/modules/core/keeper/msg_server_test.go b/modules/core/keeper/msg_server_test.go index e101d57ed06..bea3fccc45a 100644 --- a/modules/core/keeper/msg_server_test.go +++ b/modules/core/keeper/msg_server_test.go @@ -4,7 +4,6 @@ import ( "context" "errors" "fmt" - upgradetypes "cosmossdk.io/x/upgrade/types" sdk "github.com/cosmos/cosmos-sdk/types" @@ -20,7 +19,7 @@ import ( host "github.com/cosmos/ibc-go/v9/modules/core/24-host" ibcerrors "github.com/cosmos/ibc-go/v9/modules/core/errors" "github.com/cosmos/ibc-go/v9/modules/core/exported" - "github.com/cosmos/ibc-go/v9/modules/core/keeper" + internalerrors "github.com/cosmos/ibc-go/v9/modules/core/internal/errors" packetservertypes "github.com/cosmos/ibc-go/v9/modules/core/packet-server/types" ibctm "github.com/cosmos/ibc-go/v9/modules/light-clients/07-tendermint" ibctesting "github.com/cosmos/ibc-go/v9/testing" @@ -186,13 +185,13 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() { if tc.expRevert { // context events should contain error events - suite.Require().Contains(events, keeper.ConvertToErrorEvents(sdk.Events{ibcmock.NewMockRecvPacketEvent()})[0]) + suite.Require().Contains(events, internalerrors.ConvertToErrorEvents(sdk.Events{ibcmock.NewMockRecvPacketEvent()})[0]) suite.Require().NotContains(events, ibcmock.NewMockRecvPacketEvent()) } else { if tc.replay { // context should not contain application events suite.Require().NotContains(events, ibcmock.NewMockRecvPacketEvent()) - suite.Require().NotContains(events, keeper.ConvertToErrorEvents(sdk.Events{ibcmock.NewMockRecvPacketEvent()})[0]) + suite.Require().NotContains(events, internalerrors.ConvertToErrorEvents(sdk.Events{ibcmock.NewMockRecvPacketEvent()})[0]) } else { // context events should contain application events suite.Require().Contains(events, ibcmock.NewMockRecvPacketEvent()) @@ -353,13 +352,13 @@ func (suite *KeeperTestSuite) TestRecvPacketV2() { if tc.expRevert { // context events should contain error events - suite.Require().Contains(events, keeper.ConvertToErrorEvents(sdk.Events{ibcmock.NewMockRecvPacketEvent()})[0]) + suite.Require().Contains(events, internalerrors.ConvertToErrorEvents(sdk.Events{ibcmock.NewMockRecvPacketEvent()})[0]) suite.Require().NotContains(events, ibcmock.NewMockRecvPacketEvent()) } else { if tc.replay { // context should not contain application events suite.Require().NotContains(events, ibcmock.NewMockRecvPacketEvent()) - suite.Require().NotContains(events, keeper.ConvertToErrorEvents(sdk.Events{ibcmock.NewMockRecvPacketEvent()})[0]) + suite.Require().NotContains(events, internalerrors.ConvertToErrorEvents(sdk.Events{ibcmock.NewMockRecvPacketEvent()})[0]) } else { // context events should contain application events suite.Require().Contains(events, ibcmock.NewMockRecvPacketEvent()) diff --git a/testing/endpoint_v2.go b/testing/endpoint_v2.go new file mode 100644 index 00000000000..6e2fac2dc79 --- /dev/null +++ b/testing/endpoint_v2.go @@ -0,0 +1,58 @@ +package ibctesting + +import ( + "github.com/stretchr/testify/require" + + channeltypesv2 "github.com/cosmos/ibc-go/v9/modules/core/04-channel/v2/types" + hostv2 "github.com/cosmos/ibc-go/v9/modules/core/24-host/v2" +) + +// MsgSendPacket sends a packet on the associated endpoint. The constructed packet is returned. +func (endpoint *Endpoint) MsgSendPacket(timeoutTimestamp uint64, packetData channeltypesv2.PacketData) (channeltypesv2.Packet, error) { + msgSendPacket := channeltypesv2.NewMsgSendPacket(endpoint.ChannelID, timeoutTimestamp, endpoint.Chain.SenderAccount.GetAddress().String(), packetData) + + _, err := endpoint.Chain.SendMsgs(msgSendPacket) + if err != nil { + return channeltypesv2.Packet{}, err + } + + if err := endpoint.Counterparty.UpdateClient(); err != nil { + return channeltypesv2.Packet{}, err + } + + // TODO: parse the packet from events instead of manually constructing it. https://github.com/cosmos/ibc-go/issues/7459 + nextSequenceSend, ok := endpoint.Chain.GetSimApp().IBCKeeper.ChannelKeeperV2.GetNextSequenceSend(endpoint.Chain.GetContext(), endpoint.ChannelID) + require.True(endpoint.Chain.TB, ok) + packet := channeltypesv2.NewPacket(nextSequenceSend-1, endpoint.ChannelID, endpoint.Counterparty.ChannelID, timeoutTimestamp, packetData) + + return packet, nil +} + +// MsgRecvPacket sends a MsgRecvPacket on the associated endpoint with the provided packet. +func (endpoint *Endpoint) MsgRecvPacket(packet channeltypesv2.Packet) error { + // get proof of packet commitment from chainA + packetKey := hostv2.PacketCommitmentKey(packet.SourceChannel, packet.Sequence) + proof, proofHeight := endpoint.Counterparty.QueryProof(packetKey) + + msg := channeltypesv2.NewMsgRecvPacket(packet, proof, proofHeight, endpoint.Chain.SenderAccount.GetAddress().String()) + + if err := endpoint.Chain.sendMsgs(msg); err != nil { + return err + } + + return endpoint.Counterparty.UpdateClient() +} + +// MsgAcknowledgePacket sends a MsgAcknowledgement on the associated endpoint with the provided packet and ack. +func (endpoint *Endpoint) MsgAcknowledgePacket(packet channeltypesv2.Packet, ack channeltypesv2.Acknowledgement) error { + packetKey := hostv2.PacketAcknowledgementKey(packet.DestinationChannel, packet.Sequence) + proof, proofHeight := endpoint.Counterparty.QueryProof(packetKey) + + msg := channeltypesv2.NewMsgAcknowledgement(packet, ack, proof, proofHeight, endpoint.Chain.SenderAccount.GetAddress().String()) + + if err := endpoint.Chain.sendMsgs(msg); err != nil { + return err + } + + return endpoint.Counterparty.UpdateClient() +}