Skip to content

Commit

Permalink
refactor: use timeout type when possible (#5572)
Browse files Browse the repository at this point in the history
* refactor: use timeout type when possible

* review: consistent error type usage

* nit: error wording

---------

Co-authored-by: Damian Nolan <damiannolan@gmail.com>
  • Loading branch information
colin-axner and damiannolan authored Jan 11, 2024
1 parent e12ee53 commit de8eb6e
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 46 deletions.
39 changes: 9 additions & 30 deletions modules/core/04-channel/keeper/packet.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"bytes"
"slices"
"strconv"
"time"

errorsmod "cosmossdk.io/errors"

Expand Down Expand Up @@ -74,25 +73,16 @@ func (k Keeper) SendPacket(
return 0, errorsmod.Wrapf(clienttypes.ErrClientNotActive, "cannot send packet using client (%s) with status %s", connectionEnd.GetClientID(), status)
}

// check if packet is timed out on the receiving chain
latestHeight := clientState.GetLatestHeight()
if !timeoutHeight.IsZero() && latestHeight.GTE(timeoutHeight) {
return 0, errorsmod.Wrapf(
types.ErrPacketTimeout,
"receiving chain block height >= packet timeout height (%s >= %s)", latestHeight, timeoutHeight,
)
}

latestTimestamp, err := k.connectionKeeper.GetTimestampAtHeight(ctx, connectionEnd, latestHeight)
if err != nil {
return 0, err
}

if packet.GetTimeoutTimestamp() != 0 && latestTimestamp >= packet.GetTimeoutTimestamp() {
return 0, errorsmod.Wrapf(
types.ErrPacketTimeout,
"receiving chain block timestamp >= packet timeout timestamp (%s >= %s)", time.Unix(0, int64(latestTimestamp)).UTC(), time.Unix(0, int64(packet.GetTimeoutTimestamp())).UTC(),
)
// check if packet is timed out on the receiving chain
timeout := types.NewTimeout(packet.GetTimeoutHeight().(clienttypes.Height), packet.GetTimeoutTimestamp())
if timeout.Elapsed(latestHeight.(clienttypes.Height), latestTimestamp) {
return 0, errorsmod.Wrap(timeout.ErrTimeoutElapsed(latestHeight.(clienttypes.Height), latestTimestamp), "invalid packet timeout")
}

commitment := types.CommitPacket(k.cdc, packet)
Expand Down Expand Up @@ -189,22 +179,11 @@ func (k Keeper) RecvPacket(
)
}

// check if packet timeouted by comparing it with the latest height of the chain
selfHeight := clienttypes.GetSelfHeight(ctx)
timeoutHeight := packet.GetTimeoutHeight()
if !timeoutHeight.IsZero() && selfHeight.GTE(timeoutHeight) {
return errorsmod.Wrapf(
types.ErrPacketTimeout,
"block height >= packet timeout height (%s >= %s)", selfHeight, timeoutHeight,
)
}

// check if packet timeouted by comparing it with the latest timestamp of the chain
if packet.GetTimeoutTimestamp() != 0 && uint64(ctx.BlockTime().UnixNano()) >= packet.GetTimeoutTimestamp() {
return errorsmod.Wrapf(
types.ErrPacketTimeout,
"block timestamp >= packet timeout timestamp (%s >= %s)", ctx.BlockTime(), time.Unix(0, int64(packet.GetTimeoutTimestamp())).UTC(),
)
// check if packet timed out by comparing it with the latest height of the chain
selfHeight, selfTimestamp := clienttypes.GetSelfHeight(ctx), uint64(ctx.BlockTime().UnixNano())
timeout := types.NewTimeout(packet.GetTimeoutHeight().(clienttypes.Height), packet.GetTimeoutTimestamp())
if timeout.Elapsed(selfHeight, selfTimestamp) {
return errorsmod.Wrap(timeout.ErrTimeoutElapsed(selfHeight, selfTimestamp), "packet timeout elapsed")
}

commitment := types.CommitPacket(k.cdc, packet)
Expand Down
4 changes: 2 additions & 2 deletions modules/core/04-channel/keeper/packet_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,7 @@ func (suite *KeeperTestSuite) TestRecvPacket() {
packet = types.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, clienttypes.GetSelfHeight(suite.chainB.GetContext()), disabledTimeoutTimestamp)
channelCap = suite.chainB.GetChannelCapability(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
},
types.ErrPacketTimeout,
types.ErrTimeoutElapsed,
},
{
"timeout timestamp passed",
Expand All @@ -572,7 +572,7 @@ func (suite *KeeperTestSuite) TestRecvPacket() {
packet = types.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, disabledTimeoutHeight, uint64(suite.chainB.GetContext().BlockTime().UnixNano()))
channelCap = suite.chainB.GetChannelCapability(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
},
types.ErrPacketTimeout,
types.ErrTimeoutElapsed,
},
{
"next receive sequence is not found",
Expand Down
9 changes: 4 additions & 5 deletions modules/core/04-channel/keeper/timeout.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,9 @@ func (k Keeper) TimeoutPacket(
return err
}

timeoutHeight := packet.GetTimeoutHeight()
if (timeoutHeight.IsZero() || proofHeight.LT(timeoutHeight)) &&
(packet.GetTimeoutTimestamp() == 0 || proofTimestamp < packet.GetTimeoutTimestamp()) {
return errorsmod.Wrap(types.ErrPacketTimeout, "packet timeout has not been reached for height or timestamp")
timeout := types.NewTimeout(packet.GetTimeoutHeight().(clienttypes.Height), packet.GetTimeoutTimestamp())
if !timeout.Elapsed(proofHeight.(clienttypes.Height), proofTimestamp) {
return errorsmod.Wrap(timeout.ErrTimeoutNotReached(proofHeight.(clienttypes.Height), proofTimestamp), "packet timeout not reached")
}

commitment := k.GetPacketCommitment(ctx, packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence())
Expand Down Expand Up @@ -178,7 +177,7 @@ func (k Keeper) TimeoutExecuted(
// the upgrade is aborted and the channel is set to CLOSED.
if channel.State == types.FLUSHING {
// an error receipt is written to state and the channel is restored to OPEN
k.MustAbortUpgrade(ctx, packet.GetSourcePort(), packet.GetSourceChannel(), types.ErrPacketTimeout)
k.MustAbortUpgrade(ctx, packet.GetSourcePort(), packet.GetSourceChannel(), errorsmod.Wrap(types.ErrTimeoutElapsed, "packet timeout elapsed on ORDERED channel"))
}

channel.State = types.CLOSED
Expand Down
6 changes: 3 additions & 3 deletions modules/core/04-channel/keeper/timeout_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func (suite *KeeperTestSuite) TestTimeoutPacket() {
packet = types.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp)
}, false},
{"timeout", func() {
expError = types.ErrPacketTimeout
expError = types.ErrTimeoutNotReached
suite.coordinator.Setup(path)
sequence, err := path.EndpointA.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData)
suite.Require().NoError(err)
Expand Down Expand Up @@ -192,9 +192,9 @@ func (suite *KeeperTestSuite) TestTimeoutPacket() {
}, false},
}

for i, tc := range testCases {
for _, tc := range testCases {
tc := tc
suite.Run(fmt.Sprintf("Case %s, %d/%d tests", tc.msg, i, len(testCases)), func() {
suite.Run(tc.msg, func() {
var (
proof []byte
proofHeight exported.Height
Expand Down
7 changes: 2 additions & 5 deletions modules/core/04-channel/keeper/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -722,11 +722,8 @@ func (k Keeper) ChanUpgradeTimeout(

// proof must be from a height after timeout has elapsed. Either timeoutHeight or timeoutTimestamp must be defined.
// if timeoutHeight is defined and proof is from before timeout height, abort transaction
timeoutHeight := upgrade.Timeout.Height
timeoutTimeStamp := upgrade.Timeout.Timestamp
if (timeoutHeight.IsZero() || proofHeight.LT(timeoutHeight)) &&
(timeoutTimeStamp == 0 || proofTimestamp < timeoutTimeStamp) {
return errorsmod.Wrap(types.ErrInvalidUpgradeTimeout, "upgrade timeout has not been reached for height or timestamp")
if !upgrade.Timeout.Elapsed(proofHeight.(clienttypes.Height), proofTimestamp) {
return errorsmod.Wrap(upgrade.Timeout.ErrTimeoutNotReached(proofHeight.(clienttypes.Height), proofTimestamp), "upgrade timeout not reached")
}

// counterparty channel must be proved to still be in OPEN state or FLUSHING state.
Expand Down
2 changes: 1 addition & 1 deletion modules/core/04-channel/keeper/upgrade_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2032,7 +2032,7 @@ func (suite *KeeperTestSuite) TestChanUpgradeTimeout() {
channelKey := host.ChannelKey(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
proofChannel, proofHeight = path.EndpointB.QueryProof(channelKey)
},
types.ErrInvalidUpgradeTimeout,
types.ErrTimeoutNotReached,
},
{
"counterparty channel state is not OPEN or FLUSHING (crossing hellos)",
Expand Down

0 comments on commit de8eb6e

Please sign in to comment.