Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: remove usage of v1 errors from v2 #7528

Merged
merged 3 commits into from
Nov 1, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
chore: remove usage of v1 errors from v2
  • Loading branch information
bznein committed Oct 31, 2024
commit 0709f843151ad3bf9f0819eaae276ee5cb17695e
6 changes: 3 additions & 3 deletions modules/core/04-channel/v2/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
5 changes: 2 additions & 3 deletions modules/core/04-channel/v2/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -462,7 +461,7 @@ func (suite *KeeperTestSuite) TestMsgTimeout() {
return mock.MockApplicationCallbackError
}
},
expError: channeltypesv1.ErrNoOpMsg,
expError: channeltypesv2.ErrNoOpMsg,
},
{
name: "failure: callback fails",
Expand Down
31 changes: 15 additions & 16 deletions modules/core/04-channel/v2/keeper/packet.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
31 changes: 15 additions & 16 deletions modules/core/04-channel/v2/keeper/packet_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -56,7 +55,7 @@ func (suite *KeeperTestSuite) TestSendPacket() {
// invalid data
packet.Payloads = nil
},
channeltypes.ErrInvalidPacket,
types.ErrInvalidPacket,
},
{
"client status invalid",
Expand Down Expand Up @@ -84,7 +83,7 @@ func (suite *KeeperTestSuite) TestSendPacket() {
"timeout elapsed", func() {
packet.TimeoutTimestamp = 1
},
channeltypes.ErrTimeoutElapsed,
types.ErrTimeoutElapsed,
},
}

Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -262,15 +261,15 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgement() {
func() {
packet.SourceChannel = unusedChannel
},
channeltypes.ErrInvalidChannelIdentifier,
types.ErrInvalidChannelIdentifier,
},
{
"failure: ack already exists",
func() {
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",
Expand All @@ -286,7 +285,7 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgement() {
func() {
packet.Sequence = 2
},
channeltypes.ErrInvalidPacket,
types.ErrInvalidPacket,
},
}

Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -494,7 +493,7 @@ func (suite *KeeperTestSuite) TestTimeoutPacket() {

packet.DestinationChannel = unusedChannel
},
channeltypes.ErrInvalidChannelIdentifier,
types.ErrInvalidChannelIdentifier,
},
{
"failure: packet has not timed out yet",
Expand All @@ -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",
Expand All @@ -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",
Expand Down
7 changes: 7 additions & 0 deletions modules/core/04-channel/v2/types/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
)
3 changes: 1 addition & 2 deletions modules/core/04-channel/v2/types/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down
3 changes: 1 addition & 2 deletions modules/core/04-channel/v2/types/msgs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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",
Expand Down
Loading