Skip to content

Commit

Permalink
chore: remove usage of v1 errors from v2 (#7528)
Browse files Browse the repository at this point in the history
* chore: remove usage of v1 errors from v2

* linter
  • Loading branch information
bznein authored Nov 1, 2024
1 parent 28b6a89 commit 3c47bd7
Show file tree
Hide file tree
Showing 7 changed files with 48 additions and 44 deletions.
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 @@ -120,7 +120,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 @@ -195,7 +195,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 @@ -239,7 +239,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 @@ -9,7 +9,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 @@ -132,7 +131,7 @@ func (suite *KeeperTestSuite) TestMsgSendPacket() {
// ensure a message timeout.
timeoutTimestamp = uint64(1)
},
expError: channeltypesv1.ErrTimeoutElapsed,
expError: channeltypesv2.ErrTimeoutElapsed,
},
{
name: "failure: timeout timestamp exceeds max allowed input",
Expand Down Expand Up @@ -488,7 +487,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
13 changes: 11 additions & 2 deletions modules/core/04-channel/v2/types/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,15 @@ 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")
ErrMaxTimeoutDeltaExceeded = errorsmod.Register(SubModuleName, 10, "timeoutTimestamp exceeds max allowed value")
ErrTimeoutTooLow = errorsmod.Register(SubModuleName, 11, "timeoutTimestamp is less than current block timestamp")

ErrMaxTimeoutDeltaExceeded = errorsmod.Register(SubModuleName, 10, "timeoutTimestamp exceeds max allowed value")
ErrTimeoutTooLow = errorsmod.Register(SubModuleName, 11, "timeoutTimestamp is less than current block timestamp")
ErrTimeoutElapsed = errorsmod.Register(SubModuleName, 12, "timeout elapsed")

ErrInvalidChannelIdentifier = errorsmod.Register(SubModuleName, 13, "invalid channel identifier")
ErrAcknowledgementExists = errorsmod.Register(SubModuleName, 14, "acknowledgement for packet already exists")
ErrTimeoutNotReached = errorsmod.Register(SubModuleName, 15, "timeout not reached")
// Perform a no-op on the current Msg
ErrNoOpMsg = errorsmod.Register(SubModuleName, 16, "message is redundant, no-op will be performed")
ErrInvalidTimeout = errorsmod.Register(SubModuleName, 17, "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 @@ -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"
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 @@ -106,7 +105,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

0 comments on commit 3c47bd7

Please sign in to comment.