Skip to content

Commit

Permalink
chore: reduce number of timeout errors, cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
damiannolan committed Nov 4, 2024
1 parent 3c47bd7 commit 50972ad
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 46 deletions.
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")
)

0 comments on commit 50972ad

Please sign in to comment.