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: reduce number of timeout errors, cleanup #7529

Merged
merged 1 commit into from
Nov 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
70 changes: 35 additions & 35 deletions modules/core/04-channel/v2/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,33 +10,33 @@ import (
sdk "github.com/cosmos/cosmos-sdk/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"
"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"
"github.com/cosmos/ibc-go/v9/modules/core/internal/v2/telemetry"
)

var _ channeltypesv2.MsgServer = &Keeper{}
var _ types.MsgServer = &Keeper{}

// CreateChannel defines a rpc handler method for MsgCreateChannel.
func (k *Keeper) CreateChannel(goCtx context.Context, msg *channeltypesv2.MsgCreateChannel) (*channeltypesv2.MsgCreateChannelResponse, error) {
func (k *Keeper) CreateChannel(goCtx context.Context, msg *types.MsgCreateChannel) (*types.MsgCreateChannelResponse, error) {
ctx := sdk.UnwrapSDKContext(goCtx)

channelID := k.channelKeeperV1.GenerateChannelIdentifier(ctx)

// Initialize channel with empty counterparty channel identifier.
channel := channeltypesv2.NewChannel(msg.ClientId, "", msg.MerklePathPrefix)
channel := types.NewChannel(msg.ClientId, "", msg.MerklePathPrefix)
k.SetChannel(ctx, channelID, channel)
k.SetCreator(ctx, channelID, msg.Signer)
k.SetNextSequenceSend(ctx, channelID, 1)

k.EmitCreateChannelEvent(goCtx, channelID)

return &channeltypesv2.MsgCreateChannelResponse{ChannelId: channelID}, nil
return &types.MsgCreateChannelResponse{ChannelId: channelID}, nil
}

// RegisterCounterparty defines a rpc handler method for MsgRegisterCounterparty.
func (k *Keeper) RegisterCounterparty(goCtx context.Context, msg *channeltypesv2.MsgRegisterCounterparty) (*channeltypesv2.MsgRegisterCounterpartyResponse, error) {
func (k *Keeper) RegisterCounterparty(goCtx context.Context, msg *types.MsgRegisterCounterparty) (*types.MsgRegisterCounterpartyResponse, error) {
ctx := sdk.UnwrapSDKContext(goCtx)

creator, found := k.GetCreator(ctx, msg.ChannelId)
Expand All @@ -50,19 +50,19 @@ func (k *Keeper) RegisterCounterparty(goCtx context.Context, msg *channeltypesv2

channel, ok := k.GetChannel(ctx, msg.ChannelId)
if !ok {
return nil, errorsmod.Wrapf(channeltypesv2.ErrInvalidChannel, "channel must exist for channel id %s", msg.ChannelId)
return nil, errorsmod.Wrapf(types.ErrInvalidChannel, "channel must exist for channel id %s", msg.ChannelId)
}

channel.CounterpartyChannelId = msg.CounterpartyChannelId
k.SetChannel(ctx, msg.ChannelId, channel)
// Delete client creator from state as it is not needed after this point.
k.DeleteCreator(ctx, msg.ChannelId)

return &channeltypesv2.MsgRegisterCounterpartyResponse{}, nil
return &types.MsgRegisterCounterpartyResponse{}, nil
}

// SendPacket defines a rpc handler method for MsgSendPacket.
func (k *Keeper) SendPacket(ctx context.Context, msg *channeltypesv2.MsgSendPacket) (*channeltypesv2.MsgSendPacketResponse, error) {
func (k *Keeper) SendPacket(ctx context.Context, msg *types.MsgSendPacket) (*types.MsgSendPacketResponse, error) {
sdkCtx := sdk.UnwrapSDKContext(ctx)

sequence, destChannel, err := k.sendPacket(ctx, msg.SourceChannel, msg.TimeoutTimestamp, msg.Payloads)
Expand All @@ -75,12 +75,12 @@ func (k *Keeper) SendPacket(ctx context.Context, msg *channeltypesv2.MsgSendPack
// timeoutTimestamp must be greater than current block time
timeout := time.Unix(int64(msg.TimeoutTimestamp), 0)
if timeout.Before(sdkCtx.BlockTime()) {
return nil, errorsmod.Wrap(channeltypesv2.ErrTimeoutTooLow, "timeout is less than the current block timestamp")
return nil, errorsmod.Wrap(types.ErrTimeoutElapsed, "timeout is less than the current block timestamp")
}

// timeoutTimestamp must be less than current block time + MaxTimeoutDelta
if timeout.After(sdkCtx.BlockTime().Add(channeltypesv2.MaxTimeoutDelta)) {
return nil, errorsmod.Wrap(channeltypesv2.ErrMaxTimeoutDeltaExceeded, "timeout exceeds the maximum expected value")
if timeout.After(sdkCtx.BlockTime().Add(types.MaxTimeoutDelta)) {
return nil, errorsmod.Wrap(types.ErrInvalidTimeout, "timeout exceeds the maximum expected value")
}

signer, err := sdk.AccAddressFromBech32(msg.Signer)
Expand All @@ -97,11 +97,11 @@ func (k *Keeper) SendPacket(ctx context.Context, msg *channeltypesv2.MsgSendPack
}
}

return &channeltypesv2.MsgSendPacketResponse{Sequence: sequence}, nil
return &types.MsgSendPacketResponse{Sequence: sequence}, nil
}

// RecvPacket defines a rpc handler method for MsgRecvPacket.
func (k *Keeper) RecvPacket(ctx context.Context, msg *channeltypesv2.MsgRecvPacket) (*channeltypesv2.MsgRecvPacketResponse, error) {
func (k *Keeper) RecvPacket(ctx context.Context, msg *types.MsgRecvPacket) (*types.MsgRecvPacketResponse, error) {
sdkCtx := sdk.UnwrapSDKContext(ctx)

signer, err := sdk.AccAddressFromBech32(msg.Signer)
Expand All @@ -120,18 +120,18 @@ func (k *Keeper) RecvPacket(ctx context.Context, msg *channeltypesv2.MsgRecvPack
switch err {
case nil:
writeFn()
case channeltypesv2.ErrNoOpMsg:
case types.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
return &types.MsgRecvPacketResponse{Result: channeltypesv1.NOOP}, nil
default:
sdkCtx.Logger().Error("receive packet failed", "source-channel", msg.Packet.SourceChannel, "error", errorsmod.Wrap(err, "receive packet verification failed"))
return nil, errorsmod.Wrap(err, "receive packet verification failed")
}

// build up the recv results for each application callback.
ack := channeltypesv2.Acknowledgement{
AcknowledgementResults: []channeltypesv2.AcknowledgementResult{},
ack := types.Acknowledgement{
AcknowledgementResults: []types.AcknowledgementResult{},
}

for _, pd := range msg.Packet.Payloads {
Expand All @@ -140,15 +140,15 @@ func (k *Keeper) RecvPacket(ctx context.Context, msg *channeltypesv2.MsgRecvPack
cb := k.Router.Route(pd.DestinationPort)
res := cb.OnRecvPacket(cacheCtx, msg.Packet.SourceChannel, msg.Packet.DestinationChannel, pd, signer)

if res.Status != channeltypesv2.PacketStatus_Failure {
if res.Status != types.PacketStatus_Failure {
// write application state changes for asynchronous and successful acknowledgements
writeFn()
} else {
// Modify events in cached context to reflect unsuccessful acknowledgement
sdkCtx.EventManager().EmitEvents(internalerrors.ConvertToErrorEvents(cacheCtx.EventManager().Events()))
}

ack.AcknowledgementResults = append(ack.AcknowledgementResults, channeltypesv2.AcknowledgementResult{
ack.AcknowledgementResults = append(ack.AcknowledgementResults, types.AcknowledgementResult{
AppName: pd.DestinationPort,
RecvPacketResult: res,
})
Expand All @@ -157,12 +157,12 @@ func (k *Keeper) RecvPacket(ctx context.Context, msg *channeltypesv2.MsgRecvPack
// note this should never happen as the payload would have had to be empty.
if len(ack.AcknowledgementResults) == 0 {
sdkCtx.Logger().Error("receive packet failed", "source-channel", msg.Packet.SourceChannel, "error", errorsmod.Wrap(err, "invalid acknowledgement results"))
return &channeltypesv2.MsgRecvPacketResponse{Result: channeltypesv1.FAILURE}, errorsmod.Wrapf(err, "receive packet failed source-channel %s invalid acknowledgement results", msg.Packet.SourceChannel)
return &types.MsgRecvPacketResponse{Result: channeltypesv1.FAILURE}, errorsmod.Wrapf(err, "receive packet failed source-channel %s invalid acknowledgement results", msg.Packet.SourceChannel)
}

// NOTE: TBD how we will handle async acknowledgements with more than one payload.
isAsync := slices.ContainsFunc(ack.AcknowledgementResults, func(ackResult channeltypesv2.AcknowledgementResult) bool {
return ackResult.RecvPacketResult.Status == channeltypesv2.PacketStatus_Async
isAsync := slices.ContainsFunc(ack.AcknowledgementResults, func(ackResult types.AcknowledgementResult) bool {
return ackResult.RecvPacketResult.Status == types.PacketStatus_Async
})

if !isAsync {
Expand All @@ -174,14 +174,14 @@ func (k *Keeper) RecvPacket(ctx context.Context, msg *channeltypesv2.MsgRecvPack
}
}

defer telemetryv2.ReportRecvPacket(msg.Packet)
defer telemetry.ReportRecvPacket(msg.Packet)

sdkCtx.Logger().Info("receive packet callback succeeded", "source-channel", msg.Packet.SourceChannel, "dest-channel", msg.Packet.DestinationChannel, "result", channeltypesv1.SUCCESS.String())
return &channeltypesv2.MsgRecvPacketResponse{Result: channeltypesv1.SUCCESS}, nil
return &types.MsgRecvPacketResponse{Result: channeltypesv1.SUCCESS}, nil
}

// Acknowledgement defines an rpc handler method for MsgAcknowledgement.
func (k *Keeper) Acknowledgement(ctx context.Context, msg *channeltypesv2.MsgAcknowledgement) (*channeltypesv2.MsgAcknowledgementResponse, error) {
func (k *Keeper) Acknowledgement(ctx context.Context, msg *types.MsgAcknowledgement) (*types.MsgAcknowledgementResponse, error) {
sdkCtx := sdk.UnwrapSDKContext(ctx)
relayer, err := sdk.AccAddressFromBech32(msg.Signer)
if err != nil {
Expand All @@ -195,16 +195,16 @@ func (k *Keeper) Acknowledgement(ctx context.Context, msg *channeltypesv2.MsgAck
switch err {
case nil:
writeFn()
case channeltypesv2.ErrNoOpMsg:
case types.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
return &types.MsgAcknowledgementResponse{Result: channeltypesv1.NOOP}, nil
default:
sdkCtx.Logger().Error("acknowledgement failed", "source-channel", msg.Packet.SourceChannel, "error", errorsmod.Wrap(err, "acknowledge packet verification failed"))
return nil, errorsmod.Wrap(err, "acknowledge packet verification failed")
}

recvResults := make(map[string]channeltypesv2.RecvPacketResult)
recvResults := make(map[string]types.RecvPacketResult)
for _, r := range msg.Acknowledgement.AcknowledgementResults {
recvResults[r.AppName] = r.RecvPacketResult
}
Expand All @@ -217,11 +217,11 @@ func (k *Keeper) Acknowledgement(ctx context.Context, msg *channeltypesv2.MsgAck
}
}

return &channeltypesv2.MsgAcknowledgementResponse{Result: channeltypesv1.SUCCESS}, nil
return &types.MsgAcknowledgementResponse{Result: channeltypesv1.SUCCESS}, nil
}

// Timeout defines a rpc handler method for MsgTimeout.
func (k *Keeper) Timeout(ctx context.Context, timeout *channeltypesv2.MsgTimeout) (*channeltypesv2.MsgTimeoutResponse, error) {
func (k *Keeper) Timeout(ctx context.Context, timeout *types.MsgTimeout) (*types.MsgTimeoutResponse, error) {
sdkCtx := sdk.UnwrapSDKContext(ctx)

signer, err := sdk.AccAddressFromBech32(timeout.Signer)
Expand All @@ -239,10 +239,10 @@ func (k *Keeper) Timeout(ctx context.Context, timeout *channeltypesv2.MsgTimeout
switch err {
case nil:
writeFn()
case channeltypesv2.ErrNoOpMsg:
case types.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
return &types.MsgTimeoutResponse{Result: channeltypesv1.NOOP}, nil
default:
sdkCtx.Logger().Error("timeout failed", "source-channel", timeout.Packet.SourceChannel, "error", errorsmod.Wrap(err, "timeout packet verification failed"))
return nil, errorsmod.Wrap(err, "timeout packet verification failed")
Expand All @@ -256,5 +256,5 @@ func (k *Keeper) Timeout(ctx context.Context, timeout *channeltypesv2.MsgTimeout
}
}

return &channeltypesv2.MsgTimeoutResponse{Result: channeltypesv1.SUCCESS}, nil
return &types.MsgTimeoutResponse{Result: channeltypesv1.SUCCESS}, nil
}
4 changes: 2 additions & 2 deletions modules/core/04-channel/v2/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,15 +139,15 @@ func (suite *KeeperTestSuite) TestMsgSendPacket() {
// ensure message timeout exceeds max allowed input.
timeoutTimestamp = uint64(suite.chainA.GetContext().BlockTime().Add(channeltypesv2.MaxTimeoutDelta + 10*time.Second).Unix())
},
expError: channeltypesv2.ErrMaxTimeoutDeltaExceeded,
expError: channeltypesv2.ErrInvalidTimeout,
},
{
name: "failure: timeout timestamp less than current block timestamp",
malleate: func() {
// ensure message timeout exceeds max allowed input.
timeoutTimestamp = uint64(suite.chainA.GetContext().BlockTime().Unix()) - 1
},
expError: channeltypesv2.ErrTimeoutTooLow,
expError: channeltypesv2.ErrTimeoutElapsed,
},
{
name: "failure: inactive client",
Expand Down
13 changes: 4 additions & 9 deletions modules/core/04-channel/v2/types/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,10 @@ 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")
ErrTimeoutElapsed = errorsmod.Register(SubModuleName, 12, "timeout elapsed")

ErrInvalidTimeout = errorsmod.Register(SubModuleName, 10, "invalid packet timeout")
ErrTimeoutElapsed = errorsmod.Register(SubModuleName, 11, "timeout elapsed")
ErrTimeoutNotReached = errorsmod.Register(SubModuleName, 12, "timeout not reached")
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")
ErrNoOpMsg = errorsmod.Register(SubModuleName, 15, "message is redundant, no-op will be performed")
)
Loading