From 0709f843151ad3bf9f0819eaae276ee5cb17695e Mon Sep 17 00:00:00 2001 From: bznein Date: Thu, 31 Oct 2024 14:15:21 +0000 Subject: [PATCH] chore: remove usage of v1 errors from v2 --- .../core/04-channel/v2/keeper/msg_server.go | 6 ++-- .../04-channel/v2/keeper/msg_server_test.go | 5 ++- modules/core/04-channel/v2/keeper/packet.go | 31 +++++++++---------- .../core/04-channel/v2/keeper/packet_test.go | 31 +++++++++---------- modules/core/04-channel/v2/types/errors.go | 7 +++++ modules/core/04-channel/v2/types/msgs.go | 3 +- modules/core/04-channel/v2/types/msgs_test.go | 3 +- 7 files changed, 44 insertions(+), 42 deletions(-) diff --git a/modules/core/04-channel/v2/keeper/msg_server.go b/modules/core/04-channel/v2/keeper/msg_server.go index 1acaf49e30c..e9fb74d8bdc 100644 --- a/modules/core/04-channel/v2/keeper/msg_server.go +++ b/modules/core/04-channel/v2/keeper/msg_server.go @@ -106,7 +106,7 @@ func (k *Keeper) RecvPacket(ctx context.Context, msg *channeltypesv2.MsgRecvPack switch err { case nil: writeFn() - case channeltypesv1.ErrNoOpMsg: + case channeltypesv2.ErrNoOpMsg: // no-ops do not need event emission as they will be ignored sdkCtx.Logger().Debug("no-op on redundant relay", "source-channel", msg.Packet.SourceChannel) return &channeltypesv2.MsgRecvPacketResponse{Result: channeltypesv1.NOOP}, nil @@ -181,7 +181,7 @@ func (k *Keeper) Acknowledgement(ctx context.Context, msg *channeltypesv2.MsgAck switch err { case nil: writeFn() - case channeltypesv1.ErrNoOpMsg: + case channeltypesv2.ErrNoOpMsg: // no-ops do not need event emission as they will be ignored sdkCtx.Logger().Debug("no-op on redundant relay", "source-channel", msg.Packet.SourceChannel) return &channeltypesv2.MsgAcknowledgementResponse{Result: channeltypesv1.NOOP}, nil @@ -225,7 +225,7 @@ func (k *Keeper) Timeout(ctx context.Context, timeout *channeltypesv2.MsgTimeout switch err { case nil: writeFn() - case channeltypesv1.ErrNoOpMsg: + case channeltypesv2.ErrNoOpMsg: // no-ops do not need event emission as they will be ignored sdkCtx.Logger().Debug("no-op on redundant relay", "source-channel", timeout.Packet.SourceChannel) return &channeltypesv2.MsgTimeoutResponse{Result: channeltypesv1.NOOP}, nil 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 fa690b8087e..0fad058d8f6 100644 --- a/modules/core/04-channel/v2/keeper/msg_server_test.go +++ b/modules/core/04-channel/v2/keeper/msg_server_test.go @@ -8,7 +8,6 @@ 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" 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" ibcerrors "github.com/cosmos/ibc-go/v9/modules/core/errors" @@ -122,7 +121,7 @@ func (suite *KeeperTestSuite) TestMsgSendPacket() { // ensure a message timeout. timeoutTimestamp = uint64(1) }, - expError: channeltypesv1.ErrTimeoutElapsed, + expError: channeltypesv2.ErrTimeoutElapsed, }, { name: "failure: inactive client", @@ -462,7 +461,7 @@ func (suite *KeeperTestSuite) TestMsgTimeout() { return mock.MockApplicationCallbackError } }, - expError: channeltypesv1.ErrNoOpMsg, + expError: channeltypesv2.ErrNoOpMsg, }, { name: "failure: callback fails", diff --git a/modules/core/04-channel/v2/keeper/packet.go b/modules/core/04-channel/v2/keeper/packet.go index b4e0516307d..6ba078490c5 100644 --- a/modules/core/04-channel/v2/keeper/packet.go +++ b/modules/core/04-channel/v2/keeper/packet.go @@ -10,7 +10,6 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" clienttypes "github.com/cosmos/ibc-go/v9/modules/core/02-client/types" - channeltypes "github.com/cosmos/ibc-go/v9/modules/core/04-channel/types" "github.com/cosmos/ibc-go/v9/modules/core/04-channel/v2/types" hostv2 "github.com/cosmos/ibc-go/v9/modules/core/24-host/v2" "github.com/cosmos/ibc-go/v9/modules/core/exported" @@ -49,7 +48,7 @@ func (k *Keeper) sendPacket( packet := types.NewPacket(sequence, sourceChannel, destChannel, timeoutTimestamp, payloads...) if err := packet.ValidateBasic(); err != nil { - return 0, "", errorsmod.Wrapf(channeltypes.ErrInvalidPacket, "constructed packet failed basic validation: %v", err) + return 0, "", errorsmod.Wrapf(types.ErrInvalidPacket, "constructed packet failed basic validation: %v", err) } // check that the client of counterparty chain is still active @@ -72,7 +71,7 @@ func (k *Keeper) sendPacket( // thus to compare them, we convert the packet timeout to nanoseconds timeoutTimestamp = types.TimeoutTimestampToNanos(packet.TimeoutTimestamp) if latestTimestamp >= timeoutTimestamp { - return 0, "", errorsmod.Wrapf(channeltypes.ErrTimeoutElapsed, "latest timestamp: %d, timeout timestamp: %d", latestTimestamp, timeoutTimestamp) + return 0, "", errorsmod.Wrapf(types.ErrTimeoutElapsed, "latest timestamp: %d, timeout timestamp: %d", latestTimestamp, timeoutTimestamp) } commitment := types.CommitPacket(packet) @@ -111,7 +110,7 @@ func (k *Keeper) recvPacket( } if channel.CounterpartyChannelId != packet.SourceChannel { - return errorsmod.Wrapf(channeltypes.ErrInvalidChannelIdentifier, "counterparty channel id (%s) does not match packet source channel id (%s)", channel.CounterpartyChannelId, packet.SourceChannel) + return errorsmod.Wrapf(types.ErrInvalidChannelIdentifier, "counterparty channel id (%s) does not match packet source channel id (%s)", channel.CounterpartyChannelId, packet.SourceChannel) } clientID := channel.ClientId @@ -120,7 +119,7 @@ func (k *Keeper) recvPacket( sdkCtx := sdk.UnwrapSDKContext(ctx) currentTimestamp := uint64(sdkCtx.BlockTime().Unix()) if currentTimestamp >= packet.TimeoutTimestamp { - return errorsmod.Wrapf(channeltypes.ErrTimeoutElapsed, "current timestamp: %d, timeout timestamp: %d", currentTimestamp, packet.TimeoutTimestamp) + return errorsmod.Wrapf(types.ErrTimeoutElapsed, "current timestamp: %d, timeout timestamp: %d", currentTimestamp, packet.TimeoutTimestamp) } // REPLAY PROTECTION: Packet receipts will indicate that a packet has already been received @@ -131,7 +130,7 @@ func (k *Keeper) recvPacket( // This error indicates that the packet has already been relayed. Core IBC will // treat this error as a no-op in order to prevent an entire relay transaction // from failing and consuming unnecessary fees. - return channeltypes.ErrNoOpMsg + return types.ErrNoOpMsg } path := hostv2.PacketCommitmentKey(packet.SourceChannel, packet.Sequence) @@ -176,18 +175,18 @@ func (k Keeper) WriteAcknowledgement( } if channel.CounterpartyChannelId != packet.SourceChannel { - return errorsmod.Wrapf(channeltypes.ErrInvalidChannelIdentifier, "counterparty channel id (%s) does not match packet source channel id (%s)", channel.CounterpartyChannelId, packet.SourceChannel) + return errorsmod.Wrapf(types.ErrInvalidChannelIdentifier, "counterparty channel id (%s) does not match packet source channel id (%s)", channel.CounterpartyChannelId, packet.SourceChannel) } // NOTE: IBC app modules might have written the acknowledgement synchronously on // the OnRecvPacket callback so we need to check if the acknowledgement is already // set on the store and return an error if so. if k.HasPacketAcknowledgement(ctx, packet.DestinationChannel, packet.Sequence) { - return errorsmod.Wrapf(channeltypes.ErrAcknowledgementExists, "acknowledgement for channel %s, sequence %d already exists", packet.DestinationChannel, packet.Sequence) + return errorsmod.Wrapf(types.ErrAcknowledgementExists, "acknowledgement for channel %s, sequence %d already exists", packet.DestinationChannel, packet.Sequence) } if _, found := k.GetPacketReceipt(ctx, packet.DestinationChannel, packet.Sequence); !found { - return errorsmod.Wrap(channeltypes.ErrInvalidPacket, "receipt not found for packet") + return errorsmod.Wrap(types.ErrInvalidPacket, "receipt not found for packet") } // TODO: Validate Acknowledgment more thoroughly here after Issue #7472: https://github.com/cosmos/ibc-go/issues/7472 @@ -218,7 +217,7 @@ func (k *Keeper) acknowledgePacket(ctx context.Context, packet types.Packet, ack } if channel.CounterpartyChannelId != packet.DestinationChannel { - return errorsmod.Wrapf(channeltypes.ErrInvalidChannelIdentifier, "counterparty channel id (%s) does not match packet destination channel id (%s)", channel.CounterpartyChannelId, packet.DestinationChannel) + return errorsmod.Wrapf(types.ErrInvalidChannelIdentifier, "counterparty channel id (%s) does not match packet destination channel id (%s)", channel.CounterpartyChannelId, packet.DestinationChannel) } clientID := channel.ClientId @@ -232,14 +231,14 @@ func (k *Keeper) acknowledgePacket(ctx context.Context, packet types.Packet, ack // or there is a misconfigured relayer attempting to prove an acknowledgement // for a packet never sent. Core IBC will treat this error as a no-op in order to // prevent an entire relay transaction from failing and consuming unnecessary fees. - return channeltypes.ErrNoOpMsg + return types.ErrNoOpMsg } packetCommitment := types.CommitPacket(packet) // verify we sent the packet and haven't cleared it out yet if !bytes.Equal(commitment, packetCommitment) { - return errorsmod.Wrapf(channeltypes.ErrInvalidPacket, "commitment bytes are not equal: got (%v), expected (%v)", packetCommitment, commitment) + return errorsmod.Wrapf(types.ErrInvalidPacket, "commitment bytes are not equal: got (%v), expected (%v)", packetCommitment, commitment) } path := hostv2.PacketAcknowledgementKey(packet.DestinationChannel, packet.Sequence) @@ -285,7 +284,7 @@ func (k *Keeper) timeoutPacket( } if channel.CounterpartyChannelId != packet.DestinationChannel { - return errorsmod.Wrapf(channeltypes.ErrInvalidChannelIdentifier, "counterparty channel id (%s) does not match packet destination channel id (%s)", channel.CounterpartyChannelId, packet.DestinationChannel) + return errorsmod.Wrapf(types.ErrInvalidChannelIdentifier, "counterparty channel id (%s) does not match packet destination channel id (%s)", channel.CounterpartyChannelId, packet.DestinationChannel) } clientID := channel.ClientId @@ -298,7 +297,7 @@ func (k *Keeper) timeoutPacket( timeoutTimestamp := types.TimeoutTimestampToNanos(packet.TimeoutTimestamp) if proofTimestamp < timeoutTimestamp { - return errorsmod.Wrapf(channeltypes.ErrTimeoutNotReached, "proof timestamp: %d, timeout timestamp: %d", proofTimestamp, timeoutTimestamp) + return errorsmod.Wrapf(types.ErrTimeoutNotReached, "proof timestamp: %d, timeout timestamp: %d", proofTimestamp, timeoutTimestamp) } // check that the commitment has not been cleared and that it matches the packet sent by relayer @@ -309,13 +308,13 @@ func (k *Keeper) timeoutPacket( // or there is a misconfigured relayer attempting to prove a timeout // for a packet never sent. Core IBC will treat this error as a no-op in order to // prevent an entire relay transaction from failing and consuming unnecessary fees. - return channeltypes.ErrNoOpMsg + return types.ErrNoOpMsg } packetCommitment := types.CommitPacket(packet) // verify we sent the packet and haven't cleared it out yet if !bytes.Equal(commitment, packetCommitment) { - return errorsmod.Wrapf(channeltypes.ErrInvalidPacket, "packet commitment bytes are not equal: got (%v), expected (%v)", commitment, packetCommitment) + return errorsmod.Wrapf(types.ErrInvalidPacket, "packet commitment bytes are not equal: got (%v), expected (%v)", commitment, packetCommitment) } // verify packet receipt absence diff --git a/modules/core/04-channel/v2/keeper/packet_test.go b/modules/core/04-channel/v2/keeper/packet_test.go index a01c5959f64..e93e2d97259 100644 --- a/modules/core/04-channel/v2/keeper/packet_test.go +++ b/modules/core/04-channel/v2/keeper/packet_test.go @@ -5,7 +5,6 @@ import ( "time" clienttypes "github.com/cosmos/ibc-go/v9/modules/core/02-client/types" - channeltypes "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" hostv2 "github.com/cosmos/ibc-go/v9/modules/core/24-host/v2" @@ -56,7 +55,7 @@ func (suite *KeeperTestSuite) TestSendPacket() { // invalid data packet.Payloads = nil }, - channeltypes.ErrInvalidPacket, + types.ErrInvalidPacket, }, { "client status invalid", @@ -84,7 +83,7 @@ func (suite *KeeperTestSuite) TestSendPacket() { "timeout elapsed", func() { packet.TimeoutTimestamp = 1 }, - channeltypes.ErrTimeoutElapsed, + types.ErrTimeoutElapsed, }, } @@ -168,21 +167,21 @@ func (suite *KeeperTestSuite) TestRecvPacket() { func() { packet.SourceChannel = unusedChannel }, - channeltypes.ErrInvalidChannelIdentifier, + types.ErrInvalidChannelIdentifier, }, { "failure: packet has timed out", func() { suite.coordinator.IncrementTimeBy(time.Hour * 20) }, - channeltypes.ErrTimeoutElapsed, + types.ErrTimeoutElapsed, }, { "failure: packet already received", func() { suite.chainB.App.GetIBCKeeper().ChannelKeeperV2.SetPacketReceipt(suite.chainB.GetContext(), packet.DestinationChannel, packet.Sequence) }, - channeltypes.ErrNoOpMsg, + types.ErrNoOpMsg, }, { "failure: verify membership failed", @@ -262,7 +261,7 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgement() { func() { packet.SourceChannel = unusedChannel }, - channeltypes.ErrInvalidChannelIdentifier, + types.ErrInvalidChannelIdentifier, }, { "failure: ack already exists", @@ -270,7 +269,7 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgement() { ackBz := types.CommitAcknowledgement(ack) suite.chainB.App.GetIBCKeeper().ChannelKeeperV2.SetPacketAcknowledgement(suite.chainB.GetContext(), packet.DestinationChannel, packet.Sequence, ackBz) }, - channeltypes.ErrAcknowledgementExists, + types.ErrAcknowledgementExists, }, { "failure: empty ack", @@ -286,7 +285,7 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgement() { func() { packet.Sequence = 2 }, - channeltypes.ErrInvalidPacket, + types.ErrInvalidPacket, }, } @@ -371,14 +370,14 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { func() { packet.DestinationChannel = unusedChannel }, - channeltypes.ErrInvalidChannelIdentifier, + types.ErrInvalidChannelIdentifier, }, { "failure: packet commitment doesn't exist.", func() { suite.chainA.App.GetIBCKeeper().ChannelKeeperV2.DeletePacketCommitment(suite.chainA.GetContext(), packet.SourceChannel, packet.Sequence) }, - channeltypes.ErrNoOpMsg, + types.ErrNoOpMsg, }, { "failure: client status invalid", @@ -393,7 +392,7 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { // change payload after send to acknowledge different packet packet.Payloads[0].Value = []byte("different value") }, - channeltypes.ErrInvalidPacket, + types.ErrInvalidPacket, }, { "failure: verify membership fails", @@ -494,7 +493,7 @@ func (suite *KeeperTestSuite) TestTimeoutPacket() { packet.DestinationChannel = unusedChannel }, - channeltypes.ErrInvalidChannelIdentifier, + types.ErrInvalidChannelIdentifier, }, { "failure: packet has not timed out yet", @@ -506,12 +505,12 @@ func (suite *KeeperTestSuite) TestTimeoutPacket() { packet.TimeoutTimestamp, packet.Payloads) suite.Require().NoError(err, "send packet failed") }, - channeltypes.ErrTimeoutNotReached, + types.ErrTimeoutNotReached, }, { "failure: packet already timed out", func() {}, // equivalent to not sending packet at all - channeltypes.ErrNoOpMsg, + types.ErrNoOpMsg, }, { "failure: packet does not match commitment", @@ -523,7 +522,7 @@ func (suite *KeeperTestSuite) TestTimeoutPacket() { // try to timeout packet with different data packet.Payloads[0].Value = []byte("different value") }, - channeltypes.ErrInvalidPacket, + types.ErrInvalidPacket, }, { "failure: client status invalid", diff --git a/modules/core/04-channel/v2/types/errors.go b/modules/core/04-channel/v2/types/errors.go index 7a1cabe9fc0..61d80d62a2d 100644 --- a/modules/core/04-channel/v2/types/errors.go +++ b/modules/core/04-channel/v2/types/errors.go @@ -13,4 +13,11 @@ var ( ErrInvalidAcknowledgement = errorsmod.Register(SubModuleName, 7, "invalid acknowledgement") ErrPacketCommitmentNotFound = errorsmod.Register(SubModuleName, 8, "packet commitment not found") ErrAcknowledgementNotFound = errorsmod.Register(SubModuleName, 9, "packet acknowledgement not found") + ErrTimeoutElapsed = errorsmod.Register(SubModuleName, 10, "timeout elapsed") + ErrInvalidChannelIdentifier = errorsmod.Register(SubModuleName, 11, "invalid channel identifier") + ErrAcknowledgementExists = errorsmod.Register(SubModuleName, 12, "acknowledgement for packet already exists") + ErrTimeoutNotReached = errorsmod.Register(SubModuleName, 13, "timeout not reached") + // Perform a no-op on the current Msg + ErrNoOpMsg = errorsmod.Register(SubModuleName, 14, "message is redundant, no-op will be performed") + ErrInvalidTimeout = errorsmod.Register(SubModuleName, 15, "invalid packet timeout") ) diff --git a/modules/core/04-channel/v2/types/msgs.go b/modules/core/04-channel/v2/types/msgs.go index 9c6f724e40d..d7547b4c07e 100644 --- a/modules/core/04-channel/v2/types/msgs.go +++ b/modules/core/04-channel/v2/types/msgs.go @@ -6,7 +6,6 @@ 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" host "github.com/cosmos/ibc-go/v9/modules/core/24-host" @@ -102,7 +101,7 @@ func (msg *MsgSendPacket) ValidateBasic() error { } if msg.TimeoutTimestamp == 0 { - return errorsmod.Wrap(channeltypesv1.ErrInvalidTimeout, "timeout must not be 0") + return errorsmod.Wrap(ErrInvalidTimeout, "timeout must not be 0") } if len(msg.Payloads) != 1 { diff --git a/modules/core/04-channel/v2/types/msgs_test.go b/modules/core/04-channel/v2/types/msgs_test.go index 44e92c4e5a3..fd6f5ceaa0a 100644 --- a/modules/core/04-channel/v2/types/msgs_test.go +++ b/modules/core/04-channel/v2/types/msgs_test.go @@ -7,7 +7,6 @@ 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" @@ -170,7 +169,7 @@ func (s *TypesTestSuite) TestMsgSendPacketValidateBasic() { malleate: func() { msg.TimeoutTimestamp = 0 }, - expError: channeltypesv1.ErrInvalidTimeout, + expError: types.ErrInvalidTimeout, }, { name: "failure: invalid length for payload",