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

refactor: check timeouts directly in channel/v2 #7464

Merged
merged 13 commits into from
Oct 23, 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
6 changes: 3 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 @@ -84,7 +84,7 @@ func (suite *KeeperTestSuite) TestMsgSendPacket() {
path = ibctesting.NewPath(suite.chainA, suite.chainB)
path.SetupV2()

timeoutTimestamp = suite.chainA.GetTimeoutTimestamp()
timeoutTimestamp = suite.chainA.GetTimeoutTimestampSecs()
payload = mockv2.NewMockPayload(mockv2.ModuleNameA, mockv2.ModuleNameB)

expectedPacket = channeltypesv2.NewPacket(1, path.EndpointA.ChannelID, path.EndpointB.ChannelID, timeoutTimestamp, payload)
Expand Down Expand Up @@ -201,7 +201,7 @@ func (suite *KeeperTestSuite) TestMsgRecvPacket() {
path = ibctesting.NewPath(suite.chainA, suite.chainB)
path.SetupV2()

timeoutTimestamp := suite.chainA.GetTimeoutTimestamp()
timeoutTimestamp := suite.chainA.GetTimeoutTimestampSecs()

var err error
packet, err = path.EndpointA.MsgSendPacket(timeoutTimestamp, mockv2.NewMockPayload(mockv2.ModuleNameA, mockv2.ModuleNameB))
Expand Down Expand Up @@ -393,7 +393,7 @@ func (suite *KeeperTestSuite) TestMsgAcknowledgement() {
path = ibctesting.NewPath(suite.chainA, suite.chainB)
path.SetupV2()

timeoutTimestamp := suite.chainA.GetTimeoutTimestamp()
timeoutTimestamp := suite.chainA.GetTimeoutTimestampSecs()

var err error
// Send packet from A to B
Expand Down
39 changes: 16 additions & 23 deletions modules/core/04-channel/v2/keeper/packet.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"bytes"
"context"
"strconv"
"time"

errorsmod "cosmossdk.io/errors"

Expand Down Expand Up @@ -68,12 +67,12 @@ func (k *Keeper) sendPacket(
if err != nil {
return 0, "", err
}
// check if packet is timed out on the receiving chain
// convert packet timeout to nanoseconds for now to use existing helper function
// TODO: Remove this workaround with Issue #7414: https://github.com/cosmos/ibc-go/issues/7414
timeout := channeltypes.NewTimeoutWithTimestamp(uint64(time.Unix(int64(packet.GetTimeoutTimestamp()), 0).UnixNano()))
if timeout.TimestampElapsed(latestTimestamp) {
return 0, "", errorsmod.Wrap(timeout.ErrTimeoutElapsed(latestHeight, latestTimestamp), "invalid packet timeout")

// client timestamps are in nanoseconds while packet timeouts are in seconds
// 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)
}

commitment := types.CommitPacket(packet)
Expand Down Expand Up @@ -102,9 +101,6 @@ func (k *Keeper) recvPacket(
proof []byte,
proofHeight exported.Height,
) error {
// Lookup channel associated with destination channel ID and ensure
// that the packet was indeed sent by our counterparty by verifying
// packet sender is our channel's counterparty channel id.
channel, ok := k.GetChannel(ctx, packet.DestinationChannel)
if !ok {
// TODO: figure out how aliasing will work when more than one payload is sent.
Expand All @@ -113,19 +109,18 @@ func (k *Keeper) recvPacket(
return errorsmod.Wrap(types.ErrChannelNotFound, packet.DestinationChannel)
}
}

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)
}

clientID := channel.ClientId

// check if packet timed out by comparing it with the latest height of the chain
sdkCtx := sdk.UnwrapSDKContext(ctx)
selfHeight, selfTimestamp := clienttypes.GetSelfHeight(ctx), uint64(sdkCtx.BlockTime().UnixNano())
// convert packet timeout to nanoseconds for now to use existing helper function
// TODO: Remove this workaround with Issue #7414: https://github.com/cosmos/ibc-go/issues/7414
timeout := channeltypes.NewTimeoutWithTimestamp(uint64(time.Unix(int64(packet.GetTimeoutTimestamp()), 0).UnixNano()))
if timeout.Elapsed(selfHeight, selfTimestamp) {
return errorsmod.Wrap(timeout.ErrTimeoutElapsed(selfHeight, selfTimestamp), "packet timeout elapsed")
currentTimestamp := uint64(sdkCtx.BlockTime().Unix())
if currentTimestamp >= packet.TimeoutTimestamp {
return errorsmod.Wrapf(channeltypes.ErrTimeoutElapsed, "current timestamp: %d, timeout timestamp: %d", currentTimestamp, packet.TimeoutTimestamp)
}

// REPLAY PROTECTION: Packet receipts will indicate that a packet has already been received
Expand Down Expand Up @@ -284,15 +279,15 @@ func (k *Keeper) timeoutPacket(
proof []byte,
proofHeight exported.Height,
) error {
// Lookup counterparty associated with our channel and ensure
// that the packet was indeed sent by our counterparty.
channel, ok := k.GetChannel(ctx, packet.SourceChannel)
if !ok {
return errorsmod.Wrap(types.ErrChannelNotFound, packet.SourceChannel)
}

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)
}

clientID := channel.ClientId

// check that timeout height or timeout timestamp has passed on the other end
Expand All @@ -301,11 +296,9 @@ func (k *Keeper) timeoutPacket(
return err
}

// convert packet timeout to nanoseconds for now to use existing helper function
// TODO: Remove this workaround with Issue #7414: https://github.com/cosmos/ibc-go/issues/7414
timeout := channeltypes.NewTimeoutWithTimestamp(uint64(time.Unix(int64(packet.GetTimeoutTimestamp()), 0).UnixNano()))
if !timeout.Elapsed(clienttypes.ZeroHeight(), proofTimestamp) {
return errorsmod.Wrap(timeout.ErrTimeoutNotReached(proofHeight.(clienttypes.Height), proofTimestamp), "packet timeout not reached")
timeoutTimestamp := types.TimeoutTimestampToNanos(packet.TimeoutTimestamp)
if proofTimestamp < timeoutTimestamp {
return errorsmod.Wrapf(channeltypes.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 Down
6 changes: 6 additions & 0 deletions modules/core/04-channel/v2/types/packet.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package types

import (
"strings"
"time"

errorsmod "cosmossdk.io/errors"

Expand Down Expand Up @@ -78,3 +79,8 @@ func (p Payload) ValidateBasic() error {
}
return nil
}

// TimeoutTimestampToNanos takes seconds as a uint64 and returns Unix nanoseconds as uint64.
func TimeoutTimestampToNanos(seconds uint64) uint64 {
return uint64(time.Unix(int64(seconds), 0).UnixNano())
}
2 changes: 1 addition & 1 deletion modules/core/04-channel/v2/types/packet.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion proto/ibc/core/channel/v2/packet.proto
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ message Packet {
string source_channel = 2;
// identifies the receiving chain.
string destination_channel = 3;
// timeout timestamp after which the packet times out.
// timeout timestamp in seconds after which the packet times out.
uint64 timeout_timestamp = 4;
// a list of payloads, each one for a specific application.
repeated Payload payloads = 5 [(gogoproto.nullable) = false];
Expand Down
6 changes: 6 additions & 0 deletions testing/chain.go
Original file line number Diff line number Diff line change
Expand Up @@ -557,6 +557,12 @@ func (chain *TestChain) GetTimeoutTimestamp() uint64 {
return uint64(chain.GetContext().BlockTime().UnixNano()) + DefaultTimeoutTimestampDelta
}

// GetTimeoutTimestampSecs is a convenience function which returns a IBC packet timeout timestamp in seconds
// to be used for testing. It returns the current block timestamp + default timestamp delta (1 hour).
func (chain *TestChain) GetTimeoutTimestampSecs() uint64 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added but not used anywhere? Should we update the msg_server_test.go tests to use this instead?

return uint64(chain.GetContext().BlockTime().Unix()) + uint64(time.Hour.Seconds())
}

// DeleteKey deletes the specified key from the ibc store.
func (chain *TestChain) DeleteKey(key []byte) {
storeKey := chain.GetSimApp().GetKey(exported.StoreKey)
Expand Down
Loading